From 386e1a724fc3d6a4c7609ecfbdba14a2169d092b Mon Sep 17 00:00:00 2001 From: Jeppe Fredsgaard Blaabjerg Date: Tue, 27 Oct 2026 08:38:38 +0100 Subject: [PATCH 1/3] feat(manifest): build every POM a scan uploads with --dynamic-sbom-inference Maven build roots are discovered with the API's supported-file pattern for POMs, case-insensitively as the upload matches it, instead of only pom.xml. Each POM is its own build root: a non-default POM runs with `--file`, and coverage by a parent build is judged per file. Discovery also applies socket.yml's projectIgnorePaths, as the upload does, so a POM a scan would not upload is not built either. --exclude-paths and .gitignore matching stay case-sensitive. The recursive setup wizard asks once per directory holding several POMs, and no longer disables a whole directory when --exclude-paths matches only one of its POMs. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 + .../manifest/discover-manifest-roots.mts | 118 ++++++++--- .../manifest/discover-manifest-roots.test.mts | 175 +++++++++++++++- .../manifest/generate-recursive-manifests.mts | 47 +++-- .../generate-recursive-manifests.test.mts | 72 +++++++ ...output-manifest-dynamic-sbom-inference.mts | 3 +- src/commands/manifest/scripts/build-tool.mts | 27 +++ .../manifest/scripts/build-tool.test.mts | 16 +- .../setup-recursive-manifest-config.mts | 103 ++++++---- .../setup-recursive-manifest-config.test.mts | 189 +++++++++++++++--- .../pom-names/app-dev.pom.xml | 6 + .../dynamic-sbom-inference/pom-names/bare/pom | 6 + .../pom-names/other.pom.xml | 6 + .../dynamic-sbom-inference/pom-names/pom.xml | 6 + .../pom-names/pom.xml.bak | 6 + .../pom-names/upper/POM.XML | 6 + .../pom-names/vendored/lib-1.0.pom | 6 + 17 files changed, 669 insertions(+), 128 deletions(-) create mode 100644 test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/app-dev.pom.xml create mode 100644 test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/bare/pom create mode 100644 test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/other.pom.xml create mode 100644 test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/pom.xml create mode 100644 test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/pom.xml.bak create mode 100644 test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/upper/POM.XML create mode 100644 test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/vendored/lib-1.0.pom diff --git a/CHANGELOG.md b/CHANGELOG.md index aeb61ddc1..d5879af22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +## [Unreleased] + +### Changed +- `--dynamic-sbom-inference` now builds every Maven POM a scan uploads, not only `pom.xml`: `-.pom` and `.pom.xml` files, a bare `pom`, and names in any case, each as a build of its own. Like the upload, it skips POMs matched by `.gitignore` or by `projectIgnorePaths` in `socket.yml`. + ## [1.11.0](https://github.com/SocketDev/socket-cli/releases/tag/v1.11.0) - 2026-10-26 ### Added diff --git a/src/commands/manifest/discover-manifest-roots.mts b/src/commands/manifest/discover-manifest-roots.mts index 695d10e3d..98541573d 100644 --- a/src/commands/manifest/discover-manifest-roots.mts +++ b/src/commands/manifest/discover-manifest-roots.mts @@ -1,6 +1,10 @@ import { existsSync } from 'node:fs' import path from 'node:path' +import micromatch from 'micromatch' + +import { defaultEntryFile, isAddressedByFile } from './scripts/build-tool.mts' +import { findSocketYmlSync } from '../../utils/config.mts' import { realpathOrResolved } from '../../utils/fs.mts' import { globWithGitIgnore } from '../../utils/glob.mts' import { excludePathToScanIgnores } from '../scan/exclude-paths.mts' @@ -11,8 +15,19 @@ import type { SocketJson } from '../../utils/socket-json.mts' const BUILD_TOOLS: BuildTool[] = ['gradle', 'maven', 'sbt'] +// In the pattern, not the walk, whose ignores (--exclude-paths, .gitignore) +// stay case-sensitive. Assumes every letter in the pattern is a literal. +function foldCase(pattern: string): string { + return pattern.replace( + /[a-z]/gi, + c => `[${c.toLowerCase()}${c.toUpperCase()}]`, + ) +} + // One glob-suffix set per ecosystem's build-descriptor marker(s); `settings.gradle(.kts)` -// covers Kotlin-DSL roots that have no root `build.gradle`. +// covers Kotlin-DSL roots that have no root `build.gradle`. Maven mirrors the +// `pomxml` pattern of the API's supported files, case-insensitively as a scan +// matches it, so every POM a scan uploads is also a build root here. const MARKERS_BY_TOOL: Record = { __proto__: null, gradle: [ @@ -21,17 +36,34 @@ const MARKERS_BY_TOOL: Record = { 'settings.gradle', 'settings.gradle.kts', ], - maven: ['pom.xml'], + maven: [foldCase('{*-*.,}pom{.xml,}')], sbt: ['build.sbt'], } as unknown as Record -const TOOL_BY_MARKER: Record = { - __proto__: null, -} as unknown as Record -for (const tool of BUILD_TOOLS) { - for (const marker of MARKERS_BY_TOOL[tool]) { - TOOL_BY_MARKER[marker] = tool - } +const MARKER_MATCHERS_BY_TOOL = Object.fromEntries( + BUILD_TOOLS.map(tool => [ + tool, + MARKERS_BY_TOOL[tool].map(marker => + micromatch.matcher(marker, { dot: true }), + ), + ]), +) as Record boolean>> + +// `entryFile` (a basename in `dir`) is set only for a tool that addresses its +// builds by file, where one directory can hold several builds. +export type BuildRoot = { dir: string; entryFile?: string | undefined } + +export function buildRootPath(root: BuildRoot): string { + return path.join(root.dir, root.entryFile ?? '') +} + +function markerTool( + tools: readonly BuildTool[], + basename: string, +): BuildTool | undefined { + return tools.find(tool => + MARKER_MATCHERS_BY_TOOL[tool].some(matches => matches(basename)), + ) } // A disabled ecosystem is skipped entirely by the glob scan below, so a @@ -85,27 +117,39 @@ export async function addCoveredBuildFiles( } } -// `dir` must be realpath-resolved, as findBuildToolCandidates returns it. +// `root.dir` must be realpath-resolved, as findBuildToolCandidates returns it. export function isBuildRootCovered( covered: ReadonlySet, tool: BuildTool, - dir: string, + root: BuildRoot, ): boolean { + const entryFile = root.entryFile ?? defaultEntryFile(tool) + if (entryFile) { + return covered.has(path.join(root.dir, entryFile)) + } return MARKERS_BY_TOOL[tool].some(marker => - covered.has(path.join(dir, marker)), + covered.has(path.join(root.dir, marker)), ) } -function sortByDepthThenPath(dirs: readonly string[], cwd: string): string[] { - return [...dirs].sort((a, b) => { - const relA = path.relative(cwd, a) - const relB = path.relative(cwd, b) +function sortByDepthThenPath( + roots: readonly BuildRoot[], + cwd: string, +): BuildRoot[] { + return [...roots].sort((a, b) => { + const relA = path.relative(cwd, a.dir) + const relB = path.relative(cwd, b.dir) const depthA = relA === '' ? 0 : relA.split(path.sep).length const depthB = relB === '' ? 0 : relB.split(path.sep).length if (depthA !== depthB) { return depthA - depthB } - return relA < relB ? -1 : relA > relB ? 1 : 0 + if (relA !== relB) { + return relA < relB ? -1 : 1 + } + const entryA = a.entryFile ?? '' + const entryB = b.entryFile ?? '' + return entryA < entryB ? -1 : entryA > entryB ? 1 : 0 }) } @@ -119,11 +163,11 @@ export async function findBuildToolCandidates({ cwd: string excludePaths?: string[] | undefined sockJson: SocketJson -}): Promise> { +}): Promise> { const enabledTools = BUILD_TOOLS.filter( tool => !sockJson.defaults?.manifest?.[tool]?.disabled, ) - const result = new Map() + const result = new Map() if (!enabledTools.length) { return result } @@ -134,32 +178,48 @@ export async function findBuildToolCandidates({ const additionalIgnores = (excludePaths ?? []).flatMap( excludePathToScanIgnores, ) + // The same socket.yml a scan applies to its upload, so a build file a scan + // would not upload is not built either. + const socketYml = findSocketYmlSync(cwd) const hits = await globWithGitIgnore(patterns, { absolute: true, additionalIgnores, cwd, + socketConfig: socketYml.ok ? socketYml.data?.parsed : undefined, }) - const dirSetsByTool = new Map>( - enabledTools.map(tool => [tool, new Set()]), + const rootsByTool = new Map>( + enabledTools.map(tool => [tool, new Map()]), ) const resolvedHits = await Promise.all( - hits.map(async hit => ({ - dir: await realpathOrResolved(path.dirname(hit)), - tool: TOOL_BY_MARKER[path.basename(hit)], - })), + hits.map(async hit => { + const basename = path.basename(hit) + return { + basename, + dir: await realpathOrResolved(path.dirname(hit)), + tool: markerTool(enabledTools, basename), + } + }), ) - for (const { dir, tool } of resolvedHits) { - if (tool) { - dirSetsByTool.get(tool)?.add(dir) + for (const { basename, dir, tool } of resolvedHits) { + if (!tool) { + continue + } + const root = { + dir, + entryFile: isAddressedByFile(tool) ? basename : undefined, } + rootsByTool.get(tool)?.set(buildRootPath(root), root) } const realCwd = await realpathOrResolved(cwd) for (const tool of enabledTools) { result.set( tool, - sortByDepthThenPath([...(dirSetsByTool.get(tool) ?? [])], realCwd), + sortByDepthThenPath( + [...(rootsByTool.get(tool)?.values() ?? [])], + realCwd, + ), ) } return result diff --git a/src/commands/manifest/discover-manifest-roots.test.mts b/src/commands/manifest/discover-manifest-roots.test.mts index 206e988d2..97262d8b4 100644 --- a/src/commands/manifest/discover-manifest-roots.test.mts +++ b/src/commands/manifest/discover-manifest-roots.test.mts @@ -1,19 +1,48 @@ +import { promises as fs } from 'node:fs' +import { tmpdir } from 'node:os' import path from 'node:path' import { describe, expect, it } from 'vitest' -import { findBuildToolCandidates } from './discover-manifest-roots.mts' +import { + findBuildToolCandidates, + isBuildRootCovered, +} from './discover-manifest-roots.mts' import { testPath } from '../../../test/utils.mts' +import type { BuildRoot } from './discover-manifest-roots.mts' import type { SocketJson } from '../../utils/socket-json.mts' const monorepo = path.join( testPath, 'fixtures/commands/manifest/dynamic-sbom-inference/monorepo', ) +const pomNames = path.join( + testPath, + 'fixtures/commands/manifest/dynamic-sbom-inference/pom-names', +) -function relDirs(dirs: string[]): string[] { - return dirs.map(d => path.relative(monorepo, d).replaceAll('\\', '/')).sort() +function relDirs(roots: BuildRoot[]): string[] { + return roots + .map(r => path.relative(monorepo, r.dir).replaceAll('\\', '/')) + .sort() +} + +async function pomNamesRoots(excludePaths?: string[]): Promise { + const candidates = await findBuildToolCandidates({ + cwd: pomNames, + excludePaths, + sockJson: {} as SocketJson, + }) + return (candidates.get('maven') ?? []).map(r => + path + .relative(pomNames, path.join(r.dir, r.entryFile ?? '')) + .replaceAll('\\', '/'), + ) +} + +function dirsOf(roots: BuildRoot[] | undefined): string[] { + return (roots ?? []).map(r => r.dir) } describe('findBuildToolCandidates', () => { @@ -47,7 +76,7 @@ describe('findBuildToolCandidates', () => { cwd: monorepo, sockJson: {} as SocketJson, }) - const maven = candidates.get('maven') ?? [] + const maven = dirsOf(candidates.get('maven')) const rootIndex = maven.findIndex(d => d === path.join(monorepo, 'reactor')) const moduleAIndex = maven.findIndex( d => d === path.join(monorepo, 'reactor/moduleA'), @@ -61,7 +90,7 @@ describe('findBuildToolCandidates', () => { cwd: monorepo, sockJson: {} as SocketJson, }) - expect(candidates.get('gradle')).toContain( + expect(dirsOf(candidates.get('gradle'))).toContain( path.join(monorepo, 'reactor/moduleA/nested-gradle'), ) }) @@ -72,8 +101,8 @@ describe('findBuildToolCandidates', () => { sockJson: {} as SocketJson, }) const dualDir = path.join(monorepo, 'dual-marker-dir') - expect(candidates.get('maven')).toContain(dualDir) - expect(candidates.get('gradle')).toContain(dualDir) + expect(dirsOf(candidates.get('maven'))).toContain(dualDir) + expect(dirsOf(candidates.get('gradle'))).toContain(dualDir) }) it('drops a disabled ecosystem entirely', async () => { @@ -94,11 +123,137 @@ describe('findBuildToolCandidates', () => { sockJson: {} as SocketJson, }) expect(candidates.get('maven')).toEqual([ - path.join(monorepo, 'dual-marker-dir'), + { dir: path.join(monorepo, 'dual-marker-dir'), entryFile: 'pom.xml' }, ]) expect(candidates.get('gradle')).toEqual([ - path.join(monorepo, 'dual-marker-dir'), - path.join(monorepo, 'standalone-gradle'), + { dir: path.join(monorepo, 'dual-marker-dir'), entryFile: undefined }, + { dir: path.join(monorepo, 'standalone-gradle'), entryFile: undefined }, + ]) + }) + + it('lists a gradle directory once despite several markers', async () => { + const candidates = await findBuildToolCandidates({ + cwd: monorepo, + sockJson: {} as SocketJson, + }) + const standalone = path.join(monorepo, 'standalone-gradle') + expect( + dirsOf(candidates.get('gradle')).filter(d => d === standalone), + ).toHaveLength(1) + }) + + it('lists each POM the scan pattern uploads as its own maven build root', async () => { + const roots = await pomNamesRoots() + expect(roots).toEqual([ + 'app-dev.pom.xml', + 'pom.xml', + 'bare/pom', + 'upper/POM.XML', + 'vendored/lib-1.0.pom', + ]) + }) + + it('skips POMs a .gitignore covers, as a scan does when uploading', async () => { + const cwd = await fs.realpath( + await fs.mkdtemp(path.join(tmpdir(), 'gitignored-poms-')), + ) + try { + const repoDir = path.join(cwd, '.m2/repository/com/example/lib/1.0') + await fs.mkdir(repoDir, { recursive: true }) + await fs.writeFile(path.join(repoDir, 'lib-1.0.pom'), '') + await fs.writeFile(path.join(cwd, 'pom.xml'), '') + await fs.writeFile(path.join(cwd, '.gitignore'), '.m2/\n') + + const candidates = await findBuildToolCandidates({ + cwd, + sockJson: {} as SocketJson, + }) + + expect(candidates.get('maven')).toEqual([ + { dir: cwd, entryFile: 'pom.xml' }, + ]) + } finally { + await fs.rm(cwd, { recursive: true, force: true }) + } + }) + + it("skips POMs socket.yml's projectIgnorePaths covers, as a scan does when uploading", async () => { + const cwd = await fs.realpath( + await fs.mkdtemp(path.join(tmpdir(), 'project-ignored-poms-')), + ) + try { + await fs.mkdir(path.join(cwd, 'libs/repo'), { recursive: true }) + await fs.writeFile(path.join(cwd, 'libs/repo/lib-1.0.pom'), '') + await fs.writeFile(path.join(cwd, 'pom.xml'), '') + await fs.writeFile( + path.join(cwd, 'socket.yml'), + 'version: 2\nprojectIgnorePaths:\n - libs/repo\n', + ) + + const candidates = await findBuildToolCandidates({ + cwd, + sockJson: {} as SocketJson, + }) + + expect(candidates.get('maven')).toEqual([ + { dir: cwd, entryFile: 'pom.xml' }, + ]) + } finally { + await fs.rm(cwd, { recursive: true, force: true }) + } + }) + + it('respects --exclude-paths for non-default POMs, case-sensitively', async () => { + expect( + await pomNamesRoots(['vendored', '**/*.pom', 'app-dev.pom.xml']), + ).toEqual(['pom.xml', 'bare/pom', 'upper/POM.XML']) + expect(await pomNamesRoots(['UPPER', 'Bare'])).toEqual([ + 'app-dev.pom.xml', + 'pom.xml', + 'bare/pom', + 'upper/POM.XML', + 'vendored/lib-1.0.pom', ]) }) }) + +describe('isBuildRootCovered', () => { + it('covers a POM only by its own file', () => { + const covered = new Set(['/repo/service/pom.xml']) + expect( + isBuildRootCovered(covered, 'maven', { + dir: '/repo/service', + entryFile: 'pom.xml', + }), + ).toBe(true) + expect( + isBuildRootCovered(covered, 'maven', { + dir: '/repo/service', + entryFile: 'app-dev.pom.xml', + }), + ).toBe(false) + }) + + it('covers a root without an entry file through its default entry file or a marker file', () => { + expect( + isBuildRootCovered(new Set(['/repo/service/pom.xml']), 'maven', { + dir: '/repo/service', + }), + ).toBe(true) + expect( + isBuildRootCovered(new Set(['/repo/service/lib-1.0.pom']), 'maven', { + dir: '/repo/service', + }), + ).toBe(false) + expect( + isBuildRootCovered(new Set(['/repo/lib/settings.gradle.kts']), 'gradle', { + dir: '/repo/lib', + }), + ).toBe(true) + expect( + isBuildRootCovered(new Set(['/repo/lib/sub/build.gradle']), 'gradle', { + dir: '/repo/lib', + }), + ).toBe(false) + }) +}) diff --git a/src/commands/manifest/generate-recursive-manifests.mts b/src/commands/manifest/generate-recursive-manifests.mts index 3a0061f76..fcfafa31d 100644 --- a/src/commands/manifest/generate-recursive-manifests.mts +++ b/src/commands/manifest/generate-recursive-manifests.mts @@ -4,6 +4,7 @@ import { logger } from '@socketsecurity/registry/lib/logger' import { addCoveredBuildFiles, + buildRootPath, findBuildToolCandidates, isBuildRootCovered, realpathOrResolved, @@ -11,7 +12,10 @@ import { } from './discover-manifest-roots.mts' import { parseBuildToolOpts } from './parse-build-tool-opts.mts' import { runManifestFacts } from './run-manifest-facts.mts' -import { resolveBuildToolBin } from './scripts/build-tool.mts' +import { + resolveBuildToolBin, + withEntryFileOpts, +} from './scripts/build-tool.mts' import { DSI_SETUP_COMMAND, REACH_CONTINUE_ON_INSTALL_ERRORS_FLAG, @@ -24,6 +28,7 @@ import { } from '../../utils/socket-json.mts' import { projectIgnorePathsToReachExcludePaths } from '../scan/exclude-paths.mts' +import type { BuildRoot } from './discover-manifest-roots.mts' import type { BuildTool } from './scripts/build-tool.mts' import type { BuildOption } from './scripts/resolution-report-render.mts' import type { SidecarAccumulator } from './scripts/sidecar.mts' @@ -37,8 +42,7 @@ export type RecursiveManifestOutcomeStatus = | 'skippedCovered' | 'skippedDisabled' -export type RecursiveManifestOutcome = { - dir: string +export type RecursiveManifestOutcome = BuildRoot & { ecosystem: BuildTool factsPath?: string | undefined status: RecursiveManifestOutcomeStatus @@ -82,7 +86,7 @@ function getSkipReason( type DisabledRoot = { dir: string; sockJson: SocketJson } -// Nearest already-confirmed-disabled ancestor of `dir`, if any - lets the +// Nearest already-confirmed-disabled `dir` or ancestor of it, if any - lets the // caller shorten `readSocketJsonCascade`'s walk to start there instead of // all the way back at `cwd`, without skipping any nested override. function nearestDisabledRoot( @@ -92,7 +96,7 @@ function nearestDisabledRoot( let nearest: DisabledRoot | undefined for (const root of disabledRoots) { if ( - dir.startsWith(`${root.dir}${path.sep}`) && + (dir === root.dir || dir.startsWith(`${root.dir}${path.sep}`)) && (!nearest || root.dir.length > nearest.dir.length) ) { nearest = root @@ -113,7 +117,7 @@ async function runEcosystemCandidates({ verbose, withFiles, }: { - candidatesByTool: Map + candidatesByTool: Map excludePaths: string[] | undefined ignoreUnresolved: boolean | undefined optionFlags: ReadonlySet @@ -130,12 +134,13 @@ async function runEcosystemCandidates({ }): Promise { const outcomes: RecursiveManifestOutcome[] = [] const covered = new Set() - for (const [ecosystem, dirs] of candidatesByTool) { + for (const [ecosystem, roots] of candidatesByTool) { const disabledRoots: DisabledRoot[] = [] - for (let dirIndex = 0; dirIndex < dirs.length; dirIndex += 1) { - const dir = dirs[dirIndex] as string - if (isBuildRootCovered(covered, ecosystem, dir)) { - outcomes.push({ dir, ecosystem, status: 'skippedCovered' }) + for (let rootIndex = 0; rootIndex < roots.length; rootIndex += 1) { + const root = roots[rootIndex] as BuildRoot + const { dir, entryFile } = root + if (isBuildRootCovered(covered, ecosystem, root)) { + outcomes.push({ ...root, ecosystem, status: 'skippedCovered' }) continue } @@ -163,7 +168,7 @@ async function runEcosystemCandidates({ if (!nearestRoot) { logger.warn(`Skipping ${dir} (${ecosystem}): ${skipReason}.`) } - outcomes.push({ dir, ecosystem, status: 'skippedDisabled' }) + outcomes.push({ ...root, ecosystem, status: 'skippedDisabled' }) disabledRoots.push({ dir, sockJson }) continue } @@ -176,7 +181,7 @@ async function runEcosystemCandidates({ // eslint-disable-next-line no-await-in-loop const result = await runManifestFacts({ bin, - buildOpts, + buildOpts: withEntryFileOpts(ecosystem, entryFile, buildOpts), cwd: dir, ecosystem, excludeConfigs, @@ -199,20 +204,20 @@ async function runEcosystemCandidates({ }) if (result === null) { - outcomes.push({ dir, ecosystem, status: 'failed' }) - const abortedDirs = dirs.slice(dirIndex + 1) - if (abortedDirs.length) { + outcomes.push({ ...root, ecosystem, status: 'failed' }) + const abortedRoots = roots.slice(rootIndex + 1) + if (abortedRoots.length) { logger.warn( - `Aborting ${ecosystem} discovery: ${dir} failed to generate Socket facts, and without them the remaining build roots cannot be safely classified as covered or independent.`, + `Aborting ${ecosystem} discovery: ${buildRootPath(root)} failed to generate Socket facts, and without them the remaining build roots cannot be safely classified as covered or independent.`, ) } - for (const abortedDir of abortedDirs) { - outcomes.push({ dir: abortedDir, ecosystem, status: 'aborted' }) + for (const aborted of abortedRoots) { + outcomes.push({ ...aborted, ecosystem, status: 'aborted' }) } break } if (!result) { - outcomes.push({ dir, ecosystem, status: 'empty' }) + outcomes.push({ ...root, ecosystem, status: 'empty' }) continue } @@ -224,7 +229,7 @@ async function runEcosystemCandidates({ result.projects, ) outcomes.push({ - dir, + ...root, ecosystem, factsPath: result.factsPath, status: 'generated', diff --git a/src/commands/manifest/generate-recursive-manifests.test.mts b/src/commands/manifest/generate-recursive-manifests.test.mts index 5bbfd45d7..586c637d3 100644 --- a/src/commands/manifest/generate-recursive-manifests.test.mts +++ b/src/commands/manifest/generate-recursive-manifests.test.mts @@ -30,6 +30,10 @@ const monorepo = path.join( testPath, 'fixtures/commands/manifest/dynamic-sbom-inference/monorepo', ) +const pomNames = path.join( + testPath, + 'fixtures/commands/manifest/dynamic-sbom-inference/pom-names', +) const reactor = path.join(monorepo, 'reactor') const dualMarkerDir = path.join(monorepo, 'dual-marker-dir') @@ -1028,4 +1032,72 @@ describe('generateRecursiveManifests', () => { expect(opts.withFiles).toBe(true) } }) + + it('runs every POM in a directory as its own build, pointing maven at any non-default one', async () => { + vi.mocked(runManifestFacts).mockImplementation( + async ({ buildOpts, cwd }) => ({ + factsPath: path.join( + cwd, + `${buildOpts[1] ?? 'pom.xml'}.socket.facts.json`, + ), + projects: [], + }), + ) + + const outcomes = await generateRecursiveManifests({ + cwd: pomNames, + verbose: false, + }) + + const calls = vi + .mocked(runManifestFacts) + .mock.calls.map(([opts]) => [ + path.relative(pomNames, opts.cwd).replaceAll('\\', '/'), + opts.buildOpts, + ]) + expect(calls).toEqual([ + ['', ['--file', 'app-dev.pom.xml']], + ['', []], + ['bare', ['--file', 'pom']], + ['upper', ['--file', 'POM.XML']], + ['vendored', ['--file', 'lib-1.0.pom']], + ]) + expect(outcomes.every(o => o.status === 'generated')).toBe(true) + }) + + it('covers a POM by file, so a sibling POM in the same directory still runs', async () => { + vi.mocked(runManifestFacts).mockImplementation( + async ({ buildOpts, cwd }) => ({ + factsPath: path.join( + cwd, + `${buildOpts[1] ?? 'pom.xml'}.socket.facts.json`, + ), + projects: + cwd === pomNames && !buildOpts.length + ? [ + { + type: 'maven', + name: 'bare', + subprojectDir: 'bare', + manifestFiles: [{ file: 'bare/pom' }], + children: [], + }, + ] + : [], + }), + ) + + const outcomes = await generateRecursiveManifests({ + cwd: pomNames, + verbose: false, + }) + + const statusOf = (rel: string) => + outcomes.find( + o => path.join(o.dir, o.entryFile ?? '') === path.join(pomNames, rel), + )?.status + expect(statusOf('pom.xml')).toBe('generated') + expect(statusOf('app-dev.pom.xml')).toBe('generated') + expect(statusOf('bare/pom')).toBe('skippedCovered') + }) }) diff --git a/src/commands/manifest/output-manifest-dynamic-sbom-inference.mts b/src/commands/manifest/output-manifest-dynamic-sbom-inference.mts index 3112b7668..dcdfde724 100644 --- a/src/commands/manifest/output-manifest-dynamic-sbom-inference.mts +++ b/src/commands/manifest/output-manifest-dynamic-sbom-inference.mts @@ -1,5 +1,6 @@ import { logger } from '@socketsecurity/registry/lib/logger' +import { buildRootPath } from './discover-manifest-roots.mts' import { failMsgWithBadge } from '../../utils/fail-msg-with-badge.mts' import { serializeResultJson } from '../../utils/serialize-result-json.mts' @@ -12,7 +13,7 @@ function renderTable(outcomes: readonly RecursiveManifestOutcome[]): string { .filter(o => o.status !== 'skippedCovered') .map( o => - `- ${o.dir} (${o.ecosystem}): ${o.status}${o.factsPath ? ` -> ${o.factsPath}` : ''}`, + `- ${buildRootPath(o)} (${o.ecosystem}): ${o.status}${o.factsPath ? ` -> ${o.factsPath}` : ''}`, ) .join('\n') } diff --git a/src/commands/manifest/scripts/build-tool.mts b/src/commands/manifest/scripts/build-tool.mts index 53c6268f0..3b0367e36 100644 --- a/src/commands/manifest/scripts/build-tool.mts +++ b/src/commands/manifest/scripts/build-tool.mts @@ -30,6 +30,19 @@ const ADDRESSED_BY_FILE: Record = { sbt: false, } as unknown as Record +const DEFAULT_ENTRY_FILE: Partial> = { + __proto__: null, + maven: 'pom.xml', +} as unknown as Partial> + +export function defaultEntryFile(tool: BuildTool): string | undefined { + return DEFAULT_ENTRY_FILE[tool] +} + +export function isAddressedByFile(tool: BuildTool): boolean { + return ADDRESSED_BY_FILE[tool] +} + // sbt happily runs in any directory, synthesizing a default project from its // name, so an sbt run outside a build yields a plausible but bogus SBOM. Maven // and Gradle refuse such a directory themselves. @@ -40,6 +53,20 @@ export function looksLikeSbtBuild(projectDir: string): boolean { ) } +// Leads, as Maven 3 takes the first `--file`; nothing for the default entry +// file, so options a user passes keep their meaning. +export function withEntryFileOpts( + tool: BuildTool, + entryFile: string | undefined, + buildOpts: string[], +): string[] { + const defaultEntry = DEFAULT_ENTRY_FILE[tool] + if (!entryFile || !defaultEntry || entryFile === defaultEntry) { + return buildOpts + } + return ['--file', entryFile, ...buildOpts] +} + export function resolveBuildToolBin( tool: BuildTool, projectDir: string, diff --git a/src/commands/manifest/scripts/build-tool.test.mts b/src/commands/manifest/scripts/build-tool.test.mts index 7e8fb7a0f..37c88fce6 100644 --- a/src/commands/manifest/scripts/build-tool.test.mts +++ b/src/commands/manifest/scripts/build-tool.test.mts @@ -1,8 +1,22 @@ import { describe, expect, it } from 'vitest' -import { socketFactsFileName } from './build-tool.mts' +import { socketFactsFileName, withEntryFileOpts } from './build-tool.mts' import { parseRecords } from './records.mts' +describe('withEntryFileOpts', () => { + it('points a file-addressed build at a non-default entry file, ahead of the given options', () => { + expect(withEntryFileOpts('maven', 'app-dev.pom.xml', ['-P', 'ci'])).toEqual( + ['--file', 'app-dev.pom.xml', '-P', 'ci'], + ) + }) + + it('adds nothing for the default entry file or a directory-addressed build', () => { + expect(withEntryFileOpts('maven', 'pom.xml', ['-q'])).toEqual(['-q']) + expect(withEntryFileOpts('maven', undefined, [])).toEqual([]) + expect(withEntryFileOpts('gradle', 'build.gradle', [])).toEqual([]) + }) +}) + describe('socketFactsFileName', () => { it('names a directory-addressed build after its tool', () => { expect(socketFactsFileName('gradle', undefined)).toBe( diff --git a/src/commands/manifest/setup-recursive-manifest-config.mts b/src/commands/manifest/setup-recursive-manifest-config.mts index 28f7a12cb..9526a489f 100644 --- a/src/commands/manifest/setup-recursive-manifest-config.mts +++ b/src/commands/manifest/setup-recursive-manifest-config.mts @@ -8,6 +8,7 @@ import { select } from '@socketsecurity/registry/lib/prompts' import { addCoveredBuildFiles, + buildRootPath, findBuildToolCandidates, isBuildRootCovered, realpathOrResolved, @@ -15,6 +16,7 @@ import { } from './discover-manifest-roots.mts' import { enumerateWorkspaces } from './enumerate-workspaces.mts' import { resolveEcosystemConfig } from './generate-recursive-manifests.mts' +import { withEntryFileOpts } from './scripts/build-tool.mts' import { setupGradle, setupMaven, setupSbt } from './setup-manifest-config.mts' import { SOCKET_JSON } from '../../constants.mts' import { @@ -28,13 +30,14 @@ import { projectIgnorePathsToReachExcludePaths, } from '../scan/exclude-paths.mts' +import type { BuildRoot } from './discover-manifest-roots.mts' import type { BuildTool } from './scripts/build-tool.mts' import type { CResult } from '../../types.mts' import type { SocketJson } from '../../utils/socket-json.mts' -// A single discovered build root - a directory with its own gradle/sbt/maven -// marker file. -type Candidate = { dir: string; ecosystem: BuildTool } +// A single discovered build root - a directory with its own gradle/sbt marker +// file, or one Maven POM. +type Candidate = BuildRoot & { ecosystem: BuildTool } const ROOT_ECOSYSTEMS: BuildTool[] = ['maven', 'gradle', 'sbt'] @@ -113,8 +116,8 @@ function findExclusionRoot( } export type ScannedBuildRoots = { - fullByTool: Map - includedByTool: Map + fullByTool: Map + includedByTool: Map } // Both the unfiltered and --exclude-paths-filtered scans, once, so ecosystem @@ -156,21 +159,32 @@ export async function discoverBuildRoots({ }: { cwd: string excludePaths?: string[] | undefined - fullByTool: Map - includedByTool: Map + fullByTool: Map + includedByTool: Map }): Promise { const realCwd = await realpathOrResolved(cwd) const included: Candidate[] = [] const ignorePatterns = (excludePaths ?? []).flatMap(excludePathToScanIgnores) const ecosystemsByRoot = new Map>() - for (const [ecosystem, fullDirs] of fullByTool) { - const includedDirs = new Set(includedByTool.get(ecosystem) ?? []) - for (const dir of fullDirs) { + for (const [ecosystem, fullRoots] of fullByTool) { + const includedRoots = includedByTool.get(ecosystem) ?? [] + const includedPaths = new Set(includedRoots.map(buildRootPath)) + const includedDirs = new Set(includedRoots.map(root => root.dir)) + for (const root of fullRoots) { + const { dir } = root if (dir === realCwd) { continue } + if (includedPaths.has(buildRootPath(root))) { + included.push({ ...root, ecosystem }) + continue + } + // `disabled` is per directory, so it cannot exclude one build file + // without its included siblings. if (includedDirs.has(dir)) { - included.push({ dir, ecosystem }) + logger.warn( + `Not disabling ${ecosystem} in ${toPosixRelative(realCwd, dir)}: --exclude-paths matches ${buildRootPath(root)} but not the other build files there. Pass the same --exclude-paths to the scan instead.`, + ) continue } const relDir = toPosixRelative(realCwd, dir) @@ -223,7 +237,11 @@ export async function markWorkspaceCoverage({ const enumResult = await enumerateWorkspaces({ bin, - buildOpts, + buildOpts: withEntryFileOpts( + candidate.ecosystem, + candidate.entryFile, + buildOpts, + ), cwd: candidate.dir, ecosystem: candidate.ecosystem, excludePaths: excludePathsForCandidate, @@ -231,10 +249,10 @@ export async function markWorkspaceCoverage({ verbose: false, }) if (!enumResult) { - const relDir = path.relative(cwd, candidate.dir) || '.' + const relRoot = path.relative(cwd, buildRootPath(candidate)) || '.' return { ok: false, - message: `Could not determine ${candidate.ecosystem} workspace layout for ${relDir}; aborting rather than risk misclassifying its members.`, + message: `Could not determine ${candidate.ecosystem} workspace layout for ${relRoot}; aborting rather than risk misclassifying its members.`, } } @@ -744,7 +762,7 @@ export async function setupRecursiveManifestConfig( // Which detected ecosystems cwd is itself a build root for - see // setupRecursiveRootDefaults' wording split. const ecosystemsAtCwd = ROOT_ECOSYSTEMS.filter(ecosystem => - (fullByTool.get(ecosystem) ?? []).includes(realCwd), + (fullByTool.get(ecosystem) ?? []).some(root => root.dir === realCwd), ) const rootResult = await setupRecursiveRootDefaults( @@ -821,10 +839,15 @@ export async function setupRecursiveManifestConfig( // cwd never appears in `included`, but a reactor root often sits exactly // at cwd - seed coverage from it first so its declared members still get // recognized. - for (const ecosystem of ecosystemsAtCwd) { + const includedAtCwd = ROOT_ECOSYSTEMS.flatMap(ecosystem => + (includedByTool.get(ecosystem) ?? []) + .filter(root => root.dir === realCwd) + .map(root => ({ ...root, ecosystem })), + ) + for (const candidate of includedAtCwd) { // eslint-disable-next-line no-await-in-loop const coverageResult = await markWorkspaceCoverage({ - candidate: { dir: realCwd, ecosystem }, + candidate, covered, cwd: realCwd, excludePaths, @@ -842,30 +865,42 @@ export async function setupRecursiveManifestConfig( reenabled: 0, skipped: 0, } + // Settings are per directory, so a directory holding several POMs is asked + // about once and its answer counted for each. + const answersByDir = new Map< + string, + { outcome: CandidateOutcome; reenabled: boolean } + >() const orderedIncluded = sortCandidatesForDisplay(included, realCwd) for (const candidate of orderedIncluded) { - if (isBuildRootCovered(covered, candidate.ecosystem, candidate.dir)) { + if (isBuildRootCovered(covered, candidate.ecosystem, candidate)) { counts.covered += 1 continue } - // eslint-disable-next-line no-await-in-loop - const result = await processCandidate({ - cwd: realCwd, - dir: candidate.dir, - ecosystem: candidate.ecosystem, - rootSockJson, - }) - if (!result.ok) { - return result - } - if (result.data.canceled) { - // The cancellation itself (select Esc/Ctrl+C, or a sub-wizard's own - // cancel) already logged "User canceled" - don't log it twice. - return { ok: true, data: { canceled: true } } + const promptKey = `${candidate.ecosystem}:${candidate.dir}` + let answer = answersByDir.get(promptKey) + if (!answer) { + // eslint-disable-next-line no-await-in-loop + const result = await processCandidate({ + cwd: realCwd, + dir: candidate.dir, + ecosystem: candidate.ecosystem, + rootSockJson, + }) + if (!result.ok) { + return result + } + if (result.data.canceled) { + // The cancellation itself (select Esc/Ctrl+C, or a sub-wizard's own + // cancel) already logged "User canceled" - don't log it twice. + return { ok: true, data: { canceled: true } } + } + answer = result.data + answersByDir.set(promptKey, answer) } - counts[result.data.outcome] += 1 - if (result.data.reenabled) { + counts[answer.outcome] += 1 + if (answer.reenabled) { // A qualifier on the 'configured'/'inherited' bucket just incremented // above, not a separate bucket - avoids double-counting the candidate. counts.reenabled += 1 diff --git a/src/commands/manifest/setup-recursive-manifest-config.test.mts b/src/commands/manifest/setup-recursive-manifest-config.test.mts index 6c1c32f56..ddc3f8756 100644 --- a/src/commands/manifest/setup-recursive-manifest-config.test.mts +++ b/src/commands/manifest/setup-recursive-manifest-config.test.mts @@ -63,8 +63,17 @@ import { writeSocketJson, } from '../../utils/socket-json.mts' +import type { BuildRoot } from './discover-manifest-roots.mts' import type { SocketJson } from '../../utils/socket-json.mts' +function dirRoots(...dirs: string[]): BuildRoot[] { + return dirs.map(dir => ({ dir })) +} + +function mavenRoots(...dirs: string[]): BuildRoot[] { + return dirs.map(dir => ({ dir, entryFile: 'pom.xml' })) +} + function emptySockJson(): SocketJson { return { version: 1 } as SocketJson } @@ -147,8 +156,10 @@ describe('scanBuildRoots', () => { }) it('runs the unfiltered and excludePaths-filtered walks once each and returns both', async () => { - const unfiltered = new Map([['gradle', ['/repo/a', '/repo/legacy']]]) - const filtered = new Map([['gradle', ['/repo/a']]]) + const unfiltered = new Map([ + ['gradle', dirRoots('/repo/a', '/repo/legacy')], + ]) + const filtered = new Map([['gradle', dirRoots('/repo/a')]]) vi.mocked(findBuildToolCandidates).mockImplementation( async ({ excludePaths }) => excludePaths?.length ? filtered : unfiltered, @@ -174,8 +185,8 @@ describe('discoverBuildRoots', () => { it('excludes cwd itself', async () => { const result = await discoverBuildRoots({ cwd, - fullByTool: new Map([['gradle', [cwd]]]), - includedByTool: new Map([['gradle', [cwd]]]), + fullByTool: new Map([['gradle', dirRoots(cwd)]]), + includedByTool: new Map([['gradle', dirRoots(cwd)]]), }) expect(result).toEqual({ excluded: [], included: [] }) @@ -188,8 +199,8 @@ describe('discoverBuildRoots', () => { const result = await discoverBuildRoots({ cwd, excludePaths: ['legacy'], - fullByTool: new Map([['gradle', [legacy, active]]]), - includedByTool: new Map([['gradle', [active]]]), + fullByTool: new Map([['gradle', dirRoots(legacy, active)]]), + includedByTool: new Map([['gradle', dirRoots(active)]]), }) expect(result).toEqual({ @@ -210,12 +221,12 @@ describe('discoverBuildRoots', () => { cwd, excludePaths: ['legacy'], fullByTool: new Map([ - ['maven', [a]], - ['gradle', [b]], + ['maven', mavenRoots(a)], + ['gradle', dirRoots(b)], ]), includedByTool: new Map([ - ['maven', []], - ['gradle', []], + ['maven', mavenRoots()], + ['gradle', dirRoots()], ]), }) @@ -224,6 +235,38 @@ describe('discoverBuildRoots', () => { included: [], }) }) + + it('does not disable a directory to exclude one POM while a sibling POM there stays included', async () => { + const service = '/repo/service' + const warnSpy = vi.spyOn(logger, 'warn').mockImplementation(() => logger) + + try { + const result = await discoverBuildRoots({ + cwd, + excludePaths: ['service/app-dev.pom.xml'], + fullByTool: new Map([ + [ + 'maven', + [ + { dir: service, entryFile: 'app-dev.pom.xml' }, + { dir: service, entryFile: 'pom.xml' }, + ], + ], + ]), + includedByTool: new Map([['maven', mavenRoots(service)]]), + }) + + expect(result).toEqual({ + excluded: [], + included: [{ dir: service, ecosystem: 'maven', entryFile: 'pom.xml' }], + }) + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('Not disabling maven in service'), + ) + } finally { + warnSpy.mockRestore() + } + }) }) describe('markWorkspaceCoverage', () => { @@ -839,7 +882,7 @@ describe('setupRecursiveManifestConfig', () => { async ({ excludePaths }) => excludePaths?.length ? new Map() - : new Map([['maven', [`${cwd}/service`]]]), + : new Map([['maven', mavenRoots(`${cwd}/service`)]]), ) const result = await setupRecursiveManifestConfig(cwd, false, ['service']) @@ -851,7 +894,7 @@ describe('setupRecursiveManifestConfig', () => { // Maven is detected (a candidate exists, elsewhere in the tree); gradle // and sbt have none anywhere, so neither should ever be asked about. vi.mocked(findBuildToolCandidates).mockResolvedValue( - new Map([['maven', [`${cwd}/service`]]]), + new Map([['maven', mavenRoots(`${cwd}/service`)]]), ) const messages: string[] = [] vi.mocked(select).mockImplementation(async ({ message }) => { @@ -880,8 +923,8 @@ describe('setupRecursiveManifestConfig', () => { // an inheritable default for something else. vi.mocked(findBuildToolCandidates).mockResolvedValue( new Map([ - ['maven', [cwd]], - ['gradle', [`${cwd}/module-b/standalone-gradle-lib`]], + ['maven', mavenRoots(cwd)], + ['gradle', dirRoots(`${cwd}/module-b/standalone-gradle-lib`)], ]), ) const messages: string[] = [] @@ -911,7 +954,7 @@ describe('setupRecursiveManifestConfig', () => { vi.mocked(findBuildToolCandidates).mockImplementation( async ({ sockJson }) => { seenSockJsons.push(sockJson) - return new Map([['maven', [cwd]]]) + return new Map([['maven', mavenRoots(cwd)]]) }, ) @@ -925,7 +968,7 @@ describe('setupRecursiveManifestConfig', () => { it('offers to re-enable an ecosystem disabled at the root, before asking to configure it', async () => { vi.mocked(findBuildToolCandidates).mockResolvedValue( - new Map([['maven', [cwd]]]), + new Map([['maven', mavenRoots(cwd)]]), ) vi.mocked(readSocketJsonSync).mockImplementation(() => ({ ok: true, @@ -960,7 +1003,7 @@ describe('setupRecursiveManifestConfig', () => { it('declining the root re-enable question leaves the ecosystem disabled and skips configuring it', async () => { vi.mocked(findBuildToolCandidates).mockResolvedValue( - new Map([['maven', [cwd]]]), + new Map([['maven', mavenRoots(cwd)]]), ) vi.mocked(readSocketJsonSync).mockImplementation(() => ({ ok: true, @@ -980,7 +1023,7 @@ describe('setupRecursiveManifestConfig', () => { it('stops when canceling the only detected root ecosystem question', async () => { vi.mocked(findBuildToolCandidates).mockResolvedValue( - new Map([['maven', [cwd]]]), + new Map([['maven', mavenRoots(cwd)]]), ) vi.mocked(select).mockResolvedValueOnce(null) @@ -991,7 +1034,7 @@ describe('setupRecursiveManifestConfig', () => { it('stops when a root ecosystem sub-wizard is canceled', async () => { vi.mocked(findBuildToolCandidates).mockResolvedValue( - new Map([['maven', [cwd]]]), + new Map([['maven', mavenRoots(cwd)]]), ) vi.mocked(select).mockResolvedValueOnce(true) vi.mocked(setupMaven).mockResolvedValue({ @@ -1006,7 +1049,7 @@ describe('setupRecursiveManifestConfig', () => { it('propagates a hard failure reading the root socket.json', async () => { vi.mocked(findBuildToolCandidates).mockResolvedValue( - new Map([['maven', [cwd]]]), + new Map([['maven', mavenRoots(cwd)]]), ) vi.mocked(readSocketJsonSync).mockImplementation(() => ({ ok: false, @@ -1023,7 +1066,7 @@ describe('setupRecursiveManifestConfig', () => { // The only maven candidate anywhere IS cwd itself, so there's nothing // left to configure individually afterward. vi.mocked(findBuildToolCandidates).mockResolvedValue( - new Map([['maven', [cwd]]]), + new Map([['maven', mavenRoots(cwd)]]), ) vi.mocked(select) // Configure Maven? -> yes. @@ -1044,7 +1087,7 @@ describe('setupRecursiveManifestConfig', () => { it('does not write when the user says yes to configure Maven but leaves every prompt blank', async () => { vi.mocked(findBuildToolCandidates).mockResolvedValue( - new Map([['maven', [cwd]]]), + new Map([['maven', mavenRoots(cwd)]]), ) vi.mocked(select).mockResolvedValueOnce(true) // A no-op wizard: doesn't set a single field. @@ -1065,8 +1108,8 @@ describe('setupRecursiveManifestConfig', () => { vi.mocked(findBuildToolCandidates).mockImplementation( async ({ excludePaths }) => excludePaths?.length - ? new Map([['gradle', []]]) - : new Map([['gradle', [legacy]]]), + ? new Map([['gradle', dirRoots()]]) + : new Map([['gradle', dirRoots(legacy)]]), ) const result = await setupRecursiveManifestConfig(cwd, false, ['legacy']) @@ -1085,7 +1128,7 @@ describe('setupRecursiveManifestConfig', () => { it('reports nothing excluded without writing anything, then asks about the remaining candidate individually', async () => { vi.mocked(findBuildToolCandidates).mockResolvedValue( - new Map([['gradle', [`${cwd}/active`]]]), + new Map([['gradle', dirRoots(`${cwd}/active`)]]), ) vi.mocked(select) // Configure Gradle defaults? -> no (gradle is detected via `active`). @@ -1114,8 +1157,8 @@ describe('setupRecursiveManifestConfig', () => { vi.mocked(findBuildToolCandidates).mockImplementation( async ({ excludePaths }) => excludePaths?.length - ? new Map([['gradle', []]]) - : new Map([['gradle', [legacy, nested]]]), + ? new Map([['gradle', dirRoots()]]) + : new Map([['gradle', dirRoots(legacy, nested)]]), ) const result = await setupRecursiveManifestConfig(cwd, false, ['legacy']) @@ -1134,7 +1177,7 @@ describe('setupRecursiveManifestConfig', () => { it('reports the discovered count and walks the remaining candidate even when no --exclude-paths is given', async () => { vi.mocked(findBuildToolCandidates).mockResolvedValue( - new Map([['gradle', [`${cwd}/active`]]]), + new Map([['gradle', dirRoots(`${cwd}/active`)]]), ) const messages: string[] = [] vi.mocked(select).mockImplementation(async ({ message }) => { @@ -1191,8 +1234,8 @@ describe('setupRecursiveManifestConfig', () => { // --exclude-paths run, say - so neither is ever prompted. vi.mocked(findBuildToolCandidates).mockResolvedValue( new Map([ - ['maven', [serviceA, serviceB, serviceBSubmodule]], - ['gradle', [serviceC]], + ['maven', mavenRoots(serviceA, serviceB, serviceBSubmodule)], + ['gradle', dirRoots(serviceC)], ]), ) vi.mocked(readSocketJsonCascade).mockImplementation(dir => @@ -1221,6 +1264,86 @@ describe('setupRecursiveManifestConfig', () => { ) }) + it('asks once about a directory holding several POMs, counting the answer for each', async () => { + const service = `${cwd}/service` + vi.mocked(findBuildToolCandidates).mockResolvedValue( + new Map([ + [ + 'maven', + [ + { dir: service, entryFile: 'app-dev.pom.xml' }, + { dir: service, entryFile: 'pom.xml' }, + ], + ], + ]), + ) + vi.mocked(enumerateWorkspaces).mockImplementation(async ({ cwd: dir }) => ({ + buildRoot: dir, + projects: [], + })) + const messages: string[] = [] + vi.mocked(select).mockImplementation(async ({ message }) => { + messages.push(message) + if (message.startsWith('Configure other build roots')) { + return true + } + return message.startsWith('Configure') ? false : 'inherit' + }) + const logSpy = vi.spyOn(logger, 'log') + + try { + const result = await setupRecursiveManifestConfig(cwd, false) + + expect(result).toEqual({ ok: true, data: { canceled: false } }) + expect(messages.filter(m => m === 'service (maven)')).toHaveLength(1) + expect( + vi.mocked(enumerateWorkspaces).mock.calls.map(([o]) => o.buildOpts), + ).toEqual([['--file', 'app-dev.pom.xml'], []]) + const logged = logSpy.mock.calls.map(c => String(c[0])).join('\n') + expect(logged).toMatch(/0 configured \(0 re-enabled\), 2 left as-is/) + } finally { + logSpy.mockRestore() + } + }) + + it('does not enumerate a POM at cwd that --exclude-paths excludes', async () => { + vi.mocked(findBuildToolCandidates).mockImplementation( + async ({ excludePaths }) => + new Map([ + [ + 'maven', + excludePaths?.length + ? mavenRoots(cwd, `${cwd}/service`) + : [ + { dir: cwd, entryFile: 'broken.pom.xml' }, + ...mavenRoots(cwd, `${cwd}/service`), + ], + ], + ]), + ) + vi.mocked(enumerateWorkspaces).mockImplementation(async ({ cwd: dir }) => ({ + buildRoot: dir, + projects: [], + })) + vi.mocked(select).mockImplementation(async ({ message }) => { + if (message.startsWith('Configure other build roots')) { + return true + } + return message.startsWith('Configure') ? false : 'inherit' + }) + + const result = await setupRecursiveManifestConfig(cwd, false, [ + 'broken.pom.xml', + ]) + + expect(result).toEqual({ ok: true, data: { canceled: false } }) + expect( + vi + .mocked(enumerateWorkspaces) + .mock.calls.some(([o]) => o.buildOpts.includes('broken.pom.xml')), + ).toBe(false) + }) + it('seeds coverage from cwd itself, so a reactor rooted exactly at cwd still gets its declared members pruned', async () => { const cwd = await fs.realpath( await fs.mkdtemp(path.join(tmpdir(), 'seed-coverage-')), @@ -1240,7 +1363,9 @@ describe('setupRecursiveManifestConfig', () => { const moduleB = `${cwd}/module-b` vi.mocked(findBuildToolCandidates).mockResolvedValue( - new Map([['maven', [cwd, independentService, moduleA, moduleB]]]), + new Map([ + ['maven', mavenRoots(cwd, independentService, moduleA, moduleB)], + ]), ) vi.mocked(enumerateWorkspaces).mockImplementation(async ({ cwd: dir }) => dir === cwd @@ -1300,7 +1425,7 @@ describe('setupRecursiveManifestConfig', () => { const laterCandidate = `${cwd}/zzz-later` vi.mocked(findBuildToolCandidates).mockResolvedValue( - new Map([['maven', [independentService, laterCandidate]]]), + new Map([['maven', mavenRoots(independentService, laterCandidate)]]), ) vi.mocked(select).mockImplementation(async ({ message }) => { if (message.startsWith('Configure Maven defaults')) { diff --git a/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/app-dev.pom.xml b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/app-dev.pom.xml new file mode 100644 index 000000000..8a47fd0d9 --- /dev/null +++ b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/app-dev.pom.xml @@ -0,0 +1,6 @@ + + 4.0.0 + com.example + app-dev-pom-xml + 1.0.0 + diff --git a/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/bare/pom b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/bare/pom new file mode 100644 index 000000000..438851ec5 --- /dev/null +++ b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/bare/pom @@ -0,0 +1,6 @@ + + 4.0.0 + com.example + pom + 1.0.0 + diff --git a/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/other.pom.xml b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/other.pom.xml new file mode 100644 index 000000000..5865b355a --- /dev/null +++ b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/other.pom.xml @@ -0,0 +1,6 @@ + + 4.0.0 + com.example + other-pom-xml + 1.0.0 + diff --git a/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/pom.xml b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/pom.xml new file mode 100644 index 000000000..855bd9219 --- /dev/null +++ b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/pom.xml @@ -0,0 +1,6 @@ + + 4.0.0 + com.example + pom-xml + 1.0.0 + diff --git a/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/pom.xml.bak b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/pom.xml.bak new file mode 100644 index 000000000..9900ea600 --- /dev/null +++ b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/pom.xml.bak @@ -0,0 +1,6 @@ + + 4.0.0 + com.example + pom-xml-bak + 1.0.0 + diff --git a/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/upper/POM.XML b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/upper/POM.XML new file mode 100644 index 000000000..8cb052e88 --- /dev/null +++ b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/upper/POM.XML @@ -0,0 +1,6 @@ + + 4.0.0 + com.example + POM-XML + 1.0.0 + diff --git a/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/vendored/lib-1.0.pom b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/vendored/lib-1.0.pom new file mode 100644 index 000000000..f095a8483 --- /dev/null +++ b/test/fixtures/commands/manifest/dynamic-sbom-inference/pom-names/vendored/lib-1.0.pom @@ -0,0 +1,6 @@ + + 4.0.0 + com.example + lib-1-0-pom + 1.0.0 + From f7ae575d08b71198fa5e800c1487922529be73ca Mon Sep 17 00:00:00 2001 From: Jeppe Fredsgaard Blaabjerg Date: Tue, 27 Oct 2026 10:23:29 +0100 Subject: [PATCH 2/3] refactor(manifest): address review on per-POM DSI discovery - Match build markers in the glob filter, POMs case-insensitively, instead of case-folding the pattern. - Pass the POM to build as ManifestScriptOptions.entryFile; invokeMaven adds --file, and progress and failure messages name the POM. - Resolve socket.yml once per caller and pass it to discovery. - One table for file-addressed tools; one shared depth-then-path sort. - Group setup-wizard candidates by directory: one prompt and one summary count per directory, each of its POMs enumerated unless already covered, including POMs at cwd. Co-Authored-By: Claude Opus 5.5 --- .../manifest/discover-manifest-roots.mts | 96 ++++---- .../manifest/discover-manifest-roots.test.mts | 127 ++++++----- .../manifest/enumerate-workspaces.mts | 4 + .../manifest/generate-recursive-manifests.mts | 14 +- .../generate-recursive-manifests.test.mts | 35 ++- src/commands/manifest/run-manifest-facts.mts | 10 +- src/commands/manifest/scripts/build-tool.mts | 25 +-- .../manifest/scripts/build-tool.test.mts | 16 +- src/commands/manifest/scripts/run.mts | 15 +- src/commands/manifest/scripts/run.test.mts | 34 +++ .../setup-recursive-manifest-config.mts | 205 ++++++++++-------- .../setup-recursive-manifest-config.test.mts | 154 ++++++------- 12 files changed, 390 insertions(+), 345 deletions(-) diff --git a/src/commands/manifest/discover-manifest-roots.mts b/src/commands/manifest/discover-manifest-roots.mts index 98541573d..6b82f1075 100644 --- a/src/commands/manifest/discover-manifest-roots.mts +++ b/src/commands/manifest/discover-manifest-roots.mts @@ -3,8 +3,7 @@ import path from 'node:path' import micromatch from 'micromatch' -import { defaultEntryFile, isAddressedByFile } from './scripts/build-tool.mts' -import { findSocketYmlSync } from '../../utils/config.mts' +import { isAddressedByFile } from './scripts/build-tool.mts' import { realpathOrResolved } from '../../utils/fs.mts' import { globWithGitIgnore } from '../../utils/glob.mts' import { excludePathToScanIgnores } from '../scan/exclude-paths.mts' @@ -12,22 +11,14 @@ import { excludePathToScanIgnores } from '../scan/exclude-paths.mts' import type { BuildTool } from './scripts/build-tool.mts' import type { SocketFactsSbomProject } from './scripts/facts.mts' import type { SocketJson } from '../../utils/socket-json.mts' +import type { SocketYml } from '@socketsecurity/config' const BUILD_TOOLS: BuildTool[] = ['gradle', 'maven', 'sbt'] -// In the pattern, not the walk, whose ignores (--exclude-paths, .gitignore) -// stay case-sensitive. Assumes every letter in the pattern is a literal. -function foldCase(pattern: string): string { - return pattern.replace( - /[a-z]/gi, - c => `[${c.toLowerCase()}${c.toUpperCase()}]`, - ) -} - -// One glob-suffix set per ecosystem's build-descriptor marker(s); `settings.gradle(.kts)` +// One glob set per ecosystem's build-descriptor marker(s); `settings.gradle(.kts)` // covers Kotlin-DSL roots that have no root `build.gradle`. Maven mirrors the -// `pomxml` pattern of the API's supported files, case-insensitively as a scan -// matches it, so every POM a scan uploads is also a build root here. +// `pomxml` pattern of the API's supported files, so every POM a scan uploads is +// also a build root here. const MARKERS_BY_TOOL: Record = { __proto__: null, gradle: [ @@ -36,18 +27,21 @@ const MARKERS_BY_TOOL: Record = { 'settings.gradle', 'settings.gradle.kts', ], - maven: [foldCase('{*-*.,}pom{.xml,}')], + maven: ['{*-*.,}pom{.xml,}'], sbt: ['build.sbt'], } as unknown as Record -const MARKER_MATCHERS_BY_TOOL = Object.fromEntries( - BUILD_TOOLS.map(tool => [ - tool, - MARKERS_BY_TOOL[tool].map(marker => - micromatch.matcher(marker, { dot: true }), - ), - ]), -) as Record boolean>> +// Case-insensitive for POMs, as a scan matches them; Gradle and sbt look for +// their build files by exact name. +const MARKER_MATCHERS_BY_TOOL = new Map( + BUILD_TOOLS.map(tool => { + const options = { dot: true, nocase: tool === 'maven' } + const matchers = MARKERS_BY_TOOL[tool].map(marker => + micromatch.matcher(marker, options), + ) + return [tool, (basename: string) => matchers.some(m => m(basename))] + }), +) // `entryFile` (a basename in `dir`) is set only for a tool that addresses its // builds by file, where one directory can hold several builds. @@ -61,9 +55,7 @@ function markerTool( tools: readonly BuildTool[], basename: string, ): BuildTool | undefined { - return tools.find(tool => - MARKER_MATCHERS_BY_TOOL[tool].some(matches => matches(basename)), - ) + return tools.find(tool => MARKER_MATCHERS_BY_TOOL.get(tool)?.(basename)) } // A disabled ecosystem is skipped entirely by the glob scan below, so a @@ -123,33 +115,32 @@ export function isBuildRootCovered( tool: BuildTool, root: BuildRoot, ): boolean { - const entryFile = root.entryFile ?? defaultEntryFile(tool) - if (entryFile) { - return covered.has(path.join(root.dir, entryFile)) + if (root.entryFile) { + return covered.has(buildRootPath(root)) } return MARKERS_BY_TOOL[tool].some(marker => covered.has(path.join(root.dir, marker)), ) } -function sortByDepthThenPath( - roots: readonly BuildRoot[], +function compareStrings(a: string, b: string): number { + return a < b ? -1 : a > b ? 1 : 0 +} + +export function sortByDepthThenPath( + roots: readonly T[], cwd: string, -): BuildRoot[] { +): T[] { return [...roots].sort((a, b) => { const relA = path.relative(cwd, a.dir) const relB = path.relative(cwd, b.dir) const depthA = relA === '' ? 0 : relA.split(path.sep).length const depthB = relB === '' ? 0 : relB.split(path.sep).length - if (depthA !== depthB) { - return depthA - depthB - } - if (relA !== relB) { - return relA < relB ? -1 : 1 - } - const entryA = a.entryFile ?? '' - const entryB = b.entryFile ?? '' - return entryA < entryB ? -1 : entryA > entryB ? 1 : 0 + return ( + depthA - depthB || + compareStrings(relA, relB) || + compareStrings(a.entryFile ?? '', b.entryFile ?? '') + ) }) } @@ -159,9 +150,13 @@ export async function findBuildToolCandidates({ cwd, excludePaths, sockJson, + socketConfig, }: { cwd: string excludePaths?: string[] | undefined + // The socket.yml a scan applies to its upload, so a build file a scan would + // not upload is not built either. + socketConfig?: SocketYml | undefined sockJson: SocketJson }): Promise> { const enabledTools = BUILD_TOOLS.filter( @@ -172,20 +167,17 @@ export async function findBuildToolCandidates({ return result } - const patterns = enabledTools.flatMap(tool => - MARKERS_BY_TOOL[tool].map(marker => `**/${marker}`), - ) const additionalIgnores = (excludePaths ?? []).flatMap( excludePathToScanIgnores, ) - // The same socket.yml a scan applies to its upload, so a build file a scan - // would not upload is not built either. - const socketYml = findSocketYmlSync(cwd) - const hits = await globWithGitIgnore(patterns, { + // Markers are matched in the filter, after the ignores, so --exclude-paths + // and .gitignore stay case-sensitive as they are for a scan. + const hits = await globWithGitIgnore(['**/*'], { absolute: true, additionalIgnores, cwd, - socketConfig: socketYml.ok ? socketYml.data?.parsed : undefined, + filter: hit => !!markerTool(enabledTools, path.basename(hit)), + socketConfig, }) const rootsByTool = new Map>( @@ -197,14 +189,12 @@ export async function findBuildToolCandidates({ return { basename, dir: await realpathOrResolved(path.dirname(hit)), - tool: markerTool(enabledTools, basename), + // Set: the glob filter kept only marker hits. + tool: markerTool(enabledTools, basename) as BuildTool, } }), ) for (const { basename, dir, tool } of resolvedHits) { - if (!tool) { - continue - } const root = { dir, entryFile: isAddressedByFile(tool) ? basename : undefined, diff --git a/src/commands/manifest/discover-manifest-roots.test.mts b/src/commands/manifest/discover-manifest-roots.test.mts index 97262d8b4..30cfab3ab 100644 --- a/src/commands/manifest/discover-manifest-roots.test.mts +++ b/src/commands/manifest/discover-manifest-roots.test.mts @@ -5,10 +5,13 @@ import path from 'node:path' import { describe, expect, it } from 'vitest' import { + buildRootPath, findBuildToolCandidates, isBuildRootCovered, + sortByDepthThenPath, } from './discover-manifest-roots.mts' import { testPath } from '../../../test/utils.mts' +import { findSocketYmlSync } from '../../utils/config.mts' import type { BuildRoot } from './discover-manifest-roots.mts' import type { SocketJson } from '../../utils/socket-json.mts' @@ -35,12 +38,39 @@ async function pomNamesRoots(excludePaths?: string[]): Promise { sockJson: {} as SocketJson, }) return (candidates.get('maven') ?? []).map(r => - path - .relative(pomNames, path.join(r.dir, r.entryFile ?? '')) - .replaceAll('\\', '/'), + path.relative(pomNames, buildRootPath(r)).replaceAll('\\', '/'), ) } +async function rootsBesideIgnoredPom( + ignoredPomDir: string, + ignoreFile: { content: string; name: string }, +): Promise { + const cwd = await fs.realpath( + await fs.mkdtemp(path.join(tmpdir(), 'ignored-poms-')), + ) + try { + await fs.mkdir(path.join(cwd, ignoredPomDir), { recursive: true }) + await fs.writeFile( + path.join(cwd, ignoredPomDir, 'lib-1.0.pom'), + '', + ) + await fs.writeFile(path.join(cwd, 'pom.xml'), '') + await fs.writeFile(path.join(cwd, ignoreFile.name), ignoreFile.content) + const socketYml = findSocketYmlSync(cwd) + const candidates = await findBuildToolCandidates({ + cwd, + socketConfig: socketYml.ok ? socketYml.data?.parsed : undefined, + sockJson: {} as SocketJson, + }) + return candidates + .get('maven') + ?.map(root => ({ ...root, dir: path.relative(cwd, root.dir) })) + } finally { + await fs.rm(cwd, { recursive: true, force: true }) + } +} + function dirsOf(roots: BuildRoot[] | undefined): string[] { return (roots ?? []).map(r => r.dir) } @@ -154,53 +184,21 @@ describe('findBuildToolCandidates', () => { }) it('skips POMs a .gitignore covers, as a scan does when uploading', async () => { - const cwd = await fs.realpath( - await fs.mkdtemp(path.join(tmpdir(), 'gitignored-poms-')), - ) - try { - const repoDir = path.join(cwd, '.m2/repository/com/example/lib/1.0') - await fs.mkdir(repoDir, { recursive: true }) - await fs.writeFile(path.join(repoDir, 'lib-1.0.pom'), '') - await fs.writeFile(path.join(cwd, 'pom.xml'), '') - await fs.writeFile(path.join(cwd, '.gitignore'), '.m2/\n') - - const candidates = await findBuildToolCandidates({ - cwd, - sockJson: {} as SocketJson, - }) - - expect(candidates.get('maven')).toEqual([ - { dir: cwd, entryFile: 'pom.xml' }, - ]) - } finally { - await fs.rm(cwd, { recursive: true, force: true }) - } + expect( + await rootsBesideIgnoredPom('.m2/repository/com/example/lib/1.0', { + content: '.m2/\n', + name: '.gitignore', + }), + ).toEqual([{ dir: '', entryFile: 'pom.xml' }]) }) it("skips POMs socket.yml's projectIgnorePaths covers, as a scan does when uploading", async () => { - const cwd = await fs.realpath( - await fs.mkdtemp(path.join(tmpdir(), 'project-ignored-poms-')), - ) - try { - await fs.mkdir(path.join(cwd, 'libs/repo'), { recursive: true }) - await fs.writeFile(path.join(cwd, 'libs/repo/lib-1.0.pom'), '') - await fs.writeFile(path.join(cwd, 'pom.xml'), '') - await fs.writeFile( - path.join(cwd, 'socket.yml'), - 'version: 2\nprojectIgnorePaths:\n - libs/repo\n', - ) - - const candidates = await findBuildToolCandidates({ - cwd, - sockJson: {} as SocketJson, - }) - - expect(candidates.get('maven')).toEqual([ - { dir: cwd, entryFile: 'pom.xml' }, - ]) - } finally { - await fs.rm(cwd, { recursive: true, force: true }) - } + expect( + await rootsBesideIgnoredPom('libs/repo', { + content: 'version: 2\nprojectIgnorePaths:\n - libs/repo\n', + name: 'socket.yml', + }), + ).toEqual([{ dir: '', entryFile: 'pom.xml' }]) }) it('respects --exclude-paths for non-default POMs, case-sensitively', async () => { @@ -234,17 +232,7 @@ describe('isBuildRootCovered', () => { ).toBe(false) }) - it('covers a root without an entry file through its default entry file or a marker file', () => { - expect( - isBuildRootCovered(new Set(['/repo/service/pom.xml']), 'maven', { - dir: '/repo/service', - }), - ).toBe(true) - expect( - isBuildRootCovered(new Set(['/repo/service/lib-1.0.pom']), 'maven', { - dir: '/repo/service', - }), - ).toBe(false) + it('covers a root without an entry file through a marker file', () => { expect( isBuildRootCovered(new Set(['/repo/lib/settings.gradle.kts']), 'gradle', { dir: '/repo/lib', @@ -257,3 +245,26 @@ describe('isBuildRootCovered', () => { ).toBe(false) }) }) + +describe('sortByDepthThenPath', () => { + it('sorts shallower dirs first, then by path, then by entry file', () => { + expect( + sortByDepthThenPath( + [ + { dir: '/repo/b/nested' }, + { dir: '/repo/b', entryFile: 'pom.xml' }, + { dir: '/repo/b', entryFile: 'app-dev.pom.xml' }, + { dir: '/repo/a' }, + { dir: '/repo' }, + ], + '/repo', + ), + ).toEqual([ + { dir: '/repo' }, + { dir: '/repo/a' }, + { dir: '/repo/b', entryFile: 'app-dev.pom.xml' }, + { dir: '/repo/b', entryFile: 'pom.xml' }, + { dir: '/repo/b/nested' }, + ]) + }) +}) diff --git a/src/commands/manifest/enumerate-workspaces.mts b/src/commands/manifest/enumerate-workspaces.mts index 6108798cd..524de6ee9 100644 --- a/src/commands/manifest/enumerate-workspaces.mts +++ b/src/commands/manifest/enumerate-workspaces.mts @@ -24,6 +24,7 @@ export async function enumerateWorkspaces({ buildOpts, cwd, ecosystem, + entryFile, excludePaths, javaHome, verbose, @@ -32,6 +33,8 @@ export async function enumerateWorkspaces({ buildOpts: string[] cwd: string ecosystem: BuildTool + // See ManifestScriptOptions.entryFile. + entryFile?: string | undefined excludePaths?: string[] | undefined javaHome?: string | undefined verbose: boolean @@ -51,6 +54,7 @@ export async function enumerateWorkspaces({ const scriptOpts = { bin: bin || undefined, + entryFile, excludePaths: excludePaths?.length ? excludePaths : undefined, // `env` replaces the spawned process's whole environment, not just JAVA_HOME. env: resolvedJavaHome diff --git a/src/commands/manifest/generate-recursive-manifests.mts b/src/commands/manifest/generate-recursive-manifests.mts index fcfafa31d..a0a9f5416 100644 --- a/src/commands/manifest/generate-recursive-manifests.mts +++ b/src/commands/manifest/generate-recursive-manifests.mts @@ -12,15 +12,13 @@ import { } from './discover-manifest-roots.mts' import { parseBuildToolOpts } from './parse-build-tool-opts.mts' import { runManifestFacts } from './run-manifest-facts.mts' -import { - resolveBuildToolBin, - withEntryFileOpts, -} from './scripts/build-tool.mts' +import { resolveBuildToolBin } from './scripts/build-tool.mts' import { DSI_SETUP_COMMAND, REACH_CONTINUE_ON_INSTALL_ERRORS_FLAG, } from './scripts/resolution-report-render.mts' import { SOCKET_JSON } from '../../constants.mts' +import { findSocketYmlSync } from '../../utils/config.mts' import { withTmpDir } from '../../utils/fs.mts' import { readOrDefaultSocketJson, @@ -136,8 +134,7 @@ async function runEcosystemCandidates({ const covered = new Set() for (const [ecosystem, roots] of candidatesByTool) { const disabledRoots: DisabledRoot[] = [] - for (let rootIndex = 0; rootIndex < roots.length; rootIndex += 1) { - const root = roots[rootIndex] as BuildRoot + for (const { 0: rootIndex, 1: root } of roots.entries()) { const { dir, entryFile } = root if (isBuildRootCovered(covered, ecosystem, root)) { outcomes.push({ ...root, ecosystem, status: 'skippedCovered' }) @@ -181,9 +178,10 @@ async function runEcosystemCandidates({ // eslint-disable-next-line no-await-in-loop const result = await runManifestFacts({ bin, - buildOpts: withEntryFileOpts(ecosystem, entryFile, buildOpts), + buildOpts, cwd: dir, ecosystem, + entryFile, excludeConfigs, excludePaths: excludePathsForRoot, hints: { @@ -282,9 +280,11 @@ export async function generateRecursiveManifests({ // A root-disabled ecosystem must still be scanned for - a nested socket.json // may re-enable it - so the per-directory cascade below, not this scan, is // what actually decides skip vs. include. + const socketYml = findSocketYmlSync(cwd) const candidatesByTool = await findBuildToolCandidates({ cwd, excludePaths, + socketConfig: socketYml.ok ? socketYml.data?.parsed : undefined, sockJson: withoutDisabledFlags(rootSockJson), }) diff --git a/src/commands/manifest/generate-recursive-manifests.test.mts b/src/commands/manifest/generate-recursive-manifests.test.mts index 586c637d3..df90dae97 100644 --- a/src/commands/manifest/generate-recursive-manifests.test.mts +++ b/src/commands/manifest/generate-recursive-manifests.test.mts @@ -16,6 +16,7 @@ vi.mock('./run-manifest-facts.mts', () => ({ runManifestFacts: vi.fn(), })) +import { buildRootPath } from './discover-manifest-roots.mts' import { generateRecursiveManifests } from './generate-recursive-manifests.mts' import { runManifestFacts } from './run-manifest-facts.mts' import { testPath } from '../../../test/utils.mts' @@ -1033,13 +1034,10 @@ describe('generateRecursiveManifests', () => { } }) - it('runs every POM in a directory as its own build, pointing maven at any non-default one', async () => { + it('runs every POM in a directory as its own build', async () => { vi.mocked(runManifestFacts).mockImplementation( - async ({ buildOpts, cwd }) => ({ - factsPath: path.join( - cwd, - `${buildOpts[1] ?? 'pom.xml'}.socket.facts.json`, - ), + async ({ cwd, entryFile }) => ({ + factsPath: path.join(cwd, `${entryFile}.socket.facts.json`), projects: [], }), ) @@ -1053,27 +1051,24 @@ describe('generateRecursiveManifests', () => { .mocked(runManifestFacts) .mock.calls.map(([opts]) => [ path.relative(pomNames, opts.cwd).replaceAll('\\', '/'), - opts.buildOpts, + opts.entryFile, ]) expect(calls).toEqual([ - ['', ['--file', 'app-dev.pom.xml']], - ['', []], - ['bare', ['--file', 'pom']], - ['upper', ['--file', 'POM.XML']], - ['vendored', ['--file', 'lib-1.0.pom']], + ['', 'app-dev.pom.xml'], + ['', 'pom.xml'], + ['bare', 'pom'], + ['upper', 'POM.XML'], + ['vendored', 'lib-1.0.pom'], ]) expect(outcomes.every(o => o.status === 'generated')).toBe(true) }) it('covers a POM by file, so a sibling POM in the same directory still runs', async () => { vi.mocked(runManifestFacts).mockImplementation( - async ({ buildOpts, cwd }) => ({ - factsPath: path.join( - cwd, - `${buildOpts[1] ?? 'pom.xml'}.socket.facts.json`, - ), + async ({ cwd, entryFile }) => ({ + factsPath: path.join(cwd, `${entryFile}.socket.facts.json`), projects: - cwd === pomNames && !buildOpts.length + cwd === pomNames && entryFile === 'pom.xml' ? [ { type: 'maven', @@ -1093,9 +1088,7 @@ describe('generateRecursiveManifests', () => { }) const statusOf = (rel: string) => - outcomes.find( - o => path.join(o.dir, o.entryFile ?? '') === path.join(pomNames, rel), - )?.status + outcomes.find(o => buildRootPath(o) === path.join(pomNames, rel))?.status expect(statusOf('pom.xml')).toBe('generated') expect(statusOf('app-dev.pom.xml')).toBe('generated') expect(statusOf('bare/pom')).toBe('skippedCovered') diff --git a/src/commands/manifest/run-manifest-facts.mts b/src/commands/manifest/run-manifest-facts.mts index 1a69346a4..1c09c4a37 100644 --- a/src/commands/manifest/run-manifest-facts.mts +++ b/src/commands/manifest/run-manifest-facts.mts @@ -3,6 +3,7 @@ import path from 'node:path' import { logger } from '@socketsecurity/registry/lib/logger' +import { buildRootPath } from './discover-manifest-roots.mts' import { expandEnvVarRefs, formatMissingEnvVarRefs, @@ -60,6 +61,7 @@ export async function runManifestFacts({ buildOpts, cwd, ecosystem, + entryFile, excludeConfigs, excludePaths, hints = FLAG_HINTS, @@ -75,6 +77,8 @@ export async function runManifestFacts({ buildOpts: string[] cwd: string ecosystem: BuildTool + // See ManifestScriptOptions.entryFile. + entryFile?: string | undefined excludeConfigs: string excludePaths?: string[] | undefined // Where the user sets build options, for remedy hints. @@ -109,12 +113,14 @@ export async function runManifestFacts({ return null } + const location = buildRootPath({ dir: cwd, entryFile }) logger.info( - `Generating Socket facts for the ${ecosystem} project at \`${cwd}\` ...`, + `Generating Socket facts for the ${ecosystem} project at \`${location}\` ...`, ) const scriptOpts = { bin: bin || undefined, + entryFile, excludeConfigs: excludeConfigs || undefined, excludePaths: excludePaths?.length ? excludePaths : undefined, // `env` replaces the spawned process's whole environment, not just JAVA_HOME. @@ -233,7 +239,7 @@ export async function runManifestFacts({ // successful run couldn't resolve. process.exitCode = 1 logger.fail( - `The ${ecosystem} build failed (exit code ${code}) before producing any Socket facts.`, + `The ${ecosystem} build at \`${location}\` failed (exit code ${code}) before producing any Socket facts.`, ) return null } diff --git a/src/commands/manifest/scripts/build-tool.mts b/src/commands/manifest/scripts/build-tool.mts index 3b0367e36..27e5b74e1 100644 --- a/src/commands/manifest/scripts/build-tool.mts +++ b/src/commands/manifest/scripts/build-tool.mts @@ -23,13 +23,6 @@ const BUILD_TOOL_WRAPPER = { // Gradle (8+) and sbt hold one build per directory; Maven builds are addressed // by POM file, so `mvn -f other-pom.xml` puts a second build in the directory. -const ADDRESSED_BY_FILE: Record = { - __proto__: null, - gradle: false, - maven: true, - sbt: false, -} as unknown as Record - const DEFAULT_ENTRY_FILE: Partial> = { __proto__: null, maven: 'pom.xml', @@ -40,7 +33,7 @@ export function defaultEntryFile(tool: BuildTool): string | undefined { } export function isAddressedByFile(tool: BuildTool): boolean { - return ADDRESSED_BY_FILE[tool] + return !!DEFAULT_ENTRY_FILE[tool] } // sbt happily runs in any directory, synthesizing a default project from its @@ -53,20 +46,6 @@ export function looksLikeSbtBuild(projectDir: string): boolean { ) } -// Leads, as Maven 3 takes the first `--file`; nothing for the default entry -// file, so options a user passes keep their meaning. -export function withEntryFileOpts( - tool: BuildTool, - entryFile: string | undefined, - buildOpts: string[], -): string[] { - const defaultEntry = DEFAULT_ENTRY_FILE[tool] - if (!entryFile || !defaultEntry || entryFile === defaultEntry) { - return buildOpts - } - return ['--file', entryFile, ...buildOpts] -} - export function resolveBuildToolBin( tool: BuildTool, projectDir: string, @@ -87,7 +66,7 @@ export function socketFactsFileName( tool: BuildTool, entryFile: string | undefined, ): string | undefined { - if (!ADDRESSED_BY_FILE[tool]) { + if (!isAddressedByFile(tool)) { return `${tool}${constants.DOT_SOCKET_DOT_FACTS_JSON}` } return entryFile diff --git a/src/commands/manifest/scripts/build-tool.test.mts b/src/commands/manifest/scripts/build-tool.test.mts index 37c88fce6..7e8fb7a0f 100644 --- a/src/commands/manifest/scripts/build-tool.test.mts +++ b/src/commands/manifest/scripts/build-tool.test.mts @@ -1,22 +1,8 @@ import { describe, expect, it } from 'vitest' -import { socketFactsFileName, withEntryFileOpts } from './build-tool.mts' +import { socketFactsFileName } from './build-tool.mts' import { parseRecords } from './records.mts' -describe('withEntryFileOpts', () => { - it('points a file-addressed build at a non-default entry file, ahead of the given options', () => { - expect(withEntryFileOpts('maven', 'app-dev.pom.xml', ['-P', 'ci'])).toEqual( - ['--file', 'app-dev.pom.xml', '-P', 'ci'], - ) - }) - - it('adds nothing for the default entry file or a directory-addressed build', () => { - expect(withEntryFileOpts('maven', 'pom.xml', ['-q'])).toEqual(['-q']) - expect(withEntryFileOpts('maven', undefined, [])).toEqual([]) - expect(withEntryFileOpts('gradle', 'build.gradle', [])).toEqual([]) - }) -}) - describe('socketFactsFileName', () => { it('names a directory-addressed build after its tool', () => { expect(socketFactsFileName('gradle', undefined)).toBe( diff --git a/src/commands/manifest/scripts/run.mts b/src/commands/manifest/scripts/run.mts index 23ecb0c32..8f1d84ab5 100644 --- a/src/commands/manifest/scripts/run.mts +++ b/src/commands/manifest/scripts/run.mts @@ -4,7 +4,11 @@ import path from 'node:path' import { spawn } from '@socketsecurity/registry/lib/spawn' import { assembleFacts } from './assemble.mts' -import { resolveBuildToolBin, socketFactsFileName } from './build-tool.mts' +import { + defaultEntryFile, + resolveBuildToolBin, + socketFactsFileName, +} from './build-tool.mts' import { serializeExcludePathPatterns } from './exclude-paths-glob.mts' import { parseRecords } from './records.mts' import constants from '../../../constants.mts' @@ -22,6 +26,9 @@ export type ManifestScriptOptions = { projectDir: string // Unset ⇒ resolved to the project wrapper, else PATH (resolveBuildToolBin). bin?: string | undefined + // Maven only: the POM in projectDir to build; unset or `pom.xml` builds the + // default, leaving a `--file` in toolOpts in effect. + entryFile?: string | undefined // Reachability-only: also materialize resolved artifact paths (artifactPaths). withFiles?: boolean | undefined // sbt only: caller-supplied directory to use as sbt's isolated global base. @@ -389,8 +396,14 @@ async function invokeMaven( `-Dsocket.recordsFile=${recordsFile}`, ...commonProps(opts, '-D'), ] + // Ahead of toolOpts, as Maven 3 takes the first `--file`. + const fileOpts = + opts.entryFile && opts.entryFile !== defaultEntryFile('maven') + ? ['--file', opts.entryFile] + : [] const args = [ ...props, + ...fileOpts, ...(opts.toolOpts ?? []), '--batch-mode', 'validate', diff --git a/src/commands/manifest/scripts/run.test.mts b/src/commands/manifest/scripts/run.test.mts index 1caf1becb..fdf247a0b 100644 --- a/src/commands/manifest/scripts/run.test.mts +++ b/src/commands/manifest/scripts/run.test.mts @@ -11,6 +11,14 @@ import { enumerateWorkspaces, runManifestScript } from './run.mts' vi.mock('@socketsecurity/registry/lib/spawn', () => ({ spawn: vi.fn(async () => ({ code: 0, stderr: '', stdout: '' })), })) +// The Maven extension jar exists only in a built checkout. +vi.mock('node:fs', async importOriginal => { + const actual = await importOriginal() + return { + ...actual, + existsSync: (p: string) => p.endsWith('.jar') || actual.existsSync(p), + } +}) describe('build tool output', () => { it.each([ @@ -65,3 +73,29 @@ describe('shared sbt global base', () => { } }) }) + +describe('maven entry file', () => { + it.each([ + ['app-dev.pom.xml', ['--file', 'app-dev.pom.xml', '-P', 'ci']], + ['pom.xml', ['-P', 'ci']], + [undefined, ['-P', 'ci']], + ] as const)( + 'builds %s, ahead of the given options', + async (entryFile, expected) => { + vi.mocked(spawn).mockClear() + await runManifestScript('maven', { + bin: 'mvn', + entryFile, + projectDir: process.cwd(), + stdio: 'pipe', + toolOpts: ['-P', 'ci'], + }) + const args = vi.mocked(spawn).mock.calls[0]?.[1] ?? [] + expect(args.filter(a => !a.startsWith('-D'))).toEqual([ + ...expected, + '--batch-mode', + 'validate', + ]) + }, + ) +}) diff --git a/src/commands/manifest/setup-recursive-manifest-config.mts b/src/commands/manifest/setup-recursive-manifest-config.mts index 9526a489f..18a2691c9 100644 --- a/src/commands/manifest/setup-recursive-manifest-config.mts +++ b/src/commands/manifest/setup-recursive-manifest-config.mts @@ -12,13 +12,14 @@ import { findBuildToolCandidates, isBuildRootCovered, realpathOrResolved, + sortByDepthThenPath, withoutDisabledFlags, } from './discover-manifest-roots.mts' import { enumerateWorkspaces } from './enumerate-workspaces.mts' import { resolveEcosystemConfig } from './generate-recursive-manifests.mts' -import { withEntryFileOpts } from './scripts/build-tool.mts' import { setupGradle, setupMaven, setupSbt } from './setup-manifest-config.mts' import { SOCKET_JSON } from '../../constants.mts' +import { findSocketYmlSync } from '../../utils/config.mts' import { readOrDefaultSocketJson, readSocketJsonCascade, @@ -35,9 +36,11 @@ import type { BuildTool } from './scripts/build-tool.mts' import type { CResult } from '../../types.mts' import type { SocketJson } from '../../utils/socket-json.mts' -// A single discovered build root - a directory with its own gradle/sbt marker -// file, or one Maven POM. -type Candidate = BuildRoot & { ecosystem: BuildTool } +type EcosystemRoot = BuildRoot & { ecosystem: BuildTool } + +// One prompt per directory, as settings are per directory; Maven may have +// several POMs, so several roots, there. +type Candidate = { dir: string; ecosystem: BuildTool; roots: BuildRoot[] } const ROOT_ECOSYSTEMS: BuildTool[] = ['maven', 'gradle', 'sbt'] @@ -74,26 +77,6 @@ function getEcosystemSection( ) } -// Depth-then-path sort: for exclusion roots it's just stable log ordering; -// for candidates it's load-bearing, since a parent must be prompted before -// its children so a child's shown default reflects a parent's just-written -// cascade instead of its pre-run value. -export function sortCandidatesForDisplay( - candidates: readonly T[], - cwd: string, -): T[] { - return [...candidates].sort((a, b) => { - const relA = path.relative(cwd, a.dir) - const relB = path.relative(cwd, b.dir) - const depthA = relA.split(path.sep).length - const depthB = relB.split(path.sep).length - if (depthA !== depthB) { - return depthA - depthB - } - return relA < relB ? -1 : relA > relB ? 1 : 0 - }) -} - function toPosixRelative(cwd: string, dir: string): string { return path.relative(cwd, dir).split(path.sep).join('/') } @@ -132,9 +115,11 @@ export async function scanBuildRoots({ excludePaths?: string[] | undefined sockJson: SocketJson }): Promise { + const socketYml = findSocketYmlSync(cwd) + const socketConfig = socketYml.ok ? socketYml.data?.parsed : undefined const [fullByTool, includedByTool] = await Promise.all([ - findBuildToolCandidates({ cwd, sockJson }), - findBuildToolCandidates({ cwd, excludePaths, sockJson }), + findBuildToolCandidates({ cwd, socketConfig, sockJson }), + findBuildToolCandidates({ cwd, excludePaths, socketConfig, sockJson }), ]) return { fullByTool, includedByTool } } @@ -167,27 +152,43 @@ export async function discoverBuildRoots({ const ignorePatterns = (excludePaths ?? []).flatMap(excludePathToScanIgnores) const ecosystemsByRoot = new Map>() for (const [ecosystem, fullRoots] of fullByTool) { - const includedRoots = includedByTool.get(ecosystem) ?? [] - const includedPaths = new Set(includedRoots.map(buildRootPath)) - const includedDirs = new Set(includedRoots.map(root => root.dir)) + const includedPaths = new Set( + (includedByTool.get(ecosystem) ?? []).map(buildRootPath), + ) + const rootsByDir = new Map() for (const root of fullRoots) { - const { dir } = root - if (dir === realCwd) { + if (root.dir === realCwd) { continue } - if (includedPaths.has(buildRootPath(root))) { - included.push({ ...root, ecosystem }) - continue + const roots = rootsByDir.get(root.dir) + if (roots) { + roots.push(root) + } else { + rootsByDir.set(root.dir, [root]) } - // `disabled` is per directory, so it cannot exclude one build file - // without its included siblings. - if (includedDirs.has(dir)) { - logger.warn( - `Not disabling ${ecosystem} in ${toPosixRelative(realCwd, dir)}: --exclude-paths matches ${buildRootPath(root)} but not the other build files there. Pass the same --exclude-paths to the scan instead.`, - ) + } + for (const { 0: dir, 1: roots } of rootsByDir) { + const relDir = toPosixRelative(realCwd, dir) + const includedRoots: BuildRoot[] = [] + const excludedRoots: BuildRoot[] = [] + for (const root of roots) { + if (includedPaths.has(buildRootPath(root))) { + includedRoots.push(root) + } else { + excludedRoots.push(root) + } + } + if (includedRoots.length) { + included.push({ dir, ecosystem, roots: includedRoots }) + // `disabled` is per directory, so it cannot exclude one build file + // without its included siblings. + for (const root of excludedRoots) { + logger.warn( + `Not disabling ${ecosystem} in ${relDir}: --exclude-paths matches ${buildRootPath(root)} but not the other build files there. Pass the same --exclude-paths to the scan instead.`, + ) + } continue } - const relDir = toPosixRelative(realCwd, dir) const rootRelDir = findExclusionRoot(relDir, ignorePatterns) const rootDir = path.join(realCwd, rootRelDir) const ecosystems = ecosystemsByRoot.get(rootDir) ?? new Set() @@ -214,7 +215,7 @@ export async function markWorkspaceCoverage({ excludePaths, rootSockJson, }: { - candidate: Candidate + candidate: EcosystemRoot covered: Set cwd: string excludePaths?: string[] | undefined @@ -237,13 +238,10 @@ export async function markWorkspaceCoverage({ const enumResult = await enumerateWorkspaces({ bin, - buildOpts: withEntryFileOpts( - candidate.ecosystem, - candidate.entryFile, - buildOpts, - ), + buildOpts, cwd: candidate.dir, ecosystem: candidate.ecosystem, + entryFile: candidate.entryFile, excludePaths: excludePathsForCandidate, javaHome, verbose: false, @@ -260,6 +258,41 @@ export async function markWorkspaceCoverage({ return { ok: true, data: undefined } } +// In order, as a root's enumeration can cover the roots after it. +async function markRootsCoverage({ + covered, + cwd, + ecosystem, + excludePaths, + rootSockJson, + roots, +}: { + covered: Set + cwd: string + ecosystem: BuildTool + excludePaths?: string[] | undefined + rootSockJson: SocketJson + roots: readonly BuildRoot[] +}): Promise> { + for (const root of roots) { + if (isBuildRootCovered(covered, ecosystem, root)) { + continue + } + // eslint-disable-next-line no-await-in-loop + const result = await markWorkspaceCoverage({ + candidate: { ...root, ecosystem }, + covered, + cwd, + excludePaths, + rootSockJson, + }) + if (!result.ok) { + return result + } + } + return { ok: true, data: undefined } +} + // Marks one exclusion root's own socket.json `disabled: true` for whichever // of its ecosystems aren't already disabled via cascade (an already-disabled // ancestor from a prior run) - a no-op write is skipped entirely so re-running @@ -718,7 +751,7 @@ export async function setupRecursiveManifestConfig( logger.log(`Configuring ${cwd} ...`) const initialSockJson = readOrDefaultSocketJson(cwd) - // Resolved once here for sortCandidatesForDisplay/disableExclusionRoot's + // Resolved once here for sortByDepthThenPath/disableExclusionRoot's // relative-path math, consistent with discoverBuildRoots' own internal // resolution (see its comment for why this matters). const realCwd = await realpathOrResolved(cwd) @@ -793,7 +826,7 @@ export async function setupRecursiveManifestConfig( if (!excluded.length) { logger.log('None matched --exclude-paths; nothing disabled.') } else { - const orderedExcluded = sortCandidatesForDisplay(excluded, realCwd) + const orderedExcluded = sortByDepthThenPath(excluded, realCwd) for (const candidate of orderedExcluded) { // eslint-disable-next-line no-await-in-loop const result = await disableExclusionRoot({ @@ -839,19 +872,17 @@ export async function setupRecursiveManifestConfig( // cwd never appears in `included`, but a reactor root often sits exactly // at cwd - seed coverage from it first so its declared members still get // recognized. - const includedAtCwd = ROOT_ECOSYSTEMS.flatMap(ecosystem => - (includedByTool.get(ecosystem) ?? []) - .filter(root => root.dir === realCwd) - .map(root => ({ ...root, ecosystem })), - ) - for (const candidate of includedAtCwd) { + for (const ecosystem of ROOT_ECOSYSTEMS) { // eslint-disable-next-line no-await-in-loop - const coverageResult = await markWorkspaceCoverage({ - candidate, + const coverageResult = await markRootsCoverage({ covered, cwd: realCwd, + ecosystem, excludePaths, rootSockJson, + roots: (includedByTool.get(ecosystem) ?? []).filter( + root => root.dir === realCwd, + ), }) if (!coverageResult.ok) { return coverageResult @@ -865,42 +896,37 @@ export async function setupRecursiveManifestConfig( reenabled: 0, skipped: 0, } - // Settings are per directory, so a directory holding several POMs is asked - // about once and its answer counted for each. - const answersByDir = new Map< - string, - { outcome: CandidateOutcome; reenabled: boolean } - >() - const orderedIncluded = sortCandidatesForDisplay(included, realCwd) + // A parent is prompted before its children, so a child's shown default + // reflects the parent's just-written cascade. + const orderedIncluded = sortByDepthThenPath(included, realCwd) for (const candidate of orderedIncluded) { - if (isBuildRootCovered(covered, candidate.ecosystem, candidate)) { + const { ecosystem } = candidate + if ( + candidate.roots.every(root => + isBuildRootCovered(covered, ecosystem, root), + ) + ) { counts.covered += 1 continue } - const promptKey = `${candidate.ecosystem}:${candidate.dir}` - let answer = answersByDir.get(promptKey) - if (!answer) { - // eslint-disable-next-line no-await-in-loop - const result = await processCandidate({ - cwd: realCwd, - dir: candidate.dir, - ecosystem: candidate.ecosystem, - rootSockJson, - }) - if (!result.ok) { - return result - } - if (result.data.canceled) { - // The cancellation itself (select Esc/Ctrl+C, or a sub-wizard's own - // cancel) already logged "User canceled" - don't log it twice. - return { ok: true, data: { canceled: true } } - } - answer = result.data - answersByDir.set(promptKey, answer) + // eslint-disable-next-line no-await-in-loop + const result = await processCandidate({ + cwd: realCwd, + dir: candidate.dir, + ecosystem, + rootSockJson, + }) + if (!result.ok) { + return result + } + if (result.data.canceled) { + // The cancellation itself (select Esc/Ctrl+C, or a sub-wizard's own + // cancel) already logged "User canceled" - don't log it twice. + return { ok: true, data: { canceled: true } } } - counts[answer.outcome] += 1 - if (answer.reenabled) { + counts[result.data.outcome] += 1 + if (result.data.reenabled) { // A qualifier on the 'configured'/'inherited' bucket just incremented // above, not a separate bucket - avoids double-counting the candidate. counts.reenabled += 1 @@ -909,12 +935,13 @@ export async function setupRecursiveManifestConfig( // Interleaved rather than a bulk pre-pass, so this (possibly slow) // build-tool invocation only runs for a project already prompted about. // eslint-disable-next-line no-await-in-loop - const coverageResult = await markWorkspaceCoverage({ - candidate, + const coverageResult = await markRootsCoverage({ covered, cwd: realCwd, + ecosystem, excludePaths, rootSockJson, + roots: candidate.roots, }) if (!coverageResult.ok) { return coverageResult diff --git a/src/commands/manifest/setup-recursive-manifest-config.test.mts b/src/commands/manifest/setup-recursive-manifest-config.test.mts index ddc3f8756..eb1ef77e8 100644 --- a/src/commands/manifest/setup-recursive-manifest-config.test.mts +++ b/src/commands/manifest/setup-recursive-manifest-config.test.mts @@ -54,7 +54,6 @@ import { processCandidate, scanBuildRoots, setupRecursiveManifestConfig, - sortCandidatesForDisplay, } from './setup-recursive-manifest-config.mts' import { readOrDefaultSocketJson, @@ -78,6 +77,20 @@ function emptySockJson(): SocketJson { return { version: 1 } as SocketJson } +// Declines the root questions, opts into the individual walk and leaves each +// candidate inheriting. Returns the prompts asked, in order. +function inheritEveryCandidate(): string[] { + const messages: string[] = [] + vi.mocked(select).mockImplementation(async ({ message }) => { + messages.push(message) + if (message.startsWith('Configure other build roots')) { + return true + } + return message.startsWith('Configure') ? false : 'inherit' + }) + return messages +} + // In-memory `dir -> own gradle section` store backing readSocketJsonCascade/ // readOrDefaultSocketJson/writeSocketJson, so a write made while processing // one candidate is visible (via cascade) to a later candidate nested under @@ -130,24 +143,6 @@ function makeFakeGradleDisk(cwd: string) { } } -describe('sortCandidatesForDisplay', () => { - const cwd = '/repo' - - it('sorts shallower dirs before deeper ones, parent before child', () => { - const sorted = sortCandidatesForDisplay( - [ - { dir: '/repo/module-b/standalone-gradle-lib', ecosystems: ['gradle'] }, - { dir: '/repo/independent-service', ecosystems: ['maven'] }, - ], - cwd, - ) - expect(sorted).toEqual([ - { dir: '/repo/independent-service', ecosystems: ['maven'] }, - { dir: '/repo/module-b/standalone-gradle-lib', ecosystems: ['gradle'] }, - ]) - }) -}) - describe('scanBuildRoots', () => { const cwd = '/repo' @@ -205,7 +200,7 @@ describe('discoverBuildRoots', () => { expect(result).toEqual({ excluded: [{ dir: legacy, ecosystems: ['gradle'] }], - included: [{ dir: active, ecosystem: 'gradle' }], + included: [{ dir: active, ecosystem: 'gradle', roots: dirRoots(active) }], }) }) @@ -258,7 +253,9 @@ describe('discoverBuildRoots', () => { expect(result).toEqual({ excluded: [], - included: [{ dir: service, ecosystem: 'maven', entryFile: 'pom.xml' }], + included: [ + { dir: service, ecosystem: 'maven', roots: mavenRoots(service) }, + ], }) expect(warnSpy).toHaveBeenCalledWith( expect.stringContaining('Not disabling maven in service'), @@ -1179,22 +1176,7 @@ describe('setupRecursiveManifestConfig', () => { vi.mocked(findBuildToolCandidates).mockResolvedValue( new Map([['gradle', dirRoots(`${cwd}/active`)]]), ) - const messages: string[] = [] - vi.mocked(select).mockImplementation(async ({ message }) => { - messages.push(message) - // Configure Gradle defaults? -> no (gradle is detected via `active`, - // a nested project, not cwd itself). - if (message.startsWith('Configure Gradle defaults')) { - return false - } - // Configure other build roots individually? -> yes. - if (message.startsWith('Configure other build roots')) { - return true - } - // Candidate action prompt -> anything but 'configure' is a safe no-op - // (inherit), so this never touches writeSocketJson. - return 'inherit' - }) + const messages = inheritEveryCandidate() const logSpy = vi.spyOn(logger, 'log') try { @@ -1264,7 +1246,7 @@ describe('setupRecursiveManifestConfig', () => { ) }) - it('asks once about a directory holding several POMs, counting the answer for each', async () => { + it('asks once about a directory holding several POMs and enumerates each', async () => { const service = `${cwd}/service` vi.mocked(findBuildToolCandidates).mockResolvedValue( new Map([ @@ -1281,14 +1263,7 @@ describe('setupRecursiveManifestConfig', () => { buildRoot: dir, projects: [], })) - const messages: string[] = [] - vi.mocked(select).mockImplementation(async ({ message }) => { - messages.push(message) - if (message.startsWith('Configure other build roots')) { - return true - } - return message.startsWith('Configure') ? false : 'inherit' - }) + const messages = inheritEveryCandidate() const logSpy = vi.spyOn(logger, 'log') try { @@ -1297,10 +1272,10 @@ describe('setupRecursiveManifestConfig', () => { expect(result).toEqual({ ok: true, data: { canceled: false } }) expect(messages.filter(m => m === 'service (maven)')).toHaveLength(1) expect( - vi.mocked(enumerateWorkspaces).mock.calls.map(([o]) => o.buildOpts), - ).toEqual([['--file', 'app-dev.pom.xml'], []]) + vi.mocked(enumerateWorkspaces).mock.calls.map(([o]) => o.entryFile), + ).toEqual(['app-dev.pom.xml', 'pom.xml']) const logged = logSpy.mock.calls.map(c => String(c[0])).join('\n') - expect(logged).toMatch(/0 configured \(0 re-enabled\), 2 left as-is/) + expect(logged).toMatch(/0 configured \(0 re-enabled\), 1 left as-is/) } finally { logSpy.mockRestore() } @@ -1325,12 +1300,7 @@ describe('setupRecursiveManifestConfig', () => { buildRoot: dir, projects: [], })) - vi.mocked(select).mockImplementation(async ({ message }) => { - if (message.startsWith('Configure other build roots')) { - return true - } - return message.startsWith('Configure') ? false : 'inherit' - }) + inheritEveryCandidate() const result = await setupRecursiveManifestConfig(cwd, false, [ 'broken.pom.xml', @@ -1340,10 +1310,60 @@ describe('setupRecursiveManifestConfig', () => { expect( vi .mocked(enumerateWorkspaces) - .mock.calls.some(([o]) => o.buildOpts.includes('broken.pom.xml')), + .mock.calls.some(([o]) => o.entryFile === 'broken.pom.xml'), ).toBe(false) }) + it('does not enumerate a POM at cwd that another POM there already aggregates', async () => { + const cwd = await fs.realpath( + await fs.mkdtemp(path.join(tmpdir(), 'seed-coverage-poms-')), + ) + await fs.writeFile(path.join(cwd, 'all.pom.xml'), '') + await fs.writeFile(path.join(cwd, 'pom.xml'), '') + vi.mocked(findBuildToolCandidates).mockResolvedValue( + new Map([ + [ + 'maven', + [ + { dir: cwd, entryFile: 'all.pom.xml' }, + ...mavenRoots(cwd, `${cwd}/service`), + ], + ], + ]), + ) + vi.mocked(enumerateWorkspaces).mockImplementation(async ({ cwd: dir }) => ({ + buildRoot: dir, + projects: + dir === cwd + ? [ + { + type: 'maven', + name: 'core', + subprojectDir: '', + manifestFiles: [{ file: 'pom.xml' }], + children: [], + }, + ] + : [], + })) + inheritEveryCandidate() + + try { + await setupRecursiveManifestConfig(cwd, false) + + expect( + vi + .mocked(enumerateWorkspaces) + .mock.calls.map(([o]) => path.join(o.cwd, o.entryFile ?? '')), + ).toEqual([ + path.join(cwd, 'all.pom.xml'), + path.join(cwd, 'service/pom.xml'), + ]) + } finally { + await fs.rm(cwd, { recursive: true, force: true }) + } + }) + it('seeds coverage from cwd itself, so a reactor rooted exactly at cwd still gets its declared members pruned', async () => { const cwd = await fs.realpath( await fs.mkdtemp(path.join(tmpdir(), 'seed-coverage-')), @@ -1390,17 +1410,7 @@ describe('setupRecursiveManifestConfig', () => { } : { buildRoot: dir, projects: [] }, ) - const messages: string[] = [] - vi.mocked(select).mockImplementation(async ({ message }) => { - messages.push(message) - if (message.startsWith('Configure Maven settings')) { - return false - } - if (message.startsWith('Configure other build roots')) { - return true - } - return 'inherit' - }) + const messages = inheritEveryCandidate() try { const result = await setupRecursiveManifestConfig(cwd, false) @@ -1427,15 +1437,7 @@ describe('setupRecursiveManifestConfig', () => { vi.mocked(findBuildToolCandidates).mockResolvedValue( new Map([['maven', mavenRoots(independentService, laterCandidate)]]), ) - vi.mocked(select).mockImplementation(async ({ message }) => { - if (message.startsWith('Configure Maven defaults')) { - return false - } - if (message.startsWith('Configure other build roots')) { - return true - } - return 'inherit' - }) + inheritEveryCandidate() // Simulates a missing $JAVA8_HOME env var reference: enumeration fails // for independent-service specifically. vi.mocked(enumerateWorkspaces).mockImplementation(async ({ cwd: dir }) => From 971872d9bcfcd0a32fbf3d4f7c0bb68593afccd2 Mon Sep 17 00:00:00 2001 From: Jeppe Fredsgaard Blaabjerg Date: Tue, 27 Oct 2026 12:32:24 +0100 Subject: [PATCH 3/3] refactor(manifest): one build-file record per tool, per-directory DSI config - Move build-file names, the POM pattern and its case rule into one BUILD_FILES record; discovery walks narrow globs again and matches after. - Build every discovered POM with its own --file; a user -f/--file in mavenOpts is dropped with a warning during config resolution. - Resolve DSI and wizard settings once per directory. - Require socketConfig for discovery, loaded through loadSocketConfig. Co-Authored-By: Claude Opus 5.5 --- .../manifest/discover-manifest-roots.mts | 96 +++++++--------- .../manifest/discover-manifest-roots.test.mts | 13 ++- .../manifest/generate-recursive-manifests.mts | 103 ++++++++++++----- .../generate-recursive-manifests.test.mts | 43 ++++++++ src/commands/manifest/scripts/build-tool.mts | 41 ++++--- src/commands/manifest/scripts/run.mts | 15 +-- src/commands/manifest/scripts/run.test.mts | 2 +- .../setup-recursive-manifest-config.mts | 104 ++++++++---------- .../setup-recursive-manifest-config.test.mts | 65 +++++++++-- src/utils/config.mts | 6 + 10 files changed, 308 insertions(+), 180 deletions(-) diff --git a/src/commands/manifest/discover-manifest-roots.mts b/src/commands/manifest/discover-manifest-roots.mts index 6b82f1075..1b7f67555 100644 --- a/src/commands/manifest/discover-manifest-roots.mts +++ b/src/commands/manifest/discover-manifest-roots.mts @@ -3,7 +3,7 @@ import path from 'node:path' import micromatch from 'micromatch' -import { isAddressedByFile } from './scripts/build-tool.mts' +import { BUILD_FILES } from './scripts/build-tool.mts' import { realpathOrResolved } from '../../utils/fs.mts' import { globWithGitIgnore } from '../../utils/glob.mts' import { excludePathToScanIgnores } from '../scan/exclude-paths.mts' @@ -15,31 +15,15 @@ import type { SocketYml } from '@socketsecurity/config' const BUILD_TOOLS: BuildTool[] = ['gradle', 'maven', 'sbt'] -// One glob set per ecosystem's build-descriptor marker(s); `settings.gradle(.kts)` -// covers Kotlin-DSL roots that have no root `build.gradle`. Maven mirrors the -// `pomxml` pattern of the API's supported files, so every POM a scan uploads is -// also a build root here. -const MARKERS_BY_TOOL: Record = { - __proto__: null, - gradle: [ - 'build.gradle', - 'build.gradle.kts', - 'settings.gradle', - 'settings.gradle.kts', - ], - maven: ['{*-*.,}pom{.xml,}'], - sbt: ['build.sbt'], -} as unknown as Record - -// Case-insensitive for POMs, as a scan matches them; Gradle and sbt look for -// their build files by exact name. -const MARKER_MATCHERS_BY_TOOL = new Map( +const BUILD_FILE_MATCHERS = new Map( BUILD_TOOLS.map(tool => { - const options = { dot: true, nocase: tool === 'maven' } - const matchers = MARKERS_BY_TOOL[tool].map(marker => - micromatch.matcher(marker, options), - ) - return [tool, (basename: string) => matchers.some(m => m(basename))] + const files = BUILD_FILES[tool] + return [ + tool, + files.byFile + ? micromatch.matcher(files.nocaseGlob, { dot: true, nocase: true }) + : (basename: string) => files.names.includes(basename), + ] }), ) @@ -51,11 +35,11 @@ export function buildRootPath(root: BuildRoot): string { return path.join(root.dir, root.entryFile ?? '') } -function markerTool( +function buildFileTool( tools: readonly BuildTool[], basename: string, ): BuildTool | undefined { - return tools.find(tool => MARKER_MATCHERS_BY_TOOL.get(tool)?.(basename)) + return tools.find(tool => BUILD_FILE_MATCHERS.get(tool)?.(basename)) } // A disabled ecosystem is skipped entirely by the glob scan below, so a @@ -115,12 +99,11 @@ export function isBuildRootCovered( tool: BuildTool, root: BuildRoot, ): boolean { - if (root.entryFile) { - return covered.has(buildRootPath(root)) + const files = BUILD_FILES[tool] + if (files.byFile) { + return !!root.entryFile && covered.has(buildRootPath(root)) } - return MARKERS_BY_TOOL[tool].some(marker => - covered.has(path.join(root.dir, marker)), - ) + return files.names.some(name => covered.has(path.join(root.dir, name))) } function compareStrings(a: string, b: string): number { @@ -156,7 +139,7 @@ export async function findBuildToolCandidates({ excludePaths?: string[] | undefined // The socket.yml a scan applies to its upload, so a build file a scan would // not upload is not built either. - socketConfig?: SocketYml | undefined + socketConfig: SocketYml | undefined sockJson: SocketJson }): Promise> { const enabledTools = BUILD_TOOLS.filter( @@ -170,35 +153,36 @@ export async function findBuildToolCandidates({ const additionalIgnores = (excludePaths ?? []).flatMap( excludePathToScanIgnores, ) - // Markers are matched in the filter, after the ignores, so --exclude-paths - // and .gitignore stay case-sensitive as they are for a scan. - const hits = await globWithGitIgnore(['**/*'], { - absolute: true, - additionalIgnores, - cwd, - filter: hit => !!markerTool(enabledTools, path.basename(hit)), - socketConfig, - }) + // Build files are matched after the walk, as a scan matches them after its + // ignores, so --exclude-paths and .gitignore stay case-sensitive. + const hits = await globWithGitIgnore( + enabledTools.flatMap(tool => { + const files = BUILD_FILES[tool] + return (files.byFile ? [files.walkGlob] : files.names).map( + glob => `**/${glob}`, + ) + }), + { absolute: true, additionalIgnores, cwd, socketConfig }, + ) const rootsByTool = new Map>( enabledTools.map(tool => [tool, new Map()]), ) - const resolvedHits = await Promise.all( - hits.map(async hit => { - const basename = path.basename(hit) - return { - basename, + const matched = hits.flatMap(hit => { + const basename = path.basename(hit) + const tool = buildFileTool(enabledTools, basename) + return tool ? [{ basename, hit, tool }] : [] + }) + const roots = await Promise.all( + matched.map(async ({ basename, hit, tool }) => ({ + root: { dir: await realpathOrResolved(path.dirname(hit)), - // Set: the glob filter kept only marker hits. - tool: markerTool(enabledTools, basename) as BuildTool, - } - }), + entryFile: BUILD_FILES[tool].byFile ? basename : undefined, + }, + tool, + })), ) - for (const { basename, dir, tool } of resolvedHits) { - const root = { - dir, - entryFile: isAddressedByFile(tool) ? basename : undefined, - } + for (const { root, tool } of roots) { rootsByTool.get(tool)?.set(buildRootPath(root), root) } diff --git a/src/commands/manifest/discover-manifest-roots.test.mts b/src/commands/manifest/discover-manifest-roots.test.mts index 30cfab3ab..2b1c64a1e 100644 --- a/src/commands/manifest/discover-manifest-roots.test.mts +++ b/src/commands/manifest/discover-manifest-roots.test.mts @@ -11,7 +11,7 @@ import { sortByDepthThenPath, } from './discover-manifest-roots.mts' import { testPath } from '../../../test/utils.mts' -import { findSocketYmlSync } from '../../utils/config.mts' +import { loadSocketConfig } from '../../utils/config.mts' import type { BuildRoot } from './discover-manifest-roots.mts' import type { SocketJson } from '../../utils/socket-json.mts' @@ -35,6 +35,7 @@ async function pomNamesRoots(excludePaths?: string[]): Promise { const candidates = await findBuildToolCandidates({ cwd: pomNames, excludePaths, + socketConfig: undefined, sockJson: {} as SocketJson, }) return (candidates.get('maven') ?? []).map(r => @@ -57,10 +58,9 @@ async function rootsBesideIgnoredPom( ) await fs.writeFile(path.join(cwd, 'pom.xml'), '') await fs.writeFile(path.join(cwd, ignoreFile.name), ignoreFile.content) - const socketYml = findSocketYmlSync(cwd) const candidates = await findBuildToolCandidates({ cwd, - socketConfig: socketYml.ok ? socketYml.data?.parsed : undefined, + socketConfig: loadSocketConfig(cwd), sockJson: {} as SocketJson, }) return candidates @@ -79,6 +79,7 @@ describe('findBuildToolCandidates', () => { it('discovers maven, gradle, and sbt candidates in depth order', async () => { const candidates = await findBuildToolCandidates({ cwd: monorepo, + socketConfig: undefined, sockJson: {} as SocketJson, }) @@ -104,6 +105,7 @@ describe('findBuildToolCandidates', () => { it('lists a reactor root before its own members (depth-ascending)', async () => { const candidates = await findBuildToolCandidates({ cwd: monorepo, + socketConfig: undefined, sockJson: {} as SocketJson, }) const maven = dirsOf(candidates.get('maven')) @@ -118,6 +120,7 @@ describe('findBuildToolCandidates', () => { it('includes a gradle project nested inside a maven candidate directory tree', async () => { const candidates = await findBuildToolCandidates({ cwd: monorepo, + socketConfig: undefined, sockJson: {} as SocketJson, }) expect(dirsOf(candidates.get('gradle'))).toContain( @@ -128,6 +131,7 @@ describe('findBuildToolCandidates', () => { it('lists a dual-marker directory on both ecosystems', async () => { const candidates = await findBuildToolCandidates({ cwd: monorepo, + socketConfig: undefined, sockJson: {} as SocketJson, }) const dualDir = path.join(monorepo, 'dual-marker-dir') @@ -138,6 +142,7 @@ describe('findBuildToolCandidates', () => { it('drops a disabled ecosystem entirely', async () => { const candidates = await findBuildToolCandidates({ cwd: monorepo, + socketConfig: undefined, sockJson: { defaults: { manifest: { maven: { disabled: true } } }, } as SocketJson, @@ -150,6 +155,7 @@ describe('findBuildToolCandidates', () => { const candidates = await findBuildToolCandidates({ cwd: monorepo, excludePaths: ['reactor'], + socketConfig: undefined, sockJson: {} as SocketJson, }) expect(candidates.get('maven')).toEqual([ @@ -164,6 +170,7 @@ describe('findBuildToolCandidates', () => { it('lists a gradle directory once despite several markers', async () => { const candidates = await findBuildToolCandidates({ cwd: monorepo, + socketConfig: undefined, sockJson: {} as SocketJson, }) const standalone = path.join(monorepo, 'standalone-gradle') diff --git a/src/commands/manifest/generate-recursive-manifests.mts b/src/commands/manifest/generate-recursive-manifests.mts index a0a9f5416..dfdfee445 100644 --- a/src/commands/manifest/generate-recursive-manifests.mts +++ b/src/commands/manifest/generate-recursive-manifests.mts @@ -18,7 +18,7 @@ import { REACH_CONTINUE_ON_INSTALL_ERRORS_FLAG, } from './scripts/resolution-report-render.mts' import { SOCKET_JSON } from '../../constants.mts' -import { findSocketYmlSync } from '../../utils/config.mts' +import { loadSocketConfig } from '../../utils/config.mts' import { withTmpDir } from '../../utils/fs.mts' import { readOrDefaultSocketJson, @@ -84,6 +84,43 @@ function getSkipReason( type DisabledRoot = { dir: string; sockJson: SocketJson } +// Maven 3's options starting with `f`; its GnuParser takes a long name after +// one dash too, and splits `-fX` and `-f=X` into `-f X`. +const MAVEN_FILE_OPTS = new Set(['f', 'file']) +const MAVEN_OTHER_F_OPTS = new Set([ + 'fae', + 'fail-at-end', + 'fail-fast', + 'fail-never', + 'ff', + 'fn', +]) + +// Each POM is built by its own `--file`, so a user one is dropped. +function withoutMavenFileOpts(dir: string, opts: string[]): string[] { + const kept: string[] = [] + for (let i = 0, { length } = opts; i < length; i += 1) { + const opt = opts[i]! + const name = opt.replace(/^--?/, '') + if (opt === name || MAVEN_OTHER_F_OPTS.has(name)) { + kept.push(opt) + } else if (MAVEN_FILE_OPTS.has(name)) { + i += 1 + } else if ( + !MAVEN_FILE_OPTS.has(name.split('=')[0]!) && + !(opt.startsWith('-f') && !opt.startsWith('--')) + ) { + kept.push(opt) + } + } + if (kept.length !== opts.length) { + logger.warn( + `Ignoring --file in mavenOpts for ${dir}: each POM there is built by its own file.`, + ) + } + return kept +} + // Nearest already-confirmed-disabled `dir` or ancestor of it, if any - lets the // caller shorten `readSocketJsonCascade`'s walk to start there instead of // all the way back at `cwd`, without skipping any nested override. @@ -134,6 +171,10 @@ async function runEcosystemCandidates({ const covered = new Set() for (const [ecosystem, roots] of candidatesByTool) { const disabledRoots: DisabledRoot[] = [] + const configByDir = new Map< + string, + EcosystemBuildConfig & { excludePaths: string[] } + >() for (const { 0: rootIndex, 1: root } of roots.entries()) { const { dir, entryFile } = root if (isBuildRootCovered(covered, ecosystem, root)) { @@ -141,40 +182,50 @@ async function runEcosystemCandidates({ continue } - const nearestRoot = nearestDisabledRoot(dir, disabledRoots) - const sockJson = nearestRoot - ? readSocketJsonCascade(dir, nearestRoot.dir, nearestRoot.sockJson) - : readSocketJsonCascade(dir, realCwd, rootSockJson) + let dirConfig = configByDir.get(dir) + if (!dirConfig) { + const nearestRoot = nearestDisabledRoot(dir, disabledRoots) + const sockJson = nearestRoot + ? readSocketJsonCascade(dir, nearestRoot.dir, nearestRoot.sockJson) + : readSocketJsonCascade(dir, realCwd, rootSockJson) + dirConfig = { + ...resolveEcosystemConfig(ecosystem, dir, sockJson), + excludePaths: projectIgnorePathsToReachExcludePaths(excludePaths, { + cwd: realCwd, + target: dir, + }), + } + configByDir.set(dir, dirConfig) + if (dirConfig.skipReason) { + // Only warn for a genuinely new disabled root, not one already + // covered by an ancestor's warning above - otherwise a big disabled + // reactor with hundreds of nested poms would spam one warning line + // per pom for what's really a single root cause. The aggregate + // count still shows up in the final summary either way. + if (!nearestRoot) { + logger.warn( + `Skipping ${dir} (${ecosystem}): ${dirConfig.skipReason}.`, + ) + } + disabledRoots.push({ dir, sockJson }) + } + } const { bin, buildOpts, excludeConfigs, + excludePaths: excludePathsForRoot, ignoreUnresolved, includeConfigs, javaHome, skipReason, verbose: configVerbose, - } = resolveEcosystemConfig(ecosystem, dir, sockJson) - + } = dirConfig if (skipReason) { - // Only warn for a genuinely new disabled root, not one already - // covered by an ancestor's warning above - otherwise a big disabled - // reactor with hundreds of nested poms would spam one warning line - // per pom for what's really a single root cause. The aggregate - // count still shows up in the final summary either way. - if (!nearestRoot) { - logger.warn(`Skipping ${dir} (${ecosystem}): ${skipReason}.`) - } outcomes.push({ ...root, ecosystem, status: 'skippedDisabled' }) - disabledRoots.push({ dir, sockJson }) continue } - const excludePathsForRoot = projectIgnorePathsToReachExcludePaths( - excludePaths, - { cwd: realCwd, target: dir }, - ) - // eslint-disable-next-line no-await-in-loop const result = await runManifestFacts({ bin, @@ -219,7 +270,7 @@ async function runEcosystemCandidates({ continue } - // `-f`/`-p` can root the reactor away from `dir`. + // Gradle's `-p` can root the build away from `dir`. // eslint-disable-next-line no-await-in-loop await addCoveredBuildFiles( covered, @@ -280,11 +331,10 @@ export async function generateRecursiveManifests({ // A root-disabled ecosystem must still be scanned for - a nested socket.json // may re-enable it - so the per-directory cascade below, not this scan, is // what actually decides skip vs. include. - const socketYml = findSocketYmlSync(cwd) const candidatesByTool = await findBuildToolCandidates({ cwd, excludePaths, - socketConfig: socketYml.ok ? socketYml.data?.parsed : undefined, + socketConfig: loadSocketConfig(cwd), sockJson: withoutDisabledFlags(rootSockJson), }) @@ -362,7 +412,10 @@ export function resolveEcosystemConfig( const bin = config?.bin ?? undefined return { bin: bin ?? resolveBuildToolBin('maven', dir), - buildOpts: parseBuildToolOpts(config?.mavenOpts ?? undefined), + buildOpts: withoutMavenFileOpts( + dir, + parseBuildToolOpts(config?.mavenOpts ?? undefined), + ), excludeConfigs: config?.excludeConfigs ?? '', ignoreUnresolved: Boolean(config?.ignoreUnresolved), includeConfigs: config?.includeConfigs ?? '', diff --git a/src/commands/manifest/generate-recursive-manifests.test.mts b/src/commands/manifest/generate-recursive-manifests.test.mts index df90dae97..5bcf8afc2 100644 --- a/src/commands/manifest/generate-recursive-manifests.test.mts +++ b/src/commands/manifest/generate-recursive-manifests.test.mts @@ -1093,4 +1093,47 @@ describe('generateRecursiveManifests', () => { expect(statusOf('app-dev.pom.xml')).toBe('generated') expect(statusOf('bare/pom')).toBe('skippedCovered') }) + + it('resolves settings once per directory and drops a --file in mavenOpts with a warning', async () => { + vi.mocked(readSocketJsonCascade).mockClear() + vi.mocked(readOrDefaultSocketJson).mockReturnValue({ + defaults: { + manifest: { + maven: { + mavenOpts: + '--file other.xml -P ci -fae -fother.xml -fail-fast -file=x.xml', + }, + }, + }, + } as SocketJson) + vi.mocked(runManifestFacts).mockImplementation( + async ({ cwd, entryFile }) => ({ + factsPath: path.join(cwd, `${entryFile}.socket.facts.json`), + projects: [], + }), + ) + const warnSpy = vi.spyOn(logger, 'warn').mockImplementation(() => logger) + + try { + await generateRecursiveManifests({ cwd: pomNames, verbose: false }) + + const cascadeDirs = vi + .mocked(readSocketJsonCascade) + .mock.calls.map(([dir]) => dir) + expect(cascadeDirs.length).toBeGreaterThan(0) + expect(cascadeDirs).toEqual([...new Set(cascadeDirs)]) + expect( + warnSpy.mock.calls.filter(c => String(c[0]).includes('--file')), + ).toHaveLength(cascadeDirs.length) + expect( + vi.mocked(runManifestFacts).mock.calls.map(([o]) => o.buildOpts), + ).toEqual( + vi + .mocked(runManifestFacts) + .mock.calls.map(() => ['-P', 'ci', '-fae', '-fail-fast']), + ) + } finally { + warnSpy.mockRestore() + } + }) }) diff --git a/src/commands/manifest/scripts/build-tool.mts b/src/commands/manifest/scripts/build-tool.mts index 27e5b74e1..6416cc1fd 100644 --- a/src/commands/manifest/scripts/build-tool.mts +++ b/src/commands/manifest/scripts/build-tool.mts @@ -21,20 +21,33 @@ const BUILD_TOOL_WRAPPER = { maven: 'mvnw', } as unknown as Partial> -// Gradle (8+) and sbt hold one build per directory; Maven builds are addressed -// by POM file, so `mvn -f other-pom.xml` puts a second build in the directory. -const DEFAULT_ENTRY_FILE: Partial> = { - __proto__: null, - maven: 'pom.xml', -} as unknown as Partial> - -export function defaultEntryFile(tool: BuildTool): string | undefined { - return DEFAULT_ENTRY_FILE[tool] -} +// Maven builds by file (`mvn --file`), so a directory can hold several: the +// POMs a scan uploads, matched case-insensitively. `walkGlob` is a +// case-sensitive superset of `nocaseGlob`, for the walk. +export type BuildFiles = + | { byFile: false; names: readonly string[] } + | { byFile: true; nocaseGlob: string; walkGlob: string } -export function isAddressedByFile(tool: BuildTool): boolean { - return !!DEFAULT_ENTRY_FILE[tool] -} +// `settings.gradle(.kts)` covers Kotlin-DSL roots that have no root +// `build.gradle`. +export const BUILD_FILES: Record = { + __proto__: null, + gradle: { + byFile: false, + names: [ + 'build.gradle', + 'build.gradle.kts', + 'settings.gradle', + 'settings.gradle.kts', + ], + }, + maven: { + byFile: true, + nocaseGlob: '{*-*.,}pom{.xml,}', + walkGlob: '*[pP][oO][mM]*', + }, + sbt: { byFile: false, names: ['build.sbt'] }, +} as unknown as Record // sbt happily runs in any directory, synthesizing a default project from its // name, so an sbt run outside a build yields a plausible but bogus SBOM. Maven @@ -66,7 +79,7 @@ export function socketFactsFileName( tool: BuildTool, entryFile: string | undefined, ): string | undefined { - if (!isAddressedByFile(tool)) { + if (!BUILD_FILES[tool].byFile) { return `${tool}${constants.DOT_SOCKET_DOT_FACTS_JSON}` } return entryFile diff --git a/src/commands/manifest/scripts/run.mts b/src/commands/manifest/scripts/run.mts index 8f1d84ab5..e6c6e0db8 100644 --- a/src/commands/manifest/scripts/run.mts +++ b/src/commands/manifest/scripts/run.mts @@ -4,11 +4,7 @@ import path from 'node:path' import { spawn } from '@socketsecurity/registry/lib/spawn' import { assembleFacts } from './assemble.mts' -import { - defaultEntryFile, - resolveBuildToolBin, - socketFactsFileName, -} from './build-tool.mts' +import { resolveBuildToolBin, socketFactsFileName } from './build-tool.mts' import { serializeExcludePathPatterns } from './exclude-paths-glob.mts' import { parseRecords } from './records.mts' import constants from '../../../constants.mts' @@ -26,8 +22,7 @@ export type ManifestScriptOptions = { projectDir: string // Unset ⇒ resolved to the project wrapper, else PATH (resolveBuildToolBin). bin?: string | undefined - // Maven only: the POM in projectDir to build; unset or `pom.xml` builds the - // default, leaving a `--file` in toolOpts in effect. + // Maven only: the POM in projectDir to build; unset leaves Maven's default. entryFile?: string | undefined // Reachability-only: also materialize resolved artifact paths (artifactPaths). withFiles?: boolean | undefined @@ -396,11 +391,7 @@ async function invokeMaven( `-Dsocket.recordsFile=${recordsFile}`, ...commonProps(opts, '-D'), ] - // Ahead of toolOpts, as Maven 3 takes the first `--file`. - const fileOpts = - opts.entryFile && opts.entryFile !== defaultEntryFile('maven') - ? ['--file', opts.entryFile] - : [] + const fileOpts = opts.entryFile ? ['--file', opts.entryFile] : [] const args = [ ...props, ...fileOpts, diff --git a/src/commands/manifest/scripts/run.test.mts b/src/commands/manifest/scripts/run.test.mts index fdf247a0b..9192a699f 100644 --- a/src/commands/manifest/scripts/run.test.mts +++ b/src/commands/manifest/scripts/run.test.mts @@ -77,7 +77,7 @@ describe('shared sbt global base', () => { describe('maven entry file', () => { it.each([ ['app-dev.pom.xml', ['--file', 'app-dev.pom.xml', '-P', 'ci']], - ['pom.xml', ['-P', 'ci']], + ['pom.xml', ['--file', 'pom.xml', '-P', 'ci']], [undefined, ['-P', 'ci']], ] as const)( 'builds %s, ahead of the given options', diff --git a/src/commands/manifest/setup-recursive-manifest-config.mts b/src/commands/manifest/setup-recursive-manifest-config.mts index 18a2691c9..13e3f25fe 100644 --- a/src/commands/manifest/setup-recursive-manifest-config.mts +++ b/src/commands/manifest/setup-recursive-manifest-config.mts @@ -19,7 +19,7 @@ import { enumerateWorkspaces } from './enumerate-workspaces.mts' import { resolveEcosystemConfig } from './generate-recursive-manifests.mts' import { setupGradle, setupMaven, setupSbt } from './setup-manifest-config.mts' import { SOCKET_JSON } from '../../constants.mts' -import { findSocketYmlSync } from '../../utils/config.mts' +import { loadSocketConfig } from '../../utils/config.mts' import { readOrDefaultSocketJson, readSocketJsonCascade, @@ -36,8 +36,6 @@ import type { BuildTool } from './scripts/build-tool.mts' import type { CResult } from '../../types.mts' import type { SocketJson } from '../../utils/socket-json.mts' -type EcosystemRoot = BuildRoot & { ecosystem: BuildTool } - // One prompt per directory, as settings are per directory; Maven may have // several POMs, so several roots, there. type Candidate = { dir: string; ecosystem: BuildTool; roots: BuildRoot[] } @@ -115,8 +113,7 @@ export async function scanBuildRoots({ excludePaths?: string[] | undefined sockJson: SocketJson }): Promise { - const socketYml = findSocketYmlSync(cwd) - const socketConfig = socketYml.ok ? socketYml.data?.parsed : undefined + const socketConfig = loadSocketConfig(cwd) const [fullByTool, includedByTool] = await Promise.all([ findBuildToolCandidates({ cwd, socketConfig, sockJson }), findBuildToolCandidates({ cwd, excludePaths, socketConfig, sockJson }), @@ -207,88 +204,71 @@ export async function discoverBuildRoots({ } // Fails closed: without the members' build files, covered and independent -// candidates can't be told apart. +// candidates can't be told apart. In order, as a root's enumeration can cover +// the roots after it. export async function markWorkspaceCoverage({ - candidate, covered, cwd, + dir, + ecosystem, excludePaths, rootSockJson, + roots, }: { - candidate: EcosystemRoot covered: Set cwd: string + dir: string + ecosystem: BuildTool excludePaths?: string[] | undefined rootSockJson: SocketJson + roots: readonly BuildRoot[] }): Promise> { - const cascade = readSocketJsonCascade(candidate.dir, cwd, rootSockJson) + if (roots.every(root => isBuildRootCovered(covered, ecosystem, root))) { + return { ok: true, data: undefined } + } + const cascade = readSocketJsonCascade(dir, cwd, rootSockJson) const { bin, buildOpts, javaHome, skipReason } = resolveEcosystemConfig( - candidate.ecosystem, - candidate.dir, + ecosystem, + dir, cascade, ) if (skipReason) { return { ok: true, data: undefined } } - const excludePathsForCandidate = projectIgnorePathsToReachExcludePaths( + const excludePathsForDir = projectIgnorePathsToReachExcludePaths( excludePaths, - { cwd, target: candidate.dir }, + { cwd, target: dir }, ) - const enumResult = await enumerateWorkspaces({ - bin, - buildOpts, - cwd: candidate.dir, - ecosystem: candidate.ecosystem, - entryFile: candidate.entryFile, - excludePaths: excludePathsForCandidate, - javaHome, - verbose: false, - }) - if (!enumResult) { - const relRoot = path.relative(cwd, buildRootPath(candidate)) || '.' - return { - ok: false, - message: `Could not determine ${candidate.ecosystem} workspace layout for ${relRoot}; aborting rather than risk misclassifying its members.`, - } - } - - await addCoveredBuildFiles(covered, enumResult.buildRoot, enumResult.projects) - return { ok: true, data: undefined } -} - -// In order, as a root's enumeration can cover the roots after it. -async function markRootsCoverage({ - covered, - cwd, - ecosystem, - excludePaths, - rootSockJson, - roots, -}: { - covered: Set - cwd: string - ecosystem: BuildTool - excludePaths?: string[] | undefined - rootSockJson: SocketJson - roots: readonly BuildRoot[] -}): Promise> { for (const root of roots) { if (isBuildRootCovered(covered, ecosystem, root)) { continue } // eslint-disable-next-line no-await-in-loop - const result = await markWorkspaceCoverage({ - candidate: { ...root, ecosystem }, - covered, - cwd, - excludePaths, - rootSockJson, + const enumResult = await enumerateWorkspaces({ + bin, + buildOpts, + cwd: dir, + ecosystem, + entryFile: root.entryFile, + excludePaths: excludePathsForDir, + javaHome, + verbose: false, }) - if (!result.ok) { - return result + if (!enumResult) { + const relRoot = path.relative(cwd, buildRootPath(root)) || '.' + return { + ok: false, + message: `Could not determine ${ecosystem} workspace layout for ${relRoot}; aborting rather than risk misclassifying its members.`, + } } + // eslint-disable-next-line no-await-in-loop + await addCoveredBuildFiles( + covered, + enumResult.buildRoot, + enumResult.projects, + ) } return { ok: true, data: undefined } } @@ -874,9 +854,10 @@ export async function setupRecursiveManifestConfig( // recognized. for (const ecosystem of ROOT_ECOSYSTEMS) { // eslint-disable-next-line no-await-in-loop - const coverageResult = await markRootsCoverage({ + const coverageResult = await markWorkspaceCoverage({ covered, cwd: realCwd, + dir: realCwd, ecosystem, excludePaths, rootSockJson, @@ -935,9 +916,10 @@ export async function setupRecursiveManifestConfig( // Interleaved rather than a bulk pre-pass, so this (possibly slow) // build-tool invocation only runs for a project already prompted about. // eslint-disable-next-line no-await-in-loop - const coverageResult = await markRootsCoverage({ + const coverageResult = await markWorkspaceCoverage({ covered, cwd: realCwd, + dir: candidate.dir, ecosystem, excludePaths, rootSockJson, diff --git a/src/commands/manifest/setup-recursive-manifest-config.test.mts b/src/commands/manifest/setup-recursive-manifest-config.test.mts index eb1ef77e8..8358f4563 100644 --- a/src/commands/manifest/setup-recursive-manifest-config.test.mts +++ b/src/commands/manifest/setup-recursive-manifest-config.test.mts @@ -43,7 +43,10 @@ vi.mock('../../utils/socket-json.mts', () => ({ import { logger } from '@socketsecurity/registry/lib/logger' import { select } from '@socketsecurity/registry/lib/prompts' -import { findBuildToolCandidates } from './discover-manifest-roots.mts' +import { + buildRootPath, + findBuildToolCandidates, +} from './discover-manifest-roots.mts' import { enumerateWorkspaces } from './enumerate-workspaces.mts' import { setupGradle, setupMaven, setupSbt } from './setup-manifest-config.mts' import { @@ -335,10 +338,12 @@ describe('markWorkspaceCoverage', () => { const covered = new Set() const result = await markWorkspaceCoverage({ - candidate: { dir: reactor, ecosystem: 'maven' }, covered, cwd, + dir: reactor, + ecosystem: 'maven', rootSockJson: emptySockJson(), + roots: [{ dir: reactor, entryFile: 'pom.xml' }], }) expect(result).toEqual({ ok: true, data: undefined }) @@ -367,10 +372,12 @@ describe('markWorkspaceCoverage', () => { const covered = new Set() await markWorkspaceCoverage({ - candidate: { dir: reactor, ecosystem: 'maven' }, covered, cwd, + dir: reactor, + ecosystem: 'maven', rootSockJson: emptySockJson(), + roots: [{ dir: reactor, entryFile: 'pom.xml' }], }) expect(covered).toEqual(new Set([`${reactor}/build/member/pom.xml`])) @@ -399,10 +406,12 @@ describe('markWorkspaceCoverage', () => { const covered = new Set() await markWorkspaceCoverage({ - candidate: { dir: reactor, ecosystem: 'maven' }, covered, cwd, + dir: reactor, + ecosystem: 'maven', rootSockJson: emptySockJson(), + roots: [{ dir: reactor, entryFile: 'pom.xml' }], }) expect(covered).toEqual(new Set([`${reactor}/moduleA/pom.xml`])) @@ -416,10 +425,12 @@ describe('markWorkspaceCoverage', () => { const covered = new Set() const result = await markWorkspaceCoverage({ - candidate: { dir: reactor, ecosystem: 'maven' }, covered, cwd, + dir: reactor, + ecosystem: 'maven', rootSockJson: emptySockJson(), + roots: [{ dir: reactor, entryFile: 'pom.xml' }], }) expect(result).toEqual({ ok: true, data: undefined }) @@ -432,16 +443,50 @@ describe('markWorkspaceCoverage', () => { const covered = new Set() const result = await markWorkspaceCoverage({ - candidate: { dir: reactor, ecosystem: 'maven' }, covered, cwd, + dir: reactor, + ecosystem: 'maven', rootSockJson: emptySockJson(), + roots: [{ dir: reactor, entryFile: 'pom.xml' }], }) expect(result.ok).toBe(false) expect(covered.size).toBe(0) }) + it('resolves settings once per directory and skips a POM an earlier one covers', async () => { + vi.mocked(enumerateWorkspaces).mockResolvedValue({ + buildRoot: reactor, + projects: [ + { + type: 'maven', + name: 'reactor', + subprojectDir: '.', + manifestFiles: [{ file: 'pom.xml' }], + children: [], + }, + ], + }) + + await markWorkspaceCoverage({ + covered: new Set(), + cwd, + dir: reactor, + ecosystem: 'maven', + rootSockJson: emptySockJson(), + roots: [ + { dir: reactor, entryFile: 'all.pom.xml' }, + { dir: reactor, entryFile: 'pom.xml' }, + ], + }) + + expect(readSocketJsonCascade).toHaveBeenCalledTimes(1) + expect( + vi.mocked(enumerateWorkspaces).mock.calls.map(([o]) => o.entryFile), + ).toEqual(['all.pom.xml']) + }) + it('forwards --exclude-paths, re-anchored to the candidate dir, so a member excluded specifically to dodge a broken resolution is actually skipped', async () => { vi.mocked(enumerateWorkspaces).mockResolvedValue({ buildRoot: reactor, @@ -449,11 +494,13 @@ describe('markWorkspaceCoverage', () => { }) await markWorkspaceCoverage({ - candidate: { dir: reactor, ecosystem: 'maven' }, covered: new Set(), cwd, + dir: reactor, + ecosystem: 'maven', excludePaths: ['reactor/moduleB'], rootSockJson: emptySockJson(), + roots: [{ dir: reactor, entryFile: 'pom.xml' }], }) expect(enumerateWorkspaces).toHaveBeenCalledWith( @@ -1354,7 +1401,9 @@ describe('setupRecursiveManifestConfig', () => { expect( vi .mocked(enumerateWorkspaces) - .mock.calls.map(([o]) => path.join(o.cwd, o.entryFile ?? '')), + .mock.calls.map(([o]) => + buildRootPath({ dir: o.cwd, entryFile: o.entryFile }), + ), ).toEqual([ path.join(cwd, 'all.pom.xml'), path.join(cwd, 'service/pom.xml'), diff --git a/src/utils/config.mts b/src/utils/config.mts index 3d3de8c2f..57486404a 100644 --- a/src/utils/config.mts +++ b/src/utils/config.mts @@ -17,6 +17,7 @@ * Key Functions: * - findSocketYmlSync: Locate socket.yml configuration file * - getConfigValue: Retrieve configuration value by key + * - loadSocketConfig: Parsed socket.yml, if one is found and valid * - overrideCachedConfig: Apply temporary config overrides * - updateConfigValue: Persist configuration changes */ @@ -245,6 +246,11 @@ let _cachedConfig: LocalConfig | undefined // When using --config or SOCKET_CLI_CONFIG, do not persist the config. let _configFromFlag = false +export function loadSocketConfig(dir: string): SocketYml | undefined { + const result = findSocketYmlSync(dir) + return result.ok ? result.data?.parsed : undefined +} + /** * Reset config cache for testing purposes. * This allows tests to start with a fresh config state.