diff --git a/cli/__tests__/adapters.claude.environment.test.mjs b/cli/__tests__/adapters.claude.environment.test.mjs index cb4fdae817..d84b81816f 100644 --- a/cli/__tests__/adapters.claude.environment.test.mjs +++ b/cli/__tests__/adapters.claude.environment.test.mjs @@ -423,6 +423,9 @@ describe('claude adapter — ctx.environment', () => { expect(call.args).toContain('Read(./**)'); expect(call.args).toContain('mcp__capture__*'); expect(call.args).toContain('Bash'); + // TASK-287: a public seat is not offered attach_file. + expect(call.args.slice(call.args.indexOf('--disallowedTools'))) + .toContain('mcp__capture__commonly_attach_file'); expect(call.opts.env.HOME).toBe(publicState); expect(call.opts.env.TMPDIR).toBe(path.join(publicState, 'tmp')); // Without it Claude creates /tmp/claude-, which the profile denies. @@ -1272,6 +1275,8 @@ describe('claude adapter — a workspace seat', () => { 'Read(./.git/**)', 'Read(./**/.git/**)', 'mcp__commonly__commonly_self_install_into_pod', ])); + // Workspace seats follow the same list as public ones (TASK-287). + expect(denied).toContain('mcp__commonly__commonly_attach_file'); expect(opts.env.HOME).toBe(publicState); expect(opts.env.COMMONLY_HOST_SECRET).toBeUndefined(); }); diff --git a/cli/__tests__/adapters.codex.test.mjs b/cli/__tests__/adapters.codex.test.mjs index b01242b459..bc92819af4 100644 --- a/cli/__tests__/adapters.codex.test.mjs +++ b/cli/__tests__/adapters.codex.test.mjs @@ -429,6 +429,9 @@ describe('codex adapter — spawn()', () => { }); const args = calls[0].args; expect(args).toContain('mcp_servers.commonly.command="node"'); + // TASK-287: a public seat is never offered attach_file. (The private + // spawn's exact -c list above proves the private seat keeps it.) + expect(args).toContain('mcp_servers.commonly.disabled_tools=["commonly_attach_file"]'); const permissionConfig = args.find((arg) => arg.startsWith('permissions.commonly_public.filesystem=')); expect(permissionConfig).toContain(`${JSON.stringify(versionDir)}="read"`); expect(permissionConfig).not.toContain(`${JSON.stringify(home)}="read"`); diff --git a/cli/__tests__/adapters.opencode.test.mjs b/cli/__tests__/adapters.opencode.test.mjs index 7d53981704..55c37b6e84 100644 --- a/cli/__tests__/adapters.opencode.test.mjs +++ b/cli/__tests__/adapters.opencode.test.mjs @@ -231,6 +231,10 @@ describe('opencode MCP config', () => { expect(permission.external_directory).toBe('deny'); expect(permission['commonly_*']).toBe('allow'); expect(permission['room_grants_*']).toBe('allow'); + // TASK-287: attach_file is denied after the wildcard allow, so the deny wins. + const keys = Object.keys(permission); + expect(permission.commonly_commonly_attach_file).toBe('deny'); + expect(keys.indexOf('commonly_commonly_attach_file')).toBeGreaterThan(keys.indexOf('commonly_*')); expect(permission.edit['*']).toBe('allow'); expect(permission.edit['**/.npmrc']).toBe('deny'); expect(permission.edit['**/.yarnrc*']).toBe('deny'); diff --git a/cli/__tests__/adapters.qwen.test.mjs b/cli/__tests__/adapters.qwen.test.mjs index 1eee76b116..92cfb2f402 100644 --- a/cli/__tests__/adapters.qwen.test.mjs +++ b/cli/__tests__/adapters.qwen.test.mjs @@ -103,6 +103,7 @@ test('public launch isolates HOME and config, strips shadows, and never serializ expect(args[args.indexOf('--exclude-tools') + 1].split(',')) .toEqual(expect.arrayContaining([ 'agent', 'workflow', 'advisor', 'replace', 'task', 'search_file_content', + 'commonly_attach_file', 'mcp__commonly__commonly_attach_file', ])); expect(options.env.HOME).toBe(options.env.QWEN_HOME); expect(options.env.HOME).toContain('commonly-qwen-home-'); diff --git a/cli/src/lib/adapters/claude.js b/cli/src/lib/adapters/claude.js index a793bffec3..57ad9649a2 100644 --- a/cli/src/lib/adapters/claude.js +++ b/cli/src/lib/adapters/claude.js @@ -96,6 +96,7 @@ import { buildMemoryPreamble, ROW_SLICING_CUE } from '../memory-bridge.js'; import { claudeConfigDirFor, claudeContextTokens, claudeTranscriptPath } from '../session-recycle.js'; import { skillsAsText } from '../skill-prompt.js'; import { adapterFailure, spawnCredentials } from '../upstream-refusal.js'; +import { PUBLIC_WITHHELD_TOOLS } from '../public-withheld-tools.js'; // See codex.js for the rationale on bumping the default + env override. // Keeping both adapters in lockstep so any wrapper agent runtime has the @@ -446,6 +447,10 @@ const buildPublicClaudePolicyArgs = (mcpToolPatterns, { trust, allow = [], memor const memoryDenies = memoryWithheld ? MEMORY_TOOLS.flatMap((tool) => mcpToolPatterns.map((pattern) => pattern.replace(/\*$/, tool))) : []; + // Nor, on any confined trust, the tools no public seat is offered + // (public-withheld-tools.js). + const withheldDenies = PUBLIC_WITHHELD_TOOLS + .flatMap((tool) => mcpToolPatterns.map((pattern) => pattern.replace(/\*$/, tool))); return [ // HOME points at an isolated, sanitized state directory. Ignore settings // from user/project/local sources and disable slash-command extensions; @@ -457,7 +462,7 @@ const buildPublicClaudePolicyArgs = (mcpToolPatterns, { trust, allow = [], memor '--permission-mode', 'dontAsk', '--allowedTools', 'Read(./**)', ...granted, ...mcpToolPatterns, '--disallowedTools', ...PUBLIC_DENIED_TOOLS.filter((tool) => !granted.includes(tool)), - ...deniedReads, ...workspaceDenies, ...memoryDenies, + ...deniedReads, ...workspaceDenies, ...memoryDenies, ...withheldDenies, ]; }; diff --git a/cli/src/lib/adapters/codex.js b/cli/src/lib/adapters/codex.js index f6fcfc1766..2c422b7c20 100644 --- a/cli/src/lib/adapters/codex.js +++ b/cli/src/lib/adapters/codex.js @@ -71,6 +71,7 @@ import { buildMemoryPreamble, ROW_SLICING_CUE } from '../memory-bridge.js'; import { resolveSkillSource, skillsAsText } from '../skill-prompt.js'; import { isLegacySandboxTrust, normalizeSandboxTrust, refuseWorkspaceTrust } from '../environment.js'; import { adapterFailure, spawnCredentials } from '../upstream-refusal.js'; +import { PUBLIC_WITHHELD_TOOLS } from '../public-withheld-tools.js'; // Default timeout for a single codex spawn (exec mode). // @@ -212,6 +213,10 @@ const buildMcpOverrideArgs = (mcpServers, ctx = {}) => { '-c', `mcp_servers.${server.name}.default_tools_approval_mode="approve"`, ); + // ...except the tools no public seat is offered (public-withheld-tools.js). + if (ctx.publicSeat) { + flags.push('-c', `mcp_servers.${server.name}.disabled_tools=[${PUBLIC_WITHHELD_TOOLS.map(toml).join(',')}]`); + } if (rest.length) { flags.push('-c', `mcp_servers.${server.name}.args=[${rest.map(toml).join(',')}]`); } diff --git a/cli/src/lib/adapters/opencode.js b/cli/src/lib/adapters/opencode.js index b7394b425e..62f77c5d15 100644 --- a/cli/src/lib/adapters/opencode.js +++ b/cli/src/lib/adapters/opencode.js @@ -58,6 +58,7 @@ import { resolveProviderKeyFile, startProviderProxy } from '../provider-proxy.js import { isShippedCommonlyMcpCommand } from '../declared-mcp-guard.js'; import { isAnswerableToolError } from '../public-tool-error.js'; import { startInstanceProxy } from '../instance-proxy.js'; +import { PUBLIC_WITHHELD_TOOLS } from '../public-withheld-tools.js'; const DEFAULT_TIMEOUT_MS = 15 * 60 * 1000; const ISOLATED_PROXY_PORT = 8765; @@ -246,6 +247,10 @@ const publicPermissions = (mode, mcpNames) => { todoread: 'allow', lsp: 'allow', ...Object.fromEntries(mcpNames.map((name) => [`${permissionMcpName(name)}_*`, 'allow'])), + // After the wildcard allow, since the last matching rule wins + // (public-withheld-tools.js). + ...Object.fromEntries(mcpNames.flatMap((name) => PUBLIC_WITHHELD_TOOLS + .map((tool) => [`${permissionMcpName(name)}_${tool}`, 'deny']))), }; }; diff --git a/cli/src/lib/adapters/qwen.js b/cli/src/lib/adapters/qwen.js index 3713c920e7..09bf955f7f 100644 --- a/cli/src/lib/adapters/qwen.js +++ b/cli/src/lib/adapters/qwen.js @@ -15,6 +15,7 @@ import { detectBwrap, wrapArgvWithBwrap } from '../sandbox/bwrap.js'; import { wrapArgvWithSeatbelt } from '../sandbox/seatbelt.js'; import { resolveProviderKeyFile, startProviderProxy } from '../provider-proxy.js'; import { adapterFailure, spawnCredentials } from '../upstream-refusal.js'; +import { PUBLIC_WITHHELD_TOOLS } from '../public-withheld-tools.js'; // Add a version only with a passing real clean-box sandbox smoke that names it. const TESTED_QWEN_VERSIONS = Object.freeze(['0.25.1', '0.26.2']); @@ -38,6 +39,9 @@ const EXCLUDED_TOOLS = [ // per-spawn MCP credential file, so public turns cannot expose OS read tools. const PUBLIC_READ_TOOLS = [ 'read_file', 'grep_search', 'search_file_content', 'glob', 'list_directory', 'lsp', + // Both spellings of a Commonly MCP tool, as with the aliases above + // (public-withheld-tools.js). + ...PUBLIC_WITHHELD_TOOLS.flatMap((tool) => [tool, `mcp__commonly__${tool}`]), ]; const parseVersion = (output) => { diff --git a/cli/src/lib/public-withheld-tools.js b/cli/src/lib/public-withheld-tools.js new file mode 100644 index 0000000000..6b8cd14236 --- /dev/null +++ b/cli/src/lib/public-withheld-tools.js @@ -0,0 +1,5 @@ +// Commonly MCP tools that public seats are not offered, on any adapter. +// Claude workspace seats follow the same list. Each +// adapter removes them in its own vocabulary (TASK-287; Goose's public list +// already leaves them out). +export const PUBLIC_WITHHELD_TOOLS = Object.freeze(['commonly_attach_file']); diff --git a/docs/agents/COMMONLY_MCP.md b/docs/agents/COMMONLY_MCP.md index 31dab1a148..c0766e4738 100644 --- a/docs/agents/COMMONLY_MCP.md +++ b/docs/agents/COMMONLY_MCP.md @@ -113,7 +113,7 @@ All tools are namespaced `commonly_*`. Names match the earlier extension surface | `commonly_react_to_message` | React to a message with an emoji (dual-auth kernel endpoint) | `messageId`, `emoji` | | `commonly_list_files` | List files uploaded into a pod (metadata) | `podId` | | `commonly_read_file` | Read a pod file's content (text ≤256KB; binary → metadata + note) | `podId`, `fileName` | -| `commonly_attach_file` | Upload a local file into a pod + post it as a file card | `podId`, `filePath` | +| `commonly_attach_file` | Upload a local file into a pod + post it as a file card. Not offered to public seats on any adapter (`cli/src/lib/public-withheld-tools.js`) | `podId`, `filePath` | GitHub PR tools were removed (they lent a shared server credential to any caller). Shell-capable runtimes use `gh pr diff` / `gh pr review`.