From cb7c6b1bc0d6dafe49144dc829394e470c1c4b9f Mon Sep 17 00:00:00 2001 From: Bond Yan Date: Thu, 26 Feb 2026 17:27:23 -0500 Subject: [PATCH] allowed keys implementation Signed-off-by: Bond Yan --- .changeset/crazy-ravens-reply.md | 6 ++ plugins/techdocs-backend/config.d.ts | 12 +++ .../src/stages/generate/helpers.test.ts | 77 +++++++++++++++++++ .../src/stages/generate/mkdocsPatchers.ts | 17 +++- .../src/stages/generate/techdocs.ts | 9 ++- .../src/stages/generate/types.ts | 1 + 6 files changed, 119 insertions(+), 3 deletions(-) create mode 100644 .changeset/crazy-ravens-reply.md diff --git a/.changeset/crazy-ravens-reply.md b/.changeset/crazy-ravens-reply.md new file mode 100644 index 0000000000..48bc36c464 --- /dev/null +++ b/.changeset/crazy-ravens-reply.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-techdocs-backend': patch +'@backstage/plugin-techdocs-node': patch +--- + +Added `techdocs.generator.mkdocs.dangerouslyAllowAdditionalKeys` configuration option to explicit bypass MkDocs configuration key restrictions. This enables support for additional MkDocs configuration keys beyond the default safe allowlist, such as the `hooks` key which some MkDocs plugins require. diff --git a/plugins/techdocs-backend/config.d.ts b/plugins/techdocs-backend/config.d.ts index 5d3baa940a..87369f2ccd 100644 --- a/plugins/techdocs-backend/config.d.ts +++ b/plugins/techdocs-backend/config.d.ts @@ -64,6 +64,18 @@ export interface Config { * List of mkdocs plugins which should be added as default to all mkdocs.yml files. */ defaultPlugins?: string[]; + + /** + * List of additional MkDocs configuration keys to allow beyond + * the default safe allowlist. This can introduce security vulnerabilities. + * + * WARNING: Some MkDocs configuration keys can execute arbitrary code. For example, the + * 'hooks' key allows running arbitrary Python code during documentation generation. + * Only use this in trusted environments where all mkdocs.yml files are audited. + * + * @see https://www.mkdocs.org/user-guide/configuration/#hooks + */ + dangerouslyAllowAdditionalKeys?: string[]; }; }; diff --git a/plugins/techdocs-node/src/stages/generate/helpers.test.ts b/plugins/techdocs-node/src/stages/generate/helpers.test.ts index fd428fd23a..eb3c32a464 100644 --- a/plugins/techdocs-node/src/stages/generate/helpers.test.ts +++ b/plugins/techdocs-node/src/stages/generate/helpers.test.ts @@ -910,6 +910,83 @@ another_unknown: true expect(warn).toHaveBeenCalledWith(expect.stringContaining('hooks')); }); + + it('should allow additional keys when configured via dangerouslyAllowAdditionalKeys', async () => { + const mkdocsWithHooksAndCustom = `site_name: Test +hooks: + - hook.py +custom_dir: custom +some_unknown_key: value +`; + mockDir.setContent({ + 'mkdocs_allowed.yml': mkdocsWithHooksAndCustom, + }); + + // Allow 'hooks' and 'custom_dir' but not 'some_unknown_key' + await sanitizeMkdocsYml( + mockDir.resolve('mkdocs_allowed.yml'), + mockLogger, + ['hooks', 'custom_dir'], + ); + + const updatedMkdocsYml = await fs.readFile( + mockDir.resolve('mkdocs_allowed.yml'), + ); + const parsedYml = yaml.load(updatedMkdocsYml.toString()) as Record< + string, + unknown + >; + + // 'hooks' and 'custom_dir' should be preserved + expect(parsedYml.hooks).toEqual(['hook.py']); + expect(parsedYml.custom_dir).toBe('custom'); + // 'some_unknown_key' should still be removed + expect(parsedYml.some_unknown_key).toBeUndefined(); + expect(parsedYml.site_name).toBe('Test'); + + // Should warn about the dangerous configuration AND the removed key + expect(warn).toHaveBeenCalledWith( + expect.stringContaining( + 'DANGEROUS: Allowing additional MkDocs configuration keys beyond the default safe allowlist: hooks, custom_dir', + ), + ); + expect(warn).toHaveBeenCalledWith( + expect.stringContaining( + 'Removed the following unsupported configuration keys from mkdocs.yml: some_unknown_key', + ), + ); + }); + + it('should warn about dangerous keys even when no keys are removed', async () => { + mockDir.setContent({ + 'mkdocs_with_hooks.yml': mkdocsYmlWithHooks, + }); + + await sanitizeMkdocsYml( + mockDir.resolve('mkdocs_with_hooks.yml'), + mockLogger, + ['hooks'], + ); + + const updatedMkdocsYml = await fs.readFile( + mockDir.resolve('mkdocs_with_hooks.yml'), + ); + const parsedYml = yaml.load(updatedMkdocsYml.toString()) as { + hooks?: string[]; + site_name: string; + }; + + // Hooks should be preserved + expect(parsedYml.hooks).toBeDefined(); + expect(parsedYml.site_name).toBe('Test site name'); + + // Should warn about dangerous configuration + expect(warn).toHaveBeenCalledWith( + expect.stringContaining( + 'DANGEROUS: Allowing additional MkDocs configuration keys beyond the default safe allowlist: hooks', + ), + ); + }); }); describe('validateDocsDirectory', () => { diff --git a/plugins/techdocs-node/src/stages/generate/mkdocsPatchers.ts b/plugins/techdocs-node/src/stages/generate/mkdocsPatchers.ts index cd506d3b28..d676237b01 100644 --- a/plugins/techdocs-node/src/stages/generate/mkdocsPatchers.ts +++ b/plugins/techdocs-node/src/stages/generate/mkdocsPatchers.ts @@ -198,15 +198,28 @@ export const patchMkdocsYmlWithPlugins = async ( * * @param mkdocsYmlPath - Absolute path to mkdocs.yml or equivalent of a docs site * @param logger - A logger instance + * @param additionalAllowedKeys - Optional array of additional keys to allow beyond the default allowlist */ export const sanitizeMkdocsYml = async ( mkdocsYmlPath: string, logger: LoggerService, + additionalAllowedKeys?: string[], ) => { await patchMkdocsFile(mkdocsYmlPath, logger, mkdocsYml => { + // Combine default allowed keys with additional keys + const allowedKeys = new Set(ALLOWED_MKDOCS_KEYS); + if (additionalAllowedKeys && additionalAllowedKeys.length > 0) { + logger.warn( + `DANGEROUS: Allowing additional MkDocs configuration keys beyond the default safe allowlist: ${additionalAllowedKeys.join( + ', ', + )}. This may introduce security vulnerabilities. Only use in trusted environments.`, + ); + additionalAllowedKeys.forEach(key => allowedKeys.add(key)); + } + // Identify keys that will be removed for logging const removedKeys = Object.keys(mkdocsYml).filter( - key => !ALLOWED_MKDOCS_KEYS.has(key), + key => !allowedKeys.has(key), ); if (removedKeys.length > 0) { @@ -220,7 +233,7 @@ export const sanitizeMkdocsYml = async ( // Build a new object with only allowed keys const sanitized: Record = {}; - for (const key of ALLOWED_MKDOCS_KEYS) { + for (const key of allowedKeys) { if (key in mkdocsYml) { sanitized[key] = (mkdocsYml as Record)[key]; } diff --git a/plugins/techdocs-node/src/stages/generate/techdocs.ts b/plugins/techdocs-node/src/stages/generate/techdocs.ts index ed357c5c81..8a764eb391 100644 --- a/plugins/techdocs-node/src/stages/generate/techdocs.ts +++ b/plugins/techdocs-node/src/stages/generate/techdocs.ts @@ -115,7 +115,11 @@ export class TechdocsGenerator implements GeneratorBase { const docsDir = await validateMkdocsYaml(inputDir, content); // Remove unsupported configuration keys - await sanitizeMkdocsYml(mkdocsYmlPath, childLogger); + await sanitizeMkdocsYml( + mkdocsYmlPath, + childLogger, + this.options.dangerouslyAllowAdditionalKeys, + ); // Validate that no symlinks in the docs directory point outside the input directory // This prevents path traversal attacks where malicious symlinks could leak host files @@ -257,5 +261,8 @@ export function readGeneratorConfig( defaultPlugins: config.getOptionalStringArray( 'techdocs.generator.mkdocs.defaultPlugins', ), + dangerouslyAllowAdditionalKeys: config.getOptionalStringArray( + 'techdocs.generator.mkdocs.dangerouslyAllowAdditionalKeys', + ), }; } diff --git a/plugins/techdocs-node/src/stages/generate/types.ts b/plugins/techdocs-node/src/stages/generate/types.ts index 418260b819..b716e9510b 100644 --- a/plugins/techdocs-node/src/stages/generate/types.ts +++ b/plugins/techdocs-node/src/stages/generate/types.ts @@ -45,6 +45,7 @@ export type GeneratorConfig = { omitTechdocsCoreMkdocsPlugin?: boolean; legacyCopyReadmeMdToIndexMd?: boolean; defaultPlugins?: string[]; + dangerouslyAllowAdditionalKeys?: string[]; }; /**