Harden command argument validation for pip/uv checks and npm lockfile execution context by Copilot · Pull Request #53957 · github/gh-aw

Copilot · 2026-08-19T10:19:08Z

Sighthound flagged multiple exec.Command call sites as potential command-injection paths where arguments may originate from workflow/frontmatter data. This change tightens validation at argument/context boundaries so dynamic values are rejected before process execution.

Pip/uv command argument boundary hardening
- Added validatePipCommandPackageArg in pkg/workflow/pip_validation.go.
- Applied it before pip command invocation to enforce:
  - no leading -
  - no control characters
  - PyPI/PEP-508 package-name allowlist (validatePipPackageName)
npm lockfile generation execution context hardening
- Updated generatePackageLock in pkg/workflow/dependabot_manifests.go to normalize and validate workflowDir before setting cmd.Dir.
- Enforces absolute/safe path resolution and verifies the target is an existing directory.
Focused security regression coverage
- Added tests for pip command-arg validation cases in pkg/workflow/argument_injection_test.go (valid package, hyphen-prefix, control chars, whitespace, separator patterns).
- Added invalid-workflow-dir rejection test in pkg/workflow/dependabot_test.go.

funcvalidatePipCommandPackageArg(pkgNamestring) error {
ifstrings.HasPrefix(pkgName, "-") {
returnerrors.New("names must not start with '-'")
}
ifstrings.IndexFunc(pkgName, unicode.IsControl) >=0 {
returnerrors.New("names must not contain control characters")
}
returnvalidatePipPackageName(pkgName) // PEP-508 allowlist
}

Fixes[sighthound] Security findings in github/gh-aw #53913

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 10.8 AIC · ⌖ 8.39 AIC · ⊞ 9.3K · ◷
_{Comment /souschef to run again}

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2026-08-19T10:42:39Z

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

github-actions · 2026-08-19T10:42:40Z

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff

🧪 Test quality analysis by Test Quality Sentinel

github-actions · 2026-08-19T10:42:41Z

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

github-actions · 2026-08-19T10:42:50Z

✅ Ponytail Reviewer completed successfully!

Reviewed PR #53957 diff for over-engineering (ponytail-review). Changes are minimal, focused security hardening: one small validation helper (validatePipCommandPackageArg) consolidating existing checks, and path normalization/stat checks before exec.Command. No speculative abstractions, dead code, reinvented stdlib, or unnecessary flexibility found. Lean already. Ship.

Generated by Ponytail Reviewer for #53957

github-actions · 2026-08-19T10:42:58Z

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR does not have the 'implementation' label and has ≪100 new lines of code in business logic directories (69 additions across 4 files, threshold is 100).

🏗️ ADR gate enforced by Design Decision Gate 🏗️

github-actions · 2026-08-19T10:46:50Z

Comment Memory

reviewed_at: 2026-08-19T10:44:33Z
review_event: REQUEST_CHANGES
top_themes:
- inconsistent pip vs uv command-argument validation
- missing positive regression coverage for absolute workflowDir normalization
files_reviewed:
- pkg/workflow/argument_injection_test.go
- pkg/workflow/dependabot_manifests.go
- pkg/workflow/dependabot_test.go
- pkg/workflow/pip_validation.go
comment_count: 2

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 9.28 AIC · ⌖ 9.08 AIC · ⊞ 7K · ◷
_{Comment /review to run again}

github-actions

Requesting changes

This hardening pass is pointed in the right direction, but it still leaves one real gap and misses a regression test for the new npm directory validation.

The blocking themes

validatePipCommandPackageArg is only wired into the warning-only pip validation path; the uv validation path still uses the older helper directly, so control-character rejection is inconsistent across two command-invocation paths.
The new generatePackageLock path validation only has a rejection test; it does not prove the happy path still runs from the normalized absolute directory, which is the actual behavior this patch changed.

🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 9.28 AIC · ⌖ 9.08 AIC · ⊞ 7K
_{Comment /review to run again}

Copilot

Pull request overview

Hardens package-validation commands and npm lockfile generation against unsafe dynamic inputs.

Changes:

Adds strict pip package-argument validation.
Validates npm’s working directory before execution.
Adds focused security regression tests.

Show a summary per file

File	Description
`pkg/workflow/pip_validation.go`	Validates pip command arguments.
`pkg/workflow/dependabot_manifests.go`	Normalizes and checks npm’s working directory.
`pkg/workflow/argument_injection_test.go`	Tests unsafe pip arguments.
`pkg/workflow/dependabot_test.go`	Tests invalid workflow-directory rejection.

Review details

Files reviewed: 4/4 changed files
Comments generated: 1
Review effort level: Balanced

github-actions

Skills-Based Review 🧠

Applied /diagnosing-bugs and /tdd — requesting changes on one correctness gap and two test-coverage issues.

📋 Key Themes & Highlights

Key Themes

Incomplete guard coverage: validatePipCommandPackageArg (new control-char + hyphen + PEP-508 check) is only wired into the pip-fallback path. The direct uv path still calls validatePipPackageName without the control-character guard, leaving a gap for the same injection class this PR aims to close.
Stale #nosec comment: the G204 annotation in validatePythonPackagesWithPip still names validatePipPackageName instead of the new validatePipCommandPackageArg, which could mislead future security auditors.
Narrow regression tests: TestGeneratePackageLock_RejectsInvalidWorkflowDir tests only a newline; relative traversal (../) and nonexistent-path cases are not covered.

Positive Highlights

✅ The new validatePipCommandPackageArg is a clean, composable guard — centralising three distinct checks in one function is exactly the right design.
✅ The filepath.Abs → ValidateAbsolutePath → os.Stat → IsDir chain in generatePackageLock is thorough and correctly uses the resolved absWorkflowDir in both cmd.Dir and the lockfile path.
✅ The added tests are clearly structured and descriptive; the table-driven style is consistent with the rest of the test suite.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 49.2 AIC · ⌖ 13.2 AIC · ⊞ 7.8K
_{Comment /matt to run again}

Comments that could not be inline-anchored

pkg/workflow/pip_validation.go:173

[/diagnosing-bugs] Control-character injection is not blocked on the direct uv execution path. validatePipCommandPackageArg (which adds the unicode.IsControl guard) is only wired into validatePythonPackagesWithPip — the pip-fallback path. When uv is available, validatePipPackageName is still called directly at this line, so a pkgName containing \n or other control characters reaches exec.Command(uvPath, "pip", "show", pkgName) uninspected.

<details>
<summary>💡 Suggested …

pkg/workflow/pip_validation.go:88

[/diagnosing-bugs] The #nosec G204 comment at this line still says "validated above by validatePipPackageName" but the guard has been renamed to validatePipCommandPackageArg. This stale comment could mislead future auditors into thinking only PEP-508 validation is in effect, not the control-character check.

<details>
<summary>💡 Suggested fix</summary>

Update the comment to reflect the actual guard function name:

// #nosec G204 -- pipPath is resolved by fileutil.ResolveExecut…</details><details><summary>pkg/workflow/dependabot_test.go:714</summary>**[/tdd]**The`TestGeneratePackageLock_RejectsInvalidWorkflowDir`testonlycoversacontrol-characterinthepath (`&quot;bad\nworkflow-dir&quot;`). Apath-traversalinputsuchas`&quot;/tmp/../etc/passwd&quot;`orarelativepath (`&quot;../secrets&quot;`) wouldexercisethe`ValidateAbsolutePath`and`filepath.Abs`guardsaddedforthe`cmd.Dir` hardening. Withoutthesecasesafuturerefactorthataccidentallydropstheabsolute-pathcheckwouldnotbecaught.
&lt;details&gt;
&lt;summary&gt;💡 Suggestedadditionaltestcases&lt;/su…
</details>

github-actions

The hardening changes are correct and well-tested.

validatePipCommandPackageArg cleanly layers hyphen-prefix and control-character rejection before the existing PEP 508 regex, covering injection vectors not previously guarded.
generatePackageLock now resolves, validates, and stat-checks the working directory before passing it to exec.Command, eliminating the unvalidated path risk.

Test coverage is appropriate for both additions. No blocking issues.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 24 AIC · ⌖ 8.8 AIC · ⊞ 5.7K

gh-aw-bot · 2026-08-19T11:08:35Z

@copilot Please address the latest maintainer-visible blockers on this PR, refresh the branch if needed, and then run the pr-finisher skill before handing back for maintainer review.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 25 AIC · ⌖ 8.11 AIC · ⊞ 9.3K · ◷
_{Comment /souschef to run again}

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot · 2026-08-19T11:35:14Z

@copilot Please address the latest maintainer-visible blockers on this PR, refresh the branch if needed, and then run the pr-finisher skill before handing back for maintainer review.
Generated by...

Addressed the maintainer-visible blockers in d782d83 and ran the pr-finisher flow locally. CI will need a maintainer re-trigger because agent pushes do not start new workflow runs.

github-actions · 2026-08-19T12:26:24Z

PR Triage

Category: bug
Risk: high
Priority score: 72/100 (impact 35 + urgency 20 + quality 17)
Recommended action: fast_track

Security hardening: tightens command-injection validation at exec.Command argument boundaries for pip/uv and npm lockfile execution.

Generated by 🔧 PR Triage Agent · auto · 44.3 AIC · ⌖ 2.78 AIC · ⊞ 8.3K · ◷

gh-aw-bot · 2026-08-19T13:36:53Z

@copilot This PR has reviewer follow-up that appears addressed in thread replies, but the threads are still unresolved. Please verify the implemented fixes, resolve the open threads if the feedback is satisfied, and then run the pr-finisher skill.

Open review threads (newest first):

copilot-pull-request-reviewer: reject empty workflowDir before filepath.Abs — replied at Harden command argument validation for pip/uv checks and npm lockfile execution context #53957 (comment), original thread Harden command argument validation for pip/uv checks and npm lockfile execution context #53957 (comment)
github-actions: add a success-path normalization test for generatePackageLock — replied at Harden command argument validation for pip/uv checks and npm lockfile execution context #53957 (comment), original thread Harden command argument validation for pip/uv checks and npm lockfile execution context #53957 (comment)
github-actions: reuse pip package-arg validation in the uv path too — replied at Harden command argument validation for pip/uv checks and npm lockfile execution context #53957 (comment), original thread Harden command argument validation for pip/uv checks and npm lockfile execution context #53957 (comment)

Also attempt a branch refresh if GitHub offers one.

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 10.8 AIC · ⌖ 8.39 AIC · ⊞ 9.3K · ◷
_{Comment /souschef to run again}

Copilot · 2026-08-19T14:08:07Z

@copilot This PR has reviewer follow-up that appears addressed in thread replies, but the threads are still unresolved. Please verify the implemented fixes, resolve the open threads if the feedback is...

Verified the fixes and review state: all three referenced review threads are now resolved. The branch is refreshed against current main via merge commit 8f56e1c. Ran the pr-finisher flow locally; targeted workflow tests, make test-unit-all, make test, and make recompile pass. make lint still reports unrelated pre-existing pkg/intent/governance_formal_test.go testifylint issues on files outside this PR’s diff.

github-actions · 2026-08-20T22:03:29Z

🎉 This pull request is included in a new release.

Release: v0.87.2

Initial plan
b868fbe

CopilotAI assigned Copilot and pelikhan Aug 19, 2026

Copilotstarted work on behalf of pelikhan August 19, 2026 10:19 View session

CopilotAI linked an issue Aug 19, 2026 that may be closed by this pull request
[sighthound] Security findings in github/gh-aw #53913
Closed

CopilotAIand others added 2 commits August 19, 2026 10:33

Harden package argument and workflow-dir validation
160a570
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Refine pip package arg validation tests
ef52f20
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

CopilotAI changed the title ~~[WIP] Fix security findings in github/gh-aw~~Harden command argument validation for pip/uv checks and npm lockfile execution contextAug 19, 2026

Copilotfinished work on behalf of pelikhan August 19, 2026 10:39

CopilotAI requested a review from pelikhan August 19, 2026 10:39

pelikhan marked this pull request as ready for review August 19, 2026 10:41

CopilotAI balanced review requested due to automatic review settings August 19, 2026 10:41

Copilotstarted reviewing on behalf of pelikhan August 19, 2026 10:41 View session

github-actionsBot requested changes Aug 19, 2026
View reviewed changes

Comment threadpkg/workflow/pip_validation.go
Comment threadpkg/workflow/dependabot_test.go

github-actionsBot mentioned this pull request Aug 19, 2026
[aw] No-Op Runs #48838
Open

CopilotAI reviewed Aug 19, 2026
View reviewed changes

Comment threadpkg/workflow/dependabot_manifests.go

github-actionsBot requested changes Aug 19, 2026
View reviewed changes

github-actionsBot approved these changes Aug 19, 2026
View reviewed changes

github-actionsBot mentioned this pull request Aug 19, 2026
[pr-sous-chef] pr-sous-chef run report #53885
Closed

Copilotstarted work on behalf of gh-aw-bot August 19, 2026 11:08 View session

Address workflow dir review feedback
d782d83
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilotfinished work on behalf of gh-aw-bot August 19, 2026 11:36

CopilotAI requested a review from gh-aw-bot August 19, 2026 11:36

github-actionsBot added pr-action:fast_track pr-agent:copilot pr-priority:high pr-risk:high pr-type:bug labels Aug 19, 2026

github-actionsBot mentioned this pull request Aug 19, 2026
[PR Triage Report] PR Triage Report — 2026-08-19 (Run 32251836165) #53992
Closed

Merge branch 'main' into copilot/sighthound-fix-security-findings
8f56e1c

Copilotstarted work on behalf of gh-aw-bot August 19, 2026 13:39 View session

Copilotfinished work on behalf of gh-aw-bot August 19, 2026 14:08

pelikhan merged commit 22266da into mainAug 19, 2026
25 checks passed

pelikhan deleted the copilot/sighthound-fix-security-findings branch August 19, 2026 14:45

github-actionsBot mentioned this pull request Aug 20, 2026
Smoke Test: MAI-Code-1-Flash - 32375451921 #54247
Closed

Uh oh!

Conversation

CopilotAI commented Aug 19, 2026• edited by github-actions BotLoading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actionsBot commented Aug 19, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actionsBot commented Aug 19, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actionsBot commented Aug 19, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actionsBot commented Aug 19, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actionsBot commented Aug 19, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actionsBot commented Aug 19, 2026

Comment Memory

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Requesting changes

Uh oh!

Uh oh!

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Key Themes

Positive Highlights

Comments that could not be inline-anchored

Uh oh!

github-actionsBot left a comment

Choose a reason for hiding this comment

Uh oh!

gh-aw-bot commented Aug 19, 2026

Uh oh!

CopilotAI commented Aug 19, 2026

Uh oh!

github-actionsBot commented Aug 19, 2026

PR Triage

Uh oh!

gh-aw-bot commented Aug 19, 2026

Uh oh!

CopilotAI commented Aug 19, 2026

Uh oh!

Uh oh!

github-actionsBot commented Aug 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CopilotAI commented Aug 19, 2026•
edited by github-actions Bot
Loading

github-actionsBot commented Aug 19, 2026•
edited
Loading

github-actionsBot commented Aug 19, 2026•
edited
Loading

github-actionsBot commented Aug 19, 2026•
edited
Loading

github-actionsBot commented Aug 19, 2026•
edited
Loading

github-actionsBot commented Aug 19, 2026•
edited
Loading