feat: add blast radius methodlogy (CM-1328)#4377
Open
ulemons wants to merge 8 commits into
Open
Conversation
PR SummaryHigh Risk Overview Poll API: Submit hardening: Creates a pending analysis row before starting Temporal so immediate polls don’t 404; if Worker pipeline: Reviewed by Cursor Bugbot for commit 6bf837f. Bugbot is set up for automated code reviews on this repo. Configure here. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
ulemons
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Implements the npm blast-radius pipeline with database-backed state, Temporal stages, agent analysis, and public result polling.
Changes:
- Adds schema and DAL operations for analyses, dependents, verdicts, and stage runs.
- Implements intelligence, dependent scanning, reachability, and reporting stages.
- Adds supporting npm/OSV clients, agent prompts, semver utilities, and API responses.
Metadata: Rename the title to feat: add blast radius methodology (CM-1328). The PR also exceeds the recommended 1,000-line target.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
services/libs/data-access-layer/src/packages/blastRadius.ts |
Adds pipeline persistence operations. |
services/apps/packages_worker/src/blast-radius/stages/report.ts |
Finalizes analyses and costs. |
services/apps/packages_worker/src/blast-radius/stages/reachability.ts |
Analyzes dependent reachability. |
services/apps/packages_worker/src/blast-radius/stages/intel.ts |
Produces vulnerability symbol intelligence. |
services/apps/packages_worker/src/blast-radius/stages/dependents.ts |
Persists dependent scan results. |
services/apps/packages_worker/src/blast-radius/semverRange.ts |
Adds semver matching helpers. |
services/apps/packages_worker/src/blast-radius/semverRange.test.ts |
Tests semver helpers. |
services/apps/packages_worker/src/blast-radius/npmManifest.ts |
Extends npm manifest typing locally. |
services/apps/packages_worker/src/blast-radius/dependentsScan.ts |
Discovers and ranks npm dependents. |
services/apps/packages_worker/src/blast-radius/clients/osvClient.ts |
Fetches and interprets OSV records. |
services/apps/packages_worker/src/blast-radius/clients/npmTarball.ts |
Downloads and extracts npm sources. |
services/apps/packages_worker/src/blast-radius/clients/npmAbbreviated.ts |
Fetches abbreviated npm metadata. |
services/apps/packages_worker/src/blast-radius/clients/githubPatch.ts |
Downloads GitHub patches. |
services/apps/packages_worker/src/blast-radius/agent/runner.ts |
Configures Agent SDK execution. |
services/apps/packages_worker/src/blast-radius/agent/prompts.ts |
Defines agent prompts and schemas. |
services/apps/packages_worker/src/blast-radius/activities.ts |
Exposes Temporal activities. |
backend/src/osspckgs/migrations/V1784700000__blast_radius_analyses.sql |
Creates blast-radius tables and indexes. |
backend/src/api/public/v1/packages/getBlastRadiusJob.ts |
Adds analysis polling endpoint. |
backend/src/api/public/v1/packages/getBlastRadiusJob.test.ts |
Tests polling behavior. |
backend/src/api/public/v1/packages/blastRadiusAnalysis.ts |
Maps stored results to API responses. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ulemons
force-pushed
the
feat/add-blast-radius-migrations
branch
from
July 22, 2026 11:09
99915ef to
fbb843b
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (3)
backend/src/api/public/v1/packages/blastRadiusAnalysis.ts:95
- This overstates
dependentsExcludedUpfront.candidatesConsideredis every phase-1 hit, while phase 2 stops aftertopNin-range packages and also skips packument failures; therefore the difference includes candidates that were never range-checked. For example, 100 hits with the first 25 in range reports 75 exclusions even though none were excluded. Persist/use the actual excluded count (and distinguish skipped/unprocessed candidates) when building the summary.
totalDependentsInRange,
dependentsExcludedUpfront: Math.max(totalDependentsInRange - dependentsAnalyzed, 0),
services/apps/packages_worker/src/blast-radius/clients/npmTarball.ts:34
strict/preservePathsprotects archive entry paths, but it does not make symlink targets safe. A malicious tarball can extract a symlink to a file outsidetempDir;copyTreeGuardedlater usesstatSyncandcopyFileSync, both of which follow that link, copying host files into the agent workspace. Reject symbolic/hard-link entries during extraction, or uselstatplus realpath containment before copying.
const extractStream = tar.extract({
cwd: tempDir,
strict: true,
}) as unknown as NodeJS.WritableStream
services/apps/packages_worker/src/blast-radius/stages/reachability.ts:138
- The stage cost omits billed failed attempts.
runAnalysisAgentreturnstotal_cost_usdeven for non-success result messages, but this adds cost only after a successful attempt. If an attempt consumes tokens and a retry succeeds, the finalized analysis under-reports actual spend; accumulate each attempt's cost exactly once before deciding whether to retry.
results.cost += agentResult.costUsd || 0
ulemons
force-pushed
the
feat/add-blast-radius-migrations
branch
from
July 22, 2026 11:17
fbb843b to
fb94246
Compare
ulemons
force-pushed
the
feat/add-blast-radius-methodlogy
branch
from
July 22, 2026 11:28
45f337d to
b664863
Compare
![github-license-compliance[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 27 changed files in this pull request and generated 11 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (4)
services/apps/packages_worker/src/blast-radius/stages/intel.ts:67
- When a package was explicitly requested but is not present in the advisory, this silently falls back to the first npm entry. The completed job then reports results for a different package while echoing the caller's requested package. Only fall back for advisory-wide requests; reject an unmatched explicit package.
const entry =
(requestedPackage && npmEntries.find((e) => e.package.name === requestedPackage)) ||
npmEntries[0]
services/apps/packages_worker/src/blast-radius/agent/runner.ts:49
- The analyzed npm source is untrusted and can contain prompt-injection instructions. Allowing
Readwhile bypassing all permission prompts lets the agent read paths outsidecwd; the subprocess also inherits the worker's full environment, so injected source can retrieve credentials and return them in persisted/public verdict fields. Run the agent in an OS-level sandbox containing only the extracted tree, restrict file tools to that tree, and pass an allowlisted environment instead of using bypass mode.
tools: ['Read', 'Grep', 'Glob'],
disallowedTools: ['Bash', 'Write', 'Edit', 'NotebookEdit', 'WebFetch', 'WebSearch', 'Task'],
permissionMode: 'bypassPermissions',
allowDangerouslySkipPermissions: true,
services/apps/packages_worker/src/blast-radius/stages/reachability.ts:145
- Only the successful final attempt's cost is persisted here. Any billed failed attempts are discarded, and a persistently failing dependent is stored with cost 0, so
getVerdictsCostand the final analysis total under-report actual spend whenever retries occur. Accumulate and persist cost for every agent result, including failures and activity retries.
costUsd: agentResult.costUsd || 0,
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:166
- All registry errors—including rate limits and transient timeouts—are treated as “not a dependent” here. This scan issues roughly 17k requests at concurrency 32, so temporary failures can silently remove real high-impact dependents and produce a successful but incomplete blast-radius report. Retry transient/rate-limit responses (and fail or report degraded coverage after exhaustion); only
NOT_FOUNDshould be safely skipped.
const packument = await fetchAbbreviatedPackument(name)
if (isFetchError(packument)) return
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 30 changed files in this pull request and generated 8 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (3)
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:190
- Every registry error is treated as “not a dependent.” With roughly 17k concurrent requests, a 429 or transient outage can therefore remove large portions of the candidate population while the stage still reports success, producing an understated blast radius. Retry/back off
RATE_LIMIT/TRANSIENTfailures or fail the stage; only terminal per-package errors should be skipped.
const packument = await fetchAbbreviatedPackument(name)
if (isFetchError(packument)) return
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:322
- The true
excludedByRangeCountis computed, but only 200 rows are returned and the poll endpoint later derivesdependentsExcludedUpfrontfrom those persisted rows. Analyses with more than 200 exclusions therefore underreport both exclusion and total counts. Persist/use the uncapped count separately while retaining the row cap.
excludedByRange: excludedByRange.slice(0, 200),
excludedByRangeCount: excludedByRange.length,
services/apps/packages_worker/src/blast-radius/stages/reachability.ts:148
- Only the successful attempt's cost is persisted.
runAnalysisAgentalso returnscostUsdfor unsuccessful result messages, so any failed attempts before a successful retry—and all three attempts for an error verdict—are omitted from the final analysis cost. Accumulate cost across attempts and persist that total for both success and terminal failure.
costUsd: agentResult.costUsd || 0,
Comment on lines
+8
to
+12
| ranges?: Array<{ | ||
| type: string | ||
| events?: Array<{ | ||
| introduced?: string | ||
| fixed?: string |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 30 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (5)
services/apps/packages_worker/src/blast-radius/stages/intel.ts:70
- When a caller supplies a package that is not present in the advisory, this expression silently analyzes
npmEntries[0]instead. The returned job therefore concerns a different package than requested. Only use the first entry when no package was requested; otherwise reject a missing match.
const entry =
(requestedPackage && npmEntries.find((e) => e.package.name === requestedPackage)) ||
npmEntries[0]
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:190
- Every fetch failure—including
RATE_LIMITandTRANSIENT—is treated as if the package were not a dependent. During a scan of thousands of registry records, throttling or an outage therefore produces a successful but incomplete blast-radius report with false negatives. Skip only definitiveNOT_FOUNDresponses; retry transient/rate-limit responses with bounded backoff or fail the stage.
const packument = await fetchAbbreviatedPackument(name)
if (isFetchError(packument)) return
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:322
- The full
excludedByRangeCountis computed but discarded, while the read endpoint derives its summary by counting persisted rows. Once more than 200 candidates are excluded before reaching top-N,dependentsExcludedUpfrontandtotalDependentsInRangeare silently capped and underreported. Persist the aggregate count separately (or persist all exclusions) and use it in the report.
excludedByRange: excludedByRange.slice(0, 200),
excludedByRangeCount: excludedByRange.length,
services/apps/packages_worker/src/blast-radius/stages/reachability.ts:148
- Only the successful attempt's cost is persisted.
runAnalysisAgentreportstotal_cost_usdfor unsuccessful result messages too, so any paid failed attempts before a retry—and all three attempts for anunclearerror verdict—are omitted from the stage and analysis totals. Accumulate every attempt's reported cost and store the aggregate in the final verdict.
model: 'claude-sonnet-5',
turnsUsed: agentResult.numTurns,
costUsd: agentResult.costUsd || 0,
backend/src/api/public/v1/akrites-external/openapi.yaml:476
- This description is inconsistent with the implementation:
dependentsExcludedUpfrontcontains packages whose declared ranges explicitly did not include a vulnerable version, and the scan stops once top-N in-range packages are found. The sum is candidates evaluated during the ranked walk, not dependents that fell within the vulnerable range before the cutoff.
totalDependentsInRange:
type: integer
description: Sum of dependentsAnalyzed and dependentsExcludedUpfront; the count of dependents that fell within the vulnerable version range (before top-N cutoff was applied).
dependentsExcludedUpfront:
type: integer
description: totalDependentsInRange minus dependentsAnalyzed.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 30 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (5)
backend/src/api/public/v1/packages/submitBlastRadiusJob.ts:41
getPackagesTemporalClient()can itself throw or reject while connecting, but it runs before thistry. In that failure mode the pending row is never marked failed, which is exactly the stuck state this catch is intended to prevent. Acquire the client inside the guarded block.
try {
services/apps/packages_worker/src/blast-radius/stages/intel.ts:70
- When a package was explicitly requested but is absent from this advisory, this expression silently analyzes
npmEntries[0]. The poll response still echoes the requested package, so clients receive results for the wrong package. Only default to the first entry for advisory-wide requests; reject an unmatched explicit package.
const entry =
(requestedPackage && npmEntries.find((e) => e.package.name === requestedPackage)) ||
npmEntries[0]
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:190
- A
RATE_LIMITorTRANSIENTfetch result is treated as proof that the package is not a dependent. During a ~17k-package scan this silently removes candidates whenever npm throttles or times out, producing an incomplete blast-radius result. Retry transient/rate-limit errors with backoff (or fail the stage) and reserve skipping for definitive responses such as not-found.
const packument = await fetchAbbreviatedPackument(name)
if (isFetchError(packument)) return
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:322
- Only the first 200 excluded rows are persisted, while the API later derives
dependentsExcludedUpfrontby counting those rows. AlthoughexcludedByRangeCountretains the real count here, it is discarded by the stage, so summaries under-report exclusions whenever the count exceeds 200. Persist the aggregate count separately or return all rows needed by the public summary.
excludedByRange: excludedByRange.slice(0, 200),
excludedByRangeCount: excludedByRange.length,
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:244
- The npm downloads API treats both endpoints as inclusive, so subtracting 30 days produces a 31-day total while the API exposes this as
downloadsLast30Days. Use a 29-day offset for an inclusive 30-day window (or compute the previous calendar month if that is the intended contract).
const rangeEnd = new Date()
const rangeStart = new Date(rangeEnd)
rangeStart.setUTCDate(rangeStart.getUTCDate() - 30)
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 32 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (8)
services/apps/packages_worker/src/blast-radius/stages/intel.ts:70
- When a caller specifies a package that is not present in this advisory, this silently analyzes the first npm entry instead. The poll response still echoes the requested package, so consumers receive results for a different package. Only fall back for advisory-wide requests; reject an unmatched explicit package.
const entry =
(requestedPackage && npmEntries.find((e) => e.package.name === requestedPackage)) ||
npmEntries[0]
services/libs/data-access-layer/src/packages/blastRadius.ts:211
- The
FROM advisoriesinner join prevents this update entirely when the advisory has not yet been synced—the exact nullable-advisory case described by the migration. Consequently, a resolvedpackageIdis also discarded. Resolve the advisory with a scalar subquery sopackage_idis updated independently.
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:296 - A dependency on any related affected package is accepted here, but its declared range is compared against
vulnerableVersionsfor the primary package. Multi-package advisories commonly have unrelated version lines, and Stage 3 also prompts only for the primary package, so these candidates can be incorrectly excluded or analyzed against the wrong symbols. Carry package-specific ranges/specs, or exclude related entries until multi-package handling is implemented.
const depInfo = declaredDependency(manifest, vulnerablePackage, relatedAffectedPackages)
if (!depInfo) continue
const { check, includes } = rangeIncludesAny(depInfo.range, vulnerableVersions)
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:190
- Every registry error—including 429s, timeouts, and 5xx responses—is treated as “not a dependent.” With thousands of concurrent lookups, transient failures therefore produce false negatives while the analysis still completes successfully. Skip only permanent not-found/malformed responses and let transient failures trigger the activity retry.
const packument = await fetchAbbreviatedPackument(name)
if (isFetchError(packument)) return
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:322
- The full exclusion count is computed on the next line but only 200 rows are returned and persisted. The poll endpoint later counts persisted rows, so
dependentsExcludedUpfrontis silently capped at 200 for larger scans. Persist the aggregate count separately (or stop truncating) and use it in the report.
excludedByRange: excludedByRange.slice(0, 200),
excludedByRangeCount: excludedByRange.length,
services/apps/packages_worker/src/blast-radius/clients/npmTarball.ts:49
- Third-party tarballs may contain symbolic or hard links. The later
statSync/copyFileSyncpath follows links, allowing an archive entry to copy host files into the agent workspace despite the destination traversal check. Reject link entries during extraction.
filter: (_entryPath, entry) => {
extractedFiles++
backend/src/api/public/v1/packages/blastRadiusAnalysis.ts:112
dependentsExcludedUpfrontcontains packages whose declared range does not include a vulnerable version, so adding it tototalDependentsInRangemakes that field contradict its name and OpenAPI description. Either report only in-range dependents here or rename/redefine the public field as total candidates evaluated.
return {
totalDependentsInRange: dependentsAnalyzed + dependentsExcludedUpfront,
dependentsExcludedUpfront,
services/libs/data-access-layer/src/packages/osv.ts:139
- This join reconstructs every npm row's full name with a
CASE, so PostgreSQL cannot use the existing unique index on(ecosystem, COALESCE(namespace, ''), name)and may scan the entire ecosystem to resolve at most 25 names. Split the small input list into namespace/name columns and join on the indexed expressions instead.
ulemons
force-pushed
the
feat/add-blast-radius-methodlogy
branch
from
July 22, 2026 16:10
03007ce to
df65abe
Compare
| force: input.force, | ||
| error: errorMessage, | ||
| }) | ||
| throw ApplicationFailure.nonRetryable(errorMessage, 'BLAST_RADIUS_STAGE_FAILED') |
There was a problem hiding this comment.
Done analyses marked failed
High Severity
If a late pipeline stage error runs after finalizeAnalysis has set status to done, the workflow catch still calls failAnalysis, which unconditionally overwrites status to failed. Poll only loads verdicts when status is done, so a finished run can appear failed with no summary or results even though verdict rows remain in the database.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit df65abe. Configure here.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 31 changed files in this pull request and generated 7 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (12)
services/apps/packages_worker/src/blast-radius/stages/intel.ts:70
- When a package was explicitly requested but is not present in the advisory, this expression silently falls back to the advisory's first npm entry. The analysis then examines a different package while the poll response still reports the requested package. Only use the first entry when no package was requested; otherwise fail on a mismatch.
const entry =
(requestedPackage && npmEntries.find((e) => e.package.name === requestedPackage)) ||
npmEntries[0]
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:190
- All abbreviated-registry errors, including rate limits and transient network failures, are treated as a clean non-match. During an npm outage this can produce an empty candidate list and a successful zero-impact report. Ignore only genuine
NOT_FOUNDresponses and propagate other error kinds so Temporal can retry the stage.
const packument = await fetchAbbreviatedPackument(name)
if (isFetchError(packument)) return
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:283
- Transient/rate-limit failures from the full packument fetch are also silently interpreted as "not a dependent." This can remove high-ranked dependents from the result while the stage still succeeds. Skip only
NOT_FOUND; let other upstream failures fail and retry the activity.
const packument = await fetchPackument(name)
if (isFetchError(packument)) continue
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:322
- The downstream stage persists only
excludedByRange, and the GET endpoint derives its public exclusion count from those rows. Slicing this list to 200 therefore makesdependentsExcludedUpfrontand the total under-report whenever more than 200 candidates fail the range check, even though the uncapped count is computed here. Persist the uncapped count as analysis metadata (or persist all exclusions) and use it in the response.
excludedByRange: excludedByRange.slice(0, 200),
excludedByRangeCount: excludedByRange.length,
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:244
- The npm downloads API range is inclusive at both ends. Subtracting 30 days while also including today requests 31 calendar days, so ranking and the
downloadsLast30Daysresponse field are off by one day.
const rangeEnd = new Date()
const rangeStart = new Date(rangeEnd)
rangeStart.setUTCDate(rangeStart.getUTCDate() - 30)
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:253
- After the pre-scan, the download batches and sequential full-packument walk never call
onProgress. Each request can wait 30 seconds, so seven slow batches/fetches exceed the activity's 3-minute heartbeat timeout and trigger an overlapping retry even though this attempt is still working. Heartbeat after each bulk/scoped/full-packument request.
for (let i = 0; i < unscopedNames.length; i += 128) {
const batch = unscopedNames.slice(i, i + 128)
const result = await fetchBulkPointRange(batch, isoDate(rangeStart), isoDate(rangeEnd))
services/libs/data-access-layer/src/packages/osv.ts:139
- This join reconstructs a full name from package columns, so it cannot use the existing
(ecosystem, COALESCE(namespace, ''), name)index (V1779710880__initial_schema.sql:82). For npm's large package population, each analysis can scan all npm rows just to resolve at most 25 IDs. Parse the input into namespace/name pairs and join on the indexed columns instead.
backend/src/api/public/v1/packages/submitBlastRadiusJob.ts:42 - The
trystarts aftergetPackagesTemporalClient(). If client initialization rejects (for example, Temporal is unreachable), the analysis row created above remainspendingforever andfailAnalysisis never called—the exact failure mode this catch is intended to prevent. Move client acquisition inside thetry.
try {
await packagesTemporal.workflow.start('analyzeBlastRadius', {
services/libs/data-access-layer/src/packages/blastRadius.ts:78
- Both columns are BIGINT, but this connection only registers int4 as a numeric parser (
services/libs/database/src/connection.ts:84-85), so pg-promise returns these IDs as strings. Typing them asnumberrisks precision loss and conflicts with the string ID handling introduced inosv.ts:125-143. Update the row types and the relatedpackageIdparameter/lookup return type tostring | null.
advisory_id: number | null
package_id: number | null
backend/src/api/public/v1/akrites-external/openapi.yaml:473
- This description is internally contradictory: the implementation adds rows explicitly excluded because their declared ranges do not include a vulnerable version, so the sum is not a count of dependents that "fell within" the vulnerable range. Describe it as the evaluated range-walk population (including range exclusions), or change the field computation/name to represent in-range dependents.
totalDependentsInRange:
type: integer
description: Sum of dependentsAnalyzed and dependentsExcludedUpfront; the count of dependents that fell within the vulnerable version range (before top-N cutoff was applied).
services/apps/packages_worker/src/blast-radius/clients/npmTarball.ts:87
- Archive-created symlinks are followed both by this common-prefix
statSyncand later bystatSyncincopyTreeGuarded. A tarball can pointpackage/leak(or the sole top-level entry) at a host path and copy host files into the agent tree despite the destination check. UselstatSyncat both sites and reject symlinks/special files, copying regular files only.
const items = fs.readdirSync(tempDir)
const commonPrefix =
items.length === 1 && fs.statSync(path.join(tempDir, items[0])).isDirectory()
? items[0]
: null
services/apps/packages_worker/src/blast-radius/agent/runner.ts:68
- These tools are auto-allowed without a path boundary, while the analyzed npm source is untrusted and the subprocess inherits the worker environment. A prompt-injected package can induce reads outside
cwd(including mounted credentials or/proc/.../environ) and send them to the model. Enforce cwd-only tool authorization or an isolated filesystem/user, and pass only an allowlisted environment.
tools: ['Read', 'Grep', 'Glob'],
disallowedTools: ['Bash', 'Write', 'Edit', 'NotebookEdit', 'WebFetch', 'WebSearch', 'Task'],
Comment on lines
+330
to
+332
| if (dependents.length === 0) return | ||
| for (const dep of dependents) { | ||
| await qx.result( |
|
|
||
| # security-contacts-worker | ||
| SECURITY_CONTACTS_USER_AGENT="lfx-security-contacts-worker" | ||
| CROWD_PACKAGES_TEMPORAL_SERVER_URL=temporal:7233 No newline at end of file |
Comment on lines
+10
to
+14
| events?: Array<{ | ||
| introduced?: string | ||
| fixed?: string | ||
| last_affected?: string | ||
| }> |
Comment on lines
+14
to
+16
| export async function fetchPatch(url: string): Promise<string> { | ||
| const patchUrl = `${url}.patch` | ||
| const res = await fetch(patchUrl) |
Comment on lines
+141
to
+143
| if (agentResult.isError || !agentResult.structuredOutput) { | ||
| throw new Error(`Agent failed: ${agentResult.errorMessage}`) | ||
| } |
Comment on lines
+482
to
+485
| affectedPercentage: | ||
| type: number | ||
| nullable: true | ||
| description: Rounded to 1 decimal. Null when dependentsAnalyzed is 0, to avoid a misleading 0%. |
Comment on lines
+1098
to
+1100
| '429': | ||
| description: Too many requests — rate-limited independently of the other endpoints. | ||
| content: |
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
ulemons
force-pushed
the
feat/add-blast-radius-methodlogy
branch
from
July 22, 2026 16:27
50a875b to
6bf837f
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6bf837f. Configure here.
| filter: (_entryPath, entry) => { | ||
| extractedFiles++ | ||
| extractedBytes += entry.size ?? 0 | ||
| if (extractedFiles > MAX_EXTRACTED_FILES || extractedBytes > MAX_EXTRACTED_BYTES) { |
There was a problem hiding this comment.
Tar limits use header size
Medium Severity
Extraction enforces MAX_EXTRACTED_BYTES using each tar entry’s declared size, not bytes actually written. A malicious registry tarball can advertise small sizes and expand far beyond the limit, risking worker disk or memory exhaustion during reachability.
Reviewed by Cursor Bugbot for commit 6bf837f. Configure here.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 31 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (8)
services/apps/packages_worker/src/blast-radius/agent/runner.ts:73
cwdis not a filesystem sandbox, and pre-allowingRead/Grep/Globlets the agent access paths outside the extracted package. Because package source is untrusted and the child also receives the worker environment, a prompt-injected package can read files such as/proc/self/environand disclose service credentials to the model. Remove the blanket allow and enforce a canonicalizedcwdpath boundary (or run the agent in an isolated container with only the package mounted) before processing third-party source.
allowedTools: ['Read', 'Grep', 'Glob'],
services/apps/packages_worker/src/blast-radius/stages/intel.ts:70
- When a package was explicitly requested but is not one of the advisory's npm entries, this fallback silently analyzes
npmEntries[0]. The stored/APIpackage_namestill reports the requested package, so clients receive blast-radius results for a different package. Only fall back when no package was requested; otherwise fail the analysis.
const entry =
(requestedPackage && npmEntries.find((e) => e.package.name === requestedPackage)) ||
npmEntries[0]
backend/src/api/public/v1/packages/submitBlastRadiusJob.ts:39
getPackagesTemporalClient()can itself throw or reject when configuration is missing or the Temporal connection fails, but it runs outside thistry. In that case the pending row created above is never marked failed, which is the stuck state this handler is intended to prevent. Acquire the client inside the sametry.
const packagesTemporal = await getPackagesTemporalClient()
backend/.env.dist.composed:44
- The composed backend still cannot construct
PACKAGES_TEMPORAL_CONFIG:backend/src/conf/index.ts:91-95only enables it whenpackagesTemporal.namespaceexists, but this file adds only the server URL. AddCROWD_PACKAGES_TEMPORAL_NAMESPACE(the local template usesdefault) or every composed submission will fail before starting the workflow.
CROWD_PACKAGES_TEMPORAL_SERVER_URL=temporal:7233
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:238
- Progress callbacks are only emitted by the phase-1 pre-scan. The download-count loops and especially the sequential full-packument walk below can each run longer than the activity's 3-minute heartbeat timeout (each request has a 30-second timeout), causing Temporal to time out and retry healthy work. Heartbeat on a time-based cadence throughout every network loop, not only every 200 phase-1 items.
// Phase 1: concurrent, lightweight pre-scan — filters the (potentially ~17k) high-impact
// list down to the names that actually declare a dependency on the target, before doing
// any of the more expensive per-candidate work below.
const candidateNames = await candidateNamesFromScan(toScan, targets, onProgress)
backend/src/api/public/v1/packages/blastRadiusAnalysis.ts:114
- This adds packages explicitly marked
excluded_by_rangetototalDependentsInRange, so the public field reports out-of-range packages as in-range. Either make this value represent only in-range analyzed dependents, or rename the public field/description to a total-evaluated metric; the current result contradicts its API meaning.
totalDependentsInRange: dependentsAnalyzed + dependentsExcludedUpfront,
services/apps/packages_worker/src/blast-radius/stages/intel.ts:143
- An Agent SDK error result can still carry nonzero
costUsd, but this branch throws it away andfailStageRunstores no cost. If Temporal retries the activity and the next attempt succeeds, the final report includes only the successful call and underreports actual spend. Accumulate/persist failed-attempt cost as the reachability stage already does.
if (agentResult.isError || !agentResult.structuredOutput) {
throw new Error(`Agent failed: ${agentResult.errorMessage}`)
}
backend/src/api/public/v1/akrites-external/openapi.yaml:485
- The implementation calculates this percentage over conclusive verdicts and returns null when every analyzed result is
unclear, even whendependentsAnalyzedis nonzero. Document that denominator/null condition so API consumers do not infer different behavior from the schema.
description: Rounded to 1 decimal. Null when dependentsAnalyzed is 0, to avoid a misleading 0%.
Comment on lines
+98
to
+99
| await blastRadiusDal.insertDependents(qx, dependentInputs) | ||
| await blastRadiusDal.setDependentsMeta( |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 31 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (8)
backend/.env.dist.composed:44
- The composed environment still lacks
CROWD_PACKAGES_TEMPORAL_NAMESPACE.PACKAGES_TEMPORAL_CONFIGis only initialized when that namespace exists (backend/src/conf/index.ts:91-95), so the composed API will treat packages Temporal as unconfigured and every submission will fail before connecting. Define the namespace alongside the new server URL.
CROWD_PACKAGES_TEMPORAL_SERVER_URL=temporal:7233
backend/src/api/public/v1/packages/submitBlastRadiusJob.ts:39
getPackagesTemporalClient()can itself reject when configuration is missing or the connection fails, but it runs before thistry. In that case the row created above remainspendingforever, which is the exact state the catch is intended to prevent. Move client acquisition inside the guarded block.
const packagesTemporal = await getPackagesTemporalClient()
services/apps/packages_worker/src/blast-radius/stages/intel.ts:70
- When a caller narrows a multi-package advisory to a package that is not present in OSV, this expression silently falls back to the first entry. The analysis is then persisted and returned under the requested package name even though a different package was analyzed. Only use the first entry for advisory-wide requests; reject a requested package that does not match.
const entry =
(requestedPackage && npmEntries.find((e) => e.package.name === requestedPackage)) ||
npmEntries[0]
services/apps/packages_worker/src/blast-radius/dependentsScan.ts:322
- The API derives
dependentsExcludedUpfrontby counting persisted excluded rows, but this truncates those rows to 200 even thoughexcludedByRangeCountalready records the real count. Analyses with more than 200 exclusions therefore return an undercounted summary. Persist the uncapped count as analysis metadata (or otherwise use it in the read path) while retaining the row cap.
excludedByRange: excludedByRange.slice(0, 200),
excludedByRangeCount: excludedByRange.length,
services/libs/data-access-layer/src/packages/osv.ts:139
- This join applies concatenation to every
packagesrow, so PostgreSQL cannot use the existing(ecosystem, COALESCE(namespace, ''), name)index for the name lookup; it can only narrow by ecosystem and scan all npm packages. Parse each input into namespace/name on theunnestside and join on the indexed columns instead.
services/libs/data-access-layer/src/packages/blastRadius.ts:332 - This performs one database round-trip per dependent, sequentially—up to 225 INSERT/UPSERT calls for a single analysis. The DAL already provides
prepareBulkInsert; use a batched upsert (chunked if necessary) so persistence is one or a small bounded number of queries.
for (const dep of dependents) {
await qx.result(
backend/src/api/public/v1/akrites-external/openapi.yaml:485
- This description does not match the implementation: the percentage is computed over conclusive verdicts only and becomes null when every analyzed result is
unclear, even whendependentsAnalyzedis nonzero. Document the actual denominator so API consumers interpret the metric correctly.
affectedPercentage:
type: number
nullable: true
description: Rounded to 1 decimal. Null when dependentsAnalyzed is 0, to avoid a misleading 0%.
backend/src/api/public/v1/akrites-external/openapi.yaml:1099
- This GET route uses the shared 60/min
rateLimiter, not the independent blast-radius limiter used by POST. The documented 429 behavior therefore incorrectly says polling is independently rate-limited. Either give polling its own limiter or update the contract (including the Blast Radius tag description) to describe the shared limit.
'429':
description: Too many requests — rate-limited independently of the other endpoints.
| // Reachability downloads and analyzes up to 25 dependents (4 at a time, each with | ||
| // up to 3 agent attempts) — the slowest stage, so it gets the largest ceiling. | ||
| const { blastRadiusReachability } = proxyActivities<typeof activities>({ | ||
| startToCloseTimeout: '1 hour', |
| const conclusive = results.filter((r) => r.verdict !== 'unclear') | ||
|
|
||
| return { | ||
| totalDependentsInRange: dependentsAnalyzed + dependentsExcludedUpfront, |
Comment on lines
+242
to
+244
| const rangeEnd = new Date() | ||
| const rangeStart = new Date(rangeEnd) | ||
| rangeStart.setUTCDate(rangeStart.getUTCDate() - 30) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes the blast-radius analysis pipeline implementation (CM-1328) by porting the full 4-stage methodology from the Python PoC into the Temporal worker. Includes database schema migrations (from prior PR on this branch), complete data-access-layer for querying/persisting analysis state, and all 4 pipeline stages (intel, dependents, reachability, report) with Agent SDK integration for vulnerability intelligence and reachability analysis. Replaces the PoC's JSON-file-based run artifacts with database-backed resumable state, and integrates Opus/Sonnet agents for the knowledge-intensive stages (Stage 1 intel and Stage 3 per-dependent reachability).
Changes
Database (migrations, already in this branch)
blast_radius_analyses,blast_radius_symbol_specs,blast_radius_dependents,blast_radius_verdicts,blast_radius_stage_runs.DAL —
services/libs/data-access-layer/src/packages/blastRadius.tsSemver range helpers —
services/apps/packages_worker/src/blast-radius/semverRange.tsversionsInRanges,rangeIncludesAny(with unparseable-range detection),highestVersion.semverpackage (already a dependency).Clients —
services/apps/packages_worker/src/blast-radius/clients/.patchfiles from GitHub commit/PR URLs.Readable.fromWeb→.pipe()) intotar.extract()'s writable stream, then copy the extracted tree intodestDirwith a path-traversal guard, stripping the package's common top-level directory.npm registry reuse —
npmManifest.tsPackumentVersiontype (used by the ingest pipeline) only declares a few fields; blast-radius also needsdependencies/peerDependencies/optionalDependencies/dist.tarball. Rather than widen the shared type,npmManifest.tsdeclares a localNpmVersionManifest extends PackumentVersionplus anasNpmVersionManifest()cast helper, used at every access site.fetchPackument/fetchBulkPointRange/isFetchErrorclients insrc/npm/instead of ad-hocfetch()calls.Dependents scan —
dependentsScan.tsnpm-high-impactpackage list, extract candidate names, rank by downloads (last calendar month, computed at runtime — not a hardcoded date range), filter by declared dependency + range inclusion.{source, candidatesConsidered, analyzed[], excludedByRange[]}; matches PoC's Stage 2 logic.Agent integration —
services/apps/packages_worker/src/blast-radius/agent/@anthropic-ai/claude-agent-sdk'squery()with:BLAST_RADIUS_ANTHROPIC_API_KEYat runtime; if present, passes asenvoption; if absent, subprocess inheritsprocess.envfor CLI auth.tools: ['Read','Grep','Glob']+disallowedToolsbelt-and-suspenders.outputFormat: {type:'json_schema', schema}.permissionMode: 'bypassPermissions'+allowDangerouslySkipPermissions: true.AbortController.Stages —
services/apps/packages_worker/src/blast-radius/stages/scanDependents, persist analyzed + excluded-by-range rows.fs.mkdtemp, 3 retries with 15×attemptNumber sec backoff, claude-sonnet-5 agent. Error verdicts on persistent failure. AtoPromptSymbolSpec()converter maps the DAL's raw DB row onto the prompt-facingSymbolSpectype instead of casting.Activities + workflow —
services/apps/packages_worker/src/blast-radius/blastRadiusStart/blastRadiusFailown all analysis-lifecycle DB writes (create/mark-running/fail), plus the 4 stage-wrapping activities (Intel, Dependents, Reachability, Report).analyzeBlastRadiuswith real 4-stage orchestration viaproxyActivities. The workflow body does no DB/DAL access directly (Temporal determinism) — all lifecycle and stage work goes through proxied activities, looped over[stageName, runFn]tuples with per-stage failure handling.services/apps/packages_worker/src/activities.tsfor worker discovery.Design decisions (non-obvious)
envoption replaces rather than mergesprocess.env, so we build a merged env with the key if present, or omitenventirely to inherit.stage_runs.model.fs.mkdtempdir, cleaned up infinallyblock post-verdict. Avoids shared-state accumulation.stage_run.statusnot file presence.Status
pnpm tsc-check/pnpm lint) in bothpackages_workeranddata-access-layer, noanywarnings.packages_worker(18 in blast-radius: 16 semverRange + 2 ecosystemSupport), no regressions elsewhere.Limitations / future work
Type of change
JIRA ticket
CM-1328