Merge #547: destructive path operations need an allowlisted root

docs(security): destructive path targets need an allowlist, a depth floor, and an owner check

references/security-checklist.md

@@ -22,7 +22,7 @@ Quick reference for web application security. Use alongside the `security-and-ha
 
 Before reaching for controls, spend five minutes thinking like an attacker:
 
-- [ ] Trust boundaries mapped (requests, uploads, webhooks, third-party APIs, LLM output)
+- [ ] Trust boundaries mapped (requests, uploads, webhooks, third-party APIs, LLM output, and local values written by processes you don't control)
 - [ ] Assets named (credentials, PII, payment data, admin actions, money movement)
 - [ ] STRIDE run per boundary (Spoofing, Tampering, Repudiation, Info disclosure, DoS, Elevation)
 - [ ] Abuse cases written next to use cases ("how would I misuse this?")
@@ -63,6 +63,46 @@ Before reaching for controls, spend five minutes thinking like an attacker:
 - [ ] HTML output encoded (use framework auto-escaping)
 - [ ] URLs validated before redirect (prevent open redirect)
 - [ ] Server-side URL fetches allowlisted; private/reserved IPs blocked (prevent SSRF)
+- [ ] Destructive path operations (delete/move/overwrite): symlinks resolved, allowlisted root, minimum depth, ownership evidence read before the call
+
+### Destructive Path Operations
+
+Containment for a target named by data. Resolve first, then decide — and treat the
+result as a candidate, not as authorization:
+
+```typescript
+import { realpath, readFile } from 'node:fs/promises';
+import { resolve, relative, isAbsolute, join, sep } from 'node:path';
+
+const ALLOWED_ROOTS = ['/var/lib/myapp/sessions']; // an allowlist, not a pattern
+const MIN_DEPTH = 1;                               // so a root is never the target
+
+async function resolveDeletable(candidate: string, expectedOwner: string) {
+  const target = await realpath(resolve(candidate)); // symlinks resolved BEFORE the check
+  const inRoot = ALLOWED_ROOTS.some((root) => {
+    const rel = relative(root, target);
+    // `rel === '..'` / `'../'` only — a plain `startsWith('..')` would also
+    // reject a legitimate child named `..cache`.
+    if (rel === '' || rel === '..' || rel.startsWith(`..${sep}`) || isAbsolute(rel)) return false;
+    return rel.split(sep).length >= MIN_DEPTH;
+  });
+  if (!inRoot) throw new Error(`refusing: outside allowed roots (${target})`);
+
+  const owner = await readFile(join(target, '.owner'), 'utf8').catch(() => null);
+  if (owner?.trim() !== expectedOwner) throw new Error(`refusing: unproven owner (${target})`);
+  return target;
+}
+```
+
+What this does not do, and must be said where the snippet is copied from:
+
+- **The marker is self-attestation.** Anything that can write inside the root can write
+  `.owner`. `expectedOwner` has to come from authenticated state, and the marker needs
+  integrity protection (restrictive ownership, or a MAC) before it is authorization
+  rather than a consistency check against a misderived target.
+- **Returning a path leaves a check/use race.** Where an untrusted process can swap an
+  ancestor between the check and the call, operate on a descriptor with no-follow,
+  beneath-the-root semantics, or guarantee the hierarchy is immutable for the duration.
 
 ## Security Headers
 

skills/security-and-hardening/SKILL.md

@@ -22,7 +22,7 @@ Security-first development practices for web applications. Treat every external
 
 Controls bolted on without a threat model are guesses. Before hardening, spend five minutes thinking like an attacker:
 
-1. **Map the trust boundaries.** Where does untrusted data cross into your system? HTTP requests, form fields, file uploads, webhooks, third-party APIs, message queues, and **LLM output**. Every boundary is attack surface.
+1. **Map the trust boundaries.** Where does untrusted data cross into your system? HTTP requests, form fields, file uploads, webhooks, third-party APIs, message queues, and **LLM output** — plus the local values that look internal because the OS handed them to you: another process's command line or environment, filenames on a shared volume, a path in a job payload. Trust follows who *wrote* a value, not which channel delivered it. Every boundary is attack surface.
 2. **Name the assets.** What's worth stealing or breaking? Credentials, PII, payment data, admin actions, money movement.
 3. **Run STRIDE over each boundary** — a quick lens, not a ceremony:
 
@@ -269,6 +269,14 @@ function validateUpload(file: UploadedFile) {
 }
 ```
 
+### Destructive Operations on Derived Paths
+
+A delete, move, or overwrite is only as safe as the value that names its target. Reading that value from the kernel, a job payload, or a sibling service proves where it *arrived from*, not who *wrote* it — another process's command line is as attacker-controlled as a form field. A shape check ("absolute path, at least one directory deep") proves well-formedness and gets mistaken for authorization; that is how a cleanup routine deletes the root instead of the leaf.
+
+Before a destructive call, require all three: the resolved target sits under an **allowlisted root** (compare after resolving symlinks, never on the raw string); it is at least one level **below** that root, so a root is never itself the target; and it carries **evidence that it is yours**, read *before* the operation and before any teardown that removes it — otherwise "absent" and "not mine" are indistinguishable. On refusal, log the rejected target and stop: a cleanup that falls back to a broader default path is the failure this guards against. Worked example in `../../references/security-checklist.md`.
+
+Two limits, because the check reads stronger than it is. A marker inside the tree is self-attestation — anything that can write there can write the marker — so the expected owner has to come from authenticated state, and the marker needs integrity protection (restrictive ownership, or a MAC) before it counts as authorization. And resolving a path and then operating on the *name* is a check/use race wherever an untrusted process can swap an ancestor: on a shared volume, hold the target by descriptor and use no-follow, beneath-the-root operations, or make sure the hierarchy cannot change for the duration.
+
 ## Triaging Dependency Audit Results
 
 Package-manager audits report known advisories; they do not prove a package is trustworthy or that vulnerable code is reachable. Use this decision tree:
@@ -435,6 +443,7 @@ container.textContent = await llm.reply(userMessage);
 - [ ] SQL queries are parameterized
 - [ ] HTML output is encoded/escaped
 - [ ] Server-side URL fetches are allowlisted (no SSRF to internal services)
+- [ ] Delete/move/overwrite targets built from data are checked against an allowlisted root, a minimum depth, and ownership evidence read before the operation
 
 ### Data
 - [ ] No secrets in code or version control
@@ -483,6 +492,7 @@ For detailed security checklists and pre-commit verification steps, see `../../r
 ## Red Flags
 
 - User input passed directly to database queries, shell commands, or HTML rendering
+- A delete, move, or overwrite whose target comes from a payload, a config value, or another process's command line, guarded only by a shape check on the path
 - Secrets in source code or commit history
 - API endpoints without authentication or authorization checks
 - Missing CORS configuration or wildcard (`*`) origins
@@ -503,6 +513,7 @@ After implementing security-relevant code:
 - [ ] The native audit has no unmitigated reachable critical/high findings; CI preserves the authoritative lockfile and blocks unreviewed dependency scripts
 - [ ] No secrets in source code or git history
 - [ ] All user input validated at system boundaries
+- [ ] Destructive filesystem operations resolve symlinks, then verify allowlisted root, minimum depth, and ownership before running
 - [ ] Authentication and authorization checked on every protected endpoint
 - [ ] Security headers present in response (check with browser DevTools)
 - [ ] Error responses don't expose internal details