Require VS Code consent for custom theme checks

.changeset/bright-themes-ask.md

@@ -0,0 +1,8 @@
+---
+'@shopify/theme-check-node': minor
+'@shopify/theme-language-server-common': minor
+'@shopify/theme-language-server-node': minor
+'theme-check-vscode': minor
+---
+
+Ask for explicit VS Code consent before executing custom Theme Check code selected by a theme configuration.

packages/theme-check-node/src/config/load-config-description.ts

@@ -10,7 +10,7 @@ import { fileExists } from '../file-utils';
 import { AbsolutePath } from '../temp';
 import { thisNodeModuleRoot } from './installation-location';
 import { findThirdPartyChecks, loadThirdPartyChecks } from './load-third-party-checks';
-import { ConfigDescription } from './types';
+import { ConfigDescription, CustomCheckCandidate, LoadConfigOptions } from './types';
 import { URI, Utils } from 'vscode-uri';
 
 const flatten = <T>(arrs: T[][]): T[] => arrs.flat();
@@ -24,16 +24,27 @@ const flatten = <T>(arrs: T[][]): T[] => arrs.flat();
 export async function loadConfigDescription(
   configDescription: ConfigDescription,
   root: AbsolutePath,
+  options: LoadConfigOptions = {},
 ): Promise<Config> {
   const nodeModuleRoot = await findNodeModuleRoot(root);
-  const thirdPartyChecksPaths = await Promise.all([
+  const [globalThirdPartyChecksPaths, workspaceThirdPartyChecksPaths] = await Promise.all([
     findThirdPartyChecks(thisNodeModuleRoot()), // global checks
     findThirdPartyChecks(nodeModuleRoot),
-  ]).then(flatten);
-  const thirdPartyChecks = loadThirdPartyChecks([
-    ...configDescription.require,
-    ...thirdPartyChecksPaths,
   ]);
+  const customCheckCandidates = uniqueCandidates([
+    ...configDescription.require.map((path) => ({ source: 'require' as const, path })),
+    ...flatten([globalThirdPartyChecksPaths, workspaceThirdPartyChecksPaths]).map((path) => ({
+      source: 'discovery' as const,
+      path,
+    })),
+  ]);
+  const customChecksAuthorized =
+    customCheckCandidates.length === 0 ||
+    !options.authorizeCustomChecks ||
+    (await options.authorizeCustomChecks({ root, candidates: customCheckCandidates }));
+  const thirdPartyChecks = customChecksAuthorized
+    ? loadThirdPartyChecks(customCheckCandidates.map(({ path }) => path))
+    : [];
   const checks: CheckDefinition<SourceCodeType>[] = allChecks
     .concat(thirdPartyChecks)
     .filter(isEnabledBy(configDescription));
@@ -48,6 +59,15 @@ export async function loadConfigDescription(
   };
 }
 
+function uniqueCandidates(candidates: CustomCheckCandidate[]): CustomCheckCandidate[] {
+  const seen = new Set<string>();
+  return candidates.filter(({ path }) => {
+    if (seen.has(path)) return false;
+    seen.add(path);
+    return true;
+  });
+}
+
 /**
  * @param root - absolute path of the config file
  * @param pathLike - resolved textual value of the `root` property from the config files

packages/theme-check-node/src/config/load-config.spec.ts

@@ -67,6 +67,15 @@
     expect(config.ignore).to.include('src/**');
   });
 
+  it('does not request authorization when there are no custom checks', async () => {
+    const configPath = await createMockConfigFile(tempDir, `extends: theme-check:recommended`);
+    const authorizeCustomChecks = vi.fn().mockResolvedValue(false);
+
+    await loadConfig(configPath, tempDir, { authorizeCustomChecks });
+
+    expect(authorizeCustomChecks).not.toHaveBeenCalled();
+  });
+
   it('has no checks if it extends nothing', async () => {
     const configPath = await createMockConfigFile(tempDir, `extends: nothing`);
     const config = await loadConfig(configPath, tempDir);
@@ -217,6 +226,90 @@
     expect(nodeModuleCheck).to.exist;
   });
 
+  it('does not execute a required extension before it is authorized', async () => {
+    const configPath = await createMockConfigFile(
+      tempDir,
+      `
+extends: nothing
+require: './checks.js'
+NodeModuleCheck:
+  enabled: true
+      `,
+    );
+    const markerPath = path.join(tempDir, 'executed');
+    await fs.writeFile(
+      path.join(tempDir, 'checks.js'),
+      `require('node:fs').writeFileSync(${JSON.stringify(markerPath)}, 'yes');\n${mockNodeModuleCheck}`,
+    );
+    const authorizeCustomChecks = vi.fn().mockResolvedValue(false);
+
+    const config = await loadConfig(configPath, tempDir, { authorizeCustomChecks });
+
+    expect(authorizeCustomChecks).toHaveBeenCalledWith({
+      root: tempDir,
+      candidates: [{ source: 'require', path: path.join(tempDir, 'checks.js') }],
+    });
+    expect(config.checks.find((check) => check.meta.code === 'NodeModuleCheck')).not.to.exist;
+    await expect(fs.stat(markerPath)).rejects.toThrow();
+  });
+
+  it('asks for authorization for extensions inherited through extends', async () => {
+    await fs.writeFile(
+      path.join(tempDir, 'base.yml'),
+      `
+extends: nothing
+require: './checks.js'
+      `,
+    );
+    await fs.writeFile(path.join(tempDir, 'checks.js'), mockNodeModuleCheck);
+    const configPath = await createMockConfigFile(tempDir, `extends: './base.yml'`);
+    const authorizeCustomChecks = vi.fn().mockResolvedValue(false);
+
+    await loadConfig(configPath, tempDir, { authorizeCustomChecks });
+
+    expect(authorizeCustomChecks).toHaveBeenCalledWith({
+      root: tempDir,
+      candidates: [{ source: 'require', path: path.join(tempDir, 'checks.js') }],
+    });
+  });
+
+  it('asks for authorization before loading automatically discovered extensions', async () => {
+    const configPath = path.resolve(__dirname, 'fixtures/node-module-rec.yml');
+    const modulePath = await createMockNodeModule(
+      tempDir,
+      'theme-check-node-example',
+      mockNodeModuleCheck,
+    );
+    const authorizeCustomChecks = vi.fn().mockResolvedValue(false);
+
+    const config = await loadConfig(configPath, tempDir, { authorizeCustomChecks });
+
+    expect(authorizeCustomChecks).toHaveBeenCalledWith({
+      root: tempDir,
+      candidates: [{ source: 'discovery', path: modulePath }],
+    });
+    expect(config.checks.find((check) => check.meta.code === 'NodeModuleCheck')).not.to.exist;
+  });
+
+  it('loads custom checks after they are authorized', async () => {
+    const configPath = await createMockConfigFile(
+      tempDir,
+      `
+extends: nothing
+require: './checks.js'
+NodeModuleCheck:
+  enabled: true
+      `,
+    );
+    await fs.writeFile(path.join(tempDir, 'checks.js'), mockNodeModuleCheck);
+
+    const config = await loadConfig(configPath, tempDir, {
+      authorizeCustomChecks: vi.fn().mockResolvedValue(true),
+    });
+
+    expect(config.checks.find((check) => check.meta.code === 'NodeModuleCheck')).to.exist;
+  });
+
   it('loads an aliased check properly', async () => {
     const configPath = await createMockConfigFile(
       tempDir,

packages/theme-check-node/src/config/load-config.ts

@@ -2,7 +2,7 @@ import { Config } from '@shopify/theme-check-common';
 import { AbsolutePath } from '../temp';
 import { loadConfigDescription } from './load-config-description';
 import { resolveConfig } from './resolve';
-import { ModernIdentifier } from './types';
+import { LoadConfigOptions, ModernIdentifier } from './types';
 import { validateConfig } from './validation';
 import fs from 'fs/promises';
 
@@ -21,6 +21,7 @@ export async function loadConfig(
   configPath: AbsolutePath | ModernIdentifier | undefined,
   /** The root of the theme */
   root: AbsolutePath,
+  options: LoadConfigOptions = {},
 ): Promise<Config> {
   if (!root) throw new Error('loadConfig cannot be called without a root argument');
   let defaultChecks = 'theme-check:recommended';
@@ -35,7 +36,7 @@ export async function loadConfig(
   }
 
   const configDescription = await resolveConfig(configPath ?? defaultChecks, true);
-  const config = await loadConfigDescription(configDescription, root);
+  const config = await loadConfigDescription(configDescription, root, options);
   validateConfig(config);
   return config;
 }

packages/theme-check-node/src/config/types.ts

@@ -30,6 +30,25 @@ export type ConfigDescription = Omit<ConfigFragment, 'extends' | 'context'> & {
   context: Mode;
 };
 
+export interface CustomCheckCandidate {
+  source: 'require' | 'discovery';
+  path: string;
+}
+
+export interface CustomCheckAuthorizationRequest {
+  root: string;
+  candidates: CustomCheckCandidate[];
+}
+
+export interface LoadConfigOptions {
+  /**
+   * Called after custom checks have been discovered, but before any of their
+   * JavaScript is loaded. When omitted, custom checks retain their existing
+   * behaviour and are loaded without an additional authorization step.
+   */
+  authorizeCustomChecks?: (request: CustomCheckAuthorizationRequest) => Promise<boolean>;
+}
+
 export const ModernIdentifiers = [
   'theme-check:nothing',
   'theme-check:recommended',

packages/theme-language-server-common/src/types.ts

@@ -148,6 +148,23 @@ export namespace ThemeGraphDidUpdateNotification {
   }
 }
 
+export namespace CustomCheckPermissionRequest {
+  export const method = 'themeCheck/requestCustomCheckPermission';
+  export const type = new rpc.RequestType<Params, Response, void>(method);
+
+  export interface Candidate {
+    source: 'require' | 'discovery';
+    path: string;
+  }
+
+  export interface Params {
+    root: string;
+    candidates: Candidate[];
+  }
+
+  export type Response = boolean;
+}
+
 export type AugmentedLocationWithExistence = {
   uri: string;
   range: undefined;

packages/theme-language-server-node/src/dependencies.ts

@@ -1,6 +1,7 @@
 import {
   AbstractFileSystem,
   Config,
+  LoadConfigOptions,
   findRoot,
   loadConfig as nodeLoadConfig,
   makeFileExists,
@@ -28,7 +29,11 @@ const hasThemeAppExtensionConfig = async (rootUri: string, fs: AbstractFileSyste
   return files.length > 0;
 };
 
-export const loadConfig: Dependencies['loadConfig'] = async function loadConfig(uriString, fs) {
+export async function loadConfig(
+  uriString: string,
+  fs: AbstractFileSystem,
+  options: LoadConfigOptions = {},
+): Promise<Config> {
   const fileUri = path.normalize(uriString);
   const fileExists = makeFileExists(fs);
   const rootUriString = await findRoot(fileUri, fileExists);
@@ -47,11 +52,13 @@ export const loadConfig: Dependencies['loadConfig'] = async function loadConfig(
     const configPath = asFsPath(configUri);
     const rootPath = asFsPath(rootUri);
     if (configExists) {
-      return nodeLoadConfig(configPath, rootPath).then(normalizeRoot);
+      return nodeLoadConfig(configPath, rootPath, options).then(normalizeRoot);
     } else if (isDefinitelyThemeAppExtension) {
-      return nodeLoadConfig('theme-check:theme-app-extension', rootPath).then(normalizeRoot);
+      return nodeLoadConfig('theme-check:theme-app-extension', rootPath, options).then(
+        normalizeRoot,
+      );
     } else {
-      return nodeLoadConfig(undefined, rootPath).then(normalizeRoot);
+      return nodeLoadConfig(undefined, rootPath, options).then(normalizeRoot);
     }
   } else {
     // We can't load configs properly in remote environments.
@@ -64,7 +71,7 @@ export const loadConfig: Dependencies['loadConfig'] = async function loadConfig(
       rootUri: path.normalize(rootUri),
     };
   }
-};
+}
 
 function normalizeRoot(config: Config) {
   config.rootUri = path.normalize(config.rootUri);

packages/theme-language-server-node/src/index.ts

@@ -1,5 +1,9 @@
 import { ThemeLiquidDocsManager } from '@shopify/theme-check-docs-updater';
-import { AbstractFileSystem, NodeFileSystem } from '@shopify/theme-check-node';
+import {
+  AbstractFileSystem,
+  CustomCheckAuthorizationRequest,
+  NodeFileSystem,
+} from '@shopify/theme-check-node';
 import { startServer as startCoreServer } from '@shopify/theme-language-server-common';
 import { stdin, stdout } from 'node:process';
 import { createConnection } from 'vscode-languageserver/node';
@@ -11,15 +15,24 @@ export * from '@shopify/theme-language-server-common';
 
 export const getConnection = () => createConnection(stdin, stdout);
 
-export function startServer(connection = getConnection(), fs: AbstractFileSystem = NodeFileSystem) {
+export interface StartServerOptions {
+  authorizeCustomChecks?: (request: CustomCheckAuthorizationRequest) => Promise<boolean>;
+}
+
+export function startServer(
+  connection = getConnection(),
+  fs: AbstractFileSystem = NodeFileSystem,
+  options: StartServerOptions = {},
+) {
   // Using console.error to not interfere with messages sent on STDIN/OUT
   const log = (message: string) => console.error(message);
   const themeLiquidDocsManager = new ThemeLiquidDocsManager(log);
 
   startCoreServer(connection, {
     fs,
     log,
-    loadConfig,
+    loadConfig: (uri, fs) =>
+      loadConfig(uri, fs, { authorizeCustomChecks: options.authorizeCustomChecks }),
     themeDocset: themeLiquidDocsManager,
     jsonValidationSet: themeLiquidDocsManager,
     fetchMetafieldDefinitionsForURI,