Workspace trust: update dialog and status bar item styling and fix command state handling#16877
Workspace trust: update dialog and status bar item styling and fix command state handling#16877
Conversation
Follow-up to GH-16809 - Improve dialog styling with better visual hierarchy and spacing - Update status bar item to use prominent styling when in restricted mode - Update dialog message to acknowledge that workspace trust is not yet fully supported in Theia
Follow-up to GH-16809 - Return current trust state in getWorkspaceTrust instead of stale initial value for the quick input - Remove workspace from trusted folders when selecting "Don't Trust" via command
| <div className="workspace-trust-description"> | ||
| {nls.localize( | ||
| 'theia/workspace/trustDialogMessage', | ||
| `The workspace trust feature is not yet fully supported in Theia. | ||
|
|
||
| If you trust the authors of this folder, code inside may be executed. Only trust folders that you trust the contents of.` | ||
| )} |
There was a problem hiding this comment.
I'm not sure exactly what would be best for this message. The current state of the feature, as I understand it, is basically that its efficacy is restricted to (some parts of) the plugin system. That means that if all plugins behave nicely and check workspace trust before doing things that could be dangerous, it actually works as intended. But if some plugin automatically runs e.g. some compilation task with a malicious definition without first checking trust, we wouldn't stop it, and I think VSCode would.
So the sense we want to communicate is that the answer to the question is not irrelevant, but saying 'no' may not have all the effects that the user would expect.
But maybe we should just hook up workspace trust in the task and debug systems to stop those systems from running in untrusted workspaces, and then I'll stop hemming and hawing :-).
| if (!workspaceUri) { | ||
| return; | ||
| } | ||
| if (this.isWorkspaceInTrustedFolders()) { |
There was a problem hiding this comment.
I think there is a problem with this method:
protected isWorkspaceInTrustedFolders(): boolean {
const workspaceUri = this.workspaceService.workspace?.resource; // <--- Could refer to workspace file.
if (!workspaceUri) {
return false;
}
const trustedFolders = this.workspaceTrustPref[WORKSPACE_TRUST_TRUSTED_FOLDERS] || [];
const caseSensitive = !OS.backend.isWindows;
return trustedFolders.some(folder => {
try {
const folderUri = new URI(folder).normalizePath();
return workspaceUri.normalizePath().isEqual(folderUri, caseSensitive); // <----- Workspace file would never be equal to any folder.
} catch {
return false; // Invalid URI in preferences
}
});
}
There was a problem hiding this comment.
As disussed offline, I created a follow up for this part: #16887
|
Thanks for the thorough review, Colin. As discussed offline I totally agree that there are important parts that are still missing, but I created the follow-up issues for those, all tracked under the umbrella issue #12318. |


What it does
Follow-up to #16809
How to test
Manage Workspace Trustand toggle trust statesFollow-ups
https://github.com/eclipse-theia/theia/tree/GH-16872
Breaking changes
Attribution
Review checklist
nlsservice (for details, please see the Internationalization/Localization section in the Coding Guidelines)Reminder for reviewers