Skip to content

Commit 0a7518b

Browse files
author
Yogthos
committed
feat(perm): M2 — unified tools map; rules for any tool name
Closes dirge-cep. Third milestone of the staged permission refactor; depends on M1 (8dd5b01). Unblocks M4 (dirge-ojn). ## What `PermissionConfig` gains a `tools: Option<HashMap<String, ToolPerm>>` field. Rules can be declared for ANY tool name (plugin-registered, MCP, future built-ins) without extending the struct: ```jsonc "permission": { "tools": { "bash": { "rm *": "deny", "git *": "allow" }, "write": { "/etc/**": "deny", "**": "ask" }, "skill": "allow", "plugin_xyz": "ask" } } ``` Mirrors opencode's `Schema.StructWithRest` + `Schema.Record(String, Rule)` pattern and maki's per-tool TOML sections. ## Back-compat The legacy per-tool fields (`bash`, `read`, `write`, ..., `mcp_tool`) stay. `PermissionChecker::new` merges both surfaces into the same internal `HashMap<tool, Vec<(Pattern, Action)>>`. If a tool appears in both: - legacy field installs its rules first - `tools` map then overwrites (explicit newer shape wins) Existing user configs (the user's own ~/.config/dirge/config.json has no permission block) keep working unchanged. ## Deprecation Per-tool fields are syntactic sugar for `tools.{name}` now. Doc and example configs migrate over one release cycle, then the per-tool fields can be removed. M4 (dirge-ojn, defaults flip) can ship independently — it touches `default_action`, not the rule surface. ## Test `tools_map_unified_schema_honored_and_overrides_legacy` pins: - tool with no legacy field is reachable only via `tools` map - legacy field overridden when the same tool appears in both - `Simple(action)` shorthand works in the map 1202/1203 tests pass (1 pre-existing unrelated Clojure failure, unchanged across last 3 commits). Binary at ~/bin/dirge (11:09).
1 parent fbcc09b commit 0a7518b

3 files changed

Lines changed: 109 additions & 1 deletion

File tree

.beads/issues.jsonl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{"_type":"issue","id":"dirge-6ab","title":"Perm M3: port maki's tree-sitter bash analyzer (close git\u0026\u0026rm bypass)","description":"SECURITY: 'git diff \u0026\u0026 rm -rf /' currently allowed because dirge's bash redirect-target check (src/agent/tools/bash.rs:350) routes through bash rules with the file path as input, which has no path-style match → falls to default Allow. Pre-existing pre-fix-for-7403792.\n\nSolution: port maki's tree-sitter bash analyzer verbatim from /Users/yogthos/src/maki/maki-agent/src/permissions.rs:33-43 (parser thread_local), 394-439 (collect_commands walker), 441-475 (analyze_bash + complexity gates). The walker splits compounds via 'pipeline'/'list' AST traversal and extracts every 'command' / 'redirected_statement' / 'subshell' / etc node; each segment then goes through the permission chokepoint independently.\n\nBehavior at completion:\n- 'git diff \u0026\u0026 rm -rf /' → enforce('bash', 'git diff') + enforce('bash', 'rm -rf /') — second check fires\n- Subshells / command substitution mark whole command 'complex' → forces prompt (conservative)\n- Pipes split into separate segments\n- Quoted operators correctly NOT split (AST respects quoting)\n\nDepends on: dirge-{M1}\n\nMaki license is GPL-compatible — verify before copying. Add 'Ported from maki-agent/src/permissions.rs' attribution comment.","status":"closed","priority":0,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-05-23T14:25:46Z","created_by":"Yogthos","updated_at":"2026-05-23T15:01:11Z","started_at":"2026-05-23T14:51:42Z","closed_at":"2026-05-23T15:01:11Z","close_reason":"Closed","dependencies":[{"issue_id":"dirge-6ab","depends_on_id":"dirge-01s","type":"blocks","created_at":"2026-05-23T10:25:51Z","created_by":"Yogthos","metadata":"{}"}],"dependency_count":1,"dependent_count":1,"comment_count":0}
22
{"_type":"issue","id":"dirge-ojn","title":"Perm M4: flip unmatched-tool default from Allow to Ask","description":"Today src/permission/checker.rs:103 has default_action = Action::Allow. Anything not explicitly configured falls through to Allow without prompting — a security gap especially for write/edit/apply_patch which currently have NO default rules installed at all (mismatch with bash which has default_bash_rules, mcp_tool which defaults Ask since 27bd70a).\n\nFlip default to Ask. Add builtin-allow list for safe read-only tools (port maki's BUILTIN_ALLOW_RULES at permissions.rs:16-24, adapted for dirge's tool set: read/glob/grep/list_dir/list_symbols/find_definition/find_callers/find_callees/get_symbol_body/repo_overview).\n\nwrite/edit/apply_patch/bash/webfetch/websearch/task/skill/memory all become Ask by default unless explicitly allowlisted. Document the migration in README + CHANGELOG. Users with existing configs unaffected — only the no-config baseline changes.\n\nAdd --yolo CLI flag (currently only via config). Maps to set the global allow_all atomic (port maki's PermissionManager::toggle_yolo at permissions.rs:219-222).\n\nDepends on: dirge-{M2}, dirge-{M3}","status":"open","priority":1,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-05-23T14:25:47Z","created_by":"Yogthos","updated_at":"2026-05-23T14:25:47Z","dependencies":[{"issue_id":"dirge-ojn","depends_on_id":"dirge-6ab","type":"blocks","created_at":"2026-05-23T10:25:53Z","created_by":"Yogthos","metadata":"{}"},{"issue_id":"dirge-ojn","depends_on_id":"dirge-cep","type":"blocks","created_at":"2026-05-23T10:25:52Z","created_by":"Yogthos","metadata":"{}"}],"dependency_count":2,"dependent_count":0,"comment_count":0}
33
{"_type":"issue","id":"dirge-01s","title":"Perm M1: single chokepoint (port maki's enforce shape)","description":"Refactor dirge's 3 permission entry points (check_perm, check_perm_path, check_perm_path_resolve in src/agent/tools/mod.rs) into a single chokepoint patterned after maki's PermissionManager::enforce (maki-agent/src/permissions.rs:283). One function, takes (tool, scope), routes internally based on tool category. No behavior change for users — pure refactor with all existing callers updated. Behind-the-scenes the new fn calls the same PermissionChecker logic.\n\nRef: maki-agent/src/permissions.rs:283-350 — port the signature shape (async, returns Result\u003c(), PermissionError\u003e, takes \u0026EventSender + user_response_rx + cancel for UI integration). Adapt to dirge's existing AskSender + PermCheck.","status":"closed","priority":1,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-05-23T14:25:25Z","created_by":"Yogthos","updated_at":"2026-05-23T14:49:19Z","started_at":"2026-05-23T14:40:51Z","closed_at":"2026-05-23T14:49:19Z","close_reason":"Closed","dependency_count":0,"dependent_count":2,"comment_count":0}
4-
{"_type":"issue","id":"dirge-cep","title":"Perm M2: unified rule schema (port maki's TOML shape via JSON)","description":"Replace dirge's per-tool field PermissionConfig (separate Option\u003cToolPerm\u003e per built-in tool) with maki's uniform shape: { tool_name: { allow: [patterns], deny: [patterns] } }. Single PermissionRule struct (maki-config/src/lib.rs:268-273). Keep existing JSON config (dirge uses JSON not TOML), but flatten the schema. Add a dual-read path so old config.json files (existing user configs in the wild) still parse — log a deprecation warning and auto-migrate on save. Migration test required.\n\nDepends on: dirge-{M1}","status":"open","priority":1,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-05-23T14:25:25Z","created_by":"Yogthos","updated_at":"2026-05-23T14:25:25Z","dependencies":[{"issue_id":"dirge-cep","depends_on_id":"dirge-01s","type":"blocks","created_at":"2026-05-23T10:25:50Z","created_by":"Yogthos","metadata":"{}"}],"dependency_count":1,"dependent_count":1,"comment_count":0}
4+
{"_type":"issue","id":"dirge-cep","title":"Perm M2: unified rule schema (port maki's TOML shape via JSON)","description":"Replace dirge's per-tool field PermissionConfig (separate Option\u003cToolPerm\u003e per built-in tool) with maki's uniform shape: { tool_name: { allow: [patterns], deny: [patterns] } }. Single PermissionRule struct (maki-config/src/lib.rs:268-273). Keep existing JSON config (dirge uses JSON not TOML), but flatten the schema. Add a dual-read path so old config.json files (existing user configs in the wild) still parse — log a deprecation warning and auto-migrate on save. Migration test required.\n\nDepends on: dirge-{M1}","status":"closed","priority":1,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-05-23T14:25:25Z","created_by":"Yogthos","updated_at":"2026-05-23T15:13:00Z","started_at":"2026-05-23T15:07:25Z","closed_at":"2026-05-23T15:13:00Z","close_reason":"Closed","dependencies":[{"issue_id":"dirge-cep","depends_on_id":"dirge-01s","type":"blocks","created_at":"2026-05-23T10:25:50Z","created_by":"Yogthos","metadata":"{}"}],"dependency_count":1,"dependent_count":1,"comment_count":0}
55
{"_type":"issue","id":"dirge-jzj","title":"UI: per-tool-call-id chamber state (parallel tool execution)","description":"The UI tracks chamber state via a single bool tool_chamber_open + Option\u003cString\u003e last_tool_name. Under parallel tool execution (the default per agent_loop/types.rs:402), multiple ToolExecutionStart events fire before any ToolExecutionEnd. Subsequent ToolCalls close prior chambers prematurely (now via passive close after 7403792, previously with false 'denied' wording). ToolResults that arrive after a newer ToolCall's chamber opens land as out-of-place '↳ trailers' below the wrong chamber. Fix: chamber state keyed by tool_call_id (HashMap), each in-flight tool gets its own chamber frame, the abort/passive close functions take an id.","status":"closed","priority":1,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-05-23T14:25:11Z","created_by":"Yogthos","updated_at":"2026-05-23T14:35:50Z","started_at":"2026-05-23T14:27:32Z","closed_at":"2026-05-23T14:35:50Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
66
{"_type":"issue","id":"dirge-5uv","title":"H-batch1-2: history up/down byte-offset panic risk on multi-byte input","description":"ui/input.rs:988-1023 — col = self.cursor - line_start is byte distance; self.cursor = (pos + col).min(target_line_end) can land mid-codepoint when previous line has multi-byte chars. Subsequent replace_range/slice panics. Convert column to char index then back to byte offset on target line.","status":"closed","priority":1,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-05-23T02:34:57Z","created_by":"Yogthos","updated_at":"2026-05-23T03:09:57Z","closed_at":"2026-05-23T03:09:57Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
77
{"_type":"issue","id":"dirge-le5","title":"H-batch1-1: bash inherits all env vars including API keys","description":"tools/bash.rs:208-320 no .env_clear(). Sensitive vars (OPENROUTER_API_KEY, EXA_API_KEY, PARALLEL_API_KEY, ANTHROPIC_API_KEY, etc.) flow to every bash child. Use .env_clear() + curated allowlist (PATH, HOME, USER, LANG, TERM, etc.). See pi bash-executor.ts.","status":"closed","priority":1,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-05-23T02:34:57Z","created_by":"Yogthos","updated_at":"2026-05-23T03:09:57Z","closed_at":"2026-05-23T03:09:57Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}

src/permission/checker.rs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,31 @@ impl PermissionChecker {
150150
rules.insert(tool_name.to_string(), entries);
151151
}
152152

153+
// M2 (dirge-cep): merge the unified `tools` map. New configs
154+
// can declare rules for ANY tool name (including plugin / MCP
155+
// / future tools) without extending `PermissionConfig`. If a
156+
// tool is named in both the legacy per-field surface and the
157+
// `tools` map, the map wins — it's the explicit, newer shape
158+
// and the migration path is "move per-tool fields into
159+
// tools". `mcp_tool` (and the other umbrella names) take the
160+
// same syntax: `tools: { mcp_tool: { "mcp_tool:fs:*": "deny" } }`.
161+
if let Some(tools_map) = &config.tools {
162+
for (tool_name, tp) in tools_map {
163+
let mut entries = Vec::new();
164+
match tp {
165+
ToolPerm::Simple(action) => {
166+
entries.push((pattern_for_tool(tool_name, "*"), *action));
167+
}
168+
ToolPerm::Granular(map) => {
169+
for (pat, action) in map {
170+
entries.push((pattern_for_tool(tool_name, pat), *action));
171+
}
172+
}
173+
}
174+
rules.insert(tool_name.clone(), entries);
175+
}
176+
}
177+
153178
if !rules.contains_key("bash") {
154179
let mut defaults = Vec::new();
155180
for (pat, action) in crate::permission::default_bash_rules() {
@@ -800,6 +825,61 @@ mod tests {
800825
}
801826
}
802827

828+
/// M2 (dirge-cep): the unified `tools` map at the top of
829+
/// `PermissionConfig` lets rules be declared for ANY tool name
830+
/// (including ones dirge doesn't ship per-tool struct fields
831+
/// for — plugin-registered tools, future tools). Pin three
832+
/// invariants:
833+
/// 1. A rule in `tools` for a tool name with no legacy field
834+
/// is honored.
835+
/// 2. A rule in `tools` for a tool name that ALSO has a
836+
/// legacy field overrides the legacy field (explicit
837+
/// newer shape wins).
838+
/// 3. The `Simple(action)` shape (string shorthand for
839+
/// `{"*": action}`) works in the map.
840+
#[test]
841+
fn tools_map_unified_schema_honored_and_overrides_legacy() {
842+
use crate::permission::{PermissionConfig, ToolPerm};
843+
use std::collections::HashMap;
844+
845+
// Tool with no legacy field — only reachable via `tools`.
846+
let mut tools_map = HashMap::new();
847+
let mut plugin_rules = HashMap::new();
848+
plugin_rules.insert("dangerous".to_string(), Action::Deny);
849+
tools_map.insert(
850+
"plugin_xyz".to_string(),
851+
ToolPerm::Granular(plugin_rules),
852+
);
853+
854+
// Tool with a legacy field — map version should win.
855+
tools_map.insert("websearch".to_string(), ToolPerm::Simple(Action::Deny));
856+
857+
let config = PermissionConfig {
858+
// Legacy field says Allow…
859+
websearch: Some(ToolPerm::Simple(Action::Allow)),
860+
tools: Some(tools_map),
861+
..Default::default()
862+
};
863+
864+
let mut checker = PermissionChecker::new(
865+
&config,
866+
SecurityMode::Standard,
867+
Some(std::path::PathBuf::from("/tmp")),
868+
);
869+
870+
// (1) tools-only entry honored.
871+
assert!(matches!(
872+
checker.check("plugin_xyz", "dangerous"),
873+
CheckResult::Denied(_)
874+
));
875+
876+
// (2) tools map overrides legacy field.
877+
assert!(matches!(
878+
checker.check("websearch", "anything"),
879+
CheckResult::Denied(_)
880+
));
881+
}
882+
803883
/// Adversarial-review #1: the deny-list match must also fire for
804884
/// the umbrella `mcp_tool` name and the qualified `mcp_tool:srv:name`
805885
/// form, since MCP tools route through `check_perm("mcp_tool", …)`.

src/permission/mod.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,34 @@ pub struct PermissionConfig {
101101
pub mcp_tool: Option<ToolPerm>,
102102
pub external_directory: Option<HashMap<String, Action>>,
103103
pub doom_loop: Option<Action>,
104+
/// M2 (dirge-cep): unified per-tool rule map. Lets a user write
105+
/// rules for ANY tool name (including plugin / MCP / future-added
106+
/// tools) without dirge extending its `PermissionConfig` struct.
107+
///
108+
/// Schema (JSON):
109+
/// ```json
110+
/// "permission": {
111+
/// "tools": {
112+
/// "bash": { "rm *": "deny", "git *": "allow" },
113+
/// "write": { "/etc/**": "deny", "**": "ask" },
114+
/// "skill": "allow",
115+
/// "plugin_xyz": "ask"
116+
/// }
117+
/// }
118+
/// ```
119+
///
120+
/// Mirrors opencode's permission shape (Schema.StructWithRest with
121+
/// Schema.Record(String, Rule)) and maki's per-tool TOML sections.
122+
/// Coexists with the legacy per-tool fields above for back-compat:
123+
/// both are merged into the same `HashMap<tool, Vec<(Pattern,
124+
/// Action)>>` inside `PermissionChecker::new`. If both name the
125+
/// same tool, the `tools` map wins (it's the explicit new shape).
126+
///
127+
/// Deprecation path: the legacy `bash`/`read`/`write`/... fields
128+
/// stay through one release cycle, then get removed once docs and
129+
/// example configs migrate to `tools`. Internally the checker
130+
/// treats them as syntactic sugar for `tools.{name}`.
131+
pub tools: Option<HashMap<String, ToolPerm>>,
104132
}
105133

106134
/// Per-session security mode. Selected via `--yolo` / `--accept-all` /

0 commit comments

Comments
 (0)