Merge pull request #7584 from SDA-SE/feat/fetch-resolvesafe

Use `resolveSafeChildPath` in the `fetchContents` function

.changeset/lucky-countries-smile.md

@@ -0,0 +1,5 @@
+---
+'@backstage/plugin-scaffolder-backend': patch
+---
+
+Use `resolveSafeChildPath` in the `fetchContents` function to forbid reading files outside the base directory when a template is registered from a `file:` location.

plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/helpers.test.ts

@@ -67,7 +67,19 @@ describe('fetchContent helper', () => {
         fetchUrl: '/etc/passwd',
       }),
     ).rejects.toThrow(
-      'Fetch URL may not be absolute for file locations, /etc/passwd',
+      'Relative path is not allowed to refer to a directory outside its parent',
+    );
+  });
+
+  it('should reject relative file locations that exit the baseUrl', async () => {
+    await expect(
+      fetchContents({
+        ...options,
+        baseUrl: 'file:///some/path',
+        fetchUrl: '../test',
+      }),
+    ).rejects.toThrow(
+      'Relative path is not allowed to refer to a directory outside its parent',
     );
   });
 

plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/helpers.ts

@@ -14,12 +14,12 @@
  * limitations under the License.
  */
 
-import fs from 'fs-extra';
-import { resolve as resolvePath, isAbsolute } from 'path';
-import { UrlReader } from '@backstage/backend-common';
+import { resolveSafeChildPath, UrlReader } from '@backstage/backend-common';
+import { JsonValue } from '@backstage/config';
 import { InputError } from '@backstage/errors';
 import { ScmIntegrations } from '@backstage/integration';
-import { JsonValue } from '@backstage/config';
+import fs from 'fs-extra';
+import * as path from 'path';
 
 export async function fetchContents({
   reader,
@@ -52,12 +52,7 @@ export async function fetchContents({
   // We handle both file locations and url ones
   if (!fetchUrlIsAbsolute && baseUrl?.startsWith('file://')) {
     const basePath = baseUrl.slice('file://'.length);
-    if (isAbsolute(fetchUrl)) {
-      throw new InputError(
-        `Fetch URL may not be absolute for file locations, ${fetchUrl}`,
-      );
-    }
-    const srcDir = resolvePath(basePath, '..', fetchUrl);
+    const srcDir = resolveSafeChildPath(path.dirname(basePath), fetchUrl);
     await fs.copy(srcDir, outputPath);
   } else {
     let readUrl;