Skip to content

Commit d366a78

Browse files
ulemonsulemons
committed
fix: skip bypass permissions
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
1 parent c9b870a commit d366a78

11 files changed

Lines changed: 161 additions & 104 deletions

File tree

‎backend/src/api/public/v1/akrites-external/index.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,10 +73,9 @@ export function akritesExternalRouter(): Router {
7373
// (same as advisories above — see the scope-naming note in the akrites-external
7474
// OpenAPI). Not issued by Auth0 yet, so reuse READ_PACKAGES for now.
7575
const blastRadiusSubRouter = Router()
76-
blastRadiusSubRouter.use(blastRadiusRateLimiter)
7776
blastRadiusSubRouter.use(requireScopes([SCOPES.READ_PACKAGES]))
78-
blastRadiusSubRouter.post('/jobs', safeWrap(submitBlastRadiusJob))
79-
blastRadiusSubRouter.get('/jobs/:analysisId', safeWrap(getBlastRadiusJob))
77+
blastRadiusSubRouter.post('/jobs', blastRadiusRateLimiter, safeWrap(submitBlastRadiusJob))
78+
blastRadiusSubRouter.get('/jobs/:analysisId', rateLimiter, safeWrap(getBlastRadiusJob))
8079
router.use('/blast-radius', blastRadiusSubRouter)
8180

8281
return router

‎backend/src/api/public/v1/akrites-external/openapi.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ components:
470470
properties:
471471
totalDependentsInRange:
472472
type: integer
473-
description: Dependents considered after the phase-1 range filter (Stage 2's population before its top-N cutoff).
473+
description: Sum of dependentsAnalyzed and dependentsExcludedUpfront; the count of dependents that fell within the vulnerable version range (before top-N cutoff was applied).
474474
dependentsExcludedUpfront:
475475
type: integer
476476
description: totalDependentsInRange minus dependentsAnalyzed.

‎backend/src/api/public/v1/packages/submitBlastRadiusJob.test.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@ import { describe, expect, it, vi } from 'vitest'
33

44
import { submitBlastRadiusJob } from './submitBlastRadiusJob'
55

6-
const { start, createAnalysis } = vi.hoisted(() => ({
6+
const { start, createAnalysis, failAnalysis } = vi.hoisted(() => ({
77
start: vi.fn().mockResolvedValue(undefined),
88
createAnalysis: vi.fn().mockResolvedValue(undefined),
9+
failAnalysis: vi.fn().mockResolvedValue(undefined),
910
}))
1011

1112
vi.mock('@/db/packagesTemporal', () => ({
@@ -18,11 +19,13 @@ vi.mock('@/db/packagesDb', () => ({
1819

1920
vi.mock('@crowd/data-access-layer/src/packages/blastRadius', () => ({
2021
createAnalysis,
22+
failAnalysis,
2123
}))
2224

2325
function mockReqRes(body: unknown) {
2426
start.mockClear()
2527
createAnalysis.mockClear()
28+
failAnalysis.mockClear()
2629

2730
const req = { body } as unknown as Request
2831

@@ -129,4 +132,24 @@ describe('submitBlastRadiusJob', () => {
129132
expect(start).not.toHaveBeenCalled()
130133
expect(createAnalysis).not.toHaveBeenCalled()
131134
})
135+
136+
it('marks the analysis failed and rethrows when workflow.start fails', async () => {
137+
const { req, res } = mockReqRes({
138+
advisoryId: 'GHSA-jf85-cpcp-j695',
139+
ecosystem: 'npm',
140+
})
141+
start.mockRejectedValueOnce(new Error('temporal unreachable'))
142+
143+
await expect(submitBlastRadiusJob(req, res)).rejects.toThrow('temporal unreachable')
144+
145+
expect(failAnalysis).toHaveBeenCalledTimes(1)
146+
const [, input, errorMessage] = failAnalysis.mock.calls[0]
147+
expect(input).toMatchObject({
148+
advisoryOsvId: 'GHSA-jf85-cpcp-j695',
149+
packageName: null,
150+
ecosystem: 'npm',
151+
force: false,
152+
})
153+
expect(errorMessage).toBe('temporal unreachable')
154+
})
132155
})

‎backend/src/api/public/v1/packages/submitBlastRadiusJob.ts‎

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -38,20 +38,39 @@ export async function submitBlastRadiusJob(req: Request, res: Response): Promise
3838
// one (req.temporal) — starting it there would leave the workflow queued forever.
3939
const packagesTemporal = await getPackagesTemporalClient()
4040

41-
await packagesTemporal.workflow.start('analyzeBlastRadius', {
42-
taskQueue: 'blast-radius-worker',
43-
workflowId: `${TemporalWorkflowId.BLAST_RADIUS_ANALYSIS}/${analysisId}`,
44-
retry: { maximumAttempts: 1 },
45-
args: [
41+
try {
42+
await packagesTemporal.workflow.start('analyzeBlastRadius', {
43+
taskQueue: 'blast-radius-worker',
44+
workflowId: `${TemporalWorkflowId.BLAST_RADIUS_ANALYSIS}/${analysisId}`,
45+
retry: { maximumAttempts: 1 },
46+
args: [
47+
{
48+
analysisId,
49+
advisoryId: body.advisoryId,
50+
package: jobPackage,
51+
ecosystem: jobEcosystem,
52+
force: body.force,
53+
} satisfies ITriggerBlastRadiusAnalysis,
54+
],
55+
})
56+
} catch (err) {
57+
// Without this, a workflow.start failure (Temporal unreachable, task queue
58+
// misconfigured, etc.) leaves the row created above stuck 'pending' forever —
59+
// no workflow ever runs to mark it failed, so poll never reaches a terminal state.
60+
const errorMessage = err instanceof Error ? err.message : String(err)
61+
await blastRadiusDal.failAnalysis(
62+
qx,
4663
{
47-
analysisId,
48-
advisoryId: body.advisoryId,
49-
package: jobPackage,
64+
id: analysisId,
65+
advisoryOsvId: body.advisoryId,
66+
packageName: jobPackage,
5067
ecosystem: jobEcosystem,
5168
force: body.force,
52-
} satisfies ITriggerBlastRadiusAnalysis,
53-
],
54-
})
69+
},
70+
errorMessage,
71+
)
72+
throw err
73+
}
5574

5675
// 202, not the shared ok() helper (200) — the contract accepts the job, it does
5776
// not return a completed result.

‎services/apps/packages_worker/src/blast-radius/agent/runner.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,11 @@ export async function runAnalysisAgent(input: RunAnalysisAgentInput): Promise<Ag
5858
maxTurns,
5959
tools: ['Read', 'Grep', 'Glob'],
6060
disallowedTools: ['Bash', 'Write', 'Edit', 'NotebookEdit', 'WebFetch', 'WebSearch', 'Task'],
61-
permissionMode: 'bypassPermissions',
62-
allowDangerouslySkipPermissions: true,
61+
// Read/Grep/Glob are read-only — auto-allow them instead of bypassing permissions
62+
// entirely. bypassPermissions requires --dangerously-skip-permissions, which the
63+
// Claude Code CLI refuses to run as root — the exact setup packages_worker's
64+
// container runs under.
65+
allowedTools: ['Read', 'Grep', 'Glob'],
6366
outputFormat: {
6467
type: 'json_schema',
6568
schema,

‎services/apps/packages_worker/src/blast-radius/clients/npmTarball.ts‎

Lines changed: 61 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -27,73 +27,73 @@ export async function downloadAndExtractTarball(
2727
const controller = new AbortController()
2828
const timeoutHandle = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS)
2929

30-
let res: Response
3130
try {
32-
res = await fetch(tarballUrl, { signal: controller.signal })
33-
} finally {
34-
clearTimeout(timeoutHandle)
35-
}
36-
if (!res.ok) {
37-
throw new Error(`Failed to fetch tarball: ${res.status} ${res.statusText}`)
38-
}
39-
if (!res.body) {
40-
throw new Error('No response body from tarball fetch')
41-
}
31+
const res = await fetch(tarballUrl, { signal: controller.signal })
32+
if (!res.ok) {
33+
throw new Error(`Failed to fetch tarball: ${res.status} ${res.statusText}`)
34+
}
35+
if (!res.body) {
36+
throw new Error('No response body from tarball fetch')
37+
}
4238

43-
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tarball-extract-'))
44-
try {
45-
// tar rejects (preservePaths defaults to false) any entry whose path would resolve
46-
// outside cwd; strict:true makes that a thrown error instead of a silent warn+skip.
47-
let extractedBytes = 0
48-
let extractedFiles = 0
49-
const extractStream = tar.extract({
50-
cwd: tempDir,
51-
strict: true,
52-
filter: (_entryPath, entry) => {
53-
extractedFiles++
54-
extractedBytes += entry.size ?? 0
55-
if (extractedFiles > MAX_EXTRACTED_FILES || extractedBytes > MAX_EXTRACTED_BYTES) {
56-
;(extractStream as unknown as Writable).destroy(
57-
new Error('Tarball extraction exceeded size/file limits'),
58-
)
59-
return false
60-
}
61-
return true
62-
},
63-
})
39+
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'tarball-extract-'))
40+
try {
41+
// tar rejects (preservePaths defaults to false) any entry whose path would resolve
42+
// outside cwd; strict:true makes that a thrown error instead of a silent warn+skip.
43+
let extractedBytes = 0
44+
let extractedFiles = 0
45+
const extractStream = tar.extract({
46+
cwd: tempDir,
47+
strict: true,
48+
filter: (_entryPath, entry) => {
49+
extractedFiles++
50+
extractedBytes += entry.size ?? 0
51+
if (extractedFiles > MAX_EXTRACTED_FILES || extractedBytes > MAX_EXTRACTED_BYTES) {
52+
;(extractStream as unknown as Writable).destroy(
53+
new Error('Tarball extraction exceeded size/file limits'),
54+
)
55+
return false
56+
}
57+
return true
58+
},
59+
})
6460

65-
let downloadedBytes = 0
66-
const downloadLimiter = new Transform({
67-
transform(chunk, _encoding, callback) {
68-
downloadedBytes += chunk.length
69-
if (downloadedBytes > MAX_DOWNLOAD_BYTES) {
70-
callback(new Error('Tarball download exceeded size limit'))
71-
return
72-
}
73-
callback(null, chunk)
74-
},
75-
})
61+
let downloadedBytes = 0
62+
const downloadLimiter = new Transform({
63+
transform(chunk, _encoding, callback) {
64+
downloadedBytes += chunk.length
65+
if (downloadedBytes > MAX_DOWNLOAD_BYTES) {
66+
callback(new Error('Tarball download exceeded size limit'))
67+
return
68+
}
69+
callback(null, chunk)
70+
},
71+
})
7672

77-
await new Promise<void>((resolve, reject) => {
78-
Readable.fromWeb(res.body as unknown as NodeWebReadableStream<Uint8Array>)
79-
.on('error', reject)
80-
.pipe(downloadLimiter)
81-
.on('error', reject)
82-
.pipe(extractStream as unknown as NodeJS.WritableStream)
83-
.on('finish', resolve)
84-
.on('error', reject)
85-
})
73+
await new Promise<void>((resolve, reject) => {
74+
Readable.fromWeb(res.body as unknown as NodeWebReadableStream<Uint8Array>)
75+
.on('error', reject)
76+
.pipe(downloadLimiter)
77+
.on('error', reject)
78+
.pipe(extractStream as unknown as NodeJS.WritableStream)
79+
.on('finish', resolve)
80+
.on('error', reject)
81+
})
8682

87-
const items = fs.readdirSync(tempDir)
88-
const commonPrefix =
89-
items.length === 1 && fs.statSync(path.join(tempDir, items[0])).isDirectory()
90-
? items[0]
91-
: null
92-
const srcBase = commonPrefix ? path.join(tempDir, commonPrefix) : tempDir
83+
const items = fs.readdirSync(tempDir)
84+
const commonPrefix =
85+
items.length === 1 && fs.statSync(path.join(tempDir, items[0])).isDirectory()
86+
? items[0]
87+
: null
88+
const srcBase = commonPrefix ? path.join(tempDir, commonPrefix) : tempDir
9389

94-
copyTreeGuarded(srcBase, destDir)
95-
} finally {
96-
fs.rmSync(tempDir, { recursive: true, force: true })
90+
copyTreeGuarded(srcBase, destDir)
91+
} finally {
92+
fs.rmSync(tempDir, { recursive: true, force: true })
93+
}
94+
} catch (err) {
95+
clearTimeout(timeoutHandle)
96+
throw err
9797
}
9898
}
9999

‎services/apps/packages_worker/src/blast-radius/clients/osvClient.ts‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export interface OsvAffectedPackage {
1313
last_affected?: string
1414
}>
1515
}>
16+
versions?: string[]
1617
}
1718

1819
export interface OsvVuln {
@@ -51,36 +52,46 @@ export function affectedNpmEntries(vuln: OsvVuln): OsvAffectedPackage[] {
5152
// Flatten SEMVER-type ranges in an affected package into {introduced, fixed, lastAffected}
5253
// triples. last_affected is OSV's inclusive upper bound (distinct from fixed, which is
5354
// exclusive) — it closes the range at that version rather than leaving it open-ended.
55+
// If OSV provides only a versions[] list (no ranges[]), convert each exact version into
56+
// a degenerate range (introduced=v, last_affected=v) to match isInRange semantics.
5457
export function semverRangeEvents(
5558
entry: OsvAffectedPackage,
5659
): Array<{ introduced: string | null; fixed: string | null; lastAffected: string | null }> {
57-
if (!entry.ranges) return []
58-
5960
const events: Array<{
6061
introduced: string | null
6162
fixed: string | null
6263
lastAffected: string | null
6364
}> = []
64-
for (const range of entry.ranges) {
65-
if (range.type !== 'SEMVER' || !range.events) continue
6665

67-
let introduced: string | null = null
66+
if (entry.ranges) {
67+
for (const range of entry.ranges) {
68+
if (range.type !== 'SEMVER' || !range.events) continue
69+
70+
let introduced: string | null = null
6871

69-
for (const event of range.events) {
70-
if (event.introduced) introduced = event.introduced
72+
for (const event of range.events) {
73+
if (event.introduced) introduced = event.introduced
7174

72-
if (event.fixed) {
73-
events.push({ introduced, fixed: event.fixed, lastAffected: null })
74-
introduced = null
75-
} else if (event.last_affected) {
76-
events.push({ introduced, fixed: null, lastAffected: event.last_affected })
77-
introduced = null
75+
if (event.fixed) {
76+
events.push({ introduced, fixed: event.fixed, lastAffected: null })
77+
introduced = null
78+
} else if (event.last_affected) {
79+
events.push({ introduced, fixed: null, lastAffected: event.last_affected })
80+
introduced = null
81+
}
82+
}
83+
84+
// Trailing open range: introduced but never closed by fixed/last_affected.
85+
if (introduced !== null) {
86+
events.push({ introduced, fixed: null, lastAffected: null })
7887
}
7988
}
89+
}
8090

81-
// Trailing open range: introduced but never closed by fixed/last_affected.
82-
if (introduced !== null) {
83-
events.push({ introduced, fixed: null, lastAffected: null })
91+
// Fallback: if no SEMVER ranges, use explicit version list as exact vulnerable versions.
92+
if (events.length === 0 && (entry.versions ?? []).length > 0) {
93+
for (const v of entry.versions ?? []) {
94+
events.push({ introduced: v, fixed: null, lastAffected: v })
8495
}
8596
}
8697

‎services/apps/packages_worker/src/blast-radius/dependentsScan.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,11 +237,11 @@ export async function scanDependents(input: {
237237
// any of the more expensive per-candidate work below.
238238
const candidateNames = await candidateNamesFromScan(toScan, targets, onProgress)
239239

240-
// Fetch download counts (bulk, max 128 per request) for the last calendar month —
240+
// Fetch download counts (bulk, max 128 per request) for the last 30 days —
241241
// only for the filtered candidates, not the full high-impact list.
242242
const rangeEnd = new Date()
243243
const rangeStart = new Date(rangeEnd)
244-
rangeStart.setUTCMonth(rangeStart.getUTCMonth() - 1)
244+
rangeStart.setUTCDate(rangeStart.getUTCDate() - 30)
245245
const isoDate = (d: Date) => d.toISOString().slice(0, 10)
246246

247247
const downloads = new Map<string, number>()

‎services/apps/packages_worker/src/blast-radius/semverRange.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export function versionsInRanges(versions: string[], ranges: SemverRange[]): str
2727
rangeSpec = `>=${range.introduced}`
2828
}
2929

30-
if (rangeSpec && semver.satisfies(v, rangeSpec, { loose: true })) {
30+
if (rangeSpec && semver.satisfies(v, rangeSpec, { loose: true, includePrerelease: true })) {
3131
result.push(v)
3232
break
3333
}
@@ -63,7 +63,7 @@ export function rangeIncludesAny(
6363
}
6464

6565
for (const vuln of vulnerableVersions) {
66-
if (semver.satisfies(vuln, declaredRange, { loose: true })) {
66+
if (semver.satisfies(vuln, declaredRange, { loose: true, includePrerelease: true })) {
6767
return { includes: true, check: 'matched' }
6868
}
6969
}
@@ -75,7 +75,7 @@ export function rangeIncludesAny(
7575
export function highestVersion(versions: string[]): string | null {
7676
const valid = versions.filter((v) => semver.valid(v, { loose: true }))
7777
if (valid.length === 0) return null
78-
return semver.maxSatisfying(valid, '*', { loose: true }) || null
78+
return semver.maxSatisfying(valid, '*', { loose: true, includePrerelease: true }) || null
7979
}
8080

8181
// Helper: check if a range spec is parseable as semver. semver.validRange returns null

‎services/libs/data-access-layer/src/packages/blastRadius.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export interface SymbolSpecInput {
3434

3535
export interface DependentInput {
3636
analysisId: string
37-
packageId: number | null
37+
packageId: string | null
3838
name: string
3939
version: string | null
4040
downloads: number | null

0 commit comments

Comments
 (0)