Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 56 additions & 5 deletions assistant/src/permissions/checker.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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<string, RiskLevel>();

function riskCacheKey(toolName: string, input: Record<string, unknown>): string {
const inputJson = JSON.stringify(input);
const hash = createHash('sha256').update(inputJson).digest('hex');
return `${toolName}\0${hash}`;
}
Comment thread
siddseethepalli marked this conversation as resolved.

/** 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;

Expand Down Expand Up @@ -281,6 +305,33 @@ async function buildCommandCandidates(toolName: string, input: Record<string, un
}

export async function classifyRisk(toolName: string, input: Record<string, unknown>, workingDir?: string, preParsed?: ParsedCommand, manifestOverride?: ManifestOverride): Promise<RiskLevel> {
// 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);
Comment thread
siddseethepalli marked this conversation as resolved.
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<string, unknown>, workingDir?: string, preParsed?: ParsedCommand, manifestOverride?: ManifestOverride): Promise<RiskLevel> {
if (toolName === 'file_read') return RiskLevel.Low;
if (toolName === 'file_write' || toolName === 'file_edit') {
const filePath = getStringField(input, 'path', 'file_path');
Expand Down Expand Up @@ -320,7 +371,7 @@ export async function classifyRisk(toolName: string, input: Record<string, unkno
const command = (input.command as string) ?? '';
if (!command.trim()) return RiskLevel.Low;

const parsed = preParsed ?? await parse(command);
const parsed = preParsed ?? await cachedParse(command);

// Dangerous patterns → High
if (parsed.dangerousPatterns.length > 0) return RiskLevel.High;
Expand Down Expand Up @@ -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);
}
}

Expand Down
32 changes: 31 additions & 1 deletion assistant/src/permissions/shell-identity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, ParsedCommand>();

export async function cachedParse(command: string): Promise<ParsedCommand> {
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;
Expand Down Expand Up @@ -40,7 +70,7 @@ const MAX_ACTION_KEY_DEPTH = 3;
* identity information for permission decisions.
*/
export async function analyzeShellCommand(command: string, preParsed?: ParsedCommand): Promise<ShellIdentityAnalysis> {
const parsed = preParsed ?? await parse(command);
const parsed = preParsed ?? await cachedParse(command);

const operators: string[] = [];
for (const seg of parsed.segments) {
Expand Down
20 changes: 20 additions & 0 deletions assistant/src/permissions/trust-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -368,6 +383,7 @@ export function addRule(
cachedRules = rules;
rebuildPatternCache(rules);
saveToDisk(rules);
notifyRulesChanged();
log.info({ rule }, 'Added trust rule');
return rule;
}
Expand Down Expand Up @@ -395,6 +411,7 @@ export function updateRule(
cachedRules = rules;
rebuildPatternCache(rules);
saveToDisk(rules);
notifyRulesChanged();
log.info({ rule }, 'Updated trust rule');
return rule;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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)');
}

Expand Down Expand Up @@ -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 };
Expand Down