This repository was archived by the owner on Mar 29, 2022. It is now read-only.
(WIP) Rewrite common.scrub_filename: meaningful errors, better doc#131
Open
awwad wants to merge 1 commit intouptane:developfrom
Open
(WIP) Rewrite common.scrub_filename: meaningful errors, better doc#131awwad wants to merge 1 commit intouptane:developfrom
awwad wants to merge 1 commit intouptane:developfrom
Conversation
and be more readable and provide a better docstring.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
common.scrub_filenameis used to ensure that vehicle identifiers provided to the Director can be used as directories for the vehicle repositories for those vehicles. A similar function exists in the Primary module,primary.enforce_jail, which is actually security-relevant.I didn't expect either bit of code (
common.scrub_filename) to stick around: I wasn't surescrub_filenamewas important (as it's not really a security issue, I don't think), and I expected to firm upenforce_jail(which is a security issue) but after thinking about it, while I think scrub_filename falls under the category of protecting the code from itself and isn't strictly necessary, I think it is still better to retain this than to dispose of it. Unfortunately, primary has very slightly different requirements for primary.enforce_jail, so they can't really be the same piece of code, unless I use an option for excluding '/' or '' characters. /:enforce_jailmatters more because the Primary receives instructions to download filepaths that the Director specifies, and a malicious Director + any compromised Image Repository delegated role might be able to cause the Primary to overwrite arbitrary files in unexpected places, or place new files in unexpected places.TODO for this PR:
enforce_jailand try to consolidate them, perhaps.....