mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-03 19:31:34 +00:00
fix(cli): bound exec approvals --file JSON read size (#110755)
* fix(cli): bound exec approvals --file JSON read size Replace raw fs.readFile with the shared readRegularFile helper from @openclaw/fs-safe/advanced, which enforces regular-file validation and a max-bytes limit. The --stdin path already had a 1 MB bound via readStdin; --file now uses the same EXEC_APPROVALS_STDIN_MAX_BYTES limit. * test(cli): add --file read bounds regression tests Covers normal (under limit), oversized (> 1 MiB), and non-regular path (directory) --file inputs to the approvals set command. * fix: import readRegularFile from ../infra/fs-safe.js for boundary compliance * fix(cli): preserve approvals file path behavior --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -233,6 +233,24 @@ describe("exec approvals CLI", () => {
|
||||
await program.parseAsync(args, { from: "user" });
|
||||
};
|
||||
|
||||
const runNativeApprovalsFileCommand = async (filePath: string) => {
|
||||
callGatewayFromCli.mockResolvedValue({
|
||||
enabled: true,
|
||||
hash: "sha256:current",
|
||||
defaultAction: "deny",
|
||||
rules: [],
|
||||
} as never);
|
||||
await runApprovalsCommand([
|
||||
"approvals",
|
||||
"set",
|
||||
"--node",
|
||||
"windows",
|
||||
"--file",
|
||||
filePath,
|
||||
"--json",
|
||||
]);
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
resetLocalSnapshot();
|
||||
runtimeErrors.length = 0;
|
||||
@@ -857,4 +875,75 @@ describe("exec approvals CLI", () => {
|
||||
"Exec approvals stdin exceeds 5 bytes.",
|
||||
);
|
||||
});
|
||||
|
||||
it("reads approvals JSON from a regular file", async () => {
|
||||
const dir = tempDirs.make("openclaw-approvals-file-bound-");
|
||||
const filePath = path.join(dir, "approvals.json");
|
||||
fs.writeFileSync(filePath, JSON.stringify({ defaultAction: "deny", rules: [] }));
|
||||
|
||||
await runNativeApprovalsFileCommand(filePath);
|
||||
|
||||
expect(callGatewayFromCli.mock.calls.map(([method]) => method)).toEqual([
|
||||
"exec.approvals.node.get",
|
||||
"exec.approvals.node.set",
|
||||
"exec.approvals.node.get",
|
||||
]);
|
||||
expect(runtimeErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("rejects an oversized approvals file", async () => {
|
||||
const dir = tempDirs.make("openclaw-approvals-file-bound-");
|
||||
const filePath = path.join(dir, "oversized.json");
|
||||
fs.writeFileSync(filePath, Buffer.alloc(1024 * 1024 + 1, "x"));
|
||||
|
||||
await expect(runNativeApprovalsFileCommand(filePath)).rejects.toThrow("__exit__:1");
|
||||
|
||||
expect(runtimeErrors[0]).toContain("File exceeds 1048576 bytes");
|
||||
expect(callGatewayFromCli).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("preserves the directory read error", async () => {
|
||||
const dir = tempDirs.make("openclaw-approvals-file-directory-");
|
||||
|
||||
await expect(runNativeApprovalsFileCommand(dir)).rejects.toThrow("__exit__:1");
|
||||
|
||||
expect(runtimeErrors[0]).toMatch(/EISDIR|directory/i);
|
||||
expect(callGatewayFromCli).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("follows a symlinked approvals file", async () => {
|
||||
const dir = tempDirs.make("openclaw-approvals-file-symlink-");
|
||||
const targetPath = path.join(dir, "target.json");
|
||||
const symlinkPath = path.join(dir, "approvals.json");
|
||||
fs.writeFileSync(targetPath, JSON.stringify({ defaultAction: "deny", rules: [] }));
|
||||
fs.symlinkSync(targetPath, symlinkPath);
|
||||
|
||||
await runNativeApprovalsFileCommand(symlinkPath);
|
||||
|
||||
expect(callGatewayFromCli.mock.calls.map(([method]) => method)).toContain(
|
||||
"exec.approvals.node.set",
|
||||
);
|
||||
expect(runtimeErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("rejects a file that grows past the limit after opening", async () => {
|
||||
const dir = tempDirs.make("openclaw-approvals-file-growth-");
|
||||
const filePath = path.join(dir, "growing.json");
|
||||
fs.writeFileSync(filePath, Buffer.alloc(1024 * 1024, "x"));
|
||||
const open = fs.promises.open.bind(fs.promises);
|
||||
const openSpy = vi.spyOn(fs.promises, "open").mockImplementation(async (...args) => {
|
||||
const handle = await open(...args);
|
||||
fs.appendFileSync(filePath, "x");
|
||||
return handle;
|
||||
});
|
||||
|
||||
try {
|
||||
await expect(runNativeApprovalsFileCommand(filePath)).rejects.toThrow("__exit__:1");
|
||||
} finally {
|
||||
openSpy.mockRestore();
|
||||
}
|
||||
|
||||
expect(runtimeErrors[0]).toContain("File exceeds 1048576 bytes");
|
||||
expect(callGatewayFromCli).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -26,6 +26,7 @@ import {
|
||||
type ExecApprovalsDefaults,
|
||||
type ExecApprovalsFile,
|
||||
} from "../infra/exec-approvals.js";
|
||||
import { readFileDescriptorBounded } from "../infra/file-descriptor-read.js";
|
||||
import { formatTimeAgo } from "../infra/format-time/format-relative.ts";
|
||||
import { defaultRuntime } from "../runtime.js";
|
||||
import { callGatewayFromCli } from "./gateway-rpc.js";
|
||||
@@ -99,6 +100,19 @@ async function readStdin(
|
||||
return bytes.toString("utf8");
|
||||
}
|
||||
|
||||
async function readApprovalsFile(filePath: string): Promise<string> {
|
||||
// Explicit CLI file inputs have historically followed symlinks and readable
|
||||
// special files. Pin that opened target while bounding the bytes consumed.
|
||||
const handle = await fs.open(filePath, "r");
|
||||
try {
|
||||
return (await readFileDescriptorBounded(handle.fd, EXEC_APPROVALS_STDIN_MAX_BYTES)).toString(
|
||||
"utf8",
|
||||
);
|
||||
} finally {
|
||||
await handle.close();
|
||||
}
|
||||
}
|
||||
|
||||
async function resolveTargetNodeId(opts: ExecApprovalsCliOpts): Promise<string | null> {
|
||||
if (opts.gateway) {
|
||||
return null;
|
||||
@@ -799,7 +813,7 @@ export function registerExecApprovalsCli(program: Command) {
|
||||
}
|
||||
const { source, nodeId, targetLabel, baseHash, kind } =
|
||||
await loadWritableSnapshotTarget(opts);
|
||||
const raw = opts.stdin ? await readStdin() : await fs.readFile(String(opts.file), "utf8");
|
||||
const raw = opts.stdin ? await readStdin() : await readApprovalsFile(String(opts.file));
|
||||
let input: unknown;
|
||||
try {
|
||||
input = JSON5.parse(raw);
|
||||
|
||||
Reference in New Issue
Block a user