diff --git a/plugins/techdocs-node/src/stages/publish/awsS3.ts b/plugins/techdocs-node/src/stages/publish/awsS3.ts index 05902c999f..edbd6da114 100644 --- a/plugins/techdocs-node/src/stages/publish/awsS3.ts +++ b/plugins/techdocs-node/src/stages/publish/awsS3.ts @@ -48,6 +48,7 @@ import { getFileTreeRecursively, getHeadersForFileExtension, getStaleFiles, + isValidContentPath, lowerCaseEntityTriplet, lowerCaseEntityTripletInStoragePath, normalizeExternalStorageRootPath, @@ -398,6 +399,12 @@ export class AwsS3Publish implements PublisherBase { : lowerCaseEntityTriplet(entityTriplet); const entityRootDir = path.posix.join(this.bucketRootPath, entityDir); + if (!isValidContentPath(this.bucketRootPath, entityRootDir)) { + this.logger.error( + `Invalid content path found while fetching TechDocs metadata: ${entityRootDir}`, + ); + throw new Error(`Metadata Not Found`); + } try { const resp = await this.storageClient.send( @@ -446,6 +453,13 @@ export class AwsS3Publish implements PublisherBase { // Prepend the root path to the relative file path const filePath = path.posix.join(this.bucketRootPath, filePathNoRoot); + if (!isValidContentPath(this.bucketRootPath, filePath)) { + this.logger.error( + `Attempted to fetch TechDocs content for a file outside of the bucket root: ${filePathNoRoot}`, + ); + res.status(404).send('File Not Found'); + return; + } // Files with different extensions (CSS, HTML) need to be served with different headers const fileExtension = path.extname(filePath); @@ -486,6 +500,12 @@ export class AwsS3Publish implements PublisherBase { : lowerCaseEntityTriplet(entityTriplet); const entityRootDir = path.posix.join(this.bucketRootPath, entityDir); + if (!isValidContentPath(this.bucketRootPath, entityRootDir)) { + this.logger.error( + `Invalid content path found while checking if docs have been generated: ${entityRootDir}`, + ); + return Promise.resolve(false); + } await this.storageClient.send( new HeadObjectCommand({ diff --git a/plugins/techdocs-node/src/stages/publish/googleStorage.ts b/plugins/techdocs-node/src/stages/publish/googleStorage.ts index bc75a4c8bb..519b0f8c6b 100644 --- a/plugins/techdocs-node/src/stages/publish/googleStorage.ts +++ b/plugins/techdocs-node/src/stages/publish/googleStorage.ts @@ -29,6 +29,7 @@ import { Readable } from 'stream'; import { getFileTreeRecursively, getHeadersForFileExtension, + isValidContentPath, lowerCaseEntityTriplet, lowerCaseEntityTripletInStoragePath, bulkStorageOperation, @@ -262,6 +263,12 @@ export class GoogleGCSPublish implements PublisherBase { : lowerCaseEntityTriplet(entityTriplet); const entityRootDir = path.posix.join(this.bucketRootPath, entityDir); + if (!isValidContentPath(this.bucketRootPath, entityRootDir)) { + this.logger.error( + `Invalid content path found while fetching TechDocs metadata: ${entityRootDir}`, + ); + reject(new Error(`Metadata Not Found`)); + } const fileStreamChunks: Array = []; this.storageClient @@ -297,6 +304,13 @@ export class GoogleGCSPublish implements PublisherBase { // Prepend the root path to the relative file path const filePath = path.posix.join(this.bucketRootPath, filePathNoRoot); + if (!isValidContentPath(this.bucketRootPath, filePath)) { + this.logger.error( + `Attempted to fetch TechDocs content for a file outside of the bucket root: ${filePathNoRoot}`, + ); + res.status(404).send('File Not Found'); + return; + } // Files with different extensions (CSS, HTML) need to be served with different headers const fileExtension = path.extname(filePath); @@ -337,6 +351,12 @@ export class GoogleGCSPublish implements PublisherBase { : lowerCaseEntityTriplet(entityTriplet); const entityRootDir = path.posix.join(this.bucketRootPath, entityDir); + if (!isValidContentPath(this.bucketRootPath, entityRootDir)) { + this.logger.error( + `Invalid content path found while checking if docs have been generated: ${entityRootDir}`, + ); + resolve(false); + } this.storageClient .bucket(this.bucketName) diff --git a/plugins/techdocs-node/src/stages/publish/helpers.test.ts b/plugins/techdocs-node/src/stages/publish/helpers.test.ts index 2a74cbee61..abb1148d78 100644 --- a/plugins/techdocs-node/src/stages/publish/helpers.test.ts +++ b/plugins/techdocs-node/src/stages/publish/helpers.test.ts @@ -23,6 +23,7 @@ import { lowerCaseEntityTriplet, lowerCaseEntityTripletInStoragePath, normalizeExternalStorageRootPath, + isValidContentPath, } from './helpers'; import { createMockDirectory } from '@backstage/backend-test-utils'; @@ -306,4 +307,26 @@ describe('bulkStorageOperation', () => { expect(fn).toHaveBeenNthCalledWith(1, files[0]); expect(fn).toHaveBeenNthCalledWith(2, files[1]); }); + + describe('isValidContentPath', () => { + it('should return true when content path is the same as bucket root', () => { + expect(isValidContentPath('/s/t', '/s/t')).toBe(true); + expect(isValidContentPath('/s/t', '/s/t/c')).toBe(true); + expect(isValidContentPath('/s/t', '/s/t/c w s/f.txt')).toBe(true); + expect(isValidContentPath('/s/t w s/', '/s/t w s/f.txt')).toBe(true); + expect(isValidContentPath('/s/t w s/', '/s/t w s/c w s/f.txt')).toBe( + true, + ); + expect( + isValidContentPath('/s/t w s/', '/s/t w s/c w s/../c w s/f.txt'), + ).toBe(true); + }); + + it('should return false when content path is not a child of bucket root', () => { + expect(isValidContentPath('/s/t', '/s')).toBe(false); + expect(isValidContentPath('/s/t', '/s/t w s')).toBe(false); + expect(isValidContentPath('/s/t w s', '/s/c')).toBe(false); + expect(isValidContentPath('/s/t', '/s/t/../../c/f.txt')).toBe(false); + }); + }); }); diff --git a/plugins/techdocs-node/src/stages/publish/helpers.ts b/plugins/techdocs-node/src/stages/publish/helpers.ts index 0b9384dba3..a2e2e4feb2 100644 --- a/plugins/techdocs-node/src/stages/publish/helpers.ts +++ b/plugins/techdocs-node/src/stages/publish/helpers.ts @@ -230,3 +230,20 @@ export const bulkStorageOperation = async ( const limiter = createLimiter(concurrencyLimit); await Promise.all(args.map(arg => limiter(operation, arg))); }; + +// Checks content path is the same as or a child path of bucketRoot, specifically for posix paths. +export const isValidContentPath = ( + bucketRoot: string, + contentPath: string, +): boolean => { + const relativePath = path.posix.relative(bucketRoot, contentPath); + if (relativePath === '') { + // The same directory + return true; + } + + const outsideBase = relativePath.startsWith('..'); // not outside base + const differentDrive = path.posix.isAbsolute(relativePath); // on Windows, this means dir is on a different drive from base. + + return !outsideBase && !differentDrive; +};