From 57c883b7757be9faaf2e8cbb9bbdfc0b6520a64a Mon Sep 17 00:00:00 2001 From: indifferentketchup Date: Sat, 16 May 2026 05:53:56 +0000 Subject: [PATCH] chore: fix resolveProjectPath whitelist-root bypass The scope check at routes/projects.ts:56 short-circuited when real === whitelistReal, allowing the whitelist directory itself to resolve as a valid project root. Dropped the `real !== whitelistReal` half of the && so the predicate becomes the strict prefix check. Flipped the unit test from a "BEHAVIOR GAP" assertion (documenting the bug) to a strict-rejection assertion. 23/23 tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/routes/__tests__/projects.test.ts | 22 ++++++------------- apps/server/src/routes/projects.ts | 2 +- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/apps/server/src/routes/__tests__/projects.test.ts b/apps/server/src/routes/__tests__/projects.test.ts index 670ad93..055c55d 100644 --- a/apps/server/src/routes/__tests__/projects.test.ts +++ b/apps/server/src/routes/__tests__/projects.test.ts @@ -71,22 +71,14 @@ describe('resolveProjectPath', () => { expect(result.error.toLowerCase()).toContain('path must be under'); }); - it('BEHAVIOR GAP: currently accepts the whitelist itself as a project root', async () => { - // SPEC says: the whitelist directory itself should be rejected — a - // project's parent can't be the project. The current implementation does - // NOT enforce this: the scope check is - // if (real !== whitelistReal && !real.startsWith(whitelistReal + sep)) - // which evaluates to false when real === whitelistReal, so the whitelist - // path falls through and is accepted as a valid project root. - // - // This test documents the ACTUAL current behavior. Reported as a bug in - // the harness report; not silently fixed here. To tighten the check, - // change the condition to: - // if (!real.startsWith(whitelistReal + sep)) + it('rejects the whitelist directory itself as a project root', async () => { + // A project's parent can't be the project. The scope check must require + // the candidate path to be strictly below the whitelist (whitelist + sep + // prefix), not just equal to it. const result = await resolveProjectPath(whitelist, whitelist); - expect('error' in result).toBe(false); - if ('error' in result) return; - expect(result.real).toBe(whitelist); + expect('error' in result).toBe(true); + if (!('error' in result)) return; + expect(result.error.toLowerCase()).toContain('path must be under'); }); it('rejects non-directory targets (file under whitelist)', async () => { diff --git a/apps/server/src/routes/projects.ts b/apps/server/src/routes/projects.ts index e589d77..56cb106 100644 --- a/apps/server/src/routes/projects.ts +++ b/apps/server/src/routes/projects.ts @@ -53,7 +53,7 @@ export async function resolveProjectPath( return { error: 'path does not exist' }; } const whitelistReal = await realpath(whitelist); - if (real !== whitelistReal && !real.startsWith(whitelistReal + sep)) { + if (!real.startsWith(whitelistReal + sep)) { return { error: `path must be under ${whitelist}` }; } if (!(await isDir(real))) return { error: 'path is not a directory' };