Skip to content

Commit a507519

Browse files
author
Yogthos
committed
feat(perm): F1-F3 close opencode-review gaps (path args, edit alias, space-star)
Closes dirge-dvy / dirge-jlj / dirge-efw. Three gaps surfaced by cross-reviewing dirge's post-M1-M4 permission system against opencode's. All three port semantics directly from opencode for parity. ## F1 (dirge-dvy, P0): bash arg-side path checks Today (post-M3, fbcc09b) bash extracts ONLY redirect targets (`>` / `>>`) via extract_redirect_targets. Arguments of file-mutating commands (`rm` / `cp` / `mv` / etc.) went only through the bash command-pattern rules. A user with `bash: { "rm *": "allow" }` for convenience silently allowed `rm /etc/passwd` because the path-side write rules never saw the argument. Ported `extract_mutation_paths` from opencode shell.ts:30-51 (`FILES` set) + :191-221 (`pathArgs` filter logic). Walks the tree-sitter AST for `command` nodes, identifies file-mutating heads (rm/cp/mv/mkdir/rmdir/touch/chmod/chown/ln/tee/dd), and extracts each positional path arg. Each path then routes through enforce(tool="write", Scope::PathResolve) so write's deny rules apply. Skip rules ported from opencode: `-flag` args; chmod's `+x` permission specs; chmod/chown's FIRST positional arg (the mode or owner spec, not a path). Tests added: - `rm_arg_path_routes_through_write_rules`: `rm /etc/passwd` hits write deny even when bash explicitly allows `rm *` - `chmod_skips_mode_spec_routes_paths`: mode `777` is NOT a path arg; `/etc/passwd` IS and gets gated - `flags_skipped_when_extracting_paths`: `rm -rf /etc/passwd` treats `-rf` as flag, `/etc/passwd` as path ## F2 (dirge-jlj, P1): write/apply_patch alias to edit Opencode (permission/index.ts:291-301) defines `EDIT_TOOLS = ["edit", "write", "apply_patch"]` and maps all three to a single permission name "edit". A user writing `edit: deny` blocks all three uniformly. Dirge kept them as separate permission tools — `edit: deny` silently let `write` and `apply_patch` through. In `enforce()`, when the tool is `write` or `apply_patch`, ALSO consult the `edit` rules and combine via most-restrictive-wins (Deny > Ask > Allow). Strategy avoids double-prompting: both checks happen synchronously inside the checker lock, combined to one CheckResult, then the ask flow runs ONCE if needed. Tests added in src/agent/tools/mod.rs: - `enforce_write_aliases_to_edit_deny`: `edit: deny` blocks write + apply_patch - `enforce_write_alias_most_restrictive_wins`: explicit `write: allow` + `edit: deny` → deny wins - `enforce_read_does_not_alias_to_edit`: read isn't affected (negative test) ## F3 (dirge-efw, P2): space-star pattern optional-arg Opencode's wildcard matcher (util/wildcard.ts:13-15) rewrites trailing ` *` as `( .*)?` — making args optional. So `ls *` matches both `ls` and `ls -la`. Dirge required at least one arg; user accepts `ls *` from a session prompt, agent then runs bare `ls`, gets re-prompted. In src/permission/pattern.rs, when a command-style pattern ends with ` *`, wrap the head and emit `^head(?: .*)?$`. Path-style patterns unchanged (path `src/*` legitimately requires a segment). Tests added: - `f3_command_trailing_space_star_makes_args_optional`: `ls *` matches bare `ls`, `ls -la`, `ls /tmp`; doesn't match `lsof` / `less` - `f3_does_not_relax_path_patterns`: `src/*` still has the single-segment semantic - `f3_anchored_to_command_head`: `git *` doesn't match `gitk` ## Test results 1214/1215 tests pass (1 pre-existing unrelated Clojure failure per prior commits). Binary at ~/bin/dirge (12:07). Updated 1 existing test (`redirect_target_allowed_when_write_permits`) to also install `edit: allow` since the F2 aliasing means write's allow rule alone is insufficient — edit must permit too.
1 parent a9838e8 commit a507519

6 files changed

Lines changed: 547 additions & 15 deletions

File tree

.beads/issues.jsonl

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
{"_type":"issue","id":"dirge-dvy","title":"Perm review F1: bash arg-side path checks for file-mutating commands","description":"SECURITY GAP found in opencode-vs-dirge review. Today (post-M3, fbcc09b) dirge's bash permission flow extracts ONLY redirect targets (\u003e, \u003e\u003e, \u0026\u003e, etc.) via extract_redirect_targets and routes them through write rules. Arguments of file-mutating commands (rm, cp, mv, chmod, chown, ln, mkdir, rmdir, touch, tee, dd) are NOT extracted — they go only through the bash command-pattern rules. \n\nConcrete bypass: a user who configures bash rules permissively (e.g., 'rm *: allow' for convenience) silently allows 'rm /etc/passwd' even though write rules deny /etc/**. Opencode (shell.ts:374-410) walks the 'command' AST nodes, identifies file-mutating heads, and routes each positional path arg through the external_directory / write permission.\n\nPort: extend src/semantic/adapters/bash.rs with an extract_mutation_paths(command) function that walks the tree-sitter 'command' nodes; for each command whose head matches the list above, extract positional args that look like paths (skip -flags / --long-flags) and emit them. In src/agent/tools/bash.rs check_bash_segments, after the existing redirect-target loop, walk extracted mutation paths and route through enforce(tool='write', Scope::PathResolve(path)).","status":"closed","priority":0,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-05-23T15:48:18Z","created_by":"Yogthos","updated_at":"2026-05-23T15:53:28Z","started_at":"2026-05-23T15:48:30Z","closed_at":"2026-05-23T15:53:28Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
12
{"_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}
3+
{"_type":"issue","id":"dirge-jlj","title":"Perm review F2: alias write/apply_patch to edit permission","description":"HIGH-correctness gap found in opencode-vs-dirge review. Opencode (permission/index.ts:291-301) defines EDIT_TOOLS = ['edit', 'write', 'apply_patch'] and aliases all three to the same permission name 'edit' during rule evaluation. A user writing 'edit: deny' blocks all three uniformly.\n\nDirge keeps them as separate permission tools (one rule namespace each). A user writing 'permission: { edit: { **: deny } }' expecting to lock down all edits still has write and apply_patch silently going through (post-M4 they Ask, but a follow-up 'write: allow' from the user would unintentionally re-open the gate).\n\nPort: in enforce() at src/agent/tools/mod.rs, when the tool name is 'write' or 'apply_patch', ALSO consult the 'edit' rules. Take the more restrictive result of the two checks (any deny wins, any explicit ask beats allow). Document the aliasing in PermissionConfig docs.","status":"closed","priority":1,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-05-23T15:48:19Z","created_by":"Yogthos","updated_at":"2026-05-23T15:57:06Z","started_at":"2026-05-23T15:53:29Z","closed_at":"2026-05-23T15:57:06Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
24
{"_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":"closed","priority":1,"issue_type":"feature","owner":"yogthos@gmail.com","created_at":"2026-05-23T14:25:47Z","created_by":"Yogthos","updated_at":"2026-05-23T15:33:53Z","started_at":"2026-05-23T15:13:51Z","closed_at":"2026-05-23T15:33:53Z","close_reason":"Closed","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}
35
{"_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}
46
{"_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}
@@ -20,6 +22,7 @@
2022
{"_type":"issue","id":"dirge-86e","title":"ANSI injection in permission ALERT prompt","description":"ask_req.tool / ask_req.input rendered un-sanitized at mod.rs:2584-2585. Reopen path already sanitizes — asymmetric. Sec impl: ANSI at the permission-decision moment.","status":"closed","priority":1,"issue_type":"bug","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-21T22:17:34Z","created_by":"Yogthos","updated_at":"2026-05-21T22:26:37Z","started_at":"2026-05-21T22:17:42Z","closed_at":"2026-05-21T22:26:37Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
2123
{"_type":"issue","id":"dirge-9f1","title":"Chat history ignores 120-col content_width cap","description":"max_line_width and wrap_line use raw content_cols, so on wide terminals scrollback overflows the centered band into divider/panel margin.","status":"closed","priority":1,"issue_type":"bug","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-21T22:17:33Z","created_by":"Yogthos","updated_at":"2026-05-21T22:26:36Z","started_at":"2026-05-21T22:17:42Z","closed_at":"2026-05-21T22:26:36Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
2224
{"_type":"issue","id":"dirge-woq","title":"R1: fix 3 critical plugin bugs (FFI panic, dialog deadlock, init hang)","description":"From the plugin subsystem audit: (1) wrap JanetCFunctions in catch_unwind so Rust panics don't unwind across the C-FFI boundary into Janet; (2) cancel send_dialog's reply_rx.recv() on worker shutdown so the worker thread doesn't block forever when the UI exits mid-dialog; (3) add timeout to the init handshake so a worker panic before init_tx.send() doesn't hang the main thread. Also: (4) bounds-assert wrap_string's i32 cast for the unlikely \u003e2GB case, (5) make take_string_slot atomic to close the race window, (6) don't eat unrelated user events in the dialog arm.","status":"closed","priority":1,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-05-20T14:59:57Z","created_by":"Yogthos","updated_at":"2026-05-20T15:30:28Z","started_at":"2026-05-20T15:00:10Z","closed_at":"2026-05-20T15:30:28Z","dependency_count":0,"dependent_count":1,"comment_count":0}
25+
{"_type":"issue","id":"dirge-efw","title":"Perm review F3: space-star pattern optional-arg semantics","description":"LOW-polish gap found in opencode-vs-dirge review. Opencode's wildcard matcher (util/wildcard.ts:13-15) rewrites trailing ' *' as '( .*)?' — making the trailing args optional. So 'ls *' matches BOTH 'ls' and 'ls -la'. Dirge's pattern.rs glob_to_regex requires the args (' *' becomes ' .*' which needs at least the space).\n\nConcrete UX friction: a user accepts 'ls *' from a session allowlist after the agent runs 'ls -la', then the agent runs bare 'ls' — re-prompted because the saved pattern doesn't match. Port the optional-suffix rewrite to src/permission/pattern.rs's glob_to_regex.","status":"closed","priority":2,"issue_type":"task","owner":"yogthos@gmail.com","created_at":"2026-05-23T15:48:20Z","created_by":"Yogthos","updated_at":"2026-05-23T16:07:24Z","started_at":"2026-05-23T15:57:07Z","closed_at":"2026-05-23T16:07:24Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
2326
{"_type":"issue","id":"dirge-84j","title":"B3-9: edit tool fuzzy-match cascade","description":"edit.rs:155-160 returns 'old_text not found' on whitespace/indent/CRLF drift. LLMs frequently hit this. opencode edit.ts:222-432 has simple → lineTrimmed → whitespace-normalized → indentation-flexible → levenshtein. pi edit-diff.ts:91-132 has fuzzyFindText. Port the cascade.","status":"closed","priority":2,"issue_type":"feature","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-23T03:24:44Z","created_by":"Yogthos","updated_at":"2026-05-23T03:43:39Z","started_at":"2026-05-23T03:37:16Z","closed_at":"2026-05-23T03:43:39Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
2427
{"_type":"issue","id":"dirge-ims","title":"B3-10: LSP pull-diagnostic fallback for lazy servers","description":"lsp/client.rs:261-289 wait_for_push is push-only. clojure-lsp/jdtls/clangd may not push on demand. 10s timeout = 'clean' diagnostic block reported when errors exist. opencode lsp/client.ts:540-582 races push-wait against requestDocumentDiagnostics. Add pull fallback.","status":"closed","priority":2,"issue_type":"feature","assignee":"Yogthos","owner":"yogthos@gmail.com","created_at":"2026-05-23T03:24:44Z","created_by":"Yogthos","updated_at":"2026-05-23T04:40:13Z","started_at":"2026-05-23T03:43:41Z","closed_at":"2026-05-23T04:40:13Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
2528
{"_type":"issue","id":"dirge-yep","title":"B3-8: soft-wrap + mouse selection display-width awareness","description":"renderer.rs:1457-1496 wrap_input measures chars().count() instead of UnicodeWidthChar. Cursor mis-lands on CJK/emoji wrapped lines. Mouse selection columns map 1:1 char-to-display rather than width-aware. Pi interactive-mode.ts uses string-width per cell.","status":"closed","priority":2,"issue_type":"bug","owner":"yogthos@gmail.com","created_at":"2026-05-23T03:24:43Z","created_by":"Yogthos","updated_at":"2026-05-23T03:37:14Z","closed_at":"2026-05-23T03:37:14Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}

0 commit comments

Comments
 (0)