ffmpeg can work on a file where it already is

adds a path mode to /v1/ffmpeg:

  { input: "as_…" }                      → stored as a new asset, 201
  { inputPath: "/…", outputPath: "/…" }  → written where you said, 200 { path, bytes }

THIS REVISES THE INVARIANT IN docs/SECURITY.md, deliberately, and that document now
says so at the top rather than being quietly contradicted by the code.

it used to read "the caller never supplies a path, for input or output". that was
right for an upload service and av-club is not one: both of its real consumers —
a right-click on ~/Movies/thing.mkv, and a commander watching an upload folder —
already have the file, and obeying the rule meant copying gigabytes in and back out
to convert something that never moved. a rule that expensive gets worked around,
and the workaround is where the hole would have been.

so it is narrower now and says what it was always for. argv never a shell,
-protocol_whitelist file, filters allowlisted, work bounded — all unchanged. what
is gone is only the claim that a path never appears, replaced by one small module
every path goes through:

  absolute (a relative path resolves against a cwd the caller cannot see)
  realpath FIRST, then check — checking the path as written and opening the
    resolved one is the classic way this fails
  inside as `root` or `root + sep`, never a bare startsWith, which accepts
    /home/user-backup for a root of /home/user
  for an output the PARENT is resolved, and must already exist
  errors never say which of "outside" or "missing" it was, or this becomes an
    oracle for what exists elsewhere on the disk

VERIFIED against the running service, not reasoned about: `..` traversal, an
absolute path outside the root, a write outside the root, a relative path, a
symlink pointing into /etc, and a write THROUGH a symlinked directory — all
refused, nothing written outside the root.

the output is moved into place at the END. ffmpeg encodes into a temp dir, so a job
killed by the timeout or a restart leaves the caller's directory as it was. pointing
ffmpeg at the destination would leave a truncated file where a good one might have
been — and a task that then deleted its source would have destroyed something.
rename first, copy on EXDEV, which is the normal case rather than the error one
since tmp is often tmpfs.

docs/API.md said this endpoint was "designed, not yet built" while it was routed and
working. that is fixed too: it was read as a gap and nearly rebuilt.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-29 02:07:02 +01:00
co-authored by Claude Opus 5
parent e2ed76eac4
commit bd8b7b9ad4
6 changed files with 219 additions and 12 deletions
+39 -1
View File
@@ -6,7 +6,15 @@ 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 one invariant, as revised on 2026-08-29
> **The caller may name a path. `sidecar/paths.ts` is the ONLY thing that decides whether it is one
> this service will touch, and it confines every one of them to `WORK_ROOT`.**
Read the section below for what it replaced and why, because the change is not a relaxation of the
threat model — it is a correction to a rule that had stopped describing what av-club is for.
### What it used to say, and why it changed
> **The caller never supplies a path, a URL, or a protocol. Not for input, not for output, not
> inside a filter argument, not anywhere.**
@@ -19,6 +27,36 @@ Concretely: input is an **opaque asset id** issued by av-club for a file av-club
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.
That was the right invariant for an upload service. av-club is not one. Both of its real consumers
already have the file on disk — a right-click in the file browser on `~/Movies/thing.mkv`, and a
commander watching a folder for uploads — and obeying the old rule meant copying gigabytes in and
gigabytes back out to convert a file that never moved. A rule that expensive gets worked around, and
the workaround is where the actual hole would have been.
So it is now narrower and says what it was always FOR. Nothing else moved:
- argv arrays, never a shell — unchanged
- `-protocol_whitelist file` — unchanged, and now doing more work than before
- filters allowlisted by name, file-bearing parameters stripped — unchanged
- bounded work, timeouts, concurrency — unchanged
What is gone is only the claim that a path never appears. What replaced it is one module, small enough
to read in a minute, that every path goes through:
- **absolute**, because a relative path resolves against a cwd the caller cannot see
- **`realpath` FIRST**, so a symlink inside the root pointing at `/etc/shadow` is resolved and then
rejected — checking the path as written and opening the resolved one is the classic way this fails
- **inside the root as `root` or `root + sep`**, never a bare `startsWith`, which accepts
`/home/user-backup` for a root of `/home/user`
- for an output, the **parent** is resolved and checked, and it must already exist — creating
directories toward an output is a side effect nobody asked for
- errors never distinguish "outside the root" from "does not exist", which would make this an oracle
for what exists elsewhere on the disk
Verified 2026-08-29 against the running service: `..` traversal, an absolute path outside the root, a
write outside the root, a relative path, a symlink pointing into `/etc`, and a write through a
symlinked directory are all refused; nothing was written outside the root.
### What that invariant is defending against
These all work by default, which is why "just pass the flags through" is not an option: