fix: add path traversal validation and unit tests for planningDir

Reject project/workstream names containing path separators or ..
components. Covers both GSD_PROJECT and GSD_WORKSTREAM. Adds 9 tests
for the full resolution matrix and traversal rejection cases.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ned Malki
2026-04-02 09:18:39 +07:00
parent 3b0a7560e5
commit 523c7199d0
2 changed files with 85 additions and 0 deletions

View File

@@ -561,6 +561,15 @@ function planningDir(cwd, ws, project) {
if (project === undefined) project = process.env.GSD_PROJECT || null;
if (ws === undefined) ws = process.env.GSD_WORKSTREAM || null;
// Reject path separators and traversal components in project/workstream names
const BAD_SEGMENT = /[/\\]|\.\./;
if (project && BAD_SEGMENT.test(project)) {
throw new Error(`GSD_PROJECT contains invalid path characters: ${project}`);
}
if (ws && BAD_SEGMENT.test(ws)) {
throw new Error(`GSD_WORKSTREAM contains invalid path characters: ${ws}`);
}
let base = path.join(cwd, '.planning');
if (project) base = path.join(base, project);
if (ws) base = path.join(base, 'workstreams', ws);

View File

@@ -30,6 +30,7 @@ const {
findPhaseInternal,
findProjectRoot,
detectSubRepos,
planningDir,
} = require('../get-shit-done/bin/lib/core.cjs');
// ─── loadConfig ────────────────────────────────────────────────────────────────
@@ -1565,3 +1566,78 @@ describe('reapStaleTempFiles', () => {
});
});
});
// ─── planningDir ──────────────────────────────────────────────────────────────
describe('planningDir', () => {
const cwd = '/fake/repo';
let savedProject, savedWorkstream;
beforeEach(() => {
savedProject = process.env.GSD_PROJECT;
savedWorkstream = process.env.GSD_WORKSTREAM;
delete process.env.GSD_PROJECT;
delete process.env.GSD_WORKSTREAM;
});
afterEach(() => {
if (savedProject !== undefined) process.env.GSD_PROJECT = savedProject;
else delete process.env.GSD_PROJECT;
if (savedWorkstream !== undefined) process.env.GSD_WORKSTREAM = savedWorkstream;
else delete process.env.GSD_WORKSTREAM;
});
test('returns .planning/ when neither project nor workstream is set', () => {
const result = planningDir(cwd, null, null);
assert.strictEqual(result, path.join(cwd, '.planning'));
});
test('returns .planning/{project}/ when project is set', () => {
const result = planningDir(cwd, null, 'my-app');
assert.strictEqual(result, path.join(cwd, '.planning', 'my-app'));
});
test('returns .planning/workstreams/{ws}/ when workstream is set', () => {
const result = planningDir(cwd, 'feature-x', null);
assert.strictEqual(result, path.join(cwd, '.planning', 'workstreams', 'feature-x'));
});
test('returns .planning/{project}/workstreams/{ws}/ when both are set', () => {
const result = planningDir(cwd, 'feature-x', 'my-app');
assert.strictEqual(result, path.join(cwd, '.planning', 'my-app', 'workstreams', 'feature-x'));
});
test('reads GSD_PROJECT from env when project param is undefined', () => {
process.env.GSD_PROJECT = 'env-project';
const result = planningDir(cwd);
assert.strictEqual(result, path.join(cwd, '.planning', 'env-project'));
});
test('rejects path traversal in project name', () => {
assert.throws(
() => planningDir(cwd, null, '../../etc'),
/invalid path characters/
);
});
test('rejects forward slash in project name', () => {
assert.throws(
() => planningDir(cwd, null, 'foo/bar'),
/invalid path characters/
);
});
test('rejects backslash in project name', () => {
assert.throws(
() => planningDir(cwd, null, 'foo\\bar'),
/invalid path characters/
);
});
test('rejects path traversal in workstream name', () => {
assert.throws(
() => planningDir(cwd, '../../../tmp', null),
/invalid path characters/
);
});
});