Skip to content

Permission deny rules fail open for argument values containing / (matchGlob uses filepath.Match) #3604

Description

@parveshsaini

Description

A configured deny permission whose argument value contains a path separator (/, or \ on Windows)
can silently fail to match, so a tool call the operator explicitly tried to block is not denied.

The matcher matchGlob in pkg/permissions/permissions.go has two paths:

  1. A trailing-* fast path: if the pattern ends in * and the prefix contains no other glob
    metacharacters, it does a plain strings.HasPrefix. This is why shell:cmd=rm* and shell:cmd=sudo*
    correctly block rm -rf /.
  2. Every other pattern falls through to filepath.Match(pattern, value). filepath.Match's * matches
    any run of non-separator characters, so it stops at /.

As a result, a deny pattern with a non-trailing wildcard - e.g. shell:cmd=*rm -rf* - matches
rm -rf tmp but not rm -rf /. The docstring on matchGlob says "we want * to match any characters
including spaces," but that only holds for the fast path; the filepath.Match fallback silently reintroduces
separator-boundary behavior. Because this is a deny rule, the failure direction is fail-open.

Expected Behavior

A deny pattern with a wildcard should match the same command regardless of whether the argument value
happens to contain a /. shell:cmd=*rm -rf* should deny both rm -rf tmp and rm -rf /.

Actual Behavior

shell:cmd=*rm -rf* denies rm -rf tmp but returns Ask for rm -rf / - the command is not blocked.
The decision varies with argument content and with OS (separator is / on Unix, \ on Windows).

Steps to Reproduce

Config-level: configure a deny rule with a non-trailing wildcard and invoke the shell tool with an
argument containing /:

permissions:
  deny:
    - "shell:cmd=*rm -rf*"
  • shell with cmd = "rm -rf tmp" → denied ✅
  • shell with cmd = "rm -rf /" → not denied (falls through to Ask) ❌

Isolated unit reproduction against pkg/permissions (the checker's public API + the matcher):

func TestDenyGlobDoesNotCrossPathSeparator(t *testing.T) {
	checker := NewCheckerFromRules(nil, nil, []string{"shell:cmd=*rm -rf*"})
	assert.Equal(t, Deny, checker.CheckWithArgs("shell", map[string]any{"cmd": "rm -rf tmp"})) // passes
	assert.Equal(t, Deny, checker.CheckWithArgs("shell", map[string]any{"cmd": "rm -rf /"}))   // FAILS: got Ask
}

func TestMatchGlobCrossesPathSeparator(t *testing.T) {
	assert.True(t, matchGlob("*rm -rf*", "rm -rf /"))       // FAILS
	assert.True(t, matchGlob("*/etc/*", "cat /etc/passwd")) // FAILS
}

go test ./pkg/permissions/ -run 'TestDenyGlob...|TestMatchGlob...' -v

Docker Agent version

Docker version 29.5.3, build d1c06ef

OS & terminal

Reproduced on Linux (in a golang:1.26.4 container). The separator-boundary behavior is inherent to filepath.Match and also affects Windows (where the boundary is \), so the result is platform-dependent.

Model used

N/A - this is in the permission matcher; it's independent of the model.

Error output

--- FAIL: TestDenyGlobDoesNotCrossPathSeparator (0.00s)
    Not equal: expected: 2 (Deny)   actual: 0 (Ask)
    Messages: deny rule must also block 'rm -rf /' (the most dangerous case)
--- FAIL: TestMatchGlobCrossesPathSeparator (0.00s)
    Should be true
    Messages: "*rm -rf*" should match "rm -rf /"
FAIL    github.com/docker/docker-agent/pkg/permissions


(Decision enum: `Ask=0, Allow=1, Deny=2`.)

Screenshots

No response

Additional context

  • Impact: in a plain interactive session Ask still prompts, so this is a degraded control rather than
    immediate silent execution - but it becomes a real bypass under --yolo, non-interactive / auto-approval
    runs, or when an overlapping allow rule is present. The same filepath.Match path also breaks allow
    rules, but those fail closed.
  • Trigger: only patterns with a non-trailing wildcard are affected; trailing-only patterns (rm*,
    sudo*) hit the fast path and work, which tends to hide this in casual testing.

Activity

  1. added theissue type on Jul 11, 2026
  2. added
    area/securityAuthentication, authorization, secrets, vulnerabilities
    on Jul 11, 2026
  3. parveshsaini commented on Jul 11, 2026

    @parveshsaini
    ContributorAuthor

    Suggested direction: for argument-value matching, use a matcher whose * spans any character including separators - translate the glob to an anchored, case-insensitive regexp, or use doublestar with separator matching disabled - instead of filepath.Match, and add test covering non-trailing wildcards against values with / (both deny and allow).

    Happy to open a PR with the fix + tests 🚀

  4. BGMLAI commented on Jul 14, 2026

    @BGMLAI

    Command arguments are not filesystem paths, so filepath.Match brings separator semantics that are surprising and platform-dependent here. A separator-agnostic matcher would fix this specific bypass, but typed parsing is safer: identify the executable and structured arguments first, then apply policy to those fields. Please also add an invariant test that a deny rule can never degrade into ask or allow under yolo mode, including slash, backslash, quoting, and chained-command cases.

  5. parveshsaini commented on Jul 14, 2026

    @parveshsaini
    ContributorAuthor

    Command arguments are not filesystem paths, so filepath.Match brings separator semantics that are surprising and platform-dependent here. A separator-agnostic matcher would fix this specific bypass, but typed parsing is safer: identify the executable and structured arguments first, then apply policy to those fields. Please also add an invariant test that a deny rule can never degrade into ask or allow under yolo mode, including slash, backslash, quoting, and chained-command cases.

    Agreed on the framing, command args aren't paths, and inheriting filepath.Match's separator semantics for them is the bug.

    Typed parsing I'd say is right long-term but not a substitute here: you'd still glob-match policy against the parsed fields, and those fields are paths and URLs, so they're still full of / - it relocates this bug rather than removing it. It also reinterprets every existing
    shell:cmd=<glob> rule, which feels like it wants its own design issue rather than riding along on a fail-open fix. shell_grant.go already does the strict reading for safety overrides; widening that is the real proposal. Happy to open an issue if you want it tracked.

    On the yolo invariant, I can't write that test, because Decide short-circuits on yolo before any checker runs, and TestDecide_YoloShortCircuits already pins that down with a deny rule in place. Under --yolo deny isn't degraded, it's never consulted. Whether it should outrank yolo is a fair question (I'd say yes - yolo reads as "stop asking me", not "ignore my denylist"), but that's a behavior change well outside this PR; happy to raise it separately if you think it's worth having :)

    I did take the matrix and apply it where the invariant does hold, at the checker: deny now has to beat an
    overlapping allow and ask across slashes, backslashes, quoting, chaining and $( ) 👍

    @Sayt-0 @BGMLAI

  6. self-assigned this
    on Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

area/securityAuthentication, authorization, secrets, vulnerabilities

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions