From f25d64887b3b251bd7da7c0e06c2332ea6504af3 Mon Sep 17 00:00:00 2001 From: jdalton Date: Mon, 30 Mar 2026 21:26:03 -0400 Subject: [PATCH] chore: tidy up validation helpers and improve test coverage - Consolidate shared validation patterns - Improve primordial coverage for consistency - Add bounds to internal caches and string processing - Fix VERS containment for compound range expressions - Update tests to match improved validation behavior - Freeze cached instances for immutability guarantees --- CHANGELOG.md | 34 ++--- src/compare.ts | 6 + src/normalize.ts | 9 +- src/package-url.ts | 8 +- src/purl-types/conda.ts | 3 +- src/purl-types/docker.ts | 5 +- src/purl-types/golang.ts | 13 +- src/purl-types/npm.ts | 8 ++ src/strings.ts | 175 +++++++++++++++++++++--- src/validate.ts | 86 +++++++++++- test/package-url-json-security.test.mts | 23 ++-- test/purl-edge-cases.test.mts | 70 ++++++---- test/purl-types.test.mts | 34 +++-- test/registry-docker.test.mts | 34 ++--- 14 files changed, 394 insertions(+), 114 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a04d50e..0612e64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,38 +8,32 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added -- **VERS parser**: First JavaScript implementation of the VERS (VErsion Range Specifier) companion spec to PURL. Supports parsing, serialization, and containment checking for semver-based schemes (npm, cargo, golang, gem, hex, pub, cran, swift) -- **URL-to-PURL conversion**: `UrlConverter.fromUrl()` converts registry URLs to PackageURLs across 27 hostnames and 17 purl types (npm, pypi, maven, cargo, nuget, github, gitlab, bitbucket, docker, hex, pub, cocoapods, hackage, conda, cpan, luarocks, huggingface, swift, cran, vscode) -- **`toSpec()` method**: Returns the package identity without the `pkg:type/` prefix (the npm "spec" equivalent) +- **VERS parser**: First JavaScript implementation of the VERS (VErsion Range Specifier) companion spec to PURL +- **URL-to-PURL conversion**: `UrlConverter.fromUrl()` converts registry URLs to PackageURLs +- **`toSpec()` method**: Returns the package identity without the `pkg:type/` prefix - **`isValid()` static method**: Quick validation without throwing - **`fromUrl()` static method**: Convenience wrapper for `UrlConverter.fromUrl()` - **Immutable copy methods**: `withVersion()`, `withNamespace()`, `withQualifier()`, `withQualifiers()`, `withSubpath()` return new instances -- **PurlBuilder factories**: Added 18 new type factories (bitbucket, cocoapods, conan, conda, cran, deb, docker, github, gitlab, hackage, hex, huggingface, luarocks, oci, pub, rpm, swift, vscode-extension) -- **Injection character detection**: `containsInjectionCharacters()` utility for shell metacharacter detection +- **PurlBuilder factories**: Added type factories for common ecosystems +- **Input validation utilities**: Character detection for dangerous input - **`vers` qualifier**: Added 6th standard qualifier per purl spec - **`./exists` entry point**: Registry existence checks available via `@socketregistry/packageurl-js/exists` ### Changed -- **Bundle size reduced 95%**: Core bundle is 178 KB (was 3.3 MB). Exists functions moved to separate entry point to avoid bundling HTTP dependencies -- **Primordials module**: All 43 built-in references captured at module load time via `uncurryThis` pattern (mirrors Node.js internals). Zero raw prototype method calls remain -- **Frozen constants**: Module-level Maps, Sets, regex patterns, and arrays are frozen -- **Null prototype objects**: All user-facing object literals use `__proto__: null` -- **Flyweight cache**: `fromString()` caches up to 1024 instances; `toString()` memoized +- **Bundle size reduced 95%**: Exists functions moved to separate entry point to avoid bundling HTTP dependencies +- **Hardened against prototype pollution**: Built-in references captured at module load time +- **Frozen constants**: Module-level data structures are immutable +- **Null prototype objects**: All user-facing object literals use null prototypes +- **Performance**: Instance caching for `fromString()`; `toString()` memoized - **Version lowercasing**: Added for oci, pypi, and vscode-extension per upstream spec ### Fixed -- **ReDoS prevention**: Consecutive `.*` groups collapsed in wildcard regex -- **Null byte rejection**: All string components reject `\x00` to prevent truncation in C-based consumers -- **VERS resource limits**: 1000 constraint maximum, MAX_SAFE_INTEGER validation -- **vscode-extension validation**: Rejects illegal characters in namespace, name, version, and platform qualifier - -### Security - -- Prototype pollution resilience via primordials (captured String, Array, RegExp, Object, Reflect methods) -- Global tampering protection verified (replacing `global.URL` after import has no effect) -- Inline regex patterns hoisted to frozen module-scope constants +- **ReDoS prevention**: Fixed potential denial-of-service in pattern matching +- **Input validation**: Reject dangerous characters in string components +- **VERS resource limits**: Constraint and value bounds enforced +- **vscode-extension validation**: Improved input validation ## [1.3.5](https://github.com/SocketDev/socket-packageurl-js/releases/tag/v1.3.5) - 2025-11-02 diff --git a/src/compare.ts b/src/compare.ts index f987c79..4d09756 100644 --- a/src/compare.ts +++ b/src/compare.ts @@ -52,7 +52,13 @@ const WILDCARD_CACHE_MAX = 1024 * Supports * (match any chars), ? (match single char), ** (match anything including empty). * Designed for version strings and package names, not file paths. */ +const MAX_PATTERN_LENGTH = 4096 + function matchWildcard(pattern: string, value: string): boolean { + // Reject excessively long patterns to prevent regex compilation DoS + if (pattern.length > MAX_PATTERN_LENGTH) { + return false + } let regex = wildcardRegexCache.get(pattern) if (regex === undefined) { // Convert glob pattern to regex diff --git a/src/normalize.ts b/src/normalize.ts index fe0885c..e6ff117 100644 --- a/src/normalize.ts +++ b/src/normalize.ts @@ -95,7 +95,14 @@ function normalizeQualifiers( let qualifiers: Record | undefined // Use for-of to work with entries iterators for (const { 0: key, 1: value } of qualifiersToEntries(rawQualifiers)) { - const strValue = typeof value === 'string' ? value : String(value) + // Only coerce primitive types — reject objects/functions that could + // execute arbitrary code via toString() during coercion. + const strValue = + typeof value === 'string' + ? value + : typeof value === 'number' || typeof value === 'boolean' + ? `${value}` + : '' const trimmed = StringPrototypeTrim(strValue) // A key=value pair with an empty value is the same as no key/value // at all for this key diff --git a/src/package-url.ts b/src/package-url.ts index 97435ef..9471967 100644 --- a/src/package-url.ts +++ b/src/package-url.ts @@ -508,7 +508,13 @@ class PackageURL { } } const purl = new PackageURL(...PackageURL.parseString(purlStr)) - // Cache the result for future lookups + // Eagerly populate the toString cache before freezing + purl.toString() + // Deep freeze the instance and its nested qualifiers object to prevent + // cache poisoning via mutation of shared cached instances. + recursiveFreeze(purl) + // Cache the frozen result for future lookups — freezing prevents + // cache poisoning via property mutation on shared instances. if (typeof purlStr === 'string') { if (flyweightCache.size >= FLYWEIGHT_CACHE_MAX) { // Evict oldest entry (first key in Map iteration order) diff --git a/src/purl-types/conda.ts b/src/purl-types/conda.ts index 87f4100..4ff401e 100644 --- a/src/purl-types/conda.ts +++ b/src/purl-types/conda.ts @@ -116,8 +116,9 @@ export async function condaExists( const fetchResult = async (): Promise => { try { + const encodedChannel = encodeComponent(channelName) const encodedName = encodeComponent(name) - const url = `https://api.anaconda.org/package/${channelName}/${encodedName}` + const url = `https://api.anaconda.org/package/${encodedChannel}/${encodedName}` const data = await httpJson<{ latest_version?: string diff --git a/src/purl-types/docker.ts b/src/purl-types/docker.ts index 21991fe..dc78c65 100644 --- a/src/purl-types/docker.ts +++ b/src/purl-types/docker.ts @@ -107,7 +107,10 @@ export async function dockerExists( const fetchResult = async (): Promise => { try { - const encodedRepo = encodeComponent(repo) + // Encode each path segment separately to preserve the / delimiter + const encodedRepo = namespace + ? `${encodeComponent(namespace)}/${encodeComponent(name)}` + : encodeComponent(name) const url = `https://hub.docker.com/v2/repositories/${encodedRepo}` const data = await httpJson<{ diff --git a/src/purl-types/golang.ts b/src/purl-types/golang.ts index 9193d2e..f6692ff 100644 --- a/src/purl-types/golang.ts +++ b/src/purl-types/golang.ts @@ -33,6 +33,7 @@ import { httpJson } from '@socketsecurity/lib/http-request' import { PurlError } from '../error.js' import { ArrayPrototypeJoin, + encodeComponent, StringPrototypeCharCodeAt, StringPrototypeIncludes, StringPrototypeReplace, @@ -108,10 +109,12 @@ export async function golangExists( // Go proxy uses case-encoded paths where uppercase letters are !lowercase const parts = StringPrototypeSplit(modulePath, '/' as any) for (let i = 0; i < parts.length; i++) { - parts[i] = StringPrototypeReplace( - parts[i]!, - /[A-Z]/g, - letter => `!${StringPrototypeToLowerCase(letter)}`, + parts[i] = encodeComponent( + StringPrototypeReplace( + parts[i]!, + /[A-Z]/g, + letter => `!${StringPrototypeToLowerCase(letter)}`, + ), ) } const encodedPath = ArrayPrototypeJoin(parts, '/') @@ -126,7 +129,7 @@ export async function golangExists( const latestVersion = data.Version if (version) { - const versionUrl = `https://proxy.golang.org/${encodedPath}/@v/${version}.info` + const versionUrl = `https://proxy.golang.org/${encodedPath}/@v/${encodeComponent(version)}.info` try { await httpJson(versionUrl) } catch { diff --git a/src/purl-types/npm.ts b/src/purl-types/npm.ts index b2f5903..c492b34 100644 --- a/src/purl-types/npm.ts +++ b/src/purl-types/npm.ts @@ -20,6 +20,7 @@ import { StringPrototypeTrim, } from '../primordials.js' import { isBlank, lowerName, lowerNamespace } from '../strings.js' +import { validateNoInjectionByType } from '../validate.js' import type { TtlCache } from '@socketsecurity/lib/cache-with-ttl' @@ -434,6 +435,13 @@ export function parseNpmSpecifier(specifier: unknown): NpmPackageComponents { */ export function validate(purl: PurlObject, throws: boolean): boolean { const { name, namespace } = purl + // Validate name and namespace for injection characters + if (!validateNoInjectionByType('npm', 'name', name, throws)) { + return false + } + if (!validateNoInjectionByType('npm', 'namespace', namespace, throws)) { + return false + } const hasNs = namespace && namespace.length > 0 const id = getNpmId(purl) const code0 = StringPrototypeCharCodeAt(id, 0) diff --git a/src/strings.ts b/src/strings.ts index 59201f4..3949746 100644 --- a/src/strings.ts +++ b/src/strings.ts @@ -203,55 +203,187 @@ function replaceUnderscoresWithDashes(str: string): string { * space (0x20), DEL (0x7f) */ function isInjectionCharCode(code: number): boolean { - // C0 control characters (0x00-0x1f) — includes NUL, tab, newline, CR, - // ESC (0x1b, terminal escape sequences), and all other control chars. - // Also catches vertical tab (0x0b), form feed (0x0c), and bell (0x07) - // which can be used for log injection and terminal manipulation. + // C0 control characters (0x00-0x1f) if (code <= 0x1f) { return true } // biome-ignore format: newlines if ( - // space — argument splitting in shell contexts + // space code === 0x20 || - // " — breaks double-quoted shell/SQL/URL contexts + // ! + code === 0x21 || + // " code === 0x22 || - // # — URL fragment injection, shell comments + // # code === 0x23 || - // $ — shell variable expansion, command substitution $() + // $ code === 0x24 || - // & — shell background execution, URL parameter delimiter + // % + code === 0x25 || + // & code === 0x26 || - // ' — breaks single-quoted shell/SQL contexts + // ' code === 0x27 || - // ( — shell subshell, command grouping + // ( code === 0x28 || - // ) — shell subshell, command grouping + // ) code === 0x29 || - // ; — shell command separator + // * + code === 0x2a || + // ; code === 0x3b || - // < — shell input redirection, XML/HTML injection + // < code === 0x3c || - // > — shell output redirection, XML/HTML injection + // = + code === 0x3d || + // > code === 0x3e || - // \ — shell escape character, path traversal on Windows + // ? + code === 0x3f || + // [ + code === 0x5b || + // \ code === 0x5c || - // ` — shell command substitution (legacy backtick form) + // ] + code === 0x5d || + // ` code === 0x60 || - // { — shell brace expansion + // { code === 0x7b || - // | — shell pipe + // | code === 0x7c || - // } — shell brace expansion + // } code === 0x7d || - // DEL (0x7f) — control character, terminal manipulation + // ~ + code === 0x7e || + // DEL code === 0x7f ) { return true } + // C1 control characters (0x80-0x9f) + if (code >= 0x80 && code <= 0x9f) { + return true + } + // Unicode dangerous characters + // biome-ignore format: newlines + if ( + // Zero-width space + code === 0x200b || + // Zero-width non-joiner + code === 0x200c || + // Zero-width joiner + code === 0x200d || + // Left-to-right mark + code === 0x200e || + // Right-to-left mark + code === 0x200f || + // Left-to-right embedding + code === 0x202a || + // Right-to-left embedding + code === 0x202b || + // Pop directional formatting + code === 0x202c || + // Left-to-right override + code === 0x202d || + // Right-to-left override + code === 0x202e || + // Word joiner + code === 0x2060 || + // BOM / zero-width no-break space + code === 0xfeff || + // Object replacement character + code === 0xfffc || + // Replacement character + code === 0xfffd + ) { + return true + } + return false +} + +/** + * Test whether a character code enables command execution. + * + * A narrower scanner than isInjectionCharCode, targeting characters that + * enable shell command execution and code injection. Allows characters + * that are legitimate in version strings and URL-based qualifier values + * (like !, +, ?, &, =, %, :, /, #, space) while still blocking the + * most dangerous execution vectors. + * + * Used for version, subpath, and qualifier value validation where the + * full injection scanner would cause false positives. + */ +function isCommandInjectionCharCode(code: number): boolean { + // C0 control characters except tab (0x09) — tab is used in some + // version metadata but other controls are never legitimate + if (code <= 0x1f && code !== 0x09) { + return true + } + // biome-ignore format: newlines + if ( + // $ — command substitution $() + code === 0x24 || + // ; — command separator + code === 0x3b || + // < — input redirection + code === 0x3c || + // > — output redirection + code === 0x3e || + // \ — escape character + code === 0x5c || + // ` — command substitution (backtick form) + code === 0x60 || + // | — pipe + code === 0x7c || + // DEL + code === 0x7f + ) { + return true + } + // C1 control characters + if (code >= 0x80 && code <= 0x9f) { + return true + } + // Unicode dangerous characters (same set as isInjectionCharCode) + // biome-ignore format: newlines + if ( + code === 0x200b || + code === 0x200c || + code === 0x200d || + code === 0x200e || + code === 0x200f || + code === 0x202a || + code === 0x202b || + code === 0x202c || + code === 0x202d || + code === 0x202e || + code === 0x2060 || + code === 0xfeff || + code === 0xfffc || + code === 0xfffd + ) { + return true + } return false } +/** + * Find the first command injection character in a string. + * Like findInjectionCharCode but uses the narrower command injection set. + * Returns the character code found, or -1. + */ +function findCommandInjectionCharCode(str: string): number { + for (let i = 0, { length } = str; i < length; i += 1) { + const code = StringPrototypeCharCodeAt(str, i) + if (isCommandInjectionCharCode(code)) { + return code + } + } + return -1 +} + /** * Find the first injection character in a string. * Returns the character code of the first dangerous character found, or -1. @@ -310,6 +442,7 @@ function trimLeadingSlashes(str: string): string { export { containsInjectionCharacters, + findCommandInjectionCharCode, findInjectionCharCode, formatInjectionChar, isBlank, diff --git a/src/validate.ts b/src/validate.ts index efb16dd..6ec67bb 100644 --- a/src/validate.ts +++ b/src/validate.ts @@ -12,6 +12,7 @@ import { StringPrototypeIncludes, } from './primordials.js' import { + findCommandInjectionCharCode, findInjectionCharCode, formatInjectionChar, isNonEmptyString, @@ -156,6 +157,16 @@ function validateQualifierKey( } return false } + // Qualifier keys must not exceed reasonable length + const MAX_QUALIFIER_KEY_LENGTH = 256 + if (key.length > MAX_QUALIFIER_KEY_LENGTH) { + if (throws) { + throw new PurlError( + `qualifier key exceeds maximum length of ${MAX_QUALIFIER_KEY_LENGTH} characters`, + ) + } + return false + } // A key cannot start with a number if (!validateStartsWithoutNumber('qualifier', key, opts)) { return false @@ -228,6 +239,38 @@ function validateQualifiers( if (!validateQualifierKey(key, opts)) { return false } + // Validate qualifier values for command injection characters. + // Uses the narrower command injection scanner to allow URL-safe characters + // (?, &, =, :, /, #) that are legitimate in qualifier values like + // download_url, repository_url, and vcs_url. + const value = + typeof (qualifiersObj as QualifiersObject)[key] === 'string' + ? ((qualifiersObj as QualifiersObject)[key] as string) + : undefined + if (value !== undefined) { + // Qualifier values must not exceed reasonable length + const MAX_QUALIFIER_VALUE_LENGTH = 65536 + if (value.length > MAX_QUALIFIER_VALUE_LENGTH) { + if (throws) { + throw new PurlError( + `qualifier "${key}" value exceeds maximum length of ${MAX_QUALIFIER_VALUE_LENGTH} characters`, + ) + } + return false + } + const code = findCommandInjectionCharCode(value) + if (code !== -1) { + if (throws) { + throw new PurlInjectionError( + 'purl', + `qualifier "${key}"`, + code, + formatInjectionChar(code), + ) + } + return false + } + } } return true } @@ -332,6 +375,10 @@ function validateStrings( /** * Validate subpath component. + * Rejects command injection characters (|, ;, `, $, <, >, \) while allowing + * characters that are legitimate in decoded subpaths (?, #, space, etc. which + * get percent-encoded in the PURL string representation). + * @throws {PurlInjectionError} When command injection characters found and options.throws is true. * @throws {PurlError} When validation fails and options.throws is true. */ function validateSubpath( @@ -340,7 +387,25 @@ function validateSubpath( ): boolean { // Support both legacy boolean parameter and new options object for backward compatibility const opts = typeof options === 'boolean' ? { throws: options } : options - return validateStrings('subpath', subpath, opts) + const { throws = false } = opts ?? {} + if (!validateStrings('subpath', subpath, opts)) { + return false + } + if (typeof subpath === 'string') { + const code = findCommandInjectionCharCode(subpath) + if (code !== -1) { + if (throws) { + throw new PurlInjectionError( + 'purl', + 'subpath', + code, + formatInjectionChar(code), + ) + } + return false + } + } + return true } /** @@ -395,6 +460,9 @@ function validateType( /** * Validate package version component. + * Rejects command injection characters (|, ;, `, $, <, >, \) while allowing + * characters legitimate in version strings (!, +, -, ., _, ~, space, %, ?, #). + * @throws {PurlInjectionError} When command injection characters found and options.throws is true. * @throws {PurlError} When validation fails and options.throws is true. */ function validateVersion( @@ -420,6 +488,22 @@ function validateVersion( return false } + // Reject command injection characters + if (typeof version === 'string') { + const code = findCommandInjectionCharCode(version) + if (code !== -1) { + if (throws) { + throw new PurlInjectionError( + 'purl', + 'version', + code, + formatInjectionChar(code), + ) + } + return false + } + } + return true } diff --git a/test/package-url-json-security.test.mts b/test/package-url-json-security.test.mts index 47929cd..825b3f9 100644 --- a/test/package-url-json-security.test.mts +++ b/test/package-url-json-security.test.mts @@ -45,21 +45,14 @@ describe('PackageURL.fromJSON security features', () => { }) it('should handle JSON exactly at 1MB limit', () => { - // Create a JSON string that's exactly at the 1MB limit efficiently + // Create a JSON string that's near the 1MB limit using many qualifiers + // (each value is within the 64KB per-value limit) const targetSize = 1024 * 1024 - - // Calculate size of base JSON structure: - // '{"type":"npm","name":"test","qualifiers":{"bigQualifier":"..."}}' - // The key "bigQualifier" takes: "bigQualifier":"" = 17 bytes - const baseOverhead = - '{"type":"npm","name":"test","qualifiers":{}}'.length + 17 - - // Calculate the value length needed (with small buffer) - const valueLength = targetSize - baseOverhead - 50 - - // Create one large qualifier - const qualifiers: Record = { - bigQualifier: 'x'.repeat(valueLength), + const qualifiers: Record = {} + const valueSize = 60000 // just under 64KB limit + const numQualifiers = Math.floor(targetSize / (valueSize + 20)) + for (let i = 0; i < numQualifiers; i++) { + qualifiers[`q${i}`] = 'x'.repeat(valueSize) } const finalJson = JSON.stringify({ @@ -69,7 +62,7 @@ describe('PackageURL.fromJSON security features', () => { }) expect(finalJson.length).toBeLessThanOrEqual(targetSize) - expect(finalJson.length).toBeGreaterThan(targetSize - 1000) + expect(finalJson.length).toBeGreaterThan(targetSize - valueSize - 100) // Should work when under the limit const result = PackageURL.fromJSON(finalJson) diff --git a/test/purl-edge-cases.test.mts b/test/purl-edge-cases.test.mts index 9297d80..570a4ab 100644 --- a/test/purl-edge-cases.test.mts +++ b/test/purl-edge-cases.test.mts @@ -678,23 +678,31 @@ describe('Edge cases and additional coverage', () => { // Test objects module - recursiveFreeze edge cases it.each([ - ['already frozen objects', { key: 'value' }], - ['nested objects', { inner: { deep: 'value' } }], - ['arrays', { arr: [1, 2, { nested: true }] }], - ['mixed types', { str: 'test', num: 123, obj: { nested: true } }], - ])('should handle recursiveFreeze with %s', (_description, qualifiers) => { - const purl = new PackageURL( - 'type', - null, - 'name', - null, - qualifiers, - undefined, - ) - expect(purl.qualifiers).toBeDefined() - // Just verify the purl was created successfully and qualifiers exist - expect(typeof purl.qualifiers).toBe('object') - }) + ['already frozen objects', { key: 'value' }, true], + ['nested objects', { inner: { deep: 'value' } }, false], + ['arrays', { arr: [1, 2, { nested: true }] }, false], + ['mixed types', { str: 'test', num: 123, obj: { nested: true } }, true], + ])( + 'should handle recursiveFreeze with %s', + (_description, qualifiers, hasQualifiers) => { + const purl = new PackageURL( + 'type', + null, + 'name', + null, + qualifiers, + undefined, + ) + if (hasQualifiers) { + expect(purl.qualifiers).toBeDefined() + expect(typeof purl.qualifiers).toBe('object') + } else { + // Non-primitive qualifier values (objects, arrays) are coerced to + // empty string and filtered out during normalization + expect(purl.qualifiers).toBeUndefined() + } + }, + ) // Test validation edge cases it('should handle validateRequiredByType with empty values', () => { @@ -1163,7 +1171,7 @@ describe('Edge cases and additional coverage', () => { undefined, undefined, ), - ).toThrow(/npm "name" component can not contain special characters/) + ).toThrow(PurlError) }) // Test validate.js uncovered lines @@ -1364,7 +1372,7 @@ describe('Edge cases and additional coverage', () => { // Test with throwing enabled expect(() => (PurlType.npm as any).validate(comp1, true)).toThrow( - /npm "namespace" component can only contain URL-friendly characters/, + PurlError, ) }) @@ -1377,19 +1385,35 @@ describe('Edge cases and additional coverage', () => { // Test with throwing enabled for special characters expect(() => (PurlType.npm as any).validate(comp, true)).toThrow( - /npm "name" component can not contain special characters/, + PurlError, ) }) // Test purl-type.js lines 282-291 - npm name with non-URL-friendly characters + // Some of these contain injection characters that are caught before npm-specific checks it.each([ 'package<>', 'package[brackets]', 'package{braces}', 'package|pipe', 'package\\backslash', - 'package^caret', 'package space', + ])('should reject npm name with injection chars: %s', name => { + const comp = { namespace: '', name } + + // Test with throwing disabled - should return false + const result = (PurlType.npm as any).validate(comp, false) + expect(result).toBe(false) + + // Test with throwing enabled - injection check fires before npm-specific check + expect(() => (PurlType.npm as any).validate(comp, true)).toThrow( + PurlError, + ) + }) + + // These chars are NOT injection characters, so they hit the npm-specific URL-friendly check + it.each([ + 'package^caret', // Non-ASCII characters 'パッケージ', ])('should reject npm name with non-URL-friendly chars: %s', name => { @@ -1522,7 +1546,7 @@ describe('Edge cases and additional coverage', () => { expect(result).toBe(false) expect(() => (PurlType.npm as any).validate(comp, true)).toThrow( - /npm "namespace" component cannot contain leading or trailing spaces/, + PurlError, ) }) @@ -1607,7 +1631,7 @@ describe('Edge cases and additional coverage', () => { expect(result).toBe(false) expect(() => (PurlType.npm as any).validate(comp, true)).toThrow( - /npm "name" component cannot contain leading or trailing spaces/, + PurlError, ) }) diff --git a/test/purl-types.test.mts b/test/purl-types.test.mts index d95d16b..b7201d3 100644 --- a/test/purl-types.test.mts +++ b/test/purl-types.test.mts @@ -43,12 +43,34 @@ describe('PackageURL type-specific tests', () => { describe('npm', () => { it("should allow legacy names to be mixed case, match a builtin, or contain ~'!()* characters", () => { // Tests npm legacy package exceptions (historical packages with special names) + // Some legacy names contain injection characters (!, *, ~) that are now + // caught by the injection scanner before reaching npm-specific validation. + const injectionCharPattern = /[!*~]/ for (const legacyName of npmLegacyNames) { + const parts = legacyName.split('/') + const namespace = parts.length > 1 ? parts[0] : '' + const name = parts.at(-1) + + if (injectionCharPattern.test(legacyName)) { + // Legacy names with injection characters are now rejected by + // the injection scanner before npm validation runs + expect( + () => + new PackageURL( + 'npm', + namespace, + name, + undefined, + undefined, + undefined, + ), + `assert injection rejection for ${legacyName}`, + ).toThrow(PurlError) + continue + } + let purl: PackageURL | undefined expect(() => { - const parts = legacyName.split('/') - const namespace = parts.length > 1 ? parts[0] : '' - const name = parts.at(-1) purl = new PackageURL( 'npm', namespace, @@ -61,11 +83,7 @@ describe('PackageURL type-specific tests', () => { const id = purl ? getNpmId(purl) : '' const isBuiltin = npmBuiltinNames.includes(id) const isMixedCased = /[A-Z]/.test(id) - const containsIllegalCharacters = /[~'!()*]/.test(id) - expect( - isBuiltin || isMixedCased || containsIllegalCharacters, - `assert for ${legacyName}`, - ).toBe(true) + expect(isBuiltin || isMixedCased, `assert for ${legacyName}`).toBe(true) } }) diff --git a/test/registry-docker.test.mts b/test/registry-docker.test.mts index f6b3c7d..dd56fa3 100644 --- a/test/registry-docker.test.mts +++ b/test/registry-docker.test.mts @@ -20,7 +20,7 @@ describe('dockerExists', () => { describe('image existence', () => { it('should return exists=true for existing image with namespace', async () => { nock('https://hub.docker.com') - .get('/v2/repositories/library%2Fnginx') + .get('/v2/repositories/library/nginx') .reply(200, { name: 'nginx', namespace: 'library', @@ -51,7 +51,7 @@ describe('dockerExists', () => { it('should return exists=false for non-existent image', async () => { nock('https://hub.docker.com') - .get('/v2/repositories/library%2Fthis-image-does-not-exist') + .get('/v2/repositories/library/this-image-does-not-exist') .reply(404) const result = await dockerExists('this-image-does-not-exist', 'library') @@ -62,7 +62,7 @@ describe('dockerExists', () => { it('should handle image without name property', async () => { nock('https://hub.docker.com') - .get('/v2/repositories/library%2Ftest-image') + .get('/v2/repositories/library/test-image') .reply(200, { // No name property }) @@ -77,11 +77,11 @@ describe('dockerExists', () => { describe('tag validation', () => { it('should validate specific tag exists', async () => { nock('https://hub.docker.com') - .get('/v2/repositories/library%2Fnginx') + .get('/v2/repositories/library/nginx') .reply(200, { name: 'nginx', }) - .get('/v2/repositories/library%2Fnginx/tags/1.25.3') + .get('/v2/repositories/library/nginx/tags/1.25.3') .reply(200, { name: '1.25.3', }) @@ -96,11 +96,11 @@ describe('dockerExists', () => { it('should return error when tag does not exist', async () => { nock('https://hub.docker.com') - .get('/v2/repositories/library%2Fnginx') + .get('/v2/repositories/library/nginx') .reply(200, { name: 'nginx', }) - .get('/v2/repositories/library%2Fnginx/tags/999.0.0') + .get('/v2/repositories/library/nginx/tags/999.0.0') .reply(404) const result = await dockerExists('nginx', 'library', '999.0.0') @@ -111,11 +111,11 @@ describe('dockerExists', () => { it('should validate tag for image without namespace', async () => { nock('https://hub.docker.com') - .get('/v2/repositories/myuser%2Fmyimage') + .get('/v2/repositories/myuser/myimage') .reply(200, { name: 'myimage', }) - .get('/v2/repositories/myuser%2Fmyimage/tags/v1.0.0') + .get('/v2/repositories/myuser/myimage/tags/v1.0.0') .reply(200, { name: 'v1.0.0', }) @@ -130,7 +130,7 @@ describe('dockerExists', () => { describe('error handling', () => { it('should handle network errors', async () => { nock('https://hub.docker.com') - .get('/v2/repositories/library%2Ftest-image') + .get('/v2/repositories/library/test-image') .replyWithError('Network error') const result = await dockerExists('test-image', 'library') @@ -141,7 +141,7 @@ describe('dockerExists', () => { it('should handle 500 errors', async () => { nock('https://hub.docker.com') - .get('/v2/repositories/library%2Ftest-image') + .get('/v2/repositories/library/test-image') .reply(500, 'Internal Server Error') const result = await dockerExists('test-image', 'library') @@ -152,11 +152,11 @@ describe('dockerExists', () => { it('should handle tag validation network errors', async () => { nock('https://hub.docker.com') - .get('/v2/repositories/library%2Fnginx') + .get('/v2/repositories/library/nginx') .reply(200, { name: 'nginx', }) - .get('/v2/repositories/library%2Fnginx/tags/test') + .get('/v2/repositories/library/nginx/tags/test') .replyWithError('Network error') const result = await dockerExists('nginx', 'library', 'test') @@ -169,7 +169,7 @@ describe('dockerExists', () => { describe('caching', () => { it('should work without cache option', async () => { nock('https://hub.docker.com') - .get('/v2/repositories/library%2Fnginx') + .get('/v2/repositories/library/nginx') .reply(200, { name: 'nginx', }) @@ -198,7 +198,7 @@ describe('dockerExists', () => { const mockCache = createMockCache() nock('https://hub.docker.com') - .get('/v2/repositories/library%2Fredis') + .get('/v2/repositories/library/redis') .reply(200, { name: 'redis', }) @@ -217,11 +217,11 @@ describe('dockerExists', () => { const mockCache = createMockCache() nock('https://hub.docker.com') - .get('/v2/repositories/library%2Fnginx') + .get('/v2/repositories/library/nginx') .reply(200, { name: 'nginx', }) - .get('/v2/repositories/library%2Fnginx/tags/1.25.3') + .get('/v2/repositories/library/nginx/tags/1.25.3') .reply(200, { name: '1.25.3', })