Vulnerability gates + visible least-privilege fallbacks#40
Merged
Conversation
…ermissions
First local runs of the two scanners being added as CI gates surfaced
5 reachable vulnerabilities (govulncheck) and 46 static findings (gosec,
medium+ severity). Everything is either fixed or suppressed with a reason;
nothing is silently ignored.
─── govulncheck (5 reachable, all fixed) ───
- golang.org/x/net v0.52.0 → v0.56.0 (with x/sync, x/sys, x/term, x/text
riding along via `go get`/`go mod tidy`): clears GO-2026-5026 (idna
Punycode acceptance) and GO-2026-4918 (HTTP/2 infinite loop on bad
SETTINGS_MAX_FRAME_SIZE), both reachable from the PagerDuty sink's
http.Client.Do.
- `toolchain go1.26.4` added to go.mod: clears the three stdlib findings
(GO-2026-5039 net/textproto, GO-2026-5038 mime, GO-2026-5037
crypto/x509), all fixed in go1.26.4. CI's setup-go reads go.mod, so the
gate builds with a patched stdlib instead of whatever 1.26.0 resolves to.
─── gosec: real fixes (13 sites) ───
- G301 (6 sites): generated-output and baseline directories created 0o755
→ 0o750. Nothing needs world access to ./cloudgov-fixes or
~/.cloudgov/baselines.
- G306 non-executable writes (3 sites): Terraform fix files, raw policy
JSON, and baseline temp files written 0o644 → 0o600. These can contain
account IDs, ARNs, and full policy documents — owner-only is the right
default for generated security artifacts.
- G306 executable scripts (3 sites): remediation shell scripts written
0o755 → 0o700 (owner-only but still executable, which is the point of a
remediation script) with an inline justified #nosec, since gosec's 0600
ceiling can't express "executable on purpose". Tests assert only the
owner-exec bit, so behavior stays covered.
- G204 (1 site): the HTML-report browser opener runs a fixed per-OS binary
(open/xdg-open/cmd); only the report path varies and no shell is
involved. Inline justified #nosec.
─── gosec: false positives (2 sites) ───
- G101 on SecretFindingType constants ("AWS_ACCESS_KEY",
"GCP_SERVICE_ACCOUNT_KEY"): these are the secret scanner's own
classification labels, not credentials. Inline justified #nosec.
The remaining class, G304 (file inclusion via variable, 31 sites), is every
place this CLI reads a file the operator named on the command line (--from
report files, --state tfstate, benchmark/standard files). That is the
tool's contract, not taint, and is excluded at the scanner invocation with
a written reason in the security workflow rather than 31 copy-paste nosec
comments.
Co-authored-by: stxkxsbot <275011021+stxkxsbot@users.noreply.github.com>
cloudgov was the org's only service repo with no vulnerability scanning in CI. This adds .github/workflows/security.yml, adopting the shape of eks-agent-platform's security workflow (separate security workflow, SHA-pinned third-party action, SARIF upload to code scanning, exit-code-on-findings) with this repo's trigger convention (push to all branches + PRs, like ci.yml) plus a weekly Tuesday cron so newly published CVEs fail a scheduled run even when no PRs land. Two jobs, both hard gates: - govulncheck: installed fresh via `go install ...@latest` and run over ./... — symbol-level reachability analysis against the Go vuln DB, so it fails only on vulnerabilities this module's code actually calls, and exits non-zero on any. setup-go reads go.mod (toolchain go1.26.4), so the scan runs against the patched stdlib. - gosec: securego/gosec pinned to the v2.27.1 commit SHA, `-severity medium`, SARIF written and uploaded to code scanning (if: always(), so findings are visible even on a failing run). gosec exits non-zero on any finding — no -no-fail. The single suppression is `-exclude=G304` (file inclusion via variable), justified inline in the workflow: all 31 hits are operator-supplied CLI flag paths (--from report files, --state tfstate, benchmark/standard files) — reading files the operator names is this CLI's contract, not taint. Every other finding class from the first run was fixed or carries a site-local justified #nosec (see the preceding commit). Co-authored-by: stxkxsbot <275011021+stxkxsbot@users.noreply.github.com>
…am fix warnings
The bug: MinimalPolicy silently emitted `Resource: "*"` statements. Any
used permission whose audit trail carried no resource was folded into a
wildcard statement indistinguishable from a deliberately scoped one — a
least-privilege tool quietly widening grants undermines its own purpose.
Worse, a CloudTrail ResourceName that is a bare name rather than an ARN
(e.g. a bucket name) passed through verbatim as a statement Resource,
producing a policy IAM rejects outright.
The root cause: the resource classification in MinimalPolicy
(internal/cloud/aws/iam.go) only handled `""` → `"*"` and carried no
record of the substitution, so neither the generated document nor the
`iam fix` command could tell the operator which actions were unscoped
or why.
The fix, in three layers:
─── Classification (internal/cloud/aws/iam.go) ───
MinimalPolicy now classifies every used permission's resource:
- `""` or `"*"` → fallback, reason FallbackNoResourceRecorded (the
CloudTrail event carried no resource — typical for actions without
resource-level permission support; extractResource yields "*" then).
- non-`arn:`-prefixed → fallback, reason FallbackUnrecognizedResource,
keeping the recorded value. These now also group under "*" instead of
passing through as an invalid statement Resource.
- valid ARNs keep their scoped statements, unchanged.
─── Marking in the generated policy ───
The wildcard statement carries `Sid: "UnscopedFallback"` (Sid rides
along in Raw through both the Terraform and raw-JSON fix writers, so the
widened grant is visible in the committed artifact itself). Scoped
statements carry no Sid. Fallback records land on the returned
cloud.Policy via a new additive `Fallbacks []PolicyFallback` field
(internal/cloud/provider.go) — no IAMProvider interface change — sorted
by action then resource for deterministic output.
─── Surfacing in command output (cmd/iam.go) ───
`iam fix` now warns per principal on stderr, following the existing
`warn:` convention and gated on --quiet:
warn: <principal>: N action(s) could not be scoped and were granted
Resource "*" (statement Sid "UnscopedFallback"):
- <action>: <reason> [(recorded resource "<value>")]
Tests: a new table-driven TestMinimalPolicyFallbacks covers scoped-ARN
(no fallback, no Sid), empty resource, audit-recorded "*", non-ARN
resource with the recorded value preserved, mixed input with fallback
ordering, and duplicate dedup; cmd/iam_test.go covers the warning
writer (empty, both reasons, recorded-value rendering, multi-line).
Co-authored-by: stxkxsbot <275011021+stxkxsbot@users.noreply.github.com>
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See commit messages for full details.
Summary