diff --git a/CAPABILITIES.md b/CAPABILITIES.md index 513a7683f..4bd3f761d 100644 --- a/CAPABILITIES.md +++ b/CAPABILITIES.md @@ -140,6 +140,7 @@ improvising or retrying when something is off. | Check that the saved record matches its source document, not only the parameters: intake keeps each value's line, a second reading must agree before a guarded save, and a round trip compares the read-back with the document | On main | Synthetic (MockMed, text referral) | `openadapt_flow/provenance.py`, `intake`, `verify-run --provenance`, guarded save | | Guarded save: a safety layer any agent wraps around one write (check the record, hold for approval, read back) | On main ([#1258](https://github.com/OpenAdaptAI/OpenAdapt/pull/1258)) | Synthetic | `GUARDED_SAVE.md` | | A person's approval in Desktop is signed with a key kept behind Touch ID on a Mac: one-prompt setup in the app, a prompt for every answer, and Flow never sees the key | On main ([#1500](https://github.com/OpenAdaptAI/OpenAdapt/pull/1500)) | Unit tests; opt-in test on a real Secure Enclave | `packages/desktop/openadapt_desktop/approver_keychain.py`, [APPROVAL_MODES.md](packages/flow/docs/APPROVAL_MODES.md#the-approver-key) | +| The approver root Desktop trusts can be pinned by an administrator, in a file this user can't write (macOS, Linux, Windows); Desktop checks the owner, mode or ACL and refuses links. A Mac without one uses the root it set up behind Touch ID, read with Keychain prompts off; the two disagreeing is refused | On main | Unit tests per attack on real files; Windows ACL rules unit-tested, real-ACL test on Windows CI; opt-in test against the real login Keychain | `openadapt-desktop approver-pin`, `packages/desktop/openadapt_desktop/approver_anchor.py`, [APPROVAL_MODES.md](packages/flow/docs/APPROVAL_MODES.md#the-approver-key) | | Review stops: every stop as one card with the evidence side by side (the item next to each possible match, field by field), answered with one key or tap on the computer or a paired phone, signed with the approver key, with batches for similar stops. The queue presents and collects; a changed or expired card changes nothing | On main ([#1519](https://github.com/OpenAdaptAI/OpenAdapt/pull/1519)) | Unit tests; a keystroke-level estimate on 20 soak stops (26.7 s to 8.2 s to decide, a model, not a person) | `packages/desktop/openadapt_desktop/stop_queue.py`, [benchmark/stop_queue](packages/desktop/benchmark/stop_queue/README.md) | | Draft a record check from the demonstration's write steps (`scaffold-verifier`) | Released | Synthetic | `openadapt-flow scaffold-verifier` (a person fills in the draft) | | Build a working record check from the reads the app made while the person viewed the saved record, and test it against injected bad writes before anyone reviews it | On main ([#1284](https://github.com/OpenAdaptAI/OpenAdapt/pull/1284)) | Synthetic (MockMed, OpenEMR fixture) | `openadapt-flow scaffold-verifier` | diff --git a/llms-full.txt b/llms-full.txt index 490afde94..d83986b97 100644 --- a/llms-full.txt +++ b/llms-full.txt @@ -537,6 +537,7 @@ improvising or retrying when something is off. | Check that the saved record matches its source document, not only the parameters: intake keeps each value's line, a second reading must agree before a guarded save, and a round trip compares the read-back with the document | On main | Synthetic (MockMed, text referral) | `openadapt_flow/provenance.py`, `intake`, `verify-run --provenance`, guarded save | | Guarded save: a safety layer any agent wraps around one write (check the record, hold for approval, read back) | On main ([#1258](https://github.com/OpenAdaptAI/OpenAdapt/pull/1258)) | Synthetic | `GUARDED_SAVE.md` | | A person's approval in Desktop is signed with a key kept behind Touch ID on a Mac: one-prompt setup in the app, a prompt for every answer, and Flow never sees the key | On main ([#1500](https://github.com/OpenAdaptAI/OpenAdapt/pull/1500)) | Unit tests; opt-in test on a real Secure Enclave | `packages/desktop/openadapt_desktop/approver_keychain.py`, [APPROVAL_MODES.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/APPROVAL_MODES.md#the-approver-key) | +| The approver root Desktop trusts can be pinned by an administrator, in a file this user can't write (macOS, Linux, Windows); Desktop checks the owner, mode or ACL and refuses links. A Mac without one uses the root it set up behind Touch ID, read with Keychain prompts off; the two disagreeing is refused | On main | Unit tests per attack on real files; Windows ACL rules unit-tested, real-ACL test on Windows CI; opt-in test against the real login Keychain | `openadapt-desktop approver-pin`, `packages/desktop/openadapt_desktop/approver_anchor.py`, [APPROVAL_MODES.md](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/flow/docs/APPROVAL_MODES.md#the-approver-key) | | Review stops: every stop as one card with the evidence side by side (the item next to each possible match, field by field), answered with one key or tap on the computer or a paired phone, signed with the approver key, with batches for similar stops. The queue presents and collects; a changed or expired card changes nothing | On main ([#1519](https://github.com/OpenAdaptAI/OpenAdapt/pull/1519)) | Unit tests; a keystroke-level estimate on 20 soak stops (26.7 s to 8.2 s to decide, a model, not a person) | `packages/desktop/openadapt_desktop/stop_queue.py`, [benchmark/stop_queue](https://github.com/OpenAdaptAI/OpenAdapt/blob/main/packages/desktop/benchmark/stop_queue/README.md) | | Draft a record check from the demonstration's write steps (`scaffold-verifier`) | Released | Synthetic | `openadapt-flow scaffold-verifier` (a person fills in the draft) | | Build a working record check from the reads the app made while the person viewed the saved record, and test it against injected bad writes before anyone reviews it | On main ([#1284](https://github.com/OpenAdaptAI/OpenAdapt/pull/1284)) | Synthetic (MockMed, OpenEMR fixture) | `openadapt-flow scaffold-verifier` | diff --git a/packages/desktop/CHANGELOG.md b/packages/desktop/CHANGELOG.md index cc1414873..2cae2234b 100644 --- a/packages/desktop/CHANGELOG.md +++ b/packages/desktop/CHANGELOG.md @@ -1,6 +1,19 @@ # CHANGELOG +## v0.25.0 (2026-10-25) + +### Features + +- **desktop**: an administrator can pin the approver root for this computer with `openadapt-desktop approver-pin ROOT` (with sudo, or from an elevated prompt on Windows). The pin lives in a file only an administrator can write: `/Library/Application Support/OpenAdapt` (macOS), `/etc/openadapt` (Linux) or `ProgramData\OpenAdapt` (Windows). Desktop checks its owner, permissions and links before trusting it, and on Windows and Linux it is the only pin. + +### Bug Fixes + +- **desktop**: a Mac reads its Keychain approver root with Keychain prompts turned off, so an item another program planted or rewrote counts as no root instead of a prompt someone might allow. A Keychain root that disagrees with the administrator's pin is refused. +- **desktop**: the stop-review screen says when no approver root is pinned, since answers can't count until one is. + + + ## v0.24.0 (2026-10-24) ### Features diff --git a/packages/desktop/SECURITY.md b/packages/desktop/SECURITY.md index 15a44dff0..7d30f3cfe 100644 --- a/packages/desktop/SECURITY.md +++ b/packages/desktop/SECURITY.md @@ -28,3 +28,15 @@ and any suggested remediation when available. Credential exposure, keychain-boundary failures, unsafe protocol handling, sidecar command or path injection, installer provenance/signing mismatches, and release supply-chain findings are explicitly in scope. + +## Known limits + +- **The approver root on macOS.** macOS builds are ad-hoc signed without the + hardened runtime. A process running as the same user can run code inside + Desktop's engine, so the login Keychain doesn't keep the approver root + safe from it: such a process could replace the root and approve its own + saves. On a computer where agents run as the person's account, have an + administrator pin the root with `sudo openadapt-desktop approver-pin ROOT`. + Desktop then requires that root, and refuses a Keychain root that differs. + Windows and Linux use only the administrator's pin. See + [APPROVAL_MODES.md](../flow/docs/APPROVAL_MODES.md#the-approver-key). diff --git a/packages/desktop/openadapt_desktop/__init__.py b/packages/desktop/openadapt_desktop/__init__.py index f8eab681b..3699b9c58 100644 --- a/packages/desktop/openadapt_desktop/__init__.py +++ b/packages/desktop/openadapt_desktop/__init__.py @@ -21,4 +21,4 @@ +-- backends/ Storage backend plugins """ -__version__ = "0.24.0" +__version__ = "0.25.0" diff --git a/packages/desktop/openadapt_desktop/approver_anchor.py b/packages/desktop/openadapt_desktop/approver_anchor.py new file mode 100644 index 000000000..df3bc43fa --- /dev/null +++ b/packages/desktop/openadapt_desktop/approver_anchor.py @@ -0,0 +1,696 @@ +"""The approver root an administrator pins, in a file this user can't write. + +A person's answer counts only when it is signed by a key on Flow's signed +approver roster, and the roster is only as good as the root Desktop pins. A +pin this user's account can write (a Keychain item, a file under the home +folder, an environment variable) is one an agent running as this user can +replace with its own root, and then approve its own saves. This module reads a +pin that needs an administrator to write: + +* macOS: ``/Library/Application Support/OpenAdapt/approver-root.json`` +* Linux: ``/etc/openadapt/approver-root.json`` +* Windows: ``%ProgramData%\\OpenAdapt\\approver-root.json`` (the known + ProgramData folder, never the environment variable) + +:func:`read_anchor` trusts the file only when no one but an administrator +can change it or what leads to it: + +* POSIX: opened one folder at a time from ``/`` without following links, + every folder and the file owned by root and writable by no one else (a + sticky folder such as /tmp may sit above), no extended ACL granting a + write (macOS), and the file has a single link. +* Windows: the file and its folder owned by Administrators, SYSTEM or + TrustedInstaller, with no access entry of any kind letting anyone else + write (an entry of a type the check doesn't know refuses); every folder + above lets no one else replace what's inside; no links or junctions; a + single hard link. + +Anything else raises :class:`AnchorError`: Desktop then counts no person's +answer. :func:`write_anchor` is the elevated ``openadapt-desktop +approver-pin`` command. + +An administrator (or a process with a cached ``sudo``) can still change it; +that is the boundary this file draws, not more. +""" + +from __future__ import annotations + +import json +import os +import stat +import sys +from dataclasses import dataclass +from pathlib import Path +from typing import Iterable, Literal + +ANCHOR_NAME = "approver-root.json" +#: The largest anchor file read; a root pin is about 100 bytes. +_MAX_BYTES = 4096 + +Code = Literal["approver_anchor_untrusted", "approver_anchor_unreadable", "not_elevated"] + + +class AnchorError(RuntimeError): + """The anchor exists but can't be trusted; ``code`` says why.""" + + def __init__(self, code: Code, message: str) -> None: + super().__init__(message) + self.code: Code = code + + +def anchor_dir(platform: str | None = None) -> Path: + """The administrator-owned folder that holds the anchor on ``platform``.""" + + platform = platform or sys.platform + if platform == "darwin": + return Path("/Library/Application Support/OpenAdapt") + if platform == "win32": + return _program_data() / "OpenAdapt" + return Path("/etc/openadapt") + + +def anchor_path(platform: str | None = None) -> Path: + return anchor_dir(platform) / ANCHOR_NAME + + +def normalize_root(value: object) -> str: + """A 32-byte Ed25519 public key as 64 lowercase hex digits, or raise.""" + + text = str(value or "").strip().lower() + if len(text) != 64 or any(c not in "0123456789abcdef" for c in text): + raise AnchorError( + "approver_anchor_unreadable", + "The administrator's approver pin isn't a 64-digit hex public key.", + ) + return text + + +def read_anchor(path: Path | None = None, *, trusted_uids: Iterable[int] = (0,)) -> str | None: + """The administrator's pinned root, or ``None`` when no anchor exists. + + Raises :class:`AnchorError` when an anchor (or its folder) exists but + this user could have written it, or it doesn't parse. ``trusted_uids`` + is for tests: real callers trust only root. + """ + + return _read_anchor(Path(os.path.abspath(path or anchor_path())), frozenset(trusted_uids)) + + +def _read_anchor(path: Path, trusted_uids: frozenset[int]) -> str | None: + if not os.path.lexists(path): + return None + if sys.platform == "win32": + data = _read_windows(path) + else: + data = _read_posix(path, trusted_uids) + try: + parsed = json.loads(data.decode("utf-8")) + except (UnicodeDecodeError, ValueError): + raise AnchorError( + "approver_anchor_unreadable", f"{path} isn't the approver pin's JSON." + ) from None + if not isinstance(parsed, dict): + raise AnchorError("approver_anchor_unreadable", f"{path} isn't the approver pin's JSON.") + return normalize_root(parsed.get("root_public_key")) + + +def write_anchor( + root_public_key: str, + path: Path | None = None, + *, + trusted_uids: Iterable[int] = (0,), +) -> Path: + """Pin ``root_public_key`` for this computer. Needs an administrator. + + POSIX: run as root; the folder is created 0755 and the file 0644, both + owned by root, without extended ACLs, written atomically without + following links. Windows: run elevated; the folder is always created + fresh with :data:`ANCHOR_FOLDER_SDDL` (one already there is moved aside, + never adopted). The result is read back through :func:`read_anchor`. + """ + + root = normalize_root(root_public_key) + path = Path(os.path.abspath(path or anchor_path())) + payload = (json.dumps({"root_public_key": root}, indent=2) + "\n").encode("utf-8") + uids = frozenset(trusted_uids) + if sys.platform == "win32": + _require_windows_admin() + _write_windows(path, payload) + else: + if os.geteuid() not in uids: + raise AnchorError( + "not_elevated", + "Pinning the approver root needs an administrator: run it with sudo.", + ) + _write_posix(path, payload, uids) + pinned = _read_anchor(path, uids) + if pinned != root: + raise AnchorError("approver_anchor_untrusted", f"{path} didn't read back as {root}.") + return path + + +# -- POSIX ------------------------------------------------------------------------ + +#: macOS extended-ACL permissions (``acl_to_text`` names) that let someone +#: change a file or folder, or what it contains. +_MAC_WRITE_PERMS = frozenset( + { + "write", + "add_file", + "append", + "add_subdirectory", + "delete", + "delete_child", + "writeattr", + "writeextattr", + "writesecurity", + "chown", + } +) +_ACL_TYPE_EXTENDED = 0x00000100 + + +def mac_acl_write_grants(acl_text: str) -> list[str]: + """The entries of a macOS ``acl_to_text`` ACL that allow a write. + + Each entry is ``tag:uuid:name:id:KIND[,flag...]:perm[,perm...]``, where + KIND is ``allow`` or ``deny`` and the flags include ``inherited`` and + ``only_inherit`` (an entry that doesn't apply to the object itself). + """ + + grants = [] + for line in acl_text.splitlines(): + line = line.strip() + fields = line.split(":") + if not line or line.startswith("!#") or len(fields) < 2: + continue + kind = set(fields[-2].split(",")) + perms = set(fields[-1].split(",")) + if "allow" not in kind or "only_inherit" in kind | perms: + continue + if perms & _MAC_WRITE_PERMS: + grants.append(line) + return grants + + +def _mac_acl_text(fd: int) -> str: + """The extended ACL on an open file or folder (``""`` when it has none).""" + + if sys.platform != "darwin": + return "" + import ctypes + import errno as errno_codes + + libc = ctypes.CDLL("/usr/lib/libSystem.B.dylib", use_errno=True) + libc.acl_get_fd_np.restype = ctypes.c_void_p + libc.acl_get_fd_np.argtypes = [ctypes.c_int, ctypes.c_int] + libc.acl_to_text.restype = ctypes.c_void_p + libc.acl_to_text.argtypes = [ctypes.c_void_p, ctypes.c_void_p] + libc.acl_free.argtypes = [ctypes.c_void_p] + acl = libc.acl_get_fd_np(fd, _ACL_TYPE_EXTENDED) + if not acl: + if ctypes.get_errno() == errno_codes.ENOENT: + return "" + raise AnchorError("approver_anchor_unreadable", "Can't read a folder's access list.") + try: + text = libc.acl_to_text(acl, None) + if not text: + raise AnchorError("approver_anchor_unreadable", "Can't read a folder's access list.") + try: + return ctypes.string_at(text).decode("utf-8", "replace") + finally: + libc.acl_free(text) + finally: + libc.acl_free(acl) + + +def _check_fd( + fd: int, where: Path, trusted_uids: frozenset[int], *, folder: bool, sticky_ok: bool +) -> None: + info = os.fstat(fd) + if folder and not stat.S_ISDIR(info.st_mode): + raise AnchorError("approver_anchor_untrusted", f"{where} isn't a folder.") + if not folder: + if not stat.S_ISREG(info.st_mode): + raise AnchorError("approver_anchor_untrusted", f"{where} isn't a regular file.") + if info.st_nlink != 1: + raise AnchorError( + "approver_anchor_untrusted", + f"{where} has other hard links; the pin isn't trusted.", + ) + if info.st_uid not in trusted_uids: + raise AnchorError( + "approver_anchor_untrusted", + f"{where} isn't owned by the administrator; the pin isn't trusted.", + ) + if info.st_mode & (stat.S_IWGRP | stat.S_IWOTH) and not ( + sticky_ok and info.st_mode & stat.S_ISVTX + ): + raise AnchorError( + "approver_anchor_untrusted", f"Others can write {where}; the pin isn't trusted." + ) + grants = mac_acl_write_grants(_mac_acl_text(fd)) + if grants: + raise AnchorError( + "approver_anchor_untrusted", + f"An access list lets others change {where} ({grants[0]}); the pin isn't trusted.", + ) + + +def _open_folder(directory: Path, trusted_uids: frozenset[int]) -> int: + """Open ``directory`` one component at a time from ``/``, never following a + link, checking each folder as it is opened (so a folder swapped after a + check is never the one used). Folders above it may be sticky and + world-writable, as /tmp is: other users can't replace root's entries.""" + + flags = os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) + fd = os.open(directory.anchor, flags) + where = Path(directory.anchor) + parts = directory.parts[1:] + try: + _check_fd(fd, where, trusted_uids, folder=True, sticky_ok=bool(parts)) + for index, part in enumerate(parts): + where = where / part + try: + child = os.open(part, flags | getattr(os, "O_NOFOLLOW", 0), dir_fd=fd) + except OSError as exc: + raise AnchorError( + "approver_anchor_untrusted", + f"Can't open {where} as a folder without following links: {exc}", + ) from None + os.close(fd) + fd = child + _check_fd(fd, where, trusted_uids, folder=True, sticky_ok=index < len(parts) - 1) + except BaseException: + os.close(fd) + raise + return fd + + +def _read_posix(path: Path, trusted_uids: frozenset[int]) -> bytes: + folder = _open_folder(path.parent, trusted_uids) + try: + flags = os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0) | getattr(os, "O_NOCTTY", 0) + try: + fd = os.open(path.name, flags, dir_fd=folder) + except OSError as exc: + raise AnchorError( + "approver_anchor_untrusted", f"Can't open {path} without following links: {exc}" + ) from None + finally: + os.close(folder) + try: + _check_fd(fd, path, trusted_uids, folder=False, sticky_ok=False) + data = os.read(fd, _MAX_BYTES + 1) + finally: + os.close(fd) + if len(data) > _MAX_BYTES: + raise AnchorError("approver_anchor_unreadable", f"{path} is too large to be the pin.") + return data + + +def _strip_mac_acl(path: Path) -> None: + """Remove any extended ACL (``chmod -N``), such as one inherited from + ``/Library/Application Support``.""" + + if sys.platform == "darwin": + import subprocess + + subprocess.run(["/bin/chmod", "-h", "-N", str(path)], check=True, capture_output=True) + + +def _write_posix(path: Path, payload: bytes, trusted_uids: frozenset[int]) -> None: + directory = path.parent + missing: list[Path] = [] + probe = directory + while not os.path.lexists(probe): + missing.append(probe) + probe = probe.parent + os.close(_open_folder(probe, trusted_uids)) + for folder in reversed(missing): + os.mkdir(folder, 0o755) + os.chmod(folder, 0o755) + _strip_mac_acl(folder) + _strip_mac_acl(directory) + folder_fd = _open_folder(directory, trusted_uids) + try: + temp = f".{ANCHOR_NAME}.{os.getpid()}.tmp" + flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_NOFOLLOW", 0) + fd = os.open(temp, flags, 0o644, dir_fd=folder_fd) + try: + os.fchmod(fd, 0o644) + os.write(fd, payload) + os.fsync(fd) + finally: + os.close(fd) + _strip_mac_acl(directory / temp) + os.replace(temp, path.name, src_dir_fd=folder_fd, dst_dir_fd=folder_fd) + os.fsync(folder_fd) + finally: + os.close(folder_fd) + + +# -- Windows ---------------------------------------------------------------------- + +#: Owners and writers an administrator-only pin may have: Administrators, +#: SYSTEM, TrustedInstaller. +TRUSTED_SIDS = frozenset( + { + "S-1-5-32-544", + "S-1-5-18", + "S-1-5-80-956008885-3418522649-1831038044-1853292631-2271478464", + } +) +#: DACL entry types (winnt.h). Allow entries of every kind are checked; an +#: entry of a type not listed here refuses the pin rather than be skipped. +_ALLOW_ACE_TYPES = frozenset({0, 5, 9, 11}) # allowed, object, callback, callback object +_DENY_ACE_TYPES = frozenset({1, 6, 10, 12}) +_OBJECT_ACE_TYPES = frozenset({5, 6, 11, 12}) +_INHERIT_ONLY_ACE = 0x08 +#: Rights that change a file or folder, or what it contains: write/append +#: data (add file/subfolder), write EA/attributes, delete child, DELETE, +#: WRITE_DAC, WRITE_OWNER, GENERIC_ALL, GENERIC_WRITE, MAXIMUM_ALLOWED. +_WRITE_RIGHTS = ( + 0x0002 + | 0x0004 + | 0x0010 + | 0x0040 + | 0x0100 + | 0x00010000 + | 0x00040000 + | 0x00080000 + | 0x02000000 + | 0x10000000 + | 0x40000000 +) +#: Rights over a folder above the anchor's that would let someone replace +#: what's inside (adding new entries alone can't): delete child, DELETE, +#: WRITE_DAC, WRITE_OWNER, GENERIC_ALL, MAXIMUM_ALLOWED. +_REPLACE_RIGHTS = 0x0040 | 0x00010000 | 0x00040000 | 0x00080000 | 0x02000000 | 0x10000000 +_FILE_ATTRIBUTE_REPARSE_POINT = 0x400 +#: The anchor folder's security: owned by Administrators, a protected DACL +#: (nothing inherited) with full control for Administrators and SYSTEM and +#: read-and-execute for Users, inherited by what is created inside. +ANCHOR_FOLDER_SDDL = "O:BAD:P(A;OICI;FA;;;BA)(A;OICI;FA;;;SY)(A;OICI;0x1200a9;;;BU)" + + +@dataclass(frozen=True) +class Ace: + """One entry of a DACL, as :func:`dacl_admin_only` needs it.""" + + ace_type: int + flags: int + mask: int + sid: str + + +def dacl_admin_only( + owner_sid: str | None, aces: list[Ace] | None, *, rights: int = _WRITE_RIGHTS +) -> str | None: + """Why this security descriptor lets someone other than an administrator + change the object, or ``None`` when only administrators can. + + ``aces is None`` is a NULL DACL: everyone has full control. ``rights`` + is what counts as a change (all writes for the anchor and its folder; + only replacing entries for the folders above). + """ + + if owner_sid not in TRUSTED_SIDS: + return f"it is owned by {owner_sid}, not an administrator" + if aces is None: + return "it has no access control list, so everyone can change it" + for ace in aces: + if ace.ace_type in _DENY_ACE_TYPES: + continue + if ace.ace_type not in _ALLOW_ACE_TYPES: + return f"it has an access entry of a type this check doesn't know ({ace.ace_type})" + if ace.flags & _INHERIT_ONLY_ACE: + continue + if ace.mask & rights and ace.sid not in TRUSTED_SIDS: + return f"{ace.sid} can change it" + return None + + +def _check_windows(path: Path) -> None: + """The file and its folder: only administrators can change them. Every + folder above: only administrators can replace what's inside (ProgramData + lets users add folders of their own, which is fine). No links anywhere.""" + + folder = path.parent + targets = [(ancestor, _REPLACE_RIGHTS) for ancestor in reversed(folder.parents)] + targets += [(folder, _WRITE_RIGHTS), (path, _WRITE_RIGHTS)] + for target, rights in targets: + if not os.path.lexists(target): + continue + info = os.lstat(target) + if getattr(info, "st_file_attributes", 0) & _FILE_ATTRIBUTE_REPARSE_POINT: + raise AnchorError( + "approver_anchor_untrusted", f"{target} is a link; the pin isn't trusted." + ) + if target == path and os.stat(target).st_nlink != 1: + raise AnchorError( + "approver_anchor_untrusted", + f"{target} has other hard links; the pin isn't trusted.", + ) + owner, aces = _windows_security(target) + reason = dacl_admin_only(owner, aces, rights=rights) + if reason: + raise AnchorError("approver_anchor_untrusted", f"{target} isn't trusted: {reason}.") + + +def _read_windows(path: Path) -> bytes: # pragma: no cover - Windows only + _check_windows(path) + with open(path, "rb") as handle: + data = handle.read(_MAX_BYTES + 1) + if len(data) > _MAX_BYTES: + raise AnchorError("approver_anchor_unreadable", f"{path} is too large to be the pin.") + # Checked again after the read: a swap between check and read is refused. + _check_windows(path) + return data + + +def _windows_security(target: Path) -> tuple[str | None, list[Ace] | None]: # pragma: no cover + import ctypes + from ctypes import wintypes + + advapi = _windll("advapi32", use_last_error=True) + kernel = _windll("kernel32", use_last_error=True) + owner = ctypes.c_void_p() + dacl = ctypes.c_void_p() + descriptor = ctypes.c_void_p() + owner_info, dacl_info = 0x1, 0x4 + status = advapi.GetNamedSecurityInfoW( + wintypes.LPCWSTR(str(target)), + 1, # SE_FILE_OBJECT + owner_info | dacl_info, + ctypes.byref(owner), + None, + ctypes.byref(dacl), + None, + ctypes.byref(descriptor), + ) + if status != 0: + raise AnchorError( + "approver_anchor_unreadable", f"Can't read the security of {target} ({status})." + ) + try: + owner_sid = _sid_string(advapi, kernel, owner) + if not dacl.value: + return owner_sid, None + + class AclSizeInformation(ctypes.Structure): + _fields_ = [ + ("AceCount", wintypes.DWORD), + ("AclBytesInUse", wintypes.DWORD), + ("AclBytesFree", wintypes.DWORD), + ] + + size = AclSizeInformation() + if not advapi.GetAclInformation(dacl, ctypes.byref(size), ctypes.sizeof(size), 2): + raise AnchorError("approver_anchor_unreadable", f"Can't read the ACL of {target}.") + aces: list[Ace] = [] + for index in range(size.AceCount): + ace = ctypes.c_void_p() + if not advapi.GetAce(dacl, index, ctypes.byref(ace)): + raise AnchorError("approver_anchor_unreadable", f"Can't read the ACL of {target}.") + address = int(ace.value or 0) + if not address: + raise AnchorError("approver_anchor_unreadable", f"Can't read the ACL of {target}.") + header = (ctypes.c_ubyte * 4).from_address(address) + ace_type, flags = header[0], header[1] + mask = wintypes.DWORD.from_address(address + 4).value + if ace_type in _OBJECT_ACE_TYPES: + # Mask, then object flags and up to two GUIDs before the SID. + object_flags = wintypes.DWORD.from_address(address + 8).value + offset = 12 + (16 if object_flags & 0x1 else 0) + (16 if object_flags & 0x2 else 0) + elif ace_type in _ALLOW_ACE_TYPES | _DENY_ACE_TYPES: + offset = 8 + else: + # Unknown: dacl_admin_only refuses it without reading a SID. + aces.append(Ace(ace_type, flags, mask, "")) + continue + sid = ctypes.c_void_p(address + offset) + aces.append(Ace(ace_type, flags, mask, _sid_string(advapi, kernel, sid))) + return owner_sid, aces + finally: + kernel.LocalFree(descriptor) + + +def _windll(name: str, **kwargs: object): # pragma: no cover - Windows only + import ctypes + + return getattr(ctypes, "WinDLL")(name, **kwargs) + + +def _sid_string(advapi, kernel, sid) -> str: # pragma: no cover - Windows only + import ctypes + from ctypes import wintypes + + text = wintypes.LPWSTR() + if not advapi.ConvertSidToStringSidW(sid, ctypes.byref(text)): + raise AnchorError("approver_anchor_unreadable", "Can't read a security identifier.") + try: + return str(text.value) + finally: + kernel.LocalFree(text) + + +def _program_data() -> Path: # pragma: no cover - Windows only + """The ProgramData known folder; never ``%ProgramData%``, which whoever + starts the process sets.""" + + import ctypes + from ctypes import wintypes + + class Guid(ctypes.Structure): + _fields_ = [ + ("Data1", wintypes.DWORD), + ("Data2", wintypes.WORD), + ("Data3", wintypes.WORD), + ("Data4", ctypes.c_ubyte * 8), + ] + + folder = Guid( + 0x62AB5D82, + 0xFDC1, + 0x4DC3, + (ctypes.c_ubyte * 8)(0xA9, 0xDD, 0x07, 0x0D, 0x1D, 0x49, 0x5D, 0x97), + ) + shell = _windll("shell32") + ole = _windll("ole32") + result = ctypes.c_wchar_p() + if shell.SHGetKnownFolderPath(ctypes.byref(folder), 0, None, ctypes.byref(result)) != 0: + raise AnchorError("approver_anchor_unreadable", "Can't find the ProgramData folder.") + try: + return Path(str(result.value)) + finally: + ole.CoTaskMemFree(result) + + +def _require_windows_admin() -> None: # pragma: no cover - Windows only + + if not _windll("shell32").IsUserAnAdmin(): + raise AnchorError( + "not_elevated", + "Pinning the approver root needs an administrator: run it from an " + "elevated (Run as administrator) prompt.", + ) + + +def _write_windows(path: Path, payload: bytes) -> None: # pragma: no cover - Windows only + """Create the anchor folder fresh with :data:`ANCHOR_FOLDER_SDDL`. + + A folder already there is never adopted: a user who made it first could + keep a handle with WRITE_DAC open, or an entry this check skips, through + any owner or ACL change. It is renamed aside (refused if something holds + it open), and the new folder is created with its final security in one + call. + """ + + import ctypes + import secrets + from ctypes import wintypes + + directory = path.parent + for ancestor in reversed(directory.parents): + _check_windows_ancestor(ancestor) + if os.path.lexists(directory): + # Random, so no one can make the name first and block the move. + aside = directory.with_name(f"{directory.name}.replaced-{secrets.token_hex(8)}") + try: + os.rename(directory, aside) + except OSError as exc: + raise AnchorError( + "approver_anchor_untrusted", + f"{directory} already exists and can't be moved aside ({exc}); a program may " + "hold it open. Close it, or remove the folder, and pin again.", + ) from None + print(f"Moved the existing {directory} aside to {aside}.") + + class SecurityAttributes(ctypes.Structure): + _fields_ = [ + ("nLength", wintypes.DWORD), + ("lpSecurityDescriptor", ctypes.c_void_p), + ("bInheritHandle", wintypes.BOOL), + ] + + advapi = _windll("advapi32", use_last_error=True) + kernel = _windll("kernel32", use_last_error=True) + descriptor = ctypes.c_void_p() + if not advapi.ConvertStringSecurityDescriptorToSecurityDescriptorW( + wintypes.LPCWSTR(ANCHOR_FOLDER_SDDL), 1, ctypes.byref(descriptor), None + ): + raise AnchorError("approver_anchor_unreadable", "Can't build the folder's security.") + try: + attributes = SecurityAttributes(ctypes.sizeof(SecurityAttributes), descriptor, False) + if not kernel.CreateDirectoryW(wintypes.LPCWSTR(str(directory)), ctypes.byref(attributes)): + error = ctypes.get_last_error() # type: ignore[attr-defined] + raise AnchorError( + "approver_anchor_untrusted", + f"Can't create {directory} (error {error}); if something made it again " + "meanwhile, pin again.", + ) + finally: + kernel.LocalFree(descriptor) + _check_windows(path) + temp = directory / f".{ANCHOR_NAME}.{os.getpid()}.tmp" + with open(temp, "xb") as handle: + handle.write(payload) + handle.flush() + os.fsync(handle.fileno()) + os.replace(temp, path) + + +def _check_windows_ancestor(target: Path) -> None: # pragma: no cover - Windows only + info = os.lstat(target) + if getattr(info, "st_file_attributes", 0) & _FILE_ATTRIBUTE_REPARSE_POINT: + raise AnchorError("approver_anchor_untrusted", f"{target} is a link; nothing was pinned.") + owner, aces = _windows_security(target) + reason = dacl_admin_only(owner, aces, rights=_REPLACE_RIGHTS) + if reason: + raise AnchorError("approver_anchor_untrusted", f"{target} isn't trusted: {reason}.") + + +def _known_windows_dir() -> Path | None: # pragma: no cover - Windows only + import ctypes + + buffer = ctypes.create_unicode_buffer(260) + if _windll("kernel32").GetSystemWindowsDirectoryW(buffer, 260): + return Path(buffer.value) + return None + + +__all__ = [ + "ANCHOR_FOLDER_SDDL", + "ANCHOR_NAME", + "Ace", + "AnchorError", + "TRUSTED_SIDS", + "anchor_dir", + "anchor_path", + "dacl_admin_only", + "mac_acl_write_grants", + "normalize_root", + "read_anchor", + "write_anchor", +] diff --git a/packages/desktop/openadapt_desktop/approver_key.py b/packages/desktop/openadapt_desktop/approver_key.py index d2b168fda..e207dc8e2 100644 --- a/packages/desktop/openadapt_desktop/approver_key.py +++ b/packages/desktop/openadapt_desktop/approver_key.py @@ -5,7 +5,9 @@ by a key on Flow's signed approver roster, and the run pins that roster's root (the deployment config's ``approvers.root_public_key``, or ``--approver-root``; see Flow's ``approval_mode/approver_roster``). Desktop -pins only the root kept in this Mac's Keychain. +pins only the root an administrator pinned +(:mod:`openadapt_desktop.approver_anchor`) or, on a Mac, the one kept in its +Keychain. Desktop keeps one such key for each route (``desktop`` for Run it, ``portal`` for the phone's answers, which the Flow console it starts signs). For each: @@ -219,13 +221,14 @@ def setup_instructions(route: Route = "desktop") -> str: f"{PASSPHRASE_ENV} set, run `openadapt-flow jit keygen --out {path}`, " "then start the approver roster with it: " f"`openadapt-flow jit approver init --principal {name} --key {path}`. " - "Pin the public key keygen printed as approvers.root_public_key in " - "the deployment config you pass to `openadapt-flow run --config`, " - f"with the same {PASSPHRASE_ENV}; or run " + "Then an administrator pins the public key keygen printed for " + "Desktop on this computer: `sudo openadapt-desktop approver-pin " + "` (on Windows, from an elevated prompt, without sudo). " + f"Start Desktop with the same {PASSPHRASE_ENV}; or run " "`openadapt-desktop approver-key setup` instead of keygen to keep the " "passphrase in this computer's keychain. Desktop counts a person's " - "answer only with a root pinned in this Mac's Keychain, which " - "OpenAdapt sets up behind Touch ID." + "answer only with a root an administrator pinned, or on a Mac, the " + "one OpenAdapt set up behind Touch ID." ) diff --git a/packages/desktop/openadapt_desktop/approver_keychain.py b/packages/desktop/openadapt_desktop/approver_keychain.py index d0287ce3b..9767ff0ea 100644 --- a/packages/desktop/openadapt_desktop/approver_keychain.py +++ b/packages/desktop/openadapt_desktop/approver_keychain.py @@ -75,6 +75,9 @@ "approver_key_unusable", "approver_keychain_unsupported", "approver_payload_refused", + "approver_root_conflict", + "approver_anchor_untrusted", + "approver_needs_adding", ] @@ -200,20 +203,53 @@ class FlowRoster: Mac recorded after its own last change. """ - def root(self) -> str | None: - """The pin in the login Keychain, or ``None``. + #: Only a Mac's login Keychain counts as a pin; elsewhere (Windows + #: Credential Manager, Secret Service) any process of this user writes + #: it freely, so only the administrator's anchor counts. + keychain_pins: bool = sys.platform == "darwin" - Nothing else counts: not the environment (whoever starts a process - sets it) and not a file this user can write. A same-user process can - delete the Keychain items; then there is no pin and no person's - answer counts until someone sets the key up again behind Touch ID. + def root(self) -> str | None: + """The pinned approver root, or ``None``. + + The administrator's anchor (:mod:`openadapt_desktop.approver_anchor`, + a file this user can't write) comes first. Without one, a Mac uses + the root it set up itself behind Touch ID, kept in the login + Keychain and read without letting the Keychain ask (so an item + another program planted or rewrote reads as no pin, never as a + prompt someone might allow). Both present and different: refused. + Nothing else counts: not the environment, not a file this user can + write. """ - stored = _store._kr_get(_store._keyring(), ROOT_ACCOUNT) - return stored.strip().lower() if stored else None + try: + from openadapt_desktop import approver_anchor + + anchor = approver_anchor.read_anchor() + except Exception as exc: + # An anchor that can't be loaded or read is never "no anchor": + # no person's answer counts until it reads. + raise ApproverKeychainError( + "approver_anchor_untrusted", + f"The administrator's approver pin can't be checked: {exc}", + ) from exc + stored = self._read_pin(ROOT_ACCOUNT) + keychain = stored.strip().lower() if stored else None + if anchor and keychain and anchor != keychain: + raise ApproverKeychainError( + "approver_root_conflict", + "The administrator pinned a different approver root than this " + "Mac's Keychain. No person's answer counts until they agree.", + ) + return anchor or keychain + + def _read_pin(self, account: str) -> str | None: + if not self.keychain_pins: + return None + with _no_keychain_ui(): + return _store._kr_get(_store._keyring(), account) def _head(self) -> tuple[int, str] | None: - raw = _store._kr_get(_store._keyring(), HEAD_ACCOUNT) + raw = self._read_pin(HEAD_ACCOUNT) try: seq, digest = json.loads(raw) if raw else (None, None) except (ValueError, TypeError): @@ -294,12 +330,57 @@ def remove(self, principal: str, public_key: str, signed_by: str, sign: Sign) -> self._record_head() +_UI_LOCK = threading.Lock() + + +class _no_keychain_ui: + """Turn off Keychain prompts for this process while a pin is read. + + An item this app didn't write (a program deleted Desktop's and planted + its own, or rewrote it) then fails to read instead of asking the + person, who might click Allow. If prompts can't be turned off, the pin + reads as missing. + """ + + def __enter__(self) -> None: + _UI_LOCK.acquire() + self._previous: bool | None = None + try: + import ctypes + + sec = ctypes.CDLL("/System/Library/Frameworks/Security.framework/Security") + allowed = ctypes.c_ubyte() + sec.SecKeychainGetUserInteractionAllowed.argtypes = [ctypes.POINTER(ctypes.c_ubyte)] + sec.SecKeychainSetUserInteractionAllowed.argtypes = [ctypes.c_ubyte] + if sec.SecKeychainGetUserInteractionAllowed(ctypes.byref(allowed)) != 0: + raise OSError("can't read the Keychain prompt setting") + if sec.SecKeychainSetUserInteractionAllowed(0) != 0: + raise OSError("can't turn off Keychain prompts") + self._sec = sec + self._previous = bool(allowed.value) + except Exception as exc: + _UI_LOCK.release() + raise ApproverKeychainError( + "approver_keychain_unsupported", + f"Desktop can't read its approver pin without a Keychain prompt ({exc}).", + ) from exc + + def __exit__(self, *_exc: object) -> None: + try: + if self._previous is not None: + self._sec.SecKeychainSetUserInteractionAllowed(1 if self._previous else 0) + finally: + _UI_LOCK.release() + + def pinned_root() -> str | None: - """The approver root this Mac pins for its Flow runs, or ``None``. + """The approver root this computer pins for its Flow runs, or ``None``. - Only the Keychain's pin counts (see :meth:`FlowRoster.root`). With none, - Desktop removes any root or head from the config it hands Flow, so Flow - refuses every person's answer. + See :meth:`FlowRoster.root`: the administrator's anchor, else (on a + Mac) the Keychain pin set up behind Touch ID. With none, Desktop removes + any root or head from the config it hands Flow, so Flow refuses every + person's answer. Raises :class:`ApproverKeychainError` when the anchor + can't be trusted or disagrees with the Keychain. """ return FlowRoster().root() diff --git a/packages/desktop/openadapt_desktop/cli.py b/packages/desktop/openadapt_desktop/cli.py index 15da3f802..187ff56d4 100644 --- a/packages/desktop/openadapt_desktop/cli.py +++ b/packages/desktop/openadapt_desktop/cli.py @@ -527,14 +527,40 @@ def cmd_approver_key(args: argparse.Namespace, engine: types.SimpleNamespace) -> root = setup print( "A person runs these commands (with OPENADAPT_JIT_KEY_PASSPHRASE set to " - "the key's passphrase), then pins the root " - f"{root.public_key if root else ''} as approvers.root_public_key " - "in the deployment config passed to `openadapt-flow run --config`. " - "Desktop counts a person's answer only with a root pinned in this " - "Mac's Keychain; until then, Flow refuses answers from Desktop." + "the key's passphrase). Then an administrator pins the root for " + "Desktop on this computer: `sudo openadapt-desktop approver-pin " + f"{root.public_key if root else ''}` (on Windows, from an " + "elevated prompt, without sudo). Until then, Flow refuses answers " + "from Desktop." ) +def cmd_approver_pin(args: argparse.Namespace) -> int: + """Pin (or show) the approver root in the administrator-owned anchor.""" + from openadapt_desktop.approver_anchor import ( + AnchorError, + anchor_path, + read_anchor, + write_anchor, + ) + + try: + if args.show or not args.root: + pinned = read_anchor() + print(f"{anchor_path()}: {pinned or 'no approver root pinned'}") + return 0 if (pinned or args.show) else 2 + path = write_anchor(args.root) + except AnchorError as exc: + print(f"Approver pin refused ({exc.code}): {exc}") + return 1 + print(f"Pinned approver root {args.root.strip().lower()} in {path}.") + print( + "Desktop on this computer now counts a person's answer only when it is " + "signed by a key on the roster this root started." + ) + return 0 + + def cmd_config(args: argparse.Namespace, engine: types.SimpleNamespace) -> None: """Show current configuration.""" print(engine.config.model_dump_json(indent=2)) @@ -1051,8 +1077,20 @@ def main(argv: list[str] | None = None) -> None: help="The approver name to show in the register command", ) + # approver-pin + p = subparsers.add_parser( + "approver-pin", + help="Pin this computer's approver root (administrator only)", + ) + p.add_argument("root", nargs="?", help="The roster's root public key (64 hex digits)") + p.add_argument("--show", action="store_true", help="Show the pinned root and exit") + args = parser.parse_args(argv) + if args.command == "approver-pin": + # Runs elevated: touch nothing in any user's data folder. + sys.exit(cmd_approver_pin(args)) + config = EngineConfig() logger.remove() diff --git a/packages/desktop/openadapt_desktop/dispatch.py b/packages/desktop/openadapt_desktop/dispatch.py index a330d58b4..d16e83d75 100644 --- a/packages/desktop/openadapt_desktop/dispatch.py +++ b/packages/desktop/openadapt_desktop/dispatch.py @@ -74,6 +74,8 @@ "approver_presence_refused", "approver_keychain_unsupported", "approver_payload_refused", + "approver_anchor_untrusted", + "approver_root_conflict", } ) #: Stop cards a paired phone may answer through Desktop. A waiting save is @@ -4525,10 +4527,22 @@ def _approver_key_summary(self) -> dict: except Exception: in_app, stored = False, False file_key = approver_key_environment() is not None + try: + from openadapt_desktop import approver_anchor + from openadapt_desktop.approver_keychain import pinned_root + + pinned = pinned_root() is not None + anchored = approver_anchor.read_anchor() is not None + except Exception: + pinned, anchored = False, False return { "ready": stored or file_key, "in_app": in_app, "touch_id": stored, + # Without a pinned root no person's answer counts; "anchored" + # means an administrator pinned it (approver-pin). + "pinned": pinned, + "anchored": anchored, } def _sign_stop_decision(self, message: bytes, *, count: int) -> dict: diff --git a/packages/desktop/package-lock.json b/packages/desktop/package-lock.json index 5d5a9c5ba..412a43573 100644 --- a/packages/desktop/package-lock.json +++ b/packages/desktop/package-lock.json @@ -1,12 +1,12 @@ { "name": "openadapt-desktop", - "version": "0.24.0", + "version": "0.25.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "openadapt-desktop", - "version": "0.24.0", + "version": "0.25.0", "dependencies": { "@tauri-apps/api": "^2", "@tauri-apps/plugin-notification": "^2", diff --git a/packages/desktop/package.json b/packages/desktop/package.json index 145193b87..3581bf7b3 100644 --- a/packages/desktop/package.json +++ b/packages/desktop/package.json @@ -1,6 +1,6 @@ { "name": "openadapt-desktop", - "version": "0.24.0", + "version": "0.25.0", "description": "Installed companion for OpenAdapt authoring, teaching, and local pairing", "private": true, "type": "module", diff --git a/packages/desktop/pyproject.toml b/packages/desktop/pyproject.toml index d14ced2aa..1fce70e2d 100644 --- a/packages/desktop/pyproject.toml +++ b/packages/desktop/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "openadapt-desktop" -version = "0.24.0" +version = "0.25.0" description = "The OpenAdapt desktop app and local engine: record a data-entry task, run it on a list, and check each saved record" readme = "README.md" requires-python = ">=3.11" diff --git a/packages/desktop/src-tauri/Cargo.lock b/packages/desktop/src-tauri/Cargo.lock index fb9f3b2d8..d85fd9c81 100644 --- a/packages/desktop/src-tauri/Cargo.lock +++ b/packages/desktop/src-tauri/Cargo.lock @@ -2502,7 +2502,7 @@ dependencies = [ [[package]] name = "openadapt-desktop" -version = "0.24.0" +version = "0.25.0" dependencies = [ "hex", "reqwest", diff --git a/packages/desktop/src-tauri/Cargo.toml b/packages/desktop/src-tauri/Cargo.toml index 8c7680a00..126b911b7 100644 --- a/packages/desktop/src-tauri/Cargo.toml +++ b/packages/desktop/src-tauri/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "openadapt-desktop" -version = "0.24.0" +version = "0.25.0" description = "Installed companion for OpenAdapt authoring, teaching, and local pairing" authors = ["OpenAdapt AI"] license = "MIT" diff --git a/packages/desktop/src-tauri/tauri.conf.json b/packages/desktop/src-tauri/tauri.conf.json index f4b491d58..bd7ea1bbf 100644 --- a/packages/desktop/src-tauri/tauri.conf.json +++ b/packages/desktop/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://raw.githubusercontent.com/nicoverbruggen/tauri-v2-schema/main/schema.json", "productName": "OpenAdapt Desktop", - "version": "0.24.0", + "version": "0.25.0", "identifier": "ai.openadapt.desktop", "build": { "frontendDist": "../dist", diff --git a/packages/desktop/src/lib/types.ts b/packages/desktop/src/lib/types.ts index d4a953d55..d6200ccd3 100644 --- a/packages/desktop/src/lib/types.ts +++ b/packages/desktop/src/lib/types.ts @@ -1272,7 +1272,15 @@ export interface StopCardsResponse { ok: boolean; cards: StopCard[]; error?: string; - approver_key?: { ready: boolean; in_app: boolean; touch_id: boolean }; + approver_key?: { + ready: boolean; + in_app: boolean; + touch_id: boolean; + /** A root is pinned (else no person's answer counts). */ + pinned?: boolean; + /** An administrator pinned it (``openadapt-desktop approver-pin``). */ + anchored?: boolean; + }; } export interface StopAnswer { diff --git a/packages/desktop/src/screens/Stops.test.tsx b/packages/desktop/src/screens/Stops.test.tsx index 5be69df88..44f19d236 100644 --- a/packages/desktop/src/screens/Stops.test.tsx +++ b/packages/desktop/src/screens/Stops.test.tsx @@ -194,3 +194,17 @@ it("puts the item next to each possible match, field by field", async () => { expect(screen.getByText("Different").closest("td")?.className).toBe("stop-differs"); expect(screen.getByText("Same").closest("td")?.className).toBe("stop-same"); }); + +it("says answers won't count while no approver root is pinned", async () => { + vi.mocked(engineTry).mockImplementation(async (cmd) => + cmd === CMD.LIST_STOP_CARDS + ? { + ok: true, + cards: [card("a", "Fairwick")], + approver_key: { ready: true, in_app: true, touch_id: true, pinned: false }, + } + : null, + ); + render(); + expect(await screen.findByText("No approver root is pinned on this computer")).toBeTruthy(); +}); diff --git a/packages/desktop/src/screens/Stops.tsx b/packages/desktop/src/screens/Stops.tsx index 0575f0b3e..5b9c2ffa6 100644 --- a/packages/desktop/src/screens/Stops.tsx +++ b/packages/desktop/src/screens/Stops.tsx @@ -409,6 +409,12 @@ export function Stops({ ) )} + {keyReady && keyReady.ready && keyReady.pinned === false && list.length > 0 && ( + + Answers here won't count until an administrator pins the root with{" "} + openadapt-desktop approver-pin. + + )} {help && ( 1-9 answer this stop. J or ↓ next, K or ↑ previous. X adds this stop to a batch diff --git a/packages/desktop/tests/test_approver_keychain_real.py b/packages/desktop/tests/test_approver_keychain_real.py index 7a2c7ab3c..5ccc89e94 100644 --- a/packages/desktop/tests/test_approver_keychain_real.py +++ b/packages/desktop/tests/test_approver_keychain_real.py @@ -120,3 +120,62 @@ def test_a_sealed_key_opens_only_with_its_own_enclave_key(enclave): assert bytes(enclave.open(handle, sealed, reason="test")) == b"\x07" * 32 with pytest.raises(ApproverKeychainError): enclave.open(other, sealed, reason="test") + + +def test_a_pin_another_program_wrote_reads_as_no_pin(): + """Attack: a same-user process deletes Desktop's Keychain pin and plants + its own (readable by any app), or rewrites Desktop's in place. Read with + Keychain prompts off, neither may come back as a root, and the read must + not wait on a prompt someone could allow.""" + + import subprocess + import uuid + + from openadapt_desktop.auth.store import SERVICE_NAME + + account = f"desktop-approver-root-probe-{uuid.uuid4().hex[:12]}" + read = ( + "from openadapt_desktop.approver_keychain import _no_keychain_ui\n" + "from openadapt_desktop.auth import store\n" + "with _no_keychain_ui():\n" + f" print(store._kr_get(store._keyring(), {account!r}))\n" + ) + security = "/usr/bin/security" + + def desktop_reads() -> str: + done = subprocess.run( + [sys.executable, "-c", read], capture_output=True, text=True, timeout=30 + ) + assert done.returncode == 0, done.stderr + return done.stdout.strip().splitlines()[-1] + + try: + import keyring + + keyring.set_password(SERVICE_NAME, account, "a" * 64) + assert desktop_reads() == "a" * 64 + + # Rewritten in place by another program. + subprocess.run( + [security, "add-generic-password", "-U", "-s", SERVICE_NAME, "-a", account, + "-w", "b" * 64], + check=True, timeout=15, + ) + assert desktop_reads() == "None" + + # Deleted and planted fresh, readable by any app. + subprocess.run( + [security, "delete-generic-password", "-s", SERVICE_NAME, "-a", account], + check=True, capture_output=True, timeout=15, + ) + subprocess.run( + [security, "add-generic-password", "-A", "-s", SERVICE_NAME, "-a", account, + "-w", "c" * 64], + check=True, timeout=15, + ) + assert desktop_reads() == "None" + finally: + subprocess.run( + [security, "delete-generic-password", "-s", SERVICE_NAME, "-a", account], + capture_output=True, timeout=15, + ) diff --git a/packages/desktop/tests/test_engine/conftest.py b/packages/desktop/tests/test_engine/conftest.py index ef7273dcd..95156a0b7 100644 --- a/packages/desktop/tests/test_engine/conftest.py +++ b/packages/desktop/tests/test_engine/conftest.py @@ -90,3 +90,20 @@ def __init__( def json(self) -> dict: return self._json + + +@pytest.fixture(autouse=True) +def _pins_as_on_a_mac_without_an_anchor(monkeypatch) -> None: + """Every engine test sees a Mac with no administrator anchor. + + The pin reads the real ``/Library``, ``/etc`` or ProgramData anchor and + turns off Keychain prompts through Security.framework; tests use the + in-memory keyring instead, on every OS. Anchor tests set their own. + """ + import contextlib + + from openadapt_desktop import approver_anchor, approver_keychain + + monkeypatch.setattr(approver_keychain.FlowRoster, "keychain_pins", True) + monkeypatch.setattr(approver_keychain, "_no_keychain_ui", contextlib.nullcontext) + monkeypatch.setattr(approver_anchor, "read_anchor", lambda *a, **k: None) diff --git a/packages/desktop/tests/test_engine/test_approver_anchor.py b/packages/desktop/tests/test_engine/test_approver_anchor.py new file mode 100644 index 000000000..cf82e41dd --- /dev/null +++ b/packages/desktop/tests/test_engine/test_approver_anchor.py @@ -0,0 +1,457 @@ +"""The administrator's approver pin, and how Desktop chooses its root. + +Each attack is a same-user process (an agent) trying to make its own root the +one Desktop pins, so it can sign answers to its own saves. +""" + +from __future__ import annotations + +import json +import os +import sys +from pathlib import Path + +import pytest + +from openadapt_desktop import approver_anchor +from openadapt_desktop.approver_anchor import Ace, AnchorError, dacl_admin_only + +ROOT = "a" * 64 +OTHER = "b" * 64 +posix_only = pytest.mark.skipif(sys.platform == "win32", reason="POSIX owner and mode") + +# The real reader, before the engine conftest stubs it out. +_read_anchor = approver_anchor.read_anchor + + +def _me() -> frozenset[int]: + """Root plus this test's own account, standing in for the administrator.""" + return frozenset({0, os.getuid()}) + + +@pytest.fixture +def anchor(tmp_path: Path) -> Path: + folder = tmp_path.resolve() / "OpenAdapt" + return folder / approver_anchor.ANCHOR_NAME + + +def _write(path: Path, root: str = ROOT) -> Path: + return approver_anchor.write_anchor(root, path, trusted_uids=_me()) + + +# -- reading the anchor ---------------------------------------------------------- + + +@posix_only +def test_an_administrators_anchor_pins_the_root(anchor: Path) -> None: + _write(anchor) + assert _read_anchor(anchor, trusted_uids=_me()) == ROOT + assert oct(anchor.stat().st_mode & 0o777) == "0o644" + assert oct(anchor.parent.stat().st_mode & 0o777) == "0o755" + + +def test_no_anchor_is_no_pin(anchor: Path) -> None: + assert _read_anchor(anchor, trusted_uids=_me()) is None + + +@posix_only +def test_a_file_this_user_owns_is_not_an_administrators_pin(anchor: Path) -> None: + """Attack: the agent writes its own root into the anchor's place.""" + _write(anchor) + with pytest.raises(AnchorError) as raised: + _read_anchor(anchor) # only root is trusted, as in real use + assert raised.value.code == "approver_anchor_untrusted" + + +@posix_only +def test_a_symlinked_anchor_is_refused(anchor: Path, tmp_path: Path) -> None: + """Attack: point the anchor at a file the agent controls.""" + _write(anchor) + mine = tmp_path.resolve() / "mine.json" + mine.write_text(json.dumps({"root_public_key": OTHER})) + anchor.unlink() + anchor.symlink_to(mine) + with pytest.raises(AnchorError, match="without following links"): + _read_anchor(anchor, trusted_uids=_me()) + + +@posix_only +def test_a_symlinked_folder_is_refused(tmp_path: Path) -> None: + """Attack: replace the anchor's folder with a link to the agent's own.""" + base = tmp_path.resolve() + real = base / "real" + _write(real / approver_anchor.ANCHOR_NAME) + (base / "OpenAdapt").symlink_to(real) + with pytest.raises(AnchorError, match="without following links"): + _read_anchor(base / "OpenAdapt" / approver_anchor.ANCHOR_NAME, trusted_uids=_me()) + + +@posix_only +@pytest.mark.parametrize("mode", [0o664, 0o646]) +def test_an_anchor_others_can_write_is_refused(anchor: Path, mode: int) -> None: + _write(anchor) + os.chmod(anchor, mode) + with pytest.raises(AnchorError, match="Others can write"): + _read_anchor(anchor, trusted_uids=_me()) + + +@posix_only +def test_a_folder_others_can_write_is_refused(anchor: Path) -> None: + """Attack: a writable folder lets the agent rename its own file into place.""" + _write(anchor) + os.chmod(anchor.parent, 0o777) + with pytest.raises(AnchorError, match="Others can write"): + _read_anchor(anchor, trusted_uids=_me()) + + +@posix_only +def test_a_hard_linked_anchor_is_refused(anchor: Path, tmp_path: Path) -> None: + _write(anchor) + os.link(anchor, tmp_path.resolve() / "elsewhere.json") + with pytest.raises(AnchorError, match="hard links"): + _read_anchor(anchor, trusted_uids=_me()) + + +@posix_only +@pytest.mark.parametrize( + "content", + ["not json", json.dumps(["list"]), json.dumps({"root_public_key": "zz" * 32}), "{}"], +) +def test_an_unreadable_anchor_is_refused(anchor: Path, content: str) -> None: + _write(anchor) + os.chmod(anchor, 0o644) + anchor.write_text(content) + with pytest.raises(AnchorError) as raised: + _read_anchor(anchor, trusted_uids=_me()) + assert raised.value.code == "approver_anchor_unreadable" + + +@posix_only +def test_real_system_folders_pass_the_folder_check() -> None: + os.close(approver_anchor._open_folder(Path("/usr/bin"), frozenset({0}))) + + +@posix_only +@pytest.mark.skipif(hasattr(os, "geteuid") and os.geteuid() == 0, reason="runs as root") +def test_pinning_needs_an_administrator(anchor: Path) -> None: + with pytest.raises(AnchorError) as raised: + approver_anchor.write_anchor(ROOT, anchor) + assert raised.value.code == "not_elevated" + assert not anchor.exists() + + +def test_the_anchor_lives_where_only_an_administrator_writes() -> None: + assert str(approver_anchor.anchor_path("darwin")) == ( + "/Library/Application Support/OpenAdapt/approver-root.json" + ) + assert str(approver_anchor.anchor_path("linux")) == "/etc/openadapt/approver-root.json" + + +# -- macOS extended ACLs --------------------------------------------------------- + +mac_only = pytest.mark.skipif(sys.platform != "darwin", reason="macOS extended ACLs") + + +def _chmod_acl(path: Path, entry: str) -> None: + import subprocess + + subprocess.run(["/bin/chmod", "+a", entry, str(path)], check=True) + + +@mac_only +def test_an_acl_letting_everyone_write_the_anchor_is_refused(anchor: Path) -> None: + """Attack: mode bits say 0644, but an extended ACL lets everyone write.""" + _write(anchor) + _chmod_acl(anchor, "everyone allow write,append") + with pytest.raises(AnchorError, match="access list"): + _read_anchor(anchor, trusted_uids=_me()) + + +@mac_only +@pytest.mark.parametrize( + "entry", + ["everyone allow add_file", "everyone allow delete_child", "everyone allow writesecurity"], +) +def test_an_acl_letting_everyone_change_the_folder_is_refused(anchor: Path, entry: str) -> None: + """Attack: an ACL on the folder lets the agent add, replace or re-permission files.""" + _write(anchor) + _chmod_acl(anchor.parent, entry) + with pytest.raises(AnchorError, match="access list"): + _read_anchor(anchor, trusted_uids=_me()) + + +@mac_only +def test_pinning_strips_an_inherited_acl(tmp_path: Path) -> None: + """A folder above can hand new folders and files an ACL that grants + writes (only_inherit: it doesn't apply to the folder itself). Pinning + strips it, so the anchor reads back.""" + import subprocess + + parent = tmp_path.resolve() / "Support" + parent.mkdir() + _chmod_acl( + parent, + "everyone allow write,add_file,delete_child,file_inherit,directory_inherit,only_inherit", + ) + target = parent / "OpenAdapt" / approver_anchor.ANCHOR_NAME + _write(target) + assert _read_anchor(target, trusted_uids=_me()) == ROOT + listed = subprocess.run(["/bin/ls", "-led", str(target.parent)], capture_output=True, text=True) + assert "allow" not in listed.stdout + + +def test_mac_acl_text_is_read_entry_by_entry() -> None: + everyone = "group:ABCDEFAB-CDEF-ABCD-EFAB-CDEF0000000C:everyone:12" + text = ( + "!#acl 1\n" + f"{everyone}:allow:read,list\n" + f"{everyone}:deny:write,delete\n" + f"{everyone}:allow,file_inherit,directory_inherit,only_inherit:write,delete_child\n" + ) + assert approver_anchor.mac_acl_write_grants(text) == [] + inherited = f"{everyone}:allow,inherited,file_inherit,directory_inherit:write,delete_child" + assert approver_anchor.mac_acl_write_grants(text + inherited + "\n") == [inherited] + + +# -- Windows access control ------------------------------------------------------ + +ADMINS, SYSTEM, USERS, ME = "S-1-5-32-544", "S-1-5-18", "S-1-5-32-545", "S-1-5-21-1-2-3-1001" +FULL, READ = 0x001F01FF, 0x001200A9 + + +def test_only_administrators_writing_is_admin_only() -> None: + aces = [Ace(0, 0, FULL, ADMINS), Ace(0, 0, FULL, SYSTEM), Ace(0, 0, READ, USERS)] + assert dacl_admin_only(ADMINS, aces) is None + + +@pytest.mark.parametrize( + ("owner", "aces", "reason"), + [ + (ME, [Ace(0, 0, FULL, ADMINS)], "owned by"), + (ADMINS, None, "no access control list"), + (ADMINS, [Ace(0, 0, FULL, ADMINS), Ace(0, 0, 0x0002, USERS)], USERS), + (ADMINS, [Ace(0, 0, 0x00040000, ME)], ME), # WRITE_DAC + (ADMINS, [Ace(0, 0, 0x10000000, ME)], ME), # GENERIC_ALL + ], +) +def test_anyone_else_able_to_change_it_is_refused(owner, aces, reason) -> None: + assert reason in (dacl_admin_only(owner, aces) or "") + + +@pytest.mark.parametrize("ace_type", [5, 9, 11]) # object, callback, callback object +def test_every_kind_of_allow_entry_is_checked(ace_type: int) -> None: + """Attack: grant a user write through an allow entry of another type.""" + aces = [Ace(0, 0, FULL, ADMINS), Ace(ace_type, 0, 0x0002, ME)] + assert ME in (dacl_admin_only(ADMINS, aces) or "") + + +@pytest.mark.parametrize("ace_type", [4, 17, 99]) +def test_an_entry_of_an_unknown_type_refuses(ace_type: int) -> None: + aces = [Ace(0, 0, FULL, ADMINS), Ace(ace_type, 0, READ, ME)] + assert "doesn't know" in (dacl_admin_only(ADMINS, aces) or "") + + +def test_a_folder_above_may_let_users_add_but_not_replace() -> None: + """ProgramData lets users create folders; only replacing entries counts above.""" + replace = approver_anchor._REPLACE_RIGHTS + adds = [Ace(0, 0, FULL, SYSTEM), Ace(0, 0, 0x0004 | 0x0002, USERS)] + assert dacl_admin_only(SYSTEM, adds, rights=replace) is None + assert dacl_admin_only(SYSTEM, adds) is not None # but never on the anchor's own folder + deletes = adds + [Ace(0, 0, 0x0040, USERS)] # FILE_DELETE_CHILD + assert USERS in (dacl_admin_only(SYSTEM, deletes, rights=replace) or "") + + +def test_the_anchor_folder_is_created_admin_only() -> None: + sddl = approver_anchor.ANCHOR_FOLDER_SDDL + assert sddl.startswith("O:BAD:P") # owned by Administrators, protected DACL + assert "(A;OICI;0x1200a9;;;BU)" in sddl # Users read and execute only + + +def test_inherit_only_and_deny_entries_grant_nothing_here() -> None: + aces = [Ace(0, 0x08, FULL, USERS), Ace(1, 0, FULL, USERS), Ace(0, 0, FULL, ADMINS)] + assert dacl_admin_only(SYSTEM, aces) is None + + +@pytest.mark.skipif(sys.platform != "win32", reason="Windows ACLs") +def test_real_windows_acls(tmp_path: Path) -> None: # pragma: no cover - Windows CI + windir = approver_anchor._known_windows_dir() + assert windir is not None + approver_anchor._check_windows(windir / "System32" / "drivers" / "etc" / "hosts") + mine = tmp_path / approver_anchor.ANCHOR_NAME + mine.write_text(json.dumps({"root_public_key": OTHER})) + with pytest.raises(AnchorError) as raised: + approver_anchor._check_windows(mine) + assert raised.value.code == "approver_anchor_untrusted" + + +# -- which root Desktop pins ----------------------------------------------------- + + +@pytest.fixture +def roster(monkeypatch, fake_keyring): + from openadapt_desktop import approver_keychain + + state = {"anchor": None} + + def fake_read_anchor(*_a, **_k): + if isinstance(state["anchor"], Exception): + raise state["anchor"] + return state["anchor"] + + monkeypatch.setattr(approver_anchor, "read_anchor", fake_read_anchor) + return approver_keychain.FlowRoster(), state, fake_keyring + + +def _keychain_root(fake_keyring, value: str) -> None: + from openadapt_desktop.approver_keychain import ROOT_ACCOUNT + from openadapt_desktop.auth.store import SERVICE_NAME + + fake_keyring.set_password(SERVICE_NAME, ROOT_ACCOUNT, value) + + +def test_the_administrators_anchor_pins_without_a_keychain_root(roster) -> None: + flow_roster, state, _keyring = roster + state["anchor"] = ROOT + assert flow_roster.root() == ROOT + + +def test_a_keychain_root_that_disagrees_with_the_anchor_is_refused(roster) -> None: + """Attack: on a Mac the agent rewrites the Keychain pin (for example by + running code inside the ad-hoc signed engine); the anchor still decides.""" + from openadapt_desktop.approver_keychain import ApproverKeychainError, pinned_root + + flow_roster, state, keyring = roster + state["anchor"] = ROOT + _keychain_root(keyring, OTHER) + with pytest.raises(ApproverKeychainError) as raised: + flow_roster.root() + assert raised.value.code == "approver_root_conflict" + with pytest.raises(ApproverKeychainError): + pinned_root() + _keychain_root(keyring, ROOT) + assert flow_roster.root() == ROOT + + +def test_an_untrusted_anchor_counts_no_answer(roster) -> None: + from openadapt_desktop.approver_keychain import ApproverKeychainError + + flow_roster, state, keyring = roster + _keychain_root(keyring, ROOT) + state["anchor"] = AnchorError("approver_anchor_untrusted", "planted") + with pytest.raises(ApproverKeychainError) as raised: + flow_roster.root() + assert raised.value.code == "approver_anchor_untrusted" + + +def test_off_a_mac_the_credential_store_never_pins(roster, monkeypatch) -> None: + """Attack: on Windows or Linux any process of this user writes the + credential store, so a root planted there must count for nothing.""" + from openadapt_desktop.approver_keychain import FlowRoster + + flow_roster, state, keyring = roster + monkeypatch.setattr(FlowRoster, "keychain_pins", False) + _keychain_root(keyring, OTHER) + assert flow_roster.root() is None + state["anchor"] = ROOT + assert flow_roster.root() == ROOT + + +def test_with_an_anchor_this_mac_doesnt_start_another_root(roster) -> None: + from openadapt_desktop.approver_keychain import ApproverKeychainError + + flow_roster, state, _keyring = roster + state["anchor"] = ROOT + with pytest.raises(ApproverKeychainError) as raised: + flow_roster.init_root("desktop-approver", OTHER, lambda _m: "ed25519:00") + assert raised.value.code == "approver_root_conflict" + + +def test_a_run_config_carries_the_anchors_root(roster, tmp_path) -> None: + import yaml + + from openadapt_desktop.approver_keychain import pinned_root + from openadapt_desktop.private_flow_config import prepare_flow_config + + _flow_roster, state, _keyring = roster + state["anchor"] = ROOT + source = tmp_path / "deployment.json" + source.write_text(json.dumps({"approvers": {"root_public_key": OTHER}})) + with pytest.raises(Exception, match="another approver root"): + prepare_flow_config(source, None, approver_root=pinned_root()) + source.write_text(json.dumps({"name": "clinic"})) + payload = yaml.safe_load(prepare_flow_config(source, None, approver_root=pinned_root()).payload) + assert payload["approvers"] == {"root_public_key": ROOT} + + +# -- the command ----------------------------------------------------------------- + + +def test_the_pin_command_shows_and_refuses_without_elevation(monkeypatch, tmp_path, capsys) -> None: + from openadapt_desktop import cli + + target = tmp_path.resolve() / "OpenAdapt" / approver_anchor.ANCHOR_NAME + monkeypatch.setattr(approver_anchor, "anchor_path", lambda platform=None: target) + monkeypatch.setattr(approver_anchor, "read_anchor", _read_anchor) + with pytest.raises(SystemExit) as shown: + cli.main(["approver-pin", "--show"]) + assert shown.value.code == 0 + assert "no approver root pinned" in capsys.readouterr().out + if sys.platform != "win32" and os.geteuid() != 0: + with pytest.raises(SystemExit) as refused: + cli.main(["approver-pin", ROOT]) + assert refused.value.code == 1 + assert "not_elevated" in capsys.readouterr().out + assert not target.exists() + + +def test_an_anchor_that_cant_be_checked_counts_no_answer(roster) -> None: + """A load or read failure other than a refusal (a broken import, an OS + error) is never taken as "no anchor".""" + from openadapt_desktop.approver_keychain import ApproverKeychainError + + flow_roster, state, keyring = roster + _keychain_root(keyring, ROOT) + state["anchor"] = OSError("the folder vanished mid-read") + with pytest.raises(ApproverKeychainError) as raised: + flow_roster.root() + assert raised.value.code == "approver_anchor_untrusted" + + +def _windows_admin() -> bool: + if sys.platform != "win32": + return False + return bool(approver_anchor._windll("shell32").IsUserAnAdmin()) + + +@pytest.mark.skipif(not _windows_admin(), reason="needs an elevated Windows session") +def test_windows_pin_replaces_a_folder_a_user_made_first() -> None: # pragma: no cover + """Attack: a user creates the anchor folder before the administrator pins + and plants a root in it. The plant is refused; pinning moves that folder + aside and creates a fresh one only administrators can change.""" + import secrets + import shutil + + base = approver_anchor._program_data() + folder = base / f"OpenAdapt-test-{secrets.token_hex(6)}" + target = folder / approver_anchor.ANCHOR_NAME + folder.mkdir() # inherits ProgramData's ACL: Users may write here + target.write_text(json.dumps({"root_public_key": OTHER})) + try: + with pytest.raises(AnchorError) as planted: + _read_anchor(target) + assert planted.value.code == "approver_anchor_untrusted" + + assert approver_anchor.write_anchor(ROOT, target) == target + assert _read_anchor(target) == ROOT + owner, aces = approver_anchor._windows_security(folder) + assert owner == ADMINS + assert dacl_admin_only(owner, aces) is None + asides = list(base.glob(f"{folder.name}.replaced-*")) + assert len(asides) == 1 + assert ( + json.loads((asides[0] / approver_anchor.ANCHOR_NAME).read_text())["root_public_key"] + == OTHER + ) + finally: + for leftover in [folder, *base.glob(f"{folder.name}.replaced-*")]: + shutil.rmtree(leftover, ignore_errors=True) diff --git a/packages/desktop/tests/test_engine/test_item_approval.py b/packages/desktop/tests/test_engine/test_item_approval.py index c9c1f5671..18663f19a 100644 --- a/packages/desktop/tests/test_engine/test_item_approval.py +++ b/packages/desktop/tests/test_engine/test_item_approval.py @@ -181,6 +181,9 @@ def test_run_it_waits_for_a_person_and_saves_after_approval(task) -> None: result = _run(disp) + # Flow gets the staged config's digest, so the bytes it reads are pinned. + digest = flow.run_kwargs.pop("config_sha256") + assert len(digest) == 64 and int(digest, 16) >= 0 assert flow.run_kwargs == {"approval_mode": "each_item", "approval_wait_s": APPROVAL_WAIT_S} (shown,) = flow.seen assert shown["waiting"] is True @@ -595,7 +598,7 @@ def test_bridge_sends_no_answer_without_an_approver_key(monkeypatch, tmp_path: P assert f"openadapt-flow jit keygen --out {key_path}" in message assert "OPENADAPT_JIT_KEY_PASSPHRASE" in message assert "openadapt-flow jit approver init --principal" in message - assert "approvers.root_public_key" in message + assert "sudo openadapt-desktop approver-pin" in message assert "OPENADAPT_APPROVER_ROOT" not in message assert calls == [] # never an unsigned answer diff --git a/packages/desktop/tests/test_engine/test_stop_queue.py b/packages/desktop/tests/test_engine/test_stop_queue.py index 13889095a..f5473f92b 100644 --- a/packages/desktop/tests/test_engine/test_stop_queue.py +++ b/packages/desktop/tests/test_engine/test_stop_queue.py @@ -337,7 +337,13 @@ def test_without_an_approver_key_nothing_changes(tmp_path): disp = EngineDispatcher(config, services=EngineServices(config, db=db, audit=_Audit())) _stop(db, "item-40", lname="Fairwick") listed = disp.dispatch("list_stop_cards", {}) - assert listed["approver_key"] == {"ready": False, "in_app": False, "touch_id": False} + assert listed["approver_key"] == { + "ready": False, + "in_app": False, + "touch_id": False, + "pinned": False, + "anchored": False, + } answer = disp.dispatch( "answer_stop_cards", {"card_ids": [listed["cards"][0]["card_id"]], "choice": "same"} diff --git a/packages/desktop/uv.lock b/packages/desktop/uv.lock index e70f75b66..9719f8927 100644 --- a/packages/desktop/uv.lock +++ b/packages/desktop/uv.lock @@ -1172,7 +1172,7 @@ wheels = [ [[package]] name = "openadapt-desktop" -version = "0.24.0" +version = "0.25.0" source = { editable = "." } dependencies = [ { name = "cryptography" }, diff --git a/packages/flow/docs/APPROVAL_MODES.md b/packages/flow/docs/APPROVAL_MODES.md index 4162b4389..f920e1c5b 100644 --- a/packages/flow/docs/APPROVAL_MODES.md +++ b/packages/flow/docs/APPROVAL_MODES.md @@ -157,7 +157,8 @@ including an answer file written around the command - **The first approver.** On a Mac, Desktop sets it up behind Touch ID: the root is Desktop's approver key, signed after a presence prompt. Elsewhere, a person starts the roster with `openadapt-flow jit approver init`, and - the deployment config pins its root. + the deployment config pins its root (for Desktop, an administrator pins + it with `openadapt-desktop approver-pin`). Each route signs with its own key: @@ -172,15 +173,52 @@ Each route signs with its own key: stdout; Desktop checks that they are the person's answer, asks for Touch ID, and writes the signature back on stdin. A cancelled prompt or any other failure is a refusal, and Flow records nothing. - - On a Mac, Desktop's pin is the root it set up behind Touch ID, kept in - the login Keychain; nothing else names it (not the environment, not a - file the user can write). A process running as the same user can delete - the Keychain items; Desktop then has no pin, and no person's answer - counts until someone sets the key up again behind Touch ID. - - On Windows and Linux, `openadapt-desktop approver-key setup` creates it, - keeps the encrypted key in an owner-only file and its passphrase in the - OS keychain, and prints the `jit approver` commands that put it on the - roster. + - Desktop's pin comes from one of two places, and nothing else names it + (not the environment, not a file this user can write, not Windows + Credential Manager or the Linux Secret Service): + - **The administrator's anchor**, a file only an administrator can + write: `/Library/Application Support/OpenAdapt/approver-root.json` + (macOS), `/etc/openadapt/approver-root.json` (Linux), or + `OpenAdapt\approver-root.json` in the ProgramData known folder + (Windows). An administrator writes it with `sudo openadapt-desktop + approver-pin ROOT` (on Windows, from an elevated prompt). Desktop + trusts it only when no one but an administrator can change it or what + leads to it: + - macOS and Linux: Desktop opens it one folder at a time from `/` + without following links. Every folder and the file must be owned by + root and writable by no one else, with no extended ACL granting a + write (macOS). The file must have a single link. Pinning strips any + ACL the new folder or file would inherit. + - Windows: the file and its folder must be owned by Administrators, + SYSTEM or TrustedInstaller, with no access entry of any kind letting + anyone else write. An entry of a type the check doesn't know refuses + the pin. No folder above may let anyone else replace what's inside, + and there are no links, junctions or extra hard links. Pinning never + reuses a folder that's already there (whoever made it could keep a + handle open through any permission change). It moves that folder + aside and creates a new one, owned by Administrators, with a + protected access list: full control for Administrators and SYSTEM, + read for Users. + Otherwise Desktop counts no person's answer. + - **On a Mac without an anchor, the Keychain**: the root Desktop set up + itself behind Touch ID. Desktop reads it with Keychain prompts turned + off, so an item another program planted or rewrote reads as no pin + instead of asking the person, who might allow it. + - An anchor and a Keychain root that disagree are refused, and a Mac + with an anchor doesn't start a second root. + - Without a pin, Desktop sends Flow no approvers section: runs go on, and + every person's answer is refused (`approver_root_unpinned`). + - What the anchor protects against: a process running as the same user + (an agent) can delete or rewrite this user's Keychain items, and on + today's ad-hoc signed macOS builds, which have no hardened runtime, it + can run code inside Desktop's own engine and write a Keychain root + Desktop reads without a prompt. Only the administrator's anchor holds + against that. An administrator, or a process with the administrator's + password or a cached `sudo`, can still change the anchor. + - On Windows and Linux, `openadapt-desktop approver-key setup` creates the + key, keeps the encrypted key in an owner-only file and its passphrase in + the OS keychain, and prints the `jit approver` commands that put it on + the roster; an administrator then pins the root with `approver-pin`. - **The decision portal** signs with the key at `OPENADAPT_PORTAL_APPROVER_KEY` after the person answered on their phone. Without it, the portal records no answer. Its key is added to the roster like any other approver.