techdocs-node: improve bucket path handling

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
Co-authored-by: Alex Lorenzi <alorenzi@spotify.com>

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({

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<any> = [];
       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)

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);
+    });
+  });
 });

plugins/techdocs-node/src/stages/publish/helpers.ts

@@ -230,3 +230,20 @@ export const bulkStorageOperation = async <T>(
   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;
+};