feat(security): PRD-002 S1 — 4 CWE fixes (FORGEPLAN_BIN regex, symlink-guard, spawn cap, ignore-scripts)#18
Merged
Conversation
…CWE-770) Security audit on v0.1.5 surfaced two issues at the SvelteKit-to-forgeplan-CLI boundary in template/src/shared/server/forgeplan.ts: 1) FORGEPLAN_BIN reads from env and is passed to spawn with shell:true on Windows. With shell:true the executable path interpolates into cmd.exe, so any whitespace or shell metacharacter in FORGEPLAN_BIN becomes a shell token. CWE-78 hostile-env command injection. Validate at module load against a regex anchored ^[A-Za-z0-9_./:\\-]+$. On rejection: console.error the offending value, fall back to literal forgeplan, refuse every spawn with a 502-shaped envelope. 2) runForgeplan had no concurrency cap. A loopback DoS or HOST=0.0.0.0 LAN-bound instance can pile up forgeplan subprocess spawns until the event loop stalls. CWE-770 resource exhaustion. In-process semaphore caps simultaneous spawns at 4; further calls queue via acquireSpawnSlot/releaseSpawnSlot, with the slot released in a finally block on the inner Promise so timeout, error, close all return capacity. Tests: svelte-check 0 errors / 0 warnings. Smoke (npm run smoke) PASS on macOS in 11.46s; T-1 hardening visible in compiled dist/server/chunks/server-*.js (regex literal, console.error message, refuse-with-502 path). Refs: PRD-002 (FR-001, FR-004, FR-007)
…(CWE-59) bin/forgeplan-web.mjs#update calls rmSync(target, recursive, force) where target = join(cwd, .forgeplan-web). If .forgeplan-web is a symlink (legitimate dev workflow or hostile), rmSync would happily remove the resolved tree. CWE-59 link following. Two-layer guard before rmSync: (1) lstatSync target; if isSymbolicLink, fail with refusing-to-follow message and the offending path; (2) defense-in-depth equality assert resolve(target) === resolve(join(cwd, .forgeplan-web)) — tautological today, but a tripwire if a future refactor reassigns target from env or config. Both rejections console.error the failing path so operators see the cause. fail() exits non-zero. init's cpSync stays untouched: it's add-only, not destructive, and the host-isolation rule 20 already bounds its blast radius. Tests: node --check bin syntax PASS. Smoke (npm run smoke) PASS — init run 1 created scaffold, run 2 with --force passed through the new guards as a non-symlinked legitimate target. Negative path (symlinked target) verified by code review only; runtime test would require manipulating /tmp scratch outside the smoke harness. Refs: PRD-002 (FR-002, FR-003, FR-007)
scripts/build.mjs#installRuntimeDeps runs npm install --omit=dev --omit=peer inside template/build/ to populate the published dist/node_modules/. Without --ignore-scripts, every transitive runtime dep gets a chance to execute postinstall lifecycle scripts during package build. CWE-1357 supply-chain via lifecycle hooks. Adding --ignore-scripts is the npm-blessed mitigation: blocks postinstall, preinstall, and install scripts. Today's 5 runtime deps (@sveltejs/kit, d3-force, d3-selection, d3-zoom, svelte) are pure-JS and don't need any postinstall, so this is loss-free. If a future runtime dep relies on postinstall (e.g. native binding download), the documented fallback is to whitelist that dep — but smoke matrix on 3 OS will catch the breakage at PR review, not in production. Tests: smoke (npm run smoke) PASS on macOS — runtime install line in stdout ends with --ignore-scripts and added 41 packages without invoking any lifecycle hook. Refs: PRD-002 (FR-005)
Tactical hardening pass per PRD-002 (S1 batch). One bullet per CWE — FORGEPLAN_BIN regex (CWE-78), update symlink-guard (CWE-59), spawn concurrency cap (CWE-770), build --ignore-scripts (CWE-1357). Existing [Unreleased] preamble (Pending for the next release) untouched; older release sections shifted by 9 lines. Refs: PRD-002 (FR-006)
Lockfile name field still said 0.1.0; running npm install during smoke updated it to match template/package.json#version (0.1.5). Captured here so develop carries the synced lock. Refs: PRD-002 (release follow-up)
Adds the PRD-002 markdown driving this branch. Standard depth (PRD only — RFC skipped per architecture-obvious clause), 7 FRs, 4 NFRs, 4 risks. Linked artifacts: PRD-001 parent methodology, EVID-005 prior safety hardening, RFC-S1 / EVID-S1 planned. Refs: PRD-002
explosivebit
added a commit
that referenced
this pull request
May 5, 2026
Forge-cycle Step 7-8 for PR #18 (PRD-002 S1 security tactical). EVID-009 verifies all 6 SC + 4 NFR across source/compiled/CI layers. R_eff PRD-002 = 1.00 CL3 grade A. Activates both EVID-009 and PRD-002. Refs PRD-002 EVID-009.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes 4 audit findings from today's multi-expert code audit on @forgeplan/web@0.1.5. None are CRITICAL — package was publishable as-is — but each opens a narrow window. CWE-78 hostile-env command injection on Windows shell:true. CWE-59 update rmSync follows symlink. CWE-770 spawn DoS via /api/log without concurrency cap. CWE-1357 transitive postinstall during package build. Refs PRD-002. 6 commits, each git-revert-able. Smoke local PASS (3-OS CI matrix will gate merge). Test plan in PRD-002 Goals SC-1..SC-6.