From ccfbf215df98b44d8f5de7afa57d2cdd05520fcb Mon Sep 17 00:00:00 2001 From: PaulGMardling Date: Mon, 7 Sep 2026 10:23:45 +0200 Subject: [PATCH 1/2] fix(workspace-plugin): prevent epic generator command injection Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../generators/epic-generator/index.spec.ts | 192 ++++++++++++------ .../src/generators/epic-generator/index.ts | 26 ++- 2 files changed, 146 insertions(+), 72 deletions(-) diff --git a/tools/workspace-plugin/src/generators/epic-generator/index.spec.ts b/tools/workspace-plugin/src/generators/epic-generator/index.spec.ts index 4ec2128a8ab43a..b328d7f75bc596 100644 --- a/tools/workspace-plugin/src/generators/epic-generator/index.spec.ts +++ b/tools/workspace-plugin/src/generators/epic-generator/index.spec.ts @@ -1,11 +1,11 @@ import { addProjectConfiguration, ProjectType, stripIndents, writeJson } from '@nx/devkit'; import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing'; -import { execSync, spawnSync, SpawnSyncReturns } from 'child_process'; +import { execFileSync, spawnSync, SpawnSyncReturns } from 'child_process'; import { workspacePaths } from '../../utils'; import epicGenerator from './index'; jest.mock('child_process'); -const execSyncMock = execSync as unknown as jest.Mock; +const execFileSyncMock = execFileSync as unknown as jest.Mock; const spawnSyncMock = spawnSync as unknown as jest.Mock>>; type Package = { @@ -49,7 +49,7 @@ function setupTest(packages: Package[]) { }); // response to epic creation - execSyncMock.mockReturnValueOnce('epicUrl'); + execFileSyncMock.mockReturnValueOnce('epicUrl'); /** * Responses for each of the packages created @@ -68,11 +68,11 @@ function setupTest(packages: Package[]) { return acc; }, []) .forEach(owner => { - execSyncMock.mockReturnValueOnce(`issueUrl-${owner}`); + execFileSyncMock.mockReturnValueOnce(`issueUrl-${owner}`); }); // response to editing the epic - execSyncMock.mockReturnValueOnce('epicUrl'); + execFileSyncMock.mockReturnValueOnce('epicUrl'); return tree; } @@ -103,6 +103,15 @@ describe('epic-generator', () => { Please follow the format {owner}/{repositoryName}." `); }); + + it.each(['microsoft/fluentui;malicious-command', 'microsoft/fluentui/extra', 'microsoft/'])( + 'rejects a repository containing extra characters: %s', + repository => { + const tree = createTreeWithEmptyWorkspace(); + + expect(() => epicGenerator(tree, { title: 'test title', repository })).toThrow(/invalid repository name/); + }, + ); }); describe('authentication', () => { @@ -204,73 +213,130 @@ describe('epic-generator', () => { }); effectsCall(); - expect(execSyncMock).toHaveBeenNthCalledWith( - 1, - stripIndents`gh issue create --repo "cool-company/repository" --title "test title" --body "*Description to be added*"`, - ); + expect(execFileSyncMock).toHaveBeenNthCalledWith(1, 'gh', [ + 'issue', + 'create', + '--repo', + 'cool-company/repository', + '--title', + 'test title', + '--body', + '*Description to be added*', + ]); // @microsoft/cxe-red issue creation - expect(execSyncMock).toHaveBeenNthCalledWith( - 2, - stripIndents`gh issue create --repo "cool-company/repository" --title "test title - @microsoft/cxe-red" --body "🚧 This is an auto-generated issue to individually track migration progress. - - ### Packages to migrate: - - react-link - - react-button"`, - ); + expect(execFileSyncMock).toHaveBeenNthCalledWith(2, 'gh', [ + 'issue', + 'create', + '--repo', + 'cool-company/repository', + '--title', + 'test title - @microsoft/cxe-red', + '--body', + stripIndents`🚧 This is an auto-generated issue to individually track migration progress. + + ### Packages to migrate: + - react-link + - react-button`, + ]); // @microsoft/cxe-prg issue creation - expect(execSyncMock).toHaveBeenNthCalledWith( - 3, - stripIndents`gh issue create --repo "cool-company/repository" --title "test title - @microsoft/cxe-prg" --body "🚧 This is an auto-generated issue to individually track migration progress. - - ### Packages to migrate: - - react-card"`, - ); + expect(execFileSyncMock).toHaveBeenNthCalledWith(3, 'gh', [ + 'issue', + 'create', + '--repo', + 'cool-company/repository', + '--title', + 'test title - @microsoft/cxe-prg', + '--body', + stripIndents`🚧 This is an auto-generated issue to individually track migration progress. + + ### Packages to migrate: + - react-card`, + ]); // @microsoft/teams-prg issue creation - expect(execSyncMock).toHaveBeenNthCalledWith( - 4, - stripIndents`gh issue create --repo "cool-company/repository" --title "test title - @microsoft/teams-prg" --body "🚧 This is an auto-generated issue to individually track migration progress. - - ### Packages to migrate: - - react-menu - - react-accordion"`, - ); + expect(execFileSyncMock).toHaveBeenNthCalledWith(4, 'gh', [ + 'issue', + 'create', + '--repo', + 'cool-company/repository', + '--title', + 'test title - @microsoft/teams-prg', + '--body', + stripIndents`🚧 This is an auto-generated issue to individually track migration progress. + + ### Packages to migrate: + - react-menu + - react-accordion`, + ]); // no owner issue creation - expect(execSyncMock).toHaveBeenNthCalledWith( - 5, - stripIndents`gh issue create --repo "cool-company/repository" --title "test title - ownerless" --body "🚧 This is an auto-generated issue to individually track migration progress. - - ### Packages to migrate: - - misterious-unowned-package"`, - ); + expect(execFileSyncMock).toHaveBeenNthCalledWith(5, 'gh', [ + 'issue', + 'create', + '--repo', + 'cool-company/repository', + '--title', + 'test title - ownerless', + '--body', + stripIndents`🚧 This is an auto-generated issue to individually track migration progress. + + ### Packages to migrate: + - misterious-unowned-package`, + ]); // @microsoft/cxe-coastal issue creation - expect(execSyncMock).toHaveBeenNthCalledWith( - 6, - stripIndents`gh issue create --repo "cool-company/repository" --title "test title - @microsoft/cxe-coastal" --body "🚧 This is an auto-generated issue to individually track migration progress. - - ### Packages to migrate: - - react-slider"`, - ); + expect(execFileSyncMock).toHaveBeenNthCalledWith(6, 'gh', [ + 'issue', + 'create', + '--repo', + 'cool-company/repository', + '--title', + 'test title - @microsoft/cxe-coastal', + '--body', + stripIndents`🚧 This is an auto-generated issue to individually track migration progress. + + ### Packages to migrate: + - react-slider`, + ]); // epic edit to add sub-issues - expect(execSyncMock).toHaveBeenNthCalledWith( - 7, - stripIndents`gh issue edit epicUrl --body "*Description to be added* + expect(execFileSyncMock).toHaveBeenNthCalledWith(7, 'gh', [ + 'issue', + 'edit', + 'epicUrl', + '--body', + stripIndents`*Description to be added* + + ### Packages that need migration: + - [ ] issueUrl-@microsoft/cxe-red + - react-link + - react-button + - [ ] issueUrl-@microsoft/cxe-coastal + - react-card + - [ ] issueUrl-@microsoft/cxe-prg + - react-menu + - react-accordion + - [ ] issueUrl-@microsoft/teams-prg + - misterious-unowned-package + - [ ] epicUrl + - react-slider`, + ]); + }); - ### Packages that need migration: - - [ ] issueUrl-@microsoft/cxe-red - - react-link - - react-button - - [ ] issueUrl-@microsoft/cxe-coastal - - react-card - - [ ] issueUrl-@microsoft/cxe-prg - - react-menu - - react-accordion - - [ ] issueUrl-@microsoft/teams-prg - - misterious-unowned-package - - [ ] epicUrl - - react-slider"`, - ); + it('passes a shell-like title as a single argument', () => { + const tree = setupTest([]); + const title = 'Create epic"; malicious-command; #'; + + epicGenerator(tree, { title, repository: 'microsoft/fluentui' })(); + + expect(execFileSyncMock).toHaveBeenNthCalledWith(1, 'gh', [ + 'issue', + 'create', + '--repo', + 'microsoft/fluentui', + '--title', + title, + '--body', + '*Description to be added*', + ]); }); }); }); diff --git a/tools/workspace-plugin/src/generators/epic-generator/index.ts b/tools/workspace-plugin/src/generators/epic-generator/index.ts index f689f17f0c5529..f505feb14f3ffd 100644 --- a/tools/workspace-plugin/src/generators/epic-generator/index.ts +++ b/tools/workspace-plugin/src/generators/epic-generator/index.ts @@ -1,12 +1,13 @@ import { getProjects, stripIndents, Tree } from '@nx/devkit'; -import { execSync, spawnSync } from 'child_process'; +import { execFileSync, spawnSync } from 'child_process'; import { EpicGenerator } from './schema'; import { isPackageConverged, workspacePaths } from '../../utils'; const placeholderMessage = '*Description to be added*'; +const repositoryNamePattern = /^[A-Za-z0-9](?:[A-Za-z0-9-]{0,37}[A-Za-z0-9])?\/[A-Za-z0-9._-]+$/; function validateSchema(schema: EpicGenerator): Required { - if (schema.repository !== undefined && !schema.repository.match(/[A-z-]+\/[A-z-]+/)) { + if (schema.repository !== undefined && !repositoryNamePattern.test(schema.repository)) { throw new Error(stripIndents` You provided "${schema.repository}", which is an invalid repository name. Please follow the format {owner}/{repositoryName}. @@ -108,7 +109,16 @@ function getPackages(tree: Tree) { } function createEpic(repo: string, title: string) { - const issueUrl = execSync(`gh issue create --repo "${repo}" --title "${title}" --body "${placeholderMessage}"`) + const issueUrl = execFileSync('gh', [ + 'issue', + 'create', + '--repo', + repo, + '--title', + title, + '--body', + placeholderMessage, + ]) .toString() .trim(); @@ -124,9 +134,9 @@ function createIssue(repo: string, issue: MigrationIssue, templateTitle: string) ${issue.packages.map(pkg => `- ${pkg.name}`).join('\n')} `; - const command = `gh issue create --repo "${repo}" --title "${title}" --body "${message}"`; - - const issueUrl = execSync(command).toString().trim(); + const issueUrl = execFileSync('gh', ['issue', 'create', '--repo', repo, '--title', title, '--body', message]) + .toString() + .trim(); return issueUrl; } @@ -173,9 +183,7 @@ function updateEpicWithIssues(epicUrl: string, issueMap: MigrationIssues) { ${packageList} `; - const command = `gh issue edit ${epicUrl} --body "${updatedMessage}"`; - - execSync(command); + execFileSync('gh', ['issue', 'edit', epicUrl, '--body', updatedMessage]); } export default function (tree: Tree, schema: EpicGenerator) { From 10625a156aac2d5e6bdf092dd9bd0147f01939b0 Mon Sep 17 00:00:00 2001 From: PaulGMardling Date: Wed, 9 Sep 2026 10:59:00 +0200 Subject: [PATCH 2/2] fix(workspace-plugin): support EMU repository owners Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../generators/epic-generator/index.spec.ts | 60 +++++++++++++++++-- .../src/generators/epic-generator/index.ts | 2 +- 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/tools/workspace-plugin/src/generators/epic-generator/index.spec.ts b/tools/workspace-plugin/src/generators/epic-generator/index.spec.ts index b328d7f75bc596..1e609ddd1dd3a9 100644 --- a/tools/workspace-plugin/src/generators/epic-generator/index.spec.ts +++ b/tools/workspace-plugin/src/generators/epic-generator/index.spec.ts @@ -1,11 +1,12 @@ import { addProjectConfiguration, ProjectType, stripIndents, writeJson } from '@nx/devkit'; import { createTreeWithEmptyWorkspace } from '@nx/devkit/testing'; -import { execFileSync, spawnSync, SpawnSyncReturns } from 'child_process'; +import { execFileSync, execSync, spawnSync, SpawnSyncReturns } from 'child_process'; import { workspacePaths } from '../../utils'; import epicGenerator from './index'; jest.mock('child_process'); const execFileSyncMock = execFileSync as unknown as jest.Mock; +const execSyncMock = execSync as unknown as jest.Mock; const spawnSyncMock = spawnSync as unknown as jest.Mock>>; type Package = { @@ -112,6 +113,24 @@ describe('epic-generator', () => { expect(() => epicGenerator(tree, { title: 'test title', repository })).toThrow(/invalid repository name/); }, ); + + it('accepts an Enterprise Managed User repository owner', () => { + const tree = setupTest([]); + const repository = 'mona-cat_octo/migration-tracker'; + + epicGenerator(tree, { title: 'test title', repository })(); + + expect(execFileSyncMock).toHaveBeenNthCalledWith(1, 'gh', [ + 'issue', + 'create', + '--repo', + repository, + '--title', + 'test title', + '--body', + '*Description to be added*', + ]); + }); }); describe('authentication', () => { @@ -321,12 +340,20 @@ describe('epic-generator', () => { ]); }); - it('passes a shell-like title as a single argument', () => { - const tree = setupTest([]); - const title = 'Create epic"; malicious-command; #'; + it('passes special characters as literal arguments for all issue commands', () => { + const tree = setupTest([ + { + name: 'react-button', + version: '9.0.0', + projectType: 'library', + owners: ['@microsoft/cxe-red'], + }, + ]); + const title = 'Create "migration" for `react-button` 🚧'; epicGenerator(tree, { title, repository: 'microsoft/fluentui' })(); + expect(execFileSyncMock).toHaveBeenCalledTimes(3); expect(execFileSyncMock).toHaveBeenNthCalledWith(1, 'gh', [ 'issue', 'create', @@ -337,6 +364,31 @@ describe('epic-generator', () => { '--body', '*Description to be added*', ]); + expect(execFileSyncMock).toHaveBeenNthCalledWith(2, 'gh', [ + 'issue', + 'create', + '--repo', + 'microsoft/fluentui', + '--title', + `${title} - @microsoft/cxe-red`, + '--body', + stripIndents`🚧 This is an auto-generated issue to individually track migration progress. + + ### Packages to migrate: + - react-button`, + ]); + expect(execFileSyncMock).toHaveBeenNthCalledWith(3, 'gh', [ + 'issue', + 'edit', + 'epicUrl', + '--body', + stripIndents`*Description to be added* + + ### Packages that need migration: + - [ ] issueUrl-@microsoft/cxe-red + - react-button`, + ]); + expect(execSyncMock).not.toHaveBeenCalled(); }); }); }); diff --git a/tools/workspace-plugin/src/generators/epic-generator/index.ts b/tools/workspace-plugin/src/generators/epic-generator/index.ts index f505feb14f3ffd..ff14736371c0f6 100644 --- a/tools/workspace-plugin/src/generators/epic-generator/index.ts +++ b/tools/workspace-plugin/src/generators/epic-generator/index.ts @@ -4,7 +4,7 @@ import { EpicGenerator } from './schema'; import { isPackageConverged, workspacePaths } from '../../utils'; const placeholderMessage = '*Description to be added*'; -const repositoryNamePattern = /^[A-Za-z0-9](?:[A-Za-z0-9-]{0,37}[A-Za-z0-9])?\/[A-Za-z0-9._-]+$/; +const repositoryNamePattern = /^[A-Za-z0-9](?:[A-Za-z0-9_-]{0,37}[A-Za-z0-9])?\/[A-Za-z0-9._-]+$/; function validateSchema(schema: EpicGenerator): Required { if (schema.repository !== undefined && !repositoryNamePattern.test(schema.repository)) {