Skip to content

feat: [de-fork S3] HardPolicy kill gate — non-bypassable DDL denies at every dispatcher #1018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anandgupta42 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: [de-fork S3] HardPolicy kill gate — non-bypassable DDL denies at every dispatcher #1018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
412ed1d
PINEAPPLE
anandgupta42 Jul 19, 2026
2884d7e
fix: [de-fork S3] address review — audit sink can't break the "never …
anandgupta42 Jul 19, 2026
3ac7b68
fix: [de-fork S3] close MCP-prefix DDL bypass + hash audit args (revi…
anandgupta42 Jul 19, 2026
348e930
fix: [de-fork S3] address review round 3 — bash whitespace bypass + a…
anandgupta42 Jul 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
392 changes: 392 additions & 0 deletions packages/opencode/src/altimate/policy/hard-policy.ts
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions packages/opencode/src/effect/app-runtime.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
212 changes: 150 additions & 62 deletions packages/opencode/src/session/prompt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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<typeof PermissionNext.ask>[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,

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Record the task call ID rather than the part ID

On the direct-subtask path, the tool part is created with distinct id and callID values, and the execution context below correctly uses part.callID; recording part.id here therefore makes every task audit record impossible to correlate with the corresponding tool call by call ID. Use part.callID, as the other dispatchers do, so the audit trail retains its stated correlation semantics.

Useful? React with 👍 / 👎.

})
// 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<typeof PermissionNext.ask>[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)
Expand Down Expand Up @@ -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,
Expand All @@ -1597,8 +1630,30 @@ export namespace SessionPrompt {
args,
},
)
const finalArgs = hookInput.args
const policyDecision = HardPolicy.check({
toolID: item.id,
source: "native",
args: finalArgs,
Comment on lines +1634 to +1637

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Record plugin registry calls with the plugin source

When item.registrySource === "external", which ToolRegistry.fromPlugin() assigns to user and third-party plugin tools, this records the call as native; the same hard-coded value also appears in session/tools.ts. Consequently the declared plugin source is never emitted and the security audit cannot distinguish plugin execution from built-ins, so derive the policy source from item.registrySource in both registry loops.

Useful? React with 👍 / 👎.

sessionID: ctx.sessionID,
callID: ctx.callID,
})
if (!policyDecision.allow) {
return {
title: "Blocked by policy",
Comment on lines +1641 to +1643

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Surface policy denials as tool errors

When a built-in bash or sql_execute call is denied, returning this ordinary tool value produces an AI SDK tool-result, which session/processor.ts:258-275 unconditionally stores with status: "completed"; only a rejected execution produces the tool-error branch and an error state. Consequently the UI, transcript, and status-based consumers treat a blocked destructive call as successfully completed despite the metadata, unlike the pre-existing SQL and permission denials that threw; reject with the safe policy error instead.

Useful? React with 👍 / 👎.

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))

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist the post-hook arguments that are actually executed

When a tool.execute.before hook replaces output.args with a new object, this now executes finalArgs, but the tool context was created earlier from the original args and its state update still records input: args (context() at lines 1562-1584); the processor likewise began the tool part with the model-supplied input. The session transcript and UI can therefore show benign/original arguments while the tool and HardPolicy audit use different replacement arguments, so update the stored tool input to the post-hook value before dispatch.

Useful? React with 👍 / 👎.

// altimate_change end
const output = {
...result,
Expand All @@ -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,
)
Expand All @@ -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,
Expand All @@ -1652,6 +1713,29 @@ export namespace SessionPrompt {
args,
},
)
const finalArgs = hookInput.args

const policyDecision = HardPolicy.check({
toolID: key,
source: "mcp",
args: finalArgs,
Comment on lines +1718 to +1721

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Check the unprefixed MCP tool name

For every real MCP server, MCP.tools() constructs keys as <sanitized-client>_<sanitized-tool> in src/mcp/index.ts:1032-1035, but the policy rules only govern the exact IDs bash and sql_execute. Passing the flattened key here therefore means a server such as warehouse exposing sql_execute is checked as warehouse_sql_execute, receives an allow decision, and executes destructive DDL; use the original MCP tool name or remove the known client prefix before matching. The D4 test misses this because its mock supplies the production-impossible bare key bash.

Useful? React with 👍 / 👎.

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,
},
}
Comment on lines +1726 to +1736

@coderabbitai coderabbitai Bot Jul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm MCP tool consumers read `.content` off the execute() result in prompt.ts
rg -nP -C3 '\.content\b' packages/opencode/src/session/prompt.ts

Repository: AltimateAI/altimate-code

Length of output: 1149

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- prompt.ts around deny branch ---'
sed -n '1718,1745p' packages/opencode/src/session/prompt.ts

echo
echo '--- tools.ts relevant section ---'
sed -n '170,215p' packages/opencode/src/session/tools.ts

Repository: AltimateAI/altimate-code

Length of output: 3301

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- prompt.ts execute signature / return shape ---'
rg -n -C4 'return \{|type .*execute|execute\(' packages/opencode/src/session/prompt.ts

echo
echo '--- prompt.ts direct .content / .attachments consumers around execute result ---'
rg -n -C3 'result\.(content|attachments)|attachments:|content:' packages/opencode/src/session/prompt.ts

echo
echo '--- tool result type references in session files ---'
rg -n -C2 'content: \[\]|attachments: \[\]|output: .*safeReason|hard_policy_denied' packages/opencode/src/session

Repository: AltimateAI/altimate-code

Length of output: 14361

Add empty content and attachments on MCP policy deny
packages/opencode/src/session/prompt.ts:1726-1736 should match session/tools.ts and return content: [] and attachments: [] here; otherwise callers reading result.content can get undefined.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/src/session/prompt.ts` around lines 1726 - 1736, Update the
policy-deny return object in the prompt handling flow to include empty content
and attachments arrays, matching the result shape used by session tools.
Preserve the existing title, output, metadata, and denial values.

}
// 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
Expand All @@ -1666,15 +1750,19 @@ 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",
{
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,
)
Expand Down
Loading
Loading