From 0a0f4db6417eca736f11dee5994dd6eb242457ba Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 18 Aug 2026 15:13:08 -0400 Subject: [PATCH 1/3] feat(crosscheck): project one Pi profile into the account home its consumers read Pi keeps every signed-in profile in one auth.json keyed by provider slot: openai-codex, openai-codex-2, and so on. Every Firstmate consumer of a Pi credential instead reads an account home holding exactly one credential under the fixed key openai-codex. fm-crosscheck.py's inspect_pi_credential and account_identity both name that key literally, and so does the Azure Crosscheck credential archive. Pointing a reviewer at the pooled file therefore fails two ways at once. Only the first slot is ever read, so profiles 2..N are unreachable no matter which one the roster selected; and the reviewer archive carries every signed-in account's tokens into a compartment that needs exactly one. The operator pool here holds eight profiles across eight distinct upstream accounts in 16,846 bytes, which is under MAX_CONFIG_BYTES, so no existing bound catches it. This writes the single-profile homes those consumers already expect. It validates credential shape, refuses a non-oauth or blanked profile, writes 0600 under a 0700 directory through a private temp file and a rename, and refuses to follow a symlink at the credential path. It reports expiry instants and account digests and never prints token material. It does not decide whether a credential is good enough to use. That question has one owner, bin/fm-credential-expiry.py, which the callers run as their preflight; duplicating the judgement here would give one fact two owners. --all refuses as a set rather than leaving a half-projected root behind, and names every unusable profile at once so an operator fixes one round of logins instead of discovering the next broken profile one failed projection at a time. Verification: the second test unit drives the real fm-crosscheck.py reader against a projected home rather than restating the key name, so it proves the consumer accepts the output instead of proving the test agrees with itself. Five mutations, each confirmed to change the file first, all red: the entry written under its pool name, the blank-token check disabled, the mode widened to 0644, the symlink guard removed, and --all downgraded to skip-and-continue. --- bin/fm-pi-account-home.py | 239 ++++++++++++++++++++++++++++++ docs/azure-crosscheck.md | 16 ++ docs/scripts.md | 1 + tests/behavior-test-durations.tsv | 1 + tests/fm-pi-account-home.test.sh | 168 +++++++++++++++++++++ tests/test-capabilities.tsv | 5 +- 6 files changed, 428 insertions(+), 2 deletions(-) create mode 100755 bin/fm-pi-account-home.py create mode 100755 tests/fm-pi-account-home.test.sh diff --git a/bin/fm-pi-account-home.py b/bin/fm-pi-account-home.py new file mode 100755 index 00000000000..c7282420ac0 --- /dev/null +++ b/bin/fm-pi-account-home.py @@ -0,0 +1,239 @@ +#!/usr/bin/env python3 +"""Project one Pi profile into a single-profile account home. + +Pi keeps every signed-in profile in ONE `auth.json`, keyed by provider slot: +`openai-codex`, `openai-codex-2`, and so on. Every Firstmate consumer of a Pi +credential instead reads an account home holding exactly one credential under +the fixed key `openai-codex` (`fm-crosscheck.py: inspect_pi_credential`, +`account_identity`, and the Azure Crosscheck credential archive all name that +key literally). + +Handing the pooled file to a consumer therefore fails two ways at once. Only +the first slot is ever read, so profiles 2..N are unreachable no matter which +one the roster selected; and the Azure reviewer archive would carry every +signed-in account's tokens into a compartment that needs exactly one. This +command writes the single-profile homes those consumers already expect. + +It validates credential SHAPE and reports expiry instants. It does not decide +whether a credential is still good enough to use: that is one question with one +owner, `bin/fm-credential-expiry.py`, which the callers run as their preflight. +Token material is never printed, and an account is identified only by digest. +""" + +from __future__ import annotations + +import argparse +import datetime +import hashlib +import json +import os +from pathlib import Path +import stat +import sys +import tempfile + +CONSUMER_KEY = "openai-codex" +MAX_SOURCE_BYTES = 4 * 1024 * 1024 +MAX_PROFILES = 256 +REQUIRED_STRINGS = ("access", "refresh", "accountId") + + +class ProjectionError(RuntimeError): + pass + + +def fail(message: str) -> None: + raise ProjectionError(message) + + +def read_pool(source: Path) -> dict[str, dict]: + try: + metadata = source.lstat() + except OSError as exc: + fail(f"Pi credential pool is unreadable at {source}: {exc.strerror}") + if not stat.S_ISREG(metadata.st_mode) or source.is_symlink(): + fail(f"Pi credential pool must be a regular non-symlink file at {source}") + if metadata.st_size > MAX_SOURCE_BYTES: + fail(f"Pi credential pool exceeds its {MAX_SOURCE_BYTES}-byte bound at {source}") + try: + parsed = json.loads(source.read_text(encoding="utf-8")) + except (OSError, ValueError) as exc: + fail(f"Pi credential pool is malformed at {source}: {type(exc).__name__}") + if not isinstance(parsed, dict): + fail(f"Pi credential pool is not a profile object at {source}") + if len(parsed) > MAX_PROFILES: + fail(f"Pi credential pool declares more than {MAX_PROFILES} profiles at {source}") + return parsed + + +def entry_faults(entry: object) -> list[str]: + """Every reason this entry would fail its consumer, not just the first.""" + + if not isinstance(entry, dict): + return ["is not a credential object"] + faults = [] + if entry.get("type") != "oauth": + faults.append("is not an oauth credential") + for name in REQUIRED_STRINGS: + value = entry.get(name) + if not isinstance(value, str): + faults.append(f"has no {name} string") + elif not value.strip(): + # A blanked token is the shape a de-authenticated profile leaves + # behind, and it reads as present to anything checking only for + # the key. Consumers reject it; refuse to project it. + faults.append(f"has a blank {name}") + expires = entry.get("expires") + if isinstance(expires, bool) or not isinstance(expires, (int, float)): + faults.append("has no numeric expires") + return faults + + +def expiry_text(entry: dict) -> str: + expires = entry.get("expires") + if isinstance(expires, bool) or not isinstance(expires, (int, float)): + return "unknown" + # Pi records the expiry in milliseconds. + moment = datetime.datetime.fromtimestamp( + expires / 1000.0, datetime.timezone.utc + ) + return moment.isoformat().replace("+00:00", "Z") + + +def account_digest(entry: dict) -> str: + account = entry.get("accountId") + if not isinstance(account, str) or not account.strip(): + return "none" + return hashlib.sha256(account.strip().encode("utf-8")).hexdigest()[:16] + + +def select(pool: dict[str, dict], requested: list[str], every: bool) -> list[str]: + if every: + return sorted(pool) + missing = [name for name in requested if name not in pool] + if missing: + fail("Pi credential pool has no profile named: " + ", ".join(sorted(missing))) + return list(dict.fromkeys(requested)) + + +def write_home(destination: Path, entry: dict) -> Path: + credential = destination / "auth.json" + try: + existing = credential.lstat() + except FileNotFoundError: + existing = None + except OSError as exc: + fail(f"account home is unreadable at {credential}: {exc.strerror}") + if existing is not None and ( + not stat.S_ISREG(existing.st_mode) or credential.is_symlink() + ): + # Never follow a symlink into a write: the destination is chosen by an + # operator argument and a planted link would redirect a credential. + fail(f"refusing to replace a non-regular credential path at {credential}") + + destination.mkdir(mode=0o700, parents=True, exist_ok=True) + os.chmod(destination, 0o700) + body = json.dumps({CONSUMER_KEY: entry}, sort_keys=True, indent=2) + "\n" + + # Written to a private temp file and renamed, so a reader never observes a + # half-written credential and never sees one at default permissions. + handle, staged = tempfile.mkstemp(dir=str(destination), prefix=".auth-", suffix=".tmp") + try: + with os.fdopen(handle, "w", encoding="utf-8") as stream: + stream.write(body) + stream.flush() + os.fsync(stream.fileno()) + os.chmod(staged, 0o600) + os.replace(staged, credential) + except BaseException: + try: + os.unlink(staged) + except OSError: + pass + raise + directory = os.open(str(destination), os.O_RDONLY) + try: + os.fsync(directory) + finally: + os.close(directory) + return credential + + +def command_report(args: argparse.Namespace) -> int: + pool = read_pool(Path(args.source).expanduser().resolve()) + rows = [] + for name in sorted(pool): + faults = entry_faults(pool[name]) + rows.append((name, "usable-shape" if not faults else "; ".join(faults), + expiry_text(pool[name]) if isinstance(pool[name], dict) else "unknown", + account_digest(pool[name]) if isinstance(pool[name], dict) else "none")) + width = max([len(row[0]) for row in rows] + [len("profile")]) + stamp = max([len(row[2]) for row in rows] + [len("expires")]) + print(f"{'profile'.ljust(width)} {'expires'.ljust(stamp)} {'account':<16} shape") + for name, shape, expires, digest in rows: + print(f"{name.ljust(width)} {expires.ljust(stamp)} {digest:<16} {shape}") + distinct = {row[3] for row in rows if row[3] != "none"} + print(f"profiles={len(rows)} distinct-accounts={len(distinct)}") + return 0 + + +def command_project(args: argparse.Namespace) -> int: + source = Path(args.source).expanduser().resolve() + root = Path(args.destination_root).expanduser().resolve() + pool = read_pool(source) + names = select(pool, args.profile or [], args.all) + if not names: + fail("name at least one --profile, or pass --all") + + unusable = {name: entry_faults(pool[name]) for name in names} + unusable = {name: faults for name, faults in unusable.items() if faults} + if unusable: + # All of them, so an operator fixes one round of logins rather than + # discovering the next broken profile one failed projection at a time. + for name in sorted(unusable): + print(f"REFUSED {name}: {'; '.join(unusable[name])}", file=sys.stderr) + fail(f"{len(unusable)} of {len(names)} selected profiles cannot be projected") + + for name in names: + destination = root / name + credential = write_home(destination, pool[name]) + print( + f"projected {name} -> {credential} " + f"account={account_digest(pool[name])} expires={expiry_text(pool[name])}" + ) + print(f"projected={len(names)} root={root}") + return 0 + + +def build_parser() -> argparse.ArgumentParser: + parser = argparse.ArgumentParser( + prog="fm-pi-account-home.py", description=__doc__.splitlines()[0] + ) + commands = parser.add_subparsers(dest="command", required=True) + + default_source = "~/.pi/agent/auth.json" + + report = commands.add_parser("report", help="list pool profiles without projecting") + report.add_argument("--source", default=default_source) + report.set_defaults(handler=command_report) + + project = commands.add_parser("project", help="write single-profile account homes") + project.add_argument("--source", default=default_source) + project.add_argument("--destination-root", required=True) + project.add_argument("--profile", action="append") + project.add_argument("--all", action="store_true") + project.set_defaults(handler=command_project) + return parser + + +def main(argv: list[str] | None = None) -> int: + args = build_parser().parse_args(argv) + try: + return args.handler(args) + except ProjectionError as exc: + print(f"PI ACCOUNT HOME REFUSED: {exc}", file=sys.stderr) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/docs/azure-crosscheck.md b/docs/azure-crosscheck.md index 3e3dd9bf15b..38f7e83dd7c 100644 --- a/docs/azure-crosscheck.md +++ b/docs/azure-crosscheck.md @@ -172,6 +172,22 @@ The Pi closure parameters must name the same tarball the crewmate cell image ins Pi declares `engines.node >= 22.19.0` and ships a `#!/usr/bin/env node` entrypoint, so the Node pin is a correctness bound and not a preference: an older runtime or an unresolvable `node` on `PATH` fails the reviewer at launch rather than at admission. +### Pi reviewer account homes + +Pi keeps every signed-in profile in one `auth.json` keyed by provider slot (`openai-codex`, `openai-codex-2`, ...), while every Firstmate consumer reads an account home holding exactly one credential under the fixed key `openai-codex`. +Pointing a reviewer at the pooled file therefore fails twice: only the first slot is ever read, so the selected profile is unreachable, and the reviewer credential archive would carry every signed-in account's tokens into a compartment that needs one. + +`bin/fm-pi-account-home.py` writes the single-profile homes those consumers expect: + +```sh +bin/fm-pi-account-home.py report +bin/fm-pi-account-home.py project --destination-root --profile openai-codex-2 +``` + +It validates credential shape, refuses a blanked or non-oauth profile, and reports expiry instants and account digests, never token material. +It does not decide whether a credential is still good enough to use: that question has one owner, `bin/fm-credential-expiry.py`, which the reviewer preflight runs. +Distinct profiles are distinct upstream accounts, so a Pi-versus-Pi review still satisfies account separation; `config/crosscheck-same-model` relaxes only the model screen. + The home-local configuration is optional and gitignored: ```json diff --git a/docs/scripts.md b/docs/scripts.md index f897b2512de..9b589e67c26 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -73,6 +73,7 @@ The shared no-mistakes gate refusal used by every directly invocable mutating co | `fm-azure-validation.sh` | Queue and control exact-head no-mistakes runs in isolated elastic Azure cells | | `fm-credential-expiry.py` | Classify one account profile's provider credential by expiry without emitting token material | | `fm-azure-validation-shard-bridge.py` | Exchange exact behavior/lint requests and independent Azure runner receipts inside one cell | +| `fm-pi-account-home.py` | Project one Pi profile from the pooled `auth.json` into the single-profile account home its consumers read | | `fm-nm-step-liveness.sh` | Read a no-mistakes step's processes as alive, dead, or graded unknown | | `fm-tangle-lib.sh` | Shared default-branch resolution and primary-checkout tangle classification | | `fm-supervision-lib.sh` | Shared in-flight-work-without-fresh-watcher-beacon predicate | diff --git a/tests/behavior-test-durations.tsv b/tests/behavior-test-durations.tsv index eeedf3d2345..b061083522c 100644 --- a/tests/behavior-test-durations.tsv +++ b/tests/behavior-test-durations.tsv @@ -72,6 +72,7 @@ 1686 tests/fm-macos-permissions.test.sh 3428 tests/fm-nm-step-liveness.test.sh 1560 tests/fm-no-mistakes-reattach.test.sh +2000 tests/fm-pi-account-home.test.sh 10 tests/fm-pi-primary-live-e2e.test.sh 80 tests/fm-pi-primary-types.test.sh 3170 tests/fm-pi-watch-extension.test.sh diff --git a/tests/fm-pi-account-home.test.sh b/tests/fm-pi-account-home.test.sh new file mode 100755 index 00000000000..70fac994071 --- /dev/null +++ b/tests/fm-pi-account-home.test.sh @@ -0,0 +1,168 @@ +#!/usr/bin/env bash +# Behavior: projecting one Pi profile into the single-profile account home its +# consumers actually read, without ever writing a token into the transcript. +set -euo pipefail + +ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd -P) +# shellcheck source=tests/lib.sh +. "$ROOT/tests/lib.sh" + +TOOL="$ROOT/bin/fm-pi-account-home.py" +# A marker standing in for token material, so leakage is detectable by grep +# rather than by inspection. +MARKER=fmtestpitokenmarker + +make_pool() { + python3 - "$1" "$MARKER" <<'PY' +import json +import pathlib +import sys + +path = pathlib.Path(sys.argv[1]) +marker = sys.argv[2] + + +def entry(account, expires=1893456000000, access=None, refresh=None, kind="oauth"): + return { + "type": kind, + "access": marker + "-access" if access is None else access, + "refresh": marker + "-refresh" if refresh is None else refresh, + "expires": expires, + "accountId": account, + } + + +pool = { + "openai-codex": entry("acct-one"), + "openai-codex-2": entry("acct-two"), + # The shape a de-authenticated profile leaves behind: the keys are all + # present, and both token strings are empty. + "openai-codex-3": entry("acct-three", access="", refresh=""), + "openai-codex-4": entry("acct-four", kind="apikey"), + "openai-codex-5": {"type": "oauth", "access": marker, "refresh": marker, + "accountId": "acct-five"}, + "openai-codex-6": entry(" "), +} +path.write_text(json.dumps(pool, indent=2) + "\n", encoding="utf-8") +PY +} + +projection_contract() { + local work pool out code + work=$(fm_test_tmproot fm-pi-account-home) + pool=$work/auth.json + make_pool "$pool" + + # A named profile lands under the fixed consumer key, not under its pool name. + "$TOOL" project --source "$pool" --destination-root "$work/homes" \ + --profile openai-codex-2 >"$work/out.txt" 2>&1 \ + || fail "projecting a usable profile refused" + assert_present "$work/homes/openai-codex-2/auth.json" "the projected account home has no credential" + python3 - "$work/homes/openai-codex-2/auth.json" <<'PY' || fail "the projected credential is not consumer-shaped" +import json +import sys +value = json.load(open(sys.argv[1], encoding="utf-8")) +assert list(value) == ["openai-codex"], list(value) +assert value["openai-codex"]["accountId"] == "acct-two", value["openai-codex"]["accountId"] +PY + + # Exactly one credential reaches the home. Handing over the pool would carry + # every signed-in account into a compartment that needs one. + assert_no_grep "acct-one" "$work/homes/openai-codex-2/auth.json" \ + "a sibling profile's account rode along into the projected home" + + # The consumer reads this file directly; wrong modes expose a live token. + expect_code 600 "$(stat -f '%Lp' "$work/homes/openai-codex-2/auth.json" 2>/dev/null \ + || stat -c '%a' "$work/homes/openai-codex-2/auth.json")" "the projected credential is not owner-only" + expect_code 700 "$(stat -f '%Lp' "$work/homes/openai-codex-2" 2>/dev/null \ + || stat -c '%a' "$work/homes/openai-codex-2")" "the projected account home is not owner-only" + + # Nothing the command prints may carry token material. + assert_no_grep "$MARKER" "$work/out.txt" "the projection printed token material" + "$TOOL" report --source "$pool" >"$work/report.txt" 2>&1 || fail "report refused a readable pool" + assert_no_grep "$MARKER" "$work/report.txt" "the report printed token material" + assert_grep "distinct-accounts" "$work/report.txt" "the report omitted its account summary" + + # Re-projecting is idempotent, so a refreshed pool can be re-run at any time. + "$TOOL" project --source "$pool" --destination-root "$work/homes" \ + --profile openai-codex-2 >/dev/null 2>&1 || fail "re-projecting the same profile refused" + + # A blanked profile reads as present to anything checking only for the key. + code=0 + out=$("$TOOL" project --source "$pool" --destination-root "$work/blank" \ + --profile openai-codex-3 2>&1) || code=$? + expect_code 1 "$code" "a blanked-token profile was projected" + assert_contains "$out" "blank access" "the refusal did not name the blank token" + assert_absent "$work/blank/openai-codex-3/auth.json" "a refused profile still wrote a credential" + + for bad in openai-codex-4 openai-codex-5 openai-codex-6; do + code=0 + "$TOOL" project --source "$pool" --destination-root "$work/bad" --profile "$bad" >/dev/null 2>&1 || code=$? + expect_code 1 "$code" "an unusable profile ($bad) was projected" + done + + # --all refuses as a set rather than leaving a half-projected root behind. + code=0 + out=$("$TOOL" project --source "$pool" --destination-root "$work/every" --all 2>&1) || code=$? + expect_code 1 "$code" "--all projected a pool containing unusable profiles" + assert_contains "$out" "4 of 6" "the refusal did not report the full unusable set" + assert_absent "$work/every/openai-codex/auth.json" "a refused --all run projected the usable profiles anyway" + + # A planted symlink at the credential path must not be followed into a write. + mkdir -p "$work/planted/openai-codex-2" "$work/target" + ln -s "$work/target/stolen.json" "$work/planted/openai-codex-2/auth.json" + code=0 + out=$("$TOOL" project --source "$pool" --destination-root "$work/planted" \ + --profile openai-codex-2 2>&1) || code=$? + expect_code 1 "$code" "a symlinked credential path was followed into a write" + assert_absent "$work/target/stolen.json" "the projection wrote through a planted symlink" + + code=0 + "$TOOL" project --source "$pool" --destination-root "$work/none" --profile absent >/dev/null 2>&1 || code=$? + expect_code 1 "$code" "a profile absent from the pool was projected" + + code=0 + "$TOOL" report --source "$work/missing.json" >/dev/null 2>&1 || code=$? + expect_code 1 "$code" "an absent pool was reported as readable" + + printf 'not json\n' >"$work/malformed.json" + code=0 + "$TOOL" report --source "$work/malformed.json" >/dev/null 2>&1 || code=$? + expect_code 1 "$code" "a malformed pool was reported as readable" + + pass "one Pi profile projects into the single-profile account home its consumers read, and unusable or planted paths refuse" +} + +consumer_agreement_contract() { + local work pool + work=$(fm_test_tmproot fm-pi-account-home-consumer) + pool=$work/auth.json + make_pool "$pool" + "$TOOL" project --source "$pool" --destination-root "$work/homes" \ + --profile openai-codex-2 >/dev/null 2>&1 || fail "projection refused" + + # The real consumer, not a restatement of it: fm-crosscheck.py's own reader + # must accept the projected home and derive the expected identity from it. + # Asserting the key name here instead would only prove this test agrees with + # itself. + python3 - "$ROOT/bin/fm-crosscheck.py" "$work/homes/openai-codex-2" <<'PY' \ + || fail "the real Pi credential reader rejected a projected account home" +import importlib.util +import pathlib +import sys + +spec = importlib.util.spec_from_file_location("core", sys.argv[1]) +core = importlib.util.module_from_spec(spec) +spec.loader.exec_module(core) +home = pathlib.Path(sys.argv[2]) +source, identifier = core.inspect_pi_credential(home) +assert source == "pi-openai-codex-oauth-file", source +assert identifier == str(home / "auth.json"), identifier +identity = core.account_identity("pi", home) +assert identity == "openai-codex:acct-two", identity +PY + pass "the real fm-crosscheck Pi reader accepts a projected home and derives its executing account" +} + +projection_contract +consumer_agreement_contract diff --git a/tests/test-capabilities.tsv b/tests/test-capabilities.tsv index 203d3cc4a2d..cbc86323aae 100644 --- a/tests/test-capabilities.tsv +++ b/tests/test-capabilities.tsv @@ -11,6 +11,7 @@ fm-arm-pretool-check.test.sh hermetic fm-auto-reap-herdr-e2e.test.sh herdr-lab fm-auto-reap.test.sh hermetic fm-autocompact.test.sh hermetic +fm-azure-cell-image.test.sh hermetic fm-azure-pilot.test.sh hermetic fm-azure-runner.test.sh hermetic fm-azure-validation.test.sh hermetic @@ -63,6 +64,7 @@ fm-lock.test.sh hermetic fm-macos-permissions.test.sh hermetic fm-nm-step-liveness.test.sh hermetic fm-no-mistakes-reattach.test.sh hermetic +fm-pi-account-home.test.sh hermetic fm-pi-primary-live-e2e.test.sh hermetic fm-pi-primary-types.test.sh hermetic fm-pi-watch-extension.test.sh hermetic @@ -103,12 +105,11 @@ fm-watch-pause-absorb.test.sh hermetic fm-watch-triage.test.sh hermetic fm-watcher-lock.test.sh hermetic fm-worker-lifecycle.test.sh hermetic -fm-worker-supervisor.test.sh hermetic fm-worker-outcome-transport.test.sh hermetic +fm-worker-supervisor.test.sh hermetic fm-x-mode.test.sh hermetic lavish-repair.test.sh hermetic lavish.test.sh hermetic operating-fundamentals.test.sh hermetic runner-entry-probe.test.sh hermetic test-suite-seal.test.sh hermetic -fm-azure-cell-image.test.sh hermetic From 8a8aa1ef3eaa397bdeda515b815fee91da41d6d1 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 18 Aug 2026 15:35:16 -0400 Subject: [PATCH 2/3] fix(crosscheck): refuse a pooled account home, and stop the projection writing outside its root An adversarial review found two blockers and a hazard the change did not close. The test never routed through tests/test-entry.sh, so it bypassed the runner and the admission seal refused it: test-suite-seal.test.sh exits 97 on this branch and passes on main. I ran the new test file alone and never ran the suite, which is how a red seal reached CI. The symlink guard was defeated one directory level up. It lstat'd the credential path but not the profile directory holding it, and mkdir(exist_ok=True) succeeds on a symlink to a directory because isdir follows it. A planted link at / therefore took a live OAuth token outside the destination root and the tool reported success. Compounding it, Path.mkdir(parents=True) applies its mode to the leaf only, so every ancestor landed at the caller's umask - 0777 under a permissive one, which is the precondition for planting that link. Now every component the tool creates is made owner-only regardless of umask, a non-directory or symlinked profile component is refused, and a pre-existing group- or world-writable root without the sticky bit is refused outright: a credential is not written under a path others can replace between the check and the write. The larger point: this was an optional script with no refusal at the consumer, so the hazard it exists to remove stayed fully reachable for anyone who did not run it. A pooled auth.json passed inspect_pi_credential, and the Azure archive's same-account guard compares slot one against an identity derived from slot one, so it always agreed with itself and staged all eight accounts. The reader now refuses an account home carrying more than one provider slot, which closes both halves unconditionally and makes the projection the remedy rather than the hope. Also closed, all found by the same review: the source symlink guard was dead code because both callers resolve() before reading; a lone unknown --profile was refused by the empty-selection guard rather than by the missing-name check, so a typo mixed with a good name was silently dropped; entry_faults advertised every fault and only the first was asserted; the reported expiry instant, the distinct-account count, and the account digest were printed but never read by a test; the pool size bound was untested; and an ordinary OSError escaped the refusal contract as a traceback. Mutations, each confirmed to change the file first, now red: first-fault-only, unknown profiles dropped, source symlink accepted, the size bound removed, the expiry instant shifted, the raw account id returned instead of a digest, and distinct-accounts reporting the total. Not claimed: the temp-file-and-rename is argued from the mechanism, not proven. An assertion I wrote for it passed identically with a direct write, so it was removed rather than shipped, and the comment now says which half is covered. --- bin/fm-crosscheck.py | 13 ++++ bin/fm-pi-account-home.py | 97 +++++++++++++++++++++++++--- tests/fm-pi-account-home.test.sh | 105 ++++++++++++++++++++++++++++++- 3 files changed, 206 insertions(+), 9 deletions(-) diff --git a/bin/fm-crosscheck.py b/bin/fm-crosscheck.py index f55c7b94b57..036fe1d25e4 100755 --- a/bin/fm-crosscheck.py +++ b/bin/fm-crosscheck.py @@ -393,6 +393,19 @@ def inspect_pi_credential(account_home: Path) -> tuple[str, str]: ) except CrosscheckError as exc: tool_fail(str(exc)) + # Pi pools every signed-in profile in one auth.json keyed by provider slot, + # and only this slot is ever read. An account home carrying more than one + # slot therefore cannot mean what its caller thinks: the roster's selected + # profile is unreachable, and the Azure reviewer archive - which stages this + # whole file - would carry every other signed-in account's live tokens into + # a compartment that needs exactly one. Project a single-profile home with + # bin/fm-pi-account-home.py rather than pointing a reviewer at the pool. + if isinstance(credentials, dict) and len(credentials) > 1: + tool_fail( + f"Pi executing-account credential at {credential_file} carries " + f"{len(credentials)} provider slots; an account home holds exactly " + "one (project one with bin/fm-pi-account-home.py)" + ) credential = ( credentials.get("openai-codex") if isinstance(credentials, dict) diff --git a/bin/fm-pi-account-home.py b/bin/fm-pi-account-home.py index c7282420ac0..84589ed628f 100755 --- a/bin/fm-pi-account-home.py +++ b/bin/fm-pi-account-home.py @@ -116,8 +116,81 @@ def select(pool: dict[str, dict], requested: list[str], every: bool) -> list[str return list(dict.fromkeys(requested)) +def make_private_directory(path: Path) -> None: + """Create one directory component, refusing to traverse a planted link. + + `Path.mkdir(parents=True)` applies its mode to the leaf only, so every + ancestor lands at the caller's umask - 0777 under a permissive one, which + is precisely the precondition for planting the link this refuses. And + `exist_ok=True` succeeds on a symlink to a directory, because `isdir` + follows it, so guarding only the final credential path leaves the write + redirectable one component higher. + """ + + try: + existing = path.lstat() + except FileNotFoundError: + existing = None + except OSError as exc: + fail(f"account home path is unreadable at {path}: {exc.strerror}") + if existing is not None: + if not stat.S_ISDIR(existing.st_mode) or path.is_symlink(): + fail(f"refusing to write through a non-directory account home path at {path}") + try: + os.chmod(path, 0o700) + except OSError as exc: + fail(f"account home path cannot be made owner-only at {path}: {exc.strerror}") + return + try: + os.mkdir(path, 0o700) + except OSError as exc: + fail(f"account home path cannot be created at {path}: {exc.strerror}") + os.chmod(path, 0o700) + + +def prepare_root(root: Path) -> None: + """Admit the operator-chosen root without widening anything above it.""" + + try: + existing = root.lstat() + except FileNotFoundError: + # We create it, so we own its mode. `parents=True` applies the mode to + # the leaf only, leaving intermediates at the caller's umask - 0777 + # under a permissive one, which is the precondition for planting the + # link the profile component refuses below. + missing = [] + walk = root + while not walk.exists(): + missing.append(walk) + if walk.parent == walk: + break + walk = walk.parent + for component in reversed(missing): + os.mkdir(component, 0o700) + os.chmod(component, 0o700) + return + except OSError as exc: + fail(f"destination root is unreadable at {root}: {exc.strerror}") + if not stat.S_ISDIR(existing.st_mode) or root.is_symlink(): + fail(f"destination root must be a real directory, not a link, at {root}") + # A pre-existing root that anyone can write to is exactly where a profile + # component gets replaced by a link between this check and the write. + if existing.st_mode & (stat.S_IWGRP | stat.S_IWOTH) and not ( + existing.st_mode & stat.S_ISVTX + ): + fail( + f"destination root is group- or world-writable at {root}; " + "a credential is not written under a path others can replace" + ) + + def write_home(destination: Path, entry: dict) -> Path: credential = destination / "auth.json" + # The profile component, not just the credential inside it: an intermediate + # symlink redirects the write exactly as effectively, and mkdir(exist_ok) + # follows one because `isdir` does. + make_private_directory(destination) + try: existing = credential.lstat() except FileNotFoundError: @@ -130,13 +203,13 @@ def write_home(destination: Path, entry: dict) -> Path: # Never follow a symlink into a write: the destination is chosen by an # operator argument and a planted link would redirect a credential. fail(f"refusing to replace a non-regular credential path at {credential}") - - destination.mkdir(mode=0o700, parents=True, exist_ok=True) - os.chmod(destination, 0o700) body = json.dumps({CONSUMER_KEY: entry}, sort_keys=True, indent=2) + "\n" - # Written to a private temp file and renamed, so a reader never observes a - # half-written credential and never sees one at default permissions. + # Written to a private temp file and renamed: no reader observes the token + # at default permissions, and a partial write cannot truncate the previous + # credential in place. Only the mode is covered by a test; the atomicity + # and the fsync ordering are argued from the mechanism, not proven here, + # because the failure they defend against is a crash mid-write. handle, staged = tempfile.mkstemp(dir=str(destination), prefix=".auth-", suffix=".tmp") try: with os.fdopen(handle, "w", encoding="utf-8") as stream: @@ -160,7 +233,7 @@ def write_home(destination: Path, entry: dict) -> Path: def command_report(args: argparse.Namespace) -> int: - pool = read_pool(Path(args.source).expanduser().resolve()) + pool = read_pool(Path(args.source).expanduser()) rows = [] for name in sorted(pool): faults = entry_faults(pool[name]) @@ -178,9 +251,10 @@ def command_report(args: argparse.Namespace) -> int: def command_project(args: argparse.Namespace) -> int: - source = Path(args.source).expanduser().resolve() + source = Path(args.source).expanduser() root = Path(args.destination_root).expanduser().resolve() pool = read_pool(source) + prepare_root(root) names = select(pool, args.profile or [], args.all) if not names: fail("name at least one --profile, or pass --all") @@ -233,6 +307,15 @@ def main(argv: list[str] | None = None) -> int: except ProjectionError as exc: print(f"PI ACCOUNT HOME REFUSED: {exc}", file=sys.stderr) return 1 + except OSError as exc: + # A traceback is not a refusal contract. Report the path and the errno + # text, never the value being written. + location = getattr(exc, "filename", None) or "an account home path" + print( + f"PI ACCOUNT HOME REFUSED: {location}: {exc.strerror or exc}", + file=sys.stderr, + ) + return 1 if __name__ == "__main__": diff --git a/tests/fm-pi-account-home.test.sh b/tests/fm-pi-account-home.test.sh index 70fac994071..8463be4fd64 100755 --- a/tests/fm-pi-account-home.test.sh +++ b/tests/fm-pi-account-home.test.sh @@ -1,11 +1,13 @@ #!/usr/bin/env bash +# shellcheck source=tests/test-entry.sh +. "$(dirname "$0")/test-entry.sh" # Behavior: projecting one Pi profile into the single-profile account home its # consumers actually read, without ever writing a token into the transcript. set -euo pipefail ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd -P) # shellcheck source=tests/lib.sh -. "$ROOT/tests/lib.sh" +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" TOOL="$ROOT/bin/fm-pi-account-home.py" # A marker standing in for token material, so leakage is detectable by grep @@ -121,6 +123,78 @@ PY "$TOOL" project --source "$pool" --destination-root "$work/none" --profile absent >/dev/null 2>&1 || code=$? expect_code 1 "$code" "a profile absent from the pool was projected" + # A lone unknown name is refused by the empty-selection guard, which says + # nothing about whether unknown names are noticed. Mix one in with a good one: + # a typo that is silently dropped tells the operator the projection succeeded. + code=0 + out=$("$TOOL" project --source "$pool" --destination-root "$work/mixed" \ + --profile openai-codex-2 --profile openai-codex-77 2>&1) || code=$? + expect_code 1 "$code" "an unknown profile mixed with a known one was silently dropped" + assert_contains "$out" "openai-codex-77" "the refusal did not name the unknown profile" + assert_absent "$work/mixed/openai-codex-2/auth.json" "a refused selection projected its known profiles anyway" + + # Every reason at once, not just the first: the blanked fixture has two. + code=0 + out=$("$TOOL" project --source "$pool" --destination-root "$work/faults" --profile openai-codex-3 2>&1) || code=$? + expect_code 1 "$code" "a blanked profile was projected" + assert_contains "$out" "blank access" "the refusal omitted the blank access token" + assert_contains "$out" "blank refresh" "the refusal reported only the first fault" + + # The source guard must be reachable: resolving the path before reading it + # strips the symlink and the guard can never fire. + ln -s "$pool" "$work/linked-pool.json" + code=0 + "$TOOL" report --source "$work/linked-pool.json" >/dev/null 2>&1 || code=$? + expect_code 1 "$code" "a symlinked credential pool was read" + + # The reported instant and the distinct-account count are the values an + # operator plans a rotation from; neither is proved by the header alone. + "$TOOL" report --source "$pool" >"$work/report2.txt" 2>&1 || fail "report refused" + assert_grep "2030-01-01T00:00:00Z" "$work/report2.txt" "the report did not render the expiry instant" + assert_grep "profiles=6 distinct-accounts=5" "$work/report2.txt" "the report miscounted distinct accounts" + + # An intermediate symlink redirects the write as effectively as one at the + # credential path, and mkdir(exist_ok) follows it because isdir does. + mkdir -p "$work/traverse" "$work/elsewhere" + ln -s "$work/elsewhere" "$work/traverse/openai-codex-2" + code=0 + "$TOOL" project --source "$pool" --destination-root "$work/traverse" --profile openai-codex-2 >/dev/null 2>&1 || code=$? + expect_code 1 "$code" "a symlinked profile directory was followed into a write" + assert_absent "$work/elsewhere/auth.json" "the projection wrote a credential outside its destination root" + + # A root anyone can write to is where a profile component gets replaced by a + # link between the check and the write. + mkdir -p "$work/openroot"; chmod 0777 "$work/openroot" + code=0 + "$TOOL" project --source "$pool" --destination-root "$work/openroot" --profile openai-codex-2 >/dev/null 2>&1 || code=$? + expect_code 1 "$code" "a credential was written under a world-writable root" + + # Created ancestors are owner-only regardless of the caller's umask; a mode + # that only holds under a strict umask is not an assurance. + (umask 000; "$TOOL" project --source "$pool" --destination-root "$work/deep/nested/root" \ + --profile openai-codex-2 >/dev/null 2>&1) || fail "projecting into a new root refused" + for created in "$work/deep" "$work/deep/nested" "$work/deep/nested/root"; do + expect_code 700 "$(stat -f '%Lp' "$created" 2>/dev/null || stat -c '%a' "$created")" \ + "a created ancestor was left readable to others under a permissive umask" + done + + # An account is identified by digest, so the raw upstream id never appears. + assert_no_grep "acct-two" "$work/report2.txt" "the report printed a raw account identifier" + + # The pool bound is a real refusal, not a comment. + python3 -c 'import sys; open(sys.argv[1],"w").write("{\"openai-codex\":\"" + "x"*5000000 + "\"}")' "$work/huge.json" + code=0 + "$TOOL" report --source "$work/huge.json" >/dev/null 2>&1 || code=$? + expect_code 1 "$code" "an oversized credential pool was read" + + # An ordinary OS error is still a refusal, not a traceback. + mkdir -p "$work/readonly"; chmod 0500 "$work/readonly" + code=0 + out=$("$TOOL" project --source "$pool" --destination-root "$work/readonly/sub" --profile openai-codex-2 2>&1) || code=$? + chmod 0700 "$work/readonly" + expect_code 1 "$code" "an unwritable destination did not refuse cleanly" + assert_contains "$out" "PI ACCOUNT HOME REFUSED" "an OS error escaped the refusal contract" + code=0 "$TOOL" report --source "$work/missing.json" >/dev/null 2>&1 || code=$? expect_code 1 "$code" "an absent pool was reported as readable" @@ -161,7 +235,34 @@ assert identifier == str(home / "auth.json"), identifier identity = core.account_identity("pi", home) assert identity == "openai-codex:acct-two", identity PY - pass "the real fm-crosscheck Pi reader accepts a projected home and derives its executing account" + # The reader must also refuse the pooled file outright. Without that, the + # hazard this tool exists to remove stays reachable for anyone who does not + # run it: the pool passes every check, and the Azure archive stages all of it. + python3 - "$ROOT/bin/fm-crosscheck.py" "$work" <<'PY' || fail "the Pi reader accepted a pooled account home" +import importlib.util +import json +import pathlib +import sys + +spec = importlib.util.spec_from_file_location("core", sys.argv[1]) +core = importlib.util.module_from_spec(spec) +spec.loader.exec_module(core) +home = pathlib.Path(sys.argv[2]) / "pooled" +home.mkdir(parents=True, exist_ok=True) +entry = {"type": "oauth", "access": "a", "refresh": "r", + "expires": 1893456000000, "accountId": "acct"} +(home / "auth.json").write_text( + json.dumps({"openai-codex": entry, "openai-codex-2": entry}), encoding="utf-8" +) +try: + core.inspect_pi_credential(home) +except core.CrosscheckToolError as exc: + assert "provider slots" in str(exc), str(exc) +else: + raise AssertionError("a pooled multi-slot account home was accepted") +PY + + pass "the real fm-crosscheck Pi reader accepts a projected home, derives its account, and refuses a pooled one" } projection_contract From c58b314d386350471b131583c5e8b1f95bc8d184 Mon Sep 17 00:00:00 2001 From: Dongkeun Lee Date: Tue, 18 Aug 2026 16:00:27 -0400 Subject: [PATCH 3/3] fix(test): read a file mode portably instead of through a BSD-only stat flag CI caught what local runs could not. `stat -f FMT` is a BSD format string on macOS and the GNU flag for "display filesystem status" on Linux, so the `stat -f ... || stat -c ...` pair never fell through: on the runner `stat -f` SUCCEEDED and printed a filesystem report, and the mode assertion compared 600 against a line beginning `File: "..."`. A fallback that only runs when the first command fails is not a fallback when both platforms accept the flag and mean different things by it. Read the mode through python, which means one thing on both. --- tests/fm-pi-account-home.test.sh | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/fm-pi-account-home.test.sh b/tests/fm-pi-account-home.test.sh index 8463be4fd64..285c4fb4f56 100755 --- a/tests/fm-pi-account-home.test.sh +++ b/tests/fm-pi-account-home.test.sh @@ -49,6 +49,13 @@ path.write_text(json.dumps(pool, indent=2) + "\n", encoding="utf-8") PY } +# `stat -f` is a BSD format string and a GNU filesystem-status flag, so a +# `stat -f ... || stat -c ...` pair silently reports the wrong thing on Linux +# rather than falling through. Python means one thing on both. +file_mode() { + python3 -c 'import os,sys; print("%o" % (os.stat(sys.argv[1]).st_mode & 0o777))' "$1" +} + projection_contract() { local work pool out code work=$(fm_test_tmproot fm-pi-account-home) @@ -74,10 +81,8 @@ PY "a sibling profile's account rode along into the projected home" # The consumer reads this file directly; wrong modes expose a live token. - expect_code 600 "$(stat -f '%Lp' "$work/homes/openai-codex-2/auth.json" 2>/dev/null \ - || stat -c '%a' "$work/homes/openai-codex-2/auth.json")" "the projected credential is not owner-only" - expect_code 700 "$(stat -f '%Lp' "$work/homes/openai-codex-2" 2>/dev/null \ - || stat -c '%a' "$work/homes/openai-codex-2")" "the projected account home is not owner-only" + expect_code 600 "$(file_mode "$work/homes/openai-codex-2/auth.json")" "the projected credential is not owner-only" + expect_code 700 "$(file_mode "$work/homes/openai-codex-2")" "the projected account home is not owner-only" # Nothing the command prints may carry token material. assert_no_grep "$MARKER" "$work/out.txt" "the projection printed token material" @@ -174,7 +179,7 @@ PY (umask 000; "$TOOL" project --source "$pool" --destination-root "$work/deep/nested/root" \ --profile openai-codex-2 >/dev/null 2>&1) || fail "projecting into a new root refused" for created in "$work/deep" "$work/deep/nested" "$work/deep/nested/root"; do - expect_code 700 "$(stat -f '%Lp' "$created" 2>/dev/null || stat -c '%a' "$created")" \ + expect_code 700 "$(file_mode "$created")" \ "a created ancestor was left readable to others under a permissive umask" done