From 4fb8469d37993f66794a7dea11e812e51a2a83ff Mon Sep 17 00:00:00 2001 From: Justin Bryant Date: Fri, 17 Apr 2026 17:00:37 -0400 Subject: [PATCH 01/11] fix(SecureTemplater): return dispose function to clean up secure templater isolate vm & context Signed-off-by: Justin Bryant --- .../lib/templating/SecureTemplater.test.ts | 57 ++++++++++++------- .../src/lib/templating/SecureTemplater.ts | 13 ++++- .../builtin/fetch/templateActionHandler.ts | 26 +++++---- .../fetch/templateFileActionHandler.ts | 26 +++++---- .../tasks/NunjucksWorkflowRunner.ts | 26 +++++---- 5 files changed, 90 insertions(+), 58 deletions(-) diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts index 53bf67fd87..2957848279 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts @@ -18,7 +18,7 @@ import { SecureTemplater } from './SecureTemplater'; describe('SecureTemplater', () => { it('should render some templates', async () => { - const render = await SecureTemplater.loadRenderer(); + const { render, dispose } = await SecureTemplater.loadRenderer(); expect(render('${{ test }}', { test: 'my-value' })).toBe('my-value'); expect(render('${{ test | dump }}', { test: 'my-value' })).toBe( @@ -36,13 +36,16 @@ describe('SecureTemplater', () => { test: 'my-value', }), ).toThrow(/expected name as lookup value, got ./); + dispose(); }); it('should make cookiecutter compatibility available when requested', async () => { - const renderWith = await SecureTemplater.loadRenderer({ - cookiecutterCompat: true, - }); - const renderWithout = await SecureTemplater.loadRenderer(); + const { render: renderWith, dispose: disposeWith } = + await SecureTemplater.loadRenderer({ + cookiecutterCompat: true, + }); + const { render: renderWithout, dispose: disposeWithout } = + await SecureTemplater.loadRenderer(); // Same two tests repeated to make sure switching back and forth works expect(renderWith('{{ 1 | jsonify }}', {})).toBe('1'); @@ -61,6 +64,8 @@ describe('SecureTemplater', () => { /Error: filter not found: jsonify/, ); expect(renderWith('{{ 1 | jsonify }}', {})).toBe('1'); + disposeWith(); + disposeWithout(); }); it('should make parseRepoUrl available when requested', async () => { @@ -72,10 +77,12 @@ describe('SecureTemplater', () => { const projectSlug = jest.fn(() => 'my-owner/my-repo'); - const renderWith = await SecureTemplater.loadRenderer({ - templateFilters: { parseRepoUrl, projectSlug }, - }); - const renderWithout = await SecureTemplater.loadRenderer(); + const { render: renderWith, dispose: disposeWith } = + await SecureTemplater.loadRenderer({ + templateFilters: { parseRepoUrl, projectSlug }, + }); + const { render: renderWithout, dispose: disposeWithout } = + await SecureTemplater.loadRenderer(); const ctx = { repoUrl: 'https://my-host.com/my-owner/my-repo', @@ -101,6 +108,8 @@ describe('SecureTemplater', () => { expect(parseRepoUrl.mock.calls).toEqual([ ['https://my-host.com/my-owner/my-repo'], ]); + disposeWith(); + disposeWithout(); }); it('should make additional filters available when requested', async () => { @@ -108,10 +117,12 @@ describe('SecureTemplater', () => { const mockFilter2 = jest.fn((var1, var2) => `${var1} ${var2}`); const mockFilter3 = jest.fn((var1, var2) => ({ var1, var2 })); const mockFilter4 = jest.fn(() => undefined); - const renderWith = await SecureTemplater.loadRenderer({ - templateFilters: { mockFilter1, mockFilter2, mockFilter3, mockFilter4 }, - }); - const renderWithout = await SecureTemplater.loadRenderer(); + const { render: renderWith, dispose: disposeWith } = + await SecureTemplater.loadRenderer({ + templateFilters: { mockFilter1, mockFilter2, mockFilter3, mockFilter4 }, + }); + const { render: renderWithout, dispose: disposeWithout } = + await SecureTemplater.loadRenderer(); const ctx = { inputValue: 'the input value' }; @@ -149,16 +160,20 @@ describe('SecureTemplater', () => { expect(mockFilter3.mock.calls).toEqual([ ['the input value', 'another extra arg'], ]); + disposeWith(); + disposeWithout(); }); it('should make additional globals available when requested', async () => { const mockGlobal1 = jest.fn(() => 'awesome global function'); const mockGlobal2 = 'foo'; const mockGlobal3 = 123456; const mockGlobal4 = jest.fn(() => undefined); - const renderWith = await SecureTemplater.loadRenderer({ - templateGlobals: { mockGlobal1, mockGlobal2, mockGlobal3, mockGlobal4 }, - }); - const renderWithout = await SecureTemplater.loadRenderer(); + const { render: renderWith, dispose: disposeWith } = + await SecureTemplater.loadRenderer({ + templateGlobals: { mockGlobal1, mockGlobal2, mockGlobal3, mockGlobal4 }, + }); + const { render: renderWithout, dispose: disposeWithout } = + await SecureTemplater.loadRenderer(); const ctx = {}; @@ -172,10 +187,12 @@ describe('SecureTemplater', () => { expect(() => renderWithout('${{ mockGlobal1() }}', ctx)).toThrow( /Error: Unable to call `mockGlobal1`/, ); + disposeWith(); + disposeWithout(); }); it('should not allow helpers to be rewritten', async () => { - const render = await SecureTemplater.loadRenderer({ + const { render, dispose } = await SecureTemplater.loadRenderer({ templateFilters: { parseRepoUrl: () => ({ repo: 'my-repo', @@ -202,10 +219,11 @@ describe('SecureTemplater', () => { host: 'my-host.com', }), ); + dispose(); }); it('allows pollution during a single template execution', async () => { - const render = await SecureTemplater.loadRenderer(); + const { render, dispose } = await SecureTemplater.loadRenderer(); const ctx = { x: 'foo', @@ -218,5 +236,6 @@ describe('SecureTemplater', () => { ), ).toBe(''); expect(() => render('${{ x }}', ctx)).toThrow(); + dispose(); }); }); diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts index 709cb1308d..71a7a14e60 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts @@ -14,14 +14,14 @@ * limitations under the License. */ -import { Isolate } from 'isolated-vm'; import { resolvePackagePath } from '@backstage/backend-plugin-api'; import { TemplateFilter, TemplateGlobal, } from '@backstage/plugin-scaffolder-node'; -import fs from 'fs-extra'; import { JsonValue } from '@backstage/types'; +import fs from 'fs-extra'; +import { Isolate } from 'isolated-vm'; import { getMajorNodeVersion, isNoNodeSnapshotOptionProvided } from './helpers'; // language=JavaScript @@ -215,6 +215,13 @@ export class SecureTemplater { return context.evalSync(`render(templateStr, templateValues)`); }; - return render; + + return { + render, + dispose: () => { + context.release(); + isolate.dispose(); + }, + }; } } diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateActionHandler.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateActionHandler.ts index e436616651..9ee580e324 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateActionHandler.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateActionHandler.ts @@ -27,10 +27,10 @@ import { import fs from 'fs-extra'; import globby from 'globby'; import { isBinaryFile } from 'isbinaryfile'; -import { createDefaultFilters } from '../../../../lib/templating/filters/createDefaultFilters'; -import { convertFiltersToRecord } from '../../../../util/templating'; -import { SecureTemplater } from '../../../../lib/templating/SecureTemplater'; import { extname } from 'path'; +import { createDefaultFilters } from '../../../../lib/templating/filters/createDefaultFilters'; +import { SecureTemplater } from '../../../../lib/templating/SecureTemplater'; +import { convertFiltersToRecord } from '../../../../util/templating'; export type TemplateActionInput = { targetPath?: string; @@ -107,15 +107,16 @@ export async function createTemplateActionHandler< ctx.input.values, ); - const renderTemplate = await SecureTemplater.loadRenderer({ - cookiecutterCompat: ctx.input.cookiecutterCompat, - templateFilters, - templateGlobals, - nunjucksConfigs: { - trimBlocks: ctx.input.trimBlocks, - lstripBlocks: ctx.input.lstripBlocks, - }, - }); + const { render: renderTemplate, dispose } = + await SecureTemplater.loadRenderer({ + cookiecutterCompat: ctx.input.cookiecutterCompat, + templateFilters, + templateGlobals, + nunjucksConfigs: { + trimBlocks: ctx.input.trimBlocks, + lstripBlocks: ctx.input.lstripBlocks, + }, + }); for (const location of allEntriesInTemplate) { let renderContents: boolean; @@ -186,6 +187,7 @@ export async function createTemplateActionHandler< } } } + dispose(); ctx.logger.info(`Template result written to ${outputDir}`); } diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts index d82a1dd5a2..8e5e8d14e9 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import { resolveSafeChildPath } from '@backstage/backend-plugin-api'; import { ScmIntegrations } from '@backstage/integration'; import { ActionContext, @@ -20,11 +21,10 @@ import { TemplateGlobal, } from '@backstage/plugin-scaffolder-node'; import fs from 'fs-extra'; -import { createDefaultFilters } from '../../../../lib/templating/filters/createDefaultFilters'; -import { convertFiltersToRecord } from '../../../../util/templating'; -import { resolveSafeChildPath } from '@backstage/backend-plugin-api'; import path from 'path'; +import { createDefaultFilters } from '../../../../lib/templating/filters/createDefaultFilters'; import { SecureTemplater } from '../../../../lib/templating/SecureTemplater'; +import { convertFiltersToRecord } from '../../../../util/templating'; export type TemplateFileActionInput = { targetPath: string; @@ -80,20 +80,22 @@ export async function createTemplateFileActionHandler< ctx.input.values, ); - const renderTemplate = await SecureTemplater.loadRenderer({ - cookiecutterCompat, - templateFilters, - templateGlobals, - nunjucksConfigs: { - trimBlocks: ctx.input.trimBlocks, - lstripBlocks: ctx.input.lstripBlocks, - }, - }); + const { render: renderTemplate, dispose } = + await SecureTemplater.loadRenderer({ + cookiecutterCompat, + templateFilters, + templateGlobals, + nunjucksConfigs: { + trimBlocks: ctx.input.trimBlocks, + lstripBlocks: ctx.input.lstripBlocks, + }, + }); const contents = await fs.readFile(filePath, 'utf-8'); const result = renderTemplate(contents, context); await fs.ensureDir(path.dirname(outputPath)); await fs.outputFile(outputPath, result); + dispose(); ctx.logger.info(`Template file has been written to ${outputPath}`); } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index e2e36438a1..a9a3a31121 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -55,15 +55,15 @@ import { TemplateFilter, TemplateGlobal, } from '@backstage/plugin-scaffolder-node'; -import { createDefaultFilters } from '../../lib/templating/filters/createDefaultFilters'; -import { scaffolderActionRules } from '../../service/rules'; -import { createCounterMetric, createHistogramMetric } from '../../util/metrics'; -import { BackstageLoggerTransport, WinstonLogger } from './logger'; -import { convertFiltersToRecord } from '../../util/templating'; import { CheckpointContext, CheckpointState, } from '@backstage/plugin-scaffolder-node/alpha'; +import { createDefaultFilters } from '../../lib/templating/filters/createDefaultFilters'; +import { scaffolderActionRules } from '../../service/rules'; +import { createCounterMetric, createHistogramMetric } from '../../util/metrics'; +import { convertFiltersToRecord } from '../../util/templating'; +import { BackstageLoggerTransport, WinstonLogger } from './logger'; type NunjucksWorkflowRunnerOptions = { workingDirectory: string; @@ -485,13 +485,14 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { const { additionalTemplateFilters, additionalTemplateGlobals } = this.options; - const renderTemplate = await SecureTemplater.loadRenderer({ - templateFilters: { - ...this.defaultTemplateFilters, - ...additionalTemplateFilters, - }, - templateGlobals: additionalTemplateGlobals, - }); + const { render: renderTemplate, dispose } = + await SecureTemplater.loadRenderer({ + templateFilters: { + ...this.defaultTemplateFilters, + ...additionalTemplateFilters, + }, + templateGlobals: additionalTemplateGlobals, + }); try { await task.rehydrateWorkspace?.({ taskId, targetPath: workspacePath }); @@ -533,6 +534,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { const output = this.render(task.spec.output, context, renderTemplate); await taskTrack.markSuccessful(); await task.cleanWorkspace?.(); + dispose(); return { output }; } finally { From c78b3b68f6f90ddef4e13f8ba818f9bd1d6543b2 Mon Sep 17 00:00:00 2001 From: Justin Bryant Date: Fri, 17 Apr 2026 17:10:28 -0400 Subject: [PATCH 02/11] chore: add changeset Signed-off-by: Justin Bryant --- .changeset/bitter-files-flash.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/bitter-files-flash.md diff --git a/.changeset/bitter-files-flash.md b/.changeset/bitter-files-flash.md new file mode 100644 index 0000000000..1e0985c39c --- /dev/null +++ b/.changeset/bitter-files-flash.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': minor +--- + +Add explicit memory management to SecureTemplater usage From b7da1160fb33f6d5a3d7ccee27091c864bcb089c Mon Sep 17 00:00:00 2001 From: Justin Bryant Date: Fri, 17 Apr 2026 17:15:50 -0400 Subject: [PATCH 03/11] Update plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Justin Bryant --- .../src/scaffolder/tasks/NunjucksWorkflowRunner.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index a9a3a31121..51e41c7add 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -534,10 +534,14 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { const output = this.render(task.spec.output, context, renderTemplate); await taskTrack.markSuccessful(); await task.cleanWorkspace?.(); - dispose(); return { output }; } finally { + try { + dispose(); + } catch { + // Ignore disposal errors so they don't mask the original failure. + } if (workspacePath) { await fs.remove(workspacePath); } From f063176143a2999b3c4dbdce443ef368d081f582 Mon Sep 17 00:00:00 2001 From: Justin Bryant Date: Fri, 17 Apr 2026 17:17:12 -0400 Subject: [PATCH 04/11] Update plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Justin Bryant --- .../builtin/fetch/templateFileActionHandler.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts index 8e5e8d14e9..f5663ff220 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts @@ -91,11 +91,14 @@ export async function createTemplateFileActionHandler< }, }); - const contents = await fs.readFile(filePath, 'utf-8'); - const result = renderTemplate(contents, context); - await fs.ensureDir(path.dirname(outputPath)); - await fs.outputFile(outputPath, result); + try { + const contents = await fs.readFile(filePath, 'utf-8'); + const result = renderTemplate(contents, context); + await fs.ensureDir(path.dirname(outputPath)); + await fs.outputFile(outputPath, result); - dispose(); - ctx.logger.info(`Template file has been written to ${outputPath}`); + ctx.logger.info(`Template file has been written to ${outputPath}`); + } finally { + dispose(); + } } From 6a73fff696b013b18be8570c56944148bb624591 Mon Sep 17 00:00:00 2001 From: Justin Bryant Date: Fri, 17 Apr 2026 17:25:50 -0400 Subject: [PATCH 05/11] fix(SecureTemplater): implement PR suggestions Signed-off-by: Justin Bryant --- .changeset/bitter-files-flash.md | 2 +- .../src/lib/templating/SecureTemplater.ts | 12 +- .../builtin/fetch/templateActionHandler.ts | 122 +++++++++--------- 3 files changed, 73 insertions(+), 63 deletions(-) diff --git a/.changeset/bitter-files-flash.md b/.changeset/bitter-files-flash.md index 1e0985c39c..9b7ab014a5 100644 --- a/.changeset/bitter-files-flash.md +++ b/.changeset/bitter-files-flash.md @@ -1,5 +1,5 @@ --- -'@backstage/plugin-scaffolder-backend': minor +'@backstage/plugin-scaffolder-backend': major --- Add explicit memory management to SecureTemplater usage diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts index 71a7a14e60..e230dba184 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts @@ -202,8 +202,10 @@ export class SecureTemplater { await nunjucksScript.run(context); const render: SecureTemplateRenderer = (template, values) => { - if (!context) { - throw new Error('SecureTemplater has not been initialized'); + if (!context || isolate.isDisposed) { + throw new Error( + 'SecureTemplater has not been initialized or has been disposed', + ); } contextGlobal.setSync('templateStr', String(template)); @@ -219,8 +221,10 @@ export class SecureTemplater { return { render, dispose: () => { - context.release(); - isolate.dispose(); + if (context && !isolate.isDisposed) { + context.release(); + isolate.dispose(); + } }, }; } diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateActionHandler.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateActionHandler.ts index 9ee580e324..a8a4dbafd7 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateActionHandler.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateActionHandler.ts @@ -117,77 +117,83 @@ export async function createTemplateActionHandler< lstripBlocks: ctx.input.lstripBlocks, }, }); + try { + for (const location of allEntriesInTemplate) { + let renderContents: boolean; - for (const location of allEntriesInTemplate) { - let renderContents: boolean; - - let localOutputPath = location; - if (extension) { - renderContents = extname(localOutputPath) === extension; - if (renderContents) { - localOutputPath = localOutputPath.slice(0, -extension.length); - } - // extension is mutual exclusive with copyWithoutRender/copyWithoutTemplating, - // therefore the output path is always rendered. - localOutputPath = renderTemplate(localOutputPath, context); - } else { - renderContents = !nonTemplatedEntries.has(location); - // The logic here is a bit tangled because it depends on two variables. - // If renderFilename is true, which means copyWithoutTemplating is used, - // then the path is always rendered. - // If renderFilename is false, which means copyWithoutRender is used, - // then matched file/directory won't be processed, same as before. - if (renderFilename) { + let localOutputPath = location; + if (extension) { + renderContents = extname(localOutputPath) === extension; + if (renderContents) { + localOutputPath = localOutputPath.slice(0, -extension.length); + } + // extension is mutual exclusive with copyWithoutRender/copyWithoutTemplating, + // therefore the output path is always rendered. localOutputPath = renderTemplate(localOutputPath, context); } else { - localOutputPath = renderContents - ? renderTemplate(localOutputPath, context) - : localOutputPath; + renderContents = !nonTemplatedEntries.has(location); + // The logic here is a bit tangled because it depends on two variables. + // If renderFilename is true, which means copyWithoutTemplating is used, + // then the path is always rendered. + // If renderFilename is false, which means copyWithoutRender is used, + // then matched file/directory won't be processed, same as before. + if (renderFilename) { + localOutputPath = renderTemplate(localOutputPath, context); + } else { + localOutputPath = renderContents + ? renderTemplate(localOutputPath, context) + : localOutputPath; + } } - } - if (containsSkippedContent(localOutputPath)) { - continue; - } + if (containsSkippedContent(localOutputPath)) { + continue; + } - const outputPath = resolveSafeChildPath(outputDir, localOutputPath); - if (fs.existsSync(outputPath) && !ctx.input.replace) { - continue; - } + const outputPath = resolveSafeChildPath(outputDir, localOutputPath); + if (fs.existsSync(outputPath) && !ctx.input.replace) { + continue; + } - if (!renderContents && !extension) { - ctx.logger.info(`Copying file/directory ${location} without processing.`); - } - - if (location.endsWith('/')) { - ctx.logger.info(`Writing directory ${location} to template output path.`); - await fs.ensureDir(outputPath); - } else { - const inputFilePath = resolveSafeChildPath(templateDir, location); - const stats = await fs.promises.lstat(inputFilePath); - - if (stats.isSymbolicLink() || (await isBinaryFile(inputFilePath))) { + if (!renderContents && !extension) { ctx.logger.info( - `Copying file binary or symbolic link at ${location}, to template output path.`, + `Copying file/directory ${location} without processing.`, ); - await fs.copy(inputFilePath, outputPath); + } + + if (location.endsWith('/')) { + ctx.logger.info( + `Writing directory ${location} to template output path.`, + ); + await fs.ensureDir(outputPath); } else { - const statsObj = await fs.stat(inputFilePath); - ctx.logger.info( - `Writing file ${location} to template output path with mode ${statsObj.mode}.`, - ); - const inputFileContents = await fs.readFile(inputFilePath, 'utf-8'); - await fs.outputFile( - outputPath, - renderContents - ? renderTemplate(inputFileContents, context) - : inputFileContents, - { mode: statsObj.mode }, - ); + const inputFilePath = resolveSafeChildPath(templateDir, location); + const stats = await fs.promises.lstat(inputFilePath); + + if (stats.isSymbolicLink() || (await isBinaryFile(inputFilePath))) { + ctx.logger.info( + `Copying file binary or symbolic link at ${location}, to template output path.`, + ); + await fs.copy(inputFilePath, outputPath); + } else { + const statsObj = await fs.stat(inputFilePath); + ctx.logger.info( + `Writing file ${location} to template output path with mode ${statsObj.mode}.`, + ); + const inputFileContents = await fs.readFile(inputFilePath, 'utf-8'); + await fs.outputFile( + outputPath, + renderContents + ? renderTemplate(inputFileContents, context) + : inputFileContents, + { mode: statsObj.mode }, + ); + } } } + } finally { + dispose(); } - dispose(); ctx.logger.info(`Template result written to ${outputDir}`); } From 191ea1357847bf6145e17d94f98ca2a26f2d5e50 Mon Sep 17 00:00:00 2001 From: Justin Bryant Date: Fri, 17 Apr 2026 17:43:07 -0400 Subject: [PATCH 06/11] Update plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Justin Bryant --- .../src/lib/templating/SecureTemplater.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts index 2957848279..cd9eedcd96 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts @@ -36,9 +36,21 @@ describe('SecureTemplater', () => { test: 'my-value', }), ).toThrow(/expected name as lookup value, got ./); + dispose(); + + expect(() => render('${{ test }}', { test: 'my-value' })).toThrow( + /disposed/i, + ); }); + it('should allow dispose to be called more than once', async () => { + const { dispose } = await SecureTemplater.loadRenderer(); + + dispose(); + + expect(() => dispose()).not.toThrow(); + }); it('should make cookiecutter compatibility available when requested', async () => { const { render: renderWith, dispose: disposeWith } = await SecureTemplater.loadRenderer({ From 6d6c7f64a6ae23d5c4d1fd83deca6988f0f5d89d Mon Sep 17 00:00:00 2001 From: Justin Bryant Date: Fri, 17 Apr 2026 17:43:25 -0400 Subject: [PATCH 07/11] Update plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Justin Bryant --- .../src/scaffolder/tasks/NunjucksWorkflowRunner.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 57c829a499..a002d8030f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -365,8 +365,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { )) ?? {}; taskLogger.info( - `Running ${action.id - } in dry-run mode with inputs (secrets redacted): ${JSON.stringify( + `Running ${action.id} in dry-run mode with inputs (secrets redacted): ${JSON.stringify( debugInput, undefined, 2, From f32100a977ede18ec713cb95e2dfbd55f784815d Mon Sep 17 00:00:00 2001 From: Justin Bryant Date: Fri, 17 Apr 2026 17:43:38 -0400 Subject: [PATCH 08/11] Update plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Justin Bryant --- .../src/scaffolder/tasks/NunjucksWorkflowRunner.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index a002d8030f..28667ed9f2 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -435,8 +435,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { continue; } - const actionId = `${action.id}${iteration.each ? `[${iteration.each.key}]` : '' - }`; + const actionId = `${action.id}${iteration.each ? `[${iteration.each.key}]` : ''}`; if (action.schema?.input) { const validateResult = validateJsonSchema( From ac64f0ad049dbadfcf0c61a1528312923aeecae1 Mon Sep 17 00:00:00 2001 From: Justin Bryant Date: Fri, 17 Apr 2026 17:44:35 -0400 Subject: [PATCH 09/11] fix(SecureTemplater): implement PR suggestions Signed-off-by: Justin Bryant --- .../src/lib/templating/SecureTemplater.ts | 8 ++++++-- .../actions/builtin/fetch/templateFileActionHandler.ts | 4 +--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts index e230dba184..72815fbd89 100644 --- a/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts +++ b/plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts @@ -222,8 +222,12 @@ export class SecureTemplater { render, dispose: () => { if (context && !isolate.isDisposed) { - context.release(); - isolate.dispose(); + try { + context.release(); + isolate.dispose(); + } catch (error) { + // Ignore errors during dispose, as there's not much we can do about it + } } }, }; diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts index ceaeb9b984..ab85070fa3 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts +++ b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts @@ -21,10 +21,8 @@ import { TemplateGlobal, } from '@backstage/plugin-scaffolder-node'; import fs from 'fs-extra'; -import { createDefaultFilters } from '../../../../lib/templating/filters/createDefaultFilters'; -import { convertFiltersToRecord } from '../../../../util/templating'; -import { resolveSafeChildPath } from '@backstage/backend-plugin-api'; import path from 'node:path'; +import { createDefaultFilters } from '../../../../lib/templating/filters/createDefaultFilters'; import { SecureTemplater } from '../../../../lib/templating/SecureTemplater'; import { convertFiltersToRecord } from '../../../../util/templating'; From 80c5481a443499c62969afe581bcaad07fdc3953 Mon Sep 17 00:00:00 2001 From: Justin Bryant Date: Fri, 17 Apr 2026 18:02:53 -0400 Subject: [PATCH 10/11] fix: run prettier Signed-off-by: Justin Bryant --- .../tasks/NunjucksWorkflowRunner.ts | 23 +++++++++++++------ 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 28667ed9f2..0d900326c8 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -66,6 +66,11 @@ import { CheckpointState, } from '@backstage/plugin-scaffolder-node/alpha'; import { resolveDefaultEnvironment } from '../../lib/defaultEnvironment'; +import { createDefaultFilters } from '../../lib/templating/filters/createDefaultFilters'; +import { scaffolderActionRules } from '../../service/rules'; +import { createCounterMetric, createHistogramMetric } from '../../util/metrics'; +import { convertFiltersToRecord } from '../../util/templating'; +import { BackstageLoggerTransport, WinstonLogger } from './logger'; type NunjucksWorkflowRunnerOptions = { workingDirectory: string; @@ -365,7 +370,9 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { )) ?? {}; taskLogger.info( - `Running ${action.id} in dry-run mode with inputs (secrets redacted): ${JSON.stringify( + `Running ${ + action.id + } in dry-run mode with inputs (secrets redacted): ${JSON.stringify( debugInput, undefined, 2, @@ -409,8 +416,8 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { const iterations = ( resolvedEach ? Object.entries(resolvedEach).map(([key, value]) => ({ - each: { key, value }, - })) + each: { key, value }, + })) : [{}] ).map(i => { const fullContext = { ...preIterationContext, ...i }; @@ -435,7 +442,9 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { continue; } - const actionId = `${action.id}${iteration.each ? `[${iteration.each.key}]` : ''}`; + const actionId = `${action.id}${ + iteration.each ? `[${iteration.each.key}]` : '' + }`; if (action.schema?.input) { const validateResult = validateJsonSchema( @@ -668,9 +677,9 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { const [decision]: PolicyDecision[] = this.options.permissions && task.spec.steps.length ? await this.options.permissions.authorizeConditional( - [{ permission: actionExecutePermission }], - { credentials: await task.getInitiatorCredentials() }, - ) + [{ permission: actionExecutePermission }], + { credentials: await task.getInitiatorCredentials() }, + ) : [{ result: AuthorizeResult.ALLOW }]; for (const step of task.spec.steps) { From 964bc8694ff6cd17fa809a28db2f5ee9bb0ec3b1 Mon Sep 17 00:00:00 2001 From: Justin Bryant Date: Wed, 6 May 2026 10:04:32 -0400 Subject: [PATCH 11/11] fix: run prettier Signed-off-by: Justin Bryant --- .../tasks/NunjucksWorkflowRunner.ts | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 595bf19e9a..77c4239271 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -650,23 +650,24 @@ export class NunjucksWorkflowRunner implements WorkflowRunner { // Track whether a status check global (always/failure) was invoked during rendering const statusCheckInvoked = { value: false }; - const { render: renderTemplate, dispose } = await SecureTemplater.loadRenderer({ - templateFilters: { - ...this.defaultTemplateFilters, - ...additionalTemplateFilters, - }, - templateGlobals: { - ...additionalTemplateGlobals, - always: () => { - statusCheckInvoked.value = true; - return true; + const { render: renderTemplate, dispose } = + await SecureTemplater.loadRenderer({ + templateFilters: { + ...this.defaultTemplateFilters, + ...additionalTemplateFilters, }, - failure: () => { - statusCheckInvoked.value = true; - return taskState.failed; + templateGlobals: { + ...additionalTemplateGlobals, + always: () => { + statusCheckInvoked.value = true; + return true; + }, + failure: () => { + statusCheckInvoked.value = true; + return taskState.failed; + }, }, - }, - }); + }); try { await task.rehydrateWorkspace?.({ taskId, targetPath: workspacePath });