Skip to content

permissions: a DENY rule naming a tool literally fails open when the name contains fnmatch metacharacters #59

Description

@Shashankss1205

What happens

PermissionPolicy.decide matches rules with fnmatch(tool_name, rule.pattern). The pattern is documented as a glob, but the obvious way to write a rule — paste the tool's exact name — silently stops matching as soon as that name contains [], because fnmatch reads it as a character class. The rule then does not fire, evaluation falls through to whatever comes next (typically a broad ALLOW "*"), and the tool runs.

Repro (verified end to end, not just at the policy layer)

from grapharc.harness.core import Harness
from grapharc.harness.tools import ToolRegistry, ToolSpec
from grapharc.harness.permissions import Decision, PermissionPolicy, PermissionRule as R

NAME = "exfil[all]"
reg = ToolRegistry()
reg.register(ToolSpec(name=NAME, description="dangerous", fn=lambda **kw: "DID IT"))

policy = PermissionPolicy(rules=[
    R(action=Decision.DENY, pattern=NAME),   # operator: "deny this exact tool"
    R(action=Decision.ALLOW, pattern="*"),
])

print([s.name for s in reg.visible(policy)])   # ['exfil[all]']  <- offered to the model
print(Harness(reg, policy, workspace=".").call(NAME, {}))   # 'DID IT'  <- it ran

Observed: the tool is listed as visible to the model and executes.
Expected: PermissionDenied, and absent from visible().

decide() alone shows the same for "mcp__srv__do[all]". Note "tool?x" happens to still deny (a ? glob matches the literal ?), which makes the failure mode inconsistent as well as silent.

Why it matters

This is the one place in the tree where a deny decision fails open. Everywhere else the project fails closed on purpose — an unmatched tool defaults to DENY, an unregistered kind is refused, an unreachable backend raises. Here the operator writes a rule that reads exactly right, gets no error, no warning, and no deny. visible() compounds it by advertising the tool to the model, so the model is actively invited to call the thing the operator forbade.

Bracketed tool names are not hypothetical for MCP-style namespaced tools, and the operator has no reason to suspect their tool name is also a pattern.

What to consider

  • Fail closed on ambiguity: if a DENY rule's pattern contains glob metacharacters and also exactly equals a registered tool name, treat it as a literal. Or, more simply, match a rule when fnmatch(name, pattern) or name == pattern — a literal equality can only ever add denials, never remove them. (Careful: applying the same widening to ALLOW rules would loosen policy, so bind it to DENY/ASK only, or to all tiers only if the equality is exact.)
  • Or refuse the ambiguity at construction: validate patterns when the policy is built and raise on a DENY pattern that matches no registered tool but equals one literally.
  • Or escape at the edge: provide PermissionRule.literal(name) that stores glob.escape(name), and document pattern as glob-only.
  • Whatever the choice, visible() and decide() must agree, and there should be a test with a bracketed tool name in both directions.

Out of scope

The tier ordering (deny → ask → allow) and the DENY default are both correct and should not change.

Acceptance criteria

A tool named exfil[all] denied by a rule whose pattern is exactly exfil[all] is refused by Harness.call and absent from visible(); existing glob behaviour (rm* denying rmdir) is unchanged.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinghelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions