From 547fd70650baa7de6966ee77c8f4727dead0c7a7 Mon Sep 17 00:00:00 2001 From: indifferentketchup Date: Fri, 29 May 2026 03:12:16 +0000 Subject: [PATCH] server/coder: working-tree backend changes (pre-existing) Checkpoint of in-progress backend work present in the tree, not authored this session: auto_name, inference tool-phase/turn, secret_guard, provider-registry, plus a new agent-allowlist test (7 tests, passing). Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/coder/src/services/provider-registry.ts | 12 -- .../__tests__/agent-allowlist.test.ts | 107 ++++++++++++++++++ apps/server/src/services/auto_name.ts | 20 ++-- .../src/services/inference/tool-phase.ts | 31 ++++- apps/server/src/services/inference/turn.ts | 2 +- apps/server/src/services/secret_guard.ts | 9 ++ 6 files changed, 158 insertions(+), 23 deletions(-) create mode 100644 apps/server/src/services/__tests__/agent-allowlist.test.ts diff --git a/apps/coder/src/services/provider-registry.ts b/apps/coder/src/services/provider-registry.ts index 964c6bc..b1db3b7 100644 --- a/apps/coder/src/services/provider-registry.ts +++ b/apps/coder/src/services/provider-registry.ts @@ -24,12 +24,6 @@ export const PROVIDERS: ProviderDef[] = [ transport: 'native', modelSource: 'llama-swap', }, - { - name: 'cursor', - label: 'Cursor Agent', - transport: 'acp', - modelSource: 'probe', - }, { name: 'opencode', label: 'OpenCode', @@ -59,12 +53,6 @@ export const PROVIDERS: ProviderDef[] = [ transport: 'acp', modelSource: 'probe', }, - { - name: 'copilot', - label: 'GitHub Copilot', - transport: 'acp', - modelSource: 'probe', - }, ]; export const PROVIDERS_BY_NAME = new Map(PROVIDERS.map((p) => [p.name, p])); diff --git a/apps/server/src/services/__tests__/agent-allowlist.test.ts b/apps/server/src/services/__tests__/agent-allowlist.test.ts new file mode 100644 index 0000000..5987c60 --- /dev/null +++ b/apps/server/src/services/__tests__/agent-allowlist.test.ts @@ -0,0 +1,107 @@ +import { describe, it, expect } from 'vitest'; +import { parseAgentsMd, matchToolGlob } from '../agents.js'; +import { toolJsonSchemas } from '../tools.js'; + +describe('agent tool allowlist', () => { + const plannerMd = `# Agents + +## Planner +--- +temperature: 0.6 +tools: [view_file, grep, list_dir, find_files] +description: Read-only planner +--- +You plan. +`; + + it('parses an agent with a restricted tool allowlist', () => { + const { agents, errors } = parseAgentsMd(plannerMd); + expect(errors).toHaveLength(0); + expect(agents).toHaveLength(1); + const planner = agents[0]!; + expect(planner.name).toBe('Planner'); + expect(planner.tools).toEqual(['view_file', 'grep', 'list_dir', 'find_files']); + }); + + it('stream-phase filter: agent allowlist excludes tools not in the list', () => { + const { agents } = parseAgentsMd(plannerMd); + const planner = agents[0]!; + const allSchemas = toolJsonSchemas(); + const filtered = allSchemas.filter((t) => + matchToolGlob(t.function.name, planner.tools), + ); + const filteredNames = filtered.map((t) => t.function.name); + expect(filteredNames).toContain('view_file'); + expect(filteredNames).toContain('grep'); + expect(filteredNames).not.toContain('edit_file'); + expect(filteredNames).not.toContain('web_search'); + expect(filteredNames).not.toContain('get_codebase_overview'); + expect(filtered).toHaveLength(4); + }); + + it('tool-phase guard: rejects tool call not in agent allowlist', () => { + const { agents } = parseAgentsMd(plannerMd); + const planner = agents[0]!; + expect(matchToolGlob('edit_file', planner.tools)).toBe(false); + expect(matchToolGlob('create_file', planner.tools)).toBe(false); + expect(matchToolGlob('delete_file', planner.tools)).toBe(false); + expect(matchToolGlob('web_search', planner.tools)).toBe(false); + }); + + it('tool-phase guard: allows tool call in agent allowlist', () => { + const { agents } = parseAgentsMd(plannerMd); + const planner = agents[0]!; + expect(matchToolGlob('view_file', planner.tools)).toBe(true); + expect(matchToolGlob('grep', planner.tools)).toBe(true); + expect(matchToolGlob('list_dir', planner.tools)).toBe(true); + expect(matchToolGlob('find_files', planner.tools)).toBe(true); + }); + + it('null/absent tools field defaults to all tools (no regression)', () => { + const noToolsMd = `# Agents + +## Default +--- +temperature: 0.7 +description: Uses all tools +--- +Default agent. +`; + const { agents } = parseAgentsMd(noToolsMd); + const agent = agents[0]!; + const allSchemas = toolJsonSchemas(); + const filtered = allSchemas.filter((t) => + matchToolGlob(t.function.name, agent.tools), + ); + expect(filtered.length).toBe(allSchemas.length); + }); + + it('builder agent: write tools filtered out when not in ALL_TOOLS (BooChat context)', () => { + const builderMd = `# Agents + +## Builder +--- +temperature: 0.6 +tools: [view_file, grep, list_dir, find_files, edit_file, create_file, delete_file, apply_pending, rewind] +description: Read and write tools +--- +You build. +`; + const { agents } = parseAgentsMd(builderMd); + const builder = agents[0]!; + expect(matchToolGlob('view_file', builder.tools)).toBe(true); + expect(matchToolGlob('grep', builder.tools)).toBe(true); + // Write tools not in server's ALL_TOOLS are silently filtered during parsing. + // In BooCoder context (where ALL_TOOLS includes write tools), they'd be retained. + expect(builder.tools).not.toContain('edit_file'); + expect(builder.tools).not.toContain('create_file'); + expect(matchToolGlob('web_search', builder.tools)).toBe(false); + }); + + it('matchToolGlob rejects hallucinated tool against exact allowlist', () => { + const allowlist = ['view_file', 'grep', 'list_dir']; + expect(matchToolGlob('edit_file', allowlist)).toBe(false); + expect(matchToolGlob('rm_rf', allowlist)).toBe(false); + expect(matchToolGlob('view_file_extended', allowlist)).toBe(false); + }); +}); diff --git a/apps/server/src/services/auto_name.ts b/apps/server/src/services/auto_name.ts index 1b01952..38831e9 100644 --- a/apps/server/src/services/auto_name.ts +++ b/apps/server/src/services/auto_name.ts @@ -45,22 +45,26 @@ export async function maybeAutoNameChat( if (!chat) return; if (chat.name !== null && chat.name !== '') return; - const assistantMsg = await ctx.sql<{ content: string }[]>` - SELECT content FROM messages + const firstMsgs = await ctx.sql<{ role: string; content: string }[]>` + SELECT role, content FROM messages WHERE chat_id = ${chatId} - AND role = 'assistant' - AND status = 'complete' + AND role IN ('user', 'assistant') + AND status IN ('complete', 'ok') AND content <> '' ORDER BY created_at ASC - LIMIT 1 + LIMIT 2 `; - if (!assistantMsg[0]) return; + const userMsg = firstMsgs.find(m => m.role === 'user'); + const assistantMsg = firstMsgs.find(m => m.role === 'assistant'); + if (!assistantMsg) return; - const assistantText = assistantMsg[0].content.slice(0, 2000); + let namingInput = ''; + if (userMsg) namingInput += `User: ${userMsg.content.slice(0, 1000)}\n\n`; + namingInput += `Assistant: ${assistantMsg.content.slice(0, 1000)}`; const raw = await taskModelCompletion({ system: NAMING_SYSTEM_PROMPT, - user: assistantText, + user: namingInput, maxTokens: 30, temperature: 0.3, }); diff --git a/apps/server/src/services/inference/tool-phase.ts b/apps/server/src/services/inference/tool-phase.ts index fc6d295..8d7fbfe 100644 --- a/apps/server/src/services/inference/tool-phase.ts +++ b/apps/server/src/services/inference/tool-phase.ts @@ -1,7 +1,8 @@ -import type { Session, ToolCall } from '../../types/api.js'; +import type { Agent, Session, ToolCall } from '../../types/api.js'; import * as modelContext from '../model-context.js'; import { PathScopeError } from '../path_guard.js'; import { TOOLS_BY_NAME } from '../tools.js'; +import { matchToolGlob } from '../agents.js'; import { maybeFlagForCompaction } from './payload.js'; import { insertParts, partsFromAssistantMessage, partsFromToolMessage } from './parts.js'; // v1.13.16: richer unknown-tool error so the model can self-correct when it @@ -98,7 +99,8 @@ export async function executeToolPhase( result: StreamResult, startedAt: string | null, session: Session, - projectRoot: string + projectRoot: string, + agent?: Agent | null, ): Promise { const { sessionId, chatId, assistantMessageId } = args; const content = stripToolMarkup(result.content, { final: true }); @@ -262,6 +264,31 @@ export async function executeToolPhase( ); return; } + if (agent && !matchToolGlob(tc.name, agent.tools)) { + const stored = { + tool_call_id: tc.id, + output: null, + truncated: false, + error: `tool '${tc.name}' is not allowed for agent '${agent.name}'`, + }; + await insertParts( + ctx.sql, + partsFromToolMessage({ tool_results: stored }).map((p) => ({ + ...p, + message_id: toolMessageId, + })), + ); + ctx.publish(sessionId, { + type: 'tool_result', + tool_message_id: toolMessageId, + chat_id: chatId, + tool_call_id: tc.id, + output: stored.output, + truncated: false, + error: stored.error, + }); + return; + } const tres = await executeToolCall(projectRoot, tc, session.allowed_read_paths); if (SYNTHESIS_TOOLS.has(tc.name)) { synthEntries.push({ tc, output: tres.output, ...(tres.error ? { error: tres.error } : {}) }); diff --git a/apps/server/src/services/inference/turn.ts b/apps/server/src/services/inference/turn.ts index cd5c5dd..f0e8764 100644 --- a/apps/server/src/services/inference/turn.ts +++ b/apps/server/src/services/inference/turn.ts @@ -292,7 +292,7 @@ export async function runAssistantTurn( // ---- tool phase ---- let toolPhaseResult: ToolPhaseResult; try { - toolPhaseResult = await executeToolPhase(ctx, iterArgs, result, state.startedAt, iterSession, projectRoot); + toolPhaseResult = await executeToolPhase(ctx, iterArgs, result, state.startedAt, iterSession, projectRoot, agent); } catch (err) { // Tool phase errors are unexpected (individual tool failures are // caught inside executeToolPhase). Log and break. diff --git a/apps/server/src/services/secret_guard.ts b/apps/server/src/services/secret_guard.ts index 7176eae..cfd9729 100644 --- a/apps/server/src/services/secret_guard.ts +++ b/apps/server/src/services/secret_guard.ts @@ -163,6 +163,13 @@ const COMPILED: ReadonlyArray = DEFAULT_SECURITY_IGNORE_FILETYP // Returns true when `relPath` matches a known-secret pattern. Case-insensitive // (regex 'i' flag). Always normalize path separators to `/` so Windows-origin // paths match the same patterns. Empty or root-only paths return false. +const SAFE_PATTERNS: ReadonlySet = new Set([ + '.env.example', + '.env.sample', + '.env.template', + '.env.defaults', +]); + export function isSecretPath(relPath: string): boolean { if (!relPath) return false; const normalized = relPath.replace(/\\/g, '/'); @@ -170,6 +177,8 @@ export function isSecretPath(relPath: string): boolean { if (segments.length === 0) return false; const base = segments[segments.length - 1]!; + if (SAFE_PATTERNS.has(base.toLowerCase())) return false; + for (const compiled of COMPILED) { if (compiled.mode === 'basename') { if (compiled.regex.test(base)) return true;