Skip to content

Commit

Permalink
Prompt when workspace overrides clangd.path and clangd.arguments
Browse files Browse the repository at this point in the history
  • Loading branch information
sam-mccall committed Mar 2, 2021
1 parent a9653a8 commit cfe5b2e
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 9 deletions.
17 changes: 17 additions & 0 deletions docs/settings.md
Original file line number Diff line number Diff line change
@@ -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).
Binary file added docs/workspace-override.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 4 additions & 3 deletions src/clangd-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,15 @@ export class ClangdContext implements vscode.Disposable {
client: ClangdLanguageClient;

async activate(globalStoragePath: string,
outputChannel: vscode.OutputChannel) {
const clangdPath = await install.activate(this, globalStoragePath);
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');
Expand Down
83 changes: 83 additions & 0 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,44 @@ 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) {
Expand Down Expand Up @@ -54,3 +92,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});
}
}
4 changes: 2 additions & 2 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ 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');
Expand Down
13 changes: 9 additions & 4 deletions src/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@ 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);
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(
Expand All @@ -23,7 +27,8 @@ export async function activate(context: ClangdContext,

class UI {
constructor(private context: ClangdContext,
private globalStoragePath: string) {}
private globalStoragePath: string,
private workspaceState: vscode.Memento) {}

get storagePath(): string { return this.globalStoragePath; }
async choose(prompt: string, options: string[]): Promise<string|undefined> {
Expand Down Expand Up @@ -122,7 +127,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 &&
Expand Down

0 comments on commit cfe5b2e

Please sign in to comment.