chore: ban direct child_process and shell:true outside the subprocess tool - #1902
Open
iankhou wants to merge 1 commit into
Open
chore: ban direct child_process and shell:true outside the subprocess tool#1902iankhou wants to merge 1 commit into
iankhou wants to merge 1 commit into
Conversation
iankhou
marked this pull request as ready for review
August 26, 2026 17:46
aws-cdk-automation
enabled auto-merge
August 26, 2026 17:46
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1902 +/- ##
==========================================
- Coverage 91.39% 91.27% -0.12%
==========================================
Files 79 79
Lines 12139 12139
Branches 1728 1720 -8
==========================================
- Hits 11094 11080 -14
- Misses 1010 1024 +14
Partials 35 35
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
iankhou
force-pushed
the
iankhou-ban-direct-child-process
branch
from
August 26, 2026 18:15
c40d964 to
5dd2235
Compare
iankhou
force-pushed
the
iankhou-ban-direct-child-process
branch
from
August 26, 2026 18:26
5dd2235 to
a626b51
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds ESLint safeguards to centralize subprocess execution in the shared utility.
Changes:
- Restricts direct
child_processimports andshell: true. - Adds scoped exemptions for tests and integration tooling.
- Regenerates package ESLint configurations.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
projenrc/eslint/team.ts |
Adds the shell restriction. |
projenrc/eslint/index.ts |
Configures test/build exemptions. |
projenrc/eslint/imports.ts |
Restricts child-process imports. |
.projenrc.ts |
Adds integration-tool exemptions. |
.eslintrc.json |
Regenerates root ESLint rules. |
packages/cdk/.eslintrc.json |
Regenerates package rules. |
packages/cdk-assets/.eslintrc.json |
Regenerates package rules. |
packages/aws-cdk/.eslintrc.json |
Regenerates CLI rules. |
packages/@aws-cdk/yarn-cling/.eslintrc.json |
Regenerates package rules. |
packages/@aws-cdk/user-input-gen/.eslintrc.json |
Regenerates package rules. |
packages/@aws-cdk/toolkit-lib/.eslintrc.json |
Regenerates toolkit rules. |
packages/@aws-cdk/private-tools/lib/subprocess/index.ts |
Exempts sanctioned subprocess operations. |
packages/@aws-cdk/private-tools/.eslintrc.json |
Regenerates private-tools rules. |
packages/@aws-cdk/integ-runner/.eslintrc.json |
Adds integ-runner exemption. |
packages/@aws-cdk/cloudformation-diff/.eslintrc.json |
Regenerates package rules. |
packages/@aws-cdk/cloud-assembly-schema/.eslintrc.json |
Regenerates package rules. |
packages/@aws-cdk/cloud-assembly-api/.eslintrc.json |
Regenerates package rules. |
packages/@aws-cdk/cli-plugin-contract/.eslintrc.json |
Regenerates package rules. |
packages/@aws-cdk/cdk-explorer/.eslintrc.json |
Regenerates package rules. |
packages/@aws-cdk/cdk-assets-lib/.eslintrc.json |
Regenerates package rules. |
packages/@aws-cdk-testing/cli-integ/.eslintrc.json |
Adds CLI integration exemptions. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // The only sanctioned shell entry point is `runUserCommandLine` in the | ||
| // subprocess tool (which carries its own eslint-disable); everything else | ||
| // must spawn an argv array via `run`/`runSync`, which never touch a shell. | ||
| selector: "Property:matches([key.name='shell'], [key.value='shell'])[value.value=true]", |
Comment on lines
+1575
to
+1578
| rules: { | ||
| 'no-restricted-imports': ['off'], | ||
| 'no-restricted-syntax': ['off'], | ||
| }, |
Comment on lines
+27
to
+31
| message: 'Do not use `child_process` directly. Use `run`/`runSync`/`runUserCommandLine` from the subprocess tool (\'./private/tools\').', | ||
| }, | ||
| { | ||
| name: 'node:child_process', | ||
| message: 'Do not use `child_process` directly. Use `run`/`runSync`/`runUserCommandLine` from the subprocess tool (\'./private/tools\').', |
Comment on lines
+1704
to
+1707
| // cli-integ is a test harness that deliberately spawns through a shell (and a | ||
| // pty) to exercise the CLI the way a user would at a terminal — that is the | ||
| // point of these helpers, not something to migrate away. Permanently exempt, | ||
| // like test code. Scoped to the current sinks so NEW files still hit the ban |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1903
Description
Locks in the subprocess consolidation (#1763, #1849) so new code or changes to existing code can't spawn shells outside the shared tool.
child_process/node:child_processshell: truein spawn optionsSpawn paths have to go through
run/runSync/runUserCommandLinefrom the subprocess tool. The tool itself carries a localized eslint-disable at the two sanctioned lines.Test files are exempt (they mock/spy on
child_process).integ-runnerandcli-integhave temporary, tracked per-file overrides pending their migrations.cdk-build-toolsis not managed by the root projen config, so the rule does not reach it yet; will address it in a follow-up.Testing
Inserted a
child_processimport into a migrated lib/ file, and the rule fires as expected. Obviously did not keep that import.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license