diff --git a/cli/__tests__/adapters.dsh.confined.test.mjs b/cli/__tests__/adapters.dsh.confined.test.mjs index 38beb4c36d..b038f861b6 100644 --- a/cli/__tests__/adapters.dsh.confined.test.mjs +++ b/cli/__tests__/adapters.dsh.confined.test.mjs @@ -9,12 +9,12 @@ import { jest } from '@jest/globals'; import { spawn as spawnReal } from 'child_process'; import { createServer } from 'http'; -import { existsSync, readFileSync, realpathSync } from 'fs'; +import { existsSync, mkdirSync, readFileSync, realpathSync } from 'fs'; import { chmod, mkdir, mkdtemp, rm, symlink, writeFile } from 'fs/promises'; -import { tmpdir } from 'os'; +import { homedir, tmpdir } from 'os'; import { join } from 'path'; -import dsh, { isConfinedDsh, TESTED_DSH_VERSIONS } from '../src/lib/adapters/dsh.js'; +import dsh, { developerToolsDir, isConfinedDsh, systemToolsFirst, TESTED_DSH_VERSIONS } from '../src/lib/adapters/dsh.js'; import { buildSeatbeltProfile } from '../src/lib/sandbox/seatbelt.js'; import { OWN_PROCESS_GROUP } from '@commonlyai/seats/process-group'; import { superviseRun } from '@commonlyai/seats/supervisor'; @@ -66,7 +66,7 @@ const connect = (target) => new Promise((resolve) => { fs.writeFileSync('.stub-report.json', JSON.stringify({ status: response.status, relay, egress, mcp, sandboxPolicy: rows.find((row) => row.id === 'sandbox-policy').config, - env: { HTTPS_PROXY: process.env.HTTPS_PROXY, NO_PROXY: process.env.NO_PROXY, TMPDIR: process.env.TMPDIR }, + env: { HTTPS_PROXY: process.env.HTTPS_PROXY, NO_PROXY: process.env.NO_PROXY, TMPDIR: process.env.TMPDIR, DEVELOPER_DIR: process.env.DEVELOPER_DIR, PATH: process.env.PATH }, })); emit({ type: 'session', sessionId: 'session-11111111-1111-1111-1111-111111111111' }); if (!response.ok) { emit({ type: 'error', message: 'provider said ' + response.status }); process.exit(1); } @@ -114,6 +114,7 @@ const withSeat = async (fn, { provider: providerOptions = {} } = {}) => { return ['node', join(mcpPkg, 'src', 'index.js')]; }, _selectedMcpVersionDir: () => mcpDir, + _developerToolsDir: () => null, }; try { await fn({ @@ -132,6 +133,57 @@ let warn; beforeEach(() => { warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); }); afterEach(() => { warn.mockRestore(); }); +test('a confined writer gets the developer tools read-only, so git works inside the sandbox (D106)', async () => { + await withSeat(async ({ ctx, wraps, pkg, mcpDir, root, report }) => { + const tools = join(root, 'CommandLineTools'); + mkdirSync(tools); + await dsh.spawn('[[reply:git-ok]]', { ...ctx, _developerToolsDir: () => tools }); + const { options } = wraps.find((entry) => entry.inner); + expect(options.readOnlyDirectories).toEqual([realpathSync(join(pkg, '..', '..')), mcpDir, tools]); + // Read-only: the grant never widens what the seat can write. + expect(buildSeatbeltProfile(options).split('\n').filter((line) => line.includes(tools) && line.includes('file-write'))).toEqual([]); + expect(report().env.DEVELOPER_DIR).toBe(tools); + // Bare `git` must be the shim the grant serves, not a Homebrew git ahead of it. + expect(report().env.PATH).toBe(systemToolsFirst(process.env.PATH)); + }); +}); + +test('systemToolsFirst puts /usr/bin ahead of Homebrew and keeps the rest in order', () => { + expect(systemToolsFirst('/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin')).toBe('/usr/bin:/opt/homebrew/bin:/usr/local/bin:/bin'); + expect(systemToolsFirst('/opt/homebrew/bin:/bin')).toBe('/usr/bin:/opt/homebrew/bin:/bin'); + expect(systemToolsFirst('')).toBe('/usr/bin'); + expect(systemToolsFirst(undefined)).toBe('/usr/bin'); +}); + +test('developerToolsDir: the declared or selected dir when it is an Apple tools location', () => { + const at = (paths) => ({ exists: (dir) => dir in paths, realpath: (dir) => paths[dir] }); + const CLT = '/Library/Developer/CommandLineTools'; + const XCODE = '/Applications/Xcode.app/Contents/Developer'; + const fs = at({ [CLT]: CLT, [XCODE]: XCODE, '/Applications/Xcode-beta.app': '/Applications/Xcode-beta.app' }); + expect(developerToolsDir({ env: { DEVELOPER_DIR: CLT }, run: () => { throw new Error('not called'); }, ...fs })).toBe(CLT); + expect(developerToolsDir({ env: {}, run: () => ({ stdout: `${XCODE}\n` }), ...fs })).toBe(XCODE); + expect(developerToolsDir({ env: {}, run: () => ({ stdout: '' }), ...fs })).toBeNull(); + expect(developerToolsDir({ env: {}, run: () => ({ stdout: 'relative/dir\n' }), ...fs })).toBeNull(); + expect(developerToolsDir({ env: { DEVELOPER_DIR: '/nonexistent/dev-tools' }, ...fs })).toBeNull(); + // A link into an Apple location is judged by where it lands. + expect(developerToolsDir({ env: { DEVELOPER_DIR: '/opt/tools' }, ...at({ '/opt/tools': CLT }) })).toBe(CLT); +}); + +test('developerToolsDir refuses any other directory, so DEVELOPER_DIR cannot widen the sandbox', () => { + const real = (dir) => developerToolsDir({ env: { DEVELOPER_DIR: dir }, run: () => { throw new Error('not called'); } }); + expect(real('/')).toBeNull(); + expect(real(homedir())).toBeNull(); + expect(real(tmpdir())).toBeNull(); + const linked = (target) => developerToolsDir({ + env: { DEVELOPER_DIR: '/Library/Developer/CommandLineTools' }, exists: () => true, realpath: () => target, + }); + expect(linked('/')).toBeNull(); + expect(linked('/Library/Developer/CommandLineTools/..')).toBeNull(); + expect(linked('/Applications/Xcode.app')).toBeNull(); + expect(linked('/Applications/Xcode.app/Contents/Developer/usr')).toBeNull(); + expect(linked('/Users/x/Applications/Xcode.app/Contents/Developer')).toBeNull(); +}); + test('a confined turn runs through the pinned proxy, the relay and the egress fence', async () => { await withSeat(async ({ ctx, wraps, provider, instanceSeen, pkg, mcpDir, home, report }) => { const result = await dsh.spawn('[[reply:confined-ok]]', ctx); diff --git a/cli/__tests__/attach.test.mjs b/cli/__tests__/attach.test.mjs index 0b094c1150..70dceff0e8 100644 --- a/cli/__tests__/attach.test.mjs +++ b/cli/__tests__/attach.test.mjs @@ -111,6 +111,45 @@ describe('resolveAttachSandbox — the sandbox an attach runs under (TASK-113)', }); }); +const { default: dsh, TESTED_DSH_VERSIONS } = await import('../src/lib/adapters/dsh.js'); + +describe("resolveAttachSandbox — dsh's confined shape (D106)", () => { + const confined = { sandbox: { mode: 'workspace', network: { policy: 'restricted' } } }; + + test('attach dsh admits the one shape the dsh adapter enforces, with no trust', () => { + // Before this, the trust-keyed gate refused mode:'workspace' without public + // trust, while dsh throws on public or workspace trust: no env passed both. + expect(resolveAttachSandbox({ environment: confined, adapterName: 'dsh', adapter: dsh })) + .toEqual({ mode: 'workspace', trust: undefined }); + }); + + test('dsh still refuses every other sandbox shape at attach', () => { + for (const sandbox of [ + { mode: 'workspace' }, + { mode: 'workspace', network: { policy: 'open' } }, + { mode: 'workspace', network: { policy: 'restricted' }, extra: 1 }, + { mode: 'read-only', network: { policy: 'restricted' } }, + { mode: 'workspace', network: { policy: 'restricted' }, trust: 'public' }, + { mode: 'workspace', network: { policy: 'restricted' }, trust: 'workspace' }, + ]) { + expect(() => resolveAttachSandbox({ environment: { sandbox }, adapterName: 'dsh', adapter: dsh })) + .toThrow(/dsh adapter/); + } + }); + + test('the same env is still refused for adapters without their own confined shape', () => { + for (const adapterName of ['claude', 'codex', 'opencode', 'qwen', 'pi']) { + expect(() => resolveAttachSandbox({ environment: confined, adapterName, adapter: { name: adapterName } })) + .toThrow(/requires public trust and an adapter with sandbox enforcement/); + } + }); + + test('without the adapter object, the old refusal still holds for dsh', () => { + expect(() => resolveAttachSandbox({ environment: confined, adapterName: 'dsh' })) + .toThrow(/requires public trust and an adapter with sandbox enforcement/); + }); +}); + describe('setWakeOnMessage', () => { const record = { agentName: 'juno', podId: 'pod-9', instanceId: 'default', runtimeToken: 'cm_agent_j', @@ -416,6 +455,50 @@ describe('performAttach', () => { .resolves.toBeUndefined(); }); + // D106: performAttach must hand the adapter to resolveAttachSandbox, or the + // confined dsh shape meets the trust-keyed refusal. dsh's real validator runs + // (only detection is stubbed), and the Seatbelt probe is injected so both of + // its outcomes are pinned on every host. + const attachConfinedDsh = async (seatbelt) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-dsh-attach-')); + try { + const keyFile = path.join(root, 'provider-key'); + const envPath = path.join(root, 'environment.json'); + fs.writeFileSync(keyFile, 'provider-key', { mode: 0o600 }); + fs.writeFileSync(envPath, JSON.stringify({ + version: 1, + workspace: { path: path.join(root, 'workspace') }, + model: 'deepseek-chat', + provider: { id: 'litellm', baseURL: 'https://llm.example.test/v1', keyFile }, + sandbox: { mode: 'workspace', network: { policy: 'restricted' } }, + })); + const client = makeClient({ runtimeToken: 'cm_agent_dsh' }); + client.get = jest.fn(async () => ({ publicRead: false, communityListed: false })); + const adapter = { ...dsh, detect: async () => ({ path: '/usr/local/bin/dsh', version: TESTED_DSH_VERSIONS[0] }) }; + const logs = []; + const error = await performAttach({ + client, adapterName: 'dsh', agentName: 'dsh-seat', podId: 'pod-private', envPath, + log: (line) => logs.push(line), + seatbelt: () => seatbelt, + adapterRegistry: { getAdapter: () => adapter, listAdapterNames: () => ['dsh'] }, + }).then(() => null, (e) => e); + return { error, logs }; + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }; + + test('attach --env admits confined dsh and logs its Seatbelt confinement (D106)', async () => { + const { error, logs } = await attachConfinedDsh({ available: true }); + expect(error).toBeNull(); + expect(logs).toContain('sandbox: confined dsh workspace via macOS Seatbelt (restricted network)'); + }); + + test('attach --env carries confined dsh to the Seatbelt check, not the trust refusal (D106)', async () => { + const { error } = await attachConfinedDsh({ available: false, error: 'sandbox-exec not found' }); + expect(error.message).toBe('confined dsh sandbox.mode=workspace requires macOS Seatbelt: sandbox-exec not found.'); + }); + test('attach --env gives provider-backed Qwen the same Commonly MCP baseline as the daemon', async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-qwen-attach-')); try { diff --git a/cli/src/commands/agent.js b/cli/src/commands/agent.js index 4df329ab4f..cefe711e23 100644 --- a/cli/src/commands/agent.js +++ b/cli/src/commands/agent.js @@ -939,8 +939,16 @@ export const updateAgentConfiguration = async ({ * two gates, because those gates compare `trust` directly. */ export const resolveAttachSandbox = ({ - environment, adapterName, platform = process.platform, + environment, adapterName, adapter = null, platform = process.platform, } = {}) => { + // An adapter that enforces one confined shape of its own (dsh: Seatbelt with a + // restricted network and NO trust) says so here, and its check is the one that + // decides: the trust-keyed gates below cannot express that shape, so before + // this they refused the only environment dsh accepts (D106). The adapter's + // check throws on every other shape, so this admits nothing it would refuse. + if (typeof adapter?.confinesOwnSandbox === 'function' && adapter.confinesOwnSandbox(environment)) { + return { mode: environment.sandbox.mode, trust: environment.sandbox.trust }; + } const sandbox = normalizeSandboxTrust(environment?.sandbox); const trust = sandbox?.trust; // `workspace` is claude-only for now (TASK-294). Refused here with the words @@ -988,6 +996,7 @@ export const performAttach = async ({ memoryInstance = 'default', log = () => {}, adapterRegistry = { getAdapter, listAdapterNames }, + seatbelt: probeSeatbelt = detectSeatbelt, }) => { const adapter = adapterRegistry.getAdapter(adapterName); if (!adapter) { @@ -1022,7 +1031,7 @@ export const performAttach = async ({ } log(`workspace: ${workspace.path}${workspace.created ? ' (created)' : ''}`); - const { mode: sandboxMode } = resolveAttachSandbox({ environment, adapterName }); + const { mode: sandboxMode } = resolveAttachSandbox({ environment, adapterName, adapter }); if (sandboxMode === 'bwrap') { const bwrap = detectBwrap(); if (!bwrap.available) { @@ -1047,19 +1056,20 @@ export const performAttach = async ({ + '(deny-by-default filesystem + network)', ); } else { - const seatbelt = detectSeatbelt(); - if (!seatbelt.available) { - throw new Error( - `public Claude sandbox.mode=${sandboxMode} requires macOS Seatbelt: ` - + `${seatbelt.error}. On Linux use sandbox.mode=bwrap.`, - ); - } const adapterLabel = ({ opencode: 'OpenCode', qwen: 'Qwen', claude: 'Claude' })[adapterName] || adapterName; - log( - `sandbox: public ${sandboxMode} via macOS Seatbelt ` - + `(deny-by-default host filesystem; ${adapterLabel}/MCP network retained)`, - ); + const seatbelt = probeSeatbelt(); + if (!seatbelt.available) { + // dsh refuses bwrap, so it gets no Linux hint that leads to a second refusal. + throw new Error(adapterName === 'dsh' + ? `confined dsh sandbox.mode=${sandboxMode} requires macOS Seatbelt: ${seatbelt.error}.` + : `public ${adapterLabel} sandbox.mode=${sandboxMode} requires macOS Seatbelt: ` + + `${seatbelt.error}. On Linux use sandbox.mode=bwrap.`); + } + log(adapterName === 'dsh' + ? `sandbox: confined dsh ${sandboxMode} via macOS Seatbelt (restricted network)` + : `sandbox: public ${sandboxMode} via macOS Seatbelt ` + + `(deny-by-default host filesystem; ${adapterLabel}/MCP network retained)`); } } else if (sandboxMode !== 'none' && sandboxMode !== undefined) { throw new Error( diff --git a/cli/src/lib/adapters/dsh.js b/cli/src/lib/adapters/dsh.js index 4121143914..edd7502362 100644 --- a/cli/src/lib/adapters/dsh.js +++ b/cli/src/lib/adapters/dsh.js @@ -55,6 +55,43 @@ export const isConfinedDsh = (environment = {}) => { export const assertPrivateDsh = (environment = {}) => { isConfinedDsh(environment); }; +/** + * The macOS developer tools a confined writer needs for git, granted read-only. + * /usr/bin/git is an xcrun shim: inside the sandbox it cannot read the tools it + * forwards to, so `git` fails with "No developer tools were found" and a writer + * cannot commit (D106). Returns null when no tools are installed; the seat then + * runs without git rather than failing. + * + * DEVELOPER_DIR and xcode-select are both settable by whoever starts the seat, + * so the resolved path is granted only when it is one of the two places Apple + * installs developer tools. Anything else (/, a home directory) returns null + * rather than widening what the sandbox can read. + */ +const DEVELOPER_DIR_SHAPES = [ + /^\/Library\/Developer\/CommandLineTools$/, + /^\/Applications\/[^/]+\.app\/Contents\/Developer$/, +]; + +export const developerToolsDir = ({ + env = process.env, run = spawnSync, exists = existsSync, realpath = realpathSync, +} = {}) => { + const declared = env?.DEVELOPER_DIR; + const dir = declared || String(run('/usr/bin/xcode-select', ['-p'], { encoding: 'utf8' })?.stdout || '').trim(); + if (!dir || !isAbsolute(dir) || !exists(dir)) return null; + const resolved = realpath(dir); + return DEVELOPER_DIR_SHAPES.some((shape) => shape.test(resolved)) ? resolved : null; +}; + +/** + * The confined child's PATH with /usr/bin first, so bare `git` is the xcrun + * shim the developer-tools grant serves. Seat runners put /opt/homebrew/bin + * ahead of it, and Homebrew git fails inside the sandbox reading its own + * /opt/homebrew/etc/gitconfig. Other entries keep their order, so tools + * /usr/bin lacks (node, npm) still resolve as before. + */ +export const systemToolsFirst = (pathValue) => ['/usr/bin', ...String(pathValue || '').split(delimiter) + .filter((dir) => dir && dir !== '/usr/bin')].join(delimiter); + const onPath = (name, env) => { if (isAbsolute(name)) return existsSync(name) ? name : null; for (const dir of String(env?.PATH || '').split(delimiter)) { @@ -296,6 +333,9 @@ export default { requiresModel: true, name: 'dsh', runtimeType: 'dsh', supportsProvider: true, + // The attach gate defers to this for dsh's one confined shape (D106). + confinesOwnSandbox: (environment) => isConfinedDsh(environment), + async validateEnvironment(environment, workspacePath) { const validation = validateEnvironmentSpec(environment); if (!validation.ok) throw new Error(validation.errors.join('; ')); @@ -408,6 +448,11 @@ export default { for (const name of ['HTTPS_PROXY', 'https_proxy', 'HTTP_PROXY', 'http_proxy']) env[name] = egress.url; env.NO_PROXY = '127.0.0.1,localhost'; env.no_proxy = env.NO_PROXY; + const developerDir = (ctx._developerToolsDir || developerToolsDir)({ env: sourceEnv }); + if (developerDir) { + env.DEVELOPER_DIR = developerDir; + env.PATH = systemToolsFirst(env.PATH); + } [command, ...commandArgs] = (ctx._wrapArgvWithSeatbelt || wrapArgvWithSeatbelt)([process.execPath, entry, ...args], { workspacePath: ctx.cwd, workspaceAccess: 'write', @@ -416,7 +461,7 @@ export default { mcpConfigDir: tempDir, network: 'loopback', loopbackNetworkPorts: [proxy.port, relay.port, egress.port], - readOnlyDirectories: [packages, ...mcpDirectories], + readOnlyDirectories: [packages, ...mcpDirectories, ...(developerDir ? [developerDir] : [])], }); } const credentials = [...spawnCredentials(ctx, []), bearer];