Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,6 @@ config, so a worker who clones the pack cannot run the tests until someone adds
practice you write a few lines of `package.json` by hand after packing. Worth knowing before
you promise someone a pack they can `npm test` straight away.

The paths in a pack mirror the paths in your repo, with no way to remap them. Run `sparepack`
from the directory you want to be the pack's root — packing from a monorepo root gives you
`packages/api/src/...` inside the pack, which is usually not what you want.

The scanner is lexical. It finds patterns, not meaning. A business rule written in prose in a
comment, a customer name that looks like an ordinary word, an internal codename you forgot to
add to `redact` — none of those will be caught. **The manifest review is not a formality.**
Expand Down
4 changes: 4 additions & 0 deletions bin/sparepack.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ 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/...".
# stripPrefix: packages/api/

# Findings you have looked at and decided are fine. Format: rule-id:path[:line]
# Do not add entries here to make the scanner quiet. Add them when you have read the
# specific line and concluded it is genuinely safe to publish.
Expand Down
18 changes: 17 additions & 1 deletion src/config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
const KNOWN_KEYS = new Set([...FILE_KEYS, 'task', 'fixtures', 'redact', 'scanRules', 'allowFindings', 'out', 'stripPrefix'])

class ConfigError extends Error {}

Expand Down Expand Up @@ -90,6 +90,21 @@ function parseFixtures(raw) {
})
}

function parseStripPrefix(raw) {
if (raw === undefined || raw === null) return undefined
if (typeof raw !== 'string' || !raw.trim()) {
fail('"stripPrefix" must be a non-empty string')
}
const prefix = raw.trim()
if (isAbsolute(prefix)) {
fail('"stripPrefix" must be a relative path prefix, not absolute')
}
if (prefix.split(/[\\/]/).includes('..')) {
fail('"stripPrefix" must not contain ".."')
}
return prefix
}

/** Parse config text. Separated from disk access so tests need no fixtures on disk. */
export function parseConfig(text, { source = 'sparepack.yaml' } = {}) {
let raw
Expand Down Expand Up @@ -120,6 +135,7 @@ export function parseConfig(text, { source = 'sparepack.yaml' } = {}) {
const config = {
task: raw.task.trim(),
out: typeof raw.out === 'string' && raw.out.trim() ? raw.out.trim() : 'sparepack-out',
stripPrefix: parseStripPrefix(raw.stripPrefix),
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')),
Expand Down
61 changes: 59 additions & 2 deletions src/pack.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
// 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, resolve } from 'node:path'
import { dirname, join, normalize, relative, resolve } from 'node:path'

import { expand } from './config.mjs'
import { ConfigError, expand } from './config.mjs'
import { generateFixture } from './fixtures.mjs'
import { stripFile, UnsupportedLanguageError } from './interfaces.mjs'
import { countBySeverity, hasBlockingFindings, scanText, SEVERITY_ORDER } from './scan.mjs'
Expand All @@ -16,6 +16,58 @@ export const VERBATIM = 'verbatim'
export const STRIPPED = 'stripped'
export const FIXTURE = 'fixture'

/**
* Remap file destination paths by stripping the configured prefix.
*/
function applyStripPrefix(files, prefix) {
if (!prefix) 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()

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`,
)
}
}

if (destMap.has(destPath)) {
const prior = destMap.get(destPath)
throw new ConfigError(
`destination path collision after stripPrefix: "${prior}" and "${origPath}" both map to "${destPath}"`,
)
}
destMap.set(destPath, origPath)
file.path = destPath
}

if (!matchedAny) {
throw new ConfigError(
`"stripPrefix" pattern "${prefix}" matched no files. A prefix that matches nothing is an error.`,
)
}

return files
}

/** Apply the author's redact rules, reporting which ones actually fired. */
export function applyRedactions(text, rules) {
let out = text
Expand Down Expand Up @@ -141,6 +193,11 @@ 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)
}

const { active, suppressed } = partitionFindings(findings, config.allowFindings)
files.sort((a, b) => a.path.localeCompare(b.path))
return { files, findings: active, suppressed, warnings }
Expand Down
12 changes: 12 additions & 0 deletions test/config.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,18 @@ test('fixtures must map a relative path to a generator string', () => {
bad(`${base}fixtures:\n data/a.json: ""\n`, /non-empty generator/)
})

// --- stripPrefix ----------------------------------------------------------

test('stripPrefix must be non-empty, relative, and not contain ..', () => {
bad(`${base}stripPrefix: ""\n`, /"stripPrefix" must be a non-empty string/)
bad(`${base}stripPrefix: " "\n`, /"stripPrefix" must be a non-empty string/)
bad(`${base}stripPrefix: /abs/path\n`, /must be a relative path prefix/)
bad(`${base}stripPrefix: ../parent\n`, /must not contain "\.\."/)
bad(`${base}stripPrefix: foo/../bar\n`, /must not contain "\.\."/)
const config = parseConfig(`${base}stripPrefix: packages/api/\n`)
assert.equal(config.stripPrefix, 'packages/api/')
})

// --- defaults -------------------------------------------------------------

test('out defaults to sparepack-out and must stay inside the repo', () => {
Expand Down
91 changes: 91 additions & 0 deletions test/e2e.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -347,3 +347,94 @@ test('the generated template is itself a valid config', async (t) => {
assert.equal(config.interfaces.length, 1)
assert.equal(config.redact.length, 1)
})

test('stripPrefix remaps destination paths and verifies cleanly', async (t) => {
const root = await makeRepo()
t.after(() => rm(root, { recursive: true, force: true }))

// Configure stripPrefix: "src/billing" so src/billing/types.ts -> types.ts, etc.
const config = `task: "Add proportional refunds to the billing gateway"
stripPrefix: src/billing/
include:
- src/billing/types.ts
interfaces:
- src/billing/gateway.ts
tests:
- tests/billing/*.spec.ts
fixtures:
data/customers.json: shape:2
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 { dir, files } = await readPack(root)

// Verify paths were remapped inside the pack
assert.ok(files['types.ts'], 'types.ts should be at pack root')
assert.ok(files['gateway.ts'], 'gateway.ts should be at pack root')
assert.ok(files['tests/billing/charge.spec.ts'], 'non-matching prefix paths remain untouched')
assert.ok(files['data/customers.json'])

// Verify MANIFEST.json matches remapped paths
const manifest = JSON.parse(await readFile(join(root, 'pack', 'MANIFEST.json'), 'utf8'))
const manifestPaths = manifest.files.map((f) => f.path)
assert.ok(manifestPaths.includes('types.ts'))
assert.ok(manifestPaths.includes('gateway.ts'))
assert.ok(manifestPaths.includes('tests/billing/charge.spec.ts'))

// Verify pack verify works on remapped pack
const verified = await cli(root, ['verify', dir])
assert.equal(verified.code, 0, `verify failed:\n${verified.stdout}\n${verified.stderr}`)
assert.match(verified.stdout, /No problems found/)
})

test('stripPrefix that matches no files is an error', async (t) => {
const root = await makeRepo()
t.after(() => rm(root, { recursive: true, force: true }))

const config = `task: "x"
stripPrefix: non_existent_prefix/
include:
- src/billing/types.ts
out: pack
`
await writeFile(join(root, 'sparepack.yaml'), config)

const result = await cli(root, ['pack', '--yes'])
assert.equal(result.code, 2)
assert.match(result.stderr, /"stripPrefix" pattern ".*" matched no files/)
})

test('stripPrefix collision is an error naming both paths', async (t) => {
const root = await makeRepo()
t.after(() => rm(root, { recursive: true, force: true }))

// Create two files: packages/a/foo.ts and packages/b/foo.ts
// If stripPrefix is packages/a, packages/a/foo.ts -> foo.ts. If there is already foo.ts included, they collide.
await writeFile(join(root, 'foo.ts'), 'export const a = 1\n')
const config = `task: "x"
stripPrefix: src/billing
include:
- foo.ts
- src/billing/foo.ts:
`
// Actually let's create src/billing/foo.ts
await writeFile(join(root, 'src', 'billing', 'foo.ts'), 'export const b = 2\n')
const validConfig = `task: "x"
stripPrefix: src/billing
include:
- foo.ts
- src/billing/foo.ts
out: pack
`
await writeFile(join(root, 'sparepack.yaml'), validConfig)

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, /foo\.ts/)
assert.match(result.stderr, /src\/billing\/foo\.ts/)
})