diff --git a/metadata/metadata.go b/metadata/metadata.go index 2aec6f2d..ab3372e3 100644 --- a/metadata/metadata.go +++ b/metadata/metadata.go @@ -34,7 +34,7 @@ import ( "io" "math" "os" - "path/filepath" + "path" "slices" "strconv" "strings" @@ -553,7 +553,11 @@ func (role *DelegatedRole) IsDelegatedPath(targetFilepath string) (bool, error) for _, pathPattern := range role.Paths { // A delegated role path may be an explicit path or glob // pattern (Unix shell-style wildcards). - if isTargetInPathPattern(targetFilepath, pathPattern) { + matched, err := isTargetInPathPattern(targetFilepath, pathPattern) + if err != nil { + return false, err + } + if matched { return true, nil } } @@ -570,24 +574,32 @@ func (role *DelegatedRole) IsDelegatedPath(targetFilepath string) (bool, error) } // Determine whether “targetpath“ matches the “pathpattern“. -func isTargetInPathPattern(targetpath string, pathpattern string) bool { +func isTargetInPathPattern(targetpath string, pathpattern string) (bool, error) { // We need to make sure that targetpath and pathpattern are pointing to // the same directory as fnmatch doesn't threat "/" as a special symbol. targetParts := strings.Split(targetpath, "/") patternParts := strings.Split(pathpattern, "/") if len(targetParts) != len(patternParts) { - return false + return false, nil } // Every part in the pathpattern could include a glob pattern, that's why - // each of the target and pathpattern parts should match. + // each of the target and pathpattern parts should match. We use path.Match + // (not filepath.Match) so matching is independent of the host OS: TUF target + // paths are always "/"-separated and "\" is a glob escape, not a separator. + // A malformed pattern surfaces path.ErrBadPattern instead of being silently + // treated as a non-match. for i := 0; i < len(targetParts); i++ { - if ok, _ := filepath.Match(patternParts[i], targetParts[i]); !ok { - return false + ok, err := path.Match(patternParts[i], targetParts[i]) + if err != nil { + return false, err + } + if !ok { + return false, nil } } - return true + return true, nil } // GetRolesForTarget return the names and terminating status of all diff --git a/metadata/metadata_test.go b/metadata/metadata_test.go index 13838644..644caad4 100644 --- a/metadata/metadata_test.go +++ b/metadata/metadata_test.go @@ -28,6 +28,7 @@ import ( "crypto/sha256" "encoding/json" "os" + "path" "path/filepath" "testing" "time" @@ -288,6 +289,18 @@ func TestIsDelegatedPath(t *testing.T) { } } +func TestIsDelegatedPathInvalidPattern(t *testing.T) { + // A malformed glob (unterminated character class) must surface an error + // rather than being silently treated as a non-match, so callers can tell + // "does not match" apart from "invalid pattern". + role := &DelegatedRole{ + Paths: []string{"targets/["}, + } + ok, err := role.IsDelegatedPath("targets/anything") + assert.False(t, ok) + assert.ErrorIs(t, err, path.ErrBadPattern) +} + func TestClearSignatures(t *testing.T) { meta := Root() // verify signatures is empty