socket-cli: multi-root support for --dynamic-sbom-inference - #1484
Open
Jeppe Fredsgaard Blaabjerg (jfblaa) wants to merge 6 commits into
Open
Conversation
…i-root generator socket scan create --dynamic-sbom-inference (with or without --reach) now generates Socket facts recursively across every independent gradle/sbt/maven build root under the scan target, instead of only the one at the scan root. Redesign the resolved-artifact-paths sidecar passed to Coana (--compute-artifacts-sidecar) to be keyed by build root rather than a single flat, cross-root-deduplicated list, so two independent build roots emitting modules with the same package identity can never collide - each root's data is structurally isolated instead of relying on a filtering convention. Reuse a single sbt toolchain-provisioning directory across every sbt root discovered in one run instead of re-provisioning it per root. REA-704
A few comments/test titles still referred to "factsFiles" and "tagging" from an intermediate sidecar iteration that keyed entries by tag rather than by top-level facts-file key. Update them to match the shipped design.
Needed for multi-root --dynamic-sbom-inference: the per-facts-file sidecar format this branch produces requires a matching Coana CLI that consumes it.
…-cli-multi-root-support-for-dynamic-sbom-inference # Conflicts: # package.json # pnpm-lock.yaml
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Jeppe Fredsgaard Blaabjerg (jfblaa)
requested a review
from Martin Torp (mtorp)
August 6, 2026 08:27
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Scan ignores recursive manifest failures
- scan-create now throws InputError when any recursive outcome has status failed, aborting before upload or reachability.
- ✅ Fixed: Sbt global base deleted before reach
- generateRecursiveManifests accepts a caller-owned tmpDir, and scan-create passes manifestTmpDir so the sbt global base survives until reachability finishes.
Or push these changes by commenting:
@cursor push 132d76ff61
Preview (132d76ff61)
diff --git a/src/commands/manifest/generate-recursive-manifests.mts b/src/commands/manifest/generate-recursive-manifests.mts
--- a/src/commands/manifest/generate-recursive-manifests.mts
+++ b/src/commands/manifest/generate-recursive-manifests.mts
@@ -217,6 +217,7 @@ export async function generateRecursiveManifests({
cwd,
excludePaths,
sidecarAcc,
+ tmpDir,
verbose,
withFiles,
}: {
@@ -217,6 +217,7 @@ export async function generateRecursiveManifests({
cwd,
excludePaths,
sidecarAcc,
+ tmpDir,
verbose,
withFiles,
}: {
@@ -225,6 +226,12 @@ export async function generateRecursiveManifests({
// Reachability path only: run build tools with files and fold resolved
// artifact paths into sidecarAcc, keyed by each root's own factsPath.
sidecarAcc?: SidecarAccumulator | undefined
+ // Caller-owned sbt global base; see ManifestScriptOptions.tmpDir. When
+ // supplied (e.g. scan-create, which needs the Scala toolchain to outlive
+ // this call for reachability's withFiles artifactPaths), reused as the
+ // shared base across every sbt root. Unset ⇒ allocated ephemerally and
+ // cleaned up before this function returns (standalone CLI path).
+ tmpDir?: string | undefined
verbose: boolean
withFiles?: boolean | undefined
}): Promise<RecursiveManifestOutcome[]> {
@@ -225,6 +226,12 @@ export async function generateRecursiveManifests({
// Reachability path only: run build tools with files and fold resolved
// artifact paths into sidecarAcc, keyed by each root's own factsPath.
sidecarAcc?: SidecarAccumulator | undefined
+ // Caller-owned sbt global base; see ManifestScriptOptions.tmpDir. When
+ // supplied (e.g. scan-create, which needs the Scala toolchain to outlive
+ // this call for reachability's withFiles artifactPaths), reused as the
+ // shared base across every sbt root. Unset ⇒ allocated ephemerally and
+ // cleaned up before this function returns (standalone CLI path).
+ tmpDir?: string | undefined
verbose: boolean
withFiles?: boolean | undefined
}): Promise<RecursiveManifestOutcome[]> {
@@ -259,9 +266,13 @@ export async function generateRecursiveManifests({
// instead of being reprovisioned per root (the plugin file is rewritten and
// records.tsv is fully overwritten - not appended - on every invocation, so
// reuse is safe). Skipped entirely when there's no sbt root to benefit.
- const outcomes = candidatesByTool.get('sbt')?.length
- ? await withTmpDir('socket-sbt-facts-shared-', runAll)
- : await runAll(undefined)
+ // Prefer a caller-owned tmpDir so reachability can keep withFiles paths
+ // under <global.base>/boot alive after this function returns.
+ const outcomes = !candidatesByTool.get('sbt')?.length
+ ? await runAll(undefined)
+ : tmpDir
+ ? await runAll(tmpDir)
+ : await withTmpDir('socket-sbt-facts-shared-', runAll)
if (verbose) {
logger.info(`Discovered ${outcomes.length} build-tool candidate(s).`)
@@ -259,9 +266,13 @@ export async function generateRecursiveManifests({
// instead of being reprovisioned per root (the plugin file is rewritten and
// records.tsv is fully overwritten - not appended - on every invocation, so
// reuse is safe). Skipped entirely when there's no sbt root to benefit.
- const outcomes = candidatesByTool.get('sbt')?.length
- ? await withTmpDir('socket-sbt-facts-shared-', runAll)
- : await runAll(undefined)
+ // Prefer a caller-owned tmpDir so reachability can keep withFiles paths
+ // under <global.base>/boot alive after this function returns.
+ const outcomes = !candidatesByTool.get('sbt')?.length
+ ? await runAll(undefined)
+ : tmpDir
+ ? await runAll(tmpDir)
+ : await withTmpDir('socket-sbt-facts-shared-', runAll)
if (verbose) {
logger.info(`Discovered ${outcomes.length} build-tool candidate(s).`)
diff --git a/src/commands/manifest/generate-recursive-manifests.test.mts b/src/commands/manifest/generate-recursive-manifests.test.mts
--- a/src/commands/manifest/generate-recursive-manifests.test.mts
+++ b/src/commands/manifest/generate-recursive-manifests.test.mts
@@ -577,6 +577,43 @@ describe('generateRecursiveManifests', () => {
}
})
+ it('reuses a caller-supplied tmpDir as the shared sbt global base instead of allocating one', async () => {
+ const outer = await fs.mkdtemp(path.join(tmpdir(), 'sbt-caller-tmpdir-'))
+ const callerTmp = path.join(outer, 'caller-owned-sbt-base')
+ const sbtA = path.join(outer, 'sbt-a')
+ const sbtB = path.join(outer, 'sbt-b')
+ try {
+ await fs.mkdir(callerTmp, { recursive: true })
+ await fs.mkdir(sbtA, { recursive: true })
+ await fs.mkdir(sbtB, { recursive: true })
+ await fs.writeFile(path.join(sbtA, 'build.sbt'), '')
+ await fs.writeFile(path.join(sbtB, 'build.sbt'), '')
+
+ const sbtTmpDirs: Array<string | undefined> = []
+ vi.mocked(runManifestFacts).mockImplementation(
+ async ({ cwd, tmpDir }) => {
+ sbtTmpDirs.push(tmpDir)
+ return {
+ factsPath: path.join(cwd, '.socket.facts.json'),
+ projects: [],
+ }
+ },
+ )
+
+ await generateRecursiveManifests({
+ cwd: outer,
+ tmpDir: callerTmp,
+ verbose: false,
+ })
+
+ expect(sbtTmpDirs).toEqual([callerTmp, callerTmp])
+ // Caller owns the directory; it must still exist after generation returns.
+ await expect(fs.stat(callerTmp)).resolves.toBeDefined()
+ } finally {
+ await fs.rm(outer, { recursive: true, force: true })
+ }
+ })
+
it('does not allocate a shared tmpDir at all when no sbt root is discovered', async () => {
const outer = await fs.mkdtemp(path.join(tmpdir(), 'no-sbt-tmpdir-'))
try {
@@ -577,6 +577,43 @@ describe('generateRecursiveManifests', () => {
}
})
+ it('reuses a caller-supplied tmpDir as the shared sbt global base instead of allocating one', async () => {
+ const outer = await fs.mkdtemp(path.join(tmpdir(), 'sbt-caller-tmpdir-'))
+ const callerTmp = path.join(outer, 'caller-owned-sbt-base')
+ const sbtA = path.join(outer, 'sbt-a')
+ const sbtB = path.join(outer, 'sbt-b')
+ try {
+ await fs.mkdir(callerTmp, { recursive: true })
+ await fs.mkdir(sbtA, { recursive: true })
+ await fs.mkdir(sbtB, { recursive: true })
+ await fs.writeFile(path.join(sbtA, 'build.sbt'), '')
+ await fs.writeFile(path.join(sbtB, 'build.sbt'), '')
+
+ const sbtTmpDirs: Array<string | undefined> = []
+ vi.mocked(runManifestFacts).mockImplementation(
+ async ({ cwd, tmpDir }) => {
+ sbtTmpDirs.push(tmpDir)
+ return {
+ factsPath: path.join(cwd, '.socket.facts.json'),
+ projects: [],
+ }
+ },
+ )
+
+ await generateRecursiveManifests({
+ cwd: outer,
+ tmpDir: callerTmp,
+ verbose: false,
+ })
+
+ expect(sbtTmpDirs).toEqual([callerTmp, callerTmp])
+ // Caller owns the directory; it must still exist after generation returns.
+ await expect(fs.stat(callerTmp)).resolves.toBeDefined()
+ } finally {
+ await fs.rm(outer, { recursive: true, force: true })
+ }
+ })
+
it('does not allocate a shared tmpDir at all when no sbt root is discovered', async () => {
const outer = await fs.mkdtemp(path.join(tmpdir(), 'no-sbt-tmpdir-'))
try {
diff --git a/src/commands/scan/handle-create-new-scan.mts b/src/commands/scan/handle-create-new-scan.mts
--- a/src/commands/scan/handle-create-new-scan.mts
+++ b/src/commands/scan/handle-create-new-scan.mts
@@ -18,6 +18,7 @@ import constants from '../../constants.mts'
import { checkCommandInput } from '../../utils/check-input.mts'
import { compressSocketFactsForUpload } from '../../utils/coana.mts'
import { findSocketYmlSync } from '../../utils/config.mts'
+import { InputError } from '../../utils/errors.mts'
import { withTmpDir } from '../../utils/fs.mts'
import { getPackageFilesForScan } from '../../utils/path-resolve.mts'
import { readOrDefaultSocketJson } from '../../utils/socket-json.mts'
@@ -18,6 +18,7 @@ import constants from '../../constants.mts'
import { checkCommandInput } from '../../utils/check-input.mts'
import { compressSocketFactsForUpload } from '../../utils/coana.mts'
import { findSocketYmlSync } from '../../utils/config.mts'
+import { InputError } from '../../utils/errors.mts'
import { withTmpDir } from '../../utils/fs.mts'
import { getPackageFilesForScan } from '../../utils/path-resolve.mts'
import { readOrDefaultSocketJson } from '../../utils/socket-json.mts'
@@ -177,9 +178,20 @@ export async function handleCreateNewScan({
cwd,
excludePaths: reach.excludePaths,
sidecarAcc,
+ // Keep the shared sbt global base alive until reachability below
+ // consumes withFiles artifactPaths under <global.base>/boot.
+ tmpDir: manifestTmpDir,
verbose: false,
withFiles: reach.runReachabilityAnalysis,
})
+ // Fail closed like handleManifestDynamicSbomInference /
+ // abortManifestRunIfFailed: a partial multi-root run must not upload
+ // or run reachability as if every build root succeeded.
+ if (outcomes.some(o => o.status === 'failed')) {
+ throw new InputError(
+ 'One or more build roots failed to generate Socket facts; aborting (see the errors above).',
+ )
+ }
const generatedFactsPaths = outcomes
.filter(o => o.status === 'generated')
.map(o => o.factsPath!)
@@ -177,9 +178,20 @@ export async function handleCreateNewScan({
cwd,
excludePaths: reach.excludePaths,
sidecarAcc,
+ // Keep the shared sbt global base alive until reachability below
+ // consumes withFiles artifactPaths under <global.base>/boot.
+ tmpDir: manifestTmpDir,
verbose: false,
withFiles: reach.runReachabilityAnalysis,
})
+ // Fail closed like handleManifestDynamicSbomInference /
+ // abortManifestRunIfFailed: a partial multi-root run must not upload
+ // or run reachability as if every build root succeeded.
+ if (outcomes.some(o => o.status === 'failed')) {
+ throw new InputError(
+ 'One or more build roots failed to generate Socket facts; aborting (see the errors above).',
+ )
+ }
const generatedFactsPaths = outcomes
.filter(o => o.status === 'generated')
.map(o => o.factsPath!)
diff --git a/src/commands/scan/handle-create-new-scan.test.mts b/src/commands/scan/handle-create-new-scan.test.mts
--- a/src/commands/scan/handle-create-new-scan.test.mts
+++ b/src/commands/scan/handle-create-new-scan.test.mts
@@ -201,7 +201,11 @@ describe('handleCreateNewScan excludePaths', () => {
await handleCreateNewScan(config)
expect(mockGenerateRecursiveManifests).toHaveBeenCalledWith(
- expect.objectContaining({ cwd: '/repo', withFiles: false }),
+ expect.objectContaining({
+ cwd: '/repo',
+ tmpDir: expect.any(String),
+ withFiles: false,
+ }),
)
expect(mockGenerateAutoManifest).toHaveBeenCalledWith(
expect.objectContaining({
@@ -201,7 +201,11 @@ describe('handleCreateNewScan excludePaths', () => {
await handleCreateNewScan(config)
expect(mockGenerateRecursiveManifests).toHaveBeenCalledWith(
- expect.objectContaining({ cwd: '/repo', withFiles: false }),
+ expect.objectContaining({
+ cwd: '/repo',
+ tmpDir: expect.any(String),
+ withFiles: false,
+ }),
)
expect(mockGenerateAutoManifest).toHaveBeenCalledWith(
expect.objectContaining({
@@ -227,6 +231,28 @@ describe('handleCreateNewScan excludePaths', () => {
)
})
+ it('aborts the scan when recursive manifest generation reports a failed root under --dynamic-sbom-inference', async () => {
+ mockGenerateRecursiveManifests.mockResolvedValueOnce([
+ {
+ dir: '/repo/service-a',
+ ecosystem: 'maven',
+ factsPath: '/repo/service-a/.socket.facts.json',
+ status: 'generated',
+ },
+ { dir: '/repo/service-b', ecosystem: 'maven', status: 'failed' },
+ { dir: '/repo/service-c', ecosystem: 'maven', status: 'aborted' },
+ ])
+
+ const config = createConfig({ autoManifest: true, targets: ['/repo'] })
+ config.reach.dynamicSbomInference = true
+
+ await expect(handleCreateNewScan(config)).rejects.toThrow(
+ 'One or more build roots failed to generate Socket facts',
+ )
+ expect(mockGetPackageFilesForScan).not.toHaveBeenCalled()
+ expect(mockFetchCreateOrgFullScan).not.toHaveBeenCalled()
+ })
+
it('accumulates a sidecar across recursively discovered build roots and forwards it to reachability analysis', async () => {
mockGenerateRecursiveManifests.mockImplementationOnce(
async ({ sidecarAcc }) => {
@@ -227,6 +231,28 @@ describe('handleCreateNewScan excludePaths', () => {
)
})
+ it('aborts the scan when recursive manifest generation reports a failed root under --dynamic-sbom-inference', async () => {
+ mockGenerateRecursiveManifests.mockResolvedValueOnce([
+ {
+ dir: '/repo/service-a',
+ ecosystem: 'maven',
+ factsPath: '/repo/service-a/.socket.facts.json',
+ status: 'generated',
+ },
+ { dir: '/repo/service-b', ecosystem: 'maven', status: 'failed' },
+ { dir: '/repo/service-c', ecosystem: 'maven', status: 'aborted' },
+ ])
+
+ const config = createConfig({ autoManifest: true, targets: ['/repo'] })
+ config.reach.dynamicSbomInference = true
+
+ await expect(handleCreateNewScan(config)).rejects.toThrow(
+ 'One or more build roots failed to generate Socket facts',
+ )
+ expect(mockGetPackageFilesForScan).not.toHaveBeenCalled()
+ expect(mockFetchCreateOrgFullScan).not.toHaveBeenCalled()
+ })
+
it('accumulates a sidecar across recursively discovered build roots and forwards it to reachability analysis', async () => {
mockGenerateRecursiveManifests.mockImplementationOnce(
async ({ sidecarAcc }) => {You can send follow-ups to the cloud agent here.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b5130fa. Configure here.
…ared boot dir alive through reach Address three issues from PR review: - handleCreateNewScan never inspected generateRecursiveManifests' outcomes for failed/aborted build roots, so a partial multi-root JVM run could silently upload and run reachability on an incomplete facts set. Now aborts loudly, matching handleManifestDynamicSbomInference's own check. - generateRecursiveManifests always allocated and cleaned up its own ephemeral shared sbt global base, even when reachability analysis would need to read resolved paths (e.g. the Scala standard library) out of it afterward. It now accepts a caller-supplied sbtTmpDir and reuses it as-is without cleaning it up; handleCreateNewScan passes the existing manifestTmpDir (already kept alive until reach finishes) when reach is on. - Dropped a stray internal ticket reference from a source comment.
…eneration A subprojectDir that escapes its declaring reactor's own directory (e.g. Maven's <module>../shared-lib</module>, or a Gradle projectDir relocation) was previously marked covered like any nested member, so whether it got its own independent facts generation depended purely on alphabetical discovery order relative to the reactor(s) that reference it. Such a path is independently locatable and potentially resolved differently on its own (e.g. a dependency version override the referencing reactor applies but a standalone build of the same directory would not), so it's a distinct, meaningful data point, not a redundant one. Only a genuine descendant of the reactor's own directory is now treated as covered.
Martin Torp (mtorp)
approved these changes
Aug 6, 2026
Jeppe Fredsgaard Blaabjerg (jfblaa)
enabled auto-merge (squash)
August 6, 2026 11:00
auto-merge was automatically disabled
August 6, 2026 11:02
Pull request was closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
socket scan create --dynamic-sbom-inference(with or without--reach) now generates Socket facts recursively across every independent gradle/sbt/maven build root under the scan target, instead of only the one at the scan root.--compute-artifacts-sidecar) to be keyed by build root rather than a single flat, cross-root-deduplicated list, so two independent build roots emitting modules with the same package identity can never collide.@coana-tech/clito15.10.5, the published version with matching consumer-side support for the new sidecar format.Paired with the Coana-side consumer work (REA-702) — both were needed together and have been verified working end-to-end, including against the published
@coana-tech/cli@15.10.5(not just a local dev build).Linear: REA-704
Test plan
pnpm run check:tsc(both projects)pnpm run lint --allsrc/commands/manifest+src/commands/scan+src/utils+src/commands/manifest/scriptstest suites (686 tests)@coana-tech/cli@15.10.5— all expected reachable/unreachable results present per projectNote
Medium Risk
Changes the reachability auto-manifest path and the frozen Coana sidecar contract; incorrect behavior would skew multi-root JVM scans, but the bump is paired with Coana 15.10.5 and coverage is extensive in manifest/scan tests.
Overview
--dynamic-sbom-inference(with--auto-manifest) now recursively generates.socket.facts.jsonfor every independent Gradle, Maven, and sbt root under the scan target, not only the cwd. JVM generation ingenerateAutoManifestis skipped for those ecosystems when this flag is on; conda/bazel still use auto-manifest. Generated facts paths are merged into scan targets.The resolved-artifact sidecar for Coana (
--compute-artifacts-sidecar) is redesigned: keys are absolute.socket.facts.jsonpaths, each holding that file’sprojectsandcomponentswith attachedtargets/sources(full SBOM-shaped entries). Cross-reactor coordinate deduplication is removed so identical module names in different reactors cannot collide. Recursive and auto-manifest sidecars are merged when both run; reachability uses--maven-use-only-socket-factsinstead of--maven-use-only-root-socket-facts.sbt roots in one run share a single temp global base (Scala toolchain cache) when multiple sbt projects are discovered. Sidecar keys use symlink-resolved facts paths via shared
realpathOrResolvedinutils/fs.mts.Bumps
@coana-tech/clito 15.10.5 (consumer for the new sidecar shape).Reviewed by Cursor Bugbot for commit b5130fa. Configure here.