paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 48 Packages Projects Releases Wiki Activity

fix(SecureTemplater): return dispose function to clean up secure templater isolate vm & context

Browse Source 
Signed-off-by: Justin Bryant <justintbry@gmail.com>
This commit is contained in:
Justin Bryant
2026-04-17 17:00:37 -04:00
parent 6e2b7fffb4
commit 4fb8469d37
5 changed files with 90 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts
+38 -19
View File
@@ -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();
});
});
plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts
+10 -3
View File
@@ -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();
},
};
}
}
plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateActionHandler.ts
+14 -12
View File
@@ -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}`);
}
plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/templateFileActionHandler.ts
+14 -12
View File
@@ -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}`);
}
plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts
+14 -12
View File
@@ -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 {