Prompt when workspace overrides clangd.path and clangd.arguments (#160)

Prompt when workspace overrides clangd.path and clangd.arguments

Author: sam-mccall

docs/settings.md

@@ -0,0 +1,17 @@
+# Settings
+
+## Security
+
+This extension allows configuring command-line flags to the `clangd` executable.
+
+Being able to configure these per-workspace can be convenient, but runs the risk
+of an attacker changing your flags if you happen to open their repository.
+Similarly, allowing the workspace to control the `clangd.path` setting is risky.
+
+When these flags are configured in the workspace, the extension will prompt you
+whether to use them or not. If you're unsure, click "No".
+
+![Screenshot: Workspace trying to override clangd.path](workspace-override.png)
+
+If you choose "Yes" or "No", the answer will be remembered for this workspace.
+(If the value changes, you will be prompted again).

docs/workspace-override.png

@@ -45,15 +45,17 @@ export class ClangdContext implements vscode.Disposable {
   subscriptions: vscode.Disposable[] = [];
   client: ClangdLanguageClient;
 
-  async activate(globalStoragePath: string,
-                 outputChannel: vscode.OutputChannel) {
-    const clangdPath = await install.activate(this, globalStoragePath);
+  async activate(globalStoragePath: string, outputChannel: vscode.OutputChannel,
+                 workspaceState: vscode.Memento) {
+    const clangdPath =
+        await install.activate(this, globalStoragePath, workspaceState);
     if (!clangdPath)
       return;
 
     const clangd: vscodelc.Executable = {
       command: clangdPath,
-      args: config.get<string[]>('arguments'),
+      args:
+          await config.getSecureOrPrompt<string[]>('arguments', workspaceState),
       options: {cwd: vscode.workspace.rootPath || process.cwd()}
     };
     const traceFile = config.get<string>('trace');

src/clangd-context.ts

@@ -6,6 +6,47 @@ export function get<T>(key: string): T {
   return substitute(vscode.workspace.getConfiguration('clangd').get(key));
 }
 
+// Like get(), but won't load settings from workspace config unless the user has
+// previously explicitly allowed this.
+export function getSecure<T>(key: string, workspaceState: vscode.Memento): T|
+    undefined {
+  const prop = new SecureProperty<T>(key, workspaceState);
+  return prop.get(prop.blessed ?? false);
+}
+
+// Like get(), but won't implicitly load settings from workspace config.
+// If there is workspace config, prompts the user and caches the decision.
+export async function getSecureOrPrompt<T>(
+    key: string, workspaceState: vscode.Memento): Promise<T> {
+  const prop = new SecureProperty<T>(key, workspaceState);
+  // Common case: value not overridden in workspace.
+  if (!prop.mismatched)
+    return prop.get(false);
+  // Check whether user has blessed or blocked this value.
+  const blessed = prop.blessed;
+  if (blessed !== null)
+    return prop.get(blessed);
+  // No cached decision for this value, ask the user.
+  const Yes = 'Yes, use this setting', No = 'No, use my default',
+        Info = 'More Info'
+  switch (await vscode.window.showWarningMessage(
+      `This workspace wants to set clangd.${key} to ${prop.insecureJSON}.
+    \u2029
+    This will override your default of ${prop.secureJSON}.`,
+      Yes, No, Info)) {
+  case Info:
+    vscode.env.openExternal(vscode.Uri.parse(
+        'https://github.com/clangd/vscode-clangd/blob/master/docs/settings.md#security'));
+    break;
+  case Yes:
+    await prop.bless(true);
+    return prop.get(true);
+  case No:
+    await prop.bless(false);
+  }
+  return prop.get(false);
+}
+
 // Sets the config value `clangd.<key>`. Does not apply substitutions.
 export function update<T>(key: string, value: T,
                           target?: vscode.ConfigurationTarget) {
@@ -54,3 +95,48 @@ function replacement(name: string): string|null {
 
   return null;
 }
+
+// Caches a user's decision about whether to respect a workspace override of a
+// sensitive setting. Valid only for a particular variable, workspace, and
+// value.
+interface BlessCache {
+  json: string     // JSON-serialized workspace value that the decision governs.
+  allowed: boolean // Whether the user chose to allow this value.
+}
+
+// Common logic between getSecure and getSecureOrPrompt.
+class SecureProperty<T> {
+  secure: T|undefined;
+  insecure: T|undefined;
+  public secureJSON: string;
+  public insecureJSON: string;
+  blessKey: string;
+
+  constructor(key: string, private workspaceState: vscode.Memento) {
+    const cfg = vscode.workspace.getConfiguration('clangd');
+    const inspect = cfg.inspect<T>(key);
+    this.secure = inspect.globalValue ?? inspect.defaultValue;
+    this.insecure = cfg.get<T>(key);
+    this.secureJSON = JSON.stringify(this.secure);
+    this.insecureJSON = JSON.stringify(this.insecure);
+    this.blessKey = 'bless.' + key;
+  }
+
+  get mismatched(): boolean { return this.secureJSON != this.insecureJSON; }
+
+  get(trusted: boolean): T|undefined {
+    return substitute(trusted ? this.insecure : this.secure);
+  }
+
+  get blessed(): boolean|null {
+    let cache = this.workspaceState.get<BlessCache>(this.blessKey);
+    if (!cache || cache.json != this.insecureJSON)
+      return null;
+    return cache.allowed;
+  }
+
+  async bless(b: boolean): Promise<void> {
+    await this.workspaceState.update(this.blessKey,
+                                     {json: this.insecureJSON, allowed: b});
+  }
+}

src/config.ts

@@ -20,10 +20,12 @@ export async function activate(context: vscode.ExtensionContext) {
   context.subscriptions.push(
       vscode.commands.registerCommand('clangd.restart', async () => {
         await clangdContext.dispose();
-        await clangdContext.activate(context.globalStoragePath, outputChannel);
+        await clangdContext.activate(context.globalStoragePath, outputChannel,
+                                     context.workspaceState);
       }));
 
-  await clangdContext.activate(context.globalStoragePath, outputChannel);
+  await clangdContext.activate(context.globalStoragePath, outputChannel,
+                               context.workspaceState);
 
   setInterval(function() {
     const cppTools = vscode.extensions.getExtension('ms-vscode.cpptools');

src/extension.ts

@@ -10,9 +10,13 @@ import {ClangdContext} from './clangd-context';
 import * as config from './config';
 
 // Returns the clangd path to be used, or null if clangd is not installed.
-export async function activate(context: ClangdContext,
-                               globalStoragePath: string): Promise<string> {
-  const ui = new UI(context, globalStoragePath);
+export async function activate(
+    context: ClangdContext, globalStoragePath: string,
+    workspaceState: vscode.Memento): Promise<string> {
+  // If the workspace overrides clangd.path, give the user a chance to bless it.
+  await config.getSecureOrPrompt<string>('path', workspaceState);
+
+  const ui = new UI(context, globalStoragePath, workspaceState);
   context.subscriptions.push(vscode.commands.registerCommand(
       'clangd.install', async () => common.installLatest(ui)));
   context.subscriptions.push(vscode.commands.registerCommand(
@@ -22,8 +26,8 @@ export async function activate(context: ClangdContext,
 }
 
 class UI {
-  constructor(private context: ClangdContext,
-              private globalStoragePath: string) {}
+  constructor(private context: ClangdContext, private globalStoragePath: string,
+              private workspaceState: vscode.Memento) {}
 
   get storagePath(): string { return this.globalStoragePath; }
   async choose(prompt: string, options: string[]): Promise<string|undefined> {
@@ -122,7 +126,7 @@ class UI {
   }
 
   get clangdPath(): string {
-    let p = config.get<string>('path');
+    let p = config.getSecure<string>('path', this.workspaceState);
     // Backwards compatibility: if it's a relative path with a slash, interpret
     // relative to project root.
     if (!path.isAbsolute(p) && p.indexOf(path.sep) != -1 &&