diff --git a/.changeset/tough-pots-dream.md b/.changeset/tough-pots-dream.md new file mode 100644 index 0000000000..0bdc1500a9 --- /dev/null +++ b/.changeset/tough-pots-dream.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs-node': patch +--- + +Fixed bug causing `--legacyCopyReadmeMdToIndexMd` option to fail if docs directory is not present diff --git a/packages/techdocs-cli/cli-e2e-tests/techdocs-cli.test.ts b/packages/techdocs-cli/cli-e2e-tests/techdocs-cli.test.ts index a2720c934e..4a4819a715 100644 --- a/packages/techdocs-cli/cli-e2e-tests/techdocs-cli.test.ts +++ b/packages/techdocs-cli/cli-e2e-tests/techdocs-cli.test.ts @@ -60,6 +60,7 @@ jest.setTimeout(timeout * 2); describe('end-to-end', () => { const cwd = path.resolve(__dirname, '../src/example-docs'); + const legacyCwd = path.resolve(__dirname, '../src/legacy-docs'); const entryPoint = path.resolve(__dirname, '../bin/techdocs-cli'); afterEach(async () => { @@ -106,6 +107,19 @@ describe('end-to-end', () => { expect(proc.exit).toEqual(0); }); + it('can generate with --legacyCopyReadmeMdToIndexMd option', async () => { + const proc = await executeCommand( + entryPoint, + ['generate', '--no-docker', '--legacyCopyReadmeMdToIndexMd'], + { + cwd: legacyCwd, + timeout: timeout, + }, + ); + expect(proc.stdout).toContain('Successfully generated docs'); + expect(proc.exit).toEqual(0); + }); + it('can serve in mkdocs', async () => { const proc = await executeCommand( entryPoint, diff --git a/packages/techdocs-cli/src/legacy-docs/README.md b/packages/techdocs-cli/src/legacy-docs/README.md new file mode 100644 index 0000000000..ab47987eb4 --- /dev/null +++ b/packages/techdocs-cli/src/legacy-docs/README.md @@ -0,0 +1 @@ +Very simple file to test that `--legacyCopyReadmeMdToIndexMd` option works diff --git a/plugins/techdocs-node/src/stages/generate/helpers.test.ts b/plugins/techdocs-node/src/stages/generate/helpers.test.ts index ee4a07bf3c..f29b54735a 100644 --- a/plugins/techdocs-node/src/stages/generate/helpers.test.ts +++ b/plugins/techdocs-node/src/stages/generate/helpers.test.ts @@ -657,6 +657,119 @@ theme: ], ]); }); + + it.each(['README.md', 'readme.md', 'docs/README.md', 'docs/readme.md'])( + 'should use a symlink to %s if docs/index.md does not exist', + async fileName => { + mockDir.setContent({ + 'information.md': 'information.md content', + [fileName]: ctx => ctx.symlink(mockDir.resolve('information.md')), + }); + + await patchIndexPreBuild({ + inputDir: mockDir.path, + logger: mockLogger, + }); + + await expect( + fs.readFile(mockDir.resolve('docs/index.md'), 'utf-8'), + ).resolves.toEqual('information.md content'); + }, + ); + + it.each(['README.md', 'readme.md', 'docs/README.md', 'docs/readme.md'])( + 'should reject a symlink from %s to outside of the current directory', + async fileName => { + const anotherMockDir = createMockDirectory(); + + mockDir.setContent({ + 'information.md': 'information.md content', + [fileName]: ctx => ctx.symlink(anotherMockDir.resolve('tmp/secret')), + }); + + anotherMockDir.setContent({ + tmp: { + secret: 'password', + }, + }); + + await expect( + patchIndexPreBuild({ inputDir: mockDir.path, logger: mockLogger }), + ).rejects.toThrow( + /Source path .* is not allowed to refer to a location outside/i, + ); + }, + ); + + it.each(['README.md', 'readme.md', 'docs/README.md', 'docs/readme.md'])( + 'should write %s to a symlink docs directory if docs/index.md does not exist', + async fileName => { + mockDir.setContent({ + 'target/docs': {}, + docs: ctx => ctx.symlink('./target/docs'), + [fileName]: `${fileName} content`, + }); + + await patchIndexPreBuild({ + inputDir: mockDir.path, + logger: mockLogger, + }); + + await expect( + fs.readFile(mockDir.resolve('target/docs/index.md'), 'utf-8'), + ).resolves.toEqual(`${fileName} content`); + }, + ); + + it.each(['README.md', 'readme.md'])( + 'should reject creating docs dir if target symlink points to non-existing directory outside of the current directory', + async fileName => { + const anotherMockDir = createMockDirectory(); + + mockDir.setContent({ + docs: ctx => ctx.symlink(anotherMockDir.resolve('docs')), + 'information.md': 'information.md content', + [fileName]: `${fileName} content`, + }); + + await expect( + patchIndexPreBuild({ inputDir: mockDir.path, logger: mockLogger }), + ).rejects.toThrow( + /Target path .* is not allowed to refer to a location outside/i, + ); + + await expect(fs.exists(anotherMockDir.resolve('docs'))).resolves.toBe( + false, + ); + }, + ); + + it.each(['README.md', 'readme.md'])( + 'should reject creating docs dir if target symlink points to existing directory outside of the current directory', + async fileName => { + const anotherMockDir = createMockDirectory(); + + mockDir.setContent({ + docs: ctx => ctx.symlink(anotherMockDir.resolve('docs')), + 'information.md': 'information.md content', + [fileName]: `${fileName} content`, + }); + + anotherMockDir.setContent({ + docs: {}, + }); + + await expect( + patchIndexPreBuild({ inputDir: mockDir.path, logger: mockLogger }), + ).rejects.toThrow( + /Target path .* is not allowed to refer to a location outside/i, + ); + + await expect( + fs.exists(anotherMockDir.resolve('docs/index.md')), + ).resolves.toBe(false); + }, + ); }); describe('addBuildTimestampMetadata', () => { diff --git a/plugins/techdocs-node/src/stages/generate/helpers.ts b/plugins/techdocs-node/src/stages/generate/helpers.ts index 59e51ba9a6..0a33049f81 100644 --- a/plugins/techdocs-node/src/stages/generate/helpers.ts +++ b/plugins/techdocs-node/src/stages/generate/helpers.ts @@ -396,8 +396,18 @@ export const patchIndexPreBuild = async ({ path.join(inputDir, 'readme.md'), ]; + if (!isChildPath(inputDir, docsPath)) { + throw new NotAllowedError( + `Target path ${docsPath} is not allowed to refer to a location outside ${inputDir}`, + ); + } await fs.ensureDir(docsPath); for (const filePath of fallbacks) { + if (!isChildPath(inputDir, filePath)) { + throw new NotAllowedError( + `Source path ${filePath} is not allowed to refer to a location outside ${inputDir}`, + ); + } try { await fs.copyFile(filePath, indexMdPath); return; diff --git a/plugins/techdocs-node/src/stages/generate/techdocs.ts b/plugins/techdocs-node/src/stages/generate/techdocs.ts index e3492f2043..3281c90317 100644 --- a/plugins/techdocs-node/src/stages/generate/techdocs.ts +++ b/plugins/techdocs-node/src/stages/generate/techdocs.ts @@ -122,11 +122,6 @@ export class TechdocsGenerator implements GeneratorBase { 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 - const resolvedDocsDir = path.join(inputDir, docsDir ?? 'docs'); - await validateDocsDirectory(resolvedDocsDir, inputDir); - if (parsedLocationAnnotation) { await patchMkdocsYmlPreBuild( mkdocsYmlPath, @@ -140,6 +135,12 @@ export class TechdocsGenerator implements GeneratorBase { await patchIndexPreBuild({ inputDir, logger: childLogger, docsDir }); } + // 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 + const resolvedDocsDir = path.join(inputDir, docsDir ?? 'docs'); + + await validateDocsDirectory(resolvedDocsDir, inputDir); + // patch the list of mkdocs plugins const defaultPlugins = this.options.defaultPlugins ?? [];