From 779bbd2df4e19c77aa81443ffc28a37c28cf3ae0 Mon Sep 17 00:00:00 2001 From: Bastian Doetsch <bastian.doetsch@snyk.io> Date: Fri, 8 Sep 2023 14:14:00 +0200 Subject: [PATCH 1/5] feat: add detection for standalone/cli-embedded based on size, add logging args if given `-d` or env var - SNYK_LOG_DEVEL overrules all log level settings now - `-d` or `--debug` in additional parameters now puts language server in debug mode Signed-off-by: Bastian Doetsch <bastian.doetsch@snyk.io> --- CHANGELOG.md | 5 +- .../common/languageServer/languageServer.ts | 27 +++- .../languageServer/languageServer.test.ts | 134 +++++++++++++++++- 3 files changed, 157 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f91212396..9ff6411a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ # Snyk Security - Code and Open Source Dependencies Changelog -## [1.21.6] +## [1.23.0] +- add detection for standalone/cli embedded language server based on binary size + +## [1.22.0] ### Added diff --git a/src/snyk/common/languageServer/languageServer.ts b/src/snyk/common/languageServer/languageServer.ts index b9ca37a04..7d6edca88 100644 --- a/src/snyk/common/languageServer/languageServer.ts +++ b/src/snyk/common/languageServer/languageServer.ts @@ -24,10 +24,13 @@ import { LsExecutable } from './lsExecutable'; import { LanguageClientMiddleware } from './middleware'; import { InitializationOptions, LanguageServerSettings } from './settings'; import { CodeIssueData, IacIssueData, OssIssueData, Scan } from './types'; +import * as fs from 'fs'; export interface ILanguageServer { start(): Promise<void>; + stop(): Promise<void>; + showOutputChannel(): void; cliReady$: ReplaySubject<string>; @@ -77,11 +80,31 @@ export class LanguageServer implements ILanguageServer { const lsBinaryPath = LsExecutable.getPath(this.configuration.getSnykLanguageServerPath()); - this.logger.info(`Snyk Language Server path: ${lsBinaryPath}`); + // log level is set to info by default + let logLevel = 'info'; + const additionalCliParameters = this.configuration.getAdditionalCliParameters(); + if ( + additionalCliParameters != null && + additionalCliParameters.length > 0 && + (additionalCliParameters.includes('-d') || additionalCliParameters.includes('--debug')) + ) { + logLevel = 'debug'; + } + logLevel = process.env.SNYK_LOG_LEVEL ?? logLevel; + + // check size of file at lsBinaryPath to determine if cli or ls + let args = ['-l', logLevel]; + const lsBinarySize = fs.statSync(lsBinaryPath).size; + const fiftyMB = 50 * 1024 * 1024; + if (lsBinarySize > fiftyMB) { + args = ['language-server', ...args]; + } + this.logger.info(`Snyk Language Server - path: ${lsBinaryPath}`); + this.logger.info(`Snyk Language Server - args: ${args}`); const serverOptions: ServerOptions = { command: lsBinaryPath, - args: ['-l', 'info'], + args: args, options: { env: processEnv, }, diff --git a/src/test/unit/common/languageServer/languageServer.test.ts b/src/test/unit/common/languageServer/languageServer.test.ts index 8c79cc50e..3f4fc43af 100644 --- a/src/test/unit/common/languageServer/languageServer.test.ts +++ b/src/test/unit/common/languageServer/languageServer.test.ts @@ -1,5 +1,5 @@ -/* eslint-disable @typescript-eslint/no-unsafe-member-access */ -import assert, { deepStrictEqual, strictEqual } from 'assert'; +/* eslint-disable @typescript-eslint/no-empty-function */ +import assert, { deepStrictEqual, fail, strictEqual } from 'assert'; import { ReplaySubject } from 'rxjs'; import sinon from 'sinon'; import { v4 } from 'uuid'; @@ -16,6 +16,7 @@ import { defaultFeaturesConfigurationStub } from '../../mocks/configuration.mock import { LoggerMock } from '../../mocks/logger.mock'; import { windowMock } from '../../mocks/window.mock'; import { stubWorkspaceConfiguration } from '../../mocks/workspace.mock'; +import * as fs from 'fs'; suite('Language Server', () => { const authServiceMock = {} as IAuthenticationService; @@ -24,13 +25,23 @@ suite('Language Server', () => { let configurationMock: IConfiguration; let languageServer: LanguageServer; let downloadServiceMock: DownloadService; + const path = 'testPath'; + const logger = { + info(_msg: string) {}, + warn(_msg: string) {}, + log(_msg: string) {}, + error(msg: string) { + fail(msg); + }, + } as unknown as LoggerMock; + setup(() => { configurationMock = { getInsecure(): boolean { return true; }, getCliPath(): string | undefined { - return 'testPath'; + return path; }, getToken(): Promise<string | undefined> { return Promise.resolve('testToken'); @@ -38,10 +49,15 @@ suite('Language Server', () => { shouldReportEvents: true, shouldReportErrors: true, getSnykLanguageServerPath(): string { - return 'testPath'; + try { + fs.statSync(path); + } catch (_) { + fs.writeFileSync(path, 'huhu'); + } + return path; }, getAdditionalCliParameters() { - return '--all-projects'; + return '--all-projects -d'; }, isAutomaticDependencyManagementEnabled() { return true; @@ -73,9 +89,114 @@ suite('Language Server', () => { }); teardown(() => { + fs.rmSync(path, { force: true }); sinon.restore(); }); + test('LanguageServer (standalone) starts with correct args', async () => { + const lca = sinon.spy({ + create( + _id: string, + _name: string, + serverOptions: ServerOptions, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + _clientOptions: LanguageClientOptions, + ): LanguageClient { + return { + start(): Promise<void> { + assert.strictEqual('args' in serverOptions ? serverOptions?.args?.[0] : '', '-l'); + assert.strictEqual('args' in serverOptions ? serverOptions?.args?.[1] : '', 'debug'); + return Promise.resolve(); + }, + onNotification(): void { + return; + }, + onReady(): Promise<void> { + return Promise.resolve(); + }, + } as unknown as LanguageClient; + }, + }); + + languageServer = new LanguageServer( + user, + configurationMock, + lca as unknown as ILanguageClientAdapter, + stubWorkspaceConfiguration('snyk.loglevel', 'trace'), + windowMock, + authServiceMock, + { + info(_msg: string) {}, + warn(_msg: string) {}, + log(_msg: string) {}, + error(msg: string) { + fail(msg); + }, + } as unknown as LoggerMock, + downloadServiceMock, + ); + downloadServiceMock.downloadReady$.next(); + + await languageServer.start(); + sinon.assert.called(lca.create); + sinon.verify(); + }); + + test('LanguageServer (embedded) starts with language-server arg', async () => { + configurationMock.getSnykLanguageServerPath = () => { + try { + fs.statSync(path); + } catch (_) { + // write >50MB of data to simulate a CLI sized binary + let data = ''; + const size = 55 * 128 * 1024; // 128 * 8 = 1024 bytes, 55 * 1024 * 1024 = 55MB + for (let i = 0; i < size; i++) { + data += '01234567'; + } + fs.writeFileSync(path, data); + } + return path; + }; + const lca = sinon.spy({ + create( + _id: string, + _name: string, + serverOptions: ServerOptions, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + _clientOptions: LanguageClientOptions, + ): LanguageClient { + return { + start(): Promise<void> { + assert.strictEqual('args' in serverOptions ? serverOptions?.args?.[0] : '', 'language-server'); + return Promise.resolve(); + }, + onNotification(): void { + return; + }, + onReady(): Promise<void> { + return Promise.resolve(); + }, + } as unknown as LanguageClient; + }, + }); + + languageServer = new LanguageServer( + user, + configurationMock, + lca as unknown as ILanguageClientAdapter, + stubWorkspaceConfiguration('snyk.loglevel', 'trace'), + windowMock, + authServiceMock, + logger, + downloadServiceMock, + ); + downloadServiceMock.downloadReady$.next(); + + await languageServer.start(); + sinon.assert.called(lca.create); + sinon.verify(); + }); + test('LanguageServer adds proxy settings to env of started binary', async () => { const expectedProxy = 'http://localhost:8080'; const lca = sinon.spy({ @@ -90,9 +211,11 @@ suite('Language Server', () => { assert.strictEqual(id, 'Snyk LS'); assert.strictEqual(name, 'Snyk Language Server'); assert.strictEqual( + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access 'options' in serverOptions ? serverOptions?.options?.env?.http_proxy : undefined, expectedProxy, ); + // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access assert.strictEqual(clientOptions.initializationOptions.token, 'testToken'); return Promise.resolve(); }, @@ -117,7 +240,6 @@ suite('Language Server', () => { downloadServiceMock, ); downloadServiceMock.downloadReady$.next(); - await languageServer.start(); sinon.assert.called(lca.create); sinon.verify(); }); From 5301091e3821e11d26a6d2a30d9605ba3b54c65f Mon Sep 17 00:00:00 2001 From: Bastian Doetsch <bastian.doetsch@snyk.io> Date: Fri, 8 Sep 2023 14:18:37 +0200 Subject: [PATCH 2/5] fix: make test method sync'ed Signed-off-by: Bastian Doetsch <bastian.doetsch@snyk.io> --- src/test/unit/common/languageServer/languageServer.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/unit/common/languageServer/languageServer.test.ts b/src/test/unit/common/languageServer/languageServer.test.ts index 3f4fc43af..3109f6727 100644 --- a/src/test/unit/common/languageServer/languageServer.test.ts +++ b/src/test/unit/common/languageServer/languageServer.test.ts @@ -197,7 +197,7 @@ suite('Language Server', () => { sinon.verify(); }); - test('LanguageServer adds proxy settings to env of started binary', async () => { + test('LanguageServer adds proxy settings to env of started binary', () => { const expectedProxy = 'http://localhost:8080'; const lca = sinon.spy({ create( From 1c670f393bede6943d6dde81cfdbfc3064a5d42c Mon Sep 17 00:00:00 2001 From: Bastian Doetsch <bastian.doetsch@snyk.io> Date: Fri, 8 Sep 2023 14:24:30 +0200 Subject: [PATCH 3/5] fix: tests Signed-off-by: Bastian Doetsch <bastian.doetsch@snyk.io> --- .../common/languageServer/languageServer.test.ts | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/test/unit/common/languageServer/languageServer.test.ts b/src/test/unit/common/languageServer/languageServer.test.ts index 3109f6727..e31c517c1 100644 --- a/src/test/unit/common/languageServer/languageServer.test.ts +++ b/src/test/unit/common/languageServer/languageServer.test.ts @@ -125,14 +125,7 @@ suite('Language Server', () => { stubWorkspaceConfiguration('snyk.loglevel', 'trace'), windowMock, authServiceMock, - { - info(_msg: string) {}, - warn(_msg: string) {}, - log(_msg: string) {}, - error(msg: string) { - fail(msg); - }, - } as unknown as LoggerMock, + logger, downloadServiceMock, ); downloadServiceMock.downloadReady$.next(); @@ -197,7 +190,7 @@ suite('Language Server', () => { sinon.verify(); }); - test('LanguageServer adds proxy settings to env of started binary', () => { + test('LanguageServer adds proxy settings to env of started binary', async () => { const expectedProxy = 'http://localhost:8080'; const lca = sinon.spy({ create( @@ -240,6 +233,7 @@ suite('Language Server', () => { downloadServiceMock, ); downloadServiceMock.downloadReady$.next(); + await languageServer.start(); sinon.assert.called(lca.create); sinon.verify(); }); @@ -280,7 +274,7 @@ suite('Language Server', () => { automaticAuthentication: 'false', endpoint: undefined, organization: undefined, - additionalParams: '--all-projects', + additionalParams: '--all-projects -d', manageBinariesAutomatically: 'true', deviceId: user.anonymousId, filterSeverity: { critical: true, high: true, medium: true, low: true }, From 2081d454ac8a0d3e4930abb1f4708c11ed652859 Mon Sep 17 00:00:00 2001 From: Bastian Doetsch <bastian.doetsch@snyk.io> Date: Fri, 8 Sep 2023 14:32:05 +0200 Subject: [PATCH 4/5] chore: remove unnecessary check for binary size Signed-off-by: Bastian Doetsch <bastian.doetsch@snyk.io> --- .../common/languageServer/languageServer.ts | 9 +-- .../languageServer/languageServer.test.ts | 67 +------------------ 2 files changed, 4 insertions(+), 72 deletions(-) diff --git a/src/snyk/common/languageServer/languageServer.ts b/src/snyk/common/languageServer/languageServer.ts index 7d6edca88..b970f6493 100644 --- a/src/snyk/common/languageServer/languageServer.ts +++ b/src/snyk/common/languageServer/languageServer.ts @@ -92,14 +92,7 @@ export class LanguageServer implements ILanguageServer { } logLevel = process.env.SNYK_LOG_LEVEL ?? logLevel; - // check size of file at lsBinaryPath to determine if cli or ls - let args = ['-l', logLevel]; - const lsBinarySize = fs.statSync(lsBinaryPath).size; - const fiftyMB = 50 * 1024 * 1024; - if (lsBinarySize > fiftyMB) { - args = ['language-server', ...args]; - } - + const args = ['language-server', '-l', logLevel]; this.logger.info(`Snyk Language Server - path: ${lsBinaryPath}`); this.logger.info(`Snyk Language Server - args: ${args}`); const serverOptions: ServerOptions = { diff --git a/src/test/unit/common/languageServer/languageServer.test.ts b/src/test/unit/common/languageServer/languageServer.test.ts index e31c517c1..731b9f7a6 100644 --- a/src/test/unit/common/languageServer/languageServer.test.ts +++ b/src/test/unit/common/languageServer/languageServer.test.ts @@ -16,7 +16,6 @@ import { defaultFeaturesConfigurationStub } from '../../mocks/configuration.mock import { LoggerMock } from '../../mocks/logger.mock'; import { windowMock } from '../../mocks/window.mock'; import { stubWorkspaceConfiguration } from '../../mocks/workspace.mock'; -import * as fs from 'fs'; suite('Language Server', () => { const authServiceMock = {} as IAuthenticationService; @@ -49,11 +48,6 @@ suite('Language Server', () => { shouldReportEvents: true, shouldReportErrors: true, getSnykLanguageServerPath(): string { - try { - fs.statSync(path); - } catch (_) { - fs.writeFileSync(path, 'huhu'); - } return path; }, getAdditionalCliParameters() { @@ -89,67 +83,10 @@ suite('Language Server', () => { }); teardown(() => { - fs.rmSync(path, { force: true }); sinon.restore(); }); - test('LanguageServer (standalone) starts with correct args', async () => { - const lca = sinon.spy({ - create( - _id: string, - _name: string, - serverOptions: ServerOptions, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - _clientOptions: LanguageClientOptions, - ): LanguageClient { - return { - start(): Promise<void> { - assert.strictEqual('args' in serverOptions ? serverOptions?.args?.[0] : '', '-l'); - assert.strictEqual('args' in serverOptions ? serverOptions?.args?.[1] : '', 'debug'); - return Promise.resolve(); - }, - onNotification(): void { - return; - }, - onReady(): Promise<void> { - return Promise.resolve(); - }, - } as unknown as LanguageClient; - }, - }); - - languageServer = new LanguageServer( - user, - configurationMock, - lca as unknown as ILanguageClientAdapter, - stubWorkspaceConfiguration('snyk.loglevel', 'trace'), - windowMock, - authServiceMock, - logger, - downloadServiceMock, - ); - downloadServiceMock.downloadReady$.next(); - - await languageServer.start(); - sinon.assert.called(lca.create); - sinon.verify(); - }); - - test('LanguageServer (embedded) starts with language-server arg', async () => { - configurationMock.getSnykLanguageServerPath = () => { - try { - fs.statSync(path); - } catch (_) { - // write >50MB of data to simulate a CLI sized binary - let data = ''; - const size = 55 * 128 * 1024; // 128 * 8 = 1024 bytes, 55 * 1024 * 1024 = 55MB - for (let i = 0; i < size; i++) { - data += '01234567'; - } - fs.writeFileSync(path, data); - } - return path; - }; + test('LanguageServer starts with correct args', async () => { const lca = sinon.spy({ create( _id: string, @@ -161,6 +98,8 @@ suite('Language Server', () => { return { start(): Promise<void> { assert.strictEqual('args' in serverOptions ? serverOptions?.args?.[0] : '', 'language-server'); + assert.strictEqual('args' in serverOptions ? serverOptions?.args?.[1] : '', '-l'); + assert.strictEqual('args' in serverOptions ? serverOptions?.args?.[2] : '', 'debug'); return Promise.resolve(); }, onNotification(): void { From 0e68c22c45c39bb995d104938bcbcbb773c7668c Mon Sep 17 00:00:00 2001 From: Bastian Doetsch <bastian.doetsch@snyk.io> Date: Fri, 8 Sep 2023 14:35:32 +0200 Subject: [PATCH 5/5] docs: update CHANGELOG.md Signed-off-by: Bastian Doetsch <bastian.doetsch@snyk.io> --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ff6411a5..499b63c7b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,9 @@ # Snyk Security - Code and Open Source Dependencies Changelog ## [1.23.0] -- add detection for standalone/cli embedded language server based on binary size +- add `language-server` as first positional argument to language server start +- enable setting of log level in language server via SNYK_LOG_LEVEL +- enable setting of debug level in language server via `-d` or `--debug` ## [1.22.0]