generate's blob download path prints the server-derived workflow id raw in 6 error strings, while its own Saved line is gated

[ZacxDev]

Found by the round-1 adversarial audit of cli#572 (which hardens the sibling download path). Pre-existing — not introduced by #572.

internal/cmd/generate_output.go has the same renderer-sanitizes-while-its-twin-does-not split that #566 item 2 was about, one command over. #572 gates downloadBlobTo's Saved line (:434, safeTerm(target)) — so the author already treated target as untrusted — while every error string on the same function's failure paths prints it raw.

The text is genuinely server-derived

renderOutName's own doc comment states it: "🔴 BOTH INPUTS ARE UNTRUSTED. The workflow id is SERVER-supplied…". {workflow} expands to filepath.Base(strings.TrimSpace(workflowID)), outputTarget joins that into target, and downloadBlobTo derives name := filepath.Base(target). So the server's workflow id reaches the leaf of both.

cmd/civitai/main.go:57 prints err.Error() unfiltered.

Sites

line	string	carries
:408	refusing to overwrite the existing file %s	target
:419	download %s: %w	name
:433	install %s: %w	target
:388, :390, :392	blobStatusError's three arms	name

blobStatusError (:382) is the structural twin of downloadStatusError — same signature, same defer civitai.TagStatus(status, err), same arms. #572 gated that one once at the top (name = safeTerm(name)) with a comment recording that four spellings of one rule drift apart. This twin was not gated.

Deliberately NOT in scope

:413 create output directory %s: %w is correctly ungated, for the same reason #572 removed its own copy of that gate: dir is filepath.Dir(target), which is outputTarget's outDir — user-supplied via --out-dir, never the server's leaf, because outputTarget refuses any name where name != filepath.Base(name). Sanitizing it would strip user-typed bytes, which internal/saferune's rule forbids. Do not "fix" it.

Known residual, stated rather than hidden

saferune deliberately retains \n and \t, so routing these through safeTerm does not close newline line-forgery on this path either. That class is #552's, and is not part of the condition below.

Closing condition

The six sites in the table route their server-derived argument through safeTerm — blobStatusError gated once at the top like its twin, not per-arm — and safeTermCoveredBy["downloadBlobTo"] and ["blobStatusError"] carry rows whose why names them.

Checked by: go test ./internal/cmd -count=1 exits non-zero when safeTerm is removed from blobStatusError, with the failure naming that function rather than a build error.

[ZacxDev]

Closed by cli#596, squash-merged as 752bf50. Closing by hand with evidence rather than by keyword — this repo's keyword parser has fired on quoted and inherited text before (#399 twice, #569 once), so no commit or PR body in this work carried one.

The closing condition, element by element, re-run against post-merge origin/main

This issue's own stated check was:

go test ./internal/cmd -count=1 exits non-zero when safeTerm is removed from blobStatusError, with the failure naming that function rather than a build error.

Run on a clean extract of origin/main at 752bf50 — not on the PR branch, since a probe against the branch is evidence about the branch:

Baseline: go test ./internal/cmd -count=1 → ok github.com/civitai/cli/internal/cmd 37.6s.

Mutation (the narrowest expression, isolated, exactly one occurrence asserted before removal): delete name = safeTermSingle(name) from blobStatusError.

rc=1
--- FAIL: TestBlobStatusErrorCannotForgeALine (0.00s)
    generate_blob_forgery_test.go:82: #577 FORGERY in blobStatusError (HTTP Unauthorized): rendered text holds 1 newline(s), want 0 — a server-supplied name forged 1 extra terminal line(s):
    generate_blob_forgery_test.go:82: #577 FORGERY in blobStatusError (HTTP Unauthorized): rendered text holds a TAB, which on a tabwriter surface inserts a column rather than misaligning one:
    … same pair for HTTP Forbidden, HTTP Not Found, HTTP Internal Server Error

• non-zero exit ✅
• names blobStatusError ✅
• not a build error ✅ — grepped for build failed / undefined: / declared and not used / syntax error: none
• all four arms fire, including default ✅

Gated once at the top, not per-arm ✅ — name = safeTermSingle(name) sits above the switch; the three returns below render name directly, matching its structural twin downloadStatusError.

Both ledger rows carry a why that names them ✅

• safeTermCoveredBy["blobStatusError"] → TestBlobStatusErrorCannotForgeALine, "generate's blob 401/403/404/default arms: a server-derived output name, gated once at the top like its twin downloadStatusError"
• safeTermCoveredBy["downloadBlobTo"] → TestDownloadBlobToErrorsCannotForgeALine, "all four surfaces DRIVEN, not merely listed"

What #596 shipped beyond this issue's table

The six-row table here hardened the branch users almost never hit and left the reachable one fully raw. downloadOutputs pre-checks every job before any byte moves, so downloadBlobTo's own !force refusal — the one this issue named — fires only in a TOCTOU window, while downloadOutputs' batch refusal one frame up joined j.target with no sanitiser at all. Reproduced before fixing: esc=true, raw \x1b[1A\x1b[2K intact. Found by #596's round-0 audit, which questioned the requirement rather than the code.

The condition should have read "every server-derived string on this path, scoped by the function". A hand-built table read off two functions is how the reachable branch got missed — worth remembering when writing the next one of these.

Verified present in origin/main, not merely merged

git diff between the PR head a8cfa88 and origin/main over the six touched files is empty, with a positive control (pkg/civitai/retry.go differs) proving the comparison can report a difference. Merged tree tested before merge: vet clean, 21 packages ok, 0 FAIL. CI 13/13 SUCCESS.

Explicitly NOT closed by this

The \n/\t class on generate.go's and generate_wait.go's sibling sites — #604, widened during #596's audit from 7 sites to 13 after printOutputURLs was found forging a numbered row on stdout. Those are pre-existing, ledgered notCovered, and were deliberately left untouched here: a fix rolled into an audit round is a fix nobody reviewed against its own requirement.