Merge pull request #1414 from czekaj/fix/discord-exec-resolvedpath-validation
fix(exec): pass undefined instead of null for optional approval params
This commit is contained in:
@@ -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;
|
||||||
|
|||||||
@@ -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 =
|
||||||
|
|||||||
@@ -1,10 +1,54 @@
|
|||||||
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("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);
|
||||||
|
|||||||
Reference in New Issue
Block a user