docs(crypto): enumerate vet's printf gates from the analyzer source

What

Carries four review findings raised on !2293 (merged) after it merged, plus the blocking note raised on this MR. The round-6 notes posted six minutes before the merge, so nothing on that MR addressed them. All four land in internal/crypto/key_redaction.md, the design note !2293 (merged) added, plus one comment and one added subtest in key_redaction_test.go.

No production code changes.

Why they are one finding

Three of the four are the same shape: a gate whose condition is a conjunction, documented with a single conjunct. Both mechanisms the document explains are conjunctions, and each prose site restated a different subset of one.

go vet

The file bounded the %w gap on one condition — that the analyzer could fold the format argument to a constant — so a plain string-literal %w at a call site whose callee the analyzer never classified read as covered by the static check.

An earlier revision of this MR fixed that by naming a second condition, classification. The review note on this MR showed two conditions was still short: an operand passed as args... is invisible to argCanBeChecked, so matchArgType never runs even where the callee is classified and the format folds.

Verified against the vendored analyzer at go1.26.7 — findPrintLike abandons a wrapper candidate at the first assignment to its format or args parameter, printf.go:342-350 under GOROOT/src/cmd/vendor/golang.org/x/tools/.... Measured: three broken formats written as plain string literals at awsLoggerWrapper.Logf call sites leave go vet ./internal/storage/driver/s3/ at exit 0, and deleting Logf's two-line format rebuild makes vet report all three.

fmt

handleMethods requires p.arg.(error) and p.wrapErrs (GOROOT/src/fmt/print.go:626-635), and p.wrapErrs is set in exactly one place, inside fmt.Errorf (errors.go:40). The rule table stated the first conjunct alone, so an error type that carried a key read as safe under any call. Measured with a probe that is both an error and a fmt.Formatter: Errorf reaches Format and hands it 'v', while Sprintf, Printf and Fprintf hit badVerb and reflect the bytes out.

The count was the wrong shape

Rather than add the third condition to the two already listed, the analyzer's decision path is now enumerated from source: checkCallscheckPrintfokPrintfArgmatchArgType, read exit by exit and measured at go1.26.7 against a probe of NamespaceKey's shape. Seven gates, six of which fail silently, each with its source site and a measured failing shape. ### What a call must clear before vet checks an operand carries the table.

Beyond the three conditions already discussed, the silent gates measured are: a callee that does not resolve to an object at all (typeutil.Callee returns nil for a call whose function is neither identifier nor selector); a callee resolving to a variable that carries no wrapper fact; the file not being in the build; and the operand's static type, which for %w is the single predicate types.ConvertibleTo(typ, error).

The section states what the enumeration does not close. The exits are finite and every return on the path was read, but the source shapes that reach a gate are open-ended, and the analyzer is vendored into the Go toolchain rather than resolved through go.mod, so the list moves with .tool-versions and not with this repository. The derivation path is named so it can be re-run. It was identical across go1.26.2 and go1.26.7, which vendor byte-identical printf.go and types.go.

The row's "either one missing and nothing is checked" also overstated, and is corrected: from go1.24 a non-constant format whose call has no operands is itself reported. That report is about the format and never reaches an operand, so it cannot cover a key; the version gate is per file, and //go:build go1.23 silences it.

The fix is structural

Not four line patches. The contrast table now carries each mechanism's whole input set once — a Reads row, a Runs at all when row, and a per-mechanism %w rule — and the prose points at it rather than restating a subset.

Both section preambles stated an absolute that was false against rows in their own sections ("every gate in it is keyed on the verb"; "the gates here read the argument"). Both now name the table's column instead.

The fourth finding is the sentence answering the section's own title, dropped when the section was split. Restored, narrowed to what is true: go vet's printf check decides from the verb table, where %v is valid for every type, so there is no per-type ban for it to express.

Corrected beyond the notes

  • For %w the argument's type is read at matchArgType, not isFormatter. The verb gate short-circuits before isFormatter runs, so the file named the wrong site.
  • renderOne(f string, a any) was listed as an example of an unfoldable format argument. Its escape is classification — non-variadic, so never a wrapper candidate — not folding. Measured: a literal %w through a variadic helper is reported, through renderOne it is not.
  • The scan's second site had moved as main advanced, so both sites are now named by symbol rather than by line.
  • ## Where the guarantee stops listed only %w on a non-error. %w on a real error under Sprintf is the same gap, and is now named there.
  • The AST census was re-run through go/types rather than a hand-rolled folder: two non-constant format arguments at 441239ac0, and seven functions carry the (format string, args ...any) shape, of which five spend their variadic on slog attributes or SQL arguments and never reach a printf.

Tests

TestKeyBearingTypes_LeakUnderTheWrapVerb gains an error carrying a key is redacted only under Errorf, pinning the p.wrapErrs conjunct against a probe error type that also implements Formatter. Without it the document states a distinction no test covers, and the uncovered case is the one the two constructs disagree on.

It also gains a spread operand escapes the analyzer: a literal %w whose operand is passed as args.... go vet ./internal/crypto/ is silent on that call and the assertion shows the key returning as decimal bytes, so the sharper of the two escapes is pinned in the tree rather than asserted in prose. A repo census on 794646b73 finds the spread gate has no site of its own in non-test Go: the only spread calls are the forwarding bodies of usageError and Logf, which fail the fold gate and the spread gate together and are decided at their callers' call sites.

No e2e scenario is added or affected: nothing in the request path changes.

Gates

  • go build ./... and go test ./internal/crypto/... -race pass.
  • golangci-lint run --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false ./internal/crypto/... reports 0 issues on the mise-pinned 2.13.2. The one //nolint:err113 added was measured firing before it was written.
  • scripts/ci/check-comment-caps.sh passes.
  • pre-commit run --files internal/crypto/key_redaction.md internal/crypto/key_redaction_test.go: every hook passes except the repo-wide go test, which fails on TestWireEncryption_NilInfraConfigResolvesLabKitDefault in cmd/artifact-registry. That failure reproduces on a clean origin/main checkout in the same environment and is untouched by this branch.

Follows !2293 (merged), whose review threads carry the four findings.

Edited by Suleimi Ahmed

Merge request reports

Loading
Loading