Merge commit from fork

.changeset/fair-pens-wonder.md

@@ -0,0 +1,7 @@
+---
+'@backstage/plugin-scaffolder-backend': patch
+'@backstage/backend-defaults': patch
+'@backstage/plugin-scaffolder-node': patch
+---
+
+Wrap some of the action logic with `resolveSafeChildPath` and improve symlink handling when fetching remote and local files

packages/backend-defaults/src/entrypoints/urlReader/lib/tree/ReadableArrayResponse.test.ts

@@ -108,4 +108,16 @@ describe('ReadableArrayResponse', () => {
       },
     });
   });
+
+  it('should validate relative paths', async () => {
+    const arr: FromReadableArrayOptions = [
+      { data: createReadStream(path1), path: '../other/file.yaml' },
+    ];
+
+    const res = new ReadableArrayResponse(arr, targetDir.path, 'etag');
+
+    await expect(res.dir()).rejects.toThrow(
+      'Relative path is not allowed to refer to a directory outside its parent',
+    );
+  });
 });

packages/backend-defaults/src/entrypoints/urlReader/lib/tree/ReadableArrayResponse.ts

@@ -15,6 +15,7 @@
  */
 
 import {
+  resolveSafeChildPath,
   UrlReaderServiceReadTreeResponse,
   UrlReaderServiceReadTreeResponseDirOptions,
   UrlReaderServiceReadTreeResponseFile,
@@ -98,7 +99,7 @@ export class ReadableArrayResponse implements UrlReaderServiceReadTreeResponse {
 
     for (let i = 0; i < this.stream.length; i++) {
       if (!this.stream[i].path.endsWith('/')) {
-        const filePath = platformPath.join(dir, this.stream[i].path);
+        const filePath = resolveSafeChildPath(dir, this.stream[i].path);
         await fs.mkdir(dirname(filePath), { recursive: true });
         await pipeline(this.stream[i].data, fs.createWriteStream(filePath));
       }

packages/backend-defaults/src/entrypoints/urlReader/lib/tree/TarArchiveResponse.test.ts

@@ -15,9 +15,10 @@
  */
 
 import fs from 'fs-extra';
-import { resolve as resolvePath } from 'path';
+import { resolve as resolvePath, join as joinPath } from 'path';
 import { TarArchiveResponse } from './TarArchiveResponse';
 import { createMockDirectory } from '@backstage/backend-test-utils';
+import tar from 'tar';
 
 const archiveData = fs.readFileSync(
   resolvePath(__filename, '../../__fixtures__/mock-main.tar.gz'),
@@ -223,4 +224,84 @@ describe('TarArchiveResponse', () => {
     await expect(res.dir({ targetDir: sub })).rejects.toThrow('NOPE');
     await expect(fs.pathExists(sub)).resolves.toBe(true);
   });
+
+  describe('symlink handling', () => {
+    const tempDir = createMockDirectory();
+
+    it('should not extract symlinks with absolute targets', async () => {
+      const archiveSourceDir = tempDir.resolve('source');
+      await fs.ensureDir(joinPath(archiveSourceDir, 'repo'));
+      await fs.writeFile(
+        joinPath(archiveSourceDir, 'repo', 'file.txt'),
+        'file content',
+      );
+
+      // Create a symlink with an absolute path target
+      const absoluteTarget = tempDir.resolve('other.txt');
+      await fs.writeFile(absoluteTarget, 'other content');
+      await fs.symlink(
+        absoluteTarget,
+        joinPath(archiveSourceDir, 'repo', 'abs-link'),
+      );
+
+      const tarballPath = tempDir.resolve('archive.tar.gz');
+      await tar.create(
+        { gzip: true, file: tarballPath, cwd: archiveSourceDir },
+        ['repo'],
+      );
+
+      const stream = fs.createReadStream(tarballPath);
+      const res = new TarArchiveResponse(stream, '', targetDir.path, 'etag');
+
+      targetDir.addContent({ out: {} });
+      const dir = await res.dir({ targetDir: targetDir.resolve('out') });
+
+      // Regular file should be extracted
+      await expect(
+        fs.readFile(joinPath(dir, 'file.txt'), 'utf8'),
+      ).resolves.toBe('file content');
+
+      // Symlink with absolute target should not be extracted
+      await expect(fs.lstat(joinPath(dir, 'abs-link'))).rejects.toThrow(
+        'ENOENT',
+      );
+    });
+
+    it('should extract symlinks with relative targets within archive', async () => {
+      const archiveSourceDir = tempDir.resolve('source2');
+      await fs.ensureDir(joinPath(archiveSourceDir, 'repo', 'subdir'));
+      await fs.writeFile(
+        joinPath(archiveSourceDir, 'repo', 'target.txt'),
+        'target content',
+      );
+
+      // Create a relative symlink pointing to a file within the archive
+      await fs.symlink(
+        '../target.txt',
+        joinPath(archiveSourceDir, 'repo', 'subdir', 'rel-link'),
+      );
+
+      const tarballPath = tempDir.resolve('archive2.tar.gz');
+      await tar.create(
+        { gzip: true, file: tarballPath, cwd: archiveSourceDir },
+        ['repo'],
+      );
+
+      const stream = fs.createReadStream(tarballPath);
+      const res = new TarArchiveResponse(stream, '', targetDir.path, 'etag');
+
+      targetDir.addContent({ out2: {} });
+      const dir = await res.dir({ targetDir: targetDir.resolve('out2') });
+
+      // The symlink should be extracted and point to the correct file
+      const linkPath = joinPath(dir, 'subdir', 'rel-link');
+      const stats = await fs.lstat(linkPath);
+      expect(stats.isSymbolicLink()).toBe(true);
+
+      // Following the symlink should give us the target content
+      await expect(fs.readFile(linkPath, 'utf8')).resolves.toBe(
+        'target content',
+      );
+    });
+  });
 });

packages/backend-defaults/src/entrypoints/urlReader/lib/tree/TarArchiveResponse.ts

@@ -15,6 +15,7 @@
  */
 
 import {
+  isChildPath,
   UrlReaderServiceReadTreeResponse,
   UrlReaderServiceReadTreeResponseDirOptions,
   UrlReaderServiceReadTreeResponseFile,
@@ -23,7 +24,7 @@ import concatStream from 'concat-stream';
 import fs from 'fs-extra';
 import platformPath from 'path';
 import { pipeline as pipelineCb, Readable } from 'stream';
-import tar, { Parse, ParseStream, ReadEntry } from 'tar';
+import tar, { FileStat, Parse, ParseStream, ReadEntry } from 'tar';
 import { promisify } from 'util';
 import { stripFirstDirectoryFromPath } from './util';
 
@@ -182,6 +183,22 @@ export class TarArchiveResponse implements UrlReaderServiceReadTreeResponse {
             return false;
           }
 
+          // Block symlinks/hardlinks that escape the extraction directory
+          const entry = stat as FileStat & { type?: string; linkpath?: string };
+          if (
+            (entry.type === 'SymbolicLink' || entry.type === 'Link') &&
+            entry.linkpath
+          ) {
+            const strippedPath = path.split('/').slice(strip).join('/');
+            const linkDir = platformPath.dirname(
+              platformPath.join(dir, strippedPath),
+            );
+            const targetPath = platformPath.resolve(linkDir, entry.linkpath);
+            if (!isChildPath(dir, targetPath)) {
+              return false;
+            }
+          }
+
           // File path relative to the root extracted directory. Will remove the
           // top level dir name from the path since its name is hard to predetermine.
           const relativePath = this.stripFirstDirectory

packages/backend-defaults/src/entrypoints/urlReader/lib/tree/ZipArchiveResponse.test.ts

@@ -271,4 +271,47 @@ describe('ZipArchiveResponse', () => {
       'invalid relative path: ../side.txt',
     );
   });
+
+  describe('symlink handling', () => {
+    it('should extract symlink entries as regular files', async () => {
+      const externalPath = targetDir.resolve('external.txt');
+      await fs.writeFile(externalPath, 'external content');
+
+      const zipPath = targetDir.resolve('test.zip');
+      await new Promise<void>((resolve, reject) => {
+        const output = fs.createWriteStream(zipPath);
+        const archive = createArchive('zip');
+
+        output.on('close', () => resolve());
+        archive.on('error', reject);
+        archive.pipe(output);
+
+        archive.append('file content', { name: 'file.txt' });
+        archive.symlink('link', externalPath);
+
+        archive.finalize();
+      });
+
+      const stream = createReadStream(zipPath);
+      const res = new ZipArchiveResponse(stream, '', targetDir.path, 'etag');
+
+      targetDir.addContent({ out: {} });
+      const outDir = targetDir.resolve('out');
+      const dir = await res.dir({ targetDir: outDir });
+
+      await expect(
+        fs.readFile(resolvePath(dir, 'file.txt'), 'utf8'),
+      ).resolves.toBe('file content');
+
+      const entryPath = resolvePath(dir, 'link');
+      const stats = await fs.lstat(entryPath);
+
+      // yauzl extracts symlink entries as regular files
+      expect(stats.isSymbolicLink()).toBe(false);
+      expect(stats.isFile()).toBe(true);
+
+      const content = await fs.readFile(entryPath, 'utf8');
+      expect(content).toBe(externalPath);
+    });
+  });
 });

plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.test.ts

@@ -20,6 +20,7 @@ import { createDebugLogAction } from './log';
 import { join } from 'path';
 import yaml from 'yaml';
 import { createMockDirectory } from '@backstage/backend-test-utils';
+import fs from 'fs-extra';
 
 describe('debug:log', () => {
   const logger = {
@@ -139,4 +140,30 @@ describe('debug:log', () => {
       expect.stringContaining('Hello Backstage!'),
     );
   });
+
+  it('should handle symlinks to external paths', async () => {
+    const externalContent = 'external-file-content';
+    const externalPath = mockDir.resolve('external.txt');
+    await fs.writeFile(externalPath, externalContent);
+
+    const linkPath = join(mockContext.workspacePath, 'link');
+    await fs.symlink(externalPath, linkPath);
+
+    const context = {
+      ...mockContext,
+      input: {
+        listWorkspace: 'with-contents' as const,
+      },
+    };
+
+    await action.handler(context);
+
+    expect(logger.info).toHaveBeenCalledWith(expect.stringContaining('link'));
+    expect(logger.info).not.toHaveBeenCalledWith(
+      expect.stringContaining(externalContent),
+    );
+    expect(logger.info).toHaveBeenCalledWith(
+      expect.stringContaining('[skipped]'),
+    );
+  });
 });

plugins/scaffolder-backend/src/scaffolder/actions/builtin/debug/log.ts

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import { resolveSafeChildPath } from '@backstage/backend-plugin-api';
 import { readdir, stat } from 'fs-extra';
 import { join, relative } from 'path';
 import { createTemplateAction } from '@backstage/plugin-scaffolder-node';
@@ -66,8 +67,16 @@ export function createDebugLogAction() {
             .map(f => {
               const relativePath = relative(ctx.workspacePath, f);
               if (ctx.input?.listWorkspace === 'with-contents') {
-                const content = fs.readFileSync(f, 'utf-8');
-                return ` - ${relativePath}:\n\n  ${content}`;
+                try {
+                  const safePath = resolveSafeChildPath(
+                    ctx.workspacePath,
+                    relativePath,
+                  );
+                  const content = fs.readFileSync(safePath, 'utf-8');
+                  return ` - ${relativePath}:\n\n  ${content}`;
+                } catch {
+                  return ` - ${relativePath}: [skipped]`;
+                }
               }
               return `  - ${relativePath}`;
             })

plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/delete.test.ts

@@ -229,4 +229,26 @@ describe('fs:delete', () => {
       expect(fileExists).toBe(false);
     });
   });
+
+  it('should not delete files outside workspace via symlinks', async () => {
+    // Create an external file that should not be deleted
+    const externalDir = resolvePath(mockDir.path, 'external');
+    const externalFile = resolvePath(externalDir, 'config.yaml');
+    await fs.ensureDir(externalDir);
+    await fs.writeFile(externalFile, 'external content');
+
+    // Create a symlink inside workspace pointing to external directory
+    const linkPath = resolvePath(workspacePath, 'link');
+    await fs.symlink(externalDir, linkPath);
+
+    // Try to delete files through the symlink
+    await expect(() =>
+      action.handler({
+        ...mockContext,
+        input: { files: ['link/**'] },
+      }),
+    ).rejects.toThrow(
+      /Relative path is not allowed to refer to a directory outside its parent/,
+    );
+  });
 });

plugins/scaffolder-backend/src/scaffolder/actions/builtin/filesystem/delete.ts

@@ -16,7 +16,10 @@
 
 import { createTemplateAction } from '@backstage/plugin-scaffolder-node';
 import { InputError } from '@backstage/errors';
-import { resolveSafeChildPath } from '@backstage/backend-plugin-api';
+import {
+  isChildPath,
+  resolveSafeChildPath,
+} from '@backstage/backend-plugin-api';
 import fs from 'fs-extra';
 import globby from 'globby';
 import { examples } from './delete.examples';
@@ -58,10 +61,11 @@ export const createFilesystemDeleteAction = () => {
 
         for (const filepath of resolvedPaths) {
           try {
-            await fs.remove(filepath);
-            ctx.logger.info(`File ${filepath} deleted successfully`);
+            const safePath = resolveSafeChildPath(ctx.workspacePath, filepath);
+            await fs.remove(safePath);
+            ctx.logger.info(`File ${safePath} deleted successfully`);
           } catch (err) {
-            ctx.logger.error(`Failed to delete file ${filepath}:`, err);
+            ctx.logger.error(`Failed to delete file`, err);
             throw err;
           }
         }

plugins/scaffolder-node/src/actions/fetch.test.ts

@@ -86,6 +86,7 @@ describe('fetchContents helper', () => {
       expect(fs.copy).toHaveBeenCalledWith(
         resolvePath('/some/foo'),
         'somepath',
+        expect.objectContaining({ filter: expect.any(Function) }),
       );
     });
 

plugins/scaffolder-node/src/actions/fetch.ts

@@ -15,7 +15,10 @@
  */
 
 import { UrlReaderService } from '@backstage/backend-plugin-api';
-import { resolveSafeChildPath } from '@backstage/backend-plugin-api';
+import {
+  isChildPath,
+  resolveSafeChildPath,
+} from '@backstage/backend-plugin-api';
 import { InputError } from '@backstage/errors';
 import { ScmIntegrations } from '@backstage/integration';
 import fs from 'fs-extra';
@@ -50,7 +53,7 @@ export async function fetchContents(options: {
   if (!fetchUrlIsAbsolute && baseUrl?.startsWith('file://')) {
     const basePath = baseUrl.slice('file://'.length);
     const srcDir = resolveSafeChildPath(path.dirname(basePath), fetchUrl);
-    await fs.copy(srcDir, outputPath);
+    await fs.copy(srcDir, outputPath, { filter: src => isChildPath(srcDir, src) });
   } else {
     const readUrl = getReadUrl(fetchUrl, baseUrl, integrations);