Skip to content

Commit

Permalink
feat: Add untrusted mode to the config loader (#5127)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jason3S authored Dec 30, 2023
1 parent 3fd9696 commit 0ed8c1e
Show file tree
Hide file tree
Showing 9 changed files with 283 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,42 @@ describe('CSpellConfigFileReaderWriter', () => {
expect(cfg2.settings).not.toBe(cfg0.settings);
expect(cfg2.settings).toEqual(cfg0.settings);
});

test.each`
uri | content | expected
${'file:///package.json'} | ${json({ name: 'name' })} | ${'Untrusted URL: "file:///package.json"'}
${'safe-fs:///path/cspell.config.js'} | ${json({ name: 'name' })} | ${'Untrusted URL: "safe-fs:///path/cspell.config.js"'}
`('readConfig untrusted', async ({ uri, content, expected }) => {
const io: IO = {
readFile: vi.fn((url) => Promise.resolve({ url, content })),
writeFile: vi.fn(),
};
const rw = new CSpellConfigFileReaderWriterImpl(io, defaultDeserializers, defaultLoaders);
rw.setUntrustedExtensions(['.json', '.js']);

expect(rw.trustedUrls).toEqual([]);
expect(rw.untrustedExtensions).toEqual(['.json', '.js']);

await expect(rw.readConfig(uri)).rejects.toThrowError(expected);
});

test.each`
uri | content | expected
${'file:///package.json'} | ${json({ name: 'name' })} | ${oc({ url: new URL('file:///package.json') })}
${'safe-fs:///code/sample.json'} | ${json({ name: 'name' })} | ${oc({ url: new URL('safe-fs:///code/sample.json'), settings: { name: 'name' } })}
`('readConfig trusted', async ({ uri, content, expected }) => {
const io: IO = {
readFile: vi.fn((url) => Promise.resolve({ url, content })),
writeFile: vi.fn(),
};
const rw = new CSpellConfigFileReaderWriterImpl(io, defaultDeserializers, defaultLoaders);
rw.setUntrustedExtensions(['.json', '.js']).setTrustedUrls(['file:///package.json', 'safe-fs:///']);

expect(rw.trustedUrls.toString()).toEqual('file:///package.json,safe-fs:///');
expect(rw.untrustedExtensions).toEqual(['.json', '.js']);

await expect(rw.readConfig(uri)).resolves.toEqual(expected);
});
});

class Cfg extends CSpellConfigFile {
Expand Down
64 changes: 64 additions & 0 deletions packages/cspell-config-lib/src/CSpellConfigFileReaderWriter.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { extname } from 'node:path/posix';

import type { CSpellConfigFile, ICSpellConfigFile } from './CSpellConfigFile.js';
import type { FileLoaderMiddleware } from './FileLoader.js';
import type { IO } from './IO.js';
Expand All @@ -13,6 +15,17 @@ export interface CSpellConfigFileReaderWriter {
readConfig(uri: URL | string): Promise<CSpellConfigFile>;
writeConfig(configFile: CSpellConfigFile): Promise<TextFileRef>;
clearCachedFiles(): void;
setUntrustedExtensions(ext: readonly string[]): this;
setTrustedUrls(urls: readonly (URL | string)[]): this;
/**
* Untrusted extensions are extensions that are not trusted to be loaded from a file system.
* Extension are case insensitive and should include the leading dot.
*/
readonly untrustedExtensions: string[];
/**
* Urls starting with these urls are trusted to be loaded from a file system.
*/
readonly trustedUrls: URL[];
}

export class CSpellConfigFileReaderWriterImpl implements CSpellConfigFileReaderWriter {
Expand All @@ -27,7 +40,30 @@ export class CSpellConfigFileReaderWriterImpl implements CSpellConfigFileReaderW
readonly loaders: FileLoaderMiddleware[],
) {}

private _untrustedExtensions = new Set<string>();
private _trustedUrls: string[] = [];

/**
* Untrusted extensions are extensions that are not trusted to be loaded from a file system.
* Extension are case insensitive and should include the leading dot.
*/
get untrustedExtensions(): string[] {
return [...this._untrustedExtensions];
}

/**
* Urls starting with these urls are trusted to be loaded from a file system.
*/
get trustedUrls(): URL[] {
return [...this._trustedUrls].map((url) => new URL(url));
}

readConfig(uri: URL | string): Promise<CSpellConfigFile> {
const url = new URL(uri);
if (!isTrusted(url, this._trustedUrls, this._untrustedExtensions)) {
return Promise.reject(new UntrustedUrlError(url));
}

const loader = getLoader(this.loaders);
return loader({ url: toURL(uri), context: { deserialize: this.getDeserializer(), io: this.io } });
}
Expand All @@ -48,9 +84,37 @@ export class CSpellConfigFileReaderWriterImpl implements CSpellConfigFileReaderW
return { url: configFile.url };
}

setUntrustedExtensions(ext: readonly string[]): this {
this._untrustedExtensions.clear();
ext.forEach((e) => this._untrustedExtensions.add(e.toLowerCase()));
return this;
}

setTrustedUrls(urls: readonly (URL | string)[]): this {
this._trustedUrls = [...new Set([...urls.map((url) => new URL(url).href)])].sort();
return this;
}

clearCachedFiles(): void {
for (const loader of this.loaders) {
loader.reset?.();
}
}
}

function isTrusted(url: URL, trustedUrls: string[], untrustedExtensions: Set<string>): boolean {
const path = url.pathname;
const ext = extname(path).toLowerCase();

if (!untrustedExtensions.has(ext)) return true;

const href = url.href;

return trustedUrls.some((trustedUrl) => href.startsWith(trustedUrl));
}

export class UntrustedUrlError extends Error {
constructor(url: URL) {
super(`Untrusted URL: "${url.href}"`);
}
}
3 changes: 3 additions & 0 deletions packages/cspell-lib/api/api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ interface IConfigLoader {
* @returns the resulting settings
*/
searchForConfig(searchFrom: URL | string | undefined, pnpSettings?: PnPSettingsOptional): Promise<CSpellSettingsI | undefined>;
resolveConfigFileLocation(filenameOrURL: string | URL, relativeTo?: string | URL): Promise<URL | undefined>;
getGlobalSettingsAsync(): Promise<CSpellSettingsI>;
/**
* The loader caches configuration files for performance. This method clears the cache.
Expand All @@ -475,6 +476,8 @@ interface IConfigLoader {
*/
dispose(): void;
getStats(): Readonly<Record<string, Readonly<Record<string, number>>>>;
readonly isTrusted: boolean;
setIsTrusted(isTrusted: boolean): void;
}
declare function loadPnP(pnpSettings: PnPSettingsOptional, searchFrom: URL): Promise<LoaderResult>;
declare function createConfigLoader(fs?: VFileSystem): IConfigLoader;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ import {
pathPackageSamples,
pathPackageSamplesURL,
pathRepoRoot,
pathRepoRootURL,
pathRepoTestFixtures,
pathRepoTestFixturesURL,
} from '../../../../test-util/test.locations.cjs';
import { logError, logWarning } from '../../../util/logger.js';
import { resolveFileWithURL, toFilePathOrHref, toFileUrl } from '../../../util/url.js';
import { currentSettingsFileVersion, ENV_CSPELL_GLOB_ROOT } from '../../constants.js';
import { currentSettingsFileVersion, defaultConfigFileModuleRef, ENV_CSPELL_GLOB_ROOT } from '../../constants.js';
import type { ImportFileRefWithError } from '../../CSpellSettingsServer.js';
import { extractDependencies, getSources, mergeSettings } from '../../CSpellSettingsServer.js';
import { _defaultSettings, getDefaultBundledSettingsAsync } from '../../DefaultSettings.js';
Expand Down Expand Up @@ -623,9 +624,10 @@ describe('Validate search/load config files', () => {

describe('ConfigLoader with VirtualFS', () => {
const publicURL = new URL('vscode-fs://github/streetsidesoftware/public-samples/');
const publicFileURL = new URL('public-sample/', pathToFileURL(path.resolve('/')));

function pURL(path: string): URL {
return new URL(path, publicURL);
function pURL(path: string, rel = publicURL): URL {
return new URL(path, rel);
}

test.each`
Expand Down Expand Up @@ -657,11 +659,12 @@ describe('ConfigLoader with VirtualFS', () => {
});

test.each`
file | expectedConfig | expectedImportErrors
${'README.md'} | ${cfg(pURL('.cspell.json'), {})} | ${[]}
${'bug-fixes/bug345.ts'} | ${cfg(pURL('bug-fixes/cspell.json'), {})} | ${[]}
${'linked/file.txt'} | ${cfg(pURL('linked/cspell.config.js'), { __importRef: undefined })} | ${[]}
${'yaml-config/README.md'} | ${cfg(pURL('yaml-config/cspell.yaml'), { id: 'Yaml Example Config' })} | ${['cspell-imports.json']}
file | expectedConfig | expectedImportErrors
${'README.md'} | ${cfg(pURL('.cspell.json'), {})} | ${[]}
${'bug-fixes/bug345.ts'} | ${cfg(pURL('bug-fixes/cspell.json'), {})} | ${[]}
${uh('linked/file.txt', publicFileURL)} | ${cfg(uh('linked/cspell.config.js', publicFileURL), { __importRef: undefined })} | ${[]}
${'linked/file.txt'} | ${cfg(pURL('.cspell.json') /* .js not loaded */, {})} | ${[]}
${'yaml-config/README.md'} | ${cfg(pURL('yaml-config/cspell.yaml'), { id: 'Yaml Example Config' })} | ${['cspell-imports.json']}
`('Search check from $file', async ({ file, expectedConfig, expectedImportErrors }) => {
const vfs = createVirtualFS();
const redirectProvider = createRedirectProvider('test', publicURL, sURL('./'));
Expand All @@ -685,6 +688,82 @@ describe('ConfigLoader with VirtualFS', () => {
),
);
});

test.each`
file | expectedConfig | expectedImportErrors
${'README.md'} | ${cfg(u('.cspell.json', publicFileURL), {})} | ${[]}
${'bug-fixes/bug345.ts'} | ${cfg(u('bug-fixes/cspell.json', publicFileURL), {})} | ${[]}
${'linked/file.txt'} | ${cfg(u('.cspell.json', publicFileURL) /* not trusted == .js not loaded */, {})} | ${[]}
${'yaml-config/README.md'} | ${cfg(u('yaml-config/cspell.yaml', publicFileURL), { id: 'Yaml Example Config' })} | ${['cspell-imports.json']}
`('Search untrusted $file', async ({ file, expectedConfig, expectedImportErrors }) => {
const vfs = createVirtualFS();
const redirectProvider = createRedirectProvider('test', publicFileURL, sURL('./'));
vfs.registerFileSystemProvider(redirectProvider);

const fileURL = u(file, publicFileURL);
const loader = createConfigLoader(vfs.fs);
loader.setIsTrusted(false);

const searchResult = await loader.searchForConfig(fileURL);
expect(searchResult?.__importRef).toEqual(
expectedConfig.__importRef ? expect.objectContaining(expectedConfig.__importRef) : undefined,
);
// expect(searchResult).toEqual(expect.objectContaining(expectedConfig));
const errors = extractImportErrors(searchResult || {});
expect(errors).toHaveLength(expectedImportErrors.length);
expect(errors).toEqual(
expect.arrayContaining(
(expectedImportErrors as string[]).map((filename) =>
expect.objectContaining({ filename: expect.stringContaining(filename) }),
),
),
);
});

test.each`
file | expectedConfig
${'./.cspell.json'} | ${cf(u('.cspell.json', publicFileURL), {})}
${'./bug-fixes/cspell.json'} | ${cf(u('bug-fixes/cspell.json', publicFileURL), {})}
${defaultConfigFileModuleRef} | ${cf(u('packages/cspell-bundled-dicts/cspell-default.json', pathRepoRootURL), {})}
`('Search resolve untrusted $file', async ({ file, expectedConfig }) => {
const vfs = createVirtualFS();
const redirectProvider = createRedirectProvider('test', publicFileURL, sURL('./'));
vfs.registerFileSystemProvider(redirectProvider);
const loader = createConfigLoader(vfs.fs);
loader.setIsTrusted(false);

const location = await loader.resolveConfigFileLocation(file, publicFileURL);

const configFile = await loader.readConfigFile(file, publicFileURL);

expect(configFile).not.toBeInstanceOf(Error);
assert(!(configFile instanceof Error));
expect(configFile.url.href).toBe(location?.href);
expect(configFile.url.href).toBe(expectedConfig.url.href);

const config = await loader.mergeConfigFileWithImports(configFile);
expect(config).toBeDefined();
const errors = extractImportErrors(config);
expect(errors).toHaveLength(0);
});

test.each`
file
${'./linked/cspell.config.js'}
`('Error reading untrusted $file', async ({ file }) => {
const vfs = createVirtualFS();
const redirectProvider = createRedirectProvider('test', publicFileURL, sURL('./'));
vfs.registerFileSystemProvider(redirectProvider);
const loader = createConfigLoader(vfs.fs);
loader.setIsTrusted(false);

const location = await loader.resolveConfigFileLocation(file, publicFileURL);
const configFile = await loader.readConfigFile(file, publicFileURL);

expect(configFile).toBeInstanceOf(Error);
assert(configFile instanceof Error);
expect(configFile.cause).toEqual(Error(`Untrusted URL: "${location?.href}"`));
});
});

function u(url: string | URL, ref?: string | URL): URL {
Expand Down
Loading

0 comments on commit 0ed8c1e

Please sign in to comment.