Repository navigation
Make plugin signature enforcement opt-in - #2212
Conversation
jmecom
changed the title
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, integrating independent Rocket, Mantis and Groot coverage.
One actionable P2 is inline. No additional actionable findings in the signature verifier, publisher continuity, rollback restrictions, native access checks or contribution teardown. The documented any-publisher trust policy and exact 0 opt-out are intentional; signing does not sandbox plugin code.
Reviewed head: 5aec92d
Base: 063fccd
Source-only: tests, builds, CLI/app workflows, keyboard/focus behavior and packaged-app migration were not executed; CI was not assessed. The public PR description and changed source/commit text were inspected; no description attachments. This is a COMMENT review, not approval or merge authorization.
| fn verified_publisher(digest: &str, signature: Option<&Event>) -> Result<Option<String>> { | ||
| match signature { | ||
| Some(signature) => publisher_for_hash(digest, signature).map(Some), | ||
| None if signatures_required() => Err(UNSIGNED_PLUGIN_ERROR.into()), |
There was a problem hiding this comment.
🤖 [P2] Let Settings apply a signature-only repair before blocking legacy plugins.
This new gate disables existing unsigned installs. If an author signs the unchanged installed source, sign_release serializes the same artifact bytes as the unsigned import (imports.rs:418–436), so its revision hash stays the same. Native install_artifact can persist the new signature without changing that hash (the new unsigned_legacy_installation_can_be_replaced_with_a_signed_release test covers that layer). But src/app/PluginImport.tsx:361–378 compares only revisions for upToDate/updateAvailable, and lines 944–949 hide confirmation for a matching revision. The folder update check also returns only the hash (imports.rs:299–307). Settings therefore says “Up to date” and offers no repair while the plugin remains blocked for a missing signature; generic import redirects to the existing install. CLI reinstall or removal/reinstallation is a workaround.
Make update discovery and confirmation recognize a newly valid signature on an unsigned installed revision, while retaining normal publisher verification. Add a UI-level regression covering an enabled unsigned install → signed candidate with identical artifact hash → confirmed update → cleared error/activation. Changing only the native reinstall test or the dialog predicate would leave the hash-only discovery path unresolved.
947b2e7 to
ab867b0
Compare
shellz-n-stuff
left a comment
shellz-n-stuff
left a comment
There was a problem hiding this comment.
LGTM, we should get this one in sooner as it's a core building block for later stages with hierarchal enforcement and policies
ab867b0 to
7484be9
Compare
jmecom
changed the title
7484be9 to
c111df1
Compare
fsola-sq
left a comment
fsola-sq
left a comment
There was a problem hiding this comment.
Reviewed c111df1 against 3b98f2b. Assessment: 9/10 for the scoped implementation. No recommended blockers.
The Rust manager remains the enforcement owner. I traced import, catalog, enable/load, native host grants, reload, publisher continuity, rollback, and runtime disposal. The unchanged-artifact repair now reaches Settings through both folder and Git discovery, and pending update notices account for publisher changes. The prior signature-only repair finding is addressed. The UI keeps the existing shared Dialog, Button, SettingsGroup, and access-review owners; no new design exception is needed.
Optional documentation improvement: explicitly state that URL-based recommendations and artifact links are unavailable when BUZZ_REQUIRE_PLUGIN_SIGNATURES=1. prepare_url constructs a release without a signature (crates/plugin-manager/src/imports.rs:643-665), and the version-1 URL entry has no detached-signature field. Even packing an already signed folder produces an unsigned artifact. This is consistent with requiring signatures, but the adjacent URL-distribution instructions can lead an operator to expect that path to work in strict mode. A sentence in docs/managed-plugins.md#url-entries and the URL paragraph in docs/plugin-architecture.md is sufficient; adding a new release format is outside this PR.
Validation on the reviewed head: bin/cargo test -p buzzodz-plugins --locked passed; the six changed plugin/Settings Vitest files passed (269 tests); bin/node --test tests/integration/plugin-cli.test.mjs passed (4 tests); the complete plugin-import, plugin-cli-reload, and conversation browser files passed in Chromium and WebKit (36 tests, 2.3 minutes). The browser run exercised real CLI/native-catalog replacement and withdrawal, the existing Settings layout/scale checks, focus, error recovery, and both themes. I inspected the generated import/update screenshots. git diff --check passed, and the review worktree is clean.
Hosted checks have no reported failures, but six jobs were still running when checked, including Rust/tool integration and browser shards. The PR remains draft. Packaged desktop, human acceptance, and release-specific launch-environment behavior were not exercised.
… an update (#3024) ## TL;DR A `buzz://plugin-install` Git link pinned to both a `commit` and a `path`, for a plugin already installed from that same repository and folder, now opens the **Update** review that a plugin's Git `requestInstall` already gets. If the link names another branch or tag than the plugin tracks, the review says so. Before, it hit the Add path's **Already installed** refusal, so there was no reviewed way to update from such a link. Links without both pins, and links to a plugin installed from another source, are still refused as duplicates. ## What changed - When the installed plugin was imported (not bundled), `PluginImport` treats a link with both `commit` and `path` like a bound `requestInstall` request when deciding between update and duplicate (`src/app/PluginImport.tsx`). It reuses the existing same-source check: the installed `updateSource` must be a Git source whose repository matches the fetched preview's source and whose folder matches the candidate's folder, and the candidate must have the installed manifest ID. Native import saves the requested `ref`, and later update checks follow it. So when a link's or plugin request's `ref` differs from the installed Git source's ref, the review adds "Future updates will follow <new> instead of <old>." An empty ref reads as "the repository default". - Confirming a requested update (link or plugin request) re-reads the live catalog and refuses with "The installed plugin changed. Close this review and check again." if the installed revision, enabled state or saved source changed after the review opened, so the review's "stays disabled" line can't go stale. - A requested update of a disabled plugin now says "This plugin will stay disabled.", as the Details **Update** review already does. Native install already keeps the enabled state, and the review offers no enable switch for updates. - Docs: `docs/plugin-architecture.md`, `docs/deep-links.md` and `docs/buzz-help.md` describe the pinned-link update. Protections that still apply: the commit pin (native refuses a ref whose tip differs, and the review re-checks the fetched commit), the folder filter, the access review against the installed manifest, the publisher line and native publisher-continuity refusal, the unsigned-to-signed rollback warning, explicit **Update** confirmation, and preview discard on cancel. Links with only `commit`, only `path`, or neither, and pinned links to a bundled plugin, keep showing **Already installed** / **Open in Buzz**. Not in scope: update discovery, Update-all, managed policy feeds or checkers (#2504, #2621, #637), signature or publisher-trust policy (#2212, #2746), and the compact request dialog (#2625). This PR doesn't touch their hunks. #2983 and #2762 also edit `tests/browser/plugin-import.spec.mjs`; this PR leaves that file unchanged. ## Supported operation On a Buzz desktop build that includes this change, open `buzz://plugin-install?repository=<repo>&ref=<ref>&path=<folder>&commit=<40-or-64-hex>` for a plugin whose installed copy came from a Git import of that same repository and folder. The review shows **Update <name>**, with access changes listed against the installed version, and states any change to the branch or tag later updates follow. Nothing changes until you choose **Update**. Native import still refuses the update if the ref's tip isn't the pinned commit, or if the publisher changed or a signed plugin became unsigned. ## Tests New `src/app/PluginImport.test.tsx` cases, all with fictional `example.com` fixtures: - A pinned link to the installed source reviews as **Update**, installs the pinned folder, and doesn't call enable. This is the regression test for the original failure. - A disabled plugin stays disabled and the review says so. - A pinned link with another ref than the installed plugin tracks (a named ref, or the repository default) reviews as **Update**, says which ref future updates will follow, and installs on **Update**. A same-ref link shows no such line. - A link without both pins (none, path only, commit only), or a pinned link to a bundled plugin, still shows **Already installed** with **Open in Buzz**. - An installed source in another repository or folder, a local folder, or an unknown source is refused with the source-mismatch message and **Open in Buzz**. - When only another plugin ID is installed from that source, the link offers a new install. - A fetched commit that differs from the pin offers no **Update**. - Cancel discards the preview without installing. - If the installed revision, enabled state or source changes after the review opens, **Update** is refused, for both a link and a plugin's `requestInstall`. The request and link harnesses now return a ready catalog from `plugins.snapshot()`, which the new confirm-time check reads. Checked on a Linux box at `37cf73e1`: `biome check` on both files, `pnpm typecheck`, and `vitest run src/app/PluginImport.test.tsx` (100 passed). After the first review fixes, the same three checks passed locally (102 tests), and the bundled-plugin test fails on the earlier predicate. The later enabled-state drift case (two lines plus one test input) passed biome locally; host load refused a local test run, so PR CI covers it. With only the `requested` line reverted, 6 of the 8 `pinned link` tests fail. Against `main`'s `PluginImport.tsx`, the same 6 fail and the other 94 pass. The two that pass either way are the existing pin-retry test and the other-plugin-ID new-install case, which should behave the same before and after. After review feedback, the tracked-ref disclosure was verified on a Blox box at `e913727f`: `biome check`, `pnpm typecheck` and `vitest run src/app/PluginImport.test.tsx` (103 passed). With the review line disabled, the new tracked-ref test fails. The before/after screenshots predate this line and show a same-ref update. ### Before / after These screenshots come from a temporary Chromium journey using the existing `nativeImports` IPC fixture and fictional `example` data; the journey is not committed. It installs `example.linked` from `https://github.com/example/plugins` at `plugins/linked`, then delivers the same pinned link again with a newer candidate. | | Light | Dark | |---|---|---| | Before (`main`) |  |  | | After |  |  | ## Remaining gaps - No native (Tauri) run: the Playwright screenshots use the existing IPC fixture. Native import, publisher continuity and the deep-link handoff were not changed here. - No human has tried it in the app yet, so `buzz-review-completed` is not set. ## Initiative context Status: https://g2.sqprod.co/apps/kylez-buzz-improvements-status (buzz-improvements T027). Fixes NEXUS-4327 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: Kyle Zemek <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]>
Co-authored-by: Codex <[email protected]> Signed-off-by: Jordan Mecom <[email protected]>
Co-authored-by: Codex <[email protected]> Signed-off-by: Jordan Mecom <[email protected]>
Keep verified publisher metadata through update discovery and confirmation. Retire pending updates when the installed release changes. Co-authored-by: Codex <[email protected]> Signed-off-by: Jordan Mecom <[email protected]>
Allow unsigned external plugins by default. Require signatures only when BUZZ_REQUIRE_PLUGIN_SIGNATURES=1, retaining verification and publisher continuity for signed releases. Co-authored-by: Codex <[email protected]> Signed-off-by: Jordan Mecom <[email protected]>
Co-authored-by: Codex <[email protected]> Signed-off-by: Jordan Mecom <[email protected]>
4850e87 to
9793c15
Compare
…y-untagged-reactions * origin/main: (324 commits) fix(github): show private-repo body images from signed links (#3252) NEXUS-4029: Preserve semantic chips in the core composer (#2303) Make plugin signature enforcement opt-in (#2212) NEXUS-4156: Add workspace Leads and choose a chat's workspace (#2788) feat(plugins): report conclusively retired update providers (#3263) feat(messages): let attachment renderers open files from the host file card (#2239) feat(charts): bundle a Charts code-block renderer (#3226) Expose native reply grouping through Conversation.ui (#3245) NEXUS-4163: Keep plugin-owned chat deletion in Solo (#2787) NEXUS-4156: Expose Solo Workspaces to plugin regions (#2786) NEXUS-4022: Start region chats in the New Chat editor (#2784) feat(link-previews): unfurl web links below messages by default (#3225) fix(messages): keep names in literal Markdown as written when editing (#2834) feat(plugins): fetch a packaged managed plugin policy from a URL (#2504) fix(goose): enable deferred tool loading for goose agents (#3227) fix(unread): preserve manual intent in captured explicit reads (#3228) NEXUS-4337: Expose retained plugin update reviews (#3075) feat(conversation): add a link preview slot below messages (#3219) feat(plugins): let link previews read a public page's preview fields (#3217) Let sidebar row plugins decorate ordinary Solo chats (#3222) ...
* origin/main: (121 commits) Expose Solo region state before a component mounts (#3296) fix(builderlab): say a 401 was refused, not expired, and show each sign-in failure once (#3241) feat(agents): file new Solo chats in explicit personal workspaces (#2857) perf(css): limit broad style invalidation (#3267) Delay and fade in the Bestie badge celebration (#3292) fix(github): show private-repo body images from signed links (#3252) NEXUS-4029: Preserve semantic chips in the core composer (#2303) Make plugin signature enforcement opt-in (#2212) NEXUS-4156: Add workspace Leads and choose a chat's workspace (#2788) feat(plugins): report conclusively retired update providers (#3263) feat(messages): let attachment renderers open files from the host file card (#2239) feat(charts): bundle a Charts code-block renderer (#3226) Expose native reply grouping through Conversation.ui (#3245) NEXUS-4163: Keep plugin-owned chat deletion in Solo (#2787) NEXUS-4156: Expose Solo Workspaces to plugin regions (#2786) NEXUS-4022: Start region chats in the New Chat editor (#2784) feat(link-previews): unfurl web links below messages by default (#3225) fix(messages): keep names in literal Markdown as written when editing (#2834) feat(plugins): fetch a packaged managed plugin policy from a URL (#2504) fix(goose): enable deferred tool loading for goose agents (#3227) ... Signed-off-by: Tyler Longwell <[email protected]> Co-authored-by: Eva <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
* origin/main: NEXUS-4029: Offer folder chips in the composer (block#2789) perf(css): narrow effort button invalidation (block#3297) NEXUS-4433: Expose saved plugin installation source kind (block#3242) fix(identity): keep an unbacked key safe on Replace and unblock a corrupt saved key (block#3233) Expose Solo region state before a component mounts (block#3296) fix(builderlab): say a 401 was refused, not expired, and show each sign-in failure once (block#3241) feat(agents): file new Solo chats in explicit personal workspaces (block#2857) perf(css): limit broad style invalidation (block#3267) Delay and fade in the Bestie badge celebration (block#3292) fix(github): show private-repo body images from signed links (block#3252) NEXUS-4029: Preserve semantic chips in the core composer (block#2303) Make plugin signature enforcement opt-in (block#2212) NEXUS-4156: Add workspace Leads and choose a chat's workspace (block#2788) feat(plugins): report conclusively retired update providers (block#3263) feat(messages): let attachment renderers open files from the host file card (block#2239) feat(charts): bundle a Charts code-block renderer (block#3226) Expose native reply grouping through Conversation.ui (block#3245) Signed-off-by: Joah Gerstenberg <[email protected]> # Conflicts: # src/features/conversation/ComposerCompletions.test.tsx # src/features/conversation/ComposerCompletions.tsx # src/features/messages/MessageComposer.tsx
Make external plugin signature enforcement opt-in with
BUZZ_REQUIRE_PLUGIN_SIGNATURES=1. Unsigned plugins continue to install and run by default. Supplied signatures are always verified, and publisher and downgrade protections remain in place.Settings can apply a newly signed release even when its code has not changed.
Validation: 143 plugin-manager Rust tests, two focused CLI tests, and 36 plugin import/reload browser checks across Chromium and WebKit passed. Required pre-push checks also passed. Packaged-app and human testing remain pending.