Uh oh!
There was an error while loading. Please reload this page.
fix: reject package/image names starting with '-' to prevent argument injection (#23070) - #23045
Merged
Merged
Conversation
… injection Adds a prefix check for '-' before passing package names and Docker image names to exec.Command in npm_validation.go, pip_validation.go, and docker_validation.go. A name beginning with '-' would be interpreted as a flag by npm, pip, uv, or docker, causing unintended argument injection. Since exec.Command uses argv directly (not sh -c), this is not shell injection but argument injection. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6ad34640-b523-4027-bdfc-4ce8e067840e Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/6ad34640-b523-4027-bdfc-4ce8e067840e Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilotcreated this pull request from a session on behalf of pelikhanMarch 26, 2026 05:07 View session
pelikhan
approved these changes
Mar 26, 2026
pelikhan
commented
Mar 26, 2026
Collaborator
@copilot Add tests |
- Add name_validation.go with shared rejectHyphenPrefixPackages helper - Refactor npm and uv validators to use the shared helper - Move the name guard before LookPath in both validators (fail-fast, testable) - Add argument_injection_test.go with 12 unit tests covering: - rejectHyphenPrefixPackages (6 cases) - validateDockerImage with hyphen-prefix image names (3 cases) - extractor filtering of hyphen-prefix names for npx/pip (2+2 cases) - collectPackagesFromWorkflow args filtering (1 case) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c33112e5-6785-4175-8878-cec9c4ced1e0 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
ContributorAuthor
Added 12 unit tests in
All tests are |
pelikhan
marked this pull request as ready for review
March 26, 2026 05:33
Uh oh!
There was an error while loading. Please reload this page.
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents argument injection into external CLIs (npm/pip/uv/docker) during workflow compilation by rejecting package/image names that start with -.
Changes:
- Added shared validation helper to reject
--prefixed package names before any CLI invocation. - Enforced
--prefix rejection in npx/uv (hard error) and pip (warning + skip) validation paths. - Added unit tests covering the helper, Docker image rejection, and extractor defense-in-depth behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/name_validation.go | Introduces shared helper to reject --prefixed package names with a ValidationError. |
| pkg/workflow/npm_validation.go | Fail-fast rejects --prefixed npx packages before calling npm. |
| pkg/workflow/pip_validation.go | Skips/warns for --prefixed pip package names; fail-fast rejects --prefixed uv packages before calling uv/pip. |
| pkg/workflow/docker_validation.go | Rejects Docker image names starting with - before invoking docker. |
| pkg/workflow/argument_injection_test.go | Adds unit tests for the helper, Docker validation, and extractor filtering behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
| // rejectHyphenPrefixPackages returns a ValidationError if any of the provided | ||
| // names starts with '-'. The kind parameter (e.g. "npx", "pip", "uv") is used |
There was a problem hiding this comment.
Grammar nit in the doc comment: "names starts with '-'" should be "names start with '-'".
Suggested change
| // names starts with '-'. The kind parameter (e.g. "npx", "pip", "uv") is used | |
| // names start with '-'. The kind parameter (e.g. "npx", "pip", "uv") is used |
This was referenced Mar 26, 2026
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 freeto 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.
Summary
During compilation, the compiler shells out to
npm view,pip index versions,uv pip show,docker image inspect, anddocker pullto validate packages/images referenced in frontmatter MCP configs. A name beginning with-is interpreted as a flag by the downstream tool — this is argument injection (the value lands inargv[2]instead of being parsed by a shell).Severity: Low — runs on the developer's local machine during
gh aw compile, with the developer's own privileges. The developer is already trusting the frontmatter they wrote or reviewed.Changes
pkg/workflow/name_validation.go(new): SharedrejectHyphenPrefixPackageshelper used by npm and uv validators.pkg/workflow/npm_validation.go: Rejects npm package names starting with-with a hard error, checked before invokingnpm(fail-fast even when npm is not installed).pkg/workflow/pip_validation.go:validatePythonPackagesWithPip: Emits a warning and skips (consistent with the existing non-fatal pip validation approach).validateUvPackages: Uses the shared helper to reject names before invokinguvorpip(fail-fast).pkg/workflow/docker_validation.go: Rejects Docker image names starting with-with an immediate error return. This is the primary injection path — image names come directly from frontmatter, not filtered by extractors.Testing
Added
pkg/workflow/argument_injection_test.gowith 12 unit tests (//go:build !integration, no external tools required):TestRejectHyphenPrefixPackages— directly tests the shared helper (6 cases: empty list, valid names, single/double hyphen, mixed list, version-specifier variant).TestValidateDockerImage_RejectsHyphenPrefix— verifies Docker image names starting with-are rejected without needing the Docker daemon (3 cases).TestExtractNpxFromCommands_HyphenPrefixFiltered/TestExtractPipFromCommands_HyphenPrefixFiltered— confirms thePackageExtractoralready treats-prefixed words as flags and filters them before they reach the validators (defense-in-depth).TestCollectPackagesFromWorkflow_HyphenPrefixInArgs— verifies the structuredargsformat also skips-prefixed entries.Pre-existing
TestWasmGolden_CompileFixtures/basic-copilotfailure is unrelated to these changes (also fails onmain).🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.