From 0f99f1170e314c7370767564dd81417146e074dc Mon Sep 17 00:00:00 2001 From: Oliver Sand Date: Wed, 13 Oct 2021 16:34:40 +0200 Subject: [PATCH] Make sure `sourcePath` of `publish:github:pull-request` can only be used to retrieve files from the workspace Signed-off-by: Oliver Sand --- .changeset/stale-poets-destroy.md | 6 ++++ .../builtin/publish/githubPullRequest.test.ts | 29 ++++++++++++------- .../builtin/publish/githubPullRequest.ts | 3 +- 3 files changed, 26 insertions(+), 12 deletions(-) create mode 100644 .changeset/stale-poets-destroy.md diff --git a/.changeset/stale-poets-destroy.md b/.changeset/stale-poets-destroy.md new file mode 100644 index 0000000000..71f8a478ec --- /dev/null +++ b/.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. diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts index 2ba30bb3db..ed72b14a65 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.test.ts +++ b/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', () => { diff --git a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts b/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts index e21ffc5cd0..253fd72b38 100644 --- a/plugins/scaffolder-backend/src/scaffolder/actions/builtin/publish/githubPullRequest.ts +++ b/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'], {