-
Notifications
You must be signed in to change notification settings - Fork 54
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
Update wildcard pattern behavior #301
base: master
Are you sure you want to change the base?
Update wildcard pattern behavior #301
Conversation
This commit adds explicit examples of targets that are not matched by a wildcard. It also adds a note warning users that incorrect assumptions about wildcard behavior can potentially lead to an untrusted role signing for a target. Signed-off-by: Aditya Sirish <aditya@saky.in>
tuf-spec.md
Outdated
* a <a>PATHPATTERN</a> of `"foo/*"` matches `"foo/bar.tgz"` but not | ||
`"foo/baz/bar.tgz"`, `"foo/bar/baz/bar.tgz"`, and so on. | ||
|
||
Note: It is important to understand the functioning of path patterns to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd also list a note to the opposite effect first: remind the reader why this behaviour is there (not for arbitrary reasons).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tweaked the text a bit, does it help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, sorry. First, please briefly explain why the current patterns work the way they do, and as such, they are optimised for those anticipated use cases. Your use cases here are outside of that, and should thus fall under a second paragraph.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not the best person to discuss why the current patterns work the way they do as I don't actually have that context. @mnm678 can you help with some text here? Thanks!
Signed-off-by: Aditya Sirish <aditya@saky.in>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
waiting on the workflow run, but lgtm
Signed-off-by: Aditya Sirish <aditya@saky.in>
I bumped the patch version number. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, thank you. Approved as is, but I wonder if we should move the coment on wildcards not apply recursively above the examples?
It is somewhat explicit because we state that we follow shell style globbing, but for many shell is synonymous with bash, which has globstar.
security. For example, an assumption that `"foo/*"` applies recursively to | ||
all files in subdirectories of `foo` in a terminating delegation could allow | ||
a subsequent delegated role that should not be trusted to sign for a target | ||
in a subdirectory of `foo`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great clarification to add, thanks! This point on wildcards not applying recursively feels important enough that I wonder whether we should raise it above the examples so that it's a little more prominent, WDYT?
This PR adds explicit examples of targets that are not matched by a wildcard. It also adds a note warning users that incorrect assumptions about wildcard behavior can potentially lead to an untrusted role signing for a target.
Related: theupdateframework/go-tuf#590