Skip to content
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

fix: support relative path for whitelisting #2317

Merged
merged 2 commits into from May 9, 2019

Conversation

3 participants
@kevinkassimo
Copy link
Contributor

commented May 9, 2019

Using std::fs::canonicalize resolve_path to expand path to full existing path, such that
later attempt to loop-pop and compare path segment would work.

Update: use resolve_path instead of canonicalize, since allowing a path that will exist in the future does not seem to have actual negative security implications.

kevinkassimo added some commits May 9, 2019

fix: support relative path for whitelisting
Using `std::fs::canonicalize` to expand path to full existing path, such that
later attempt to loop-pop and compare path segment would work.
fix: use `resolve_path` instead of `canonicalize`
Allow on a path that will be existing in the future seems acceptible

@kevinkassimo kevinkassimo force-pushed the kevinkassimo:whitelist/relative branch from 99bcd41 to d38d142 May 9, 2019

@ry

This comment has been minimized.

Copy link
Collaborator

commented May 9, 2019

Please review @afinch7

@afinch7
Copy link
Contributor

left a comment

LGTM

read_wl.map(std::string::ToString::to_string).collect();
flags.read_whitelist = resolve_paths(raw_read_whitelist);
debug!("read whitelist: {:#?}", &flags.read_whitelist);

This comment has been minimized.

Copy link
@afinch7

afinch7 May 9, 2019

Contributor

👍 Nice to have some debug prints for this.

@afinch7

afinch7 approved these changes May 9, 2019

@ry

ry approved these changes May 9, 2019

Copy link
Collaborator

left a comment

LGTM - no comments

@ry ry merged commit d9cdc67 into denoland:master May 9, 2019

3 checks passed

Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
license/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.