chore(gosec): reviewed per-site #nosec waivers for the 8 reported findings (18 real sites)

[LukasWodka]

Summary

Per-site, individually reviewed inline #nosec waivers for the gosec backlog the advisory golangci job reports, so the job goes to 0 findings and the follow-up in tracebloc/backend#1305 can drop --issues-exit-code=0 (and eventually flip the check to required). Comment-only change — no behavior touched.

The 8 findings are actually 18

The "8 findings" sized on #423 are an artifact of golangci-lint's default issues.max-same-issues: 3: all 13 G304s share the identical message text ("Potential file inclusion via variable"), so any given run surfaces only 3 of them — and which 3 flaps with analysis-cache state (reproduced locally: cold-cache runs consistently showed one triple, warm-cache reruns showed different triples; CI caches too, via golangci-lint-action). 3 G304 + 3 same-texted G204 + 1 differently-texted G204 + 1 G115 = the reported 8.

Uncapped (--max-same-issues=0 --max-issues-per-linter=0) the real backlog is 18: 4x G204, 13x G304, 1x G115. Waiving only the visible 8 would have been whack-a-mole — each waived G304 would just let the cap surface a previously hidden one. All 18 are reviewed and waived here.

Recommendation for the backend#1305 follow-up that drops the advisory flag: consider also setting issues.max-same-issues: 0 (or a high cap) in .golangci.yml, so a future PR that introduces several instances of the same mistake shows all of them instead of 3.

Threat model in one line

This is a customer-installed local CLI that runs with the invoking operator's privileges on the operator's own machine: every "variable" input at these sites is a compile-time constant, the operator's own flag/arg/env/config, or a path enumerated by our own walk of the operator-chosen dataset root — there is no privilege boundary for injection or traversal to cross.

Per-site review

Site	Rule	Action	Justification
internal/cli/prepare_host.go:113	G204	waiver	argv fully compile-time constant: literal bash -c + installerRunScript("prepare-host"), built only from the installerURL const. No runtime input.
internal/cli/upgrade.go:123	G204	waiver	upgradePlanFor(runtime.GOOS) returns compile-time constants (bash -c installerRunScript("")); only the GOOS branch varies. gosec can't see through the struct.
internal/helm/upgrade.go:56	G204	waiver	Test-seam Runner var; every caller passes the literal "helm"; args are the operator's own release/kubeconfig flags, exec'd as an argv array (no shell interpolation anywhere).
internal/nodeboot/nodeboot.go:35	G204	waiver	Same test-seam pattern; callers pass literal tool names (k3d/helm/docker) with argv from package consts + the operator's own cluster/release names; no shell.
internal/config/config.go:125	G304	waiver	Reads the CLI's own config.json under ~/.tracebloc or the operator's explicit $TRACEBLOC_CONFIG_DIR override, as the invoking user.
internal/cli/home_local_fallback.go:113	G304	waiver	Fixed filename next to os.Executable() (the tb.cmd shim ownership probe); whoever controls that directory already controls the binary itself.
internal/cli/ingest.go:78	G304	waiver	Reading the ingest.yaml the operator named as the positional arg is the command's documented job (tracebloc ingest validate <path>).
internal/cli/installlog.go:37	G304	waiver	Creates (never reads) the install log at a timestamp-generated name under the CLI's own 0700 config dir, mode 0600.
internal/cli/update_check.go:167	G304	waiver	The CLI's own update-check throttle cache: config.Dir() + constant filename; contents JSON-validated before use.
internal/push/detect.go:30	G304	waiver	Image-header decode of a file the symlink-rejecting dataset walk found under the operator-chosen root.
internal/push/image_resolution.go:30	G304	waiver	Same provenance; mirrors the in-cluster resolution validator on the operator's own files.
internal/push/preflight.go:45	G304	waiver	Opens the operator's own dataset CSV to mirror the ingestor's checks locally before upload.
internal/push/preflight.go:81	G304	waiver	3-byte BOM sniff of the same operator-supplied dataset CSV.
internal/push/preflight.go:805	G304	waiver	Encoding check of the operator's own dataset CSV, size-capped via LimitReader.
internal/push/stream.go:472	G304	waiver	src comes from the symlink-rejecting walk and the Lstat guard directly above re-rejects symlinks at stream time (existing defense-in-depth, Bugbot r4/r8).
internal/push/tabular.go:391	G304	waiver	Dataset CSV found by DiscoverTabular's symlink-rejecting walk of the operator-chosen root.
internal/push/text.go:165	G304	waiver	Path is looked up in byBase/byStem, maps keyed only by files the symlink-vetted walk found on disk — a manifest entry can select among them but cannot point outside the dataset dir.
internal/slug/slug.go:90	G115	waiver	False positive: range-over-string runes are non-negative and the r < 128 guard bounds them, so byte(r) is a lossless ASCII conversion.

Hardening considered, none bolted on

Per-site hardening was weighed before each waiver (preferring a fix over a waiver where one was real):

• The meaningful G304 control for the one place a path could smuggle content off-machine (dataset streaming to the cluster) already exists: rejectSymlink on every Discover walk plus a stream-time Lstat re-check in writeTarFile (added for Bugbot r4/r8 on the push PRs). The waivers cite it rather than duplicate it.
• Wrapping the remaining reads in filepath.Clean(...) would silence gosec without confining anything (most of these paths already flow through filepath.Join, which Cleans) — cosmetic linter-dodging, rejected in favor of honest, reviewable waivers.
• slug.go could dodge G115 by switching to b.WriteRune(r), but that hides the deliberate ASCII-only byte write behind an equivalent call; the explicit waiver documents why the conversion is safe.

Verification

• Reproduced the findings locally with the CI-pinned golangci-lint v2.12.2 (built with go1.26.3) against .golangci.yml: 8 reported with default caps, 18 uncapped, membership flap across cache states confirmed.
• After this change: golangci-lint run --max-same-issues=0 --max-issues-per-linter=0 → 0 issues (exit 0). gofmt -l clean, go build ./... and go test ./... green.
• The PR's own golangci job annotations dropping to 0 is the CI-side confirmation.

This unblocks removing --issues-exit-code=0 from .github/workflows/golangci.yml — deliberately not done in this PR; that's the follow-up tracked in tracebloc/backend#1305.

Part of tracebloc/backend#1305

🤖 Generated with Claude Code

---

Note

Low Risk
Comment-only linter suppressions with documented rationale; no logic, I/O, or exec behavior is modified.

Overview
Adds comment-only inline #nosec annotations at 18 call sites so the advisory golangci-lint job (with gosec) can report 0 findings, paving the way to drop --issues-exit-code=0 in a follow-up (backend#1305). No runtime behavior changes.

The waivers cover the full backlog golangci’s default caps hide (4× G204, 13× G304, 1× G115): CLI config/install paths, dataset preflight reads, helm/nodeboot test exec seams, and a bounded ASCIIbyte(r) in slug. Each comment documents why the site is acceptable for a local operator-run CLI (fixed paths, operator-chosen files under symlink-vetted walks, compile-time shell scripts, argv-only helm/docker/k3d invocations).

^{Reviewed by Cursor Bugbot for commit dce4f3c. Bugbot is set up for automated code reviews on this repo. Configure here.}

[LukasWodka]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit dce4f3c. Configure here.}

[saadqbal]

Approve — careful PR 👍 Every one of the 18 rationales checks out against the code: the rejectSymlink walks, the writeTarFile Lstat re-check, the LimitReader cap, the compile-time argv on both exec sites, and the guarded byte(r) in slug are all real. No waiver hides a site where its justification is false.

Two low-severity notes, neither blocking:

• The G204 waivers on the helm/nodeboot Runner seams sit at the choke point, so they blanket future callers too — the 'every caller passes the literal helm/k3d' property can't be enforced from there. Fine for now (all current callers are literal); a tiny allow-listed exec wrapper would be the durable fix if we ever want one.
• stream.go's Lstat→Open is technically a TOCTOU pair; a non-issue under the local-operator threat model, just noting the 'symlinks re-rejected at stream time' wording doesn't fully close that window.

Good to merge.