Skip to content

fix: never persist apiKey in shared document storage#102

Open
bobrnor wants to merge 1 commit into
tolgee:mainfrom
bobrnor:fix/api-key-leaks-to-document
Open

fix: never persist apiKey in shared document storage#102
bobrnor wants to merge 1 commit into
tolgee:mainfrom
bobrnor:fix/api-key-leaks-to-document

Conversation

@bobrnor

@bobrnor bobrnor commented Jun 12, 2026

Copy link
Copy Markdown

Problem

The plugin persists the Tolgee API key into the shared Figma document (figma.root pluginData), not just into per-user figma.clientStorage.

setPluginData writes the key to both stores, and getPluginData merges document settings over clientStorage:

return {
  ...(await getGlobalSettings()), // per-user clientStorage
  ...getDocumentData(),           // shared document — overrides the above
  ...getPageData(),
};

figma.root pluginData is stored in the .fig file, syncs to every collaborator and travels with file copies/exports. The practical effect for a multi-user setup:

  • The last person to enter a key writes it into the document, and it becomes the effective key for everyone opening that file — their own per-user key is overridden.
  • Changing the key changes it for all collaborators.
  • The key is readable by anyone with access to the file (and ends up in exported .fig files in plaintext).

This is the root cause behind the symptoms in #50 ("API key is getting forgotten" — the document copy clobbers the per-user one), and it contributes to the oversized document pluginData flagged by Figma in #56.

Fix

Keep the API key in per-user clientStorage only — the secret never touches the shared document:

  • types: CurrentDocumentSettings now Omits apiKey; TolgeeConfig Picks it back from GlobalSettings so the UI is unchanged.
  • setDocumentData: strips apiKey before writing (defense in depth).
  • getDocumentData: strips apiKey on read, so a key already leaked into an existing document can no longer override the per-user key or be surfaced to other collaborators (self-heal).
  • routing: because the key is now per-user, a collaborator can open an already-configured file (documentInfo is set in the document) while having no key of their own. Router previously gated the setup screen on documentInfo alone, dropping such a user on a non-working Index. It now forces the Settings view when apiKey is missing.

apiUrl / namespace / branch intentionally stay document-scoped — they are connection/mapping info, not secrets, and sharing them keeps file setup low-friction.

Notes

  • Existing files: the self-heal removes the leaked key from what the plugin reads and rewrites, but a key already written into a distributed/exported .fig stays at rest in that file until a settings re-save. Any key that was ever stored in a document should be rotated. (An active one-time scrub on init could be a follow-up.)
  • Follow-up (not in this PR): apiUrl / ignorePrefix / ignoreNumbers are currently written to both stores with the document winning, leaving stale clientStorage copies — worth consolidating to one home per field (relates to Figma plugin consuming too much pluginData #56). Happy to open a separate PR.
  • Tests: the repo has no unit tests around settingsTools (jest is a devDependency but unconfigured; Cypress covers the web harness, which doesn't reach these functions). The key invariants worth guarding if you'd like a test setup added: (1) setDocumentData never writes apiKey; (2) getDocumentData strips a leaked apiKey; (3) getPluginData still returns the apiKey from clientStorage. Glad to add these if you point me at a preferred test approach.

Verified locally: npm run build (typecheck + minify) and eslint clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed a security issue where API keys could be shared with collaborators on shared documents. API keys are now stored securely as per-user settings and never written to shared document data.

The plugin wrote the Tolgee apiKey into figma.root pluginData in addition
to figma.clientStorage. Document pluginData is stored in the .fig file in
plaintext, syncs to every collaborator and travels with file copies and
exports, so the last-entered key leaked to all users and overrode their
own key (getPluginData merges document settings over clientStorage).

Keep the apiKey in per-user clientStorage only:
- types: CurrentDocumentSettings now omits apiKey; TolgeeConfig picks it
  from GlobalSettings so the UI still works with it.
- setDocumentData strips apiKey defensively before writing.
- getDocumentData strips apiKey on read to self-heal documents that
  already contain a leaked key.

Because the key is now per-user, a collaborator can open an already
configured file (documentInfo is set in the document) while having no key
of their own. Router previously gated the setup screen on documentInfo
alone, so such a collaborator landed on a non-working Index. Force the
Settings view when the apiKey is missing so they are prompted to enter one.

apiUrl/namespace/branch intentionally stay document-scoped (connection
info, not secrets). Relates to tolgee#50 (key "forgotten" = document override)
and reduces document pluginData per Figma's request in tolgee#56.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 18f0493e-1180-4927-9e92-15245c205170

📥 Commits

Reviewing files that changed from the base of the PR and between f1b742d and 981ca9e.

📒 Files selected for processing (3)
  • src/main/utils/settingsTools.ts
  • src/types.ts
  • src/ui/views/Router.tsx

Walkthrough

This PR hardens credential handling by preventing the apiKey from being shared in Figma documents. Type definitions now exclude apiKey from document-scoped settings, the persistence layer defensively strips it before writing to shared storage, and routing logic checks per-user credentials independently.

Changes

API Key Non-Shareability

Layer / File(s) Summary
Type contract and config redesign
src/types.ts
CurrentDocumentSettings omits apiKey via Omit<GlobalSettings, "apiKey">. TolgeeConfig explicitly picks apiKey from GlobalSettings while using the updated CurrentDocumentSettings, establishing the contract that secrets do not flow into shared document settings.
Document persistence hardening
src/main/utils/settingsTools.ts
getDocumentData self-heals by removing any apiKey before returning typed settings. setDocumentData defensively strips apiKey from input before writing to shared root plugin data. setPluginData persists apiKey only to clientStorage and excludes it from document data payloads.
Per-user auth routing
src/ui/views/Router.tsx
Router derives apiKey from per-user config state and updates forceSettings logic to require both document presence and a non-empty user-level apiKey, ensuring users without credentials see the Settings view even when a collaborator has opened the document.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • dkrizan

Poem

🔐 A secret kept safe, not shared in the cloud,
When documents open, this rabbit's quite proud,
Each user holds tight to their own API key,
No snooping collaborators—just how it should be! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title directly and specifically describes the main security fix: preventing API key persistence in shared document storage, which is the central objective of all three file changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed due to a network error.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant