Skip to content

Commit

Permalink
i/prompting: implement path pattern matching (#13866)
Browse files Browse the repository at this point in the history
Path pattern matching is implemented via the doublestar package, which
emulates bash's globstar matching. Patterns may include '*' wildcard
characters (which match any number of non-separator characters), '**'
doublestars (which match zero or more subdirectories), '?' wildcard
characters (which match exactly one non-separator character), and nested
groups delimited by '{' and '}'. Notably, path patterns are *not* allowed
to have character classes delimited by '[' and ']', nor inverted
classes of the form "[^abc]".

There is a limit on the number of groups allowed in path patterns, but
up to that limit, groups may be arbitrarily nested or sequential.

Signed-off-by: Oliver Calder <[email protected]>

i/prompting: fix typo and add notes to remove test boilerplate

Signed-off-by: Oliver Calder <[email protected]>

i/prompting: use separate test suite for patterns

Signed-off-by: Oliver Calder <[email protected]>

i/prompting: improve unit test coverage

Signed-off-by: Oliver Calder <[email protected]>

* i/prompting: count and validate true number of expanded patterns

Rather than counting the number of groups and using it as a heuristic
for the number of patterns into which a given path pattern will expand,
instead compute the true number of expanded patterns and compare it
against a set limit.

Signed-off-by: Oliver Calder <[email protected]>

* i/prompting: implement path pattern checks in constraints

Signed-off-by: Oliver Calder <[email protected]>

* i/prompting: throw error if group depth exceeds maximum expanded patterns

Signed-off-by: Oliver Calder <[email protected]>

* packaging: add doublestar dependency for prompting pattern matching

Signed-off-by: Oliver Calder <[email protected]>

* i/prompting: remove standalone path pattern validation

Signed-off-by: Oliver Calder <[email protected]>

---------

Signed-off-by: Oliver Calder <[email protected]>
  • Loading branch information
olivercalder authored Jun 17, 2024
1 parent 8128ed2 commit 6905775
Show file tree
Hide file tree
Showing 8 changed files with 460 additions and 28 deletions.
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ go 1.18
replace maze.io/x/crypto => github.com/snapcore/maze.io-x-crypto v0.0.0-20190131090603-9b94c9afe066

require (
github.com/bmatcuk/doublestar/v4 v4.6.1
github.com/canonical/go-efilib v0.4.0
github.com/canonical/go-sp800.90a-drbg v0.0.0-20210314144037-6eeb1040d6c3 // indirect
github.com/canonical/go-tpm2 v0.0.0-20210827151749-f80ff5afff61
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
github.com/bmatcuk/doublestar/v4 v4.6.1 h1:FH9SifrbvJhnlQpztAx++wlkk70QBf0iBWDwNy7PA4I=
github.com/bmatcuk/doublestar/v4 v4.6.1/go.mod h1:xBQ8jztBU6kakFMg+8WGxn0c6z1fTSPVIjEY1Wr7jzc=
github.com/canonical/go-efilib v0.4.0 h1:2ee5pvhIZ+g1EO4HxFE/owBgs5Up2g7dw1+Ls9/fiSs=
github.com/canonical/go-efilib v0.4.0/go.mod h1:9b2PNAuPcZsB76x75/uwH99D8CyH/A2y4rq1/+bvplg=
github.com/canonical/go-sp800.108-kdf v0.0.0-20210314145419-a3359f2d21b9 h1:USzKjrfWo/ESzozv2i3OMM7XDgxrZRvaHFrKkIKRtwU=
Expand Down
8 changes: 1 addition & 7 deletions interfaces/prompting/constraints.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ type Constraints struct {
// ValidateForInterface returns nil if the constraints are valid for the given
// interface, otherwise returns an error.
func (c *Constraints) ValidateForInterface(iface string) error {
// TODO: change to this once PR #13866 is merged:
// if err := ValidatePathPattern(c.PathPattern); err != nil {
// return err
// }
return c.validatePermissions(iface)
}

Expand Down Expand Up @@ -81,9 +77,7 @@ func (c *Constraints) validatePermissions(iface string) error {
//
// If the constraints or path are invalid, returns an error.
func (c *Constraints) Match(path string) (bool, error) {
// TODO: change to this once PR #13866 is merged:
// return PathPatternMatch(c.PathPattern, path)
return true, nil
return PathPatternMatch(c.PathPattern, path)
}

// RemovePermission removes every instance of the given permission from the
Expand Down
30 changes: 9 additions & 21 deletions interfaces/prompting/constraints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ import (

. "gopkg.in/check.v1"

// TODO: add this once PR #13730 is merged:
// doublestar "github.com/bmatcuk/doublestar/v4"
doublestar "github.com/bmatcuk/doublestar/v4"

"github.com/snapcore/snapd/interfaces/prompting"
"github.com/snapcore/snapd/sandbox/apparmor/notify"
Expand All @@ -44,17 +43,10 @@ func (s *constraintsSuite) TestConstraintsValidateForInterface(c *C) {
}{
{
"foo",
"invalid/path",
"/invalid/path",
[]string{"read"},
"unsupported interface.*",
},
// TODO: add this once PR #13730 is merged:
// {
// "home",
// "invalid/path",
// []string{"read"},
// "invalid path pattern.*",
// },
{
"home",
"/valid/path",
Expand Down Expand Up @@ -151,12 +143,11 @@ func (*constraintsSuite) TestConstraintsMatch(c *C) {
"/home/test/Documents/foo.txt",
true,
},
// TODO: add this once PR #13730 is merged:
// {
// "/home/test/Documents/foo",
// "/home/test/Documents/foo.txt",
// false,
// },
{
"/home/test/Documents/foo",
"/home/test/Documents/foo.txt",
false,
},
}
for _, testCase := range cases {
constraints := &prompting.Constraints{
Expand All @@ -176,11 +167,8 @@ func (s *constraintsSuite) TestConstraintsMatchUnhappy(c *C) {
Permissions: []string{"read"},
}
matches, err := badConstraints.Match(badPath)
// TODO: change to this once PR #13730 is merged:
// c.Check(err, Equals, doublestar.ErrBadPattern)
// c.Check(matches, Equals, false)
c.Check(err, Equals, nil)
c.Check(matches, Equals, true)
c.Check(err, Equals, doublestar.ErrBadPattern)
c.Check(matches, Equals, false)
}

func (s *constraintsSuite) TestConstraintsRemovePermission(c *C) {
Expand Down
66 changes: 66 additions & 0 deletions interfaces/prompting/patterns.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// -*- Mode: Go; indent-tabs-mode: t -*-

/*
* Copyright (C) 2024 Canonical Ltd
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 3 as
* published by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
*/

package prompting

import (
"strings"

doublestar "github.com/bmatcuk/doublestar/v4"
)

// PathPatternMatch returns true if the given pattern matches the given path.
//
// The pattern should not contain groups, and should likely have been an output
// of ExpandPathPattern.
//
// Paths to directories are received with trailing slashes, but we don't want
// to require the user to include a trailing '/' if they want to match
// directories (and end their pattern with `{,/}` if they want to match both
// directories and non-directories). Thus, we want to ensure that patterns
// without trailing slashes match paths with trailing slashes. However,
// patterns with trailing slashes should not match paths without trailing
// slashes.
//
// The doublestar package has special cases for patterns ending in `/**` and
// `/**/`: `/foo/**`, and `/foo/**/` both match `/foo` and `/foo/`. We want to
// override this behavior to make `/foo/**/` not match `/foo`. We also want to
// override doublestar to make `/foo` match `/foo/`.
func PathPatternMatch(pattern string, path string) (bool, error) {
// Check the usual doublestar match first, in case the pattern is malformed
// and causes an error, and return the error if so.
matched, err := doublestar.Match(pattern, path)
if err != nil {
return false, err
}
// No matter if doublestar matched, return false if pattern ends in '/' but
// path is not a directory.
if strings.HasSuffix(pattern, "/") && !strings.HasSuffix(path, "/") {
return false, nil
}
if matched {
return true, nil
}
if strings.HasSuffix(pattern, "/") {
return false, nil
}
// Try again with a '/' appended to the pattern, so patterns like `/foo`
// match paths like `/foo/`.
return doublestar.Match(pattern+"/", path)
}
Loading

0 comments on commit 6905775

Please sign in to comment.