Repository navigation
feat(dev): just desktop me keeps Keychain approvals across rebuilds - #3002
Conversation
Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
…elper request Replace the helper's human, agent-store and session-store modes with one --item <operation> <service> <account> request, used by every credential-store read, add, replace and delete in macOS debug builds when BUZZ_DEV_CREDENTIAL_HELPER is set. The release-key read for `me`, Jev, paired tools and external-app pairings now go through credential-store instead of calling the Keychain directly. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
On macOS, `me` installs the helper outside the repository once (again only when this checkout needs a newer protocol), creates a local code-signing certificate, records the requirement the helper checks, and starts the debug build through a Cargo runner that signs it first. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
…ctly Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8016e259c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 Two actionable findings: non-macOS lint failure and inherited Cargo runner configuration.
…helpers - Allow dead code only off macOS for the policy items the macOS adapter alone uses, so Linux clippy -D warnings passes. - dev-sign-run.sh drops the Cargo runner variables and the signing identity before starting the app, so agents it starts can still `cargo run`. - Install one helper per protocol (buzz-dev-credential-helper-v<N>), so worktrees on different protocols never replace each other's helper. Co-authored-by: mbot <3754f8729004d95654c46dbab3129e4ab9ef05cc2534e2a3fbfc155983bd637b@buzz.block.builderlab.xyz> Signed-off-by: Bradley Axen <baxen@squareup.com>
…elper Linux lint: the policy module's macOS-only members are dead code there. dev-sign-run.sh unsets the runner and signing variables before exec, so processes the app starts don't inherit a runner that fails without them. The release human key is no longer served by the helper (protocol 2): `me` reads it directly, so it keeps the release app's Keychain approval. Docs state the same-user trust boundary, including the shared agent store, and how to revoke a protocol-1 helper's release-key approval. Co-authored-by: Quinn <96f056ad5f2305c8ddf637dc65d048aa4c12d7daeb8867690e34fca46b0ef64c@buzz.block.builderlab.xyz> Co-authored-by: Tyler Longwell <tlongwell@squareup.com> Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
| assert!(read_human().is_ok()); | ||
| std::env::remove_var("BUZZ_DEV_CREDENTIAL_HELPER_AUTHORIZE"); | ||
|
|
||
| // `just desktop me` reads the release key straight from the Keychain, never |
There was a problem hiding this comment.
🤖 [P3] Distinguish fallback tests from backend-routing coverage
The fake-reader unit test covers release_or_own fallback ordering, but not the production macOS selection of read_keychain at src/lib.rs:315–318. That selection is excluded from test builds, so changing it back to the helper-backed reader would leave these tests green while release-key sign-in fails with Denied. Reword this comment to describe fallback-only coverage, or add a testable backend selector.
| if (installedProtocol(helper) !== needed) | ||
| throw new Error("the installed helper did not start"); | ||
| console.log( | ||
| "Installed the Keychain helper. Click Always Allow on each Keychain dialog this launch; later launches ask only for your installed Buzz's key.", |
There was a problem hiding this comment.
🤖 [P3] Align the release-key prompt guidance
This message describes later launches, while docs/contributing.md:113 says they ask only once and the identity/help documentation describes a release-key prompt after each rebuild. These descriptions leave developers with conflicting expectations about repeated prompts. Align the guidance around the documented after-each-rebuild behavior; actual native prompt persistence remains unverified.
…ychain helper" This reverts commit 3a40477. Co-authored-by: Quinn <96f056ad5f2305c8ddf637dc65d048aa4c12d7daeb8867690e34fca46b0ef64c@buzz.block.builderlab.xyz> Co-authored-by: Tyler Longwell <tlongwell@squareup.com> Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
Why
just desktop measks for Keychain access about six times on every restart. A debugBuzzbinary is only ad-hoc signed, so Keychain ties each approval to that exact build: every rebuild, and every worktree, asks again, once per saved item.The login keychain shows the build-up:
jev-key.debughas 46 per-build entries,identity.debug35,agents30. A self-signed certificate alone doesn't fix it. A rebuilt binary signed with the same certificate is still refused, because code with no Apple team ID gets a per-build partition ID.What
just desktop menow does the Keychain work through a small helper that's installed once and kept. Keychain approvals belong to the helper, so they last.meon a Mac:crates/dev-credential-helperat~/.buzz-foundation/dev-credential-helper/.codesign, then one per saved item.just desktop, agents, CI and release builds, plus Linux and Windows.How
crates/dev-credential-helper)[--authorize] --item <read|add|replace|delete> <service> <account>, replacing the human,--agent-storeand--session-storemodes from feat(identity): support a local debug restore helper #1517 / Support agent credentials through the macOS debug helper #2174 / Route debug sessions through the credential helper #2541.me), the debug human key (create-only), paired tools, agents, debug federated sessions, the Jev and Claude debug keys, and external-app pairings.identifier "xyz.block.buzz.desktop.dev" and certificate leaf = H"…".--authorize, is retried once with a dialog allowed.--versionreports its protocol.mereinstalls the helper only when the checkout needs a newer protocol.crates/credential-store)BUZZ_DEV_CREDENTIAL_HELPERset, everyread/add/replace/delete, and the session functions, go throughdev_item.jev.rs,host_mcp/pairing.rsand the macOS humanaddinidentity.rsused to callSecKeychaindirectly. They now usecredential-store, with the same semantics and messages.addgoes through the helper too.scripts/desktop-dev.mjs,scripts/dev-credential-helper.mjs,scripts/dev-sign-run.sh)meon macOS turns this on.BUZZ_DEV_CREDENTIAL_HELPER*from.env.localor the shell is dropped, asBUZZ_DEV_RELEASE_KEYalready is.devstarts the app withcargo run, verified with a logging--runner(run --no-default-features --color always --). SoCARGO_TARGET_<triple>_RUNNER=scripts/dev-sign-run.shsigns the binary just before it starts. That takes about 1 s on the 420 MB debug binary.docs/identity.md: the "Local restore helper" section is rewritten as "Local credential helper on macOS", including the trust trade-off.docs/contributing.md,docs/buzz-help.mdandAGENTS.md(agents never runme) are updated.Risk
me. The certificate's key letscodesignuse it without asking. The docs say this, and say not to usemeon a shared account. Before this change, the per-rebuild prompt was the only gate.me, every helper call uses the 2-minute approval timeout, since any item may still need its one approval. A hung helper therefore takes 2 minutes to fail.Testing
cargo test -p buzz-dev-credential-helper: parsing and the item list, including what it refuses.cargo test -p buzz-credential-store:tests/dev_helper.rscovers--itemrouting, themerelease read through the helper, no fallback on a denied release read, add/replace/delete with Occupied, sessions, limits, timeouts and a retained socket.closed_helper.rsupdated.cargo test -p buzz-agent-controller:dev_credentials.rsupdated.cargo test -p Buzz --lib: 1060 pass.window_state::tests::onboarding_snapshot_is_kept_until_next_launch_restoresegfaults on this branch even when run alone; its file is not in this diff, and I have not checked it on main.node --test tests/integration/dev-commands.test.mjs: 56 pass. The helper environment and runner are set only formeon macOS; values from.env.localor the shell are ignored; setup failure falls back.Not yet done: a human run of
just desktop me(first launch approves; restart, rebuild and a second worktree show no dialogs).