`@backstage/backend-common` vulnerable to path traversal through symlinks

@@ -16,7 +16,6 @@
 
 import { createMockDirectory } from '@backstage/backend-test-utils';
 import { resolveSafeChildPath } from './paths';
-import fs from 'fs/promises';
 
 describe('paths', () => {
   describe('resolveSafeChildPath', () => {
@@ -42,9 +41,9 @@ describe('paths', () => {
       );
     });
 
-    it('should resolve to the full path if the target is inside the directory', async () => {
+    it('should resolve to the full path if the target is inside the directory', () => {
       expect(resolveSafeChildPath(workspacePath, './README.md')).toEqual(
-        `${await fs.realpath(workspacePath)}/README.md`,
+        `${workspacePath}/README.md`,
       );
     });
 
@@ -60,5 +59,11 @@ describe('paths', () => {
         'Relative path is not allowed to refer to a directory outside its parent',
       );
     });
+
+    it('should not throw an error when a folder is referenced that doesnt already exist', () => {
+      expect(resolveSafeChildPath(workspacePath, 'template')).toEqual(
+        `${workspacePath}/template`,
+      );
+    });
   });
 });

@@ -72,7 +72,8 @@ export function resolveSafeChildPath(base: string, path: string): string {
     );
   }
 
-  return targetPath;
+  // Don't return the resolved path as the original could be a symlink
+  return resolvePath(base, path);
 }
 
 function resolveRealPath(path: string): string {

@@ -16,6 +16,7 @@
 
 import { createMockDirectory } from '@backstage/backend-test-utils';
 import { resolveSafeChildPath } from './paths';
+import fs from 'fs/promises';
 
 describe('paths', () => {
   describe('resolveSafeChildPath', () => {
@@ -41,9 +42,9 @@ describe('paths', () => {
       );
     });
 
-    it('should resolve to the full path if the target is inside the directory', () => {
+    it('should resolve to the full path if the target is inside the directory', async () => {
       expect(resolveSafeChildPath(workspacePath, './README.md')).toEqual(
-        `${workspacePath}/README.md`,
+        `${await fs.realpath(workspacePath)}/README.md`,
       );
     });
 

@@ -63,9 +63,10 @@ export function resolvePackagePath(name: string, ...paths: string[]) {
  * @returns A path that is guaranteed to point to or within the base path.
  */
 export function resolveSafeChildPath(base: string, path: string): string {
-  const targetPath = resolvePath(base, path);
+  const resolvedBasePath = resolveRealPath(base);
+  const targetPath = resolvePath(resolvedBasePath, path);
 
-  if (!isChildPath(resolveRealPath(base), resolveRealPath(targetPath))) {
+  if (!isChildPath(resolvedBasePath, resolveRealPath(targetPath))) {
     throw new NotAllowedError(
       'Relative path is not allowed to refer to a directory outside its parent',
     );

@@ -0,0 +1,63 @@
+/*
+ * Copyright 2024 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { createMockDirectory } from '@backstage/backend-test-utils';
+import { resolveSafeChildPath } from './paths';
+
+describe('paths', () => {
+  describe('resolveSafeChildPath', () => {
+    const mockDir = createMockDirectory();
+    const secondDirectory = createMockDirectory();
+
+    const workspacePath = mockDir.resolve('workspace');
+
+    beforeEach(() => {
+      mockDir.setContent({
+        [`${workspacePath}/README.md`]: '### README.md',
+      });
+      secondDirectory.setContent({
+        [`index.md`]: '### index.md',
+      });
+    });
+
+    it('should throw an error if the path is outside of the base path', () => {
+      expect(() =>
+        resolveSafeChildPath(workspacePath, secondDirectory.path),
+      ).toThrow(
+        'Relative path is not allowed to refer to a directory outside its parent',
+      );
+    });
+
+    it('should resolve to the full path if the target is inside the directory', () => {
+      expect(resolveSafeChildPath(workspacePath, './README.md')).toEqual(
+        `${workspacePath}/README.md`,
+      );
+    });
+
+    it('should throw an error if the path is a symlink to a directory outside of the base path', () => {
+      mockDir.addContent({
+        [`${workspacePath}/symlink`]: ({ symlink }) =>
+          symlink(secondDirectory.path),
+      });
+
+      expect(() =>
+        resolveSafeChildPath(workspacePath, './symlink/index.md'),
+      ).toThrow(
+        'Relative path is not allowed to refer to a directory outside its parent',
+      );
+    });
+  });
+});

@@ -17,6 +17,7 @@
 import { isChildPath } from '@backstage/cli-common';
 import { NotAllowedError } from '@backstage/errors';
 import { resolve as resolvePath } from 'path';
+import { realpathSync as realPath } from 'fs';
 
 /** @internal */
 export const packagePathMocks = new Map<
@@ -64,7 +65,7 @@ export function resolvePackagePath(name: string, ...paths: string[]) {
 export function resolveSafeChildPath(base: string, path: string): string {
   const targetPath = resolvePath(base, path);
 
-  if (!isChildPath(base, targetPath)) {
+  if (!isChildPath(resolveRealPath(base), resolveRealPath(targetPath))) {
     throw new NotAllowedError(
       'Relative path is not allowed to refer to a directory outside its parent',
     );
@@ -73,5 +74,16 @@ export function resolveSafeChildPath(base: string, path: string): string {
   return targetPath;
 }
 
+function resolveRealPath(path: string): string {
+  try {
+    return realPath(path);
+  } catch (ex) {
+    if (ex.code !== 'ENOENT') {
+      throw ex;
+    }
+  }
+
+  return path;
+}
 // Re-export isChildPath so that backend packages don't need to depend on cli-common
 export { isChildPath };