fix(studio-server): background removal writes only where it checked, even through planted links (#4540)

* fix(studio-server): background removal writes only where it checked, even through planted links

* test(cli): read the cutout through a no-follow handle instead of checking then reading
This commit is contained in:
Miguel Ángel
2026-09-28 02:07:30 +00:00
committed by GitHub
parent 918e39e785
commit 03e3755aaa
4 changed files with 197 additions and 20 deletions
@@ -14,7 +14,9 @@
* decode the alpha plane.
*/
import { spawn } from "node:child_process";
import { extname } from "node:path";
import { randomUUID } from "node:crypto";
import { renameSync, rmSync } from "node:fs";
import { basename, dirname, extname, join } from "node:path";
import { findFFmpeg, findFFprobe, getFFmpegInstallHint } from "../browser/ffmpeg.js";
import { createSession, type Session } from "./inference.js";
import { type Device, type ModelId } from "./manager.js";
@@ -296,16 +298,20 @@ export async function render(options: RenderOptions): Promise<RenderResult> {
onProgress: (msg) => options.onProgress?.({ kind: "info", message: msg }),
});
const output = tempBeside(options.outputPath);
const background = options.backgroundOutputPath && tempBeside(options.backgroundOutputPath);
try {
const start = Date.now();
const framesProcessed = await runPipeline(
options,
{ ...options, outputPath: output, backgroundOutputPath: background },
session,
media,
format,
bgFormat,
ffmpegPath,
);
renameSync(output, options.outputPath);
if (background) renameSync(background, options.backgroundOutputPath!);
const durationSeconds = (Date.now() - start) / 1000;
const avgMsPerFrame = framesProcessed ? (durationSeconds * 1000) / framesProcessed : 0;
@@ -320,9 +326,16 @@ export async function render(options: RenderOptions): Promise<RenderResult> {
};
} finally {
await session.close();
rmSync(output, { force: true, maxRetries: 3 });
if (background) rmSync(background, { force: true, maxRetries: 3 });
}
}
/** ffmpeg follows a link planted at its output mid-job; a fresh unguessable name renamed over the output cannot. */
function tempBeside(path: string): string {
return join(dirname(path), `.tmp-${randomUUID()}-${basename(path)}`);
}
const RECENT_WINDOW = 30;
interface FfmpegProc {
@@ -1,4 +1,16 @@
import { EventEmitter } from "node:events";
import {
closeSync,
constants,
mkdtempSync,
openSync,
readFileSync,
rmSync,
symlinkSync,
writeFileSync,
} from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { Readable } from "node:stream";
import { describe, expect, it, vi } from "vitest";
@@ -29,7 +41,7 @@ vi.mock("@hyperframes/engine", () => ({
import { render } from "./pipeline.js";
function fakeFfmpeg(stdout: Readable) {
function fakeFfmpeg(stdout: Readable, output?: string) {
const proc = new EventEmitter() as EventEmitter & {
stdout: Readable;
stderr: EventEmitter;
@@ -40,17 +52,20 @@ function fakeFfmpeg(stdout: Readable) {
proc.stderr = new EventEmitter();
proc.stdin = Object.assign(new EventEmitter(), { write: vi.fn(() => true), end: vi.fn() });
proc.kill = vi.fn();
if (output) writeFileSync(output, "rendered");
queueMicrotask(() => proc.emit("exit", 0, null));
return proc;
}
describe("background-removal FFmpeg child-process options", () => {
it("hides every FFmpeg console window", async () => {
const dir = mkdtempSync(join(tmpdir(), "hf-bg-removal-"));
spawnMock
.mockImplementationOnce(() => fakeFfmpeg(Readable.from([Buffer.alloc(3)])))
.mockImplementationOnce(() => fakeFfmpeg(Readable.from([])));
.mockImplementationOnce((_, args: string[]) => fakeFfmpeg(Readable.from([]), args.at(-1)));
await render({ inputPath: "/tmp/input.mp4", outputPath: "/tmp/output.webm" });
await render({ inputPath: "/tmp/input.mp4", outputPath: join(dir, "output.webm") });
rmSync(dir, { recursive: true, force: true });
expect(spawnMock).toHaveBeenCalledTimes(2);
for (const call of spawnMock.mock.calls) {
@@ -58,3 +73,33 @@ describe("background-removal FFmpeg child-process options", () => {
}
});
});
describe("background-removal output", () => {
it.skipIf(process.platform === "win32")(
"lands at the output path even when a link is planted there mid-job, leaving its target alone",
async () => {
const dir = mkdtempSync(join(tmpdir(), "hf-bg-removal-"));
const outside = join(dir, "outside.webm");
const output = join(dir, "cutout.webm");
writeFileSync(outside, "keep");
spawnMock
.mockImplementationOnce(() => fakeFfmpeg(Readable.from([Buffer.alloc(3)])))
.mockImplementationOnce((_, args: string[]) => {
symlinkSync(outside, output);
return fakeFfmpeg(Readable.from([]), args.at(-1));
});
try {
await render({ inputPath: "/tmp/input.mp4", outputPath: output });
expect(readFileSync(outside, "utf-8")).toBe("keep");
const fd = openSync(output, constants.O_RDONLY | constants.O_NOFOLLOW);
try {
expect(readFileSync(fd, "utf-8")).toBe("rendered");
} finally {
closeSync(fd);
}
} finally {
rmSync(dir, { recursive: true, force: true });
}
},
);
});
@@ -1,7 +1,15 @@
// fallow-ignore-file code-duplication
import { afterEach, describe, expect, it, vi } from "vitest";
import { Hono } from "hono";
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
import {
mkdirSync,
mkdtempSync,
readdirSync,
readFileSync,
rmSync,
symlinkSync,
writeFileSync,
} from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { registerMediaRoutes } from "./media";
@@ -117,6 +125,34 @@ describe("registerMediaRoutes", () => {
expect(response.status).toBe(501);
});
// Windows needs a privilege to create symlinks.
it.skipIf(process.platform === "win32")(
"writes background-removal output to the folder it checked, though a link on its path is retargeted",
async () => {
const outside = mkdtempSync(join(tmpdir(), "hf-media-outside-"));
tempProjectDirs.push(outside);
let dir = "";
const { app, projectDir } = createAdapter((opts) => {
rmSync(join(dir, "cutouts"));
symlinkSync(outside, join(dir, "cutouts"), "dir");
writeFileSync(opts.outputPath, "cutout");
return completeJob(opts);
});
dir = projectDir;
mkdirSync(join(projectDir, "real"));
symlinkSync(join(projectDir, "real"), join(projectDir, "cutouts"), "dir");
await app.request("http://localhost/projects/demo/media/remove-background", {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify({ inputPath: "assets/photo.jpg", outputPath: "cutouts/photo.png" }),
});
expect(readdirSync(outside)).toEqual([]);
expect(readFileSync(join(projectDir, "real", "photo.png"), "utf-8")).toBe("cutout");
},
);
it("rejects remote input paths", async () => {
const { app, startBackgroundRemoval } = createAdapter(completeJob);
@@ -185,6 +221,62 @@ describe("registerMediaRoutes", () => {
);
});
it("gives a second removal of the same clip its own output names while the first still runs", async () => {
const { app } = createAdapter((opts) => ({
...completeJob(opts),
status: "processing" as const,
backgroundOutputAssetPath: opts.backgroundOutputAssetPath,
}));
const start = async () =>
(await (
await app.request("http://localhost/projects/demo/media/remove-background", {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify({ inputPath: "assets/clip.mp4", createBackgroundPlate: true }),
})
).json()) as { outputPath: string; backgroundOutputPath: string };
const [first, second] = [await start(), await start()];
expect([first.outputPath, second.outputPath]).toEqual([
"assets/cutouts/clip-cutout.webm",
"assets/cutouts/clip-cutout-2.webm",
]);
expect(second.backgroundOutputPath).toBe("assets/cutouts/clip-plate-2.webm");
});
it.each([
[
"an explicit output spelled another way",
{ outputPath: "assets//cutouts/clip-cutout.webm" },
"clip-cutout-2.webm",
],
[
"a plate asked for under the cutout's own name",
{ outputPath: "assets/cutouts/clip-plate.webm" },
"clip-plate-2.webm",
],
])("never gives two outputs one name: %s", async (_, extra, plate) => {
const { app } = createAdapter((opts) => ({
...completeJob(opts),
status: "processing" as const,
backgroundOutputAssetPath: opts.backgroundOutputAssetPath,
}));
const start = async (body: object) =>
(await (
await app.request("http://localhost/projects/demo/media/remove-background", {
method: "POST",
headers: { "content-type": "application/json" },
body: JSON.stringify({ inputPath: "assets/clip.mp4", ...body }),
})
).json()) as { outputPath: string; backgroundOutputPath?: string };
const first = await start({});
const second = await start({ ...extra, createBackgroundPlate: true });
const names = [first.outputPath, second.outputPath, second.backgroundOutputPath];
expect(new Set(names).size).toBe(3);
expect(names.join(" ")).toContain(plate);
});
it("normalizes query strings from local media paths", async () => {
const { app, startBackgroundRemoval } = createAdapter(completeJob);
+41 -14
View File
@@ -1,9 +1,9 @@
import type { Hono } from "hono";
import { streamSSE } from "hono/streaming";
import { existsSync, mkdirSync } from "node:fs";
import { basename, dirname, extname, join } from "node:path";
import { basename, dirname, extname, join, posix } from "node:path";
import type { MediaProcessingJobState, StudioApiAdapter } from "../types.js";
import { resolveWithinProject } from "../helpers/safePath.js";
import { pinWithinProject, resolveWithinProject } from "../helpers/safePath.js";
import { probeMediaMetadata } from "../helpers/mediaMetadata.js";
const VIDEO_EXTENSIONS = new Set([
@@ -63,23 +63,46 @@ function slugFileBase(path: string): string {
return name || "media";
}
function uniqueAssetPath(projectDir: string, assetPath: string): string {
function uniqueAssetPath(
projectDir: string,
assetPath: string,
taken: ReadonlySet<string>,
): string {
const ext = extname(assetPath);
const withoutExt = assetPath.slice(0, -ext.length);
let candidate = assetPath;
for (let index = 2; existsSync(join(projectDir, candidate)); index++) {
for (let index = 2; taken.has(candidate) || existsSync(join(projectDir, candidate)); index++) {
candidate = `${withoutExt}-${index}${ext}`;
}
return candidate;
}
function defaultOutputPath(projectDir: string, inputPath: string): string {
function defaultOutputPath(
projectDir: string,
inputPath: string,
taken: ReadonlySet<string>,
): string {
const ext = isImagePath(inputPath) ? ".png" : ".webm";
return uniqueAssetPath(projectDir, `assets/cutouts/${slugFileBase(inputPath)}-cutout${ext}`);
return uniqueAssetPath(
projectDir,
`assets/cutouts/${slugFileBase(inputPath)}-cutout${ext}`,
taken,
);
}
function defaultPlatePath(projectDir: string, inputPath: string): string {
return uniqueAssetPath(projectDir, `assets/cutouts/${slugFileBase(inputPath)}-plate.webm`);
function defaultPlatePath(
projectDir: string,
inputPath: string,
taken: ReadonlySet<string>,
): string {
return uniqueAssetPath(projectDir, `assets/cutouts/${slugFileBase(inputPath)}-plate.webm`, taken);
}
function outputsInFlight(mediaJobs: Map<string, JobWithCreatedAt>): Set<string> {
const running = [...mediaJobs.values()].filter((job) => job.status === "processing");
return new Set(
running.flatMap((job) => [job.outputAssetPath, job.backgroundOutputAssetPath ?? ""]),
);
}
function makeJobId(projectId: string, mediaJobs: Map<string, JobWithCreatedAt>): string {
@@ -171,17 +194,20 @@ export function registerMediaRoutes(
return c.json({ error: "background removal supports video or image assets only" }, 400);
}
const requestedOutput = body.outputPath ? normalizeProjectAssetPath(body.outputPath) : "";
const requestedOutput = body.outputPath
? posix.normalize(normalizeProjectAssetPath(body.outputPath))
: "";
if (requestedOutput && containsNullByte(requestedOutput)) {
return c.json({ error: "forbidden" }, 403);
}
if (requestedOutput && !resolveWithinProject(project.dir, requestedOutput)) {
return c.json({ error: "forbidden" }, 403);
}
const taken = outputsInFlight(mediaJobs);
const outputAssetPath = requestedOutput
? uniqueAssetPath(project.dir, requestedOutput)
: defaultOutputPath(project.dir, inputAssetPath);
const outputPath = resolveWithinProject(project.dir, outputAssetPath);
? uniqueAssetPath(project.dir, requestedOutput, taken)
: defaultOutputPath(project.dir, inputAssetPath, taken);
const outputPath = pinWithinProject(project.dir, outputAssetPath);
if (!outputPath) return c.json({ error: "forbidden" }, 403);
if (inputIsVideo && !VIDEO_OUTPUT_EXTENSIONS.has(extname(outputAssetPath).toLowerCase())) {
return c.json({ error: "video background removal output must be .webm or .mov" }, 400);
@@ -196,9 +222,10 @@ export function registerMediaRoutes(
if (!inputIsVideo) {
return c.json({ error: "background plates are only supported for video inputs" }, 400);
}
backgroundOutputAssetPath = defaultPlatePath(project.dir, inputAssetPath);
taken.add(outputAssetPath);
backgroundOutputAssetPath = defaultPlatePath(project.dir, inputAssetPath, taken);
backgroundOutputPath =
resolveWithinProject(project.dir, backgroundOutputAssetPath) ?? undefined;
pinWithinProject(project.dir, backgroundOutputAssetPath) ?? undefined;
if (!backgroundOutputPath) {
return c.json({ error: "forbidden" }, 403);
}