Conversation
Signed-off-by: Moritz Johner <beller.moritz@googlemail.com>
WalkthroughThe action.yml file is refactored to generate per-module SBOMs from discovered go.mod files instead of producing a single SBOM. Each module receives an individual SBOM file with a naming convention. The attestation workflow is modified to attest each per-module SBOM separately. Changes
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/actions/sign/action.yml (1)
141-145: Fail fast when no module SBOM files match before attestation.With
nullglob, this loop can silently do nothing. Add an explicit match-count check so attestation cannot pass as a no-op.Proposed refactor
shopt -s nullglob - for SBOM_FILE in sbom.gomod.*.${IMAGE_TAG}.spdx.json; do + GO_MODULE_SBOMS=(sbom.gomod.*.${IMAGE_TAG}.spdx.json) + if [[ ${`#GO_MODULE_SBOMS`[@]} -eq 0 ]]; then + echo "No Go module SBOM files found for attestation." + exit 1 + fi + for SBOM_FILE in "${GO_MODULE_SBOMS[@]}"; do echo "Attesting ${SBOM_FILE}" cosign attest --yes --new-bundle-format=false --use-signing-config=false --predicate "${SBOM_FILE}" --type spdx "${IMAGE_NAME}@${CONTAINER_DIGEST}" done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/sign/action.yml around lines 141 - 145, The attestation loop uses `shopt -s nullglob` so it can silently skip when no sbom.gomod.*.${IMAGE_TAG}.spdx.json files exist; add a pre-check that counts matching files (e.g., expand into an array like matches=(sbom.gomod.*.${IMAGE_TAG}.spdx.json)) and if the array is empty, print an error and exit non‑zero to fail fast before running the `for SBOM_FILE ...` loop and calling `cosign attest`; keep references to the `SBOM_FILE` variable, the `IMAGE_TAG`/`IMAGE_NAME`/`CONTAINER_DIGEST` environment variables, and the existing loop logic unchanged otherwise.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/sign/action.yml:
- Line 106: The find invocation that populates GO_MOD_FILES currently only
excludes vendor dirs at repo root ("./vendor/*"); update the exclusion to ignore
any nested vendor directories by changing the -not -path argument on the find
command used to set GO_MOD_FILES (the line starting with "mapfile -t
GO_MOD_FILES < <(find ... )") to use a glob that matches any vendor path (e.g.
use "*/vendor/*" style pattern) so nested ./foo/vendor/... entries are also
omitted from module discovery.
- Around line 119-120: MODULE_PATH is being flattened into MODULE_PATH_SAFE by
simply replacing '/' with '-' which can produce collisions (e.g., a/b-c vs
a-b/c); update the transformation that sets MODULE_PATH_SAFE so it uses a
collision-safe, reversible encoding of MODULE_PATH (for example percent-encode
slashes as '%2F' or use base64/url-safe base64 of MODULE_PATH) and then build
SBOM_FILE using that encoded MODULE_PATH_SAFE and IMAGE_TAG; adjust the
assignment that references MODULE_PATH_SAFE, MODULE_PATH and SBOM_FILE in the
action.yml accordingly so distinct module paths produce distinct SBOM filenames.
---
Nitpick comments:
In @.github/actions/sign/action.yml:
- Around line 141-145: The attestation loop uses `shopt -s nullglob` so it can
silently skip when no sbom.gomod.*.${IMAGE_TAG}.spdx.json files exist; add a
pre-check that counts matching files (e.g., expand into an array like
matches=(sbom.gomod.*.${IMAGE_TAG}.spdx.json)) and if the array is empty, print
an error and exit non‑zero to fail fast before running the `for SBOM_FILE ...`
loop and calling `cosign attest`; keep references to the `SBOM_FILE` variable,
the `IMAGE_TAG`/`IMAGE_NAME`/`CONTAINER_DIGEST` environment variables, and the
existing loop logic unchanged otherwise.
| syft dir:. -o spdx-json=sbom.gomod.${IMAGE_TAG}.spdx.json | ||
| # Go modules SBOMs (one per go.mod). This avoids a single very large | ||
| # monolithic predicate when the repo contains many modules. | ||
| mapfile -t GO_MOD_FILES < <(find . -type f -name go.mod -not -path "./vendor/*" | sort) |
There was a problem hiding this comment.
Broaden vendor exclusion in module discovery.
Line 106 only excludes ./vendor/* at repo root. Nested vendor directories (for example, ./foo/vendor/...) are still included and can pollute Go-module SBOMs.
Proposed fix
- mapfile -t GO_MOD_FILES < <(find . -type f -name go.mod -not -path "./vendor/*" | sort)
+ mapfile -t GO_MOD_FILES < <(find . -type f -name go.mod -not -path "*/vendor/*" | sort)📝 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.
| mapfile -t GO_MOD_FILES < <(find . -type f -name go.mod -not -path "./vendor/*" | sort) | |
| mapfile -t GO_MOD_FILES < <(find . -type f -name go.mod -not -path "*/vendor/*" | sort) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/sign/action.yml at line 106, The find invocation that
populates GO_MOD_FILES currently only excludes vendor dirs at repo root
("./vendor/*"); update the exclusion to ignore any nested vendor directories by
changing the -not -path argument on the find command used to set GO_MOD_FILES
(the line starting with "mapfile -t GO_MOD_FILES < <(find ... )") to use a glob
that matches any vendor path (e.g. use "*/vendor/*" style pattern) so nested
./foo/vendor/... entries are also omitted from module discovery.
| MODULE_PATH_SAFE="${MODULE_PATH//\//-}" | ||
| SBOM_FILE="sbom.gomod.${MODULE_PATH_SAFE}.${IMAGE_TAG}.spdx.json" |
There was a problem hiding this comment.
Avoid SBOM filename collisions across module paths.
Line 119-Line 120 can generate identical names for distinct module dirs (e.g., a/b-c and a-b/c both become a-b-c), causing overwrite and missing attestations.
Proposed fix
- MODULE_PATH_SAFE="${MODULE_PATH//\//-}"
- SBOM_FILE="sbom.gomod.${MODULE_PATH_SAFE}.${IMAGE_TAG}.spdx.json"
+ MODULE_PATH_SAFE="${MODULE_PATH//\//-}"
+ MODULE_PATH_HASH="$(printf '%s' "${MODULE_PATH}" | sha256sum | cut -c1-12)"
+ SBOM_FILE="sbom.gomod.${MODULE_PATH_SAFE}.${MODULE_PATH_HASH}.${IMAGE_TAG}.spdx.json"📝 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.
| MODULE_PATH_SAFE="${MODULE_PATH//\//-}" | |
| SBOM_FILE="sbom.gomod.${MODULE_PATH_SAFE}.${IMAGE_TAG}.spdx.json" | |
| MODULE_PATH_SAFE="${MODULE_PATH//\//-}" | |
| MODULE_PATH_HASH="$(printf '%s' "${MODULE_PATH}" | sha256sum | cut -c1-12)" | |
| SBOM_FILE="sbom.gomod.${MODULE_PATH_SAFE}.${MODULE_PATH_HASH}.${IMAGE_TAG}.spdx.json" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/sign/action.yml around lines 119 - 120, MODULE_PATH is being
flattened into MODULE_PATH_SAFE by simply replacing '/' with '-' which can
produce collisions (e.g., a/b-c vs a-b/c); update the transformation that sets
MODULE_PATH_SAFE so it uses a collision-safe, reversible encoding of MODULE_PATH
(for example percent-encode slashes as '%2F' or use base64/url-safe base64 of
MODULE_PATH) and then build SBOM_FILE using that encoded MODULE_PATH_SAFE and
IMAGE_TAG; adjust the assignment that references MODULE_PATH_SAFE, MODULE_PATH
and SBOM_FILE in the action.yml accordingly so distinct module paths produce
distinct SBOM filenames.
|
closing this one in favour of #6004 |
Problem Statement
What is the problem you're trying to solve?
Related Issue
Fixes #...
Proposed Changes
How do you like to solve the issue and why?
Format
Please ensure that your PR follows the following format for the title:
Where
scopeis optionally one of:Checklist
git commit --signoffmake testmake reviewableChanges
Refactors the SBOM generation workflow to create per-module SBOM files instead of a single monolithic file:
sbom.gomod.<module-path>.<IMAGE_TAG>.spdx.jsonusing syft with go-module-file-cataloger.File:
.github/actions/sign/action.yml(+40/-5 lines)