From c703cc50ebeb0caedecc992a89db135b0db29033 Mon Sep 17 00:00:00 2001 From: fred Date: Tue, 18 Aug 2026 17:04:55 -0500 Subject: [PATCH] git-credential-mosaic: escape the escalation record, and stop naming a record that was never written MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: /.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. --- packages/mosaic/framework/tools/git/README.md | 20 +++--- .../framework/tools/git/git-credential-mosaic | 35 +++++++++- .../tools/git/test-git-credential-mosaic.sh | 70 ++++++++++++++++++- 3 files changed, 111 insertions(+), 14 deletions(-) diff --git a/packages/mosaic/framework/tools/git/README.md b/packages/mosaic/framework/tools/git/README.md index 9dd6e05a..377c913c 100644 --- a/packages/mosaic/framework/tools/git/README.md +++ b/packages/mosaic/framework/tools/git/README.md @@ -55,10 +55,10 @@ identity**: The store is chosen by what the identity **is**, not by which file happens to exist first: -| The identity | Its credential is read from | -| --- | --- | -| has a directory at `/fleet/agents//` — it is a **seat** | `/fleet/agents//secrets/gitea-{usc,mosaicstack}-.token` | -| does not — it is a **service identity** | `~/.config/mosaic/secrets/gitea-tokens/gitea-{usc,mosaicstack}-.token` | +| The identity | Its credential is read from | +| ------------------------------------------------------------------ | -------------------------------------------------------------------------- | +| has a directory at `/fleet/agents//` — it is a **seat** | `/fleet/agents//secrets/gitea-{usc,mosaicstack}-.token` | +| does not — it is a **service identity** | `~/.config/mosaic/secrets/gitea-tokens/gitea-{usc,mosaicstack}-.token` | `` is `MOSAIC_BRAIN_HOME` if set, else `~/.mosaic` — the same resolution `packages/mosaic/src/fleet/brain-home.ts` performs. @@ -71,12 +71,12 @@ revoked token and sends whoever debugs it to the wrong place. ### What happens when nothing resolves -| 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 | +| 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 `/fleet/agents` exists — the same signal `brain-home.ts` uses to decide a brain is active. diff --git a/packages/mosaic/framework/tools/git/git-credential-mosaic b/packages/mosaic/framework/tools/git/git-credential-mosaic index 87fa54d6..931a70df 100755 --- a/packages/mosaic/framework/tools/git/git-credential-mosaic +++ b/packages/mosaic/framework/tools/git/git-credential-mosaic @@ -141,18 +141,42 @@ ts=$(date -u +%Y-%m-%dT%H:%M:%SZ) # top of it is best-effort. Record and alert are deduplicated separately — a cap # on the alert alone lets the spool grow without bound exactly while the operator # is being told nothing, so the louder the failure the quieter it gets. +# +# A record field is arbitrary operator-supplied text: an identity comes from git +# config or the environment, and cwd is whatever directory git ran in. Either can +# contain a quote or a backslash, which would make the line unparseable JSON -- +# and a spool that silently stops parsing is worse than no spool, because the +# operator only discovers it while reading the record that explains an outage. +json_escape() { + local s=$1 + s=${s//\\/\\\\} + s=${s//\"/\\\"} + s=${s//$'\t'/\\t} + s=${s//$'\r'/\\r} + s=${s//$'\n'/\\n} + printf '%s' "$s" +} + spool="${MOSAIC_CREDENTIAL_SPOOL:-$HOME/.local/state/mosaic-credential-escalations}" +spool_record="" if mkdir -p "$spool" 2>/dev/null; then chmod 700 "$spool" 2>/dev/null + spoolfile="$spool/$(date -u +%Y%m%d).jsonl" dedupe="$spool/.spooled-${seat}-${ident:-none}-${reason}-$(date -u +%Y%m%d%H%M)" if [ ! -e "$dedupe" ]; then : > "$dedupe" 2>/dev/null - spoolfile="$spool/$(date -u +%Y%m%d).jsonl" printf '{"ts":"%s","reason":"%s","identity":"%s","identity_source":"%s","kind":"%s","seat":"%s","host":"%s","cwd":"%s"}\n' \ - "$ts" "$reason" "${ident:-}" "$ident_src" "${ident_kind:-none}" "$seat" "$host" "$PWD" \ + "$(json_escape "$ts")" "$(json_escape "$reason")" \ + "$(json_escape "${ident:-}")" "$(json_escape "$ident_src")" \ + "$(json_escape "${ident_kind:-none}")" "$(json_escape "$seat")" \ + "$(json_escape "$host")" "$(json_escape "$PWD")" \ >> "$spoolfile" 2>/dev/null chmod 600 "$spoolfile" 2>/dev/null fi + # Name the record only if one is actually on disk. Printing the path + # unconditionally sends the operator to a file that does not exist on exactly + # the hosts where the spool could not be created. + [ -s "$spoolfile" ] && spool_record="$spoolfile" find "$spool" -maxdepth 1 -name '.spooled-*' -mmin +120 -delete 2>/dev/null fi @@ -186,6 +210,11 @@ seat and is read ONLY from its own secrets/ slot; any other identity is read fro If this identity legitimately needs git access and has none, ask the orchestrator to provision one. - record: ${spool}/$(date -u +%Y%m%d).jsonl EOF + +if [ -n "$spool_record" ]; then + echo " record: ${spool_record}" >&2 +else + echo " record: NOT WRITTEN — spool unavailable at ${spool}" >&2 +fi exit 1 diff --git a/packages/mosaic/framework/tools/git/test-git-credential-mosaic.sh b/packages/mosaic/framework/tools/git/test-git-credential-mosaic.sh index 92b0ad0f..6c18c3cd 100755 --- a/packages/mosaic/framework/tools/git/test-git-credential-mosaic.sh +++ b/packages/mosaic/framework/tools/git/test-git-credential-mosaic.sh @@ -290,13 +290,81 @@ assert_eq "unknown host on a fleet host: still passthrough, not a refusal" "" "$ # --------------------------------------------------------------------------- store_out=$(cd "$REPO_DIR" && env -i HOME="$FAKE_HOME" PATH="$PATH" bash "$HELPER" store </dev/null 2>&1 +host=git.mosaicstack.dev +username=no-such-agent + +EOF +) || true +# Deliberately not `ls ... | head -1`: under `set -o pipefail` a missed glob +# makes ls exit 2, the pipeline inherits it, and `set -e` kills this suite with +# zero output — the same silent-nonzero failure this file exists to catch. +record_file="" +for candidate in "$hostile_spool"/*.jsonl; do + if [[ -e "$candidate" ]]; then + record_file="$candidate" + break + fi +done +if [[ -z "$record_file" ]]; then + echo "FAIL: hostile cwd — no escalation record was written at all" >&2 + fail=1 +elif ! python3 -c 'import json,sys +for line in open(sys.argv[1]): + line = line.strip() + if line: + json.loads(line)' "$record_file" 2>/dev/null; then + echo "FAIL: hostile cwd — escalation record is not parseable JSONL:" >&2 + cat "$record_file" >&2 + fail=1 +fi + +# --------------------------------------------------------------------------- +# 15. When the spool cannot be created, the diagnostic must NOT name a record +# path. Naming a file that was never written sends the operator to an +# empty path on exactly the hosts where the escalation was lost. +# --------------------------------------------------------------------------- +unwritable_spool="/proc/mosaic-credential-spool-cannot-exist" +nospool_err=$( + cd "$REPO_DIR" + env -i HOME="$FAKE_HOME" PATH="$PATH" MOSAIC_CREDENTIAL_SPOOL="$unwritable_spool" \ + MOSAIC_GIT_IDENTITY=no-such-agent \ + bash "$HELPER" get <&1 >/dev/null +host=git.mosaicstack.dev +username=no-such-agent + +EOF +) || true +if [[ "$nospool_err" == *"record: $unwritable_spool/"* ]]; then + echo "FAIL: unwritable spool — diagnostic names a record file that was never written" >&2 + fail=1 +fi +if [[ "$nospool_err" != *"NOT WRITTEN"* ]]; then + echo "FAIL: unwritable spool — diagnostic does not say the record was not written" >&2 + echo "$nospool_err" >&2 + fail=1 +fi + if [[ "$fail" -eq 0 ]]; then echo "git-credential-mosaic identity resolution regression passed" fi