From 412ed1ddf76be4396b31ed4ae1415b56f68798f6 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Sat, 18 Jul 2026 23:24:13 -0700 Subject: [PATCH 1/4] PINEAPPLE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Stage **S3** of the de-fork spike — **the kill gate**. Enforces **non-bypassable hard denies** (`sql_execute` DDL: `DROP DATABASE`/`DROP SCHEMA`/`TRUNCATE`; bash DDL) at **every model-invoked tool-execution dispatcher**, relocating today's scattered per-tool safety checks into one audited chokepoint. **Behavior-preserving** — these operations already block; S3 consolidates and closes a bypass. - **NEW `packages/opencode/src/altimate/policy/hard-policy.ts`** — pure, synchronous, **total** `HardPolicy.check()`: malformed/missing args return a deny (`policy_internal_error`), never throw, never implicit-allow. Emits a structured **audit record** on every call — this (not the trace) is the enforcement oracle the tests assert against. - **`HardPolicy.check()` inserted before `tool.execute` at every dispatcher** from the S2 route matrix: D1 (registry loop) + D2 (MCP loop) + D6 (direct Task dispatch) in `session/prompt.ts`, D3/D4 in `session/tools.ts`, D5 (BatchTool inner dispatch) in `tool/batch.ts`. Fail-closed init via `assertInitialized()` at the app-runtime composition seam. - **Closes a real bypass:** the fork's resolvers were *discarding* `tool.execute.before`'s return value, so a plugin hook that mutated args (e.g. rewrote a benign `SELECT` into `DROP DATABASE`) executed the mutation **unchecked**. Every site now captures the post-hook **final args** and checks *those*. The batch inner dispatch checks the **inner** tool ID + args (not `toolID="batch"`), closing the nested-dispatch gap. - **On deny:** returns a `hard_policy_denied` tool-error result (or throws in batch) — `execute` and `tool.execute.after` are both skipped. **Scope preserved:** bash DDL patterns are byte-identical to today's `agent.ts` safety table; the sql check reuses `sql-execute.ts`'s classifier + error string. `rm -rf` / `git push --force` / `git reset --hard` correctly stay **ask-tier** (not promoted to hard deny). Also includes `route-sentinels.test.ts` made **line-number- and arg-agnostic** (matches `args` or S3's `finalArgs`) so it passes with or without S3 applied — shared identically with PR #1014, mergeable in any order. ### Review Two independent adversarial reviews (a Claude reviewer + the orchestrator's own trace of every `.execute()` site) found **no bypass** via malformed args, mutated args, or allow-all permission config, and confirmed all 7 safety properties (coverage, final-args, total-function, deny-terminal, config-independence, fail-closed init, behavior-preservation). ## Test Plan - `bun test test/altimate/defork/hardpolicy.test.ts`: **19 pass / 0 fail** — the bypass matrix across D1–D6: DDL denied + not executed at every route, near-miss controls allowed (proves rules aren't over-broad), allow-all config still denies, malformed args fail closed, deny is terminal, mutated-args checked on the post-hook value (audit `finalArgsDigest` asserted). - Always-allow red-proof: stubbing `check()` to always-allow fails every dispatcher-integration test. - `bunx tsc --noEmit -p packages/opencode`: clean for the changed files. ## Checklist - [x] Tests added/updated - [x] Documentation updated (spec §S3 + route matrix) - [ ] CHANGELOG updated — N/A (internal safety guardrail) Closes #1017 --- _This is a security boundary and the spike's decision checkpoint — **held for human go/no-go; do not auto-merge.**_ --- .../src/altimate/policy/hard-policy.ts | 337 ++++++ packages/opencode/src/effect/app-runtime.ts | 8 + packages/opencode/src/session/prompt.ts | 212 +++- packages/opencode/src/session/tools.ts | 76 +- packages/opencode/src/tool/batch.ts | 18 + .../test/altimate/defork/hardpolicy.test.ts | 1064 +++++++++++++++++ .../altimate/defork/route-sentinels.test.ts | 346 ++++++ 7 files changed, 1993 insertions(+), 68 deletions(-) create mode 100644 packages/opencode/src/altimate/policy/hard-policy.ts create mode 100644 packages/opencode/test/altimate/defork/hardpolicy.test.ts create mode 100644 packages/opencode/test/altimate/defork/route-sentinels.test.ts diff --git a/packages/opencode/src/altimate/policy/hard-policy.ts b/packages/opencode/src/altimate/policy/hard-policy.ts new file mode 100644 index 000000000..a4b07abdb --- /dev/null +++ b/packages/opencode/src/altimate/policy/hard-policy.ts @@ -0,0 +1,337 @@ +// altimate_change - HardPolicy: single fork-owned chokepoint for non-bypassable hard denies (S3, de-fork spike) +// +// This consolidates two hard blocks that already existed in scattered form: +// 1. sql_execute DDL block (previously inline in altimate/tools/sql-execute.ts) +// 2. bash DDL block (previously inline in agent.ts's safetyDenials permission table) +// into ONE pure, synchronous, TOTAL function so every tool-execution dispatcher can call +// the same check instead of re-implementing (or forgetting) it. +// +// Design contract (see docs/internal/2026-07-18-defork-spike-spec.md §S3): +// - TOTAL: never throws, never implicitly allows on malformed input. Any internal failure +// (bad args shape, broken rule table, classifier exception) resolves to a deny with +// ruleID "policy_internal_error" — fail closed, not fail open. +// - PURE + SYNCHRONOUS: no I/O, no async, safe to call from both Promise-based and +// Effect-based dispatchers without adapters. +// - v1 rules are BEHAVIOR-PRESERVING relocations of existing blocks, not new blocks. Do +// NOT add new hard denies here (e.g. rm -rf, git push --force) without a product +// decision — those stay ask-tier in the Ruleset, user-overridable. +// - Every check() call emits a structured audit record. The audit log — not trace +// evidence — is the security oracle: TraceSpan.status is only ok|error (no "denied" +// state), and denials throw DeniedError without publishing permission events, so an +// absent execute span does NOT prove enforcement. Tests must assert against the audit +// log and execute-not-called counters. + +import { Wildcard } from "@/util/wildcard" +import { classifyAndCheck } from "@/altimate/tools/sql-classify" + +export namespace HardPolicy { + // --------------------------------------------------------------------------------- + // Public types + // --------------------------------------------------------------------------------- + + export type Source = "native" | "plugin" | "mcp" | "batch" | "task" + + export interface Input { + toolID: string + source: Source + args: unknown + sessionID: string + agentID?: string + callID?: string + } + + export type Decision = { allow: true } | { allow: false; ruleID: string; safeReason: string } + + export interface AuditRecord { + ruleID: string + dispatcher: Source + toolID: string + source: Source + finalArgsDigest: string + sessionID: string + callID?: string + decision: Decision + timestamp: number + } + + // --------------------------------------------------------------------------------- + // Rule table (v1) — versioned, each rule is a relocation of a pre-existing hard block. + // --------------------------------------------------------------------------------- + + interface Rule { + ruleID: string + toolID: string + // Returns true if the args match this rule's deny condition. MUST throw (not return + // false) if `args` is not shaped as this rule expects — that throw is caught by + // check() and converted into a policy_internal_error deny. This keeps malformed-args + // handling fail-closed for GOVERNED tool IDs (those with an applicable rule) without + // extending denial behavior to tool IDs HardPolicy was never meant to touch. + match: (args: unknown) => boolean + safeReason: string + positiveExamples: unknown[] + negativeExamples: unknown[] + } + + function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null + } + + // Mirrors altimate/tools/sql-execute.ts:24-28 — same classifier, same denied types + // (DROP DATABASE, DROP SCHEMA, TRUNCATE), same "cannot be overridden" behavior. + function matchSqlDdl(args: unknown): boolean { + if (!isRecord(args) || typeof args.query !== "string") { + throw new Error("hard-policy: sql_execute args missing string 'query' field") + } + return classifyAndCheck(args.query).blocked + } + + // Mirrors agent.ts's safetyDenials bash table (9 patterns; UPPER/lowercase/Title-Case + // only — NOT fully case-insensitive, because Wildcard.match is case-sensitive on + // non-Windows). Whole-command-string match, not per-AST-subcommand — a documented v1 + // simplification vs. bash.ts's tree-sitter subcommand splitting used for the ask-tier + // permission flow. + const BASH_DDL_PATTERNS = [ + "DROP DATABASE *", + "DROP SCHEMA *", + "TRUNCATE *", + "drop database *", + "drop schema *", + "truncate *", + "Drop Database *", + "Drop Schema *", + "Truncate *", + ] + + function matchBashDdl(args: unknown): boolean { + if (!isRecord(args) || typeof args.command !== "string") { + throw new Error("hard-policy: bash args missing string 'command' field") + } + const command = args.command + return BASH_DDL_PATTERNS.some((pattern) => Wildcard.match(command, pattern)) + } + + const SQL_DDL_SAFE_REASON = + "DROP DATABASE, DROP SCHEMA, and TRUNCATE are blocked for safety. This cannot be overridden." + const BASH_DDL_SAFE_REASON = + "DROP DATABASE, DROP SCHEMA, and TRUNCATE via bash are blocked for safety. This cannot be overridden." + + export const RULES: readonly Rule[] = [ + { + ruleID: "sql_execute_ddl_v1", + toolID: "sql_execute", + match: matchSqlDdl, + safeReason: SQL_DDL_SAFE_REASON, + positiveExamples: [{ query: "DROP DATABASE prod" }, { query: "TRUNCATE TABLE users" }], + negativeExamples: [{ query: "SELECT 1" }, { query: "DROP TABLE staging_tmp" }], + }, + { + ruleID: "bash_ddl_v1", + toolID: "bash", + match: matchBashDdl, + safeReason: BASH_DDL_SAFE_REASON, + positiveExamples: [{ command: "DROP DATABASE prod" }, { command: "truncate users" }], + negativeExamples: [{ command: "ls -la" }, { command: "DROP TABLE staging_tmp" }], + }, + ] + + // Index rules by toolID for O(1) "is this toolID governed at all" lookups. A toolID + // with no entry here is UNGOVERNED — check() always allows it regardless of args shape, + // preserving "v1 is behavior-preserving, not new blocks." + function buildRulesByTool(rules: readonly Rule[]): Map { + const byTool = new Map() + for (const rule of rules) { + const list = byTool.get(rule.toolID) ?? [] + list.push(rule) + byTool.set(rule.toolID, list) + } + return byTool + } + + // --------------------------------------------------------------------------------- + // Fail-closed initialization — validated once at module load. A broken rule table must + // make app composition fail loudly (via assertInitialized() thrown at the app-runtime + // composition seam), not silently degrade to allow-everything. + // --------------------------------------------------------------------------------- + + function validateRuleTable(rules: readonly Rule[]): string | null { + if (!Array.isArray(rules) || rules.length === 0) return "RULES table is empty or not an array" + const seenIDs = new Set() + for (const rule of rules) { + if (!rule || typeof rule !== "object") return "RULES contains a non-object entry" + if (typeof rule.ruleID !== "string" || rule.ruleID.length === 0) return "rule missing ruleID" + if (seenIDs.has(rule.ruleID)) return `duplicate ruleID: ${rule.ruleID}` + seenIDs.add(rule.ruleID) + if (typeof rule.toolID !== "string" || rule.toolID.length === 0) { + return `rule ${rule.ruleID} missing toolID` + } + if (typeof rule.match !== "function") return `rule ${rule.ruleID} missing match()` + if (typeof rule.safeReason !== "string" || rule.safeReason.length === 0) { + return `rule ${rule.ruleID} missing safeReason` + } + // Positive examples must actually match; negative examples must not. A rule table + // that fails its own examples is broken and must not silently pass through. + for (const example of rule.positiveExamples) { + try { + if (!rule.match(example)) return `rule ${rule.ruleID} failed its own positive example` + } catch { + return `rule ${rule.ruleID} threw on its own positive example` + } + } + for (const example of rule.negativeExamples) { + try { + if (rule.match(example)) return `rule ${rule.ruleID} matched its own negative example` + } catch { + return `rule ${rule.ruleID} threw on its own negative example` + } + } + } + return null + } + + const rulesByTool = buildRulesByTool(RULES) + const initError = validateRuleTable(RULES) + + /** + * Must be called at the app-runtime composition seam so a broken rule table fails app + * startup instead of silently allowing everything. Throws (does not process.exit — + * this is library code) if the rule table failed self-validation at module load. + */ + export function assertInitialized(): void { + if (initError) { + throw new Error(`HardPolicy: rule table failed validation and cannot be used: ${initError}`) + } + } + + // --------------------------------------------------------------------------------- + // Audit probe — THE security oracle. Deterministic, in-memory, test-collectable. + // Not the tracer: tracing is best-effort/failure-suppressing and cannot prove a deny + // actually blocked execution. Every check() call — allow or deny, governed or not — + // appends a record, so the audit log itself proves the chokepoint was reached. + // --------------------------------------------------------------------------------- + + const MAX_AUDIT_LOG = 1000 + let auditLog: AuditRecord[] = [] + let auditSink: ((record: AuditRecord) => void) | null = null + + function emitAudit(record: AuditRecord): void { + auditLog.push(record) + if (auditLog.length > MAX_AUDIT_LOG) auditLog.shift() + if (auditSink) auditSink(record) + } + + /** Test support: read the in-memory audit log (oldest first). */ + export function getAuditLog(): readonly AuditRecord[] { + return auditLog + } + + /** Test support: reset the in-memory audit log between test cases. */ + export function clearAuditLog(): void { + auditLog = [] + } + + /** Test support: install a callback invoked synchronously on every audit emission. */ + export function setAuditSink(sink: ((record: AuditRecord) => void) | null): void { + auditSink = sink + } + + // --------------------------------------------------------------------------------- + // Deterministic, bounded digest of the final args snapshot — for audit correlation in + // tests (e.g. "was the deny keyed off the post-hook-mutated args?"), not a + // cryptographic hash and not for security purposes on its own. + // --------------------------------------------------------------------------------- + + const MAX_DIGEST_LENGTH = 2000 + + function stableStringify(value: unknown): string { + const seen = new WeakSet() + function normalize(input: unknown): unknown { + if (input === undefined) return null + if (typeof input === "function" || typeof input === "symbol") return String(input) + if (typeof input === "bigint") return input.toString() + if (input === null || typeof input !== "object") return input + if (seen.has(input)) return "[circular]" + seen.add(input) + if (Array.isArray(input)) return input.map(normalize) + const record = input as Record + const out: Record = {} + for (const key of Object.keys(record).sort()) out[key] = normalize(record[key]) + return out + } + try { + return JSON.stringify(normalize(value)) ?? "null" + } catch { + return "[unstringifiable]" + } + } + + function finalArgsDigest(args: unknown): string { + const str = stableStringify(args) + return str.length > MAX_DIGEST_LENGTH ? str.slice(0, MAX_DIGEST_LENGTH) : str + } + + // --------------------------------------------------------------------------------- + // The chokepoint. + // --------------------------------------------------------------------------------- + + const INTERNAL_ERROR_RULE_ID = "policy_internal_error" + const INTERNAL_ERROR_SAFE_REASON = "Policy could not be evaluated; denying for safety." + + export function check(input: Input): Decision { + let decision: Decision + let toolID = "unknown" + let source: Source = "native" + let sessionID = "" + let callID: string | undefined + let digest = "" + + try { + if (!isRecord(input as unknown)) { + throw new Error("hard-policy: input is not an object") + } + toolID = typeof input.toolID === "string" ? input.toolID : "unknown" + source = input.source + sessionID = typeof input.sessionID === "string" ? input.sessionID : "" + callID = input.callID + digest = finalArgsDigest(input.args) + + assertInitialized() + + const rules = rulesByTool.get(toolID) + if (!rules || rules.length === 0) { + // Ungoverned toolID — HardPolicy has no rule for it, so it always allows, + // regardless of args shape. Denying here would be a NEW block, not a relocation + // of an existing one. + decision = { allow: true } + } else { + let matched: Rule | null = null + for (const rule of rules) { + if (rule.match(input.args)) { + matched = rule + break + } + } + decision = matched + ? { allow: false, ruleID: matched.ruleID, safeReason: matched.safeReason } + : { allow: true } + } + } catch { + // Any failure — malformed top-level input, a rule's match() throwing on malformed + // args, a broken rule table, or an unexpected classifier exception — fails closed. + decision = { allow: false, ruleID: INTERNAL_ERROR_RULE_ID, safeReason: INTERNAL_ERROR_SAFE_REASON } + } + + emitAudit({ + ruleID: decision.allow ? "" : decision.ruleID, + dispatcher: source, + toolID, + source, + finalArgsDigest: digest, + sessionID, + callID, + decision, + timestamp: Date.now(), + }) + + return decision + } +} diff --git a/packages/opencode/src/effect/app-runtime.ts b/packages/opencode/src/effect/app-runtime.ts index c596a45b2..073654222 100644 --- a/packages/opencode/src/effect/app-runtime.ts +++ b/packages/opencode/src/effect/app-runtime.ts @@ -51,6 +51,14 @@ import { memoMap } from "@opencode-ai/core/effect/memo-map" import { BackgroundJob } from "@/background/job" import { RuntimeFlags } from "@/effect/runtime-flags" import { EventV2Bridge } from "@/event-v2-bridge" +import { HardPolicy } from "@/altimate/policy/hard-policy" + +// altimate_change start — HardPolicy enforcement (S3): fail-closed at the app-runtime +// composition seam. If the rule table is malformed, this throws at module load — the whole +// app-runtime module (and therefore anything that imports AppRuntime) fails to compose. This +// is a composition failure, not process.exit in library code, and not a silent implicit-allow. +HardPolicy.assertInitialized() +// altimate_change end // altimate_change start — Layer.suspend defers all the .defaultLayer reads past circular // module-init (fork Service facades participate in import cycles; eager access yields undefined). diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 289bd8a74..3f95fb23a 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -74,6 +74,9 @@ import { Tracer } from "../altimate/observability/tracing" // altimate_change — stamp an authoritative tool source + humanized MCP title import { stampRegistryToolSource, describeMcpTool } from "../altimate/tool-source" // altimate_change end +// altimate_change start — HardPolicy enforcement (S3) +import { HardPolicy } from "@/altimate/policy/hard-policy" +// altimate_change end import { Telemetry } from "@/telemetry" // altimate_change — session telemetry // @ts-ignore @@ -577,7 +580,11 @@ export namespace SessionPrompt { subagent_type: task.agent, command: task.command, } - await Plugin.trigger( + // altimate_change start — HardPolicy enforcement (S3) + // upstream_fix: Plugin.trigger's return value was previously discarded, so a + // tool.execute.before hook that mutated `args` had no effect on what actually + // ran — HardPolicy and execute must see the SAME final, post-hook args. + const hookInput = await Plugin.trigger( "tool.execute.before", { tool: "task", @@ -586,63 +593,85 @@ export namespace SessionPrompt { }, { args: taskArgs }, ) - let executionError: Error | undefined - const taskAgent = await Agent.get(task.agent) - const taskCtx: Tool.Context = { - agent: task.agent, - messageID: assistantMessage.id, - sessionID: sessionID, - abort, - callID: part.callID, - extra: { bypassAgentCheck: true }, - // altimate_change start — fork MessageV2.WithParts ≡ core SessionV1.WithParts at the Tool.Context boundary - messages: msgs as unknown as Tool.Context["messages"], - metadata: (input) => - // altimate_change start — Tool.Context.metadata/ask now return Effect (v1.17.9) - Effect.promise(async () => { - part = (await Session.updatePart({ - ...part, - type: "tool", - state: { - ...part.state, - ...input, - }, - } satisfies MessageV2.ToolPart)) as MessageV2.ToolPart - }), - ask: (req) => - Effect.promise(async () => { - // altimate_change start — core PermissionV1.Request uses readonly arrays; ask() validates the shape at runtime - await PermissionNext.ask({ - ...req, - sessionID: sessionID, - ruleset: PermissionNext.merge(taskAgent.permission, session.permission ?? []), - } as Parameters[0]) - // altimate_change end - }), - // altimate_change end - // altimate_change end - } - const result = await AppRuntime.runPromise(taskTool.execute(taskArgs, taskCtx)).catch((error) => { - executionError = error - log.error("subtask execution failed", { error, agent: task.agent, description: task.description }) - return undefined - }) - const attachments = result?.attachments?.map((attachment) => ({ - ...attachment, - id: PartID.ascending(), + const finalTaskArgs = hookInput.args + const policyDecision = HardPolicy.check({ + toolID: "task", + source: "task", + args: finalTaskArgs, sessionID, - messageID: assistantMessage.id, - })) - await Plugin.trigger( - "tool.execute.after", - { - tool: "task", + callID: part.id, + }) + // On deny: do NOT call taskTool.execute and do NOT fire tool.execute.after — + // fall through to the same error-surfacing machinery used for a thrown + // execution error below (part status "error", assistantMessage still finishes). + const { result, executionError, attachments } = await (async () => { + if (!policyDecision.allow) { + return { + result: undefined, + executionError: new Error(policyDecision.safeReason), + attachments: undefined, + } + } + let executionError: Error | undefined + const taskAgent = await Agent.get(task.agent) + const taskCtx: Tool.Context = { + agent: task.agent, + messageID: assistantMessage.id, + sessionID: sessionID, + abort, + callID: part.callID, + extra: { bypassAgentCheck: true }, + // altimate_change start — fork MessageV2.WithParts ≡ core SessionV1.WithParts at the Tool.Context boundary + messages: msgs as unknown as Tool.Context["messages"], + metadata: (input) => + // altimate_change start — Tool.Context.metadata/ask now return Effect (v1.17.9) + Effect.promise(async () => { + part = (await Session.updatePart({ + ...part, + type: "tool", + state: { + ...part.state, + ...input, + }, + } satisfies MessageV2.ToolPart)) as MessageV2.ToolPart + }), + ask: (req) => + Effect.promise(async () => { + // altimate_change start — core PermissionV1.Request uses readonly arrays; ask() validates the shape at runtime + await PermissionNext.ask({ + ...req, + sessionID: sessionID, + ruleset: PermissionNext.merge(taskAgent.permission, session.permission ?? []), + } as Parameters[0]) + // altimate_change end + }), + // altimate_change end + // altimate_change end + } + const result = await AppRuntime.runPromise(taskTool.execute(finalTaskArgs, taskCtx)).catch((error) => { + executionError = error + log.error("subtask execution failed", { error, agent: task.agent, description: task.description }) + return undefined + }) + await Plugin.trigger( + "tool.execute.after", + { + tool: "task", + sessionID, + callID: part.id, + args: finalTaskArgs, + }, + result, + ) + const attachments = result?.attachments?.map((attachment) => ({ + ...attachment, + id: PartID.ascending(), sessionID, - callID: part.id, - args: taskArgs, - }, - result, - ) + messageID: assistantMessage.id, + })) + return { result, executionError, attachments } + })() + // altimate_change end assistantMessage.finish = "tool-calls" assistantMessage.time.completed = Date.now() await Session.updateMessage(assistantMessage) @@ -1586,7 +1615,11 @@ export namespace SessionPrompt { inputSchema: jsonSchema(schema as any), async execute(args, options) { const ctx = context(args, options) - await Plugin.trigger( + // altimate_change start — HardPolicy enforcement (S3) + // upstream_fix: Plugin.trigger's return value was previously discarded, so a + // tool.execute.before hook that mutated `args` had no effect on what actually + // ran — HardPolicy and execute must see the SAME final, post-hook args. + const hookInput = await Plugin.trigger( "tool.execute.before", { tool: item.id, @@ -1597,8 +1630,30 @@ export namespace SessionPrompt { args, }, ) + const finalArgs = hookInput.args + const policyDecision = HardPolicy.check({ + toolID: item.id, + source: "native", + args: finalArgs, + sessionID: ctx.sessionID, + callID: ctx.callID, + }) + if (!policyDecision.allow) { + return { + title: "Blocked by policy", + output: policyDecision.safeReason, + metadata: { + error: policyDecision.safeReason, + hard_policy_denied: true, + code: "hard_policy_denied", + ruleID: policyDecision.ruleID, + success: false, + }, + } + } + // altimate_change end // altimate_change start — v1.17.9: Tool.Def.execute returns an Effect - const result = await AppRuntime.runPromise(item.execute(args, ctx)) + const result = await AppRuntime.runPromise(item.execute(finalArgs, ctx)) // altimate_change end const output = { ...result, @@ -1618,7 +1673,9 @@ export namespace SessionPrompt { tool: item.id, sessionID: ctx.sessionID, callID: ctx.callID, - args, + // altimate_change start — upstream_fix: after-hook sees the post-hook final args (was `args`) + args: finalArgs, + // altimate_change end }, stamped, ) @@ -1641,7 +1698,11 @@ export namespace SessionPrompt { item.execute = async (args, opts) => { const ctx = context(args, opts) - await Plugin.trigger( + // altimate_change start — HardPolicy enforcement (S3) + // upstream_fix: Plugin.trigger's return value was previously discarded, so a + // tool.execute.before hook that mutated `args` had no effect on what actually + // ran — HardPolicy and execute must see the SAME final, post-hook args. + const hookInput = await Plugin.trigger( "tool.execute.before", { tool: key, @@ -1652,6 +1713,29 @@ export namespace SessionPrompt { args, }, ) + const finalArgs = hookInput.args + + const policyDecision = HardPolicy.check({ + toolID: key, + source: "mcp", + args: finalArgs, + sessionID: ctx.sessionID, + callID: opts.toolCallId, + }) + if (!policyDecision.allow) { + return { + title: "Blocked by policy", + output: policyDecision.safeReason, + metadata: { + error: policyDecision.safeReason, + hard_policy_denied: true, + code: "hard_policy_denied", + ruleID: policyDecision.ruleID, + success: false, + }, + } + } + // altimate_change end // altimate_change start — upstream_fix: ctx.ask is Effect-valued; `await` on it only awaits the // Effect object and NEVER runs PermissionNext.ask, so MCP tools executed with NO permission @@ -1666,7 +1750,9 @@ export namespace SessionPrompt { ) // altimate_change end - const result = await execute(args, opts) + // altimate_change start — upstream_fix: dispatch the post-hook final args (was `args`) + const result = await execute(finalArgs, opts) + // altimate_change end await Plugin.trigger( "tool.execute.after", @@ -1674,7 +1760,9 @@ export namespace SessionPrompt { tool: key, sessionID: ctx.sessionID, callID: opts.toolCallId, - args, + // altimate_change start — upstream_fix: after-hook sees the post-hook final args (was `args`) + args: finalArgs, + // altimate_change end }, result, ) diff --git a/packages/opencode/src/session/tools.ts b/packages/opencode/src/session/tools.ts index 907d8eb79..ba3df13bc 100644 --- a/packages/opencode/src/session/tools.ts +++ b/packages/opencode/src/session/tools.ts @@ -23,6 +23,9 @@ import { stampRegistryToolSource, describeMcpTool } from "@/altimate/tool-source // altimate_change start — upstream_fix: ToolRegistry expects fork-branded model ids here import { ModelID } from "@/provider/schema" // altimate_change end +// altimate_change start — HardPolicy enforcement (S3) +import { HardPolicy } from "@/altimate/policy/hard-policy" +// altimate_change end export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { agent: Agent.Info @@ -89,12 +92,39 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { return run.promise( Effect.gen(function* () { const ctx = context(args, options) - yield* plugin.trigger( + // altimate_change start — HardPolicy enforcement (S3) + // upstream_fix: plugin.trigger's returned output was previously discarded, so a + // tool.execute.before hook that mutated `args` had no effect on what actually + // ran — HardPolicy and execute must see the SAME final, post-hook args. + const hookOutput = yield* plugin.trigger( "tool.execute.before", { tool: item.id, sessionID: ctx.sessionID, callID: ctx.callID }, { args }, ) - const result = yield* item.execute(args, ctx) + const finalArgs = hookOutput.args + const policyDecision = HardPolicy.check({ + toolID: item.id, + source: "native", + args: finalArgs, + sessionID: ctx.sessionID, + callID: ctx.callID, + }) + if (!policyDecision.allow) { + return { + title: "Blocked by policy", + output: policyDecision.safeReason, + metadata: { + error: policyDecision.safeReason, + hard_policy_denied: true, + code: "hard_policy_denied", + ruleID: policyDecision.ruleID, + success: false, + }, + } + } + // upstream_fix: dispatch the post-hook final args (was `args`). + const result = yield* item.execute(finalArgs, ctx) + // altimate_change end const output = { ...result, attachments: result.attachments?.map((attachment) => ({ @@ -108,7 +138,9 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { const stamped = stampRegistryToolSource(output, item) yield* plugin.trigger( "tool.execute.after", - { tool: item.id, sessionID: ctx.sessionID, callID: ctx.callID, args }, + // altimate_change start — upstream_fix: after-hook sees the post-hook final args (was `args`) + { tool: item.id, sessionID: ctx.sessionID, callID: ctx.callID, args: finalArgs }, + // altimate_change end stamped, ) if (options.abortSignal?.aborted) { @@ -135,14 +167,44 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { run.promise( Effect.gen(function* () { const ctx = context(args, opts) - yield* plugin.trigger( + // altimate_change start — HardPolicy enforcement (S3) + // upstream_fix: plugin.trigger's returned output was previously discarded, so a + // tool.execute.before hook that mutated `args` had no effect on what actually + // ran — HardPolicy and execute must see the SAME final, post-hook args. + const hookOutput = yield* plugin.trigger( "tool.execute.before", { tool: key, sessionID: ctx.sessionID, callID: opts.toolCallId }, { args }, ) + const finalArgs = hookOutput.args + const policyDecision = HardPolicy.check({ + toolID: key, + source: "mcp", + args: finalArgs, + sessionID: ctx.sessionID, + callID: opts.toolCallId, + }) + if (!policyDecision.allow) { + return { + title: "Blocked by policy", + metadata: { + error: policyDecision.safeReason, + hard_policy_denied: true, + code: "hard_policy_denied", + ruleID: policyDecision.ruleID, + success: false, + }, + output: policyDecision.safeReason, + attachments: [], + content: [], + } + } + // altimate_change end const result: Awaited>> = yield* Effect.gen(function* () { yield* ctx.ask({ permission: key, metadata: {}, patterns: ["*"], always: ["*"] }) - return yield* Effect.promise(() => execute(args, opts)) + // altimate_change start — upstream_fix: dispatch the post-hook final args (was `args`) + return yield* Effect.promise(() => execute(finalArgs, opts)) + // altimate_change end }).pipe( Effect.withSpan("Tool.execute", { attributes: { @@ -155,7 +217,9 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { ) yield* plugin.trigger( "tool.execute.after", - { tool: key, sessionID: ctx.sessionID, callID: opts.toolCallId, args }, + // altimate_change start — upstream_fix: after-hook sees the post-hook final args (was `args`) + { tool: key, sessionID: ctx.sessionID, callID: opts.toolCallId, args: finalArgs }, + // altimate_change end result, ) diff --git a/packages/opencode/src/tool/batch.ts b/packages/opencode/src/tool/batch.ts index 202f386e8..2d5e63941 100644 --- a/packages/opencode/src/tool/batch.ts +++ b/packages/opencode/src/tool/batch.ts @@ -11,6 +11,10 @@ import DESCRIPTION from "./batch.txt" const DISALLOWED = new Set(["batch"]) const FILTERED_FROM_SUGGESTIONS = new Set(["invalid", "patch", ...DISALLOWED]) +// altimate_change start — HardPolicy enforcement (S3) +import { HardPolicy } from "../altimate/policy/hard-policy" +// altimate_change end + // altimate_change start — v1.17.9: BatchTool is a legacy zod/Promise tool (adapted by tool-zod-compat). // Bridge the Promise-based LegacyContext back into the Effect-based Tool.Context the inner tools expect. function toEffectContext(ctx: LegacyContext, callID: string): Tool.Context { @@ -100,6 +104,20 @@ export const BatchTool = Tool.define("batch", { }, }) + // altimate_change start — HardPolicy enforcement (S3) + // BatchTool's inner dispatch has no tool.execute.before hook, so validatedParams is + // already the final args seen by both HardPolicy and execute. + const policyDecision = HardPolicy.check({ + toolID: call.tool, + source: "batch", + args: validatedParams, + sessionID: ctx.sessionID, + callID: partID, + }) + if (!policyDecision.allow) { + throw new Error(policyDecision.safeReason) + } + // altimate_change end // altimate_change start — v1.17.9: Tool.Def.execute returns an Effect; run via AppRuntime const result = await AppRuntime.runPromise(tool.execute(validatedParams, toEffectContext(ctx, partID))) // altimate_change end diff --git a/packages/opencode/test/altimate/defork/hardpolicy.test.ts b/packages/opencode/test/altimate/defork/hardpolicy.test.ts new file mode 100644 index 000000000..3aeb6ff95 --- /dev/null +++ b/packages/opencode/test/altimate/defork/hardpolicy.test.ts @@ -0,0 +1,1064 @@ +// De-fork spike S3 — HardPolicy kill-gate coverage. +// +// Oracle: HardPolicy.getAuditLog()/clearAuditLog()/setAuditSink() + execute-not-called +// counters — NOT trace evidence. TraceSpan.status is only ok|error (no "denied" state), +// and denials never call the underlying tool's execute, so an absent execute span does +// NOT by itself prove enforcement. Every assertion below is anchored to either (a) the +// structured `hard_policy_denied` result HardPolicy itself returns, (b) the audit log, +// or (c) a real execute-call counter. +// +// This file is analysis/test-only. No product code is modified here. + +import { test, expect, beforeEach } from "bun:test" +import * as nodePath from "path" +import { jsonSchema } from "ai" +import { Effect, Layer } from "effect" +import { NodeFileSystem } from "@effect/platform-node" +import { FetchHttpClient } from "effect/unstable/http" +import { Database } from "@opencode-ai/core/database/database" +import { FSUtil } from "@opencode-ai/core/fs-util" +import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" +import { Ripgrep } from "@opencode-ai/core/ripgrep" +import { ProviderV2 } from "@opencode-ai/core/provider" +import { ModelV2 } from "@opencode-ai/core/model" + +import { Instance } from "../../../src/project/instance" +import { MessageID, PartID, SessionID } from "../../../src/session/schema" +import { BatchTool } from "../../../src/tool/batch" +import { SessionTools } from "../../../src/session/tools" + +import { tmpdir, TestInstance } from "../../fixture/fixture" +import { testEffect } from "../../lib/effect" +import { TestLLMServer } from "../../lib/llm-server" +import { withLegacyInstanceRunner } from "../../session/legacy-instance" +import { initTool, toolInfo } from "../tool-fixture" + +import { ModelID, ProviderID } from "@/provider/schema" +import { InvalidTool } from "@/tool/invalid" +import { Plugin } from "@/plugin" +import { Permission } from "@/permission" +import { ToolRegistry } from "@/tool/registry" +import { MCP } from "@/mcp" +import { Truncate } from "@/tool/truncate" +import type { Tool } from "@/tool/tool" +import { HardPolicy } from "@/altimate/policy/hard-policy" +import { classifyAndCheck } from "@/altimate/tools/sql-classify" + +import { Agent as AgentSvc } from "@/agent/agent" +import { BackgroundJob } from "@/background/job" +import { Command } from "@/command" +import { Config } from "@/config/config" +import { LSP } from "@/lsp/lsp" +import { Env } from "@/env" +import { Git } from "@/git" +import { Image } from "@/image/image" +import { Question } from "@/question" +import { Todo } from "@/session/todo" +import { Session } from "@/session/session" +import { LLM } from "@/session/llm" +import { MessageV2 } from "@/session/message-v2" +import { SessionCompaction } from "@/session/compaction" +import { SessionSummary } from "@/session/summary" +import { Instruction } from "@/session/instruction" +import { SessionProcessor } from "@/session/processor" +import { SessionPrompt } from "@/session/prompt" +import { SessionRevert } from "@/session/revert" +import { SessionRunState } from "@/session/run-state" +import { SessionStatus } from "@/session/status" +import { Skill } from "@/skill" +import { Snapshot } from "@/snapshot" +import { Format } from "@/format" +import { RuntimeFlags } from "@/effect/runtime-flags" +import { EventV2Bridge } from "@/event-v2-bridge" +import { Provider as ProviderSvc } from "@/provider/provider" + +beforeEach(() => { + HardPolicy.clearAuditLog() + HardPolicy.setAuditSink(null) +}) + +// --------------------------------------------------------------------------- +// Section A — HardPolicy.check() core contract (pure, no dispatcher harness). +// --------------------------------------------------------------------------- + +test("A: assertInitialized does not throw against the real rule table", () => { + expect(() => HardPolicy.assertInitialized()).not.toThrow() +}) + +test("A: sql_execute DDL is denied", () => { + const decision = HardPolicy.check({ + toolID: "sql_execute", + source: "native", + args: { query: "DROP DATABASE prod" }, + sessionID: "ses_a1", + }) + expect(decision.allow).toBe(false) + if (!decision.allow) { + expect(decision.ruleID).toBe("sql_execute_ddl_v1") + expect(decision.safeReason).toContain("DROP DATABASE, DROP SCHEMA, and TRUNCATE are blocked") + } +}) + +test("A: bash DDL is denied", () => { + const decision = HardPolicy.check({ + toolID: "bash", + source: "native", + args: { command: "DROP DATABASE prod" }, + sessionID: "ses_a2", + }) + expect(decision.allow).toBe(false) + if (!decision.allow) { + expect(decision.ruleID).toBe("bash_ddl_v1") + expect(decision.safeReason).toContain("DROP DATABASE, DROP SCHEMA, and TRUNCATE via bash") + } +}) + +test("A: near-miss controls are allowed — proves the rule table is not trivially over-broad", () => { + expect( + HardPolicy.check({ toolID: "sql_execute", source: "native", args: { query: "DROP TABLE staging_tmp" }, sessionID: "s" }) + .allow, + ).toBe(true) + expect(HardPolicy.check({ toolID: "sql_execute", source: "native", args: { query: "SELECT 1" }, sessionID: "s" }).allow).toBe( + true, + ) + expect( + HardPolicy.check({ toolID: "bash", source: "native", args: { command: "DROP TABLE staging_tmp" }, sessionID: "s" }).allow, + ).toBe(true) + expect(HardPolicy.check({ toolID: "bash", source: "native", args: { command: "ls -la" }, sessionID: "s" }).allow).toBe(true) +}) + +test("A: ungoverned toolIDs are always allowed regardless of args shape", () => { + expect(HardPolicy.check({ toolID: "read", source: "native", args: { anything: true }, sessionID: "s" }).allow).toBe(true) + expect(HardPolicy.check({ toolID: "read", source: "native", args: undefined, sessionID: "s" }).allow).toBe(true) + expect(HardPolicy.check({ toolID: "read", source: "native", args: "not even an object", sessionID: "s" }).allow).toBe(true) +}) + +test("A: malformed args for a governed toolID fail closed — total function, never throws", () => { + expect(() => HardPolicy.check({ toolID: "sql_execute", source: "native", args: {}, sessionID: "s" })).not.toThrow() + const missingQuery = HardPolicy.check({ toolID: "sql_execute", source: "native", args: {}, sessionID: "s" }) + expect(missingQuery.allow).toBe(false) + if (!missingQuery.allow) expect(missingQuery.ruleID).toBe("policy_internal_error") + + expect(() => HardPolicy.check({ toolID: "bash", source: "native", args: null, sessionID: "s" })).not.toThrow() + const nullArgs = HardPolicy.check({ toolID: "bash", source: "native", args: null, sessionID: "s" }) + expect(nullArgs.allow).toBe(false) + if (!nullArgs.allow) expect(nullArgs.ruleID).toBe("policy_internal_error") + + // Circular-reference args must not throw during audit digesting either. + const circular: Record = {} + circular.self = circular + expect(() => HardPolicy.check({ toolID: "bash", source: "native", args: circular, sessionID: "s" })).not.toThrow() +}) + +test("A: malformed top-level input never throws and fails closed", () => { + expect(() => HardPolicy.check(null as never)).not.toThrow() + expect(() => HardPolicy.check(undefined as never)).not.toThrow() + expect(() => HardPolicy.check("not an object" as never)).not.toThrow() + expect(HardPolicy.check(null as never).allow).toBe(false) +}) + +test("A: every check() call emits an audit record, allow and deny alike", () => { + HardPolicy.clearAuditLog() + HardPolicy.check({ toolID: "read", source: "native", args: {}, sessionID: "ses_audit" }) + HardPolicy.check({ toolID: "bash", source: "native", args: { command: "DROP DATABASE prod" }, sessionID: "ses_audit" }) + const log = HardPolicy.getAuditLog() + expect(log.length).toBe(2) + expect(log[0]!.decision.allow).toBe(true) + expect(log[1]!.decision.allow).toBe(false) + expect(log[1]!.sessionID).toBe("ses_audit") +}) + +test("A: setAuditSink receives a synchronous callback on every emission", () => { + const seen: string[] = [] + HardPolicy.setAuditSink((record) => seen.push(record.toolID)) + HardPolicy.check({ toolID: "bash", source: "native", args: { command: "ls" }, sessionID: "s" }) + HardPolicy.setAuditSink(null) + expect(seen).toEqual(["bash"]) +}) + +test("A: SqlExecuteTool's own internal DDL check is a byte-identical, pre-existing redundant layer", () => { + // Confirms sql-classify.ts's own error string matches HardPolicy's SQL_DDL_SAFE_REASON. + // Not a HardPolicy blocker — flagged in the S3 report as a redundant protection layer. + const classified = classifyAndCheck("DROP DATABASE prod") + expect(classified.blocked).toBe(true) + const decision = HardPolicy.check({ + toolID: "sql_execute", + source: "native", + args: { query: "DROP DATABASE prod" }, + sessionID: "s", + }) + expect(decision.allow).toBe(false) + if (!decision.allow) { + // classifyAndCheck is the SAME function HardPolicy's matchSqlDdl calls, so blocked must agree. + expect(classified.blocked).toBe(true) + expect(decision.ruleID).toBe("sql_execute_ddl_v1") + } +}) + +// --------------------------------------------------------------------------- +// Section B — D1: SessionPrompt.resolveTools, registry-tools loop (ACTIVE). +// Harness adapted from route-sentinels.test.ts's own D1 test. +// --------------------------------------------------------------------------- + +test("D1: bash DDL through the real registry-tools loop is denied by HardPolicy, not executed", async () => { + await using tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const { Session } = await import("../../../src/session") + const { SessionPrompt } = await import("../../../src/session/prompt") + const { SessionProcessor } = await import("../../../src/session/processor") + + const session = await Session.create({}) + const providerID = ProviderID.make("test") + const modelID = ModelID.make("test-model") + const assistantID = MessageID.ascending() + + const assistantMessage = await Session.updateMessage({ + id: assistantID, + sessionID: session.id, + role: "assistant" as const, + time: { created: Date.now() }, + parentID: MessageID.ascending(), + modelID, + providerID, + mode: "build", + agent: "build", + cost: 0, + path: { cwd: tmp.path, root: tmp.path }, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + }) + + const processor = SessionProcessor.create({ + assistantMessage: assistantMessage as any, + sessionID: session.id, + model: { providerID, api: { id: modelID } } as any, + abort: new AbortController().signal, + }) + + const input: any = { + // Ruleset must be an array, not `{}` — see route-sentinels.test.ts D1 test for the + // Wildcard.match crash this avoids inside Skill.available. + agent: { name: "build", mode: "primary", permission: [{ permission: "*", pattern: "*", action: "allow" }], options: {} }, + model: { providerID, api: { id: modelID } }, + session: { ...session, permission: [{ permission: "*", pattern: "*", action: "allow" }] }, + processor, + bypassAgentCheck: true, + messages: [], + } + + const tools = await SessionPrompt.resolveTools(input) + expect(Object.keys(tools)).toContain("bash") + + HardPolicy.clearAuditLog() + const denied: any = await tools["bash"].execute!({ command: "DROP DATABASE prod" }, { toolCallId: "call_ddl", messages: [] } as any) + + // Even under an allow-all permission ruleset (`agent`/`session` permission above), the + // deny still fires — HardPolicy is not consulted through, and cannot be bypassed by, Permission. + expect(denied.metadata?.hard_policy_denied).toBe(true) + expect(denied.metadata?.ruleID).toBe("bash_ddl_v1") + expect(denied.metadata?.success).toBe(false) + + const lastDeny = HardPolicy.getAuditLog().at(-1) + expect(lastDeny?.decision.allow).toBe(false) + expect(lastDeny?.toolID).toBe("bash") + expect(lastDeny?.source).toBe("native") + if (lastDeny && !lastDeny.decision.allow) expect(lastDeny.decision.ruleID).toBe("bash_ddl_v1") + + // Near-miss control: a real, harmless command is NOT denied and actually executes for + // real (real spawn, real stdout) — proving the deny above wasn't just "bash never runs". + const allowed: any = await tools["bash"].execute!( + { command: "echo hardpolicy-nearmiss-ok", description: "echo a marker string" }, + { toolCallId: "call_safe", messages: [], abortSignal: new AbortController().signal } as any, + ) + expect(allowed.metadata?.hard_policy_denied).toBeUndefined() + expect(String(allowed.output ?? "")).toContain("hardpolicy-nearmiss-ok") + + const lastAllow = HardPolicy.getAuditLog().at(-1) + expect(lastAllow?.decision.allow).toBe(true) + expect(lastAllow?.toolID).toBe("bash") + }, + }) +}) + +// --------------------------------------------------------------------------- +// Section C — D3/D4: SessionTools.resolve, registry + MCP loops (LATENT, but +// structurally identical HardPolicy insertion to the ACTIVE D1/D2 in prompt.ts — +// confirmed via source diff during this build). Harness adapted from +// route-sentinels.test.ts's own D3/D4 mock-Layer pattern. +// --------------------------------------------------------------------------- + +function fakeProcessor(msgID: string) { + return { + message: { id: msgID }, + updateToolCall: () => Effect.succeed(undefined), + completeToolCall: () => Effect.void, + } +} + +const noopMcp = Layer.succeed( + MCP.Service, + MCP.Service.of({ + status: () => Effect.succeed({}), + clients: () => Effect.succeed({}), + tools: () => Effect.succeed({}), + prompts: () => Effect.succeed({}), + resources: () => Effect.succeed({}), + add: () => Effect.succeed({ status: {} }), + connect: () => Effect.void, + disconnect: () => Effect.void, + remove: () => Effect.void, + getPrompt: () => Effect.die(new Error("not implemented")), + } as any), +) + +const noopTruncate = Layer.succeed( + Truncate.Service, + Truncate.Service.of({ + cleanup: () => Effect.void, + write: () => Effect.succeed(""), + output: (content: string) => Effect.succeed({ content, truncated: false as const }), + limits: () => Effect.succeed({ maxLines: Number.MAX_SAFE_INTEGER, maxBytes: Number.MAX_SAFE_INTEGER }), + } as any), +) + +const noopPermission = Layer.succeed( + Permission.Service, + Permission.Service.of({ ask: () => Effect.void, reply: () => Effect.void, list: () => Effect.succeed([]) } as any), +) + +const passthroughPlugin = Layer.succeed( + Plugin.Service, + Plugin.Service.of({ + trigger: (_name: string, _input: unknown, output: unknown) => Effect.succeed(output), + list: () => Effect.succeed([]), + init: () => Effect.void, + } as any), +) + +test("D3 (registry loop): governed stub tool is denied for DDL and reached for a near-miss, with a real execute counter", async () => { + const invalidInfo = await toolInfo(InvalidTool) + const invalidDef = await Effect.runPromise(invalidInfo.init({ agent: undefined as any })) + let execCount = 0 + const stubBash = { + id: "bash", + registrySource: "builtin" as const, + description: invalidDef.description, + parameters: invalidDef.parameters, + execute: (args: unknown, ctx: Tool.Context) => + Effect.gen(function* () { + execCount++ + return yield* invalidDef.execute(args as any, ctx) + }), + } + + const layer = Layer.mergeAll( + passthroughPlugin, + noopPermission, + Layer.succeed( + ToolRegistry.Service, + ToolRegistry.Service.of({ + ids: () => Effect.succeed(["bash"]), + allInfos: () => Effect.succeed([]), + register: () => Effect.void, + tools: () => Effect.succeed([stubBash as any]), + } as any), + ), + noopMcp, + noopTruncate, + ) + + const input: any = { + agent: { name: "build", permission: [] }, + model: { providerID: ProviderID.make("test"), api: { id: ModelID.make("test-model") } }, + session: { id: "ses_d3", permission: [] }, + processor: fakeProcessor("msg_d3"), + bypassAgentCheck: true, + messages: [], + promptOps: {}, + } + + HardPolicy.clearAuditLog() + const tools = await Effect.runPromise(SessionTools.resolve(input).pipe(Effect.provide(layer))) + + const denied: any = await tools["bash"].execute!( + { tool: "invalid", error: "sentinel-probe", command: "DROP DATABASE prod" }, + { toolCallId: "call_d3_deny", messages: [] } as any, + ) + expect(denied.metadata?.hard_policy_denied).toBe(true) + expect(denied.metadata?.ruleID).toBe("bash_ddl_v1") + expect(execCount).toBe(0) + + const allowed: any = await tools["bash"].execute!( + { tool: "invalid", error: "sentinel-probe", command: "ls -la" }, + { toolCallId: "call_d3_allow", messages: [] } as any, + ) + expect(allowed.metadata?.hard_policy_denied).toBeUndefined() + expect(execCount).toBe(1) +}) + +test("D3 (registry loop): tool.execute.before mutation is what HardPolicy inspects, not the caller's original args", async () => { + const invalidInfo = await toolInfo(InvalidTool) + const invalidDef = await Effect.runPromise(invalidInfo.init({ agent: undefined as any })) + let execCount = 0 + const stubBash = { + id: "bash", + registrySource: "builtin" as const, + description: invalidDef.description, + parameters: invalidDef.parameters, + execute: (args: unknown, ctx: Tool.Context) => + Effect.gen(function* () { + execCount++ + return yield* invalidDef.execute(args as any, ctx) + }), + } + + const mutatingPlugin = Layer.succeed( + Plugin.Service, + Plugin.Service.of({ + trigger: (name: string, _input: unknown, output: unknown) => { + if (name === "tool.execute.before") { + return Effect.succeed({ args: { tool: "invalid", error: "sentinel-probe", command: "DROP DATABASE prod" } }) + } + return Effect.succeed(output) + }, + list: () => Effect.succeed([]), + init: () => Effect.void, + } as any), + ) + + const layer = Layer.mergeAll( + mutatingPlugin, + noopPermission, + Layer.succeed( + ToolRegistry.Service, + ToolRegistry.Service.of({ + ids: () => Effect.succeed(["bash"]), + allInfos: () => Effect.succeed([]), + register: () => Effect.void, + tools: () => Effect.succeed([stubBash as any]), + } as any), + ), + noopMcp, + noopTruncate, + ) + + const input: any = { + agent: { name: "build", permission: [] }, + model: { providerID: ProviderID.make("test"), api: { id: ModelID.make("test-model") } }, + session: { id: "ses_d3_mutate", permission: [] }, + processor: fakeProcessor("msg_d3_mutate"), + bypassAgentCheck: true, + messages: [], + promptOps: {}, + } + + HardPolicy.clearAuditLog() + const tools = await Effect.runPromise(SessionTools.resolve(input).pipe(Effect.provide(layer))) + + // Caller's original args look innocuous; the mocked tool.execute.before hook mutates them + // to a DDL command. HardPolicy must see the mutated (final) args, same as execute would. + const result: any = await tools["bash"].execute!( + { tool: "invalid", error: "sentinel-probe", command: "ls -la" }, + { toolCallId: "call_d3_final_args", messages: [] } as any, + ) + + expect(result.metadata?.hard_policy_denied).toBe(true) + expect(result.metadata?.ruleID).toBe("bash_ddl_v1") + expect(execCount).toBe(0) + + const last = HardPolicy.getAuditLog().at(-1) + expect(last?.decision.allow).toBe(false) + if (last && !last.decision.allow) expect(last.decision.ruleID).toBe("bash_ddl_v1") + + // The audit record's finalArgsDigest is the compliance oracle for "what HardPolicy actually + // checked" — it must reflect the POST-hook (mutated, DDL) args, not the caller's pre-hook + // (benign) args. Catches a digest/decision divergence a decision-only assertion would miss. + expect(last?.finalArgsDigest).toContain("DROP DATABASE prod") + expect(last?.finalArgsDigest).not.toContain("ls -la") +}) + +test("D3 (registry loop): audit finalArgsDigest reflects post-hook args in the ALLOW direction too — mirror of the deny-direction mutation test above, proves the digest isn't computed from the caller's pre-hook args", async () => { + const invalidInfo = await toolInfo(InvalidTool) + const invalidDef = await Effect.runPromise(invalidInfo.init({ agent: undefined as any })) + let execCount = 0 + const stubBash = { + id: "bash", + registrySource: "builtin" as const, + description: invalidDef.description, + parameters: invalidDef.parameters, + execute: (args: unknown, ctx: Tool.Context) => + Effect.gen(function* () { + execCount++ + return yield* invalidDef.execute(args as any, ctx) + }), + } + + // A before-hook that rewrites a DDL-looking caller command into a benign one — the mirror + // image of the "benign -> DDL" mutation test above. If finalArgsDigest were ever computed + // from the CALLER's pre-hook args instead of the post-hook args HardPolicy actually + // evaluated, this test would see the pre-hook DDL command in the digest even though the + // decision correctly allowed the (post-hook) benign command. + const rewritingPlugin = Layer.succeed( + Plugin.Service, + Plugin.Service.of({ + trigger: (name: string, _input: unknown, output: unknown) => { + if (name === "tool.execute.before") { + return Effect.succeed({ args: { tool: "invalid", error: "sentinel-probe", command: "ls -la" } }) + } + return Effect.succeed(output) + }, + list: () => Effect.succeed([]), + init: () => Effect.void, + } as any), + ) + + const layer = Layer.mergeAll( + rewritingPlugin, + noopPermission, + Layer.succeed( + ToolRegistry.Service, + ToolRegistry.Service.of({ + ids: () => Effect.succeed(["bash"]), + allInfos: () => Effect.succeed([]), + register: () => Effect.void, + tools: () => Effect.succeed([stubBash as any]), + } as any), + ), + noopMcp, + noopTruncate, + ) + + const input: any = { + agent: { name: "build", permission: [] }, + model: { providerID: ProviderID.make("test"), api: { id: ModelID.make("test-model") } }, + session: { id: "ses_d3_digest", permission: [] }, + processor: fakeProcessor("msg_d3_digest"), + bypassAgentCheck: true, + messages: [], + promptOps: {}, + } + + HardPolicy.clearAuditLog() + const tools = await Effect.runPromise(SessionTools.resolve(input).pipe(Effect.provide(layer))) + + // Caller's original args look dangerous; the mocked tool.execute.before hook rewrites them to + // a benign command. HardPolicy must both DECIDE and AUDIT off the post-hook (final) args. + const result: any = await tools["bash"].execute!( + { tool: "invalid", error: "sentinel-probe", command: "DROP DATABASE prod" }, + { toolCallId: "call_d3_digest", messages: [] } as any, + ) + + expect(result.metadata?.hard_policy_denied).toBeUndefined() + expect(execCount).toBe(1) + + const last = HardPolicy.getAuditLog().at(-1) + expect(last?.decision.allow).toBe(true) + // The digest is the compliance oracle for "what HardPolicy actually checked" — it must show + // the post-hook benign command and must NOT show the caller's pre-hook DDL command. + expect(last?.finalArgsDigest).toContain("ls -la") + expect(last?.finalArgsDigest).not.toContain("DROP DATABASE prod") +}) + +test("D4 (MCP loop): governed stub MCP tool is denied for DDL args, mcp execute never runs", async () => { + let mcpExecCount = 0 + const mcpMock = Layer.succeed( + MCP.Service, + MCP.Service.of({ + status: () => Effect.succeed({}), + clients: () => Effect.succeed({}), + tools: () => + Effect.succeed({ + bash: { + client: "test-mcp-client", + description: "stub mcp bash for HardPolicy D4 test", + inputSchema: jsonSchema({ type: "object", properties: { command: { type: "string" } } }), + execute: async (_args: any) => { + mcpExecCount++ + return { content: [{ type: "text", text: "mcp-executed" }] } + }, + }, + }), + prompts: () => Effect.succeed({}), + resources: () => Effect.succeed({}), + add: () => Effect.succeed({ status: {} }), + connect: () => Effect.void, + disconnect: () => Effect.void, + remove: () => Effect.void, + getPrompt: () => Effect.die(new Error("not implemented")), + } as any), + ) + + const layer = Layer.mergeAll( + passthroughPlugin, + noopPermission, + Layer.succeed( + ToolRegistry.Service, + ToolRegistry.Service.of({ + ids: () => Effect.succeed([]), + allInfos: () => Effect.succeed([]), + register: () => Effect.void, + tools: () => Effect.succeed([]), + } as any), + ), + mcpMock, + noopTruncate, + ) + + const input: any = { + agent: { name: "build", permission: [] }, + model: { providerID: ProviderID.make("test"), api: { id: ModelID.make("test-model") } }, + session: { id: "ses_d4", permission: [] }, + processor: fakeProcessor("msg_d4"), + bypassAgentCheck: true, + messages: [], + promptOps: {}, + } + + HardPolicy.clearAuditLog() + const tools = await Effect.runPromise(SessionTools.resolve(input).pipe(Effect.provide(layer))) + expect(Object.keys(tools)).toContain("bash") + + const denied: any = await tools["bash"].execute!({ command: "DROP DATABASE prod" }, { toolCallId: "call_d4_deny", messages: [] } as any) + expect(denied.metadata?.hard_policy_denied).toBe(true) + expect(denied.metadata?.ruleID).toBe("bash_ddl_v1") + expect(mcpExecCount).toBe(0) + + const last = HardPolicy.getAuditLog().at(-1) + expect(last?.decision.allow).toBe(false) + expect(last?.source).toBe("mcp") + expect(last?.toolID).toBe("bash") +}) + +// --------------------------------------------------------------------------- +// Section D — D5: BatchTool inner dispatch (ACTIVE). batch.ts has no +// tool.execute.before hook, so the final-args-mutation scenario is N/A here +// (validatedParams already IS the final args both HardPolicy and execute see). +// --------------------------------------------------------------------------- + +test("D5: BatchTool denies a bash DDL call via HardPolicy without running it, and executes a real near-miss", async () => { + await using tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const { Session } = await import("../../../src/session") + + const session = await Session.create({}) + const assistantID = MessageID.ascending() + const providerID = ProviderID.make("test") + const modelID = ModelID.make("test-model") + await Session.updateMessage({ + id: assistantID, + sessionID: session.id, + role: "assistant" as const, + time: { created: Date.now() }, + parentID: MessageID.ascending(), + modelID, + providerID, + mode: "build", + agent: "build", + cost: 0, + path: { cwd: tmp.path, root: tmp.path }, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + }) + + const batch = await initTool(BatchTool) + + HardPolicy.clearAuditLog() + const denyResult = await batch.execute( + { + tool_calls: [ + { tool: "bash", parameters: { command: "DROP DATABASE prod", description: "drop the prod database" } }, + ], + }, + { + sessionID: session.id, + messageID: assistantID, + agent: "build", + abort: new AbortController().signal, + messages: [], + } as any, + ) + expect((denyResult as any).metadata.details[0].success).toBe(false) + + const denyAudit = HardPolicy.getAuditLog().at(-1) + expect(denyAudit?.decision.allow).toBe(false) + expect(denyAudit?.toolID).toBe("bash") + expect(denyAudit?.source).toBe("batch") + if (denyAudit && !denyAudit.decision.allow) expect(denyAudit.decision.ruleID).toBe("bash_ddl_v1") + + // Near-miss: a real, harmless command still runs for real through the same dispatcher. + const allowResult = await batch.execute( + { + tool_calls: [ + { + tool: "bash", + parameters: { command: "echo hardpolicy-batch-nearmiss-ok", description: "echo a marker string" }, + }, + ], + }, + { + sessionID: session.id, + messageID: assistantID, + agent: "build", + abort: new AbortController().signal, + messages: [], + } as any, + ) + expect((allowResult as any).metadata.details[0].success).toBe(true) + expect(String((allowResult as any).output ?? "")).toContain("All 1 tools executed successfully") + + const allowAudit = HardPolicy.getAuditLog().at(-1) + expect(allowAudit?.decision.allow).toBe(true) + expect(allowAudit?.toolID).toBe("bash") + }, + }) +}) + +test("D5: BatchTool denies sql_execute DDL via HardPolicy (redundant with SqlExecuteTool's own internal check, which fires first)", async () => { + await using tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const { Session } = await import("../../../src/session") + + const session = await Session.create({}) + const assistantID = MessageID.ascending() + const providerID = ProviderID.make("test") + const modelID = ModelID.make("test-model") + await Session.updateMessage({ + id: assistantID, + sessionID: session.id, + role: "assistant" as const, + time: { created: Date.now() }, + parentID: MessageID.ascending(), + modelID, + providerID, + mode: "build", + agent: "build", + cost: 0, + path: { cwd: tmp.path, root: tmp.path }, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + }) + + const batch = await initTool(BatchTool) + + HardPolicy.clearAuditLog() + const result = await batch.execute( + { tool_calls: [{ tool: "sql_execute", parameters: { query: "DROP DATABASE prod" } }] }, + { + sessionID: session.id, + messageID: assistantID, + agent: "build", + abort: new AbortController().signal, + messages: [], + } as any, + ) + expect((result as any).metadata.details[0].success).toBe(false) + + // HardPolicy's own dispatcher-level deny is what we assert here (audit log), independent + // of whichever of the two redundant checks the caller happens to observe in the error text. + const audit = HardPolicy.getAuditLog().at(-1) + expect(audit?.decision.allow).toBe(false) + expect(audit?.toolID).toBe("sql_execute") + expect(audit?.source).toBe("batch") + if (audit && !audit.decision.allow) expect(audit.decision.ruleID).toBe("sql_execute_ddl_v1") + }, + }) +}) + +// --------------------------------------------------------------------------- +// Section E — D6: direct "task" tool dispatch inside SessionPrompt.loop's +// subtask-handling branch (ACTIVE). Real subtask driven end-to-end through +// SessionPrompt.Service.loop() — the S3 deliverable deferred from S2's +// citation-only route-sentinels.test.ts D6 coverage. +// +// toolID/source for D6 are literally "task"/"task" (src/session/prompt.ts). +// No v1 rule targets toolID "task" (only sql_execute/bash are governed), so +// under REAL rules this route is always allow — genuine subagent DROP DATABASE +// protection comes from the CHILD session's own D1/D2 (reached via D7 +// recursion), not from D6 itself. The forced-deny test below proves the D6 +// wiring/order by temporarily monkeypatching HardPolicy.check. +// --------------------------------------------------------------------------- + +const ref = { providerID: ProviderV2.ID.make("test"), modelID: ModelV2.ID.make("test-model") } + +const summary = Layer.succeed( + SessionSummary.Service, + SessionSummary.Service.of({ + summarize: () => Effect.void, + diff: () => Effect.succeed([]), + computeDiff: () => Effect.succeed([]), + } as any), +) + +const mcp = Layer.succeed(MCP.Service, MCP.Service.of({} as any)) +const lsp = Layer.succeed(LSP.Service, LSP.Service.of({} as any)) + +const status = SessionStatus.layer.pipe(Layer.provideMerge(EventV2Bridge.defaultLayer)) +const run = SessionRunState.layer.pipe(Layer.provide(status)) +const infra = Layer.mergeAll(NodeFileSystem.layer, CrossSpawnSpawner.defaultLayer) + +function makePrompt() { + const deps = Layer.mergeAll( + Session.defaultLayer, + Snapshot.defaultLayer, + LLM.defaultLayer, + Env.defaultLayer, + AgentSvc.defaultLayer, + Command.defaultLayer, + Permission.defaultLayer, + Plugin.defaultLayer, + Config.defaultLayer, + ProviderSvc.defaultLayer, + lsp, + mcp, + FSUtil.defaultLayer, + BackgroundJob.defaultLayer, + status, + Database.defaultLayer, + EventV2Bridge.defaultLayer, + ).pipe(Layer.provideMerge(infra)) + const question = Question.layer.pipe(Layer.provideMerge(deps)) + const todo = Todo.layer.pipe(Layer.provideMerge(deps)) + const registry = ToolRegistry.layer.pipe( + Layer.provide(Skill.defaultLayer), + Layer.provide(FetchHttpClient.layer), + Layer.provide(CrossSpawnSpawner.defaultLayer), + Layer.provide(Git.defaultLayer), + Layer.provide(Ripgrep.defaultLayer), + Layer.provide(Format.defaultLayer), + Layer.provide(RuntimeFlags.layer({ experimentalEventSystem: true })), + Layer.provideMerge(todo), + Layer.provideMerge(question), + Layer.provideMerge(deps), + ) + const trunc = Truncate.layer.pipe(Layer.provideMerge(deps)) + const proc = SessionProcessor.layer.pipe( + Layer.provide(summary), + Layer.provide(Image.defaultLayer), + Layer.provide(RuntimeFlags.layer({ experimentalEventSystem: true })), + Layer.provideMerge(deps), + ) + const compact = SessionCompaction.layer.pipe( + Layer.provide(RuntimeFlags.layer({ experimentalEventSystem: true })), + Layer.provideMerge(proc), + Layer.provideMerge(deps), + ) + return SessionPrompt.layer.pipe( + Layer.provide(SessionRevert.defaultLayer), + Layer.provide(Image.defaultLayer), + Layer.provide(summary), + Layer.provideMerge(run), + Layer.provideMerge(compact), + Layer.provideMerge(proc), + Layer.provideMerge(registry), + Layer.provideMerge(trunc), + Layer.provide(Instruction.defaultLayer), + Layer.provide(RuntimeFlags.layer({ experimentalEventSystem: true })), + Layer.provideMerge(deps), + Layer.provide(summary), + ) +} +function makeHttp() { + return Layer.mergeAll(TestLLMServer.layer, makePrompt()) +} + +const it = withLegacyInstanceRunner(testEffect(makeHttp())) + +const cfg = { + provider: { + test: { + name: "Test", + id: "test", + env: [], + npm: "@ai-sdk/openai-compatible", + models: { + "test-model": { + id: "test-model", + name: "Test Model", + attachment: false, + reasoning: false, + temperature: false, + tool_call: true, + release_date: "2025-01-01", + limit: { context: 100000, output: 10000 }, + cost: { input: 0, output: 0 }, + options: {}, + }, + }, + options: { apiKey: "test-key", baseURL: "http://localhost:1/v1" }, + }, + }, +} +function providerCfg(url: string) { + return { + ...cfg, + provider: { ...cfg.provider, test: { ...cfg.provider.test, options: { ...cfg.provider.test.options, baseURL: url } } }, + } +} + +const writeText = (filePath: string, content: string) => + Effect.promise(async () => { + const fs = await import("fs/promises") + await fs.writeFile(filePath, content, "utf8") + }) +const writeConfig = Effect.fn("hardpolicy.writeConfig")(function* (dir: string, config: unknown) { + yield* writeText(nodePath.join(dir, "opencode.json"), JSON.stringify({ $schema: "https://opencode.ai/config.json", ...(config as any) })) +}) +const useServerConfig = Effect.fn("hardpolicy.useServerConfig")(function* (config: (url: string) => unknown) { + const { directory: dir } = yield* TestInstance + const llm = yield* TestLLMServer + yield* writeConfig(dir, config(llm.url)) + return { dir, llm } +}) + +const user = Effect.fn("hardpolicy.user")(function* (sessionID: SessionID, text: string) { + const session = yield* Session.Service + const msg = yield* session.updateMessage({ + id: MessageID.ascending(), + role: "user", + sessionID, + agent: "build", + model: ref, + time: { created: Date.now() }, + } as any) + yield* session.updatePart({ id: PartID.ascending(), messageID: msg.id, sessionID, type: "text", text } as any) + return msg +}) + +const addSubtask = (sessionID: SessionID, messageID: MessageID, model = ref) => + Effect.gen(function* () { + const session = yield* Session.Service + yield* session.updatePart({ + id: PartID.ascending(), + messageID, + sessionID, + type: "subtask", + prompt: "look into the cache key path", + description: "inspect bug", + agent: "general", + model, + } as any) + }) + +function errorTool(parts: readonly any[]): any { + return parts.find((p) => p?.type === "tool" && p?.state?.status === "error") +} +function completedTool(parts: readonly any[]): any { + return parts.find((p) => p?.type === "tool" && p?.state?.status === "completed") +} + +it.instance("D6: wiring/order proof — real subtask run reaches HardPolicy.check with toolID/source 'task' and is allowed", () => + Effect.gen(function* () { + const { llm } = yield* useServerConfig((url) => ({ + ...providerCfg(url), + agent: { general: { model: "test/test-model" } }, + })) + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const chat = yield* sessions.create({ title: "Pinned" }) + + yield* llm.tool("task", { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + }) + yield* llm.text("done") // consumed by the child agent's own turn inside taskTool.execute + yield* llm.text("done") // consumed by the parent loop's follow-up turn after the tool result + + const msg = yield* user(chat.id, "hello") + yield* addSubtask(chat.id, msg.id) + + HardPolicy.clearAuditLog() + const result = yield* prompt.loop({ sessionID: chat.id }) + expect(result.info.role).toBe("assistant") + expect(yield* llm.calls).toBeGreaterThanOrEqual(2) + + const msgs = yield* MessageV2.filterCompactedEffect(chat.id) + const taskMsg = msgs.find((item: any) => item.info.role === "assistant" && item.info.agent === "general") + expect(taskMsg?.info.role).toBe("assistant") + + if (taskMsg && taskMsg.info.role === "assistant") { + const completed = completedTool(taskMsg.parts) + expect(completed).toBeDefined() + } + + const taskAudit = HardPolicy.getAuditLog().find((r) => r.toolID === "task" && r.source === "task" && r.sessionID === chat.id) + expect(taskAudit).toBeDefined() + expect(taskAudit?.decision.allow).toBe(true) + }), +) + +it.instance("D6: forced-deny proof — a HardPolicy deny at the task dispatcher blocks taskTool.execute entirely (child agent turn never runs)", () => + Effect.gen(function* () { + const { llm } = yield* useServerConfig((url) => ({ + ...providerCfg(url), + agent: { general: { model: "test/test-model" } }, + })) + const prompt = yield* SessionPrompt.Service + const sessions = yield* Session.Service + const chat = yield* sessions.create({ title: "Pinned" }) + + yield* llm.tool("task", { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + }) + yield* llm.text("done") // scheduled for the child agent turn — must go UNCONSUMED if D6 truly denies + + const msg = yield* user(chat.id, "hello") + yield* addSubtask(chat.id, msg.id) + + const originalCheck = HardPolicy.check + const FORCED_SAFE_REASON = "test-forced-deny-reason (D6 wiring proof)" + ;(HardPolicy as any).check = (input: HardPolicy.Input) => { + if (input.toolID === "task") { + return { allow: false, ruleID: "test_forced_task_deny", safeReason: FORCED_SAFE_REASON } + } + return originalCheck(input) + } + + try { + HardPolicy.clearAuditLog() + const result = yield* prompt.loop({ sessionID: chat.id }) + expect(result.info.role).toBe("assistant") + // Per the wiring/order proof test above, a real (allowed) task run consumes 3 LLM calls: + // (1) parent emits the "task" tool call, (2) the child agent's own turn inside + // taskTool.execute, (3) the parent's mandatory follow-up turn after the tool result comes + // back — that follow-up happens regardless of whether the tool succeeded or errored. Under + // a forced deny, taskTool.execute is never invoked, so call (2) never happens — only calls + // (1) and (3) occur, consuming the single scheduled llm.text("done") as the PARENT's + // follow-up, not a child turn. This is the execute-not-called proof for D6: exactly 2 calls, + // not 3 — confirmed below by the tool part on this same message being an ERROR, not a + // COMPLETED result (a completed result is only possible if taskTool.execute actually ran + // the child agent to produce it). + expect(yield* llm.calls).toBe(2) + + const msgs = yield* MessageV2.filterCompactedEffect(chat.id) + const parentMsg = msgs.find((item: any) => item.info.role === "assistant" && item.info.agent === "general") + expect(parentMsg?.info.role).toBe("assistant") + + if (parentMsg && parentMsg.info.role === "assistant") { + const errored = errorTool(parentMsg.parts) + expect(errored).toBeDefined() + expect(String(errored?.state?.error ?? "")).toContain("Tool execution failed") + expect(String(errored?.state?.error ?? "")).toContain(FORCED_SAFE_REASON) + } + + const denyAudit = HardPolicy.getAuditLog().find((r) => r.toolID === "task" && r.sessionID === chat.id) + // Note: this record was emitted by the ORIGINAL HardPolicy.check via emitAudit before we + // monkeypatched the outer property — the monkeypatch replaces the whole `check` function, + // so under the forced-deny path the audit record instead comes from our test's own + // bookkeeping. We assert the OBSERVABLE effect (no child turn, error surfaced) as the + // primary oracle, per the file's stated design (audit log is the additional check, not + // the sole one, precisely because a monkeypatched check() has its own emission behavior). + void denyAudit + } finally { + ;(HardPolicy as any).check = originalCheck + } + }), +) diff --git a/packages/opencode/test/altimate/defork/route-sentinels.test.ts b/packages/opencode/test/altimate/defork/route-sentinels.test.ts new file mode 100644 index 000000000..b502e266e --- /dev/null +++ b/packages/opencode/test/altimate/defork/route-sentinels.test.ts @@ -0,0 +1,346 @@ +// De-fork spike S2 — execution-route sentinels. +// +// Each `active` dispatcher documented in docs/internal/defork-route-matrix.md +// gets a real-execution test here (driving a harmless tool through the real +// dispatch code path and asserting execution actually reached it), or — where +// full real execution is impractical to set up without a heavy production +// harness — a structural/source-text regression sentinel that pins the exact +// dispatch chokepoint plus a citation of an existing, currently-passing +// real-execution test that already proves the route works end to end. +// `latent` dispatchers get a reachability guard designed to fail the moment +// the route gains a real production caller. +// +// This file is analysis/test-only. No product code is modified here. + +import { test, expect } from "bun:test" +import { Effect, Layer } from "effect" +import { readFileSync } from "fs" +import { join } from "path" +import { Instance } from "../../../src/project/instance" +import { tmpdir } from "../../fixture/fixture" +import { ModelID, ProviderID } from "@/provider/schema" +import { MessageID } from "../../../src/session/schema" +import { BatchTool } from "../../../src/tool/batch" +import { InvalidTool } from "@/tool/invalid" +import { initTool, toolInfo } from "../tool-fixture" +import { Plugin } from "@/plugin" +import { Permission } from "@/permission" +import { ToolRegistry } from "@/tool/registry" +import { MCP } from "@/mcp" +import { Truncate } from "@/tool/truncate" +import { SessionTools } from "../../../src/session/tools" + +const SRC = join(import.meta.dir, "..", "..", "..", "src") + +function readSrc(relPath: string): string { + return readFileSync(join(SRC, relPath), "utf8") +} + +// --------------------------------------------------------------------------- +// D1 — SessionPrompt.resolveTools, registry-tools loop (src/session/prompt.ts) +// --------------------------------------------------------------------------- + +test("D1: resolveTools reaches item.execute for a real registry tool", async () => { + await using tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const { Session } = await import("../../../src/session") + const { SessionPrompt } = await import("../../../src/session/prompt") + const { SessionProcessor } = await import("../../../src/session/processor") + + const session = await Session.create({}) + const providerID = ProviderID.make("test") + const modelID = ModelID.make("test-model") + const assistantID = MessageID.ascending() + + const assistantMessage = await Session.updateMessage({ + id: assistantID, + sessionID: session.id, + role: "assistant" as const, + time: { created: Date.now() }, + parentID: MessageID.ascending(), + modelID, + providerID, + mode: "build", + agent: "build", + cost: 0, + path: { cwd: tmp.path, root: tmp.path }, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + }) + + const processor = SessionProcessor.create({ + assistantMessage: assistantMessage as any, + sessionID: session.id, + model: { providerID, api: { id: modelID } } as any, + abort: new AbortController().signal, + }) + + const input: any = { + // `permission` is a PermissionV1.Ruleset (array of rules), NOT a bare object — + // Permission.evaluate() does `rulesets.flat().findLast(...)` (src/permission/index.ts:43-53), + // so passing `{}` instead of `[]` makes `rule.permission`/`rule.pattern` undefined and + // crashes Wildcard.match() deep inside Skill.available (src/skill/index.ts:379), which + // resolveTools's real ToolRegistry.tools() call transitively reaches. + agent: { name: "build", mode: "primary", permission: [], options: {} }, + model: { providerID, api: { id: modelID } }, + session: { ...session, permission: [] }, + processor, + bypassAgentCheck: true, + messages: [], + } + + const tools = await SessionPrompt.resolveTools(input) + expect(Object.keys(tools)).toContain("invalid") + + const result = await tools["invalid"].execute!( + { tool: "invalid", error: "sentinel-probe" }, + { toolCallId: "call_1", messages: [] } as any, + ) + expect((result as any).output).toContain("The arguments provided to the tool are invalid: sentinel-probe") + }, + }) +}) + +test("D1: resolveTools dispatch chokepoint is present and unique", () => { + const lines = readSrc("session/prompt.ts").split("\n") + // Assert by content (exactly one occurrence), NOT a hardcoded line number, so + // insertions above the chokepoint don't break the pin. Arg-agnostic: matches + // both the upstream `item.execute(args, ctx)` and S3's `item.execute(finalArgs, + // ctx)` (S3 passes the post-`tool.execute.before` args), so this same test + // passes with or without the S3 HardPolicy changes applied. + expect(lines.filter((l) => l.includes("AppRuntime.runPromise(item.execute(") && l.includes(", ctx))")).length).toBe(1) + expect(lines.filter((l) => l.includes("stampRegistryToolSource(output, item)")).length).toBe(1) +}) + +// --------------------------------------------------------------------------- +// D3/D4 — SessionTools.resolve, registry + MCP loops (src/session/tools.ts) +// Documented as `latent`: wired but with zero production callers at HEAD. +// --------------------------------------------------------------------------- + +test("D3/D4: SessionTools.resolve has no production caller (latent guard)", () => { + // If this ever finds a real call site outside src/session/tools.ts itself + // (not a doc-comment mentioning the name), D3/D4's classification must be + // revisited from `latent` to `active` and this test updated accordingly. + const promptSrc = readSrc("session/prompt.ts") + const toolSourceSrc = readSrc("altimate/tool-source.ts") + + // Known, expected occurrences: doc-comments only, never a call. + const promptMatches = [...promptSrc.matchAll(/SessionTools/g)] + expect(promptMatches).toHaveLength(2) + for (const match of promptMatches) { + const lineStart = promptSrc.lastIndexOf("\n", match.index) + 1 + const line = promptSrc.slice(lineStart, promptSrc.indexOf("\n", match.index)) + expect(line.trim().startsWith("//")).toBe(true) + expect(line).not.toContain("SessionTools.resolve(") + } + + const toolSourceMatches = [...toolSourceSrc.matchAll(/SessionTools/g)] + expect(toolSourceMatches).toHaveLength(2) + for (const match of toolSourceMatches) { + const lineStart = toolSourceSrc.lastIndexOf("\n", match.index) + 1 + const line = toolSourceSrc.slice(lineStart, toolSourceSrc.indexOf("\n", match.index)) + expect(line.trim().startsWith("*")).toBe(true) + expect(line).not.toContain("SessionTools.resolve(") + } +}) + +test("D3/D4: SessionTools.resolve dispatches a tool when invoked directly", async () => { + await Instance.provide({ + directory: "/tmp", + fn: async () => { + const invalidInfo = await toolInfo(InvalidTool) + const invalidDef = await Effect.runPromise(invalidInfo.init({ agent: undefined as any })) + + const layer = Layer.mergeAll( + Layer.succeed( + Plugin.Service, + Plugin.Service.of({ + trigger: (_name, _input, output) => Effect.succeed(output), + list: () => Effect.succeed([]), + init: () => Effect.void, + }), + ), + Layer.succeed( + Permission.Service, + Permission.Service.of({ + ask: () => Effect.void, + reply: () => Effect.void, + list: () => Effect.succeed([]), + }), + ), + Layer.succeed( + ToolRegistry.Service, + ToolRegistry.Service.of({ + ids: () => Effect.succeed(["invalid"]), + allInfos: () => Effect.succeed([]), + register: () => Effect.void, + tools: () => + Effect.succeed([ + { + id: "invalid", + registrySource: "builtin", + description: invalidDef.description, + parameters: invalidDef.parameters, + execute: invalidDef.execute, + } as any, + ]), + }), + ), + Layer.succeed( + MCP.Service, + MCP.Service.of({ + status: () => Effect.succeed({}), + clients: () => Effect.succeed({}), + tools: () => Effect.succeed({}), + prompts: () => Effect.succeed({}), + resources: () => Effect.succeed({}), + add: () => Effect.succeed({ status: {} }), + connect: () => Effect.void, + disconnect: () => Effect.void, + remove: () => Effect.void, + getPrompt: () => Effect.die(new Error("not implemented")), + } as any), + ), + Layer.succeed( + Truncate.Service, + Truncate.Service.of({ + cleanup: () => Effect.void, + write: () => Effect.succeed(""), + output: (content: string) => Effect.succeed({ content, truncated: false as const }), + limits: () => Effect.succeed({ maxLines: Number.MAX_SAFE_INTEGER, maxBytes: Number.MAX_SAFE_INTEGER }), + }), + ), + ) + + const fakeProcessor = { + message: { id: "msg_fake" } as any, + updateToolCall: () => Effect.succeed(undefined), + completeToolCall: () => Effect.void, + } + + const input: any = { + agent: { name: "build", mode: "primary", permission: {}, options: {} }, + model: { providerID: "test", api: { id: "test-model" } }, + session: { id: "ses_fake", permission: [] }, + processor: fakeProcessor, + bypassAgentCheck: true, + messages: [], + promptOps: {}, + } + + const tools = await Effect.runPromise(SessionTools.resolve(input).pipe(Effect.provide(layer))) + expect(Object.keys(tools)).toContain("invalid") + + const result = await tools["invalid"].execute!( + { tool: "invalid", error: "sentinel-probe" }, + { toolCallId: "call_1", messages: [] } as any, + ) + expect((result as any).output).toContain("The arguments provided to the tool are invalid: sentinel-probe") + }, + }) +}) + +test("D3/D4: SessionTools.resolve dispatch chokepoints are present and unique", () => { + const lines = readSrc("session/tools.ts").split("\n") + // Content-based + arg-agnostic (matches `args` or S3's `finalArgs`) — see D1's note. + expect(lines.filter((l) => l.includes("yield* item.execute(") && l.includes(", ctx)")).length).toBe(1) + expect(lines.filter((l) => l.includes("Effect.promise(() => execute(") && l.includes(", opts))")).length).toBe(1) +}) + +// --------------------------------------------------------------------------- +// D5 — BatchTool inner dispatch (src/tool/batch.ts) +// --------------------------------------------------------------------------- + +test("D5: BatchTool dispatches inner tool.execute without Plugin.trigger", async () => { + await using tmp = await tmpdir({ git: true }) + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const { Session } = await import("../../../src/session") + const session = await Session.create({}) + + const providerID = ProviderID.make("test") + const modelID = ModelID.make("test-model") + const assistantID = MessageID.ascending() + + const msg = await Session.updateMessage({ + id: assistantID, + sessionID: session.id, + role: "assistant" as const, + time: { created: Date.now() }, + parentID: MessageID.ascending(), + modelID, + providerID, + mode: "build", + agent: "build", + cost: 0, + path: { cwd: tmp.path, root: tmp.path }, + tokens: { input: 0, output: 0, reasoning: 0, cache: { read: 0, write: 0 } }, + }) + + const batch = await initTool(BatchTool) + const result = await batch.execute( + { + tool_calls: [{ tool: "invalid", parameters: { tool: "invalid", error: "sentinel-probe" } }], + }, + { + sessionID: session.id, + messageID: msg.id, + agent: "build", + abort: new AbortController().signal, + messages: [], + }, + ) + expect(result.output).toContain("All 1 tools executed successfully") + expect((result.metadata as any).details[0].success).toBe(true) + }, + }) +}) + +test("D5: BatchTool never calls Plugin.trigger for inner tool calls (structural bypass)", () => { + const src = readSrc("tool/batch.ts") + expect(src).not.toContain("Plugin.trigger(") + expect(src).toContain("await AppRuntime.runPromise(tool.execute(validatedParams, toEffectContext(ctx, partID)))") +}) + +// --------------------------------------------------------------------------- +// D6 — direct Task-tool dispatch inside SessionPrompt.loop (src/session/prompt.ts) +// +// PARTIAL COMPLIANCE NOTE (flagged for team-lead / S3, not silently assumed): +// the S2 spec (docs/internal/2026-07-18-defork-spike-spec.md:61) requires "a +// harmless sentinel tool invocation driven end-to-end through that route" for +// every `active` dispatcher, and states "An unproven `active` route fails the +// S2 gate." The spec text does NOT itself carve out a citation-based fallback +// — that is a pragmatic judgment call made here, not a quoted spec allowance. +// +// Rationale: real end-to-end execution of this exact chokepoint IS already +// proven by the existing, currently-passing test "failed subtask preserves +// metadata on error tool state" (test/session/prompt.test.ts, `it.instance` +// block at line 806), which drives a real subtask through prompt.loop() and +// asserts on the resulting tool-state metadata that only D6's taskCtx wiring +// produces. Reconstructing an equivalent from scratch here would require +// duplicating ~250 lines of module-private test harness from that file +// (`it.instance`, `useServerConfig`, `TestLLMServer`, `addSubtask` — none of +// which are exported) for a single additional route. Given that cost, this +// route is covered by citation of that real test plus a structural +// regression sentinel pinning the exact dispatch line numbers, so any +// refactor that moves the dispatch or renames the cited test breaks CI here +// too. If stricter literal compliance is required, the fix is either to +// export a shared harness from prompt.test.ts, or to accept this as a +// documented S2 exception — team-lead's call. +// --------------------------------------------------------------------------- + +test("D6: direct Task dispatch chokepoints are present and unique", () => { + const lines = readSrc("session/prompt.ts").split("\n") + // Content-based + arg-agnostic (matches `taskArgs` or S3's `finalTaskArgs`) — see D1's note. + expect(lines.filter((l) => l.includes("PermissionNext.merge(taskAgent.permission, session.permission ?? []),")).length).toBe(1) + expect(lines.filter((l) => l.includes("AppRuntime.runPromise(taskTool.execute(") && l.includes(", taskCtx)")).length).toBe(1) +}) + +test("D6: existing real-execution proof exists in test/session/prompt.test.ts", () => { + const promptTestSrc = readFileSync(join(SRC, "..", "test", "session", "prompt.test.ts"), "utf8") + expect(promptTestSrc).toContain('it.instance("failed subtask preserves metadata on error tool state"') +}) From 2884d7e7211195967d93ec0ba68e5c7377e92f5d Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Sun, 19 Jul 2026 07:36:00 -0700 Subject: [PATCH 2/4] =?UTF-8?q?fix:=20[de-fork=20S3]=20address=20review=20?= =?UTF-8?q?=E2=80=94=20audit=20sink=20can't=20break=20the=20"never=20throw?= =?UTF-8?q?s"=20contract?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `emitAudit` now wraps the external `auditSink(record)` call in try/catch: a throwing installed sink must never propagate out of `check()`, which is contractually total. The decision is already computed and returned regardless of audit outcome (kilo-code-bot + CodeRabbit, both flagged). - Replaced the O(n) `auditLog.shift()`-per-call with a batched splice (O(1) amortized) — `check()` runs on the tool-execution hot path (kilo-code-bot). - Latent-guard test now scans the entire production src tree for `SessionTools.resolve(` callers (matches the S2 PR's version so the shared file merges identically in any order). Co-Authored-By: Claude Fable 5 --- .../src/altimate/policy/hard-policy.ts | 16 ++++++- .../altimate/defork/route-sentinels.test.ts | 42 +++++++++---------- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/packages/opencode/src/altimate/policy/hard-policy.ts b/packages/opencode/src/altimate/policy/hard-policy.ts index a4b07abdb..09746f86e 100644 --- a/packages/opencode/src/altimate/policy/hard-policy.ts +++ b/packages/opencode/src/altimate/policy/hard-policy.ts @@ -215,8 +215,20 @@ export namespace HardPolicy { function emitAudit(record: AuditRecord): void { auditLog.push(record) - if (auditLog.length > MAX_AUDIT_LOG) auditLog.shift() - if (auditSink) auditSink(record) + // Batched trim (O(1) amortized) instead of an O(n) shift() on every call: + // let the log overshoot, then splice the oldest entries once it grows past + // twice the cap. check() runs on the tool-execution hot path. + if (auditLog.length > MAX_AUDIT_LOG * 2) auditLog.splice(0, auditLog.length - MAX_AUDIT_LOG) + // A faulty externally-installed sink must NEVER propagate out of the + // enforcement chokepoint — check() is contractually total (never throws). + // The decision is already computed and returned regardless of audit outcome. + if (auditSink) { + try { + auditSink(record) + } catch { + // Intentionally swallowed: audit-side failure cannot affect enforcement. + } + } } /** Test support: read the in-memory audit log (oldest first). */ diff --git a/packages/opencode/test/altimate/defork/route-sentinels.test.ts b/packages/opencode/test/altimate/defork/route-sentinels.test.ts index b502e266e..e339ff023 100644 --- a/packages/opencode/test/altimate/defork/route-sentinels.test.ts +++ b/packages/opencode/test/altimate/defork/route-sentinels.test.ts @@ -120,30 +120,26 @@ test("D1: resolveTools dispatch chokepoint is present and unique", () => { // --------------------------------------------------------------------------- test("D3/D4: SessionTools.resolve has no production caller (latent guard)", () => { - // If this ever finds a real call site outside src/session/tools.ts itself - // (not a doc-comment mentioning the name), D3/D4's classification must be - // revisited from `latent` to `active` and this test updated accordingly. - const promptSrc = readSrc("session/prompt.ts") - const toolSourceSrc = readSrc("altimate/tool-source.ts") - - // Known, expected occurrences: doc-comments only, never a call. - const promptMatches = [...promptSrc.matchAll(/SessionTools/g)] - expect(promptMatches).toHaveLength(2) - for (const match of promptMatches) { - const lineStart = promptSrc.lastIndexOf("\n", match.index) + 1 - const line = promptSrc.slice(lineStart, promptSrc.indexOf("\n", match.index)) - expect(line.trim().startsWith("//")).toBe(true) - expect(line).not.toContain("SessionTools.resolve(") - } - - const toolSourceMatches = [...toolSourceSrc.matchAll(/SessionTools/g)] - expect(toolSourceMatches).toHaveLength(2) - for (const match of toolSourceMatches) { - const lineStart = toolSourceSrc.lastIndexOf("\n", match.index) + 1 - const line = toolSourceSrc.slice(lineStart, toolSourceSrc.indexOf("\n", match.index)) - expect(line.trim().startsWith("*")).toBe(true) - expect(line).not.toContain("SessionTools.resolve(") + // Scan the ENTIRE production src tree (not just prompt.ts + tool-source.ts): + // a future caller added in ANY source file must flip D3/D4 from `latent` to + // `active` so HardPolicy coverage there is re-verified. We look for a real + // `SessionTools.resolve(` CALL on a non-comment line, excluding the resolver's + // own definition file (session/tools.ts) where the name is declared. + const glob = new Bun.Glob("**/*.ts") + const callers: string[] = [] + for (const rel of glob.scanSync({ cwd: SRC })) { + if (rel === join("session", "tools.ts")) continue // the definition itself + const src = readFileSync(join(SRC, rel), "utf8") + if (!src.includes("SessionTools.resolve(")) continue + src.split("\n").forEach((line, i) => { + const t = line.trim() + const isComment = t.startsWith("//") || t.startsWith("*") || t.startsWith("/*") + if (!isComment && line.includes("SessionTools.resolve(")) callers.push(`${rel}:${i + 1}`) + }) } + // If this fails, a production caller of SessionTools.resolve appeared — + // D3/D4 are no longer latent and HardPolicy must be confirmed to gate them. + expect(callers).toEqual([]) }) test("D3/D4: SessionTools.resolve dispatches a tool when invoked directly", async () => { From 3ac7b683246e1aed2c8d2d4666f572ffe8bfbe52 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Sun, 19 Jul 2026 07:59:03 -0700 Subject: [PATCH 3/4] fix: [de-fork S3] close MCP-prefix DDL bypass + hash audit args (review P1s) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two P1 review findings on the HardPolicy chokepoint: - MCP-prefix bypass (prompt.ts D2 / tools.ts D4 path): real MCP tools are registered with a flattened id `_` (see `MCP.tools()` in `src/mcp/index.ts`), but `RULES` are keyed by the bare governed id (`sql_execute`, `bash`). A warehouse server exposing its own `sql_execute` therefore arrived as `warehouse_sql_execute`, matched no rule, and was allowed to run `DROP DATABASE`. Add `resolveGovernedKey()`: for the `mcp` source ONLY, resolve a governed key that is a full `_`-delimited suffix of the flattened id. Native/plugin/batch/task ids keep strict exact-match, so this only ADDS coverage for MCP — it never un-governs an exact match. The audit record still retains the original flattened id for forensics. (The old D4 mock used the production-impossible bare key `bash`, hiding this.) - Audit arg retention: `finalArgsDigest` stored the first 2000 chars of the raw stable-JSON args. Tool args routinely carry secrets (`write` of `.env` contents, `bash` with an auth token) and the audit log is retained (up to MAX_AUDIT_LOG, readable via `getAuditLog()` / an installed sink), so truncation did not protect secrets near the start of the payload. Digest is now a SHA-256 hex over the stable-stringified args — non-reversible, no plaintext retained, correlation property preserved. Export `digestArgs()` so the D3 mutation tests recompute the expected digest instead of substring- matching the (now-hashed) value. Tests: +3 Section-A unit tests (MCP-flattened deny for sql_execute/bash; mcp-only scoping — native suffix + partial `mysql_execute` stay allowed; digest is 64-hex and contains no plaintext). D3 mutation/allow digest assertions rewritten to compare against `digestArgs(...)`. 22 pass, typecheck clean. Co-Authored-By: Claude Fable 5 --- .../src/altimate/policy/hard-policy.ts | 46 ++++++++-- .../test/altimate/defork/hardpolicy.test.ts | 91 +++++++++++++++++-- 2 files changed, 122 insertions(+), 15 deletions(-) diff --git a/packages/opencode/src/altimate/policy/hard-policy.ts b/packages/opencode/src/altimate/policy/hard-policy.ts index 09746f86e..049e9c6a9 100644 --- a/packages/opencode/src/altimate/policy/hard-policy.ts +++ b/packages/opencode/src/altimate/policy/hard-policy.ts @@ -21,6 +21,7 @@ // absent execute span does NOT prove enforcement. Tests must assert against the audit // log and execute-not-called counters. +import { createHash } from "node:crypto" import { Wildcard } from "@/util/wildcard" import { classifyAndCheck } from "@/altimate/tools/sql-classify" @@ -191,6 +192,23 @@ export namespace HardPolicy { const rulesByTool = buildRulesByTool(RULES) const initError = validateRuleTable(RULES) + // MCP tools are registered with a flattened id `_` + // (see src/mcp/index.ts's MCP.tools()), but RULES are keyed by the bare governed tool id + // (`sql_execute`, `bash`). Without this resolution an MCP server exposing its own + // `sql_execute` arrives as e.g. `warehouse_sql_execute`, matches no rule, and is allowed + // to run DDL — a silent bypass. For MCP-sourced calls, also resolve a governed key that is + // a full `_`-delimited suffix of the flattened id. Native/plugin/batch/task tools keep + // strict exact-match (their ids are not client-prefixed), so this only ever ADDS coverage + // for the mcp source; it never un-governs a tool that exact-match already caught. + function resolveGovernedKey(toolID: string, source: Source): string | undefined { + if (rulesByTool.has(toolID)) return toolID + if (source !== "mcp") return undefined + for (const governed of rulesByTool.keys()) { + if (toolID.endsWith(`_${governed}`)) return governed + } + return undefined + } + /** * Must be called at the app-runtime composition seam so a broken rule table fails app * startup instead of silently allowing everything. Throws (does not process.exit — @@ -247,13 +265,17 @@ export namespace HardPolicy { } // --------------------------------------------------------------------------------- - // Deterministic, bounded digest of the final args snapshot — for audit correlation in - // tests (e.g. "was the deny keyed off the post-hook-mutated args?"), not a - // cryptographic hash and not for security purposes on its own. + // Deterministic, non-reversible digest of the final args snapshot — for audit + // correlation in tests (e.g. "was the deny keyed off the post-hook-mutated args?"). + // It is a SHA-256 over the stable-stringified args, NOT the raw args: tool arguments + // routinely carry secrets (a `write` of `.env` contents, a `bash` command with an auth + // token). The audit log is retained (up to MAX_AUDIT_LOG records, readable via + // getAuditLog() or an installed sink), so storing raw or truncated args would retain + // those secrets near the start of the payload. The hash preserves the correlation + // property (same args -> same digest, different args -> different digest) without + // retaining the plaintext. Tests recompute the expected digest via exported digestArgs(). // --------------------------------------------------------------------------------- - const MAX_DIGEST_LENGTH = 2000 - function stableStringify(value: unknown): string { const seen = new WeakSet() function normalize(input: unknown): unknown { @@ -277,8 +299,16 @@ export namespace HardPolicy { } function finalArgsDigest(args: unknown): string { - const str = stableStringify(args) - return str.length > MAX_DIGEST_LENGTH ? str.slice(0, MAX_DIGEST_LENGTH) : str + return createHash("sha256").update(stableStringify(args)).digest("hex") + } + + /** + * Test/forensic support: the exact non-reversible digest check() records for a given + * args snapshot. Lets tests assert the audit digest reflects the post-hook (final) args + * HardPolicy actually evaluated, without exposing or reconstructing the plaintext. + */ + export function digestArgs(args: unknown): string { + return finalArgsDigest(args) } // --------------------------------------------------------------------------------- @@ -308,7 +338,7 @@ export namespace HardPolicy { assertInitialized() - const rules = rulesByTool.get(toolID) + const rules = rulesByTool.get(resolveGovernedKey(toolID, source) ?? "") if (!rules || rules.length === 0) { // Ungoverned toolID — HardPolicy has no rule for it, so it always allows, // regardless of args shape. Denying here would be a NEW block, not a relocation diff --git a/packages/opencode/test/altimate/defork/hardpolicy.test.ts b/packages/opencode/test/altimate/defork/hardpolicy.test.ts index 3aeb6ff95..c01f3b91f 100644 --- a/packages/opencode/test/altimate/defork/hardpolicy.test.ts +++ b/packages/opencode/test/altimate/defork/hardpolicy.test.ts @@ -133,6 +133,73 @@ test("A: ungoverned toolIDs are always allowed regardless of args shape", () => expect(HardPolicy.check({ toolID: "read", source: "native", args: "not even an object", sessionID: "s" }).allow).toBe(true) }) +test("A: MCP-flattened governed tool id is resolved — `_sql_execute` DDL is denied, not allowed via prefix bypass", () => { + // Real MCP tools are keyed `_` (src/mcp/index.ts), so a + // warehouse server exposing `sql_execute` arrives here as `warehouse_sql_execute`. Exact + // rule-key match would miss it and allow DDL; resolveGovernedKey closes that bypass. + const sql = HardPolicy.check({ + toolID: "warehouse_sql_execute", + source: "mcp", + args: { query: "DROP DATABASE prod" }, + sessionID: "s", + }) + expect(sql.allow).toBe(false) + if (!sql.allow) expect(sql.ruleID).toBe("sql_execute_ddl_v1") + + const bash = HardPolicy.check({ + toolID: "toolbox_bash", + source: "mcp", + args: { command: "DROP DATABASE prod" }, + sessionID: "s", + }) + expect(bash.allow).toBe(false) + if (!bash.allow) expect(bash.ruleID).toBe("bash_ddl_v1") + + // The audit record retains the ORIGINAL flattened id for forensics, even though the rule + // was resolved via the bare suffix. + expect(HardPolicy.getAuditLog().at(-1)?.toolID).toBe("toolbox_bash") +}) + +test("A: suffix resolution is scoped to the mcp source only — a non-mcp `_sql_execute`-suffixed id stays ungoverned (exact-match)", () => { + // Native/plugin/batch/task ids are never client-prefixed, so suffix resolution there would be + // an over-broad NEW block. A hypothetical native `warehouse_sql_execute` must fall through to + // exact-match (no rule) and allow — proving the fix only ADDS coverage for mcp, per its scope. + const nativeSuffix = HardPolicy.check({ + toolID: "warehouse_sql_execute", + source: "native", + args: { query: "DROP DATABASE prod" }, + sessionID: "s", + }) + expect(nativeSuffix.allow).toBe(true) + + // And a partial (non-`_`-delimited) suffix never matches even under mcp: `mysql_execute` + // ends with `sql_execute` textually but not as a `_sql_execute` segment, so it stays allowed. + const partial = HardPolicy.check({ + toolID: "mysql_execute", + source: "mcp", + args: { query: "DROP DATABASE prod" }, + sessionID: "s", + }) + expect(partial.allow).toBe(true) +}) + +test("A: audit finalArgsDigest is a non-reversible hash — no raw args plaintext retained", () => { + HardPolicy.clearAuditLog() + HardPolicy.check({ + toolID: "bash", + source: "native", + args: { command: "DROP DATABASE prod", secret: "sk-live-abc123" }, + sessionID: "s", + }) + const digest = HardPolicy.getAuditLog().at(-1)?.finalArgsDigest ?? "" + // 64-hex SHA-256, and it must NOT contain any plaintext fragment of the args. + expect(digest).toMatch(/^[0-9a-f]{64}$/) + expect(digest).not.toContain("DROP DATABASE prod") + expect(digest).not.toContain("sk-live-abc123") + // But it IS a faithful digest of the exact args snapshot (correlation preserved). + expect(digest).toBe(HardPolicy.digestArgs({ command: "DROP DATABASE prod", secret: "sk-live-abc123" })) +}) + test("A: malformed args for a governed toolID fail closed — total function, never throws", () => { expect(() => HardPolicy.check({ toolID: "sql_execute", source: "native", args: {}, sessionID: "s" })).not.toThrow() const missingQuery = HardPolicy.check({ toolID: "sql_execute", source: "native", args: {}, sessionID: "s" }) @@ -474,9 +541,14 @@ test("D3 (registry loop): tool.execute.before mutation is what HardPolicy inspec // The audit record's finalArgsDigest is the compliance oracle for "what HardPolicy actually // checked" — it must reflect the POST-hook (mutated, DDL) args, not the caller's pre-hook - // (benign) args. Catches a digest/decision divergence a decision-only assertion would miss. - expect(last?.finalArgsDigest).toContain("DROP DATABASE prod") - expect(last?.finalArgsDigest).not.toContain("ls -la") + // (benign) args. The digest is a non-reversible hash (no plaintext retained), so we recompute + // the expected digest from the known final/pre-hook args instead of substring-matching. + expect(last?.finalArgsDigest).toBe( + HardPolicy.digestArgs({ tool: "invalid", error: "sentinel-probe", command: "DROP DATABASE prod" }), + ) + expect(last?.finalArgsDigest).not.toBe( + HardPolicy.digestArgs({ tool: "invalid", error: "sentinel-probe", command: "ls -la" }), + ) }) test("D3 (registry loop): audit finalArgsDigest reflects post-hook args in the ALLOW direction too — mirror of the deny-direction mutation test above, proves the digest isn't computed from the caller's pre-hook args", async () => { @@ -555,10 +627,15 @@ test("D3 (registry loop): audit finalArgsDigest reflects post-hook args in the A const last = HardPolicy.getAuditLog().at(-1) expect(last?.decision.allow).toBe(true) - // The digest is the compliance oracle for "what HardPolicy actually checked" — it must show - // the post-hook benign command and must NOT show the caller's pre-hook DDL command. - expect(last?.finalArgsDigest).toContain("ls -la") - expect(last?.finalArgsDigest).not.toContain("DROP DATABASE prod") + // The digest is the compliance oracle for "what HardPolicy actually checked" — it must + // reflect the post-hook benign command, not the caller's pre-hook DDL command. Recompute + // from the known args (the digest is a non-reversible hash, not substring-inspectable). + expect(last?.finalArgsDigest).toBe( + HardPolicy.digestArgs({ tool: "invalid", error: "sentinel-probe", command: "ls -la" }), + ) + expect(last?.finalArgsDigest).not.toBe( + HardPolicy.digestArgs({ tool: "invalid", error: "sentinel-probe", command: "DROP DATABASE prod" }), + ) }) test("D4 (MCP loop): governed stub MCP tool is denied for DDL args, mcp execute never runs", async () => { From 348e930cf9e5ab5a7ff2dd54bcbb4c5d6b2c0fa8 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Sun, 19 Jul 2026 09:43:12 -0700 Subject: [PATCH 4/4] =?UTF-8?q?fix:=20[de-fork=20S3]=20address=20review=20?= =?UTF-8?q?round=203=20=E2=80=94=20bash=20whitespace=20bypass=20+=20audit/?= =?UTF-8?q?ctx=20fidelity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Human review (dev-punia-altimate) + bot findings on the HardPolicy chokepoint: - HIGH — bash DDL whitespace bypass: Wildcard.match compiles each pattern with a leading `^` anchor, so `" DROP DATABASE prod"` (or a leading tab/newline) slipped past `"DROP DATABASE *"` and defeated the hard deny. matchBashDdl now trims the command first (leading/trailing whitespace is never semantically meaningful here). +1 test over several whitespace-prefixed DDL forms. - MEDIUM — audit digest Map/Set collision: Object.keys(new Map()) is [], so distinct Maps/Sets (a tool.execute.before hook could inject one) both stringified to `{}` and collided to the same digest. stableStringify now serializes their contents under a distinguishing tag. +1 test. - MEDIUM — tool-call metadata recorded pre-hook args: `ctx` was built from the caller's original `args` before the tool.execute.before hook, so ctx.metadata()'s updateToolCall recorded the ORIGINAL args while HardPolicy and execute used the post-hook finalArgs. Both dispatch sites (native + MCP) now build `ctx` from finalArgs AFTER the hook, passing input.session.id / toolCallId directly to the hook input (both independent of args). UI/state now reflects what actually ran. - MEDIUM — tool.execute.after on hard deny: documented that skipping the after-hook on a policy deny is intentional (the tool never executed; synthesizing an after-event would hand plugins a result they'd treat as a real post-execute outcome). The security invariant holds regardless. 24 pass, typecheck clean. Markers clean (changes within altimate_change blocks). Co-Authored-By: Claude Fable 5 --- .../src/altimate/policy/hard-policy.ts | 15 +++++++++++- packages/opencode/src/session/tools.ts | 24 ++++++++++++++----- .../test/altimate/defork/hardpolicy.test.ts | 19 +++++++++++++++ 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/packages/opencode/src/altimate/policy/hard-policy.ts b/packages/opencode/src/altimate/policy/hard-policy.ts index 049e9c6a9..0054122ff 100644 --- a/packages/opencode/src/altimate/policy/hard-policy.ts +++ b/packages/opencode/src/altimate/policy/hard-policy.ts @@ -107,7 +107,11 @@ export namespace HardPolicy { if (!isRecord(args) || typeof args.command !== "string") { throw new Error("hard-policy: bash args missing string 'command' field") } - const command = args.command + // Trim first: Wildcard.match compiles each pattern with a leading `^` anchor, so a raw + // `" DROP DATABASE prod"` (or a leading tab/newline) would slip past `"DROP DATABASE *"` + // and bypass the hard deny. Leading/trailing whitespace is never semantically meaningful + // to the shell here, so trimming closes that bypass without altering any real command. + const command = args.command.trim() return BASH_DDL_PATTERNS.some((pattern) => Wildcard.match(command, pattern)) } @@ -285,6 +289,15 @@ export namespace HardPolicy { if (input === null || typeof input !== "object") return input if (seen.has(input)) return "[circular]" seen.add(input) + // Map/Set stringify to `{}` under Object.keys(), so distinct ones would collide to the + // same digest (a `tool.execute.before` hook could inject one). Serialize their contents + // under a distinguishing tag so different collections produce different digests. + if (input instanceof Map) { + return { __map__: [...input.entries()].map(([k, v]) => [normalize(k), normalize(v)]) } + } + if (input instanceof Set) { + return { __set__: [...input.values()].map(normalize) } + } if (Array.isArray(input)) return input.map(normalize) const record = input as Record const out: Record = {} diff --git a/packages/opencode/src/session/tools.ts b/packages/opencode/src/session/tools.ts index ba3df13bc..7bc74d82e 100644 --- a/packages/opencode/src/session/tools.ts +++ b/packages/opencode/src/session/tools.ts @@ -91,17 +91,20 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { execute(args, options) { return run.promise( Effect.gen(function* () { - const ctx = context(args, options) // altimate_change start — HardPolicy enforcement (S3) // upstream_fix: plugin.trigger's returned output was previously discarded, so a // tool.execute.before hook that mutated `args` had no effect on what actually - // ran — HardPolicy and execute must see the SAME final, post-hook args. + // ran — HardPolicy and execute must see the SAME final, post-hook args. The hook + // input uses input.session.id / options.toolCallId directly (both independent of + // args) so that ctx — and every ctx.metadata() tool-call record — is built from the + // post-hook finalArgs the tool actually ran with, not the caller's original args. const hookOutput = yield* plugin.trigger( "tool.execute.before", - { tool: item.id, sessionID: ctx.sessionID, callID: ctx.callID }, + { tool: item.id, sessionID: input.session.id, callID: options.toolCallId }, { args }, ) const finalArgs = hookOutput.args + const ctx = context(finalArgs, options) const policyDecision = HardPolicy.check({ toolID: item.id, source: "native", @@ -110,6 +113,10 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { callID: ctx.callID, }) if (!policyDecision.allow) { + // Intentionally short-circuit WITHOUT firing tool.execute.after: the tool never + // executed, so there is no result to post-process, and synthesizing an after-event + // from a hard deny would hand plugins something they'd treat as a real post-execute + // outcome. The security invariant (execute not called) holds either way. return { title: "Blocked by policy", output: policyDecision.safeReason, @@ -166,17 +173,20 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { item.execute = (args, opts) => run.promise( Effect.gen(function* () { - const ctx = context(args, opts) // altimate_change start — HardPolicy enforcement (S3) // upstream_fix: plugin.trigger's returned output was previously discarded, so a // tool.execute.before hook that mutated `args` had no effect on what actually - // ran — HardPolicy and execute must see the SAME final, post-hook args. + // ran — HardPolicy and execute must see the SAME final, post-hook args. The hook + // input uses input.session.id / opts.toolCallId directly (both independent of args) + // so that ctx — and every ctx.metadata() tool-call record — is built from the + // post-hook finalArgs the tool actually ran with, not the caller's original args. const hookOutput = yield* plugin.trigger( "tool.execute.before", - { tool: key, sessionID: ctx.sessionID, callID: opts.toolCallId }, + { tool: key, sessionID: input.session.id, callID: opts.toolCallId }, { args }, ) const finalArgs = hookOutput.args + const ctx = context(finalArgs, opts) const policyDecision = HardPolicy.check({ toolID: key, source: "mcp", @@ -185,6 +195,8 @@ export const resolve = Effect.fn("SessionTools.resolve")(function* (input: { callID: opts.toolCallId, }) if (!policyDecision.allow) { + // Intentionally short-circuit WITHOUT firing tool.execute.after (see the native + // dispatch above): the tool never executed, so there is no result to post-process. return { title: "Blocked by policy", metadata: { diff --git a/packages/opencode/test/altimate/defork/hardpolicy.test.ts b/packages/opencode/test/altimate/defork/hardpolicy.test.ts index c01f3b91f..c0b7e1482 100644 --- a/packages/opencode/test/altimate/defork/hardpolicy.test.ts +++ b/packages/opencode/test/altimate/defork/hardpolicy.test.ts @@ -127,6 +127,25 @@ test("A: near-miss controls are allowed — proves the rule table is not trivial expect(HardPolicy.check({ toolID: "bash", source: "native", args: { command: "ls -la" }, sessionID: "s" }).allow).toBe(true) }) +test("A: bash DDL is denied even with leading/trailing whitespace — no `^`-anchor bypass", () => { + // Wildcard.match anchors patterns at `^`, so an untrimmed " DROP DATABASE prod" would slip + // past "DROP DATABASE *". matchBashDdl trims first; verify the common whitespace prefixes. + for (const command of [" DROP DATABASE prod", "\tDROP DATABASE prod", "\n TRUNCATE users", "DROP SCHEMA x "]) { + const d = HardPolicy.check({ toolID: "bash", source: "native", args: { command }, sessionID: "s" }) + expect(d.allow).toBe(false) + if (!d.allow) expect(d.ruleID).toBe("bash_ddl_v1") + } +}) + +test("A: digest distinguishes Map/Set args instead of collapsing them to {} ", () => { + // Object.keys(new Map()) is [] -> different Maps would otherwise both stringify to {} and + // collide to the same audit digest. Serializing their contents keeps them distinct. + expect(HardPolicy.digestArgs({ x: new Map([["a", 1]]) })).not.toBe(HardPolicy.digestArgs({ x: new Map([["a", 2]]) })) + expect(HardPolicy.digestArgs({ x: new Set([1, 2]) })).not.toBe(HardPolicy.digestArgs({ x: new Set([1, 3]) })) + // A populated Map must not be confused with an empty plain object. + expect(HardPolicy.digestArgs({ x: new Map([["a", 1]]) })).not.toBe(HardPolicy.digestArgs({ x: {} })) +}) + test("A: ungoverned toolIDs are always allowed regardless of args shape", () => { expect(HardPolicy.check({ toolID: "read", source: "native", args: { anything: true }, sessionID: "s" }).allow).toBe(true) expect(HardPolicy.check({ toolID: "read", source: "native", args: undefined, sessionID: "s" }).allow).toBe(true)