PastilhasandClaude Opus 5
1597fccd75
Exposes sharp and ffprobe with their own vocabulary — operations are named what sharp names them, ffprobe fields come back spelled the way ffprobe spells them — so the tools' own documentation applies and there is no house dialect to learn. Safety comes from one architectural invariant rather than from filtering: the caller never supplies a path, a URL or a protocol. Input is an id we issued, output is a name we chose. That removes the entire class of attack that makes "just pass the flags through" a remote shell, which is precisely what makes it safe to open up everything else. docs/SECURITY.md has the reasoning and the specific attacks. The vocabulary is a data table, so the validator and docs/SHARP.md are generated from one source and cannot drift. Two things are deliberately absent from it: withMetadata, because that is how a client's GPS coordinates get published, and limitInputPixels, because a caller who could raise it could turn a 1 KB PNG into a 40 GB allocation. Verified against a running server, not asserted: traversal ids rejected, unknown operations and typo'd parameters rejected by name, out-of-range values rejected with the bound, a GPS-tagged photo coming out clean end to end, video probe, Range requests, and a rejected upload leaving nothing on disk. Found while building: config threw at import if AV_CLUB_TOKEN was unset, so generating documentation required a production secret. The token is read at boot in server.ts now — still a refusal to start, no longer a module-scope landmine. ffmpeg is designed in docs/API.md and not built yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
7.4 KiB
Security model
av-club exposes ffmpeg, sharp and exiftool over HTTP. Done naively that is a remote shell. This document is the set of invariants that make it not one, and the reasoning for each — so that a future change can be checked against the reasoning rather than against a vibe.
Read this before adding an endpoint, an option, or a filter.
The one invariant everything else rests on
The caller never supplies a path, a URL, or a protocol. Not for input, not for output, not inside a filter argument, not anywhere.
Every dangerous thing these tools can do is reached through a path or a URL. Remove the caller's ability to name one and the entire class disappears, which is what makes it safe to expose almost every other option these tools have.
Concretely: input is an opaque asset id issued by av-club for a file av-club received. Output is a path av-club chooses. Both are resolved server-side into paths under a job directory. Anything that looks like a path in a caller-supplied value is rejected, not sanitised.
What that invariant is defending against
These all work by default, which is why "just pass the flags through" is not an option:
| Attack | How |
|---|---|
| Read any file the process can | -i /etc/passwd, then download the "converted" output |
| Read a file the input filter never sees | -f lavfi -i "movie=/home/user/.ssh/id_ed25519" |
| Read arbitrary paths via a demuxer | -f concat -i list.txt with a caller-controlled list |
| SSRF into the private network | ffmpeg speaks http: — cloud metadata endpoints, internal services |
| Write anywhere the process can | the output path is just an argument: ~/.ssh/authorized_keys |
| Load arbitrary native code | frei0r, ladspa, lv2 filters load plugins by name from disk |
Note the second row especially. A guard that only inspected -i would pass it, because the read
happens inside the filter graph. This is why the rule is about values, not about which flag they
arrived in.
Layer 1 — no shell, ever
Processes are spawned with an argv array, never a command string. There is no shell to quote for, so shell metacharacters are inert data rather than a parsing problem.
Anything that builds a command by string concatenation is a bug regardless of how well it escapes.
Layer 2 — the caller names nothing
| Input | An asset id (opaque, server-issued) or a multipart upload. Never a path or URL. |
| Output | av-club picks the filename and directory. The caller picks the format, not where it lands. |
| Working dir | One temp directory per job, removed when the job ends. |
Asset ids are validated against a strict pattern before they are used to build a path, and the resolved path is checked to still be inside the store afterwards. Two checks, because the first is about intent and the second is about what actually happened.
Layer 3 — ffmpeg runs with protocols disabled
-protocol_whitelist file (nothing else — no http, no concat, no subfile, no data)
-f <explicit> inputs are demuxed as what we probed them to be, not auto-detected
-nostdin ffmpeg never waits on stdin
-protocol_whitelist file alone kills SSRF and the concat:/subfile: tricks outright. It is not
a mitigation for a path leak — Layer 2 is — it is the second lock.
Layer 4 — filters are allowlisted by name, and their arguments are checked
ffmpeg's filter graph is a language. Most of it is arithmetic on pixels and samples and is safe. A small, specific part of it touches the filesystem or loads code.
Never allowed — these read files or load native code, by design:
movie, amovie, subtitles, ass, frei0r, frei0r_src, ladspa, lv2, sendcmd,
asendcmd, vidstabtransform, libvmaf, openclsrc
Allowed, but with file-bearing parameters stripped: drawtext is genuinely useful and carries
textfile= and fontfile=. The filter is permitted; those two parameters are rejected. Text comes
as a literal, fonts come from a fixed server-side set referenced by name.
The list is a default-deny allowlist, not a blocklist. A filter nobody has classified is refused. A blocklist would silently admit whatever the next ffmpeg release adds.
Layer 5 — bounded work
Unbounded is a denial of service even when it is not a breach.
| Bound | Why |
|---|---|
| Upload size cap | Multipart bodies are buffered; an unbounded one is an OOM |
| Wall-clock timeout per job, process killed on expiry | A crafted file can make an encoder run effectively forever |
| Output dimension cap | resize(50000, 50000) is an allocation, not an image |
limitInputPixels on sharp |
Decompression bombs: a 1 KB PNG that expands to 40 GB |
| Concurrency cap | ffmpeg will use every core it is given; two callers should not be able to take the box down |
| Job directory removed on completion and on failure | Otherwise disk fills with the failures |
Layer 6 — the untrusted-parser problem, stated honestly
Everything above constrains what a caller may ask for. None of it helps with the fact that ffmpeg and libvips are large C/C++ codebases parsing hostile binary input, which is a memory-safety attack surface with a long CVE history — ImageTragick (CVE-2016-3714) and exiftool's DjVu RCE (CVE-2021-22204) are the famous ones, and they are not the last.
The honest mitigation is process isolation, and it is a deployment property rather than a code one:
- run the service as a dedicated unprivileged user that owns nothing else
- no network egress from the process (with protocols disabled it has no reason to make one)
- read-only filesystem apart from the job directory
- ideally a container or
bubblewrap/nsjailsandbox per job
Status: not implemented yet. It is written down here because it is the difference between "a bug in ffmpeg is a crash" and "a bug in ffmpeg is our whole machine", and because the thing most likely to happen otherwise is that it is never considered at all.
exiftool is treated differently
exiftool is a Perl program with a plugin architecture and a history of code execution from parsing — not from its arguments. It is therefore used narrowly:
- reading only, with a fixed argument list, never caller-supplied options
- never
-@(argfile), never-config, never a user-supplied-ifexpression - stripping metadata is done by sharp's re-encode, not by exiftool
Sharp drops EXIF by default on re-encode, which is verified on every scripts/verify.sh run. That
is a smaller, better-understood surface than asking exiftool to rewrite a file in place.
Authentication
The service is not public and has no anonymous surface. Every endpoint requires a bearer token issued out of band. This is deliberately dull — it is an internal service between two things we operate, not a product.
Even so, it must not be reachable from the internet. Auth is the second lock; the first is not listening on a public interface.
Reviewing a change against this document
- Does the caller name a path, a URL, or a protocol anywhere in the new surface? → no
- Is a new ffmpeg filter classified in the allowlist, with a reason? → required
- Is the new option's value constrained by type and range, or is it a free string? → constrain it
- Can it produce unbounded work — time, memory, pixels, files? → bound it
- Is a command being built as a string? → it must not be