From 2ad3508a33a6c49d6abcdd07c9ede0f17c5d560a Mon Sep 17 00:00:00 2001 From: Vignesh Natarajan Date: Sun, 25 Jan 2026 00:29:28 -0800 Subject: [PATCH 1/5] feat(config): add tools.alsoAllow additive allowlist --- src/agents/pi-tools.policy.ts | 22 ++++++++++++++- src/agents/pi-tools.ts | 21 +++++++++++--- src/config/schema.ts | 2 ++ src/config/types.tools.ts | 13 +++++++++ src/config/zod-schema.agent-runtime.ts | 4 +++ src/gateway/tools-invoke-http.test.ts | 38 +++++++++++++++++++++++++- src/gateway/tools-invoke-http.ts | 17 ++++++++++-- 7 files changed, 109 insertions(+), 8 deletions(-) diff --git a/src/agents/pi-tools.policy.ts b/src/agents/pi-tools.policy.ts index 98585ca9d..1879a6218 100644 --- a/src/agents/pi-tools.policy.ts +++ b/src/agents/pi-tools.policy.ts @@ -96,13 +96,22 @@ export function filterToolsByPolicy(tools: AnyAgentTool[], policy?: SandboxToolP type ToolPolicyConfig = { allow?: string[]; + alsoAllow?: string[]; deny?: string[]; profile?: string; }; +function unionAllow(base?: string[], extra?: string[]) { + if (!Array.isArray(extra) || extra.length === 0) return base; + if (!Array.isArray(base) || base.length === 0) return base; + return Array.from(new Set([...base, ...extra])); +} + function pickToolPolicy(config?: ToolPolicyConfig): SandboxToolPolicy | undefined { if (!config) return undefined; - const allow = Array.isArray(config.allow) ? config.allow : undefined; + const allow = Array.isArray(config.allow) + ? unionAllow(config.allow, config.alsoAllow) + : undefined; const deny = Array.isArray(config.deny) ? config.deny : undefined; if (!allow && !deny) return undefined; return { allow, deny }; @@ -195,6 +204,17 @@ export function resolveEffectiveToolPolicy(params: { agentProviderPolicy: pickToolPolicy(agentProviderPolicy), profile, providerProfile: agentProviderPolicy?.profile ?? providerPolicy?.profile, + // alsoAllow is applied at the profile stage (to avoid being filtered out early). + profileAlsoAllow: Array.isArray(agentTools?.alsoAllow) + ? agentTools?.alsoAllow + : Array.isArray(globalTools?.alsoAllow) + ? globalTools?.alsoAllow + : undefined, + providerProfileAlsoAllow: Array.isArray(agentProviderPolicy?.alsoAllow) + ? agentProviderPolicy?.alsoAllow + : Array.isArray(providerPolicy?.alsoAllow) + ? providerPolicy?.alsoAllow + : undefined, }; } diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 9013f1e52..6f293514d 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -157,6 +157,8 @@ export function createClawdbotCodingTools(options?: { agentProviderPolicy, profile, providerProfile, + profileAlsoAllow, + providerProfileAlsoAllow, } = resolveEffectiveToolPolicy({ config: options?.config, sessionKey: options?.sessionKey, @@ -175,14 +177,25 @@ export function createClawdbotCodingTools(options?: { }); const profilePolicy = resolveToolProfilePolicy(profile); const providerProfilePolicy = resolveToolProfilePolicy(providerProfile); + + const mergeAlsoAllow = (policy: typeof profilePolicy, alsoAllow?: string[]) => { + if (!policy?.allow || !Array.isArray(alsoAllow) || alsoAllow.length === 0) return policy; + return { ...policy, allow: Array.from(new Set([...policy.allow, ...alsoAllow])) }; + }; + + const profilePolicyWithAlsoAllow = mergeAlsoAllow(profilePolicy, profileAlsoAllow); + const providerProfilePolicyWithAlsoAllow = mergeAlsoAllow( + providerProfilePolicy, + providerProfileAlsoAllow, + ); const scopeKey = options?.exec?.scopeKey ?? (agentId ? `agent:${agentId}` : undefined); const subagentPolicy = isSubagentSessionKey(options?.sessionKey) && options?.sessionKey ? resolveSubagentToolPolicy(options.config) : undefined; const allowBackground = isToolAllowedByPolicies("process", [ - profilePolicy, - providerProfilePolicy, + profilePolicyWithAlsoAllow, + providerProfilePolicyWithAlsoAllow, globalPolicy, globalProviderPolicy, agentPolicy, @@ -340,11 +353,11 @@ export function createClawdbotCodingTools(options?: { return expandPolicyWithPluginGroups(resolved.policy, pluginGroups); }; const profilePolicyExpanded = resolvePolicy( - profilePolicy, + profilePolicyWithAlsoAllow, profile ? `tools.profile (${profile})` : "tools.profile", ); const providerProfileExpanded = resolvePolicy( - providerProfilePolicy, + providerProfilePolicyWithAlsoAllow, providerProfile ? `tools.byProvider.profile (${providerProfile})` : "tools.byProvider.profile", ); const globalPolicyExpanded = resolvePolicy(globalPolicy, "tools.allow"); diff --git a/src/config/schema.ts b/src/config/schema.ts index 24d6bccfe..9627d64f3 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -165,7 +165,9 @@ const FIELD_LABELS: Record = { "tools.links.models": "Link Understanding Models", "tools.links.scope": "Link Understanding Scope", "tools.profile": "Tool Profile", + "tools.alsoAllow": "Tool Allowlist Additions", "agents.list[].tools.profile": "Agent Tool Profile", + "agents.list[].tools.alsoAllow": "Agent Tool Allowlist Additions", "tools.byProvider": "Tool Policy by Provider", "agents.list[].tools.byProvider": "Agent Tool Policy by Provider", "tools.exec.applyPatch.enabled": "Enable apply_patch", diff --git a/src/config/types.tools.ts b/src/config/types.tools.ts index ad7f69d85..d84dd1aa7 100644 --- a/src/config/types.tools.ts +++ b/src/config/types.tools.ts @@ -140,12 +140,21 @@ export type ToolProfileId = "minimal" | "coding" | "messaging" | "full"; export type ToolPolicyConfig = { allow?: string[]; + /** + * Additional allowlist entries merged into the effective allowlist. + * + * Intended for additive configuration (e.g., "also allow lobster") without forcing + * users to replace/duplicate an existing allowlist or profile. + */ + alsoAllow?: string[]; deny?: string[]; profile?: ToolProfileId; }; export type GroupToolPolicyConfig = { allow?: string[]; + /** Additional allowlist entries merged into allow. */ + alsoAllow?: string[]; deny?: string[]; }; @@ -188,6 +197,8 @@ export type AgentToolsConfig = { /** Base tool profile applied before allow/deny lists. */ profile?: ToolProfileId; allow?: string[]; + /** Additional allowlist entries merged into allow and/or profile allowlist. */ + alsoAllow?: string[]; deny?: string[]; /** Optional tool policy overrides keyed by provider id or "provider/model". */ byProvider?: Record; @@ -312,6 +323,8 @@ export type ToolsConfig = { /** Base tool profile applied before allow/deny lists. */ profile?: ToolProfileId; allow?: string[]; + /** Additional allowlist entries merged into allow and/or profile allowlist. */ + alsoAllow?: string[]; deny?: string[]; /** Optional tool policy overrides keyed by provider id or "provider/model". */ byProvider?: Record; diff --git a/src/config/zod-schema.agent-runtime.ts b/src/config/zod-schema.agent-runtime.ts index c733dcfa9..e08f08d6e 100644 --- a/src/config/zod-schema.agent-runtime.ts +++ b/src/config/zod-schema.agent-runtime.ts @@ -150,6 +150,7 @@ export const SandboxPruneSchema = z export const ToolPolicySchema = z .object({ allow: z.array(z.string()).optional(), + alsoAllow: z.array(z.string()).optional(), deny: z.array(z.string()).optional(), }) .strict() @@ -202,6 +203,7 @@ export const ToolProfileSchema = z export const ToolPolicyWithProfileSchema = z .object({ allow: z.array(z.string()).optional(), + alsoAllow: z.array(z.string()).optional(), deny: z.array(z.string()).optional(), profile: ToolProfileSchema, }) @@ -231,6 +233,7 @@ export const AgentToolsSchema = z .object({ profile: ToolProfileSchema, allow: z.array(z.string()).optional(), + alsoAllow: z.array(z.string()).optional(), deny: z.array(z.string()).optional(), byProvider: z.record(z.string(), ToolPolicyWithProfileSchema).optional(), elevated: z @@ -425,6 +428,7 @@ export const ToolsSchema = z .object({ profile: ToolProfileSchema, allow: z.array(z.string()).optional(), + alsoAllow: z.array(z.string()).optional(), deny: z.array(z.string()).optional(), byProvider: z.record(z.string(), ToolPolicyWithProfileSchema).optional(), web: ToolsWebSchema, diff --git a/src/gateway/tools-invoke-http.test.ts b/src/gateway/tools-invoke-http.test.ts index 18c23692d..956ac51dd 100644 --- a/src/gateway/tools-invoke-http.test.ts +++ b/src/gateway/tools-invoke-http.test.ts @@ -1,12 +1,19 @@ -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import type { IncomingMessage, ServerResponse } from "node:http"; + import { installGatewayTestHooks, getFreePort, startGatewayServer } from "./test-helpers.server.js"; import { resetTestPluginRegistry, setTestPluginRegistry, testState } from "./test-helpers.mocks.js"; import { createTestRegistry } from "../test-utils/channel-plugins.js"; installGatewayTestHooks({ scope: "suite" }); +beforeEach(() => { + // Ensure these tests are not affected by host env vars. + delete process.env.CLAWDBOT_GATEWAY_TOKEN; + delete process.env.CLAWDBOT_GATEWAY_PASSWORD; +}); + const resolveGatewayToken = (): string => { const token = (testState.gatewayAuth as { token?: string } | undefined)?.token; if (!token) throw new Error("test gateway token missing"); @@ -47,6 +54,35 @@ describe("POST /tools/invoke", () => { await server.close(); }); + it("supports tools.alsoAllow as additive allowlist (profile stage)", async () => { + // No explicit tool allowlist; rely on profile + alsoAllow. + testState.agentsConfig = { + list: [{ id: "main" }], + } as any; + + // minimal profile does NOT include sessions_list, but alsoAllow should. + const { writeConfigFile } = await import("../config/config.js"); + await writeConfigFile({ + tools: { profile: "minimal", alsoAllow: ["sessions_list"] }, + } as any); + + const port = await getFreePort(); + const server = await startGatewayServer(port, { bind: "loopback" }); + const token = resolveGatewayToken(); + + const res = await fetch(`http://127.0.0.1:${port}/tools/invoke`, { + method: "POST", + headers: { "content-type": "application/json", authorization: `Bearer ${token}` }, + body: JSON.stringify({ tool: "sessions_list", action: "json", args: {}, sessionKey: "main" }), + }); + + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.ok).toBe(true); + + await server.close(); + }); + it("accepts password auth when bearer token matches", async () => { testState.agentsConfig = { list: [ diff --git a/src/gateway/tools-invoke-http.ts b/src/gateway/tools-invoke-http.ts index 80e2f295e..5fd525c8c 100644 --- a/src/gateway/tools-invoke-http.ts +++ b/src/gateway/tools-invoke-http.ts @@ -130,9 +130,22 @@ export async function handleToolsInvokeHttpRequest( agentProviderPolicy, profile, providerProfile, + profileAlsoAllow, + providerProfileAlsoAllow, } = resolveEffectiveToolPolicy({ config: cfg, sessionKey }); const profilePolicy = resolveToolProfilePolicy(profile); const providerProfilePolicy = resolveToolProfilePolicy(providerProfile); + + const mergeAlsoAllow = (policy: typeof profilePolicy, alsoAllow?: string[]) => { + if (!policy?.allow || !Array.isArray(alsoAllow) || alsoAllow.length === 0) return policy; + return { ...policy, allow: Array.from(new Set([...policy.allow, ...alsoAllow])) }; + }; + + const profilePolicyWithAlsoAllow = mergeAlsoAllow(profilePolicy, profileAlsoAllow); + const providerProfilePolicyWithAlsoAllow = mergeAlsoAllow( + providerProfilePolicy, + providerProfileAlsoAllow, + ); const groupPolicy = resolveGroupToolPolicy({ config: cfg, sessionKey, @@ -183,11 +196,11 @@ export async function handleToolsInvokeHttpRequest( return expandPolicyWithPluginGroups(resolved.policy, pluginGroups); }; const profilePolicyExpanded = resolvePolicy( - profilePolicy, + profilePolicyWithAlsoAllow, profile ? `tools.profile (${profile})` : "tools.profile", ); const providerProfileExpanded = resolvePolicy( - providerProfilePolicy, + providerProfilePolicyWithAlsoAllow, providerProfile ? `tools.byProvider.profile (${providerProfile})` : "tools.byProvider.profile", ); const globalPolicyExpanded = resolvePolicy(globalPolicy, "tools.allow"); From d62b7c0d1ef776f87d2d62f45c48eb91cbff7738 Mon Sep 17 00:00:00 2001 From: Vignesh Natarajan Date: Sun, 25 Jan 2026 00:36:47 -0800 Subject: [PATCH 2/5] fix: treat tools.alsoAllow as implicit allow-all when no allowlist --- src/agents/pi-tools.policy.ts | 10 ++++++++-- src/gateway/tools-invoke-http.test.ts | 28 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/agents/pi-tools.policy.ts b/src/agents/pi-tools.policy.ts index 1879a6218..d6e125e33 100644 --- a/src/agents/pi-tools.policy.ts +++ b/src/agents/pi-tools.policy.ts @@ -103,7 +103,11 @@ type ToolPolicyConfig = { function unionAllow(base?: string[], extra?: string[]) { if (!Array.isArray(extra) || extra.length === 0) return base; - if (!Array.isArray(base) || base.length === 0) return base; + // If the user is using alsoAllow without an allowlist, treat it as additive on top of + // an implicit allow-all policy. + if (!Array.isArray(base) || base.length === 0) { + return Array.from(new Set(["*", ...extra])); + } return Array.from(new Set([...base, ...extra])); } @@ -111,7 +115,9 @@ function pickToolPolicy(config?: ToolPolicyConfig): SandboxToolPolicy | undefine if (!config) return undefined; const allow = Array.isArray(config.allow) ? unionAllow(config.allow, config.alsoAllow) - : undefined; + : Array.isArray(config.alsoAllow) && config.alsoAllow.length > 0 + ? unionAllow(undefined, config.alsoAllow) + : undefined; const deny = Array.isArray(config.deny) ? config.deny : undefined; if (!allow && !deny) return undefined; return { allow, deny }; diff --git a/src/gateway/tools-invoke-http.test.ts b/src/gateway/tools-invoke-http.test.ts index 956ac51dd..f08035885 100644 --- a/src/gateway/tools-invoke-http.test.ts +++ b/src/gateway/tools-invoke-http.test.ts @@ -83,6 +83,34 @@ describe("POST /tools/invoke", () => { await server.close(); }); + it("supports tools.alsoAllow without allow/profile (implicit allow-all)", async () => { + testState.agentsConfig = { + list: [{ id: "main" }], + } as any; + + await fs.mkdir(path.dirname(CONFIG_PATH_CLAWDBOT), { recursive: true }); + await fs.writeFile( + CONFIG_PATH_CLAWDBOT, + JSON.stringify({ tools: { alsoAllow: ["sessions_list"] } }, null, 2), + "utf-8", + ); + + const port = await getFreePort(); + const server = await startGatewayServer(port, { bind: "loopback" }); + + const res = await fetch(`http://127.0.0.1:${port}/tools/invoke`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ tool: "sessions_list", action: "json", args: {}, sessionKey: "main" }), + }); + + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.ok).toBe(true); + + await server.close(); + }); + it("accepts password auth when bearer token matches", async () => { testState.agentsConfig = { list: [ From 3497be29630db2166afd00e9733cc38e10cb4717 Mon Sep 17 00:00:00 2001 From: Vignesh Natarajan Date: Sun, 25 Jan 2026 00:40:13 -0800 Subject: [PATCH 3/5] docs: recommend tools.alsoAllow for optional plugin tools --- docs/automation/cron-vs-heartbeat.md | 2 +- docs/tools/lobster.md | 18 +++++++++++++++--- src/agents/pi-tools.ts | 2 +- src/agents/tool-policy.ts | 6 ++++++ src/gateway/tools-invoke-http.ts | 2 +- 5 files changed, 24 insertions(+), 6 deletions(-) diff --git a/docs/automation/cron-vs-heartbeat.md b/docs/automation/cron-vs-heartbeat.md index 333a45d0b..325575602 100644 --- a/docs/automation/cron-vs-heartbeat.md +++ b/docs/automation/cron-vs-heartbeat.md @@ -201,7 +201,7 @@ For ad-hoc workflows, call Lobster directly. - Lobster runs as a **local subprocess** (`lobster` CLI) in tool mode and returns a **JSON envelope**. - If the tool returns `needs_approval`, you resume with a `resumeToken` and `approve` flag. -- The tool is an **optional plugin**; you must allowlist `lobster` in `tools.allow`. +- The tool is an **optional plugin**; enable it additively via `tools.alsoAllow: ["lobster"]` (recommended). - If you pass `lobsterPath`, it must be an **absolute path**. See [Lobster](/tools/lobster) for full usage and examples. diff --git a/docs/tools/lobster.md b/docs/tools/lobster.md index daf04fd39..f4718c4b5 100644 --- a/docs/tools/lobster.md +++ b/docs/tools/lobster.md @@ -158,7 +158,19 @@ If you want to use a custom binary location, pass an **absolute** `lobsterPath` ## Enable the tool -Lobster is an **optional** plugin tool (not enabled by default). Allow it per agent: +Lobster is an **optional** plugin tool (not enabled by default). + +Recommended (additive, safe): + +```json +{ + "tools": { + "alsoAllow": ["lobster"] + } +} +``` + +Or per-agent: ```json { @@ -167,7 +179,7 @@ Lobster is an **optional** plugin tool (not enabled by default). Allow it per ag { "id": "main", "tools": { - "allow": ["lobster"] + "alsoAllow": ["lobster"] } } ] @@ -175,7 +187,7 @@ Lobster is an **optional** plugin tool (not enabled by default). Allow it per ag } ``` -You can also allow it globally with `tools.allow` if every agent should see it. +Avoid using `tools.allow: ["lobster"]` unless you intend to run in restrictive allowlist mode. Note: allowlists are opt-in for optional plugins. If your allowlist only names plugin tools (like `lobster`), Clawdbot keeps core tools enabled. To restrict core diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 6f293514d..4a0bebed0 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -346,7 +346,7 @@ export function createClawdbotCodingTools(options?: { if (resolved.unknownAllowlist.length > 0) { const entries = resolved.unknownAllowlist.join(", "); const suffix = resolved.strippedAllowlist - ? "Ignoring allowlist so core tools remain available." + ? "Ignoring allowlist so core tools remain available. Use tools.alsoAllow for additive plugin tool enablement." : "These entries won't match any tool unless the plugin is enabled."; logWarn(`tools: ${label} allowlist contains unknown entries (${entries}). ${suffix}`); } diff --git a/src/agents/tool-policy.ts b/src/agents/tool-policy.ts index ac2b1a91c..85152069e 100644 --- a/src/agents/tool-policy.ts +++ b/src/agents/tool-policy.ts @@ -209,6 +209,12 @@ export function stripPluginOnlyAllowlist( if (!isCoreEntry && !isPluginEntry) unknownAllowlist.push(entry); } const strippedAllowlist = !hasCoreEntry; + // When an allowlist contains only plugin tools, we strip it to avoid accidentally + // disabling core tools. Users who want additive behavior should prefer `tools.alsoAllow`. + if (strippedAllowlist) { + // Note: logging happens in the caller (pi-tools/tools-invoke) after this function returns. + // We keep this note here for future maintainers. + } return { policy: strippedAllowlist ? { ...policy, allow: undefined } : policy, unknownAllowlist: Array.from(new Set(unknownAllowlist)), diff --git a/src/gateway/tools-invoke-http.ts b/src/gateway/tools-invoke-http.ts index 5fd525c8c..b747e2561 100644 --- a/src/gateway/tools-invoke-http.ts +++ b/src/gateway/tools-invoke-http.ts @@ -189,7 +189,7 @@ export async function handleToolsInvokeHttpRequest( if (resolved.unknownAllowlist.length > 0) { const entries = resolved.unknownAllowlist.join(", "); const suffix = resolved.strippedAllowlist - ? "Ignoring allowlist so core tools remain available." + ? "Ignoring allowlist so core tools remain available. Use tools.alsoAllow for additive plugin tool enablement." : "These entries won't match any tool unless the plugin is enabled."; logWarn(`tools: ${label} allowlist contains unknown entries (${entries}). ${suffix}`); } From 42d039998d73c47dec264377668ef1be00595bc2 Mon Sep 17 00:00:00 2001 From: Pocket Clawd Date: Mon, 26 Jan 2026 10:17:50 -0800 Subject: [PATCH 4/5] feat(config): forbid allow+alsoAllow in same scope; auto-merge --- src/config/legacy.migrations.part-3.ts | 85 ++++++++++++++++++++++++++ src/config/zod-schema.agent-runtime.ts | 43 +++++++++++-- 2 files changed, 124 insertions(+), 4 deletions(-) diff --git a/src/config/legacy.migrations.part-3.ts b/src/config/legacy.migrations.part-3.ts index 9db9e3ede..d4b75e871 100644 --- a/src/config/legacy.migrations.part-3.ts +++ b/src/config/legacy.migrations.part-3.ts @@ -9,6 +9,84 @@ import { resolveDefaultAgentIdFromRaw, } from "./legacy.shared.js"; +function mergeAlsoAllowIntoAllow(node: unknown): boolean { + if (!isRecord(node)) return false; + const allow = node.allow; + const alsoAllow = node.alsoAllow; + if (!Array.isArray(allow) || allow.length === 0) return false; + if (!Array.isArray(alsoAllow) || alsoAllow.length === 0) return false; + const merged = Array.from(new Set([...(allow as unknown[]), ...(alsoAllow as unknown[])])); + node.allow = merged; + delete node.alsoAllow; + return true; +} + +function migrateAlsoAllowInToolConfig(raw: Record, changes: string[]) { + let mutated = false; + + // Global tools + const tools = getRecord(raw.tools); + if (mergeAlsoAllowIntoAllow(tools)) { + mutated = true; + changes.push("Merged tools.alsoAllow into tools.allow (and removed tools.alsoAllow)."); + } + + // tools.byProvider.* + const byProvider = getRecord(tools?.byProvider); + if (byProvider) { + for (const [key, value] of Object.entries(byProvider)) { + if (mergeAlsoAllowIntoAllow(value)) { + mutated = true; + changes.push(`Merged tools.byProvider.${key}.alsoAllow into allow (and removed alsoAllow).`); + } + } + } + + // agents.list[].tools + const agentsList = getAgentsList(raw); + for (const agent of agentsList) { + const agentTools = getRecord(agent.tools); + if (mergeAlsoAllowIntoAllow(agentTools)) { + mutated = true; + const id = typeof agent.id === "string" ? agent.id : ""; + changes.push(`Merged agents.list[${id}].tools.alsoAllow into allow (and removed alsoAllow).`); + } + + const agentByProvider = getRecord(agentTools?.byProvider); + if (agentByProvider) { + for (const [key, value] of Object.entries(agentByProvider)) { + if (mergeAlsoAllowIntoAllow(value)) { + mutated = true; + const id = typeof agent.id === "string" ? agent.id : ""; + changes.push( + `Merged agents.list[${id}].tools.byProvider.${key}.alsoAllow into allow (and removed alsoAllow).`, + ); + } + } + } + } + + // Provider group tool policies: channels..groups.*.tools and similar nested tool policy objects. + const channels = getRecord(raw.channels); + if (channels) { + for (const [provider, providerCfg] of Object.entries(channels)) { + const groups = getRecord(getRecord(providerCfg)?.groups); + if (!groups) continue; + for (const [groupKey, groupCfg] of Object.entries(groups)) { + const toolsCfg = getRecord(getRecord(groupCfg)?.tools); + if (mergeAlsoAllowIntoAllow(toolsCfg)) { + mutated = true; + changes.push( + `Merged channels.${provider}.groups.${groupKey}.tools.alsoAllow into allow (and removed alsoAllow).`, + ); + } + } + } + } + + return mutated; +} + export const LEGACY_CONFIG_MIGRATIONS_PART_3: LegacyConfigMigration[] = [ { id: "auth.anthropic-claude-cli-mode-oauth", @@ -24,6 +102,13 @@ export const LEGACY_CONFIG_MIGRATIONS_PART_3: LegacyConfigMigration[] = [ changes.push('Updated auth.profiles["anthropic:claude-cli"].mode → "oauth".'); }, }, + { + id: "tools.alsoAllow-merge", + describe: "Merge tools.alsoAllow into allow when allow is present", + apply: (raw, changes) => { + migrateAlsoAllowInToolConfig(raw, changes); + }, + }, { id: "tools.bash->tools.exec", describe: "Move tools.bash to tools.exec", diff --git a/src/config/zod-schema.agent-runtime.ts b/src/config/zod-schema.agent-runtime.ts index e08f08d6e..99074c55e 100644 --- a/src/config/zod-schema.agent-runtime.ts +++ b/src/config/zod-schema.agent-runtime.ts @@ -147,14 +147,22 @@ export const SandboxPruneSchema = z .strict() .optional(); -export const ToolPolicySchema = z +const ToolPolicyBaseSchema = z .object({ allow: z.array(z.string()).optional(), alsoAllow: z.array(z.string()).optional(), deny: z.array(z.string()).optional(), }) - .strict() - .optional(); + .strict(); + +export const ToolPolicySchema = ToolPolicyBaseSchema.superRefine((value, ctx) => { + if (value.allow && value.allow.length > 0 && value.alsoAllow && value.alsoAllow.length > 0) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: "tools policy cannot set both allow and alsoAllow in the same scope (merge alsoAllow into allow, or remove allow and use profile + alsoAllow)", + }); + } +}).optional(); export const ToolsWebSearchSchema = z .object({ @@ -207,7 +215,16 @@ export const ToolPolicyWithProfileSchema = z deny: z.array(z.string()).optional(), profile: ToolProfileSchema, }) - .strict(); + .strict() + .superRefine((value, ctx) => { + if (value.allow && value.allow.length > 0 && value.alsoAllow && value.alsoAllow.length > 0) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: + "tools.byProvider policy cannot set both allow and alsoAllow in the same scope (merge alsoAllow into allow, or remove allow and use profile + alsoAllow)", + }); + } + }); // Provider docking: allowlists keyed by provider id (no schema updates when adding providers). export const ElevatedAllowFromSchema = z @@ -274,6 +291,15 @@ export const AgentToolsSchema = z .optional(), }) .strict() + .superRefine((value, ctx) => { + if (value.allow && value.allow.length > 0 && value.alsoAllow && value.alsoAllow.length > 0) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: + "agent tools cannot set both allow and alsoAllow in the same scope (merge alsoAllow into allow, or remove allow and use profile + alsoAllow)", + }); + } + }) .optional(); export const MemorySearchSchema = z @@ -511,4 +537,13 @@ export const ToolsSchema = z .optional(), }) .strict() + .superRefine((value, ctx) => { + if (value.allow && value.allow.length > 0 && value.alsoAllow && value.alsoAllow.length > 0) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: + "tools cannot set both allow and alsoAllow in the same scope (merge alsoAllow into allow, or remove allow and use profile + alsoAllow)", + }); + } + }) .optional(); From f625303d13ffce8ad25dd2fe54a43df4b93cb16b Mon Sep 17 00:00:00 2001 From: Pocket Clawd Date: Mon, 26 Jan 2026 10:42:03 -0800 Subject: [PATCH 5/5] test(config): enforce allow+alsoAllow mutual exclusion --- src/config/config.tools-alsoAllow.test.ts | 53 ++++++++++++++ src/config/legacy.migrations.part-3.ts | 86 +---------------------- 2 files changed, 56 insertions(+), 83 deletions(-) create mode 100644 src/config/config.tools-alsoAllow.test.ts diff --git a/src/config/config.tools-alsoAllow.test.ts b/src/config/config.tools-alsoAllow.test.ts new file mode 100644 index 000000000..aea4f02d9 --- /dev/null +++ b/src/config/config.tools-alsoAllow.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from "vitest"; + +import { validateConfigObject } from "./validation.js"; + +// NOTE: These tests ensure allow + alsoAllow cannot be set in the same scope. + +describe("config: tools.alsoAllow", () => { + it("rejects tools.allow + tools.alsoAllow together", () => { + const res = validateConfigObject({ + tools: { + allow: ["group:fs"], + alsoAllow: ["lobster"], + }, + }); + + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.issues.some((i) => i.path === "tools")).toBe(true); + } + }); + + it("rejects agents.list[].tools.allow + alsoAllow together", () => { + const res = validateConfigObject({ + agents: { + list: [ + { + id: "main", + tools: { + allow: ["group:fs"], + alsoAllow: ["lobster"], + }, + }, + ], + }, + }); + + expect(res.ok).toBe(false); + if (!res.ok) { + expect(res.issues.some((i) => i.path.includes("agents.list"))).toBe(true); + } + }); + + it("allows profile + alsoAllow", () => { + const res = validateConfigObject({ + tools: { + profile: "coding", + alsoAllow: ["lobster"], + }, + }); + + expect(res.ok).toBe(true); + }); +}); diff --git a/src/config/legacy.migrations.part-3.ts b/src/config/legacy.migrations.part-3.ts index d4b75e871..21589e4fa 100644 --- a/src/config/legacy.migrations.part-3.ts +++ b/src/config/legacy.migrations.part-3.ts @@ -9,83 +9,9 @@ import { resolveDefaultAgentIdFromRaw, } from "./legacy.shared.js"; -function mergeAlsoAllowIntoAllow(node: unknown): boolean { - if (!isRecord(node)) return false; - const allow = node.allow; - const alsoAllow = node.alsoAllow; - if (!Array.isArray(allow) || allow.length === 0) return false; - if (!Array.isArray(alsoAllow) || alsoAllow.length === 0) return false; - const merged = Array.from(new Set([...(allow as unknown[]), ...(alsoAllow as unknown[])])); - node.allow = merged; - delete node.alsoAllow; - return true; -} +// NOTE: tools.alsoAllow was introduced after legacy migrations; no legacy migration needed. -function migrateAlsoAllowInToolConfig(raw: Record, changes: string[]) { - let mutated = false; - - // Global tools - const tools = getRecord(raw.tools); - if (mergeAlsoAllowIntoAllow(tools)) { - mutated = true; - changes.push("Merged tools.alsoAllow into tools.allow (and removed tools.alsoAllow)."); - } - - // tools.byProvider.* - const byProvider = getRecord(tools?.byProvider); - if (byProvider) { - for (const [key, value] of Object.entries(byProvider)) { - if (mergeAlsoAllowIntoAllow(value)) { - mutated = true; - changes.push(`Merged tools.byProvider.${key}.alsoAllow into allow (and removed alsoAllow).`); - } - } - } - - // agents.list[].tools - const agentsList = getAgentsList(raw); - for (const agent of agentsList) { - const agentTools = getRecord(agent.tools); - if (mergeAlsoAllowIntoAllow(agentTools)) { - mutated = true; - const id = typeof agent.id === "string" ? agent.id : ""; - changes.push(`Merged agents.list[${id}].tools.alsoAllow into allow (and removed alsoAllow).`); - } - - const agentByProvider = getRecord(agentTools?.byProvider); - if (agentByProvider) { - for (const [key, value] of Object.entries(agentByProvider)) { - if (mergeAlsoAllowIntoAllow(value)) { - mutated = true; - const id = typeof agent.id === "string" ? agent.id : ""; - changes.push( - `Merged agents.list[${id}].tools.byProvider.${key}.alsoAllow into allow (and removed alsoAllow).`, - ); - } - } - } - } - - // Provider group tool policies: channels..groups.*.tools and similar nested tool policy objects. - const channels = getRecord(raw.channels); - if (channels) { - for (const [provider, providerCfg] of Object.entries(channels)) { - const groups = getRecord(getRecord(providerCfg)?.groups); - if (!groups) continue; - for (const [groupKey, groupCfg] of Object.entries(groups)) { - const toolsCfg = getRecord(getRecord(groupCfg)?.tools); - if (mergeAlsoAllowIntoAllow(toolsCfg)) { - mutated = true; - changes.push( - `Merged channels.${provider}.groups.${groupKey}.tools.alsoAllow into allow (and removed alsoAllow).`, - ); - } - } - } - } - - return mutated; -} +// tools.alsoAllow legacy migration intentionally omitted (field not shipped in prod). export const LEGACY_CONFIG_MIGRATIONS_PART_3: LegacyConfigMigration[] = [ { @@ -102,13 +28,7 @@ export const LEGACY_CONFIG_MIGRATIONS_PART_3: LegacyConfigMigration[] = [ changes.push('Updated auth.profiles["anthropic:claude-cli"].mode → "oauth".'); }, }, - { - id: "tools.alsoAllow-merge", - describe: "Merge tools.alsoAllow into allow when allow is present", - apply: (raw, changes) => { - migrateAlsoAllowInToolConfig(raw, changes); - }, - }, + // tools.alsoAllow migration removed (field not shipped in prod; enforce via schema instead). { id: "tools.bash->tools.exec", describe: "Move tools.bash to tools.exec",