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.
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(fs): Escape paths in Scope methods #2070
fix(fs): Escape paths in Scope methods #2070
Changes from 6 commits
b42064d
caac44d
f323eb8
0f2e408
e8fabf0
6b344c8
ad3156f
00b3d16
cc182fb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do really need to manually call
is_forbidden
, doesn'tscope.is_allowed
check for forbidden/denied patterns?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the problem is that is_allowed returns false for paths that are explictly denied and paths that are neither allowed nor denied.
When we're checking for 2 scopes at the same time we only care about the explicitly denied paths because those take precedence of explictly allowed paths.
A path that's neither allowed nor denied in the first scope could still be explicitly allowed by the second scope.
Alternatively a way to create a new Scope from 2 existing scopes would be cool too, then is_allowed would work for us here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can improve this a bit by adding the
Scope::is_forbidden
in tauriThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this implemented separately here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i can implement it in tauri::fs::Scope too and re-use that but wanted to keep the plugin compatible with all tauri v2 versions for now.
not too happy about the approach itself but merging instances of
tauri::fs::Scope
s isn't possible rn so this was the next best thing to make the draft work.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's ship with this for now, but I think we should still add it in
tauri
and then migrate thefs
plugin later to that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, will open a pr later
if i don't forget