Skip to content

Wo 4 5/spectre family review - #1

Merged
ppiankov merged 10 commits into
mainfrom
wo-4-5/spectre-family-review
Jul 31, 2026
Merged

Wo 4 5/spectre family review#1
ppiankov merged 10 commits into
mainfrom
wo-4-5/spectre-family-review

Conversation

@ppiankov

Copy link
Copy Markdown
Owner

No description provided.

ppiankov added 10 commits July 31, 2026 23:31
WO-10: findings printed in scan order, burying critical findings among
medium noise on large accounts. Sort a cloned slice before rendering.
WO-11: the GCP UNUSED_READ_REPLICA finding fired unconditionally with
High severity and the full instance cost as EstimatedMonthlyWaste,
unlike the AWS path which only fires on a confirmed zero-connection
signal. Downgrade to Low severity with usage-unknown wording; surface
cost as informational metadata instead of claimed waste.
WO-9: excludeIDs map construction was byte-for-byte duplicated in
aws.go/gcp.go, exclusion checks re-implemented the same map lookup in
both scanners, and reportProgress boilerplate only differed by scanner
name. Extract ExcludeConfig.IsExcluded, buildExcludeIDs, and
database.ReportProgress as shared helpers.
WO-7: three Config fields were parsed but never reached runtime:
Provider was never validated against the invoked subcommand, Timeout
was parsed but no call site ever read it (only the --timeout flag was
used), and Exclude.Tags was merged into ScanConfig but neither scanner
ever checked it despite ListTagsForResource already existing on the
AWS interface.

- aws.go/gcp.go now error if config provider doesn't match the
  invoked subcommand.
- Config load moved before timeout-context setup so cfg.Timeout can
  fall back when --timeout isn't explicitly passed (Flags().Changed).
- Added ExcludeConfig.MatchesExcludedTags plus AWS FetchTags and a
  new cloudsql Instance.Labels field so both scanners can exclude by
  tag/label, not just by resource ID.
WO-6: GoReleaser ships windows/amd64+arm64 archives but CI only ran on
ubuntu-latest and README had no Windows install instructions. Add a
windows-latest job running go build/go test directly (no make, no
race detector, to avoid MinGW/CC requirements on the runner) and a
Windows quick-start section documenting the release zip download.
Self-review pass: this is a public repo with no prior WO-N comment
convention (git grep confirms zero hits on main), so drop those
tokens from new comments while keeping the rationale. Also log a
warning when AWS tag fetch fails during scan instead of silently
treating the fetch error the same as "not excluded".
Every consequential changed hunk on this branch now carries a leading
WO-N citation (WO-7/8/9/10/11) per the wo-provenance-annotation
convention, so the repo's wolint pre-push gate can attribute each
region to the work order that motivated it.
wolint flagged 4 hunks where the citation trailed or missed its
target line: a header comment doesn't lead a var-block-adjacent
function (rankSeverity), and a citation placed after the changed
line doesn't count as governing it (flags := cmd.Flags() in both
applyAWSConfigDefaults and applyGCPConfigDefaults). Also restores a
citation on TestRunAWSProviderMismatch that a transient tool error
dropped from the prior commit.
WO-12: selectReporter opened --output via os.Create but never closed
it, leaking the handle until process exit. Harmless on POSIX but
fatal on Windows, where TestSelectReporterOutputFile's t.TempDir()
cleanup cannot remove a directory containing an open file. Return an
io.Closer alongside the reporter and defer-close it in both
runAWS/runGCP.
WO-13: CHANGELOG.md only ever had an Unreleased section despite two
real tags (v0.1.0, v0.1.1) existing, failing ANCC's
changelog-version-entry check on every push since March. Backfill
versioned entries from git log and move the pre-existing bullets to
where they historically belong.
@ppiankov
ppiankov merged commit 52b667e into main Jul 31, 2026
5 checks passed
@ppiankov
ppiankov deleted the wo-4-5/spectre-family-review branch July 31, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant