fix(permissions): match glob wildcards across path separators

[parveshsaini]

Closes #3604

What

matchGlob in pkg/permissions falls back to filepath.Match for any pattern that isn't a plain trailing
wildcard. filepath.Match's * and ? match only non-separator characters, so they stop at / (and at
\ on Windows). This translates the glob to an anchored regexp whose */? span any character instead.

Why

Permission patterns match argument values — shell commands, file paths, URLs — which routinely contain /.
Because of the separator boundary, a rule with a non-trailing wildcard silently fails to match once the
value contains a /:

permissions:
  deny:
    - "shell:cmd=*rm -rf*"

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

For a deny rule this is the unsafe direction — it fails open. In an interactive session the fall-through
to Ask still prompts, so the practical exposure is a degraded control; it becomes a real bypass under
--yolo, non-interactive / auto-approval runs, or when an overlapping allow rule is present. The same
boundary also silently breaks allow rules, though those fail closed.

The existing docstring already stated the intent — "for argument matching we want * to match any characters
including spaces" — but that only held for the trailing-wildcard fast path (sudo*), which is why casual
testing with trailing-only patterns never surfaced this.

How

• Replace the filepath.Match fallback in matchGlob with globToRegexp + regexp.MatchString.
• globToRegexp translates a filepath.Match-style glob into an anchored regexp: * → .*, ? → .,
  character classes ([...]) copied through (bracket syntax is shared with regexp), everything else matched
  literally via regexp.QuoteMeta.
• The trailing-* prefix-match fast path and the case-insensitive lowercasing are unchanged.
• No new dependencies (regexp is stdlib; path/filepath import dropped).

This keeps all existing matchGlob semantics (*, ?, [...], case-insensitivity) — only the separator
boundary changes.

Tests

• New TestDenyGlobCrossesPathSeparator: end-to-end via CheckWithArgs, asserting a *rm -rf* deny rule
  blocks both rm -rf tmp and rm -rf /.
• Extended the TestMatchGlob table with separator-crossing cases (*rm -rf*, */etc/*, a?c vs a/c).

These fail on main and pass with the fix. Full pkg/permissions suite, go vet, gofmt, and
golangci-lint are all green (Go 1.26.4).

--- PASS: TestDenyGlobCrossesPathSeparator (0.00s)
--- PASS: TestMatchGlob (0.00s)
ok  	github.com/docker/docker-agent/pkg/permissions

[Sayt-0]

The diagnosis in #3604 is correct and the glob-to-regexp translation is the right direction: RE2 is linear-time (no ReDoS), the \A/\z anchoring is right, and the fast path is preserved. Two correctness issues in globToRegexp block the merge; both have small fixes, detailed in the inline comments.

#	Finding	Severity	Effect
1	Translated regexp lacks the (?s) flag, so . and .* stop at \n	blocker	multi-line commands still bypass non-trailing-wildcard deny rules; strict regression vs main for newline-containing values
2	Byte-wise QuoteMeta(string(c)) corrupts multi-byte UTF-8 runes	blocker	patterns containing non-ASCII stop matching (café no longer matches café)
3	\ escape semantics drift outside character classes	minor	foo\* matched literal foo* on main (non-Windows), now matches foo\ + anything
4	Classes copied verbatim expose RE2 Perl classes	minor	[\d] meant literal d under filepath.Match, now means any digit
5	Regexp compiled on every matchGlob call	nit	avoidable, patterns are static per Checker

Finding 1 matters most: it recreates the exact failure class this PR sets out to fix, with the boundary moved from / to \n.

Pre-existing and out of scope, noted for a possible follow-up: a pattern that fails to compile silently disables its rule (return false, same direction as ErrBadPattern on main), so a typo in a deny rule fails open; validating patterns at config load would surface this.

[Sayt-0]

regexp.Compile runs on every matchGlob call, i.e. per pattern per tool call (tool name plus each argument). Patterns are static for a Checker's lifetime, so regexps could be compiled once at construction, or memoized in a package-level sync.Map keyed by the lowered pattern. Fine as a follow-up; not blocking.

[parveshsaini]

Did it rather than defer, it was small 😅
Package-level sync.Map keyed by the lowered pattern, so a rule compiles once. Cache is bounded by the number of configured rules since patterns come from config, not from tool input.

[Sayt-0]

In Go regexp, . does not match \n unless the s flag is set, so the translated wildcards stop at newlines exactly the way filepath.Match stopped at /. Shell commands are routinely multi-line (heredocs, && chains), so a non-trailing-wildcard deny rule still fails open for them, and some values that matched on main stop matching:

pattern	value	main	this branch
*rm -rf*	"echo hi\nrm -rf /"	no match	no match (still fails open)
*b*	"a\nb"	match	no match (regression)
a?b	"a\nb"	match	no match (regression)

(filepath.Match wildcards stop only at path separators; they do cross \n, hence the regression rows.)

Fix: b.WriteString(`(?s)\A`). Included in the full rewrite suggested in the comment below on the default: branch.

[parveshsaini]

Added your regression rows: {"*b*", "a\nb", true} and {"a?b", "a\nb", true} (both were regressions vs main),
plus *rm -rf* against "echo hi\nrm -rf /", and a multi-line case in the deny test since that's the whole point of the PR.

[Sayt-0]

Two semantic notes on copying class bodies verbatim:

• RE2 accepts more inside [...] than glob: [\d] meant literal d under filepath.Match but is the digit class in RE2 (d no longer matches, 5 does). Perl/POSIX classes silently leak into glob syntax. Worth either escaping \ inside copied classes or documenting the extension.
• The leading-]-is-literal rule ([]a], [^]]) is POSIX glob behavior, not filepath.Match behavior (main returned ErrBadPattern, so no match). Arguably an improvement, but the PR description's "keeps all existing matchGlob semantics" does not hold for these; a line in the doc comment would help.

[parveshsaini]

Both fixed rather than documented. Class bodies are translated member-by-member now: glob \x escapes resolve
to the literal, and the other members get escaped, so [\d] is the literal d and [[:alpha:]] can't smuggle
in a POSIX class. And I dropped my leading-]-is-literal rule , you're right it was POSIX, not
filepath.Match, so []a] is back to matching nothing like it did under ErrBadPattern. Table has [\d]
against both d and 5.

[Sayt-0]

c is a single byte. For a multi-byte UTF-8 rune, string(c) converts each byte to its own (wrong) rune, so the compiled regexp contains mojibake and literal non-ASCII patterns stop matching:

pattern	value	main	this branch
café	café	match	no match (emitted regexp: \Acafé\z)
*café*	xx café yy	match	no match

Byte-wise scanning itself is safe (all glob metacharacters are ASCII, UTF-8 continuation bytes are >= 0x80); the fix is to accumulate literal runs and quote whole substrings instead of one byte at a time. Suggested replacement (also includes the (?s) fix):

func globToRegexp(pattern string) (*regexp.Regexp, error) {
	var b strings.Builder
	// (?s): argument values are often multi-line, "." must match "\n" too.
	b.WriteString(`(?s)\A`)
	lit := 0
	flush := func(end int) {
		if lit < end {
			b.WriteString(regexp.QuoteMeta(pattern[lit:end]))
		}
	}
	for i := 0; i < len(pattern); {
		switch pattern[i] {
		case '*':
			flush(i)
			b.WriteString(`.*`)
			i++
			lit = i
		case '?':
			flush(i)
			b.WriteByte('.')
			i++
			lit = i
		case '[':
			if end := classEnd(pattern, i); end > 0 {
				flush(i)
				b.WriteString(pattern[i : end+1])
				i = end + 1
				lit = i
				continue
			}
			// Unterminated class: "[" stays in the literal run.
			i++
		default:
			i++
		}
	}
	flush(len(pattern))
	b.WriteString(`\z`)
	return regexp.Compile(b.String())
}

This variant passes the existing TestMatchGlob table, the new separator-crossing cases, and the newline/UTF-8 cases from this review.

[parveshsaini]

Good catch! string(c) on a byte was just wrong. Took your rewrite , accumulating literal runs and quoting
whole substrings. Kept the byte-wise scan for the reasons you gave (metacharacters are all ASCII, continuation
bytes are >= 0x80) and noted that in the doc comment so the next reader doesn't "fix" it back. café and
*café* are in the table now.

[Sayt-0]

classEnd honors \] escapes inside a class, but outside classes \ is now always a literal backslash. On main, filepath.Match treated \ as an escape on non-Windows (foo\* matched literal foo*, and the fast-path comment mentions exactly that case) and as a literal on Windows. This branch makes it a literal everywhere, which is a defensible unification but is a behavior change on non-Windows. Options:

1. Handle \x outside classes as literal x (one extra case '\\': in the translator), keeping non-Windows main behavior and making escapes work consistently inside and outside classes.
2. Keep backslash-as-literal and document the divergence in the matchGlob doc comment.

Either works; the current state (escape inside classes, literal outside) is the one combination that is hard to explain.

[parveshsaini]

Went with option 1. \x outside a class is now a literal x, so escapes behave the same on both sides of a
bracket and foo\* matches literal foo* again like it did on main (non-Windows). You were right that the
old split , escape inside, literal outside , was the one combination nobody could explain. The remaining
Windows divergence (\ was a literal there) is deliberate and now in the doc comment.

[Sayt-0]

The new rows cover the / boundary but not the two other boundaries the rewrite is sensitive to. Suggested additions:

// "*" and "?" must also span newlines: shell commands are often multi-line.
{"*rm -rf*", "echo hi\nrm -rf /", true},
{"a?b", "a\nb", true},
// Multi-byte UTF-8 in patterns must keep matching literally.
{"café", "café", true},
{"*café*", "xx café yy", true},

All four rows fail on this branch as-is and pass with the fixes from the other comments.

[parveshsaini]

All four added, plus {"*b*", "a\nb", true} and the escape/class rows (foo\* vs foo*/foobar, [\d] vs
d/5). Also extended TestDenyGlobCrossesPathSeparator with the multi-line command.

Confirmed they fail on the previous commit and pass now 👍

[parveshsaini]

Hey @Sayt-0! I've addressed the requested changes here. Would appreciate a quick review whenever you get the chance!

Many Thanks 🚀

[Sayt-0]

LGTM ! Thank you for the contribution ! One more thing : can you sign your commits ?

[parveshsaini]

LGTM ! Thank you for the contribution ! One more thing : can you sign your commits ?

Many Thanks @Sayt-0! I just signed them 🚀