From edf65d7d31e027599c2415f597d085ee84807871 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 22 Feb 2024 21:19:34 +0100 Subject: [PATCH 1/3] fix: make sure to construct the target from the resolved base path too Signed-off-by: blam --- packages/backend-common/src/paths.test.ts | 5 +++-- packages/backend-common/src/paths.ts | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/backend-common/src/paths.test.ts b/packages/backend-common/src/paths.test.ts index a9a557c8fe..94b96580e9 100644 --- a/packages/backend-common/src/paths.test.ts +++ b/packages/backend-common/src/paths.test.ts @@ -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`, ); }); diff --git a/packages/backend-common/src/paths.ts b/packages/backend-common/src/paths.ts index 0e784a60db..b5d4b331fd 100644 --- a/packages/backend-common/src/paths.ts +++ b/packages/backend-common/src/paths.ts @@ -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', ); From 742243032fc1f733bbb8726c0d4e9989106f8bc1 Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 22 Feb 2024 21:23:19 +0100 Subject: [PATCH 2/3] chore: add changeset Signed-off-by: blam --- .changeset/calm-pans-work.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/calm-pans-work.md diff --git a/.changeset/calm-pans-work.md b/.changeset/calm-pans-work.md new file mode 100644 index 0000000000..b74c02d243 --- /dev/null +++ b/.changeset/calm-pans-work.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Resolve the `basePath` before constructing the target path From 1ad2b1b61ebb430051f7d804b0cc7ebfe7922b6f Mon Sep 17 00:00:00 2001 From: blam Date: Thu, 22 Feb 2024 22:16:25 +0100 Subject: [PATCH 3/3] chore: return the original input path, and add a test Signed-off-by: blam --- packages/backend-common/src/paths.test.ts | 11 ++++++++--- packages/backend-common/src/paths.ts | 3 ++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/backend-common/src/paths.test.ts b/packages/backend-common/src/paths.test.ts index 94b96580e9..6073d43a24 100644 --- a/packages/backend-common/src/paths.test.ts +++ b/packages/backend-common/src/paths.test.ts @@ -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`, + ); + }); }); }); diff --git a/packages/backend-common/src/paths.ts b/packages/backend-common/src/paths.ts index b5d4b331fd..c77f232633 100644 --- a/packages/backend-common/src/paths.ts +++ b/packages/backend-common/src/paths.ts @@ -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 {