feat(hooks): CI annotations and SARIF upload for scanner findings - #2631
Merged
Andriy Knysh (aknysh) merged 38 commits intoJun 24, 2026
Conversation
… in Docker image Security scanner hooks (Checkov, Trivy, KICS, Infracost) already render a markdown findings summary to the terminal, but it never reached the CI pipeline — findings were buried in the `atmos terraform plan` log stream. Route that summary to the GitHub Actions job step summary automatically when running in GitHub Actions ($GITHUB_STEP_SUMMARY), reusing the existing CI provider OutputWriter seam: - pkg/ci/summary.go: new ci.WriteStepSummary helper (no-op outside CI / when no summary destination), hiding the internal provider OutputWriter type. - pkg/hooks/command_engine.go: renderCISummary, called after renderTerminal in CommandEngine.Run. Best-effort — a step-summary write failure logs at debug and never fails the hook. Covers all four CommandEngine-based scanner/cost kinds uniformly. Also fix Checkov crashing inside the official Atmos Docker image: bump the base image from debian:bookworm-slim (glibc 2.36) to debian:trixie-slim (glibc 2.41) so the PyInstaller-bundled Checkov binary (needs GLIBC_2.38+) loads its frozen Python runtime instead of failing with a missing-version error. Includes unit + end-to-end tests, hooks docs, changelog blog post, and roadmap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses CodeRabbit review on PR #2617: the milestone description grouped Infracost under "security scanner hooks"; Infracost is a cost-analysis tool. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…canner findings Surface scanner-hook findings (checkov/trivy/kics) in CI beyond the job summary — as inline GitHub annotations on the PR diff and as uploaded SARIF in GitHub Code Scanning (the Security tab) — natively, without the github/codeql-action. CI reporting is modeled as provider capabilities, not hooks (the deprecated ci.* hook kinds are not revived): - pkg/ci/internal/provider: new optional capability interfaces Annotator and SARIFReporter + neutral Annotation/SARIFReport types (opt-in via type assertion, like CacheProvider/DebugModeDetector). - pkg/ci/results.go: ci.Annotate / ci.ReportSARIF helpers (Detect → type-assert → call; no-op otherwise) + public type aliases, mirroring ci.WriteStepSummary. - pkg/ci/providers/github: implement Annotate (::error/::warning workflow commands with proper escaping; severity → level) and ReportSARIF (CodeScanning.UploadSarif; gzip+base64; category stamped into runs[].automationDetails.id so per-component uploads don't overwrite). - pkg/schema: new ci.annotations (default on) and ci.results (default off) feature gates, siblings of ci.summary/checks/comments; all require ci.enabled. The hook hands findings to the provider when the gate is on: - pkg/hooks/sarif: the shared SARIF handler now surfaces parsed findings + raw SARIF on the Summary (so checkov/trivy/kics are all covered by one change), labeling by the SARIF tool name when no kind is set. - pkg/hooks/command_engine.go: emitCIAnnotations + publishCIResults after renderCISummary; auto-derive the Code Scanning category from the scan target (component vs stack/component). Reconcile the step-summary to the ci.enabled gate (was provider-detection-only). - Custom hooks: a format-handler registry lets kind: command opt in via format: sarif (+ optional results path), reusing the shared SARIF handler for any SARIF-emitting tool with no Go code. Includes unit + end-to-end tests, hooks docs (incl. required GitHub Actions permissions), changelog blog post, and a roadmap milestone. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
requested a review
from a team
as a code owner
June 19, 2026 15:57
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Erik Osterman (Cloud Posse) (osterman)
added
the
minor
Dependency Review✅ No vulnerabilities or license issues found.Scanned Files
|
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds GitHub Actions CI reporting outputs for hook findings, migrates AWS endpoint configuration to ChangesScanner Hook CI Reporting
AWS Custom Endpoint URL Refactoring
GitHub Actions and Tooling Hardening
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
website/docs/stacks/hooks.mdx (1)
531-531: 💤 Low valueOptional style improvements: typographic quotation marks and capitalization.
- Line 531: Use typographic quotation marks for "terminal-friendly", "Pro-friendly", and "Markdown" (proper noun)
- Line 598: Add comma after "e.g." and use typographic quotes around "tfsec"
These are cosmetic refinements for polish.
Also applies to: 598-598
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@website/docs/stacks/hooks.mdx` at line 531, Replace straight quotation marks with typographic (curly) quotation marks around the phrases "terminal-friendly", "Pro-friendly", and "tfsec". Additionally, capitalize "Markdown" as it is a proper noun, and add a comma after "e.g." to follow standard punctuation rules. These cosmetic refinements improve the overall polish and consistency of the documentation.website/blog/2026-06-15-scanner-findings-in-ci-job-summary.mdx (1)
35-35: 💤 Low valuePolish grammar and capitalization per style hints.
Three minor style improvements flagged by LanguageTool:
- Line 35: Use typographic quotation marks around "no findings"
- Line 39: Add comma after "GitHub Actions" → "GitHub Actions (i.e.,
GITHUB_STEP_SUMMARY)"- Line 45: Capitalize "Markdown" as a proper noun
✏️ Proposed style improvements
- Checkov crashed before it could produce any findings: + Checkov crashed before it could produce any findings: - Scanner summaries are now appended to the GitHub Actions job step summary whenever Atmos detects it is running in GitHub Actions (i.e. `GITHUB_STEP_SUMMARY` is set). It requires **no configuration** — the same markdown you already see in your terminal shows up in the run summary: + Scanner summaries are now appended to the GitHub Actions job step summary whenever Atmos detects it is running in GitHub Actions (i.e., `GITHUB_STEP_SUMMARY` is set). It requires **no configuration** — the same Markdown you already see in your terminal shows up in the run summary: - Write the markdown once, get it everywhere. Writing to the step summary is best-effort: if the summary file can't be written, it never fails the hook or the terraform command — the findings already rendered to the log. + Write the Markdown once, get it everywhere. Writing to the step summary is best-effort: if the summary file can't be written, it never fails the hook or the terraform command — the findings already rendered to the log.Also applies to: 39-39, 45-45
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@website/blog/2026-06-15-scanner-findings-in-ci-job-summary.mdx` at line 35, Apply three style improvements to the markdown blog post: replace the straight quotation marks around "no findings" with typographic (curly) quotation marks on line 35, add a comma after "GitHub Actions" on line 39 before the parenthetical remark, and capitalize "Markdown" as a proper noun on line 45 to follow standard style conventions flagged by LanguageTool.website/blog/2026-06-19-scanner-annotations-and-code-scanning.mdx (1)
8-8: 💤 Low valueMinor style improvements: capitalization and word repetition.
- Line 8: Capitalize "GitHub" fully (appears to be missing capital H in one place)
- Line 34: Word repetition — "analysis" used twice in close proximity; consider synonym on second use
- Line 60: Similar word repetition issue; "needs" appears twice within a few lines
Also applies to: 34-34, 60-60
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@website/blog/2026-06-19-scanner-annotations-and-code-scanning.mdx` at line 8, Fix three style issues in the markdown file. On line 8, ensure all instances of "GitHub" are fully capitalized with a capital H. On line 34, the word "analysis" is repeated twice in close proximity; replace one instance with an appropriate synonym to improve readability. On line 60, the word "needs" appears twice within a few lines; similarly replace one instance with a suitable alternative word to avoid repetition.pkg/ci/results_test.go (1)
31-99: ⚡ Quick winAdd explicit provider-error propagation subtests.
These tests validate dispatch/no-op, but they don’t yet assert what happens when
Annotator/SARIFReporterreturns an error.Proposed extension.
type reportingMockProvider struct { mockProvider annotations []provider.Annotation sarif []provider.SARIFReport + annotateErr error + reportErr error } func (m *reportingMockProvider) Annotate(a []provider.Annotation) error { m.annotations = append(m.annotations, a...) - return nil + return m.annotateErr } func (m *reportingMockProvider) ReportSARIF(_ context.Context, r provider.SARIFReport) error { m.sarif = append(m.sarif, r) - return nil + return m.reportErr } @@ t.Run("no-op on empty input", func(t *testing.T) { ... }) + + t.Run("returns provider annotate error", func(t *testing.T) { + restore := SwapRegistryForTest() + defer restore() + m := &reportingMockProvider{ + mockProvider: mockProvider{name: "cap", detected: true}, + annotateErr: assert.AnError, + } + Register(m) + require.ErrorIs(t, Annotate([]Annotation{{Path: "a.tf", Message: "x"}}), assert.AnError) + }) } @@ t.Run("no-op on empty body", func(t *testing.T) { ... }) + + t.Run("returns provider SARIF error", func(t *testing.T) { + restore := SwapRegistryForTest() + defer restore() + m := &reportingMockProvider{ + mockProvider: mockProvider{name: "cap", detected: true}, + reportErr: assert.AnError, + } + Register(m) + require.ErrorIs(t, ReportSARIF(context.Background(), SARIFReport{Body: []byte(`{}`)}), assert.AnError) + }) }As per coding guidelines, “All features need tests,” and negative-path behavior should be explicitly covered.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/ci/results_test.go` around lines 31 - 99, Add new subtests to both TestAnnotate and TestReportSARIF functions to validate error propagation from providers. In TestAnnotate, add a subtest that registers a reportingMockProvider configured to return an error from its Annotate method, then verify that the Annotate function properly propagates that error. Similarly, in TestReportSARIF, add a subtest that registers a reportingMockProvider configured to return an error from its ReportSARIF method, then verify that the ReportSARIF function properly propagates that error. These tests ensure that negative-path behavior (when providers fail) is explicitly covered as part of the test suite.Source: Coding guidelines
pkg/ci/providers/github/codescanning_test.go (1)
60-70: ⚡ Quick winAssert HTTP method and endpoint in the upload test spy.
Right now the test validates payload content only; a wrong API route/method could still pass.
Proposed hardening.
-type captureTransport struct{ body []byte } +type captureTransport struct { + body []byte + method string + path string +} func (c *captureTransport) RoundTrip(req *http.Request) (*http.Response, error) { + c.method = req.Method + c.path = req.URL.Path if req.Body != nil { c.body, _ = io.ReadAll(req.Body) } @@ require.NoError(t, err) + assert.Equal(t, http.MethodPost, ct.method) + assert.Contains(t, ct.path, "/code-scanning/sarifs")As per coding guidelines, “All features need tests” and tests should cover command-flow behavior, not only payload internals.
Also applies to: 72-104
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/ci/providers/github/codescanning_test.go` around lines 60 - 70, The RoundTrip method in the captureTransport type currently only captures the request body but does not validate the HTTP method or endpoint being called, allowing incorrect API routes or methods to pass the test silently. Add assertions or validation logic within the RoundTrip method to verify that the incoming request uses the correct HTTP method and calls the expected API endpoint, ensuring the test covers the complete command flow behavior and not just the payload content. Store the method and request URL so that test assertions can verify these critical details match expectations.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/ci/providers/github/codescanning.go`:
- Around line 27-29: The error returned from the ensureClient() call in the
ReportSARIF function is not wrapped with the ErrCISARIFUploadFailed sentinel
error, while the rest of the function wraps errors with this sentinel. Update
the error handling block where ensureClient() is called to wrap the returned
error with ErrCISARIFUploadFailed instead of returning the raw error, ensuring
consistency across the ReportSARIF function and compliance with the coding
guidelines that require all errors to be wrapped using static error sentinels
from errors/errors.go.
In `@pkg/ci/results.go`:
- Around line 37-52: The Annotate function and the related code section (lines
58-73) are returning provider errors directly without wrapping them in static
error sentinels, which violates the API boundary contract. Wrap the errors
returned from provider operations (specifically the return statement returning
a.Annotate(annotations) and similar provider error returns in the other section)
using the static error sentinels defined in errors/errors.go before returning
them to ensure all errors at this CI package boundary are properly wrapped
according to the coding guidelines.
In `@pkg/hooks/sarif/format.go`:
- Around line 32-34: The customFormatOutputPath function joins ctx.OutputDir
with ctx.Hook.Results without validating the input, allowing path traversal
attacks such as ../other.sarif to escape the output directory. Add validation
before the filepath.Join call to enforce that ctx.Hook.Results is a non-escaping
relative path by rejecting absolute paths (check if filepath.IsAbs returns true)
and rejecting paths containing parent directory traversal attempts (check if the
path contains .. components). Only allow the filepath.Join to proceed if the
validation confirms the path is a safe relative subpath within the output
directory.
---
Nitpick comments:
In `@pkg/ci/providers/github/codescanning_test.go`:
- Around line 60-70: The RoundTrip method in the captureTransport type currently
only captures the request body but does not validate the HTTP method or endpoint
being called, allowing incorrect API routes or methods to pass the test
silently. Add assertions or validation logic within the RoundTrip method to
verify that the incoming request uses the correct HTTP method and calls the
expected API endpoint, ensuring the test covers the complete command flow
behavior and not just the payload content. Store the method and request URL so
that test assertions can verify these critical details match expectations.
In `@pkg/ci/results_test.go`:
- Around line 31-99: Add new subtests to both TestAnnotate and TestReportSARIF
functions to validate error propagation from providers. In TestAnnotate, add a
subtest that registers a reportingMockProvider configured to return an error
from its Annotate method, then verify that the Annotate function properly
propagates that error. Similarly, in TestReportSARIF, add a subtest that
registers a reportingMockProvider configured to return an error from its
ReportSARIF method, then verify that the ReportSARIF function properly
propagates that error. These tests ensure that negative-path behavior (when
providers fail) is explicitly covered as part of the test suite.
In `@website/blog/2026-06-15-scanner-findings-in-ci-job-summary.mdx`:
- Line 35: Apply three style improvements to the markdown blog post: replace the
straight quotation marks around "no findings" with typographic (curly) quotation
marks on line 35, add a comma after "GitHub Actions" on line 39 before the
parenthetical remark, and capitalize "Markdown" as a proper noun on line 45 to
follow standard style conventions flagged by LanguageTool.
In `@website/blog/2026-06-19-scanner-annotations-and-code-scanning.mdx`:
- Line 8: Fix three style issues in the markdown file. On line 8, ensure all
instances of "GitHub" are fully capitalized with a capital H. On line 34, the
word "analysis" is repeated twice in close proximity; replace one instance with
an appropriate synonym to improve readability. On line 60, the word "needs"
appears twice within a few lines; similarly replace one instance with a suitable
alternative word to avoid repetition.
In `@website/docs/stacks/hooks.mdx`:
- Line 531: Replace straight quotation marks with typographic (curly) quotation
marks around the phrases "terminal-friendly", "Pro-friendly", and "tfsec".
Additionally, capitalize "Markdown" as it is a proper noun, and add a comma
after "e.g." to follow standard punctuation rules. These cosmetic refinements
improve the overall polish and consistency of the documentation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6b6ec586-c457-44f2-aa6d-05e137f51013
📒 Files selected for processing (25)
Dockerfileerrors/errors.gopkg/ci/internal/provider/findings.gopkg/ci/providers/github/annotations.gopkg/ci/providers/github/annotations_test.gopkg/ci/providers/github/codescanning.gopkg/ci/providers/github/codescanning_test.gopkg/ci/results.gopkg/ci/results_test.gopkg/ci/summary.gopkg/ci/summary_test.gopkg/hooks/ci_reporting_test.gopkg/hooks/command_engine.gopkg/hooks/command_engine_ci_summary_test.gopkg/hooks/format.gopkg/hooks/format_sarif_test.gopkg/hooks/hook.gopkg/hooks/kind.gopkg/hooks/sarif/format.gopkg/hooks/sarif/handler.gopkg/schema/schema.gowebsite/blog/2026-06-15-scanner-findings-in-ci-job-summary.mdxwebsite/blog/2026-06-19-scanner-annotations-and-code-scanning.mdxwebsite/docs/stacks/hooks.mdxwebsite/src/data/roadmap.js
… for ci.results Code Scanning is free on public repos; private repos need GitHub Advanced Security, which GitHub licenses as a paid add-on per active committer — Atmos charges nothing for any CI reporting. Annotations and the summary need no add-on. Make this explicit in the hooks docs and blog. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
website/blog/2026-06-19-scanner-annotations-and-code-scanning.mdx (1)
60-60: 💤 Low valueStreamline the permissions paragraph for clarity.
Line 60 is dense and repeats key terms. The LanguageTool hints flag word repetition ("add-on", "paid") and the overuse of "exactly". Consider tightening this sentence for better flow:
Current: "Annotations and the summary need no special permissions and no paid add-on — they work on any repo.
ci.resultsuploads to Code Scanning, which needssecurity-events: writeand, on private repos, GitHub Advanced Security — a paid add-on that GitHub licenses per active committer. Code Scanning is free on public repos. That's exactly why annotations default on andci.resultsdefaults off — everyone gets inline feedback for free, and only GHAS subscribers opt into the Security-tab integration."Suggested revision: "Annotations and the summary need no special permissions and work on any repo.
ci.resultsuploads to Code Scanning, which requiressecurity-events: writeand, on private repos, GitHub Advanced Security (licensed per active committer). Code Scanning is free on public repos. This is why annotations default on andci.resultsdefaults off — everyone gets inline feedback at no cost; GHAS subscribers gain the Security-tab integration."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@website/blog/2026-06-19-scanner-annotations-and-code-scanning.mdx` at line 60, The paragraph starting with "Annotations and the summary need no special permissions..." contains redundant word repetitions ("add-on", "paid") and overuse of "exactly" that makes it dense and harder to read. Streamline this paragraph by removing the phrase "no paid add-on" from the first sentence (keep just "need no special permissions and work on any repo"), replacing "needs" with "requires", condensing the GitHub Advanced Security description from "a paid add-on that GitHub licenses per active committer" to just "(licensed per active committer)", and changing "That's exactly why" to "This is why". Additionally, tighten the final clause by changing "everyone gets inline feedback for free, and only GHAS subscribers opt into" to "everyone gets inline feedback at no cost; GHAS subscribers gain" to improve flow and reduce repetition.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@website/blog/2026-06-19-scanner-annotations-and-code-scanning.mdx`:
- Line 60: The paragraph starting with "Annotations and the summary need no
special permissions..." contains redundant word repetitions ("add-on", "paid")
and overuse of "exactly" that makes it dense and harder to read. Streamline this
paragraph by removing the phrase "no paid add-on" from the first sentence (keep
just "need no special permissions and work on any repo"), replacing "needs" with
"requires", condensing the GitHub Advanced Security description from "a paid
add-on that GitHub licenses per active committer" to just "(licensed per active
committer)", and changing "That's exactly why" to "This is why". Additionally,
tighten the final clause by changing "everyone gets inline feedback for free,
and only GHAS subscribers opt into" to "everyone gets inline feedback at no
cost; GHAS subscribers gain" to improve flow and reduce repetition.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e500925d-f169-4d47-8dad-282480eb29ba
📒 Files selected for processing (2)
website/blog/2026-06-19-scanner-annotations-and-code-scanning.mdxwebsite/docs/stacks/hooks.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- website/docs/stacks/hooks.mdx
Erik Osterman (Cloud Posse) (osterman)
had a problem deploying
to
screengrabs
June 19, 2026 20:01 — with 
GitHub Actions
Failure
GitHub Actions
Failure
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/test.yml (1)
666-670: Disable implicit setup-node package-manager caching in this validation step.Since this job only runs schema validation with no npm dependency installation, add
package-manager-cache: falseto the setup-node action to keep the step's behavior explicit and avoid potential cache side effects. This follows setup-node v6 best practices for workflows that don't need caching.Suggested hardening.
- name: Set up Node for YAML schema validation uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: 24 + package-manager-cache: false🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test.yml around lines 666 - 670, In the "Set up Node for YAML schema validation" step, add `package-manager-cache: false` to the with section of the setup-node action following the node-version configuration. Since this validation step does not perform npm dependency installation, explicitly disabling the package manager cache keeps the behavior clear and prevents potential cache side effects that could occur with the implicit caching behavior in setup-node v6.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/screengrabs.yaml:
- Around line 88-93: The `actions/create-github-app-token` action is currently
not restricting token permissions to what the workflow actually needs. Add a
`permissions` field within the `with` section to scope the token to only the
necessary permissions for creating and pushing pull requests. The minimal
required permissions should include contents write access for pushing commits
and pull-requests write access for creating and managing PRs, removing any
unnecessary permissions that the token would otherwise inherit from the GitHub
App installation.
In @.github/workflows/website-preview-deploy.yml:
- Around line 55-60: The GitHub App token created by the
actions/create-github-app-token action with id github-app is inheriting the full
installation permissions of the app, violating the principle of least privilege.
Since this token is only used for artifact downloads, add a permission-actions
field to the with block of the github-app step and set its value to read to
restrict the token to read-only access for GitHub Actions.
---
Nitpick comments:
In @.github/workflows/test.yml:
- Around line 666-670: In the "Set up Node for YAML schema validation" step, add
`package-manager-cache: false` to the with section of the setup-node action
following the node-version configuration. Since this validation step does not
perform npm dependency installation, explicitly disabling the package manager
cache keeps the behavior clear and prevents potential cache side effects that
could occur with the implicit caching behavior in setup-node v6.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7b18a116-18b6-46ed-b2df-8f21a3d5332c
📒 Files selected for processing (24)
.github/actions/go-version-check/action.yml.github/actions/verify-sha-pinning/action.yml.github/workflows/algolia.yaml.github/workflows/atmos-pro.yaml.github/workflows/codeql.yml.github/workflows/dependency-review.yml.github/workflows/pre-commit.yml.github/workflows/screengrabs.yaml.github/workflows/setup-go-cache-warmup.yml.github/workflows/test.yml.github/workflows/validate-codeowners.yml.github/workflows/website-deploy-prod.yml.github/workflows/website-preview-build.yml.github/workflows/website-preview-deploy.yml.github/workflows/website-preview-destroy.ymlerrors/errors.gopkg/ci/providers/github/codescanning.gopkg/ci/providers/github/codescanning_test.gopkg/ci/results.gopkg/ci/results_test.gopkg/hooks/sarif/format.gopkg/hooks/sarif/format_test.gotests/snapshots/TestCLICommands_atmos_describe_config.stdout.goldentests/snapshots/TestCLICommands_secrets-masking_describe_config.stdout.golden
✅ Files skipped from review due to trivial changes (5)
- .github/workflows/atmos-pro.yaml
- .github/workflows/pre-commit.yml
- tests/snapshots/TestCLICommands_atmos_describe_config.stdout.golden
- .github/workflows/algolia.yaml
- .github/workflows/website-preview-build.yml
🚧 Files skipped from review as they are similar to previous changes (4)
- pkg/ci/results.go
- pkg/ci/providers/github/codescanning.go
- errors/errors.go
- pkg/hooks/sarif/format.go
Erik Osterman (Cloud Posse) (osterman)
requested a review
from a team
as a code owner
June 19, 2026 20:34
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
… in Docker image Security scanner hooks (Checkov, Trivy, KICS, Infracost) already render a markdown findings summary to the terminal, but it never reached the CI pipeline — findings were buried in the `atmos terraform plan` log stream. Route that summary to the GitHub Actions job step summary automatically when running in GitHub Actions ($GITHUB_STEP_SUMMARY), reusing the existing CI provider OutputWriter seam: - pkg/ci/summary.go: new ci.WriteStepSummary helper (no-op outside CI / when no summary destination), hiding the internal provider OutputWriter type. - pkg/hooks/command_engine.go: renderCISummary, called after renderTerminal in CommandEngine.Run. Best-effort — a step-summary write failure logs at debug and never fails the hook. Covers all four CommandEngine-based scanner/cost kinds uniformly. Also fix Checkov crashing inside the official Atmos Docker image: bump the base image from debian:bookworm-slim (glibc 2.36) to debian:trixie-slim (glibc 2.41) so the PyInstaller-bundled Checkov binary (needs GLIBC_2.38+) loads its frozen Python runtime instead of failing with a missing-version error. Includes unit + end-to-end tests, hooks docs, changelog blog post, and roadmap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses CodeRabbit review on PR #2617: the milestone description grouped Infracost under "security scanner hooks"; Infracost is a cost-analysis tool. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/hooks/hooks.go (1)
115-172: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd perf tracking to
RunAllto match the public-function contract.
RunAllis touched and remains a public orchestration path, but it still lacksdefer perf.Track(atmosConfig, "hooks.Hooks.RunAll")().Suggested patch
func (h *Hooks) RunAll(event HookEvent, atmosConfig *schema.AtmosConfiguration, info *schema.ConfigAndStacksInfo, cmd *cobra.Command, args []string) error { + defer perf.Track(atmosConfig, "hooks.Hooks.RunAll")() + log.Debug("Running hooks", "count", len(h.items)) skipPredicate := newSkipPredicate(resolveSkipHooks(cmd))As per coding guidelines, public functions should include
defer perf.Track(atmosConfig, "pkg.FuncName")()unless explicitly exempt, and this function does not match the listed exceptions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/hooks/hooks.go` around lines 115 - 172, The RunAll method is a public function that orchestrates hook execution but is missing the required performance tracking instrumentation. Add a defer statement with perf.Track at the beginning of the RunAll function body that tracks performance using the atmosConfig parameter and a string identifier in the format "hooks.Hooks.RunAll" to comply with the public function instrumentation guidelines.Source: Coding guidelines
🧹 Nitpick comments (3)
website/docs/cli/configuration/stores.mdx (1)
237-280: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winClarify endpoint field naming—use
endpoint_urlconsistently for stores.The individual store sections (SSM, Secrets Manager, GCP at lines 153, 174, 223) document
endpoint:as the primary field with a note that "endpoint_urlis an alias;endpointwins if both are set." However, the new Floci example usesendpoint_url:directly (line 262). This inconsistency will confuse users about which field to use.Since the PR migrates to
spec.endpoint_urlfor auth identities, consider updating the individual store sections to showendpoint_url:as the primary field (matching the Floci example and the auth.identities precedent). This makes the documentation clearer and more consistent across the feature.📝 Suggested updates to align endpoint field naming
Update lines 153, 174, and 223 to prefer
endpoint_url:as the primary field:# Optional: custom endpoint for AWS-compatible local/test APIs - # (`endpoint_url` is an accepted alias; `endpoint` wins if both are set) - endpoint: http://localhost:4566 + # (`endpoint` is an accepted alias; `endpoint_url` wins if both are set) + endpoint_url: http://localhost:4566Apply the same change to the GCP section at line 223.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@website/docs/cli/configuration/stores.mdx` around lines 237 - 280, The documentation uses inconsistent endpoint field naming across store configuration examples. In the individual store sections for aws/ssm, gcp/secretmanager, and azure/keyvault (referenced at lines 153, 174, and 223), the primary field is documented as `endpoint:` with a note that `endpoint_url` is an alias. However, the new Floci Local Endpoints example uses `endpoint_url:` directly. To align the documentation, update those three earlier store configuration examples to show `endpoint_url:` as the primary field instead of `endpoint:`, removing or de-emphasizing the note about aliasing. This makes the field naming consistent throughout the documentation and matches the precedent set by auth.identities.spec.endpoint_url and the new Floci example.pkg/config/identity_dots_test.go (1)
76-100: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winMake the key-preservation check actually exercise preservation.
This test name says preserving keys, but
floci.superuseris already lowercase and won’t catch case-preservation regressions. Use a mixed-case identity key and assert the preserved/original mapping alongside theendpoint_urlassertion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/config/identity_dots_test.go` around lines 76 - 100, The test TestAuthIdentitiesResolveYAMLFunctionsInSpecWhenPreservingKeys currently uses the all-lowercase identity key floci.superuser, which doesn't actually exercise key case preservation since there's no case variation to preserve. Change the identity key in the configContent YAML to use mixed case (e.g., Floci.Superuser or similar) and update the corresponding retrieval from config.Auth.Identities to use the same mixed-case key. Then add an assertion to verify that the original casing of the identity key is preserved in config.Auth.Identities alongside the existing endpoint_url check, ensuring the test actually validates that case preservation is working correctly.pkg/auth/identities/aws/assume_role_test.go (1)
661-730: 🧹 Nitpick | 🔵 Trivial | 🏗️ Heavy liftPrefer dependency-injected unit tests for
newSTSClientpaths.Using
tests.RequireAWSProfile(...)here makes endpoint behavior checks environment-dependent and skippable; these endpoint/no-endpoint paths are better asserted with mocked loaders/clients so they always execute in CI.
As per coding guidelines, "Use interfaces + dependency injection for testability. Generate mocks ... Prefer unit tests with mocks over integration tests. Target >80% coverage."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/auth/identities/aws/assume_role_test.go` around lines 661 - 730, Replace the integration test approach in both TestAssumeRoleIdentity_newSTSClient_WithEndpoint and TestAssumeRoleIdentity_newSTSClient_WithoutEndpoint with unit tests that use dependency injection and mocks. Remove the tests.RequireAWSProfile calls that make these tests environment-dependent and skippable. Instead, mock the STS client creation and any loaders used by the newSTSClient method so that the endpoint and non-endpoint code paths are always tested in CI without requiring real AWS credentials or specific AWS profiles. This ensures consistent test execution and maintains >80% coverage per coding guidelines.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/native-ci.yml:
- Around line 73-97: The workflow downloads Trivy and KICS binaries without
verifying their integrity, creating a supply-chain security risk. After each
curl command that downloads trivy.tar.gz and kics.tar.gz, add integrity
verification by downloading the corresponding checksum files from the same
GitHub release URLs, then use sha256sum to verify the downloaded archives match
their expected checksums before proceeding with the tar extraction commands.
This ensures the binaries have not been tampered with before execution.
In `@pkg/auth/cloud/aws/resolver_test.go`:
- Line 23: The single-line comments in the resolver_test.go file are missing
trailing periods, which violates the repo's Go comment style enforced by the
godot linter. Add a period to the end of the comment "Test legacy identity
resolver takes precedence" on line 23 and also add a period to the comment on
line 303 to ensure all comments follow the required format of ending with
periods.
In `@pkg/auth/cloud/aws/setup.go`:
- Around line 127-142: The issue is that when the identity lookup succeeds via
lowercase fallback at lines 132-133, the provider fallback on line 141 still
uses the original identityName casing instead of the key that actually matched.
This causes ResolveProviderConfig to potentially miss the correct provider
configuration. Modify the code to track which identity key actually matched
during the lookup (either the original casing or the lowercase version), and
pass that resolved key to ResolveProviderConfig instead of always using the
original identityName parameter.
In `@pkg/auth/identities/aws/assume_role.go`:
- Around line 61-62: The comment "// Add custom endpoint if configured" on line
61 is missing a trailing period, which violates the Go comment style guidelines
enforced by the godot linter. Add a period at the end of the comment so it reads
"// Add custom endpoint if configured." to ensure compliance with the coding
standards that require all comments to end with periods.
In `@pkg/auth/identities/aws/permission_set.go`:
- Line 408: The inline comment "Add custom endpoint if configured" is missing a
terminal period at the end. Add a period to the end of this comment to comply
with the godot linter rule that requires all comments in Go files to end with
periods.
In `@pkg/auth/providers/aws/saml_test.go`:
- Line 717: The comments on lines 717 and 745 in the SAML test file are missing
trailing periods, which violates the godot linter rules. Add a period at the end
of the comment "Test SAML provider with custom endpoint configuration" on line
717 and ensure any other comment on line 745 also ends with a period to comply
with the coding guidelines that require all comments to end with periods.
In `@pkg/auth/providers/aws/saml.go`:
- Around line 293-294: The comment on line 293 that precedes the
awsCloud.GetBaseEndpointConfigOption call is missing a period at the end. Update
the comment "// Add custom endpoint if configured" to "// Add custom endpoint if
configured." by adding a period to the end to comply with the repository's Go
comment rule enforced by the godot linter, which requires all comments to end
with periods.
In `@pkg/hooks/sarif/handler_test.go`:
- Around line 20-22: The three self-registration import comments for the
checkov, kics, and trivy kind packages are missing terminal periods, which
violates the godot comment rule that requires all Go comments to end with
periods. Add a period at the end of each inline comment following the import
paths for the checkov, kics, and trivy imports to ensure they read as complete
sentences ending with punctuation.
In `@pkg/hooks/sarif/normalize.go`:
- Line 20: The inline comment on the bindGitHubWorkspaceOnce variable
declaration is missing a period at the end. Add a period to the end of the
comment that says "one-time viper env binding" to comply with Go comment
punctuation standards.
---
Outside diff comments:
In `@pkg/hooks/hooks.go`:
- Around line 115-172: The RunAll method is a public function that orchestrates
hook execution but is missing the required performance tracking instrumentation.
Add a defer statement with perf.Track at the beginning of the RunAll function
body that tracks performance using the atmosConfig parameter and a string
identifier in the format "hooks.Hooks.RunAll" to comply with the public function
instrumentation guidelines.
---
Nitpick comments:
In `@pkg/auth/identities/aws/assume_role_test.go`:
- Around line 661-730: Replace the integration test approach in both
TestAssumeRoleIdentity_newSTSClient_WithEndpoint and
TestAssumeRoleIdentity_newSTSClient_WithoutEndpoint with unit tests that use
dependency injection and mocks. Remove the tests.RequireAWSProfile calls that
make these tests environment-dependent and skippable. Instead, mock the STS
client creation and any loaders used by the newSTSClient method so that the
endpoint and non-endpoint code paths are always tested in CI without requiring
real AWS credentials or specific AWS profiles. This ensures consistent test
execution and maintains >80% coverage per coding guidelines.
In `@pkg/config/identity_dots_test.go`:
- Around line 76-100: The test
TestAuthIdentitiesResolveYAMLFunctionsInSpecWhenPreservingKeys currently uses
the all-lowercase identity key floci.superuser, which doesn't actually exercise
key case preservation since there's no case variation to preserve. Change the
identity key in the configContent YAML to use mixed case (e.g., Floci.Superuser
or similar) and update the corresponding retrieval from config.Auth.Identities
to use the same mixed-case key. Then add an assertion to verify that the
original casing of the identity key is preserved in config.Auth.Identities
alongside the existing endpoint_url check, ensuring the test actually validates
that case preservation is working correctly.
In `@website/docs/cli/configuration/stores.mdx`:
- Around line 237-280: The documentation uses inconsistent endpoint field naming
across store configuration examples. In the individual store sections for
aws/ssm, gcp/secretmanager, and azure/keyvault (referenced at lines 153, 174,
and 223), the primary field is documented as `endpoint:` with a note that
`endpoint_url` is an alias. However, the new Floci Local Endpoints example uses
`endpoint_url:` directly. To align the documentation, update those three earlier
store configuration examples to show `endpoint_url:` as the primary field
instead of `endpoint:`, removing or de-emphasizing the note about aliasing. This
makes the field naming consistent throughout the documentation and matches the
precedent set by auth.identities.spec.endpoint_url and the new Floci example.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0e815307-d611-452d-b3c8-4481502080e7
📒 Files selected for processing (67)
.github/workflows/native-ci.yml.github/workflows/screengrabs.yaml.github/workflows/test.yml.github/workflows/website-preview-deploy.yml.tool-versionsagent-skills/skills/atmos-auth/references/providers-and-identities.mdcmd/root.godemo/screengrabs/build-all.shdocs/fixes/2026-06-17-aws-stores-secrets-auth-and-gists.mdexamples/auth-stores/atmos.yamlexamples/demo-floci/atmos.yamlexamples/quick-start-advanced/stacks/catalog/vpc-flow-logs-bucket/defaults.yamlexamples/quick-start-advanced/stacks/catalog/vpc/defaults.yamllychee.tomlpkg/auth/cloud/aws/README.mdpkg/auth/cloud/aws/resolver.gopkg/auth/cloud/aws/resolver_test.gopkg/auth/cloud/aws/setup.gopkg/auth/cloud/aws/setup_test.gopkg/auth/identities/aws/assume_role.gopkg/auth/identities/aws/assume_role_test.gopkg/auth/identities/aws/assume_root_test.gopkg/auth/identities/aws/permission_set.gopkg/auth/identities/aws/sts_client.gopkg/auth/identities/aws/sts_client_test.gopkg/auth/identities/aws/user.gopkg/auth/providers/aws/saml.gopkg/auth/providers/aws/saml_test.gopkg/auth/providers/aws/sso.gopkg/auth/providers/aws/sso_test.gopkg/ci/providers/github/codescanning.gopkg/ci/providers/github/codescanning_test.gopkg/config/identity_dots_test.gopkg/hooks/ci_reporting_test.gopkg/hooks/command_engine.gopkg/hooks/format_sarif_test.gopkg/hooks/hooks.gopkg/hooks/hooks_test.gopkg/hooks/sarif/handler.gopkg/hooks/sarif/handler_test.gopkg/hooks/sarif/markdown.gopkg/hooks/sarif/normalize.gopkg/hooks/sarif/normalize_test.gopkg/hooks/sarif/sarif_test.gopkg/schema/schema.gopkg/schema/schema_auth.gotests/fixtures/scenarios/aws-secrets-floci/atmos.yamltests/fixtures/scenarios/aws-store-hooks-floci/atmos.yamltests/fixtures/scenarios/native-ci-e2e/.gitignoretests/fixtures/scenarios/native-ci-e2e/README.mdtests/fixtures/scenarios/native-ci-e2e/atmos.yamltests/fixtures/scenarios/native-ci-e2e/components/terraform/bucket/.terraform.lock.hcltests/fixtures/scenarios/native-ci-e2e/components/terraform/bucket/checkov_target.tftests/fixtures/scenarios/native-ci-e2e/components/terraform/bucket/kics_target.tftests/fixtures/scenarios/native-ci-e2e/components/terraform/bucket/trivy_target.tftests/fixtures/scenarios/native-ci-e2e/stacks/catalog/bucket.yamltests/fixtures/scenarios/native-ci-e2e/stacks/deploy/test.yamltests/snapshots/TestCLICommands_describe_component_provenance_advanced.stdout.goldentests/snapshots/TestCLICommands_describe_component_with_provenance_and_stack.stdout.goldentests/snapshots/TestCLICommands_describe_component_with_stack_flag.stdout.goldenwebsite/blog/2026-06-19-scanner-annotations-and-code-scanning.mdxwebsite/docs/cli/configuration/auth/identities.mdxwebsite/docs/cli/configuration/auth/index.mdxwebsite/docs/cli/configuration/auth/providers.mdxwebsite/docs/cli/configuration/stores.mdxwebsite/docs/stacks/hooks.mdxwebsite/src/data/roadmap.js
💤 Files with no reviewable changes (3)
- tests/snapshots/TestCLICommands_describe_component_with_stack_flag.stdout.golden
- examples/quick-start-advanced/stacks/catalog/vpc/defaults.yaml
- examples/quick-start-advanced/stacks/catalog/vpc-flow-logs-bucket/defaults.yaml
✅ Files skipped from review due to trivial changes (16)
- pkg/auth/identities/aws/sts_client.go
- tests/fixtures/scenarios/native-ci-e2e/.gitignore
- website/docs/cli/configuration/auth/index.mdx
- pkg/schema/schema_auth.go
- .tool-versions
- website/docs/cli/configuration/auth/providers.mdx
- tests/fixtures/scenarios/aws-store-hooks-floci/atmos.yaml
- tests/fixtures/scenarios/native-ci-e2e/components/terraform/bucket/.terraform.lock.hcl
- pkg/hooks/sarif/sarif_test.go
- agent-skills/skills/atmos-auth/references/providers-and-identities.md
- .github/workflows/screengrabs.yaml
- tests/fixtures/scenarios/native-ci-e2e/README.md
- website/blog/2026-06-19-scanner-annotations-and-code-scanning.mdx
- website/src/data/roadmap.js
- tests/snapshots/TestCLICommands_describe_component_provenance_advanced.stdout.golden
- website/docs/stacks/hooks.mdx
🚧 Files skipped from review as they are similar to previous changes (9)
- cmd/root.go
- pkg/hooks/format_sarif_test.go
- pkg/ci/providers/github/codescanning_test.go
- tests/fixtures/scenarios/native-ci-e2e/stacks/deploy/test.yaml
- tests/fixtures/scenarios/native-ci-e2e/atmos.yaml
- pkg/ci/providers/github/codescanning.go
- .github/workflows/website-preview-deploy.yml
- pkg/schema/schema.go
- pkg/hooks/sarif/handler.go
🛑 Comments failed to post (9)
.github/workflows/native-ci.yml (1)
73-97:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winVerify scanner archives before extraction/execution.
The workflow downloads and runs Trivy/KICS binaries without integrity verification. That creates a supply-chain exposure in CI.
Suggested hardening patch
env: NATIVE_CI_TRIVY_VERSION: "0.70.0" NATIVE_CI_KICS_VERSION: "2.1.20" + NATIVE_CI_TRIVY_SHA256: "<pin-release-sha256>" + NATIVE_CI_KICS_SHA256: "<pin-release-sha256>" ... - name: Install scanner tools for native CI fixture run: | @@ curl -fsSL \ -o "$RUNNER_TEMP/trivy.tar.gz" \ "https://github.com/aquasecurity/trivy/releases/download/v${NATIVE_CI_TRIVY_VERSION}/trivy_${NATIVE_CI_TRIVY_VERSION}_Linux-64bit.tar.gz" + echo "${NATIVE_CI_TRIVY_SHA256} $RUNNER_TEMP/trivy.tar.gz" | sha256sum -c - tar -xzf "$RUNNER_TEMP/trivy.tar.gz" -C "$scanner_bin" trivy @@ curl -fsSL \ -o "$RUNNER_TEMP/kics.tar.gz" \ "https://github.com/Checkmarx/kics/releases/download/v${NATIVE_CI_KICS_VERSION}/kics_${NATIVE_CI_KICS_VERSION}_linux_amd64.tar.gz" + echo "${NATIVE_CI_KICS_SHA256} $RUNNER_TEMP/kics.tar.gz" | sha256sum -c - tar -xzf "$RUNNER_TEMP/kics.tar.gz" -C "$scanner_bin" kics🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/native-ci.yml around lines 73 - 97, The workflow downloads Trivy and KICS binaries without verifying their integrity, creating a supply-chain security risk. After each curl command that downloads trivy.tar.gz and kics.tar.gz, add integrity verification by downloading the corresponding checksum files from the same GitHub release URLs, then use sha256sum to verify the downloaded archives match their expected checksums before proceeding with the tar extraction commands. This ensures the binaries have not been tampered with before execution.pkg/auth/cloud/aws/resolver_test.go (1)
23-23:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFinish single-line comments with periods.
Line 23 and Line 303 comments are missing trailing periods; this violates repo Go comment style and can trip linting.
As per coding guidelines:
**/*.go: “All comments must end with periods (enforced by godot linter).”Also applies to: 303-303
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/auth/cloud/aws/resolver_test.go` at line 23, The single-line comments in the resolver_test.go file are missing trailing periods, which violates the repo's Go comment style enforced by the godot linter. Add a period to the end of the comment "Test legacy identity resolver takes precedence" on line 23 and also add a period to the comment on line 303 to ensure all comments follow the required format of ending with periods.Source: Coding guidelines
pkg/auth/cloud/aws/setup.go (1)
127-142:
⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize provider fallback lookup to the resolved identity key.
Line 141 still calls
ResolveProviderConfig(identityName)with original casing, even when Line 132-133 succeeded via lowercase fallback. If identity casing differs and identity-level endpoint is empty, provider fallback can miss and silently dropEndpointURL.💡 Suggested fix
func endpointURLFromManager(manager types.AuthManager, identityName string) string { if manager == nil { return "" } var identity *schema.Identity + resolvedIdentityName := identityName identities := manager.GetIdentities() if len(identities) > 0 { if found, ok := identities[identityName]; ok { identity = &found } else if found, ok := identities[strings.ToLower(identityName)]; ok { identity = &found + resolvedIdentityName = strings.ToLower(identityName) } } if url := baseEndpointURL(identity, nil); url != "" { return url } - provider, _ := manager.ResolveProviderConfig(identityName) + provider, _ := manager.ResolveProviderConfig(resolvedIdentityName) return baseEndpointURL(nil, provider) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/auth/cloud/aws/setup.go` around lines 127 - 142, The issue is that when the identity lookup succeeds via lowercase fallback at lines 132-133, the provider fallback on line 141 still uses the original identityName casing instead of the key that actually matched. This causes ResolveProviderConfig to potentially miss the correct provider configuration. Modify the code to track which identity key actually matched during the lookup (either the original casing or the lowercase version), and pass that resolved key to ResolveProviderConfig instead of always using the original identityName parameter.pkg/auth/identities/aws/assume_role.go (1)
61-62:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a trailing period to the comment.
Line 61 should end with a period to match enforced Go comment style.
As per coding guidelines:
**/*.go: “All comments must end with periods (enforced by godot linter).”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/auth/identities/aws/assume_role.go` around lines 61 - 62, The comment "// Add custom endpoint if configured" on line 61 is missing a trailing period, which violates the Go comment style guidelines enforced by the godot linter. Add a period at the end of the comment so it reads "// Add custom endpoint if configured." to ensure compliance with the coding standards that require all comments to end with periods.Source: Coding guidelines
pkg/auth/identities/aws/permission_set.go (1)
408-408:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnd the inline comment with a period.
Line 408 comment is missing terminal punctuation and can violate the repo’s
godotrule.✏️ Suggested fix
- // Add custom endpoint if configured + // Add custom endpoint if configured.As per coding guidelines:
**/*.go: All comments must end with periods (enforced bygodotlinter).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.// Add custom endpoint if configured.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/auth/identities/aws/permission_set.go` at line 408, The inline comment "Add custom endpoint if configured" is missing a terminal period at the end. Add a period to the end of this comment to comply with the godot linter rule that requires all comments in Go files to end with periods.Source: Coding guidelines
pkg/auth/providers/aws/saml_test.go (1)
717-717:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnd changed inline comments with periods.
Line 717 and Line 745 comments are missing trailing periods.
As per coding guidelines, "All comments must end with periods (enforced bygodotlinter)."Also applies to: 745-745
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/auth/providers/aws/saml_test.go` at line 717, The comments on lines 717 and 745 in the SAML test file are missing trailing periods, which violates the godot linter rules. Add a period at the end of the comment "Test SAML provider with custom endpoint configuration" on line 717 and ensure any other comment on line 745 also ends with a period to comply with the coding guidelines that require all comments to end with periods.Source: Coding guidelines
pkg/auth/providers/aws/saml.go (1)
293-294:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the missing period in the comment.
Line 293 should end with a period to satisfy the repo’s Go comment rule.
As per coding guidelines:
**/*.go: “All comments must end with periods (enforced by godot linter).”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/auth/providers/aws/saml.go` around lines 293 - 294, The comment on line 293 that precedes the awsCloud.GetBaseEndpointConfigOption call is missing a period at the end. Update the comment "// Add custom endpoint if configured" to "// Add custom endpoint if configured." by adding a period to the end to comply with the repository's Go comment rule enforced by the godot linter, which requires all comments to end with periods.Source: Coding guidelines
pkg/hooks/sarif/handler_test.go (1)
20-22:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEnd self-registration import comments with periods.
These inline comments are missing terminal periods and can fail the
godotcomment rule.Suggested patch
- _ "github.com/cloudposse/atmos/pkg/hooks/kinds/checkov" // self-register checkov kind - _ "github.com/cloudposse/atmos/pkg/hooks/kinds/kics" // self-register kics kind - _ "github.com/cloudposse/atmos/pkg/hooks/kinds/trivy" // self-register trivy kind + _ "github.com/cloudposse/atmos/pkg/hooks/kinds/checkov" // self-register checkov kind. + _ "github.com/cloudposse/atmos/pkg/hooks/kinds/kics" // self-register kics kind. + _ "github.com/cloudposse/atmos/pkg/hooks/kinds/trivy" // self-register trivy kind.As per coding guidelines, all Go comments must end with periods.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements._ "github.com/cloudposse/atmos/pkg/hooks/kinds/checkov" // self-register checkov kind. _ "github.com/cloudposse/atmos/pkg/hooks/kinds/kics" // self-register kics kind. _ "github.com/cloudposse/atmos/pkg/hooks/kinds/trivy" // self-register trivy kind.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/hooks/sarif/handler_test.go` around lines 20 - 22, The three self-registration import comments for the checkov, kics, and trivy kind packages are missing terminal periods, which violates the godot comment rule that requires all Go comments to end with periods. Add a period at the end of each inline comment following the import paths for the checkov, kics, and trivy imports to ensure they read as complete sentences ending with punctuation.Source: Coding guidelines
pkg/hooks/sarif/normalize.go (1)
20-20:
⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTerminate the inline global-state comment with a period.
The inline comment should end with a period to satisfy the Go comment punctuation rule.
Suggested patch
-var bindGitHubWorkspaceOnce sync.Once //nolint:gochecknoglobals // one-time viper env binding +var bindGitHubWorkspaceOnce sync.Once //nolint:gochecknoglobals // one-time viper env binding.As per coding guidelines, all Go comments must end with periods.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.var bindGitHubWorkspaceOnce sync.Once //nolint:gochecknoglobals // one-time viper env binding.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/hooks/sarif/normalize.go` at line 20, The inline comment on the bindGitHubWorkspaceOnce variable declaration is missing a period at the end. Add a period to the end of the comment that says "one-time viper env binding" to comply with Go comment punctuation standards.Source: Coding guidelines
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
…nnotations-results # Conflicts: # lychee.toml
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
June 22, 2026 01:05 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
- native-ci.yml: verify Trivy/KICS binary downloads against their published sha256 checksums before extraction (supply-chain hardening); KICS extracted-info.zip has no published checksum and is left as-is. - hooks.RunAll: add missing perf.Track instrumentation. - aws endpointURLFromManager: use the actually-matched identity key (original or lowercase fallback) when resolving the provider config. - identity_dots test: exercise real key-case preservation with a mixed-case key and assert IdentityCaseMap retains original casing. - stores docs: use endpoint: consistently in the Floci example. - godot: add trailing periods to several comments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
June 22, 2026 21:11 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
Add white-box unit tests for the uncovered/partial branches in the scanner CI-reporting paths, raising patch coverage on the changed files from ~82% to ~94% (set-mode) with comfortable margin over Codecov's partial-counted 80% gate. - pkg/hooks: emitCIAnnotations, publishCIResults (warn-normalize + verbatim + no-op branches), reportsAsWarning, firstSARIFToolName, normalizeSARIFLevels edge/error cases, deriveSARIFCategory fallback, and the format-handler registry (RegisterFormatHandler / formatHandlerFor / resolveResultHandler). - pkg/hooks/sarif: isWindowsDrivePath, cleanAbs, relUnder, fileExists, relativeBases, normalizeAbs/normalizeRel fallbacks, atmosBasePath, sourceComponentPath, customFormatOutputPath nil-guard. - pkg/ci/providers/github: withCategory malformed-run branches, ReportSARIF missing-context + upload-failure wrapping, Annotate write-error propagation. Test-only change; no production code modified. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
June 22, 2026 21:14 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
…nnotations-results
Erik Osterman (Cloud Posse) (osterman)
temporarily deployed
to
screengrabs
June 22, 2026 21:15 — with
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Warning SHA Pin Verification Failed10 of 101 SHA-pinned action(s) failed verification.
See the action run for full details. |
Andriy Knysh (aknysh)
approved these changes
Jun 24, 2026
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
|
Warning Release Documentation RequiredThis PR is labeled
|
|
These changes were released in v1.222.0-rc.9. |
This was referenced Jul 13, 2026
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
3 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.
what
ci.annotations(default on) — inline GitHub::error/::warningannotations anchored at each finding's file and line on the PR diff. The non-Code-Scanning path: needs no GitHub Advanced Security.ci.results(default off) — upload the raw SARIF to GitHub Code Scanning (Security tab) natively, with nogithub/codeql-actionstep. Analysis category is auto-derived from the scan target so per-component uploads don't overwrite each other.Annotator,SARIFReporter) — siblings of the existing check-run/comment/summary capabilities — not as hooks. All three reporting outputs (ci.summary/ci.annotations/ci.results) are gated byci.enabled.format: sarifto akind: commandhook — any SARIF-emitting tool (tfsec, semgrep, gitleaks, …) gets findings, annotations, and upload with no Go code.why
ci.*hook kinds) keeps CI reporting where it belongs and lets every SARIF-emitting hook, built-in or custom, participate through one shared path.references
main; feat(hooks): surface scanner findings in CI job summary + fix Checkov in Docker image #2617 is not yet merged, so this PR's diff currently includes feat(hooks): surface scanner findings in CI job summary + fix Checkov in Docker image #2617's commits — they drop out once feat(hooks): surface scanner findings in CI job summary + fix Checkov in Docker image #2617 merges and this branch is rebased.🤖 Generated with Claude Code