Skip to content

feat(desktop): 0.24.0, the Touch ID approver key and one-tap stop review - #1519

Merged
abrichr merged 34 commits into
mainfrom
feat/stop-review-queue
Oct 25, 2026
Merged

abrichr merged 34 commits into
mainfrom
feat/stop-review-queue

Conversation

@abrichr

@abrichr abrichr commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

Desktop 0.24.0. This PR carries #1500 (the approver key set up in the app, behind Touch ID), which was closed and folded in here, and builds on #1486 (Desktop 0.23.0, signed approvals), which merges first. The founder approved the 0.24.0 release.

#1500, folded in: the approver key behind Touch ID (a Secure Enclave key wraps the Ed25519 key, and every signature asks for presence). It also includes --approver-signer stdio in Flow, so Flow never holds the key, and the security-review fixes: a stored key is re-registered only after it opens behind Touch ID, and a Touch ID Mac never falls back to a terminal-made key.

Problem

Most operator time in the production soak (#1513) goes to stops. For each stop, a person opens the item, switches to the app, looks the patient up, compares, comes back and picks an action. The soak's cost model charges 90 s per stop plus a 30 s interrupt.

Change

Review stops shows every stop as one card. A card has three parts:

  • the question ("Same person?", "Did it save?", "Approve this save?");
  • the evidence side by side: the item's values next to each possible match, field by field, each value marked Same or Different, or next to the form about to be saved;
  • the choices, each with what it does.

The queue presents and collects; it never decides.

  • Digest-bound. A card's id is a digest of everything it shows, and the engine rebuilds the cards before acting. A changed, expired or missing card, or a choice the card doesn't offer, changes nothing. "Not sure, leave it" changes nothing and signs nothing.
  • Signed. An answer to a stopped item is a decision receipt signed with the approver key: Touch ID on a Mac (feat(desktop): set up the approver key in the app, behind Touch ID #1500), otherwise the file key. The item's existing action then runs with its usual checks. A waiting save goes through the signed item approval (fix: a person's approval needs a registered approver key's signature #1486).
  • Logged. Receipts and how long each card was on screen go to stop_decisions.jsonl. Real operator times can then replace the cost model's assumption.
  • Desktop view. Needs you has Review one by one. Keys: 1-9 answer, J/K move, X adds a card to a batch, Shift+1-9 answers the batch, ? lists the keys. No choice is styled as the default.
  • Batches. Only similar stops batch: same kind, task and reason. A batch takes one choice and one Touch ID, each card is still checked, and one stale card stops the whole batch. Save approvals never batch.
  • Phone. /stops on the existing paired decision portal shows the same cards.
    • Every route needs an approved device session. Answers also need the portal origin and the CSRF token.
    • Phone answers are signed with the decision portal's approver key, and the receipt names the key and the device. The window's IPC can't select that key.
    • Waiting saves stay on the portal's Decisions page; re-arm is answered on the computer.
    • Remote access goes only through the existing customer ingress (the clinic's HTTPS proxy, VPN or ZTNA) plus pairing. There is no relay and nothing is served unauthenticated.
  • Re-arm card. "Turn only ask about problems back on?" shows why the switch went off, when, and the 10 items saved and checked in a row since, by name. Flow offers it only after 10.
  • Locked Mac. A Mac whose screen is locked no longer reads as having no Secure Enclave. The refusal now says to unlock it. The same fix is on feat(desktop): set up the approver key in the app, behind Touch ID #1500.
  • New action. A stop where nothing was written now also offers "I did it by hand". For a possible duplicate that is a different person, running it again would stop at the same check.

Measurement: 20 recorded soak stops

packages/desktop/benchmark/stop_queue/ turns the first 20 stops of the 2026-10-24 production soak into the cards Desktop builds. The ground truth for each comes from the soak. The possible-match rows are reconstructed, because the soak didn't keep the app's list. Ten of the cards are "hard": the possible match is a different person, or the save went in wrong.

A keystroke-level model (Card, Moran, Newell 1983; Kieras 2001) estimates the seconds to decide one stop:

Stops Before (Needs you plus the app) After (the card)
All 20, mean 26.7 s 8.2 s
15 "Same person?", mean 29.6 s 5.5 s
3 "Did it save?", mean 25.6 s 23.5 s

This is a model, not a person. No one has timed either arm yet. "Did it save?" cards barely improve because they don't carry the record check's read-back values yet.

A practice page runs the real Review stops screen on the 20 cards, so a person can time it:

cd packages/desktop
npx vite --port 1420
# open http://localhost:1420/benchmark/stop_queue/practice.html

The page scores accuracy only (right answers, and hard stops caught). Time goes into the downloaded file and is never scored. Pressing 1 blindly scores 8 of 20 and catches 0 of 10 hard stops.

Not done

Tests

  • Engine (test_stop_queue.py):
    • Card kinds and the marked differences.
    • A signed answer verifies against the key; a cancelled prompt changes nothing.
    • A changed card, an unoffered choice and an expired save all change nothing.
    • A batch is one prompt and never mixes kinds; one stale card stops it.
    • With no approver key, nothing changes.
    • A waiting save goes through the signed item approval.
  • Phone (test_stop_queue_portal.py):
    • Route auth and origin checks.
    • A portal-signed answer verifies; the window can't select the portal key.
    • Stale cards, batches, and the narrowed lock: closing a stop goes through while a run waits, and running an item again waits.
  • Drill (test_stop_queue_drill.py): every right answer is offered, and the card ids are digests of what the cards show.
  • UI (Stops.test.tsx, ItemApproval.test.tsx): keys, batch, timing sent, setup prompt and the comparison table.
  • Results:
    • Desktop: 1,364 passed, 48 skipped. Ruff clean.
    • Frontend: 390 passed, tsc clean.
    • Checked by hand in the browser: the practice page runs to the results screen.

🤖 Generated with Claude Code

abrichr and others added 17 commits October 24, 2026 12:33
`item-approval approve|decline --approver-signer stdio --approver-public-key HEX`
lets the process that holds a registered approver key sign the answer
without handing the key to Flow. Flow looks the public key up under
`approvers`, writes the exact bytes to sign as one JSON line on stdout and
reads one line back: a signature, checked against the registered key before
use, or a refusal. Anything else is a refusal, and nothing is recorded.

Desktop uses this to sign with a key kept behind Touch ID. Also adds
`authority.remove_principal`, which removes a registered key only while it is
still the one registered, for key rotation and removal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On a Mac with a Secure Enclave, Run it no longer needs terminal setup. The
approval screen shows "Set up your approver key" until the key exists: one
click and one Touch ID (or login password) prompt.

- The Ed25519 approver key is sealed to a Secure Enclave key whose access
  control asks for user presence on every use, and kept in the login
  Keychain. It is never written to disk unsealed, never logged and never
  given to Flow.
- Setup registers the public key under `approvers` with Flow's own
  `register_principal`, the function `jit register` calls.
- Each answer runs Flow with `--approver-signer stdio`. Desktop checks that
  the bytes Flow asks it to sign are the answer the person chose, prompts,
  signs and drops the key.
- No key, a cancelled prompt or a Keychain error means no answer.
- Settings > Approver key replaces the key (new one registered before the
  old one is retired) or removes it.
- Windows, Linux and Macs without a Secure Enclave keep the terminal setup.

The Keychain's own access-control flags need a keychain-access-groups
entitlement the Python engine doesn't have (errSecMissingEntitlement), so the
key is sealed to the Secure Enclave instead, through Security.framework over
ctypes. Real-hardware tests are opt-in (OPENADAPT_REAL_KEYCHAIN_TESTS=1) and
never show a prompt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds a stop-review queue over Needs you and waiting save approvals:

- stop_queue.py builds one card per stop: the question ("Same person?",
  "Did it save?", "Approve this save?"), the item's values next to the
  record or form with differences marked, and choices that say what they do.
- A card id is a digest of everything it shows. The dispatcher rebuilds the
  cards before acting, so a changed, expired or missing card, or a choice
  the card doesn't offer, changes nothing.
- Answers to stopped items are signed with the approver key (Touch ID on a
  Mac, one prompt per batch), then applied through the item's existing
  action and checks. Waiting saves go through the signed item approval.
- Similar stops batch under one choice; each card is still checked.
- Review stops screen with keys: 1-9 answer, J/K move, X batch, Shift+1-9
  answer the batch. Each answer records how long its card was shown.
- A nothing-written stop now also allows "I did it by hand".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Adds authenticated portal routes for the stop queue (list, per-card
screenshot, single and batch answers) and a phone page at /stops.
Phone answers are signed with the decision portal's approver key
through a dispatcher method no IPC command registers; the receipt
records key "portal" and the device session. Waiting saves stay on
the portal's save approval and re-arming stays on the computer.
Phone answers share the window's dispatch lock.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Same-person cards put the item next to each possible match in one table,
  each value marked Same or Different. No choice is styled as the default.
- Review stops takes a backend, so a practice page can run the real screen
  on recorded stops (benchmark/stop_queue/practice.html, dev only).
- benchmark/stop_queue: the first 20 stops of the 2026-10-24 production
  soak as cards, with the answer the ground truth calls right; a
  keystroke-level estimate of seconds per stop before and after (26.7 s to
  8.2 s mean, a model, not a person); a practice page that scores accuracy
  only and records time for the measurement.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…a run waits

- The phone's stop page renders each possible match field by field with
  Same/Different marks, like the window.
- A phone answer waits for the window's lock only when it runs an item
  again; closing a stop (skip, done by hand, saved) goes through while a
  run waits for a save.
- OPERATOR_SCREENS: Review stops (cards, signing, keys, batches);
  CAPABILITIES row.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- The re-arm card shows Flow's re-arm request: why the switch went off,
  when, and the 10 items saved and checked in a row since, named. Its id
  covers the request, so a re-arm signs only what the person saw.
- Answering it is one Touch ID over Flow's own bytes (rearm_to_sign); Flow
  verifies against the approvers registry, never a key Desktop supplies.
  Desktop's switch view honours a re-arm by a registered approver.
- Feature-detected: the card appears only with a Flow that has signed
  re-arm (#1517). Checked against #1517's real switch log.
- A locked Mac no longer reads as having no Secure Enclave: the check isn't
  remembered, and the refusal says to unlock the Mac.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
errSecInteractionNotAllowed (-25308) while the screen is locked made the
availability check remember False, so Desktop showed the terminal setup
until restart. The check is no longer remembered then, and the refusal says
to unlock the Mac instead of blaming Touch ID.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… Touch ID Macs

From the security review of #1500:

- setup() re-registered a stored but unregistered key without a prompt, so
  anything that can write the Keychain item could plant its own public key.
  It now requires the key to open behind a presence prompt (a fresh
  signature over a setup challenge) first, and refuses an item that admits
  it has no presence check.
- On a Mac that can keep the key behind Touch ID but has none set up, Run it
  fell back to a terminal-made file key, which asks for no presence. It now
  refuses and points to setup in OpenAdapt; the file key counts only where
  the app can't keep the key. Settings says so.
- Desktop 0.24.0 with its changelog (after #1486's 0.23.0).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From the security review of #1519:

- Answering stops re-checks every card after the signature returns and
  before anything changes, so a card that changed or expired while the
  Touch ID prompt was up changes nothing.
- Queue answers on a Mac that can keep the key behind Touch ID never fall
  back to a terminal-made key (as Run it, #1500).
- Removed the unreachable re-arm branch and a method pasted into the
  portal's StopQueueBackend Protocol.
- 0.24.0 changelog entries for the stop queue and re-arm.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abrichr abrichr changed the title feat(desktop): review stops as one-tap cards, on the computer or a paired phone feat(desktop): 0.24.0, the Touch ID approver key and one-tap stop review Oct 25, 2026
Desktop's Touch ID setup now goes through Flow's signed roster:

- No roster yet: the new key signs the roster's root behind the one
  Touch ID prompt (the signature is the presence proof), and this Mac pins
  that root in the login Keychain; Flow children get it as
  OPENADAPT_APPROVER_ROOT.
- Rotation: the new key proves it opens, the old key signs it onto the
  roster, and a non-root old key is retired with a signature.
- A roster this Mac didn't start: an approver on it adds the key; Desktop
  never adopts a root it finds.
- Removal takes a non-root key off the roster with its own signature; the
  root stays and its key is only deleted here.
- The stdio signer and the re-arm's approver keys read the pinned roster.
- The real Secure Enclave tests skip, saying why, while the Mac is locked.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@abrichr abrichr left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coordinator security review (AI-assisted), head a12ab81. Changes requested.

  1. The env var overrides the Keychain pin. OPENADAPT_APPROVER_ROOT overrides it in FlowRoster.root() and flow_bridge._subprocess_env. The Keychain pin must win, and a differing env value should be refused.
  2. Replay and rollback, inherited from #1486. Store the roster head in the Keychain.
  3. Consider protecting the pin with the Secure Enclave key. A keyring item is readable by the same binary without a prompt.

abrichr and others added 3 commits October 25, 2026 00:50
… rollback

From the security review of a12ab81, on top of #1486's 56d0e4a
(config-only pins on governed runs, a chained roster, recorded heads):

- FlowRoster.root(): the pin this Mac set up behind Touch ID (login
  Keychain) wins. OPENADAPT_APPROVER_ROOT is an administrator's setting that
  counts only when the Keychain holds no pin; a different value is refused
  (approver_root_conflict), and Desktop then starts no Flow process.
- A Mac that pins a root never starts another one.
- The roster head (seq, hash) is kept in the Keychain after each change
  Desktop makes; lookups and changes require the roster to extend it, so an
  older roster put back approves nobody.
- Governed runs take their pin only from their deployment config, so
  Desktop writes this Mac's pin into each run's config snapshot; a config
  that names another root is refused before Flow runs.
- 0.24.0 changelog entry.

Not done: sealing the pin behind the Secure Enclave. The pin and its
signature would live in the same user-writable Keychain as everything a
same-user process can rewrite, so a presence-signed pin has no anchor the
attacker can't replace; the run-level config pin and recorded heads are the
controls that hold.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@abrichr abrichr left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coordinator security re-review (AI-assisted), head 95b3e93. Fix (5) holds: the Keychain pin wins. Changes requested. This PR inherits H1-H3 from #1486, plus two of its own:

  1. Pass the Keychain roster head into Flow runs. Today only Desktop's own lookups check it, so a Flow run accepted an approver that Desktop refused.
  2. Stage the deployment config snapshot outside run_dir (dispatch.py:1530); the agent's user can write the run dir. Have Flow parse --config once, so it can't be swapped mid-run.

abrichr and others added 2 commits October 25, 2026 01:50
…un dirs

From the second security review:

- Each run's deployment config snapshot carries this Mac's roster head
  (approvers.head) next to its pin, so the Flow run refuses a roster that
  Desktop would refuse.
- The snapshot is staged in an owner-only folder under Desktop's data
  directory, not in the run directory the run (and anything it drives)
  can write.
- Run it answers pass this Mac's pin explicitly (--approver-root): the
  reader brings its own pin, and the run's record must match it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Verification no longer reads OPENADAPT_APPROVER_ROOT (#1486 ab8a9ba), so
these tests name the root they pin.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abrichr

abrichr commented Oct 25, 2026

Copy link
Copy Markdown
Member Author

AI re-review (coordinator)

Re-reviewed at bb671b1. Verdict: not ready. This PR is report-only here because it ships Desktop 0.24.0 after 0.23.0, so I did not enable auto-merge.

This PR contains #1486, so it inherits both items from the #1486 re-review:

  • Blocking: serve --guard takes the agent's env pin (agent/cli.py:1006, guard.py:526).
  • Should fix: a resumed governed run's gate is not config_only.

Both probes reproduce on this head. This PR also adds one blocking hole of its own.

Blocking: the agent can swap the staged config before Flow parses it

dispatch.py (around line 1539) writes the Keychain root and head into a snapshot under data_dir/staged-configs. It then passes the path to Flow with --config. The folder is 0700 and the file is 0600, which keeps out other local accounts. The agent runs as the same user, so neither stops it.

The file exists for the whole Flow call, and Flow parses it once, after interpreter start-up. A same-user process that watches the folder can replace the file in that window with a config that names its own root and key_dir and has no head. Flow (config_only) then trusts that root, which defeats the Keychain anchor. ensure_private_dir also follows a pre-planted symlink for staged-configs.

Probe: prepare_flow_config with Keychain root R and a head, stage_private_yaml, and a watcher thread that runs os.replace on new .deployment-*.yaml files. load_deployment runs 0.3 s later:

staged dir mode: 0o700 (owner = the agent's user too)
Flow parsed root: AGENT
Flow parsed head: None key_dir: .../agent-keys
HOLE: Keychain pin replaced by the agent's in the staged snapshot

Fix (either works):

  • (a) Desktop passes --config-sha256 <digest of prepared.payload> on argv, which another process can't change. Flow reads the file's bytes once, refuses on a mismatch, and parses those same bytes.
  • (b) Desktop passes the snapshot over an inherited pipe or stdin (--config -) and writes no file.

Also refuse a symlinked staged-configs folder.

Minor

  • Desktop's item-approval approve|decline passes --approver-root but not the Keychain head. The waiting run still checks its config head, so this isn't a hole, but the signing side can't detect a rollback.
  • The Keychain root and head are written through Python keyring, as generic-password items. Their protection against a same-user process depends on the item ACL trusting only Desktop's signed sidecar binary. Please confirm that a plain python -c 'import keyring...' or security delete-generic-password can't read, rewrite or delete them. If the root item can be deleted, FlowRoster.root() falls back to OPENADAPT_APPROVER_ROOT.

What holds

  • The stdio signer keeps the key in the Secure Enclave behind presence. Desktop checks that the bytes it signs are the person's answer.
  • Flow parses the config once per command.
  • The Keychain head is required to extend.

…ed dirs

From the third security review, with #1486's 7fd4699 merged:

- Every run and qualification case passes --config-sha256 with the digest
  of the bytes Desktop wrote (staged byte-exact, no newline translation).
  Flow reads the file once, refuses it if it changed, and parses only those
  bytes, so a same-user process can't swap the staged config between
  Desktop writing it and Flow reading it.
- Run it answers pass the Keychain's roster head (--approver-head) with the
  pin.
- ensure_private_dir refuses a symlinked folder.
- OPENADAPT_APPROVER_ROOT never counts for Desktop. With no Keychain pin
  (never set, or deleted by a same-user process), only an administrator's
  approvers.root_public_key in Desktop's own deployment.json does; one that
  differs from the Keychain's is refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abrichr

abrichr commented Oct 25, 2026

Copy link
Copy Markdown
Member Author

AI re-review (coordinator), round 2

Re-reviewed at eced3b1. This PR is report-only here (it ships Desktop 0.24.0 after 0.23.0), so I did not enable auto-merge. Verdict: not ready. Two blocking holes remain:

  1. The --config-sha256 symlink bypass inherited from fix: a person's approval needs a registered approver key's signature #1486 (see the fix: a person's approval needs a registered approver key's signature #1486 comment). It applies directly here, because the digest is what protects Desktop's staged snapshot.
  2. A new one, below.

Blocking (new): the env fallback now reads an agent-writable file

approver_keychain.py, FlowRoster.root() / _admin_root(): without a Keychain root item, the root comes from <data_dir>/deployment.json (approvers.root_public_key). That file is a plain file the same OS user can write. The PR's own test (test_an_admin_root_counts_only_without_a_keychain_pin) notes: "A Keychain pin deleted by a same-user process leaves only the admin's."

The environment was at least set by whoever launched Desktop. deployment.json can be written by the agent itself. dispatch.py also uses the same file as base_config, so its approvers.key_dir flows into the snapshot as well.

Probe, with no Keychain item and an agent-written deployment.json:

_admin_root(): AGENT
snapshot Flow gets: approvers: | root_public_key: bbbb… | key_dir: …/agent-keys

Desktop then hashes this snapshot and passes --config-sha256, so Flow trusts the agent's root.

Fix: drop the fallback. With no Keychain root, pinned_root() returns None, and person answers are refused, which fails closed. An administrator who needs a root on a Mac that didn't start the roster should go through the Touch ID setup ("an approver adds this key"), which records the root in the Keychain. If an admin pin outside the Keychain is required, it has to live somewhere the user can't write (for example, a root-owned /Library/... file or an MDM profile), and Desktop must check that location's owner and mode.

Fixed and confirmed

  • ensure_private_dir refuses a symlinked folder.
  • --approver-head is passed on answers and parsed by item-approval.
  • Desktop passes --config-sha256 on run and run-case whenever it stages a snapshot. When it stages nothing, there's no approvers section and no pin, so person answers are refused.
  • With no --config, the guard and governed-resume pins refuse (approver_root_unpinned).
  • OPENADAPT_APPROVER_ROOT is no longer read in FlowRoster.

Still to confirm

Whether a same-user process can delete the Keychain root or head items. The python keyring generic-password ACL matters more now, because deleting the root item triggers the fallback above. With the fallback removed, deleting the root only causes a refusal.

abrichr and others added 2 commits October 25, 2026 02:35
From the fourth security review: with no Keychain root, FlowRoster read
<data_dir>/deployment.json, which the user (and so a same-user agent) can
write, so the agent's root reached the staged config and its digest.

- FlowRoster.root() returns the Keychain pin or None; the deployment.json
  fallback is gone. With no pin, Desktop's runs record none and every
  person's answer is refused.
- APPROVAL_MODES.md notes that a same-user process can delete the Keychain
  items, which now fails closed. An administrator's pin for a Mac that
  didn't start the roster would need a location the user can't write (a
  root-owned file or an MDM profile); not built.
- Inherits #1486's a6f2e00 (config pinned by path, O_NOFOLLOW).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abrichr

abrichr commented Oct 25, 2026

Copy link
Copy Markdown
Member Author

AI re-review (coordinator), round 3

Re-reviewed at 70184d3. This PR is report-only here (it ships Desktop 0.24.0 after 0.23.0), so I did not enable auto-merge. Verdict: not ready, one blocking hole.

Fixed and confirmed

  • FlowRoster.root() returns only the Keychain pin. _admin_root() and the environment variable are gone.
  • The --config-sha256 pin now survives a symlink swap of the file or of its parent folder, and a symlink at pin time is refused.
  • The guard and governed-resume pins refuse when there's no config (approver_root_unpinned).

Blocking: with no Keychain pin, an agent-written deployment.json still reaches Flow

private_flow_config.py, prepare_flow_config: when approver_root is None, the approvers section of base_config passes through unchanged. In dispatch.py (around line 1493), base_config is <data_dir>/deployment.json when the operator picked no file. Same-user processes can write that file. The Keychain items can also be deleted by a same-user process, as FlowRoster.root()'s docstring says.

So the agent can:

  1. delete the Keychain root, and
  2. write deployment.json with its own root_public_key and key_dir.

Desktop then stages that section, hashes it and passes --config-sha256. Flow (config_only) trusts the agent's root. This is the same outcome as the _admin_root() fallback that this push removed, reached through a different path.

Probe output from prepare_flow_config(deployment.json, None, approver_root=None):

snapshot Flow gets: approvers: | root_public_key: bbbb… | key_dir: …/agent-keys |
HOLE

Fix: when Desktop has no Keychain pin, remove root_public_key and head from the snapshot's approvers section (or refuse a section that names a root). Flow then refuses every person's answer with approver_root_unpinned, which fails closed. On Windows and Linux, where no Keychain pin exists, document that the operator's deployment config is the anchor and must be kept away from agents. Alternatively, apply the same strip there and require an admin pin outside the user's write access.

Minor

  • flow_bridge.py:548-555 still sets OPENADAPT_APPROVER_ROOT in the Flow child's environment. Governed runs ignore it, so it does no harm, but it's misleading.
  • approver_key.py:221 still tells people to start Desktop with that variable.

With no Keychain pin, the config snapshot Desktop hands Flow drops any
approvers root or head from the base config (a file the agent's user can
write), so Flow refuses person answers. The Flow child environment never
carries OPENADAPT_APPROVER_ROOT. Setup hints no longer point at the env var.
Flow pins --config by its absolute path, so a later chdir can't miss the pin.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abrichr

abrichr commented Oct 25, 2026

Copy link
Copy Markdown
Member Author

AI re-review (coordinator), round 4

Re-reviewed at beb1d2f. Verdict: no blocking hole left in the approver trust root. This PR is report-only here (it ships Desktop 0.24.0, which goes out after 0.23.0), so I did not enable auto-merge.

Probe results on this head

  • No Keychain pin, root in an agent-written deployment.json. prepare_flow_config drops root_public_key and head, and the snapshot carries only key_dir. The strip runs after the target is merged in, so a root carried by the target is dropped too.
  • Relative --config plus a chdir. I ran main() with --config c.yaml --config-sha256 …, then changed directory and wrote a rogue c.yaml holding the agent's root.
    • args.config is now absolute.
    • A fresh _deployment_approvers load returns the pinned root.
  • Other root sources. I found none.
    • Approvers has no aliases.
    • _subprocess_env removes OPENADAPT_APPROVER_ROOT.
    • Governed runs ignore --approver-root.
    • Desktop passes --approver-root and --approver-head only on answers, and only when the Keychain holds a pin.

Non-blocking: a key_dir without a root refuses the whole run

Approvers.root_public_key is required. A snapshot that keeps approvers: {key_dir: …} therefore fails validation, and Flow refuses the entire run:

REFUSED (deployment_config_unreadable): … can't be read, so its approver pin can't be checked

That fails closed. But it also blocks not_gated runs, and the message points at an unreadable file rather than the missing approver setup. When there's no Keychain pin, drop the whole approvers section instead. Flow then runs, and every person's answer is refused with approver_root_unpinned, which names the real cause.

Checks: 13 pending, 2 skipping, 0 failing.

abrichr and others added 3 commits October 25, 2026 02:51
A section with key_dir but no root is unreadable to Flow, which refused
every run (deployment_config_unreadable), including runs no person answers.
Drop the whole section: Flow runs as normal and refuses only a person's
answer (approver_root_unpinned).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abrichr
abrichr enabled auto-merge October 25, 2026 12:20
@abrichr
abrichr merged commit 6934c56 into main Oct 25, 2026
47 checks passed
@abrichr
abrichr deleted the feat/stop-review-queue branch October 25, 2026 12:42
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