feat(security): PRD-002 S1 — 4 CWE fixes (FORGEPLAN_BIN regex, symlink-guard, spawn cap, ignore-scripts)

.forgeplan/prds/PRD-002-security-tactical-pr-s1-forgeplan-bin-validation-update-symlink-guard-log-rate-limit-build-ignore-scripts.md

@@ -0,0 +1,118 @@
+---
+created: 2026-05-04
+depth: standard
+id: PRD-002
+kind: prd
+priority: P1
+status: draft
+title: "Security tactical PR S1: FORGEPLAN_BIN validation, update symlink-guard, log rate-limit, build ignore-scripts"
+updated: 2026-05-04
+---
+
+# PRD-002: Security tactical PR S1
+
+## Problem
+
+The multi-expert code audit run on `@forgeplan/web@0.1.5` (security-auditor
+domain) surfaced **1 HIGH and 3 MEDIUM** findings on the npm-package
+runtime + build path. None are CRITICAL — package is publishable as-is —
+but each opens a narrow window that compounds with future feature work.
+
+The four findings:
+
+- **CWE-78 (HIGH)** — `FORGEPLAN_BIN` command injection on Windows.
+  `template/src/shared/server/forgeplan.ts:38,68` reads `FORGEPLAN_BIN`
+  from env and passes it to `spawn(..., { shell: process.platform === "win32" })`.
+  On Windows with `shell: true`, the executable path is interpolated into
+  a `cmd.exe` string; spaces / metacharacters in `FORGEPLAN_BIN` interpret
+  as shell tokens. Threat: hostile env -> code execution as the user.
+
+- **CWE-59 (MEDIUM)** — `update` rmSync follows symlink.
+  `bin/forgeplan-web.mjs:195` calls `rmSync(target, { recursive, force })`.
+  If `.forgeplan-web` is a symlink, rmSync removes the target tree.
+
+- **CWE-770 (MEDIUM)** — `/api/log?limit=` no rate-limit.
+  No per-IP rate limit, no concurrency cap on `runForgeplan`. Loopback by
+  default but `HOST=0.0.0.0` opens LAN DoS.
+
+- **CWE-1357 (MEDIUM)** — supply-chain via transitive postinstall.
+  `scripts/build.mjs:100-106` runs `npm install --omit=dev --omit=peer`
+  without `--ignore-scripts`. Future tampered dep could inject postinstall
+  into published `dist/node_modules/`.
+
+**Impact**: combined, an attacker with local write to host project plus
+knowledge of FORGEPLAN_BIN could execute code; LAN-bound instance is
+DoSable. None remotely exploitable today, but each violates defense-in-depth.
+
+## Target Users
+
+| Persona                                         | Description                             | Pain                                                                           |
+| ----------------------------------------------- | --------------------------------------- | ------------------------------------------------------------------------------ |
+| End-user developer                              | Runs `npx @forgeplan/web init && start` | Inherits security posture; cannot inspect every spawn                          |
+| Repo contributor                                | Hacks on this codebase                  | Wants audit findings closed before next dep bump grows new postinstall surface |
+| Security reviewer (CI / Dependabot / npm audit) | Scans the published artifact            | Wants `--ignore-scripts` + validator at every spawn boundary                   |
+
+## Goals
+
+| ID   | Criterion                                              | Metric                                     | Target                      | How to measure |
+| ---- | ------------------------------------------------------ | ------------------------------------------ | --------------------------- | -------------- |
+| SC-1 | `FORGEPLAN_BIN` regex-validated at module load         | grep `FORGEPLAN_BIN_RE` in `forgeplan.ts`  | regex anchored `^...$`      | shell          |
+| SC-2 | `update` refuses symlinked `.forgeplan-web`            | `lstat` + `isSymbolicLink()` branch in bin | present, with `fail()` exit | grep           |
+| SC-3 | `runForgeplan` capped by in-process semaphore          | semaphore impl                             | concurrency cap = 4         | grep + manual  |
+| SC-4 | `scripts/build.mjs` passes `--ignore-scripts`          | grep                                       | exact match                 | shell          |
+| SC-5 | Smoke matrix green on 3 OS x Node 22                   | `gh pr checks`                             | 3/3 pass                    | CI             |
+| SC-6 | `npm audit --omit=dev` template/ unchanged or improved | `npm audit --json`                         | not worse than 0/0/0/2      | shell          |
+
+## Non-Goals
+
+- Do **not** add zod / valibot runtime validators yet (that is PR T1).
+- Do **not** add `hooks.server.ts` with CSP — defense-in-depth, no `{@html}` sinks today; can wait for markdown-rendering feature.
+- Do **not** bump `@sveltejs/kit` to fix cookie GHSA — separate dep-bump PR.
+- Do **not** introduce per-IP rate-limit middleware — concurrency cap is minimum viable.
+- Do **not** refactor unrelated code. Surgical only.
+
+## Functional Requirements
+
+| ID     | Category            | Priority | Requirement                                                                                                              | Acceptance                                                       |
+| ------ | ------------------- | -------- | ------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------- |
+| FR-001 | Spawn safety        | Must     | `FORGEPLAN_BIN` regex-validated `^[A-Za-z0-9_./:\\-]+$` at module load; if fails, refuse spawn with clear error envelope | grep regex; manual `FORGEPLAN_BIN=";rm -rf /"` -> 502            |
+| FR-002 | Filesystem safety   | Must     | `update` `lstat`s target before `rmSync`; if `isSymbolicLink()`, fail with «refusing to follow symlink»                  | grep `lstat` + `isSymbolicLink`; manual symlink test fail-fast   |
+| FR-003 | Filesystem safety   | Must     | `update` asserts target equals `${cwd}/.forgeplan-web` (path-resolve equality, not endsWith)                             | grep `resolve` equality check                                    |
+| FR-004 | Resource exhaustion | Must     | `runForgeplan` enforces in-process concurrency cap of 4; further requests queue                                          | grep semaphore; manual 10 concurrent curls show queueing         |
+| FR-005 | Supply chain        | Must     | `scripts/build.mjs#installRuntimeDeps` passes `--ignore-scripts`                                                         | grep `--ignore-scripts`                                          |
+| FR-006 | Documentation       | Should   | CHANGELOG.md entry under `[Unreleased]` describes each CWE fix                                                           | grep CWE-78, CWE-59, CWE-770, CWE-1357 in CHANGELOG              |
+| FR-007 | Observability       | Could    | Validation rejection emits `console.error` with failing input                                                            | grep `console.error.*FORGEPLAN_BIN` and `console.error.*symlink` |
+
+## Non-Functional Requirements
+
+| ID      | Category      | Requirement                                                           | Metric                                   |
+| ------- | ------------- | --------------------------------------------------------------------- | ---------------------------------------- |
+| NFR-001 | Compatibility | All 4 fixes work on ubuntu / macos / windows x Node 22                | smoke matrix 3/3 green                   |
+| NFR-002 | Performance   | Concurrency cap does not raise polling p95 latency by more than 10 ms | local benchmark                          |
+| NFR-003 | Reversibility | Each FR is independently revertable                                   | 4+ commits in PR, each `git revert`-able |
+| NFR-004 | Tarball drift | `npm pack --dry-run` file count unchanged                             | diff before/after                        |
+
+## Affected Files
+
+- `template/src/shared/server/forgeplan.ts` — FR-001 + FR-004 + FR-007
+- `bin/forgeplan-web.mjs` — FR-002 + FR-003 + FR-007
+- `scripts/build.mjs` — FR-005
+- `CHANGELOG.md` — FR-006
+
+## Related Artifacts
+
+| Artifact | Relation                                   | Status  |
+| -------- | ------------------------------------------ | ------- |
+| PRD-001  | Parent methodology bootstrap               | active  |
+| EVID-005 | Prior safety hardening (init --force hook) | active  |
+| RFC-S1   | Architectural shape of the 4 fixes         | planned |
+| EVID-S1  | Smoke matrix + per-CWE acceptance          | planned |
+
+## Risks & Mitigations
+
+| ID  | Risk                                                                 | Probability | Impact | Mitigation                                                        |
+| --- | -------------------------------------------------------------------- | ----------- | ------ | ----------------------------------------------------------------- |
+| R-1 | FR-005 breaks a transitive dep relying on postinstall                | Low         | High   | Smoke matrix on 3 OS validates; if fail, narrow to dep allow-list |
+| R-2 | FR-001 regex blocks legitimate Windows paths (spaces, drive letters) | Medium      | Medium | Regex permits `[A-Za-z0-9_./:\\-]`; widen if smoke fails          |
+| R-3 | FR-004 semaphore introduces deadlock if route waits on another       | Low         | Medium | Routes don't fan out; semaphore is leaky bucket not lock          |
+| R-4 | FR-002 symlink-guard breaks dev workflows that intentionally symlink | Low         | Low    | Document override env `FORGE_ALLOW_SYMLINK_TARGET=1` if requested |

.forgeplan/session.yaml

@@ -1,7 +1,7 @@
-phase: routing
-active_artifact: null
-route_depth: tactical
-phase_started_at: 2026-05-04T15:37:06.933414+00:00
+phase: coding
+active_artifact: PRD-002
+route_depth: deep
+phase_started_at: 2026-05-04T21:13:18.863028+00:00
 history:
 - from: routing
   to: shaping
@@ -59,3 +59,11 @@ history:
   to: idle
   artifact: null
   timestamp: 2026-05-04T13:26:40.972633+00:00
+- from: routing
+  to: shaping
+  artifact: null
+  timestamp: 2026-05-04T21:11:22.319888+00:00
+- from: shaping
+  to: coding
+  artifact: PRD-002
+  timestamp: 2026-05-04T21:13:18.863024+00:00

CHANGELOG.md

@@ -19,6 +19,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   on the npm registry landing page, not only on GitHub).
 - `.claude/rules/00-index.md` now lists rule 12 (`forgeplan-agent-dispatch`).
 
+### Security
+
+Tactical hardening pass per PRD-002 (S1 batch).
+
+- **CWE-78** — `FORGEPLAN_BIN` regex-validated at module load in `template/src/shared/server/forgeplan.ts`. Refuses spawn (returns `502` envelope) when the env var contains characters outside `[A-Za-z0-9_./:\-]`. Closes a Windows `shell:true` command-injection vector.
+- **CWE-59** — `bin/forgeplan-web.mjs#update` now `lstat`s `.forgeplan-web/` before `rmSync`; refuses to operate when target is a symlink. Plus a `resolve()`-equality assert as defense-in-depth against future refactors.
+- **CWE-770** — `template/src/shared/server/forgeplan.ts` enforces in-process spawn concurrency cap (4 simultaneous `forgeplan` processes). Bounds loopback / LAN-bound DoS surface.
+- **CWE-1357** — `scripts/build.mjs#installRuntimeDeps` passes `--ignore-scripts` to `npm install`. Blocks transitive postinstall hooks from baking arbitrary code into published `dist/node_modules/`.
+
 ## [0.1.4] - 2026-05-04
 
 ### Added

bin/forgeplan-web.mjs

@@ -3,6 +3,7 @@ import { spawn, spawnSync } from "node:child_process";
 import {
   cpSync,
   existsSync,
+  lstatSync,
   mkdirSync,
   readFileSync,
   rmSync,
@@ -163,11 +164,11 @@ function update() {
   ensureForgeplanWorkspace(cwd);
   ensureForgeplanBinary();
 
-  const target = join(cwd, '.forgeplan-web');
+  const target = join(cwd, ".forgeplan-web");
   if (!existsSync(target)) {
     fail(
       `no scaffold at ${target}.\n` +
-      `       Run \`npx @forgeplan/web init\` first.`
+        `       Run \`npx @forgeplan/web init\` first.`,
     );
   }
 
@@ -177,24 +178,50 @@ function update() {
 
   if (!FORCE && fromVersion && toVersion && fromVersion === toVersion) {
     log(`✓ already at v${toVersion}`);
-    log('  Use --force to re-copy anyway.');
+    log("  Use --force to re-copy anyway.");
     return;
   }
 
-  const fromLabel = fromVersion ? `v${fromVersion}` : 'unknown';
-  const toLabel = toVersion ? `v${toVersion}` : 'unknown';
+  const fromLabel = fromVersion ? `v${fromVersion}` : "unknown";
+  const toLabel = toVersion ? `v${toVersion}` : "unknown";
   log(`→ updating ${target} (${fromLabel} → ${toLabel})`);
 
   if (!existsSync(DIST_DIR)) {
     fail(
       `pre-built artifact missing at ${DIST_DIR}.\n` +
-      `       Reinstall @forgeplan/web or build from source via \`npm run build\`.`
+        `       Reinstall @forgeplan/web or build from source via \`npm run build\`.`,
+    );
+  }
+
+  // FR-002: rmSync follows symlinks; a symlinked .forgeplan-web would
+  // delete the link's target tree (CWE-59). Refuse before destructive ops.
+  const targetStat = lstatSync(target);
+  if (targetStat.isSymbolicLink()) {
+    fail(
+      `refusing to follow symlink at ${target}.\n` +
+        `       \`update\` will not rmSync through a symlinked .forgeplan-web/.\n` +
+        `       Remove the symlink manually and re-run.`,
+    );
+  }
+
+  // FR-003: defense-in-depth — even though target is constructed from
+  // join(cwd, '.forgeplan-web'), assert post-resolve equality so any
+  // future refactor (env override, alias) cannot widen the rmSync blast
+  // radius beyond the canonical path.
+  const expected = resolve(join(cwd, ".forgeplan-web"));
+  if (resolve(target) !== expected) {
+    fail(
+      `refusing to rmSync unexpected path ${resolve(target)} (expected ${expected}).`,
     );
   }
 
   rmSync(target, { recursive: true, force: true });
   mkdirSync(target, { recursive: true });
-  cpSync(DIST_DIR, target, { recursive: true, dereference: false, force: true });
+  cpSync(DIST_DIR, target, {
+    recursive: true,
+    dereference: false,
+    force: true,
+  });
 
   const now = new Date().toISOString();
   const cfg = {
@@ -203,11 +230,11 @@ function update() {
     version: toVersion,
     updatedAt: now,
   };
-  writeFileSync(join(target, CFG_FILE), JSON.stringify(cfg, null, 2) + '\n');
+  writeFileSync(join(target, CFG_FILE), JSON.stringify(cfg, null, 2) + "\n");
 
-  log('');
+  log("");
   log(`✓ updated to ${toLabel}`);
-  log('  npx @forgeplan/web start');
+  log("  npx @forgeplan/web start");
 }
 
 function start() {

scripts/build.mjs

@@ -98,9 +98,18 @@ function emitDistPackageJson() {
 }
 
 function installRuntimeDeps() {
+  // FR-005 / CWE-1357: --ignore-scripts blocks transitive postinstall hooks
+  // from baking attacker-controlled code into published dist/node_modules/.
   run(
     "npm",
-    ["install", "--omit=dev", "--omit=peer", "--no-fund", "--no-audit"],
+    [
+      "install",
+      "--omit=dev",
+      "--omit=peer",
+      "--no-fund",
+      "--no-audit",
+      "--ignore-scripts",
+    ],
     TEMPLATE_BUILD,
   );
 }