From 45fd4a043983fec456bf7eea6562147e8e9cd2b4 Mon Sep 17 00:00:00 2001 From: notbucki <103531753+notbucki@users.noreply.github.com> Date: Fri, 4 Sep 2026 09:52:18 +0200 Subject: [PATCH] docs(security): destructive path targets need an allowlist, a depth floor, and an owner check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The trust-boundary list names only remote channels, so local values that the OS hands you — another process's command line or environment, a filename on a shared volume, a path in a job payload — read as internal. The one rule about destructive actions (LLM06) governs who may act, not what gets hit: a confirmed call with a wrongly derived target deletes the root just the same. A shape check ("absolute, at least one directory deep") proves well-formedness and gets mistaken for authorization. SKILL.md gets the extended trust-boundary item, a prose-only "Destructive Operations on Derived Paths" next to File Upload Safety, and one line each in the Input checklist, Red Flags and Verification. The worked example lives in references/security-checklist.md, per skill-anatomy's rule to move detailed material out of SKILL.md — the file is already over the 500-line budget, so this keeps the per-activation cost to +11 lines. Two limits are stated where the snippet is copied from, rather than implied away: a marker inside the tree is self-attestation until its integrity is protected and the expected owner comes from authenticated state, and resolving a path and then operating on the name is a check/use race wherever an untrusted process can swap an ancestor. No new directory, frontmatter unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- references/security-checklist.md | 42 +++++++++++++++++++++++++- skills/security-and-hardening/SKILL.md | 13 +++++++- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/references/security-checklist.md b/references/security-checklist.md index 0f656479..b7df3eb0 100644 --- a/references/security-checklist.md +++ b/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 diff --git a/skills/security-and-hardening/SKILL.md b/skills/security-and-hardening/SKILL.md index cf093e93..05debf5e 100644 --- a/skills/security-and-hardening/SKILL.md +++ b/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