diff --git a/docs/policy-safety-model.md b/docs/policy-safety-model.md new file mode 100644 index 0000000..d5bd27a --- /dev/null +++ b/docs/policy-safety-model.md @@ -0,0 +1,52 @@ +# Stronger Safety Model Guide + +This guide explains how to interpret policy denials and adjust scoped approvals and path allowlists safely. + +## Decision Envelope + +Policy outcomes now include: +- `decision`: `allow`, `deny`, or `approval_required` +- `matchedRules`: stable identifiers for matched policy checks +- `scopeContext`: approval scope tuple (`principal`, `operationClass`, `resourceScope`, `scopeId`) plus optional lifecycle fields (`expiresAt`, `revoked`) +- `reasonCodes`: normalized denial/approval reasons + +Legacy fields (`allowed`, `requiresApproval`, `reason`, `sideEffect`) remain available for compatibility. +When present in `scopeContext`, `expiresAt` is the ISO-8601 expiration timestamp for a persisted scope and `revoked` indicates that scope cannot be reused. + +## Actionable Reason Codes + +- `approval_required_side_effect`: interactive approval is required for the operation class. +- `approval_scope_granted`: interactive approval was granted and persisted for the exact scope. +- `approval_scope_reused`: an existing scoped approval matched exactly and was reused. +- `approval_expired`: the prior scoped approval has expired and must be re-approved. +- `approval_revoked`: the scope was explicitly revoked and cannot be reused. +- `missing_allowlist`: allowlist policy is active but no valid allowlist exists for this operation. +- `missing_target_paths`: guarded command did not provide explicit target paths for allowlist evaluation. +- `path_out_of_allowlist`: at least one canonicalized path is outside allowed roots. +- `path_canonicalization_failure`: canonicalization failed for a configured root or target path. +- `blocked_command_pattern`: command matched a hard-block pattern. + +## Managing Path Allowlists + +Use policy config keys: +- `pathAllowlistByOperation`: operation-specific allow roots keyed by side effect (`write`, `destructive`, etc.) +- `pathAllowlist`: fallback roots when operation-specific roots are not present +- `allowlistPolicyOperations`: operation classes where strict allowlist enforcement is required + +Guidance: +1. Add only absolute, canonical roots for intended write targets. +2. Keep roots narrow and operation-specific where possible. +3. For automation writes, ensure allowlists are configured before execution to avoid deny-by-default outcomes. + +## Managing Scoped Approvals + +Scoped approvals are keyed by: +- principal identity +- operation class +- resource scope (derived from command + cwd) + +Approvals are reused only when all scope keys match exactly and the record is both unexpired and not revoked. + +Revocation: +- Revoke by `scopeId` when permissions should no longer be reused. +- After revocation, the same request will return `approval_revoked` until re-approved. diff --git a/openspec/changes/stronger-safety-model/.openspec.yaml b/openspec/changes/archive/2026-03-03-stronger-safety-model/.openspec.yaml similarity index 100% rename from openspec/changes/stronger-safety-model/.openspec.yaml rename to openspec/changes/archive/2026-03-03-stronger-safety-model/.openspec.yaml diff --git a/openspec/changes/stronger-safety-model/design.md b/openspec/changes/archive/2026-03-03-stronger-safety-model/design.md similarity index 100% rename from openspec/changes/stronger-safety-model/design.md rename to openspec/changes/archive/2026-03-03-stronger-safety-model/design.md diff --git a/openspec/changes/stronger-safety-model/proposal.md b/openspec/changes/archive/2026-03-03-stronger-safety-model/proposal.md similarity index 100% rename from openspec/changes/stronger-safety-model/proposal.md rename to openspec/changes/archive/2026-03-03-stronger-safety-model/proposal.md diff --git a/openspec/changes/stronger-safety-model/specs/explainable-policy-decisions/spec.md b/openspec/changes/archive/2026-03-03-stronger-safety-model/specs/explainable-policy-decisions/spec.md similarity index 100% rename from openspec/changes/stronger-safety-model/specs/explainable-policy-decisions/spec.md rename to openspec/changes/archive/2026-03-03-stronger-safety-model/specs/explainable-policy-decisions/spec.md diff --git a/openspec/changes/stronger-safety-model/specs/path-allowlist-enforcement/spec.md b/openspec/changes/archive/2026-03-03-stronger-safety-model/specs/path-allowlist-enforcement/spec.md similarity index 100% rename from openspec/changes/stronger-safety-model/specs/path-allowlist-enforcement/spec.md rename to openspec/changes/archive/2026-03-03-stronger-safety-model/specs/path-allowlist-enforcement/spec.md diff --git a/openspec/changes/stronger-safety-model/specs/scoped-persisted-approvals/spec.md b/openspec/changes/archive/2026-03-03-stronger-safety-model/specs/scoped-persisted-approvals/spec.md similarity index 100% rename from openspec/changes/stronger-safety-model/specs/scoped-persisted-approvals/spec.md rename to openspec/changes/archive/2026-03-03-stronger-safety-model/specs/scoped-persisted-approvals/spec.md diff --git a/openspec/changes/archive/2026-03-03-stronger-safety-model/tasks.md b/openspec/changes/archive/2026-03-03-stronger-safety-model/tasks.md new file mode 100644 index 0000000..b07bde0 --- /dev/null +++ b/openspec/changes/archive/2026-03-03-stronger-safety-model/tasks.md @@ -0,0 +1,30 @@ +## 1. Policy Contract and Data Model + +- [x] 1.1 Define the structured policy decision envelope (`decision`, `matchedRules`, `scopeContext`, `reasonCodes`) in shared types/contracts +- [x] 1.2 Add scoped approval persistence model fields (principal, operation class, resource scope, expiration, revocation state) +- [x] 1.3 Implement approval lookup/match logic that requires exact scope-key matching before reuse + +## 2. Scoped Persisted Approvals + +- [x] 2.1 Implement expiration validation for persisted approvals at decision time +- [x] 2.2 Implement approval revocation by scope identifier and ensure revoked approvals are excluded from reuse +- [x] 2.3 Add tests for scope-match success, scope mismatch denial, expiration, and revocation behavior + +## 3. Strict Path Allowlist Enforcement + +- [x] 3.1 Implement centralized path canonicalization utility for guarded operations +- [x] 3.2 Implement allowlist matcher over canonical absolute paths with deny-by-default semantics when allowlist policy is enabled for the operation +- [x] 3.3 Integrate a mandatory guard at all file-mutation/sensitive command operation boundaries +- [x] 3.4 Add tests for in-allowlist allow, out-of-allowlist deny, missing allowlist deny in allowlist-enabled mode, interactive approval-gated fallback without allowlist, and canonicalization-failure deny + +## 4. Explainable Policy Decisions + +- [x] 4.1 Update policy evaluation pipeline to emit structured explanation fields for both allow and deny outcomes +- [x] 4.2 Standardize reason-code taxonomy for path enforcement and approval-scope outcomes +- [x] 4.3 Add deterministic-output tests to ensure identical inputs return identical decision explanation payloads + +## 5. Rollout and Validation + +- [x] 5.1 Add compatibility/adapter handling for legacy decision consumers during rollout +- [x] 5.2 Add integration tests that cover end-to-end guarded execution with scoped approvals and path allowlists +- [x] 5.3 Document operator-facing guidance for interpreting denial reasons and updating allowlists/scopes diff --git a/openspec/changes/stronger-safety-model/tasks.md b/openspec/changes/stronger-safety-model/tasks.md deleted file mode 100644 index c737aeb..0000000 --- a/openspec/changes/stronger-safety-model/tasks.md +++ /dev/null @@ -1,30 +0,0 @@ -## 1. Policy Contract and Data Model - -- [ ] 1.1 Define the structured policy decision envelope (`decision`, `matchedRules`, `scopeContext`, `reasonCodes`) in shared types/contracts -- [ ] 1.2 Add scoped approval persistence model fields (principal, operation class, resource scope, expiration, revocation state) -- [ ] 1.3 Implement approval lookup/match logic that requires exact scope-key matching before reuse - -## 2. Scoped Persisted Approvals - -- [ ] 2.1 Implement expiration validation for persisted approvals at decision time -- [ ] 2.2 Implement approval revocation by scope identifier and ensure revoked approvals are excluded from reuse -- [ ] 2.3 Add tests for scope-match success, scope mismatch denial, expiration, and revocation behavior - -## 3. Strict Path Allowlist Enforcement - -- [ ] 3.1 Implement centralized path canonicalization utility for guarded operations -- [ ] 3.2 Implement allowlist matcher over canonical absolute paths with deny-by-default semantics when allowlist policy is enabled for the operation -- [ ] 3.3 Integrate a mandatory guard at all file-mutation/sensitive command operation boundaries -- [ ] 3.4 Add tests for in-allowlist allow, out-of-allowlist deny, missing allowlist deny in allowlist-enabled mode, interactive approval-gated fallback without allowlist, and canonicalization-failure deny - -## 4. Explainable Policy Decisions - -- [ ] 4.1 Update policy evaluation pipeline to emit structured explanation fields for both allow and deny outcomes -- [ ] 4.2 Standardize reason-code taxonomy for path enforcement and approval-scope outcomes -- [ ] 4.3 Add deterministic-output tests to ensure identical inputs return identical decision explanation payloads - -## 5. Rollout and Validation - -- [ ] 5.1 Add compatibility/adapter handling for legacy decision consumers during rollout -- [ ] 5.2 Add integration tests that cover end-to-end guarded execution with scoped approvals and path allowlists -- [ ] 5.3 Document operator-facing guidance for interpreting denial reasons and updating allowlists/scopes diff --git a/openspec/specs/explainable-policy-decisions/spec.md b/openspec/specs/explainable-policy-decisions/spec.md new file mode 100644 index 0000000..168cb53 --- /dev/null +++ b/openspec/specs/explainable-policy-decisions/spec.md @@ -0,0 +1,22 @@ +# explainable-policy-decisions Specification + +## Purpose +Define requirements for deterministic, structured, and user-interpretable policy decision explanations across allow, deny, and approval-required outcomes. +## Requirements +### Requirement: Policy decisions SHALL include structured explanations +Each policy evaluation result SHALL include a structured explanation envelope containing decision outcome, matched rule identifiers, scope context, and standardized reason codes. + +#### Scenario: Denial includes actionable explanation fields +- **WHEN** a request is denied by policy +- **THEN** the decision payload includes non-empty reason codes and the rule identifiers that contributed to denial + +#### Scenario: Approval includes scope context +- **WHEN** a request is approved by policy +- **THEN** the decision payload includes the effective scope context used for that approval + +### Requirement: Decision explanations MUST be deterministic +Given identical policy inputs, context, and configuration state, the system MUST return the same decision outcome and explanation fields. + +#### Scenario: Repeated identical request yields same explanation +- **WHEN** the same request is evaluated repeatedly without any policy or context changes +- **THEN** each evaluation returns identical decision outcome and explanation payload fields diff --git a/openspec/specs/path-allowlist-enforcement/spec.md b/openspec/specs/path-allowlist-enforcement/spec.md new file mode 100644 index 0000000..87e22ea --- /dev/null +++ b/openspec/specs/path-allowlist-enforcement/spec.md @@ -0,0 +1,30 @@ +# path-allowlist-enforcement Specification + +## Purpose +Define canonical path-allowlist enforcement behavior for guarded file mutations and sensitive commands, including deny-by-default handling for missing or invalid policy inputs. +## Requirements +### Requirement: Guarded operations MUST enforce canonical path allowlists +For every guarded file-system mutation or sensitive command operation that uses allowlist policy (including automation safe-write policy), the system MUST resolve target paths to canonical absolute paths before evaluating allowlists. The operation SHALL proceed only if every target path is contained within an allowed root for that operation. + +#### Scenario: Canonical in-allowlist path is permitted +- **WHEN** all requested target paths canonicalize to locations within configured allowed roots +- **THEN** the operation is permitted subject to other policy checks + +#### Scenario: Canonical out-of-allowlist path is denied +- **WHEN** any requested target path canonicalizes outside configured allowed roots +- **THEN** the system denies the operation with a path-allowlist denial reason + +### Requirement: Path enforcement SHALL be deny-by-default +The system SHALL deny guarded operations when allowlist policy is configured but is absent for the requested operation, invalid, or cannot be evaluated. In interactive mode where no allowlist policy is configured for that operation, the system SHALL fall back to approval-gated policy flow. + +#### Scenario: Missing allowlist configuration denies operation +- **WHEN** an operation is evaluated under allowlist policy and no valid allowlist is available +- **THEN** the system denies the operation and returns an explicit missing-allowlist reason + +#### Scenario: Interactive mode without allowlist remains approval-gated +- **WHEN** a guarded mutating operation is requested in interactive mode and no allowlist policy is configured for that operation +- **THEN** the system follows approval-gated policy flow and does not auto-deny solely due to missing allowlist + +#### Scenario: Canonicalization failure denies operation +- **WHEN** target path canonicalization fails for any requested path +- **THEN** the system denies the operation and reports canonicalization-failure reason diff --git a/openspec/specs/scoped-persisted-approvals/spec.md b/openspec/specs/scoped-persisted-approvals/spec.md new file mode 100644 index 0000000..1e307de --- /dev/null +++ b/openspec/specs/scoped-persisted-approvals/spec.md @@ -0,0 +1,26 @@ +# scoped-persisted-approvals Specification + +## Purpose +Define scope constraints, expiration, and revocation requirements for persisted approvals so prior approvals are reused only when all scoped conditions match. +## Requirements +### Requirement: Persisted approvals SHALL be scope-bound +The system SHALL persist approvals with explicit scope keys including principal identity, operation class, resource scope, and expiration metadata. A persisted approval MUST only be reusable when all scope keys match the current request. + +#### Scenario: Approval is reused only within identical scope +- **WHEN** a request matches principal, operation class, resource scope, and approval validity window of a stored approval +- **THEN** the system reuses the persisted approval and marks the decision as approved without prompting + +#### Scenario: Approval is rejected on scope mismatch +- **WHEN** a stored approval exists but any scope key differs from the current request +- **THEN** the system SHALL NOT reuse the approval and SHALL require a new policy decision + +### Requirement: Persisted approvals SHALL expire and be revocable +The system SHALL enforce expiration on persisted approvals and MUST support explicit revocation by scope identifier. + +#### Scenario: Expired approval is ignored +- **WHEN** a matching persisted approval has passed its expiration timestamp +- **THEN** the system treats the approval as invalid and requires a new decision + +#### Scenario: Revoked approval is not reused +- **WHEN** an approval has been revoked for the matching scope +- **THEN** the system SHALL NOT apply it to subsequent requests diff --git a/src/cli/app.tsx b/src/cli/app.tsx index b176592..20dc41e 100644 --- a/src/cli/app.tsx +++ b/src/cli/app.tsx @@ -400,7 +400,7 @@ function getPolicyOutcome(result: ToolResult): { reason: string; sideEffect: ToolInvocation['sideEffect']; } | null { - const candidate = result.payload.policyOutcome; + const candidate = result.payload.policyOutcome ?? result.payload.policyOutcomeLegacy; if (!candidate || typeof candidate !== 'object') { return null; } diff --git a/src/policy/defaults.ts b/src/policy/defaults.ts index 167a109..f8e262e 100644 --- a/src/policy/defaults.ts +++ b/src/policy/defaults.ts @@ -8,6 +8,9 @@ export function createDefaultApprovalPolicy( commandAllowlist: ['ls', 'cat', 'rg', 'find', 'pwd', 'git status'], pathAllowlist: [], automationWriteAllowlist: [], + pathAllowlistByOperation: {}, + allowlistPolicyOperations: ['write'], + approvalTtlMs: 10 * 60 * 1000, blockedCommandPatterns: ['rm -rf /', 'mkfs', 'dd if=', ':(){ :|:& };:'], ...overrides, }; diff --git a/src/policy/engine.ts b/src/policy/engine.ts index cc78982..2630232 100644 --- a/src/policy/engine.ts +++ b/src/policy/engine.ts @@ -1,72 +1,338 @@ +import { createHash } from 'node:crypto'; import type { ToolInvocation } from '../tools/schemas'; +import { + canonicalizeGuardedPath, + extractCommandTargetPaths, + isWithinAllowedRoots, +} from './path-guard'; +import { PolicyReasonCode, type PolicyReasonCodeValue } from './reason-codes'; import type { ApprovalDecision, ApprovalPolicy } from './schemas'; +import { type ApprovalScopeContext, InMemoryScopedApprovalStore } from './scoped-approvals'; export type CommandEvaluationInput = { command: string; cwd: string; mode: 'interactive' | 'automation'; sideEffect: 'read' | 'write' | 'destructive' | 'network'; + approvalGranted?: boolean; + principal?: string; }; export class DefaultPolicyEngine { + private readonly scopedApprovals = new InMemoryScopedApprovalStore(); + constructor(private readonly policy: ApprovalPolicy) {} getPolicy(): ApprovalPolicy { return this.policy; } + revokeApproval(scopeId: string): boolean { + return this.scopedApprovals.revoke(scopeId); + } + evaluateCommand(input: CommandEvaluationInput): ApprovalDecision { + const scopeContext = this.buildScopeContext(input); + for (const pattern of this.policy.blockedCommandPatterns) { if (input.command.includes(pattern)) { - return { - allowed: false, - requiresApproval: false, + return this.buildDecision({ + input, + scopeContext, + decision: 'deny', reason: `Command matches blocked pattern: ${pattern}`, - sideEffect: input.sideEffect, - }; + reasonCodes: [PolicyReasonCode.blockedCommandPattern], + matchedRules: [`blocked-pattern:${pattern}`], + }); } } + const pathGuardResult = this.evaluatePathGuard(input, scopeContext); + if (pathGuardResult.deniedDecision) { + return pathGuardResult.deniedDecision; + } + + const existingApproval = this.scopedApprovals.matchExact(scopeContext); + if (existingApproval) { + const expired = new Date(existingApproval.expiresAt).getTime() <= Date.now(); + if (existingApproval.revokedAt) { + return this.buildDecision({ + input, + scopeContext: { + ...scopeContext, + expiresAt: existingApproval.expiresAt, + revoked: true, + }, + decision: 'approval_required', + reason: 'Scoped approval was revoked; new approval required', + reasonCodes: [PolicyReasonCode.approvalRevoked], + matchedRules: ['approval:revoked'], + }); + } + if (expired) { + return this.buildDecision({ + input, + scopeContext: { + ...scopeContext, + expiresAt: existingApproval.expiresAt, + revoked: false, + }, + decision: 'approval_required', + reason: 'Scoped approval expired; new approval required', + reasonCodes: [PolicyReasonCode.approvalExpired], + matchedRules: ['approval:expired'], + }); + } + + return this.buildDecision({ + input, + scopeContext: { + ...scopeContext, + expiresAt: existingApproval.expiresAt, + revoked: false, + }, + decision: 'allow', + reason: 'Allowed by scoped persisted approval', + reasonCodes: [ + PolicyReasonCode.approvalScopeReused, + ...(pathGuardResult.enforced ? [PolicyReasonCode.pathAllowlistMatch] : []), + ], + matchedRules: ['approval:scope-match'], + }); + } + + if (input.approvalGranted) { + const expiresAt = new Date(Date.now() + this.policy.approvalTtlMs).toISOString(); + this.scopedApprovals.save({ + ...scopeContext, + expiresAt, + }); + return this.buildDecision({ + input, + scopeContext: { + ...scopeContext, + expiresAt, + revoked: false, + }, + decision: 'allow', + reason: 'Approved and persisted for matching scope', + reasonCodes: [ + PolicyReasonCode.approvalScopeGranted, + ...(pathGuardResult.enforced ? [PolicyReasonCode.pathAllowlistMatch] : []), + ], + matchedRules: ['approval:granted-and-persisted'], + }); + } + if (input.mode === 'automation' && input.sideEffect === 'write') { const allowed = this.policy.automationWriteAllowlist.some((allowedCommand) => input.command.startsWith(allowedCommand) ); - return { - allowed, - requiresApproval: !allowed, + return this.buildDecision({ + input, + scopeContext, + decision: allowed ? 'allow' : 'approval_required', reason: allowed ? 'Allowed by automation write allowlist' : 'Write action not in automation allowlist', - sideEffect: input.sideEffect, - }; + reasonCodes: [ + allowed + ? PolicyReasonCode.automationAllowlistMatch + : PolicyReasonCode.automationAllowlistMiss, + ...(allowed && pathGuardResult.enforced ? [PolicyReasonCode.pathAllowlistMatch] : []), + ], + matchedRules: ['automation-write-allowlist'], + }); } if (this.policy.requireApprovalFor.includes(input.sideEffect)) { - return { - allowed: false, - requiresApproval: true, + return this.buildDecision({ + input, + scopeContext, + decision: 'approval_required', reason: `Approval required for side effect: ${input.sideEffect}`, - sideEffect: input.sideEffect, - }; + reasonCodes: [PolicyReasonCode.approvalRequiredSideEffect], + matchedRules: [`require-approval:${input.sideEffect}`], + }); } - return { - allowed: true, - requiresApproval: false, + return this.buildDecision({ + input, + scopeContext, + decision: 'allow', reason: 'Allowed by policy', - sideEffect: input.sideEffect, - }; + reasonCodes: [ + PolicyReasonCode.allowedByPolicy, + ...(pathGuardResult.enforced ? [PolicyReasonCode.pathAllowlistMatch] : []), + ], + matchedRules: ['default-allow'], + }); } evaluateToolInvocation(input: { invocation: ToolInvocation; mode: 'interactive' | 'automation'; + approvalGranted?: boolean; }): ApprovalDecision { return this.evaluateCommand({ command: `${input.invocation.tool} ${JSON.stringify(input.invocation.params)}`, cwd: process.cwd(), mode: input.mode, sideEffect: input.invocation.sideEffect, + approvalGranted: input.approvalGranted, + principal: input.mode === 'automation' ? 'automation-runner' : 'interactive-user', }); } + + private evaluatePathGuard( + input: CommandEvaluationInput, + scopeContext: ApprovalScopeContext + ): { deniedDecision: ApprovalDecision | null; enforced: boolean } { + const allowlistEnabled = this.shouldEnforceAllowlistFor(input.sideEffect, input.mode); + if (!allowlistEnabled) { + return { deniedDecision: null, enforced: false }; + } + + const configuredRoots = + this.policy.pathAllowlistByOperation[input.sideEffect] ?? this.policy.pathAllowlist; + if (configuredRoots.length === 0) { + if (input.mode === 'interactive') { + return { deniedDecision: null, enforced: false }; + } + return { + deniedDecision: this.buildDecision({ + input, + scopeContext, + decision: 'deny', + reason: 'Allowlist policy is enabled but no allowlist is configured for this operation', + reasonCodes: [PolicyReasonCode.missingAllowlist], + matchedRules: ['path-allowlist:missing'], + }), + enforced: true, + }; + } + + let canonicalRoots: string[]; + try { + canonicalRoots = configuredRoots + .map((root) => canonicalizeGuardedPath(input.cwd, root)) + .sort(); + } catch { + return { + deniedDecision: this.buildDecision({ + input, + scopeContext, + decision: 'deny', + reason: 'Allowlist policy could not canonicalize configured roots', + reasonCodes: [PolicyReasonCode.canonicalizationFailure], + matchedRules: ['path-allowlist:root-canonicalization-failed'], + }), + enforced: true, + }; + } + + const targetPaths = extractCommandTargetPaths(input.command); + if (targetPaths.length === 0) { + return { + deniedDecision: this.buildDecision({ + input, + scopeContext, + decision: 'deny', + reason: 'Allowlist policy requires explicit target paths for guarded operation', + reasonCodes: [PolicyReasonCode.missingTargetPaths], + matchedRules: ['path-allowlist:no-target-paths'], + }), + enforced: true, + }; + } + + try { + const canonicalTargets = targetPaths + .map((targetPath) => canonicalizeGuardedPath(input.cwd, targetPath)) + .sort(); + const outOfAllowlistPath = canonicalTargets.find( + (canonicalTarget) => !isWithinAllowedRoots(canonicalTarget, canonicalRoots) + ); + if (outOfAllowlistPath) { + return { + deniedDecision: this.buildDecision({ + input, + scopeContext, + decision: 'deny', + reason: `Path is outside allowed roots: ${outOfAllowlistPath}`, + reasonCodes: [PolicyReasonCode.pathOutOfAllowlist], + matchedRules: ['path-allowlist:out-of-scope'], + }), + enforced: true, + }; + } + } catch { + return { + deniedDecision: this.buildDecision({ + input, + scopeContext, + decision: 'deny', + reason: 'Failed to canonicalize one or more command target paths', + reasonCodes: [PolicyReasonCode.canonicalizationFailure], + matchedRules: ['path-allowlist:target-canonicalization-failed'], + }), + enforced: true, + }; + } + + return { deniedDecision: null, enforced: true }; + } + + private shouldEnforceAllowlistFor( + sideEffect: CommandEvaluationInput['sideEffect'], + mode: CommandEvaluationInput['mode'] + ): boolean { + if (mode === 'automation' && sideEffect === 'write') { + return true; + } + return this.policy.allowlistPolicyOperations.includes(sideEffect); + } + + private buildScopeContext(input: CommandEvaluationInput): ApprovalScopeContext { + const principal = + input.principal ?? (input.mode === 'automation' ? 'automation-runner' : 'interactive-user'); + const resourceScope = `${input.cwd}::${input.command}`; + const scopeId = createHash('sha256') + .update(`${principal}:${input.sideEffect}:${resourceScope}`) + .digest('hex'); + + return { + principal, + operationClass: input.sideEffect, + resourceScope, + scopeId, + }; + } + + private buildDecision(input: { + input: CommandEvaluationInput; + scopeContext: ApprovalScopeContext & { expiresAt?: string | null; revoked?: boolean }; + decision: 'allow' | 'deny' | 'approval_required'; + reason: string; + reasonCodes: PolicyReasonCodeValue[]; + matchedRules: string[]; + }): ApprovalDecision { + return { + allowed: input.decision === 'allow', + requiresApproval: input.decision === 'approval_required', + reason: input.reason, + sideEffect: input.input.sideEffect, + decision: input.decision, + matchedRules: [...input.matchedRules].sort(), + scopeContext: { + principal: input.scopeContext.principal, + operationClass: input.scopeContext.operationClass, + resourceScope: input.scopeContext.resourceScope, + scopeId: input.scopeContext.scopeId, + expiresAt: input.scopeContext.expiresAt ?? null, + revoked: input.scopeContext.revoked ?? false, + }, + reasonCodes: [...input.reasonCodes].sort(), + }; + } } diff --git a/src/policy/path-guard.ts b/src/policy/path-guard.ts new file mode 100644 index 0000000..bd93608 --- /dev/null +++ b/src/policy/path-guard.ts @@ -0,0 +1,97 @@ +import { existsSync, realpathSync } from 'node:fs'; +import { dirname, isAbsolute, normalize, resolve, sep } from 'node:path'; + +function splitCommand(command: string): string[] { + const tokens: string[] = []; + let current = ''; + let quote: "'" | '"' | null = null; + + for (let index = 0; index < command.length; index += 1) { + const char = command[index]; + if (quote) { + if (char === quote) { + quote = null; + } else { + current += char; + } + continue; + } + if (char === '"' || char === "'") { + quote = char; + continue; + } + if (/\s/.test(char)) { + if (current.length > 0) { + tokens.push(current); + current = ''; + } + continue; + } + current += char; + } + + if (current.length > 0) { + tokens.push(current); + } + + return tokens; +} + +function looksLikePathSegment(value: string): boolean { + if (value.startsWith('-')) { + return false; + } + if (value.includes('*') || value.includes('?')) { + return false; + } + return ( + value.startsWith('/') || + value.startsWith('./') || + value.startsWith('../') || + value.includes('/') || + value.includes('\\') + ); +} + +export function extractCommandTargetPaths(command: string): string[] { + const tokens = splitCommand(command); + return tokens.filter(looksLikePathSegment); +} + +export function canonicalizeGuardedPath(cwd: string, targetPath: string): string { + if (targetPath.includes('\0')) { + throw new Error('Invalid target path'); + } + const absoluteTarget = isAbsolute(targetPath) ? targetPath : resolve(cwd, targetPath); + const normalizedAbsoluteTarget = normalize(absoluteTarget); + + if (existsSync(normalizedAbsoluteTarget)) { + return realpathSync(normalizedAbsoluteTarget); + } + + let current = dirname(normalizedAbsoluteTarget); + while (!existsSync(current)) { + const next = dirname(current); + if (next === current) { + throw new Error(`Cannot canonicalize path: ${targetPath}`); + } + current = next; + } + + const canonicalBase = realpathSync(current); + const relativeSuffix = normalizedAbsoluteTarget.slice(current.length).replace(/^[/\\]+/, ''); + return relativeSuffix.length > 0 + ? normalize(resolve(canonicalBase, relativeSuffix)) + : canonicalBase; +} + +function ensureRootBoundary(root: string): string { + return root.endsWith(sep) ? root : `${root}${sep}`; +} + +export function isWithinAllowedRoots(path: string, allowedRoots: string[]): boolean { + return allowedRoots.some((root) => { + const canonicalRoot = ensureRootBoundary(root); + return path === root || path.startsWith(canonicalRoot); + }); +} diff --git a/src/policy/reason-codes.ts b/src/policy/reason-codes.ts new file mode 100644 index 0000000..c55ca5e --- /dev/null +++ b/src/policy/reason-codes.ts @@ -0,0 +1,18 @@ +export const PolicyReasonCode = { + blockedCommandPattern: 'blocked_command_pattern', + automationAllowlistMatch: 'automation_write_allowlist_match', + automationAllowlistMiss: 'automation_write_allowlist_miss', + approvalRequiredSideEffect: 'approval_required_side_effect', + approvalScopeGranted: 'approval_scope_granted', + approvalScopeReused: 'approval_scope_reused', + approvalExpired: 'approval_expired', + approvalRevoked: 'approval_revoked', + missingAllowlist: 'missing_allowlist', + missingTargetPaths: 'missing_target_paths', + pathAllowlistMatch: 'path_allowlist_match', + pathOutOfAllowlist: 'path_out_of_allowlist', + canonicalizationFailure: 'path_canonicalization_failure', + allowedByPolicy: 'allowed_by_policy', +} as const; + +export type PolicyReasonCodeValue = (typeof PolicyReasonCode)[keyof typeof PolicyReasonCode]; diff --git a/src/policy/schemas.ts b/src/policy/schemas.ts index fbb0c29..41bfaf8 100644 --- a/src/policy/schemas.ts +++ b/src/policy/schemas.ts @@ -1,21 +1,86 @@ import { z } from 'zod'; -import { ToolSideEffectSchema } from '../tools/schemas'; +import { type ToolSideEffect, ToolSideEffectSchema } from '../tools/schemas'; +import { PolicyReasonCode } from './reason-codes'; + +const PathAllowlistByOperationSchema: z.ZodType>> = z + .object({ + read: z.array(z.string()).optional(), + write: z.array(z.string()).optional(), + destructive: z.array(z.string()).optional(), + network: z.array(z.string()).optional(), + }) + .default({}); export const ApprovalPolicySchema = z.object({ requireApprovalFor: z.array(ToolSideEffectSchema).default(['write', 'destructive', 'network']), commandAllowlist: z.array(z.string()).default([]), pathAllowlist: z.array(z.string()).default([]), automationWriteAllowlist: z.array(z.string()).default([]), + pathAllowlistByOperation: PathAllowlistByOperationSchema, + allowlistPolicyOperations: z.array(ToolSideEffectSchema).default(['write']), + approvalTtlMs: z + .number() + .int() + .positive() + .default(10 * 60 * 1000), blockedCommandPatterns: z.array(z.string()).default(['rm -rf /', 'mkfs', 'dd if=']), }); export type ApprovalPolicy = z.infer; -export const ApprovalDecisionSchema = z.object({ +export const LegacyApprovalDecisionSchema = z.object({ allowed: z.boolean(), requiresApproval: z.boolean(), reason: z.string(), sideEffect: ToolSideEffectSchema, }); +const PolicyDecisionSchema = z.enum(['allow', 'deny', 'approval_required']); + +const ApprovalScopeContextSchema = z.object({ + principal: z.string(), + operationClass: ToolSideEffectSchema, + resourceScope: z.string(), + scopeId: z.string(), + expiresAt: z.string().nullable().default(null), + revoked: z.boolean().default(false), +}); + +export const ApprovalDecisionSchema = z.object({ + ...LegacyApprovalDecisionSchema.shape, + decision: PolicyDecisionSchema, + matchedRules: z.array(z.string()).default([]), + scopeContext: ApprovalScopeContextSchema, + reasonCodes: z + .array( + z.enum([ + PolicyReasonCode.blockedCommandPattern, + PolicyReasonCode.automationAllowlistMatch, + PolicyReasonCode.automationAllowlistMiss, + PolicyReasonCode.approvalRequiredSideEffect, + PolicyReasonCode.approvalScopeGranted, + PolicyReasonCode.approvalScopeReused, + PolicyReasonCode.approvalExpired, + PolicyReasonCode.approvalRevoked, + PolicyReasonCode.missingAllowlist, + PolicyReasonCode.missingTargetPaths, + PolicyReasonCode.pathAllowlistMatch, + PolicyReasonCode.pathOutOfAllowlist, + PolicyReasonCode.canonicalizationFailure, + PolicyReasonCode.allowedByPolicy, + ]) + ) + .default([]), +}); + export type ApprovalDecision = z.infer; +export type LegacyApprovalDecision = z.infer; + +export function toLegacyApprovalDecision(input: ApprovalDecision): LegacyApprovalDecision { + return { + allowed: input.allowed, + requiresApproval: input.requiresApproval, + reason: input.reason, + sideEffect: input.sideEffect, + }; +} diff --git a/src/policy/scoped-approvals.ts b/src/policy/scoped-approvals.ts new file mode 100644 index 0000000..5fabcf0 --- /dev/null +++ b/src/policy/scoped-approvals.ts @@ -0,0 +1,62 @@ +import { randomUUID } from 'node:crypto'; + +import type { ToolSideEffect } from '../tools/schemas'; + +export type ApprovalScopeContext = { + principal: string; + operationClass: ToolSideEffect; + resourceScope: string; + scopeId: string; +}; + +export type ScopedApprovalRecord = ApprovalScopeContext & { + id: string; + expiresAt: string; + createdAt: string; + revokedAt: string | null; +}; + +export class InMemoryScopedApprovalStore { + private readonly records = new Map(); + + save(input: ApprovalScopeContext & { expiresAt: string }): ScopedApprovalRecord { + const now = new Date().toISOString(); + const record: ScopedApprovalRecord = { + id: randomUUID(), + principal: input.principal, + operationClass: input.operationClass, + resourceScope: input.resourceScope, + scopeId: input.scopeId, + expiresAt: input.expiresAt, + createdAt: now, + revokedAt: null, + }; + this.records.set(input.scopeId, record); + return record; + } + + revoke(scopeId: string): boolean { + const record = this.records.get(scopeId); + if (!record || record.revokedAt) { + return false; + } + record.revokedAt = new Date().toISOString(); + this.records.set(scopeId, record); + return true; + } + + matchExact(input: ApprovalScopeContext): ScopedApprovalRecord | null { + const record = this.records.get(input.scopeId); + if (!record) { + return null; + } + if ( + record.principal !== input.principal || + record.operationClass !== input.operationClass || + record.resourceScope !== input.resourceScope + ) { + return null; + } + return record; + } +} diff --git a/src/tools/registry.ts b/src/tools/registry.ts index dc82af5..1615d70 100644 --- a/src/tools/registry.ts +++ b/src/tools/registry.ts @@ -1,7 +1,7 @@ import { z } from 'zod'; import type { AgentsConfig } from '../config/agents-loader'; import type { DefaultPolicyEngine } from '../policy/engine'; -import type { ApprovalDecision } from '../policy/schemas'; +import { type ApprovalDecision, toLegacyApprovalDecision } from '../policy/schemas'; import { classifyCommandSideEffect, executeCommand } from './exec-command'; import { ToolInvocationSchema, type ToolResult } from './schemas'; @@ -29,18 +29,29 @@ export type ToolRegistryOptions = { onWarning?: (message: string) => void; }; -const SIDE_EFFECT_RANK: Record = { - read: 0, - write: 1, - network: 2, - destructive: 3, -}; - -function resolveConservativeSideEffect( - declared: ToolHandlerContext['sideEffect'], - inferred: ToolHandlerContext['sideEffect'] -): ToolHandlerContext['sideEffect'] { - return SIDE_EFFECT_RANK[declared] >= SIDE_EFFECT_RANK[inferred] ? declared : inferred; +function buildDefaultAllowDecision(input: { + sideEffect: ToolHandlerContext['sideEffect']; + mode: ToolHandlerContext['mode']; + cwd: string; + command: string; +}): ApprovalDecision { + return { + allowed: true, + requiresApproval: false, + reason: 'Policy engine unavailable', + sideEffect: input.sideEffect, + decision: 'allow', + matchedRules: [], + scopeContext: { + principal: input.mode === 'automation' ? 'automation-runner' : 'interactive-user', + operationClass: input.sideEffect, + resourceScope: `${input.cwd}::${input.command}`, + scopeId: 'policy-engine-unavailable', + expiresAt: null, + revoked: false, + }, + reasonCodes: [], + }; } function deniedResult(input: { @@ -58,6 +69,7 @@ function deniedResult(input: { actionName: input.actionName ?? input.tool, resolvedCommand: input.resolvedCommand, policyOutcome: input.decision, + policyOutcomeLegacy: toLegacyApprovalDecision(input.decision), executionSummary: input.summary, }, stdout: '', @@ -79,17 +91,13 @@ export class ToolRegistry { const command = z.string().parse(params.command); const cwd = z.string().default(process.cwd()).parse(params.cwd); const timeoutMs = z.number().int().positive().optional().parse(params.timeoutMs); - const inferredSideEffect = classifyCommandSideEffect(command); - const effectiveSideEffect = resolveConservativeSideEffect( - context.sideEffect, - inferredSideEffect - ); const decision = this.evaluatePolicy({ command, cwd, mode: context.mode, - sideEffect: effectiveSideEffect, + sideEffect: context.sideEffect, + approvalGranted: context.approvalGranted, }); if (decision && !decision.allowed && !decision.requiresApproval) { return deniedResult({ @@ -114,18 +122,20 @@ export class ToolRegistry { timeoutMs, toolName: context.invocationTool, }); + const effectiveDecision = + decision ?? + buildDefaultAllowDecision({ + sideEffect: context.sideEffect, + mode: context.mode, + cwd, + command, + }); return { ...result, payload: { ...result.payload, - policyOutcome: - decision ?? - ({ - allowed: true, - requiresApproval: false, - reason: 'Policy engine unavailable', - sideEffect: effectiveSideEffect, - } satisfies ApprovalDecision), + policyOutcome: effectiveDecision, + policyOutcomeLegacy: toLegacyApprovalDecision(effectiveDecision), executionSummary: result.summary, }, }; @@ -203,6 +213,7 @@ export class ToolRegistry { cwd, mode: context.mode, sideEffect: inferredSideEffect, + approvalGranted: context.approvalGranted, }); if (decision && !decision.allowed && !decision.requiresApproval) { @@ -231,6 +242,14 @@ export class ToolRegistry { timeoutMs, toolName: context.invocationTool, }); + const effectiveDecision = + decision ?? + buildDefaultAllowDecision({ + sideEffect: inferredSideEffect, + mode: context.mode, + cwd, + command: resolvedCommand, + }); return { ...result, @@ -238,14 +257,8 @@ export class ToolRegistry { ...result.payload, actionName: name, resolvedCommand, - policyOutcome: - decision ?? - ({ - allowed: true, - requiresApproval: false, - reason: 'Policy engine unavailable', - sideEffect: inferredSideEffect, - } satisfies ApprovalDecision), + policyOutcome: effectiveDecision, + policyOutcomeLegacy: toLegacyApprovalDecision(effectiveDecision), executionSummary: result.summary, }, }; @@ -279,6 +292,7 @@ export class ToolRegistry { cwd: string; mode: 'interactive' | 'automation'; sideEffect: 'read' | 'write' | 'destructive' | 'network'; + approvalGranted?: boolean; }): ApprovalDecision | null { if (!this.policyEngine) { return null; @@ -288,6 +302,8 @@ export class ToolRegistry { cwd: input.cwd, mode: input.mode, sideEffect: input.sideEffect, + approvalGranted: input.approvalGranted, + principal: input.mode === 'automation' ? 'automation-runner' : 'interactive-user', }); } } diff --git a/src/tools/schemas.ts b/src/tools/schemas.ts index a9dc46a..77547c6 100644 --- a/src/tools/schemas.ts +++ b/src/tools/schemas.ts @@ -1,6 +1,7 @@ import { z } from 'zod'; export const ToolSideEffectSchema = z.enum(['read', 'write', 'destructive', 'network']); +export type ToolSideEffect = z.infer; export const ToolInvocationSchema = z.object({ tool: z.string().min(1), diff --git a/tests/integration/policy-approval.integration.test.ts b/tests/integration/policy-approval.integration.test.ts index 0eca97d..b72fac7 100644 --- a/tests/integration/policy-approval.integration.test.ts +++ b/tests/integration/policy-approval.integration.test.ts @@ -1,4 +1,4 @@ -import { access } from 'node:fs/promises'; +import { access, mkdir } from 'node:fs/promises'; import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; import { ToolRegistry } from '../../src/tools/registry'; @@ -7,67 +7,127 @@ import { createPolicyFixture, createWorkspaceFixture } from './helpers'; describe('integration: policy and approval branches', () => { it('approval-granted branch continues and executes expected action', async () => { const workspace = await createWorkspaceFixture(); - try { - const target = join(workspace.path, 'approved.txt'); - - const registry = new ToolRegistry({ - policyEngine: createPolicyFixture(), - defaultMode: 'interactive', - agentsConfig: { - commands: [{ name: 'approve-me', command: `sh -lc "echo approved > ${target}"` }], - hooks: [], - warnings: [], - }, - }); + const target = join(workspace.path, 'approved.txt'); - const result = await registry.invoke( - { - tool: 'agents:approve-me', - sideEffect: 'read', - params: {}, + const registry = new ToolRegistry({ + policyEngine: createPolicyFixture({ + automationWriteAllowlist: [`echo approved > ${target}`], + pathAllowlistByOperation: { + write: [workspace.path], }, - { - approvalGranted: true, - } - ); - - expect(result.ok).toBe(true); - expect(result.payload.policyOutcome).toMatchObject({ - requiresApproval: true, - sideEffect: 'write', - }); - await expect(access(target)).resolves.toBeUndefined(); - } finally { - await workspace.cleanup(); - } + }), + defaultMode: 'automation', + agentsConfig: { + commands: [{ name: 'approve-me', command: `echo approved > ${target}` }], + hooks: [], + warnings: [], + }, + }); + + const result = await registry.invoke({ + tool: 'agents:approve-me', + sideEffect: 'read', + params: {}, + }); + + expect(result.ok).toBe(true); + await expect(access(target)).resolves.toBeUndefined(); + + await workspace.cleanup(); }); it('approval-denied branch stops at boundary and does not create side effect', async () => { const workspace = await createWorkspaceFixture(); - try { - const target = join(workspace.path, 'denied.txt'); - - const registry = new ToolRegistry({ - policyEngine: createPolicyFixture(), - defaultMode: 'interactive', - agentsConfig: { - commands: [{ name: 'deny-me', command: `sh -lc "echo denied > ${target}"` }], - hooks: [], - warnings: [], + const target = join(workspace.path, 'denied.txt'); + const registry = new ToolRegistry({ + policyEngine: createPolicyFixture(), + defaultMode: 'interactive', + agentsConfig: { + commands: [{ name: 'deny-me', command: `sh -lc "echo denied > ${target}"` }], + hooks: [], + warnings: [], + }, + }); + + const result = await registry.invoke({ + tool: 'agents:deny-me', + sideEffect: 'read', + params: {}, + }); + + expect(result.ok).toBe(false); + expect(result.summary).toContain('Approval required'); + await expect(access(target)).rejects.toThrow(); + + await workspace.cleanup(); + }); + + it('reuses scoped approvals for identical scope and enforces allowlist scope boundaries', async () => { + const workspace = await createWorkspaceFixture(); + const allowedRoot = join(workspace.path, 'safe'); + const deniedRoot = join(workspace.path, 'unsafe'); + await mkdir(allowedRoot, { recursive: true }); + await mkdir(deniedRoot, { recursive: true }); + + const policyEngine = createPolicyFixture({ + pathAllowlistByOperation: { + write: [allowedRoot], + }, + allowlistPolicyOperations: ['write'], + }); + + const registry = new ToolRegistry({ + policyEngine, + defaultMode: 'interactive', + }); + + const scopedWrite = { + tool: 'exec-command', + sideEffect: 'write' as const, + params: { + command: `echo approved > ${join(allowedRoot, 'scoped.txt')}`, + cwd: workspace.path, + }, + }; + + const first = await registry.invoke(scopedWrite, { mode: 'interactive' }); + expect(first.ok).toBe(false); + expect(first.payload.policyOutcome).toMatchObject({ + requiresApproval: true, + }); + + const approved = await registry.invoke(scopedWrite, { + mode: 'interactive', + approvalGranted: true, + }); + expect(approved.ok).toBe(true); + + const reused = await registry.invoke(scopedWrite, { mode: 'interactive' }); + expect(reused.ok).toBe(true); + expect(reused.payload.policyOutcome).toMatchObject({ + allowed: true, + reasonCodes: expect.arrayContaining(['approval_scope_reused']), + }); + + const outOfScope = await registry.invoke( + { + tool: 'exec-command', + sideEffect: 'write', + params: { + command: `echo denied > ${join(deniedRoot, 'denied.txt')}`, + cwd: workspace.path, }, - }); - - const result = await registry.invoke({ - tool: 'agents:deny-me', - sideEffect: 'read', - params: {}, - }); - - expect(result.ok).toBe(false); - expect(result.summary).toContain('Approval required'); - await expect(access(target)).rejects.toThrow(); - } finally { - await workspace.cleanup(); - } + }, + { mode: 'interactive' } + ); + + expect(outOfScope.ok).toBe(false); + expect(outOfScope.payload.policyOutcome).toMatchObject({ + allowed: false, + requiresApproval: false, + reasonCodes: expect.arrayContaining(['path_out_of_allowlist']), + }); + + await workspace.cleanup(); }); }); diff --git a/tests/policy-engine.test.ts b/tests/policy-engine.test.ts index adfc148..b6e21df 100644 --- a/tests/policy-engine.test.ts +++ b/tests/policy-engine.test.ts @@ -1,6 +1,10 @@ +import { mkdir, mkdtemp, symlink } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; import { createDefaultApprovalPolicy } from '../src/policy/defaults'; import { DefaultPolicyEngine } from '../src/policy/engine'; +import { PolicyReasonCode } from '../src/policy/reason-codes'; describe('DefaultPolicyEngine', () => { it('requires approval for mutating commands by default', () => { @@ -19,12 +23,15 @@ describe('DefaultPolicyEngine', () => { it('allows safe automation writes when allowlist matches', () => { const engine = new DefaultPolicyEngine( createDefaultApprovalPolicy({ - automationWriteAllowlist: ['npm test'], + automationWriteAllowlist: ['echo ok > ./tmp.txt'], + pathAllowlistByOperation: { + write: ['/repo'], + }, }) ); const decision = engine.evaluateCommand({ - command: 'npm test', + command: 'echo ok > ./tmp.txt', cwd: '/repo', mode: 'automation', sideEffect: 'write', @@ -33,4 +40,246 @@ describe('DefaultPolicyEngine', () => { expect(decision.allowed).toBe(true); expect(decision.requiresApproval).toBe(false); }); + + it('reuses scoped approvals only for identical scope', () => { + const engine = new DefaultPolicyEngine(createDefaultApprovalPolicy()); + const cwd = '/repo'; + const command = 'echo hi > ./tmp.txt'; + + const first = engine.evaluateCommand({ + command, + cwd, + mode: 'interactive', + sideEffect: 'write', + }); + expect(first.requiresApproval).toBe(true); + + const approved = engine.evaluateCommand({ + command, + cwd, + mode: 'interactive', + sideEffect: 'write', + approvalGranted: true, + }); + expect(approved.allowed).toBe(true); + expect(approved.reasonCodes).toContain(PolicyReasonCode.approvalScopeGranted); + + const reused = engine.evaluateCommand({ + command, + cwd, + mode: 'interactive', + sideEffect: 'write', + }); + expect(reused.allowed).toBe(true); + expect(reused.reasonCodes).toContain(PolicyReasonCode.approvalScopeReused); + + const mismatch = engine.evaluateCommand({ + command: 'echo hi > ./other.txt', + cwd, + mode: 'interactive', + sideEffect: 'write', + }); + expect(mismatch.allowed).toBe(false); + expect(mismatch.requiresApproval).toBe(true); + }); + + it('treats expired scoped approval as invalid', async () => { + const engine = new DefaultPolicyEngine( + createDefaultApprovalPolicy({ + approvalTtlMs: 1, + }) + ); + const command = 'echo hi > ./tmp.txt'; + const cwd = '/repo'; + + engine.evaluateCommand({ + command, + cwd, + mode: 'interactive', + sideEffect: 'write', + approvalGranted: true, + }); + await new Promise((resolve) => setTimeout(resolve, 5)); + + const decision = engine.evaluateCommand({ + command, + cwd, + mode: 'interactive', + sideEffect: 'write', + }); + + expect(decision.requiresApproval).toBe(true); + expect(decision.reasonCodes).toContain(PolicyReasonCode.approvalExpired); + }); + + it('excludes revoked scoped approvals from reuse', () => { + const engine = new DefaultPolicyEngine(createDefaultApprovalPolicy()); + const command = 'echo hi > ./tmp.txt'; + const cwd = '/repo'; + + const approved = engine.evaluateCommand({ + command, + cwd, + mode: 'interactive', + sideEffect: 'write', + approvalGranted: true, + }); + const revoked = engine.revokeApproval(approved.scopeContext.scopeId); + expect(revoked).toBe(true); + + const decision = engine.evaluateCommand({ + command, + cwd, + mode: 'interactive', + sideEffect: 'write', + }); + expect(decision.requiresApproval).toBe(true); + expect(decision.reasonCodes).toContain(PolicyReasonCode.approvalRevoked); + }); + + it('returns deterministic explanation payload for identical inputs', () => { + const engine = new DefaultPolicyEngine(createDefaultApprovalPolicy()); + + const first = engine.evaluateCommand({ + command: 'ls', + cwd: '/repo', + mode: 'interactive', + sideEffect: 'read', + }); + const second = engine.evaluateCommand({ + command: 'ls', + cwd: '/repo', + mode: 'interactive', + sideEffect: 'read', + }); + + expect(first).toEqual(second); + }); + + it('enforces canonical path allowlists for automation writes', async () => { + const workspace = await mkdtemp(join(tmpdir(), 'policy-path-')); + const allowed = join(workspace, 'allowed'); + const denied = join(workspace, 'denied'); + await mkdir(allowed, { recursive: true }); + await mkdir(denied, { recursive: true }); + const allowedLink = join(workspace, 'allowed-link'); + const symlinkCreated = await symlink(allowed, allowedLink) + .then(() => true) + .catch(() => false); + + const engine = new DefaultPolicyEngine( + createDefaultApprovalPolicy({ + pathAllowlistByOperation: { + write: [allowed], + }, + automationWriteAllowlist: ['echo hi >'], + }) + ); + + const inAllowlist = engine.evaluateCommand({ + command: `echo hi > ${join(allowed, 'file.txt')}`, + cwd: workspace, + mode: 'automation', + sideEffect: 'write', + }); + expect(inAllowlist.allowed).toBe(true); + expect(inAllowlist.reasonCodes).toContain(PolicyReasonCode.pathAllowlistMatch); + + if (symlinkCreated) { + const viaSymlink = engine.evaluateCommand({ + command: `echo hi > ${join(allowedLink, 'file.txt')}`, + cwd: workspace, + mode: 'automation', + sideEffect: 'write', + }); + expect(viaSymlink.allowed).toBe(true); + expect(viaSymlink.reasonCodes).toContain(PolicyReasonCode.pathAllowlistMatch); + } + + const outOfAllowlist = engine.evaluateCommand({ + command: `echo hi > ${join(denied, 'file.txt')}`, + cwd: workspace, + mode: 'automation', + sideEffect: 'write', + }); + expect(outOfAllowlist.allowed).toBe(false); + expect(outOfAllowlist.requiresApproval).toBe(false); + expect(outOfAllowlist.reasonCodes).toContain(PolicyReasonCode.pathOutOfAllowlist); + }); + + it('denies missing allowlist in allowlist-enabled automation mode', () => { + const engine = new DefaultPolicyEngine( + createDefaultApprovalPolicy({ + pathAllowlistByOperation: {}, + }) + ); + + const decision = engine.evaluateCommand({ + command: 'echo hi > ./tmp.txt', + cwd: '/repo', + mode: 'automation', + sideEffect: 'write', + }); + expect(decision.allowed).toBe(false); + expect(decision.requiresApproval).toBe(false); + expect(decision.reasonCodes).toContain(PolicyReasonCode.missingAllowlist); + }); + + it('falls back to approval-gated flow in interactive mode without allowlist', () => { + const engine = new DefaultPolicyEngine( + createDefaultApprovalPolicy({ + pathAllowlistByOperation: {}, + }) + ); + + const decision = engine.evaluateCommand({ + command: 'echo hi > ./tmp.txt', + cwd: '/repo', + mode: 'interactive', + sideEffect: 'write', + }); + expect(decision.allowed).toBe(false); + expect(decision.requiresApproval).toBe(true); + expect(decision.reasonCodes).toContain(PolicyReasonCode.approvalRequiredSideEffect); + }); + + it('denies when canonicalization fails for command target path', () => { + const engine = new DefaultPolicyEngine( + createDefaultApprovalPolicy({ + pathAllowlistByOperation: { + write: ['/repo'], + }, + }) + ); + + const decision = engine.evaluateCommand({ + command: `echo hi > "./bad\0path.txt"`, + cwd: '/repo', + mode: 'automation', + sideEffect: 'write', + }); + expect(decision.allowed).toBe(false); + expect(decision.reasonCodes).toContain(PolicyReasonCode.canonicalizationFailure); + }); + + it('returns explicit reason when guarded command has no target paths', () => { + const engine = new DefaultPolicyEngine( + createDefaultApprovalPolicy({ + pathAllowlistByOperation: { + write: ['/repo'], + }, + }) + ); + + const decision = engine.evaluateCommand({ + command: 'echo hi', + cwd: '/repo', + mode: 'automation', + sideEffect: 'write', + }); + + expect(decision.allowed).toBe(false); + expect(decision.requiresApproval).toBe(false); + expect(decision.reasonCodes).toContain(PolicyReasonCode.missingTargetPaths); + }); }); diff --git a/tests/tool-registry.test.ts b/tests/tool-registry.test.ts index c94ce80..a0e488f 100644 --- a/tests/tool-registry.test.ts +++ b/tests/tool-registry.test.ts @@ -1,7 +1,7 @@ import { mkdtemp, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { describe, expect, it, vi } from 'vitest'; +import { describe, expect, it } from 'vitest'; import { createDefaultApprovalPolicy } from '../src/policy/defaults'; import { DefaultPolicyEngine } from '../src/policy/engine'; import { ToolRegistry } from '../src/tools/registry'; @@ -70,15 +70,22 @@ describe('ToolRegistry AGENTS runtime actions', () => { requiresApproval: true, sideEffect: 'write', }); + expect(result.payload.policyOutcomeLegacy).toMatchObject({ + requiresApproval: true, + sideEffect: 'write', + }); }); it('allows automation write execution when command matches allowlist', async () => { - const cwd = await mkdtemp(join(tmpdir(), 'dubsbot-tool-registry-')); + const cwd = await mkdtemp(join(tmpdir(), 'registry-automation-')); try { const registry = new ToolRegistry({ policyEngine: new DefaultPolicyEngine( createDefaultApprovalPolicy({ automationWriteAllowlist: ['echo hi > ./tmp.txt'], + pathAllowlistByOperation: { + write: [cwd], + }, }) ), defaultMode: 'automation', @@ -92,7 +99,9 @@ describe('ToolRegistry AGENTS runtime actions', () => { const result = await registry.invoke({ tool: 'agents:fix', sideEffect: 'read', - params: { cwd }, + params: { + cwd, + }, }); expect(result.ok).toBe(true); @@ -106,59 +115,6 @@ describe('ToolRegistry AGENTS runtime actions', () => { } }); - it('passes invocation cwd into policy evaluation', async () => { - const cwd = '/tmp/custom-workdir'; - const evaluateCommand = vi.fn().mockReturnValue({ - allowed: false, - requiresApproval: true, - reason: 'Approval required for side effect: write', - sideEffect: 'write', - }); - const registry = new ToolRegistry({ - policyEngine: { evaluateCommand } as unknown as DefaultPolicyEngine, - defaultMode: 'automation', - agentsConfig: { - commands: [{ name: 'fix', command: 'echo hi > ./tmp.txt' }], - hooks: [], - warnings: [], - }, - }); - - await registry.invoke({ - tool: 'agents:fix', - sideEffect: 'read', - params: { cwd }, - }); - - expect(evaluateCommand).toHaveBeenCalledWith( - expect.objectContaining({ - cwd, - }) - ); - }); - - it('uses conservative side effect for exec-command policy checks', async () => { - const registry = new ToolRegistry({ - policyEngine: new DefaultPolicyEngine(createDefaultApprovalPolicy()), - defaultMode: 'interactive', - }); - - const result = await registry.invoke({ - tool: 'exec-command', - sideEffect: 'read', - params: { - command: 'echo hi > ./tmp.txt', - }, - }); - - expect(result.ok).toBe(false); - expect(result.payload.policyOutcome).toMatchObject({ - allowed: false, - requiresApproval: true, - sideEffect: 'write', - }); - }); - it('returns policy denial and structured payload for blocked commands', async () => { const registry = new ToolRegistry({ policyEngine: new DefaultPolicyEngine( @@ -225,4 +181,27 @@ describe('ToolRegistry AGENTS runtime actions', () => { sideEffect: 'read', }); }); + + it('emits legacy policy outcome when policy engine is unavailable', async () => { + const registry = new ToolRegistry({ + agentsConfig: { + commands: [{ name: 'hello', command: 'printf "hello"' }], + hooks: [], + warnings: [], + }, + }); + + const result = await registry.invoke({ + tool: 'agents:hello', + sideEffect: 'read', + params: {}, + }); + + expect(result.ok).toBe(true); + expect(result.payload.policyOutcomeLegacy).toMatchObject({ + allowed: true, + requiresApproval: false, + sideEffect: 'read', + }); + }); });