fix(exec_policy): resolve type mismatch and approval gate ordering for per-agent exec_policy override#222
Open
capt-marbles wants to merge 1 commit intoRightNow-AI:mainfrom
Conversation
…nt exec_policy override Fixes RightNow-AI#182. Two bugs caused `exec_policy = "allow"` in agent manifests to be silently ignored, leaving every shell_exec call blocked by the approval gate: **Bug 1 — serde type mismatch (agent.rs / serde_compat.rs)** `AgentManifest::exec_policy` was typed as `Option<ExecPolicy>` (a struct), but manifest authors write a bare string like `exec_policy = "allow"`. Serde could not deserialize a string into ExecPolicy, silently fell back to `None` via `#[serde(default)]`, and the kernel then inherited the global Allowlist policy. Fix: add `exec_policy_override` deserializer to `serde_compat` that accepts either a bare `ExecSecurityMode` string or a full `ExecPolicy` struct, and wire it onto the `exec_policy` field with `deserialize_with`. A bare string is expanded into an `ExecPolicy` with that mode and all other fields at their defaults, preserving backwards compatibility with existing DB blobs (absent field → None → global fallback, same as before). **Bug 2 — approval gate fires before exec_mode check (tool_runner.rs)** Even if the field were correctly parsed as `Full`, the approval gate in `execute_tool` fired unconditionally before any exec_policy inspection. Scheduled / headless runs have no human in the loop; the gate always timed out and blocked execution. Fix: compute `is_full_exec` from the effective exec_policy before the gate and skip it entirely when the agent is running in Full mode. The allowlist check and taint check already had this guard (`is_full_exec` was computed further down); this change moves the gate bypass to the correct location. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #182.
Root Cause
Two independent bugs caused
exec_policy = "allow"in agent manifests to be silently ignored, leaving everyshell_execcall blocked by the human approval gate even when an agent explicitly opts into full exec mode.Bug 1 — serde type mismatch (
agent.rs)AgentManifest::exec_policywas typed asOption<ExecPolicy>(a struct), but manifest authors write a bare string:Serde cannot deserialize a string into
ExecPolicy, so#[serde(default)]silently returnsNone. The kernel then inherits the globalAllowlistpolicy. This is confirmed in the kernel log from v0.3.0:ExecSecurityModealready has the correct aliases ("allow"→Full,"restricted"→Allowlist,"deny"→Deny), but the field type is wrong so they are never reached.Bug 2 — approval gate fires before exec_mode check (
tool_runner.rs)Even if the field parsed correctly, the approval gate in
execute_toolfired unconditionally before any exec_policy inspection. For scheduled / headless agents there is no human in the loop, so the gate always times out and denies the tool call. Theis_full_execguard already existed further down in the function (for the taint check); it was just missing at the gate.Fix
crates/openfang-types/src/serde_compat.rs— addsexec_policy_overridedeserializer that accepts either a bareExecSecurityModestring or a fullExecPolicystruct. A bare string is expanded into anExecPolicywith that mode and all other fields at defaults. Absent / null fields returnNone, preserving backward compatibility with existing DB blobs.crates/openfang-types/src/agent.rs— wires the new deserializer ontoAgentManifest::exec_policyviadeserialize_with.crates/openfang-runtime/src/tool_runner.rs— computesis_full_execbefore the approval gate and skips the gate when the agent is in Full exec mode.Tests
Six new tests in
serde_compat::tests:exec_policy_override_string_allow"allow"→ExecSecurityMode::Fullexec_policy_override_string_deny"deny"→ExecSecurityMode::Denyexec_policy_override_string_restricted"restricted"→ExecSecurityMode::Allowlistexec_policy_override_absent_gives_noneNoneexec_policy_override_full_struct_roundtripexec_policy_override_msgpack_none_roundtripNone(no regression)All 279 existing tests pass. Build is clean.
Verification
Tested against a live v0.3.0 daemon before this PR. After applying the patch and spawning an agent with
exec_policy = "allow"+tools = ["shell_exec"], the kernel log shows:And
shell_execruns without hitting the approval gate.