Compare commits

...

2 Commits

Author SHA1 Message Date
Peter Steinberger
43b55a8c3a fix: enforce strict exec approval optionals (#1414) (thanks @czekaj) 2026-01-22 03:11:06 +00:00
Lucas Czekaj
fc6a35797a fix(exec): pass undefined instead of null for optional approval params
TypeBox Type.Optional(Type.String()) accepts string|undefined but NOT null.
Discord exec was failing with 'resolvedPath must be string' because callers
passed null explicitly. Web UI worked because it skipped the approval request.

Fixes exec approval validation error in Discord-triggered sessions.
2026-01-22 02:54:36 +00:00
4 changed files with 90 additions and 6 deletions

View File

@ -14,6 +14,7 @@ Docs: https://docs.clawd.bot
- Config: avoid stack traces for invalid configs and log the config path. - Config: avoid stack traces for invalid configs and log the config path.
- Doctor: warn when gateway.mode is unset with configure/config guidance. - Doctor: warn when gateway.mode is unset with configure/config guidance.
- macOS: include Textual syntax highlighting resources in packaged app to prevent chat crashes. (#1362) - macOS: include Textual syntax highlighting resources in packaged app to prevent chat crashes. (#1362)
- Exec approvals: send optional approval params as undefined instead of null. (#1414) Thanks @czekaj.
- UI: refresh debug panel on route-driven tab changes. (#1373) Thanks @yazinsai. - UI: refresh debug panel on route-driven tab changes. (#1373) Thanks @yazinsai.
## 2026.1.21 ## 2026.1.21

View File

@ -896,8 +896,8 @@ export function createExecTool(
security: hostSecurity, security: hostSecurity,
ask: hostAsk, ask: hostAsk,
agentId: defaults?.agentId, agentId: defaults?.agentId,
resolvedPath: null, resolvedPath: undefined,
sessionKey: defaults?.sessionKey ?? null, sessionKey: defaults?.sessionKey,
timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS, timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS,
}, },
)) as { decision?: string } | null; )) as { decision?: string } | null;
@ -1060,7 +1060,7 @@ export function createExecTool(
const approvalSlug = createApprovalSlug(approvalId); const approvalSlug = createApprovalSlug(approvalId);
const expiresAtMs = Date.now() + DEFAULT_APPROVAL_TIMEOUT_MS; const expiresAtMs = Date.now() + DEFAULT_APPROVAL_TIMEOUT_MS;
const contextKey = `exec:${approvalId}`; const contextKey = `exec:${approvalId}`;
const resolvedPath = analysis.segments[0]?.resolution?.resolvedPath ?? null; const resolvedPath = analysis.segments[0]?.resolution?.resolvedPath;
const noticeSeconds = Math.max(1, Math.round(approvalRunningNoticeMs / 1000)); const noticeSeconds = Math.max(1, Math.round(approvalRunningNoticeMs / 1000));
const commandText = params.command; const commandText = params.command;
const effectiveTimeout = const effectiveTimeout =
@ -1082,7 +1082,7 @@ export function createExecTool(
ask: hostAsk, ask: hostAsk,
agentId: defaults?.agentId, agentId: defaults?.agentId,
resolvedPath, resolvedPath,
sessionKey: defaults?.sessionKey ?? null, sessionKey: defaults?.sessionKey,
timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS, timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS,
}, },
)) as { decision?: string } | null; )) as { decision?: string } | null;

View File

@ -267,8 +267,8 @@ export function registerNodesInvokeCommands(nodes: Command) {
security: hostSecurity, security: hostSecurity,
ask: hostAsk, ask: hostAsk,
agentId, agentId,
resolvedPath: null, resolvedPath: undefined,
sessionKey: null, sessionKey: undefined,
timeoutMs: 120_000, timeoutMs: 120_000,
})) as { decision?: string } | null; })) as { decision?: string } | null;
const decision = const decision =

View File

@ -1,10 +1,93 @@
import { describe, expect, it, vi } from "vitest"; import { describe, expect, it, vi } from "vitest";
import { ExecApprovalManager } from "../exec-approval-manager.js"; import { ExecApprovalManager } from "../exec-approval-manager.js";
import { createExecApprovalHandlers } from "./exec-approval.js"; import { createExecApprovalHandlers } from "./exec-approval.js";
import { validateExecApprovalRequestParams } from "../protocol/index.js";
const noop = () => {}; const noop = () => {};
describe("exec approval handlers", () => { describe("exec approval handlers", () => {
describe("ExecApprovalRequestParams validation", () => {
it("accepts request with resolvedPath omitted", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
host: "node",
};
expect(validateExecApprovalRequestParams(params)).toBe(true);
});
it("accepts request with resolvedPath as string", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
host: "node",
resolvedPath: "/usr/bin/echo",
};
expect(validateExecApprovalRequestParams(params)).toBe(true);
});
it("accepts request with resolvedPath as undefined", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
host: "node",
resolvedPath: undefined,
};
expect(validateExecApprovalRequestParams(params)).toBe(true);
});
// This documents the TypeBox/AJV behavior that caused the Discord exec bug:
// Type.Optional(Type.String()) does NOT accept null, only string or undefined.
it("rejects request with resolvedPath as null", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
host: "node",
resolvedPath: null,
};
expect(validateExecApprovalRequestParams(params)).toBe(false);
});
it("accepts request with sessionKey omitted", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
host: "node",
};
expect(validateExecApprovalRequestParams(params)).toBe(true);
});
it("accepts request with sessionKey as string", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
host: "node",
sessionKey: "session-1",
};
expect(validateExecApprovalRequestParams(params)).toBe(true);
});
it("accepts request with sessionKey as undefined", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
host: "node",
sessionKey: undefined,
};
expect(validateExecApprovalRequestParams(params)).toBe(true);
});
it("rejects request with sessionKey as null", () => {
const params = {
command: "echo hi",
cwd: "/tmp",
host: "node",
sessionKey: null,
};
expect(validateExecApprovalRequestParams(params)).toBe(false);
});
});
it("broadcasts request + resolve", async () => { it("broadcasts request + resolve", async () => {
const manager = new ExecApprovalManager(); const manager = new ExecApprovalManager();
const handlers = createExecApprovalHandlers(manager); const handlers = createExecApprovalHandlers(manager);