Skip to content

Commit 9a6be85

Browse files
bomly-guyclaude
andauthored
fix(components): PR 386 deferred review follow-ups (#388)
* 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent e53a546 commit 9a6be85

20 files changed

Lines changed: 404 additions & 75 deletions

components/analyzers/govulncheck/analyzer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ func (a Analyzer) cache() *resultCache {
219219
if a.DisableCache {
220220
return nil
221221
}
222-
return newResultCache(a.CacheDir, a.CacheTTL)
222+
return newResultCache(a.CacheDir, a.CacheTTL, a.logger())
223223
}
224224

225225
func (a Analyzer) logger() *zap.Logger { return ensureLogger(a.Logger) }

components/analyzers/govulncheck/cache.go

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"time"
1212

1313
cachepkg "github.com/bomly-dev/bomly-sdk/filecache"
14+
"go.uber.org/zap"
1415

1516
"github.com/bomly-dev/bomly-sdk/system"
1617
)
@@ -47,35 +48,44 @@ type cachedRunnerResult struct {
4748

4849
// newResultCache constructs a result cache rooted at dir. If dir is
4950
// empty, the OS user cache directory is used. Errors creating the cache
50-
// directory are non-fatal — they return a nil resultCache that the caller
51-
// can use without checks.
52-
func newResultCache(dir string, ttl time.Duration) *resultCache {
51+
// directory are non-fatal — they log one WARN and return a nil
52+
// resultCache that the caller can use without checks.
53+
func newResultCache(dir string, ttl time.Duration, logger *zap.Logger) *resultCache {
54+
logger = ensureLogger(logger)
5355
if ttl <= 0 {
5456
ttl = defaultCacheTTL
5557
}
5658
root := dir
5759
if root == "" {
58-
root = defaultCacheRoot()
59-
}
60-
if root == "" {
61-
return nil
60+
defaultRoot, err := defaultCacheRoot()
61+
if err != nil {
62+
logger.Warn("govulncheck: result cache disabled: user cache directory unavailable (non-fatal)",
63+
zap.Error(err))
64+
return nil
65+
}
66+
root = defaultRoot
6267
}
6368
store, err := cachepkg.NewFileCache(root, ttl)
6469
if err != nil {
70+
logger.Warn("govulncheck: result cache disabled: cache initialization failed (non-fatal)",
71+
zap.String("dir", root), zap.Error(err))
6572
return nil
6673
}
6774
return &resultCache{store: store}
6875
}
6976

7077
// defaultCacheRoot returns the platform-appropriate cache directory for
71-
// govulncheck analyzer results, or "" if the user cache directory cannot
72-
// be determined.
73-
func defaultCacheRoot() string {
78+
// govulncheck analyzer results, or an error when the user cache
79+
// directory cannot be determined.
80+
func defaultCacheRoot() (string, error) {
7481
base, err := os.UserCacheDir()
75-
if err != nil || base == "" {
76-
return ""
82+
if err != nil {
83+
return "", err
84+
}
85+
if base == "" {
86+
return "", errors.New("user cache directory is empty")
7787
}
78-
return filepath.Join(base, "bomly", "analyzers", "govulncheck")
88+
return filepath.Join(base, "bomly", "analyzers", "govulncheck"), nil
7989
}
8090

8191
// keyFor builds a stable cache key for one module run. The key folds

components/analyzers/govulncheck/cache_test.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ import (
77
"testing"
88

99
model "github.com/bomly-dev/bomly-sdk"
10+
"go.uber.org/zap"
11+
"go.uber.org/zap/zaptest/observer"
1012
)
1113

1214
func TestResultCacheRoundTrip(t *testing.T) {
1315
dir := t.TempDir()
14-
cache := newResultCache(dir, 0)
16+
cache := newResultCache(dir, 0, nil)
1517
if cache == nil {
1618
t.Fatal("newResultCache returned nil for a writable dir")
1719
}
@@ -40,7 +42,7 @@ func TestResultCacheRoundTrip(t *testing.T) {
4042

4143
func TestResultCacheIsolatesByRunnerName(t *testing.T) {
4244
dir := t.TempDir()
43-
cache := newResultCache(dir, 0)
45+
cache := newResultCache(dir, 0, nil)
4446
moduleDir := newGoModuleDir(t)
4547

4648
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) {
5355

5456
func TestResultCacheInvalidatesOnGoSumChange(t *testing.T) {
5557
dir := t.TempDir()
56-
cache := newResultCache(dir, 0)
58+
cache := newResultCache(dir, 0, nil)
5759
moduleDir := newGoModuleDir(t)
5860

5961
// Seed go.sum so checksum is stable across writes.
@@ -145,3 +147,18 @@ func TestAnalyzerDisableCacheAlwaysRunsRunner(t *testing.T) {
145147
t.Errorf("DisableCache should re-run runner per call; got %d calls", runner.called)
146148
}
147149
}
150+
151+
func TestNewResultCacheWarnsWhenInitFails(t *testing.T) {
152+
core, logs := observer.New(zap.WarnLevel)
153+
blocker := filepath.Join(t.TempDir(), "blocker")
154+
if err := os.WriteFile(blocker, []byte("not a directory"), 0o600); err != nil {
155+
t.Fatalf("write blocker file: %v", err)
156+
}
157+
cache := newResultCache(filepath.Join(blocker, "nested"), 0, zap.New(core))
158+
if cache != nil {
159+
t.Fatal("expected nil cache when the cache root cannot be created")
160+
}
161+
if got := logs.FilterLevelExact(zap.WarnLevel).Len(); got != 1 {
162+
t.Fatalf("expected exactly one WARN log, got %d: %v", got, logs.All())
163+
}
164+
}

components/analyzers/jsreach/analyzer.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,7 @@ func (a Analyzer) cache() *resultCache {
376376
if a.DisableCache {
377377
return nil
378378
}
379-
return newResultCache(a.CacheDir, a.CacheTTL)
379+
return newResultCache(a.CacheDir, a.CacheTTL, a.logger())
380380
}
381381

382382
func resultFromRequest(req model.AnalyzeRequest) model.AnalyzeResult {

components/analyzers/jsreach/cache.go

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"time"
1111

1212
cachepkg "github.com/bomly-dev/bomly-sdk/filecache"
13+
"go.uber.org/zap"
1314

1415
"github.com/bomly-dev/bomly-sdk/system"
1516
)
@@ -48,35 +49,44 @@ type cachedRunnerResult struct {
4849

4950
// newResultCache constructs a result cache rooted at dir. If dir is
5051
// empty, the OS user cache directory is used. Errors creating the
51-
// cache directory are non-fatal — they return a nil resultCache that
52-
// the caller can use without checks.
53-
func newResultCache(dir string, ttl time.Duration) *resultCache {
52+
// cache directory are non-fatal — they log one WARN and return a nil
53+
// resultCache that the caller can use without checks.
54+
func newResultCache(dir string, ttl time.Duration, logger *zap.Logger) *resultCache {
55+
logger = ensureLogger(logger)
5456
if ttl <= 0 {
5557
ttl = defaultCacheTTL
5658
}
5759
root := dir
5860
if root == "" {
59-
root = defaultCacheRoot()
60-
}
61-
if root == "" {
62-
return nil
61+
defaultRoot, err := defaultCacheRoot()
62+
if err != nil {
63+
logger.Warn("jsreach: result cache disabled: user cache directory unavailable (non-fatal)",
64+
zap.Error(err))
65+
return nil
66+
}
67+
root = defaultRoot
6368
}
6469
store, err := cachepkg.NewFileCache(root, ttl)
6570
if err != nil {
71+
logger.Warn("jsreach: result cache disabled: cache initialization failed (non-fatal)",
72+
zap.String("dir", root), zap.Error(err))
6673
return nil
6774
}
6875
return &resultCache{store: store}
6976
}
7077

7178
// defaultCacheRoot returns the platform-appropriate cache directory
72-
// for jsreach analyzer results, or "" if the user cache directory
73-
// cannot be determined.
74-
func defaultCacheRoot() string {
79+
// for jsreach analyzer results, or an error when the user cache
80+
// directory cannot be determined.
81+
func defaultCacheRoot() (string, error) {
7582
base, err := os.UserCacheDir()
76-
if err != nil || base == "" {
77-
return ""
83+
if err != nil {
84+
return "", err
85+
}
86+
if base == "" {
87+
return "", errors.New("user cache directory is empty")
7888
}
79-
return filepath.Join(base, "bomly", "analyzers", "jsreach")
89+
return filepath.Join(base, "bomly", "analyzers", "jsreach"), nil
8090
}
8191

8292
// keyFor builds a stable cache key for one project pass. Folds every

components/analyzers/jsreach/cache_test.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ import (
77
"testing"
88

99
model "github.com/bomly-dev/bomly-sdk"
10+
"go.uber.org/zap"
11+
"go.uber.org/zap/zaptest/observer"
1012
)
1113

1214
func TestResultCacheRoundTrip(t *testing.T) {
1315
dir := t.TempDir()
14-
cache := newResultCache(dir, 0)
16+
cache := newResultCache(dir, 0, nil)
1517
if cache == nil {
1618
t.Fatal("newResultCache returned nil for a writable dir")
1719
}
@@ -42,7 +44,7 @@ func TestResultCacheRoundTrip(t *testing.T) {
4244

4345
func TestResultCacheIsolatesByRunnerName(t *testing.T) {
4446
dir := t.TempDir()
45-
cache := newResultCache(dir, 0)
47+
cache := newResultCache(dir, 0, nil)
4648
projectDir := newNPMProjectDir(t)
4749

4850
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) {
5557

5658
func TestResultCacheIsolatesByRunnerVersion(t *testing.T) {
5759
dir := t.TempDir()
58-
cache := newResultCache(dir, 0)
60+
cache := newResultCache(dir, 0, nil)
5961
projectDir := newNPMProjectDir(t)
6062

6163
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) {
6870

6971
func TestResultCacheInvalidatesOnLockfileChange(t *testing.T) {
7072
dir := t.TempDir()
71-
cache := newResultCache(dir, 0)
73+
cache := newResultCache(dir, 0, nil)
7274
projectDir := newNPMProjectDir(t)
7375

7476
// Seed package-lock.json so checksum is stable across writes.
@@ -159,3 +161,18 @@ func TestAnalyzerDisableCacheAlwaysRunsRunner(t *testing.T) {
159161
t.Errorf("DisableCache should re-run runner per call; got %d calls", runner.called)
160162
}
161163
}
164+
165+
func TestNewResultCacheWarnsWhenInitFails(t *testing.T) {
166+
core, logs := observer.New(zap.WarnLevel)
167+
blocker := filepath.Join(t.TempDir(), "blocker")
168+
if err := os.WriteFile(blocker, []byte("not a directory"), 0o600); err != nil {
169+
t.Fatalf("write blocker file: %v", err)
170+
}
171+
cache := newResultCache(filepath.Join(blocker, "nested"), 0, zap.New(core))
172+
if cache != nil {
173+
t.Fatal("expected nil cache when the cache root cannot be created")
174+
}
175+
if got := logs.FilterLevelExact(zap.WarnLevel).Len(); got != 1 {
176+
t.Fatalf("expected exactly one WARN log, got %d: %v", got, logs.All())
177+
}
178+
}

components/analyzers/jsreach/entrypoints.go

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"os"
88
"path/filepath"
9+
"sort"
910

1011
"github.com/bomly-dev/bomly-sdk/system"
1112
)
@@ -148,9 +149,17 @@ func binEntryStrings(raw json.RawMessage) []string {
148149
}
149150
var m map[string]string
150151
if err := json.Unmarshal(raw, &m); err == nil {
151-
out := make([]string, 0, len(m))
152-
for _, value := range m {
153-
if value != "" {
152+
names := make([]string, 0, len(m))
153+
for name := range m {
154+
names = append(names, name)
155+
}
156+
// Emit in sorted key order so the entry list (and everything
157+
// derived from it — logs, cache keys, fuzz determinism) is
158+
// stable across runs.
159+
sort.Strings(names)
160+
out := make([]string, 0, len(names))
161+
for _, name := range names {
162+
if value := m[name]; value != "" {
154163
out = append(out, value)
155164
}
156165
}
@@ -175,8 +184,15 @@ func walkJSONStrings(raw json.RawMessage, emit func(string)) {
175184
}
176185
var asObject map[string]json.RawMessage
177186
if err := json.Unmarshal(raw, &asObject); err == nil {
178-
for _, child := range asObject {
179-
walkJSONStrings(child, emit)
187+
// Walk object members in sorted key order so emission order is
188+
// deterministic (Go map iteration is randomized).
189+
keys := make([]string, 0, len(asObject))
190+
for key := range asObject {
191+
keys = append(keys, key)
192+
}
193+
sort.Strings(keys)
194+
for _, key := range keys {
195+
walkJSONStrings(asObject[key], emit)
180196
}
181197
}
182198
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package jsreach
2+
3+
import (
4+
"encoding/json"
5+
"reflect"
6+
"testing"
7+
8+
testutil "github.com/bomly-dev/bomly-sdk/testkit"
9+
)
10+
11+
// FuzzEntryPointStrings verifies that the package.json entry-point
12+
// helpers never panic and produce deterministic output for arbitrary
13+
// (valid, malformed, or truncated) JSON input within the shared fuzz
14+
// input bound. The helpers tolerate any shape by design, so every
15+
// input is expected to succeed; determinism is the real contract —
16+
// walkJSONStrings and binEntryStrings walk JSON objects, and emission
17+
// order must not depend on Go's randomized map iteration.
18+
func FuzzEntryPointStrings(f *testing.F) {
19+
for _, seed := range []string{
20+
`"./index.js"`,
21+
`{"my-cli": "./cli.js", "other": "./other.js"}`,
22+
`{".": {"import": "./esm/index.js", "require": "./cjs/index.js"}, "./util": "./util.js"}`,
23+
`["./a.js", {"b": "./b.js"}, ["./c.js"]]`,
24+
`{"browser": {"./fs": false}}`,
25+
`{"unterminated": "./x.js"`,
26+
`null`,
27+
`42`,
28+
``,
29+
} {
30+
f.Add([]byte(seed))
31+
}
32+
f.Fuzz(func(t *testing.T, data []byte) {
33+
if len(data) > testutil.MaxFuzzInputSize {
34+
return
35+
}
36+
raw := json.RawMessage(data)
37+
helpers := map[string]func(json.RawMessage) []string{
38+
"browserEntryStrings": browserEntryStrings,
39+
"exportsEntryStrings": exportsEntryStrings,
40+
"binEntryStrings": binEntryStrings,
41+
}
42+
for name, helper := range helpers {
43+
first := helper(raw)
44+
second := helper(raw)
45+
if !reflect.DeepEqual(first, second) {
46+
t.Fatalf("%s changed result for identical input: first=%v second=%v", name, first, second)
47+
}
48+
}
49+
})
50+
}

0 commit comments

Comments
 (0)