feat: Supply chain attack prevention — version quarantine, new package & low download checks#16
Merged
ProduktEntdecker merged 3 commits intomainfrom Apr 2, 2026
Merged
Conversation
…kage detection, low download check Add three heuristics that run in parallel with existing OSV/CVE checks to catch supply chain attacks like the Axios compromise (March 2026): - Version Quarantine (72h): flags recently published versions and suggests the previous stable release as alternative - New Package Detection (7d): flags packages created less than a week ago - Low Downloads (<100/week): flags packages with minimal community adoption All three would have caught the Axios attack vector (plain-crypto-js). Registry checks fail-open and use 'ask' (never 'deny') to avoid false-positive blocking. Zero additional latency — runs parallel to OSV. Closes #15 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughFügt parallele Supply‑Chain‑Registry‑Checks (npm, PyPI, Homebrew) und eine neue Entscheidungsfunktion Changes
Sequence DiagramsequenceDiagram
rect rgba(200,200,255,0.5)
actor Client
end
rect rgba(200,255,200,0.5)
participant Index
end
rect rgba(255,200,200,0.5)
participant OSVCheck
participant RegistryCheck
end
rect rgba(255,255,200,0.5)
participant Decision
end
Client->>Index: install(name, version, ecosystem)
par OSV & Registry Parallel
Index->>OSVCheck: checkOSV(name, version, ecosystem)
OSVCheck-->>Index: vulnerabilities | error (fail‑closed)
and
Index->>RegistryCheck: checkRegistryMetadata(name, version, ecosystem)
RegistryCheck-->>Index: RegistryResult{status, signals, previousStableVersion} (fail‑open → empty signals on error)
end
Index->>Decision: makeFullDecision(vulnerabilities, signals)
alt CVE denies
Decision-->>Client: deny (append registry details if signals exist)
else CVE allows & signals present
Decision-->>Client: ask (reason from supply‑chain signals)
else CVE asks
Decision-->>Client: ask (merge CVE + registry reasons)
else no signals
Decision-->>Client: allow/ask (CVE result only)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/index.ts (1)
23-23: Unused import:makeDecisionis no longer called directly.The
makeDecisionfunction is imported but onlymakeFullDecisionis used in this file (line 211).makeDecisionis called internally bymakeFullDecisionindecision.ts, so this import is redundant here.Suggested fix
-import { makeDecision, makeFullDecision, type Vulnerability } from './decision.js'; +import { makeFullDecision, type Vulnerability } from './decision.js';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` at line 23, The import line currently brings in makeDecision, makeFullDecision, and the Vulnerability type even though makeDecision is unused; remove makeDecision from the import to eliminate the unused symbol and keep makeFullDecision and Vulnerability (i.e., update the import that references makeDecision/makeFullDecision/Vulnerability to only import makeFullDecision and type Vulnerability) so there are no unused imports reported.src/registry.test.ts (1)
204-238: Consider adding boundary value tests.The tests cover
< 100and>= 100(with 50000), but no test verifies the exact boundary at 100 downloads. Similar pattern applies to 72h quarantine and 7d new-package thresholds. While the current tests provide good coverage, exact boundary tests can catch off-by-one errors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/registry.test.ts` around lines 204 - 238, Add boundary-value tests around the threshold checks: in registry.test.ts add a case in the "npm — low downloads" suite that calls checkRegistryMetadata with npmDownloadsResponse(100) and asserts the low-downloads signal behaves exactly as expected at 100 (presence/absence and severity/detail). Similarly add tests for the 72-hour quarantine and 7-day new-package thresholds by constructing registry responses with package creation/publish timestamps exactly at those boundaries and asserting checkRegistryMetadata returns/does not return the "quarantine" and "new-package" signals as intended; use the same helpers (npmDownloadsResponse, npmRegistryResponse) and the checkRegistryMetadata function to locate where to add these specs.src/registry.ts (1)
38-38: Pre-release pattern may not catch all conventions.The regex covers common patterns but misses some conventions like
1.0.0-0(numeric pre-release),1.0.0-SNAPSHOT, or1.0.0.dev1(PEP 440 dev releases without hyphen). Consider if broader coverage is needed.What are all the common pre-release version string patterns for npm and PyPI packages?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/registry.ts` at line 38, The PRE_RELEASE_PATTERN constant in registry.ts is too narrow and misses variants like numeric pre-releases (e.g., -0), SNAPSHOT, and PEP 440 style dev releases (e.g., .dev1) or dot-separated identifiers; update PRE_RELEASE_PATTERN to match additional common conventions including numeric prerelease tokens (e.g., -\d+), SNAPSHOT (case-insensitive), PEP 440 .devN and .a/.b/.rc forms, and allow both hyphen and dot separators while keeping existing alpha/beta/rc/canary/dev/preview/next/experimental matches; modify the PRE_RELEASE_PATTERN regular expression to cover these cases and ensure it remains case-insensitive and well-documented in a nearby comment for future maintenance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/registry.ts`:
- Line 204: The current fallback for resolvedVersion uses
Object.keys(releases).pop(), which assumes ordering; instead, when version is
not provided inspect the releases map to pick the release whose files contain
the newest upload_time_iso_8601. Implement logic in the code path around
resolvedVersion: iterate releases entries (version string -> file array), for
each file parse upload_time_iso_8601 (or upload_time) to find that version's
latest timestamp, track the version with the overall most recent timestamp, and
set resolvedVersion to that version; keep the existing behavior if the releases
object is empty or timestamps are missing by falling back to a safe default.
---
Nitpick comments:
In `@src/index.ts`:
- Line 23: The import line currently brings in makeDecision, makeFullDecision,
and the Vulnerability type even though makeDecision is unused; remove
makeDecision from the import to eliminate the unused symbol and keep
makeFullDecision and Vulnerability (i.e., update the import that references
makeDecision/makeFullDecision/Vulnerability to only import makeFullDecision and
type Vulnerability) so there are no unused imports reported.
In `@src/registry.test.ts`:
- Around line 204-238: Add boundary-value tests around the threshold checks: in
registry.test.ts add a case in the "npm — low downloads" suite that calls
checkRegistryMetadata with npmDownloadsResponse(100) and asserts the
low-downloads signal behaves exactly as expected at 100 (presence/absence and
severity/detail). Similarly add tests for the 72-hour quarantine and 7-day
new-package thresholds by constructing registry responses with package
creation/publish timestamps exactly at those boundaries and asserting
checkRegistryMetadata returns/does not return the "quarantine" and "new-package"
signals as intended; use the same helpers (npmDownloadsResponse,
npmRegistryResponse) and the checkRegistryMetadata function to locate where to
add these specs.
In `@src/registry.ts`:
- Line 38: The PRE_RELEASE_PATTERN constant in registry.ts is too narrow and
misses variants like numeric pre-releases (e.g., -0), SNAPSHOT, and PEP 440
style dev releases (e.g., .dev1) or dot-separated identifiers; update
PRE_RELEASE_PATTERN to match additional common conventions including numeric
prerelease tokens (e.g., -\d+), SNAPSHOT (case-insensitive), PEP 440 .devN and
.a/.b/.rc forms, and allow both hyphen and dot separators while keeping existing
alpha/beta/rc/canary/dev/preview/next/experimental matches; modify the
PRE_RELEASE_PATTERN regular expression to cover these cases and ensure it
remains case-insensitive and well-documented in a nearby comment for future
maintenance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3c9dab68-8cbe-4116-baf6-473e4576c91f
📒 Files selected for processing (4)
src/decision.tssrc/index.tssrc/registry.test.tssrc/registry.ts
PyPI's JSON API does not guarantee the order of releases in the releases object. Replace Object.keys().pop() with timestamp-based resolution to find the actual latest version and previous stable version. Addresses CodeRabbit review on PR #16. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/registry.ts`:
- Around line 101-107: The current Promise.all call with fetchWithTimeout for
registryResp and downloadsResp causes any downloads failure to reject the whole
batch and drop registry signals in checkRegistryMetadata; replace the
Promise.all with Promise.allSettled for the two fetches, extract registryResp
from its settled result even if downloadsResp failed, and then treat
downloadsResp failure (rejected or non-OK) as "downloads unavailable" without
throwing—update the downloads check logic to read the settled downloads result,
parse it only when status === "fulfilled" and response.ok, and proceed to
compute registry signals from registryResp regardless of downloads outcome.
- Line 38: PRE_RELEASE_PATTERN currently only matches hyphenated pre-release
tags (npm style); update the PRE_RELEASE_PATTERN definition so it also detects
PEP 440 pre-release forms used by PyPI (e.g., suffixes like aN, bN, rcN, and
.devN without a hyphen) and keep the existing hyphenated alternatives; modify
the single PRE_RELEASE_PATTERN constant (used by checkPyPI) to include both
hyphenated keywords and PEP 440-style suffixes so checkPyPI correctly treats
those versions as pre-releases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
- Extend PRE_RELEASE_PATTERN to match PEP 440 suffixes (1.0a1, 1.0b2, 1.0rc1, 1.0.dev1) alongside npm-style (-alpha, -beta) - Use Promise.allSettled instead of Promise.all so downloads API failure doesn't discard registry signals (quarantine, new-package) Addresses CodeRabbit re-review on PR #16. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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
Adds three supply chain heuristics that run in parallel with existing OSV/CVE checks — zero additional latency:
All three would have caught the Axios supply chain attack (March 31, 2026) where
plain-crypto-jswas hours old, had 0 downloads, and no history.Design decisions
ask(neverdeny) — heuristics have false-positive potential, human decidesregistry.tsalongside existingosv.ts— clean separationFiles changed
src/registry.ts(new) — npm/PyPI registry API calls + 3 heuristicssrc/registry.test.ts(new) — 16 tests with mocked fetchsrc/decision.ts— AddedmakeFullDecisioncombining CVE + supply chain signalssrc/index.ts— Wired parallel registry checksTest plan
Closes #15
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Verhalten
Tests