Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -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`.

Expand Down
3 changes: 1 addition & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -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`.

Expand Down
1 change: 0 additions & 1 deletion dev-docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions internal/auditors/license/auditor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down
51 changes: 51 additions & 0 deletions internal/detectors/guards_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
117 changes: 0 additions & 117 deletions internal/licenseexpr/licenseexpr.go

This file was deleted.

Loading
Loading