-
-
Notifications
You must be signed in to change notification settings - Fork 220
feat: detect npm publish security downgrade #1053
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
e01c4aa
9e30e9f
aa4bf07
30b8653
495d79d
e3424a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,18 +1,42 @@ | ||||||||||||||||||||||||||||
| import type { Packument, SlimPackument, SlimVersion, SlimPackumentVersion } from '#shared/types' | ||||||||||||||||||||||||||||
| import type { | ||||||||||||||||||||||||||||
| Packument, | ||||||||||||||||||||||||||||
| SlimPackument, | ||||||||||||||||||||||||||||
| SlimVersion, | ||||||||||||||||||||||||||||
| SlimPackumentVersion, | ||||||||||||||||||||||||||||
| PackumentVersion, | ||||||||||||||||||||||||||||
| PublishTrustLevel, | ||||||||||||||||||||||||||||
| } from '#shared/types' | ||||||||||||||||||||||||||||
| import { NPM_REGISTRY } from '~/utils/npm/common' | ||||||||||||||||||||||||||||
| import { extractInstallScriptsInfo } from '~/utils/install-scripts' | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| /** Number of recent versions to include in initial payload */ | ||||||||||||||||||||||||||||
| const RECENT_VERSIONS_COUNT = 5 | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| function hasAttestations(version: PackumentVersion): boolean { | ||||||||||||||||||||||||||||
| return Boolean(version.dist.attestations) | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| function hasTrustedPublisher(version: PackumentVersion): boolean { | ||||||||||||||||||||||||||||
| return Boolean(version._npmUser?.trustedPublisher) | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| function getTrustLevel(version: PackumentVersion): PublishTrustLevel { | ||||||||||||||||||||||||||||
| if (hasAttestations(version)) return 'provenance' | ||||||||||||||||||||||||||||
| if (hasTrustedPublisher(version)) return 'trustedPublisher' | ||||||||||||||||||||||||||||
| return 'none' | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||
| * Transform a full Packument into a slimmed version for client-side use. | ||||||||||||||||||||||||||||
| * Reduces payload size by: | ||||||||||||||||||||||||||||
| * - Removing readme (fetched separately) | ||||||||||||||||||||||||||||
| * - Including only: 5 most recent versions + one version per dist-tag + requested version | ||||||||||||||||||||||||||||
| * - Stripping unnecessary fields from version objects | ||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||
| function transformPackument(pkg: Packument, requestedVersion?: string | null): SlimPackument { | ||||||||||||||||||||||||||||
| export function transformPackument( | ||||||||||||||||||||||||||||
| pkg: Packument, | ||||||||||||||||||||||||||||
| requestedVersion?: string | null, | ||||||||||||||||||||||||||||
| ): SlimPackument { | ||||||||||||||||||||||||||||
| // Get versions pointed to by dist-tags | ||||||||||||||||||||||||||||
| const distTagVersions = new Set(Object.values(pkg['dist-tags'] ?? {})) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
@@ -35,6 +59,17 @@ function transformPackument(pkg: Packument, requestedVersion?: string | null): S | |||||||||||||||||||||||||||
| includedVersions.add(requestedVersion) | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| const securityVersions = Object.entries(pkg.versions).map(([version, metadata]) => { | ||||||||||||||||||||||||||||
| const trustLevel = getTrustLevel(metadata) | ||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||
| version, | ||||||||||||||||||||||||||||
| time: pkg.time[version], | ||||||||||||||||||||||||||||
| hasProvenance: trustLevel !== 'none', | ||||||||||||||||||||||||||||
| trustLevel, | ||||||||||||||||||||||||||||
| deprecated: metadata.deprecated, | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| // Build filtered versions object with install scripts info per version | ||||||||||||||||||||||||||||
| const filteredVersions: Record<string, SlimVersion> = {} | ||||||||||||||||||||||||||||
| let versionData: SlimPackumentVersion | null = null | ||||||||||||||||||||||||||||
|
|
@@ -52,8 +87,12 @@ function transformPackument(pkg: Packument, requestedVersion?: string | null): S | |||||||||||||||||||||||||||
| installScripts: installScripts ?? undefined, | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| const trustLevel = getTrustLevel(version) | ||||||||||||||||||||||||||||
| const hasProvenance = trustLevel !== 'none' | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| filteredVersions[v] = { | ||||||||||||||||||||||||||||
| ...((version?.dist as { attestations?: unknown }) ? { hasProvenance: true } : {}), | ||||||||||||||||||||||||||||
| hasProvenance, | ||||||||||||||||||||||||||||
| trustLevel, | ||||||||||||||||||||||||||||
|
Comment on lines
+90
to
+95
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same Lines 90–91 replicate the same Proposed fix- const trustLevel = getTrustLevel(version)
- const hasProvenance = trustLevel !== 'none'
+ const trustLevel = getTrustLevel(version)
+ const hasProvenance = hasAttestations(version)📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||
| version: version.version, | ||||||||||||||||||||||||||||
| deprecated: version.deprecated, | ||||||||||||||||||||||||||||
| tags: version.tags as string[], | ||||||||||||||||||||||||||||
|
|
@@ -91,6 +130,7 @@ function transformPackument(pkg: Packument, requestedVersion?: string | null): S | |||||||||||||||||||||||||||
| 'bugs': pkg.bugs, | ||||||||||||||||||||||||||||
| 'requestedVersion': versionData, | ||||||||||||||||||||||||||||
| 'versions': filteredVersions, | ||||||||||||||||||||||||||||
| 'securityVersions': securityVersions, | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| import type { PackageVersionInfo, PublishTrustLevel } from '#shared/types' | ||
| import { compare } from 'semver' | ||
|
|
||
| export interface PublishSecurityDowngrade { | ||
| downgradedVersion: string | ||
| downgradedPublishedAt?: string | ||
| trustedVersion: string | ||
| trustedPublishedAt?: string | ||
| } | ||
|
|
||
| type VersionWithIndex = PackageVersionInfo & { | ||
| index: number | ||
| timestamp: number | ||
| trustRank: number | ||
| } | ||
|
|
||
| const TRUST_RANK: Record<PublishTrustLevel, number> = { | ||
| none: 0, | ||
| trustedPublisher: 1, | ||
| provenance: 2, | ||
| } | ||
|
|
||
| function getTrustRank(version: PackageVersionInfo): number { | ||
| if (version.trustLevel) return TRUST_RANK[version.trustLevel] | ||
| return version.hasProvenance ? TRUST_RANK.provenance : TRUST_RANK.none | ||
| } | ||
|
|
||
| function toTimestamp(time?: string): number { | ||
| if (!time) return Number.NaN | ||
| return Date.parse(time) | ||
| } | ||
|
|
||
| function sortByRecency(a: VersionWithIndex, b: VersionWithIndex): number { | ||
| const aValid = !Number.isNaN(a.timestamp) | ||
| const bValid = !Number.isNaN(b.timestamp) | ||
|
|
||
| if (!aValid && !bValid) { | ||
| // Fall back to semver comparison if no valid timestamps | ||
| const semverOrder = compare(b.version, a.version) | ||
| if (semverOrder !== 0) return semverOrder | ||
|
|
||
| // If semver is also equal, maintain original order | ||
| return a.index - b.index | ||
| } | ||
|
|
||
| if (aValid !== bValid) { | ||
| return aValid ? -1 : 1 | ||
| } | ||
|
|
||
| return b.timestamp - a.timestamp | ||
| } | ||
|
|
||
| /** | ||
| * Detects a security downgrade for a specific viewed version. | ||
| * A version is considered downgraded when it has no provenance and | ||
| * there exists an older trusted release. | ||
| */ | ||
| export function detectPublishSecurityDowngradeForVersion( | ||
| versions: PackageVersionInfo[], | ||
| viewedVersion: string, | ||
| ): PublishSecurityDowngrade | null { | ||
| if (versions.length < 2 || !viewedVersion) return null | ||
|
|
||
| const sorted = versions | ||
| .map((version, index) => ({ | ||
| ...version, | ||
| index, | ||
| timestamp: toTimestamp(version.time), | ||
| trustRank: getTrustRank(version), | ||
| })) | ||
| .sort(sortByRecency) | ||
|
|
||
| const currentIndex = sorted.findIndex(version => version.version === viewedVersion) | ||
| if (currentIndex === -1) return null | ||
|
|
||
| const current = sorted.at(currentIndex) | ||
| if (!current) return null | ||
|
|
||
| let strongestOlder: VersionWithIndex | null = null | ||
| for (const version of sorted.slice(currentIndex + 1)) { | ||
| if (!strongestOlder || version.trustRank > strongestOlder.trustRank) { | ||
| strongestOlder = version | ||
| } | ||
| } | ||
|
|
||
| if (!strongestOlder || strongestOlder.trustRank <= current.trustRank) return null | ||
|
|
||
| return { | ||
| downgradedVersion: current.version, | ||
| downgradedPublishedAt: current.time, | ||
| trustedVersion: strongestOlder.version, | ||
| trustedPublishedAt: strongestOlder.time, | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hasProvenanceconflates trusted-publisher evidence with attestation provenance.On line 64,
hasProvenance: trustLevel !== 'none'means a version published via a trusted publisher (OIDC) but without SLSA attestations is marked ashasProvenance: true. This is semantically incorrect — "provenance" specifically refers to verifiable build attestations, not publisher identity trust. The downstreamgetTrustRankfallback inpublish-security.ts(line 25:return version.hasProvenance ? TRUST_RANK.provenance : TRUST_RANK.none) would then incorrectly elevate atrustedPublisherversion toprovenancerank whentrustLevelis absent.Consider using the attestation-only check for
hasProvenanceand relying ontrustLevelfor the broader trust signal:🐛 Proposed fix
return { version, time: pkg.time[version], - hasProvenance: trustLevel !== 'none', + hasProvenance: hasAttestations(metadata), trustLevel, deprecated: metadata.deprecated, }📝 Committable suggestion