mirror of
https://github.com/addyosmani/agent-skills.git
synced 2026-10-26 15:05:04 +00:00
docs(security): destructive path targets need an allowlist, a depth floor, and an owner check
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) <noreply@anthropic.com>
This commit is contained in:
1 parent
1c760d6434
commit
45fd4a0439
2 files changed
+53
-2
No files matched your search
@@ -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
|
||||
|
||||
|
||||
Reference in new issue
Block a user