Skip to content

fix(sbom): retain workspace modules under the selected Go source root - #878

Draft
gael-obin wants to merge 2 commits into
codefly-dev:mainfrom
gael-obin:fix/go-workspace-sbom-root
Draft

gael-obin wants to merge 2 commits into
codefly-dev:mainfrom
gael-obin:fix/go-workspace-sbom-root

Conversation

@gael-obin

@gael-obin gael-obin commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Go workspaces can contain several main modules. Core's source SBOM previously chose the last one, potentially attaching an executable to the wrong source and omitting other workspace modules. This selects the requested source through Go's metadata, retains the other mains, and connects requirements to the selected workspace modules.

Classification: fix at the inventory owner. It reuses Core's existing Go inventory and artifact attachment and preserves isolated-module behavior. Service-go still owns selecting the graph for each build target. No scanner is added in a consumer, and this changes package evidence rather than PDF/image extraction.

Independent tests at 2b077f2e127efc07eeef9e6c577685f1f5229ad7 passed for workspace order, nested/aliased source paths, alternate module files, graph references and artifact attachment. Current main and the inspected Core 0.19.18 release still lack this correction.

Release and adoption by the Go packager remain necessary before normal CLI packaging can be qualified: codefly-dev/service-go#163 . Source-level tests do not establish a clean NYL installation, cross-platform runtime behavior, released artifact or deployment.

@gael-obin

Copy link
Copy Markdown
Contributor Author

Independent platform review — ACCEPTED (design) / QUALIFIED (implementation)

Fresh-context review under the architecture-bouncer skill at head 2b077f2e12 (scope 15e7e12257..2b077f2e12), taken together with the consumer-side codefly-dev/service-go#163. No conversation history with the implementing agents.

The allocation across the two PRs is right. Neither does the other's job, and the split follows the walk obin/handbook/design/dependency-ownership.md mandates. Both are needed; neither suffices alone. Nothing here blocks.

What this actually fixes, stated as a supply-chain consequence

Today, with released service-go v0.0.74 and Core v0.19.10: a Go service with with-workspace: true, packaged with include_sbom under an ambient multi-module go.work, writes a .cdx.json whose metadata.component names a different module and which omits the subject entirely, and AttachArtifact binds the executable's SHA-256 to that wrong subject. Reproduced by mutation — expected pkg:golang/example.test/app, actual pkg:golang/rsc.io/quote. The binary is correct; only the evidence is wrong.

service-go#159 alone would widen it: the build starts honouring the workspace while the inventory still follows Settings.WithWorkspace. Reproduced — a binary printing workspace sibling inventoried as rsc.io/quote@v1.5.1.

With #878 and #163 together, each target's inventory matches its own build — verified end to end through the public RPC with real binaries, native and Docker cross.

Credit

The os.SameFile-on-directories invariant is the right one and survived every workspace shape constructed against it: transitive workspace, unreferenced member, nested module in both directions, -modfile outside the module root, ambient go.work, go.work replace, go.mod replace, vendored. The PR caught and fixed its own GoMod→Dir regression inside the PR, with a test. No filesystem paths leak into the published BOM. The second commit ("preserve alternate Go module manifests") is a real fix, not churn.

Additive for existing consumers. Only UseWorkspace: true output changes, and an org-wide search finds service-go to be the only such caller (obin/service-docx:138 and obin/service-pdf:134 call sbom.Golang, pinned at core v0.17.0). sbom.Golang output was byte-identical in every probe. The cost is one extra go env GOMOD subprocess and one changed error message on a non-module directory.

C-A1 — advisory (design): GolangOptions.UseWorkspace means "inherit ambient GOWORK", not "the graph my build used"

agents/services/sbom/golang.go:67-73 either appends GOWORK=off or inherits os.Environ() unchanged — yet golang.go:25-30 and docs/sbom.md describe it as preserving "the workspace selected by the caller's build". A caller that sets GOWORK per command, which is exactly what service-go does for cross targets (pkg/builder/builder.go:610), cannot say so through the option.

Reproduced counterexample: service-go deliberately tolerates a padded GOWORK (package_gowork_test.go:55-58 asserts " /tmp/x/go.work " is trimmed). With that value the build succeeds and the inventory fails — go: invalid GOWORK: not an absolute path. Build and evidence disagree because one is explicit and the other ambient.

Core already answers this question explicitly elsewhere: runners/golang/formula.go:227-230 with workspace.go:29-69 resolves a source-owned go.work and sets GOWORK=<file> or off per command, deliberately refusing ambient parent workspaces. Two Core-owned answers to the same question, neither referencing the other, and the SBOM one is the weaker.

Closure: either the option carries the value (or reuses runners/golang's selection), or docs/sbom.md states the caller's obligation truthfully.

C-A2 — advisory (docs): two properties of a workspace inventory are unstated

Workspace main modules carry no version — a probe emits pkg:golang/example.test/sib with an empty version. That is honest, but it means workspace evidence is not release evidence and cannot be version-matched by a scanner. And all workspace mains become components, including ones the subject does not depend on; dropping an unselected released version's edges also leaves that version's dependencies as unreachable orphan components. Both are over-reports rather than under-reports — the safe direction — but a consumer reading the flat component list sees modules that are not in the binary. Two sentences in docs/sbom.md.

C-P1 — pre-existing gap, named because replace was in scope; not caused or worsened here

golang.go:97-100 overrides the version only when the replacement has one, so a replace => ../local reports the replaced version. Two probes (a go.work replace, and a go.mod replace under GOWORK=off): a binary containing LOCAL UNCOMMITTED code is inventoried as pkg:golang/example.test/lib@v1.4.2. This is deliberate — sbom_test.go:69 enshrines it — but unlike the workspace case it is an affirmative false identity claim, and it is reachable with no workspace at all. Medium as a standing gap. Not a blocker for this PR.

Release note

This fix is in no release: 15e7e12257 (this PR's merge base, equal to main) is v0.19.18-1-g15e7e122, and git merge-base --is-ancestor 15e7e12257 v0.19.18 is false. The newest released Core is v0.19.18 (2026-10-29). service-go#163 currently pins v0.19.10 and cannot go green until a release carries this.

What was run

go build, go vet, go test ./agents/services/sbom/... (pass), and full go test ./... (110 packages ok; one unrelated macOS failure in internal/testgit TestCancellationKillsGitEditorChildren — this PR touches only agents/services/sbom/* and docs/sbom.md). A mutation reverting the root selection fails the test correctly. Seven workspace-shape probes as listed above. Worktree left clean; no containers left behind.

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