fix(permissions): reach codex's own-named built-ins and MCP spelling

handbook/architecture/cli-integration.md

@@ -280,12 +280,35 @@ These are the seams that stop backend identity leaking into shared code. The rul
      keeps its hook settings in bypass mode.
 
   The payload codex sends is Claude-compatible (`tool_name`/`tool_input`, deny by exit 2 +
-  stderr), so `pre-tool-use.sh` needs no codex argument. Its shell tool already arrives as
-  `Bash`; a file edit arrives as `apply_patch` with **no path in `tool_input` at all** — the paths
-  are inside the `command` string as an apply_patch envelope, which is why
-  `codex_tool_vocabulary.lua` has no `normalize_input` and says so at length. Consequence, stated
-  there and repeated here because it is a permission gap rather than a cosmetic one: granular
-  `paths` rules never match a codex edit, and `request_diff.capture` backs nothing up for one.
-  Filling `file_path` with the first path a multi-file patch names would read as working while
-  letting a deny rule be evaded by patch ordering, so fixing it properly means teaching
-  `matchers.lua` about a set of paths.
+  stderr), so `pre-tool-use.sh` needs no codex argument.
+
+  **Codex converges on Claude's vocabulary only where it matters.** Read off
+  `codex-rs/core/src/tools/hook_names.rs` at codex 0.154.0: `HookToolName::bash()` is what every
+  shell-like handler reports (`unified_exec` included), `apply_patch()` serializes as
+  `apply_patch` while accepting `Write`/`Edit` as matcher aliases, and `spawn_agent()` accepts
+  `Agent`. Its remaining built-ins are `ToolName::plain` and arrive under their own names. That
+  asymmetry is load-bearing for us in both directions: nothing that writes or executes was ever
+  unmapped, so an incomplete table was never a deny-side hole — but `view_image` and `web_search`
+  are unreachable by a `Read`/`WebSearch` rule without entries, which they now have.
+
+  `view_image` declares one required `path` (`view_image_spec.rs`), so `normalize_input` lifts it
+  to `file_path`. Without that, mapping it onto `Read` would read as covered by a `Read(...)` rule
+  while never matching — and `Read` is in `ALWAYS_ALLOWED_TOOLS`, so a path-scoped deny is the only
+  thing that can stop it.
+
+  A file edit still arrives as `apply_patch` with **no path in `tool_input` at all** — the paths
+  are inside the `command` string as an apply_patch envelope. Consequence, stated in
+  `codex_tool_vocabulary.lua` and repeated here because it is a permission gap rather than a
+  cosmetic one: granular `paths` rules never match a codex edit, and `request_diff.capture` backs
+  nothing up for one. Filling `file_path` with the first path a multi-file patch names would read
+  as working while letting a deny rule be evaded by patch ordering, so fixing it properly means
+  teaching `matchers.lua` about a set of paths.
+
+  **MCP server labels are the other spelling difference.** Codex normalizes `-` to `_` before
+  composing an MCP tool name, so `chrome-devtools` reaches the hook as `mcp__chrome_devtools__*`
+  where claude sends it verbatim. `matchers.lua` folds both sides onto the `_` spelling when both
+  start with `mcp__`, so one allow entry covers every backend. The fold direction is the safe one:
+  nothing in `mcp__a_b__x` says which underscore used to be a hyphen, which is why
+  `codex_tool_vocabulary.lua` can only restore the one prefix it anchors on — and must keep doing
+  so, because `can_use_tool.is_vibing_nvim_mcp_tool` matches the hyphenated spelling directly
+  rather than going through `matchers.lua`.

lua/vibing/infrastructure/adapter/modules/codex_tool_vocabulary.lua

@@ -18,10 +18,21 @@ local M = {}
 ---
 --- So codex already speaks Claude's name for its shell tool and needs no entry for it. `shell` is
 --- mapped anyway because it is what older codex sent and the mapping costs nothing.
+---
+--- Codex converges on Claude's vocabulary only for the tools that carry risk: `hook_names.rs` in
+--- codex 0.154.0 serializes shell-likes (including `unified_exec`) as `Bash`, and gives
+--- `apply_patch` the matcher aliases `Write`/`Edit` and `spawn_agent` the alias `Agent`. Its
+--- remaining built-ins reach PreToolUse under their own names (`ToolName::plain`), so a `Read` or
+--- `WebSearch` rule would miss them without the entries below. That asymmetry is also why the gap
+--- was benign rather than a hole: nothing that writes or executes was ever unmapped.
 --- @type table<string, string>
 local NATIVE_TO_CANONICAL = {
   apply_patch = "Edit", -- Codex's file patch tool maps to Claude's Edit
   shell = "Bash",
+  -- Reads an image off disk to attach it. `Read` is in ALWAYS_ALLOWED_TOOLS, so this also stops
+  -- codex prompting for every image the way an unmapped name does.
+  view_image = "Read",
+  web_search = "WebSearch",
 }
 
 -- MCP server labels are normalized before Codex exposes them as tool names. In particular, the
@@ -47,11 +58,26 @@ function M.to_canonical(native_tool_name)
   return NATIVE_TO_CANONICAL[native_tool_name]
 end
 
---- **Deliberately absent: `normalize_input`.** Known gap, not an oversight.
+--- Where codex puts the path a tool is about, for the tools that name one at all. `view_image`
+--- declares a single required `path` (`view_image_spec.rs`), which is the same shape copilot uses,
+--- so a granular `Read(...)` paths rule can reach it. Without this, mapping `view_image` to `Read`
+--- above would read as covered by such a rule while silently never matching.
+--- @param tool_input table
+--- @return table input with `file_path` filled in when codex named it `path`. The original is
+---   never mutated: the same payload is also used to render the approval UI.
+function M.normalize_input(tool_input)
+  if type(tool_input) ~= "table" or tool_input.file_path or not tool_input.path then
+    return tool_input
+  end
+
+  return vim.tbl_extend("force", tool_input, { file_path = tool_input.path })
+end
+
+--- **Still uncovered above: `apply_patch`.** Known gap, not an oversight.
 ---
---- Codex does not put the edited path in a sibling key the way grok (`target_file`) and copilot
---- (`path`) do -- there is no path in `tool_input` at all. It is inside the `command` string, as an
---- apply_patch envelope that may name several files at once:
+--- Codex does not put the *edited* path in a sibling key the way grok (`target_file`) and copilot
+--- (`path`) do -- there is no path in an apply_patch `tool_input` at all. It is inside the
+--- `command` string, as an envelope that may name several files at once:
 ---
 ---   *** Begin Patch
 ---   *** Update File: a.lua

lua/vibing/infrastructure/permissions/matchers.lua

@@ -162,6 +162,24 @@ function M.matches_permission(tool_name, input, permission_str)
       local perm_tool_name = parsed.tool_name
       local actual_tool_name = tool_name:lower()
 
+      -- Backends disagree on how an MCP server label is spelled in the tool name: codex normalizes
+      -- `-` to `_` before composing it, so `chrome-devtools` arrives as `mcp__chrome_devtools__*`
+      -- while claude sends it verbatim. Comparing both sides in the `_` spelling lets one allow
+      -- entry cover every backend, instead of the user listing each spelling by hand.
+      --
+      -- The `_` direction is the safe one: it folds a pattern *down* onto the name a backend
+      -- actually sends. Restoring `-` in the tool name would be guesswork -- nothing in
+      -- `mcp__a_b__x` says which underscore used to be a hyphen. (`codex_tool_vocabulary` can
+      -- afford that guess only because it anchors on one known prefix.)
+      --
+      -- Scoped to `mcp__` on both sides so no built-in tool name is affected. It does merge two
+      -- servers whose names differ solely by `-` vs `_`, which is the cost of not requiring the
+      -- user to know each backend's spelling rule.
+      if vim.startswith(perm_tool_name, "mcp__") and vim.startswith(actual_tool_name, "mcp__") then
+        perm_tool_name = perm_tool_name:gsub("%-", "_")
+        actual_tool_name = actual_tool_name:gsub("%-", "_")
+      end
+
       if vim.endswith(perm_tool_name, "*") then
         local prefix = perm_tool_name:sub(1, -2)
         return vim.startswith(actual_tool_name, prefix)

tests/lua/infrastructure/permissions/can_use_tool_spec.lua

@@ -173,6 +173,38 @@ describe("can_use_tool", function()
     end)
   end)
 
+  describe("MCP server labels spelled with `-` or `_` by different backends", function()
+    -- Codex normalizes an MCP server label's `-` to `_` before composing the tool name, so
+    -- `chrome-devtools` reaches the hook as `mcp__chrome_devtools__*` while claude sends it
+    -- verbatim. One allow entry has to cover both, or the rule silently misses on one backend.
+    local function decision(tool_name, overrides)
+      return can_use_tool.can_use_tool(tool_name, {}, make_config(vim.tbl_extend("force", {
+        permission_mode = "default",
+        mcp_enabled = true,
+      }, overrides or {}))).behavior
+    end
+
+    it("matches a hyphenated allow pattern against either spelling", function()
+      local allowed = { allowed_tools = { "mcp__chrome-devtools__*" } }
+      assert.equals("allow", decision("mcp__chrome-devtools__take_snapshot", allowed))
+      assert.equals("allow", decision("mcp__chrome_devtools__take_snapshot", allowed))
+    end)
+
+    it("closes the same gap on the deny side, where a miss falls open", function()
+      local denied = { denied_tools = { "mcp__chrome-devtools__*" } }
+      assert.equals("deny", decision("mcp__chrome-devtools__take_snapshot", denied))
+      assert.equals("deny", decision("mcp__chrome_devtools__take_snapshot", denied))
+    end)
+
+    it("does not widen the match beyond the separator", function()
+      -- Folding `-` onto `_` must not turn the pattern into a looser prefix: a different server
+      -- whose name merely starts the same way stays unmatched.
+      assert.equals("ask", decision("mcp__chrome-devtools-beta__take_snapshot", {
+        allowed_tools = { "mcp__chrome-devtools__*" },
+      }))
+    end)
+  end)
+
   describe("is_vibing_nvim_mcp_tool", function()
     it("matches a specific tool regardless of registration namespace", function()
       assert.is_true(

tests/lua/infrastructure/rpc/handlers/permission_vocabulary_spec.lua

@@ -118,6 +118,45 @@ describe("permission handler tool vocabulary", function()
     assert.is_nil(vocabulary.to_canonical("Read"))
   end)
 
+  it("maps the codex built-ins that reach the hook under their own names", function()
+    -- codex 0.154.0 renames only what carries risk: hook_names.rs serializes shell-likes as
+    -- `Bash` and aliases `apply_patch` to Write/Edit. Its remaining built-ins arrive as
+    -- `ToolName::plain`, so without these a `Read`/`WebSearch` rule never sees them.
+    local vocabulary = require("vibing.infrastructure.adapter.modules.codex_tool_vocabulary")
+    assert.equals("Read", vocabulary.to_canonical("view_image"))
+    assert.equals("WebSearch", vocabulary.to_canonical("web_search"))
+  end)
+
+  it("lifts view_image's `path` so a Read paths rule can reach it", function()
+    -- Mapping view_image onto Read without this would read as covered by `Read(...)` while
+    -- silently never matching: codex declares the argument as `path`, not `file_path`.
+    local vocabulary = require("vibing.infrastructure.adapter.modules.codex_tool_vocabulary")
+    local normalized = vocabulary.normalize_input({ path = "/tmp/project/secret.png" })
+    assert.equals("/tmp/project/secret.png", normalized.file_path)
+  end)
+
+  it("leaves an apply_patch input alone, since it carries no path at all", function()
+    local vocabulary = require("vibing.infrastructure.adapter.modules.codex_tool_vocabulary")
+    local input = { command = "*** Begin Patch\n*** Update File: a.lua\n*** End Patch" }
+    assert.is_nil(vocabulary.normalize_input(input).file_path)
+  end)
+
+  it("denies a codex image read when a path-scoped Read deny covers the file", function()
+    -- The whole chain end to end: view_image -> Read, `path` -> `file_path`, then the glob. Read
+    -- is in ALWAYS_ALLOWED_TOOLS, so only the path-scoped deny can stop it -- and that deny reads
+    -- `file_path`, which nothing but normalize_input puts there.
+    local vocabulary = require("vibing.infrastructure.adapter.modules.codex_tool_vocabulary")
+    permission.set_active_opts(HANDLE_ID, {
+      permissions_deny = { "Read(**/secret.png)" },
+      _tool_vocabulary = vocabulary,
+    })
+
+    write_request("req-view-image", "view_image", { path = "/tmp/project/secret.png" })
+    local result = permission.check_tool_permission({ request_id = "req-view-image", handle_id = HANDLE_ID })
+
+    assert.equals("denied", result.status)
+  end)
+
   it("pre-approves Codex's normalized name for the bundled vibing-nvim MCP server", function()
     local vocabulary = require("vibing.infrastructure.adapter.modules.codex_tool_vocabulary")
     permission.set_active_opts(HANDLE_ID, {