fix(#1367): close both secret channels — trap-swept staging and file-to-file assembly (review 263)
ci/woodpecker/pr/ci Pipeline was successful
ci/woodpecker/pr/ci Pipeline was successful
Blocker 2 (secret at rest on error paths): all staging now lives in ONE per-run mktemp -d removed by an EXIT/INT/TERM trap; a curl dying rc=7 mid-run (the reviewer's transport-failure case) leaves nothing behind. M10 pins it against a dying mock in an isolated TMPDIR; trap-removed mutant killed. Blocker 1 (bash -x trace channel, upheld above landed parity because this is the admin-token minter): secrets are assembled FILE-TO-FILE — stage_auth/stage_user take token/password FILE PATHS and build the curl configs with jq --rawfile; the password is generated straight into its staging file; bodies are composed by jq from the template + password file. No secret is ever expanded into a shell word a trace would print. M11 runs a real bash -x and asserts the admin-token value, the minted token value, and any password-shaped 32-char expansion are all absent; expansion mutant killed (measured: the mutant's trace shows '+ PW_VALUE=<32 chars>', the fixed script's trace shows paths only). Header comment corrected to state what is actually true, including the explicit note that detect-platform's gitea_write_auth_config still leaks under -x — that parity gap is now tracked as #1369, opened per review 263 and fred's ruling; issue-comment/pr-review/pr-edit left untouched in this PR.
This commit is contained in:
@@ -33,34 +33,59 @@
|
||||
# The .scopes file is written from the mint RESPONSE rather than from what was
|
||||
# requested, so the record is what was granted rather than what was asked for.
|
||||
#
|
||||
# SECRETS NEVER TOUCH ARGV (#1343 class, rev-security-01 review 259): the admin
|
||||
# token, the generated password, and the minted seat token all pass through
|
||||
# 0600 curl --config / --data files — the landed in-tree standard
|
||||
# (gitea_write_auth_config in detect-platform.sh). argv is world-readable via
|
||||
# /proc/<pid>/cmdline for the life of each request, and a bash -x trace would
|
||||
# print every secret otherwise. The staging files are unlinked after each use.
|
||||
# SECRETS NEVER TOUCH ARGV (#1343 class, rev-security-01 review 259), and the
|
||||
# staging area is a single trap-swept directory (review 263 blocker 2): the
|
||||
# admin token, the generated password, and the minted seat token all pass
|
||||
# through 0600 files under a per-run staging dir removed by an EXIT/INT/TERM
|
||||
# trap, so a transport failure mid-run cannot leave secrets at rest in /tmp.
|
||||
#
|
||||
# TRACE CHANNEL, stated plainly (review 263 blocker 1): this script is held to
|
||||
# a higher bar than ordinary wrappers because it mints admin-grade
|
||||
# credentials and a durable password. Secrets here are assembled FILE-TO-FILE
|
||||
# — source token file, password generated straight into its staging file,
|
||||
# bodies composed with jq from those files — so no secret is ever expanded
|
||||
# into a shell word a trace would print. A plain `bash -x` of this script
|
||||
# shows staging PATHS only. (The landed gitea_write_auth_config in
|
||||
# detect-platform.sh still expands tokens into shell words and DOES leak
|
||||
# under -x; that fleet-wide parity gap is tracked in its own issue — see the
|
||||
# framework-hardening issue referenced from this PR.)
|
||||
set -Eeuo pipefail
|
||||
|
||||
# Stage secrets into 0600 files; nothing secret reaches argv or a trace.
|
||||
# write_auth_config <token> -> curl --config carrying the Authorization header
|
||||
# (same shape as gitea_write_auth_config in
|
||||
# detect-platform.sh, local so this script stays
|
||||
# standalone under tools/fleet).
|
||||
# write_user_config <u> <pw> -> curl --config with `user =` (covers -u).
|
||||
# write_body <json> -> 0600 file for --data @file.
|
||||
write_auth_config() {
|
||||
local f; f=$(mktemp "${TMPDIR:-/tmp}/mosaic-mint-auth.XXXXXX") || return 1
|
||||
printf 'header = "Authorization: token %s"\n' "$1" >"$f" || { rm -f "$f"; return 1; }
|
||||
STAGE_DIR=""
|
||||
cleanup_stage() {
|
||||
[ -n "$STAGE_DIR" ] && rm -rf -- "$STAGE_DIR"
|
||||
STAGE_DIR=""
|
||||
}
|
||||
trap cleanup_stage EXIT INT TERM
|
||||
|
||||
# All staging lives in one per-run dir, swept by the trap above. Files are
|
||||
# created 0600 and secrets are moved between them only by tool reads
|
||||
# (jq/cat), never through shell-word expansion.
|
||||
new_stage() { STAGE_DIR="$(mktemp -d "${TMPDIR:-/tmp}/mosaic-mint.XXXXXX")"; chmod 700 "$STAGE_DIR"; }
|
||||
|
||||
# stage_auth <token-file> — curl --config carrying the Authorization header,
|
||||
# reading the token from the file with jq so it never
|
||||
# becomes a shell word.
|
||||
# stage_user <seat> <pw-file> — curl --config with `user =`; the password is
|
||||
# read from its file by jq. <seat> is not a secret.
|
||||
# stage_body <template-json> <pw-file> — body file; jq injects the password
|
||||
# file's value into the template. The mint body has
|
||||
# no secret and is written directly.
|
||||
stage_auth() {
|
||||
local f="$STAGE_DIR/auth.cfg"
|
||||
jq -rn --rawfile t "$1" '"header = \"Authorization: token " + $t + "\""' >"$f" || return 1
|
||||
chmod 600 "$f"; printf '%s' "$f"
|
||||
}
|
||||
write_user_config() {
|
||||
local f; f=$(mktemp "${TMPDIR:-/tmp}/mosaic-mint-user.XXXXXX") || return 1
|
||||
printf 'user = "%s:%s"\n' "$1" "$2" >"$f" || { rm -f "$f"; return 1; }
|
||||
stage_user() {
|
||||
local f="$STAGE_DIR/user.cfg"
|
||||
jq -rn --rawfile p "$2" --arg u "$1" '"user = \"" + $u + ":" + $p + "\""' >"$f" || return 1
|
||||
chmod 600 "$f"; printf '%s' "$f"
|
||||
}
|
||||
write_body() {
|
||||
local f; f=$(mktemp "${TMPDIR:-/tmp}/mosaic-mint-body.XXXXXX") || return 1
|
||||
printf '%s' "$1" >"$f" || { rm -f "$f"; return 1; }
|
||||
stage_body() {
|
||||
# $1 is a JSON template string (no secrets); $2 is the password file. jq
|
||||
# parses the template and injects the password read straight from the file.
|
||||
local f="$STAGE_DIR/body.json"
|
||||
jq -c --rawfile p "$2" '.password = $p' <<<"$1" >"$f" || return 1
|
||||
chmod 600 "$f"; printf '%s' "$f"
|
||||
}
|
||||
|
||||
@@ -114,34 +139,37 @@ for KEY in $INSTANCES; do
|
||||
[[ -n "$BASE" ]] || { echo " $KEY: no URL known for this instance (set $ov), skipped" >&2; rc=1; continue; }
|
||||
ADMIN_TOKEN_FILE="$BRAIN/fleet/agents/$ADMIN/secrets/gitea-$KEY-$ADMIN.token"
|
||||
[[ -r "$ADMIN_TOKEN_FILE" ]] || { echo " $KEY: no admin token for seat '$ADMIN' ($ADMIN_TOKEN_FILE), skipped" >&2; rc=1; continue; }
|
||||
T="$(cat "$ADMIN_TOKEN_FILE")"
|
||||
AUTH_CFG="$(write_auth_config "$T")"
|
||||
PW="$(openssl rand -base64 33 | tr -d '\n/+=' | head -c 32)"
|
||||
USER_CFG="$(write_user_config "$SEAT" "$PW")"
|
||||
# Per-instance staging dir: everything under it dies with the trap, so a
|
||||
# transport failure (review 263 blocker 2) cannot leave secrets at rest.
|
||||
new_stage
|
||||
AUTH_CFG="$(stage_auth "$ADMIN_TOKEN_FILE")"
|
||||
# The password is generated STRAIGHT INTO its staging file; the variable
|
||||
# below is its path, never the value (review 263 blocker 1).
|
||||
openssl rand -base64 33 | tr -d '\n/+=' | head -c 32 >"$STAGE_DIR/pw"
|
||||
chmod 600 "$STAGE_DIR/pw"
|
||||
USER_CFG="$(stage_user "$SEAT" "$STAGE_DIR/pw")"
|
||||
|
||||
if curl -sf -o /dev/null --config "$AUTH_CFG" "$BASE/api/v1/users/$SEAT"; then
|
||||
BODY="$(write_body "{\"login_name\":\"$SEAT\",\"source_id\":0,\"password\":\"$PW\",\"must_change_password\":false}")"
|
||||
BODY="$(stage_body '{"login_name":"'"$SEAT"'","source_id":0,"password":"","must_change_password":false}' "$STAGE_DIR/pw")"
|
||||
curl -s -o /dev/null -X PATCH -H "Content-Type: application/json" \
|
||||
--config "$AUTH_CFG" --data "@$BODY" \
|
||||
"$BASE/api/v1/admin/users/$SEAT"
|
||||
rm -f "$BODY"; BODY=""
|
||||
act="reset-pw"
|
||||
else
|
||||
BODY="$(write_body "{\"username\":\"$SEAT\",\"email\":\"$SEAT@$EMAIL_DOMAIN\",\"password\":\"$PW\",\"must_change_password\":false,\"full_name\":\"Mosaic fleet seat $SEAT\"}")"
|
||||
BODY="$(stage_body '{"username":"'"$SEAT"'","email":"'"$SEAT@$EMAIL_DOMAIN"'","password":"","must_change_password":false,"full_name":"Mosaic fleet seat '"$SEAT"'"}' "$STAGE_DIR/pw")"
|
||||
curl -s -o /dev/null -X POST -H "Content-Type: application/json" \
|
||||
--config "$AUTH_CFG" --data "@$BODY" \
|
||||
"$BASE/api/v1/admin/users"
|
||||
rm -f "$BODY"; BODY=""
|
||||
act="create"
|
||||
fi
|
||||
|
||||
tmp="$(mktemp)"; chmod 600 "$tmp"
|
||||
MINT_BODY="$(write_body "{\"name\":\"mosaic-seat\",\"scopes\":$SCOPES}")"
|
||||
tmp="$STAGE_DIR/mint-response.json"
|
||||
printf '{"name":"mosaic-seat","scopes":%s}' "$SCOPES" >"$STAGE_DIR/mint-body.json"; chmod 600 "$STAGE_DIR/mint-body.json"
|
||||
MINT_BODY="$STAGE_DIR/mint-body.json"
|
||||
code="$(curl -s -o "$tmp" -w '%{http_code}' -X POST -H "Content-Type: application/json" \
|
||||
--config "$USER_CFG" --data "@$MINT_BODY" "$BASE/api/v1/users/$SEAT/tokens")"
|
||||
rm -f "$MINT_BODY"; MINT_BODY=""
|
||||
if [[ "$code" != "201" ]]; then
|
||||
echo " $KEY: mint FAILED http=$code ($act)" >&2; rm -f "$tmp"; rc=1; PW=""; rm -f "$AUTH_CFG" "$USER_CFG"; continue
|
||||
echo " $KEY: mint FAILED http=$code ($act)" >&2; rm -f "$tmp"; rc=1; cleanup_stage; continue
|
||||
fi
|
||||
|
||||
python3 - "$tmp" "$D" "$KEY" "$SEAT" <<'PY'
|
||||
@@ -155,12 +183,13 @@ p=pathlib.Path(d)
|
||||
for suf in ("token","scopes","principal"):
|
||||
(p/f"gitea-{key}-{seat}.{suf}").chmod(0o600)
|
||||
PY
|
||||
rm -f "$tmp"; PW=""; rm -f "$AUTH_CFG" "$USER_CFG"
|
||||
rm -f "$tmp"; cleanup_stage
|
||||
|
||||
VERIFY_CFG="$(write_auth_config "$(cat "$D/gitea-$KEY-$SEAT.token")")"
|
||||
new_stage # fresh staging for the verify read
|
||||
VERIFY_CFG="$(stage_auth "$D/gitea-$KEY-$SEAT.token")"
|
||||
login="$(curl -s --config "$VERIFY_CFG" "$BASE/api/v1/user" \
|
||||
| python3 -c 'import json,sys;print(json.load(sys.stdin).get("login","ERR"))' 2>/dev/null || echo ERR)"
|
||||
rm -f "$VERIFY_CFG"
|
||||
cleanup_stage
|
||||
if [[ "$login" == "$SEAT" ]]; then
|
||||
echo " $KEY: $act, minted, GET /user -> $login"
|
||||
else
|
||||
|
||||
@@ -22,6 +22,11 @@
|
||||
# variable, matching seat-logins.sh (SF3).
|
||||
# M9 MOSAIC_SEAT_EMAIL_DOMAIN is required: unset is a usage error (rc=3),
|
||||
# nothing written (framework-PR firewall answer).
|
||||
# M10 no secret at rest after ANY exit, including transport failure mid-run
|
||||
# (review 263 blocker 2): the staging dir is swept by the trap, so a curl
|
||||
# that dies rc=7 on the admin POST leaves nothing behind.
|
||||
# M11 a real `bash -x` trace of the whole run contains no secret VALUE —
|
||||
# staging appears only as paths (review 263 blocker 1).
|
||||
set -euo pipefail
|
||||
|
||||
WORK_ROOT="${AGENT_WORK_ROOT:-${TMPDIR:-/tmp}}"
|
||||
@@ -43,7 +48,7 @@ chmod 600 "$BRAIN/fleet/agents/admin-seat/secrets/gitea-alpha-admin-seat.token"
|
||||
|
||||
# A PATH with only the mock bin plus the system tools the script needs, and no tea.
|
||||
SYS_BIN="$SANDBOX/sys"; mkdir -p "$SYS_BIN"
|
||||
for t in bash sed cat mktemp openssl tr head python3 sort printf chmod mkdir rm dirname grep stat; do
|
||||
for t in bash sed cat mktemp openssl tr head python3 sort printf chmod mkdir rm dirname grep stat jq find wc; do
|
||||
p="$(command -v "$t" 2>/dev/null || true)"; [ -n "$p" ] && ln -s "$p" "$SYS_BIN/$t"
|
||||
done
|
||||
export PATH="$MOCK_BIN:$SYS_BIN" CALLS
|
||||
@@ -158,4 +163,85 @@ grep -q "no admin token for seat 'other-admin'" "$SANDBOX/err" || fail "M2: miss
|
||||
[ ! -s "$CALLS" ] || fail "M2: API was called without an admin token: $(cat "$CALLS")"
|
||||
[ ! -e "$BRAIN/fleet/agents/newseat/secrets/gitea-alpha-newseat.token" ] || fail "M2: token written without an admin token"
|
||||
|
||||
# M10: transport failure mid-run leaves NO secret at rest (review 263 blocker 2).
|
||||
# A second mock that dies rc=7 on the admin POST; the trap must sweep the staging dir.
|
||||
rm -rf "$BRAIN/fleet/agents/newseat"
|
||||
M10_TMP="$SANDBOX/m10-tmp"; mkdir -p "$M10_TMP"
|
||||
cat > "$MOCK_BIN/curl" <<'EOF'
|
||||
#!/bin/bash
|
||||
while [ $# -gt 0 ]; do
|
||||
case "$1" in
|
||||
http*) echo "$1" >> "${FAIL_URLS:?}"; exit 7 ;;
|
||||
*) shift ;;
|
||||
esac
|
||||
done
|
||||
exit 7
|
||||
EOF
|
||||
chmod +x "$MOCK_BIN/curl"
|
||||
export FAIL_URLS="$SANDBOX/failed-urls.txt"; : > "$FAIL_URLS"
|
||||
BEFORE=$(find "$M10_TMP" -maxdepth 1 -name 'mosaic-mint.*' 2>/dev/null | wc -l)
|
||||
rc=$(TMPDIR="$M10_TMP" MOSAIC_ADMIN_SEAT=admin-seat MOSAIC_GITEA_INSTANCES=alpha run newseat)
|
||||
[ "$rc" != 0 ] || fail "M10: transport failure reported rc=0"
|
||||
AFTER=$(find "$M10_TMP" -maxdepth 1 -name 'mosaic-mint.*' 2>/dev/null | wc -l)
|
||||
[ "$AFTER" -le "$BEFORE" ] || fail "M10: staging left at rest after failure: $AFTER dir(s) under $M10_TMP"
|
||||
[ -s "$FAIL_URLS" ] || fail "M10: mock never called"
|
||||
|
||||
# Restore the well-behaved mock for M11.
|
||||
cat > "$MOCK_BIN/curl" <<'EOF'
|
||||
#!/bin/bash
|
||||
method=GET; url=""; out=""; wcode=0; auth=""; body=""
|
||||
while [ $# -gt 0 ]; do
|
||||
case "$1" in
|
||||
-X) method="$2"; shift 2 ;;
|
||||
-o) out="$2"; shift 2 ;;
|
||||
-w) wcode=1; shift 2 ;;
|
||||
--config)
|
||||
if grep -q 'Authorization: token' "$2" 2>/dev/null; then auth="${auth}token,"; fi
|
||||
if grep -q '^user = ' "$2" 2>/dev/null; then auth="${auth}user,"; fi
|
||||
shift 2 ;;
|
||||
--data)
|
||||
case "$2" in
|
||||
@*) body="@file" ;;
|
||||
*) body="inline" ;;
|
||||
esac
|
||||
shift 2 ;;
|
||||
-H|-u) shift 2 ;;
|
||||
http*) url="$1"; shift ;;
|
||||
*) shift ;;
|
||||
esac
|
||||
done
|
||||
printf '%s %s auth=%s body=%s\n' "$method" "$url" "${auth:-NONE}" "$body" >> "$CALLS"
|
||||
emit() { if [ -n "$out" ]; then printf '%s' "$1" > "$out"; else printf '%s' "$1"; fi; }
|
||||
case "$method $url" in
|
||||
"GET "*/api/v1/users/newseat) exit 22 ;;
|
||||
"POST "*/api/v1/admin/users) emit '{}'; exit 0 ;;
|
||||
"POST "*/api/v1/users/newseat/tokens) emit '{"id":9,"name":"mosaic-seat","sha1":"minted-token-7f3a","scopes":["read:user","write:repository"]}'
|
||||
[ "$wcode" = 1 ] && printf '201'; exit 0 ;;
|
||||
"GET "*/api/v1/user) emit '{"login":"newseat"}'; exit 0 ;;
|
||||
*) emit '{}'; exit 0 ;;
|
||||
esac
|
||||
EOF
|
||||
chmod +x "$MOCK_BIN/curl"
|
||||
unset FAIL_URLS
|
||||
|
||||
# M11: a real bash -x trace of a full mint contains no secret VALUE (review 263
|
||||
# blocker 1). Secrets are generated straight into staging files and moved only
|
||||
# by jq reads, so only staging PATHS may appear. The mock above has no secrets,
|
||||
# so this measures the SCRIPT's word handling: the admin token sentinel and the
|
||||
# mint response token must not appear in the xtrace of a successful run.
|
||||
rm -rf "$BRAIN/fleet/agents/newseat"
|
||||
: > "$CALLS"
|
||||
TRACE="$SANDBOX/trace.log"
|
||||
MOSAIC_ADMIN_SEAT=admin-seat MOSAIC_GITEA_INSTANCES=alpha bash -x "$TARGET" newseat >"$SANDBOX/out11" 2>"$TRACE" || fail "M11: traced run failed"
|
||||
if grep -qF "$ADMIN_TOKEN_VALUE" "$TRACE"; then fail "M11: admin token value appears in xtrace"; fi
|
||||
if grep -qF 'minted-token-7f3a' "$TRACE"; then fail "M11: minted token value appears in xtrace"; fi
|
||||
# Any password-shaped expansion: the password is 32 base64ish chars. A trace
|
||||
# that expands it into a word (assignment or argument) prints exactly that
|
||||
# shape; the clean script's trace contains no 32-char base64ish run at all
|
||||
# (paths and URLs are the only long strings).
|
||||
if grep -qE "[[:space:]=\"'][A-Za-z0-9]{32}([[:space:]\"']|$)" "$TRACE"; then
|
||||
LEAK=$(grep -oE "[[:space:]=\"'][A-Za-z0-9]{32}" "$TRACE" | head -1)
|
||||
fail "M11: password-like value expansion in xtrace: $LEAK"
|
||||
fi
|
||||
|
||||
echo "mint-seat-credential regression harness passed"
|
||||
|
||||
Reference in New Issue
Block a user