fix(security): expand denylist to block /proc subtree (#29)

[RapierCraft]

Summary

Expands the upload path denylist to block the entire /proc subtree instead of the single leaf path /proc/self/environ. The /proc filesystem exposes sensitive runtime data — memory maps, open file descriptors, environment variables — for arbitrary PIDs, all of which appear as regular files and would previously have passed validation.

Changes

• src/upload-validator.ts: Replace "/proc/self/environ" with "/proc" in blockedPrefixes — blocks entire subtree via existing prefix-match loop
• tests/unit/upload-validator.test.ts: New file with 19 unit tests covering /proc subtree blocking, home-dir sensitive paths, .env blocking, COMET_UPLOAD_ROOT allowlist, and error cases

Testing

• /proc/self/environ blocked [type:unit]
• /proc/self/maps blocked [type:unit]
• /proc/self/fd/0 blocked [type:unit]
• /proc/1/environ blocked [type:unit]
• /proc/12345/mem blocked [type:unit]
• All 19 unit tests pass [type:unit]
• TypeScript build passes [type:unit]

---

Closes #29
Implementation branch: fix/expand-proc-denylist-29
Base: main

[RapierCraft]

Security Review — PR #35

Reviewed commit: 8b5e8a8
Agent: General Security
Scope: src/upload-validator.ts, tests/unit/upload-validator.test.ts

---

Findings Summary

No blocking security issues found.

Analysis

1. Core Fix Verification — prefix correctness

The change replaces "/proc/self/environ" with "/proc" at line 118.

The prefix-match loop (lines 121–129):

const withSep = prefix.endsWith(PATH_SEP) ? prefix : prefix + PATH_SEP;
if (real === prefix || real.startsWith(withSep)) { throw ... }

For prefix = "/proc":

• withSep = "/proc/"
• real === "/proc" → blocks exact /proc mount point
• real.startsWith("/proc/") → blocks /proc/self/environ, /proc/self/maps, /proc/self/fd/0, /proc/1/environ, /proc/12345/mem, etc.

False positive check: /procdata/something — withSep = "/proc/" → "/procdata/something".startsWith("/proc/") = false ✓ Not falsely blocked.

2. Test Coverage Assessment

Tests correctly stub realpathSync and statSync via vi.mock. All 6 /proc subtree cases from the issue acceptance criteria are explicitly tested. Tests are portable (no OS dependency). 19 tests pass.

3. No Injection, SSRF, or Other Vulnerabilities

• No user input reaches SQL or shell commands
• No new env vars, no hardcoded secrets
• The .aws/credentials string in the diff is a test path literal (mock return value), not a real credential
• Function signature unchanged — no callers affected

4. Broader Denylist Coverage Check

No gaps identified: the fix specifically addresses the issue's scope. Note for future: /sys (Linux kernel interfaces) is another pseudo-filesystem that could be worth adding, but that is out of scope for this PR.

---

Verdict: APPROVED. The fix is correct, minimal, and well-tested. Safe to merge.

---

[RapierCraft]

APPROVED: commit 8b5e8a8 after context-aware security review (1 agent: General Security). 0 findings. Safe to merge.

• Build: PASS (tsc clean)
• Tests: 19/19 new tests pass; 2 pre-existing failures in unrelated cdp-client.screenshot.test.ts are not introduced by this PR
• Security: fix correctly blocks entire /proc subtree via withSep prefix match; no false positives for /procdata paths
• No blocking issues found

[RapierCraft]

PR Review Summary: #35 — fix(security): expand denylist to block /proc subtree (#29)

Review Integrity

Reviewed commit: 8b5e8a8 | Current HEAD: 8b5e8a8 | Status: CURRENT

Verdict: APPROVED

Context-Aware Review

Domains: Security | Agents: 1 (General Security)

Integration Checks (Phase 2.5)

Code registration: PASS — no new exports or router registrations needed
SOPS deploy chain: N/A
Purpose Regression Gate (7A): N/A — non-milestone PR

Risk Matrix

Category	Risk	Blocking?	Confidence
/proc prefix correctness	LOW	No	CONFIRMED safe
False positive (/procdata)	LOW	No	CONFIRMED no false positives
Test portability	LOW	No	CONFIRMED mocked

Findings

No findings — review is clean.

Automated Checks

Check	Result
TypeScript build (tsc)	PASS
Unit tests (new)	19/19 PASS
Pre-existing test failures	2 (cdp-client.screenshot — not introduced by this PR)
Secrets detection	PASS
Env var audit	PASS (no new vars)

Recommendation

Merged. The 1-line fix correctly expands to in the denylist, blocking the entire /proc subtree via the existing prefix-match pattern. 19 portable unit tests added.

---

Context-aware review complete. 1 agent + integration checks. 0 findings triaged. PR merged to main.