Merge pull request #1417 from czekaj/fix/exec-allowlist-agentid-derivation
fix(exec): derive agentId from sessionKey for allowlist lookup
This commit is contained in:
@@ -18,6 +18,7 @@ Docs: https://docs.clawd.bot
|
|||||||
- 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)
|
||||||
- Cron: cap reminder context history to 10 messages and honor `contextMessages`. (#1103) Thanks @mkbehr.
|
- Cron: cap reminder context history to 10 messages and honor `contextMessages`. (#1103) Thanks @mkbehr.
|
||||||
|
- Exec approvals: treat main as the default agent + migrate legacy default allowlists. (#1417) 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
|
||||||
|
|||||||
@@ -54,6 +54,7 @@ import { callGatewayTool } from "./tools/gateway.js";
|
|||||||
import { listNodes, resolveNodeIdFromList } from "./tools/nodes-utils.js";
|
import { listNodes, resolveNodeIdFromList } from "./tools/nodes-utils.js";
|
||||||
import { getShellConfig, sanitizeBinaryOutput } from "./shell-utils.js";
|
import { getShellConfig, sanitizeBinaryOutput } from "./shell-utils.js";
|
||||||
import { buildCursorPositionResponse, stripDsrRequests } from "./pty-dsr.js";
|
import { buildCursorPositionResponse, stripDsrRequests } from "./pty-dsr.js";
|
||||||
|
import { parseAgentSessionKey, resolveAgentIdFromSessionKey } from "../routing/session-key.js";
|
||||||
|
|
||||||
const DEFAULT_MAX_OUTPUT = clampNumber(
|
const DEFAULT_MAX_OUTPUT = clampNumber(
|
||||||
readEnvInt("PI_BASH_MAX_OUTPUT_CHARS"),
|
readEnvInt("PI_BASH_MAX_OUTPUT_CHARS"),
|
||||||
@@ -659,6 +660,11 @@ export function createExecTool(
|
|||||||
const notifyOnExit = defaults?.notifyOnExit !== false;
|
const notifyOnExit = defaults?.notifyOnExit !== false;
|
||||||
const notifySessionKey = defaults?.sessionKey?.trim() || undefined;
|
const notifySessionKey = defaults?.sessionKey?.trim() || undefined;
|
||||||
const approvalRunningNoticeMs = resolveApprovalRunningNoticeMs(defaults?.approvalRunningNoticeMs);
|
const approvalRunningNoticeMs = resolveApprovalRunningNoticeMs(defaults?.approvalRunningNoticeMs);
|
||||||
|
// Derive agentId only when sessionKey is an agent session key.
|
||||||
|
const parsedAgentSession = parseAgentSessionKey(defaults?.sessionKey);
|
||||||
|
const agentId =
|
||||||
|
defaults?.agentId ??
|
||||||
|
(parsedAgentSession ? resolveAgentIdFromSessionKey(defaults?.sessionKey) : undefined);
|
||||||
|
|
||||||
return {
|
return {
|
||||||
name: "exec",
|
name: "exec",
|
||||||
@@ -799,7 +805,7 @@ export function createExecTool(
|
|||||||
|
|
||||||
if (host === "node") {
|
if (host === "node") {
|
||||||
const approvals = resolveExecApprovals(
|
const approvals = resolveExecApprovals(
|
||||||
defaults?.agentId,
|
agentId,
|
||||||
host === "node" ? { security: "allowlist" } : undefined,
|
host === "node" ? { security: "allowlist" } : undefined,
|
||||||
);
|
);
|
||||||
const hostSecurity = minSecurity(security, approvals.agent.security);
|
const hostSecurity = minSecurity(security, approvals.agent.security);
|
||||||
@@ -865,7 +871,7 @@ export function createExecTool(
|
|||||||
cwd: workdir,
|
cwd: workdir,
|
||||||
env: nodeEnv,
|
env: nodeEnv,
|
||||||
timeoutMs: typeof params.timeout === "number" ? params.timeout * 1000 : undefined,
|
timeoutMs: typeof params.timeout === "number" ? params.timeout * 1000 : undefined,
|
||||||
agentId: defaults?.agentId,
|
agentId,
|
||||||
sessionKey: defaults?.sessionKey,
|
sessionKey: defaults?.sessionKey,
|
||||||
approved: approvedByAsk,
|
approved: approvedByAsk,
|
||||||
approvalDecision: approvalDecision ?? undefined,
|
approvalDecision: approvalDecision ?? undefined,
|
||||||
@@ -895,9 +901,9 @@ export function createExecTool(
|
|||||||
host: "node",
|
host: "node",
|
||||||
security: hostSecurity,
|
security: hostSecurity,
|
||||||
ask: hostAsk,
|
ask: hostAsk,
|
||||||
agentId: defaults?.agentId,
|
agentId,
|
||||||
resolvedPath: undefined,
|
resolvedPath: null,
|
||||||
sessionKey: defaults?.sessionKey,
|
sessionKey: defaults?.sessionKey ?? null,
|
||||||
timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS,
|
timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS,
|
||||||
},
|
},
|
||||||
)) as { decision?: string } | null;
|
)) as { decision?: string } | null;
|
||||||
@@ -1026,7 +1032,7 @@ export function createExecTool(
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (host === "gateway") {
|
if (host === "gateway") {
|
||||||
const approvals = resolveExecApprovals(defaults?.agentId, { security: "allowlist" });
|
const approvals = resolveExecApprovals(agentId, { security: "allowlist" });
|
||||||
const hostSecurity = minSecurity(security, approvals.agent.security);
|
const hostSecurity = minSecurity(security, approvals.agent.security);
|
||||||
const hostAsk = maxAsk(ask, approvals.agent.ask);
|
const hostAsk = maxAsk(ask, approvals.agent.ask);
|
||||||
const askFallback = approvals.agent.askFallback;
|
const askFallback = approvals.agent.askFallback;
|
||||||
@@ -1060,7 +1066,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;
|
const resolvedPath = analysis.segments[0]?.resolution?.resolvedPath ?? null;
|
||||||
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 =
|
||||||
@@ -1080,9 +1086,9 @@ export function createExecTool(
|
|||||||
host: "gateway",
|
host: "gateway",
|
||||||
security: hostSecurity,
|
security: hostSecurity,
|
||||||
ask: hostAsk,
|
ask: hostAsk,
|
||||||
agentId: defaults?.agentId,
|
agentId,
|
||||||
resolvedPath,
|
resolvedPath,
|
||||||
sessionKey: defaults?.sessionKey,
|
sessionKey: defaults?.sessionKey ?? null,
|
||||||
timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS,
|
timeoutMs: DEFAULT_APPROVAL_TIMEOUT_MS,
|
||||||
},
|
},
|
||||||
)) as { decision?: string } | null;
|
)) as { decision?: string } | null;
|
||||||
@@ -1123,7 +1129,7 @@ export function createExecTool(
|
|||||||
for (const segment of analysis.segments) {
|
for (const segment of analysis.segments) {
|
||||||
const pattern = segment.resolution?.resolvedPath ?? "";
|
const pattern = segment.resolution?.resolvedPath ?? "";
|
||||||
if (pattern) {
|
if (pattern) {
|
||||||
addAllowlistEntry(approvals.file, defaults?.agentId, pattern);
|
addAllowlistEntry(approvals.file, agentId, pattern);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1152,7 +1158,7 @@ export function createExecTool(
|
|||||||
seen.add(match.pattern);
|
seen.add(match.pattern);
|
||||||
recordAllowlistUse(
|
recordAllowlistUse(
|
||||||
approvals.file,
|
approvals.file,
|
||||||
defaults?.agentId,
|
agentId,
|
||||||
match,
|
match,
|
||||||
commandText,
|
commandText,
|
||||||
resolvedPath ?? undefined,
|
resolvedPath ?? undefined,
|
||||||
@@ -1242,7 +1248,7 @@ export function createExecTool(
|
|||||||
seen.add(match.pattern);
|
seen.add(match.pattern);
|
||||||
recordAllowlistUse(
|
recordAllowlistUse(
|
||||||
approvals.file,
|
approvals.file,
|
||||||
defaults?.agentId,
|
agentId,
|
||||||
match,
|
match,
|
||||||
params.command,
|
params.command,
|
||||||
analysis.segments[0]?.resolution?.resolvedPath,
|
analysis.segments[0]?.resolution?.resolvedPath,
|
||||||
|
|||||||
@@ -14,6 +14,7 @@ import {
|
|||||||
normalizeSafeBins,
|
normalizeSafeBins,
|
||||||
resolveCommandResolution,
|
resolveCommandResolution,
|
||||||
resolveExecApprovals,
|
resolveExecApprovals,
|
||||||
|
resolveExecApprovalsFromFile,
|
||||||
type ExecAllowlistEntry,
|
type ExecAllowlistEntry,
|
||||||
} from "./exec-approvals.js";
|
} from "./exec-approvals.js";
|
||||||
|
|
||||||
@@ -227,3 +228,35 @@ describe("exec approvals wildcard agent", () => {
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("exec approvals default agent migration", () => {
|
||||||
|
it("migrates legacy default agent entries to main", () => {
|
||||||
|
const file = {
|
||||||
|
version: 1,
|
||||||
|
agents: {
|
||||||
|
default: { allowlist: [{ pattern: "/bin/legacy" }] },
|
||||||
|
},
|
||||||
|
};
|
||||||
|
const resolved = resolveExecApprovalsFromFile({ file });
|
||||||
|
expect(resolved.allowlist.map((entry) => entry.pattern)).toEqual(["/bin/legacy"]);
|
||||||
|
expect(resolved.file.agents?.default).toBeUndefined();
|
||||||
|
expect(resolved.file.agents?.main?.allowlist?.[0]?.pattern).toBe("/bin/legacy");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("prefers main agent settings when both main and default exist", () => {
|
||||||
|
const file = {
|
||||||
|
version: 1,
|
||||||
|
agents: {
|
||||||
|
main: { ask: "always", allowlist: [{ pattern: "/bin/main" }] },
|
||||||
|
default: { ask: "off", allowlist: [{ pattern: "/bin/legacy" }] },
|
||||||
|
},
|
||||||
|
};
|
||||||
|
const resolved = resolveExecApprovalsFromFile({ file });
|
||||||
|
expect(resolved.agent.ask).toBe("always");
|
||||||
|
expect(resolved.allowlist.map((entry) => entry.pattern)).toEqual([
|
||||||
|
"/bin/main",
|
||||||
|
"/bin/legacy",
|
||||||
|
]);
|
||||||
|
expect(resolved.file.agents?.default).toBeUndefined();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -4,6 +4,8 @@ import net from "node:net";
|
|||||||
import os from "node:os";
|
import os from "node:os";
|
||||||
import path from "node:path";
|
import path from "node:path";
|
||||||
|
|
||||||
|
import { DEFAULT_AGENT_ID } from "../routing/session-key.js";
|
||||||
|
|
||||||
export type ExecHost = "sandbox" | "gateway" | "node";
|
export type ExecHost = "sandbox" | "gateway" | "node";
|
||||||
export type ExecSecurity = "deny" | "allowlist" | "full";
|
export type ExecSecurity = "deny" | "allowlist" | "full";
|
||||||
export type ExecAsk = "off" | "on-miss" | "always";
|
export type ExecAsk = "off" | "on-miss" | "always";
|
||||||
@@ -84,6 +86,35 @@ export function resolveExecApprovalsSocketPath(): string {
|
|||||||
return expandHome(DEFAULT_SOCKET);
|
return expandHome(DEFAULT_SOCKET);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function normalizeAllowlistPattern(value: string | undefined): string | null {
|
||||||
|
const trimmed = value?.trim() ?? "";
|
||||||
|
return trimmed ? trimmed.toLowerCase() : null;
|
||||||
|
}
|
||||||
|
|
||||||
|
function mergeLegacyAgent(
|
||||||
|
current: ExecApprovalsAgent,
|
||||||
|
legacy: ExecApprovalsAgent,
|
||||||
|
): ExecApprovalsAgent {
|
||||||
|
const allowlist: ExecAllowlistEntry[] = [];
|
||||||
|
const seen = new Set<string>();
|
||||||
|
const pushEntry = (entry: ExecAllowlistEntry) => {
|
||||||
|
const key = normalizeAllowlistPattern(entry.pattern);
|
||||||
|
if (!key || seen.has(key)) return;
|
||||||
|
seen.add(key);
|
||||||
|
allowlist.push(entry);
|
||||||
|
};
|
||||||
|
for (const entry of current.allowlist ?? []) pushEntry(entry);
|
||||||
|
for (const entry of legacy.allowlist ?? []) pushEntry(entry);
|
||||||
|
|
||||||
|
return {
|
||||||
|
security: current.security ?? legacy.security,
|
||||||
|
ask: current.ask ?? legacy.ask,
|
||||||
|
askFallback: current.askFallback ?? legacy.askFallback,
|
||||||
|
autoAllowSkills: current.autoAllowSkills ?? legacy.autoAllowSkills,
|
||||||
|
allowlist: allowlist.length > 0 ? allowlist : undefined,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
function ensureDir(filePath: string) {
|
function ensureDir(filePath: string) {
|
||||||
const dir = path.dirname(filePath);
|
const dir = path.dirname(filePath);
|
||||||
fs.mkdirSync(dir, { recursive: true });
|
fs.mkdirSync(dir, { recursive: true });
|
||||||
@@ -92,6 +123,13 @@ function ensureDir(filePath: string) {
|
|||||||
export function normalizeExecApprovals(file: ExecApprovalsFile): ExecApprovalsFile {
|
export function normalizeExecApprovals(file: ExecApprovalsFile): ExecApprovalsFile {
|
||||||
const socketPath = file.socket?.path?.trim();
|
const socketPath = file.socket?.path?.trim();
|
||||||
const token = file.socket?.token?.trim();
|
const token = file.socket?.token?.trim();
|
||||||
|
const agents = { ...file.agents };
|
||||||
|
const legacyDefault = agents.default;
|
||||||
|
if (legacyDefault) {
|
||||||
|
const main = agents[DEFAULT_AGENT_ID];
|
||||||
|
agents[DEFAULT_AGENT_ID] = main ? mergeLegacyAgent(main, legacyDefault) : legacyDefault;
|
||||||
|
delete agents.default;
|
||||||
|
}
|
||||||
const normalized: ExecApprovalsFile = {
|
const normalized: ExecApprovalsFile = {
|
||||||
version: 1,
|
version: 1,
|
||||||
socket: {
|
socket: {
|
||||||
@@ -104,7 +142,7 @@ export function normalizeExecApprovals(file: ExecApprovalsFile): ExecApprovalsFi
|
|||||||
askFallback: file.defaults?.askFallback,
|
askFallback: file.defaults?.askFallback,
|
||||||
autoAllowSkills: file.defaults?.autoAllowSkills,
|
autoAllowSkills: file.defaults?.autoAllowSkills,
|
||||||
},
|
},
|
||||||
agents: file.agents ?? {},
|
agents,
|
||||||
};
|
};
|
||||||
return normalized;
|
return normalized;
|
||||||
}
|
}
|
||||||
@@ -231,7 +269,7 @@ export function resolveExecApprovalsFromFile(params: {
|
|||||||
}): ExecApprovalsResolved {
|
}): ExecApprovalsResolved {
|
||||||
const file = normalizeExecApprovals(params.file);
|
const file = normalizeExecApprovals(params.file);
|
||||||
const defaults = file.defaults ?? {};
|
const defaults = file.defaults ?? {};
|
||||||
const agentKey = params.agentId ?? "default";
|
const agentKey = params.agentId ?? DEFAULT_AGENT_ID;
|
||||||
const agent = file.agents?.[agentKey] ?? {};
|
const agent = file.agents?.[agentKey] ?? {};
|
||||||
const wildcard = file.agents?.["*"] ?? {};
|
const wildcard = file.agents?.["*"] ?? {};
|
||||||
const fallbackSecurity = params.overrides?.security ?? DEFAULT_SECURITY;
|
const fallbackSecurity = params.overrides?.security ?? DEFAULT_SECURITY;
|
||||||
|
|||||||
Reference in New Issue
Block a user