From 5296b00d4a8c7b7079ccb11b27eb1376e40b8125 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Wed, 8 Jun 2022 14:37:40 +0200 Subject: [PATCH] chore: fix local entity path join Signed-off-by: Eric Peterson --- .../src/stages/publish/local.test.ts | 137 ++++++++++++------ .../techdocs-node/src/stages/publish/local.ts | 111 +++++++++----- 2 files changed, 167 insertions(+), 81 deletions(-) diff --git a/plugins/techdocs-node/src/stages/publish/local.test.ts b/plugins/techdocs-node/src/stages/publish/local.test.ts index 761de03c5a..daf943dce4 100644 --- a/plugins/techdocs-node/src/stages/publish/local.test.ts +++ b/plugins/techdocs-node/src/stages/publish/local.test.ts @@ -54,62 +54,107 @@ const resolvedDir = resolvePackagePath( ); describe('local publisher', () => { - it('should publish generated documentation dir', async () => { - mockFs({ - [tmpDir]: { - 'index.html': '', - }, + describe('publish', () => { + beforeEach(() => { + mockFs({ + [tmpDir]: { + 'index.html': '', + }, + }); }); - const mockConfig = new ConfigReader({}); - - const publisher = LocalPublish.fromConfig( - mockConfig, - logger, - testDiscovery, - ); - const mockEntity = createMockEntity(); - const lowerMockEntity = createMockEntity(undefined, true); - - await publisher.publish({ entity: mockEntity, directory: tmpDir }); - - expect(await publisher.hasDocsBeenGenerated(mockEntity)).toBe(true); - - // Lower/upper should be treated the same. - expect(await publisher.hasDocsBeenGenerated(lowerMockEntity)).toBe(true); - - mockFs.restore(); - }); - - it('should respect legacy casing', async () => { - mockFs({ - [tmpDir]: { - 'index.html': '', - }, + afterEach(() => { + mockFs.restore(); }); - const mockConfig = new ConfigReader({ - techdocs: { - legacyUseCaseSensitiveTripletPaths: true, - }, + it('should publish generated documentation dir', async () => { + const mockConfig = new ConfigReader({}); + + const publisher = LocalPublish.fromConfig( + mockConfig, + logger, + testDiscovery, + ); + const mockEntity = createMockEntity(); + const lowerMockEntity = createMockEntity(undefined, true); + + await publisher.publish({ entity: mockEntity, directory: tmpDir }); + + expect(await publisher.hasDocsBeenGenerated(mockEntity)).toBe(true); + + // Lower/upper should be treated the same. + expect(await publisher.hasDocsBeenGenerated(lowerMockEntity)).toBe(true); + + mockFs.restore(); }); - const publisher = LocalPublish.fromConfig( - mockConfig, - logger, - testDiscovery, - ); - const mockEntity = createMockEntity(); - const lowerMockEntity = createMockEntity(undefined, true); + it('should respect legacy casing', async () => { + const mockConfig = new ConfigReader({ + techdocs: { + legacyUseCaseSensitiveTripletPaths: true, + }, + }); - await publisher.publish({ entity: mockEntity, directory: tmpDir }); + const publisher = LocalPublish.fromConfig( + mockConfig, + logger, + testDiscovery, + ); + const mockEntity = createMockEntity(); + const lowerMockEntity = createMockEntity(undefined, true); - expect(await publisher.hasDocsBeenGenerated(mockEntity)).toBe(true); + await publisher.publish({ entity: mockEntity, directory: tmpDir }); - // Lower/upper should be treated differently. - expect(await publisher.hasDocsBeenGenerated(lowerMockEntity)).toBe(false); + expect(await publisher.hasDocsBeenGenerated(mockEntity)).toBe(true); - mockFs.restore(); + // Lower/upper should be treated differently. + expect(await publisher.hasDocsBeenGenerated(lowerMockEntity)).toBe(false); + + mockFs.restore(); + }); + + it('should throw with unsafe triplet', async () => { + const mockConfig = new ConfigReader({}); + const publisher = LocalPublish.fromConfig( + mockConfig, + logger, + testDiscovery, + ); + const mockEntity = { + ...createMockEntity(), + ...{ + kind: '..', + metadata: { name: '..', namespace: '..' }, + }, + }; + + await expect(() => + publisher.publish({ entity: mockEntity, directory: tmpDir }), + ).rejects.toThrowError('Unable to publish TechDocs site'); + }); + + it('should throw with unsafe name', async () => { + const mockConfig = new ConfigReader({}); + const publisher = LocalPublish.fromConfig( + mockConfig, + logger, + testDiscovery, + ); + const mockEntity = { + ...createMockEntity(), + ...{ + kind: 'component', + metadata: { + name: '../component/other-component', + namespace: 'default', + }, + }, + }; + + await expect(() => + publisher.publish({ entity: mockEntity, directory: tmpDir }), + ).rejects.toThrowError('Unable to publish TechDocs site'); + }); }); describe('docsRouter', () => { diff --git a/plugins/techdocs-node/src/stages/publish/local.ts b/plugins/techdocs-node/src/stages/publish/local.ts index 4539d389df..c0de46eb22 100644 --- a/plugins/techdocs-node/src/stages/publish/local.ts +++ b/plugins/techdocs-node/src/stages/publish/local.ts @@ -16,8 +16,13 @@ import { PluginEndpointDiscovery, resolvePackagePath, + resolveSafeChildPath, } from '@backstage/backend-common'; -import { Entity, CompoundEntityRef } from '@backstage/catalog-model'; +import { + Entity, + CompoundEntityRef, + stringifyEntityRef, +} from '@backstage/catalog-model'; import { Config } from '@backstage/config'; import express from 'express'; import fs from 'fs-extra'; @@ -37,7 +42,7 @@ import { getHeadersForFileExtension, lowerCaseEntityTripletInStoragePath, } from './helpers'; -import { assertError } from '@backstage/errors'; +import { ForwardedError } from '@backstage/errors'; // TODO: Use a more persistent storage than node_modules or /tmp directory. // Make it configurable with techdocs.publisher.local.publishDirectory @@ -103,12 +108,22 @@ export class LocalPublish implements PublisherBase { directory, }: PublishRequest): Promise { const entityNamespace = entity.metadata.namespace ?? 'default'; + let publishDir: string; - const publishDir = this.staticEntityPathJoin( - entityNamespace, - entity.kind, - entity.metadata.name, - ); + try { + publishDir = this.staticEntityPathJoin( + entityNamespace, + entity.kind, + entity.metadata.name, + ); + } catch (error) { + throw new ForwardedError( + `Unable to publish TechDocs site for entity: ${stringifyEntityRef( + entity, + )}`, + error, + ); + } if (!fs.existsSync(publishDir)) { this.logger.info(`Could not find ${publishDir}, creating the directory.`); @@ -144,21 +159,31 @@ export class LocalPublish implements PublisherBase { async fetchTechDocsMetadata( entityName: CompoundEntityRef, ): Promise { - const metadataPath = this.staticEntityPathJoin( - entityName.namespace, - entityName.kind, - entityName.name, - 'techdocs_metadata.json', - ); + let metadataPath: string; + + try { + metadataPath = this.staticEntityPathJoin( + entityName.namespace, + entityName.kind, + entityName.name, + 'techdocs_metadata.json', + ); + } catch (err) { + throw new ForwardedError( + `Unexpected entity when fetching metadata: ${stringifyEntityRef( + entityName, + )}`, + err, + ); + } try { return await fs.readJson(metadataPath); } catch (err) { - assertError(err); - this.logger.error( + throw new ForwardedError( `Unable to read techdocs_metadata.json at ${metadataPath}. Error: ${err}`, + err, ); - throw new Error(err.message); } } @@ -217,18 +242,26 @@ export class LocalPublish implements PublisherBase { async hasDocsBeenGenerated(entity: Entity): Promise { const namespace = entity.metadata.namespace ?? 'default'; - const indexHtmlPath = this.staticEntityPathJoin( - namespace, - entity.kind, - entity.metadata.name, - 'index.html', - ); - // Check if the file exists try { + const indexHtmlPath = this.staticEntityPathJoin( + namespace, + entity.kind, + entity.metadata.name, + 'index.html', + ); + await fs.access(indexHtmlPath, fs.constants.F_OK); + return true; } catch (err) { + if (err.name === 'NotAllowedError') { + this.logger.error( + `Unexpected entity when checking if generated: ${stringifyEntityRef( + entity, + )}`, + ); + } return false; } } @@ -278,17 +311,25 @@ export class LocalPublish implements PublisherBase { * Utility wrapper around path.join(), used to control legacy case logic. */ protected staticEntityPathJoin(...allParts: string[]): string { - if (this.legacyPathCasing) { - const [namespace, kind, name, ...parts] = allParts; - return path.join(staticDocsDir, namespace, kind, name, ...parts); - } - const [namespace, kind, name, ...parts] = allParts; - return path.join( - staticDocsDir, - namespace.toLowerCase(), - kind.toLowerCase(), - name.toLowerCase(), - ...parts, - ); + let staticEntityPath = staticDocsDir; + + allParts + .map(part => part.split(path.sep)) + .flat() + .forEach((part, index) => { + // Respect legacy path casing when operating on namespace, kind, or name. + if (index < 3) { + staticEntityPath = resolveSafeChildPath( + staticEntityPath, + this.legacyPathCasing ? part : part.toLowerCase(), + ); + return; + } + + // Otherwise, respect the provided case. + staticEntityPath = resolveSafeChildPath(staticEntityPath, part); + }); + + return staticEntityPath; } }