From f64f8a0e7b01fa1d938aa1e4036f9233c5f7e536 Mon Sep 17 00:00:00 2001 From: sim Date: Sat, 12 Sep 2026 10:24:48 -0400 Subject: [PATCH] test(#4652): correct the absolute-filename test to match the basename guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test asserted that an absolute filename is folded under the pending dir and fails as "not found" rather than being rejected. That held for exactly one commit. The basename guard rejects any name containing a separator before any join happens, so an absolute path never reaches containment or the filesystem at all. Now asserts the USAGE rejection the CLI actually emits, verified by running it. All four outside-file protections are kept unchanged — the file still exists, its content is byte-identical, it never lands in completed/, and cleanup runs in finally. Those are the assertions that carry the security value; only the claim about HOW the rejection happens was stale. Co-Authored-By: Claude Opus 5 --- tests/commands.test.cjs | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/tests/commands.test.cjs b/tests/commands.test.cjs index dc97f7360..a03efb6b9 100644 --- a/tests/commands.test.cjs +++ b/tests/commands.test.cjs @@ -890,25 +890,24 @@ describe('todo complete — containment boundary (#4327)', () => { }); } - test('[#4327] an absolute filename is folded under the pending dir, not rejected as containment violation — the outside file is untouched', () => { - // MEASURED: path.join(pendingDir, '/abs/outside/evil.md') === `${pendingDir}/abs/outside/evil.md` - // — Node's path.join does not let a later absolute segment escape a prior - // one. So an absolute `filename` is folded INSIDE todosRoot, passes - // containment, and simply 404s as "Todo not found" (unless a file of - // that joined name happens to exist under pendingDir). It is NOT - // rejected as a containment/escape violation. + test('[#4327] an absolute filename is rejected as a non-basename before any join — the outside file is untouched', () => { + // A basename guard added since #4327 rejects any filename containing `/` + // or `\` BEFORE it is ever joined against pendingDir — so an absolute + // path never reaches path.join, containment, or the filesystem at all. + // It is a USAGE rejection, not a containment/escape check and not a + // plain "not found". This test also pins the thing that actually + // matters: the real outside file is never read, moved, or deleted. const outsideDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-todo-outside-')); try { const outsideFile = path.join(outsideDir, 'evil.md'); const sentinel = '---\nstatus: pending\n---\nSENTINEL\n'; fs.writeFileSync(outsideFile, sentinel); - const result = runGsdTools(['todo', 'complete', outsideFile], tmpDir); + const result = runGsdTools(['--json-errors', 'todo', 'complete', outsideFile], tmpDir); - assert.strictEqual(result.success, false, 'the command must fail (the folded path does not exist under pending/)'); - assert.ok( - (result.error || '').includes('not found'), - `must fail as a plain "not found", not a containment rejection (got: ${result.error})`, - ); + assert.strictEqual(result.success, false, 'the command must fail (a filename containing a separator is rejected)'); + const parsed = JSON.parse(result.error); + assert.strictEqual(parsed.ok, false); + assert.strictEqual(parsed.reason, 'usage'); assert.ok(fs.existsSync(outsideFile), 'the real outside file must still exist'); assert.strictEqual( fs.readFileSync(outsideFile, 'utf-8'),