From 4eaf310c845b1aa9ff0bc2ea1591f6b2cab80fe1 Mon Sep 17 00:00:00 2001 From: Ahmed ElMallah Date: Wed, 12 Aug 2026 23:53:46 -0700 Subject: [PATCH 1/2] fix(components): PR 386 deferred review follow-ups - jsreach: make esbuild builds cancellable via api.Context/Rebuild with a ctx watcher calling Cancel and a deferred Dispose; return ctx.Err() on cancellation so 'The build was canceled' never surfaces as a build error - jsreach: deterministic emission order in walkJSONStrings/binEntryStrings; bounded FuzzEntryPointStrings target for the package.json entry-point helpers - jvmreach: deterministic readGradleModules order; bounded FuzzReadMavenProject and FuzzReadGradleModules targets - all four analyzer modules: newResultCache takes a logger and logs one WARN when the user cache dir or file cache init fails instead of silently disabling caching; regression tests via zap observer - register new fuzz targets in scripts/run-fuzz.sh and the parser-fuzzing inventory Co-Authored-By: Claude Fable 5 --- components/analyzers/govulncheck/analyzer.go | 2 +- components/analyzers/govulncheck/cache.go | 36 +++++--- .../analyzers/govulncheck/cache_test.go | 23 ++++- components/analyzers/jsreach/analyzer.go | 2 +- components/analyzers/jsreach/cache.go | 36 +++++--- components/analyzers/jsreach/cache_test.go | 25 +++++- components/analyzers/jsreach/entrypoints.go | 26 ++++-- .../jsreach/entrypoints_fuzz_test.go | 50 +++++++++++ .../analyzers/jsreach/runner_library.go | 29 ++++++- .../analyzers/jsreach/runner_testdata_test.go | 9 ++ components/analyzers/jvmreach/analyzer.go | 2 +- components/analyzers/jvmreach/cache.go | 35 ++++++-- components/analyzers/jvmreach/cache_test.go | 21 ++++- components/analyzers/jvmreach/discover.go | 3 + .../analyzers/jvmreach/discover_fuzz_test.go | 86 +++++++++++++++++++ components/analyzers/pyreach/analyzer.go | 2 +- components/analyzers/pyreach/cache.go | 36 +++++--- components/analyzers/pyreach/cache_test.go | 23 ++++- scripts/run-fuzz.sh | 3 + test/assurance/PARSER_FUZZING.md | 7 +- 20 files changed, 381 insertions(+), 75 deletions(-) create mode 100644 components/analyzers/jsreach/entrypoints_fuzz_test.go create mode 100644 components/analyzers/jvmreach/discover_fuzz_test.go diff --git a/components/analyzers/govulncheck/analyzer.go b/components/analyzers/govulncheck/analyzer.go index 947b1282..3473532d 100644 --- a/components/analyzers/govulncheck/analyzer.go +++ b/components/analyzers/govulncheck/analyzer.go @@ -219,7 +219,7 @@ func (a Analyzer) cache() *resultCache { if a.DisableCache { return nil } - return newResultCache(a.CacheDir, a.CacheTTL) + return newResultCache(a.CacheDir, a.CacheTTL, a.logger()) } func (a Analyzer) logger() *zap.Logger { return ensureLogger(a.Logger) } diff --git a/components/analyzers/govulncheck/cache.go b/components/analyzers/govulncheck/cache.go index 4e05a4c2..0fa76ffa 100644 --- a/components/analyzers/govulncheck/cache.go +++ b/components/analyzers/govulncheck/cache.go @@ -11,6 +11,7 @@ import ( "time" cachepkg "github.com/bomly-dev/bomly-sdk/filecache" + "go.uber.org/zap" "github.com/bomly-dev/bomly-sdk/system" ) @@ -47,35 +48,44 @@ type cachedRunnerResult struct { // newResultCache constructs a result cache rooted at dir. If dir is // empty, the OS user cache directory is used. Errors creating the cache -// directory are non-fatal — they return a nil resultCache that the caller -// can use without checks. -func newResultCache(dir string, ttl time.Duration) *resultCache { +// directory are non-fatal — they log one WARN and return a nil +// resultCache that the caller can use without checks. +func newResultCache(dir string, ttl time.Duration, logger *zap.Logger) *resultCache { + logger = ensureLogger(logger) if ttl <= 0 { ttl = defaultCacheTTL } root := dir if root == "" { - root = defaultCacheRoot() - } - if root == "" { - return nil + defaultRoot, err := defaultCacheRoot() + if err != nil { + logger.Warn("govulncheck: result cache disabled: user cache directory unavailable (non-fatal)", + zap.Error(err)) + return nil + } + root = defaultRoot } store, err := cachepkg.NewFileCache(root, ttl) if err != nil { + logger.Warn("govulncheck: result cache disabled: cache initialization failed (non-fatal)", + zap.String("dir", root), zap.Error(err)) return nil } return &resultCache{store: store} } // defaultCacheRoot returns the platform-appropriate cache directory for -// govulncheck analyzer results, or "" if the user cache directory cannot -// be determined. -func defaultCacheRoot() string { +// govulncheck analyzer results, or an error when the user cache +// directory cannot be determined. +func defaultCacheRoot() (string, error) { base, err := os.UserCacheDir() - if err != nil || base == "" { - return "" + if err != nil { + return "", err + } + if base == "" { + return "", errors.New("user cache directory is empty") } - return filepath.Join(base, "bomly", "analyzers", "govulncheck") + return filepath.Join(base, "bomly", "analyzers", "govulncheck"), nil } // keyFor builds a stable cache key for one module run. The key folds diff --git a/components/analyzers/govulncheck/cache_test.go b/components/analyzers/govulncheck/cache_test.go index 79151531..8c99e008 100644 --- a/components/analyzers/govulncheck/cache_test.go +++ b/components/analyzers/govulncheck/cache_test.go @@ -7,11 +7,13 @@ import ( "testing" model "github.com/bomly-dev/bomly-sdk" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" ) func TestResultCacheRoundTrip(t *testing.T) { dir := t.TempDir() - cache := newResultCache(dir, 0) + cache := newResultCache(dir, 0, nil) if cache == nil { t.Fatal("newResultCache returned nil for a writable dir") } @@ -40,7 +42,7 @@ func TestResultCacheRoundTrip(t *testing.T) { func TestResultCacheIsolatesByRunnerName(t *testing.T) { dir := t.TempDir() - cache := newResultCache(dir, 0) + cache := newResultCache(dir, 0, nil) moduleDir := newGoModuleDir(t) if err := cache.set(moduleDir, "builtin", RunnerResult{Findings: map[string]Finding{"A": {OSV: "A"}}}); err != nil { @@ -53,7 +55,7 @@ func TestResultCacheIsolatesByRunnerName(t *testing.T) { func TestResultCacheInvalidatesOnGoSumChange(t *testing.T) { dir := t.TempDir() - cache := newResultCache(dir, 0) + cache := newResultCache(dir, 0, nil) moduleDir := newGoModuleDir(t) // Seed go.sum so checksum is stable across writes. @@ -145,3 +147,18 @@ func TestAnalyzerDisableCacheAlwaysRunsRunner(t *testing.T) { t.Errorf("DisableCache should re-run runner per call; got %d calls", runner.called) } } + +func TestNewResultCacheWarnsWhenInitFails(t *testing.T) { + core, logs := observer.New(zap.WarnLevel) + blocker := filepath.Join(t.TempDir(), "blocker") + if err := os.WriteFile(blocker, []byte("not a directory"), 0o600); err != nil { + t.Fatalf("write blocker file: %v", err) + } + cache := newResultCache(filepath.Join(blocker, "nested"), 0, zap.New(core)) + if cache != nil { + t.Fatal("expected nil cache when the cache root cannot be created") + } + if got := logs.FilterLevelExact(zap.WarnLevel).Len(); got != 1 { + t.Fatalf("expected exactly one WARN log, got %d: %v", got, logs.All()) + } +} diff --git a/components/analyzers/jsreach/analyzer.go b/components/analyzers/jsreach/analyzer.go index 86dfca8b..61ae44c7 100644 --- a/components/analyzers/jsreach/analyzer.go +++ b/components/analyzers/jsreach/analyzer.go @@ -376,7 +376,7 @@ func (a Analyzer) cache() *resultCache { if a.DisableCache { return nil } - return newResultCache(a.CacheDir, a.CacheTTL) + return newResultCache(a.CacheDir, a.CacheTTL, a.logger()) } func resultFromRequest(req model.AnalyzeRequest) model.AnalyzeResult { diff --git a/components/analyzers/jsreach/cache.go b/components/analyzers/jsreach/cache.go index d90f2389..b9fa722e 100644 --- a/components/analyzers/jsreach/cache.go +++ b/components/analyzers/jsreach/cache.go @@ -10,6 +10,7 @@ import ( "time" cachepkg "github.com/bomly-dev/bomly-sdk/filecache" + "go.uber.org/zap" "github.com/bomly-dev/bomly-sdk/system" ) @@ -48,35 +49,44 @@ type cachedRunnerResult struct { // newResultCache constructs a result cache rooted at dir. If dir is // empty, the OS user cache directory is used. Errors creating the -// cache directory are non-fatal — they return a nil resultCache that -// the caller can use without checks. -func newResultCache(dir string, ttl time.Duration) *resultCache { +// cache directory are non-fatal — they log one WARN and return a nil +// resultCache that the caller can use without checks. +func newResultCache(dir string, ttl time.Duration, logger *zap.Logger) *resultCache { + logger = ensureLogger(logger) if ttl <= 0 { ttl = defaultCacheTTL } root := dir if root == "" { - root = defaultCacheRoot() - } - if root == "" { - return nil + defaultRoot, err := defaultCacheRoot() + if err != nil { + logger.Warn("jsreach: result cache disabled: user cache directory unavailable (non-fatal)", + zap.Error(err)) + return nil + } + root = defaultRoot } store, err := cachepkg.NewFileCache(root, ttl) if err != nil { + logger.Warn("jsreach: result cache disabled: cache initialization failed (non-fatal)", + zap.String("dir", root), zap.Error(err)) return nil } return &resultCache{store: store} } // defaultCacheRoot returns the platform-appropriate cache directory -// for jsreach analyzer results, or "" if the user cache directory -// cannot be determined. -func defaultCacheRoot() string { +// for jsreach analyzer results, or an error when the user cache +// directory cannot be determined. +func defaultCacheRoot() (string, error) { base, err := os.UserCacheDir() - if err != nil || base == "" { - return "" + if err != nil { + return "", err + } + if base == "" { + return "", errors.New("user cache directory is empty") } - return filepath.Join(base, "bomly", "analyzers", "jsreach") + return filepath.Join(base, "bomly", "analyzers", "jsreach"), nil } // keyFor builds a stable cache key for one project pass. Folds every diff --git a/components/analyzers/jsreach/cache_test.go b/components/analyzers/jsreach/cache_test.go index 48c34d82..2a83c91b 100644 --- a/components/analyzers/jsreach/cache_test.go +++ b/components/analyzers/jsreach/cache_test.go @@ -7,11 +7,13 @@ import ( "testing" model "github.com/bomly-dev/bomly-sdk" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" ) func TestResultCacheRoundTrip(t *testing.T) { dir := t.TempDir() - cache := newResultCache(dir, 0) + cache := newResultCache(dir, 0, nil) if cache == nil { t.Fatal("newResultCache returned nil for a writable dir") } @@ -42,7 +44,7 @@ func TestResultCacheRoundTrip(t *testing.T) { func TestResultCacheIsolatesByRunnerName(t *testing.T) { dir := t.TempDir() - cache := newResultCache(dir, 0) + cache := newResultCache(dir, 0, nil) projectDir := newNPMProjectDir(t) if err := cache.set(projectDir, "builtin", "1.0", RunnerResult{ImportedPackages: map[string]struct{}{"a": {}}}); err != nil { @@ -55,7 +57,7 @@ func TestResultCacheIsolatesByRunnerName(t *testing.T) { func TestResultCacheIsolatesByRunnerVersion(t *testing.T) { dir := t.TempDir() - cache := newResultCache(dir, 0) + cache := newResultCache(dir, 0, nil) projectDir := newNPMProjectDir(t) if err := cache.set(projectDir, "builtin", "1.0", RunnerResult{ImportedPackages: map[string]struct{}{"a": {}}}); err != nil { @@ -68,7 +70,7 @@ func TestResultCacheIsolatesByRunnerVersion(t *testing.T) { func TestResultCacheInvalidatesOnLockfileChange(t *testing.T) { dir := t.TempDir() - cache := newResultCache(dir, 0) + cache := newResultCache(dir, 0, nil) projectDir := newNPMProjectDir(t) // Seed package-lock.json so checksum is stable across writes. @@ -159,3 +161,18 @@ func TestAnalyzerDisableCacheAlwaysRunsRunner(t *testing.T) { t.Errorf("DisableCache should re-run runner per call; got %d calls", runner.called) } } + +func TestNewResultCacheWarnsWhenInitFails(t *testing.T) { + core, logs := observer.New(zap.WarnLevel) + blocker := filepath.Join(t.TempDir(), "blocker") + if err := os.WriteFile(blocker, []byte("not a directory"), 0o600); err != nil { + t.Fatalf("write blocker file: %v", err) + } + cache := newResultCache(filepath.Join(blocker, "nested"), 0, zap.New(core)) + if cache != nil { + t.Fatal("expected nil cache when the cache root cannot be created") + } + if got := logs.FilterLevelExact(zap.WarnLevel).Len(); got != 1 { + t.Fatalf("expected exactly one WARN log, got %d: %v", got, logs.All()) + } +} diff --git a/components/analyzers/jsreach/entrypoints.go b/components/analyzers/jsreach/entrypoints.go index 40b8a78d..fce7e5f0 100644 --- a/components/analyzers/jsreach/entrypoints.go +++ b/components/analyzers/jsreach/entrypoints.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "sort" "github.com/bomly-dev/bomly-sdk/system" ) @@ -148,9 +149,17 @@ func binEntryStrings(raw json.RawMessage) []string { } var m map[string]string if err := json.Unmarshal(raw, &m); err == nil { - out := make([]string, 0, len(m)) - for _, value := range m { - if value != "" { + names := make([]string, 0, len(m)) + for name := range m { + names = append(names, name) + } + // Emit in sorted key order so the entry list (and everything + // derived from it — logs, cache keys, fuzz determinism) is + // stable across runs. + sort.Strings(names) + out := make([]string, 0, len(names)) + for _, name := range names { + if value := m[name]; value != "" { out = append(out, value) } } @@ -175,8 +184,15 @@ func walkJSONStrings(raw json.RawMessage, emit func(string)) { } var asObject map[string]json.RawMessage if err := json.Unmarshal(raw, &asObject); err == nil { - for _, child := range asObject { - walkJSONStrings(child, emit) + // Walk object members in sorted key order so emission order is + // deterministic (Go map iteration is randomized). + keys := make([]string, 0, len(asObject)) + for key := range asObject { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + walkJSONStrings(asObject[key], emit) } } } diff --git a/components/analyzers/jsreach/entrypoints_fuzz_test.go b/components/analyzers/jsreach/entrypoints_fuzz_test.go new file mode 100644 index 00000000..248a7adb --- /dev/null +++ b/components/analyzers/jsreach/entrypoints_fuzz_test.go @@ -0,0 +1,50 @@ +package jsreach + +import ( + "encoding/json" + "reflect" + "testing" + + testutil "github.com/bomly-dev/bomly-sdk/testkit" +) + +// FuzzEntryPointStrings verifies that the package.json entry-point +// helpers never panic and produce deterministic output for arbitrary +// (valid, malformed, or truncated) JSON input within the shared fuzz +// input bound. The helpers tolerate any shape by design, so every +// input is expected to succeed; determinism is the real contract — +// walkJSONStrings and binEntryStrings walk JSON objects, and emission +// order must not depend on Go's randomized map iteration. +func FuzzEntryPointStrings(f *testing.F) { + for _, seed := range []string{ + `"./index.js"`, + `{"my-cli": "./cli.js", "other": "./other.js"}`, + `{".": {"import": "./esm/index.js", "require": "./cjs/index.js"}, "./util": "./util.js"}`, + `["./a.js", {"b": "./b.js"}, ["./c.js"]]`, + `{"browser": {"./fs": false}}`, + `{"unterminated": "./x.js"`, + `null`, + `42`, + ``, + } { + f.Add([]byte(seed)) + } + f.Fuzz(func(t *testing.T, data []byte) { + if len(data) > testutil.MaxFuzzInputSize { + return + } + raw := json.RawMessage(data) + helpers := map[string]func(json.RawMessage) []string{ + "browserEntryStrings": browserEntryStrings, + "exportsEntryStrings": exportsEntryStrings, + "binEntryStrings": binEntryStrings, + } + for name, helper := range helpers { + first := helper(raw) + second := helper(raw) + if !reflect.DeepEqual(first, second) { + t.Fatalf("%s changed result for identical input: first=%v second=%v", name, first, second) + } + } + }) +} diff --git a/components/analyzers/jsreach/runner_library.go b/components/analyzers/jsreach/runner_library.go index 90f74149..4924538d 100644 --- a/components/analyzers/jsreach/runner_library.go +++ b/components/analyzers/jsreach/runner_library.go @@ -109,14 +109,35 @@ func (r libraryRunner) Run(ctx context.Context, projectDir string) (RunnerResult }, } - // Honor cancellation by surfacing it as a runner error. esbuild - // itself doesn't take a context; we check before/after so a - // long-running pass still cancels at boundary. + // Honor cancellation mid-build. esbuild's Build call doesn't take + // a context, but its incremental Context API exposes Cancel(), so + // we run one Rebuild through a build context and cancel it from a + // watcher goroutine when ctx is done. Dispose always runs so the + // context's service goroutines don't leak. if err := ctx.Err(); err != nil { return RunnerResult{}, err } - result := api.Build(options) + buildCtx, ctxErr := api.Context(options) + if ctxErr != nil { + return RunnerResult{}, fmt.Errorf("esbuild context: %s", summarizeMessages(ctxErr.Errors, 3)) + } + defer buildCtx.Dispose() + + watchDone := make(chan struct{}) + defer close(watchDone) + go func() { + select { + case <-ctx.Done(): + // Cancel stops the in-flight build; Rebuild then returns + // promptly with a "The build was canceled" error, which the + // ctx.Err() check below converts into the cancellation error. + buildCtx.Cancel() + case <-watchDone: + } + }() + + result := buildCtx.Rebuild() if err := ctx.Err(); err != nil { return RunnerResult{}, err } diff --git a/components/analyzers/jsreach/runner_testdata_test.go b/components/analyzers/jsreach/runner_testdata_test.go index cf5afae9..cc11dab3 100644 --- a/components/analyzers/jsreach/runner_testdata_test.go +++ b/components/analyzers/jsreach/runner_testdata_test.go @@ -53,6 +53,15 @@ func TestLibraryRunnerWalksJSTestdata(t *testing.T) { } } +func TestLibraryRunnerHonorsCancelledContext(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err := NewRunner(nil).Run(ctx, jsProjectFixture("entrypoints")) + if !errors.Is(err, context.Canceled) { + t.Fatalf("err = %v, want context.Canceled", err) + } +} + func TestJSDynamicImportDetectionFromTestdata(t *testing.T) { if !detectDynamicImports(jsProjectFixture("entrypoints")) { t.Fatal("dynamic fixture was not detected") diff --git a/components/analyzers/jvmreach/analyzer.go b/components/analyzers/jvmreach/analyzer.go index 35018791..1eae1d18 100644 --- a/components/analyzers/jvmreach/analyzer.go +++ b/components/analyzers/jvmreach/analyzer.go @@ -383,7 +383,7 @@ func (a Analyzer) cache() *resultCache { if a.DisableCache { return nil } - return newResultCache(a.CacheDir, a.CacheTTL) + return newResultCache(a.CacheDir, a.CacheTTL, a.logger()) } func resultFromRequest(req model.AnalyzeRequest) model.AnalyzeResult { diff --git a/components/analyzers/jvmreach/cache.go b/components/analyzers/jvmreach/cache.go index 5d670b64..62c5e628 100644 --- a/components/analyzers/jvmreach/cache.go +++ b/components/analyzers/jvmreach/cache.go @@ -10,6 +10,7 @@ import ( "time" cachepkg "github.com/bomly-dev/bomly-sdk/filecache" + "go.uber.org/zap" "github.com/bomly-dev/bomly-sdk/system" ) @@ -29,30 +30,46 @@ type cachedRunnerResult struct { DynamicImportsDetected bool `json:"dynamic_imports_detected,omitempty"` } -func newResultCache(dir string, ttl time.Duration) *resultCache { +// newResultCache constructs a result cache rooted at dir. If dir is +// empty, the OS user cache directory is used. Errors creating the +// cache directory are non-fatal — they log one WARN and return a nil +// resultCache that the caller can use without checks. +func newResultCache(dir string, ttl time.Duration, logger *zap.Logger) *resultCache { + logger = ensureLogger(logger) if ttl <= 0 { ttl = defaultCacheTTL } root := dir if root == "" { - root = defaultCacheRoot() - } - if root == "" { - return nil + defaultRoot, err := defaultCacheRoot() + if err != nil { + logger.Warn("jvmreach: result cache disabled: user cache directory unavailable (non-fatal)", + zap.Error(err)) + return nil + } + root = defaultRoot } store, err := cachepkg.NewFileCache(root, ttl) if err != nil { + logger.Warn("jvmreach: result cache disabled: cache initialization failed (non-fatal)", + zap.String("dir", root), zap.Error(err)) return nil } return &resultCache{store: store} } -func defaultCacheRoot() string { +// defaultCacheRoot returns the platform-appropriate cache directory +// for jvmreach analyzer results, or an error when the user cache +// directory cannot be determined. +func defaultCacheRoot() (string, error) { base, err := os.UserCacheDir() - if err != nil || base == "" { - return "" + if err != nil { + return "", err + } + if base == "" { + return "", errors.New("user cache directory is empty") } - return filepath.Join(base, "bomly", "analyzers", "jvmreach") + return filepath.Join(base, "bomly", "analyzers", "jvmreach"), nil } func keyFor(projectDir, runnerName, runnerVersion string) (cachepkg.Key, error) { diff --git a/components/analyzers/jvmreach/cache_test.go b/components/analyzers/jvmreach/cache_test.go index 58f340e7..287f39d0 100644 --- a/components/analyzers/jvmreach/cache_test.go +++ b/components/analyzers/jvmreach/cache_test.go @@ -7,11 +7,13 @@ import ( "testing" model "github.com/bomly-dev/bomly-sdk" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" ) func TestResultCacheRoundTrip(t *testing.T) { dir := t.TempDir() - cache := newResultCache(dir, 0) + cache := newResultCache(dir, 0, nil) if cache == nil { t.Fatal("newResultCache returned nil") } @@ -41,7 +43,7 @@ func TestResultCacheRoundTrip(t *testing.T) { func TestResultCacheInvalidatesOnBuildFileChange(t *testing.T) { dir := t.TempDir() - cache := newResultCache(dir, 0) + cache := newResultCache(dir, 0, nil) projectDir := newJVMProjectDir(t) pom := filepath.Join(projectDir, "pom.xml") if err := cache.set(projectDir, "fake", "1.0", RunnerResult{}); err != nil { @@ -89,3 +91,18 @@ func TestAnalyzerWithCacheServesSecondCallFromCache(t *testing.T) { t.Errorf("cached path did not produce a reachable annotation: %+v", r) } } + +func TestNewResultCacheWarnsWhenInitFails(t *testing.T) { + core, logs := observer.New(zap.WarnLevel) + blocker := filepath.Join(t.TempDir(), "blocker") + if err := os.WriteFile(blocker, []byte("not a directory"), 0o600); err != nil { + t.Fatalf("write blocker file: %v", err) + } + cache := newResultCache(filepath.Join(blocker, "nested"), 0, zap.New(core)) + if cache != nil { + t.Fatal("expected nil cache when the cache root cannot be created") + } + if got := logs.FilterLevelExact(zap.WarnLevel).Len(); got != 1 { + t.Fatalf("expected exactly one WARN log, got %d: %v", got, logs.All()) + } +} diff --git a/components/analyzers/jvmreach/discover.go b/components/analyzers/jvmreach/discover.go index 18a10de6..a4bcdb5a 100644 --- a/components/analyzers/jvmreach/discover.go +++ b/components/analyzers/jvmreach/discover.go @@ -232,6 +232,9 @@ func readGradleModules(root string) []jvmModule { for _, module := range seen { modules = append(modules, module) } + // Sort so callers (and fuzz determinism checks) see a stable order + // regardless of Go's randomized map iteration. + sort.Slice(modules, func(i, j int) bool { return modules[i].Dir < modules[j].Dir }) return modules } diff --git a/components/analyzers/jvmreach/discover_fuzz_test.go b/components/analyzers/jvmreach/discover_fuzz_test.go new file mode 100644 index 00000000..f6aa43fb --- /dev/null +++ b/components/analyzers/jvmreach/discover_fuzz_test.go @@ -0,0 +1,86 @@ +package jvmreach + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + testutil "github.com/bomly-dev/bomly-sdk/testkit" +) + +// FuzzReadMavenProject verifies that the Maven pom.xml module reader +// never panics and produces deterministic results for arbitrary +// (valid, malformed, or truncated) XML input within the shared fuzz +// input bound. The reader is file-backed, so each iteration writes the +// input as pom.xml in a fresh temp dir and reads it back twice. +func FuzzReadMavenProject(f *testing.F) { + for _, seed := range []string{ + "", + `com.exampleapp`, + `com.examplechildcore../escape`, + `a`, + `not xml at all`, + "\xff\xfe", + } { + f.Add([]byte(seed)) + } + f.Fuzz(func(t *testing.T, data []byte) { + if len(data) > testutil.MaxFuzzInputSize { + return + } + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "pom.xml"), data, 0o600); err != nil { + t.Fatalf("write pom.xml: %v", err) + } + first, firstOK := readMavenProject(dir) + second, secondOK := readMavenProject(dir) + if firstOK != secondOK { + t.Fatalf("read changed success state: first=%v second=%v", firstOK, secondOK) + } + if !firstOK { + return + } + if !reflect.DeepEqual(first, second) { + t.Fatal("read changed result for identical input") + } + }) +} + +// FuzzReadGradleModules verifies that the Gradle settings module reader +// never panics and produces deterministic results for arbitrary +// (valid, malformed, or truncated) settings-script input within the +// shared fuzz input bound. Determinism matters here because the reader +// collects modules through a map; its output order must not depend on +// Go's randomized map iteration. +func FuzzReadGradleModules(f *testing.F) { + for _, seed := range []string{ + "", + `include ':core', ':app'`, + "include(\":a\")\ninclude ':b'\nproject(':a').projectDir = file('modules/a')\n", + `include ':escape'` + "\n" + `project(':escape').projectDir = file('../outside')`, + `include "unterminated`, + "rootProject.name = 'demo'\n/* include ':commented' */\n", + } { + f.Add([]byte(seed)) + } + f.Fuzz(func(t *testing.T, data []byte) { + if len(data) > testutil.MaxFuzzInputSize { + return + } + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, "settings.gradle"), data, 0o600); err != nil { + t.Fatalf("write settings.gradle: %v", err) + } + first := readGradleModules(root) + second := readGradleModules(root) + if !reflect.DeepEqual(first, second) { + t.Fatal("read changed result for identical input") + } + for _, module := range first { + if !pathContainsRoot(module.Dir, root) { + t.Fatalf("module dir %q escapes root %q", module.Dir, root) + } + } + }) +} diff --git a/components/analyzers/pyreach/analyzer.go b/components/analyzers/pyreach/analyzer.go index a54ca3f1..b4c28f63 100644 --- a/components/analyzers/pyreach/analyzer.go +++ b/components/analyzers/pyreach/analyzer.go @@ -250,7 +250,7 @@ func (a Analyzer) cache() *resultCache { if a.DisableCache { return nil } - return newResultCache(a.CacheDir, a.CacheTTL) + return newResultCache(a.CacheDir, a.CacheTTL, a.logger()) } // resultFromRequest returns the legacy-path result: the (in-place diff --git a/components/analyzers/pyreach/cache.go b/components/analyzers/pyreach/cache.go index d34d5856..a694942d 100644 --- a/components/analyzers/pyreach/cache.go +++ b/components/analyzers/pyreach/cache.go @@ -10,6 +10,7 @@ import ( "time" cachepkg "github.com/bomly-dev/bomly-sdk/filecache" + "go.uber.org/zap" "github.com/bomly-dev/bomly-sdk/system" ) @@ -43,35 +44,44 @@ type cachedRunnerResult struct { // newResultCache constructs a result cache rooted at dir. If dir is // empty, the OS user cache directory is used. Errors creating the -// cache directory are non-fatal — they return a nil resultCache that -// the caller can use without checks. -func newResultCache(dir string, ttl time.Duration) *resultCache { +// cache directory are non-fatal — they log one WARN and return a nil +// resultCache that the caller can use without checks. +func newResultCache(dir string, ttl time.Duration, logger *zap.Logger) *resultCache { + logger = ensureLogger(logger) if ttl <= 0 { ttl = defaultCacheTTL } root := dir if root == "" { - root = defaultCacheRoot() - } - if root == "" { - return nil + defaultRoot, err := defaultCacheRoot() + if err != nil { + logger.Warn("pyreach: result cache disabled: user cache directory unavailable (non-fatal)", + zap.Error(err)) + return nil + } + root = defaultRoot } store, err := cachepkg.NewFileCache(root, ttl) if err != nil { + logger.Warn("pyreach: result cache disabled: cache initialization failed (non-fatal)", + zap.String("dir", root), zap.Error(err)) return nil } return &resultCache{store: store} } // defaultCacheRoot returns the platform-appropriate cache directory -// for pyreach analyzer results, or "" if the user cache directory -// cannot be determined. -func defaultCacheRoot() string { +// for pyreach analyzer results, or an error when the user cache +// directory cannot be determined. +func defaultCacheRoot() (string, error) { base, err := os.UserCacheDir() - if err != nil || base == "" { - return "" + if err != nil { + return "", err + } + if base == "" { + return "", errors.New("user cache directory is empty") } - return filepath.Join(base, "bomly", "analyzers", "pyreach") + return filepath.Join(base, "bomly", "analyzers", "pyreach"), nil } // keyFor builds a stable cache key for one project pass. Folds every diff --git a/components/analyzers/pyreach/cache_test.go b/components/analyzers/pyreach/cache_test.go index 529affff..464cb46c 100644 --- a/components/analyzers/pyreach/cache_test.go +++ b/components/analyzers/pyreach/cache_test.go @@ -7,11 +7,13 @@ import ( "testing" model "github.com/bomly-dev/bomly-sdk" + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" ) func TestResultCacheRoundTrip(t *testing.T) { dir := t.TempDir() - cache := newResultCache(dir, 0) + cache := newResultCache(dir, 0, nil) if cache == nil { t.Fatal("newResultCache returned nil for a writable dir") } @@ -42,7 +44,7 @@ func TestResultCacheRoundTrip(t *testing.T) { func TestResultCacheIsolatesByRunnerVersion(t *testing.T) { dir := t.TempDir() - cache := newResultCache(dir, 0) + cache := newResultCache(dir, 0, nil) projectDir := newPythonProjectDir(t) if err := cache.set(projectDir, "library", "1.0", RunnerResult{ImportedDistributions: map[string]struct{}{"a": {}}}); err != nil { @@ -55,7 +57,7 @@ func TestResultCacheIsolatesByRunnerVersion(t *testing.T) { func TestResultCacheInvalidatesOnLockfileChange(t *testing.T) { dir := t.TempDir() - cache := newResultCache(dir, 0) + cache := newResultCache(dir, 0, nil) projectDir := newPythonProjectDir(t) lockfile := filepath.Join(projectDir, "requirements.txt") @@ -142,3 +144,18 @@ func TestAnalyzerDisableCacheAlwaysRunsRunner(t *testing.T) { t.Errorf("DisableCache should re-run runner per call; got %d calls", runner.called) } } + +func TestNewResultCacheWarnsWhenInitFails(t *testing.T) { + core, logs := observer.New(zap.WarnLevel) + blocker := filepath.Join(t.TempDir(), "blocker") + if err := os.WriteFile(blocker, []byte("not a directory"), 0o600); err != nil { + t.Fatalf("write blocker file: %v", err) + } + cache := newResultCache(filepath.Join(blocker, "nested"), 0, zap.New(core)) + if cache != nil { + t.Fatal("expected nil cache when the cache root cannot be created") + } + if got := logs.FilterLevelExact(zap.WarnLevel).Len(); got != 1 { + t.Fatalf("expected exactly one WARN log, got %d: %v", got, logs.All()) + } +} diff --git a/scripts/run-fuzz.sh b/scripts/run-fuzz.sh index 6e54ab88..f082318f 100755 --- a/scripts/run-fuzz.sh +++ b/scripts/run-fuzz.sh @@ -10,8 +10,11 @@ targets=( "github.com/bomly-dev/bomly-cli/internal/config FuzzLoadFile" "github.com/bomly-dev/bomly-cli/components/analyzers/govulncheck FuzzParseGovulncheckJSON" "github.com/bomly-dev/bomly-cli/components/analyzers/jsreach FuzzExtractImportedPackages" + "github.com/bomly-dev/bomly-cli/components/analyzers/jsreach FuzzEntryPointStrings" "github.com/bomly-dev/bomly-cli/components/analyzers/pyreach FuzzScanImports" "github.com/bomly-dev/bomly-cli/components/analyzers/jvmreach FuzzScanImports" + "github.com/bomly-dev/bomly-cli/components/analyzers/jvmreach FuzzReadMavenProject" + "github.com/bomly-dev/bomly-cli/components/analyzers/jvmreach FuzzReadGradleModules" "github.com/bomly-dev/bomly-cli/internal/detectors/cargo FuzzDepGraphFromCargoLock" "github.com/bomly-dev/bomly-cli/internal/detectors/cocoapods FuzzDepGraphFromPodfileLock" "github.com/bomly-dev/bomly-cli/internal/detectors/composer FuzzDepGraphFromComposerLock" diff --git a/test/assurance/PARSER_FUZZING.md b/test/assurance/PARSER_FUZZING.md index d3501e70..519a377a 100644 --- a/test/assurance/PARSER_FUZZING.md +++ b/test/assurance/PARSER_FUZZING.md @@ -20,6 +20,7 @@ the reader inventory, cache behavior, and intentional exclusions. | SBOM | automatic SPDX and CycloneDX decoding; Syft JSON identification and deterministic rejection (the format is no longer ingested) | | Analyzer output | govulncheck JSON stream, esbuild metafile | | Analyzer source scanning | Python import scanner, JVM import scanner | +| Analyzer project configuration | package.json entry-point helpers (jsreach), Maven pom.xml module reader and Gradle settings module reader (jvmreach) | | Node lockfiles | npm, pnpm, Yarn, Bun | | Node project configuration | package.json, pnpm-workspace.yaml, and .npmrc behind the package-manager warning checks | | Python lockfiles | Poetry, uv, Pipenv | @@ -36,8 +37,10 @@ oversized structures within the bound, and arbitrary path/reference text. - Command-backed detectors are exercised through fake-binary unit tests and smoke tests. Their parsers are fuzzed only when the command output has an isolated, deterministic in-process parser. -- Maven, Gradle, and SBT XML/tree output is coupled to command execution and - does not currently expose a pure parser boundary. +- Maven, Gradle, and SBT XML/tree *command output* is coupled to command + execution and does not currently expose a pure parser boundary. The + jvmreach analyzer's file-backed pom.xml and Gradle settings readers are a + separate surface and are fuzzed (see the native-target table above). - Archive extraction uses Go standard-library readers plus explicit path containment checks; hostile archive path behavior remains covered by security tests coordinated with the threat-model work. From b378934d0df329d2fab8419902f03a329136ed81 Mon Sep 17 00:00:00 2001 From: Ahmed ElMallah Date: Thu, 13 Aug 2026 00:01:19 -0700 Subject: [PATCH 2/2] fix(components): close jsreach cancel race before Rebuild starts esbuild's Cancel() only cuts short an active build; a cancel landing between api.Context and Rebuild's build start was a no-op and the build ran to completion. Run Rebuild on a goroutine and, on ctx.Done, retry Cancel until Rebuild returns. Add a race-exercising test. Co-Authored-By: Claude Fable 5 --- .../analyzers/jsreach/runner_library.go | 38 +++++++++++-------- .../analyzers/jsreach/runner_testdata_test.go | 15 ++++++++ 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/components/analyzers/jsreach/runner_library.go b/components/analyzers/jsreach/runner_library.go index 4924538d..f5ab1d65 100644 --- a/components/analyzers/jsreach/runner_library.go +++ b/components/analyzers/jsreach/runner_library.go @@ -5,6 +5,7 @@ import ( "fmt" "path/filepath" "runtime/debug" + "time" "github.com/evanw/esbuild/pkg/api" "go.uber.org/zap" @@ -111,9 +112,9 @@ func (r libraryRunner) Run(ctx context.Context, projectDir string) (RunnerResult // Honor cancellation mid-build. esbuild's Build call doesn't take // a context, but its incremental Context API exposes Cancel(), so - // we run one Rebuild through a build context and cancel it from a - // watcher goroutine when ctx is done. Dispose always runs so the - // context's service goroutines don't leak. + // we run one Rebuild on a goroutine and cancel it when ctx is + // done. Dispose always runs so the context's service goroutines + // don't leak. if err := ctx.Err(); err != nil { return RunnerResult{}, err } @@ -124,20 +125,27 @@ func (r libraryRunner) Run(ctx context.Context, projectDir string) (RunnerResult } defer buildCtx.Dispose() - watchDone := make(chan struct{}) - defer close(watchDone) - go func() { - select { - case <-ctx.Done(): - // Cancel stops the in-flight build; Rebuild then returns - // promptly with a "The build was canceled" error, which the - // ctx.Err() check below converts into the cancellation error. + resultCh := make(chan api.BuildResult, 1) + go func() { resultCh <- buildCtx.Rebuild() }() + + var result api.BuildResult + select { + case result = <-resultCh: + case <-ctx.Done(): + // Cancel only cuts short a build that is already in flight; if + // the goroutine above hasn't started Rebuild's build yet the + // call is a no-op, so retry until Rebuild returns. A canceled + // build finishes promptly with a "The build was canceled" + // error, which we fold into the cancellation error here. + for { buildCtx.Cancel() - case <-watchDone: + select { + case <-resultCh: + return RunnerResult{}, ctx.Err() + case <-time.After(10 * time.Millisecond): + } } - }() - - result := buildCtx.Rebuild() + } if err := ctx.Err(); err != nil { return RunnerResult{}, err } diff --git a/components/analyzers/jsreach/runner_testdata_test.go b/components/analyzers/jsreach/runner_testdata_test.go index cc11dab3..3e97731e 100644 --- a/components/analyzers/jsreach/runner_testdata_test.go +++ b/components/analyzers/jsreach/runner_testdata_test.go @@ -62,6 +62,21 @@ func TestLibraryRunnerHonorsCancelledContext(t *testing.T) { } } +func TestLibraryRunnerCancelRacingBuild(t *testing.T) { + // Cancel concurrently with Run so the cancellation lands anywhere + // between context creation and build completion — including before + // Rebuild has started an active build, the window where a single + // Cancel call would be a no-op. Either outcome is valid: the build + // finished first (nil error) or cancellation won (context.Canceled). + // The invariant is that Run returns and never reports anything else. + ctx, cancel := context.WithCancel(context.Background()) + go cancel() + _, err := NewRunner(nil).Run(ctx, jsProjectFixture("entrypoints")) + if err != nil && !errors.Is(err, context.Canceled) { + t.Fatalf("err = %v, want nil or context.Canceled", err) + } +} + func TestJSDynamicImportDetectionFromTestdata(t *testing.T) { if !detectDynamicImports(jsProjectFixture("entrypoints")) { t.Fatal("dynamic fixture was not detected")