diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml new file mode 100644 index 0000000000..903c27146f --- /dev/null +++ b/.github/workflows/pr-review.yml @@ -0,0 +1,207 @@ +name: Code Review Bot + +on: + pull_request: + types: [opened, synchronize, reopened] + workflow_dispatch: + inputs: + pr_number: + description: 'PR number para revisar' + required: true + type: number + +permissions: + contents: read + pull-requests: write + +jobs: + review: + runs-on: ubuntu-latest + steps: + - name: Code Review Bot + env: + GH_TOKEN: ${{ github.token }} + OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} + PR_NUMBER: ${{ github.event.pull_request.number || github.event.inputs.pr_number }} + PR_TITLE: ${{ github.event.pull_request.title }} + REPO_NAME: ${{ github.repository }} + run: | + set -euo pipefail + + if [ -z "${OPENROUTER_API_KEY:-}" ]; then + echo "OPENROUTER_API_KEY ausente, pulando review" + exit 0 + fi + + if [ -z "${PR_NUMBER:-}" ]; then + echo "PR number ausente, pulando review" + exit 0 + fi + + if [ -z "${PR_TITLE:-}" ]; then + PR_TITLE=$(gh pr view "$PR_NUMBER" --repo "$REPO_NAME" --json title -q '.title') + fi + + set +o pipefail + DIFF=$(gh pr diff "$PR_NUMBER" --repo "$REPO_NAME" --patch 2>/dev/null | head -c 30000) + set -o pipefail + + if [ -z "$DIFF" ]; then + echo "Diff vazio, pulando review" + exit 0 + fi + + DIFF_HASH=$(echo "$DIFF" | sha256sum | cut -d' ' -f1) + LAST_COMMENT=$(gh pr view "$PR_NUMBER" --repo "$REPO_NAME" --comments \ + --json comments -q '[.comments[] | select(.author.login == "github-actions[bot]" and (.body | contains("Code Review Bot")))] | last | .body' \ + 2>/dev/null || true) + if [ -n "$LAST_COMMENT" ]; then + PREV_HASH=$(echo "$LAST_COMMENT" | grep -oP '(?<=diff-sha: )[a-f0-9]+' || true) + if [ "$PREV_HASH" = "$DIFF_HASH" ]; then + echo "Diff identico ao ultimo review (sha: ${DIFF_HASH:0:12}), pulando" + exit 0 + fi + fi + + FILE_CONTEXT="" + CHANGED_FILES=$(gh pr diff "$PR_NUMBER" --repo "$REPO_NAME" --name-only 2>/dev/null | head -10) + BRANCH=$(gh pr view "$PR_NUMBER" --repo "$REPO_NAME" --json headRefName -q '.headRefName') + for file in $CHANGED_FILES; do + if [[ "$file" == *.ts || "$file" == *.tsx || "$file" == *.js || "$file" == *.jsx || "$file" == *.prisma || "$file" == *.json || "$file" == *.yml ]]; then + FILE_CONTENT=$(gh api "repos/$REPO_NAME/contents/$file?ref=$BRANCH" --jq '.content' 2>/dev/null | base64 -d 2>/dev/null | head -c 5000 || true) + if [ -n "$FILE_CONTENT" ]; then + FILE_CONTEXT+=" + + === ARQUIVO: $file === + $FILE_CONTENT + " + fi + fi + done + + PROMPT="Voce e um engenheiro senior revisando esta PR. Analise SOMENTE o diff abaixo. + + REGRAS OBRIGATORIAS: + - Analise APENAS linhas marcadas com + (adicionadas) e - (removidas) no diff + - NUNCA mencione codigo, arquivos ou linhas que NAO aparecem no diff + - Se uma linha foi removida (-) e substituida por outra (+), o bug antigo ja foi corrigido — NAO reporte + - Cite arquivo e numero de linha EXATOS do diff. Se nao conseguir citar, nao reporte + - Se nao houver problemas reais: responda apenas 'Sem problemas detectados.' + - Resposta em portugues, concisa, sem introducao + + ANTI-ALUCINACAO (CRITICO): + - Para CADA problema reportado, voce DEVE citar o texto EXATO da linha do diff que contem o bug + - Formato obrigatorio: [SEVERIDADE] arquivo:linha — COPIE A LINHA EXATA AQUI — descricao + fix + - Se voce nao consegue copiar a linha exata do diff, o problema NAO EXISTE — nao reporte + - NUNCA invente valores numericos, nomes de variaveis ou operadores que nao aparecem no diff + - Se o diff mostra 'x / 1024', NAO reporte 'x / 10.24' — cite o que esta escrito, nao o que voce acha que deveria ser + - Duvida = nao reporte. Apenas problemas com evidencia direta no diff + + PR: ${PR_TITLE} + Repo: ${REPO_NAME} + + Diff: + ${DIFF} + + Contexto dos arquivos modificados (conteudo completo): + ${FILE_CONTEXT} + + Categorias para analise: + 1. Bugs reais - erros logicos, referencias quebradas, falhas silenciosas nas linhas NOVAS (+) + 2. Seguranca - secrets hardcoded, permissoes excessivas, SQL injection, XSS nas linhas NOVAS (+) + 3. Qualidade - apenas problemas graves nas linhas NOVAS (+) + + Formato por problema: [CRITICAL|HIGH|MEDIUM] arquivo:linha — \"texto exato da linha\" — descricao curta + fix sugerido" + + PAYLOAD=$(jq -n \ + --arg content "$PROMPT" \ + '{ + model: "minimax/minimax-m2.5", + max_tokens: 1500, + temperature: 0, + messages: [{role: "user", content: $content}] + }') + + RESPONSE=$(echo "$PAYLOAD" | curl -s -X POST \ + "https://openrouter.ai/api/v1/chat/completions" \ + -H "Authorization: Bearer $OPENROUTER_API_KEY" \ + -H "Content-Type: application/json" \ + -H "HTTP-Referer: https://github.com/$REPO_NAME" \ + -H "X-Title: Workspace PR Review" \ + -d @-) + + REVIEW=$(echo "$RESPONSE" | jq -r '.choices[0].message.content // empty') + REVIEW_MODEL="minimax/minimax-m2.5" + + if [ -z "$REVIEW" ]; then + echo "minimax-m2.5 falhou, tentando fallback minimax/minimax-m2.7..." + PAYLOAD_FB=$(jq -n \ + --arg content "$PROMPT" \ + '{ + model: "minimax/minimax-m2.7", + max_tokens: 1500, + temperature: 0, + messages: [{role: "user", content: $content}] + }') + RESPONSE=$(echo "$PAYLOAD_FB" | curl -s -X POST \ + "https://openrouter.ai/api/v1/chat/completions" \ + -H "Authorization: Bearer $OPENROUTER_API_KEY" \ + -H "Content-Type: application/json" \ + -H "HTTP-Referer: https://github.com/$REPO_NAME" \ + -H "X-Title: Workspace PR Review" \ + -d @-) + REVIEW=$(echo "$RESPONSE" | jq -r '.choices[0].message.content // empty') + REVIEW_MODEL="minimax/minimax-m2.7 (fallback)" + fi + + if [ -z "$REVIEW" ]; then + ERROR=$(echo "$RESPONSE" | jq -r '.error.message // "resposta vazia"') + REVIEW="Review indisponivel: $ERROR" + fi + + if [ "$REVIEW" != "Sem problemas detectados." ] && [ -n "$REVIEW" ]; then + VERIFY_PROMPT="Voce e um verificador de review de codigo. Abaixo esta um diff e uma lista de problemas reportados. + Sua tarefa: para CADA problema, verificar se a linha citada realmente existe no diff EXATAMENTE como descrito. + - Se o problema cita um valor ou operador que NAO aparece no diff, marque como FALSO POSITIVO + - Se a linha citada existe no diff e o problema e legitimo, marque como CONFIRMADO + - Se nao tem certeza, marque como FALSO POSITIVO + + Responda APENAS com a lista filtrada, mantendo o mesmo formato. Remova todos os falsos positivos. + Se sobrar zero problemas, responda apenas 'Sem problemas detectados.' + + Diff: + ${DIFF} + + Problemas reportados: + ${REVIEW}" + + VERIFY_PAYLOAD=$(jq -n \ + --arg content "$VERIFY_PROMPT" \ + '{ + model: "minimax/minimax-m2.5", + max_tokens: 1500, + temperature: 0, + messages: [{role: "user", content: $content}] + }') + + VERIFY_RESPONSE=$(echo "$VERIFY_PAYLOAD" | curl -s -X POST \ + "https://openrouter.ai/api/v1/chat/completions" \ + -H "Authorization: Bearer $OPENROUTER_API_KEY" \ + -H "Content-Type: application/json" \ + -H "HTTP-Referer: https://github.com/$REPO_NAME" \ + -H "X-Title: Workspace PR Review Verification" \ + -d @-) + + VERIFIED_REVIEW=$(echo "$VERIFY_RESPONSE" | jq -r '.choices[0].message.content // empty') + if [ -n "$VERIFIED_REVIEW" ]; then + REVIEW="$VERIFIED_REVIEW" + REVIEW_MODEL="$REVIEW_MODEL + verification pass" + fi + fi + + gh pr comment "$PR_NUMBER" --repo "$REPO_NAME" --body "### Code Review Bot + + ${REVIEW} + + --- + _Modelo: ${REVIEW_MODEL} via OpenRouter | diff-sha: ${DIFF_HASH}_" \ No newline at end of file diff --git a/src/commands/plugin/ManagePlugins.tsx b/src/commands/plugin/ManagePlugins.tsx index 1b7d6ca4a7..535df68607 100644 --- a/src/commands/plugin/ManagePlugins.tsx +++ b/src/commands/plugin/ManagePlugins.tsx @@ -35,12 +35,11 @@ import { loadInstalledPluginsV2 } from '../../utils/plugins/installedPluginsMana import { getMarketplace } from '../../utils/plugins/marketplaceManager.js'; import { isMcpbSource, loadMcpbFile, type McpbNeedsConfigResult, type UserConfigValues } from '../../utils/plugins/mcpbHandler.js'; import { getPluginDataDirSize, pluginDataDirPath } from '../../utils/plugins/pluginDirectories.js'; -import { getFlaggedPlugins, markFlaggedPluginsSeen, removeFlaggedPlugin } from '../../utils/plugins/pluginFlagging.js'; +import { markFlaggedPluginsSeen, removeFlaggedPlugin } from '../../utils/plugins/pluginFlagging.js'; import { type PersistablePluginScope, parsePluginIdentifier } from '../../utils/plugins/pluginIdentifier.js'; import { loadAllPlugins } from '../../utils/plugins/pluginLoader.js'; import { loadPluginOptions, type PluginOptionSchema, savePluginOptions } from '../../utils/plugins/pluginOptionsStorage.js'; import { isPluginBlockedByPolicy } from '../../utils/plugins/pluginPolicy.js'; -import { getPluginEditableScopes } from '../../utils/plugins/pluginStartupCheck.js'; import { getSettings_DEPRECATED, getSettingsForSource, updateSettingsForSource } from '../../utils/settings/settings.js'; import { jsonParse } from '../../utils/slowOperations.js'; import { plural } from '../../utils/stringUtils.js'; @@ -51,6 +50,7 @@ import type { ViewState as ParentViewState } from './types.js'; import { UnifiedInstalledCell } from './UnifiedInstalledCell.js'; import type { UnifiedInstalledItem } from './unifiedTypes.js'; import { usePagination } from './usePagination.js'; +import { useUnifiedItems } from './useUnifiedItems.js'; type Props = { setViewState: (state: ParentViewState) => void; setResult: (result: string | null) => void; @@ -407,7 +407,6 @@ export function ManagePlugins({ const mcpClients = useAppState(s => s.mcp.clients); const mcpTools = useAppState(s_0 => s_0.mcp.tools); const pluginErrors = useAppState(s_1 => s_1.plugins.errors); - const flaggedPlugins = getFlaggedPlugins(); // Search state const [isSearchMode, setIsSearchModeRaw] = useState(false); @@ -509,277 +508,13 @@ export function ManagePlugins({ isActive: (viewState !== 'plugin-list' || !isSearchMode) && viewState !== 'confirm-project-uninstall' && !(typeof viewState === 'object' && viewState.type === 'confirm-data-cleanup') }); - // Helper to get MCP status - const getMcpStatus = (client: MCPServerConnection): 'connected' | 'disabled' | 'pending' | 'needs-auth' | 'failed' => { - if (client.type === 'connected') return 'connected'; - if (client.type === 'disabled') return 'disabled'; - if (client.type === 'pending') return 'pending'; - if (client.type === 'needs-auth') return 'needs-auth'; - return 'failed'; - }; - // Derive unified items from plugins and MCP servers - const unifiedItems = useMemo(() => { - const mergedSettings = getSettings_DEPRECATED(); - - // Build map of plugin name -> child MCPs - // Plugin MCPs have names like "plugin:pluginName:serverName" - const pluginMcpMap = new Map>(); - for (const client_0 of mcpClients) { - if (client_0.name.startsWith('plugin:')) { - const parts = client_0.name.split(':'); - if (parts.length >= 3) { - const pluginName = parts[1]!; - const serverName = parts.slice(2).join(':'); - const existing = pluginMcpMap.get(pluginName) || []; - existing.push({ - displayName: serverName, - client: client_0 - }); - pluginMcpMap.set(pluginName, existing); - } - } - } - - // Build plugin items (unsorted for now) - type PluginWithChildren = { - item: UnifiedInstalledItem & { - type: 'plugin'; - }; - originalScope: 'user' | 'project' | 'local' | 'managed' | 'builtin'; - childMcps: Array<{ - displayName: string; - client: MCPServerConnection; - }>; - }; - const pluginsWithChildren: PluginWithChildren[] = []; - for (const state of pluginStates) { - const pluginId = `${state.plugin.name}@${state.marketplace}`; - const isEnabled = mergedSettings?.enabledPlugins?.[pluginId] !== false; - const errors = pluginErrors.filter(e => 'plugin' in e && e.plugin === state.plugin.name || e.source === pluginId || e.source.startsWith(`${state.plugin.name}@`)); - - // Built-in plugins use 'builtin' scope; others look up from V2 data. - const originalScope = state.plugin.isBuiltin ? 'builtin' : state.scope || 'user'; - pluginsWithChildren.push({ - item: { - type: 'plugin', - id: pluginId, - name: state.plugin.name, - description: state.plugin.manifest.description, - marketplace: state.marketplace, - scope: originalScope, - isEnabled, - errorCount: errors.length, - errors, - plugin: state.plugin, - pendingEnable: state.pendingEnable, - pendingUpdate: state.pendingUpdate, - pendingToggle: pendingToggles.get(pluginId) - }, - originalScope, - childMcps: pluginMcpMap.get(state.plugin.name) || [] - }); - } - - // Find orphan errors (errors for plugins that failed to load entirely) - const matchedPluginIds = new Set(pluginsWithChildren.map(({ - item - }) => item.id)); - const matchedPluginNames = new Set(pluginsWithChildren.map(({ - item: item_0 - }) => item_0.name)); - const orphanErrorsBySource = new Map(); - for (const error of pluginErrors) { - if (matchedPluginIds.has(error.source) || 'plugin' in error && typeof error.plugin === 'string' && matchedPluginNames.has(error.plugin)) { - continue; - } - const existing_0 = orphanErrorsBySource.get(error.source) || []; - existing_0.push(error); - orphanErrorsBySource.set(error.source, existing_0); - } - const pluginScopes = getPluginEditableScopes(); - const failedPluginItems: UnifiedInstalledItem[] = []; - for (const [pluginId_0, errors_0] of orphanErrorsBySource) { - // Skip plugins that are already shown in the flagged section - if (pluginId_0 in flaggedPlugins) continue; - const parsed = parsePluginIdentifier(pluginId_0); - const pluginName_0 = parsed.name || pluginId_0; - const marketplace = parsed.marketplace || 'unknown'; - const rawScope = pluginScopes.get(pluginId_0); - // 'flag' is session-only (from --plugin-dir / flagSettings) and undefined - // means the plugin isn't in any settings source. Default both to 'user' - // since UnifiedInstalledItem doesn't have a 'flag' scope variant. - const scope = rawScope === 'flag' || rawScope === undefined ? 'user' : rawScope; - failedPluginItems.push({ - type: 'failed-plugin', - id: pluginId_0, - name: pluginName_0, - marketplace, - scope, - errorCount: errors_0.length, - errors: errors_0 - }); - } - - // Build standalone MCP items - const standaloneMcps: UnifiedInstalledItem[] = []; - for (const client_1 of mcpClients) { - if (client_1.name === 'ide') continue; - if (client_1.name.startsWith('plugin:')) continue; - standaloneMcps.push({ - type: 'mcp', - id: `mcp:${client_1.name}`, - name: client_1.name, - description: undefined, - scope: client_1.config.scope, - status: getMcpStatus(client_1), - client: client_1 - }); - } - - // Define scope order for display - const scopeOrder: Record = { - flagged: -1, - project: 0, - local: 1, - user: 2, - enterprise: 3, - managed: 4, - dynamic: 5, - builtin: 6 - }; - - // Build final list by merging plugins (with their child MCPs) and standalone MCPs - // Group by scope to avoid duplicate scope headers - const unified: UnifiedInstalledItem[] = []; - - // Create a map of scope -> items for proper merging - const itemsByScope = new Map(); - - // Add plugins with their child MCPs - for (const { - item: item_1, - originalScope: originalScope_0, - childMcps - } of pluginsWithChildren) { - const scope_0 = item_1.scope; - if (!itemsByScope.has(scope_0)) { - itemsByScope.set(scope_0, []); - } - itemsByScope.get(scope_0)!.push(item_1); - // Add child MCPs right after the plugin, indented (use original scope, not 'flagged'). - // Built-in plugins map to 'user' for display since MCP ConfigScope doesn't include 'builtin'. - for (const { - displayName, - client: client_2 - } of childMcps) { - const displayScope = originalScope_0 === 'builtin' ? 'user' : originalScope_0; - if (!itemsByScope.has(displayScope)) { - itemsByScope.set(displayScope, []); - } - itemsByScope.get(displayScope)!.push({ - type: 'mcp', - id: `mcp:${client_2.name}`, - name: displayName, - description: undefined, - scope: displayScope, - status: getMcpStatus(client_2), - client: client_2, - indented: true - }); - } - } - - // Add standalone MCPs to their respective scope groups - for (const mcp of standaloneMcps) { - const scope_1 = mcp.scope; - if (!itemsByScope.has(scope_1)) { - itemsByScope.set(scope_1, []); - } - itemsByScope.get(scope_1)!.push(mcp); - } - - // Add failed plugins to their respective scope groups - for (const failedPlugin of failedPluginItems) { - const scope_2 = failedPlugin.scope; - if (!itemsByScope.has(scope_2)) { - itemsByScope.set(scope_2, []); - } - itemsByScope.get(scope_2)!.push(failedPlugin); - } - - // Add flagged (delisted) plugins from user settings. - // Reason/text are looked up from the cached security messages file. - for (const [pluginId_1, entry] of Object.entries(flaggedPlugins)) { - const parsed_0 = parsePluginIdentifier(pluginId_1); - const pluginName_1 = parsed_0.name || pluginId_1; - const marketplace_0 = parsed_0.marketplace || 'unknown'; - if (!itemsByScope.has('flagged')) { - itemsByScope.set('flagged', []); - } - itemsByScope.get('flagged')!.push({ - type: 'flagged-plugin', - id: pluginId_1, - name: pluginName_1, - marketplace: marketplace_0, - scope: 'flagged', - reason: 'delisted', - text: 'Removed from marketplace', - flaggedAt: entry.flaggedAt - }); - } - - // Sort scopes and build final list - const sortedScopes = [...itemsByScope.keys()].sort((a, b) => (scopeOrder[a] ?? 99) - (scopeOrder[b] ?? 99)); - for (const scope_3 of sortedScopes) { - const items = itemsByScope.get(scope_3)!; - - // Separate items into plugin groups (with their child MCPs) and standalone MCPs - // This preserves parent-child relationships that would be broken by naive sorting - const pluginGroups: UnifiedInstalledItem[][] = []; - const standaloneMcpsInScope: UnifiedInstalledItem[] = []; - let i = 0; - while (i < items.length) { - const item_2 = items[i]!; - if (item_2.type === 'plugin' || item_2.type === 'failed-plugin' || item_2.type === 'flagged-plugin') { - // Collect the plugin and its child MCPs as a group - const group: UnifiedInstalledItem[] = [item_2]; - i++; - // Look ahead for indented child MCPs - let nextItem = items[i]; - while (nextItem?.type === 'mcp' && nextItem.indented) { - group.push(nextItem); - i++; - nextItem = items[i]; - } - pluginGroups.push(group); - } else if (item_2.type === 'mcp' && !item_2.indented) { - // Standalone MCP (not a child of a plugin) - standaloneMcpsInScope.push(item_2); - i++; - } else { - // Skip orphaned indented MCPs (shouldn't happen) - i++; - } - } - - // Sort plugin groups by the plugin name (first item in each group) - pluginGroups.sort((a_0, b_0) => a_0[0]!.name.localeCompare(b_0[0]!.name)); - - // Sort standalone MCPs by name - standaloneMcpsInScope.sort((a_1, b_1) => a_1.name.localeCompare(b_1.name)); - - // Build final list: plugins (with their children) first, then standalone MCPs - for (const group_0 of pluginGroups) { - unified.push(...group_0); - } - unified.push(...standaloneMcpsInScope); - } - return unified; - }, [pluginStates, mcpClients, pluginErrors, pendingToggles, flaggedPlugins]); + const unifiedItems = useUnifiedItems({ + pluginStates, + mcpClients, + pluginErrors, + pendingToggles, + }); // Mark flagged plugins as seen when the Installed view renders them. // After 48 hours from seenAt, they auto-clear on next load. diff --git a/src/commands/plugin/unifiedTypes.ts b/src/commands/plugin/unifiedTypes.ts new file mode 100644 index 0000000000..2d89013bf7 --- /dev/null +++ b/src/commands/plugin/unifiedTypes.ts @@ -0,0 +1,59 @@ +import type { ConfigScope } from '../../services/mcp/types.js' +import type { MCPServerConnection } from '../../services/mcp/types.js' +import type { PluginError } from '../../types/plugin.js' +import type { LoadedPlugin } from '../../types/plugin.js' +import type { PersistablePluginScope } from '../../utils/plugins/pluginIdentifier.js' + +export type PluginItem = { + type: 'plugin' + id: string + name: string + description: string | undefined + marketplace: string + scope: PersistablePluginScope | 'builtin' + isEnabled: boolean + errorCount: number + errors: PluginError[] + plugin: LoadedPlugin + pendingEnable?: boolean + pendingUpdate?: boolean + pendingToggle?: 'will-enable' | 'will-disable' +} + +export type FailedPluginItem = { + type: 'failed-plugin' + id: string + name: string + marketplace: string + scope: PersistablePluginScope + errorCount: number + errors: PluginError[] +} + +export type McpItem = { + type: 'mcp' + id: string + name: string + description: string | undefined + scope: ConfigScope | 'user' + status: 'connected' | 'disabled' | 'pending' | 'needs-auth' | 'failed' + client: MCPServerConnection + indented?: boolean +} + +export type FlaggedPluginItem = { + type: 'flagged-plugin' + id: string + name: string + marketplace: string + scope: 'flagged' + reason: string + text: string + flaggedAt: string +} + +export type UnifiedInstalledItem = + | PluginItem + | FailedPluginItem + | McpItem + | FlaggedPluginItem \ No newline at end of file diff --git a/src/commands/plugin/useUnifiedItems.ts b/src/commands/plugin/useUnifiedItems.ts new file mode 100644 index 0000000000..5f579d58f7 --- /dev/null +++ b/src/commands/plugin/useUnifiedItems.ts @@ -0,0 +1,266 @@ +import { useMemo } from 'react' +import type { MCPServerConnection } from '../../services/mcp/types.js' +import type { PluginError } from '../../types/plugin.js' +import type { LoadedPlugin } from '../../types/plugin.js' +import { getSettings_DEPRECATED } from '../../utils/settings/settings.js' +import { getFlaggedPlugins, type FlaggedPlugin } from '../../utils/plugins/pluginFlagging.js' +import { getPluginEditableScopes } from '../../utils/plugins/pluginStartupCheck.js' +import { type PersistablePluginScope, parsePluginIdentifier } from '../../utils/plugins/pluginIdentifier.js' +import type { UnifiedInstalledItem } from './unifiedTypes.js' + +type PluginState = { + plugin: LoadedPlugin + marketplace: string + scope?: 'user' | 'project' | 'local' | 'managed' | 'builtin' + pendingEnable?: boolean + pendingUpdate?: boolean +} + +type UseUnifiedItemsInput = { + pluginStates: PluginState[] + mcpClients: MCPServerConnection[] + pluginErrors: PluginError[] + pendingToggles: Map +} + +function getMcpStatus(client: MCPServerConnection): 'connected' | 'disabled' | 'pending' | 'needs-auth' | 'failed' { + if (client.type === 'connected') return 'connected' + if (client.type === 'disabled') return 'disabled' + if (client.type === 'pending') return 'pending' + if (client.type === 'needs-auth') return 'needs-auth' + return 'failed' +} + +export function useUnifiedItems({ + pluginStates, + mcpClients, + pluginErrors, + pendingToggles, +}: UseUnifiedItemsInput): UnifiedInstalledItem[] { + const flaggedPlugins = getFlaggedPlugins() + + return useMemo(() => { + const mergedSettings = getSettings_DEPRECATED() + + // Build map of plugin name -> child MCPs + const pluginMcpMap = new Map>() + for (const client of mcpClients) { + if (client.name.startsWith('plugin:')) { + const parts = client.name.split(':') + if (parts.length >= 3) { + const pluginName = parts[1]! + const serverName = parts.slice(2).join(':') + const existing = pluginMcpMap.get(pluginName) || [] + existing.push({ displayName: serverName, client }) + pluginMcpMap.set(pluginName, existing) + } + } + } + + // Build plugin items + type PluginWithChildren = { + item: UnifiedInstalledItem & { type: 'plugin' } + originalScope: 'user' | 'project' | 'local' | 'managed' | 'builtin' + childMcps: Array<{ displayName: string; client: MCPServerConnection }> + } + const pluginsWithChildren: PluginWithChildren[] = [] + for (const state of pluginStates) { + const pluginId = `${state.plugin.name}@${state.marketplace}` + const isEnabled = mergedSettings?.enabledPlugins?.[pluginId] !== false + const errors = pluginErrors.filter( + e => ('plugin' in e && e.plugin === state.plugin.name) || e.source === pluginId || e.source.startsWith(`${state.plugin.name}@`), + ) + + const originalScope = state.plugin.isBuiltin ? 'builtin' : state.scope || 'user' + pluginsWithChildren.push({ + item: { + type: 'plugin', + id: pluginId, + name: state.plugin.name, + description: state.plugin.manifest.description, + marketplace: state.marketplace, + scope: originalScope, + isEnabled, + errorCount: errors.length, + errors, + plugin: state.plugin, + pendingEnable: state.pendingEnable, + pendingUpdate: state.pendingUpdate, + pendingToggle: pendingToggles.get(pluginId), + }, + originalScope, + childMcps: pluginMcpMap.get(state.plugin.name) || [], + }) + } + + // Find orphan errors + const matchedPluginIds = new Set(pluginsWithChildren.map(({ item }) => item.id)) + const matchedPluginNames = new Set(pluginsWithChildren.map(({ item }) => item.name)) + const orphanErrorsBySource = new Map() + for (const error of pluginErrors) { + if (matchedPluginIds.has(error.source) || ('plugin' in error && typeof error.plugin === 'string' && matchedPluginNames.has(error.plugin))) { + continue + } + const existing = orphanErrorsBySource.get(error.source) || [] + existing.push(error) + orphanErrorsBySource.set(error.source, existing) + } + + const pluginScopes = getPluginEditableScopes() + const failedPluginItems: UnifiedInstalledItem[] = [] + for (const [pluginId, errors] of orphanErrorsBySource) { + if (pluginId in flaggedPlugins) continue + const parsed = parsePluginIdentifier(pluginId) + const pluginName = parsed.name || pluginId + const marketplace = parsed.marketplace || 'unknown' + const rawScope = pluginScopes.get(pluginId) + const scope: PersistablePluginScope = rawScope === 'flag' || rawScope === undefined ? 'user' : rawScope + failedPluginItems.push({ + type: 'failed-plugin', + id: pluginId, + name: pluginName, + marketplace, + scope, + errorCount: errors.length, + errors, + }) + } + + // Build standalone MCP items + const standaloneMcps: UnifiedInstalledItem[] = [] + for (const client of mcpClients) { + if (client.name === 'ide') continue + if (client.name.startsWith('plugin:')) continue + standaloneMcps.push({ + type: 'mcp', + id: `mcp:${client.name}`, + name: client.name, + description: undefined, + scope: client.config.scope, + status: getMcpStatus(client), + client, + }) + } + + // Define scope order for display + const scopeOrder: Record = { + flagged: -1, + project: 0, + local: 1, + user: 2, + enterprise: 3, + managed: 4, + dynamic: 5, + builtin: 6, + } + + // Build final list by merging plugins (with child MCPs) and standalone MCPs + const unified: UnifiedInstalledItem[] = [] + const itemsByScope = new Map() + + // Add plugins with their child MCPs + for (const { item, originalScope, childMcps } of pluginsWithChildren) { + if (!itemsByScope.has(item.scope)) { + itemsByScope.set(item.scope, []) + } + itemsByScope.get(item.scope)!.push(item) + for (const { displayName, client } of childMcps) { + const displayScope = originalScope === 'builtin' ? 'user' : originalScope + if (!itemsByScope.has(displayScope)) { + itemsByScope.set(displayScope, []) + } + itemsByScope.get(displayScope)!.push({ + type: 'mcp', + id: `mcp:${client.name}`, + name: displayName, + description: undefined, + scope: displayScope, + status: getMcpStatus(client), + client, + indented: true, + }) + } + } + + // Add standalone MCPs + for (const mcp of standaloneMcps) { + if (!itemsByScope.has(mcp.scope)) { + itemsByScope.set(mcp.scope, []) + } + itemsByScope.get(mcp.scope)!.push(mcp) + } + + // Add failed plugins + for (const failedPlugin of failedPluginItems) { + if (!itemsByScope.has(failedPlugin.scope)) { + itemsByScope.set(failedPlugin.scope, []) + } + itemsByScope.get(failedPlugin.scope)!.push(failedPlugin) + } + + // Add flagged plugins + for (const [pluginId, entry] of Object.entries(flaggedPlugins)) { + const parsed = parsePluginIdentifier(pluginId) + const pluginName = parsed.name || pluginId + const marketplace = parsed.marketplace || 'unknown' + if (!itemsByScope.has('flagged')) { + itemsByScope.set('flagged', []) + } + itemsByScope.get('flagged')!.push({ + type: 'flagged-plugin', + id: pluginId, + name: pluginName, + marketplace, + scope: 'flagged', + reason: 'delisted', + text: 'Removed from marketplace', + flaggedAt: entry.flaggedAt, + }) + } + + // Sort scopes and build final list + const sortedScopes = [...itemsByScope.keys()].sort( + (a, b) => (scopeOrder[a] ?? 99) - (scopeOrder[b] ?? 99), + ) + for (const scope of sortedScopes) { + const items = itemsByScope.get(scope)! + + // Separate items into plugin groups and standalone MCPs + const pluginGroups: UnifiedInstalledItem[][] = [] + const standaloneMcpsInScope: UnifiedInstalledItem[] = [] + let i = 0 + while (i < items.length) { + const item = items[i]! + if (item.type === 'plugin' || item.type === 'failed-plugin' || item.type === 'flagged-plugin') { + const group: UnifiedInstalledItem[] = [item] + i++ + let nextItem = items[i] + while (nextItem?.type === 'mcp' && nextItem.indented) { + group.push(nextItem) + i++ + nextItem = items[i] + } + pluginGroups.push(group) + } else if (item.type === 'mcp' && !item.indented) { + standaloneMcpsInScope.push(item) + i++ + } else { + i++ + } + } + + pluginGroups.sort((a, b) => a[0]!.name.localeCompare(b[0]!.name)) + standaloneMcpsInScope.sort((a, b) => a.name.localeCompare(b.name)) + + for (const group of pluginGroups) { + unified.push(...group) + } + unified.push(...standaloneMcpsInScope) + } + + return unified + }, [pluginStates, mcpClients, pluginErrors, pendingToggles, flaggedPlugins]) +} \ No newline at end of file diff --git a/src/components/ProviderManager.tsx b/src/components/ProviderManager.tsx index 0fe8a8a711..23fc0e5629 100644 --- a/src/components/ProviderManager.tsx +++ b/src/components/ProviderManager.tsx @@ -9,7 +9,7 @@ import { readCodexCredentialsAsync, } from '../utils/codexCredentials.js' import { isBareMode, isEnvTruthy } from '../utils/envUtils.js' -import { getPrimaryModel, hasMultipleModels, parseModelList } from '../utils/providerModels.js' +import { getPrimaryModel, hasMultipleModels } from '../utils/providerModels.js' import { applySavedProfileToCurrentSession, buildCodexOAuthProfileEnv, @@ -33,7 +33,6 @@ import { GITHUB_MODELS_HYDRATED_ENV_MARKER, hydrateGithubModelsTokenFromSecureStorage, readGithubModelsToken, - readGithubModelsTokenAsync, } from '../utils/githubModelsCredentials.js' import { hasLocalOllama, @@ -44,6 +43,25 @@ import { recommendOllamaModel, } from '../utils/providerRecommendation.js' import { updateSettingsForSource } from '../utils/settings/settings.js' +import { + type DraftField, + type ProviderDraft, + type GithubCredentialSource, + GITHUB_PROVIDER_ID, + GITHUB_PROVIDER_LABEL, + GITHUB_PROVIDER_DEFAULT_MODEL, + GITHUB_PROVIDER_DEFAULT_BASE_URL, + toDraft, + presetToDraft, + profileSummary, + getGithubCredentialSourceFromEnv, + resolveGithubCredentialSource, + isGithubProviderAvailable, + getGithubProviderModel, + getGithubProviderSummary, + findCodexOAuthProfile, + isCodexOAuthProfile, +} from './providerManagerHelpers.js' import { type OptionWithDescription, Select, @@ -74,10 +92,6 @@ type Screen = | 'select-edit' | 'select-delete' -type DraftField = 'name' | 'baseUrl' | 'model' | 'apiKey' - -type ProviderDraft = Record - type OllamaSelectionState = | { state: 'idle' } | { state: 'loading' } @@ -122,123 +136,9 @@ const FORM_STEPS: Array<{ }, ] -const GITHUB_PROVIDER_ID = '__github_models__' -const GITHUB_PROVIDER_LABEL = 'GitHub Models' -const GITHUB_PROVIDER_DEFAULT_MODEL = 'github:copilot' -const GITHUB_PROVIDER_DEFAULT_BASE_URL = 'https://models.github.ai/inference' const CODEX_OAUTH_PROVIDER_NAME = 'Codex OAuth' const CODEX_OAUTH_PROVIDER_MODEL = 'codexplan' -type GithubCredentialSource = 'stored' | 'env' | 'none' - -function toDraft(profile: ProviderProfile): ProviderDraft { - return { - name: profile.name, - baseUrl: profile.baseUrl, - model: profile.model, - apiKey: profile.apiKey ?? '', - } -} - -function presetToDraft(preset: ProviderPreset): ProviderDraft { - const defaults = getProviderPresetDefaults(preset) - return { - name: defaults.name, - baseUrl: defaults.baseUrl, - model: defaults.model, - apiKey: defaults.apiKey ?? '', - } -} - -function profileSummary(profile: ProviderProfile, isActive: boolean): string { - const activeSuffix = isActive ? ' (active)' : '' - const keyInfo = profile.apiKey ? 'key set' : 'no key' - const providerKind = - profile.provider === 'anthropic' ? 'anthropic' : 'openai-compatible' - const models = parseModelList(profile.model) - const modelDisplay = - models.length <= 3 - ? models.join(', ') - : `${models[0]}, ${models[1]} + ${models.length - 2} more` - return `${providerKind} · ${profile.baseUrl} · ${modelDisplay} · ${keyInfo}${activeSuffix}` -} - -function getGithubCredentialSourceFromEnv( - processEnv: NodeJS.ProcessEnv = process.env, -): GithubCredentialSource { - if (processEnv.GITHUB_TOKEN?.trim() || processEnv.GH_TOKEN?.trim()) { - return 'env' - } - return 'none' -} - -async function resolveGithubCredentialSource( - processEnv: NodeJS.ProcessEnv = process.env, -): Promise { - const envSource = getGithubCredentialSourceFromEnv(processEnv) - if (envSource !== 'none') { - return envSource - } - - if (await readGithubModelsTokenAsync()) { - return 'stored' - } - - return 'none' -} - -function isGithubProviderAvailable( - credentialSource: GithubCredentialSource, - processEnv: NodeJS.ProcessEnv = process.env, -): boolean { - if (isEnvTruthy(processEnv.CLAUDE_CODE_USE_GITHUB)) { - return true - } - return credentialSource !== 'none' -} - -function getGithubProviderModel( - processEnv: NodeJS.ProcessEnv = process.env, -): string { - if (isEnvTruthy(processEnv.CLAUDE_CODE_USE_GITHUB)) { - return processEnv.OPENAI_MODEL?.trim() || GITHUB_PROVIDER_DEFAULT_MODEL - } - return GITHUB_PROVIDER_DEFAULT_MODEL -} - -function getGithubProviderSummary( - isActive: boolean, - credentialSource: GithubCredentialSource, - processEnv: NodeJS.ProcessEnv = process.env, -): string { - const credentialSummary = - credentialSource === 'stored' - ? 'token stored' - : credentialSource === 'env' - ? 'token via env' - : 'no token found' - const activeSuffix = isActive ? ' (active)' : '' - return `github-models · ${GITHUB_PROVIDER_DEFAULT_BASE_URL} · ${getGithubProviderModel(processEnv)} · ${credentialSummary}${activeSuffix}` -} - -function findCodexOAuthProfile( - profiles: ProviderProfile[], - profileId?: string, -): ProviderProfile | undefined { - if (!profileId) { - return undefined - } - - return profiles.find(profile => profile.id === profileId) -} - -function isCodexOAuthProfile( - profile: ProviderProfile | null | undefined, - profileId?: string, -): boolean { - return Boolean(profile && profileId && profile.id === profileId) -} - function CodexOAuthSetup({ onBack, onConfigured, diff --git a/src/components/providerManagerHelpers.ts b/src/components/providerManagerHelpers.ts new file mode 100644 index 0000000000..f64d9df93c --- /dev/null +++ b/src/components/providerManagerHelpers.ts @@ -0,0 +1,128 @@ +import type { ProviderProfile } from '../utils/config.js' +import { + clearGithubModelsToken, + readGithubModelsTokenAsync, +} from '../utils/githubModelsCredentials.js' +import { isEnvTruthy } from '../utils/envUtils.js' +import { getProviderPresetDefaults } from '../utils/providerProfiles.js' +import { parseModelList } from '../utils/providerModels.js' +import type { ProviderPreset, ProviderProfileInput } from '../utils/providerProfiles.js' + +export type DraftField = 'name' | 'baseUrl' | 'model' | 'apiKey' + +export type ProviderDraft = Record + +export type GithubCredentialSource = 'stored' | 'env' | 'none' + +export const GITHUB_PROVIDER_ID = '__github_models__' +export const GITHUB_PROVIDER_LABEL = 'GitHub Models' +export const GITHUB_PROVIDER_DEFAULT_MODEL = 'github:copilot' +export const GITHUB_PROVIDER_DEFAULT_BASE_URL = 'https://models.github.ai/inference' + +export function toDraft(profile: ProviderProfile): ProviderDraft { + return { + name: profile.name, + baseUrl: profile.baseUrl, + model: profile.model, + apiKey: profile.apiKey ?? '', + } +} + +export function presetToDraft(preset: ProviderPreset): ProviderDraft { + const defaults = getProviderPresetDefaults(preset) + return { + name: defaults.name, + baseUrl: defaults.baseUrl, + model: defaults.model, + apiKey: defaults.apiKey ?? '', + } +} + +export function profileSummary(profile: ProviderProfile, isActive: boolean): string { + const activeSuffix = isActive ? ' (active)' : '' + const keyInfo = profile.apiKey ? 'key set' : 'no key' + const providerKind = + profile.provider === 'anthropic' ? 'anthropic' : 'openai-compatible' + const models = parseModelList(profile.model) + const modelDisplay = + models.length <= 3 + ? models.join(', ') + : `${models[0]}, ${models[1]} + ${models.length - 2} more` + return `${providerKind} · ${profile.baseUrl} · ${modelDisplay} · ${keyInfo}${activeSuffix}` +} + +export function getGithubCredentialSourceFromEnv( + processEnv: NodeJS.ProcessEnv = process.env, +): GithubCredentialSource { + if (processEnv.GITHUB_TOKEN?.trim() || processEnv.GH_TOKEN?.trim()) { + return 'env' + } + return 'none' +} + +export async function resolveGithubCredentialSource( + processEnv: NodeJS.ProcessEnv = process.env, +): Promise { + const envSource = getGithubCredentialSourceFromEnv(processEnv) + if (envSource !== 'none') { + return envSource + } + + if (await readGithubModelsTokenAsync()) { + return 'stored' + } + + return 'none' +} + +export function isGithubProviderAvailable( + credentialSource: GithubCredentialSource, + processEnv: NodeJS.ProcessEnv = process.env, +): boolean { + if (isEnvTruthy(processEnv.CLAUDE_CODE_USE_GITHUB)) { + return true + } + return credentialSource !== 'none' +} + +export function getGithubProviderModel( + processEnv: NodeJS.ProcessEnv = process.env, +): string { + if (isEnvTruthy(processEnv.CLAUDE_CODE_USE_GITHUB)) { + return processEnv.OPENAI_MODEL?.trim() || GITHUB_PROVIDER_DEFAULT_MODEL + } + return GITHUB_PROVIDER_DEFAULT_MODEL +} + +export function getGithubProviderSummary( + isActive: boolean, + credentialSource: GithubCredentialSource, + processEnv: NodeJS.ProcessEnv = process.env, +): string { + const credentialSummary = + credentialSource === 'stored' + ? 'token stored' + : credentialSource === 'env' + ? 'token via env' + : 'no token found' + const activeSuffix = isActive ? ' (active)' : '' + return `github-models · ${GITHUB_PROVIDER_DEFAULT_BASE_URL} · ${getGithubProviderModel(processEnv)} · ${credentialSummary}${activeSuffix}` +} + +export function findCodexOAuthProfile( + profiles: ProviderProfile[], + profileId?: string, +): ProviderProfile | undefined { + if (!profileId) { + return undefined + } + + return profiles.find(profile => profile.id === profileId) +} + +export function isCodexOAuthProfile( + profile: ProviderProfile | null | undefined, + profileId?: string, +): boolean { + return Boolean(profile && profileId && profile.id === profileId) +} \ No newline at end of file diff --git a/src/hooks/typeaheadHelpers.ts b/src/hooks/typeaheadHelpers.ts new file mode 100644 index 0000000000..cc41d1a6bd --- /dev/null +++ b/src/hooks/typeaheadHelpers.ts @@ -0,0 +1,291 @@ +import type { Command } from '../commands.js' +import type { SuggestionItem } from '../components/PromptInput/PromptInputFooterSuggestions.js' +import { logEvent } from '../services/analytics/index.js' +import { type ShellCompletionType, getShellCompletions } from '../utils/bash/shellCompletion.js' +import { isCommandInput } from '../utils/suggestions/commandSuggestions.js' + +// Unicode-aware character class for file path tokens: +// \p{L} = letters (CJK, Latin, Cyrillic, etc.) +// \p{N} = numbers (incl. fullwidth) +// \p{M} = combining marks (macOS NFD accents, Devanagari vowel signs) +export const AT_TOKEN_HEAD_RE = /^@[\p{L}\p{N}\p{M}_\-./\\()[\]~:]*/u +export const PATH_CHAR_HEAD_RE = /^[\p{L}\p{N}\p{M}_\-./\\()[\]~:]+/u +export const TOKEN_WITH_AT_RE = /(@[\p{L}\p{N}\p{M}_\-./\\()[\]~:]*|[\p{L}\p{N}\p{M}_\-./\\()[\]~:]+)$/u +export const TOKEN_WITHOUT_AT_RE = /[\p{L}\p{N}\p{M}_\-./\\()[\]~:]+$/u +export const HAS_AT_SYMBOL_RE = /(^|\s)@([\p{L}\p{N}\p{M}_\-./\\()[\]~:]*|"[^"]*"?)$/u +export const HASH_CHANNEL_RE = /(^|\s)#([a-z0-9][a-z0-9_-]*)$/ +export const DM_MEMBER_RE = /(^|\s)@[\w-]*$/ + +// Type guard for path completion metadata +export function isPathMetadata(metadata: unknown): metadata is { + type: 'directory' | 'file' +} { + return typeof metadata === 'object' && metadata !== null && 'type' in metadata && (metadata.type === 'directory' || metadata.type === 'file') +} + +// Helper to determine selectedSuggestion when updating suggestions +export function getPreservedSelection(prevSuggestions: SuggestionItem[], prevSelection: number, newSuggestions: SuggestionItem[]): number { + // No new suggestions + if (newSuggestions.length === 0) { + return -1 + } + + // No previous selection + if (prevSelection < 0) { + return 0 + } + + // Get the previously selected item + const prevSelectedItem = prevSuggestions[prevSelection] + if (!prevSelectedItem) { + return 0 + } + + // Try to find the same item in the new list by ID + const newIndex = newSuggestions.findIndex(item => item.id === prevSelectedItem.id) + + // Return the new index if found, otherwise default to 0 + return newIndex >= 0 ? newIndex : 0 +} + +export function buildResumeInputFromSuggestion(suggestion: SuggestionItem): string { + const metadata = suggestion.metadata as { + sessionId: string + } | undefined + return metadata?.sessionId ? `/resume ${metadata.sessionId}` : `/resume ${suggestion.displayText}` +} + +/** + * Extract search token from a completion token by removing @ prefix and quotes + * @param completionToken The completion token + * @returns The search token with @ and quotes removed + */ +export function extractSearchToken(completionToken: { + token: string + isQuoted?: boolean +}): string { + if (completionToken.isQuoted) { + // Remove @" prefix and optional closing " + return completionToken.token.slice(2).replace(/"$/, '') + } else if (completionToken.token.startsWith('@')) { + return completionToken.token.substring(1) + } else { + return completionToken.token + } +} + +/** + * Format a replacement value with proper @ prefix and quotes based on context + * @param options Configuration for formatting + * @param options.displayText The text to display + * @param options.mode The current mode (bash or prompt) + * @param options.hasAtPrefix Whether the original token has @ prefix + * @param options.needsQuotes Whether the text needs quotes (contains spaces) + * @param options.isQuoted Whether the original token was already quoted (user typed @"...) + * @param options.isComplete Whether this is a complete suggestion (adds trailing space) + * @returns The formatted replacement value + */ +export function formatReplacementValue(options: { + displayText: string + mode: string + hasAtPrefix: boolean + needsQuotes: boolean + isQuoted?: boolean + isComplete: boolean +}): string { + const { + displayText, + mode, + hasAtPrefix, + needsQuotes, + isQuoted, + isComplete + } = options + const space = isComplete ? ' ' : '' + if (isQuoted || needsQuotes) { + // Use quoted format + return mode === 'bash' ? `"${displayText}"${space}` : `@"${displayText}"${space}` + } else if (hasAtPrefix) { + return mode === 'bash' ? `${displayText}${space}` : `@${displayText}${space}` + } else { + return displayText + } +} + +/** + * Apply a shell completion suggestion by replacing the current word + */ +export function applyShellSuggestion(suggestion: SuggestionItem, input: string, cursorOffset: number, onInputChange: (value: string) => void, setCursorOffset: (offset: number) => void, completionType: ShellCompletionType | undefined): void { + const beforeCursor = input.slice(0, cursorOffset) + const lastSpaceIndex = beforeCursor.lastIndexOf(' ') + const wordStart = lastSpaceIndex + 1 + + // Prepare the replacement text based on completion type + let replacementText: string + if (completionType === 'variable') { + replacementText = '$' + suggestion.displayText + ' ' + } else if (completionType === 'command') { + replacementText = suggestion.displayText + ' ' + } else { + replacementText = suggestion.displayText + } + const newInput = input.slice(0, wordStart) + replacementText + input.slice(cursorOffset) + onInputChange(newInput) + setCursorOffset(wordStart + replacementText.length) +} + +export function applyTriggerSuggestion(suggestion: SuggestionItem, input: string, cursorOffset: number, triggerRe: RegExp, onInputChange: (value: string) => void, setCursorOffset: (offset: number) => void): void { + const m = input.slice(0, cursorOffset).match(triggerRe) + if (!m || m.index === undefined) return + const prefixStart = m.index + (m[1]?.length ?? 0) + const before = input.slice(0, prefixStart) + const newInput = before + suggestion.displayText + ' ' + input.slice(cursorOffset) + onInputChange(newInput) + setCursorOffset(before.length + suggestion.displayText.length + 1) +} + +let currentShellCompletionAbortController: AbortController | null = null + +/** + * Generate bash shell completion suggestions + */ +export async function generateBashSuggestions(input: string, cursorOffset: number): Promise { + try { + if (currentShellCompletionAbortController) { + currentShellCompletionAbortController.abort() + } + currentShellCompletionAbortController = new AbortController() + const suggestions = await getShellCompletions(input, cursorOffset, currentShellCompletionAbortController.signal) + return suggestions + } catch { + // Silent failure - don't break UX + logEvent('tengu_shell_completion_failed', {}) + return [] + } +} + +/** + * Apply a directory/path completion suggestion to the input + * Always adds @ prefix since we're replacing the entire token (including any existing @) + * + * @param input The current input text + * @param suggestionId The ID of the suggestion to apply + * @param tokenStartPos The start position of the token being replaced + * @param tokenLength The length of the token being replaced + * @param isDirectory Whether the suggestion is a directory (adds / suffix) or file (adds space) + * @returns Object with the new input text and cursor position + */ +export function applyDirectorySuggestion(input: string, suggestionId: string, tokenStartPos: number, tokenLength: number, isDirectory: boolean): { + newInput: string + cursorPos: number +} { + const suffix = isDirectory ? '/' : ' ' + const before = input.slice(0, tokenStartPos) + const after = input.slice(tokenStartPos + tokenLength) + // Always add @ prefix - if token already has it, we're replacing + // the whole token (including @) with @suggestion.id + const replacement = '@' + suggestionId + suffix + const newInput = before + replacement + after + return { + newInput, + cursorPos: before.length + replacement.length + } +} + +/** + * Extract a completable token at the cursor position + * @param text The input text + * @param cursorPos The cursor position + * @param includeAtSymbol Whether to consider @ symbol as part of the token + * @returns The completable token and its start position, or null if not found + */ +export function extractCompletionToken(text: string, cursorPos: number, includeAtSymbol = false): { + token: string + startPos: number + isQuoted?: boolean +} | null { + // Empty input check + if (!text) return null + + // Get text up to cursor + const textBeforeCursor = text.substring(0, cursorPos) + + // Check for quoted @ mention first (e.g., @"my file with spaces") + if (includeAtSymbol) { + const quotedAtRegex = /@"([^"]*)"?$/ + const quotedMatch = textBeforeCursor.match(quotedAtRegex) + if (quotedMatch && quotedMatch.index !== undefined) { + // Include any remaining quoted content after cursor until closing quote or end + const textAfterCursor = text.substring(cursorPos) + const afterQuotedMatch = textAfterCursor.match(/^[^"]*"?/) + const quotedSuffix = afterQuotedMatch ? afterQuotedMatch[0] : '' + return { + token: quotedMatch[0] + quotedSuffix, + startPos: quotedMatch.index, + isQuoted: true + } + } + } + + // Fast path for @ tokens: use lastIndexOf to avoid expensive $ anchor scan + if (includeAtSymbol) { + const atIdx = textBeforeCursor.lastIndexOf('@') + if (atIdx >= 0 && (atIdx === 0 || /\s/.test(textBeforeCursor[atIdx - 1]!))) { + const fromAt = textBeforeCursor.substring(atIdx) + const atHeadMatch = fromAt.match(AT_TOKEN_HEAD_RE) + if (atHeadMatch && atHeadMatch[0].length === fromAt.length) { + const textAfterCursor = text.substring(cursorPos) + const afterMatch = textAfterCursor.match(PATH_CHAR_HEAD_RE) + const tokenSuffix = afterMatch ? afterMatch[0] : '' + return { + token: atHeadMatch[0] + tokenSuffix, + startPos: atIdx, + isQuoted: false + } + } + } + } + + // Non-@ token or cursor outside @ token — use $ anchor on (short) tail + const tokenRegex = includeAtSymbol ? TOKEN_WITH_AT_RE : TOKEN_WITHOUT_AT_RE + const match = textBeforeCursor.match(tokenRegex) + if (!match || match.index === undefined) { + return null + } + + // Check if cursor is in the MIDDLE of a token (more word characters after cursor) + // If so, extend the token to include all characters until whitespace or end of string + const textAfterCursor = text.substring(cursorPos) + const afterMatch = textAfterCursor.match(PATH_CHAR_HEAD_RE) + const tokenSuffix = afterMatch ? afterMatch[0] : '' + return { + token: match[0] + tokenSuffix, + startPos: match.index, + isQuoted: false + } +} + +export function extractCommandNameAndArgs(value: string): { + commandName: string + args: string +} | null { + if (isCommandInput(value)) { + const spaceIndex = value.indexOf(' ') + if (spaceIndex === -1) return { + commandName: value.slice(1), + args: '' + } + return { + commandName: value.slice(1, spaceIndex), + args: value.slice(spaceIndex + 1) + } + } + return null +} + +export function hasCommandWithArguments(isAtEndWithWhitespace: boolean, value: string) { + // If value.endsWith(' ') but the user is not at the end, then the user has + // potentially gone back to the command in an effort to edit the command name + // (but preserve the arguments). + return !isAtEndWithWhitespace && value.includes(' ') && !value.endsWith(' ') +} \ No newline at end of file diff --git a/src/hooks/useTypeahead.tsx b/src/hooks/useTypeahead.tsx index 2b6a159f69..5cc9baaaea 100644 --- a/src/hooks/useTypeahead.tsx +++ b/src/hooks/useTypeahead.tsx @@ -2,7 +2,6 @@ import * as React from 'react'; import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useNotifications } from 'src/context/notifications.js'; import { Text } from 'src/ink.js'; -import { logEvent } from 'src/services/analytics/index.js'; import { useDebounceCallback } from 'usehooks-ts'; import { type Command, getCommandName } from '../commands.js'; import { getModeFromInput, getValueFromInput } from '../components/PromptInput/inputModes.js'; @@ -19,7 +18,7 @@ import type { AgentDefinition } from '../tools/AgentTool/loadAgentsDir.js'; import type { InlineGhostText, PromptInputMode } from '../types/textInputTypes.js'; import { isAgentSwarmsEnabled } from '../utils/agentSwarmsEnabled.js'; import { generateProgressiveArgumentHint, parseArguments } from '../utils/argumentSubstitution.js'; -import { getShellCompletions, type ShellCompletionType } from '../utils/bash/shellCompletion.js'; +import { type ShellCompletionType } from '../utils/bash/shellCompletion.js'; import { formatLogMetadata } from '../utils/format.js'; import { getSessionIdFromLog, searchSessionsByCustomTitle } from '../utils/sessionStorage.js'; import { applyCommandSuggestion, findMidInputSlashCommand, generateCommandSuggestions, getBestCommandMatch, isCommandInput } from '../utils/suggestions/commandSuggestions.js'; @@ -29,55 +28,29 @@ import { getSlackChannelSuggestions, hasSlackMcpServer } from '../utils/suggesti import { TEAM_LEAD_NAME } from '../utils/swarm/constants.js'; import { applyFileSuggestion, findLongestCommonPrefix, onIndexBuildComplete, startBackgroundCacheRefresh } from './fileSuggestions.js'; import { generateUnifiedSuggestions } from './unifiedSuggestions.js'; - -// Unicode-aware character class for file path tokens: -// \p{L} = letters (CJK, Latin, Cyrillic, etc.) -// \p{N} = numbers (incl. fullwidth) -// \p{M} = combining marks (macOS NFD accents, Devanagari vowel signs) -const AT_TOKEN_HEAD_RE = /^@[\p{L}\p{N}\p{M}_\-./\\()[\]~:]*/u; -const PATH_CHAR_HEAD_RE = /^[\p{L}\p{N}\p{M}_\-./\\()[\]~:]+/u; -const TOKEN_WITH_AT_RE = /(@[\p{L}\p{N}\p{M}_\-./\\()[\]~:]*|[\p{L}\p{N}\p{M}_\-./\\()[\]~:]+)$/u; -const TOKEN_WITHOUT_AT_RE = /[\p{L}\p{N}\p{M}_\-./\\()[\]~:]+$/u; -const HAS_AT_SYMBOL_RE = /(^|\s)@([\p{L}\p{N}\p{M}_\-./\\()[\]~:]*|"[^"]*"?)$/u; -const HASH_CHANNEL_RE = /(^|\s)#([a-z0-9][a-z0-9_-]*)$/; +import { + AT_TOKEN_HEAD_RE, + PATH_CHAR_HEAD_RE, + TOKEN_WITH_AT_RE, + TOKEN_WITHOUT_AT_RE, + HAS_AT_SYMBOL_RE, + HASH_CHANNEL_RE, + DM_MEMBER_RE, + isPathMetadata, + getPreservedSelection, + buildResumeInputFromSuggestion, + extractSearchToken, + formatReplacementValue, + applyShellSuggestion, + applyTriggerSuggestion, + generateBashSuggestions, + applyDirectorySuggestion, + extractCompletionToken, + extractCommandNameAndArgs, + hasCommandWithArguments, +} from './typeaheadHelpers.js'; // Type guard for path completion metadata -function isPathMetadata(metadata: unknown): metadata is { - type: 'directory' | 'file'; -} { - return typeof metadata === 'object' && metadata !== null && 'type' in metadata && (metadata.type === 'directory' || metadata.type === 'file'); -} - -// Helper to determine selectedSuggestion when updating suggestions -function getPreservedSelection(prevSuggestions: SuggestionItem[], prevSelection: number, newSuggestions: SuggestionItem[]): number { - // No new suggestions - if (newSuggestions.length === 0) { - return -1; - } - - // No previous selection - if (prevSelection < 0) { - return 0; - } - - // Get the previously selected item - const prevSelectedItem = prevSuggestions[prevSelection]; - if (!prevSelectedItem) { - return 0; - } - - // Try to find the same item in the new list by ID - const newIndex = newSuggestions.findIndex(item => item.id === prevSelectedItem.id); - - // Return the new index if found, otherwise default to 0 - return newIndex >= 0 ? newIndex : 0; -} -function buildResumeInputFromSuggestion(suggestion: SuggestionItem): string { - const metadata = suggestion.metadata as { - sessionId: string; - } | undefined; - return metadata?.sessionId ? `/resume ${metadata.sessionId}` : `/resume ${suggestion.displayText}`; -} type Props = { onInputChange: (value: string) => void; onSubmit: (value: string, isSubmittingSlashCommand?: boolean) => void; @@ -115,238 +88,6 @@ type UseTypeaheadResult = { handleKeyDown: (e: KeyboardEvent) => void; }; -/** - * Extract search token from a completion token by removing @ prefix and quotes - * @param completionToken The completion token - * @returns The search token with @ and quotes removed - */ -export function extractSearchToken(completionToken: { - token: string; - isQuoted?: boolean; -}): string { - if (completionToken.isQuoted) { - // Remove @" prefix and optional closing " - return completionToken.token.slice(2).replace(/"$/, ''); - } else if (completionToken.token.startsWith('@')) { - return completionToken.token.substring(1); - } else { - return completionToken.token; - } -} - -/** - * Format a replacement value with proper @ prefix and quotes based on context - * @param options Configuration for formatting - * @param options.displayText The text to display - * @param options.mode The current mode (bash or prompt) - * @param options.hasAtPrefix Whether the original token has @ prefix - * @param options.needsQuotes Whether the text needs quotes (contains spaces) - * @param options.isQuoted Whether the original token was already quoted (user typed @"...) - * @param options.isComplete Whether this is a complete suggestion (adds trailing space) - * @returns The formatted replacement value - */ -export function formatReplacementValue(options: { - displayText: string; - mode: string; - hasAtPrefix: boolean; - needsQuotes: boolean; - isQuoted?: boolean; - isComplete: boolean; -}): string { - const { - displayText, - mode, - hasAtPrefix, - needsQuotes, - isQuoted, - isComplete - } = options; - const space = isComplete ? ' ' : ''; - if (isQuoted || needsQuotes) { - // Use quoted format - return mode === 'bash' ? `"${displayText}"${space}` : `@"${displayText}"${space}`; - } else if (hasAtPrefix) { - return mode === 'bash' ? `${displayText}${space}` : `@${displayText}${space}`; - } else { - return displayText; - } -} - -/** - * Apply a shell completion suggestion by replacing the current word - */ -export function applyShellSuggestion(suggestion: SuggestionItem, input: string, cursorOffset: number, onInputChange: (value: string) => void, setCursorOffset: (offset: number) => void, completionType: ShellCompletionType | undefined): void { - const beforeCursor = input.slice(0, cursorOffset); - const lastSpaceIndex = beforeCursor.lastIndexOf(' '); - const wordStart = lastSpaceIndex + 1; - - // Prepare the replacement text based on completion type - let replacementText: string; - if (completionType === 'variable') { - replacementText = '$' + suggestion.displayText + ' '; - } else if (completionType === 'command') { - replacementText = suggestion.displayText + ' '; - } else { - replacementText = suggestion.displayText; - } - const newInput = input.slice(0, wordStart) + replacementText + input.slice(cursorOffset); - onInputChange(newInput); - setCursorOffset(wordStart + replacementText.length); -} -const DM_MEMBER_RE = /(^|\s)@[\w-]*$/; -function applyTriggerSuggestion(suggestion: SuggestionItem, input: string, cursorOffset: number, triggerRe: RegExp, onInputChange: (value: string) => void, setCursorOffset: (offset: number) => void): void { - const m = input.slice(0, cursorOffset).match(triggerRe); - if (!m || m.index === undefined) return; - const prefixStart = m.index + (m[1]?.length ?? 0); - const before = input.slice(0, prefixStart); - const newInput = before + suggestion.displayText + ' ' + input.slice(cursorOffset); - onInputChange(newInput); - setCursorOffset(before.length + suggestion.displayText.length + 1); -} -let currentShellCompletionAbortController: AbortController | null = null; - -/** - * Generate bash shell completion suggestions - */ -async function generateBashSuggestions(input: string, cursorOffset: number): Promise { - try { - if (currentShellCompletionAbortController) { - currentShellCompletionAbortController.abort(); - } - currentShellCompletionAbortController = new AbortController(); - const suggestions = await getShellCompletions(input, cursorOffset, currentShellCompletionAbortController.signal); - return suggestions; - } catch { - // Silent failure - don't break UX - logEvent('tengu_shell_completion_failed', {}); - return []; - } -} - -/** - * Apply a directory/path completion suggestion to the input - * Always adds @ prefix since we're replacing the entire token (including any existing @) - * - * @param input The current input text - * @param suggestionId The ID of the suggestion to apply - * @param tokenStartPos The start position of the token being replaced - * @param tokenLength The length of the token being replaced - * @param isDirectory Whether the suggestion is a directory (adds / suffix) or file (adds space) - * @returns Object with the new input text and cursor position - */ -export function applyDirectorySuggestion(input: string, suggestionId: string, tokenStartPos: number, tokenLength: number, isDirectory: boolean): { - newInput: string; - cursorPos: number; -} { - const suffix = isDirectory ? '/' : ' '; - const before = input.slice(0, tokenStartPos); - const after = input.slice(tokenStartPos + tokenLength); - // Always add @ prefix - if token already has it, we're replacing - // the whole token (including @) with @suggestion.id - const replacement = '@' + suggestionId + suffix; - const newInput = before + replacement + after; - return { - newInput, - cursorPos: before.length + replacement.length - }; -} - -/** - * Extract a completable token at the cursor position - * @param text The input text - * @param cursorPos The cursor position - * @param includeAtSymbol Whether to consider @ symbol as part of the token - * @returns The completable token and its start position, or null if not found - */ -export function extractCompletionToken(text: string, cursorPos: number, includeAtSymbol = false): { - token: string; - startPos: number; - isQuoted?: boolean; -} | null { - // Empty input check - if (!text) return null; - - // Get text up to cursor - const textBeforeCursor = text.substring(0, cursorPos); - - // Check for quoted @ mention first (e.g., @"my file with spaces") - if (includeAtSymbol) { - const quotedAtRegex = /@"([^"]*)"?$/; - const quotedMatch = textBeforeCursor.match(quotedAtRegex); - if (quotedMatch && quotedMatch.index !== undefined) { - // Include any remaining quoted content after cursor until closing quote or end - const textAfterCursor = text.substring(cursorPos); - const afterQuotedMatch = textAfterCursor.match(/^[^"]*"?/); - const quotedSuffix = afterQuotedMatch ? afterQuotedMatch[0] : ''; - return { - token: quotedMatch[0] + quotedSuffix, - startPos: quotedMatch.index, - isQuoted: true - }; - } - } - - // Fast path for @ tokens: use lastIndexOf to avoid expensive $ anchor scan - if (includeAtSymbol) { - const atIdx = textBeforeCursor.lastIndexOf('@'); - if (atIdx >= 0 && (atIdx === 0 || /\s/.test(textBeforeCursor[atIdx - 1]!))) { - const fromAt = textBeforeCursor.substring(atIdx); - const atHeadMatch = fromAt.match(AT_TOKEN_HEAD_RE); - if (atHeadMatch && atHeadMatch[0].length === fromAt.length) { - const textAfterCursor = text.substring(cursorPos); - const afterMatch = textAfterCursor.match(PATH_CHAR_HEAD_RE); - const tokenSuffix = afterMatch ? afterMatch[0] : ''; - return { - token: atHeadMatch[0] + tokenSuffix, - startPos: atIdx, - isQuoted: false - }; - } - } - } - - // Non-@ token or cursor outside @ token — use $ anchor on (short) tail - const tokenRegex = includeAtSymbol ? TOKEN_WITH_AT_RE : TOKEN_WITHOUT_AT_RE; - const match = textBeforeCursor.match(tokenRegex); - if (!match || match.index === undefined) { - return null; - } - - // Check if cursor is in the MIDDLE of a token (more word characters after cursor) - // If so, extend the token to include all characters until whitespace or end of string - const textAfterCursor = text.substring(cursorPos); - const afterMatch = textAfterCursor.match(PATH_CHAR_HEAD_RE); - const tokenSuffix = afterMatch ? afterMatch[0] : ''; - return { - token: match[0] + tokenSuffix, - startPos: match.index, - isQuoted: false - }; -} -function extractCommandNameAndArgs(value: string): { - commandName: string; - args: string; -} | null { - if (isCommandInput(value)) { - const spaceIndex = value.indexOf(' '); - if (spaceIndex === -1) return { - commandName: value.slice(1), - args: '' - }; - return { - commandName: value.slice(1, spaceIndex), - args: value.slice(spaceIndex + 1) - }; - } - return null; -} -function hasCommandWithArguments(isAtEndWithWhitespace: boolean, value: string) { - // If value.endsWith(' ') but the user is not at the end, then the user has - // potentially gone back to the command in an effort to edit the command name - // (but preserve the arguments). - return !isAtEndWithWhitespace && value.includes(' ') && !value.endsWith(' '); -} - /** * Hook for handling typeahead functionality for both commands and file paths */ diff --git a/src/services/tools/toolExecution.ts b/src/services/tools/toolExecution.ts index 518f4623c9..48bb2f7dc5 100644 --- a/src/services/tools/toolExecution.ts +++ b/src/services/tools/toolExecution.ts @@ -64,9 +64,7 @@ import { logForDebugging } from '../../utils/debug.js' import { AbortError, errorMessage, - getErrnoCode, ShellError, - TelemetrySafeError_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, } from '../../utils/errors.js' import { executePermissionDeniedHooks } from '../../utils/hooks.js' import { logError } from '../../utils/log.js' @@ -79,7 +77,6 @@ import { withMemoryCorrectionHint, } from '../../utils/messages.js' import type { - PermissionDecisionReason, PermissionResult, } from '../../utils/permissions/PermissionResult.js' import { @@ -116,8 +113,6 @@ import { McpAuthError, McpToolCallError_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS, } from '../mcp/client.js' -import { mcpInfoFromString } from '../mcp/mcpStringUtils.js' -import { normalizeNameForMCP } from '../mcp/normalization.js' import type { MCPServerConnection } from '../mcp/types.js' import { getLoggingSafeMcpBaseUrl, @@ -130,137 +125,19 @@ import { runPostToolUseHooks, runPreToolUseHooks, } from './toolHooks.js' +import { + type McpServerType, + SLOW_PHASE_LOG_THRESHOLD_MS, + classifyToolError, + decisionReasonToOTelSource, + findMcpServerConnection, + getMcpServerBaseUrlFromToolName, + getMcpServerType, + getNextImagePasteId, +} from './toolExecutionHelpers.js' /** Minimum total hook duration (ms) to show inline timing summary */ export const HOOK_TIMING_DISPLAY_THRESHOLD_MS = 500 -/** Log a debug warning when hooks/permission-decision block for this long. Matches - * BashTool's PROGRESS_THRESHOLD_MS — the collapsed view feels stuck past this. */ -const SLOW_PHASE_LOG_THRESHOLD_MS = 2000 - -/** - * Classify a tool execution error into a telemetry-safe string. - * - * In minified/external builds, `error.constructor.name` is mangled into - * short identifiers like "nJT" or "Chq" — useless for diagnostics. - * This function extracts structured, telemetry-safe information instead: - * - TelemetrySafeError: use its telemetryMessage (already vetted) - * - Node.js fs errors: log the error code (ENOENT, EACCES, etc.) - * - Known error types: use their unminified name - * - Fallback: "Error" (better than a mangled 3-char identifier) - */ -export function classifyToolError(error: unknown): string { - if ( - error instanceof TelemetrySafeError_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS - ) { - return error.telemetryMessage.slice(0, 200) - } - if (error instanceof Error) { - // Node.js filesystem errors have a `code` property (ENOENT, EACCES, etc.) - // These are safe to log and much more useful than the constructor name. - const errnoCode = getErrnoCode(error) - if (typeof errnoCode === 'string') { - return `Error:${errnoCode}` - } - // ShellError, ImageSizeError, etc. have stable `.name` properties - // that survive minification (they're set in the constructor). - if (error.name && error.name !== 'Error' && error.name.length > 3) { - return error.name.slice(0, 60) - } - return 'Error' - } - return 'UnknownError' -} - -/** - * Map a rule's origin to the documented OTel `source` vocabulary, matching - * the interactive path's semantics (permissionLogging.ts:81): session-scoped - * grants are temporary, on-disk grants are permanent, and user-authored - * denies are user_reject regardless of persistence. Everything the user - * didn't write (cliArg, policySettings, projectSettings, flagSettings) is - * config. - */ -function ruleSourceToOTelSource( - ruleSource: string, - behavior: 'allow' | 'deny', -): string { - switch (ruleSource) { - case 'session': - return behavior === 'allow' ? 'user_temporary' : 'user_reject' - case 'localSettings': - case 'userSettings': - return behavior === 'allow' ? 'user_permanent' : 'user_reject' - default: - return 'config' - } -} - -/** - * Map a PermissionDecisionReason to the OTel `source` label for the - * non-interactive tool_decision path, staying within the documented - * vocabulary (config, hook, user_permanent, user_temporary, user_reject). - * - * For permissionPromptTool, the SDK host may set decisionClassification on - * the PermissionResult to tell us exactly what happened (once vs always vs - * cache hit — the host knows, we can't tell from {behavior:'allow'} alone). - * Without it, we fall back conservatively: allow → user_temporary, - * deny → user_reject. - */ -function decisionReasonToOTelSource( - reason: PermissionDecisionReason | undefined, - behavior: 'allow' | 'deny', -): string { - if (!reason) { - return 'config' - } - switch (reason.type) { - case 'permissionPromptTool': { - // toolResult is typed `unknown` on PermissionDecisionReason but carries - // the parsed Output from PermissionPromptToolResultSchema. Narrow at - // runtime rather than widen the cross-file type. - const toolResult = reason.toolResult as - | { decisionClassification?: string } - | undefined - const classified = toolResult?.decisionClassification - if ( - classified === 'user_temporary' || - classified === 'user_permanent' || - classified === 'user_reject' - ) { - return classified - } - return behavior === 'allow' ? 'user_temporary' : 'user_reject' - } - case 'rule': - return ruleSourceToOTelSource(reason.rule.source, behavior) - case 'hook': - return 'hook' - case 'mode': - case 'classifier': - case 'subcommandResults': - case 'asyncAgent': - case 'sandboxOverride': - case 'workingDir': - case 'safetyCheck': - case 'other': - return 'config' - default: { - const _exhaustive: never = reason - return 'config' - } - } -} - -function getNextImagePasteId(messages: Message[]): number { - let maxId = 0 - for (const message of messages) { - if (message.type === 'user' && message.imagePasteIds) { - for (const id of message.imagePasteIds) { - if (id > maxId) maxId = id - } - } - } - return maxId + 1 -} export type MessageUpdateLazy = { message: M @@ -270,71 +147,6 @@ export type MessageUpdateLazy = { } } -export type McpServerType = - | 'stdio' - | 'sse' - | 'http' - | 'ws' - | 'sdk' - | 'sse-ide' - | 'ws-ide' - | 'claudeai-proxy' - | undefined - -function findMcpServerConnection( - toolName: string, - mcpClients: MCPServerConnection[], -): MCPServerConnection | undefined { - if (!toolName.startsWith('mcp__')) { - return undefined - } - - const mcpInfo = mcpInfoFromString(toolName) - if (!mcpInfo) { - return undefined - } - - // mcpInfo.serverName is normalized (e.g., "claude_ai_Slack"), but client.name - // is the original name (e.g., "claude.ai Slack"). Normalize both for comparison. - return mcpClients.find( - client => normalizeNameForMCP(client.name) === mcpInfo.serverName, - ) -} - -/** - * Extracts the MCP server transport type from a tool name. - * Returns the server type (stdio, sse, http, ws, sdk, etc.) for MCP tools, - * or undefined for built-in tools. - */ -function getMcpServerType( - toolName: string, - mcpClients: MCPServerConnection[], -): McpServerType { - const serverConnection = findMcpServerConnection(toolName, mcpClients) - - if (serverConnection?.type === 'connected') { - // Handle stdio configs where type field is optional (defaults to 'stdio') - return serverConnection.config.type ?? 'stdio' - } - - return undefined -} - -/** - * Extracts the MCP server base URL for a tool by looking up its server connection. - * Returns undefined for stdio servers, built-in tools, or if the server is not connected. - */ -function getMcpServerBaseUrlFromToolName( - toolName: string, - mcpClients: MCPServerConnection[], -): string | undefined { - const serverConnection = findMcpServerConnection(toolName, mcpClients) - if (serverConnection?.type !== 'connected') { - return undefined - } - return getLoggingSafeMcpBaseUrl(serverConnection.config) -} - export async function* runToolUse( toolUse: ToolUseBlock, assistantMessage: AssistantMessage, diff --git a/src/services/tools/toolExecutionHelpers.ts b/src/services/tools/toolExecutionHelpers.ts new file mode 100644 index 0000000000..d22968874a --- /dev/null +++ b/src/services/tools/toolExecutionHelpers.ts @@ -0,0 +1,202 @@ +import type { ToolUseBlock } from '@anthropic-ai/sdk/resources/index.mjs' +import { getErrnoCode, TelemetrySafeError_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS } from '../../utils/errors.js' +import type { Message } from '../../types/message.js' +import type { PermissionDecisionReason } from '../../utils/permissions/PermissionResult.js' +import { mcpInfoFromString } from '../mcp/mcpStringUtils.js' +import { normalizeNameForMCP } from '../mcp/normalization.js' +import type { MCPServerConnection } from '../mcp/types.js' +import { getLoggingSafeMcpBaseUrl } from '../mcp/utils.js' + +/** Log a debug warning when hooks/permission-decision block for this long. Matches + * BashTool's PROGRESS_THRESHOLD_MS — the collapsed view feels stuck past this. */ +export const SLOW_PHASE_LOG_THRESHOLD_MS = 2000 + +/** + * Classify a tool execution error into a telemetry-safe string. + * + * In minified/external builds, `error.constructor.name` is mangled into + * short identifiers like "nJT" or "Chq" — useless for diagnostics. + * This function extracts structured, telemetry-safe information instead: + * - TelemetrySafeError: use its telemetryMessage (already vetted) + * - Node.js fs errors: log the error code (ENOENT, EACCES, etc.) + * - Known error types: use their unminified name + * - Fallback: "Error" (better than a mangled 3-char identifier) + */ +export function classifyToolError(error: unknown): string { + if ( + error instanceof TelemetrySafeError_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS + ) { + return error.telemetryMessage.slice(0, 200) + } + if (error instanceof Error) { + // Node.js filesystem errors have a `code` property (ENOENT, EACCES, etc.) + // These are safe to log and much more useful than the constructor name. + const errnoCode = getErrnoCode(error) + if (typeof errnoCode === 'string') { + return `Error:${errnoCode}` + } + // ShellError, ImageSizeError, etc. have stable `.name` properties + // that survive minification (they're set in the constructor). + if (error.name && error.name !== 'Error' && error.name.length > 3) { + return error.name.slice(0, 60) + } + return 'Error' + } + return 'UnknownError' +} + +/** + * Map a rule's origin to the documented OTel `source` vocabulary, matching + * the interactive path's semantics (permissionLogging.ts:81): session-scoped + * grants are temporary, on-disk grants are permanent, and user-authored + * denies are user_reject regardless of persistence. Everything the user + * didn't write (cliArg, policySettings, projectSettings, flagSettings) is + * config. + */ +export function ruleSourceToOTelSource( + ruleSource: string, + behavior: 'allow' | 'deny', +): string { + switch (ruleSource) { + case 'session': + return behavior === 'allow' ? 'user_temporary' : 'user_reject' + case 'localSettings': + case 'userSettings': + return behavior === 'allow' ? 'user_permanent' : 'user_reject' + default: + return 'config' + } +} + +/** + * Map a PermissionDecisionReason to the OTel `source` label for the + * non-interactive tool_decision path, staying within the documented + * vocabulary (config, hook, user_permanent, user_temporary, user_reject). + * + * For permissionPromptTool, the SDK host may set decisionClassification on + * the PermissionResult to tell us exactly what happened (once vs always vs + * cache hit — the host knows, we can't tell from {behavior:'allow'} alone). + * Without it, we fall back conservatively: allow → user_temporary, + * deny → user_reject. + */ +export function decisionReasonToOTelSource( + reason: PermissionDecisionReason | undefined, + behavior: 'allow' | 'deny', +): string { + if (!reason) { + return 'config' + } + switch (reason.type) { + case 'permissionPromptTool': { + // toolResult is typed `unknown` on PermissionDecisionReason but carries + // the parsed Output from PermissionPromptToolResultSchema. Narrow at + // runtime rather than widen the cross-file type. + const toolResult = reason.toolResult as + | { decisionClassification?: string } + | undefined + const classified = toolResult?.decisionClassification + if ( + classified === 'user_temporary' || + classified === 'user_permanent' || + classified === 'user_reject' + ) { + return classified + } + return behavior === 'allow' ? 'user_temporary' : 'user_reject' + } + case 'rule': + return ruleSourceToOTelSource(reason.rule.source, behavior) + case 'hook': + return 'hook' + case 'mode': + case 'classifier': + case 'subcommandResults': + case 'asyncAgent': + case 'sandboxOverride': + case 'workingDir': + case 'safetyCheck': + case 'other': + return 'config' + default: { + const _exhaustive: never = reason + return 'config' + } + } +} + +export function getNextImagePasteId(messages: Message[]): number { + let maxId = 0 + for (const message of messages) { + if (message.type === 'user' && message.imagePasteIds) { + for (const id of message.imagePasteIds) { + if (id > maxId) maxId = id + } + } + } + return maxId + 1 +} + +export type McpServerType = + | 'stdio' + | 'sse' + | 'http' + | 'ws' + | 'sdk' + | 'sse-ide' + | 'ws-ide' + | 'claudeai-proxy' + | undefined + +export function findMcpServerConnection( + toolName: string, + mcpClients: MCPServerConnection[], +): MCPServerConnection | undefined { + if (!toolName.startsWith('mcp__')) { + return undefined + } + + const mcpInfo = mcpInfoFromString(toolName) + if (!mcpInfo) { + return undefined + } + + // mcpInfo.serverName is normalized (e.g., "claude_ai_Slack"), but client.name + // is the original name (e.g., "claude.ai Slack"). Normalize both for comparison. + return mcpClients.find( + client => normalizeNameForMCP(client.name) === mcpInfo.serverName, + ) +} + +/** + * Extracts the MCP server transport type from a tool name. + * Returns the server type (stdio, sse, http, ws, sdk, etc.) for MCP tools, + * or undefined for built-in tools. + */ +export function getMcpServerType( + toolName: string, + mcpClients: MCPServerConnection[], +): McpServerType { + const serverConnection = findMcpServerConnection(toolName, mcpClients) + + if (serverConnection?.type === 'connected') { + // Handle stdio configs where type field is optional (defaults to 'stdio') + return serverConnection.config.type ?? 'stdio' + } + + return undefined +} + +/** + * Extracts the MCP server base URL for a tool by looking up its server connection. + * Returns undefined for stdio servers, built-in tools, or if the server is not connected. + */ +export function getMcpServerBaseUrlFromToolName( + toolName: string, + mcpClients: MCPServerConnection[], +): string | undefined { + const serverConnection = findMcpServerConnection(toolName, mcpClients) + if (serverConnection?.type !== 'connected') { + return undefined + } + return getLoggingSafeMcpBaseUrl(serverConnection.config) +} \ No newline at end of file diff --git a/src/tools/BashTool/bashPermissions.ts b/src/tools/BashTool/bashPermissions.ts index 288c7e978d..6a44f22705 100644 --- a/src/tools/BashTool/bashPermissions.ts +++ b/src/tools/BashTool/bashPermissions.ts @@ -51,9 +51,22 @@ import type { import { extractRules } from '../../utils/permissions/PermissionUpdate.js' import type { PermissionUpdate } from '../../utils/permissions/PermissionUpdateSchema.js' import { permissionRuleValueToString } from '../../utils/permissions/permissionRuleParser.js' +import { + BINARY_HIJACK_VARS, + ANT_ONLY_SAFE_ENV_VARS, + ENV_VAR_ASSIGN_RE, + SAFE_ENV_VARS, + stripAllLeadingEnvVars, + stripCommentLines, + stripSafeWrappers, + stripWrappersFromArgv, +} from '../../utils/permissions/commandWrapperDetection.js' +import { + matchingRulesForInput as sharedMatchingRulesForInput, + type FilterRulesFn, +} from '../../utils/permissions/permissionChecker.js' import { createPermissionRequestMessage, - getRuleByContentsForTool, } from '../../utils/permissions/permissions.js' import { parsePermissionRule, @@ -88,9 +101,7 @@ import { shouldUseSandbox } from './shouldUseSandbox.js' const bashCommandIsSafeAsync = bashCommandIsSafeAsync_DEPRECATED const splitCommand = splitCommand_DEPRECATED -// Env-var assignment prefix (VAR=value). Shared across three while-loops that -// skip safe env vars before extracting the command name. -const ENV_VAR_ASSIGN_RE = /^[A-Za-z_]\w*=/ +// ENV_VAR_ASSIGN_RE imported from commandWrapperDetection.ts // CC-643: On complex compound commands, splitCommand_DEPRECATED can produce a // very large subcommands array (possible exponential growth; #21405's ReDoS fix @@ -344,413 +355,9 @@ export const bashPermissionRule: ( permissionRule: string, ) => ShellPermissionRule = parsePermissionRule -/** - * Whitelist of environment variables that are safe to strip from commands. - * These variables CANNOT execute code or load libraries. - * - * SECURITY: These must NEVER be added to the whitelist: - * - PATH, LD_PRELOAD, LD_LIBRARY_PATH, DYLD_* (execution/library loading) - * - PYTHONPATH, NODE_PATH, CLASSPATH, RUBYLIB (module loading) - * - GOFLAGS, RUSTFLAGS, NODE_OPTIONS (can contain code execution flags) - * - HOME, TMPDIR, SHELL, BASH_ENV (affect system behavior) - */ -const SAFE_ENV_VARS = new Set([ - // Go - build/runtime settings only - 'GOEXPERIMENT', // experimental features - 'GOOS', // target OS - 'GOARCH', // target architecture - 'CGO_ENABLED', // enable/disable CGO - 'GO111MODULE', // module mode - - // Rust - logging/debugging only - 'RUST_BACKTRACE', // backtrace verbosity - 'RUST_LOG', // logging filter - - // Node - environment name only (not NODE_OPTIONS!) - 'NODE_ENV', - - // Python - behavior flags only (not PYTHONPATH!) - 'PYTHONUNBUFFERED', // disable buffering - 'PYTHONDONTWRITEBYTECODE', // no .pyc files - - // Pytest - test configuration - 'PYTEST_DISABLE_PLUGIN_AUTOLOAD', // disable plugin loading - 'PYTEST_DEBUG', // debug output - - // API keys and authentication - 'ANTHROPIC_API_KEY', // API authentication - - // Locale and character encoding - 'LANG', // default locale - 'LANGUAGE', // language preference list - 'LC_ALL', // override all locale settings - 'LC_CTYPE', // character classification - 'LC_TIME', // time format - 'CHARSET', // character set preference - - // Terminal and display - 'TERM', // terminal type - 'COLORTERM', // color terminal indicator - 'NO_COLOR', // disable color output (universal standard) - 'FORCE_COLOR', // force color output - 'TZ', // timezone - - // Color configuration for various tools - 'LS_COLORS', // colors for ls (GNU) - 'LSCOLORS', // colors for ls (BSD/macOS) - 'GREP_COLOR', // grep match color (deprecated) - 'GREP_COLORS', // grep color scheme - 'GCC_COLORS', // GCC diagnostic colors - - // Display formatting - 'TIME_STYLE', // time display format for ls - 'BLOCK_SIZE', // block size for du/df - 'BLOCKSIZE', // alternative block size -]) - -/** - * Environment variables that are safe to strip from commands. - * - * SECURITY: These env vars are stripped before permission-rule matching, which - * means `DOCKER_HOST=tcp://evil.com docker ps` matches a `Bash(docker ps:*)` - * rule after stripping. DOCKER_HOST redirects the Docker - * daemon endpoint — stripping it defeats prefix-based permission restrictions - * by hiding the network endpoint from the permission check. KUBECONFIG - * similarly controls which cluster kubectl talks to. These are convenience - * strippings for internal power users who accept the risk. - * - * Based on analysis of 30 days of tengu_internal_bash_tool_use_permission_request events. - */ -const ANT_ONLY_SAFE_ENV_VARS = new Set([ - // Kubernetes and container config (config file pointers, not execution) - 'KUBECONFIG', // kubectl config file path — controls which cluster kubectl uses - 'DOCKER_HOST', // Docker daemon socket/endpoint — controls which daemon docker talks to - - // Cloud provider project/profile selection (just names/identifiers) - 'AWS_PROFILE', // AWS profile name selection - 'CLOUDSDK_CORE_PROJECT', // GCP project ID - 'CLUSTER', // generic cluster name - - // Internal cluster selection (just names/identifiers) - 'COO_CLUSTER', // coo cluster name - 'COO_CLUSTER_NAME', // coo cluster name (alternate) - 'COO_NAMESPACE', // coo namespace - 'COO_LAUNCH_YAML_DRY_RUN', // dry run mode - - // Feature flags (boolean/string flags only) - 'SKIP_NODE_VERSION_CHECK', // skip version check - 'EXPECTTEST_ACCEPT', // accept test expectations - 'CI', // CI environment indicator - 'GIT_LFS_SKIP_SMUDGE', // skip LFS downloads - - // GPU/Device selection (just device IDs) - 'CUDA_VISIBLE_DEVICES', // GPU device selection - 'JAX_PLATFORMS', // JAX platform selection - - // Display/terminal settings - 'COLUMNS', // terminal width - 'TMUX', // TMUX socket info - - // Test/debug configuration - 'POSTGRESQL_VERSION', // postgres version string - 'FIRESTORE_EMULATOR_HOST', // emulator host:port - 'HARNESS_QUIET', // quiet mode flag - 'TEST_CROSSCHECK_LISTS_MATCH_UPDATE', // test update flag - 'DBT_PER_DEVELOPER_ENVIRONMENTS', // DBT config - 'STATSIG_FORD_DB_CHECKS', // statsig DB check flag - - // Build configuration - 'ANT_ENVIRONMENT', // Anthropic environment name - 'ANT_SERVICE', // Anthropic service name - 'MONOREPO_ROOT_DIR', // monorepo root path - - // Version selectors - 'PYENV_VERSION', // Python version selection - - // Credentials (approved subset - these don't change exfil risk) - 'PGPASSWORD', // Postgres password - 'GH_TOKEN', // GitHub token - 'GROWTHBOOK_API_KEY', // self-hosted growthbook -]) - -/** - * Strips full-line comments from a command. - * This handles cases where Claude adds comments in bash commands, e.g.: - * "# Check the logs directory\nls /home/user/logs" - * Should be stripped to: "ls /home/user/logs" - * - * Only strips full-line comments (lines where the entire line is a comment), - * not inline comments that appear after a command on the same line. - */ -function stripCommentLines(command: string): string { - const lines = command.split('\n') - const nonCommentLines = lines.filter(line => { - const trimmed = line.trim() - // Keep lines that are not empty and don't start with # - return trimmed !== '' && !trimmed.startsWith('#') - }) - - // If all lines were comments/empty, return original - if (nonCommentLines.length === 0) { - return command - } - - return nonCommentLines.join('\n') -} - -export function stripSafeWrappers(command: string): string { - // SECURITY: Use [ \t]+ not \s+ — \s matches \n/\r which are command - // separators in bash. Matching across a newline would strip the wrapper from - // one line and leave a different command on the next line for bash to execute. - // - // SECURITY: `(?:--[ \t]+)?` consumes the wrapper's own `--` so - // `nohup -- rm -- -/../foo` strips to `rm -- -/../foo` (not `-- rm ...` - // which would skip path validation with `--` as an unknown baseCmd). - const SAFE_WRAPPER_PATTERNS = [ - // timeout: enumerate GNU long flags — no-value (--foreground, - // --preserve-status, --verbose), value-taking in both =fused and - // space-separated forms (--kill-after=5, --kill-after 5, --signal=TERM, - // --signal TERM). Short: -v (no-arg), -k/-s with separate or fused value. - // SECURITY: flag VALUES use allowlist [A-Za-z0-9_.+-] (signals are - // TERM/KILL/9, durations are 5/5s/10.5). Previously [^ \t]+ matched - // $ ( ) ` | ; & — `timeout -k$(id) 10 ls` stripped to `ls`, matched - // Bash(ls:*), while bash expanded $(id) during word splitting BEFORE - // timeout ran. Contrast ENV_VAR_PATTERN below which already allowlists. - /^timeout[ \t]+(?:(?:--(?:foreground|preserve-status|verbose)|--(?:kill-after|signal)=[A-Za-z0-9_.+-]+|--(?:kill-after|signal)[ \t]+[A-Za-z0-9_.+-]+|-v|-[ks][ \t]+[A-Za-z0-9_.+-]+|-[ks][A-Za-z0-9_.+-]+)[ \t]+)*(?:--[ \t]+)?\d+(?:\.\d+)?[smhd]?[ \t]+/, - /^time[ \t]+(?:--[ \t]+)?/, - // SECURITY: keep in sync with checkSemantics wrapper-strip (ast.ts - // ~:1990-2080) AND stripWrappersFromArgv (pathValidation.ts ~:1260). - // Previously this pattern REQUIRED `-n N`; checkSemantics already handled - // bare `nice` and legacy `-N`. Asymmetry meant checkSemantics exposed the - // wrapped command to semantic checks but deny-rule matching and the cd+git - // gate saw the wrapper name. `nice rm -rf /` with Bash(rm:*) deny became - // ask instead of deny; `cd evil && nice git status` skipped the bare-repo - // RCE gate. PR #21503 fixed stripWrappersFromArgv; this was missed. - // Now matches: `nice cmd`, `nice -n N cmd`, `nice -N cmd` (all forms - // checkSemantics strips). - /^nice(?:[ \t]+-n[ \t]+-?\d+|[ \t]+-\d+)?[ \t]+(?:--[ \t]+)?/, - // stdbuf: fused short flags only (-o0, -eL). checkSemantics handles more - // (space-separated, long --output=MODE), but we fail-closed on those - // above so not over-stripping here is safe. Main need: `stdbuf -o0 cmd`. - /^stdbuf(?:[ \t]+-[ioe][LN0-9]+)+[ \t]+(?:--[ \t]+)?/, - /^nohup[ \t]+(?:--[ \t]+)?/, - ] as const - - // Pattern for environment variables: - // ^([A-Za-z_][A-Za-z0-9_]*) - Variable name (standard identifier) - // = - Equals sign - // ([A-Za-z0-9_./:-]+) - Value: alphanumeric + safe punctuation only - // [ \t]+ - Required HORIZONTAL whitespace after value - // - // SECURITY: Only matches unquoted values with safe characters (no $(), `, $var, ;|&). - // - // SECURITY: Trailing whitespace MUST be [ \t]+ (horizontal only), NOT \s+. - // \s matches \n/\r. If reconstructCommand emits an unquoted newline between - // `TZ=UTC` and `echo`, \s+ would match across it and strip `TZ=UTC`, - // leaving `echo curl evil.com` to match Bash(echo:*). But bash treats the - // newline as a command separator. Defense-in-depth with needsQuoting fix. - const ENV_VAR_PATTERN = /^([A-Za-z_][A-Za-z0-9_]*)=([A-Za-z0-9_./:-]+)[ \t]+/ - - let stripped = command - let previousStripped = '' - - // Phase 1: Strip leading env vars and comments only. - // In bash, env var assignments before a command (VAR=val cmd) are genuine - // shell-level assignments. These are safe to strip for permission matching. - while (stripped !== previousStripped) { - previousStripped = stripped - stripped = stripCommentLines(stripped) - - const envVarMatch = stripped.match(ENV_VAR_PATTERN) - if (envVarMatch) { - const varName = envVarMatch[1]! - const isAntOnlySafe = - process.env.USER_TYPE === 'ant' && ANT_ONLY_SAFE_ENV_VARS.has(varName) - if (SAFE_ENV_VARS.has(varName) || isAntOnlySafe) { - stripped = stripped.replace(ENV_VAR_PATTERN, '') - } - } - } - - // Phase 2: Strip wrapper commands and comments only. Do NOT strip env vars. - // Wrapper commands (timeout, time, nice, nohup) use execvp to run their - // arguments, so VAR=val after a wrapper is treated as the COMMAND to execute, - // not as an env var assignment. Stripping env vars here would create a - // mismatch between what the parser sees and what actually executes. - // (HackerOne #3543050) - previousStripped = '' - while (stripped !== previousStripped) { - previousStripped = stripped - stripped = stripCommentLines(stripped) - - for (const pattern of SAFE_WRAPPER_PATTERNS) { - stripped = stripped.replace(pattern, '') - } - } - - return stripped.trim() -} - -// SECURITY: allowlist for timeout flag VALUES (signals are TERM/KILL/9, -// durations are 5/5s/10.5). Rejects $ ( ) ` | ; & and newlines that -// previously matched via [^ \t]+ — `timeout -k$(id) 10 ls` must NOT strip. -const TIMEOUT_FLAG_VALUE_RE = /^[A-Za-z0-9_.+-]+$/ - -/** - * Parse timeout's GNU flags (long + short, fused + space-separated) and - * return the argv index of the DURATION token, or -1 if flags are unparseable. - * Enumerates: --foreground/--preserve-status/--verbose (no value), - * --kill-after/--signal (value, both =fused and space-separated), -v (no - * value), -k/-s (value, both fused and space-separated). - * - * Extracted from stripWrappersFromArgv to keep bashToolHasPermission under - * Bun's feature() DCE complexity threshold — inlining this breaks - * feature('BASH_CLASSIFIER') evaluation in classifier tests. - */ -function skipTimeoutFlags(a: readonly string[]): number { - let i = 1 - while (i < a.length) { - const arg = a[i]! - const next = a[i + 1] - if ( - arg === '--foreground' || - arg === '--preserve-status' || - arg === '--verbose' - ) - i++ - else if (/^--(?:kill-after|signal)=[A-Za-z0-9_.+-]+$/.test(arg)) i++ - else if ( - (arg === '--kill-after' || arg === '--signal') && - next && - TIMEOUT_FLAG_VALUE_RE.test(next) - ) - i += 2 - else if (arg === '--') { - i++ - break - } // end-of-options marker - else if (arg.startsWith('--')) return -1 - else if (arg === '-v') i++ - else if ( - (arg === '-k' || arg === '-s') && - next && - TIMEOUT_FLAG_VALUE_RE.test(next) - ) - i += 2 - else if (/^-[ks][A-Za-z0-9_.+-]+$/.test(arg)) i++ - else if (arg.startsWith('-')) return -1 - else break - } - return i -} - -/** - * Argv-level counterpart to stripSafeWrappers. Strips the same wrapper - * commands (timeout, time, nice, nohup) from AST-derived argv. Env vars - * are already separated into SimpleCommand.envVars so no env-var stripping. - * - * KEEP IN SYNC with SAFE_WRAPPER_PATTERNS above — if you add a wrapper - * there, add it here too. - */ -export function stripWrappersFromArgv(argv: string[]): string[] { - // SECURITY: Consume optional `--` after wrapper options, matching what the - // wrapper does. Otherwise `['nohup','--','rm','--','-/../foo']` yields `--` - // as baseCmd and skips path validation. See SAFE_WRAPPER_PATTERNS comment. - let a = argv - for (;;) { - if (a[0] === 'time' || a[0] === 'nohup') { - a = a.slice(a[1] === '--' ? 2 : 1) - } else if (a[0] === 'timeout') { - const i = skipTimeoutFlags(a) - if (i < 0 || !a[i] || !/^\d+(?:\.\d+)?[smhd]?$/.test(a[i]!)) return a - a = a.slice(i + 1) - } else if ( - a[0] === 'nice' && - a[1] === '-n' && - a[2] && - /^-?\d+$/.test(a[2]) - ) { - a = a.slice(a[3] === '--' ? 4 : 3) - } else { - return a - } - } -} - -/** - * Env vars that make a *different binary* run (injection or resolution hijack). - * Heuristic only — export-&& form bypasses this, and excludedCommands isn't a - * security boundary anyway. - */ -export const BINARY_HIJACK_VARS = /^(LD_|DYLD_|PATH$)/ - -/** - * Strip ALL leading env var prefixes from a command, regardless of whether the - * var name is in the safe-list. - * - * Used for deny/ask rule matching: when a user denies `claude` or `rm`, the - * command should stay blocked even if prefixed with arbitrary env vars like - * `FOO=bar claude`. The safe-list restriction in stripSafeWrappers is correct - * for allow rules (prevents `DOCKER_HOST=evil docker ps` from auto-matching - * `Bash(docker ps:*)`), but deny rules must be harder to circumvent. - * - * Also used for sandbox.excludedCommands matching (not a security boundary — - * permission prompts are), with BINARY_HIJACK_VARS as a blocklist. - * - * SECURITY: Uses a broader value pattern than stripSafeWrappers. The value - * pattern excludes only actual shell injection characters ($, backtick, ;, |, - * &, parens, redirects, quotes, backslash) and whitespace. Characters like - * =, +, @, ~, , are harmless in unquoted env var assignment position and must - * be matched to prevent trivial bypass via e.g. `FOO=a=b denied_command`. - * - * @param blocklist - optional regex tested against each var name; matching vars - * are NOT stripped (and stripping stops there). Omit for deny rules; pass - * BINARY_HIJACK_VARS for excludedCommands. - */ -export function stripAllLeadingEnvVars( - command: string, - blocklist?: RegExp, -): string { - // Broader value pattern for deny-rule stripping. Handles: - // - // - Standard assignment (FOO=bar), append (FOO+=bar), array (FOO[0]=bar) - // - Single-quoted values: '[^'\n\r]*' — bash suppresses all expansion - // - Double-quoted values with backslash escapes: "(?:\\.|[^"$`\\\n\r])*" - // In bash double quotes, only \$, \`, \", \\, and \newline are special. - // Other \x sequences are harmless, so we allow \. inside double quotes. - // We still exclude raw $ and ` (without backslash) to block expansion. - // - Unquoted values: excludes shell metacharacters, allows backslash escapes - // - Concatenated segments: FOO='x'y"z" — bash concatenates adjacent segments - // - // SECURITY: Trailing whitespace MUST be [ \t]+ (horizontal only), NOT \s+. - // - // The outer * matches one atomic unit per iteration: a complete quoted - // string, a backslash-escape pair, or a single unquoted safe character. - // The inner double-quote alternation (?:...|...)* is bounded by the - // closing ", so it cannot interact with the outer * for backtracking. - // - // Note: $ is excluded from unquoted/double-quoted value classes to block - // dangerous forms like $(cmd), ${var}, and $((expr)). This means - // FOO=$VAR is not stripped — adding $VAR matching creates ReDoS risk - // (CodeQL #671) and $VAR bypasses are low-priority. - const ENV_VAR_PATTERN = - /^([A-Za-z_][A-Za-z0-9_]*(?:\[[^\]]*\])?)\+?=(?:'[^'\n\r]*'|"(?:\\.|[^"$`\\\n\r])*"|\\.|[^ \t\n\r$`;|&()<>\\\\'"])*[ \t]+/ - - let stripped = command - let previousStripped = '' - - while (stripped !== previousStripped) { - previousStripped = stripped - stripped = stripCommentLines(stripped) - - const m = stripped.match(ENV_VAR_PATTERN) - if (!m) continue - if (blocklist?.test(m[1]!)) break - stripped = stripped.slice(m[0].length) - } - - return stripped.trim() -} +// SAFE_ENV_VARS, ANT_ONLY_SAFE_ENV_VARS, stripCommentLines, stripSafeWrappers, +// stripWrappersFromArgv, stripAllLeadingEnvVars, BINARY_HIJACK_VARS, and +// skipTimeoutFlags moved to commandWrapperDetection.ts function filterRulesByContentsMatchingInput( input: z.infer, @@ -917,49 +524,26 @@ function matchingRulesForInput( matchMode: 'exact' | 'prefix', { skipCompoundCheck = false }: { skipCompoundCheck?: boolean } = {}, ) { - const denyRuleByContents = getRuleByContentsForTool( - toolPermissionContext, - BashTool, - 'deny', - ) - // SECURITY: Deny/ask rules use aggressive env var stripping so that - // `FOO=bar denied_command` still matches a deny rule for `denied_command`. - const matchingDenyRules = filterRulesByContentsMatchingInput( - input, - denyRuleByContents, - matchMode, - { stripAllEnvVars: true, skipCompoundCheck: true }, - ) - - const askRuleByContents = getRuleByContentsForTool( - toolPermissionContext, - BashTool, - 'ask', - ) - const matchingAskRules = filterRulesByContentsMatchingInput( - input, - askRuleByContents, - matchMode, - { stripAllEnvVars: true, skipCompoundCheck: true }, - ) + // Delegate to shared implementation with a bash-specific filter function + const filterFn: FilterRulesFn = (rules, mode, behavior) => { + const options: { stripAllEnvVars?: boolean; skipCompoundCheck?: boolean } = {} + // SECURITY: Deny/ask rules use aggressive env var stripping so that + // `FOO=bar denied_command` still matches a deny rule for `denied_command`. + if (behavior === 'deny' || behavior === 'ask') { + options.stripAllEnvVars = true + options.skipCompoundCheck = true + } else { + options.skipCompoundCheck = skipCompoundCheck + } + return filterRulesByContentsMatchingInput(input, rules, mode, options) + } - const allowRuleByContents = getRuleByContentsForTool( + return sharedMatchingRulesForInput( + BashTool.name, toolPermissionContext, - BashTool, - 'allow', - ) - const matchingAllowRules = filterRulesByContentsMatchingInput( - input, - allowRuleByContents, matchMode, - { skipCompoundCheck }, + filterFn, ) - - return { - matchingDenyRules, - matchingAskRules, - matchingAllowRules, - } } /** diff --git a/src/tools/BashTool/pathValidation.ts b/src/tools/BashTool/pathValidation.ts index 6fa8ca04c2..492010df28 100644 --- a/src/tools/BashTool/pathValidation.ts +++ b/src/tools/BashTool/pathValidation.ts @@ -21,7 +21,7 @@ import { validatePath, } from '../../utils/permissions/pathValidation.js' import type { BashTool } from './BashTool.js' -import { stripSafeWrappers } from './bashPermissions.js' +import { stripSafeWrappers } from '../../utils/permissions/commandWrapperDetection.js' import { sedCommandIsAllowedByAllowlist } from './sedValidation.js' export type PathCommand = diff --git a/src/tools/BashTool/shouldUseSandbox.ts b/src/tools/BashTool/shouldUseSandbox.ts index 486e367830..fbc3a4a194 100644 --- a/src/tools/BashTool/shouldUseSandbox.ts +++ b/src/tools/BashTool/shouldUseSandbox.ts @@ -4,10 +4,12 @@ import { SandboxManager } from '../../utils/sandbox/sandbox-adapter.js' import { getSettings_DEPRECATED } from '../../utils/settings/settings.js' import { BINARY_HIJACK_VARS, - bashPermissionRule, - matchWildcardPattern, stripAllLeadingEnvVars, stripSafeWrappers, +} from '../../utils/permissions/commandWrapperDetection.js' +import { + bashPermissionRule, + matchWildcardPattern, } from './bashPermissions.js' type SandboxInput = { diff --git a/src/tools/PowerShellTool/powershellPermissions.ts b/src/tools/PowerShellTool/powershellPermissions.ts index 942991e7cf..fe20ab5c66 100644 --- a/src/tools/PowerShellTool/powershellPermissions.ts +++ b/src/tools/PowerShellTool/powershellPermissions.ts @@ -13,9 +13,12 @@ import { getCwd } from '../../utils/cwd.js' import { isCurrentDirectoryBareGitRepo } from '../../utils/git.js' import type { PermissionRule } from '../../utils/permissions/PermissionRule.js' import type { PermissionUpdate } from '../../utils/permissions/PermissionUpdateSchema.js' +import { + matchingRulesForInput as sharedMatchingRulesForInput, + type FilterRulesFn, +} from '../../utils/permissions/permissionChecker.js' import { createPermissionRequestMessage, - getRuleByContentsForToolName, } from '../../utils/permissions/permissions.js' import { matchWildcardPattern, @@ -334,49 +337,22 @@ function filterRulesByContentsMatchingInput( /** * Get matching rules for input across all rule types (deny, ask, allow) + * Delegates to shared implementation with PowerShell-specific filter function. */ function matchingRulesForInput( input: PowerShellInput, toolPermissionContext: ToolPermissionContext, matchMode: 'exact' | 'prefix', ) { - const denyRuleByContents = getRuleByContentsForToolName( - toolPermissionContext, - POWERSHELL_TOOL_NAME, - 'deny', - ) - const matchingDenyRules = filterRulesByContentsMatchingInput( - input, - denyRuleByContents, - matchMode, - 'deny', - ) + const filterFn: FilterRulesFn = (rules, mode, behavior) => + filterRulesByContentsMatchingInput(input, rules, mode, behavior) - const askRuleByContents = getRuleByContentsForToolName( - toolPermissionContext, + return sharedMatchingRulesForInput( POWERSHELL_TOOL_NAME, - 'ask', - ) - const matchingAskRules = filterRulesByContentsMatchingInput( - input, - askRuleByContents, - matchMode, - 'ask', - ) - - const allowRuleByContents = getRuleByContentsForToolName( toolPermissionContext, - POWERSHELL_TOOL_NAME, - 'allow', - ) - const matchingAllowRules = filterRulesByContentsMatchingInput( - input, - allowRuleByContents, matchMode, - 'allow', + filterFn, ) - - return { matchingDenyRules, matchingAskRules, matchingAllowRules } } /** diff --git a/src/utils/permissions/commandWrapperDetection.ts b/src/utils/permissions/commandWrapperDetection.ts new file mode 100644 index 0000000000..11e432efe2 --- /dev/null +++ b/src/utils/permissions/commandWrapperDetection.ts @@ -0,0 +1,431 @@ +/** + * Bash command-wrapper detection and stripping. + * + * Extracted from bashPermissions.ts to reduce its size and isolate + * the complex regex-based wrapper-stripping logic. These functions + * are bash-specific (timeout, time, nice, nohup, stdbuf) and have + * no PowerShell equivalent. + * + * KEEP IN SYNC with: + * - checkSemantics wrapper-strip (ast.ts ~:1990-2080) + * - stripWrappersFromArgv (this file) + */ + +// Env-var assignment prefix (VAR=value). Shared across three while-loops that +// skip safe env vars before extracting the command name. +export const ENV_VAR_ASSIGN_RE = /^[A-Za-z_]\w*=/ + +/** + * Whitelist of environment variables that are safe to strip from commands. + * These variables CANNOT execute code or load libraries. + * + * SECURITY: These must NEVER be added to the whitelist: + * - PATH, LD_PRELOAD, LD_LIBRARY_PATH, DYLD_* (execution/library loading) + * - PYTHONPATH, NODE_PATH, CLASSPATH, RUBYLIB (module loading) + * - GOFLAGS, RUSTFLAGS, NODE_OPTIONS (can contain code execution flags) + * - HOME, TMPDIR, SHELL, BASH_ENV (affect system behavior) + */ +export const SAFE_ENV_VARS = new Set([ + // Go - build/runtime settings only + 'GOEXPERIMENT', // experimental features + 'GOOS', // target OS + 'GOARCH', // target architecture + 'CGO_ENABLED', // enable/disable CGO + 'GO111MODULE', // module mode + + // Rust - logging/debugging only + 'RUST_BACKTRACE', // backtrace verbosity + 'RUST_LOG', // logging filter + + // Node - environment name only (not NODE_OPTIONS!) + 'NODE_ENV', + + // Python - behavior flags only (not PYTHONPATH!) + 'PYTHONUNBUFFERED', // disable buffering + 'PYTHONDONTWRITEBYTECODE', // no .pyc files + + // Pytest - test configuration + 'PYTEST_DISABLE_PLUGIN_AUTOLOAD', // disable plugin loading + 'PYTEST_DEBUG', // debug output + + // API keys and authentication + 'ANTHROPIC_API_KEY', // API authentication + + // Locale and character encoding + 'LANG', // default locale + 'LANGUAGE', // language preference list + 'LC_ALL', // override all locale settings + 'LC_CTYPE', // character classification + 'LC_TIME', // time format + 'CHARSET', // character set preference + + // Terminal and display + 'TERM', // terminal type + 'COLORTERM', // color terminal indicator + 'NO_COLOR', // disable color output (universal standard) + 'FORCE_COLOR', // force color output + 'TZ', // timezone + + // Color configuration for various tools + 'LS_COLORS', // colors for ls (GNU) + 'LSCOLORS', // colors for ls (BSD/macOS) + 'GREP_COLOR', // grep match color (deprecated) + 'GREP_COLORS', // grep color scheme + 'GCC_COLORS', // GCC diagnostic colors + + // Display formatting + 'TIME_STYLE', // time display format for ls + 'BLOCK_SIZE', // block size for du/df + 'BLOCKSIZE', // alternative block size +]) + +/** + * Environment variables that are safe to strip from commands. + * + * SECURITY: These env vars are stripped before permission-rule matching, which + * means `DOCKER_HOST=tcp://evil.com docker ps` matches a `Bash(docker ps:*)` + * rule after stripping. DOCKER_HOST redirects the Docker + * daemon endpoint — stripping it defeats prefix-based permission restrictions + * by hiding the network endpoint from the permission check. KUBECONFIG + * similarly controls which cluster kubectl talks to. These are convenience + * strippings for internal power users who accept the risk. + * + * Based on analysis of 30 days of tengu_internal_bash_tool_use_permission_request events. + */ +export const ANT_ONLY_SAFE_ENV_VARS = new Set([ + // Kubernetes and container config (config file pointers, not execution) + 'KUBECONFIG', // kubectl config file path — controls which cluster kubectl uses + 'DOCKER_HOST', // Docker daemon socket/endpoint — controls which daemon docker talks to + + // Cloud provider project/profile selection (just names/identifiers) + 'AWS_PROFILE', // AWS profile name selection + 'CLOUDSDK_CORE_PROJECT', // GCP project ID + 'CLUSTER', // generic cluster name + + // Internal cluster selection (just names/identifiers) + 'COO_CLUSTER', // coo cluster name + 'COO_CLUSTER_NAME', // coo cluster name (alternate) + 'COO_NAMESPACE', // coo namespace + 'COO_LAUNCH_YAML_DRY_RUN', // dry run mode + + // Feature flags (boolean/string flags only) + 'SKIP_NODE_VERSION_CHECK', // skip version check + 'EXPECTTEST_ACCEPT', // accept test expectations + 'CI', // CI environment indicator + 'GIT_LFS_SKIP_SMUDGE', // skip LFS downloads + + // GPU/Device selection (just device IDs) + 'CUDA_VISIBLE_DEVICES', // GPU device selection + 'JAX_PLATFORMS', // JAX platform selection + + // Display/terminal settings + 'COLUMNS', // terminal width + 'TMUX', // TMUX socket info + + // Test/debug configuration + 'POSTGRESQL_VERSION', // postgres version string + 'FIRESTORE_EMULATOR_HOST', // emulator host:port + 'HARNESS_QUIET', // quiet mode flag + 'TEST_CROSSCHECK_LISTS_MATCH_UPDATE', // test update flag + 'DBT_PER_DEVELOPER_ENVIRONMENTS', // DBT config + 'STATSIG_FORD_DB_CHECKS', // statsig DB check flag + + // Build configuration + 'ANT_ENVIRONMENT', // Anthropic environment name + 'ANT_SERVICE', // Anthropic service name + 'MONOREPO_ROOT_DIR', // monorepo root path + + // Version selectors + 'PYENV_VERSION', // Python version selection + + // Credentials (approved subset - these don't change exfil risk) + 'PGPASSWORD', // Postgres password + 'GH_TOKEN', // GitHub token + 'GROWTHBOOK_API_KEY', // self-hosted growthbook +]) + +/** + * Strip comment lines (lines starting with #) and empty lines from a command. + */ +export function stripCommentLines(command: string): string { + const lines = command.split('\n') + const nonCommentLines = lines.filter(line => { + const trimmed = line.trim() + // Keep lines that are not empty and don't start with # + return trimmed !== '' && !trimmed.startsWith('#') + }) + + // If all lines were comments/empty, return original + if (nonCommentLines.length === 0) { + return command + } + + return nonCommentLines.join('\n') +} + +/** + * Strip safe wrapper commands (timeout, time, nice, nohup) and env vars + * from a bash command string for permission matching. + * + * This allows rules like Bash(npm install:*) to match "timeout 10 npm install foo" + * or "GOOS=linux go build". + * + * SECURITY: Uses [ \t]+ not \s+ — \s matches \n/\r which are command + * separators in bash. Matching across a newline would strip the wrapper from + * one line and leave a different command on the next line for bash to execute. + * + * KEEP IN SYNC with stripWrappersFromArgv below and checkSemantics in ast.ts. + */ +export function stripSafeWrappers(command: string): string { + // SECURITY: Use [ \t]+ not \s+ — \s matches \n/\r which are command + // separators in bash. Matching across a newline would strip the wrapper from + // one line and leave a different command on the next line for bash to execute. + // + // SECURITY: `(?:--[ \t]+)?` consumes the wrapper's own `--` so + // `nohup -- rm -- -/../foo` strips to `rm -- -/../foo` (not `-- rm ...` + // which would skip path validation with `--` as an unknown baseCmd). + const SAFE_WRAPPER_PATTERNS = [ + // timeout: enumerate GNU long flags — no-value (--foreground, + // --preserve-status, --verbose), value-taking in both =fused and + // space-separated forms (--kill-after=5, --kill-after 5, --signal=TERM, + // --signal TERM). Short: -v (no-arg), -k/-s with separate or fused value. + // SECURITY: flag VALUES use allowlist [A-Za-z0-9_.+-] (signals are + // TERM/KILL/9, durations are 5/5s/10.5). Previously [^ \t]+ matched + // $ ( ) ` | ; & — `timeout -k$(id) 10 ls` stripped to `ls`, matched + // Bash(ls:*), while bash expanded $(id) during word splitting BEFORE + // timeout ran. Contrast ENV_VAR_PATTERN below which already allowlists. + /^timeout[ \t]+(?:(?:--(?:foreground|preserve-status|verbose)|--(?:kill-after|signal)=[A-Za-z0-9_.+-]+|--(?:kill-after|signal)[ \t]+[A-Za-z0-9_.+-]+|-v|-[ks][ \t]+[A-Za-z0-9_.+-]+|-[ks][A-Za-z0-9_.+-]+)[ \t]+)*(?:--[ \t]+)?\d+(?:\.\d+)?[smhd]?[ \t]+/, + /^time[ \t]+(?:--[ \t]+)?/, + // SECURITY: keep in sync with checkSemantics wrapper-strip (ast.ts + // ~:1990-2080) AND stripWrappersFromArgv (pathValidation.ts ~:1260). + // Previously this pattern REQUIRED `-n N`; checkSemantics already handled + // bare `nice` and legacy `-N`. Asymmetry meant checkSemantics exposed the + // wrapped command to semantic checks but deny-rule matching and the cd+git + // gate saw the wrapper name. `nice rm -rf /` with Bash(rm:*) deny became + // ask instead of deny; `cd evil && nice git status` skipped the bare-repo + // RCE gate. PR #21503 fixed stripWrappersFromArgv; this was missed. + // Now matches: `nice cmd`, `nice -n N cmd`, `nice -N cmd` (all forms + // checkSemantics strips). + /^nice(?:[ \t]+-n[ \t]+-?\d+|[ \t]+-\d+)?[ \t]+(?:--[ \t]+)?/, + // stdbuf: fused short flags only (-o0, -eL). checkSemantics handles more + // (space-separated, long --output=MODE), but we fail-closed on those + // above so not over-stripping here is safe. Main need: `stdbuf -o0 cmd`. + /^stdbuf(?:[ \t]+-[ioe][LN0-9]+)+[ \t]+(?:--[ \t]+)?/, + /^nohup[ \t]+(?:--[ \t]+)?/, + ] as const + + // Pattern for environment variables: + // ^([A-Za-z_][A-Za-z0-9_]*) - Variable name (standard identifier) + // = - Equals sign + // ([A-Za-z0-9_./:-]+) - Value: alphanumeric + safe punctuation only + // [ \t]+ - Required HORIZONTAL whitespace after value + // + // SECURITY: Only matches unquoted values with safe characters (no $(), `, $var, ;|&). + // + // SECURITY: Trailing whitespace MUST be [ \t]+ (horizontal only), NOT \s+. + // \s matches \n/\r. If reconstructCommand emits an unquoted newline between + // `TZ=UTC` and `echo`, \s+ would match across it and strip `TZ=UTC`, + // leaving `echo curl evil.com` to match Bash(echo:*). But bash treats the + // newline as a command separator. Defense-in-depth with needsQuoting fix. + const ENV_VAR_PATTERN = /^([A-Za-z_][A-Za-z0-9_]*)=([A-Za-z0-9_./:-]+)[ \t]+/ + + let stripped = command + let previousStripped = '' + + // Phase 1: Strip leading env vars and comments only. + // In bash, env var assignments before a command (VAR=val cmd) are genuine + // shell-level assignments. These are safe to strip for permission matching. + while (stripped !== previousStripped) { + previousStripped = stripped + stripped = stripCommentLines(stripped) + + const envVarMatch = stripped.match(ENV_VAR_PATTERN) + if (envVarMatch) { + const varName = envVarMatch[1]! + const isAntOnlySafe = + process.env.USER_TYPE === 'ant' && ANT_ONLY_SAFE_ENV_VARS.has(varName) + if (SAFE_ENV_VARS.has(varName) || isAntOnlySafe) { + stripped = stripped.replace(ENV_VAR_PATTERN, '') + } + } + } + + // Phase 2: Strip wrapper commands and comments only. Do NOT strip env vars. + // Wrapper commands (timeout, time, nice, nohup) use execvp to run their + // arguments, so VAR=val after a wrapper is treated as the COMMAND to execute, + // not as an env var assignment. Stripping env vars here would create a + // mismatch between what the parser sees and what actually executes. + // (HackerOne #3543050) + previousStripped = '' + while (stripped !== previousStripped) { + previousStripped = stripped + stripped = stripCommentLines(stripped) + + for (const pattern of SAFE_WRAPPER_PATTERNS) { + stripped = stripped.replace(pattern, '') + } + } + + return stripped.trim() +} + +// SECURITY: allowlist for timeout flag VALUES (signals are TERM/KILL/9, +// durations are 5/5s/10.5). Rejects $ ( ) ` | ; & and newlines that +// previously matched via [^ \t]+ — `timeout -k$(id) 10 ls` must NOT strip. +const TIMEOUT_FLAG_VALUE_RE = /^[A-Za-z0-9_.+-]+$/ + +/** + * Parse timeout's GNU flags (long + short, fused + space-separated) and + * return the argv index of the DURATION token, or -1 if flags are unparseable. + * Enumerates: --foreground/--preserve-status/--verbose (no value), + * --kill-after/--signal (value, both =fused and space-separated), -v (no + * value), -k/-s (value, both fused and space-separated). + * + * Extracted from stripWrappersFromArgv to keep bashToolHasPermission under + * Bun's feature() DCE complexity threshold — inlining this breaks + * feature('BASH_CLASSIFIER') evaluation in classifier tests. + */ +export function skipTimeoutFlags(a: readonly string[]): number { + let i = 1 + while (i < a.length) { + const arg = a[i]! + const next = a[i + 1] + if ( + arg === '--foreground' || + arg === '--preserve-status' || + arg === '--verbose' + ) + i++ + else if (/^--(?:kill-after|signal)=[A-Za-z0-9_.+-]+$/.test(arg)) i++ + else if ( + (arg === '--kill-after' || arg === '--signal') && + next && + TIMEOUT_FLAG_VALUE_RE.test(next) + ) + i += 2 + else if (arg === '--') { + i++ + break + } // end-of-options marker + else if (arg.startsWith('--')) return -1 + else if (arg === '-v') i++ + else if ( + (arg === '-k' || arg === '-s') && + next && + TIMEOUT_FLAG_VALUE_RE.test(next) + ) + i += 2 + else if (/^-[ks][A-Za-z0-9_.+-]+$/.test(arg)) i++ + else if (arg.startsWith('-')) return -1 + else break + } + return i +} + +/** + * Argv-level counterpart to stripSafeWrappers. Strips the same wrapper + * commands (timeout, time, nice, nohup) from AST-derived argv. Env vars + * are already separated into SimpleCommand.envVars so no env-var stripping. + * + * KEEP IN SYNC with SAFE_WRAPPER_PATTERNS above — if you add a wrapper + * there, add it here too. + */ +export function stripWrappersFromArgv(argv: string[]): string[] { + // SECURITY: Consume optional `--` after wrapper options, matching what the + // wrapper does. Otherwise `['nohup','--','rm','--','-/../foo']` yields `--` + // as baseCmd and skips path validation. See SAFE_WRAPPER_PATTERNS comment. + let a = argv + for (;;) { + if (a[0] === 'time' || a[0] === 'nohup') { + a = a.slice(a[1] === '--' ? 2 : 1) + } else if (a[0] === 'timeout') { + const i = skipTimeoutFlags(a) + if (i < 0 || !a[i] || !/^\d+(?:\.\d+)?[smhd]?$/.test(a[i]!)) return a + a = a.slice(i + 1) + } else if ( + a[0] === 'nice' && + a[1] === '-n' && + a[2] && + /^-?\d+$/.test(a[2]) + ) { + a = a.slice(a[3] === '--' ? 4 : 3) + } else { + return a + } + } +} + +/** + * Env vars that make a *different binary* run (injection or resolution hijack). + * Heuristic only — export-&& form bypasses this, and excludedCommands isn't a + * security boundary anyway. + */ +export const BINARY_HIJACK_VARS = /^(LD_|DYLD_|PATH$)/ + +/** + * Strip ALL leading env var prefixes from a command, regardless of whether the + * var name is in the safe-list. + * + * Used for deny/ask rule matching: when a user denies `claude` or `rm`, the + * command should stay blocked even if prefixed with arbitrary env vars like + * `FOO=bar claude`. The safe-list restriction in stripSafeWrappers is correct + * for allow rules (prevents `DOCKER_HOST=evil docker ps` from auto-matching + * `Bash(docker ps:*)`), but deny rules must be harder to circumvent. + * + * Also used for sandbox.excludedCommands matching (not a security boundary — + * permission prompts are), with BINARY_HIJACK_VARS as a blocklist. + * + * SECURITY: Uses a broader value pattern than stripSafeWrappers. The value + * pattern excludes only actual shell injection characters ($, backtick, ;, |, + * &, parens, redirects, quotes, backslash) and whitespace. Characters like + * =, +, @, ~, , are harmless in unquoted env var assignment position and must + * be matched to prevent trivial bypass via e.g. `FOO=a=b denied_command`. + * + * @param blocklist - optional regex tested against each var name; matching vars + * are NOT stripped (and stripping stops there). Omit for deny rules; pass + * BINARY_HIJACK_VARS for excludedCommands. + */ +export function stripAllLeadingEnvVars( + command: string, + blocklist?: RegExp, +): string { + // Broader value pattern for deny-rule stripping. Handles: + // + // - Standard assignment (FOO=bar), append (FOO+=bar), array (FOO[0]=bar) + // - Single-quoted values: '[^'\n\r]*' — bash suppresses all expansion + // - Double-quoted values with backslash escapes: "(?:\\.|[^"$`\\\n\r])*" + // In bash double quotes, only \$, \`, \", \\, and \newline are special. + // Other \x sequences are harmless, so we allow \. inside double quotes. + // We still exclude raw $ and ` (without backslash) to block expansion. + // - Unquoted values: excludes shell metacharacters, allows backslash escapes + // - Concatenated segments: FOO='x'y"z" — bash concatenates adjacent segments + // + // SECURITY: Trailing whitespace MUST be [ \t]+ (horizontal only), NOT \s+. + // + // The outer * matches one atomic unit per iteration: a complete quoted + // string, a backslash-escape pair, or a single unquoted safe character. + // The inner double-quote alternation (?:...|...)* is bounded by the + // closing ", so it cannot interact with the outer * for backtracking. + // + // Note: $ is excluded from unquoted/double-quoted value classes to block + // dangerous forms like $(cmd), ${var}, and $((expr)). This means + // FOO=$VAR is not stripped — adding $VAR matching creates ReDoS risk + // (CodeQL #671) and $VAR bypasses are low-priority. + const ENV_VAR_PATTERN = + /^([A-Za-z_][A-Za-z0-9_]*(?:\[[^\]]*\])?)\+?=(?:'[^'\n\r]*'|"(?:\\.|[^"$`\\\n\r])*"|\\.|[^ \t\n\r$`;|&()<>\\\\'"])*[ \t]+/ + + let stripped = command + let previousStripped = '' + + while (stripped !== previousStripped) { + previousStripped = stripped + stripped = stripCommentLines(stripped) + + const m = stripped.match(ENV_VAR_PATTERN) + if (!m) continue + if (blocklist?.test(m[1]!)) break + stripped = stripped.slice(m[0].length) + } + + return stripped.trim() +} \ No newline at end of file diff --git a/src/utils/permissions/permissionChecker.ts b/src/utils/permissions/permissionChecker.ts new file mode 100644 index 0000000000..31f6bd8fcd --- /dev/null +++ b/src/utils/permissions/permissionChecker.ts @@ -0,0 +1,78 @@ +/** + * Shared permission-checking logic for shell tool permission matching. + * + * Both BashTool and PowerShellTool follow the same pattern for + * `matchingRulesForInput`: fetch deny/ask/allow rule maps, then + * call a shell-specific filter function on each. This module + * extracts that shared orchestration logic. + * + * Shell-specific `filterRulesByContentsMatchingInput` implementations + * remain in their respective tool directories — they differ too much + * in redirection handling, env-var stripping, case sensitivity, + * and canonical resolution to share directly. + */ + +import type { ToolPermissionContext } from '../../Tool.js' +import type { PermissionRule } from './PermissionRule.js' +import { getRuleByContentsForToolName } from './permissions.js' + +/** + * Generic signature for shell-specific rule filtering. + * Each shell tool provides its own implementation that knows + * how to normalize and match commands against rules. + */ +export type FilterRulesFn = ( + rules: Map, + matchMode: 'exact' | 'prefix', + behavior: 'deny' | 'ask' | 'allow', +) => PermissionRule[] + +/** + * Fetch matching deny/ask/allow rules for a shell tool input. + * + * This is the shared logic previously duplicated between + * bashPermissions.ts and powershellPermissions.ts. + * Callers provide: + * - `toolName`: the tool's permission name (e.g. "Bash" or "PowerShell") + * - `toolPermissionContext`: current permission state + * - `matchMode`: 'exact' for full-command matching, 'prefix' for subcommand + * - `filterFn`: shell-specific filter function + * + * SECURITY: Deny/ask rules pass `behavior` to the filter so the shell + * implementation can apply more aggressive stripping (e.g. all env vars + * for bash, module prefix stripping for PowerShell). Allow rules are + * intentionally stricter to prevent over-matching. + */ +export function matchingRulesForInput( + toolName: string, + toolPermissionContext: ToolPermissionContext, + matchMode: 'exact' | 'prefix', + filterFn: FilterRulesFn, +): { + matchingDenyRules: PermissionRule[] + matchingAskRules: PermissionRule[] + matchingAllowRules: PermissionRule[] +} { + const denyRuleByContents = getRuleByContentsForToolName( + toolPermissionContext, + toolName, + 'deny', + ) + const matchingDenyRules = filterFn(denyRuleByContents, matchMode, 'deny') + + const askRuleByContents = getRuleByContentsForToolName( + toolPermissionContext, + toolName, + 'ask', + ) + const matchingAskRules = filterFn(askRuleByContents, matchMode, 'ask') + + const allowRuleByContents = getRuleByContentsForToolName( + toolPermissionContext, + toolName, + 'allow', + ) + const matchingAllowRules = filterFn(allowRuleByContents, matchMode, 'allow') + + return { matchingDenyRules, matchingAskRules, matchingAllowRules } +} \ No newline at end of file