Skip to content

fix(workspace-plugin): epic generator command injection #36702

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Paul Mardling (PaulGMardling) wants to merge 2 commits into
base: master
Choose a base branch
from
+197 −71
Open

fix(workspace-plugin): epic generator command injection #36702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ccfbf21
fix(workspace-plugin): prevent epic generator command injection
PaulGMardling Sep 7, 2026
10625a1
fix(workspace-plugin): support EMU repository owners
PaulGMardling Sep 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
242 changes: 180 additions & 62 deletions tools/workspace-plugin/src/generators/epic-generator/index.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +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, 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<string>;
const execSyncMock = execSync as unknown as jest.Mock<string>;
const spawnSyncMock = spawnSync as unknown as jest.Mock<Partial<SpawnSyncReturns<string[]>>>;

Expand Down Expand Up @@ -49,7 +50,7 @@ function setupTest(packages: Package[]) {
});

// response to epic creation
execSyncMock.mockReturnValueOnce('epicUrl');
execFileSyncMock.mockReturnValueOnce('epicUrl');

/**
* Responses for each of the packages created
Expand All @@ -68,11 +69,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;
}
Expand Down Expand Up @@ -103,6 +104,33 @@ 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/);
},
);

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', () => {
Expand Down Expand Up @@ -204,73 +232,163 @@ 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 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',
'--repo',
'microsoft/fluentui',
'--title',
title,
'--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();
});
});
});
26 changes: 17 additions & 9 deletions tools/workspace-plugin/src/generators/epic-generator/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<EpicGenerator> {
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}.
Expand Down Expand Up @@ -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();

Expand All @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand Down
Loading