Repository navigation
acpmux: a paired device's Deny (and the person's proved Allow) counts under approve-reads; no Allow drawn that cannot count - #24233
Conversation
|
Owned by hb-acpmux on big-red · session |
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Unclear: CI failed on
Failures:
Matched log linesNot re-run automatically: Written by red: web / Web status; base red: the same failure on the base 7c2a29c; does not block Tour of Gallery before/after for
States
Pick other states with a |
|
Found 2 test failures on Blacksmith runners: Failures
|
|
Orphaned: no activity for 13 h, and no lane holds a lease on it. Adopted by the orphan sweep: merged |
|
|
|
|
|
|
|
|
|
|
…ever lasts, changes no mode or policy, and stops when the phone is unpaired (#24233) Tests for the five landing terms of #24233: a phone's signed Allow from a Web connection under approve-reads is refused for allow_always and when expired, counts once for its own prompt and nonce, is refused when replayed on the next prompt, and is refused when the phone is unpaired after it signed; a deny still counts. The person's proved Allow on a session whose mode does not ask counts once and leaves the mode as it was. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
13b6a71 to
7655146
Compare
|
|
|
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0d3bec5 to
cb1878d
Compare
…ted (#24233, term 4; red) ApproverSet::verifies_then keeps the current check: the key is copied under the lock, and the signature check and the commit run outside it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ne of its signatures counts (#24233, term 4) The approver set's lock is held from the key lookup through the nonce spend (approver_spends), so _acpmux/approvers_set returns only after an in-flight commit, and no signature of a removed phone counts after it. Lock order: the prompt's lock, the approver set's, the spent-nonce store. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review (codex gpt-6-astra xhigh, next-review): SHIP at 7a718b7 No blocking findings. |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Review (codex gpt-6-astra xhigh, next-review): SHIP at b04671c No blocking findings. Reviewed the diff and supplied surrounding code. Tests were not run because the snapshot lacks build manifests and dependencies. |
|
Shipped in:
|
![Fix with [code]smith](https://pr-comments-assets.blacksmith.sh/codesmith/fix-with-codesmith-light-3x.png)
Adopted and rebased (2026-10-25)
The lane that opened this is gone. Its change (e01aa4e..cdfb86c) is squashed onto
feat-cmux-next, now ec66898. Conflicts:webviews/test/permission-ask-row.test.tsx(this PR's test now comes after base's newer grouped-row tests);hub/permissions.rs, where base'srespond_permission_annotated(question annotations, #24356) now holds this PR's Web answer check in place of the plainweb_control_check; andserver/peer_forward.rs, which keeps base's attach presence beside this PR'sallowRefused. The commits added on top are tests and rustfmt only. The daemon and pane code is the original lane's.Rebased again onto a8d57c9, over #25424, which lets a question answered in the pane count on another machine under the person's proof. There was one conflict, in
respond_permission_annotated:needs_proofand its annotations refusal are kept.answer_proof. It now takes the annotations and passes the answers and annotations todecision_proves, as base does.answeredByand no annotations.What changes, plainly: a paired device (a Web connection: the iPhone, the dashboard, a peer that serves this Mac as Web) can now:
An Allow with neither is refused exactly as before. The pane no longer draws an Allow that can't count.
Landing terms (2026-10-25), and where each holds
hub/approvers.rsmessage) covers the daemon run (personInstance), the Mac id, the approver, the session id, the permission id, the option (the decision), the prompt's one-timedecisionNonce,expiresAt(at most 300 s ahead) and a digest of any answers. The person's HMAC proof (person.rsdecision_proof) covers the run, session, permission id, option and nonce, and since acpmux: a question answered in the pane counts on another machine under the person's proof (re-cut of #23869, daemon side) #25424 any answers and annotations.Uuid::now_v7) for eachsession/request_permissionit receives, and each request carries exactly onetoolCall. A re-ask is a new request with a new id.reregister_permission). It gives the request a new nonce and a new run id, so a proof from before the restart counts nothing.spend_decision), and a second use is refused.toolCallIditself, or a digest of the tool call's input, would be a later cnx-ios + acpmux change. This PR doesn't need it.allow_alwaysoption is refused (remote.lasting_grant_refused), signed or not.allow_chatfrom the Web is refused.remote.policy_not_asking). A Deny from any client counts.verifieslooks the approver up when the answer arrives, so a signature made before_acpmux/approvers_setremoved the phone counts nothing.ApproverSet::verifies_thennow holds the approvers lock through the ECDSA check and the commit, andPerson::approver_spendsspends the nonce inside it. A revocation either lands first, and the signature counts nothing, or waits until the decision is spent.verifies_then): the prompt's lock first, then the approvers lock, then the spent-nonce store. Nothing awaits while either lock is held; both are std mutexes taken and released inside one synchronous call.a_revocation_during_a_commit_waits_for_it_and_then_the_signature_counts_nothingshows the race itself. A commit blocks insideverifies_thenwhile another thread revokes the phone; the revocation waits for the commit, and the same signature then counts nothing. It is red at 0159569 (the test with today's semantics, run 40359378490: the revocation went through mid-commit) and green at 2e7593c (run 40359378532). 7a718b7 is rustfmt only (fmt and clippy clean, run 40360085403)._acpmux/approvers_settakes only the 65-byte public key. The set lives in the daemon's memory, never in a file, and only the person's app may set or read it (person_required). So another agent on the Mac can't sign as the phone. The person's HMAC key stays in the app's and the daemon's memory (the keyed daemon is not dumpable).paired-devices.json, which another process of the same user could rewrite with a key of its own. That hole is in base's app code (Settings > Devices: each paired phone gets its own acpmux credential and the Full access switch (off by default) #25088), not in this PR. The review asked whether this PR makes it worse. The answer was yes for Web exposure: a key planted that way now also counts for a paired device's Allow. But the same attacker already had equal or broader approval power through the local socket on base. cmux NEXT: paired-devices.json counts only when it is the list the app saved last #25535 makes the file count only when it is the list the app saved last (a digest in the app's data-protection Keychain), so a rewritten or restored list enrolls no approver. It landed first, as 06d4a46.Tests:
tests/web_mode_current.rsa_phones_signed_allow_from_a_paired_device_counts_once_never_lasts_and_stops_when_unpaired, under approve-reads from a Web connection, for points 1-4:allow_alwaysis refused;a_proved_allow_from_a_paired_device_changes_no_mode(point 2): a session whose mode does not ask. The person's proved allow counts once, the mode staysnormal, and the next unproved Web allow is still refused.tests/durable_agents.rsan_adopted_prompt_gets_a_new_decision_nonce_that_a_relayed_allow_can_use(on base) covers replay after a restart: the same id comes back with a new nonce and run, the old proof is refused, and the new one counts.tests/person_allow.rsa_phones_signed_allow_counts_once_and_stops_when_revoked(on base) covers another option, Mac, run or key over the unix socket.Runs:
durable_agents53,peer_integration44,person_allow25,web_mode_current13,web_mode_drift3, andhub_integrationpermission 25.agent_tools::tests::own_servers_run_under_the_daemon_prefix(run 40294994012);hermetic_daemon_guard's two tests, which flag base's newtests/linux_sandbox_init.rs(base run 40353228261).Review: a fresh Codex gpt-6-astra xhigh review at 7a718b7 found landing terms 1, 1(a), 1(b), 1(c), 2, 3 and 4 hold, and said SHIP for this PR's own scope, with term 5's base hole set aside for #25535 (above). After the seal landed, the branch took a merge of feat-cmux-next (no conflicts), and a fresh Codex gpt-6-astra xhigh review of the merged head b04671c found terms 1 to 5 hold and said SHIP. At that head: cargo fmt and clippy
-D warningsare clean; lib 1236/1237 (the one failure,own_servers_run_under_the_daemon_prefix, fails on base too); web_mode_current, web_mode_drift, peer_integration, person_allow, full_access_managed and hub_integration pass; the webview permission suites pass 9/9 and tsc is clean.UI: the gallery's new
Panelvariantallow-refusedshows the pane state: Deny only, plus where to approve. For the before/after, see the PR's gallery-advisory run.No capture: the only native-mapped file is the generated protocol
types.ts(a new optional field); the visible change is the webview permission panel, shown by the gallery variant above.Overlap with the #23027 recut (same file,
webviews/.../permissions/client.ts): this PR's hunks there only carryallowRefusedin the per-chatlastReadcache and in the refresh result. They neither read nor setloading,readyorerror, and the reselect restore keeps base'sloading: falseanderror: undefined, so they do not depend on the disconnected-error or loading state that recut changes. Whichever lands second rebases.Original description
Leo, 2026-10-21: "this literally doesn't work".
What broke
Leo had a chat open that runs on his peer machine air-blue, under the approve-reads policy. The grouped ask showed Allow / Always / Deny. Every click came back with this error: "this session's permission settings do not ask before it acts (policy approve-reads); a paired device cannot prompt it or answer its permissions until the local user sets ask or deny-all". The chat stayed stuck.
Why it refused:
web_control_verdictholds every Web permission answer to the asking rule, Deny included. A session whose policy doesn't ask therefore refuses every answer.hub/person.rs) allows only from its own app, and a forwarding daemon is never that app. Allow and Always were drawn anyway, and those buttons could never work.The fix
Hub::web_answer_check). It can only stop a tool call, and the person rule already lets any client deny.personDecision) for that exact pending prompt. It also covers an allow signed by an enrolled approver, such as the person's phone. Such an allow no longer gets the mode, policy and lasting-grant refusals. Nothing else changes:_acpmux/permission_groupsnow returnsallowRefused: "permission.person_required"to a reader that is not the person.Security reasoning
remote.policy_not_askingfrom a Web client,permission.person_requiredfrom a peer.Still open
From Leo's Mac, an Allow for a chat on a peer Mac still can't count. That peer's acpmux only trusts its own app's key, and nothing proves this Mac's click to it. That needs the device-bound proof that the peer test names (cx-aocz). It is a new cross-machine trust grant, so it isn't built here. Until then the pane says where to approve and offers the Deny that works, instead of dead buttons. The single-ask card (
PermissionCard) still learns this only after a click, through its "Remote approvals are off" notice. That is not changed here.Tests
tests/web_mode_current.rsa_paired_device_answers_an_approve_reads_prompt_with_a_deny_or_the_persons_proof. On one daemon with an approve-reads session and a Web (paired device) connection:allowRefusedand the person's read doesn't;remote.policy_not_asking;no);yes).tests/peer_integration.rsa_peer_chats_deny_goes_through_under_approve_reads_and_its_read_refuses_allowsreproduces Leo's setup:allowRefused;tests/web_mode_current.rsandtests/web_mode_drift.rspreviously asserted that a Web group deny is refused. They now assert that no asking rule holds it.permission-ask-row.test.tsx: an ask withallowRefusedoffers only Deny, says where to approve, and ignores the allow shortcuts.permission-groups-switch-keeps-last.test.ts: the refused allows stay with their chat across a switch.Red on the test-only commit and green at the head on the fleet.
Original lane: cosmic-gray
Agent session: agent session
🤖 Generated with Claude Code