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

scaffolder-backend: refactor SecureTemplater to use separate sandboxes

Browse Source 
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
This commit is contained in:
Patrik Oldsberg
2021-11-24 20:59:52 +01:00
parent 4f268b8ef1
commit e7cc6bf28f
4 changed files with 122 additions and 125 deletions
Download Patch File Download Diff File Expand all files Collapse all files
plugins/scaffolder-backend/src/lib/templating/SecureTemplater.test.ts
+46 -42
View File
@@ -18,24 +18,21 @@ import { SecureTemplater } from './SecureTemplater';
describe('SecureTemplater', () => {
it('should render some templates', async () => {
const templater = new SecureTemplater();
await templater.initializeIfNeeded();
expect(templater.render('${{ test }}', { test: 'my-value' })).toBe(
'my-value',
);
const render = await SecureTemplater.loadRenderer();
expect(render('${{ test }}', { test: 'my-value' })).toBe('my-value');
expect(templater.render('${{ test | dump }}', { test: 'my-value' })).toBe(
expect(render('${{ test | dump }}', { test: 'my-value' })).toBe(
'"my-value"',
);
expect(
templater.render('${{ test | replace("my-", "our-") }}', {
render('${{ test | replace("my-", "our-") }}', {
test: 'my-value',
}),
).toBe('our-value');
expect(() =>
templater.render('${{ invalid...syntax }}', {
render('${{ invalid...syntax }}', {
test: 'my-value',
}),
).toThrow(
@@ -44,34 +41,28 @@ describe('SecureTemplater', () => {
});
it('should make cookiecutter compatibility available when requested', async () => {
const templater = new SecureTemplater();
await templater.initializeIfNeeded();
const renderWith = await SecureTemplater.loadRenderer({
cookiecutterCompat: true,
});
const renderWithout = await SecureTemplater.loadRenderer();
// Same two tests repeated to make sure switching back and forth works
expect(
templater.render('{{ 1 | jsonify }}', {}, { cookiecutterCompat: true }),
).toBe('1');
expect(
templater.render('{{ 1 | jsonify }}', {}, { cookiecutterCompat: true }),
).toBe('1');
expect(() => templater.render('${{ 1 | jsonify }}', {})).toThrow(
expect(renderWith('{{ 1 | jsonify }}', {})).toBe('1');
expect(renderWith('{{ 1 | jsonify }}', {})).toBe('1');
expect(() => renderWithout('${{ 1 | jsonify }}', {})).toThrow(
'(unknown path)\n Error: filter not found: jsonify',
);
expect(
templater.render('{{ 1 | jsonify }}', {}, { cookiecutterCompat: true }),
).toBe('1');
expect(() => templater.render('${{ 1 | jsonify }}', {})).toThrow(
expect(renderWith('{{ 1 | jsonify }}', {})).toBe('1');
expect(() => renderWithout('${{ 1 | jsonify }}', {})).toThrow(
'(unknown path)\n Error: filter not found: jsonify',
);
expect(() => templater.render('${{ 1 | jsonify }}', {})).toThrow(
expect(() => renderWithout('${{ 1 | jsonify }}', {})).toThrow(
'(unknown path)\n Error: filter not found: jsonify',
);
expect(() => templater.render('${{ 1 | jsonify }}', {})).toThrow(
expect(() => renderWithout('${{ 1 | jsonify }}', {})).toThrow(
'(unknown path)\n Error: filter not found: jsonify',
);
expect(
templater.render('{{ 1 | jsonify }}', {}, { cookiecutterCompat: true }),
).toBe('1');
expect(renderWith('{{ 1 | jsonify }}', {})).toBe('1');
});
it('should make parseRepoUrl available when requested', async () => {
@@ -80,33 +71,29 @@ describe('SecureTemplater', () => {
owner: 'my-owner',
host: 'my-host.com',
}));
const templaterWith = new SecureTemplater({ parseRepoUrl });
await templaterWith.initializeIfNeeded();
const templaterWithout = new SecureTemplater();
await templaterWithout.initializeIfNeeded();
const renderWith = await SecureTemplater.loadRenderer({ parseRepoUrl });
const renderWithout = await SecureTemplater.loadRenderer();
const ctx = {
repoUrl: 'https://my-host.com/my-owner/my-repo',
};
expect(
templaterWith.render('${{ repoUrl | parseRepoUrl | dump }}', ctx),
).toBe(
expect(renderWith('${{ repoUrl | parseRepoUrl | dump }}', ctx)).toBe(
JSON.stringify({
repo: 'my-repo',
owner: 'my-owner',
host: 'my-host.com',
}),
);
expect(templaterWith.render('${{ repoUrl | projectSlug }}', ctx)).toBe(
expect(renderWith('${{ repoUrl | projectSlug }}', ctx)).toBe(
'my-owner/my-repo',
);
expect(() =>
templaterWithout.render('${{ repoUrl | parseRepoUrl | dump }}', ctx),
renderWithout('${{ repoUrl | parseRepoUrl | dump }}', ctx),
).toThrow('(unknown path)\n Error: filter not found: parseRepoUrl');
expect(() =>
templaterWithout.render('${{ repoUrl | projectSlug }}', ctx),
).toThrow('(unknown path)\n Error: filter not found: projectSlug');
expect(() => renderWithout('${{ repoUrl | projectSlug }}', ctx)).toThrow(
'(unknown path)\n Error: filter not found: projectSlug',
);
expect(parseRepoUrl.mock.calls).toEqual([
['https://my-host.com/my-owner/my-repo'],
@@ -115,26 +102,25 @@ describe('SecureTemplater', () => {
});
it('should not allow helpers to be rewritten', async () => {
const templater = new SecureTemplater({
const render = await SecureTemplater.loadRenderer({
parseRepoUrl: () => ({
repo: 'my-repo',
owner: 'my-owner',
host: 'my-host.com',
}),
});
await templater.initializeIfNeeded();
const ctx = {
repoUrl: 'https://my-host.com/my-owner/my-repo',
};
expect(
templater.render(
render(
'${{ ({}).constructor.constructor("parseRepoUrl = () => JSON.stringify(`inject`)")() }}',
ctx,
),
).toBe('');
expect(templater.render('${{ repoUrl | parseRepoUrl | dump }}', ctx)).toBe(
expect(render('${{ repoUrl | parseRepoUrl | dump }}', ctx)).toBe(
JSON.stringify({
repo: 'my-repo',
owner: 'my-owner',
@@ -142,4 +128,22 @@ describe('SecureTemplater', () => {
}),
);
});
it('allows pollution during a single template execution', async () => {
const render = await SecureTemplater.loadRenderer();
const ctx = {
x: 'foo',
};
expect(render('${{ x }}', ctx)).toBe('foo');
expect(
render(
'${{ ({}).constructor.constructor("Array.prototype.forEach = () => {}")() }}',
ctx,
),
).toBe('');
expect(() => render('${{ x }}', ctx)).toThrow(
`TypeError: Cannot read property 'length' of undefined`,
);
});
});
plugins/scaffolder-backend/src/lib/templating/SecureTemplater.ts
+37 -50
View File
@@ -87,68 +87,55 @@ const { render, renderCompat } = (() => {
})();
`;
export interface SecureTemplaterRenderOptions {
export interface SecureTemplaterOptions {
/* Optional implementation of the parseRepoUrl filter */
parseRepoUrl?(repoUrl: string): RepoSpec;
/* Enables jinja compatibility and the "jsonify" filter */
cookiecutterCompat?: boolean;
}
export interface SecureTemplaterOptions {
parseRepoUrl?(repoUrl: string): RepoSpec;
}
export type SecureTemplateRenderer = (
template: string,
values: unknown,
) => string;
export class SecureTemplater {
#vm?: VM;
static async loadRenderer(options: SecureTemplaterOptions = {}) {
const { parseRepoUrl, cookiecutterCompat } = options;
let sandbox = undefined;
#parseRepoUrl?: (repoUrl: string) => RepoSpec;
constructor(options?: SecureTemplaterOptions) {
this.#parseRepoUrl = options?.parseRepoUrl;
}
render(
template: string,
values: unknown,
options?: SecureTemplaterRenderOptions,
): string {
if (!this.#vm) {
throw new Error('SecureTemplater has not been initialized');
}
this.#vm.setGlobal('templateStr', template);
this.#vm.setGlobal('templateValues', JSON.stringify(values));
if (options?.cookiecutterCompat) {
return this.#vm.run(`renderCompat(templateStr, templateValues)`);
if (parseRepoUrl) {
sandbox = {
parseRepoUrl: (url: string) => JSON.stringify(parseRepoUrl(url)),
};
}
return this.#vm.run(`render(templateStr, templateValues)`);
}
const vm = new VM({ timeout: 1000, sandbox });
async initializeIfNeeded() {
if (!this.#vm) {
let sandbox = undefined;
const nunjucksSource = await fs.readFile(
resolvePackagePath(
'@backstage/plugin-scaffolder-backend',
'assets/nunjucks.js.txt',
),
'utf-8',
);
if (this.#parseRepoUrl) {
const parseRepoUrl = this.#parseRepoUrl;
sandbox = {
parseRepoUrl: (url: string) => JSON.stringify(parseRepoUrl(url)),
};
vm.run(mkScript(nunjucksSource));
const render: SecureTemplateRenderer = (template, values) => {
if (!vm) {
throw new Error('SecureTemplater has not been initialized');
}
vm.setGlobal('templateStr', template);
vm.setGlobal('templateValues', JSON.stringify(values));
if (cookiecutterCompat) {
return vm.run(`renderCompat(templateStr, templateValues)`);
}
// Note that helpers in the sandbox can be rewritten with a script injection.
// In order to mitigate that each helper must be assigned to a constant variable
// inside the closure that houses the nunjucks implementation.
this.#vm = new VM({ timeout: 1000, sandbox });
const nunjucksSource = await fs.readFile(
resolvePackagePath(
'@backstage/plugin-scaffolder-backend',
'assets/nunjucks.js.txt',
),
'utf-8',
);
this.#vm.run(mkScript(nunjucksSource));
}
return this.#vm;
return vm.run(`render(templateStr, templateValues)`);
};
return render;
}
}
plugins/scaffolder-backend/src/scaffolder/actions/builtin/fetch/template.ts
+6 -10
View File
@@ -47,8 +47,6 @@ export function createFetchTemplateAction(options: {
}) {
const { reader, integrations } = options;
const templater = new SecureTemplater();
return createTemplateAction<FetchTemplateInput>({
id: 'fetch:template',
description:
@@ -102,8 +100,6 @@ export function createFetchTemplateAction(options: {
async handler(ctx) {
ctx.logger.info('Fetching template content from remote URL');
await templater.initializeIfNeeded();
const workDir = await ctx.createTemporaryDirectory();
const templateDir = resolvePath(workDir, 'template');
@@ -184,6 +180,10 @@ export function createFetchTemplateAction(options: {
ctx.input.values,
);
const renderTemplate = await SecureTemplater.loadRenderer({
cookiecutterCompat: ctx.input.cookiecutterCompat,
});
for (const location of allEntriesInTemplate) {
let renderFilename: boolean;
let renderContents: boolean;
@@ -199,9 +199,7 @@ export function createFetchTemplateAction(options: {
renderFilename = renderContents = !nonTemplatedEntries.has(location);
}
if (renderFilename) {
localOutputPath = templater.render(localOutputPath, context, {
cookiecutterCompat: ctx.input.cookiecutterCompat,
});
localOutputPath = renderTemplate(localOutputPath, context);
}
const outputPath = resolvePath(outputDir, localOutputPath);
// variables have been expanded to make an empty file name
@@ -238,9 +236,7 @@ export function createFetchTemplateAction(options: {
await fs.outputFile(
outputPath,
renderContents
? templater.render(inputFileContents, context, {
cookiecutterCompat: ctx.input.cookiecutterCompat,
})
? renderTemplate(inputFileContents, context)
: inputFileContents,
{ mode: statsObj.mode },
);
plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts
+33 -23
View File
@@ -34,7 +34,10 @@ import { isTruthy } from './helper';
import { validate as validateJsonSchema } from 'jsonschema';
import { parseRepoUrl } from '../actions/builtin/publish/util';
import { TemplateActionRegistry } from '../actions';
import { SecureTemplater } from '../../lib/templating/SecureTemplater';
import {
SecureTemplater,
SecureTemplateRenderer,
} from '../../lib/templating/SecureTemplater';
type NunjucksWorkflowRunnerOptions = {
workingDirectory: string;
@@ -86,21 +89,7 @@ const createStepLogger = ({
};
export class NunjucksWorkflowRunner implements WorkflowRunner {
private readonly templater: SecureTemplater;
constructor(private readonly options: NunjucksWorkflowRunnerOptions) {
const { integrations } = options;
this.templater = new SecureTemplater({
// TODO(blam): let's work out how we can deprecate this.
// We shouldn't really need to be exposing these now we can deal with
// objects in the params block.
// Maybe we can expose a new RepoUrlPicker with secrets for V3 that provides an object already.
parseRepoUrl(url: string) {
return parseRepoUrl(url, integrations);
},
});
}
constructor(private readonly options: NunjucksWorkflowRunnerOptions) {}
private isSingleTemplateString(input: string) {
const { parser, nodes } = nunjucks as unknown as {
@@ -132,7 +121,11 @@ export class NunjucksWorkflowRunner implements WorkflowRunner {
);
}
private render<T>(input: T, context: TemplateContext): T {
private render<T>(
input: T,
context: TemplateContext,
renderTemplate: SecureTemplateRenderer,
): T {
return JSON.parse(JSON.stringify(input), (_key, value) => {
try {
if (typeof value === 'string') {
@@ -145,7 +138,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner {
);
// Run the templating
const templated = this.templater.render(wrappedDumped, context);
const templated = renderTemplate(wrappedDumped, context);
// If there's an empty string returned, then it's undefined
if (templated === '') {
@@ -162,7 +155,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner {
}
// Fallback to default behaviour
const templated = this.templater.render(value, context);
const templated = renderTemplate(value, context);
if (templated === '') {
return undefined;
@@ -187,12 +180,23 @@ export class NunjucksWorkflowRunner implements WorkflowRunner {
this.options.workingDirectory,
await task.getWorkspaceName(),
);
const { integrations } = this.options;
const renderTemplate = await SecureTemplater.loadRenderer({
// TODO(blam): let's work out how we can deprecate this.
// We shouldn't really need to be exposing these now we can deal with
// objects in the params block.
// Maybe we can expose a new RepoUrlPicker with secrets for V3 that provides an object already.
parseRepoUrl(url: string) {
return parseRepoUrl(url, integrations);
},
});
try {
await fs.ensureDir(workspacePath);
await task.emitLog(
`Starting up task with ${task.spec.steps.length} steps`,
);
await this.templater.initializeIfNeeded();
const context: TemplateContext = {
parameters: task.spec.parameters,
@@ -202,7 +206,11 @@ export class NunjucksWorkflowRunner implements WorkflowRunner {
for (const step of task.spec.steps) {
try {
if (step.if) {
const ifResult = await this.render(step.if, context);
const ifResult = await this.render(
step.if,
context,
renderTemplate,
);
if (!isTruthy(ifResult)) {
await task.emitLog(
`Skipping step ${step.id} because it's if condition was false`,
@@ -220,7 +228,9 @@ export class NunjucksWorkflowRunner implements WorkflowRunner {
const action = this.options.actionRegistry.get(step.action);
const { taskLogger, streamLogger } = createStepLogger({ task, step });
const input = (step.input && this.render(step.input, context)) ?? {};
const input =
(step.input && this.render(step.input, context, renderTemplate)) ??
{};
if (action.schema?.input) {
const validateResult = validateJsonSchema(
@@ -283,7 +293,7 @@ export class NunjucksWorkflowRunner implements WorkflowRunner {
}
}
const output = this.render(task.spec.output, context);
const output = this.render(task.spec.output, context, renderTemplate);
return { output };
} finally {