fix(permissions): match glob wildcards across path separators#3605
Open
parveshsaini wants to merge 1 commit into
Open
fix(permissions): match glob wildcards across path separators#3605parveshsaini wants to merge 1 commit into
parveshsaini wants to merge 1 commit into
Conversation
Signed-off-by: parveshsaini <parveshsaini619@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3604
What
matchGlobinpkg/permissionsfalls back tofilepath.Matchfor any pattern that isn't a plain trailingwildcard.
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
/:shellwithcmd = "rm -rf tmp"→ denied ✅shellwithcmd = "rm -rf /"→ not denied (falls through toAsk) ❌For a
denyrule this is the unsafe direction — it fails open. In an interactive session the fall-throughto
Askstill prompts, so the practical exposure is a degraded control; it becomes a real bypass under--yolo, non-interactive / auto-approval runs, or when an overlappingallowrule is present. The sameboundary also silently breaks
allowrules, though those fail closed.The existing docstring already stated the intent — "for argument matching we want
*to match any charactersincluding spaces" — but that only held for the trailing-wildcard fast path (
sudo*), which is why casualtesting with trailing-only patterns never surfaced this.
How
filepath.Matchfallback inmatchGlobwithglobToRegexp+regexp.MatchString.globToRegexptranslates afilepath.Match-style glob into an anchored regexp:*→.*,?→.,character classes (
[...]) copied through (bracket syntax is shared with regexp), everything else matchedliterally via
regexp.QuoteMeta.*prefix-match fast path and the case-insensitive lowercasing are unchanged.regexpis stdlib;path/filepathimport dropped).This keeps all existing
matchGlobsemantics (*,?,[...], case-insensitivity) — only the separatorboundary changes.
Tests
TestDenyGlobCrossesPathSeparator: end-to-end viaCheckWithArgs, asserting a*rm -rf*deny ruleblocks both
rm -rf tmpandrm -rf /.TestMatchGlobtable with separator-crossing cases (*rm -rf*,*/etc/*,a?cvsa/c).These fail on
mainand pass with the fix. Fullpkg/permissionssuite,go vet,gofmt, andgolangci-lintare all green (Go 1.26.4).