Pular para conteúdo

AUR-492: real linters complement the review with a verifiable origin

Measured before (base 20565f39)

  • analysis.Vet (internal/analysis/vet.go) had no caller. It parsed the text form of go vet ./... and, when the run failed but some line looked like a diagnostic, returned those findings with a nil error. Measured on go1.27.1: a module with one package that does not type-check and one printf diagnostic exits 1, and a missing dependency prints p/a.go:3:8: no required module provides package ..., a line the text parser read as a finding. Both read as a (partial) clean result.
  • scanner.ExecCommand (internal/scanner/executor.go) ran every engine with cmd.Env == nil, i.e. the whole environment of the reviewing process (LLM_API_KEY, GITHUB_TOKEN, any other job secret) reached semgrep, gitleaks and git, which read content the pull request's author controls. Output was buffered without bound and a grandchild holding the pipes could keep Wait blocked past the 120 s timeout.

What changed

  • internal/scanner/environment.go: BaseEnvironment (PATH, HOME, TMPDIR, LANG, LC_ALL, SSL_CERT_FILE, SSL_CERT_DIR), Environment{Pass, Fixed, Extra} per engine, ChildEnvironment (never nil) and GitSafeDirectories (only the safe.directory entries of GIT_CONFIG_*, renumbered; an http.extraheader is dropped).
  • internal/scanner/executor.go: IsolatedCommand(env) is the production command: explicit cmd.Env, WaitDelay 5 s, 32 MiB per stream (ErrOutputTooLarge, an invalid output). The executor builds each engine's command from Engine.Environment; ExecCommand keeps its signature with the base environment only.
  • internal/scanner/govet (new engine govet, category lint, origin govet, no options): go env GOVERSION (identity go vet <version>), git diff --unified=0 <base>...<head> for the added lines, then analysis.Vet; only findings on added lines are kept. Empty range is an error (never a whole-tree scan). Environment: GOCACHE, GOPATH, GOMODCACHE, GOROOT passed; GOTOOLCHAIN=local, GOPROXY=off fixed; GOFLAGS never passed. Registered in internal/scanner/engines.
  • internal/analysis/vet.go: go vet -json ./...; any runner error is an error with no findings; the JSON report is decoded (never interpreted); a position outside the root, without a line or without an analyzer is ErrInvalidOutput. Rule id go-vet/<analyzer>, severity warning.
  • gitleaks declares Environment{Extra: GitSafeDirectories}; semgrep uses the base environment.
  • Docs: docs/configuration.md (govet section, child environment and limits, lint in the gate.sources table); docs/tutorials/sast.md Caso 5 and "Sem go no PATH"; demo cases govet-achado (a fake go that answers exactly what the real go vet -json printed for the fixture, and records whether LLM_API_KEY/GITHUB_TOKEN reached it) and govet-sem-go (the real image, without Go).

Review follow-up

  • CGO_ENABLED=0 is fixed in govet's environment: the pull request controls the #cgo directives, so vet never invokes the C toolchain (TestGoChildHasCgoDisabled; the all-engine leak test also requires it).
  • git diff --relative from the review root, and a root without go.mod is lint_execution_error (TestDiffIsRelativeToTheRoot, TestRootWithoutModuleIsInconclusive; TestRealGitSubdirectoryRoot with a real repository whose root is a subdirectory, where git exists).
  • A path git quotes in +++ "b/..." is ErrInvalidOutput, never dropped (TestParseAddedLinesRefusesQuotedPath).

How each scenario is proved

tests/acceptance/AUR-492.sh (selectors all, AC-001..AC-004, MUT-001..MUT-004; unknown selector exits 64).

  • AC-001: TestRealVetFindingOnAddedLine runs the real go vet through the executor with the production command (git answered by a canned diff, so the sealed image without git can run it): one finding calc/calc.go:9, rule go-vet/printf, origin govet, category lint. TestRealVetCorrectedChangeIsClean: the fixed change has none. TestRealVetIgnoresUntouchedLines: a diagnostic in an untouched file is dropped. TestRealGitRangeEndToEnd repeats it with a real git repository where git exists (go-shared). Demo: govet-achado publishes only calc/calc.go:9, never legado/legado.go, exit 3.
  • AC-002: TestMissingGoIsUnavailable (lint_unavailable), TestRealVetFailureIsInconclusive (a broken package next to a real diagnostic: lint_execution_error, no findings), TestNoRangeIsInconclusive, TestVetRefusesUntrustedReport, TestRefusesOptions. Demo: govet-sem-go is inconclusivo (lint_unavailable), exit 1.
  • AC-003: TestNoEngineChildInheritsSecrets registers every engine, puts fake semgrep, gitleaks, git and go first in PATH, sets LLM_API_KEY, GITHUB_TOKEN, AURUMCODE_LLM_API_KEY, GOFLAGS and an http.extraheader in GIT_CONFIG_*, runs each engine through Executor{} and asserts for each binary that it ran (PATH present), no sentinel reached it, every variable is in the declared list and git kept safe.directory. TestIsolatedCommandUsesOnlyGivenEnvironment, TestChildEnvironmentIsExplicit, TestGitSafeDirectoriesDropsOtherKeys, TestIsolatedCommandBoundsOutput. Demo: both ausente lines.
  • AC-004: the five pre-existing sast cases, recorded without the entry, never mention govet or lint_; TestGovetRunsOnlyWhenDeclared (config.Parse) where internal/config's imports exist. In the sealed image they do not (the card's paths/read_paths lack internal/dtrack and other config imports), and the script prints AC-004/config/not-materialized.
  • MUT-001 (failed vet read as findings, if runErr != nil && stdout == ""): TestRealVetFailureIsInconclusive RED with the calc finding.
  • MUT-002 (findings outside the diff kept): TestRealVetIgnoresUntouchedLines RED with legado/legado.go:6.
  • MUT-003 (govet import removed from engines.go, engine ignored): TestNoEngineChildInheritsSecrets RED (govet is not registered).
  • MUT-004 (cmd.Env = nil, environment inherited): TestNoEngineChildInheritsSecrets RED (semgrep received a secret of the reviewing process) and TestIsolatedCommandUsesOnlyGivenEnvironment RED.

Last lines, go-shared (bash tests/acceptance/AUR-492.sh all) and sealed (oci-run --profile go-unit-offline-v1 --card AUR-492):

AUR-492/MUT-004/rejected
AUR-492/all/pass

Limits

  • The engine vets the module at the review root (./...); nested modules are outside its coverage. Proxy variables are not passed to any engine.
  • The product image does not carry Go; govet in CI needs a runner image with Go on PATH. Nothing is installed by the review.