From ae4dd5d1572a4f639e1a466fd982656b50f8e692 Mon Sep 17 00:00:00 2001 From: Ben Lambert Date: Tue, 20 Jan 2026 16:04:33 +0100 Subject: [PATCH] Merge commit from fork --- .changeset/breezy-moles-sin.md | 6 ++ packages/backend-plugin-api/src/paths.test.ts | 80 +++++++++++++++++++ packages/backend-plugin-api/src/paths.ts | 17 +--- packages/cli-common/src/isChildPath.ts | 49 +++++++++++- 4 files changed, 135 insertions(+), 17 deletions(-) create mode 100644 .changeset/breezy-moles-sin.md diff --git a/.changeset/breezy-moles-sin.md b/.changeset/breezy-moles-sin.md new file mode 100644 index 0000000000..c2e7c36d80 --- /dev/null +++ b/.changeset/breezy-moles-sin.md @@ -0,0 +1,6 @@ +--- +'@backstage/backend-plugin-api': patch +'@backstage/cli-common': patch +--- + +Move some of the symlink resolution to `isChildPath` diff --git a/packages/backend-plugin-api/src/paths.test.ts b/packages/backend-plugin-api/src/paths.test.ts index 6073d43a24..946250372c 100644 --- a/packages/backend-plugin-api/src/paths.test.ts +++ b/packages/backend-plugin-api/src/paths.test.ts @@ -65,5 +65,85 @@ describe('paths', () => { `${workspacePath}/template`, ); }); + + it('should throw an error if the path is a symlink pointing to a non-existent target outside base', () => { + // This tests the case where realpathSync would fail with ENOENT + // but the symlink itself exists and points outside the base directory + const nonExistentTarget = `${secondDirectory.path}/does-not-exist.txt`; + mockDir.addContent({ + [`${workspacePath}/dangling-link`]: ({ symlink }) => + symlink(nonExistentTarget), + }); + + expect(() => + resolveSafeChildPath(workspacePath, './dangling-link'), + ).toThrow( + 'Relative path is not allowed to refer to a directory outside its parent', + ); + }); + + it('should allow symlinks pointing to non-existent targets within base directory', () => { + mockDir.addContent({ + [`${workspacePath}/internal-link`]: ({ symlink }) => + symlink('./future-file.txt'), + }); + + expect(resolveSafeChildPath(workspacePath, './internal-link')).toEqual( + `${workspacePath}/internal-link`, + ); + }); + + it('should throw an error when writing through a symlink to a non-existent file outside base', () => { + // Symlink in workspace points outside, target directory exists but file doesn't + // e.g., /workspace/evil -> /etc, then write to evil/newfile.conf + // The check should catch that evil/newfile.conf resolves to /etc/newfile.conf + mockDir.addContent({ + [`${workspacePath}/escape-link`]: ({ symlink }) => + symlink(secondDirectory.path), + }); + + // This should throw because escape-link/new-file.txt would write to secondDirectory/new-file.txt + expect(() => + resolveSafeChildPath(workspacePath, './escape-link/new-file.txt'), + ).toThrow( + 'Relative path is not allowed to refer to a directory outside its parent', + ); + }); + + it('should throw an error for symlink chains pointing outside base', () => { + // link1 -> link2 -> outside + // Even if final target doesn't exist, should detect the escape + const nonExistentOutside = `${secondDirectory.path}/does-not-exist`; + mockDir.addContent({ + [`${workspacePath}/link2`]: ({ symlink }) => + symlink(nonExistentOutside), + [`${workspacePath}/link1`]: ({ symlink }) => symlink('./link2'), + }); + + expect(() => resolveSafeChildPath(workspacePath, './link1')).toThrow( + 'Relative path is not allowed to refer to a directory outside its parent', + ); + }); + + it('should throw when deeply nested non-existent path has symlink ancestor pointing outside', () => { + // Tests the recursive parent-walking behavior in resolveRealPath. + // When given a/b/c/d/file.txt where none of b/c/d exist, the code walks up + // the tree until finding 'a' (a symlink), resolves it, then rebuilds the path. + mockDir.addContent({ + [`${workspacePath}/escape`]: ({ symlink }) => + symlink(secondDirectory.path), + }); + + // escape/deeply/nested/path/file.txt requires walking up 4 levels + // before finding the symlink at 'escape' + expect(() => + resolveSafeChildPath( + workspacePath, + './escape/deeply/nested/path/file.txt', + ), + ).toThrow( + 'Relative path is not allowed to refer to a directory outside its parent', + ); + }); }); }); diff --git a/packages/backend-plugin-api/src/paths.ts b/packages/backend-plugin-api/src/paths.ts index c77f232633..7ec661cf75 100644 --- a/packages/backend-plugin-api/src/paths.ts +++ b/packages/backend-plugin-api/src/paths.ts @@ -17,7 +17,6 @@ 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< @@ -63,10 +62,9 @@ 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 resolvedBasePath = resolveRealPath(base); - const targetPath = resolvePath(resolvedBasePath, path); + const targetPath = resolvePath(base, path); - if (!isChildPath(resolvedBasePath, resolveRealPath(targetPath))) { + if (!isChildPath(base, targetPath)) { throw new NotAllowedError( 'Relative path is not allowed to refer to a directory outside its parent', ); @@ -76,16 +74,5 @@ export function resolveSafeChildPath(base: string, path: string): string { return resolvePath(base, path); } -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 }; diff --git a/packages/cli-common/src/isChildPath.ts b/packages/cli-common/src/isChildPath.ts index f66a0cfa94..ba680a6243 100644 --- a/packages/cli-common/src/isChildPath.ts +++ b/packages/cli-common/src/isChildPath.ts @@ -14,7 +14,49 @@ * limitations under the License. */ -import { relative, isAbsolute } from 'path'; +import { + relative, + isAbsolute, + resolve as resolvePath, + dirname, + basename, +} from 'path'; +import { realpathSync, lstatSync, readlinkSync } from 'fs'; + +// Resolves a path to its real location, following symlinks. +// Handles cases where the final target doesn't exist by recursively +// resolving parent directories. +function resolveRealPath(path: string): string { + try { + return realpathSync(path); + } catch (ex) { + if (ex.code !== 'ENOENT') { + throw ex; + } + } + + // Check if path itself is a dangling symlink - recursively resolve the target + // to handle symlink chains (e.g., link1 -> link2 -> /outside) + try { + if (lstatSync(path).isSymbolicLink()) { + const target = resolvePath(dirname(path), readlinkSync(path)); + return resolveRealPath(target); + } + } catch (ex) { + if (ex.code !== 'ENOENT') { + throw ex; + } + } + + // Path doesn't exist - walk up the tree until we find an existing path, + // resolve it, then rebuild the non-existent portion on top + const parent = dirname(path); + if (parent === path) { + return path; // Hit filesystem root + } + + return resolvePath(resolveRealPath(parent), basename(path)); +} /** * Checks if path is the same as or a child path of base. @@ -22,7 +64,10 @@ import { relative, isAbsolute } from 'path'; * @public */ export function isChildPath(base: string, path: string): boolean { - const relativePath = relative(base, path); + const resolvedBase = resolveRealPath(base); + const resolvedPath = resolveRealPath(path); + + const relativePath = relative(resolvedBase, resolvedPath); if (relativePath === '') { // The same directory return true;