safeTerm is unpinned at most of its call sites: deleting it at 20 of 25 sampled sites leaves the suite green

[ZacxDev]

safeTerm is the CLI's one gate on server-supplied text reaching a terminal, and it is called from 150 sites in 56 functions (AST enumeration of internal/, non-test files, at 27cd5de+). Only a handful of those sites are pinned by a test: an adversarial audit of #398 deleted safeTerm(...) at each site in turn and 20 of 25 sampled sites left the whole suite green.

The pinned ones are all failure-reason fields — the surfaces #367, #382 and #393 touched. Everything else is unguarded: a refactor can drop the call and CI says nothing.

Not a regression, and not #398's to fix

Measured identical on 27cd5de (before #398) and on the #398 branch: the same 20 survive on both. The audit's instruction was explicit — do not fix it in the #393 PR, file it so it is not lost.

What is unpinned (sample)

surface	file	what a hostile field could do there
model / version names, creators, tags	internal/cmd/models.go, model_versions.go	overwrite the SHA256 verified line during a download
app name, slug, tagline, description	internal/cmd/apps.go (11 calls in printAppDetail alone)	forge a review-status line
image metadata, prompts read back from the API	internal/cmd/images.go (7 in printImageMetaBlock)	reorder a displayed line with a bidi control (before #393)
download plan: file names, targets, notes	internal/cmd/download.go	forge the "already present" / "Saved" lines
app metrics scope + endpoint tokens	internal/cmd/app_metrics.go	— (item 8 keeps these raw on purpose)
collections, articles, users, tags, creators	five more files	—

Shape of a fix

Not 150 tests. The cheap version is one table-driven test per renderer family that feeds a hostile string through the real command and asserts the paired predicate the reason-path tests already use (assertNoEscapeBytes / assertNoInvisibleRunes: the class is gone AND the words arrived). The expensive-but-durable version is a ledger: enumerate the safeTerm call sites by AST, and require each enclosing function to appear in an asserted list of "covered by test X" — failing when the set grows.

Note #398 added TestSafeTermIsNeverAppliedToUserTypedInput, which already walks every call site by AST for the opposite property (that none of them sanitises user-typed input). The same walk is the natural place to hang a coverage ledger.

Why it matters

safeTerm is a security control. The repo's own rule is that a guard nobody has watched fail proves nothing — and for 20 of 25 of these sites, nobody has.