Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces Nix flake-based builds for DevGuard OCI images and updates CI to build images with Nix, alongside SBOM graph/rendering tweaks to better support new SBOM inputs and visualization needs.
Changes:
- Add
flake.nix/flake.lockto builddevguardanddevguard-scannerOCI images via Nix and provide SBOM generation apps. - Update the GitHub Actions workflow to build OCI tarballs using
nix build(instead of the reusable build-image workflow). - Extend
devguard-cli sbom renderwith layout selection, subgraph rendering (--from), and optional inclusion of file components.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
normalize/sbom_graph.go |
Adjusts package-PURL detection used when building SBOM graphs from CycloneDX. |
go.mod |
Promotes gopkg.in/yaml.v3 to a direct dependency. |
flake.nix |
Adds Nix flake packaging for Go binaries, OCI images, and SBOM helper apps/scripts. |
flake.lock |
Pins Nix flake inputs for reproducible builds. |
cmd/devguard-cli/commands/sbom_render.go |
Enhances SBOM Graphviz rendering options and traversal behavior. |
.github/workflows/devguard-scanner.yaml |
Adds Nix-based image build jobs and artifacts for scanner/devguard images. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // For other formats (pdf, png, svg), call the chosen layout engine binary | ||
| if _, err := exec.LookPath(layout); err != nil { | ||
| return fmt.Errorf("graphviz layout engine %q not found in PATH\nInstall with: brew install graphviz (macOS) or apt-get install graphviz (Linux)", layout) | ||
| } | ||
|
|
||
| // Call dot command to generate the output | ||
| cmd := exec.Command("dot", "-T"+format, "-o", outputFile) | ||
| cmd := exec.Command(layout, "-T"+format, "-o", outputFile) | ||
| cmd.Stdin = strings.NewReader(dotContent) |
There was a problem hiding this comment.
Since the executable is now the selected layout engine (exec.Command(layout, ...)), the later failure message should no longer say "failed to run graphviz dot command". Update the error wording to reference the chosen layout (or the actual command) so users aren’t misled when twopi/sfdp fails.
| isFileNode := !includeFiles && | ||
| node.Type == normalize.GraphNodeTypeComponent && | ||
| node.Component != nil && | ||
| node.Component.Type == cdx.ComponentTypeFile | ||
|
|
||
| if isFileNode { | ||
| // Elide this node: don't render it, but keep traversing its | ||
| // children with the same effectiveParentID so transitive deps | ||
| // are still connected (A → file → C becomes A → C). | ||
| if visited[node.BOMRef] { | ||
| return // already processed this file node, avoid cycles | ||
| } | ||
| visited[node.BOMRef] = true | ||
| for child := range graph.Children(node.BOMRef) { | ||
| traverse(child, depth+1, effectiveParentID) | ||
| } | ||
| return |
There was a problem hiding this comment.
The new file-node elision uses a global visited check for file-type nodes. If the same file node is reachable from multiple parents, the second traversal will return early and you’ll miss edges from that other parent to the file node’s children (e.g., A→file→C and B→file→C renders only A→C). Consider tracking cycle-prevention separately (e.g., a recursion stack) or allowing file nodes to be “expanded” per-parent while still deduping rendered (non-elided) nodes.
|
|
||
| echo "[1/3] Nix closure SBOM" | ||
|
|
||
| store_path=$(nix path-info --derivation .#devguard) |
There was a problem hiding this comment.
In sbomBuildtime, store_path=$(nix path-info --derivation .#devguard) refers to the host-system derivation. On macOS this will resolve to a Darwin build, which won’t match the Linux OCI contents and may not be buildable without a Linux builder. Use the Linux-target derivation that backs the image (e.g., the pkgsLinux build or the OCI image derivation) so the SBOM reflects the actual container closure.
| store_path=$(nix path-info --derivation .#devguard) | |
| # Use the Linux-target OCI image derivation backing DevGuard, | |
| # not the host-system derivation, so the SBOM matches the container. | |
| store_path="${devguardOCI}" |
| - name: Upload oci-image artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: oci-image | ||
| path: image.tar | ||
| - name: Set IMAGE_TAG |
There was a problem hiding this comment.
This file previously pins GitHub Actions to commit SHAs (e.g., checkout/setup-go/upload-artifact earlier in the workflow), but the newly added steps use floating tags like actions/upload-artifact@v4. Pin these action versions to a commit SHA as well to keep supply-chain hardening consistent within the workflow.
normalize/sbom_graph.go
Outdated
|
|
||
| func looksLikePackagePURL(id string) bool { | ||
| return strings.HasPrefix(id, "pkg:") && strings.Contains(id, "@") | ||
| return strings.HasPrefix(id, "pkg:") // && strings.Contains(id, "@") |
There was a problem hiding this comment.
looksLikePackagePURL now contains commented-out logic (// && strings.Contains(id, "@")). Leaving dead code in a predicate that influences graph construction makes intent unclear. Either remove the commented condition or add a brief comment explaining why versionless PURLs must be accepted.
| return strings.HasPrefix(id, "pkg:") // && strings.Contains(id, "@") | |
| // Accept any string that starts with the PURL scheme; versionless PURLs are valid. | |
| return strings.HasPrefix(id, "pkg:") |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Tim Bastin <38261809+timbastin@users.noreply.github.com>
No description provided.