Newline in a server-supplied file name still forges lines on four download surfaces, and #552's closure left it unowned

[ZacxDev]

Split out of #552, which was closed by #573 (2026-09-12T17:30:31Z). #573's measured scope was ~13 tabwriter renderers and it deliberately excluded free-text surfaces — its own message says those "deliberately keep safeTerm + indentContinuation — they are not cells." internal/cmd/download.go is not on #552's table and was never covered by it.

So the residual below is real, documented in the tree, and as of #573 had no open tracking object. Found by the round-2 audit of #572.

The residual

internal/saferune deliberately retains \n and \t (Cc, minus \n and \t). safeTerm therefore does not stop a server-supplied files[].name containing a newline from forging whole lines on four surfaces that #572 otherwise hardened:

site	surface
download.go (*progressWriter).line	the \r-rewritten progress line, re-emitted at 10 Hz on a TTY
download.go checkTargetCollisions	the same-target overwrite refusal — forged - [id N] rows inside the group listing
download.go downloadOne	the SHA256-mismatch message, i.e. the CLI asserting an integrity failure
download.go downloadStatusError	the 401/403/404 arms

Measured with files[].name = "weights.safetensors\nSaved /home/u/legit.safetensors (4.0 GiB) (SHA256 verified)": the progress line renders the forged Saved … (SHA256 verified) before any transfer completes, and done() leaves it standing.

Not a regression. Every one of these surfaces was fully raw before #572; that PR is a strict improvement and states this residual rather than hiding it. What was missing is an owner.

Why it is not simply "apply safeTermSingle"

These are free-text lines, not tabwriter cells, so #573's cell rule does not apply to them unmodified: collapsing \n to a space in a \r-rewritten progress line and in a multi-line refusal has different display consequences than it does in a cell, and the refusal is a fail-safe whose group structure must stay readable. Deciding which transform each surface takes is the work here — this issue does not prescribe one.

Correction to this issue's first draft

The original closing condition below said the four in-tree statements were "three comment blocks in download.go and the README Troubleshooting row". That was wrong, and #572's round-2 fix pass measured it: the README row carries no issue pointer at all. The fourth statement is the header paragraph of internal/cmd/download_safeterm_test.go. Left visible rather than silently edited, since the miscount would have let the condition read as satisfied while a statement still pointed at a closed issue.

Those four are already repointed at this issue by #572, so that half of the condition is discharged when #572 merges.

Closing condition

The four surfaces in the table above render a server-supplied name containing \n (and \t) as a single terminal line, and a test drives each one with a newline-bearing name and asserts the rendered output occupies the expected number of lines — failing if a forged line appears.

Checked by: go test ./internal/cmd -count=1 exits non-zero when the newline transform is removed from (*progressWriter).line, naming that function rather than a build error.

[ZacxDev]

Condition discharged — closed by hand, with the evidence

cli#590 merged as 3b222c6, plus two follow-ups: e3a7635 (#594) and the post-range commits a parallel session landed on the same PR.

This issue's own stated check, run against origin/main

Checked by: go test ./internal/cmd -count=1 exits non-zero when the newline transform is removed from (*progressWriter).line, naming that function rather than a build error.

Reverting safeTermSingle → safeTerm at that one call site, in a .git-less export of origin/main:

rc=1
#574 FORGERY in (*progressWriter).line
build failed: 0

Non-zero, named, not a build error. The condition is met as written.

The four surfaces

All take safeTermSingle, which collapses \n and \t to a space. The issue deliberately declined to prescribe a transform, calling that decision the work — the answer turned out uniform, because a group listing is made of rows and a row is a line.

Every site in download.go moved, not the four reported: 31 of 32. Leaving structural siblings raw is the mechanism that produced #566.

🔴 The first pass closed it on ONE HALF of each %s: %w pair

Worth recording, because it is #566's own shape reproduced by the PR that claimed to close this issue. safeTermErr still used safeTerm, and *fs.PathError renders its Path unquoted — so the operand was one line and the cause was not:

create /…/weights.safetensors Saved … (SHA256 verified).part: open /…/weights.safetensors
Saved /home/u/legit.safetensors (4.0 GiB)  (SHA256 verified).part: no such file or directory

Found by the round-0 audit, reproduced, fixed, and pinned by TestWrappedCauseCannotForgeALine — which did not exist until a mutation check showed that reverting the fix reddened nothing.

The 32nd site is deliberate

targetPath's unusable filename %q refusal keeps bare safeTerm: %q escapes \n and \t rather than passing them through, so the line cannot be forged there, and collapsing them would destroy the one thing that message exists to convey — that the server sent a newline. Four files now say so; #594 fixed the one that still denied it.

🔴 Residual, stated rather than renamed away — NOT covered by this issue

The identical forgery is reachable with no \n and no \t. p.name is length-unbounded, so a name padded to the terminal width soft-wraps and strands the same (SHA256 verified) at column zero. Measured at widths 80, 100, 120 and 132: one logical line occupying 14 display rows, 12 of them forged, before any bytes are verified. safeTermSingle is a no-op on it and assertOneLine is green.

That is a different primitive — display width, not control runes — and outside this issue's scope. The machinery to bound it already exists (hardSplitOverlong). TestProgressLineCannotForgeALine's name is not the outcome, and the test says so in its own doc comment.

No closing keyword is used here or in any of the merged PRs; this issue is being closed by hand.