Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible bug in isTargetInPathPattern #590

Closed
adityasaky opened this issue Jan 31, 2024 · 2 comments
Closed

Possible bug in isTargetInPathPattern #590

adityasaky opened this issue Jan 31, 2024 · 2 comments

Comments

@adityasaky
Copy link
Contributor

The implementation for this helper is here:

go-tuf/metadata/metadata.go

Lines 535 to 554 in 9d57731

// Determine whether “targetpath“ matches the “pathpattern“.
func isTargetInPathPattern(targetpath string, pathpattern string) bool {
// 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
}
// Every part in the pathpattern could include a glob pattern, that's why
// each of the target and pathpattern parts should match.
for i := 0; i < len(targetParts); i++ {
if ok, _ := filepath.Match(patternParts[i], targetParts[i]); !ok {
return false
}
}
return true
}

If I'm reading this right, this helper is responsible for identifying if a delegation pattern matches a target path. However, it incorrectly says a pattern like foo/* does not match foo/bar/foobar.txt. The same pattern does correctly match foo/foobar.txt. This is because both the pattern and the target path are split into their components using the separator, and if they don't have the same number of components, the helper returns false.

See: https://go.dev/play/p/6Mswjm_fM-4

@adityasaky
Copy link
Contributor Author

After a discussion with @rdimitrov, @trishankatdatadog, and @mnm678, this is not an issue but likely deserving of some further clarification in the docs and in the TUF spec. See: https://theupdateframework.github.io/specification/latest/index.html#file-formats-targets for a description of how patterns work.

I'm going to submit some updated text to the specification, but this issue can be closed. :)

@trishankatdatadog
Copy link
Member

IOW, the current behaviour is intended to be a feature not a bug in TUF 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants