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.policy.ts b/src/agents/pi-tools.policy.ts index 98585ca9d..d6e125e33 100644 --- a/src/agents/pi-tools.policy.ts +++ b/src/agents/pi-tools.policy.ts @@ -96,13 +96,28 @@ 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 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])); +} + 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) + : 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 }; @@ -195,6 +210,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..4a0bebed0 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, @@ -333,18 +346,18 @@ 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}`); } 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/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/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 9db9e3ede..21589e4fa 100644 --- a/src/config/legacy.migrations.part-3.ts +++ b/src/config/legacy.migrations.part-3.ts @@ -9,6 +9,10 @@ import { resolveDefaultAgentIdFromRaw, } from "./legacy.shared.js"; +// NOTE: tools.alsoAllow was introduced after legacy migrations; no legacy migration needed. + +// tools.alsoAllow legacy migration intentionally omitted (field not shipped in prod). + export const LEGACY_CONFIG_MIGRATIONS_PART_3: LegacyConfigMigration[] = [ { id: "auth.anthropic-claude-cli-mode-oauth", @@ -24,6 +28,7 @@ export const LEGACY_CONFIG_MIGRATIONS_PART_3: LegacyConfigMigration[] = [ changes.push('Updated auth.profiles["anthropic:claude-cli"].mode → "oauth".'); }, }, + // 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", 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..99074c55e 100644 --- a/src/config/zod-schema.agent-runtime.ts +++ b/src/config/zod-schema.agent-runtime.ts @@ -147,13 +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({ @@ -202,10 +211,20 @@ 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, }) - .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 @@ -231,6 +250,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 @@ -271,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 @@ -425,6 +454,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, @@ -507,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(); diff --git a/src/gateway/tools-invoke-http.test.ts b/src/gateway/tools-invoke-http.test.ts index 18c23692d..f08035885 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,63 @@ 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("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: [ diff --git a/src/gateway/tools-invoke-http.ts b/src/gateway/tools-invoke-http.ts index 80e2f295e..b747e2561 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, @@ -176,18 +189,18 @@ 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}`); } 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");