From b9c1d86d6880c3cc474117ffe336ca87b972f931 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Tue, 28 Jul 2026 15:57:33 -0700 Subject: [PATCH] fix: reject a valueless --target instead of falling back to implicit selection A trailing --target, an empty --target=, or --target followed by another flag used to degrade into implicit root selection, letting a mutating helper (poll, accept, complete) act on the most recent live app instead of the one the caller tried to name. consumeTargetArg now throws on those shapes and enterLiveRoot exits with a clear error before any session state can be touched. Unit tests cover the malformed shapes and a subprocess test proves the helper body never runs. AI-assisted (Claude Code). Co-Authored-By: Claude Code --- skill/scripts/live/roots.mjs | 26 ++++++++++++++++++----- tests/live-roots.test.mjs | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/skill/scripts/live/roots.mjs b/skill/scripts/live/roots.mjs index 3c0a32e9e..a34b68c1e 100644 --- a/skill/scripts/live/roots.mjs +++ b/skill/scripts/live/roots.mjs @@ -442,13 +442,22 @@ export function resolveLiveRoots(cwd = process.cwd(), { targetPath = null } = {} export function consumeTargetArg(argv = process.argv) { for (let i = 0; i < argv.length; i++) { const arg = argv[i]; - if (arg === '--target' && typeof argv[i + 1] === 'string') { + if (arg === '--target') { const value = argv[i + 1]; + // A --target with no usable value must not degrade into implicit root + // selection: these helpers mutate session state, and "the most recent + // app" is exactly what the caller was trying NOT to get. + if (typeof value !== 'string' || value === '' || value.startsWith('--')) { + throw new Error('--target requires a path value (use --target or --target=)'); + } argv.splice(i, 2); return value; } if (typeof arg === 'string' && arg.startsWith('--target=')) { const value = arg.slice('--target='.length); + if (value === '') { + throw new Error('--target requires a path value (use --target or --target=)'); + } argv.splice(i, 1); return value; } @@ -462,12 +471,19 @@ export function consumeTargetArg(argv = process.argv) { * with the boot. An explicit `--target ` on the helper's command line * overrides pointer resolution, which is what disambiguates a repo with * several live apps (the multi-app warning names this escape hatch, so it - * has to actually work on every helper). Returns the manifest. Never - * throws; on selection ambiguity it stays in the current directory (the - * boot flow handles prompting). + * has to actually work on every helper). Returns the manifest. On selection + * ambiguity it stays in the current directory (the boot flow handles + * prompting); a malformed --target exits with an error instead of silently + * falling back to implicit selection, which could mutate the wrong app. */ export function enterLiveRoot(cwd = process.cwd()) { - const targetPath = consumeTargetArg(process.argv); + let targetPath; + try { + targetPath = consumeTargetArg(process.argv); + } catch (err) { + console.error(`[impeccable live] ${err.message}`); + process.exit(1); + } const resolved = resolveLiveRoots(cwd, targetPath ? { targetPath } : {}); if (!resolved.manifest) return null; const appRoot = resolved.manifest.appRoot; diff --git a/tests/live-roots.test.mjs b/tests/live-roots.test.mjs index 3a5704215..2f5f8d127 100644 --- a/tests/live-roots.test.mjs +++ b/tests/live-roots.test.mjs @@ -6,6 +6,7 @@ import { join, dirname } from 'node:path'; import { tmpdir } from 'node:os'; import { fileURLToPath } from 'node:url'; import { + consumeTargetArg, discoverAppCandidates, findGitRoot, resolveLiveRoots, @@ -322,6 +323,45 @@ describe('review regressions: helper --target', () => { rmSync(repo, { recursive: true, force: true }); } }); + + it('rejects a --target with no usable value instead of falling back to implicit selection', () => { + for (const argv of [ + ['node', 'live-complete.mjs', '--target'], + ['node', 'live-complete.mjs', '--target='], + ['node', 'live-complete.mjs', '--target', '--id'], + ]) { + assert.throws(() => consumeTargetArg([...argv]), /--target requires a path value/); + } + // Well-formed values still parse and are consumed. + const argv = ['node', 'live-complete.mjs', '--target', 'appB', '--id', 'x']; + assert.equal(consumeTargetArg(argv), 'appB'); + assert.deepEqual(argv, ['node', 'live-complete.mjs', '--id', 'x']); + }); + + it('enterLiveRoot exits with an error on a valueless --target rather than picking an app', () => { + const repo = realpathSync(mkdtempSync(join(tmpdir(), 'impeccable-roots-target-bad-'))); + try { + mkdirSync(join(repo, '.git'), { recursive: true }); + write(repo, 'appA/vite.config.js', 'export default {};'); + const a = resolveRoots({ cwd: repo, targetPath: join(repo, 'appA/vite.config.js') }).manifest; + writeRootsManifest(a); + write(repo, 'appA/.impeccable/live/server.json', JSON.stringify({ pid: process.pid, port: 1, token: 't' })); + + const res = spawnSync(process.execPath, [ + '-e', + `import(${JSON.stringify(ROOTS_MODULE)}).then((m) => { + process.argv.push('--target'); + m.enterLiveRoot(); + console.log('reached:' + process.cwd()); + });`, + ], { cwd: repo, encoding: 'utf-8' }); + assert.notEqual(res.status, 0, 'malformed --target must not proceed'); + assert.match(res.stderr, /--target requires a path value/); + assert.doesNotMatch(res.stdout, /reached:/, 'helper body must not run'); + } finally { + rmSync(repo, { recursive: true, force: true }); + } + }); }); describe('review regressions: pid reuse', () => {