Merge pull request #7588 from SDA-SE/feat/githubpraction

Make sure `sourcePath` of `publish:github:pull-request` can only be used to retrieve files from the workspace.

.changeset/stale-poets-destroy.md

@@ -0,0 +1,6 @@
+---
+'@backstage/plugin-scaffolder-backend': patch
+---
+
+Make sure `sourcePath` of `publish:github:pull-request` can only be used to
+retrieve files from the workspace.

plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts

@@ -14,21 +14,20 @@
  * limitations under the License.
  */
 
+import { getRootLogger } from '@backstage/backend-common';
+import { ConfigReader } from '@backstage/config';
+import { ScmIntegrations } from '@backstage/integration';
 import mockFs from 'mock-fs';
-import { Writable } from 'stream';
 import os from 'os';
 import { resolve as resolvePath } from 'path';
-import {
-  PullRequestCreator,
-  GithubPullRequestActionInput,
-  createPublishGithubPullRequestAction,
-  ClientFactoryInput,
-} from './githubPullRequest';
+import { Writable } from 'stream';
 import { ActionContext, TemplateAction } from '../../types';
-import { getRootLogger } from '@backstage/backend-common';
-
-import { ScmIntegrations } from '@backstage/integration';
-import { ConfigReader } from '@backstage/config';
+import {
+  ClientFactoryInput,
+  createPublishGithubPullRequestAction,
+  GithubPullRequestActionInput,
+  PullRequestCreator,
+} from './githubPullRequest';
 
 const root = os.platform() === 'win32' ? 'C:\\root' : '/root';
 const workspacePath = resolvePath(root, 'my-workspace');
@@ -174,6 +173,14 @@ describe('createPublishGithubPullRequestAction', () => {
         ],
       });
     });
+
+    it('should not allow to use files outside of the workspace', async () => {
+      input.sourcePath = '../../test';
+
+      await expect(instance.handler(ctx)).rejects.toThrow(
+        'Relative path is not allowed to refer to a directory outside its parent',
+      );
+    });
   });
 
   describe('with repoUrl', () => {

plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts

@@ -28,6 +28,7 @@ import { Octokit } from '@octokit/rest';
 import { InputError, CustomErrorBase } from '@backstage/errors';
 import { createPullRequest } from 'octokit-plugin-create-pull-request';
 import globby from 'globby';
+import { resolveSafeChildPath } from '@backstage/backend-common';
 
 class GithubResponseError extends CustomErrorBase {}
 
@@ -183,7 +184,7 @@ export const createPublishGithubPullRequestAction = ({
 
       const client = await clientFactory({ integrations, host, owner, repo });
       const fileRoot = sourcePath
-        ? path.resolve(ctx.workspacePath, sourcePath)
+        ? resolveSafeChildPath(ctx.workspacePath, sourcePath)
         : ctx.workspacePath;
 
       const localFilePaths = await globby(['./**', './**/.*', '!.git'], {

plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/util.test.ts

@@ -0,0 +1,43 @@
+/*
+ * Copyright 2021 The Backstage Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { getRepoSourceDirectory } from './util';
+
+describe('getRepoSourceDirectory', () => {
+  test('should return workspace root if no sub folder is given', () => {
+    expect(getRepoSourceDirectory('/var/workspace', undefined)).toEqual(
+      '/var/workspace',
+    );
+  });
+
+  test('should return path in workspace if sub folder is given', () => {
+    expect(
+      getRepoSourceDirectory('/var/workspace', 'path/of/subfolder'),
+    ).toEqual('/var/workspace/path/of/subfolder');
+  });
+
+  test('should not allow traversal outside the workspace root', () => {
+    expect(getRepoSourceDirectory('/var/workspace', '../secret')).toEqual(
+      '/var/workspace/secret',
+    );
+    expect(
+      getRepoSourceDirectory('/var/workspace', './path/../../secret'),
+    ).toEqual('/var/workspace/secret');
+    expect(
+      getRepoSourceDirectory('/var/workspace', '/absolute/secret'),
+    ).toEqual('/var/workspace/absolute/secret');
+  });
+});