check if absolute path when add target into repo #878
Conversation
Signed-off-by: lixuefeng (Cloud) <[email protected]>
|
@sebastien Awwad, It seems the CI has problems? The codes should be all right. |
There was a problem hiding this comment.
Thanks for pointing out this issue and for providing a patch, @lixuefeng2!
I think the main problem here is that repo.py wants to preserve the full relative path of each target file (target_path) when copying it to the tuf repo directory (repo_targets_path):
As you pointed out, this breaks if target_path is an absolute path (see os.path.join) or has up-level references (see securesystemslib.util.ensure_parent_dir).
I suggest to either,
(1) first normalize the path, and only consider it (i.e. copy to repo, add to metadata) if it exists and is not absolute, i.e.
foo/../bar --> foo/bar (if exists) --> <REPO_DIR>/targets/foo/bar, or
(2) allow any file (or directory) that exists, but don't preserve the full path when copying, i.e.
/foo/bar --> <REPO_DIR>/targets/bar, and
../baz --> <REPO_DIR>/targets/baz
I think the second approach is less surprising to the user. What do others think? @awwad? @lixuefeng2?
Btw. it might be worth to revise the other file/path related operations in repo.py for consistency, and also update the argparse help messages.
|
|
||
|
|
||
| elif os.path.isabs(target_path): | ||
| logger.warn('omit this file:' + target_path + ', the target ' |
There was a problem hiding this comment.
logger.warn is deprecated, use logger.warning instead (see linter message)
|
Closing due to inactivity. Re-visit with #811. |
Signed-off-by: lixuefeng (Cloud) [email protected]
repo.py --add doesn't support absolute path. I add check for it in codes.
Actually, relative path with ..(upper folder) has also problem.
The document should add comment to forbid upper folder.