-
Notifications
You must be signed in to change notification settings - Fork 28.8k
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
pipe workspace boolean for opener service validator #155545
Conversation
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.
So when would opening a link not be from the workspace? I am not sure I fully understand when this is true
and when this is false
, maybe the name is just confusing.
The case that spawned this change is the sponsor link from the extension description page. That is not from the workspace. I also did not add this for link detection in terminals since they can be created by tooling very easily. |
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.
For editors: is it possible that you open an untrusted file in a trusted workspace and then we wrongly set fromWorkspace
? Also, what if the file is outside of the workspace, does it matter?
For webview: I am not entirely sure this covers only the cases we expect it to, so I suggest to let more people review this change, including someone from the editor team and someone from the webview team?
Finally, is the sponsor link considered fromWorkspace
or not? And does fromWorkspace
bring up a dialog or not?
Once an untrusted file is brought into the workspace, it can be treated as part of the workspace. This is the current model workspace trust uses. However, it is bleeding that assumption into the opener service's model which is a problem with the change as a whole. Though, the alternative is to remove this feature entirely or associate an source with every open call and have validators figure that out. Though we have exactly 1 validator.
|
@bpasero had this set to auto merge so it got merged, if you have additional comments, we can revert or make additional changes |
The changes to editor and notebook look safe to me but as @mjbvz pointed out above, setting
In both cases, we don't guarantee that the extensions don't generate on their own. With that said, there is no way to know unless we treat all content from extensions as "non-workspace". |
fixes #150828
after trying to implement the proposal I realize that the opener service needs to know validator specific knowledge to execute this flow properly. in order to avoid pollution of knowledge, I went with something like
fromUserGesture
butfromWorkspace
. I adopted it for clicking links in webviews (markdown preview) and editors link detection.