A deny rule naming a tool literally failed open when the name was also a glob (#75)

`PermissionPolicy.decide` matched rules with `fnmatch(tool_name, rule.pattern)` and
nothing else. `pattern` is documented as a glob, so the obvious way to write a rule —
paste the tool's exact name — silently stopped matching the moment that name carried
`[`…`]`, because fnmatch reads it as a character class:

    DENY "exfil[all]"   does not match the tool exfil[all]
    ALLOW "*"           does
    -> allow

The operator got no error, no warning and no deny. `ToolRegistry.visible()` decides
through the same call, so the tool the operator had just forbidden was also described
to the model as available — the model was actively invited to call the thing it may
not have, and the call went through. Verified end to end, not only at the policy layer.

The failure was inconsistent as well as silent: `DENY "tool?x"` happened to hold,
because a `?` glob matches a literal `?`. And it was the one place in this tree where
a *deny* failed open — an unmatched tool defaults to DENY, an unregistered kind is
refused, an unreachable backend raises.

A rule now matches on `fnmatch(name, pattern) or name == pattern`, with the equality
bound to the DENY and ASK tiers. That binding is the whole of the safety argument:
adding a match to deny or ask can only ever refuse or gate a call that would otherwise
have run, so it cannot loosen any policy. The same widening on ALLOW could grant a
tool on a pattern the operator wrote as a glob, so ALLOW stays glob-only, and the case
it needs is served by `PermissionRule.literal(action, name)`, which stores
`glob.escape(name)` and so names one tool exactly at any tier. `default_harness`
builds its ALLOW rules from registry names rather than from operator patterns, so it
uses `literal` now; the core tool names carry no metacharacters, so nothing there
changes behaviour today.

Glob semantics are untouched, and tested as such: `rm*` still spans `rmdir`, `*` still
matches everything, a glob still matches through every tier rather than only its own
text, the deny -> ask -> allow ordering is unchanged, and an unmatched tool still falls
to the DENY default.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Author: Shashankss1205

CHANGELOG.md

@@ -20,3 +20,4 @@ Entries are newest-last within a release, matching the order they were written.
 - a **NUL byte in a path came back as silence**, the worst answer a chat bot can give: `Path(raw).resolve()` raises `ValueError`, `handle_text_live` catches only `SlackCommandError`, so `trace a\x00b` escaped the bolt listener as an unhandled exception and the requester saw no reply at all — indistinguishable from the bot being down. A NUL anywhere in the request is now a refusal in the same voice the core tools already use ("cannot name a file"), and `_confined` turns any `ValueError`/`OSError` out of the filesystem into a refusal too, for callers of its own. Folded in from the same report: the flag allowlist tested `token.startswith("--")`, so a single-dash token slipped it and was spent as a positional — `trace -h` was admitted with `-h` as the path. Any leading dash is a flag now, and one not on the list is refused like any other.
 - the `/live` **token check crashed on the strangers it exists to refuse**. `secrets.compare_digest` rejects `str` outside ASCII, and `_authorized` handed it the raw query parameter, so `?token=café` raised `TypeError` through the handler: an unauthenticated 500 with a traceback in the log on all four `/live` routes, where every ASCII guess correctly got a 401. The 500-vs-401 split was itself an oracle about how the token is compared. Both sides are encoded to UTF-8 now, which drops the ASCII restriction and keeps the constant-time comparison that is the whole reason `compare_digest` is there. A NUL byte in `?trace=` was the same shape one function over — `resolve_trace` raises `ValueError`, not the `LivePathError` the route caught — and is a 404 like any other malformed path now.
 - the `/live` **index advertised traces the reader refuses to serve**. `scan_traces` walked the live root with `rglob("*.jsonl")`, which matches a symlinked file by name, then parsed it and published its name, size, mtime and **run ids** on `GET /live/api/runs` and the HTML index — for a file outside the root that `/live/api/stream` then 404s, the 404 being the proof of intent. One contract, two code paths, and only the reader enforced it; the live root is documented as the Slack bot's working directory, i.e. somewhere other things write. `scan_traces` routes every candidate through `resolve_trace` now and skips symlinks outright, so a refactor of either check cannot reopen the leak. The reader's confinement — `../`, `%2e%2e%2f`, absolute paths, `sub/../../`, symlinked directories — is unchanged.
+- a **`deny` rule naming a tool literally failed open** when the name carried fnmatch metacharacters. `PermissionPolicy.decide` matched with `fnmatch(name, pattern)` alone, so `DENY "exfil[all]"` read as a character class, did not match the tool it spells, and evaluation fell through to whatever came next — typically a broad `ALLOW "*"`. The operator got no error, no warning and no deny; worse, `visible()` decides the same way, so the tool the operator had just forbidden was described to the model as available and then ran when it asked. The failure was inconsistent as well as silent: `DENY "tool?x"` happened to hold, because a `?` glob matches a literal `?`. This was the one place in the tree where a deny failed open — an unmatched tool, an unregistered kind and an unreachable backend all refuse. A `deny` or `ask` rule now also fires on an exact literal match. The widening is bound to those two tiers on purpose: equality can only add a rule that refuses or gates a call, never one that permits it, so it cannot loosen a policy the way the same change on `allow` could. For the `allow` case there is `PermissionRule.literal(action, name)`, which `glob.escape`s the name rather than widening the match, and which `default_harness` now uses for the registry names it allows. Glob semantics are untouched: `rm*` still spans `rmdir`, `*` still matches everything, the tier order and the `deny` default are unchanged.

docs/cookbook/03-agents-and-tools.md

@@ -546,6 +546,16 @@ Patterns match the **tool name** only, never its arguments. `DENY "run_command"`
 stops the shell tool entirely; it cannot express "deny `rm` but allow `ls`". That
 distinction belongs in a pre-hook, two recipes down.
 
+**A name that is also a pattern.** `fnmatch` reads `[`…`]` as a character class,
+so a tool called `exfil[all]` — or an MCP-style `mcp__srv__do[all]` — is not the
+same string as the pattern that spells it. A `deny` or `ask` rule therefore also
+fires on an **exact literal match**, so pasting a tool's name into a rule refuses
+it whatever characters it holds. That fallback is deliberately not extended to
+`allow`: equality can only ever add a refusal, never a grant. To *allow* one tool
+whose name carries `*`, `?` or `[`, build the rule with
+`PermissionRule.literal(Decision.ALLOW, name)`, which escapes the name instead of
+widening the match.
+
 ---
 
 ## How do I make sure a denied tool is never even offered to the model?

grapharc/harness/permissions.py

@@ -4,10 +4,17 @@
 harness, never by prompts — instructions are advisory, this is not. A broad
 deny always beats a narrower allow (no allowlist exceptions inside a deny),
 matching the semantics that survived contact with reality in Claude Code.
+
+A pattern is a glob, but a DENY or ASK rule also fires on an exact literal
+match, so a rule that simply names a tool refuses it even when the name carries
+fnmatch metacharacters (`exfil[all]`). ALLOW keeps glob-only matching — see
+`PermissionRule.matches` for why the two tiers differ, and
+`PermissionRule.literal` for naming one tool exactly at any tier.
 """
 
 from __future__ import annotations
 
+import glob
 from enum import StrEnum
 from fnmatch import fnmatch
 
@@ -24,9 +31,47 @@ class PermissionDenied(Exception):
     """A tool call was refused by policy (or by an absent/negative approval)."""
 
 
+#: Tiers where an exact literal name is also honoured as a match, on top of the
+#: glob. Restricting a tool can only ever *narrow* what runs, so widening these
+#: two is safe in the direction this module already fails; see `matches`.
+_LITERAL_TIERS = frozenset({Decision.DENY, Decision.ASK})
+
+
 class PermissionRule(BaseModel):
     action: Decision
-    pattern: str  # fnmatch pattern over the tool name
+    pattern: str  # fnmatch pattern over the tool name; DENY/ASK also match literally
+
+    @classmethod
+    def literal(cls, action: Decision, name: str) -> PermissionRule:
+        """A rule matching exactly one tool name, whatever characters it holds.
+
+        `pattern` is a glob, so a name containing `*`, `?` or `[` is not the
+        rule that names it — `exfil[all]` reads as a character class. This
+        escapes the name (`glob.escape`) so the rule means the tool and nothing
+        else. Use it whenever the name comes from a registry rather than from an
+        operator writing a pattern by hand, and especially for ALLOW, where the
+        literal fallback in `matches` deliberately does not apply.
+        """
+        return cls(action=action, pattern=glob.escape(name))
+
+    def matches(self, tool_name: str) -> bool:
+        """Does this rule fire for `tool_name`?
+
+        The glob, plus — for DENY and ASK only — an exact string equality. A
+        tool whose name contains fnmatch metacharacters (`mcp__srv__do[all]`)
+        would otherwise slip past the rule that names it exactly, and the
+        evaluation would fall through to a broader ALLOW: the one place in this
+        tree where a *deny* failed open.
+
+        Bound to DENY/ASK on purpose. Equality can only add rules that refuse or
+        gate a call, never ones that permit it, so it cannot loosen a policy;
+        the same widening on ALLOW could grant a tool the operator never allowed.
+        For an ALLOW rule naming a metacharacter-bearing tool, write it with
+        `PermissionRule.literal`, which escapes rather than widens.
+        """
+        return fnmatch(tool_name, self.pattern) or (
+            self.action in _LITERAL_TIERS and tool_name == self.pattern
+        )
 
 
 class PermissionPolicy(BaseModel):
@@ -42,6 +87,6 @@ class PermissionPolicy(BaseModel):
     def decide(self, tool_name: str) -> Decision:
         for tier in (Decision.DENY, Decision.ASK, Decision.ALLOW):
             for rule in self.rules:
-                if rule.action == tier and fnmatch(tool_name, rule.pattern):
+                if rule.action == tier and rule.matches(tool_name):
                     return tier
         return self.default

grapharc/stdlib.py

@@ -256,8 +256,11 @@ def default_harness(tools: tuple[str, ...], workspace: Any = None) -> Any:
     registry = ToolRegistry()
     for spec in core_tools(Path(workspace or Path.cwd()), include=tools):
         registry.register(spec)
+    # `literal`, not a bare pattern: these names come from a registry, not from
+    # an operator writing globs, and an ALLOW rule is the one tier where a name
+    # read as a pattern could grant more than was asked for.
     policy = PermissionPolicy(
-        rules=[PermissionRule(action=Decision.ALLOW, pattern=name) for name in tools],
+        rules=[PermissionRule.literal(Decision.ALLOW, name) for name in tools],
         default=Decision.DENY,
     )
     return Harness(registry=registry, policy=policy, executor=LocalExecutor())

tests/test_harness_gate.py

@@ -68,6 +68,79 @@ def test_denied_tools_are_never_visible():
     assert visible == ["read"]  # rm's schema is never exposed
 
 
+def test_deny_by_literal_name_beats_a_broad_allow_when_the_name_is_a_glob():
+    """A DENY rule that *is* the tool's name refuses it, metacharacters and all.
+
+    `exfil[all]` reads as a character class to fnmatch, so the rule naming it
+    used to miss, evaluation fell through to `ALLOW "*"`, and the tool both
+    appeared in `visible()` and ran. This is the one place a deny failed open.
+    """
+    name = "exfil[all]"
+    reg = ToolRegistry()
+    reg.register(ToolSpec(name=name, description="dangerous", fn=_echo))
+    policy = _policy([{"action": "deny", "pattern": name}, {"action": "allow", "pattern": "*"}])
+
+    assert policy.decide(name) is Decision.DENY
+    assert [t.name for t in reg.visible(policy)] == []  # never offered to the model
+    with pytest.raises(PermissionDenied):
+        Harness(reg, policy).call(name, {})  # and never runs if it asks anyway
+
+
+def test_ask_by_literal_name_gates_a_glob_shaped_name():
+    """The same widening on ASK: a gate that reads right is a gate that holds."""
+    name = "mcp__srv__do[all]"
+    reg = ToolRegistry()
+    reg.register(ToolSpec(name=name, description="", fn=_echo))
+    policy = _policy([{"action": "ask", "pattern": name}, {"action": "allow", "pattern": "*"}])
+
+    assert policy.decide(name) is Decision.ASK
+    assert [t.name for t in reg.visible(policy)] == [name]  # ASK is not DENY
+    with pytest.raises(PermissionDenied, match="requires approval"):
+        Harness(reg, policy).call(name, {})  # no approval callback
+
+
+def test_allow_stays_glob_only_and_literal_escapes_instead():
+    """Literal equality is bound to DENY/ASK — it may only ever refuse more.
+
+    Widening ALLOW the same way would grant a tool on a pattern the operator
+    wrote as a glob, so `PermissionRule.literal` escapes the name instead.
+    """
+    name = "exfil[all]"
+    glob_rule = PermissionPolicy(rules=[PermissionRule(action=Decision.ALLOW, pattern=name)])
+    assert glob_rule.decide(name) is Decision.DENY  # the DENY default still holds
+
+    literal_rule = PermissionPolicy(rules=[PermissionRule.literal(Decision.ALLOW, name)])
+    assert literal_rule.decide(name) is Decision.ALLOW
+    assert literal_rule.decide("exfila") is Decision.DENY  # and nothing the class covers
+
+
+def test_glob_matching_is_unchanged_by_the_literal_fallback():
+    """Regression: every existing glob semantic, at every tier."""
+    policy = _policy([{"action": "deny", "pattern": "rm*"}, {"action": "allow", "pattern": "*"}])
+    assert policy.decide("rmdir") is Decision.DENY  # prefix glob still spans
+    assert policy.decide("rm") is Decision.DENY
+    assert policy.decide("read_file") is Decision.ALLOW  # "*" still matches everything
+
+    # A glob still matches through every tier, and never only its own text.
+    for action in ("deny", "ask", "allow"):
+        tier = _policy([{"action": action, "pattern": "danger_?"}])
+        assert tier.decide("danger_1") is Decision(action)
+        assert tier.decide("danger_1x") is Decision.DENY  # unmatched -> default
+    # deny -> ask -> allow ordering, independent of rule order
+    ordered = _policy(
+        [
+            {"action": "allow", "pattern": "x_*"},
+            {"action": "ask", "pattern": "x_a*"},
+            {"action": "deny", "pattern": "x_ab*"},
+        ]
+    )
+    assert (ordered.decide("x_abc"), ordered.decide("x_ax"), ordered.decide("x_b")) == (
+        Decision.DENY,
+        Decision.ASK,
+        Decision.ALLOW,
+    )
+
+
 def test_ask_without_approval_fails_closed():
     reg = ToolRegistry()
     reg.register(ToolSpec(name="send", description="", fn=_echo))