feat(crosscheck): project one Pi profile into the account home its consumers read - #248
Merged
Conversation
ruby-dlee
force-pushed
the
fm/pi-account-home-projection
branch
2 times, most recently
from
August 18, 2026 20:35
06a5459 to
2e2a174
Compare
…nsumers 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.
…n 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 <root>/<profile> 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.
…at 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.
ruby-dlee
force-pushed
the
fm/pi-account-home-projection
branch
from
August 18, 2026 20:52
2e2a174 to
c58b314
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The mismatch
Pi keeps every signed-in profile in one
auth.json, keyed by provider slot (openai-codex,openai-codex-2, ...). Every Firstmate consumer instead reads an account home holding exactly one credential under the fixed keyopenai-codex—fm-crosscheck.py: inspect_pi_credential,account_identity, andfm-crosscheck-azure.py: create_credential_archiveall name that key literally.Pointing a reviewer at the pooled file fails two ways at once:
The operator pool holds 8 profiles across 8 distinct upstream accounts in 16,846 bytes.
MAX_CONFIG_BYTESis 65,536, so no existing bound catches it.What this adds
bin/fm-pi-account-home.pywrites the single-profile homes those consumers already expect.It validates credential shape, refuses a non-oauth or blanked profile, writes
0600under a0700directory via a private temp file and a rename, and refuses to follow a symlink at the credential path. It reports expiry instants and account digests, never token material.It deliberately does not decide whether a credential is still 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.--allrefuses 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.Test
The second unit drives the real
fm-crosscheck.pyreader against a projected home and asserts the identity it derives, rather than restating the key name. Asserting the key name would only prove the test agrees with itself.openai-codex--alldowngraded to skip-and-continueEach was confirmed to change the file before the run.
shellcheck -xexits 0;bin/fm-behavior-shards.sh --check 8plans 113 tests.Note on model independence
The 8 profiles are 8 distinct upstream accounts. Account separation is therefore already satisfied for a Pi-versus-Pi review;
config/crosscheck-same-modelrelaxes only the model screen, not the account screen.