-
Notifications
You must be signed in to change notification settings - Fork 311
cue/interpreter/embed: globs in hidden directories are ignored #3889
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
Comments
DO NOT REVIEW DO NOT SUBMIT WIP because of cue-lang/cue#3889. Preview-Path: /docs/draft/cldd/validating-several-github-actions-files/ Preview-Path: /docs/draft/cldd/checking-existing-github-actions-files/ Signed-off-by: Jonathan Matthews <[email protected]> Change-Id: I90511bcb8cf4d1f3f8253f07a8a2a39b7fff752f Dispatch-Trailer: {"type":"trybot","CL":1213359,"patchset":2,"ref":"refs/changes/59/1213359/2","targetBranch":"master"}
DO NOT REVIEW DO NOT SUBMIT WIP because of cue-lang/cue#3889. Preview-Path: /docs/draft/cldd/validating-several-github-actions-files/ Preview-Path: /docs/draft/cldd/checking-existing-github-actions-files/ Signed-off-by: Jonathan Matthews <[email protected]> Change-Id: I90511bcb8cf4d1f3f8253f07a8a2a39b7fff752f Dispatch-Trailer: {"type":"trybot","CL":1213359,"patchset":3,"ref":"refs/changes/59/1213359/3","targetBranch":"master"}
I note that this behaviour is intended, as per https://cuelang.org/cl/1196775. Without requiring that CL's mention of an option to vary embed's behaviour such that hidden directories can be permitted, perhaps a reasonable compromise that doesn't downgrade security could be for globs not to include hidden directories (or files) unless no wildcard elements of the glob match a hidden path component. In other words, to permit hidden directories (and files?) so long as each dot-prefixed path-/file-name element is explicitly mentioned in the |
It is indeed behaving as documented there, but I believe it is overly restrictive. I propose relaxing the logic as follows:
Thus the following will result in embedded files (assuming the corresponding files exist on disk) where they currently do not:
|
@mvdan points out the following wording from the bash manpage as an alternative to my wording in the second bullet above:
|
This adds a draft guide demonstrating the validation of multiple configuration files against the same GitHub Actions schema using file embedding; and links to it from the "preceding" guide in the narrative flow. This new guide will be used as a template for other curated modules whose technologies support the existence of multiple config files. For technologies whose config files' canonical filesystem locations includes a "." prefix (such as GitHub Actions) cue-lang/cue#3889 means that file embedding has to use the "file" parameter rather than "glob". This leads to some less optimal CUE, which is acceptable whilst a relaxation of the rules causing that issue is considered. DO NOT SUBMIT until this change contains more than just GitHub Actions guides. Preview-Path: /docs/draft/cldd/validating-several-github-actions-files/ Preview-Path: /docs/draft/cldd/checking-existing-github-actions-files/ Signed-off-by: Jonathan Matthews <[email protected]> Change-Id: I90511bcb8cf4d1f3f8253f07a8a2a39b7fff752f Dispatch-Trailer: {"type":"trybot","CL":1213359,"patchset":4,"ref":"refs/changes/59/1213359/4","targetBranch":"master"}
After some good offline exchanges with @rogpeppe and @mvdan here is a hopefully complete summary:
Looking at the benefits of moving forward on this basis:
Looking at the costs of moving forward on this basis:
Assuming there is not disagreement with the above, we should move forward with fixing the bug presented in this issue, and carefully documenting that the rules regarding |
What version of CUE are you using (
cue version
)?Does this issue reproduce with the latest stable release?
Yes, v0.12.1 fails identically.
What did you do?
What did you expect to see?
A passing test.
What did you see instead?
The text was updated successfully, but these errors were encountered: