Skip to content

Commit

Permalink
(feat) add workspace trust (#1086)
Browse files Browse the repository at this point in the history
#1051

Not removing the the ls server path scope for now to ensure backwards compatibility. Lift that restriction and bump the minimum required VS Code version later.
  • Loading branch information
dummdidumm authored Jul 8, 2021
1 parent 0e8f5b2 commit 7abeef4
Show file tree
Hide file tree
Showing 9 changed files with 83 additions and 5 deletions.
17 changes: 16 additions & 1 deletion packages/language-server/src/importPackage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,17 @@ import * as svelte from 'svelte/compiler';
import sveltePreprocess from 'svelte-preprocess';
import { Logger } from './logger';

/**
* Whether or not the current workspace can be trusted.
* TODO rework this to a class which depends on the LsConfigManager
* and inject that class into all places where it's needed (Document etc.)
*/
let isTrusted = true;

export function setIsTrusted(_isTrusted: boolean) {
isTrusted = _isTrusted;
}

/**
* This function encapsulates the require call in one place
* so we can replace its content inside rollup builds
Expand All @@ -15,8 +26,12 @@ function dynamicRequire(dynamicFileToRequire: string): any {
}

export function getPackageInfo(packageName: string, fromPath: string) {
const paths = [__dirname];
if (isTrusted) {
paths.unshift(fromPath);
}
const packageJSONPath = require.resolve(`${packageName}/package.json`, {
paths: [fromPath, __dirname]
paths
});
// eslint-disable-next-line @typescript-eslint/no-var-requires
const { version } = dynamicRequire(packageJSONPath);
Expand Down
12 changes: 11 additions & 1 deletion packages/language-server/src/lib/documents/configLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export class ConfigLoader {
private configFiles = new Map<string, SvelteConfig>();
private configFilesAsync = new Map<string, Promise<SvelteConfig>>();
private filePathToConfigPath = new Map<string, string>();
private disabled = false;

constructor(
private globSync: typeof _glob.sync,
Expand All @@ -60,6 +61,13 @@ export class ConfigLoader {
private dynamicImport: typeof _dynamicImport
) {}

/**
* Enable/disable loading of configs (for security reasons for example)
*/
setDisabled(disabled: boolean): void {
this.disabled = disabled;
}

/**
* Tries to load all `svelte.config.js` files below given directory
* and the first one found inside/above that directory.
Expand Down Expand Up @@ -141,7 +149,9 @@ export class ConfigLoader {

private async loadConfig(configPath: string, directory: string) {
try {
let config = (await this.dynamicImport(pathToFileURL(configPath)))?.default;
let config = this.disabled
? {}
: (await this.dynamicImport(pathToFileURL(configPath)))?.default;
config = {
...config,
compilerOptions: {
Expand Down
13 changes: 13 additions & 0 deletions packages/language-server/src/ls-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ export class LSConfigManager {
};
private prettierConfig: any = {};
private emmetConfig: VSCodeEmmetConfig = {};
private isTrusted = true;

/**
* Updates config.
Expand Down Expand Up @@ -324,6 +325,18 @@ export class LSConfigManager {
});
}

/**
* Whether or not the current workspace can be trusted.
* If not, certain operations should be disabled.
*/
getIsTrusted(): boolean {
return this.isTrusted;
}

updateIsTrusted(isTrusted: boolean): void {
this.isTrusted = isTrusted;
}

private _updateTsUserPreferences(lang: TsUserConfigLang, config: TSUserConfig) {
this.tsUserPreferences[lang] = {
...this.tsUserPreferences[lang],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export class SveltePlugin
constructor(private configManager: LSConfigManager) {}

async getDiagnostics(document: Document): Promise<Diagnostic[]> {
if (!this.featureEnabled('diagnostics')) {
if (!this.featureEnabled('diagnostics') || !this.configManager.getIsTrusted()) {
return [];
}

Expand Down
10 changes: 10 additions & 0 deletions packages/language-server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import {
} from './plugins';
import { debounceThrottle, isNotNullOrUndefined, normalizeUri, urlToPath } from './utils';
import { FallbackWatcher } from './lib/FallbackWatcher';
import { configLoader } from './lib/documents/configLoader';
import { setIsTrusted } from './importPackage';

namespace TagCloseRequest {
export const type: RequestType<TextDocumentPositionParams, string | null, any> =
Expand Down Expand Up @@ -102,6 +104,14 @@ export function startServer(options?: LSOptions) {
watcher.onDidChangeWatchedFiles(onDidChangeWatchedFiles);
}

const isTrusted: boolean = evt.initializationOptions?.isTrusted ?? true;
configLoader.setDisabled(!isTrusted);
setIsTrusted(isTrusted);
configManager.updateIsTrusted(isTrusted);
if (!isTrusted) {
Logger.log('Workspace is not trusted, running with reduced capabilities.');
}

// Backwards-compatible way of setting initialization options (first `||` is the old style)
configManager.update(
evt.initializationOptions?.configuration?.svelte?.plugin ||
Expand Down
14 changes: 14 additions & 0 deletions packages/language-server/test/lib/documents/configLoader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { ConfigLoader } from '../../../src/lib/documents/configLoader';
import path from 'path';
import { pathToFileURL, URL } from 'url';
import assert from 'assert';
import { spy } from 'sinon';

describe('ConfigLoader', () => {
function configFrom(path: string) {
Expand Down Expand Up @@ -163,4 +164,17 @@ describe('ConfigLoader', () => {
configFrom(normalizePath('some/svelte.config.js'))
);
});

it('should not load config when disabled', async () => {
const moduleLoader = spy();
const configLoader = new ConfigLoader(
() => [],
{ existsSync: () => true },
path,
moduleLoader
);
configLoader.setDisabled(true);
await configLoader.awaitConfig(normalizePath('some/file.svelte'));
assert.deepStrictEqual(moduleLoader.notCalled, true);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ import * as importPackage from '../../../src/importPackage';
import sinon from 'sinon';

describe('Svelte Plugin', () => {
function setup(content: string, prettierConfig?: any) {
function setup(content: string, prettierConfig?: any, trusted = true) {
const document = new Document('file:///hello.svelte', content);
const docManager = new DocumentManager(() => document);
const pluginManager = new LSConfigManager();
pluginManager.updateIsTrusted(trusted);
pluginManager.updatePrettierConfig(prettierConfig);
const plugin = new SveltePlugin(pluginManager);
docManager.openDocument(<any>'some doc');
Expand Down Expand Up @@ -52,6 +53,14 @@ describe('Svelte Plugin', () => {
assert.deepStrictEqual(diagnostics, [diagnostic]);
});

it('provides no diagnostic errors when untrusted', async () => {
const { plugin, document } = setup('<div bind:whatever></div>', {}, false);

const diagnostics = await plugin.getDiagnostics(document);

assert.deepStrictEqual(diagnostics, []);
});

describe('#formatDocument', () => {
function stubPrettier(config: any) {
const formatStub = sinon.stub().returns('formatted');
Expand Down
6 changes: 6 additions & 0 deletions packages/svelte-vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@
"onLanguage:svelte",
"onCommand:svelte.restartLanguageServer"
],
"capabilities": {
"untrustedWorkspaces": {
"supported": "limited",
"description": "The extension requires workspace trust because it executes code specified by the workspace. Loading the user's node_modules and loading svelte config files is disabled when untrusted"
}
},
"contributes": {
"typescriptServerPlugins-disabled": [
{
Expand Down
3 changes: 2 additions & 1 deletion packages/svelte-vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ export function activate(context: ExtensionContext) {
typescript: workspace.getConfiguration('typescript'),
javascript: workspace.getConfiguration('javascript')
},
dontFilterIncompleteCompletions: true // VSCode filters client side and is smarter at it than us
dontFilterIncompleteCompletions: true, // VSCode filters client side and is smarter at it than us
isTrusted: (workspace as any).isTrusted
}
};

Expand Down

0 comments on commit 7abeef4

Please sign in to comment.