Skip to content

Commit 82dec08

Browse files
committed
Surface invalid delegation path patterns and match OS-independently
isTargetInPathPattern discarded the error from filepath.Match, so a malformed delegation path pattern (like an unterminated character class "targets/[") was silently treated as a non-match. Callers could not tell "does not match" apart from "invalid pattern". It also used filepath.Match, whose separator handling is OS-dependent, while TUF target paths are always "/"-separated. Switch to path.Match, propagate the error through IsDelegatedPath, and add a regression test. Refs #747. Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
1 parent 0b10964 commit 82dec08

2 files changed

Lines changed: 33 additions & 8 deletions

File tree

metadata/metadata.go

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import (
3434
"io"
3535
"math"
3636
"os"
37-
"path/filepath"
37+
"path"
3838
"slices"
3939
"strconv"
4040
"strings"
@@ -540,7 +540,11 @@ func (role *DelegatedRole) IsDelegatedPath(targetFilepath string) (bool, error)
540540
for _, pathPattern := range role.Paths {
541541
// A delegated role path may be an explicit path or glob
542542
// pattern (Unix shell-style wildcards).
543-
if isTargetInPathPattern(targetFilepath, pathPattern) {
543+
matched, err := isTargetInPathPattern(targetFilepath, pathPattern)
544+
if err != nil {
545+
return false, err
546+
}
547+
if matched {
544548
return true, nil
545549
}
546550
}
@@ -557,24 +561,32 @@ func (role *DelegatedRole) IsDelegatedPath(targetFilepath string) (bool, error)
557561
}
558562

559563
// Determine whether “targetpath“ matches the “pathpattern“.
560-
func isTargetInPathPattern(targetpath string, pathpattern string) bool {
564+
func isTargetInPathPattern(targetpath string, pathpattern string) (bool, error) {
561565
// We need to make sure that targetpath and pathpattern are pointing to
562566
// the same directory as fnmatch doesn't threat "/" as a special symbol.
563567
targetParts := strings.Split(targetpath, "/")
564568
patternParts := strings.Split(pathpattern, "/")
565569
if len(targetParts) != len(patternParts) {
566-
return false
570+
return false, nil
567571
}
568572

569573
// Every part in the pathpattern could include a glob pattern, that's why
570-
// each of the target and pathpattern parts should match.
574+
// each of the target and pathpattern parts should match. We use path.Match
575+
// (not filepath.Match) so matching is independent of the host OS: TUF target
576+
// paths are always "/"-separated and "\" is a glob escape, not a separator.
577+
// A malformed pattern surfaces path.ErrBadPattern instead of being silently
578+
// treated as a non-match.
571579
for i := range targetParts {
572-
if ok, _ := filepath.Match(patternParts[i], targetParts[i]); !ok {
573-
return false
580+
ok, err := path.Match(patternParts[i], targetParts[i])
581+
if err != nil {
582+
return false, err
583+
}
584+
if !ok {
585+
return false, nil
574586
}
575587
}
576588

577-
return true
589+
return true, nil
578590
}
579591

580592
// GetRolesForTarget return the names and terminating status of all

metadata/metadata_test.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"crypto/sha256"
2929
"encoding/json"
3030
"os"
31+
"path"
3132
"path/filepath"
3233
"testing"
3334
"time"
@@ -288,6 +289,18 @@ func TestIsDelegatedPath(t *testing.T) {
288289
}
289290
}
290291

292+
func TestIsDelegatedPathInvalidPattern(t *testing.T) {
293+
// A malformed glob (unterminated character class) must surface an error
294+
// rather than being silently treated as a non-match, so callers can tell
295+
// "does not match" apart from "invalid pattern".
296+
role := &DelegatedRole{
297+
Paths: []string{"targets/["},
298+
}
299+
ok, err := role.IsDelegatedPath("targets/anything")
300+
assert.False(t, ok)
301+
assert.ErrorIs(t, err, path.ErrBadPattern)
302+
}
303+
291304
func TestClearSignatures(t *testing.T) {
292305
meta := Root()
293306
// verify signatures is empty

0 commit comments

Comments
 (0)