fix(3589)(security): validate workstream in relPlanningPath against traversal

`relPlanningPath(workstream)` previously called `posix.join('.planning',
'workstreams', workstream)` without validating the workstream argument.
Direct SDK callers — and `planningPaths` / `ContextEngine` which both
forward through `relPlanningPath` — could pass values like
`'../../../outside'`, `'foo/bar'`, or `'foo\\bar'` and route planning
operations outside the intended `.planning/workstreams/<name>` subtree.

The env-sourced workstream code path in `planningPaths` already validated
via `validateWorkstreamName` (line 444-445, pre-filtering to `null` on
failure per the #2791 silent-fallback contract). Explicit SDK arguments
had no equivalent gate.

Fix: validate inside `relPlanningPath` using the same shared
`validateWorkstreamName` policy. Every caller — direct SDK use,
`planningPaths`, `ContextEngine` — fails closed at the same seam.
Empty/undefined workstream still returns `.planning` for back-compat
(treated as "no workstream provided"); non-empty invalid names throw a
synchronous Error with the offending value in the message.

Env-sourced behaviour is unchanged: `planningPaths` continues to filter
invalid env values to `null` before they reach `relPlanningPath`, so the
silent-fallback path for malformed `GSD_WORKSTREAM` env still works.

Regression test
(sdk/src/bug-3589-planning-paths-validation.test.ts):

  - 9 traversal/invalid cases (.., /, \\, spaces, .hidden, /abs,
    -leading-hyphen) all throw with a `/workstream/i`-matching message.
  - Valid names (`frontend`, `api_v2`, `alpha.beta-1`) continue to
    produce the expected `.planning/workstreams/<name>` path.
  - `planningPaths('/tmp', '../../../outside')` rejects before path
    construction (proven via try/catch — resultPath stays null).
  - Valid workstream + `planningPaths` produces the expected subtree
    (`.planning/workstreams/frontend/STATE.md` etc.).
  - Omitted workstream still returns root `.planning` with no `workstreams`
    segment.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

.changeset/3589-planning-paths-workstream-validation.md

@@ -0,0 +1,5 @@
+---
+type: Security
+issue: 3589
+---
+**`relPlanningPath()` now validates explicit workstream names** — direct SDK callers passing a workstream argument to `relPlanningPath`, `planningPaths`, or `ContextEngine` previously had no path-traversal gate. A value like `'../../../outside'` flowed through `posix.join('.planning', 'workstreams', name)` and routed planning operations outside the intended `.planning/workstreams/<name>` subtree. The fix runs the shared `validateWorkstreamName` policy inside `relPlanningPath` so every consumer fails closed at the same seam. Env-sourced workstreams continue to fall back silently to root `.planning/` per the #2791 contract (they are filtered to `null` by `planningPaths` before reaching `relPlanningPath`).

sdk/src/bug-3589-planning-paths-validation.test.ts

@@ -0,0 +1,88 @@
+/**
+ * Bug #3589 (security): SDK `planningPaths(projectDir, workstream)` and
+ * `relPlanningPath(workstream)` accepted unvalidated explicit workstream
+ * names from direct SDK callers. Path-traversal segments (`..`, `/`, `\\`)
+ * would flow through `posix.join('.planning', 'workstreams', name)` and
+ * route planning operations outside the intended `.planning/workstreams/<name>`
+ * subtree.
+ *
+ * Env-sourced workstreams are pre-validated inside `planningPaths` and fall
+ * back to root .planning/ silently (#2791 contract). Explicit SDK arguments
+ * had no such gate.
+ *
+ * Fix: validate inside `relPlanningPath` so every caller — direct SDK use,
+ * `planningPaths`, `ContextEngine` — is protected at the same seam.
+ * Explicit invalid names throw; env-sourced ones still silently fall back
+ * because `planningPaths` filters them to `null` before calling
+ * `relPlanningPath`.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { relPlanningPath } from './workstream-utils.js';
+import { planningPaths } from './query/helpers.js';
+
+// Empty string is treated as "no workstream provided" (returns `.planning`)
+// for back-compat with pre-fix behaviour; only non-empty invalid names throw.
+const TRAVERSAL_CASES = [
+  '../../../outside',
+  '../escape',
+  '..',
+  'foo/bar',
+  'foo\\bar',
+  'foo bar',
+  '.hidden',
+  '/abs',
+  '-leading-hyphen',
+];
+
+describe('bug #3589: relPlanningPath rejects path-traversal and invalid workstream names', () => {
+  it('returns .planning when workstream is omitted (unchanged)', () => {
+    expect(relPlanningPath()).toBe('.planning');
+    expect(relPlanningPath(undefined)).toBe('.planning');
+  });
+
+  it('returns .planning/workstreams/<name> for valid workstream names (unchanged)', () => {
+    expect(relPlanningPath('frontend')).toBe('.planning/workstreams/frontend');
+    expect(relPlanningPath('api_v2')).toBe('.planning/workstreams/api_v2');
+    expect(relPlanningPath('alpha.beta-1')).toBe('.planning/workstreams/alpha.beta-1');
+  });
+
+  for (const bad of TRAVERSAL_CASES) {
+    it(`throws for invalid workstream name ${JSON.stringify(bad)}`, () => {
+      expect(() => relPlanningPath(bad)).toThrow(/workstream/i);
+    });
+  }
+
+  it('throws BEFORE constructing the path (no partial side effect)', () => {
+    let resultPath: string | null = null;
+    try {
+      resultPath = relPlanningPath('../../../outside');
+    } catch {
+      /* expected */
+    }
+    expect(resultPath).toBeNull();
+  });
+});
+
+describe('bug #3589: planningPaths rejects explicit invalid workstream names', () => {
+  it('throws for explicit ../../../outside (was silently constructing a traversal path)', () => {
+    expect(() => planningPaths('/tmp/projectDir', '../../../outside')).toThrow(/workstream/i);
+  });
+
+  it('throws for explicit slash-bearing names', () => {
+    expect(() => planningPaths('/tmp/projectDir', 'foo/bar')).toThrow(/workstream/i);
+  });
+
+  it('accepts valid explicit names and constructs the expected planning subtree', () => {
+    const paths = planningPaths('/tmp/projectDir', 'frontend');
+    expect(paths.planning.endsWith('.planning/workstreams/frontend')).toBe(true);
+    expect(paths.state.endsWith('.planning/workstreams/frontend/STATE.md')).toBe(true);
+    expect(paths.roadmap.endsWith('.planning/workstreams/frontend/ROADMAP.md')).toBe(true);
+  });
+
+  it('still returns root .planning when workstream is omitted', () => {
+    const paths = planningPaths('/tmp/projectDir');
+    expect(paths.planning.endsWith('.planning')).toBe(true);
+    expect(paths.planning).not.toContain('workstreams');
+  });
+});

sdk/src/workstream-utils.ts

@@ -6,6 +6,7 @@
  */
 
 import { posix } from 'node:path';
+import { validateWorkstreamName } from './workstream-name-policy.js';
 export { validateWorkstreamName, toWorkstreamSlug } from './workstream-name-policy.js';
 
 /**
@@ -13,9 +14,23 @@ export { validateWorkstreamName, toWorkstreamSlug } from './workstream-name-poli
  *
  * - Without workstream: `.planning`
  * - With workstream: `.planning/workstreams/<name>`
+ *
+ * #3589 (security): validates the explicit workstream name against the
+ * shared `validateWorkstreamName` policy before path construction. Path
+ * traversal segments (`..`, `/`, `\\`) and other invalid identifiers throw
+ * synchronously, so every caller — direct SDK use, `planningPaths`,
+ * `ContextEngine` — fails closed at the same seam. Env-sourced workstreams
+ * are still pre-filtered to `null` by `planningPaths` (the #2791 silent
+ * fallback contract), so this guard does NOT change env-sourced behaviour.
  */
 export function relPlanningPath(workstream?: string): string {
   if (!workstream) return '.planning';
+  if (!validateWorkstreamName(workstream)) {
+    throw new Error(
+      `Invalid workstream name: ${JSON.stringify(workstream)}. ` +
+        `Workstream names must match /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/ and may not contain '..'.`,
+    );
+  }
   // Use POSIX segments so the same logical path string is used on all platforms (Windows included).
   return posix.join('.planning', 'workstreams', workstream);
 }