Skip to content
Permalink
Browse files Browse the repository at this point in the history
scaffolder-backend: removed all usaged and prevent new usage of path.…
…resolve

Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
  • Loading branch information
Rugvip committed Nov 24, 2021
1 parent 103ca7e commit f9352ab
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/serious-pens-tease.md
@@ -0,0 +1,5 @@
---
'@backstage/plugin-scaffolder-backend': patch
---

Removed all usages of `path.resolve` in order to ensure that template paths are resolved in a safe way.
33 changes: 33 additions & 0 deletions plugins/scaffolder-backend/.eslintrc.js
@@ -1,8 +1,41 @@
const parent = require('@backstage/cli/config/eslint.backend');

module.exports = {
extends: [require.resolve('@backstage/cli/config/eslint.backend')],
ignorePatterns: ['sample-templates/'],
rules: {
'no-console': 0, // Permitted in console programs
'new-cap': ['error', { capIsNew: false }], // Because Express constructs things e.g. like 'const r = express.Router()'
// Usage of path.resolve is extra sensitive in the scaffolder, so forbid it in non-test code
'no-restricted-imports': [
'error',
{
...parent.rules['no-restricted-imports'][1],
paths: [
{
name: 'path',
importNames: ['resolve'],
message:
'Do not use path.resolve, use `resolveSafeChildPath` from `@backstage/backend-common` instead as it prevents security issues',
},
],
},
],
'no-restricted-syntax': parent.rules['no-restricted-syntax'].concat([
{
message:
'Do not use path.resolve, use `resolveSafeChildPath` from `@backstage/backend-common` instead as it prevents security issues',
selector:
'MemberExpression[object.name="path"][property.name="resolve"]',
},
]),
},
overrides: [
{
files: ['*.test.*', 'src/setupTests.*', 'dev/**'],
rules: {
'no-restricted-imports': parent.rules['no-restricted-imports'],
},
},
],
};
Expand Up @@ -15,10 +15,10 @@
*/

import fs from 'fs-extra';
import { resolve as resolvePath } from 'path';
import { createTemplateAction } from '../../createTemplateAction';
import * as yaml from 'yaml';
import { Entity } from '@backstage/catalog-model';
import { resolveSafeChildPath } from '@backstage/backend-common';

export function createCatalogWriteAction() {
return createTemplateAction<{ name?: string; entity: Entity }>({
Expand All @@ -42,7 +42,7 @@ export function createCatalogWriteAction() {
const { entity } = ctx.input;

await fs.writeFile(
resolvePath(ctx.workspacePath, 'catalog-info.yaml'),
resolveSafeChildPath(ctx.workspacePath, 'catalog-info.yaml'),
yaml.stringify(entity),
);
},
Expand Down
Expand Up @@ -15,7 +15,7 @@
*/

import { readdir, stat } from 'fs-extra';
import { relative, resolve } from 'path';
import { relative, join } from 'path';
import { createTemplateAction } from '../../createTemplateAction';

/**
Expand Down Expand Up @@ -68,7 +68,7 @@ export async function recursiveReadDir(dir: string): Promise<string[]> {
const subdirs = await readdir(dir);
const files = await Promise.all(
subdirs.map(async subdir => {
const res = resolve(dir, subdir);
const res = join(dir, subdir);
return (await stat(res)).isDirectory() ? recursiveReadDir(res) : [res];
}),
);
Expand Down
Expand Up @@ -19,7 +19,7 @@ import { JsonValue } from '@backstage/types';
import { InputError } from '@backstage/errors';
import { ScmIntegrations } from '@backstage/integration';
import fs from 'fs-extra';
import * as path from 'path';
import path from 'path';

export async function fetchContents({
reader,
Expand Down
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import { resolve as resolvePath, extname } from 'path';
import { extname } from 'path';
import { resolveSafeChildPath, UrlReader } from '@backstage/backend-common';
import { InputError } from '@backstage/errors';
import { ScmIntegrations } from '@backstage/integration';
Expand Down Expand Up @@ -114,7 +114,7 @@ export function createFetchTemplateAction(options: {
ctx.logger.info('Fetching template content from remote URL');

const workDir = await ctx.createTemporaryDirectory();
const templateDir = resolvePath(workDir, 'template');
const templateDir = resolveSafeChildPath(workDir, 'template');

const targetPath = ctx.input.targetPath ?? './';
const outputDir = resolveSafeChildPath(ctx.workspacePath, targetPath);
Expand Down Expand Up @@ -240,7 +240,7 @@ export function createFetchTemplateAction(options: {
if (renderFilename) {
localOutputPath = templater.renderString(localOutputPath, context);
}
const outputPath = resolvePath(outputDir, localOutputPath);
const outputPath = resolveSafeChildPath(outputDir, localOutputPath);
// variables have been expanded to make an empty file name
// this is due to a conditional like if values.my_condition then file-name.txt else empty string so skip
if (outputDir === outputPath) {
Expand All @@ -259,7 +259,7 @@ export function createFetchTemplateAction(options: {
);
await fs.ensureDir(outputPath);
} else {
const inputFilePath = resolvePath(templateDir, location);
const inputFilePath = resolveSafeChildPath(templateDir, location);

if (await isBinaryFile(inputFilePath)) {
ctx.logger.info(
Expand Down
Expand Up @@ -15,7 +15,6 @@
*/

import fs from 'fs-extra';
import path from 'path';
import { parseRepoUrl, isExecutable } from './util';

import {
Expand Down Expand Up @@ -197,7 +196,7 @@ export const createPublishGithubPullRequestAction = ({

const fileContents = await Promise.all(
localFilePaths.map(filePath => {
const absPath = path.resolve(fileRoot, filePath);
const absPath = resolveSafeChildPath(fileRoot, filePath);
const base64EncodedContent = fs
.readFileSync(absPath)
.toString('base64');
Expand Down

0 comments on commit f9352ab

Please sign in to comment.