pastilhasandClaude Opus 5
bd8b7b9ad4
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>
76 lines
3.9 KiB
TypeScript
76 lines
3.9 KiB
TypeScript
import { realpath } from "node:fs/promises";
|
|
import { isAbsolute, resolve, sep } from "node:path";
|
|
import { WORK_ROOT } from "./config";
|
|
import { invalid } from "./errors";
|
|
|
|
/**
|
|
* Turning a caller-supplied path into one this service will touch.
|
|
*
|
|
* ── This is a deliberate change to the invariant in docs/SECURITY.md ──
|
|
*
|
|
* That document said the caller never supplies a path, for input or output, and everything else was
|
|
* built on it. It was the right invariant for an upload service and the wrong one for what av-club is
|
|
* actually used by: a right-click on a file that is already on disk, and a commander watching a folder
|
|
* of them. Both had the file already; both would have had to copy gigabytes in and gigabytes back out
|
|
* to obey a rule whose purpose was to stop ffmpeg reading things it should not.
|
|
*
|
|
* So the rule is now narrower and says what it was always for: **the caller may name a path, and this
|
|
* function is the only thing that decides whether it is one we will touch.** Every other layer is
|
|
* unchanged — argv never a shell, `-protocol_whitelist file`, filters allowlisted by name, work
|
|
* bounded. What is gone is only the claim that a path never appears.
|
|
*
|
|
* ── What it checks, and why each one ──
|
|
*
|
|
* ABSOLUTE. A relative path resolves against this process's cwd, which is not the caller's and is not
|
|
* anything they can see. There is no useful meaning for it, so it is refused rather than guessed at.
|
|
*
|
|
* REAL. `realpath` follows every symlink before the check, so a link 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 goes wrong.
|
|
*
|
|
* INSIDE THE ROOT, tested as `root` or `root + sep`. A bare `startsWith` accepts `/home/user-backup`
|
|
* for a root of `/home/user`, which is a different person's files — the same bug the platform's own
|
|
* file browser documents in `isInside`.
|
|
*/
|
|
|
|
/** A path this service is willing to read from or write to. Branded so it cannot be confused with a string. */
|
|
export type SafePath = string & { readonly __safe: unique symbol };
|
|
|
|
const inside = (root: string, target: string) => target === root || target.startsWith(root + sep);
|
|
|
|
/**
|
|
* For an INPUT: the file must exist, so its real path can be resolved and checked.
|
|
*
|
|
* Errors deliberately do not distinguish "outside the root" from "does not exist". Telling a caller
|
|
* which it was turns this into an oracle for what exists on the filesystem outside their root.
|
|
*/
|
|
export async function safeInputPath(raw: unknown, field: string): Promise<SafePath> {
|
|
if (typeof raw !== "string" || !raw.trim()) throw invalid(`${field} must be a path`, field);
|
|
if (!isAbsolute(raw)) throw invalid(`${field} must be an absolute path`, field);
|
|
|
|
const real = await realpath(resolve(raw)).catch(() => null);
|
|
if (!real || !inside(WORK_ROOT, real)) throw invalid(`${field} is not a readable path`, field);
|
|
|
|
return real as SafePath;
|
|
}
|
|
|
|
/**
|
|
* For an OUTPUT: the file does not exist yet, so its PARENT is what gets resolved and checked.
|
|
*
|
|
* The parent must already exist. Creating directories on the way to an output is a side effect nobody
|
|
* asked for, and a typo would silently scatter empty folders through somebody's home.
|
|
*/
|
|
export async function safeOutputPath(raw: unknown, field: string): Promise<SafePath> {
|
|
if (typeof raw !== "string" || !raw.trim()) throw invalid(`${field} must be a path`, field);
|
|
if (!isAbsolute(raw)) throw invalid(`${field} must be an absolute path`, field);
|
|
|
|
const full = resolve(raw);
|
|
const name = full.slice(full.lastIndexOf(sep) + 1);
|
|
if (!name || name === "." || name === "..") throw invalid(`${field} must name a file`, field);
|
|
|
|
const parent = await realpath(full.slice(0, full.lastIndexOf(sep)) || sep).catch(() => null);
|
|
if (!parent || !inside(WORK_ROOT, parent)) throw invalid(`${field} is not a writable path`, field);
|
|
|
|
return `${parent}${sep}${name}` as SafePath;
|
|
}
|