diff --git a/dsh/sync/sync-from-upstream.mjs b/dsh/sync/sync-from-upstream.mjs index b4cdd8b..517a87e 100644 --- a/dsh/sync/sync-from-upstream.mjs +++ b/dsh/sync/sync-from-upstream.mjs @@ -45,7 +45,7 @@ */ import { rmSync } from 'node:fs' import { cp, mkdir, readFile, readdir, rename, rm, stat, writeFile } from 'node:fs/promises' -import { dirname, join, relative, resolve, sep } from 'node:path' +import { dirname, isAbsolute, join, relative, resolve, sep } from 'node:path' import { fileURLToPath } from 'node:url' const scriptDir = dirname(fileURLToPath(import.meta.url)) @@ -322,6 +322,12 @@ function parseArgs(argv) { } } +/** True when `parent` is `child` or one of its ancestors (`/a` → `/a/b`). */ +function isAncestorOrSame(parent, child) { + const rel = relative(parent, child) + return rel === '' || (!rel.startsWith('..') && !isAbsolute(rel)) +} + /** @returns {Promise} repo-relative file paths under `root`. */ async function walk(root, current = root) { const out = [] @@ -368,6 +374,13 @@ async function main() { const upstreamRoot = args.upstream const upstreamSkills = join(upstreamRoot, 'skills') + // The destination must never overlap the source: swapping it in would + // delete the checkout we read from, and `--out /skills` would + // delete the very tree being walked. Refuse before any write or delete. + if (isAncestorOrSame(args.out, upstreamRoot) || args.out === upstreamSkills) { + fail(`--out collides with the upstream checkout (${args.out})`) + } + const upstreamStats = await stat(upstreamSkills).catch(() => null) if (!upstreamStats?.isDirectory()) { fail(`no skills/ directory in ${upstreamRoot}`) diff --git a/dsh/tests/sync.test.mjs b/dsh/tests/sync.test.mjs index bd75ec0..60a593b 100644 --- a/dsh/tests/sync.test.mjs +++ b/dsh/tests/sync.test.mjs @@ -107,6 +107,93 @@ test('sync fails loud on a broken anchor of another transform', () => { } }) +test('sync refuses --out that collides with the upstream checkout', () => { + const work = tempDir('dsh-sync-collision-') + try { + const upstream = fixtureCopy(work) + write(join(upstream, 'sentinel.txt'), 'source stays\n') + + // `--out` equal to the checkout, an ancestor of it, and the skills/ tree + // itself — each would delete the source the sync is reading. + for (const out of [upstream, work, join(upstream, 'skills')]) { + const result = runSync([upstream, '--out', out, '--ref', 'v7.0.1']) + + assert.equal(result.status, 1, `--out ${out} must be refused`) + assert.match(result.stderr, /--out collides with the upstream checkout/) + } + + assert.equal(readFileSync(join(upstream, 'sentinel.txt'), 'utf8'), 'source stays\n') + assert.equal(existsSync(join(upstream, '.claude-plugin', 'plugin.json')), true) + assert.equal(readdirSync(join(upstream, 'skills')).length, 15) + assert.deepEqual( + readdirSync(work).filter((name) => name.includes('.tmp-')), + [], + 'a refused sync must not create its staging tree', + ) + } finally { + cleanup(work) + } +}) + +test('sync fails on a transform whose source file upstream dropped', () => { + const work = tempDir('dsh-sync-missing-transform-file-') + try { + const upstream = fixtureCopy(work) + rmSync(join(upstream, 'skills', 'brainstorming', 'visual-companion.md')) + + const result = runSync([upstream, '--out', join(work, 'out'), '--ref', 'v7.0.1']) + + assert.equal(result.status, 1) + assert.match(result.stderr, /never matched/) + assert.match(result.stderr, /DSH platform note for the visual companion/) + assert.equal(existsSync(join(work, 'out')), false) + } finally { + cleanup(work) + } +}) + +test('sync fails when an anchor matches twice', () => { + const work = tempDir('dsh-sync-duplicate-anchor-') + try { + const upstream = fixtureCopy(work) + const skillPath = join(upstream, 'skills', 'executing-plans', 'SKILL.md') + const anchor = "Run this skill's `scripts/task-start PLAN_FILE N`." + const original = readFileSync(skillPath, 'utf8') + assert.ok(original.includes(anchor), 'fixture must carry the task-start anchor verbatim') + write(skillPath, original.replace(anchor, `${anchor}\n\n${anchor}`)) + + const result = runSync([upstream, '--out', join(work, 'out'), '--ref', 'v7.0.1']) + + assert.equal(result.status, 1) + assert.match(result.stderr, /matched 2 times/) + assert.match(result.stderr, /invoke task-start through bash/) + } finally { + cleanup(work) + } +}) + +test('sync warns, but succeeds, when the optional transform anchor is gone', () => { + const work = tempDir('dsh-sync-optional-anchor-') + try { + const upstream = fixtureCopy(work) + const skillPath = join(upstream, 'skills', 'subagent-driven-development', 'SKILL.md') + const anchor = 'You: "User level (~/.config/superpowers/hooks/)"' + const original = readFileSync(skillPath, 'utf8') + assert.ok(original.includes(anchor), 'fixture must carry the optional anchor verbatim') + write(skillPath, original.replace(anchor, 'You: "Install the hook at the project level"')) + + const out = join(work, 'out') + const result = runSync([upstream, '--out', out, '--ref', 'v7.0.1']) + + assert.equal(result.status, 0, `stderr: ${result.stderr}`) + assert.match(result.stderr, /\[warn\] optional transform/) + assert.match(result.stdout, /transforms applied: 15/) + assert.equal(existsSync(join(out, '.sync-meta.json')), true) + } finally { + cleanup(work) + } +}) + test('sync fails on a changed skill set', () => { const work = tempDir('dsh-sync-skill-set-') try {