mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(cli): reject incomplete hosted video downloads (#117893)
* fix(cli): validate streamed video downloads * test: repair CLI and plugin test gates
This commit is contained in:
committed by
GitHub
parent
d5f265f245
commit
83fa625fb6
@@ -240,6 +240,8 @@ openclaw infer video describe --file ./clip.mp4 --model openai/gpt-5.4-mini --js
|
||||
Notes:
|
||||
|
||||
- `video generate` accepts `--size`, `--aspect-ratio`, `--resolution`, `--duration`, `--audio`, `--watermark`, and `--timeout-ms`, forwarded to the video-generation runtime.
|
||||
- Provider-hosted video downloads reject empty, text, and JSON responses instead of reporting an unusable file as successful output.
|
||||
- With `--output`, URL-backed video streams to a sibling temporary file and replaces the destination only after the complete non-empty download succeeds; a failed stream leaves an existing destination unchanged.
|
||||
- `--model` must be `<provider/model>` for `video describe`.
|
||||
|
||||
## Web
|
||||
|
||||
+135
-31
@@ -5,12 +5,14 @@ import path from "node:path";
|
||||
import { expectDefined } from "@openclaw/normalization-core";
|
||||
import { Command } from "commander";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js";
|
||||
import type { inspectLocalAudioSelection } from "../media-understanding/local-audio.js";
|
||||
import { runRegisteredCli } from "../test-utils/command-runner.js";
|
||||
import { CAPABILITY_METADATA, registerCapabilityCli } from "./capability-cli.js";
|
||||
|
||||
const PNG_1X1_BASE64 =
|
||||
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+yf7kAAAAASUVORK5CYII=";
|
||||
const tempDirs = useAutoCleanupTempDirTracker(afterEach);
|
||||
|
||||
function runCap(...argv: string[]): Promise<void> {
|
||||
return runRegisteredCli({ register: registerCapabilityCli as (program: Command) => void, argv });
|
||||
@@ -1943,33 +1945,87 @@ describe("capability cli", () => {
|
||||
);
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
|
||||
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-video-generate-"));
|
||||
const tempDir = tempDirs.make("openclaw-video-generate-");
|
||||
const outputBase = path.join(tempDir, "result");
|
||||
|
||||
await runCapability(
|
||||
"video",
|
||||
"generate",
|
||||
"--prompt",
|
||||
"friendly lobster",
|
||||
"--output",
|
||||
outputBase,
|
||||
"--json",
|
||||
);
|
||||
|
||||
const outputPath = `${outputBase}.mp4`;
|
||||
const fetchCalls = fetchMock.mock.calls as unknown as Array<[string, { signal?: unknown }]>;
|
||||
const fetchCall = fetchCalls[0];
|
||||
expect(fetchCall?.[0]).toBe("https://example.com/generated-video.mp4");
|
||||
expect(fetchCall?.[1]?.signal).toBeInstanceOf(AbortSignal);
|
||||
expect(await fs.readFile(outputPath, "utf8")).toBe("video-bytes");
|
||||
const output = firstJsonOutput();
|
||||
const outputs = output?.outputs as Array<Record<string, unknown>>;
|
||||
expect(output?.capability).toBe("video.generate");
|
||||
expect(output?.provider).toBe("vydra");
|
||||
expect(outputs).toHaveLength(1);
|
||||
expect(outputs[0]?.path).toBe(outputPath);
|
||||
expect(outputs[0]?.mimeType).toBe("video/mp4");
|
||||
expect(outputs[0]?.size).toBe(11);
|
||||
await fs.writeFile(outputPath, "previous-video");
|
||||
await fs.chmod(outputPath, 0o640);
|
||||
|
||||
try {
|
||||
await runCapability(
|
||||
"video",
|
||||
"generate",
|
||||
"--prompt",
|
||||
"friendly lobster",
|
||||
"--output",
|
||||
outputBase,
|
||||
"--json",
|
||||
);
|
||||
|
||||
const fetchCalls = fetchMock.mock.calls as unknown as Array<[string, { signal?: unknown }]>;
|
||||
const fetchCall = fetchCalls[0];
|
||||
expect(fetchCall?.[0]).toBe("https://example.com/generated-video.mp4");
|
||||
expect(fetchCall?.[1]?.signal).toBeInstanceOf(AbortSignal);
|
||||
expect(await fs.readFile(outputPath, "utf8")).toBe("video-bytes");
|
||||
if (process.platform !== "win32") {
|
||||
expect((await fs.stat(outputPath)).mode & 0o777).toBe(0o640);
|
||||
}
|
||||
expect(await fs.readdir(tempDir)).toEqual(["result.mp4"]);
|
||||
const output = firstJsonOutput();
|
||||
const outputs = output?.outputs as Array<Record<string, unknown>>;
|
||||
expect(output?.capability).toBe("video.generate");
|
||||
expect(output?.provider).toBe("vydra");
|
||||
expect(outputs).toHaveLength(1);
|
||||
expect(outputs[0]?.path).toBe(outputPath);
|
||||
expect(outputs[0]?.mimeType).toBe("video/mp4");
|
||||
expect(outputs[0]?.size).toBe(11);
|
||||
} finally {
|
||||
await fs.rm(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves an existing --output and removes its temp when a video stream fails", async () => {
|
||||
primeGeneratedVideoUrl("https://example.com/broken-video.mp4");
|
||||
const stream = new ReadableStream<Uint8Array>({
|
||||
start(controller) {
|
||||
controller.enqueue(new TextEncoder().encode("partial-video"));
|
||||
controller.error(new Error("video stream exploded"));
|
||||
},
|
||||
});
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(
|
||||
async () =>
|
||||
new Response(stream, {
|
||||
status: 200,
|
||||
headers: { "content-type": "video/mp4" },
|
||||
}),
|
||||
),
|
||||
);
|
||||
const tempDir = tempDirs.make("openclaw-video-stream-fail-");
|
||||
const outputBase = path.join(tempDir, "result");
|
||||
const outputPath = `${outputBase}.mp4`;
|
||||
await fs.writeFile(outputPath, "keep-existing-video");
|
||||
|
||||
try {
|
||||
await expect(
|
||||
runCapability(
|
||||
"video",
|
||||
"generate",
|
||||
"--prompt",
|
||||
"friendly lobster",
|
||||
"--output",
|
||||
outputBase,
|
||||
"--json",
|
||||
),
|
||||
).rejects.toThrow("exit 1");
|
||||
|
||||
expectRuntimeErrorContains("video stream exploded");
|
||||
expect(await fs.readFile(outputPath, "utf8")).toBe("keep-existing-video");
|
||||
expect(await fs.readdir(tempDir)).toEqual(["result.mp4"]);
|
||||
} finally {
|
||||
await fs.rm(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("blocks private-network url-only generated video downloads by default", async () => {
|
||||
@@ -2224,8 +2280,10 @@ describe("capability cli", () => {
|
||||
expect(enqueued).toBeLessThanOrEqual(4);
|
||||
});
|
||||
|
||||
it("buffers an empty-body url-only generated video without error", async () => {
|
||||
// Boundary: a 0-byte body is trivially under the cap and must not error.
|
||||
it.each([
|
||||
{ mode: "buffered", withOutput: false },
|
||||
{ mode: "streamed", withOutput: true },
|
||||
])("rejects an empty-body url-only generated video in $mode mode", async ({ withOutput }) => {
|
||||
mocks.loadConfig.mockReturnValue({});
|
||||
primeGeneratedVideoUrl("https://example.com/empty-video.mp4");
|
||||
const fetchMock = vi.fn(
|
||||
@@ -2236,12 +2294,58 @@ describe("capability cli", () => {
|
||||
}),
|
||||
);
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
const tempDir = withOutput ? tempDirs.make("openclaw-empty-video-") : undefined;
|
||||
const outputBase = tempDir ? path.join(tempDir, "result") : undefined;
|
||||
const outputPath = outputBase ? `${outputBase}.mp4` : undefined;
|
||||
if (outputPath) {
|
||||
await fs.writeFile(outputPath, "keep-existing-video");
|
||||
}
|
||||
|
||||
await runCapability("video", "generate", "--prompt", "friendly lobster", "--json");
|
||||
try {
|
||||
await expect(
|
||||
runCapability(
|
||||
"video",
|
||||
"generate",
|
||||
"--prompt",
|
||||
"friendly lobster",
|
||||
...(outputBase ? ["--output", outputBase] : []),
|
||||
"--json",
|
||||
),
|
||||
).rejects.toThrow("exit 1");
|
||||
|
||||
const output = firstJsonOutput();
|
||||
expect(output?.capability).toBe("video.generate");
|
||||
expect(runtimeErrorMessages().join("\n")).not.toContain("exceeds");
|
||||
expectRuntimeErrorContains("Generated media output is empty");
|
||||
expect(mocks.runtime.writeJson).not.toHaveBeenCalled();
|
||||
if (tempDir && outputPath) {
|
||||
expect(await fs.readFile(outputPath, "utf8")).toBe("keep-existing-video");
|
||||
expect(await fs.readdir(tempDir)).toEqual(["result.mp4"]);
|
||||
}
|
||||
} finally {
|
||||
if (tempDir) {
|
||||
await fs.rm(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects successful textual responses from generated video URLs", async () => {
|
||||
mocks.loadConfig.mockReturnValue({});
|
||||
primeGeneratedVideoUrl("https://example.com/not-a-video.mp4");
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(
|
||||
async () =>
|
||||
new Response("render still processing", {
|
||||
status: 200,
|
||||
headers: { "content-type": "text/plain" },
|
||||
}),
|
||||
),
|
||||
);
|
||||
|
||||
await expect(
|
||||
runCapability("video", "generate", "--prompt", "friendly lobster", "--json"),
|
||||
).rejects.toThrow("exit 1");
|
||||
|
||||
expectRuntimeErrorContains("vydra generated video download: malformed video response");
|
||||
expect(mocks.runtime.writeJson).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejects partial image generate count before provider dispatch", async () => {
|
||||
|
||||
@@ -6,11 +6,15 @@ import { pipeline } from "node:stream/promises";
|
||||
import { extensionForMime, normalizeMimeType } from "@openclaw/media-core/mime";
|
||||
import type { Command } from "commander";
|
||||
import { resolveAgentDir, resolveDefaultAgentId } from "../../agents/agent-scope.js";
|
||||
import { assertOkOrThrowHttpError } from "../../agents/provider-http-errors.js";
|
||||
import {
|
||||
assertOkOrThrowHttpError,
|
||||
assertProviderBinaryResponseContent,
|
||||
} from "../../agents/provider-http-errors.js";
|
||||
import { getRuntimeConfig } from "../../config/config.js";
|
||||
import { resolveAgentModelPrimaryValue } from "../../config/model-input.js";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
import { readResponseWithLimit } from "../../infra/http-body.js";
|
||||
import { writeSiblingTempFile } from "../../infra/sibling-temp-file.js";
|
||||
import { buildMediaUnderstandingRegistry } from "../../media-understanding/provider-registry.js";
|
||||
import { describeVideoFile } from "../../media-understanding/runtime.js";
|
||||
import { resolveGeneratedMediaMaxBytes } from "../../media/configured-max-bytes.js";
|
||||
@@ -41,6 +45,18 @@ import {
|
||||
} from "./shared.js";
|
||||
|
||||
const GENERATED_VIDEO_DOWNLOAD_TIMEOUT_MS = 120_000;
|
||||
const GENERATED_VIDEO_OUTPUT_TEMP_PREFIX = ".openclaw-video-output";
|
||||
|
||||
async function resolveExistingVideoOutputMode(filePath: string): Promise<number | undefined> {
|
||||
try {
|
||||
return (await fs.stat(filePath)).mode & 0o7777;
|
||||
} catch (error) {
|
||||
if ((error as NodeJS.ErrnoException).code === "ENOENT") {
|
||||
return undefined;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
function normalizeVideoResolution(raw: string | undefined): VideoGenerationResolution | undefined {
|
||||
const normalized = raw?.trim().toUpperCase();
|
||||
@@ -90,6 +106,11 @@ async function fetchGeneratedVideoDownload(params: {
|
||||
result.response,
|
||||
`${params.provider} generated video download failed`,
|
||||
);
|
||||
assertProviderBinaryResponseContent(
|
||||
result.response,
|
||||
`${params.provider} generated video download`,
|
||||
"video",
|
||||
);
|
||||
return result;
|
||||
} catch (error) {
|
||||
await result.release();
|
||||
@@ -148,20 +169,37 @@ async function runVideoGenerate(params: {
|
||||
const ext =
|
||||
extensionForMime(mimeType) ||
|
||||
path.extname(video.fileName ?? "") ||
|
||||
path.extname(params.output ?? "");
|
||||
path.extname(params.output);
|
||||
const resolvedOutput = path.resolve(params.output);
|
||||
const parsed = path.parse(resolvedOutput);
|
||||
const filePath =
|
||||
result.videos.length <= 1
|
||||
? path.join(parsed.dir, `${parsed.name}${ext}`)
|
||||
: path.join(parsed.dir, `${parsed.name}-${String(index + 1)}${ext}`);
|
||||
await fs.mkdir(path.dirname(filePath), { recursive: true });
|
||||
await pipeline(
|
||||
Readable.fromWeb(response.body as import("node:stream/web").ReadableStream),
|
||||
createWriteStream(filePath),
|
||||
);
|
||||
const stat = await fs.stat(filePath);
|
||||
return { path: filePath, mimeType: video.mimeType, size: stat.size };
|
||||
const dir = path.dirname(filePath);
|
||||
await fs.mkdir(dir, { recursive: true });
|
||||
const mode = await resolveExistingVideoOutputMode(filePath);
|
||||
const { result: size } = await writeSiblingTempFile({
|
||||
dir,
|
||||
chmodDir: false,
|
||||
tempPrefix: GENERATED_VIDEO_OUTPUT_TEMP_PREFIX,
|
||||
...(mode === undefined ? {} : { mode }),
|
||||
writeTemp: async (tempPath) => {
|
||||
await pipeline(
|
||||
Readable.fromWeb(
|
||||
response.body as import("node:stream/web").ReadableStream<Uint8Array>,
|
||||
),
|
||||
createWriteStream(tempPath, { flags: "wx" }),
|
||||
);
|
||||
const writtenSize = (await fs.stat(tempPath)).size;
|
||||
if (writtenSize === 0) {
|
||||
throw new Error("Generated media output is empty.");
|
||||
}
|
||||
return writtenSize;
|
||||
},
|
||||
resolveFinalPath: () => filePath,
|
||||
});
|
||||
return { path: filePath, mimeType: video.mimeType, size };
|
||||
}
|
||||
// Provider-supplied video URLs are untrusted external sources, and the
|
||||
// in-memory fallback (no --output) must not buffer an unbounded body:
|
||||
@@ -181,6 +219,9 @@ async function runVideoGenerate(params: {
|
||||
`${result.provider} generated video download exceeds ${maxBytes} bytes; pass --output to stream large videos to disk`,
|
||||
),
|
||||
});
|
||||
if (videoBuffer.byteLength === 0) {
|
||||
throw new Error("Generated media output is empty.");
|
||||
}
|
||||
} finally {
|
||||
await download.release();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user