Restrict loading of workspace versions of TSLint

Do not load the workspace version of TSLint by default

.vscode/launch.json

@@ -6,18 +6,12 @@
     "configurations": [
         {
             "type": "node",
-            "request": "launch",
-            "name": "Mocha Tests",
-            "program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
-            "args": [
-                "-u",
-                "tdd",
-                "--timeout",
-                "999999",
-                "--colors",
-                "${workspaceFolder}/out/runner/test"
-            ],
-            "internalConsoleOptions": "openOnSessionStart"
+            "request": "attach",
+            "name": "Attach",
+            "port": 9999,
+            "skipFiles": [
+                "<node_internals>/**"
+            ]
         }
     ]
 }

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## 1.0.0 - November 30, 2020
+- Restricts when tslint is loaded from the workspace.
+    - Global TSLint versions can always be loaded.
+    - TSLint versions installed next to the plugin can always be loaded.
+    - Otherwise, the consumer of the plugin must use `onConfigurationChanged` and explicitly enable `allowWorkspaceLibraryExecution`.
+    - You can also force allow workspace versions of TSLint to be loaded by setting a `TS_TSLINT_ENABLE_WORKSPACE_LIBRARY_EXECUTION`  environment variable.
+
 ## 0.5.5 - November 11, 2019
 - Restore old cwd after linting finishes.
 

README.md

@@ -10,9 +10,9 @@ TypeScript [language service plugin](https://blogs.msdn.microsoft.com/typescript
 
 To use the plugin:
 
-* Install TSLint 5+ in your workspace or globally.
+* Install TSLint 5+ in your workspace or globally (if you are using a local TSLint, see [workspace library execution](#workspace-library-execution))
 
-* Install the plugin with `npm install typescript-tslint-plugin`
+* Install the plugin with `npm install typescript-tslint-plugin` 
 
 * Enable the plugin in your `tsconfig.json` file:
 
@@ -28,6 +28,20 @@ To use the plugin:
 
 See [editor support](#editor-support) for more detailed setup instructions.
 
+## Workspace Library Execution
+
+By default this plugin will not load TSLint or custom rules from the workspace if you are using a global version of TypeScript. This is done for security reasons. The plugin always allows using the global version of TSLint.
+
+To use enable using a local TSLint install and custom rules from the workspace, you must either:
+
+- Use a workspace version of TypeScript that is installed alongside TSLint.
+
+- Enable workspace library execution in your editor of choice. This must be done through an editor and cannot be configured in a `tsconfig`.
+
+    In VS Code for example, you can run the `TSLint: Manage Workspace Library Execution` command to enable using the TSLint for the current workspace or for all workspaces.
+
+- Set a `TS_TSLINT_ENABLE_WORKSPACE_LIBRARY_EXECUTION=1` environment variable and make sure the TypeScript server is run in an environment where this variable is set to true.
+
 ## Configuration options
 
 **Notice**: This configuration settings allow you to configure the behavior of the typescript-tslint-plugin itself. To configure rules and tslint options you should use the `tslint.json` file.

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "typescript-tslint-plugin",
-  "version": "0.5.5",
+  "version": "1.0.0",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "typescript-tslint-plugin",
-  "version": "0.5.5",
+  "version": "1.0.0",
   "description": "TypeScript tslint language service plugin",
   "main": "out/index.js",
   "author": "Microsoft",

src/index.ts

@@ -3,10 +3,14 @@ import * as ts_module from 'typescript/lib/tsserverlibrary';
 import { Logger } from './logger';
 import { ConfigurationManager } from './settings';
 import * as mockRequire from 'mock-require';
+import { WorkspaceLibraryExecution } from './runner';
+
+const enableWorkspaceLibraryExecutionEnvVar = 'TS_TSLINT_ENABLE_WORKSPACE_LIBRARY_EXECUTION';
 
 export = function init({ typescript }: { typescript: typeof ts_module }) {
     const configManager = new ConfigurationManager(typescript);
     let logger: Logger | undefined;
+    let plugin: TSLintPlugin | undefined;
 
     // Make sure TS Lint imports the correct version of TS
     mockRequire('typescript', typescript);
@@ -24,13 +28,26 @@ export = function init({ typescript }: { typescript: typeof ts_module }) {
                 return info.languageService;
             }
 
-            return new TSLintPlugin(typescript, info.languageServiceHost, logger, info.project, configManager)
-                .decorate(info.languageService);
+            plugin = new TSLintPlugin(typescript, info.languageServiceHost, logger, info.project, configManager);
+
+            // Allow clients that don't use onConfigurationChanged to still securely enable
+            // workspace library execution with an env var.
+            const workspaceLibraryFromEnv = process.env[enableWorkspaceLibraryExecutionEnvVar] ? WorkspaceLibraryExecution.Allow : WorkspaceLibraryExecution.Unknown;
+            plugin.updateWorkspaceTrust(workspaceLibraryFromEnv);
+
+            return plugin.decorate(info.languageService);
         },
         onConfigurationChanged(config: any) {
             if (logger) {
                 logger.info('onConfigurationChanged');
             }
+
+            if (plugin) {
+                if ('allowWorkspaceLibraryExecution' in config) {
+                    plugin.updateWorkspaceTrust(config.allowWorkspaceLibraryExecution ? WorkspaceLibraryExecution.Allow : WorkspaceLibraryExecution.Disallow);
+                }
+            }
+
             configManager.updateFromPluginConfig(config);
         },
     };

src/plugin.ts

@@ -4,7 +4,7 @@ import * as ts_module from 'typescript/lib/tsserverlibrary';
 import { TSLINT_ERROR_CODE, TSLINT_ERROR_SOURCE } from './config';
 import { ConfigFileWatcher } from './configFileWatcher';
 import { Logger } from './logger';
-import { RunResult, TsLintRunner, toPackageManager } from './runner';
+import { RunResult, TsLintRunner, toPackageManager, WorkspaceLibraryExecution } from './runner';
 import { ConfigurationManager } from './settings';
 import { getNonOverlappingReplacements, filterProblemsForFile, getReplacements } from './runner/failures';
 
@@ -52,7 +52,10 @@ class ProblemMap {
 export class TSLintPlugin {
     private readonly codeFixActions = new Map<string, ProblemMap>();
     private readonly configFileWatcher: ConfigFileWatcher;
-    private readonly runner: TsLintRunner;
+
+    private runner: TsLintRunner;
+
+    private workspaceTrust = WorkspaceLibraryExecution.Unknown;
 
     public constructor(
         private readonly ts: typeof ts_module,
@@ -118,6 +121,13 @@ export class TSLintPlugin {
         });
     }
 
+    public updateWorkspaceTrust(workspaceTrust: WorkspaceLibraryExecution) {
+        this.workspaceTrust = workspaceTrust;
+
+        // Reset the runner
+        this.runner = new TsLintRunner(message => { this.logger.info(message); });
+    }
+
     private getSemanticDiagnostics(
         delegate: (fileName: string) => ts_module.Diagnostic[],
         fileName: string,
@@ -150,6 +160,7 @@ export class TSLintPlugin {
                         ? Array.isArray(config.exclude) ? config.exclude : [config.exclude]
                         : [],
                     packageManager: toPackageManager(config.packageManager),
+                    workspaceLibraryExecution: this.workspaceTrust,
                 });
                 if (result.configFilePath) {
                     this.configFileWatcher.ensureWatching(result.configFilePath);

src/runner/index.ts

@@ -1,7 +1,8 @@
 import * as cp from 'child_process';
 import * as minimatch from 'minimatch';
-import { delimiter, dirname, relative } from 'path';
+import { dirname, join, normalize, relative, sep } from 'path';
 import type * as tslint from 'tslint';
+import type { IConfigurationFile } from 'tslint/lib/configuration';
 import type * as typescript from 'typescript';
 import * as util from 'util';
 import * as server from 'vscode-languageserver';
@@ -28,6 +29,29 @@ export interface RunConfiguration {
     readonly packageManager?: PackageManager;
     readonly traceLevel?: 'verbose' | 'normal';
     readonly workspaceFolderPath?: string;
+
+    /**
+     * Controls where TSlint and other scripts can be loaded from.
+     */
+    readonly workspaceLibraryExecution: WorkspaceLibraryExecution;
+}
+
+/**
+ * Controls where TSlint and other scripts can be loaded from.
+ */
+export enum WorkspaceLibraryExecution {
+    /**
+     * Block executing TSLint, linter rules, and other scripts from the current workspace.
+     */
+    Disallow = 1,
+    /**
+     * Enable loading TSLint and rules from the workspace.
+     */
+    Allow = 2,
+    /**
+     * The workspace library execution has not yet been configured or cannot be determined.
+     */
+    Unknown = 3,
 }
 
 interface Configuration {
@@ -87,8 +111,13 @@ const emptyResult: RunResult = {
 };
 
 export class TsLintRunner {
-    private readonly tslintPath2Library = new Map<string, typeof tslint | undefined>();
-    private readonly document2LibraryCache = new MruCache<() => typeof tslint | undefined>(100);
+    private readonly tslintPath2Library = new Map<string, { tslint: typeof tslint, path: string } | undefined>();
+
+    private readonly document2LibraryCache = new MruCache<{
+        readonly workspaceTslintPath: string | undefined,
+        readonly globalTsLintPath: string | undefined,
+        getTSLint(isTrusted: boolean): { tslint: typeof tslint, path: string } | undefined
+    }>(100);
 
     // map stores undefined values to represent failed resolutions
     private readonly globalPackageManagerPath = new Map<PackageManager, string | undefined>();
@@ -115,7 +144,41 @@ export class TsLintRunner {
             return emptyResult;
         }
 
-        const library = this.document2LibraryCache.get(filePath)!();
+        const cacheEntry = this.document2LibraryCache.get(filePath)!;
+
+        let library: { tslint: typeof tslint, path: string } | undefined;
+
+        switch (configuration.workspaceLibraryExecution) {
+            case WorkspaceLibraryExecution.Disallow:
+                library = cacheEntry.getTSLint(false);
+                break;
+
+            case WorkspaceLibraryExecution.Allow:
+                library = cacheEntry.getTSLint(true);
+                break;
+
+            default:
+                if (cacheEntry.workspaceTslintPath) {
+                    if (this.isWorkspaceImplicitlyTrusted(cacheEntry.workspaceTslintPath)) {
+                        configuration = { ...configuration, workspaceLibraryExecution: WorkspaceLibraryExecution.Allow };
+                        library = cacheEntry.getTSLint(true);
+                        break;
+                    }
+
+                    // If the user has not explicitly trusted/not trusted the workspace AND we have a workspace TS version
+                    // show a special error that lets the user trust/untrust the workspace
+                    return {
+                        lintResult: emptyLintResult,
+                        warnings: [
+                            getWorkspaceNotTrustedMessage(filePath),
+                        ],
+                    };
+                } else if (cacheEntry.globalTsLintPath) {
+                    library = cacheEntry.getTSLint(false);
+                }
+                break;
+        }
+
         if (!library) {
             return {
                 lintResult: emptyLintResult,
@@ -141,37 +204,61 @@ export class TsLintRunner {
 
     private loadLibrary(filePath: string, configuration: RunConfiguration): void {
         this.traceMethod('loadLibrary', `trying to load ${filePath}`);
-        const getGlobalPath = () => this.getGlobalPackageManagerPath(configuration.packageManager);
         const directory = dirname(filePath);
 
-        let tsLintPath: string;
+        const tsLintPaths = this.getTsLintPaths(directory, configuration.packageManager);
 
-        try {
-            tsLintPath = this.resolveTsLint(undefined, directory);
-            if (tsLintPath.length === 0) {
-                tsLintPath = this.resolveTsLint(getGlobalPath(), directory);
-            }
-        } catch {
-            tsLintPath = this.resolveTsLint(getGlobalPath(), directory);
-        }
+        this.traceMethod('loadLibrary', `Resolved tslint to workspace: '${tsLintPaths.workspaceTsLintPath}' global: '${tsLintPaths.globalTsLintPath}'`);
 
-        this.traceMethod('loadLibrary', `Resolved tslint to ${tsLintPath}`);
+        this.document2LibraryCache.set(filePath, {
+            workspaceTslintPath: tsLintPaths.workspaceTsLintPath || undefined,
+            globalTsLintPath: tsLintPaths.globalTsLintPath || undefined,
+            getTSLint: (allowWorkspaceLibraryExecution: boolean) => {
+                const tsLintPath = allowWorkspaceLibraryExecution
+                    ? tsLintPaths.workspaceTsLintPath || tsLintPaths.globalTsLintPath
+                    : tsLintPaths.globalTsLintPath;
 
-        this.document2LibraryCache.set(filePath, () => {
-            let library;
-            if (!this.tslintPath2Library.has(tsLintPath)) {
-                try {
-                    library = require(tsLintPath);
-                } catch (e) {
-                    this.tslintPath2Library.set(tsLintPath, undefined);
+                if (!tsLintPath) {
                     return;
                 }
-                this.tslintPath2Library.set(tsLintPath, library);
+
+                let library;
+                if (!this.tslintPath2Library.has(tsLintPath)) {
+                    try {
+                        library = require(tsLintPath);
+                    } catch (e) {
+                        this.tslintPath2Library.set(tsLintPath, undefined);
+                        return;
+                    }
+                    this.tslintPath2Library.set(tsLintPath, { tslint: library, path: tsLintPath });
+                }
+                return this.tslintPath2Library.get(tsLintPath);
             }
-            return this.tslintPath2Library.get(tsLintPath);
         });
     }
 
+    private getTsLintPaths(directory: string, packageManager: PackageManager | undefined) {
+        const globalPath = this.getGlobalPackageManagerPath(packageManager);
+
+        let workspaceTsLintPath: string | undefined;
+        try {
+            workspaceTsLintPath = this.resolveTsLint({ nodePath: undefined, cwd: directory }) || undefined;
+        } catch {
+            // noop
+        }
+
+        let globalTSLintPath: string | undefined;
+        try {
+            globalTSLintPath = this.resolveTsLint({ nodePath: undefined, cwd: globalPath });
+        } catch {
+            // noop
+        }
+        if (!globalTSLintPath) {
+            globalTSLintPath = this.resolveTsLint({ nodePath: globalPath, cwd: globalPath });
+        }
+        return { workspaceTsLintPath, globalTsLintPath: globalTSLintPath };
+    }
+
     private getGlobalPackageManagerPath(packageManager: PackageManager = 'npm'): string | undefined {
         this.traceMethod('getGlobalPackageManagerPath', `Begin - Resolve Global Package Manager Path for: ${packageManager}`);
 
@@ -193,7 +280,7 @@ export class TsLintRunner {
     private doRun(
         filePath: string,
         contents: string | typescript.Program,
-        library: typeof import('tslint'),
+        library: { tslint: typeof tslint, path: string },
         configuration: RunConfiguration,
         warnings: string[],
     ): RunResult {
@@ -210,7 +297,7 @@ export class TsLintRunner {
         }
 
         let cwdToRestore: string | undefined;
-        if (cwd) {
+        if (cwd && configuration.workspaceLibraryExecution === WorkspaceLibraryExecution.Allow) {
             this.traceMethod('doRun', `Changed directory to ${cwd}`);
             cwdToRestore = process.cwd();
             process.chdir(cwd);
@@ -221,7 +308,7 @@ export class TsLintRunner {
             let linterConfiguration: Configuration | undefined;
             this.traceMethod('doRun', 'About to getConfiguration');
             try {
-                linterConfiguration = this.getConfiguration(filePath, filePath, library, configFile);
+                linterConfiguration = this.getConfiguration(filePath, filePath, library.tslint, configFile);
             } catch (err) {
                 this.traceMethod('doRun', `No linting: exception when getting tslint configuration for ${filePath}, configFile= ${configFile}`);
                 warnings.push(getConfigurationFailureMessage(err));
@@ -253,10 +340,16 @@ export class TsLintRunner {
             }
 
             let result: tslint.LintResult;
+
+            const isTrustedWorkspace = configuration.workspaceLibraryExecution === WorkspaceLibraryExecution.Allow;
+
+            // Only allow using a custom rules directory if the workspace has been trusted by the user
+            const rulesDirectory = isTrustedWorkspace ? configuration.rulesDirectory : [];
+
             const options: tslint.ILinterOptions = {
                 formatter: "json",
                 fix: false,
-                rulesDirectory: configuration.rulesDirectory || undefined,
+                rulesDirectory,
                 formattersDirectory: undefined,
             };
             if (configuration.traceLevel && configuration.traceLevel === 'verbose') {
@@ -271,10 +364,16 @@ export class TsLintRunner {
             };
             console.warn = captureWarnings;
 
+            const sanitizedLintConfiguration = { ...linterConfiguration.linterConfiguration } as IConfigurationFile;
+            // Only allow using a custom rules directory if the workspace has been trusted by the user
+            if (!isTrustedWorkspace) {
+                sanitizedLintConfiguration.rulesDirectory = [];
+            }
+
             try { // clean up if tslint crashes
-                const linter = new library.Linter(options, typeof contents === 'string' ? undefined : contents);
+                const linter = new library.tslint.Linter(options, typeof contents === 'string' ? undefined : contents);
                 this.traceMethod('doRun', `Linting: start linting`);
-                linter.lint(filePath, typeof contents === 'string' ? contents : '', linterConfiguration.linterConfiguration);
+                linter.lint(filePath, typeof contents === 'string' ? contents : '', sanitizedLintConfiguration);
                 result = linter.getResult();
                 this.traceMethod('doRun', `Linting: ended linting`);
             } finally {
@@ -294,6 +393,21 @@ export class TsLintRunner {
         }
     }
 
+    /**
+     * Check if `tslintPath` is next to the running TS version. This indicates that the user has
+     * implicitly trusted the workspace since they are already running TS from it.
+     */
+    private isWorkspaceImplicitlyTrusted(tslintPath: string): boolean {
+        const tsPath = process.argv[1];
+        const nodeModulesPath = join(tsPath, '..', '..', '..');
+
+        const rel = relative(nodeModulesPath, normalize(tslintPath));
+        if (rel === `tslint${sep}lib${sep}index.js`) {
+            return true;
+        }
+        return false;
+    }
+
     private getConfiguration(uri: string, filePath: string, library: typeof tslint, configFileName: string | null): Configuration | undefined {
         this.traceMethod('getConfiguration', `Starting for ${uri}`);
 
@@ -350,7 +464,7 @@ export class TsLintRunner {
         this.trace("tslint configuration:" + util.inspect(configuration, undefined, 4));
     }
 
-    private resolveTsLint(nodePath: string | undefined, cwd: string): string {
+    private resolveTsLint(options: { nodePath: string | undefined; cwd: string | undefined; }): string | undefined {
         const nodePathKey = 'NODE_PATH';
         const app = [
             "console.log(require.resolve('tslint'));",
@@ -359,17 +473,13 @@ export class TsLintRunner {
         const env = process.env;
         const newEnv = Object.create(null);
         Object.keys(env).forEach(key => newEnv[key] = env[key]);
-        if (nodePath) {
-            if (newEnv[nodePathKey]) {
-                newEnv[nodePathKey] = nodePath + delimiter + newEnv[nodePathKey];
-            } else {
-                newEnv[nodePathKey] = nodePath;
-            }
-            this.traceMethod('resolveTsLint', `NODE_PATH value is: ${newEnv[nodePathKey]}`);
+        if (options.nodePath) {
+            newEnv[nodePathKey] = options.nodePath;
         }
         newEnv.ELECTRON_RUN_AS_NODE = '1';
-        const spawnResults = cp.spawnSync(process.argv0, ['-e', app], { cwd, env: newEnv });
-        return spawnResults.stdout.toString().trim();
+
+        const spawnResults = cp.spawnSync(process.argv0, ['-e', app], { cwd: options.cwd, env: newEnv });
+        return spawnResults.stdout.toString().trim() || undefined;
     }
 }
 
@@ -407,6 +517,13 @@ function getInstallFailureMessage(filePath: string, packageManager: PackageManag
     ].join('\n');
 }
 
+function getWorkspaceNotTrustedMessage(filePath: string) {
+    return [
+        `Not using the local TSLint version found for '${filePath}'`,
+        'To enable code execution from the current workspace you must enable workspace library execution.',
+    ].join('\n');
+}
+
 function isJsDocument(filePath: string): boolean {
     return /\.(jsx?|mjs)$/i.test(filePath);
 }

src/runner/test/runner.test.ts

@@ -2,7 +2,7 @@ import { expect } from 'chai';
 import * as fs from 'fs';
 import 'mocha';
 import * as path from 'path';
-import { RunConfiguration, TsLintRunner } from '../index';
+import { RunConfiguration, TsLintRunner, WorkspaceLibraryExecution } from '../index';
 import { getNonOverlappingReplacements, filterProblemsForFile } from '../failures';
 
 const testDataRoot = path.join(__dirname, '..', '..', '..', 'test-data');
@@ -11,6 +11,7 @@ const defaultRunConfiguration: RunConfiguration = {
     exclude: [],
     jsEnable: false,
     ignoreDefinitionFiles: true,
+    workspaceLibraryExecution: WorkspaceLibraryExecution.Unknown
 };
 
 describe('TSLintRunner', () => {

tsconfig.json

@@ -7,6 +7,7 @@
 		"noImplicitAny": true,
 		"noUnusedParameters": true,
 		"noUnusedLocals": true,
+		"forceConsistentCasingInFileNames": true,
 		"lib": [
 			"es6"
 		],