Skip to content

Commit

Permalink
src/goVulncheck: replace gopls custom command with gopls vulncheck
Browse files Browse the repository at this point in the history
We learned that it is better to run the vulncheck analysis outside
the main gopls process because

- The custom vulncheck message blocks all the LSP messages
including the workDoneProgress cancellation message. For a large
project, the vulncheck can take a while to complete. Blocking
gopls's normal operation and not being able to cancel the task
is not ideal.

- The vulncheck may panic. The panics sometimes occur in goroutines
created inside libraries that we cannot control, and thus we cannot
recover from the panic either. Bringing down the main gopls isn't
a great experience either.

`gopls vulncheck` outputs a JSON-encoded VulncheckResult message.
`gopls vulncheck` can be thought as `govulncheck` shipped with
`gopls` which most users already have, and outputs the results
in the format editors can utilize.

This change allows cancellation of a long running analysis by killing
the command.

This allows us to prevent the panics from killing the main gopls.

This also forwards stderr to the output channel and as the progress
report.

Fixes #2185
Fixes #2186
Fixes #2214

Change-Id: Icd9f04ee3f3dc8379d832c224b5397d5f57c2b4e
Reviewed-on: https://go-review.googlesource.com/c/vscode-go/+/404576
Run-TryBot: Hyang-Ah Hana Kim <[email protected]>
TryBot-Result: kokoro <[email protected]>
Reviewed-by: Jamal Carvalho <[email protected]>
  • Loading branch information
hyangah committed May 10, 2022
1 parent 68e1355 commit d9980fd
Showing 1 changed file with 75 additions and 28 deletions.
103 changes: 75 additions & 28 deletions src/goVulncheck.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
*--------------------------------------------------------*/

import path from 'path';
import { pathToFileURL } from 'url';
import * as vscode from 'vscode';
import { ExecuteCommandRequest } from 'vscode-languageserver-protocol';
import { GoExtensionContext } from './context';
import { getBinPath } from './util';
import * as cp from 'child_process';
import { toolExecutionEnvironment } from './goEnv';
import { killProcessTree } from './utils/processUtils';
import * as readline from 'readline';

export class VulncheckProvider {
static scheme = 'govulncheck';
Expand Down Expand Up @@ -69,22 +72,22 @@ export class VulncheckProvider {
return;
}

let result = '\nNo known vulnerabilities found.';
this.channel.clear();
this.channel.appendLine(`cd ${dir}; gopls vulncheck ${pattern}`);

let result = '';
try {
const vuln = await vulncheck(goCtx, dir, pattern);
if (vuln?.Vuln) {
result = vuln.Vuln.map(renderVuln).join('----------------------\n');
const vuln = await vulncheck(goCtx, dir, pattern, this.channel);
if (vuln) {
result = vuln.Vuln
? vuln.Vuln.map(renderVuln).join('----------------------\n')
: 'No known vulnerability found.';
}
} catch (e) {
if (e instanceof Error) {
result = e.message;
}
vscode.window.showErrorMessage(`error running vulncheck: ${e}`);
}

result = `DIR=${dir} govulncheck ${pattern}\n${result}`;
this.channel.clear();
this.channel.append(result);
this.channel.appendLine(result);
this.channel.show();
}

Expand All @@ -104,26 +107,70 @@ export class VulncheckProvider {
}
}

async function vulncheck(
// run `gopls vulncheck`.
export async function vulncheck(
goCtx: GoExtensionContext,
dir: string,
pattern = './...'
): Promise<VulncheckReponse | undefined> {
pattern = './...',
channel: { appendLine: (msg: string) => void }
): Promise<VulncheckResponse> {
const { languageClient, serverInfo } = goCtx;
const COMMAND = 'gopls.run_vulncheck_exp';
if (languageClient && serverInfo?.Commands?.includes(COMMAND)) {
const request = {
command: COMMAND,
arguments: [
{
Dir: pathToFileURL(dir).toString(),
Pattern: pattern
}
]
};
const resp = await languageClient.sendRequest(ExecuteCommandRequest.type, request);
return resp;
if (!languageClient || !serverInfo?.Commands?.includes(COMMAND)) {
throw Promise.reject('this feature requires gopls v0.8.4 or newer');
}
// TODO: read back the actual package configuration from gopls.
const gopls = getBinPath('gopls');
const options: vscode.ProgressOptions = {
cancellable: true,
title: 'Run govulncheck',
location: vscode.ProgressLocation.Notification
};
const task = vscode.window.withProgress<VulncheckResponse>(options, (progress, token) => {
const p = cp.spawn(gopls, ['vulncheck', pattern], {
cwd: dir,
env: toolExecutionEnvironment(vscode.Uri.file(dir))
});

progress.report({ message: `starting command ${gopls} from ${dir} (pid; ${p.pid})` });

const d = token.onCancellationRequested(() => {
channel.appendLine(`gopls vulncheck (pid: ${p.pid}) is cancelled`);
killProcessTree(p);
d.dispose();
});

const promise = new Promise<VulncheckResponse>((resolve, reject) => {
const rl = readline.createInterface({ input: p.stderr });
rl.on('line', (line) => {
channel.appendLine(line);
const msg = line.match(/^\d+\/\d+\/\d+\s+\d+:\d+:\d+\s+(.*)/);
if (msg && msg[1]) {
progress.report({ message: msg[1] });
}
});

let buf = '';
p.stdout.on('data', (chunk) => {
buf += chunk;
});
p.stdout.on('close', () => {
try {
const res: VulncheckResponse = JSON.parse(buf);
resolve(res);
} catch (e) {
if (token.isCancellationRequested) {
reject('analysis cancelled');
} else {
channel.appendLine(buf);
reject(`result in unexpected format: ${e}`);
}
}
});
});
return promise;
});
return await task;
}

const renderVuln = (v: Vuln) => `
Expand Down Expand Up @@ -168,7 +215,7 @@ const renderUri = (stack: CallStack) => {
return `${stack.URI}#${line}:${stack.Pos.character}`;
};

interface VulncheckReponse {
interface VulncheckResponse {
Vuln?: Vuln[];
}

Expand Down

0 comments on commit d9980fd

Please sign in to comment.