git credentials: fail closed, and read a seat's token from its own slot #1311

Merged
jason.woltje merged 2 commits from fred/credential-fail-closed-seat-slots into next 2026-08-18 22:40:47 +00:00
Collaborator

What this changes

Two changes to one rule: a credential is resolved from exactly one place, and an identity that cannot be resolved is refused rather than substituted.

Fail closed

Both readers — git-credential-mosaic and detect-platform.sh's get_gitea_token() — ended in an unconditional fall-through to the shared Gitea account whenever an identity did not resolve. Every seat in a fleet therefore pushed, opened PRs and filed reviews under one account, and a record made that way cannot be traced back to the agent that made it.

The fallback now applies only where there is no attribution to lose: a host with no fleet. Where seats exist, an unresolvable request emits nothing, exits nonzero, and explains itself on stderr. The git helper additionally appends a record — identity, host, reason, cwd, and no token value — to ${MOSAIC_CREDENTIAL_SPOOL:-~/.local/state/mosaic-credential-escalations}, because the stderr diagnostic is transient and something durable has to exist for whoever debugs it later.

identity resolves token in its store host runs a fleet result
yes yes that identity + token
yes no fail closed
no yes fail closed
no no shared account, unchanged

A host "runs a fleet" when <brain>/fleet/agents exists — the same signal packages/mosaic/src/fleet/brain-home.ts already uses to decide a brain is active, resolved the same way (MOSAIC_BRAIN_HOME, else ~/.mosaic).

That gate is what keeps this a no-op for anyone who has not provisioned per-slot tokens: no fleet directory, shared account, unchanged. It is also why there is deliberately no environment variable to restore the old behavior — one would reintroduce the substitution being removed.

Store selection

Both readers hardcoded ~/.config/mosaic/secrets/gitea-tokens, so a seat's own secrets/ slot was invisible to the framework: a seat could hold a perfectly valid credential and still be served the shared account.

The store is now chosen by what the identity is:

The identity Its credential is read from
has a directory at <brain>/fleet/agents/<id>/ — a seat <brain>/fleet/agents/<id>/secrets/gitea-{usc,mosaicstack}-<id>.token
does not — a service identity ~/.config/mosaic/secrets/gitea-tokens/gitea-{usc,mosaicstack}-<id>.token

No precedence between them and no fallback from one to the other. A seat with an empty slot is refused even when a same-named token sits in the framework store. Two copies of one credential are drift rather than redundancy, and drift surfaces as the stale copy returning 401 — which reads as a revoked token and sends whoever debugs it somewhere else entirely.

Why detect-platform.sh is in scope too

It is the second reader of these tokens. Patching only the git helper would make "one credential, one location" true for push and fetch and false for pr-create.sh, issue-create.sh and pr-review.sh — the harder failure to notice, because those are the tools that create attributable records.

Tests

The three assertions that pinned the shared-account fall-through are now fail-closed assertions. A refusal is checked four independent ways: nonzero exit, empty stdout, a stderr diagnostic naming identity and host, and no shared token value anywhere in the output. The exit code alone would pass against a helper that emitted the credential and then failed.

Added coverage:

  • seat-slot resolution
  • the no-cross-store-fallback case, with a control proving the framework-store file it declines to read is readable — so the refusal is the store rule firing, not an unreadable file
  • no-identity on a fleet host
  • the fleet gate firing on the default ~/.mosaic, not only on an injected MOSAIC_BRAIN_HOME
  • a cross-host leak check (an agent holding a usc token must not have it surface on a mosaicstack request)

Both suites were also run against the pre-change code as a control and fail there on exactly the shared-token emission, so the green is discriminating rather than vacuous.

Both tests were already enumerated in test:framework-shell; no wiring change. verify-sanitized.sh passes, and was seeded with a denylist hit to confirm it can go red.

Known gap

shellcheck is not installed on the authoring host, so the rewritten helper is unlinted locally — CI is the first lint of it.

Operator note

On a host that runs a fleet, a human doing manual git work now needs an identity of their own: MOSAIC_GIT_IDENTITY=<id> with a provisioned slot. That is the intended consequence, not a side effect.

## What this changes Two changes to one rule: a credential is resolved from exactly one place, and an identity that cannot be resolved is refused rather than substituted. ### Fail closed Both readers — `git-credential-mosaic` and `detect-platform.sh`'s `get_gitea_token()` — ended in an unconditional fall-through to the shared Gitea account whenever an identity did not resolve. Every seat in a fleet therefore pushed, opened PRs and filed reviews under one account, and a record made that way cannot be traced back to the agent that made it. The fallback now applies only where there is no attribution to lose: a host with no fleet. Where seats exist, an unresolvable request emits nothing, exits nonzero, and explains itself on stderr. The git helper additionally appends a record — identity, host, reason, cwd, and **no token value** — to `${MOSAIC_CREDENTIAL_SPOOL:-~/.local/state/mosaic-credential-escalations}`, because the stderr diagnostic is transient and something durable has to exist for whoever debugs it later. | identity resolves | token in its store | host runs a fleet | result | | --- | --- | --- | --- | | yes | yes | — | that identity + token | | yes | no | — | **fail closed** | | no | — | yes | **fail closed** | | no | — | no | shared account, unchanged | A host "runs a fleet" when `<brain>/fleet/agents` exists — the same signal `packages/mosaic/src/fleet/brain-home.ts` already uses to decide a brain is active, resolved the same way (`MOSAIC_BRAIN_HOME`, else `~/.mosaic`). That gate is what keeps this a **no-op for anyone who has not provisioned per-slot tokens**: no fleet directory, shared account, unchanged. It is also why there is deliberately no environment variable to restore the old behavior — one would reintroduce the substitution being removed. ### Store selection Both readers hardcoded `~/.config/mosaic/secrets/gitea-tokens`, so a seat's own `secrets/` slot was invisible to the framework: a seat could hold a perfectly valid credential and still be served the shared account. The store is now chosen by what the identity **is**: | The identity | Its credential is read from | | --- | --- | | has a directory at `<brain>/fleet/agents/<id>/` — a **seat** | `<brain>/fleet/agents/<id>/secrets/gitea-{usc,mosaicstack}-<id>.token` | | does not — a **service identity** | `~/.config/mosaic/secrets/gitea-tokens/gitea-{usc,mosaicstack}-<id>.token` | No precedence between them and no fallback from one to the other. A seat with an empty slot is refused even when a same-named token sits in the framework store. Two copies of one credential are drift rather than redundancy, and drift surfaces as the stale copy returning 401 — which reads as a revoked token and sends whoever debugs it somewhere else entirely. ### Why `detect-platform.sh` is in scope too It is the second reader of these tokens. Patching only the git helper would make "one credential, one location" true for push and fetch and false for `pr-create.sh`, `issue-create.sh` and `pr-review.sh` — the harder failure to notice, because those are the tools that create attributable records. ## Tests The three assertions that pinned the shared-account fall-through are now fail-closed assertions. A refusal is checked four independent ways: nonzero exit, empty stdout, a stderr diagnostic naming identity and host, and **no shared token value anywhere in the output**. The exit code alone would pass against a helper that emitted the credential and then failed. Added coverage: - seat-slot resolution - the no-cross-store-fallback case, with a control proving the framework-store file it declines to read *is* readable — so the refusal is the store rule firing, not an unreadable file - no-identity on a fleet host - the fleet gate firing on the default `~/.mosaic`, not only on an injected `MOSAIC_BRAIN_HOME` - a cross-host leak check (an agent holding a usc token must not have it surface on a mosaicstack request) Both suites were also run against the pre-change code as a control and fail there on exactly the shared-token emission, so the green is discriminating rather than vacuous. Both tests were already enumerated in `test:framework-shell`; no wiring change. `verify-sanitized.sh` passes, and was seeded with a denylist hit to confirm it can go red. ## Known gap `shellcheck` is not installed on the authoring host, so the rewritten helper is unlinted locally — CI is the first lint of it. ## Operator note On a host that runs a fleet, a human doing manual git work now needs an identity of their own: `MOSAIC_GIT_IDENTITY=<id>` with a provisioned slot. That is the intended consequence, not a side effect.
fred added 1 commit 2026-08-18 21:29:31 +00:00
Two changes to one rule: a credential is resolved from exactly one place,
and an identity that cannot be resolved is refused rather than substituted.

FAIL CLOSED. Both readers ended in an unconditional fall-through to the
shared Gitea account whenever an identity did not resolve. Every seat in a
fleet therefore pushed, opened PRs and filed reviews under one account, and
a record made that way cannot be traced to the agent that made it
afterwards. The fallback now applies only where there is no attribution to
lose: a host with no fleet. Where seats exist, an unresolvable request emits
nothing, exits nonzero, explains itself on stderr, and — in the git helper —
appends a record naming the identity, host, reason and cwd, and no token
value, to ${MOSAIC_CREDENTIAL_SPOOL:-~/.local/state/mosaic-credential-escalations}.

A host runs a fleet when <brain>/fleet/agents exists, which is the signal
packages/mosaic/src/fleet/brain-home.ts already uses to decide a brain is
active, resolved the same way (MOSAIC_BRAIN_HOME, else ~/.mosaic). This is
what keeps the change a no-op for an operator who has not provisioned
per-slot tokens: no fleet directory, shared account, unchanged. It is also
why there is no environment variable to restore the old behavior — one would
reintroduce the substitution being removed.

STORE SELECTION. Both readers hardcoded ~/.config/mosaic/secrets/gitea-tokens,
so a seat's own secrets/ slot was invisible to the framework: a seat could
hold a valid credential and still be served the shared account. The store is
now chosen by what the identity is. An identity with a directory under
<brain>/fleet/agents/ is a seat and is read only from
<brain>/fleet/agents/<id>/secrets/; any other identity is a service identity
and is read from the framework store. There is no precedence between them
and no fallback from one to the other, so a seat with an empty slot is
refused even when a same-named token sits in the framework store. Two copies
of one credential are drift rather than redundancy, and drift surfaces as
the stale copy returning 401, which reads as a revoked token and sends
whoever debugs it somewhere else.

detect-platform.sh is in scope alongside git-credential-mosaic because they
are the two readers of these tokens. Patching only the git helper would make
"one credential, one location" true for push and fetch and false for
pr-create.sh, issue-create.sh and pr-review.sh, which is the harder failure
to notice.

TESTS. The three assertions that pinned the shared-account fall-through are
now fail-closed assertions, and a refusal is checked four independent ways:
nonzero exit, empty stdout, a stderr diagnostic naming identity and host,
and no shared token value anywhere in the output. The exit code alone would
pass against a helper that emitted the credential and then failed. Added:
seat-slot resolution, the no-cross-store-fallback case with a control
proving the framework-store file it declines to read is readable, no-identity
on a fleet host, the fleet gate firing on the default ~/.mosaic and not only
on an injected MOSAIC_BRAIN_HOME, and a cross-host leak check. Both suites
were run against the pre-change code as a control and fail there on exactly
the shared-token emission.

shellcheck is not installed on the authoring host, so the rewritten helper
is unlinted locally and CI is the first lint of it.
rev-code-01 requested changes 2026-08-18 21:57:40 +00:00
Dismissed
rev-code-01 left a comment
Collaborator

Review by rev-code-01 (Gate-16: independent of author). Head reviewed: 3d2b712 — content on the remote ref.

Verdict: Request changes — one blocker, minor should-fixes.

[BLOCKER] CI red on head sha. Pipeline 2508 format step exit 1: prettier --check flags packages/mosaic/framework/tools/git/README.md ("Code style issues found"). All other steps green — including lint (shellcheck; your known gap resolved green in CI) and test. A red pipeline cannot merge. Fix: prettier --write that file, push, terminal-green before merge.

[SHOULD FIX] git-credential-mosaic prints record: ${spool}/YYYYMMDD.jsonl unconditionally in the fail-closed stderr — even when the spool write was skipped because mkdir -p "$spool" failed. The diagnostic claims a durable record that does not exist. Guard the line on spool success.

[SHOULD FIX] Spool JSONL is printf'd with unescaped $PWD / $ident / $MOSAIC_AGENT_NAME — a value containing " produces an invalid JSON line in the operator-facing record.

[SUGGESTION] Dedupe marker ([ ! -e ] then create) is racy under concurrent helpers; duplicate records are harmless, an O_EXCL-style create closes it.

Deploy impact (intended, but flag at rollout): on fleet hosts every no-identity caller of get_gitea_token() now fails closed — ci-queue-wait.sh, issue-{create,close,reopen,view}.sh, pr-{create,merge,diff,metadata,ci-wait}.sh, and humans using the shared account. Cron/service jobs must set MOSAIC_GIT_IDENTITY before this deploys.

Verified:

  • Both suites green at 3d2b712 locally (full tools/ tree): cred-mosaic rc=0, token-identity rc=0.
  • Control: the same new tests against base 245e0c42 go red — 10 FAILs in the credential suite, all naming the shared-token emission; identity suite aborts red at case 7. The green is discriminating.
  • Both suites confirmed wired in test:framework-shell (CI test step ran them, green).
  • brain-home.ts parity claim verified (MOSAIC_BRAIN_HOME else default; fleet/agents existence signal).
  • No token values on any refusal path (code-read + the leak assertions).
  • Unknown-host passthrough preserved (tests 12/13).

Everything else — fail-closed matrix, seat-slot store selection with no cross-store fallback, cross-host leak refusal — behaves as described and is properly tested. After the prettier fix (and optionally F2/F3), this is approve-ready from my side.

**Review by rev-code-01** (Gate-16: independent of author). Head reviewed: `3d2b712` — content on the remote ref. **Verdict: Request changes — one blocker, minor should-fixes.** **[BLOCKER] CI red on head sha.** Pipeline 2508 `format` step exit 1: `prettier --check` flags `packages/mosaic/framework/tools/git/README.md` ("Code style issues found"). All other steps green — including `lint` (shellcheck; your known gap resolved green in CI) and `test`. A red pipeline cannot merge. Fix: prettier --write that file, push, terminal-green before merge. **[SHOULD FIX]** `git-credential-mosaic` prints `record: ${spool}/YYYYMMDD.jsonl` unconditionally in the fail-closed stderr — even when the spool write was skipped because `mkdir -p "$spool"` failed. The diagnostic claims a durable record that does not exist. Guard the line on spool success. **[SHOULD FIX]** Spool JSONL is printf'd with unescaped `$PWD` / `$ident` / `$MOSAIC_AGENT_NAME` — a value containing `"` produces an invalid JSON line in the operator-facing record. **[SUGGESTION]** Dedupe marker (`[ ! -e ]` then create) is racy under concurrent helpers; duplicate records are harmless, an O_EXCL-style create closes it. **Deploy impact (intended, but flag at rollout):** on fleet hosts every no-identity caller of `get_gitea_token()` now fails closed — ci-queue-wait.sh, issue-{create,close,reopen,view}.sh, pr-{create,merge,diff,metadata,ci-wait}.sh, and humans using the shared account. Cron/service jobs must set `MOSAIC_GIT_IDENTITY` before this deploys. **Verified:** - Both suites green at `3d2b712` locally (full `tools/` tree): cred-mosaic rc=0, token-identity rc=0. - Control: the same new tests against base `245e0c42` go red — 10 FAILs in the credential suite, all naming the shared-token emission; identity suite aborts red at case 7. The green is discriminating. - Both suites confirmed wired in `test:framework-shell` (CI test step ran them, green). - `brain-home.ts` parity claim verified (MOSAIC_BRAIN_HOME else default; `fleet/agents` existence signal). - No token values on any refusal path (code-read + the leak assertions). - Unknown-host passthrough preserved (tests 12/13). Everything else — fail-closed matrix, seat-slot store selection with no cross-store fallback, cross-host leak refusal — behaves as described and is properly tested. After the prettier fix (and optionally F2/F3), this is approve-ready from my side.
fred added 1 commit 2026-08-18 22:05:02 +00:00
Both defects found in review by rev-code-01 on #1311.

F3 — the JSONL record interpolated every field with a bare %s. An identity comes
from git config or the environment and a cwd is whatever directory git ran in, so
either can contain a quote or a backslash. One such refusal turned the day's spool
into unparseable JSONL, and the operator would only discover it while reading the
record that explains an outage. Fields are now JSON-escaped.

F2 — the diagnostic printed "record: <spool>/<date>.jsonl" unconditionally, but
the record is only written inside the branch where mkdir -p succeeded. When the
spool cannot be created the helper named a file that does not exist, on exactly
the hosts where the escalation was lost. It now reports the real path or says
NOT WRITTEN.

Also: prettier on README.md, which was the format-step failure on pipeline 2508.
It reflowed only the two tables this branch added.

Tests: cases 14 and 15 cover both. Verified discriminating — against the previous
helper with these same tests, case 14 fails with the unparseable record printed
and case 15 fails on both assertions; against this one both pass.

The first draft of case 14 used `ls "$spool"/*.jsonl | head -1`, which under
`set -o pipefail` exits 2 on a missed glob and killed the suite with zero output
— the same silent-nonzero failure rev-code-01 hit from a partial tools/ extraction
and the reason this file exists. Replaced with a glob loop and a comment.
rev-code-01 approved these changes 2026-08-18 22:17:12 +00:00
rev-code-01 left a comment
Collaborator

Re-review by rev-code-01 at head c703cc50 (content-verified on origin; PR head sha matches). Approving.

All findings from my previous review (id 189) resolved:

  • B1 (was BLOCKER): format red. Pipeline 2509 terminal green on c703cc50 — all 9 steps success, including format and test. README delta confirmed prettier table reflow only, no unrelated churn.
  • F2: spool-record line. Verified in code (spool_record set only when the jsonl is non-empty on disk; else NOT WRITTEN — spool unavailable at <path>) and by test 15's two assertions against /proc/..., which cannot be created. Correct.
  • F3: unescaped JSONL. json_escape covers backslash, quote, tab, CR, LF — the classes that can occur in a git-config identity or cwd. My own probe beyond test 14's inputs: a single string carrying all five round-trips through Python's json parser exactly.

Verification run just now (pipe-free, full tools/ tree extracted at c703cc50):

  • test-git-credential-mosaic.sh rc=0 (cases 1-15), test-gitea-token-identity.sh rc=0.
  • Control: new tests + the OLD helper (3d2b712) go red on exactly the three new assertions — case 14 prints the unparseable record, case 15 fails both. Discriminating as claimed.

F4 (dedupe race) stays a non-blocking suggestion — duplicate records are harmless.

R3 hardening (_lib-absent silent suite death): agreed it belongs outside this PR. Filed as #1312 as the reviewer who measured it.

Merge-ready from my side once the queue guard is run.

**Re-review by rev-code-01 at head `c703cc50`** (content-verified on origin; PR head sha matches). Approving. All findings from my previous review (id 189) resolved: - **B1 (was BLOCKER): format red.** Pipeline 2509 terminal green on `c703cc50` — all 9 steps success, including `format` and `test`. README delta confirmed prettier table reflow only, no unrelated churn. - **F2: spool-record line.** Verified in code (`spool_record` set only when the jsonl is non-empty on disk; else `NOT WRITTEN — spool unavailable at <path>`) and by test 15's two assertions against `/proc/...`, which cannot be created. Correct. - **F3: unescaped JSONL.** `json_escape` covers backslash, quote, tab, CR, LF — the classes that can occur in a git-config identity or cwd. My own probe beyond test 14's inputs: a single string carrying all five round-trips through Python's `json` parser exactly. Verification run just now (pipe-free, full `tools/` tree extracted at `c703cc50`): - `test-git-credential-mosaic.sh` rc=0 (cases 1-15), `test-gitea-token-identity.sh` rc=0. - Control: new tests + the OLD helper (3d2b712) go red on exactly the three new assertions — case 14 prints the unparseable record, case 15 fails both. Discriminating as claimed. F4 (dedupe race) stays a non-blocking suggestion — duplicate records are harmless. R3 hardening (`_lib`-absent silent suite death): agreed it belongs outside this PR. Filed as #1312 as the reviewer who measured it. Merge-ready from my side once the queue guard is run.
jason.woltje merged commit d4d32a80b2 into next 2026-08-18 22:40:47 +00:00
Sign in to join this conversation.