refactor: SPDX expression handling is the SDK's, not a local wrapper (#428)

Phase 2.2, which the train skipped: internal/licenseexpr exposed Valid,
ValidateAll, Identifier, Compose, Satisfies and Extract, and bomly-sdk/spdxkit
exposes the same six with the same signatures. Two implementations of one
specification's semantics is the drift this program exists to remove, and it
was sitting directly under the next phase's work.

The call sites move to the kit, the package is deleted, and go-spdx drops to
indirect. No golden moves: the two Compose implementations agree on every
fixture the smoke suite produces, which is the check that mattered, since
composition is the one function where parenthesization could have differed.

The panic guard survives the move -- the kit carries it -- so the rule it
protected changes shape rather than disappearing. It used to be "route through
the local wrapper"; it is now "do not import the parser at all", and
TestNoDirectSPDXExpressionUse moves to internal/detectors/guards_test.go where
the other tree-wide structural guards live. It scans test files too, because a
test reaching the parser directly proves the same crash is reachable and is
where the temptation lives. Mutation-checked by importing go-spdx into
internal/sbom.

This is the prerequisite for 2.4 (#410): LicenseRef-* minting,
hasExtractedLicensingInfos, and mixed-validity composition are all already in
spdxkit -- MintLicenseRef, Classify, Compose -- and its doc comments cite the
issue by number. Building that on the local wrapper would have meant
hand-rolling identifier sanitization and collision-avoidance beside a library
that already owns both.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: bomly-guy

AGENTS.md

@@ -67,7 +67,6 @@ See [`dev-docs/ARCHITECTURE.md`](dev-docs/ARCHITECTURE.md) for full detail (the
 | `internal/baseline`    | Portable package-finding baseline codec and audit-integrated policy-status resolver               |
 | `internal/remediation` | Canonical vulnerability fix status, version, detector-hint validation, and occurrence suggestions |
 | `internal/sbom`        | SBOM codec (SPDX 2.3, CycloneDX)                                                                  |
-| `internal/licenseexpr` | SPDX license expression parsing and identifier classification (guards the parser's panics)        |
 | `internal/benchmark`   | Hidden local dependency-graph benchmark, baseline comparison, scoring, and embedded presets       |
 | `internal/output`      | Output rendering plus structured command payloads and schema generation for `scan`, `diff`, `explain`, JSON, and SARIF 2.1.0 |
 | `internal/plugin`      | Plugin discovery, protocol, handshake, and pooled subprocess execution                            |
@@ -111,7 +110,7 @@ Runtime preparation is owned by `internal/engine`: build the filtered registry o
 - `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.
 - `internal/baseline` owns the baseline document and matching implementation. It depends on the SDK policy contracts and must not be imported by `internal/engine`.
 - `internal/remediation` owns canonical vulnerability remediation decisions. Detectors may supply validated read-only strategy hints, but they do not choose final actions or versions.
-- `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.
+- SPDX license expression handling is `bomly-sdk/spdxkit`'s: validation, identifier classification, composition, deprecated-ID canonicalization, and `LicenseRef-*` minting. The underlying parser panics on some malformed input, and license strings come from untrusted lockfiles and registry APIs, so no package under `internal/` may import `github.com/github/go-spdx` directly — the kit carries the panic guard. `TestNoDirectSPDXExpressionUse` (in `internal/detectors/guards_test.go`) enforces this across the whole tree, test files included. The CLI's own `internal/licenseexpr` wrapper is deleted; it duplicated the kit function for function.
 - `internal/registry` owns package-manager discovery, support lookups, and built-in registry wiring in `internal/registry/builder.go`. Do not create or reintroduce a separate `registrybuilder` package.
 - `internal/engine` may import `internal/detectors` and `internal/registry`, but detector packages must not point back into `internal/engine`. Runtime planning, prepared subprojects, and detector-chain reuse belong in `internal/engine`.
 

CLAUDE.md

@@ -67,7 +67,6 @@ See [`dev-docs/ARCHITECTURE.md`](dev-docs/ARCHITECTURE.md) for full detail (the
 | `internal/baseline`    | Portable package-finding baseline codec and audit-integrated policy-status resolver               |
 | `internal/remediation` | Canonical vulnerability fix status, version, detector-hint validation, and occurrence suggestions |
 | `internal/sbom`        | SBOM codec (SPDX 2.3, CycloneDX)                                                                  |
-| `internal/licenseexpr` | SPDX license expression parsing and identifier classification (guards the parser's panics)        |
 | `internal/benchmark`   | Hidden local dependency-graph benchmark, baseline comparison, scoring, and embedded presets       |
 | `internal/output`      | Output rendering plus structured command payloads and schema generation for `scan`, `diff`, `explain`, JSON, and SARIF 2.1.0 |
 | `internal/plugin`      | Plugin discovery, protocol, handshake, and pooled subprocess execution                            |
@@ -111,7 +110,7 @@ Runtime preparation is owned by `internal/engine`: build the filtered registry o
 - `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.
 - `internal/baseline` owns the baseline document and matching implementation. It depends on the SDK policy contracts and must not be imported by `internal/engine`.
 - `internal/remediation` owns canonical vulnerability remediation decisions. Detectors may supply validated read-only strategy hints, but they do not choose final actions or versions.
-- `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.
+- SPDX license expression handling is `bomly-sdk/spdxkit`'s: validation, identifier classification, composition, deprecated-ID canonicalization, and `LicenseRef-*` minting. The underlying parser panics on some malformed input, and license strings come from untrusted lockfiles and registry APIs, so no package under `internal/` may import `github.com/github/go-spdx` directly — the kit carries the panic guard. `TestNoDirectSPDXExpressionUse` (in `internal/detectors/guards_test.go`) enforces this across the whole tree, test files included. The CLI's own `internal/licenseexpr` wrapper is deleted; it duplicated the kit function for function.
 - `internal/registry` owns package-manager discovery, support lookups, and built-in registry wiring in `internal/registry/builder.go`. Do not create or reintroduce a separate `registrybuilder` package.
 - `internal/engine` may import `internal/detectors` and `internal/registry`, but detector packages must not point back into `internal/engine`. Runtime planning, prepared subprojects, and detector-chain reuse belong in `internal/engine`.
 

dev-docs/ARCHITECTURE.md

@@ -247,7 +247,6 @@ Cache failures are non-fatal. The command should warn and continue rather than f
 | `internal/engine/scan` | Scan command pipeline API                                                                    |
 | `internal/output`     | Text, JSON, SARIF rendering, plus structured response payloads and schema generation            |
 | `internal/sbom`       | SPDX and CycloneDX codecs                                                                       |
-| `internal/licenseexpr` | SPDX license expression parsing and identifier classification                                  |
 | `internal/benchmark`  | Hidden local dependency-graph benchmark, baseline comparison, scoring, and embedded presets      |
 | `sdk`      | Shared domain types                                                                             |
 | `internal/plugin`     | Managed plugin manifests, installation, verification, store state, adapters, and runtime glue  |

go.mod

@@ -19,7 +19,6 @@ require (
 	github.com/charmbracelet/bubbles v1.0.0
 	github.com/charmbracelet/bubbletea v1.3.10
 	github.com/charmbracelet/lipgloss v1.1.0
-	github.com/github/go-spdx/v2 v2.7.0
 	github.com/hashicorp/go-hclog v1.6.3
 	github.com/hashicorp/go-plugin v1.8.0
 	github.com/mark3labs/mcp-go v0.58.0
@@ -150,6 +149,7 @@ require (
 	github.com/felixge/httpsnoop v1.0.4 // indirect
 	github.com/fsnotify/fsnotify v1.9.0 // indirect
 	github.com/gabriel-vasile/mimetype v1.4.15 // indirect
+	github.com/github/go-spdx/v2 v2.7.0 // indirect
 	github.com/glebarez/go-sqlite v1.22.0 // indirect
 	github.com/glebarez/sqlite v1.11.0 // indirect
 	github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect

internal/auditors/license/auditor.go

@@ -7,8 +7,8 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/bomly-dev/bomly-cli/internal/licenseexpr"
 	"github.com/bomly-dev/bomly-sdk"
+	"github.com/bomly-dev/bomly-sdk/spdxkit"
 )
 
 const (
@@ -100,15 +100,15 @@ func (a Auditor) Audit(_ context.Context, req sdk.AuditRequest) (sdk.AuditResult
 			findings = append(findings, finding(purl, dep.NodeID(), "unknown-license", "Package license is unknown", sdk.FindingPolicyStatusWarn))
 			continue
 		}
-		valid, invalid := licenseexpr.ValidateAll(licenses)
+		valid, invalid := spdxkit.ValidateAll(licenses)
 		if !valid {
 			findings = append(findings, finding(purl, dep.NodeID(), "invalid-license", "Package has invalid SPDX license: "+strings.Join(invalid, ", "), sdk.FindingPolicyStatusFail))
 			continue
 		}
 		if len(a.AllowLicenses) > 0 {
 			allowed := false
 			for _, expr := range licenses {
-				ok, err := licenseexpr.Satisfies(expr, a.AllowLicenses)
+				ok, err := spdxkit.Satisfies(expr, a.AllowLicenses)
 				if err == nil && ok {
 					allowed = true
 					break
@@ -121,7 +121,7 @@ func (a Auditor) Audit(_ context.Context, req sdk.AuditRequest) (sdk.AuditResult
 		}
 		if len(a.DenyLicenses) > 0 {
 			for _, expr := range licenses {
-				used, err := licenseexpr.Extract(expr)
+				used, err := spdxkit.Extract(expr)
 				if err != nil {
 					continue
 				}

internal/detectors/guards_test.go

@@ -87,3 +87,54 @@ func TestExportNeverReadsResolvedURL(t *testing.T) {
 		t.Fatalf("the export layer references ResolvedURL; it must read Origin.Normalized() only: %v", offenders)
 	}
 }
+
+// The SPDX expression parser panics on some malformed input, and license
+// strings arrive from lockfiles and registry APIs that a repository controls,
+// so no package here may call it directly.
+//
+// The guard used to live in internal/licenseexpr, which wrapped the parser and
+// caught its panics. That package is gone: bomly-sdk/spdxkit owns expression
+// handling now, panic guard included, so the rule is no longer "route through
+// the local wrapper" but "do not import the parser at all". Written as a
+// module-path check rather than an import-list scan of one package, because
+// the point is that nothing under internal/ reaches the parser by any route.
+func TestNoDirectSPDXExpressionUse(t *testing.T) {
+	const spdxModule = "github.com/github/go-spdx"
+
+	// This file names the module in the rule it enforces, so it is the one
+	// exemption -- the same shape the guard had when it lived beside the
+	// wrapper it policed.
+	self := "guards_test.go"
+
+	var offenders []string
+	walkInternalGo(t, func(path, body string) {
+		if strings.Contains(body, spdxModule) {
+			offenders = append(offenders, path)
+		}
+	})
+	// Test files too: a test reaching the parser directly proves the same
+	// crash is reachable, and it is where the temptation lives.
+	err := filepath.Walk(internalRoot, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		if info.IsDir() || !strings.HasSuffix(path, "_test.go") || filepath.Base(path) == self {
+			return nil
+		}
+		body, err := os.ReadFile(path)
+		if err != nil {
+			return err
+		}
+		if strings.Contains(string(body), spdxModule) {
+			offenders = append(offenders, path)
+		}
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("walk tests: %v", err)
+	}
+	if len(offenders) > 0 {
+		t.Fatalf("these files reference %s directly; the parser panics on malformed input, "+
+			"so go through bomly-sdk/spdxkit instead: %v", spdxModule, offenders)
+	}
+}