diff --git a/get-shit-done/bin/lib/active-workstream-store.cjs b/get-shit-done/bin/lib/active-workstream-store.cjs index bf5a99a15..5bfe6192f 100644 --- a/get-shit-done/bin/lib/active-workstream-store.cjs +++ b/get-shit-done/bin/lib/active-workstream-store.cjs @@ -6,11 +6,10 @@ */ const { getActiveWorkstream } = require('./planning-workspace.cjs'); -const { isValidActiveWorkstreamName } = require('./workstream-name-policy.cjs'); - -function validateWorkstreamName(name) { - return isValidActiveWorkstreamName(name); -} +const { + validateWorkstreamName, + assertValidActiveWorkstreamName, +} = require('./workstream-name-policy.cjs'); function parseCliWorkstream(args) { const wsEqArg = args.find(arg => arg.startsWith('--ws=')); @@ -61,8 +60,8 @@ function resolveActiveWorkstream(cwd, args, env = process.env, deps = {}) { source = ws ? 'store' : 'none'; } - if (ws && !validateWorkstreamName(ws)) { - throw new Error('Invalid workstream name: must be alphanumeric, hyphens, underscores, or dots'); + if (ws) { + assertValidActiveWorkstreamName(ws); } return { diff --git a/get-shit-done/bin/lib/planning-workspace.cjs b/get-shit-done/bin/lib/planning-workspace.cjs index bb195bb39..e24ac5b74 100644 --- a/get-shit-done/bin/lib/planning-workspace.cjs +++ b/get-shit-done/bin/lib/planning-workspace.cjs @@ -12,7 +12,10 @@ const os = require('os'); const path = require('path'); const crypto = require('crypto'); const { probeTty, platformWriteSync, platformReadSync, platformEnsureDir } = require('./shell-command-projection.cjs'); -const { isValidActiveWorkstreamName } = require('./workstream-name-policy.cjs'); +const { + validateWorkstreamName, + assertValidActiveWorkstreamName, +} = require('./workstream-name-policy.cjs'); const WORKSTREAM_SESSION_ENV_KEYS = [ 'GSD_SESSION_KEY', @@ -241,10 +244,6 @@ function pickActiveWorkstreamAdapter(cwd, opts = {}) { return createSharedPointerAdapter(cwd); } -function validateWorkstreamName(name) { - return isValidActiveWorkstreamName(name); -} - function withPlanningLock(cwd, fn) { const lockPath = path.join(planningDir(cwd), '.lock'); const lockTimeout = 10000; // 10 seconds @@ -346,9 +345,7 @@ function createPlanningWorkspace(cwd, opts = {}) { adapter.clear(); return; } - if (!validateWorkstreamName(name)) { - throw new Error('Invalid workstream name: must be alphanumeric, hyphens, underscores, or dots'); - } + assertValidActiveWorkstreamName(name); const wsDir = path.join(planningRoot(cwd), 'workstreams', name); platformEnsureDir(wsDir); diff --git a/get-shit-done/bin/lib/workstream-name-policy.cjs b/get-shit-done/bin/lib/workstream-name-policy.cjs index 27f1ec23e..508157ea7 100644 --- a/get-shit-done/bin/lib/workstream-name-policy.cjs +++ b/get-shit-done/bin/lib/workstream-name-policy.cjs @@ -11,6 +11,35 @@ */ const ACTIVE_WORKSTREAM_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/; +const INVALID_ACTIVE_WORKSTREAM_NAME_MESSAGE = 'Invalid workstream name: must be alphanumeric, hyphens, underscores, or dots'; + +function normalizeWorkstreamNameInput(name) { + const value = String(name || '').trim(); + return value || null; +} + +function validateActiveWorkstreamName(name) { + const value = normalizeWorkstreamNameInput(name); + if (!value) { + return { + ok: false, + reason: 'empty', + value: null, + }; + } + if (hasInvalidPathSegment(value) || !ACTIVE_WORKSTREAM_RE.test(value)) { + return { + ok: false, + reason: 'invalid', + value, + }; + } + return { + ok: true, + reason: null, + value, + }; +} /** * Validate a workstream name. * Allowed: alphanumeric, hyphens, underscores, dots. @@ -47,15 +76,24 @@ function hasInvalidPathSegment(name) { * - Must not contain path traversal sequences (..) */ function isValidActiveWorkstreamName(name) { - const value = String(name || ''); - if (value === '..' || value.startsWith('../') || value.includes('..')) - return false; - return ACTIVE_WORKSTREAM_RE.test(value); + return validateActiveWorkstreamName(name).ok; +} + +function assertValidActiveWorkstreamName(name, errorMessage = INVALID_ACTIVE_WORKSTREAM_NAME_MESSAGE) { + const validation = validateActiveWorkstreamName(name); + if (!validation.ok) { + throw new Error(errorMessage); + } + return validation.value; } module.exports = { + INVALID_ACTIVE_WORKSTREAM_NAME_MESSAGE, + normalizeWorkstreamNameInput, + validateActiveWorkstreamName, validateWorkstreamName, toWorkstreamSlug, hasInvalidPathSegment, isValidActiveWorkstreamName, + assertValidActiveWorkstreamName, }; diff --git a/get-shit-done/bin/lib/workstream.cjs b/get-shit-done/bin/lib/workstream.cjs index b538b4052..2bf62a744 100644 --- a/get-shit-done/bin/lib/workstream.cjs +++ b/get-shit-done/bin/lib/workstream.cjs @@ -13,7 +13,12 @@ const path = require('path'); const { output, error, toPosixPath, getMilestoneInfo, generateSlugInternal } = require('./core.cjs'); const { platformWriteSync, platformEnsureDir } = require('./shell-command-projection.cjs'); const { planningRoot, setActiveWorkstream, getActiveWorkstream } = require('./planning-workspace.cjs'); -const { toWorkstreamSlug, hasInvalidPathSegment, isValidActiveWorkstreamName } = require('./workstream-name-policy.cjs'); +const { + toWorkstreamSlug, + assertValidActiveWorkstreamName, + isValidActiveWorkstreamName, + INVALID_ACTIVE_WORKSTREAM_NAME_MESSAGE, +} = require('./workstream-name-policy.cjs'); const { formatGsdSlash, resolveRuntime } = require('./runtime-slash.cjs'); const { getOtherActiveWorkstreamInventories, @@ -30,7 +35,9 @@ const { * milestones/, research/, codebase/, todos/) stay in place. */ function migrateToWorkstreams(cwd, workstreamName) { - if (!workstreamName || hasInvalidPathSegment(workstreamName)) { + try { + assertValidActiveWorkstreamName(workstreamName, 'Invalid workstream name for migration'); + } catch (err) { throw new Error('Invalid workstream name for migration'); } @@ -207,7 +214,11 @@ function cmdWorkstreamList(cwd, raw) { function cmdWorkstreamStatus(cwd, name, raw) { if (!name) error('workstream name required. Usage: workstream status '); - if (hasInvalidPathSegment(name)) error('Invalid workstream name'); + try { + assertValidActiveWorkstreamName(name, INVALID_ACTIVE_WORKSTREAM_NAME_MESSAGE); + } catch { + error(INVALID_ACTIVE_WORKSTREAM_NAME_MESSAGE); + } const wsDir = path.join(planningRoot(cwd), 'workstreams', name); if (!fs.existsSync(wsDir)) { @@ -233,7 +244,11 @@ function cmdWorkstreamStatus(cwd, name, raw) { function cmdWorkstreamComplete(cwd, name, options, raw) { if (!name) error('workstream name required. Usage: workstream complete '); - if (hasInvalidPathSegment(name)) error('Invalid workstream name'); + try { + assertValidActiveWorkstreamName(name, INVALID_ACTIVE_WORKSTREAM_NAME_MESSAGE); + } catch { + error(INVALID_ACTIVE_WORKSTREAM_NAME_MESSAGE); + } const root = planningRoot(cwd); const wsRoot = path.join(root, 'workstreams'); diff --git a/tests/workstream-name-policy.test.cjs b/tests/workstream-name-policy.test.cjs new file mode 100644 index 000000000..f9f674d7f --- /dev/null +++ b/tests/workstream-name-policy.test.cjs @@ -0,0 +1,51 @@ +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); + +const { + normalizeWorkstreamNameInput, + validateActiveWorkstreamName, + assertValidActiveWorkstreamName, + isValidActiveWorkstreamName, + INVALID_ACTIVE_WORKSTREAM_NAME_MESSAGE, +} = require('../get-shit-done/bin/lib/workstream-name-policy.cjs'); + +describe('workstream-name-policy', () => { + test('normalizeWorkstreamNameInput trims and nulls empty input', () => { + assert.equal(normalizeWorkstreamNameInput(' alpha '), 'alpha'); + assert.equal(normalizeWorkstreamNameInput(' '), null); + assert.equal(normalizeWorkstreamNameInput(null), null); + }); + + test('validateActiveWorkstreamName returns structured validation', () => { + assert.deepEqual( + validateActiveWorkstreamName('alpha_1'), + { ok: true, reason: null, value: 'alpha_1' } + ); + assert.deepEqual( + validateActiveWorkstreamName('alpha beta'), + { ok: false, reason: 'invalid', value: 'alpha beta' } + ); + assert.deepEqual( + validateActiveWorkstreamName('../alpha'), + { ok: false, reason: 'invalid', value: '../alpha' } + ); + assert.deepEqual( + validateActiveWorkstreamName(' '), + { ok: false, reason: 'empty', value: null } + ); + }); + + test('assertValidActiveWorkstreamName returns normalized value and throws canonical error', () => { + assert.equal(assertValidActiveWorkstreamName(' alpha '), 'alpha'); + assert.throws( + () => assertValidActiveWorkstreamName('alpha/beta'), + new RegExp(INVALID_ACTIVE_WORKSTREAM_NAME_MESSAGE.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')) + ); + }); + + test('isValidActiveWorkstreamName accepts canonical and rejects invalid names', () => { + assert.equal(isValidActiveWorkstreamName('alpha-1'), true); + assert.equal(isValidActiveWorkstreamName('ws..traversal'), false); + assert.equal(isValidActiveWorkstreamName('alpha beta'), false); + }); +}); diff --git a/tests/workstream.test.cjs b/tests/workstream.test.cjs index 6660a9a9a..3d4239615 100644 --- a/tests/workstream.test.cjs +++ b/tests/workstream.test.cjs @@ -795,6 +795,26 @@ describe('path traversal rejection', () => { } }); + describe('cmdWorkstreamStatus rejects invalid names consistently', () => { + for (const name of maliciousNames) { + test(`rejects status ${name}`, () => { + const result = runGsdTools(['workstream', 'status', name, '--raw'], tmpDir); + assert.ok(!result.success, `status should reject invalid name: ${name}`); + assert.ok(result.error.includes('Invalid workstream name'), `error should mention invalid name for: ${name}`); + }); + } + }); + + describe('cmdWorkstreamComplete rejects invalid names consistently', () => { + for (const name of maliciousNames) { + test(`rejects complete ${name}`, () => { + const result = runGsdTools(['workstream', 'complete', name, '--raw'], tmpDir); + assert.ok(!result.success, `complete should reject invalid name: ${name}`); + assert.ok(result.error.includes('Invalid workstream name'), `error should mention invalid name for: ${name}`); + }); + } + }); + describe('getActiveWorkstream rejects poisoned active-workstream file', () => { for (const name of maliciousNames) { test(`rejects poisoned file containing ${name}`, () => {