[abrichr]

Why

A same-user process (an agent) could make its own root the one Desktop pins, then sign answers to its own saves. A probe on a real Mac found three ways:

• /usr/bin/security delete-generic-password deletes Desktop's Keychain root and head items with no prompt.
• An item another program plants or rewrites in place makes Desktop's read prompt the person, and one "Allow" would pin the attacker's root.
• Released macOS builds are ad-hoc signed with the hardened runtime off (scripts/build_frozen_engine.py). A same-user process can run code inside Desktop's engine (DYLD_INSERT_LIBRARIES) and write a Keychain root Desktop reads with no prompt at all.

Data-protection Keychain access groups would need Developer ID plus a provisioning profile, which the shipped app doesn't have. So no Keychain design can anchor the root today; a file only an administrator can write can.

What changes

• Administrator's anchor (openadapt_desktop/approver_anchor.py):
  • Location: /Library/Application Support/OpenAdapt/approver-root.json (macOS), /etc/openadapt/approver-root.json (Linux), or OpenAdapt\approver-root.json under the ProgramData known folder (Windows; the folder comes from the known-folder API, never the environment variable).
  • It's trusted only when the file and every folder above it are owned by root (or Administrators, SYSTEM or TrustedInstaller), nobody else can write any of them, and nothing on the path is a symlink, reparse point or hard link.
  • The file is opened with O_NOFOLLOW and checked with fstat.
  • Anything else raises, and no person's answer counts.
• openadapt-desktop approver-pin ROOT writes the anchor. It refuses unless elevated.
  • POSIX: the folder is 0755 and the file 0644, written atomically.
  • Windows: icacls sets the folder's owner to Administrators and leaves Users read-only. It refuses a junction.
  • Every pin is read back.
  • --show prints the current pin.
• Which root Desktop pins (FlowRoster.root):
  • The anchor comes first.
  • Without one, a Mac uses the Keychain root it set up behind Touch ID. Desktop reads it with Keychain prompts turned off (SecKeychainSetUserInteractionAllowed), so a planted or rewritten item reads as no pin.
  • An anchor and a Keychain root that disagree are refused (approver_root_conflict), and with an anchor present the Mac doesn't start another root.
  • Off a Mac, the credential store never pins.
• Stops screen: shows when no root is pinned. The approver-key summary gains pinned and anchored.
• Docs: setup hints, APPROVAL_MODES.md and CAPABILITIES.md.
• Release: Desktop 0.25.0 with its CHANGELOG entry.
• Test fix: test_run_it_waits_for_a_person_and_saves_after_approval expected no config digest. It runs only where openadapt_flow is installed, which CI's environment doesn't have, so it never ran there.

Tests, one per attack

• File checks, on real files:
  • a file the user owns;
  • a symlinked anchor, and a symlinked folder;
  • a group- or other-writable file, and a writable folder;
  • a hard link;
  • bad JSON, and a key that isn't hex;
  • pinning without elevation.
• Real system folders (/usr/bin) pass the folder check.
• Windows ACL rules: owner, NULL DACL, a Users write ACE, WRITE_DAC, GENERIC_ALL, and inherit-only and deny entries. A real-ACL test on Windows CI uses hosts and a user-owned temp file.
• Which root wins:
  • the anchor alone;
  • a Keychain root that disagrees with the anchor;
  • an untrusted anchor;
  • a root planted in the credential store off a Mac;
  • a second root while an anchor exists;
  • a run config carrying the anchor's root.
• Real login Keychain (opt-in, OPENADAPT_REAL_KEYCHAIN_TESTS=1, passed locally): Desktop's own item reads back. An item rewritten in place by /usr/bin/security, or deleted and planted fresh with -A, reads as no pin, instantly and without a prompt.
• CLI: --show, and the refusal without elevation.

Local results: the Desktop Python suite passed 1469 tests. Of the seven failures, the one that came from this change (test_stop_queue) is fixed. The other six fail only in a fresh venv that has openadapt-types 0.15 and no openadapt-privacy; all six pass in a synced environment. Vitest Stops.test.tsx passes 8 tests, and tsc and ruff are clean.

Limits

An administrator, or a process with the administrator's password or a cached sudo, can still change the anchor. Only Developer ID signing with the hardened runtime makes the Keychain path a real anchor on its own.

🤖 Generated with Claude Code

[abrichr]

AI review (coordinator)

Reviewed at 96951a5. This PR is report-only here; I did not enable auto-merge. Verdict: not ready.

• The POSIX reader is sound for the default macOS and Linux layouts.
• The Windows path has a blocking hole: a folder pre-created in ProgramData.
• Desktop can't use any of this yet, because the Flow it pins is too old.

What holds (POSIX)

• Folder chain and file checks. Every folder is checked with lstat for owner root, no group/other write and no symlink. The file is opened with O_NOFOLLOW and checked with fstat for a regular file, root owner, no group/other write and st_nlink == 1.
• No pre-creation on macOS. On a stock Mac, a non-root user can't create /Library/Application Support/OpenAdapt: /Library/Application Support is drwxr-xr-x root admin with no ACL, and os.access(W_OK) is False for an admin-group user. _write_posix refuses, rather than adopts, a folder that a non-root user owns.
• No race between the checks and the open, given those modes. No user other than root can rename or replace anything in a chain that root owns with no group/other write. An fd-based walk (openat with O_DIRECTORY|O_NOFOLLOW on each part, then fstat) would still be better hardening; see the ACL gap below.
• Linux. POSIX ACLs that grant a named user write show up as w in the group mode bits (the mask), so the check catches them. Bind mounts need root. A user namespace only changes what processes started inside it see, which is the documented "starts its own run" limit.
• Keychain fallback. On a Mac with no anchor, a same-user process can still plant the Keychain pin. The new APPROVAL_MODES.md text documents this and says that only the anchor holds against it. That's acceptable.
• Off-Mac credential stores never pin.

Blocking (Windows): a folder pre-created in ProgramData stays under the agent's control after approver-pin

By default, a standard user may create folders in C:\ProgramData, and CREATOR OWNER gives them full control of what they create.

The attack:

1. Before an administrator pins, the agent creates ProgramData\OpenAdapt, so it owns the folder.
2. It then does one of two things:
   • (a) keeps a handle open on the folder with WRITE_DAC. Windows checks access only when a handle opens, so the handle keeps that right after the owner and ACL change.
   • (b) adds an explicit, inheritable ACCESS_ALLOWED_CALLBACK_ACE (type 9, a conditional ACE) granting itself write.

Why _write_windows doesn't stop either:

• It runs mkdir(exist_ok=True) on the existing folder, then icacls /setowner.
• /inheritance:r removes only inherited ACEs.
• /grant:r replaces grants only for the SIDs it names.

So in (b) the agent's explicit ACE survives, and the new approver-root.json inherits it. In (a), the agent can later add a write ACE through its handle, rewrite the anchor file (still owned by Administrators), and remove the ACE again, so the check passes afterwards.

dacl_admin_only checks only type-0 allow ACEs, so a type-9 ACE passes. Probe output:

type0 allow Users write: refused (S-1-5-32-545 can change it)
type0 allow inherited (0x10) Everyone write: refused (S-1-1-0 can change it)
type9 ACCESS_ALLOWED_CALLBACK (conditional) ME write: ACCEPTED
type5 ACCESS_ALLOWED_OBJECT ME write: ACCEPTED
type11 ACCESS_ALLOWED_CALLBACK_OBJECT ME write: ACCEPTED

(I couldn't run the end-to-end Windows steps here. The dacl_admin_only results above are from the real function.)

Fix:

1. In approver-pin, never adopt an existing folder that isn't already trusted.
   • Rename it aside, which takes it out from under any handle (a handle follows the object, not the name), or refuse.
   • Then create a fresh folder with a protected DACL in the same call: CreateDirectoryW with SECURITY_ATTRIBUTES built from an SDDL such as O:BAD:P(A;OICI;FA;;;BA)(A;OICI;FA;;;SY)(A;OICI;0x1200a9;;;BU).
   • If the rename fails because a handle without FILE_SHARE_DELETE blocks it, refuse; that fails closed.
2. In dacl_admin_only, refuse any allow-type ACE other than 0 (types 5, 9, 11 and the other callback or object types) unless its SID is trusted, and fail closed on ACE types you don't recognise.
3. For parity, check nNumberOfLinks == 1 on Windows (GetFileInformationByHandle).
4. _check_windows checks only the parent folder and the file, but the docs say "every folder above it". Either walk the whole chain or change the docs.

Should fix (macOS): ACLs are not checked

The macOS mode-bit checks ignore ACLs. Probe, with a folder and file that pass the owner and mode checks:

OpenAdapt:           0: group:everyone allow add_file,delete_child,writesecurity
approver-root.json:  0: group:everyone allow write,writesecurity
with write ACLs for everyone: ACCEPTED

Only root can add such an ACL to a folder root owns, and there is none by default. But write_anchor creates the folder and file as root, and on macOS they inherit any inheritable ACE from /Library/Application Support. Management software sometimes adds ACEs like that. A write ACL is also the one condition that would make a race between the path checks and the open exploitable.

Fix: on darwin, refuse when the file or any folder in the chain has an ACL entry that grants write, append, add_file, add_subdirectory, delete_child, delete, writeattr, writeextattr, writesecurity or chown to anyone other than root. The simplest version refuses any extended ACL at all, using acl_get_link_np through ctypes, or acl_get_fd_np on the descriptor O_NOFOLLOW returns. Also have write_anchor strip ACLs from what it creates (chmod -N).

Dependency: the bundled Flow can't receive the anchor's root yet

Desktop pins openadapt-flow[browser,console]==2.1.0 (and uv.lock resolves 2.1.0 from PyPI). Flow v2.1.0 (and v2.3.0) has no --config-sha256, no approvers deployment section, and no item-approval --approver-root, --approver-head or --approver-signer stdio. Its main() uses strict parse_args.

So with the pinned Flow:

• Every Desktop run that stages a config (dispatch.py adds --config-sha256 with no capability check) exits with "unrecognized arguments".
• The anchor's root never reaches Flow.

Those flags were added by #1519, which has merged, so Desktop 0.24.0 already behaves this way with the pinned Flow. This fails closed, but it means approval runs don't work.

Next step: release a Flow that includes #1486 and bump Desktop's pin before 0.24.0 or 0.25.0 ships. Alternatively, pass --config-sha256 only when run --help lists it, as Desktop already does for its other flags.

Checks: 13 pass, 15 pending, 18 skipping, 0 failing.

[abrichr]

AI re-review (coordinator), round 2

Re-reviewed at b01c639. Report only; I did not enable auto-merge. Verdict: not ready. The anchor checks have no hole left, but the module doesn't parse on Python 3.11 (see below), and the Flow dependency from round 1 still blocks the feature working end to end.

macOS (probes on this Mac)

• Round-1 write ACLs are now refused. With everyone allowed add_file,delete_child,writesecurity on the folder and write,writesecurity on the file, the read gives approver_anchor_untrusted.
• Inherited entries are parsed correctly. A file created in a folder with an inheritable write ACE shows group:everyone inherited allow write. acl_to_text gives …:12:allow,inherited:write, which is refused.
• Entries that grant no write pass.
  • An only_inherit write ACE on the folder passes. It doesn't apply to the folder itself; a file created later inherits it, which the file check then refuses, and write_anchor strips ACLs from what it creates.
  • Deny-only entries pass.
  • Read-only allow entries pass.
• No false refusal on the real path. The real /Library/Application Support chain passes the fd walk with root as the only trusted uid.
• The fd walk (openat with O_NOFOLLOW|O_DIRECTORY, checking each fd as it opens) closes the race between checking a path and opening it.

Windows (dacl_admin_only with real values; the end-to-end steps can't run here)

Case	Result
Allow ACEs of types 9, 5, 11 granting a user write	refused
Unknown ACE type	refused
Inherited OI-only allow (flags 0x11) granting Everyone write	refused (OI without IO applies to the folder)
Deny-write followed by allow-write for the same user	refused. Conservative: Windows would deny, so at worst this refuses something safe
MAXIMUM_ALLOWED for Users	refused
CREATOR OWNER entry that applies to the object (no IO)	refused
Default C:\ ACL (Authenticated Users AD, inherit-only M), owner TrustedInstaller, under the ancestor rule	passes
Default C:\ProgramData ACL (CREATOR OWNER (OI)(CI)(IO)F, Users (CI)(WD,AD,WEA,WA)), owner SYSTEM, under the ancestor rule	passes
ANCHOR_FOLDER_SDDL, and the file inheriting from it	passes

Pre-created folder.

• _write_windows now renames an existing OpenAdapt aside, so a handle the agent holds follows the old object. It refuses if a handle without FILE_SHARE_DELETE blocks the rename.
• It then creates the folder with the protected SDDL in a single CreateDirectoryW call.

Where the CREATOR OWNER default leaves the agent. It owns everything it creates in ProgramData, including the moved-aside folder, but Desktop never reads that folder. The agent can do two things, and both fail closed:

• recreate OpenAdapt between the rename and CreateDirectoryW, which makes CreateDirectoryW fail;
• pre-create the OpenAdapt.replaced-<timestamp> target, which makes the rename fail.

The new folder has a protected DACL and the owner Administrators. ProgramData gives Users no DELETE_CHILD, so they can't replace it.

Minor

• Weak test coverage on Windows. test_real_windows_acls covers hosts and a user-owned temp file. It doesn't cover ProgramData or write_anchor. GitHub's windows-latest runners run elevated, so an end-to-end test is possible: pre-create ProgramData\OpenAdapt as a folder with an extra explicit ACE, keep a handle open with FILE_SHARE_DELETE, run write_anchor, then check that read_anchor passes and that the handle can't change the new folder.
• The aside name is predictable. It has one-second resolution, so a pre-created target can stop approver-pin (fails closed). Adding a random suffix would remove that.

Still open (round 1): the bundled Flow

Desktop pins openadapt-flow==2.1.0. That Flow has no --config-sha256 and no approvers section, so the anchor's root can't reach Flow. Runs that stage a config also exit on the unknown flag. Ship a Flow release that includes #1486 and bump Desktop's pin first, or gate the flag on run --help.

Blocking (new): approver_anchor.py doesn't parse on Python 3.11

approver_anchor.py:648 puts double quotes inside a double-quoted f-string:

f"Can't create {directory} (error {getattr(ctypes, "get_last_error")()}); if something "

Python 3.12 accepts this (PEP 701), but Desktop declares requires-python = ">=3.11", and on 3.11 the whole module raises SyntaxError. Locally: python3.10 and python3.11 give SyntaxError: f-string: unmatched '(', and python3.12 parses it.

On 3.11, every import of approver_anchor therefore fails: FlowRoster.root(), pinned_root(), the approver status in dispatch, and approver-pin. The run path catches only ApproverKeychainError, so a run raises an uncaught SyntaxError there instead of refusing cleanly.

CI shows the same failure. In the dispatched Packages run 40203007572, "desktop test / Python engine tests (windows-latest, py3.11)" failed at ruff check with "Cannot reuse outer quote character in f-strings on Python 3.11". Because of that, pytest (and so the Windows anchor tests) never ran in that job. The py3.12 Windows job was still queued when I posted this.

Fix: ctypes.get_last_error(), or single quotes inside the braces. Then re-run the Windows jobs so that test_real_windows_acls actually runs.