Skip to content

Commit f7ae575

Browse files
jfblaaclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 386e1a7 commit f7ae575

12 files changed

Lines changed: 390 additions & 345 deletions

‎src/commands/manifest/discover-manifest-roots.mts‎

Lines changed: 43 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -3,31 +3,22 @@ import path from 'node:path'
33

44
import micromatch from 'micromatch'
55

6-
import { defaultEntryFile, isAddressedByFile } from './scripts/build-tool.mts'
7-
import { findSocketYmlSync } from '../../utils/config.mts'
6+
import { isAddressedByFile } from './scripts/build-tool.mts'
87
import { realpathOrResolved } from '../../utils/fs.mts'
98
import { globWithGitIgnore } from '../../utils/glob.mts'
109
import { excludePathToScanIgnores } from '../scan/exclude-paths.mts'
1110

1211
import type { BuildTool } from './scripts/build-tool.mts'
1312
import type { SocketFactsSbomProject } from './scripts/facts.mts'
1413
import type { SocketJson } from '../../utils/socket-json.mts'
14+
import type { SocketYml } from '@socketsecurity/config'
1515

1616
const BUILD_TOOLS: BuildTool[] = ['gradle', 'maven', 'sbt']
1717

18-
// In the pattern, not the walk, whose ignores (--exclude-paths, .gitignore)
19-
// stay case-sensitive. Assumes every letter in the pattern is a literal.
20-
function foldCase(pattern: string): string {
21-
return pattern.replace(
22-
/[a-z]/gi,
23-
c => `[${c.toLowerCase()}${c.toUpperCase()}]`,
24-
)
25-
}
26-
27-
// One glob-suffix set per ecosystem's build-descriptor marker(s); `settings.gradle(.kts)`
18+
// One glob set per ecosystem's build-descriptor marker(s); `settings.gradle(.kts)`
2819
// covers Kotlin-DSL roots that have no root `build.gradle`. Maven mirrors the
29-
// `pomxml` pattern of the API's supported files, case-insensitively as a scan
30-
// matches it, so every POM a scan uploads is also a build root here.
20+
// `pomxml` pattern of the API's supported files, so every POM a scan uploads is
21+
// also a build root here.
3122
const MARKERS_BY_TOOL: Record<BuildTool, string[]> = {
3223
__proto__: null,
3324
gradle: [
@@ -36,18 +27,21 @@ const MARKERS_BY_TOOL: Record<BuildTool, string[]> = {
3627
'settings.gradle',
3728
'settings.gradle.kts',
3829
],
39-
maven: [foldCase('{*-*.,}pom{.xml,}')],
30+
maven: ['{*-*.,}pom{.xml,}'],
4031
sbt: ['build.sbt'],
4132
} as unknown as Record<BuildTool, string[]>
4233

43-
const MARKER_MATCHERS_BY_TOOL = Object.fromEntries(
44-
BUILD_TOOLS.map(tool => [
45-
tool,
46-
MARKERS_BY_TOOL[tool].map(marker =>
47-
micromatch.matcher(marker, { dot: true }),
48-
),
49-
]),
50-
) as Record<BuildTool, Array<(basename: string) => boolean>>
34+
// Case-insensitive for POMs, as a scan matches them; Gradle and sbt look for
35+
// their build files by exact name.
36+
const MARKER_MATCHERS_BY_TOOL = new Map(
37+
BUILD_TOOLS.map(tool => {
38+
const options = { dot: true, nocase: tool === 'maven' }
39+
const matchers = MARKERS_BY_TOOL[tool].map(marker =>
40+
micromatch.matcher(marker, options),
41+
)
42+
return [tool, (basename: string) => matchers.some(m => m(basename))]
43+
}),
44+
)
5145

5246
// `entryFile` (a basename in `dir`) is set only for a tool that addresses its
5347
// builds by file, where one directory can hold several builds.
@@ -61,9 +55,7 @@ function markerTool(
6155
tools: readonly BuildTool[],
6256
basename: string,
6357
): BuildTool | undefined {
64-
return tools.find(tool =>
65-
MARKER_MATCHERS_BY_TOOL[tool].some(matches => matches(basename)),
66-
)
58+
return tools.find(tool => MARKER_MATCHERS_BY_TOOL.get(tool)?.(basename))
6759
}
6860

6961
// A disabled ecosystem is skipped entirely by the glob scan below, so a
@@ -123,33 +115,32 @@ export function isBuildRootCovered(
123115
tool: BuildTool,
124116
root: BuildRoot,
125117
): boolean {
126-
const entryFile = root.entryFile ?? defaultEntryFile(tool)
127-
if (entryFile) {
128-
return covered.has(path.join(root.dir, entryFile))
118+
if (root.entryFile) {
119+
return covered.has(buildRootPath(root))
129120
}
130121
return MARKERS_BY_TOOL[tool].some(marker =>
131122
covered.has(path.join(root.dir, marker)),
132123
)
133124
}
134125

135-
function sortByDepthThenPath(
136-
roots: readonly BuildRoot[],
126+
function compareStrings(a: string, b: string): number {
127+
return a < b ? -1 : a > b ? 1 : 0
128+
}
129+
130+
export function sortByDepthThenPath<T extends BuildRoot>(
131+
roots: readonly T[],
137132
cwd: string,
138-
): BuildRoot[] {
133+
): T[] {
139134
return [...roots].sort((a, b) => {
140135
const relA = path.relative(cwd, a.dir)
141136
const relB = path.relative(cwd, b.dir)
142137
const depthA = relA === '' ? 0 : relA.split(path.sep).length
143138
const depthB = relB === '' ? 0 : relB.split(path.sep).length
144-
if (depthA !== depthB) {
145-
return depthA - depthB
146-
}
147-
if (relA !== relB) {
148-
return relA < relB ? -1 : 1
149-
}
150-
const entryA = a.entryFile ?? ''
151-
const entryB = b.entryFile ?? ''
152-
return entryA < entryB ? -1 : entryA > entryB ? 1 : 0
139+
return (
140+
depthA - depthB ||
141+
compareStrings(relA, relB) ||
142+
compareStrings(a.entryFile ?? '', b.entryFile ?? '')
143+
)
153144
})
154145
}
155146

@@ -159,9 +150,13 @@ export async function findBuildToolCandidates({
159150
cwd,
160151
excludePaths,
161152
sockJson,
153+
socketConfig,
162154
}: {
163155
cwd: string
164156
excludePaths?: string[] | undefined
157+
// The socket.yml a scan applies to its upload, so a build file a scan would
158+
// not upload is not built either.
159+
socketConfig?: SocketYml | undefined
165160
sockJson: SocketJson
166161
}): Promise<Map<BuildTool, BuildRoot[]>> {
167162
const enabledTools = BUILD_TOOLS.filter(
@@ -172,20 +167,17 @@ export async function findBuildToolCandidates({
172167
return result
173168
}
174169

175-
const patterns = enabledTools.flatMap(tool =>
176-
MARKERS_BY_TOOL[tool].map(marker => `**/${marker}`),
177-
)
178170
const additionalIgnores = (excludePaths ?? []).flatMap(
179171
excludePathToScanIgnores,
180172
)
181-
// The same socket.yml a scan applies to its upload, so a build file a scan
182-
// would not upload is not built either.
183-
const socketYml = findSocketYmlSync(cwd)
184-
const hits = await globWithGitIgnore(patterns, {
173+
// Markers are matched in the filter, after the ignores, so --exclude-paths
174+
// and .gitignore stay case-sensitive as they are for a scan.
175+
const hits = await globWithGitIgnore(['**/*'], {
185176
absolute: true,
186177
additionalIgnores,
187178
cwd,
188-
socketConfig: socketYml.ok ? socketYml.data?.parsed : undefined,
179+
filter: hit => !!markerTool(enabledTools, path.basename(hit)),
180+
socketConfig,
189181
})
190182

191183
const rootsByTool = new Map<BuildTool, Map<string, BuildRoot>>(
@@ -197,14 +189,12 @@ export async function findBuildToolCandidates({
197189
return {
198190
basename,
199191
dir: await realpathOrResolved(path.dirname(hit)),
200-
tool: markerTool(enabledTools, basename),
192+
// Set: the glob filter kept only marker hits.
193+
tool: markerTool(enabledTools, basename) as BuildTool,
201194
}
202195
}),
203196
)
204197
for (const { basename, dir, tool } of resolvedHits) {
205-
if (!tool) {
206-
continue
207-
}
208198
const root = {
209199
dir,
210200
entryFile: isAddressedByFile(tool) ? basename : undefined,

‎src/commands/manifest/discover-manifest-roots.test.mts‎

Lines changed: 69 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,13 @@ import path from 'node:path'
55
import { describe, expect, it } from 'vitest'
66

77
import {
8+
buildRootPath,
89
findBuildToolCandidates,
910
isBuildRootCovered,
11+
sortByDepthThenPath,
1012
} from './discover-manifest-roots.mts'
1113
import { testPath } from '../../../test/utils.mts'
14+
import { findSocketYmlSync } from '../../utils/config.mts'
1215

1316
import type { BuildRoot } from './discover-manifest-roots.mts'
1417
import type { SocketJson } from '../../utils/socket-json.mts'
@@ -35,12 +38,39 @@ async function pomNamesRoots(excludePaths?: string[]): Promise<string[]> {
3538
sockJson: {} as SocketJson,
3639
})
3740
return (candidates.get('maven') ?? []).map(r =>
38-
path
39-
.relative(pomNames, path.join(r.dir, r.entryFile ?? ''))
40-
.replaceAll('\\', '/'),
41+
path.relative(pomNames, buildRootPath(r)).replaceAll('\\', '/'),
4142
)
4243
}
4344

45+
async function rootsBesideIgnoredPom(
46+
ignoredPomDir: string,
47+
ignoreFile: { content: string; name: string },
48+
): Promise<BuildRoot[] | undefined> {
49+
const cwd = await fs.realpath(
50+
await fs.mkdtemp(path.join(tmpdir(), 'ignored-poms-')),
51+
)
52+
try {
53+
await fs.mkdir(path.join(cwd, ignoredPomDir), { recursive: true })
54+
await fs.writeFile(
55+
path.join(cwd, ignoredPomDir, 'lib-1.0.pom'),
56+
'<project/>',
57+
)
58+
await fs.writeFile(path.join(cwd, 'pom.xml'), '<project/>')
59+
await fs.writeFile(path.join(cwd, ignoreFile.name), ignoreFile.content)
60+
const socketYml = findSocketYmlSync(cwd)
61+
const candidates = await findBuildToolCandidates({
62+
cwd,
63+
socketConfig: socketYml.ok ? socketYml.data?.parsed : undefined,
64+
sockJson: {} as SocketJson,
65+
})
66+
return candidates
67+
.get('maven')
68+
?.map(root => ({ ...root, dir: path.relative(cwd, root.dir) }))
69+
} finally {
70+
await fs.rm(cwd, { recursive: true, force: true })
71+
}
72+
}
73+
4474
function dirsOf(roots: BuildRoot[] | undefined): string[] {
4575
return (roots ?? []).map(r => r.dir)
4676
}
@@ -154,53 +184,21 @@ describe('findBuildToolCandidates', () => {
154184
})
155185

156186
it('skips POMs a .gitignore covers, as a scan does when uploading', async () => {
157-
const cwd = await fs.realpath(
158-
await fs.mkdtemp(path.join(tmpdir(), 'gitignored-poms-')),
159-
)
160-
try {
161-
const repoDir = path.join(cwd, '.m2/repository/com/example/lib/1.0')
162-
await fs.mkdir(repoDir, { recursive: true })
163-
await fs.writeFile(path.join(repoDir, 'lib-1.0.pom'), '<project/>')
164-
await fs.writeFile(path.join(cwd, 'pom.xml'), '<project/>')
165-
await fs.writeFile(path.join(cwd, '.gitignore'), '.m2/\n')
166-
167-
const candidates = await findBuildToolCandidates({
168-
cwd,
169-
sockJson: {} as SocketJson,
170-
})
171-
172-
expect(candidates.get('maven')).toEqual([
173-
{ dir: cwd, entryFile: 'pom.xml' },
174-
])
175-
} finally {
176-
await fs.rm(cwd, { recursive: true, force: true })
177-
}
187+
expect(
188+
await rootsBesideIgnoredPom('.m2/repository/com/example/lib/1.0', {
189+
content: '.m2/\n',
190+
name: '.gitignore',
191+
}),
192+
).toEqual([{ dir: '', entryFile: 'pom.xml' }])
178193
})
179194

180195
it("skips POMs socket.yml's projectIgnorePaths covers, as a scan does when uploading", async () => {
181-
const cwd = await fs.realpath(
182-
await fs.mkdtemp(path.join(tmpdir(), 'project-ignored-poms-')),
183-
)
184-
try {
185-
await fs.mkdir(path.join(cwd, 'libs/repo'), { recursive: true })
186-
await fs.writeFile(path.join(cwd, 'libs/repo/lib-1.0.pom'), '<project/>')
187-
await fs.writeFile(path.join(cwd, 'pom.xml'), '<project/>')
188-
await fs.writeFile(
189-
path.join(cwd, 'socket.yml'),
190-
'version: 2\nprojectIgnorePaths:\n - libs/repo\n',
191-
)
192-
193-
const candidates = await findBuildToolCandidates({
194-
cwd,
195-
sockJson: {} as SocketJson,
196-
})
197-
198-
expect(candidates.get('maven')).toEqual([
199-
{ dir: cwd, entryFile: 'pom.xml' },
200-
])
201-
} finally {
202-
await fs.rm(cwd, { recursive: true, force: true })
203-
}
196+
expect(
197+
await rootsBesideIgnoredPom('libs/repo', {
198+
content: 'version: 2\nprojectIgnorePaths:\n - libs/repo\n',
199+
name: 'socket.yml',
200+
}),
201+
).toEqual([{ dir: '', entryFile: 'pom.xml' }])
204202
})
205203

206204
it('respects --exclude-paths for non-default POMs, case-sensitively', async () => {
@@ -234,17 +232,7 @@ describe('isBuildRootCovered', () => {
234232
).toBe(false)
235233
})
236234

237-
it('covers a root without an entry file through its default entry file or a marker file', () => {
238-
expect(
239-
isBuildRootCovered(new Set(['/repo/service/pom.xml']), 'maven', {
240-
dir: '/repo/service',
241-
}),
242-
).toBe(true)
243-
expect(
244-
isBuildRootCovered(new Set(['/repo/service/lib-1.0.pom']), 'maven', {
245-
dir: '/repo/service',
246-
}),
247-
).toBe(false)
235+
it('covers a root without an entry file through a marker file', () => {
248236
expect(
249237
isBuildRootCovered(new Set(['/repo/lib/settings.gradle.kts']), 'gradle', {
250238
dir: '/repo/lib',
@@ -257,3 +245,26 @@ describe('isBuildRootCovered', () => {
257245
).toBe(false)
258246
})
259247
})
248+
249+
describe('sortByDepthThenPath', () => {
250+
it('sorts shallower dirs first, then by path, then by entry file', () => {
251+
expect(
252+
sortByDepthThenPath(
253+
[
254+
{ dir: '/repo/b/nested' },
255+
{ dir: '/repo/b', entryFile: 'pom.xml' },
256+
{ dir: '/repo/b', entryFile: 'app-dev.pom.xml' },
257+
{ dir: '/repo/a' },
258+
{ dir: '/repo' },
259+
],
260+
'/repo',
261+
),
262+
).toEqual([
263+
{ dir: '/repo' },
264+
{ dir: '/repo/a' },
265+
{ dir: '/repo/b', entryFile: 'app-dev.pom.xml' },
266+
{ dir: '/repo/b', entryFile: 'pom.xml' },
267+
{ dir: '/repo/b/nested' },
268+
])
269+
})
270+
})

‎src/commands/manifest/enumerate-workspaces.mts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ export async function enumerateWorkspaces({
2424
buildOpts,
2525
cwd,
2626
ecosystem,
27+
entryFile,
2728
excludePaths,
2829
javaHome,
2930
verbose,
@@ -32,6 +33,8 @@ export async function enumerateWorkspaces({
3233
buildOpts: string[]
3334
cwd: string
3435
ecosystem: BuildTool
36+
// See ManifestScriptOptions.entryFile.
37+
entryFile?: string | undefined
3538
excludePaths?: string[] | undefined
3639
javaHome?: string | undefined
3740
verbose: boolean
@@ -51,6 +54,7 @@ export async function enumerateWorkspaces({
5154

5255
const scriptOpts = {
5356
bin: bin || undefined,
57+
entryFile,
5458
excludePaths: excludePaths?.length ? excludePaths : undefined,
5559
// `env` replaces the spawned process's whole environment, not just JAVA_HOME.
5660
env: resolvedJavaHome

0 commit comments

Comments
 (0)