Merge pull request #23180 from backstage/blam/resolve-safe-child

Improve check in `resolveSafeChildPath`

.changeset/lazy-needles-lick.md

@@ -0,0 +1,5 @@
+---
+'@backstage/backend-common': patch
+---
+
+Fix issue where `resolveSafeChildPath` path would incorrectly resolve when operating on a symlink

packages/backend-common/src/paths.test.ts

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

packages/backend-common/src/paths.ts

@@ -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 };