From afda3461320cfb21ed0f80e4a9adaeb216f3c753 Mon Sep 17 00:00:00 2001 From: siddseethepalli Date: Tue, 24 Feb 2026 19:54:03 +0000 Subject: [PATCH] perf: cache shell-parsing and risk classification results Co-Authored-By: Claude --- assistant/src/permissions/checker.ts | 61 +++++++++++++++++++-- assistant/src/permissions/shell-identity.ts | 32 ++++++++++- assistant/src/permissions/trust-store.ts | 20 +++++++ 3 files changed, 107 insertions(+), 6 deletions(-) diff --git a/assistant/src/permissions/checker.ts b/assistant/src/permissions/checker.ts index 2b4c92faa88..6e06af79eb3 100644 --- a/assistant/src/permissions/checker.ts +++ b/assistant/src/permissions/checker.ts @@ -1,6 +1,6 @@ +import { createHash } from 'node:crypto'; import { RiskLevel, type PermissionCheckResult, type AllowlistOption, type ScopeOption, type PolicyContext } from './types.js'; -import { findHighestPriorityRule } from './trust-store.js'; -import { parse } from '../tools/terminal/parser.js'; +import { findHighestPriorityRule, onRulesChanged } from './trust-store.js'; import { resolveSkillSelector } from '../config/skills.js'; import { computeSkillVersionHash } from '../skills/version-hash.js'; import { getTool } from '../tools/registry.js'; @@ -11,9 +11,33 @@ import { homedir } from 'node:os'; import { looksLikeHostPortShorthand, looksLikePathOnlyInput } from '../tools/network/url-safety.js'; import { normalizeFilePath, isSkillSourcePath } from '../skills/path-classifier.js'; import { isWorkspaceScopedInvocation } from './workspace-policy.js'; -import { buildShellCommandCandidates, buildShellAllowlistOptions, type ParsedCommand } from './shell-identity.js'; +import { buildShellCommandCandidates, buildShellAllowlistOptions, cachedParse, type ParsedCommand } from './shell-identity.js'; import type { ManifestOverride } from '../tools/execution-target.js'; +// ── Risk classification cache ──────────────────────────────────────────────── +// classifyRisk() is called on every permission check and can invoke WASM +// parsing for shell commands. Cache results keyed on (toolName, inputHash). +// Invalidated when trust rules change since risk classification for file tools +// depends on skill source path checks which reference config, but the core +// risk logic is input-deterministic. +const RISK_CACHE_MAX = 256; +const riskCache = new Map(); + +function riskCacheKey(toolName: string, input: Record): string { + const inputJson = JSON.stringify(input); + const hash = createHash('sha256').update(inputJson).digest('hex'); + return `${toolName}\0${hash}`; +} + +/** Clear the risk classification cache. Called when trust rules change. */ +export function clearRiskCache(): void { + riskCache.clear(); +} + +// Invalidate risk cache whenever trust rules change so that risk decisions +// referencing config-dependent checks (e.g. skill source paths) stay fresh. +onRulesChanged(clearRiskCache); + // Ensures the legacy mode deprecation warning fires at most once per process. let _legacyDeprecationWarned = false; @@ -281,6 +305,33 @@ async function buildCommandCandidates(toolName: string, input: Record, workingDir?: string, preParsed?: ParsedCommand, manifestOverride?: ManifestOverride): Promise { + // Check cache first (skip when preParsed is provided since caller already + // parsed and we'd just be duplicating the key computation cost). + const cacheKey = preParsed ? null : riskCacheKey(toolName, input); + if (cacheKey) { + const cached = riskCache.get(cacheKey); + if (cached !== undefined) { + // LRU refresh + riskCache.delete(cacheKey); + riskCache.set(cacheKey, cached); + return cached; + } + } + + const result = await classifyRiskUncached(toolName, input, workingDir, preParsed, manifestOverride); + + if (cacheKey) { + if (riskCache.size >= RISK_CACHE_MAX) { + const oldest = riskCache.keys().next().value; + if (oldest !== undefined) riskCache.delete(oldest); + } + riskCache.set(cacheKey, result); + } + + return result; +} + +async function classifyRiskUncached(toolName: string, input: Record, workingDir?: string, preParsed?: ParsedCommand, manifestOverride?: ManifestOverride): Promise { if (toolName === 'file_read') return RiskLevel.Low; if (toolName === 'file_write' || toolName === 'file_edit') { const filePath = getStringField(input, 'path', 'file_path'); @@ -320,7 +371,7 @@ export async function classifyRisk(toolName: string, input: Record 0) return RiskLevel.High; @@ -417,7 +468,7 @@ export async function check( if (toolName === 'bash' || toolName === 'host_bash') { const command = ((input.command as string) ?? '').trim(); if (command) { - shellParsed = await parse(command); + shellParsed = await cachedParse(command); } } diff --git a/assistant/src/permissions/shell-identity.ts b/assistant/src/permissions/shell-identity.ts index 127ad29ebfb..7b4949f2365 100644 --- a/assistant/src/permissions/shell-identity.ts +++ b/assistant/src/permissions/shell-identity.ts @@ -3,6 +3,36 @@ import type { AllowlistOption } from './types.js'; export type { ParsedCommand }; +// ── Shell parse result cache ───────────────────────────────────────────────── +// Shell parsing via web-tree-sitter WASM is deterministic — the same command +// string always produces the same ParsedCommand. Cache results to avoid +// redundant WASM invocations on repeated permission checks. +const PARSE_CACHE_MAX = 256; +const parseCache = new Map(); + +export async function cachedParse(command: string): Promise { + const cached = parseCache.get(command); + if (cached !== undefined) { + // LRU refresh: move to end of insertion order + parseCache.delete(command); + parseCache.set(command, cached); + return cached; + } + const result = await parse(command); + // Evict oldest entry if at capacity + if (parseCache.size >= PARSE_CACHE_MAX) { + const oldest = parseCache.keys().next().value; + if (oldest !== undefined) parseCache.delete(oldest); + } + parseCache.set(command, result); + return result; +} + +/** Clear the shell parse cache. Exposed for testing. */ +export function clearShellParseCache(): void { + parseCache.clear(); +} + export interface ShellActionKey { /** e.g. "action:gh", "action:gh pr", "action:gh pr view" */ key: string; @@ -40,7 +70,7 @@ const MAX_ACTION_KEY_DEPTH = 3; * identity information for permission decisions. */ export async function analyzeShellCommand(command: string, preParsed?: ParsedCommand): Promise { - const parsed = preParsed ?? await parse(command); + const parsed = preParsed ?? await cachedParse(command); const operators: string[] = []; for (const seg of parsed.segments) { diff --git a/assistant/src/permissions/trust-store.ts b/assistant/src/permissions/trust-store.ts index a8a4d15ae65..607e5f06bb8 100644 --- a/assistant/src/permissions/trust-store.ts +++ b/assistant/src/permissions/trust-store.ts @@ -21,6 +21,21 @@ interface TrustFile { let cachedRules: TrustRule[] | null = null; let cachedStarterBundleAccepted: boolean | null = null; +// Callbacks invoked when trust rules change (add/update/remove/clear). +// Used by the permission checker to invalidate dependent caches. +const rulesChangedListeners: Array<() => void> = []; + +/** Register a callback to be invoked whenever trust rules change. */ +export function onRulesChanged(listener: () => void): void { + rulesChangedListeners.push(listener); +} + +function notifyRulesChanged(): void { + for (const listener of rulesChangedListeners) { + listener(); + } +} + /** * Cache of pre-compiled Minimatch objects keyed by pattern string. * Rebuilt whenever cachedRules changes. Avoids re-parsing glob patterns @@ -368,6 +383,7 @@ export function addRule( cachedRules = rules; rebuildPatternCache(rules); saveToDisk(rules); + notifyRulesChanged(); log.info({ rule }, 'Added trust rule'); return rule; } @@ -395,6 +411,7 @@ export function updateRule( cachedRules = rules; rebuildPatternCache(rules); saveToDisk(rules); + notifyRulesChanged(); log.info({ rule }, 'Updated trust rule'); return rule; } @@ -412,6 +429,7 @@ export function removeRule(id: string): boolean { cachedRules = rules; rebuildPatternCache(rules); saveToDisk(rules); + notifyRulesChanged(); log.info({ id }, 'Removed trust rule'); return true; } @@ -508,6 +526,7 @@ export function clearAllRules(): void { cachedRules = rules; rebuildPatternCache(rules); saveToDisk(rules); + notifyRulesChanged(); log.info('Cleared all user trust rules (default rules preserved)'); } @@ -608,6 +627,7 @@ export function acceptStarterBundle(): AcceptStarterBundleResult { cachedRules = rules; rebuildPatternCache(rules); saveToDisk(rules); + notifyRulesChanged(); log.info({ rulesAdded: added }, 'Starter approval bundle accepted'); return { accepted: true, rulesAdded: added, alreadyAccepted: false };