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
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_filename
is 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_filename
was 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_jail
matters 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_jail
and try to consolidate them, perhaps.....