diff --git a/docs/cli/secrets.md b/docs/cli/secrets.md index c0be847e0a88..852f174e8d7e 100644 --- a/docs/cli/secrets.md +++ b/docs/cli/secrets.md @@ -110,6 +110,7 @@ Notes: - Supports creating new `auth-profiles.json` mappings directly in the picker flow. - Runs preflight resolution before apply. - Generated plans default to scrub options enabled (`scrubEnv`, `scrubAuthProfilesForProviderTargets`, `scrubLegacyAuthJson`). Apply is one-way for scrubbed plaintext values. +- `--plan-out` refuses to create a plan whose UTF-8 serialized form exceeds 16 MiB (16,777,216 bytes), matching the `apply --from` input limit. - Without `--apply`, the CLI still prompts `Apply this plan now?` after preflight. - With `--apply` (and no `--yes`), the CLI prompts an extra irreversible-migration confirmation. - `--json` prints the plan + preflight report, but still requires an interactive TTY. @@ -130,6 +131,8 @@ openclaw secrets apply --from /tmp/openclaw-secrets-plan.json --json `--dry-run` validates preflight without writing files; exec SecretRef checks are skipped by default in dry-run. Write mode rejects plans containing exec SecretRefs/providers unless `--allow-exec`. Use `--allow-exec` to opt in to exec provider checks/execution in either mode. +`--from` must point to a regular file no larger than 16 MiB (16,777,216 bytes). The byte limit applies to the complete serialized file, including whitespace. + What `apply` may update: - `openclaw.json` (SecretRef targets + provider upserts/deletes) diff --git a/docs/docs_map.md b/docs/docs_map.md index 57f0a77294d8..6e4c6fe45237 100644 --- a/docs/docs_map.md +++ b/docs/docs_map.md @@ -3877,6 +3877,7 @@ Do not edit it by hand; run `pnpm docs:map:gen`. - Route: /gateway/secrets-plan-contract - Headings: + - H2: Plan file requirements - H2: Plan file shape - H2: Provider upserts and deletes - H2: Supported target scope diff --git a/docs/gateway/secrets-plan-contract.md b/docs/gateway/secrets-plan-contract.md index 8eaf46d07bfa..1ca3f9827bd5 100644 --- a/docs/gateway/secrets-plan-contract.md +++ b/docs/gateway/secrets-plan-contract.md @@ -9,6 +9,12 @@ title: "Secrets apply plan contract" This page defines the strict contract enforced by `openclaw secrets apply`. If a target does not match these rules, apply fails before mutating any file. +## Plan file requirements + +`openclaw secrets apply --from ` accepts regular files up to 16 MiB (16,777,216 bytes). The limit applies to the complete serialized file, including whitespace. Directories, FIFOs, device files, and files larger than the limit are rejected before JSON parsing or target validation. + +`openclaw secrets configure --plan-out ` enforces the same limit on the UTF-8 serialized output before creating the file. Hand-written plans and external plan generators must also keep the serialized file within this boundary. + ## Plan file shape `openclaw secrets apply --from ` expects a `targets` array of plan targets: diff --git a/src/cli/secrets-cli.test.ts b/src/cli/secrets-cli.test.ts index e82d4ee05a94..9e1491be4967 100644 --- a/src/cli/secrets-cli.test.ts +++ b/src/cli/secrets-cli.test.ts @@ -1,7 +1,9 @@ // Secrets CLI tests cover secret command registration, reads, writes, and redaction. +import { execFile } from "node:child_process"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { promisify } from "node:util"; import { Command } from "commander"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { @@ -11,6 +13,8 @@ import { } from "../test-utils/mock-call-assertions.js"; import { registerSecretsCli } from "./secrets-cli.js"; +const execFileAsync = promisify(execFile); + const mocks = await vi.hoisted(async () => { const { createCliRuntimeMock } = await import("./test-runtime-mock.js"); const runtime = createCliRuntimeMock(vi); @@ -105,6 +109,29 @@ function createConfigureInteractiveResult(options?: { }; } +function createConfigureInteractiveResultWithPlanBytes(bytes: number) { + const configured = createConfigureInteractiveResult({ + targets: [ + { + type: "models.providers.apiKey", + path: "models.providers.openai.apiKey", + pathSegments: ["models", "providers", "openai", "apiKey"], + ref: { + source: "file", + provider: "default", + id: "", + }, + providerId: "openai", + }, + ], + }); + const target = configured.plan.targets[0] as { ref: { id: string } }; + const emptyBytes = Buffer.byteLength(`${JSON.stringify(configured.plan, null, 2)}\n`, "utf8"); + target.ref.id = "x".repeat(bytes - emptyBytes); + expect(Buffer.byteLength(`${JSON.stringify(configured.plan, null, 2)}\n`, "utf8")).toBe(bytes); + return configured; +} + function createSecretsApplyResult(options?: { mode?: "dry-run" | "write"; changed?: boolean; @@ -357,6 +384,120 @@ describe("secrets CLI", () => { }); }); + it("writes generated secrets plan files at the apply limit", async () => { + const planPath = path.join( + os.tmpdir(), + `openclaw-secrets-configure-test-${Date.now()}-${Math.random().toString(16).slice(2)}.json`, + ); + runSecretsConfigureInteractive.mockResolvedValue( + createConfigureInteractiveResultWithPlanBytes(16 * 1024 * 1024), + ); + confirm.mockResolvedValue(false); + + try { + await createProgram().parseAsync(["secrets", "configure", "--plan-out", planPath], { + from: "user", + }); + + expect((await fs.stat(planPath)).size).toBe(16 * 1024 * 1024); + expect(runtimeLogs).toContain(`Plan written to ${planPath}`); + expect(runSecretsApply).not.toHaveBeenCalled(); + } finally { + await fs.rm(planPath, { force: true }); + } + }); + + it("rejects generated secrets plan files that exceed the apply limit", async () => { + const planPath = path.join( + os.tmpdir(), + `openclaw-secrets-configure-test-${Date.now()}-${Math.random().toString(16).slice(2)}.json`, + ); + runSecretsConfigureInteractive.mockResolvedValue( + createConfigureInteractiveResultWithPlanBytes(16 * 1024 * 1024 + 1), + ); + + try { + await expect( + createProgram().parseAsync(["secrets", "configure", "--plan-out", planPath], { + from: "user", + }), + ).rejects.toThrow("__exit__:1"); + + expect(runtimeErrors.at(-1)).toContain("Secrets plan exceeds 16777216 bytes"); + await expect(fs.access(planPath)).rejects.toMatchObject({ code: "ENOENT" }); + expect(confirm).not.toHaveBeenCalled(); + expect(runSecretsApply).not.toHaveBeenCalled(); + } finally { + await fs.rm(planPath, { force: true }); + } + }); + + it("rejects oversized secrets plan files before parsing", async () => { + await withPlanFile(async (planPath) => { + await fs.truncate(planPath, 16 * 1024 * 1024 + 1); + await expect( + createProgram().parseAsync(["secrets", "apply", "--from", planPath, "--dry-run"], { + from: "user", + }), + ).rejects.toThrow("__exit__:1"); + + expect(runSecretsApply).not.toHaveBeenCalled(); + expect(runtimeErrors.at(-1)).toContain("Secrets plan file exceeds 16777216 bytes"); + }); + }); + + it.skipIf(process.platform === "win32")( + "rejects FIFO secrets plan paths without blocking", + async () => { + runSecretsApply.mockResolvedValue(createSecretsApplyResult()); + await withPlanFile(async (planPath) => { + await createProgram().parseAsync(["secrets", "apply", "--from", planPath, "--dry-run"], { + from: "user", + }); + }); + runSecretsApply.mockReset(); + runtimeLogs.length = 0; + runtimeErrors.length = 0; + + const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-secrets-cli-fifo-")); + const fifoPath = path.join(tmpDir, "plan.json"); + await execFileAsync("mkfifo", [fifoPath]); + + let timedOut = false; + let timeout: NodeJS.Timeout | undefined; + const parse = createProgram().parseAsync( + ["secrets", "apply", "--from", fifoPath, "--dry-run"], + { from: "user" }, + ); + + try { + await expect( + Promise.race([ + parse, + new Promise((_, reject) => { + timeout = setTimeout(() => { + timedOut = true; + reject(new Error("Timed out waiting for FIFO plan rejection")); + }, 1_000); + }), + ]), + ).rejects.toThrow("__exit__:1"); + + expect(runSecretsApply).not.toHaveBeenCalled(); + expect(runtimeErrors.at(-1)).toContain("Secrets plan path is not a regular file"); + } finally { + if (timeout) { + clearTimeout(timeout); + } + if (timedOut) { + const releaseWriter = execFileAsync("sh", ["-c", 'printf x > "$1"', "sh", fifoPath]); + await Promise.allSettled([parse, releaseWriter]); + } + await fs.rm(tmpDir, { recursive: true, force: true }); + } + }, + ); + it("forwards --allow-exec to secrets apply dry-run", async () => { await withPlanFile(async (planPath) => { runSecretsApply.mockResolvedValue(createSecretsApplyResult()); diff --git a/src/cli/secrets-cli.ts b/src/cli/secrets-cli.ts index 4ef1d93bd37e..5dd2be11a7bf 100644 --- a/src/cli/secrets-cli.ts +++ b/src/cli/secrets-cli.ts @@ -48,22 +48,50 @@ const secretsApplyLoader = createLazyImportLoader( class SecretsPlanFileNotFoundError extends Error {} +const SECRETS_PLAN_MAX_BYTES = 16 * 1024 * 1024; + +function serializePlanFile(plan: SecretsApplyPlan, pathname: string): string { + const raw = `${JSON.stringify(plan, null, 2)}\n`; + if (Buffer.byteLength(raw, "utf8") > SECRETS_PLAN_MAX_BYTES) { + throw new RangeError( + `Secrets plan exceeds ${SECRETS_PLAN_MAX_BYTES} bytes and cannot be written: ${pathname}`, + ); + } + return raw; +} + async function readPlanFile(pathname: string): Promise { // Apply consumes a generated plan shape, not arbitrary JSON. - const [{ readFileSync }, { isSecretsApplyPlan }] = await Promise.all([ + const [fsModule, { readFileDescriptorBounded }, { isSecretsApplyPlan }] = await Promise.all([ fsModuleLoader.load(), + import("../infra/file-descriptor-read.js"), import("../secrets/plan.js"), ]); - let raw: string; - try { - raw = readFileSync(pathname, "utf8"); - } catch (err) { + const fsConstants = fsModule.constants as typeof fsModule.constants & { O_NONBLOCK?: number }; + // Non-blocking open lets descriptor stat reject special files without a FIFO stalling first. + const openFlags = fsConstants.O_RDONLY | (fsConstants.O_NONBLOCK ?? 0); + const file = await fsModule.promises.open(pathname, openFlags).catch((err: unknown) => { if (hasErrnoCode(err, "ENOENT")) { throw new SecretsPlanFileNotFoundError(`Secrets plan file not found: ${pathname}`, { cause: err, }); } throw err; + }); + let raw: string; + try { + const stat = await file.stat(); + if (!stat.isFile()) { + throw new Error(`Secrets plan path is not a regular file: ${pathname}`); + } + if (stat.size > SECRETS_PLAN_MAX_BYTES) { + throw new RangeError( + `Secrets plan file exceeds ${SECRETS_PLAN_MAX_BYTES} bytes: ${pathname}`, + ); + } + raw = (await readFileDescriptorBounded(file.fd, SECRETS_PLAN_MAX_BYTES)).toString("utf8"); + } finally { + await file.close(); } let parsed: unknown; try { @@ -198,7 +226,7 @@ export function registerSecretsCli(program: Command): void { "Allow exec SecretRef preflight checks (may execute provider commands)", false, ) - .option("--plan-out ", "Write generated plan JSON to a file") + .option("--plan-out ", "Write generated plan JSON to a file (max 16 MiB)") .option("--json", "Output JSON", false) .action(async (opts: SecretsConfigureOptions) => { try { @@ -211,7 +239,7 @@ export function registerSecretsCli(program: Command): void { }); if (opts.planOut) { const { writeFileSync } = await fsModuleLoader.load(); - writeFileSync(opts.planOut, `${JSON.stringify(configured.plan, null, 2)}\n`, "utf8"); + writeFileSync(opts.planOut, serializePlanFile(configured.plan, opts.planOut), "utf8"); } let shouldApply = Boolean(opts.apply || opts.yes); @@ -307,7 +335,7 @@ export function registerSecretsCli(program: Command): void { secrets .command("apply") .description("Apply a previously generated secrets plan") - .requiredOption("--from ", "Path to plan JSON") + .requiredOption("--from ", "Path to plan JSON (max 16 MiB)") .option("--dry-run", "Validate/preflight only", false) .option("--allow-exec", "Allow exec SecretRef checks (may execute provider commands)", false) .option("--json", "Output JSON", false)