fix(plugins): let language packs translate plugin chrome

[smwbev]

Summary

protectedTranslation refuses any language-pack key under auto.components.settings. whose module starts with plugin. The breadth is deliberate and the comment says why — a consent surface added tomorrow is covered the day it lands, the lesson from PluginConsentProvenance. That reasoning is sound and this PR keeps it.

The side effect is that the prefix also catches copy that carries no security meaning. 214 keys sit behind the rule across 16 modules. The existing test names 8 of those modules as security copy:

it.each([
  'PluginConsentDialog',
  // The provenance badge and install-error copy carry the trust decision.
  'PluginConsentProvenance',
  'pluginError',
  'PluginKeybindingConsentPreview',
  'PluginMarketplaceListingRow',
  'PluginMarketplacePreviewDialog',
  'PluginMarketplaceSourceDialog',
  'PluginVmRecipeConsentPreview'
])('prevents language packs from rewriting %s security copy', …)

Those 8 modules hold 110 keys. The other 104 are protected as a side effect of the prefix, not because anyone classified them as a trust surface — section titles, empty states, Refresh, Add path.

The boundary is visibly accidental in one place: PluginMarketplaceBrowser.tsx is protected and renders PluginCatalogLayout from components/plugin-catalog/, which is not. One pane, nested components, and translatability decided by which folder the file lives in. In a localized build the inner list translates while the frame around it stays English.

What changed

A narrow allowlist of 35 exact paths, in plugin-translatable-chrome.ts. Exact paths rather than a pattern on purpose: anything new stays protected until someone deliberately adds it, so the fail-safe survives.

A protected container stays walkable only when exempt chrome sits below it — every leaf is still matched against its own full path, so PluginsSettingsSection.title resolves while PluginsSettingsSection.description right next to it does not.

Module	Exempt	Kept protected
PluginsSettingsSection	9 — title, Experimental, Plugin system, list states	description, systemDescription, featureOff, every *Failed
PluginMarketplaceBrowser	14 — refresh, search, empty states, Manage sources	noSources (names the official/community tiers), every *Failed
PluginDevelopmentSection	6 — field labels, validation, Development	help, saveFailed
plugins.search	6 — settings-search index entries	—

What deliberately stays protected

A path qualifies only if rewriting it cannot mislead the person deciding whether to trust a plugin. Reading every one of the 104 keys, these do not qualify:

• Promises about what plugins may do. PluginsSettingsSection.systemDescription says "Nothing runs until you review and enable it"; .description says "Plugins run on this computer"; .featureOff says installed plugins "stay disabled while the system is off"; PluginDevelopmentSection.help says "Dev plugins still require permission review". Swapping any of these for reassuring text is the attack, so they stay behind the rule even though they read like plain descriptions.
• Failure copy that reports a trust event. PluginMarketplaceBrowser.installFailed is "The reviewed source may have changed". Rather than judge each message, the whole *Failed family stays protected — a test enforces that, so the boundary is a rule and not a case-by-case call.
• Safety status and badges — PluginSettingsRow.blocked, PluginMarketplaceListingRow.official, .blocked.
• Destructive confirmations — PluginRemoveDialog, PluginRollbackDialog.
• Everything the existing security tests already name, unchanged.

Two follow-ups I did not take unilaterally: PluginMarketplaceListingRow.installed and .checkUpdate are procedural, but they live beside official and blocked, so splitting that module is your call. Same for PluginInstallDialog, which mixes a button label with consent copy.

Screenshots

No visual change in English — no English string is added, removed, or reworded.

Settings → Plugins in a build with a Russian language pack, today. The inner list (pluginCatalog) is translated; the frame around it (settings.Plugin*) is not:

With this change the frame resolves too, while every string listed above stays English by design.

Testing

• pnpm lint
• pnpm typecheck
• pnpm test
• pnpm build
• Added or updated high-quality tests that would catch regressions

New coverage in plugin-language-pack-artifact.test.ts: chrome keys parse, and the trust promises above are still refused — including a pack that rewrites systemDescription to "Every plugin here has been reviewed by Orca." and one that rewrites installFailed to "Installed successfully."

plugin-chrome-allowlist.test.ts guards the list itself against rot in both directions: every exempt path must still exist in en.json, none may belong to a module the security tests pin, and none may be *Failed.

Verified against a real pack as well — the 11,742-string smwbev.russian catalog parses today, parses with the chrome translated, and is still rejected when it reaches for a trust promise.

AI Review Report

Reviewed for security-boundary correctness first, then for the walk semantics.

• Flagged and fixed during development: the first attempt allowlisted leaf paths only, and every chrome key was still refused. The walk rejects a protected path the moment it sees it, so PluginsSettingsSection was rejected as a container before any leaf was reached. Fixed by letting a protected container stay walkable only when exempt chrome sits below it; leaves are unchanged and still matched on their full path. Without this the feature silently did nothing.
• Boundary review: read all 104 side-effect keys individually rather than exempting whole modules. That is what moved systemDescription, description, featureOff, help, and noSources out of the list — they read as chrome but assert something about plugin behavior or trust tiers.
• Cross-platform: pure string-set logic in shared code, no paths, no shell, no platform APIs. PluginDevelopmentSection.placeholder contains both POSIX and Windows example paths and is exempt as a label; it is illustrative text, not a path the code consumes.
• Verified: the pre-existing protection tests pass unchanged, so no previously refused catalog is now accepted.

Security Audit

• Threat model: a language pack is plugin-supplied content, so the question for each exempt path is whether rewriting it can change a trust decision. Trust badges, safety status, behavior promises, trust-event failure copy, and destructive confirmations are all out of scope of the exemption, by list and by test.
• Default posture unchanged: the prefix rule still matches first. A key added to any plugin module tomorrow is protected until someone edits the allowlist, and that edit is a reviewable diff naming the exact path.
• Walk safety: the container exemption does not weaken leaf checks — it only permits descent. A protected leaf inside an exempted container is still refused, covered by the systemDescription cases.
• Input handling / IPC / dependencies: untouched. No new parsing paths, no new dependencies, catalog limits (entries, depth, 8192 chars, unsafe keys) unchanged.

Notes

The 35 paths are the conservative floor, not a claim that the remaining 69 side-effect keys must stay protected forever. If you would rather split PluginMarketplaceListingRow and PluginInstallDialog so their procedural labels move too, I am happy to extend this in the same shape.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e50486e6-4880-49d8-9916-74011de28753

📥 Commits

Reviewing files that changed from the base of the PR and between 5780ee1 and c4f7122.

📒 Files selected for processing (2)

• src/renderer/src/i18n/plugin-chrome-allowlist.test.ts
• src/shared/plugins/plugin-language-pack-artifact.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• src/renderer/src/i18n/plugin-chrome-allowlist.test.ts
• src/shared/plugins/plugin-language-pack-artifact.test.ts

---

📝 Walkthrough

Walkthrough

The change adds an explicit allowlist for translatable plugin Chrome paths. The language-pack parser uses the allowlist for exact paths and protected containers. Tests verify catalog coverage, exclusion of security-related paths, rejection of failure messages, and preservation of protected security claims.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: allowing language packs to translate plugin Chrome.
Description check	✅ Passed	The description covers the required sections, explains the security boundary, documents testing, and reports the review and audit results.
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.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (3)

src/shared/plugins/plugin-translatable-chrome.ts (1)

1-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Shorten the allowlist policy comment.

Keep one non-obvious invariant beside the allowlist. Remove the repeated examples and explanatory prose.

For example: // Only exact non-security plugin chrome paths are translatable; new paths remain protected by default.

As per coding guidelines, “Comments must be concise, limited to non-obvious information, and preferably one line.”

Source: Coding guidelines

src/renderer/src/i18n/plugin-chrome-allowlist.test.ts (1)

13-22: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Protect destructive-dialog paths in the regression fixtures.

SECURITY_COPY_COMPONENTS omits PluginRemoveDialog and PluginRollbackDialog. A future allowlist path below either component will pass this guard. Add both names here and to the matching component fixture in src/shared/plugins/plugin-language-pack-artifact.test.ts.

Proposed fixture update

 const SECURITY_COPY_COMPONENTS = [
   'PluginConsentDialog',
+  'PluginRemoveDialog',
+  'PluginRollbackDialog',
   'PluginConsentProvenance',

src/shared/plugins/plugin-language-pack-artifact.test.ts (1)

60-72: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover the nested settings-search path.

The success table only traverses one protected component container. It does not exercise auto.components.settings.plugins.search.title, which requires both plugins and plugins.search to remain walkable. Add a successful parse case for that path.

Proposed test

+  it('lets a language pack translate nested settings-search chrome', () => {
+    expect(
+      parsePluginLanguagePackArtifact(
+        JSON.stringify({
+          auto: {
+            components: {
+              settings: { plugins: { search: { title: 'Плагины' } } }
+            }
+          }
+        })
+      ).ok
+    ).toBe(true)
+  })

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 778c5af1-5ff8-47ed-8ea1-abf5da8c4802

📥 Commits

Reviewing files that changed from the base of the PR and between 29bfc1e and 5780ee1.

📒 Files selected for processing (4)

• src/renderer/src/i18n/plugin-chrome-allowlist.test.ts
• src/shared/plugins/plugin-language-pack-artifact.test.ts
• src/shared/plugins/plugin-language-pack-artifact.ts
• src/shared/plugins/plugin-translatable-chrome.ts

[smwbev]

@nwparker — this one has no reviewer: CODEOWNERS routes src/renderer/src/i18n/locales/ and the localization scripts, but nothing covers src/shared/plugins/, so it did not get auto-assigned. You wrote the rule this touches (#8549, #11232), so you are the right person to say yes or no to it.

Short version of the change, since the boundary is yours to defend:

protectedTranslation refuses every auto.components.settings.plugin* key — 214 of them across 16 modules. The security tests name 8 of those modules; the other 104 are behind the rule as a side effect of the prefix, not because anyone classified them as a trust surface. In a localized build that leaves the plugins pane visibly half-translated: PluginMarketplaceBrowser.tsx is protected and renders PluginCatalogLayout from components/plugin-catalog/, which is not, so the inner list translates and the frame around it does not.

The prefix rule is unchanged. The PR adds an allowlist of 35 exact paths — section titles, empty states, Refresh, the local development form — and a protected container only stays walkable when exempt chrome sits below it, so every leaf is still matched on its full path. A key added to any plugin module tomorrow stays protected until someone edits the list, and that edit is a reviewable diff naming the path.

What I deliberately left protected is probably the more interesting half of the review: anything that promises something about plugin behavior (systemDescription's "Nothing runs until you review and enable it", PluginDevelopmentSection.help's "Dev plugins still require permission review"), the whole *Failed family because installFailed reports "The reviewed source may have changed", the blocked / official badges, and the remove/rollback confirmations. Tests enforce each of those boundaries, including that no exempt path is *Failed and none belongs to a module the security tests pin.

35 is the conservative floor, not a claim about the remaining 69. PluginMarketplaceListingRow.installed / .checkUpdate and the button labels in PluginInstallDialog are procedural too, but they sit beside trust copy in the same module, so splitting those is your call rather than mine.

Verified against a real pack as well: an 11,742-string Russian catalog parses today, parses with the chrome translated, and is still rejected when it rewrites systemDescription to "Every plugin here has been reviewed by Orca."

[smwbev]

@coderabbitai Addressed in c4f7122:

• Destructive-dialog fixtures: added PluginRemoveDialog and PluginRollbackDialog to SECURITY_COPY_COMPONENTS in plugin-chrome-allowlist.test.ts and to the pinned-component table in plugin-language-pack-artifact.test.ts (kept both lists alphabetical rather than inserting after PluginConsentDialog). You were right that a future allowlist path under either dialog would have passed the guard silently — the PR description already claims these confirmations stay protected, so the fixtures now enforce that claim.
• Nested settings-search coverage: added a success case for auto.components.settings.plugins.search.title, which is the only exempt family that needs two protected containers (plugins and plugins.search) to stay walkable. The existing table only exercised single-container component paths.

Not applied: shortening the policy comment in plugin-translatable-chrome.ts. That header is the design record of the security boundary — each line documents a deliberate exclusion (trust badges, behavior promises, *Failed family, destructive confirmations), which is exactly the non-obvious 'why' the guideline asks comments to keep. Since the boundary is under review by its owner right now, trimming its rationale is the author's call, not a drive-by cleanup.

pnpm lint, pnpm typecheck, and vitest on both files (65 tests) are green.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git show --stat --oneline c4f71221a
git show --format=fuller --find-renames -- \
  src/renderer/src/i18n/plugin-chrome-allowlist.test.ts \
  src/shared/plugins/plugin-language-pack-artifact.test.ts \
  c4f71221a

printf '\n--- Relevant assertions on the PR branch ---\n'
rg -n -C 5 \
  'PluginRemoveDialog|PluginRollbackDialog|auto\.components\.settings\.plugins\.search\.title|SECURITY_COPY_COMPONENTS|pinned' \
  src/renderer/src/i18n/plugin-chrome-allowlist.test.ts \
  src/shared/plugins/plugin-language-pack-artifact.test.ts

Length of output: 7274

---

@smwbev Verified in c4f71221a.

PluginRemoveDialog and PluginRollbackDialog now appear in both security-copy fixture lists. A future allowlist path below either dialog will fail the guard.

The new auto.components.settings.plugins.search.title test verifies traversal through both protected containers.

Keeping the policy comment is appropriate. It documents the security boundary and the deliberate exclusions.

---

✏️ Learnings added

Learnt from: smwbev
URL: https://github.com/stablyai/orca/pull/12455

Timestamp: 2026-08-04T09:06:13.979Z
Learning: In the plugin language-pack translation boundary, the policy comment in `src/shared/plugins/plugin-translatable-chrome.ts` is a design record for deliberate security exclusions. Retain this comment when it documents trust badges, behavior promises, `*Failed` messages, and destructive confirmations.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

_{You are interacting with an AI system.}

[nwparker]

Merged into main via #12514, which consolidated the open community translation work into one branch. Your commit is on main with you as its author — I used a rebase merge specifically so every contributor's commit and co-author trailer survived intact rather than being squashed into one.

Closing this in favour of that. Thank you for the fix, and sorry it took as long as it did to get through.