diff --git a/README.md b/README.md index 5806e5e..248c39c 100644 --- a/README.md +++ b/README.md @@ -102,12 +102,21 @@ redact: # names the scanner cannot know about - pattern: "acme-corp|ACME" replace: "example-org" -stripPrefix: packages/api/ # strip this from every path inside the pack +remap: # rewrite destination paths inside the pack + - from: src/importer/ + to: src/ + - from: tests/importer/ + to: tests/ ``` -**Destination path remapping (`stripPrefix`).** Packing from a monorepo root otherwise gives -you `packages/api/src/...` inside the pack. Setting `stripPrefix` strips that leading directory -prefix from destination paths inside the pack so the receiver gets clean paths like `src/...`. +**Destination path remapping (`remap`).** Mappings are evaluated in order and the first match +wins for each selected file. Every configured `from` must match at least one file; a stale or +shadowed rule is a hard error. Results must remain relative to the pack root, and two source +paths may not collide after remapping. `MANIFEST.json` records the post-remap paths. + +For the common single-prefix case, `stripPrefix: packages/api/` remains backward-compatible +sugar for `remap: [{ from: packages/api/, to: "" }]`. Do not set `stripPrefix` and `remap` +together. **Fixture generators.** `shape[:n]` reads the real JSON and rebuilds it with the same keys and nesting but fake values, capping arrays at `n` elements. `rows:n` keeps a delimited file's diff --git a/bin/sparepack.mjs b/bin/sparepack.mjs index beadb2c..db6cf1f 100755 --- a/bin/sparepack.mjs +++ b/bin/sparepack.mjs @@ -71,8 +71,16 @@ redact: # pattern: "\\\\b(billing|ledger)-internal\\\\b" # severity: high -# Strip a leading path prefix from destination paths inside the pack. -# Useful when running from a monorepo root to avoid paths like "packages/api/src/...". +# Remap destination paths with ordered mappings. The first matching rule wins, and every +# "from" must match at least one file. Remapped paths are also written to MANIFEST.json. +# remap: +# - from: packages/api/ +# to: "" +# - from: packages/web/ +# to: apps/web/ +# +# For one prefix, stripPrefix is backward-compatible sugar for one {from, to: ""} rule. +# Do not set stripPrefix and remap together. # stripPrefix: packages/api/ # Findings you have looked at and decided are fine. Format: rule-id:path[:line] diff --git a/src/config.mjs b/src/config.mjs index 8a5648d..2086938 100644 --- a/src/config.mjs +++ b/src/config.mjs @@ -14,7 +14,7 @@ import { compileCustomRule } from './scan.mjs' export const CONFIG_NAMES = ['sparepack.yaml', 'sparepack.yml'] const FILE_KEYS = ['include', 'interfaces', 'tests'] -const KNOWN_KEYS = new Set([...FILE_KEYS, 'task', 'fixtures', 'redact', 'scanRules', 'allowFindings', 'out', 'stripPrefix']) +const KNOWN_KEYS = new Set([...FILE_KEYS, 'task', 'fixtures', 'redact', 'scanRules', 'allowFindings', 'out', 'stripPrefix', 'remap']) class ConfigError extends Error {} @@ -105,6 +105,31 @@ function parseStripPrefix(raw) { return prefix } +/** + * Parse remap entries. Each entry must have `from` and `to` strings. + * Both values are validated against traversal and absoluteness. + * Order matters: first match wins at pack time. + */ +function parseRemap(raw) { + if (raw === undefined || raw === null) return [] + if (!Array.isArray(raw)) fail('"remap" must be a list of {from, to} mappings') + return raw.map((entry, i) => { + if (typeof entry !== 'object' || entry === null) { + fail(`remap[${i}] must be a mapping with "from" and "to"`) + } + if (typeof entry.from !== 'string' || !entry.from.trim()) { + fail(`remap[${i}].from must be a non-empty string`) + } + if (typeof entry.to !== 'string') { + fail(`remap[${i}].to must be a string (use "" to strip the prefix entirely)`) + } + const from = validatePattern(entry.from.trim(), `remap[${i}].from`) + const to = entry.to.trim() + // `to` may be empty (strip), but if present it must be safe + if (to) validatePattern(to, `remap[${i}].to`) + return { from, to } + }) +} /** Parse config text. Separated from disk access so tests need no fixtures on disk. */ export function parseConfig(text, { source = 'sparepack.yaml' } = {}) { let raw @@ -128,6 +153,10 @@ export function parseConfig(text, { source = 'sparepack.yaml' } = {}) { fail(`unknown key(s) in ${source}: ${unknown.join(', ')} (prefix a key with "_" for notes)`) } + if (raw.stripPrefix !== undefined && raw.remap !== undefined) { + fail('"stripPrefix" and "remap" cannot both be set. Use "remap" only — stripPrefix is sugar for a single {from, to: ""} mapping.') + } + if (typeof raw.task !== 'string' || !raw.task.trim()) { fail('"task" is required: one line saying what this pack is for. The worker reads it first.') } @@ -136,6 +165,7 @@ export function parseConfig(text, { source = 'sparepack.yaml' } = {}) { task: raw.task.trim(), out: typeof raw.out === 'string' && raw.out.trim() ? raw.out.trim() : 'sparepack-out', stripPrefix: parseStripPrefix(raw.stripPrefix), + remap: parseRemap(raw.remap), include: asArray(raw.include, 'include').map((p) => validatePattern(p, 'include')), interfaces: asArray(raw.interfaces, 'interfaces').map((p) => validatePattern(p, 'interfaces')), tests: asArray(raw.tests, 'tests').map((p) => validatePattern(p, 'tests')), @@ -150,6 +180,11 @@ export function parseConfig(text, { source = 'sparepack.yaml' } = {}) { }), } + // Backward compatibility: convert stripPrefix to remap internally if remap is empty + if (config.stripPrefix && config.remap.length === 0) { + config.remap = [{ from: config.stripPrefix, to: '' }] + } + validatePattern(config.out, 'out') const total = FILE_KEYS.reduce((n, key) => n + config[key].length, 0) diff --git a/src/pack.mjs b/src/pack.mjs index ec3fa0c..c1554e1 100644 --- a/src/pack.mjs +++ b/src/pack.mjs @@ -5,9 +5,10 @@ // author rejected still exists in a directory they might later publish by accident. import { mkdir, readFile, rm, writeFile } from 'node:fs/promises' -import { dirname, join, normalize, relative, resolve } from 'node:path' +import { dirname, join, posix, resolve } from 'node:path' import { ConfigError, expand } from './config.mjs' +import { assertInsideRoot } from './config.mjs' import { generateFixture } from './fixtures.mjs' import { stripFile, UnsupportedLanguageError } from './interfaces.mjs' import { countBySeverity, hasBlockingFindings, scanText, SEVERITY_ORDER } from './scan.mjs' @@ -17,52 +18,74 @@ export const STRIPPED = 'stripped' export const FIXTURE = 'fixture' /** - * Remap file destination paths by stripping the configured prefix. + * Remap file destination paths using ordered {from, to} mappings. + * First match wins. Validates traversal on both configured values and results. + * Reports collisions with both source paths. */ -function applyStripPrefix(files, prefix) { - if (!prefix) return files +function applyRemap(files, remapRules, root) { + if (!remapRules || remapRules.length === 0) return files - // Normalize prefix to forward slashes without leading/trailing slashes for uniform matching - const cleanPrefix = prefix.replace(/^[\\/]+|[\\/]+$/g, '') - if (!cleanPrefix) return files - - let matchedAny = false const destMap = new Map() + const matchedRules = new Set() for (const file of files) { const origPath = file.path const normalized = origPath.replace(/\\/g, '/') - let destPath = origPath - - if (normalized === cleanPrefix || normalized.startsWith(cleanPrefix + '/')) { - matchedAny = true - destPath = normalized === cleanPrefix ? '' : normalized.slice(cleanPrefix.length + 1) - if (destPath === '') { - throw new ConfigError( - `stripping prefix "${prefix}" from "${origPath}" produces an empty destination path`, - ) - } - if (destPath.startsWith('/') || destPath.split('/').includes('..')) { - throw new ConfigError( - `stripping prefix "${prefix}" from "${origPath}" produces an invalid path "${destPath}" escaping pack root`, - ) + let destPath = null + + for (const [ruleIndex, rule] of remapRules.entries()) { + const cleanFrom = posix.normalize( + rule.from.replace(/\\/g, '/').replace(/^\/+|\/+$/g, ''), + ) + if (!cleanFrom) continue + + if (normalized === cleanFrom || normalized.startsWith(cleanFrom + '/')) { + matchedRules.add(ruleIndex) + const remainder = normalized === cleanFrom ? '' : normalized.slice(cleanFrom.length + 1) + const cleanTo = rule.to.replace(/\\/g, '/').replace(/^\/+|\/+$/g, '') + destPath = cleanTo ? (remainder ? `${cleanTo}/${remainder}` : cleanTo) : remainder + break // first match wins } } - if (destMap.has(destPath)) { - const prior = destMap.get(destPath) + const rawFinalPath = destPath !== null ? destPath : normalized + if (rawFinalPath === '') { throw new ConfigError( - `destination path collision after stripPrefix: "${prior}" and "${origPath}" both map to "${destPath}"`, + `remapping "${origPath}" produces an empty destination path`, ) } - destMap.set(destPath, origPath) - file.path = destPath + // Reject traversal before normalizing: collapsing an attempted escape would + // hide the evidence. Then canonicalize aliases before collision detection + // and writing so ./a.ts, a//b.ts, and a/./b.ts cannot name the same file. + if (rawFinalPath.startsWith('/') || rawFinalPath.split('/').includes('..')) { + throw new ConfigError( + `remapping "${origPath}" produces invalid path "${rawFinalPath}" escaping pack root`, + ) + } + const finalPath = posix.normalize(rawFinalPath) + if (finalPath === '.' || posix.isAbsolute(finalPath)) { + throw new ConfigError( + `remapping "${origPath}" produces invalid path "${rawFinalPath}" escaping pack root`, + ) + } + assertInsideRoot(root, finalPath, `remap result for "${origPath}"`) + + if (destMap.has(finalPath)) { + const prior = destMap.get(finalPath) + throw new ConfigError( + `destination path collision after remap: "${prior}" and "${origPath}" both map to "${finalPath}"`, + ) + } + destMap.set(finalPath, origPath) + file.path = finalPath } - if (!matchedAny) { - throw new ConfigError( - `"stripPrefix" pattern "${prefix}" matched no files. A prefix that matches nothing is an error.`, - ) + for (const [ruleIndex, rule] of remapRules.entries()) { + if (!matchedRules.has(ruleIndex)) { + throw new ConfigError( + `"remap" pattern "${rule.from}" matched no files. A remap rule that matches nothing is an error.`, + ) + } } return files @@ -193,9 +216,9 @@ export async function buildPack(root, config) { findings.push(...scanText(text, { path: file.path, customRules: config.scanRules })) } - // Remap destination paths inside the pack if stripPrefix is set. - if (config.stripPrefix) { - applyStripPrefix(files, config.stripPrefix) + // Remap destination paths inside the pack after content processing. + if (config.remap && config.remap.length > 0) { + applyRemap(files, config.remap, root) } const { active, suppressed } = partitionFindings(findings, config.allowFindings) @@ -396,3 +419,4 @@ export async function writePack(outDir, manifest, files) { } export { hasBlockingFindings } +export { applyRemap } diff --git a/test/config.test.mjs b/test/config.test.mjs index 13ce08d..374aa53 100644 --- a/test/config.test.mjs +++ b/test/config.test.mjs @@ -2,6 +2,7 @@ import { test } from 'node:test' import assert from 'node:assert/strict' import { assertInsideRoot, ConfigError, parseConfig } from '../src/config.mjs' +import { applyRemap } from '../src/pack.mjs' const base = 'task: "do a thing"\ninclude:\n - src/a.ts\n' @@ -136,3 +137,143 @@ test('out defaults to sparepack-out and must stay inside the repo', () => { assert.equal(parseConfig(`${base}out: dist/pack\n`).out, 'dist/pack') bad(`${base}out: /tmp/anywhere\n`, /must be relative/) }) + +// --- remap ---------------------------------------------------------------- + +test('remap: overlapping mappings use the first match and later rules can still match', () => { + const files = [ + { path: 'src/special/file.ts' }, + { path: 'src/other.ts' }, + ] + const rules = [ + { from: 'src/special', to: 'special' }, + { from: 'src', to: 'lib' }, + ] + + applyRemap(files, rules, '/tmp') + + assert.deepEqual(files.map((file) => file.path), ['special/file.ts', 'lib/other.ts']) +}) + +test('remap: collision error includes mapped and untouched source paths in either order', () => { + const rules = [{ from: 'src/a', to: 'out' }] + const sourceOrders = [ + ['src/a/file.ts', 'out/file.ts'], + ['out/file.ts', 'src/a/file.ts'], + ] + + for (const sourcePaths of sourceOrders) { + const files = sourcePaths.map((path) => ({ path })) + assert.throws( + () => applyRemap(files, rules, '/tmp'), + (err) => { + assert.ok(err instanceof Error, `expected Error, got ${err.constructor.name}`) + assert.match(err.message, /after remap/) + assert.match(err.message, /src\/a\/file\.ts/) + assert.match(err.message, /out\/file\.ts/) + return true + }, + ) + } +}) + +test('remap: canonical path aliases cannot bypass collision detection', () => { + const aliases = [ + { source: 'src/file.ts', to: '.', untouched: 'file.ts' }, + { source: 'src/file.ts', to: 'out//nested', untouched: 'out/nested/file.ts' }, + { source: 'src/file.ts', to: 'out/.', untouched: 'out/file.ts' }, + { source: 'src/file.ts', to: 'out\\\\nested', untouched: 'out/nested/file.ts' }, + { source: 'src\\\\file.ts', to: 'out', untouched: 'out/file.ts' }, + ] + + for (const { source, to, untouched } of aliases) { + const files = [{ path: source }, { path: untouched }] + assert.throws( + () => applyRemap(files, [{ from: 'src', to }], '/tmp'), + (err) => { + assert.ok(err instanceof Error, `expected Error, got ${err.constructor.name}`) + assert.match(err.message, /destination path collision after remap/) + assert.ok(err.message.includes(source), `missing source path in: ${err.message}`) + assert.ok(err.message.includes(untouched), `missing destination peer in: ${err.message}`) + return true + }, + ) + } +}) + +test('remap: two mapped sources colliding name both sources', () => { + const files = [{ path: 'src/a/file.ts' }, { path: 'src/b/file.ts' }] + const rules = [ + { from: 'src/a', to: 'out' }, + { from: 'src/b', to: 'out' }, + ] + + assert.throws( + () => applyRemap(files, rules, '/tmp'), + (err) => { + assert.ok(err instanceof Error, `expected Error, got ${err.constructor.name}`) + assert.match(err.message, /src\/a\/file\.ts/) + assert.match(err.message, /src\/b\/file\.ts/) + return true + }, + ) +}) + +test('remap: traversal in from is rejected at parse time', () => { + bad(`${base}remap:\n - from: ../escape\n to: safe\n`, /must not contain "\.\."/) +}) + +test('remap: traversal in to is rejected at parse time', () => { + bad(`${base}remap:\n - from: src\n to: ../escape\n`, /must not contain "\.\."/) +}) + +test('remap: absolute path in from is rejected', () => { + bad(`${base}remap:\n - from: /absolute/path\n to: out\n`, /must be relative/) +}) + +test('remap: traversal introduced in the result is rejected', () => { + const files = [{ path: 'src/../escape.ts' }] + const rules = [{ from: 'src', to: 'safe' }] + assert.throws( + () => applyRemap(files, rules, '/tmp'), + (err) => { + assert.ok(err instanceof Error, `expected Error, got ${err.constructor.name}`) + assert.match(err.message, /invalid path "safe\/\.\.\/escape\.ts" escaping pack root/) + return true + }, + ) +}) + +test('remap: every individual rule must match a file', () => { + const files = [{ path: 'src/file.ts' }] + const rules = [ + { from: 'src', to: 'lib' }, + { from: 'tests', to: 'spec' }, + ] + assert.throws( + () => applyRemap(files, rules, '/tmp'), + (err) => { + assert.ok(err instanceof Error, `expected Error, got ${err.constructor.name}`) + assert.match(err.message, /"remap" pattern "tests" matched no files/) + assert.doesNotMatch(err.message, /"stripPrefix" pattern/) + return true + }, + ) +}) + +test('remap: a fully shadowed rule is treated as unused', () => { + const files = [{ path: 'src/special/file.ts' }] + const rules = [ + { from: 'src', to: 'lib' }, + { from: 'src/special', to: 'special' }, + ] + + assert.throws( + () => applyRemap(files, rules, '/tmp'), + /"remap" pattern "src\/special" matched no files/, + ) +}) + +test('stripPrefix and remap mutual exclusion', () => { + bad(`${base}stripPrefix: packages/api/\nremap:\n - from: src\n to: lib\n`, /cannot both be set/) +}) diff --git a/test/e2e.test.mjs b/test/e2e.test.mjs index f97819b..c4b9149 100644 --- a/test/e2e.test.mjs +++ b/test/e2e.test.mjs @@ -132,18 +132,21 @@ async function cli(root, args, { input } = {}) { } } -async function readPack(root) { - const dir = join(root, 'pack') +async function readPack(root, out = 'pack', encoding = 'utf8') { + const dir = join(root, out) const files = {} const walk = async (rel) => { for (const entry of await readdir(join(dir, rel), { withFileTypes: true })) { const next = rel ? join(rel, entry.name) : entry.name if (entry.isDirectory()) await walk(next) - else files[next] = await readFile(join(dir, next), 'utf8') + else { + const key = next.replace(/\\/g, '/') + files[key] = encoding === null ? await readFile(join(dir, next)) : await readFile(join(dir, next), encoding) + } } } await walk('') - return { dir, files, all: Object.values(files).join('\n') } + return { dir, files, all: encoding === null ? undefined : Object.values(files).join('\n') } } test('a pack built from a repo full of secrets contains none of them', async (t) => { @@ -330,6 +333,9 @@ test('init writes a template and refuses to clobber an existing config', async ( const template = await readFile(join(root, 'sparepack.yaml'), 'utf8') assert.match(template, /allowlist/) assert.match(template, /task:/) + assert.match(template, /remap:/) + assert.match(template, /first matching rule wins/) + assert.match(template, /stripPrefix is backward-compatible sugar/) const second = await cli(root, ['init']) assert.equal(second.code, 1) @@ -348,6 +354,69 @@ test('the generated template is itself a valid config', async (t) => { assert.equal(config.redact.length, 1) }) +test('remap applies overlapping rules in order and writes remapped manifest paths', async (t) => { + const root = await makeRepo() + t.after(() => rm(root, { recursive: true, force: true })) + await writeFile(join(root, 'src', 'shared.ts'), 'export const shared = true\n') + + const config = `task: "Remap ordered source roots" +remap: + - from: src/billing + to: lib + - from: src + to: source +include: + - src/billing/types.ts + - src/shared.ts +out: pack +` + await writeFile(join(root, 'sparepack.yaml'), config) + + const packed = await cli(root, ['pack', '--yes', '--no-color']) + assert.equal(packed.code, 0, `pack failed:\n${packed.stdout}\n${packed.stderr}`) + + const { files } = await readPack(root) + assert.ok(files['lib/types.ts'], 'the more specific first rule should win') + assert.ok(files['source/shared.ts'], 'the later rule should still match another file') + assert.equal(files['source/billing/types.ts'], undefined) + + const manifest = JSON.parse(await readFile(join(root, 'pack', 'MANIFEST.json'), 'utf8')) + assert.deepEqual( + manifest.files.map((file) => file.path).sort(), + ['lib/types.ts', 'source/shared.ts'], + ) +}) + +test('stripPrefix and its single-rule remap form produce byte-identical packs', async (t) => { + const root = await makeRepo() + t.after(() => rm(root, { recursive: true, force: true })) + const common = `task: "Publish billing types at the pack root" +include: + - src/billing/types.ts +` + + await writeFile( + join(root, 'sparepack.yaml'), + `${common}stripPrefix: src/billing\nout: pack-strip\n`, + ) + const stripped = await cli(root, ['pack', '--yes', '--no-color']) + assert.equal(stripped.code, 0, `stripPrefix pack failed:\n${stripped.stdout}\n${stripped.stderr}`) + + await writeFile( + join(root, 'sparepack.yaml'), + `${common}remap:\n - from: src/billing\n to: ""\nout: pack-remap\n`, + ) + const remapped = await cli(root, ['pack', '--yes', '--no-color']) + assert.equal(remapped.code, 0, `remap pack failed:\n${remapped.stdout}\n${remapped.stderr}`) + + const stripFiles = (await readPack(root, 'pack-strip', null)).files + const remapFiles = (await readPack(root, 'pack-remap', null)).files + assert.deepEqual(Object.keys(remapFiles).sort(), Object.keys(stripFiles).sort()) + for (const path of Object.keys(stripFiles)) { + assert.deepEqual(remapFiles[path], stripFiles[path], `${path} differs byte for byte`) + } +}) + test('stripPrefix remaps destination paths and verifies cleanly', async (t) => { const root = await makeRepo() t.after(() => rm(root, { recursive: true, force: true })) @@ -405,7 +474,7 @@ out: pack const result = await cli(root, ['pack', '--yes']) assert.equal(result.code, 2) - assert.match(result.stderr, /"stripPrefix" pattern ".*" matched no files/) + assert.match(result.stderr, /"remap" pattern ".*" matched no files/) }) test('stripPrefix collision is an error naming both paths', async (t) => { @@ -434,7 +503,7 @@ out: pack const result = await cli(root, ['pack', '--yes']) assert.equal(result.code, 2) - assert.match(result.stderr, /destination path collision after stripPrefix/) + assert.match(result.stderr, /destination path collision after remap/) assert.match(result.stderr, /foo\.ts/) assert.match(result.stderr, /src\/billing\/foo\.ts/) })