Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use xdg dir as default cli Path #563

Merged
merged 10 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@
"@babel/traverse": "^7.23.9",
"@babel/types": "^7.23.9",
"@deepcode/dcignore": "^1.0.4",
"axios": "^1.7.4",
"axios": "^1.7.8",
"diff": "^5.2.0",
"glob": "^9.3.5",
"he": "^1.2.0",
Expand Down
36 changes: 31 additions & 5 deletions src/snyk/cli/cliExecutable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,22 @@ import { Checksum } from './checksum';
import { Platform } from '../common/platform';

export class CliExecutable {
public static defaultPaths: Record<CliSupportedPlatform, string> = {
linux: process.env.XDG_DATA_HOME ?? '/.local/share/',
// eslint-disable-next-line camelcase
linux_arm64: process.env.XDG_DATA_HOME ?? '/.local/share/',
// eslint-disable-next-line camelcase
linux_alpine: process.env.XDG_DATA_HOME ?? '/.local/share/',
// eslint-disable-next-line camelcase
linux_alpine_arm64: process.env.XDG_DATA_HOME ?? '/.local/share/',
macos: process.env.XDG_DATA_HOME ?? '/Library/Application Support/',
// eslint-disable-next-line camelcase
macos_arm64: process.env.XDG_DATA_HOME ?? '/Library/Application Support/',
windows: process.env.XDG_DATA_HOME ?? '\\AppData\\Local\\',
// eslint-disable-next-line camelcase
windows_arm64: process.env.XDG_DATA_HOME ?? '\\AppData\\Local\\',
};

public static filenameSuffixes: Record<CliSupportedPlatform, string> = {
linux: 'snyk-linux',
// eslint-disable-next-line camelcase
Expand All @@ -20,16 +36,19 @@ export class CliExecutable {
// eslint-disable-next-line camelcase
windows_arm64: 'snyk-win.exe',
};

constructor(public readonly version: string, public readonly checksum: Checksum) {}

static async getPath(extensionDir: string, customPath?: string): Promise<string> {
static async getPath(customPath?: string): Promise<string> {
if (customPath) {
return customPath;
}

const platform = await CliExecutable.getCurrentWithArch();
const homeDir = Platform.getHomeDir();
const defaultPath = this.defaultPaths[platform];
const fileName = CliExecutable.getFileName(platform);
return path.join(extensionDir, fileName);
const cliDir = path.join(homeDir, defaultPath, 'snyk', 'vscode-cli');
return path.join(cliDir, fileName);
}

static getFileName(platform: CliSupportedPlatform): string {
Expand Down Expand Up @@ -67,9 +86,16 @@ export class CliExecutable {
return platform;
}

static async exists(extensionDir: string, customPath?: string): Promise<boolean> {
public static isPathInExtensionDirectory(dirPath: string, filePath: string): boolean {
const normalizedDir = path.resolve(dirPath) + path.sep;
const normalizedFile = path.resolve(filePath);

return normalizedFile.toLowerCase().startsWith(normalizedDir.toLowerCase());
}

static async exists(customPath?: string): Promise<boolean> {
return fs
.access(await CliExecutable.getPath(extensionDir, customPath))
.access(await CliExecutable.getPath(customPath))
.then(() => true)
.catch(() => false);
}
Expand Down
8 changes: 4 additions & 4 deletions src/snyk/common/configuration/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import SecretStorageAdapter from '../vscode/secretStorage';
import { IVSCodeWorkspace } from '../vscode/workspace';
import { CliExecutable } from '../../cli/cliExecutable';
import { extensionContext } from '../vscode/extensionContext';

Check warning on line 36 in src/snyk/common/configuration/configuration.ts

View workflow job for this annotation

GitHub Actions / Build and Test (ubuntu-latest)

'extensionContext' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 36 in src/snyk/common/configuration/configuration.ts

View workflow job for this annotation

GitHub Actions / Build and Test (macos-latest)

'extensionContext' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 36 in src/snyk/common/configuration/configuration.ts

View workflow job for this annotation

GitHub Actions / Build and Test (windows-latest)

'extensionContext' is defined but never used. Allowed unused vars must match /^_/u

const NEWISSUES = 'Net new issues';

Expand Down Expand Up @@ -122,7 +122,7 @@

isAutomaticDependencyManagementEnabled(): boolean;

getCliPath(): Promise<string | undefined>;
getCliPath(): Promise<string>;
getCliReleaseChannel(): Promise<string>;
getCliBaseDownloadUrl(): string;
getInsecure(): boolean;
Expand Down Expand Up @@ -352,7 +352,7 @@

async setCliPath(cliPath: string | undefined): Promise<void> {
if (!cliPath) {
cliPath = await CliExecutable.getPath(extensionContext.extensionPath);
cliPath = await CliExecutable.getPath();
}
return this.workspace.updateConfiguration(
CONFIGURATION_IDENTIFIER,
Expand Down Expand Up @@ -555,7 +555,7 @@
);
}

async getCliPath(): Promise<string | undefined> {
async getCliPath(): Promise<string> {
let cliPath = this.workspace.getConfiguration<string>(
CONFIGURATION_IDENTIFIER,
this.getConfigName(ADVANCED_CLI_PATH),
Expand All @@ -574,7 +574,7 @@
const isAutomaticDependencyManagementEnabled = this.isAutomaticDependencyManagementEnabled();
const snykLsPath = this.getSnykLanguageServerPath();
if (!isAutomaticDependencyManagementEnabled && snykLsPath) return snykLsPath;
const defaultPath = await CliExecutable.getPath(extensionContext.extensionPath);
const defaultPath = await CliExecutable.getPath();
return defaultPath;
}
getTrustedFolders(): string[] {
Expand Down
9 changes: 5 additions & 4 deletions src/snyk/common/download/downloader.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import axios, { CancelTokenSource } from 'axios';
import * as fs from 'fs';
import * as path from 'path';
import * as fsPromises from 'fs/promises';
import * as stream from 'stream';
import { mkdirSync } from 'fs';
import { Progress } from 'vscode';
import { Checksum } from '../../cli/checksum';
import { messages } from '../../cli/messages/messages';
Expand Down Expand Up @@ -43,10 +45,9 @@ export class Downloader {
}

private async getCliExecutable(platform: CliSupportedPlatform): Promise<CliExecutable | null> {
const cliPath = await CliExecutable.getPath(
this.extensionContext.extensionPath,
await this.configuration.getCliPath(),
);
const cliPath = await this.configuration.getCliPath();
const cliDir = path.dirname(cliPath);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was the old logic that ensure cliDir exists.

mkdirSync(cliDir, { recursive: true });
if (await this.binaryExists(cliPath)) {
await this.deleteFileAtPath(cliPath);
}
Expand Down
11 changes: 2 additions & 9 deletions src/snyk/common/languageServer/languageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import { LanguageClient, LanguageClientOptions, ServerOptions } from '../vscode/types';
import { IVSCodeWindow } from '../vscode/window';
import { IVSCodeWorkspace } from '../vscode/workspace';
import { CliExecutable } from '../../cli/cliExecutable';

Check warning on line 22 in src/snyk/common/languageServer/languageServer.ts

View workflow job for this annotation

GitHub Actions / Build and Test (ubuntu-latest)

'CliExecutable' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 22 in src/snyk/common/languageServer/languageServer.ts

View workflow job for this annotation

GitHub Actions / Build and Test (macos-latest)

'CliExecutable' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 22 in src/snyk/common/languageServer/languageServer.ts

View workflow job for this annotation

GitHub Actions / Build and Test (windows-latest)

'CliExecutable' is defined but never used. Allowed unused vars must match /^_/u
import { LanguageClientMiddleware } from './middleware';
import { LanguageServerSettings, ServerSettings } from './settings';
import { CodeIssueData, IacIssueData, OssIssueData, Scan } from './types';
Expand Down Expand Up @@ -78,10 +78,7 @@
};
}

const cliBinaryPath = await CliExecutable.getPath(
this.extensionContext.extensionPath,
await this.configuration.getCliPath(),
);
const cliBinaryPath = await this.configuration.getCliPath();

// log level is set to info by default
let logLevel = 'info';
Expand Down Expand Up @@ -164,11 +161,7 @@
// Initialization options are not semantically equal to server settings, thus separated here
// https://github.com/microsoft/language-server-protocol/issues/567
async getInitializationOptions(): Promise<ServerSettings> {
const settings = await LanguageServerSettings.fromConfiguration(
this.configuration,
this.user,
this.extensionContext,
);
const settings = await LanguageServerSettings.fromConfiguration(this.configuration, this.user);
return settings;
}

Expand Down
6 changes: 1 addition & 5 deletions src/snyk/common/languageServer/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,7 @@ export class LanguageClientMiddleware implements Middleware {
return [];
}

const serverSettings = await LanguageServerSettings.fromConfiguration(
this.configuration,
this.user,
this.extensionContext,
);
const serverSettings = await LanguageServerSettings.fromConfiguration(this.configuration, this.user);
return [serverSettings];
},
};
Expand Down
9 changes: 2 additions & 7 deletions src/snyk/common/languageServer/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
import { Configuration, FolderConfig, IConfiguration, SeverityFilter } from '../configuration/configuration';
import { User } from '../user';
import { PROTOCOL_VERSION } from '../constants/languageServer';
import { CliExecutable } from '../../cli/cliExecutable';
import { ExtensionContext } from '../vscode/extensionContext';

Check warning on line 6 in src/snyk/common/languageServer/settings.ts

View workflow job for this annotation

GitHub Actions / Build and Test (ubuntu-latest)

'ExtensionContext' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 6 in src/snyk/common/languageServer/settings.ts

View workflow job for this annotation

GitHub Actions / Build and Test (macos-latest)

'ExtensionContext' is defined but never used. Allowed unused vars must match /^_/u

Check warning on line 6 in src/snyk/common/languageServer/settings.ts

View workflow job for this annotation

GitHub Actions / Build and Test (windows-latest)

'ExtensionContext' is defined but never used. Allowed unused vars must match /^_/u

export type ServerSettings = {
// Feature toggles
Expand Down Expand Up @@ -50,11 +49,7 @@
};

export class LanguageServerSettings {
static async fromConfiguration(
configuration: IConfiguration,
user: User,
extensionContext: ExtensionContext,
): Promise<ServerSettings> {
static async fromConfiguration(configuration: IConfiguration, user: User): Promise<ServerSettings> {
const featuresConfiguration = configuration.getFeaturesConfiguration();

const ossEnabled = _.isUndefined(featuresConfiguration.ossEnabled) ? true : featuresConfiguration.ossEnabled;
Expand All @@ -74,7 +69,7 @@
activateSnykIac: `${iacEnabled}`,
enableDeltaFindings: `${configuration.getDeltaFindingsEnabled()}`,
sendErrorReports: `${configuration.shouldReportErrors}`,
cliPath: await CliExecutable.getPath(extensionContext.extensionPath, await configuration.getCliPath()),
cliPath: await configuration.getCliPath(),
endpoint: configuration.snykApiEndpoint,
organization: configuration.organization,
token: await configuration.getToken(),
Expand Down
22 changes: 13 additions & 9 deletions src/snyk/common/services/downloadService.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ReplaySubject } from 'rxjs';
import * as fsPromises from 'fs/promises';
import { Checksum } from '../../cli/checksum';
import { messages } from '../../cli/messages/messages';
import { IConfiguration } from '../configuration/configuration';
Expand Down Expand Up @@ -89,10 +90,7 @@ export class DownloadService {
}

async isCliInstalled() {
const cliExecutableExists = await CliExecutable.exists(
this.extensionContext.extensionPath,
await this.configuration.getCliPath(),
);
const cliExecutableExists = await CliExecutable.exists(this.extensionContext.extensionPath);
const cliChecksumWritten = !!this.getCliChecksum();

return cliExecutableExists && cliChecksumWritten;
Expand All @@ -105,11 +103,17 @@ export class DownloadService {
return false;
}
const latestChecksum = await this.cliApi.getSha256Checksum(version, platform);
const path = await CliExecutable.getPath(
this.extensionContext.extensionPath,
await this.configuration.getCliPath(),
);

const path = await this.configuration.getCliPath();
// migration for moving from default extension path to xdg dirs.
if (CliExecutable.isPathInExtensionDirectory(this.extensionContext.extensionPath, path)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might be unnecessary. But I want to ensure extension doesn't break for current preview users.

try {
await fsPromises.unlink(path);
} catch {
// eslint-disable-next-line no-empty
}
await this.configuration.setCliPath(await CliExecutable.getPath());
return true;
}
// Update is available if fetched checksum not matching the current one
const checksum = await Checksum.getChecksumOf(path, latestChecksum);
if (checksum.verify()) {
Expand Down
49 changes: 30 additions & 19 deletions src/test/unit/cli/cliExecutable.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { strictEqual } from 'assert';
import path from 'path';
import os from 'os';
import fs from 'fs/promises';
import sinon from 'sinon';
import { CliExecutable } from '../../../snyk/cli/cliExecutable';
Expand All @@ -19,52 +20,62 @@ suite('CliExecutable', () => {
});

test('Returns correct extension paths', async () => {
const unixExtensionDir = '/Users/user/.vscode/extensions/snyk-security.snyk-vulnerability-scanner-1.1.0';
const winExtensionDir = `C:\\Users\\user\\.vscode\\extensions`;
const homedirStub = sinon.stub(os, 'homedir');
const unixExtensionDir = '/.local/share/snyk/vscode-cli';
const macOsExtensionDir = '/Library/Application Support/snyk/vscode-cli';

const osStub = sinon.stub(Platform, 'getCurrent').returns('darwin');
const archStub = sinon.stub(Platform, 'getArch').returns('x64');
const fsStub = sinon.stub(fs, 'access').returns(Promise.reject());
let homedir = '/home/user';
homedirStub.returns(homedir);

let expectedCliPath = path.join(unixExtensionDir, 'snyk-macos');
strictEqual(await CliExecutable.getPath(unixExtensionDir), expectedCliPath);
let expectedCliPath = path.join(Platform.getHomeDir(), macOsExtensionDir, 'snyk-macos');
strictEqual(await CliExecutable.getPath(), expectedCliPath);

osStub.returns('linux');
expectedCliPath = path.join(unixExtensionDir, 'snyk-linux');
strictEqual(await CliExecutable.getPath(unixExtensionDir), expectedCliPath);
expectedCliPath = path.join(Platform.getHomeDir(), unixExtensionDir, 'snyk-linux');
strictEqual(await CliExecutable.getPath(), expectedCliPath);

fsStub.returns(Promise.resolve());
expectedCliPath = path.join(unixExtensionDir, 'snyk-alpine');
strictEqual(await CliExecutable.getPath(unixExtensionDir), expectedCliPath);
expectedCliPath = path.join(Platform.getHomeDir(), unixExtensionDir, 'snyk-alpine');
strictEqual(await CliExecutable.getPath(), expectedCliPath);
fsStub.returns(Promise.reject());

osStub.returns('win32');
expectedCliPath = path.join(winExtensionDir, 'snyk-win.exe');
strictEqual(await CliExecutable.getPath(winExtensionDir), expectedCliPath);
homedir = 'C:\\Users\\user';
homedirStub.returns(homedir);
expectedCliPath = path.join(Platform.getHomeDir(), '\\AppData\\Local\\', 'snyk', 'vscode-cli', 'snyk-win.exe');
const actualPath = await CliExecutable.getPath();
strictEqual(actualPath, expectedCliPath);

// test arm64
archStub.returns('arm64');

osStub.returns('darwin');
expectedCliPath = path.join(unixExtensionDir, 'snyk-macos-arm64');
strictEqual(await CliExecutable.getPath(unixExtensionDir), expectedCliPath);
homedir = '/home/user';
homedirStub.returns(homedir);
expectedCliPath = path.join(Platform.getHomeDir(), macOsExtensionDir, 'snyk-macos-arm64');
strictEqual(await CliExecutable.getPath(), expectedCliPath);

osStub.returns('linux');
expectedCliPath = path.join(unixExtensionDir, 'snyk-linux-arm64');
strictEqual(await CliExecutable.getPath(unixExtensionDir), expectedCliPath);
expectedCliPath = path.join(Platform.getHomeDir(), unixExtensionDir, 'snyk-linux-arm64');
strictEqual(await CliExecutable.getPath(), expectedCliPath);

fsStub.returns(Promise.resolve());
expectedCliPath = path.join(unixExtensionDir, 'snyk-alpine-arm64');
strictEqual(await CliExecutable.getPath(unixExtensionDir), expectedCliPath);
expectedCliPath = path.join(Platform.getHomeDir(), unixExtensionDir, 'snyk-alpine-arm64');
strictEqual(await CliExecutable.getPath(), expectedCliPath);
fsStub.returns(Promise.reject());

osStub.returns('win32');
expectedCliPath = path.join(winExtensionDir, 'snyk-win.exe');
strictEqual(await CliExecutable.getPath(winExtensionDir), expectedCliPath);
homedir = 'C:\\Users\\user';
homedirStub.returns(homedir);
expectedCliPath = path.join(Platform.getHomeDir(), '\\AppData\\Local\\', 'snyk', 'vscode-cli', 'snyk-win.exe');
strictEqual(await CliExecutable.getPath(), expectedCliPath);
});

test('Return custom path, if provided', async () => {
const customPath = '/path/to/cli';
strictEqual(await CliExecutable.getPath('', customPath), customPath);
strictEqual(await CliExecutable.getPath(customPath), customPath);
});
});
2 changes: 1 addition & 1 deletion src/test/unit/common/configuration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ suite('Configuration', () => {

const cliPath = await configuration.getCliPath();

const expectedCliPath = path.join('path/to/extension/', 'snyk-linux');
const expectedCliPath = path.join(Platform.getHomeDir(), '.local/share/snyk/vscode-cli', 'snyk-linux');
strictEqual(cliPath, expectedCliPath);
});

Expand Down
5 changes: 1 addition & 4 deletions src/test/unit/common/languageServer/middleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,7 @@ suite('Language Server: Middleware', () => {
serverResult.manageBinariesAutomatically,
`${configuration.isAutomaticDependencyManagementEnabled()}`,
);
assert.strictEqual(
serverResult.cliPath,
await CliExecutable.getPath(extensionContextMock.extensionPath, await configuration.getCliPath()),
);
assert.strictEqual(serverResult.cliPath, await configuration.getCliPath());
assert.strictEqual(serverResult.enableTrustedFoldersFeature, 'true');
assert.deepStrictEqual(serverResult.trustedFolders, configuration.getTrustedFolders());
});
Expand Down
Loading
Loading