diff --git a/docs/agents-and-tools.md b/docs/agents-and-tools.md index 02ecb3db27..37094bb0d4 100644 --- a/docs/agents-and-tools.md +++ b/docs/agents-and-tools.md @@ -38,16 +38,21 @@ and at GET, whatever the client. ### Trust gate for repository `.agents` directories `loadLocalAgents` dynamically imports every `.ts`/`.tsx`/`.js`/`.mjs`/`.cjs` -file under `{cwd}/.agents` and `{cwd}/../.agents`, and `loadMCPConfigSync` -reads an `mcp.json` there whose stdio servers are spawned on the first prompt -with `$VAR` env values filled from the CLI's own environment. Both execute +file under `{cwd}/.agents` and `{cwd}/../.agents`, except files under +`skills/`, `scripts/`, `hooks/`, `vendor/`, `node_modules/` or dot-directories +(support code, not agent definitions). `vendor/` and `hooks/` are reserved +names: an agent file under either is skipped without a warning, so keep agents +of your own in another directory. `loadMCPConfigSync` reads an `mcp.json` +there whose stdio servers are spawned on the first prompt with `$VAR` env +values filled from the CLI's own environment. Both execute repository-supplied code, so cloning a hostile repo and running the CLI inside it must not run anything without consent. The CLI therefore gates those two repository-scoped directories (`cli/src/utils/agent-dir-trust.ts`): - **What needs trust**: a directory containing at least one executable agent - file (the loader's extension filter, minus `.d.ts` and tests) or an - `mcp.json`. A directory holding only `skills/` markdown does not, and + file (a file the loader would import: extension filter, minus `.d.ts` and + tests, outside skipped directories) or an `mcp.json`. A directory holding + only `skills/` markdown does not, and `~/.agents` never does (it is the user's own). Skills still load from an untrusted directory; only agents and `mcp.json` are withheld. - **The prompt**: on an interactive launch (stdin and stdout are TTYs) the CLI diff --git a/sdk/CHANGELOG.md b/sdk/CHANGELOG.md index ba741447d8..fb1faaff30 100644 --- a/sdk/CHANGELOG.md +++ b/sdk/CHANGELOG.md @@ -4,6 +4,8 @@ All notable changes to the @codebuff/sdk package will be documented in this file ## [Unreleased] +- `loadLocalAgents` now skips `hooks/` and `vendor/` directories inside `.agents`. A third-party plugin's hook script was imported as an agent, which ran its top-level code; ponytail's `hooks/ponytail-subagent.js` calls `process.exit(0)`, so the CLI exited before the TUI. + - **Security / breaking:** a fresh session no longer imports `process.cwd()/.agents` or its parent `.agents` when no agents were supplied. Agent files are executable modules (importing one runs its top-level code), so a cloned repository's `.agents` ran on the developer's machine before any prompt or approval. Now `agentDefinitions`, when given, is the whole answer: an empty array loads nothing (it used to fall back to the directories above, re-importing what a host's trust gate had just refused). With `agentDefinitions` omitted, only `~/.agents` is loaded, or the directories in the new `agentDirs` option (`run`, `CodebuffClient`, `initialSessionState`, `generateInitialRunState`). To keep loading a repository's agents, pass `agentDirs: [path.join(repo, '.agents')]` once you trust it, or pass `agentDefinitions` from `loadLocalAgents({ agentDirs })`. `loadLocalAgents()` called with no directories still searches the old defaults. A string agent id that came only from a repository `.agents` file is no longer found locally and may resolve from the `codebuff/` registry; set `disableAgentRegistry: true` if that fallback is unwanted. - New `skipSymbolIndex` run option: the run builds the file tree but skips the tree-sitter parse of every project file, leaving `fileTokenScores` and `tokenCallers` empty. Set it when your agents do not use the file-picker, `read_subtree` symbols or `read_files`' `referencedBy`. diff --git a/sdk/src/__tests__/load-agents.test.ts b/sdk/src/__tests__/load-agents.test.ts index 84fc11645a..eed6053dea 100644 --- a/sdk/src/__tests__/load-agents.test.ts +++ b/sdk/src/__tests__/load-agents.test.ts @@ -376,6 +376,63 @@ describe('loadLocalAgents', () => { expect(existsSync(markerFile)).toBe(false) }) + test('skips plugin hook directories without importing executable scripts', async () => { + const vendorHookDir = path.join(agentsDir, 'vendor', 'ponytail', 'hooks') + const directHookDir = path.join(agentsDir, 'ponytail', 'hooks') + mkdirSync(vendorHookDir, { recursive: true }) + mkdirSync(directHookDir, { recursive: true }) + const vendorMarker = path.join(tempDir, 'vendor-hook-side-effect') + const directMarker = path.join(tempDir, 'direct-hook-side-effect') + const vendorSupportMarker = path.join( + tempDir, + 'vendor-support-side-effect', + ) + writeAgentFile( + vendorHookDir, + 'ponytail-subagent.js', + ` + const { writeFileSync } = require('fs') + writeFileSync(${JSON.stringify(vendorMarker)}, 'imported') + `, + ) + writeAgentFile( + path.join(agentsDir, 'vendor'), + 'support.js', + ` + const { writeFileSync } = require('fs') + writeFileSync(${JSON.stringify(vendorSupportMarker)}, 'imported') + `, + ) + writeAgentFile( + directHookDir, + 'subagent.js', + ` + const { writeFileSync } = require('fs') + writeFileSync(${JSON.stringify(directMarker)}, 'imported') + `, + ) + writeAgentFile( + agentsDir, + 'real-agent.ts', + ` + export default { + id: 'real-agent', + displayName: 'Real Agent', + model: '${MODEL_NAME}' + } + `, + ) + + const result: LoadedAgents = await loadLocalAgents({ + agentsPath: agentsDir, + }) + + expect(result['real-agent']).toBeDefined() + expect(existsSync(vendorMarker)).toBe(false) + expect(existsSync(vendorSupportMarker)).toBe(false) + expect(existsSync(directMarker)).toBe(false) + }) + test('converts handleSteps function to string', async () => { mkdirSync(agentsDir, { recursive: true }) writeAgentFile( @@ -984,6 +1041,10 @@ describe('loadLocalAgents', () => { mkdirSync(path.join(agentsDir, 'skills', 'x'), { recursive: true }) mkdirSync(path.join(agentsDir, 'nested'), { recursive: true }) mkdirSync(path.join(agentsDir, 'node_modules'), { recursive: true }) + mkdirSync(path.join(agentsDir, 'vendor', 'ponytail', 'hooks'), { + recursive: true, + }) + mkdirSync(path.join(agentsDir, 'ponytail', 'hooks'), { recursive: true }) writeFileSync(path.join(agentsDir, 'a.ts'), '') writeFileSync(path.join(agentsDir, 'b.mjs'), '') writeFileSync(path.join(agentsDir, 'nested', 'c.js'), '') @@ -992,6 +1053,12 @@ describe('loadLocalAgents', () => { writeFileSync(path.join(agentsDir, 'README.md'), '') writeFileSync(path.join(agentsDir, 'skills', 'x', 'SKILL.md'), '') writeFileSync(path.join(agentsDir, 'node_modules', 'dep.js'), '') + writeFileSync( + path.join(agentsDir, 'vendor', 'ponytail', 'hooks', 'hook.js'), + '', + ) + writeFileSync(path.join(agentsDir, 'vendor', 'support.js'), '') + writeFileSync(path.join(agentsDir, 'ponytail', 'hooks', 'hook.js'), '') expect(listLocalAgentFiles(agentsDir).sort()).toEqual([ path.join(agentsDir, 'a.ts'), diff --git a/sdk/src/agents/load-agents.ts b/sdk/src/agents/load-agents.ts index 112f2d1e5c..9a684d38d6 100644 --- a/sdk/src/agents/load-agents.ts +++ b/sdk/src/agents/load-agents.ts @@ -112,7 +112,9 @@ const shouldSkipAgentDirectory = (name: string): boolean => name.startsWith('.') || name === 'node_modules' || name === 'scripts' || + name === 'hooks' || name === 'skills' || + name === 'vendor' || name.startsWith('skills-') const isLoadableAgentFileName = (fileName: string): boolean => { @@ -127,9 +129,9 @@ const isLoadableAgentFileName = (fileName: string): boolean => { /** * List every file under `dir` that `loadLocalAgents` would import: `.ts`, * `.tsx`, `.js`, `.mjs` or `.cjs`, minus `.d.ts` and test files, skipping - * `skills/`, `scripts/`, `node_modules` and dot-directories. Hosts use this to - * decide whether a repository-scoped `.agents` directory contains anything - * executable before importing it. + * `skills/`, `scripts/`, `hooks/`, `vendor/`, `node_modules` and + * dot-directories. Hosts use this to decide whether a repository-scoped + * `.agents` directory contains anything executable before importing it. * * @returns Absolute file paths; empty when the directory does not exist. */