Skip to content

Commit 909825d

Browse files
bomly-guyclaude
andauthored
feat!: adopt the typed graph node union (bomly-sdk v0.9.0, ADR-0041) (#423)
* feat!: adopt the typed graph node union across the CLI bomly-sdk v0.8.0 makes the dependency graph a sealed union of manifest, module, and dependency nodes, with identity minted as a canonical package URL at construction (ADR-0041). This is the CLI's adoption of that model. What changes in the pipeline: - The project's own artifacts are module nodes. Every detector that used to mint a dependency node and then mark it -- FirstParty on coordinates, a forced "workspace:<dir>" ID, DependencySourceWorkspace as a stand-in -- now builds a module node whose ID carries the declaring manifest path. Ownership is the node kind; there is nothing to set afterwards. - Duplicate identities fold. The occurrence machinery is gone: two records that resolved one name@version from different places become one node whose Origins list carries both, which is a stronger dependency-confusion signal than two nodes with byte-identical identity. - Application-typed imports are ordinary dependency nodes, so they diff and match like any other package (ADR-0015). Three helpers absorb what the migration would otherwise have scattered: - internal/nodes reads a node of any kind -- coordinates, display fields, narrowing. The GraphNode interface exposes only what every kind has, so every renderer needed the same type switch; written per caller it disagrees with itself about what a manifest looks like. The SDK is the deeper home (ADR-0040) and v0.8.0 has no coordinates accessor, so this delegates once bomly-dev/bomly-sdk#33 ships. - detectors.EnsureNode is generic in the node type, so inserting a module returns a module. A survivor of another kind is an error rather than a silent nil. - detectors.PropagateScopes replaces three copies of the same BFS scope walk, one per Python lockfile detector. Only the seed differed, so that is the parameter. Tests are migrated in the following commit. * test: migrate the suite to the typed graph node union Fixtures can no longer be struct literals with a hand-chosen ID, and graph traversal yields the union rather than dependency nodes, so every test that built or read a node had to change. Two helpers keep that from becoming a type switch per assertion: - internal/testnodes takes the fixture shapes the tests already used and routes them through the real constructors, panicking rather than returning an error -- a fixture whose coordinates cannot mint an identity is a broken test, not a condition under test. - a package-local mustDep narrows a node the case is asserting about, failing instead of panicking when the graph holds another kind. Three suites tested behaviour that no longer exists and were rewritten rather than adjusted, because adjusting them would have left tests that pass while pinning nothing: - The cargo dual-source cases asserted two nodes for one crate resolved from two remotes. They now assert the fold and both origins on the survivor, with the reasoning recorded in detectors.EnsureNode. - The consolidation origin suite was almost entirely about occurrence-ID minting and its order-independence. It now covers what folding must not lose: both resolutions, scopes, locations, the stronger relationship, and the separation between a project module and an external package that names the same coordinates. - detectors.EnsureOccurrence's test became a fold test over the same inputs. Node IDs in assertions are canonical package URLs now; lookups that keyed on the old "name@version" string either use the PURL or find the node by name. * test: resolve node lookups by label rather than by raw ID * test: restore the vulnerability, finding, and manifest IDs an over-broad rewrite dropped * test: match nodes by label through one helper rather than by raw ID * fix: keep gradle scopes and let a promoted module report its new ID * fix: detectors that build a node from a prototype keep every field they set The npm, pnpm, yarn, and bun lockfile parsers each described a package as one struct literal and passed only its coordinates to the constructor, so ResolvedURL, Source, the integrity digests, and the npm metadata stopped reaching the graph -- silently, in four places, for the same reason. detectors.NewDependencyFrom is that copy written once. * fix: npm workspace members are module nodes; node tests assert the fold * fix: cargo, swiftpm, and gradle carry ownership and origins through the union * fix: gradle subprojects and python roots are module nodes; detector suite green * test: engine, consolidation, and auditor suites assert the union * test: remediation and auditor fixtures use canonical identities * fix: restore the SBOM component group from the package URL namespace * test: output and TUI fixtures carry identities their ecosystems can mint * test: accounting invariants count structural nodes as their own kinds * fix: explain paths keep their project root and scan lists modules Two user-visible losses the union adoption would otherwise have shipped: - A dependency path rendered only its dependency nodes, so every explain path started at the first consumed package and never said which module pulled it in -- which is the question explain answers. Structural nodes render too now, through output.PackageFromGraphNode. - The scan document's manifest listing walked dependency nodes only, so every depends_on chain was headless. Modules are listed alongside packages; manifests are not, being what the listing is about. Also: explain resolves a target by its ecosystem-native name as well as its bare one. Normalization splits a qualified name into org and name, so "bomly explain golang.org/x/text" stopped matching anything. * test: the embedded plugin fixture compiles against the SDK's node constructor * docs: regenerate schemas for the SDK's reachability evidence types * test: pin that a baseline survives the identity change * chore: document the new packages, guard testnodes, drop the anchore purl fork go mod tidy demotes github.com/anchore/packageurl-go to an indirect dependency: nothing in this repository imports it any more, which is the removal ADR-0041 slated. purlkit over the official packageurl-go is the parser now. * fix: clear the dead assignments the migration left behind Five ineffectual assignments and one empty branch, each a leftover from a rewrite rather than a style nit -- and one was a swallowed error: cargo's root construction returned an err the next if-statement shadowed, so a node that failed to build would have been added as nil. The consolidation helper that searched for an uncolliding synthesized root ID is deleted with them: a manifest node's ID is minted from its path by a grammar that cannot collide with a package URL or a module ID. * style: regroup imports into stdlib and non-stdlib blocks The migration inserted imports at the top of each block, so the SDK and internal packages landed among the standard library. gofmt sorts within a group but never regroups, and nothing in .golangci.yml checks it. * test(smoke): make goldens machine-independent, then regenerate Regenerating locally surfaced three ways the goldens recorded the machine that produced them rather than anything Bomly decided. Each would have made a local regeneration break CI, and each was already latent: - Container scans baked in the runner's architecture. A multi-arch image resolves to the host, so amd64 on CI and arm64 on a laptop differ on every package in the image. Normalized to <arch>, with the alternation listing both spellings a package manager uses for one machine (amd64/x86_64) so a genuinely wrong arch still shows up. - Absolute temp paths leaked through the reachability evidence added in bomly-sdk v0.8.0: module_root carried /var/folders/... on macOS and /tmp on Linux. The random suffix was normalized; the prefix was not. - Strings inside arrays were never normalized at all -- only map values were -- so a package URL in a depends_on list kept the host arch while the same URL as a map value did not. And one that hid real coverage: the golden filter dropped every entry whose ID was not a package URL. That was written when synthetic roots had ad-hoc IDs; every node ID is minted by a grammar now, so the filter was silently removing every module and manifest node -- the project's own code, which is exactly what a workspace or reactor-build case exists to check. The min-version plugin fixture gets its own source. That test builds the example plugin against the oldest SDK release whose binaries must keep loading, and the guarantee is about the wire, not the source API: the node constructors replaced sdk.NewDependency, so one source cannot compile against both v0.1.0 and the pin. Both sources are now compile-checked against the release each targets. * fix(sbom): a module component publishes a package URL, not its node ID Adding a golden for the exported SBOM found this on the first run: a module component carried module:apps/web/package.json#pkg:npm/web@1.0.0 in the purl field, which both formats define as a Package URL and no consumer can parse. The bom-ref is where the node identity belongs; the purl is the one its coordinates mint. The export had assertions but no golden -- it is the artifact most consumers actually read, and nothing pinned its shape, so a change to which components appear at all went unchecked. Both formats are now exported from one scan and compared, so they cannot drift apart. Documents get their own golden path. normalizeJSON knows the scan/diff/explain response schema and edits it on sight -- it zeroes metadata.duration_ms, and a CycloneDX document has a metadata object too -- so running it over an SBOM invented a field neither side produced. * docs: record the verification gate and the pre-push hook * refactor: the SDK owns node reading and detector graph building bomly-sdk v0.9.0 ships what this branch had been carrying locally, so the stopgaps go and the call sites point at the model. Deleted, each replaced by its SDK counterpart: internal/nodes -> sdk.NodeCoordinates, NodeDisplayName, NodeVersion, AsDependencyNode, DependencyNodesOf, IsProjectOwned detectors.EnsureNode -> detectorkit.EnsureNode detectors.PromoteToModule -> detectorkit.PromoteToModule detectors.PropagateScopes -> detectorkit.PropagateScopes detectors.NewDependencyFrom -> sdk.NewDependencyNodeFrom detectors.NewDependencyOrGeneric -> sdk.NewDependencyNode (the generic fallback is the constructor's now) detectors.RefineOrigins -> sdk.MergeOrigins (which drops a superseded origin itself) internal/testnodes keeps only what is CLI test ergonomics -- fixture builders that panic instead of taking a testing.TB, so a table entry stays one expression. Its label lookups delegate to bomly-sdk/testkit, and its copy of the matching rules is gone: two answers to "which node is this" is how the version fold drifted in the first place. Its hand-maintained field list goes too, DepFrom now being NewDependencyNodeFrom. The two structural guards move to internal/detectors/guards_test.go and stay CLI-side, because they police this tree: TestNodeInsertionGoesThroughTheSharedHelper now names detectorkit.EnsureNode and exempts nothing, there being no local copy left to reach for. The Cargo fold decision moves to internal/detectors/cargo/lock_index.go, next to the fold it explains, rather than being deleted with the helper it was written above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: address the typed-union review findings Seven findings from review, each verified against the behavior before being believed, and each with the assertion that was missing: Remediation stopped recognizing project roots. executableRootOf narrowed to *DependencyNode, so a module root -- what a normal graph now has -- was rejected before executableRoot ran. inferredPlacement then found no root and every dependency whose detector left Relationship unset degraded to manual review, which is most of a normal scan. executableRoot's own module clause was dead for the same reason and is gone. The provider graph clone kept only dependency nodes, dropping the modules, manifests and root edges, and ignored AddEdge errors. Providers may inspect RemediationHintRequest.Detection, so a disconnected clone changes the plugin contract. It now clones every kind through CopyEdgesInto, edge kinds included. Explain paths published "name@version" while the focused dependency published its canonical node ID, so nothing could join a path entry to a dependency, package, or finding -- and the short form collides across ecosystems. The legacy rewrite is deleted and the function renamed for what it still does. A module component published no group: componentOrg parsed NodeID, which for a module is the structural "module:<path>#<purl>" grammar and no package URL at all. It reads componentPURL now, so group and purl agree. A manifest root decided the document's primary component. The manifest is not a component, so the encoder silently promoted whichever package sorted first to be the subject of the whole document -- a project with two top-level packages described itself as one of them. Structural roots now resolve to the exported nodes beneath them, which lets the synthesized project root form. Module locations were never rebased onto the subproject path, so a recursively discovered module reported "pom.xml" where the repository holds "apps/service/pom.xml", and scan JSON published it. The enrichment INFO log counted every structural node as an excluded package, contradicting the comment above it. Also: make verify now compiles the smoke suite. It is behind a build tag, so `go vet ./...` never saw it, and this branch pushed a smoke file that did not compile. Running smoke needs the network; compiling it costs a second. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: keep folded origins, module identities, and structural roots intact The rest of the review findings, plus the smoke coverage they exposed. Exported SBOMs published only the first origin. ADR-0041 folds equal-identity records and keeps their disagreement as a list, which is the dependency-confusion signal the fold exists to preserve -- so publishing one described a package that resolved from two registries as though it came from one, backwards for the case that matters most. Component carries the whole list now: CycloneDX emits an external reference per origin, and SPDX, which defines one download location per package, emits the rest as OTHER-category references under a document-defined type. The SPDX category and refType strings are the library's constants rather than transcriptions of them. Module identities were never rebased onto the subproject path. A module's ID carries its declaring manifest path, and a detector writes that relative to its own working directory, so with --recursive two nested projects sharing a package name both minted "module:package.json#pkg:npm/app@1.0.0" and folded into one node holding both projects' edges. Consolidation rebases them where it already rebases manifest paths and locations; the monorepo smoke golden now shows "module:fixtures/webapp/package.json#...". ensureEntryRoot attached only dependency roots, leaving an independent module or manifest root loose -- so the entry still had several roots and every root-based projection downstream got an ambiguous graph, which is the condition that function exists to remove. Non-root module nodes were missing from the top-level parents, so a module another module depends on had its own direct dependencies reported as transitive. The existing test missed it by building its "modules" as application-typed dependency nodes; the new one uses real module nodes and fails without the fix. The pre-push hook accepted a stale stamp on Linux. GNU stat reads -f as "filesystem" and succeeds, so choosing the BSD spelling by "did stdout come back non-empty" filled newest with an inode report, and the comparison then errored inside a condition the hook did not check. It probes the flag now. Smoke: the SBOM export case ran against a mutable branch, so its golden tracked the fixture repository rather than Bomly -- pinned to the tag, like its sibling. The CycloneDX tool version is a separate field the string normalizer could not reach, so the golden carried a literal release number that would fail on the next bump. SARIF had no smoke coverage at all, though it is what GitHub code scanning reads and how Guard annotates a pull request. The new case pins a real rule, result, severity and repo-relative location, driven by a denied package rather than advisory data: the shared mock OSV server answers every query with no vulnerabilities, so a vulnerability-auditor case would have pinned an empty document and caught nothing. Goldens regenerated. The reachability goldens show the remediation fix end-to-end -- manual-review becomes direct-bump and transitive-override across Go, npm and Maven -- and explain paths now publish canonical package URLs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: Python roots name their real manifest, and structural nodes their real PURL Two findings from Codex's review of 75f4889. Four Python parsers each hard-coded "requirements.txt" as the module's declaring manifest. A Pipenv project is declared by Pipfile and a Poetry project by pyproject.toml, and the declaring path is part of a module's identity now -- published in scan JSON, SBOM references and explain paths -- so the literal was both a wrong cross-reference and a fold risk between two projects declared by different files on matching coordinates. The rule gets one home, pythonModuleRoot, which derives the manifest from the package manager already on the coordinates, and TestPythonRootsGoThroughTheSharedConstructor fails if a direct NewModuleNode call reappears in the package. Routing uv through it exposed that its coordinates never stated their package manager -- it had the right literal by hand -- so it states it now and keeps pyproject.toml. pip and the pip-inspect synthetic root keep requirements.txt, so their identities are unchanged. The TUI rendered NodeID under a column labelled "PURL". For a module that is the structural "module:<path>#<purl>" grammar and for a manifest there is no package URL at all, so an interactive scan showed a value no consumer can parse while scan JSON and both SBOM exports had it right. Both TUI paths now use output.PurlFromGraphNode. That projection belongs on the SDK's GraphNode (ADR-0040) and is filed as bomly-dev/bomly-sdk#43. Until it ships there are two copies, and deliberately so: internal/sbom keeps the codec's own rather than importing the CLI's output layer, which would be backwards. Both are commented with the issue and converge on the accessor when it lands. Goldens: only the pipenv and poetry cases change, and neither could be regenerated here -- pipenv, poetry and uv are not installed on this machine, so those five smoke cases skip. Both parsers are pure, so TestPythonParserRootsNameTheDeclaringManifest asserts the new identities directly instead. Smoke runs in the merge queue, not on pull requests, so those two goldens need an Update Smoke Goldens dispatch before merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: one root identity per Python project, and a hook that sees the whole tree Three findings from review of 49b50a0. The manager-aware declaring manifest only reached the lockfile parsers. baseDetector.resolveGraph is the pip-inspect path shared by pip, Pipenv, Poetry and uv, and it built the root without saying which manager it spoke for, so every successful pip-inspect graph declared itself from requirements.txt -- and a Pipenv project got its correct Pipfile identity only when it fell back to the pure lock parser. One project, two identities, decided by which strategy happened to succeed. baseDetector carries its Manager now, and filterPythonToolPackages takes it too, because that function looks the root up by ID and would otherwise miss it and silently skip re-parenting orphans. TestPythonRootIdentityAgreesAcrossResolutionStrategies pins the two strategies to one answer. The Python smoke goldens are regenerated, and all five had been stale for the whole migration rather than only the two I claimed: every project root was still a dependency node. They now read module:Pipfile#..., module:pyproject.toml#... for Poetry and uv, and module:requirements.txt#... for pip -- the correct manifests only because of the fix above, which the regeneration is the evidence for. Each inventory loses exactly one package, its own project root, which is what a module node means; no dependency moved. The pre-push hook compared modification times of a hand-written file list covering 541 of 816 tracked files. Editing a shell script, a workflow, an npm wrapper source or a nested testdata fixture left the stamp looking fresh, a deleted file vanished from the list rather than invalidating anything, and a restored mtime read as unchanged -- each reporting a passing verification for work that was never tested, which is the one thing the hook exists to prevent. It compares a digest of HEAD plus the full diff against it now, so git decides what the repository contains rather than a list that has to be kept in step. scripts/verify-snapshot.sh defines it once and `make verify` records it. Exercised against all four previously invisible cases. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: explain keeps the project's module in view Both halves of the interactive explain view narrowed to dependency nodes, so the node that answers "which of my modules pulled this in" was dropped -- which is the question explain exists to answer. explainRelationships labelled the union's direct parents but walked only dependency nodes when labelling everything else. A normal graph's root is a module node, and for a transitive target it is not a direct parent, so it fell into that loop and was skipped: no label, no count, and a header reporting "Roots: 0" for a scan that plainly has one. Nested workspace modules were hidden the same way. buildExplainComponentListModel repeated the narrowing, leaving the project's own module out of the component list -- inconsistent with the ordinary component tree beside it, which walks the union and renders whatever kind it finds. Both iterate Graph.Nodes now. The new test drives the transitive case, where the root is an ancestor rather than a parent; restoring either narrowing reproduces "Roots: 0". Interactive rendering only, so no golden moves. Also corrects the verification snapshot added in the previous commit. It folded HEAD into the digest, so `git commit` invalidated a verification that was still perfectly valid -- the content had not changed, only which side of the HEAD boundary it sat on. A gate that fails on every commit is one that teaches people to pass --no-verify, which is worse than no gate. It digests the index plus the worktree diff against it instead: unchanged across a commit, and still changing on any edit or deletion. All three properties exercised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: a corrupt SBOM fails loudly, and depends_on never dangles Two findings from review of 262043a. Both are ways this migration could return a smaller answer than the input described, without saying so. SBOM ingest dropped a component whose identity could not mint a well-formed package URL, along with every relationship naming it, and returned no error. A document carrying one malformed Maven purl therefore produced a smaller graph than it described, and a scan of it read clean while a genuinely vulnerable dependency was simply absent from the answer. For a tool whose whole output is "what are you shipping and is it vulnerable", quietly returning fewer dependencies than the input listed is the worst available failure. It is an error now, naming the component and the purl so the author can find it -- the same rule ADR-0041 applies at the plugin wire: no lenient path, no pkg:generic coercion for an identity that was asserted and is invalid. Reproduced before fixing; the full smoke suite, SBOM ingest cases included, is unchanged by it. DependenciesFromGraph published a manifest node's ID in depends_on while deliberately omitting manifests from the listing, so a workspace path (module -> child manifest -> child module) left a reference no consumer could resolve. Dropping the ID instead would have severed the workspace, so the hop is stepped through: the parent module depends on the child module, expressed only in IDs the document defines. The walk is bounded against structural cycles. Also corrects the verification snapshot for the second time, and this time against the property rather than a symptom. It read git's bookkeeping -- first HEAD, then the index -- and both invalidated a verification that was still valid, because `git add` and `git commit` move content across those boundaries without changing a byte of it. It digests the worktree tree of everything .gitignore does not exclude, built in a throwaway index, so it changes when a file's bytes change, when one appears, and when one is deleted, and at no other time. All five properties exercised; it costs 0.16s. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: one home for the three questions a document asks of a node Three findings from review of 33b9baf, all the same shape: a projection over graph nodes existed once per surface, and the copies had each learned different things. The SBOM export named structural nodes it does not export. A workspace is module -> child manifest -> child module, and the two edges type differently -- the first derives depends-on, the second describes -- so publishing the first left CycloneDX with a dependsOn pointing at no bom-ref, while filtering the second dropped the hop and cut the child module's subtree loose. Reproduced both before fixing. Scan JSON had this same defect fixed one commit earlier, in its own local copy, which is the whole argument. The diff TUI's directness classifier read graph roots only, so a workspace module that another module depends on was never a parent and its immediate packages read transitive -- corrupting the relationship summary and every filter built on it. renderDirectDepsTable had already been taught this; the classifier beside it had not. The raw Relationships view iterated dependency nodes as parents, so it omitted every module-to-package edge: for a project with only direct dependencies it rendered empty while the count beside it reported the edges it was not showing. internal/graphview now owns all three -- the package URL a node publishes, the children a document can name, and which nodes count as top-level parents. It is a leaf, SDK only, so the codec, the renderers and the TUI reach it without depending on each other; that was the obstacle that left two copies of the purl projection behind last time, and this removes it. Every mutation was checked with the tree still compiling, after an earlier attempt "passed" because the mutations broke the build instead of the tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 79b38c9 commit 909825d

280 files changed

Lines changed: 28259 additions & 11377 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.githooks/pre-push

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
#!/usr/bin/env sh
2+
#
3+
# Refuse to push work that has not been verified since it last changed.
4+
#
5+
# The failure this prevents is a slow one: a push lands, CI runs for six
6+
# minutes, and comes back red on something `go test ./...` would have caught
7+
# in one. Worse on a pull request, where the reviewer has already started.
8+
#
9+
# The stamp is written by `make verify` and records which suites ran. This
10+
# hook only checks that a passing stamp exists and is newer than every
11+
# tracked source file -- it does not run the suites itself, because a hook
12+
# that takes six minutes gets bypassed, and a bypassed hook enforces nothing.
13+
#
14+
# Smoke is not required by default: it needs the network and several minutes.
15+
# `make verify SMOKE=1` records it, and `BOMLY_REQUIRE_SMOKE=1` makes this
16+
# hook insist on it -- worth setting when a change touches detector output.
17+
#
18+
# To push without verifying, say so out loud: `git push --no-verify`.
19+
set -eu
20+
21+
repo_root=$(git rev-parse --show-toplevel)
22+
stamp="$repo_root/.verify-stamp"
23+
24+
fail() {
25+
printf 'pre-push: %s\n' "$1" >&2
26+
printf 'pre-push: run `make verify` (or `make verify SMOKE=1`), then push again.\n' >&2
27+
printf 'pre-push: to skip deliberately, use `git push --no-verify`.\n' >&2
28+
exit 1
29+
}
30+
31+
[ -f "$stamp" ] || fail 'no verification stamp; this work has never been verified.'
32+
33+
# shellcheck disable=SC1090
34+
. "$stamp"
35+
36+
[ "${VERIFY_STATUS:-}" = "pass" ] || fail "the last verification did not pass (status: ${VERIFY_STATUS:-unknown})."
37+
38+
# The stamp records a digest of the repository state that was verified;
39+
# scripts/verify-snapshot.sh defines it and `make verify` writes it, so there
40+
# is one definition rather than two that drift.
41+
#
42+
# This replaced comparing modification times of a hand-written file list. That
43+
# list named 541 of the repository's 816 tracked files, so editing a shell
44+
# script, a workflow, an npm wrapper source or a nested testdata fixture left
45+
# the stamp looking fresh; a deleted file vanished from the list rather than
46+
# invalidating anything; and a restored mtime read as unchanged. Each of those
47+
# reported a passing verification for work that was never tested, which is the
48+
# one thing this hook exists to prevent.
49+
snapshot_script="$repo_root/scripts/verify-snapshot.sh"
50+
[ -x "$snapshot_script" ] || fail 'scripts/verify-snapshot.sh is missing or not executable.'
51+
52+
current_snapshot=$("$snapshot_script") || fail 'could not snapshot the repository state.'
53+
[ -n "${VERIFY_SNAPSHOT:-}" ] || fail 'the stamp predates snapshot verification; run `make verify` again.'
54+
[ "${VERIFY_SNAPSHOT}" = "${current_snapshot}" ] || fail 'the repository changed after the last verification.'
55+
56+
if [ "${BOMLY_REQUIRE_SMOKE:-0}" = "1" ] && [ "${VERIFY_SMOKE:-no}" != "yes" ]; then
57+
fail 'BOMLY_REQUIRE_SMOKE=1 but the stamp records no smoke run.'
58+
fi
59+
60+
printf 'pre-push: verified at %s (smoke: %s).\n' "${VERIFY_AT_HUMAN:-?}" "${VERIFY_SMOKE:-no}"

.github/workflows/smoke.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ jobs:
6565
- name: go-reachability
6666
run: 'TestScan$/scan-go-reachability'
6767
- name: node
68-
run: 'TestScan$/(scan-npm$|scan-npm-scope-runtime|scan-npm-workspaces|scan-yarn|scan-pnpm|scan-bun$|scan-github-actions)|TestDiff/diff-npm|TestAuditScan/scan-npm-audit'
68+
run: 'TestScan$/(scan-npm$|scan-npm-scope-runtime|scan-npm-workspaces|scan-yarn|scan-pnpm|scan-bun$|scan-github-actions)|TestDiff/diff-npm|TestAuditScan/scan-npm-audit|TestAuditScanSARIFGolden$'
6969
node: true
7070
- name: node-detail-policy
7171
run: 'TestDependencyDetailRiskPolicy$'
@@ -97,7 +97,7 @@ jobs:
9797
- name: ruby
9898
run: 'TestScan$/scan-bundler'
9999
- name: sbom
100-
run: 'TestScan$/scan-sbom|TestDiff/(diff-sbom$|diff-sbom-detail-change$)|TestLiteScan/lite-scan-sbom|TestScanSBOMSyftJSONRejected$|TestScanSBOMExportOrigin$'
100+
run: 'TestScan$/scan-sbom|TestDiff/(diff-sbom$|diff-sbom-detail-change$)|TestLiteScan/lite-scan-sbom|TestScanSBOMSyftJSONRejected$|TestScanSBOMExportOrigin$|TestScanSBOMExportGolden$'
101101
node: true
102102
- name: dotnet
103103
run: 'TestScan$/scan-nuget'

.github/workflows/update-smoke-goldens.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ jobs:
7474
- name: go-reachability
7575
run: 'TestScan$/scan-go-reachability'
7676
- name: node
77-
run: 'TestScan$/(scan-npm$|scan-npm-scope-runtime|scan-npm-workspaces|scan-yarn|scan-pnpm|scan-bun$|scan-github-actions)|TestDiff/diff-npm|TestAuditScan/scan-npm-audit'
77+
run: 'TestScan$/(scan-npm$|scan-npm-scope-runtime|scan-npm-workspaces|scan-yarn|scan-pnpm|scan-bun$|scan-github-actions)|TestDiff/diff-npm|TestAuditScan/scan-npm-audit|TestAuditScanSARIFGolden$'
7878
node: true
7979
- name: node-detail-policy
8080
run: 'TestDependencyDetailRiskPolicy$'
@@ -122,7 +122,7 @@ jobs:
122122
- name: cpp
123123
run: 'TestScan$/scan-cpp-conan'
124124
- name: sbom
125-
run: 'TestScan$/scan-sbom|TestDiff/(diff-sbom$|diff-sbom-detail-change$)|TestLiteScan/lite-scan-sbom|TestScanSBOMSyftJSONRejected$|TestScanSBOMExportOrigin$'
125+
run: 'TestScan$/scan-sbom|TestDiff/(diff-sbom$|diff-sbom-detail-change$)|TestLiteScan/lite-scan-sbom|TestScanSBOMSyftJSONRejected$|TestScanSBOMExportOrigin$|TestScanSBOMExportGolden$'
126126
node: true
127127
- name: plugin
128128
run: 'TestPluginWorkflows'

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,6 @@ qodana.yaml
4545
# main must stay replace-free so remote go install keeps working
4646
go.work
4747
go.work.sum
48+
49+
# Written by `make verify`, read by .githooks/pre-push.
50+
.verify-stamp

AGENTS.md

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ make build-lite # go build -tags "bomly_external_syft,bomly_external_gr
1717
make test # go test ./...
1818
make smoke # end-to-end tests driving the built binary (slow, requires network)
1919
make smoke ARGS="-update" # regenerate smoke golden files
20+
make verify # everything that gates a push; writes .verify-stamp
21+
make verify SMOKE=1 # the same, including the network-driven smoke suite
2022
make fuzz FUZZTIME=5s # run every registered fuzz target with a short per-target budget
2123
make benchmark # run the hidden local dependency-graph benchmark
2224
make benchmark-report # analyze local benchmark artifacts with Copilot CLI
@@ -25,7 +27,16 @@ make run ARGS="scan" # go run ./cmd/bomly <ARGS>
2527
make generate # regenerate config reference, JSON schemas, schema docs, support matrix, and component docs (binary-driven)
2628
```
2729

28-
Always run `make test` after changes. All tests must pass before marking work is done.
30+
Always run `make verify` before pushing or updating a pull request; it runs formatting, lint, vet and build on both build variants, the unit suite, and the generated-docs drift check. All of it must pass before marking work done.
31+
`.githooks/pre-push` refuses a push unless `make verify` has passed since the
32+
last source change (`git config core.hooksPath .githooks`, or `make
33+
install-hooks`, enables it). The check is a stamp read, not a test run: a
34+
six-minute hook gets bypassed, and a bypassed hook enforces nothing. Smoke is
35+
not required by default because it needs the network and several minutes --
36+
run `make verify SMOKE=1` when a change touches detector output, and set
37+
`BOMLY_REQUIRE_SMOKE=1` to make the hook insist on it. `git push --no-verify`
38+
skips the gate deliberately.
39+
2940
If you change `internal/cli/config.go`, `internal/output/*`, or `internal/registry/support.go`, or bump the pinned `bomly-dev/bomly-sdk` version (its catalog or support-matrix data feeds the generated docs), also run `make generate` and commit the docs drift.
3041

3142
`go.mod` pins released versions and must not contain `replace` directives on main (CI enforces this), so remote `go install github.com/bomly-dev/bomly-cli/cmd/bomly@latest` stays supported. External component modules (`bomly-plugin-*`) are ordinary pinned dependencies bumped by Dependabot. Local cross-repo development: `go work init . ../bomly-sdk` (never commit `go.work`).
@@ -51,6 +62,8 @@ See [`dev-docs/ARCHITECTURE.md`](dev-docs/ARCHITECTURE.md) for full detail (the
5162
| `internal/detectors/*` | Concrete native dependency resolution per ecosystem (gomod, gradle, maven, node, python, sbom); the Syft catch-all detector lives in `bomly-plugin-syft-detector` |
5263
| `bomly-plugin-*` (external modules) | External-integration components consumed as pinned Go modules: enrichment matchers (osv, grype, deps.dev license, scorecard), reachability analyzers (govulncheck, jsreach, pyreach, jvmreach), and the Syft detector; ClearlyDefined and eol run as external matcher plugins; the shared cache lives in `bomly-sdk/filecache` |
5364
| `internal/auditors/*` | Policy evaluators and audit-only logic (policy, noop) |
65+
| `internal/graphview` | Reads a graph for presentation and publication: a node's published package URL, the children a document can name, top-level parents |
66+
| `internal/testnodes` | Test-only fixture builders for graph nodes (panic on an unbuildable fixture); label lookups delegate to `bomly-sdk/testkit` |
5467
| `internal/baseline` | Portable package-finding baseline codec and audit-integrated policy-status resolver |
5568
| `internal/remediation` | Canonical vulnerability fix status, version, detector-hint validation, and occurrence suggestions |
5669
| `internal/sbom` | SBOM codec (SPDX 2.3, CycloneDX) |
@@ -93,6 +106,9 @@ Runtime preparation is owned by `internal/engine`: build the filtered registry o
93106
- Built-in reachability analyzers live in their own `bomly-plugin-*-analyzer` repositories, consumed as pinned Go modules. They depend only on the SDK and its helper subpackages (`system`, `filecache`, `logkit`) and must not import any `internal/*` package.
94107
- `internal/detectors` owns detector-facing contracts such as `Detector`, `DetectorDescriptor`, `ResolveGraphRequest`, and detector helper functions.
95108
- The SDK owns neutral shared identifiers and support metadata that would otherwise create package cycles, including ecosystems, package managers, detector types, and support-matrix data.
109+
- Reading a node of any kind -- coordinates, display name, version, narrowing over the sealed union -- is the SDK's: `sdk.NodeCoordinates`, `sdk.NodeDisplayName`, `sdk.NodeVersion`, `sdk.AsDependencyNode`, `sdk.DependencyNodesOf`, `sdk.IsProjectOwned`. Building or mutating a detector graph is `bomly-sdk/detectorkit`: `EnsureNode`, `PromoteToModule`, `PropagateScopes`. Do not reintroduce a CLI-local copy of either — both were CLI stopgaps until bomly-sdk v0.9.0 and were deleted when it shipped.
110+
- `internal/graphview` owns the three questions every renderer and exporter asks of a node: what package URL it publishes, which of its children a document can actually name (structural nodes are stepped through, never named), and which nodes count as top-level parents. It is a leaf -- SDK only -- so the SBOM codec, the renderers, and the TUI all reach it without depending on each other. A copy per surface is what this replaces, and every one of those copies had shipped a defect the others had already fixed.
111+
- `internal/testnodes` is test-only: it routes fixture shapes through the real node constructors, panicking rather than taking a `testing.TB` so a table entry stays one expression. Label lookups ("name@version" to the canonical package URLs node IDs now are) delegate to `bomly-sdk/testkit` — the matching rules have one home, not two. Non-test code must not import it.
96112
- `internal/baseline` owns the baseline document and matching implementation. It depends on the SDK policy contracts and must not be imported by `internal/engine`.
97113
- `internal/remediation` owns canonical vulnerability remediation decisions. Detectors may supply validated read-only strategy hints, but they do not choose final actions or versions.
98114
- `internal/licenseexpr` owns all SPDX license expression parsing. The underlying parser panics on some malformed input, and license strings come from untrusted lockfiles and registry APIs, so no other package under `internal/` may import `github.com/github/go-spdx` directly; `TestNoDirectSPDXExpressionUse` enforces this.
@@ -124,7 +140,7 @@ In practice:
124140
second says the rule has no home. Give it one — a named helper, a shared
125141
entry point, or an invariant enforced where the data is created — and route
126142
every site through it.
127-
- **Name the concept, not the mechanics.** `detectors.EnsureNode(g, node)`
143+
- **Name the concept, not the mechanics.** `detectorkit.EnsureNode(g, node)`
128144
says what the caller is doing — insert or return the existing node; a
129145
hand-written lookup-then-insert at each site says only what to type, and
130146
each copy decides duplicate handling differently.

CLAUDE.md

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ make build-lite # go build -tags "bomly_external_syft,bomly_external_gr
1717
make test # go test ./...
1818
make smoke # end-to-end tests driving the built binary (slow, requires network)
1919
make smoke ARGS="-update" # regenerate smoke golden files
20+
make verify # everything that gates a push; writes .verify-stamp
21+
make verify SMOKE=1 # the same, including the network-driven smoke suite
2022
make fuzz FUZZTIME=5s # run every registered fuzz target with a short per-target budget
2123
make benchmark # run the hidden local dependency-graph benchmark
2224
make benchmark-report # analyze local benchmark artifacts with Copilot CLI
@@ -25,7 +27,16 @@ make run ARGS="scan" # go run ./cmd/bomly <ARGS>
2527
make generate # regenerate config reference, JSON schemas, schema docs, support matrix, and component docs (binary-driven)
2628
```
2729

28-
Always run `make test` after changes. All tests must pass before marking work is done.
30+
Always run `make verify` before pushing or updating a pull request; it runs formatting, lint, vet and build on both build variants, the unit suite, and the generated-docs drift check. All of it must pass before marking work done.
31+
`.githooks/pre-push` refuses a push unless `make verify` has passed since the
32+
last source change (`git config core.hooksPath .githooks`, or `make
33+
install-hooks`, enables it). The check is a stamp read, not a test run: a
34+
six-minute hook gets bypassed, and a bypassed hook enforces nothing. Smoke is
35+
not required by default because it needs the network and several minutes --
36+
run `make verify SMOKE=1` when a change touches detector output, and set
37+
`BOMLY_REQUIRE_SMOKE=1` to make the hook insist on it. `git push --no-verify`
38+
skips the gate deliberately.
39+
2940
If you change `internal/cli/config.go`, `internal/output/*`, or `internal/registry/support.go`, or bump the pinned `bomly-dev/bomly-sdk` version (its catalog or support-matrix data feeds the generated docs), also run `make generate` and commit the docs drift.
3041

3142
`go.mod` pins released versions and must not contain `replace` directives on main (CI enforces this), so remote `go install github.com/bomly-dev/bomly-cli/cmd/bomly@latest` stays supported. External component modules (`bomly-plugin-*`) are ordinary pinned dependencies bumped by Dependabot. Local cross-repo development: `go work init . ../bomly-sdk` (never commit `go.work`).
@@ -51,6 +62,8 @@ See [`dev-docs/ARCHITECTURE.md`](dev-docs/ARCHITECTURE.md) for full detail (the
5162
| `internal/detectors/*` | Concrete native dependency resolution per ecosystem (gomod, gradle, maven, node, python, sbom); the Syft catch-all detector lives in `bomly-plugin-syft-detector` |
5263
| `bomly-plugin-*` (external modules) | External-integration components consumed as pinned Go modules: enrichment matchers (osv, grype, deps.dev license, scorecard), reachability analyzers (govulncheck, jsreach, pyreach, jvmreach), and the Syft detector; ClearlyDefined and eol run as external matcher plugins; the shared cache lives in `bomly-sdk/filecache` |
5364
| `internal/auditors/*` | Policy evaluators and audit-only logic (policy, noop) |
65+
| `internal/graphview` | Reads a graph for presentation and publication: a node's published package URL, the children a document can name, top-level parents |
66+
| `internal/testnodes` | Test-only fixture builders for graph nodes (panic on an unbuildable fixture); label lookups delegate to `bomly-sdk/testkit` |
5467
| `internal/baseline` | Portable package-finding baseline codec and audit-integrated policy-status resolver |
5568
| `internal/remediation` | Canonical vulnerability fix status, version, detector-hint validation, and occurrence suggestions |
5669
| `internal/sbom` | SBOM codec (SPDX 2.3, CycloneDX) |
@@ -93,6 +106,9 @@ Runtime preparation is owned by `internal/engine`: build the filtered registry o
93106
- Built-in reachability analyzers live in their own `bomly-plugin-*-analyzer` repositories, consumed as pinned Go modules. They depend only on the SDK and its helper subpackages (`system`, `filecache`, `logkit`) and must not import any `internal/*` package.
94107
- `internal/detectors` owns detector-facing contracts such as `Detector`, `DetectorDescriptor`, `ResolveGraphRequest`, and detector helper functions.
95108
- The SDK owns neutral shared identifiers and support metadata that would otherwise create package cycles, including ecosystems, package managers, detector types, and support-matrix data.
109+
- Reading a node of any kind -- coordinates, display name, version, narrowing over the sealed union -- is the SDK's: `sdk.NodeCoordinates`, `sdk.NodeDisplayName`, `sdk.NodeVersion`, `sdk.AsDependencyNode`, `sdk.DependencyNodesOf`, `sdk.IsProjectOwned`. Building or mutating a detector graph is `bomly-sdk/detectorkit`: `EnsureNode`, `PromoteToModule`, `PropagateScopes`. Do not reintroduce a CLI-local copy of either — both were CLI stopgaps until bomly-sdk v0.9.0 and were deleted when it shipped.
110+
- `internal/graphview` owns the three questions every renderer and exporter asks of a node: what package URL it publishes, which of its children a document can actually name (structural nodes are stepped through, never named), and which nodes count as top-level parents. It is a leaf -- SDK only -- so the SBOM codec, the renderers, and the TUI all reach it without depending on each other. A copy per surface is what this replaces, and every one of those copies had shipped a defect the others had already fixed.
111+
- `internal/testnodes` is test-only: it routes fixture shapes through the real node constructors, panicking rather than taking a `testing.TB` so a table entry stays one expression. Label lookups ("name@version" to the canonical package URLs node IDs now are) delegate to `bomly-sdk/testkit` — the matching rules have one home, not two. Non-test code must not import it.
96112
- `internal/baseline` owns the baseline document and matching implementation. It depends on the SDK policy contracts and must not be imported by `internal/engine`.
97113
- `internal/remediation` owns canonical vulnerability remediation decisions. Detectors may supply validated read-only strategy hints, but they do not choose final actions or versions.
98114
- `internal/licenseexpr` owns all SPDX license expression parsing. The underlying parser panics on some malformed input, and license strings come from untrusted lockfiles and registry APIs, so no other package under `internal/` may import `github.com/github/go-spdx` directly; `TestNoDirectSPDXExpressionUse` enforces this.
@@ -124,7 +140,7 @@ In practice:
124140
second says the rule has no home. Give it one — a named helper, a shared
125141
entry point, or an invariant enforced where the data is created — and route
126142
every site through it.
127-
- **Name the concept, not the mechanics.** `detectors.EnsureNode(g, node)`
143+
- **Name the concept, not the mechanics.** `detectorkit.EnsureNode(g, node)`
128144
says what the caller is doing — insert or return the existing node; a
129145
hand-written lookup-then-insert at each site says only what to type, and
130146
each copy decides duplicate handling differently.

0 commit comments

Comments
 (0)