dgf1988/codewhale
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

feat(execpolicy): wire permissions.toml ask-rules into runtime

Browse Source 
Harvested from PR #2885 by @greyfreedom. Wires ask-rules into the
app-server and core ExecPolicyEngine (previously inert). Removes the
original PR's NeedsApproval arm that incorrectly allow-listed the
working directory as a network host.

Co-Authored-By: greyfreedom <11493871+greyfreedom@users.noreply.github.com>
This commit is contained in:
greyfreedom
2026-06-07 20:07:46 +08:00
committed by Hunter B
parent 4e3184eae9
commit 17dbed13c7
6 changed files with 365 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files
config.example.toml
+3 -3
View File
@@ -167,9 +167,9 @@ sandbox_mode = "workspace-write" # read-only | workspace-write | danger-full-acc
# prompt_suggestion = true # opt-in: show ghost-text follow-up question in composer after each turn
# Typed permission rules live in a sibling `permissions.toml` file, not in
# config.toml. This schema slice is ask-only and is parsed for follow-up
# approval-flow wiring; allow/deny records and UI persistence are intentionally
# out of scope here.
# config.toml. This shape is ask-only and feeds the execution policy engine;
# allow/deny records, glob expansion, and UI persistence are intentionally out
# of scope here.
#
# Example ~/.codewhale/permissions.toml:
#
crates/app-server/src/lib.rs
+43 -3
View File
@@ -12,7 +12,6 @@ use axum::{Json, Router};
use codewhale_agent::ModelRegistry;
use codewhale_config::{CliRuntimeOverrides, ConfigStore};
use codewhale_core::Runtime;
use codewhale_execpolicy::ExecPolicyEngine;
use codewhale_hooks::{HookDispatcher, JsonlHookSink, StdoutHookSink, UnixSocketHookSink};
use codewhale_mcp::McpManager;
use codewhale_protocol::{
@@ -319,6 +318,7 @@ async fn app_handler(
fn build_state(config_path: Option<PathBuf>, auth_token: Option<String>) -> Result<AppState> {
let store = ConfigStore::load(config_path.clone())?;
let config = store.config.clone();
let exec_policy = store.exec_policy_engine();
let registry = ModelRegistry::default();
let state_db_path = config_path
@@ -349,7 +349,7 @@ fn build_state(config_path: Option<PathBuf>, auth_token: Option<String>) -> Resu
state_store,
Arc::new(ToolRegistry::default()),
Arc::new(McpManager::default()),
ExecPolicyEngine::new(Vec::new(), Vec::new()),
exec_policy,
hooks,
);
@@ -1057,6 +1057,43 @@ mod tests {
);
}
#[tokio::test]
async fn build_state_loads_permissions_into_runtime_policy() {
let tmp = tempfile::tempdir().expect("tempdir");
let config_path = tmp.path().join("config.toml");
fs::write(&config_path, "api_key = \"sk-deepseek-secret\"\n").expect("write config");
fs::write(
tmp.path().join("permissions.toml"),
r#"
[[rules]]
tool = "exec_shell"
command = "cargo test"
"#,
)
.expect("write permissions");
let state = build_state(Some(config_path), None).expect("state");
let runtime = state.runtime.lock().await;
let decision = runtime
.exec_policy
.check(codewhale_execpolicy::ExecPolicyContext {
command: "cargo test --workspace",
cwd: "/workspace",
tool: Some("exec_shell"),
path: None,
ask_for_approval: codewhale_execpolicy::AskForApproval::UnlessTrusted,
sandbox_mode: Some("workspace-write"),
})
.expect("policy check");
assert!(decision.allow);
assert!(decision.requires_approval);
assert_eq!(
decision.matched_rule.as_deref(),
Some("tool=exec_shell command=cargo test")
);
}
#[test]
fn non_loopback_bind_without_auth_fails_fast() {
let options = AppServerOptions {
@@ -1076,7 +1113,10 @@ mod tests {
#[tokio::test]
async fn stdio_transport_keeps_raw_config_get_for_legacy_clients() {
let state = build_state(None, None).expect("state");
let tmp = tempfile::tempdir().expect("tempdir");
let config_path = tmp.path().join("config.toml");
fs::write(&config_path, "").expect("write config");
let state = build_state(Some(config_path), None).expect("state");
{
let mut cfg = state.config.write().await;
cfg.api_key = Some("sk-deepseek-secret".to_string());
crates/config/src/lib.rs
+63
View File
@@ -10,6 +10,7 @@ use std::sync::OnceLock;
use anyhow::{Context, Result, bail};
pub use codewhale_execpolicy::ToolAskRule;
use codewhale_execpolicy::{ExecPolicyEngine, Ruleset};
use codewhale_secrets::SecretSource;
pub use codewhale_secrets::Secrets;
use serde::{Deserialize, Serialize};
@@ -295,6 +296,11 @@ impl PermissionsToml {
pub fn is_empty(&self) -> bool {
self.rules.is_empty()
}
#[must_use]
pub fn ruleset(&self) -> Ruleset {
Ruleset::user(Vec::new(), Vec::new()).with_ask_rules(self.rules.clone())
}
}
impl ProvidersToml {
@@ -2771,6 +2777,15 @@ impl ConfigStore {
pub fn permissions_path(&self) -> PathBuf {
permissions_path_for_config_path(&self.path)
}
#[must_use]
pub fn exec_policy_engine(&self) -> ExecPolicyEngine {
if self.permissions.is_empty() {
ExecPolicyEngine::new(Vec::new(), Vec::new())
} else {
ExecPolicyEngine::with_rulesets(vec![self.permissions.ruleset()])
}
}
}
/// Process-wide default [`Secrets`] façade. The first caller wins; the
@@ -3614,6 +3629,54 @@ action = "mode.agent"
let _ = fs::remove_dir_all(dir);
}
#[test]
fn config_store_exec_policy_engine_uses_sibling_permissions() {
use std::time::{SystemTime, UNIX_EPOCH};
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.expect("clock")
.as_nanos();
let dir = std::env::temp_dir().join(format!(
"codewhale-permissions-engine-{}-{unique}",
std::process::id()
));
fs::create_dir_all(&dir).expect("mkdir");
let config_path = dir.join(CONFIG_FILE_NAME);
fs::write(&config_path, "model = \"deepseek-v4-flash\"\n").expect("write config");
fs::write(
dir.join(PERMISSIONS_FILE_NAME),
r#"
[[rules]]
tool = "exec_shell"
command = "cargo test"
"#,
)
.expect("write permissions");
let store = ConfigStore::load(Some(config_path)).expect("load config store");
let decision = store
.exec_policy_engine()
.check(codewhale_execpolicy::ExecPolicyContext {
command: "cargo test --workspace",
cwd: "/workspace",
tool: Some("exec_shell"),
path: None,
ask_for_approval: codewhale_execpolicy::AskForApproval::UnlessTrusted,
sandbox_mode: Some("workspace-write"),
})
.expect("policy check");
assert!(decision.allow);
assert!(decision.requires_approval);
assert_eq!(
decision.matched_rule.as_deref(),
Some("tool=exec_shell command=cargo test")
);
let _ = fs::remove_dir_all(dir);
}
struct EnvGuard {
deepseek_api_key: Option<OsString>,
deepseek_base_url: Option<OsString>,
crates/core/src/lib.rs
+76 -1
View File
@@ -1097,11 +1097,12 @@ impl Runtime {
ToolPayload::LocalShell { .. } => "exec_shell",
_ => call.name.as_str(),
};
let policy_path = permission_path_for_call(&call);
let decision = self.exec_policy.check(ExecPolicyContext {
command: &command,
cwd: &policy_cwd,
tool: Some(policy_tool),
path: None,
path: policy_path.as_deref(),
ask_for_approval: approval_mode,
sandbox_mode: None,
})?;
@@ -1502,6 +1503,24 @@ fn preview_from_initial_history(initial_history: &InitialHistory) -> String {
}
}
fn permission_path_for_call(call: &ToolCall) -> Option<String> {
match &call.payload {
ToolPayload::Function { arguments } => serde_json::from_str::<Value>(arguments)
.ok()
.and_then(|value| {
value
.get("path")
.and_then(Value::as_str)
.map(str::to_string)
}),
ToolPayload::Mcp { raw_arguments, .. } => raw_arguments
.get("path")
.and_then(Value::as_str)
.map(str::to_string),
ToolPayload::Custom { .. } | ToolPayload::LocalShell { .. } => None,
}
}
fn truncate_preview(value: &str) -> String {
value.chars().take(120).collect()
}
@@ -1808,9 +1827,65 @@ fn job_state_status_to_runtime(status: JobStateStatus) -> JobStatus {
#[cfg(test)]
mod tests {
use super::*;
use codewhale_tools::ToolCallSource;
// ── JobManager: lifecycle ──────────────────────────────────────────
#[test]
fn permission_path_for_call_extracts_function_path_argument() {
let call = ToolCall {
name: "read_file".to_string(),
payload: ToolPayload::Function {
arguments: json!({ "path": "README.md" }).to_string(),
},
source: ToolCallSource::Direct,
raw_tool_call_id: None,
};
assert_eq!(
permission_path_for_call(&call).as_deref(),
Some("README.md")
);
}
#[test]
fn permission_path_for_call_extracts_mcp_path_argument() {
let call = ToolCall {
name: "mcp_fs_read".to_string(),
payload: ToolPayload::Mcp {
server: "fs".to_string(),
tool: "read".to_string(),
raw_arguments: json!({ "path": "secrets/token.txt" }),
raw_tool_call_id: None,
},
source: ToolCallSource::Direct,
raw_tool_call_id: None,
};
assert_eq!(
permission_path_for_call(&call).as_deref(),
Some("secrets/token.txt")
);
}
#[test]
fn permission_path_for_call_ignores_shell_payload() {
let call = ToolCall {
name: "exec_shell".to_string(),
payload: ToolPayload::LocalShell {
params: codewhale_protocol::LocalShellParams {
command: "cargo test".to_string(),
cwd: None,
timeout_ms: None,
},
},
source: ToolCallSource::Direct,
raw_tool_call_id: None,
};
assert_eq!(permission_path_for_call(&call), None);
}
#[test]
fn enqueue_creates_queued_job_with_zero_progress() {
let mut jm = JobManager::default();
crates/execpolicy/src/lib.rs
+176 -62
View File
@@ -313,21 +313,26 @@ impl ExecPolicyEngine {
self.rulesets
.iter()
.flat_map(|ruleset| ruleset.ask_rules.iter())
.filter(|rule| rule.tool == tool)
.filter(|rule| match rule.command.as_deref() {
.flat_map(|ruleset| {
ruleset
.ask_rules
.iter()
.map(move |rule| (ruleset.layer, rule))
})
.filter(|(_, rule)| rule.tool == tool)
.filter(|(_, rule)| match rule.command.as_deref() {
Some(command) => self.arity_dict.allow_rule_matches(command, ctx.command),
None => true,
})
.filter(|rule| match (rule.path.as_deref(), ctx.path) {
.filter(|(_, rule)| match (rule.path.as_deref(), ctx.path) {
(Some(pattern), Some(path)) => {
normalize_path_value(pattern) == normalize_path_value(path)
}
(Some(_), None) => false,
(None, _) => true,
})
.max_by_key(|rule| ask_rule_specificity(rule))
.cloned()
.max_by_key(|(layer, rule)| (*layer, ask_rule_specificity(rule)))
.map(|(_, rule)| rule.clone())
}
/// Records an approval key for the current session so subsequent checks skip approval.
@@ -377,51 +382,82 @@ impl ExecPolicyEngine {
let ask_rule = self.matching_ask_rule(&ctx);
let requirement = match &ctx.ask_for_approval {
AskForApproval::Never => {
if let Some(rule) = &ask_rule {
ExecApprovalRequirement::Forbidden {
reason: format!(
"Typed ask rule '{}' requires approval, but approval policy is never.",
rule.label()
),
}
} else {
ExecApprovalRequirement::Skip {
bypass_sandbox: false,
proposed_execpolicy_amendment: None,
let mut matched_ask_rule = None;
// Resolve a matching typed ask-rule first. Ask-rules take precedence over
// mode-based handling for everything except `Never` (which forbids,
// because no prompt can be shown) and `Reject { rules: true }` (which
// explicitly rejects rule-exceptions). This ordering is checked against
// the experimental `if let` match-guard the original PR used; it is
// reproduced here with plain control flow for edition-2024 stable.
let ask_rule_requirement = match &ctx.ask_for_approval {
AskForApproval::Never | AskForApproval::Reject { rules: true, .. } => None,
_ => ask_rule.as_ref().map(|rule| {
matched_ask_rule = Some(rule.label());
ExecApprovalRequirement::NeedsApproval {
reason: format!("Typed ask rule '{}' requires approval.", rule.label()),
proposed_execpolicy_amendment: None,
// A typed ask-rule approval (exec/fn/MCP) must not touch
// network policy. The original PR allow-listed `ctx.cwd` as a
// network host here, which is incorrect and security-relevant:
// approving e.g. an exec rule should never create a network
// allow-entry. Emit no network amendments for ask-rule prompts.
proposed_network_policy_amendments: Vec::new(),
}
}),
};
let requirement = if let Some(req) = ask_rule_requirement {
req
} else {
match &ctx.ask_for_approval {
AskForApproval::Never => {
if let Some(rule) = &ask_rule {
matched_ask_rule = Some(rule.label());
ExecApprovalRequirement::Forbidden {
reason: format!(
"Typed ask rule '{}' requires approval, but approval policy is never.",
rule.label()
),
}
} else {
ExecApprovalRequirement::Skip {
bypass_sandbox: false,
proposed_execpolicy_amendment: None,
}
}
}
AskForApproval::Reject { rules, .. } if *rules => {
ExecApprovalRequirement::Forbidden {
reason: "Policy is configured to reject rule-exceptions.".to_string(),
}
}
AskForApproval::UnlessTrusted if is_trusted => ExecApprovalRequirement::Skip {
bypass_sandbox: false,
proposed_execpolicy_amendment: None,
},
AskForApproval::OnFailure => ExecApprovalRequirement::Skip {
bypass_sandbox: false,
proposed_execpolicy_amendment: None,
},
_ => ExecApprovalRequirement::NeedsApproval {
reason: if is_trusted {
"Approval requested by policy mode.".to_string()
} else {
"Unmatched command prefix requires approval.".to_string()
},
proposed_execpolicy_amendment: if is_trusted {
None
} else {
Some(ExecPolicyAmendment {
prefixes: vec![first_token(ctx.command)],
})
},
proposed_network_policy_amendments: vec![NetworkPolicyAmendment {
host: ctx.cwd.to_string(),
action: NetworkPolicyRuleAction::Allow,
}],
},
}
AskForApproval::UnlessTrusted if is_trusted => ExecApprovalRequirement::Skip {
bypass_sandbox: false,
proposed_execpolicy_amendment: None,
},
AskForApproval::OnFailure => ExecApprovalRequirement::Skip {
bypass_sandbox: false,
proposed_execpolicy_amendment: None,
},
AskForApproval::Reject { rules, .. } if *rules => ExecApprovalRequirement::Forbidden {
reason: "Policy is configured to reject rule-exceptions.".to_string(),
},
_ => ExecApprovalRequirement::NeedsApproval {
reason: if is_trusted {
"Approval requested by policy mode.".to_string()
} else {
"Unmatched command prefix requires approval.".to_string()
},
proposed_execpolicy_amendment: if is_trusted {
None
} else {
Some(ExecPolicyAmendment {
prefixes: vec![first_token(ctx.command)],
})
},
proposed_network_policy_amendments: vec![NetworkPolicyAmendment {
host: ctx.cwd.to_string(),
action: NetworkPolicyRuleAction::Allow,
}],
},
};
let (allow, requires_approval) = match requirement {
@@ -430,12 +466,6 @@ impl ExecPolicyEngine {
ExecApprovalRequirement::Forbidden { .. } => (false, false),
};
let matched_ask_rule = if matches!(&ctx.ask_for_approval, AskForApproval::Never) {
ask_rule.map(|rule| rule.label())
} else {
None
};
Ok(ExecPolicyDecision {
allow,
requires_approval,
@@ -639,7 +669,7 @@ mod tests {
}
#[test]
fn typed_ask_rule_is_ignored_outside_never_mode_for_now() {
fn typed_ask_rule_requires_approval_under_unless_trusted() {
let engine = ExecPolicyEngine::with_rulesets(vec![
Ruleset::user(vec![], vec![])
.with_ask_rules(vec![ToolAskRule::exec_shell("cargo test")]),
@@ -651,18 +681,49 @@ mod tests {
assert!(decision.allow);
assert!(decision.requires_approval);
assert_eq!(decision.matched_rule, None);
assert_eq!(
decision.matched_rule.as_deref(),
Some("tool=exec_shell command=cargo test")
);
match decision.requirement {
ExecApprovalRequirement::NeedsApproval {
proposed_execpolicy_amendment: Some(amendment),
proposed_execpolicy_amendment,
proposed_network_policy_amendments,
..
} => assert_eq!(amendment.prefixes, vec!["cargo"]),
other => panic!("expected unchanged approval behavior, got {other:?}"),
} => {
assert_eq!(proposed_execpolicy_amendment, None);
// A typed ask-rule approval must not allow-list the cwd (or
// anything else) as a network host. See the NeedsApproval arm.
assert!(
proposed_network_policy_amendments.is_empty(),
"ask-rule approval must not propose network amendments, got {proposed_network_policy_amendments:?}"
);
}
other => panic!("expected typed ask approval, got {other:?}"),
}
}
#[test]
fn typed_ask_rule_does_not_change_allow_deny_precedence() {
fn typed_ask_rule_requires_approval_under_on_failure() {
let engine = ExecPolicyEngine::with_rulesets(vec![
Ruleset::user(vec![], vec![])
.with_ask_rules(vec![ToolAskRule::exec_shell("cargo test")]),
]);
let decision = engine
.check(ctx("cargo test --workspace", AskForApproval::OnFailure))
.unwrap();
assert!(decision.allow);
assert!(decision.requires_approval);
assert_eq!(
decision.reason(),
"Typed ask rule 'tool=exec_shell command=cargo test' requires approval."
);
}
#[test]
fn typed_ask_rule_overrides_trusted_but_not_deny() {
let engine = ExecPolicyEngine::with_rulesets(vec![
Ruleset::user(
vec!["cargo test".to_string()],
@@ -675,8 +736,11 @@ mod tests {
.check(ctx("cargo test --workspace", AskForApproval::UnlessTrusted))
.unwrap();
assert!(trusted.allow);
assert!(!trusted.requires_approval);
assert_eq!(trusted.matched_rule.as_deref(), Some("cargo test"));
assert!(trusted.requires_approval);
assert_eq!(
trusted.matched_rule.as_deref(),
Some("tool=exec_shell command=cargo test")
);
let denied = engine
.check(ctx("cargo test --danger", AskForApproval::Never))
@@ -690,6 +754,56 @@ mod tests {
);
}
#[test]
fn typed_ask_rule_prefers_higher_layer_before_specificity() {
let engine = ExecPolicyEngine::with_rulesets(vec![
Ruleset::agent(vec![], vec![])
.with_ask_rules(vec![ToolAskRule::exec_shell("cargo test --workspace")]),
Ruleset::user(vec![], vec![])
.with_ask_rules(vec![ToolAskRule::exec_shell("cargo test")]),
]);
let decision = engine
.check(ctx(
"cargo test --workspace --all-features",
AskForApproval::UnlessTrusted,
))
.unwrap();
assert!(decision.requires_approval);
assert_eq!(
decision.matched_rule.as_deref(),
Some("tool=exec_shell command=cargo test")
);
}
#[test]
fn reject_rules_mode_still_forbids_matching_ask_rule() {
let engine = ExecPolicyEngine::with_rulesets(vec![
Ruleset::user(vec![], vec![])
.with_ask_rules(vec![ToolAskRule::exec_shell("cargo test")]),
]);
let decision = engine
.check(ctx(
"cargo test --workspace",
AskForApproval::Reject {
sandbox_approval: false,
rules: true,
mcp_elicitations: false,
},
))
.unwrap();
assert!(!decision.allow);
assert!(!decision.requires_approval);
assert_eq!(decision.matched_rule, None);
assert_eq!(
decision.reason(),
"Policy is configured to reject rule-exceptions."
);
}
#[test]
fn typed_ask_rule_label_wins_when_never_blocks_trusted_command() {
let engine = ExecPolicyEngine::with_rulesets(vec![
docs/CONFIGURATION.md
+4 -2
View File
@@ -875,8 +875,10 @@ If you are upgrading from older releases:
records loaded next to `config.toml`, for example
`~/.codewhale/permissions.toml`. This schema foundation accepts
`[[rules]]` entries with `tool` plus optional `command` or `path` fields.
It intentionally does not accept typed allow/deny records or provide approval
UI persistence yet.
Loaded rules feed the execution policy engine and force approval in approval
modes that can ask; under `approval_policy = "never"`, matching ask rules are
rejected because no prompt can be shown. This intentionally does not accept
typed allow/deny records, glob expansion, or approval UI persistence yet.
- `managed_config_path` (string, optional): managed config file loaded after user/env config.
- `requirements_path` (string, optional): requirements file used to enforce allowed approval/sandbox values.
- `max_subagents` (int, optional): defaults to `10` and is clamped to `1..=20`.