Harden project GraphQL lookups by replacing interpolated owner inputs with variableized requests - #51143
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Hey What looks great:
The fix is aligned with the contributing guidelines for agentic development and follows the project's security-first approach. The changes are minimal and targeted — exactly what a security remediation should look like. Once you're ready to exit draft status and complete the final validation steps (
|
There was a problem hiding this comment.
Pull request overview
Hardens project GraphQL lookups by sending user-controlled values as JSON variables instead of CLI field arguments.
Changes:
- Adds a shared variableized GraphQL request helper.
- Migrates owner and status-field lookups.
- Updates tests to verify query separation, variable types, and CLI arguments.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/project_command.go |
Uses JSON GraphQL variables for project lookups. |
pkg/cli/project_command_test.go |
Validates request bodies and invocation arguments. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
|
@copilot Please triage this PR, refresh the branch if possible, and run the Failed checks to inspect:
Run context: https://github.com/github/gh-aw/actions/runs/31204567664
|
Triage ResultCategory: bug (security hardening) · Risk: medium · Priority: high (score 74/100 — impact 30, urgency 26, quality 18) Notes: Addresses a code-scanning GraphQL injection finding by parameterizing queries — security-relevant, prioritize review. CI:
|
|
@copilot Please triage this PR, refresh the branch if GitHub can update it cleanly, inspect the failing checks below, and run the Failed checks to inspect:
This security hardening change appears blocked by the same D-L failure tracked by #51151. Please confirm whether this branch only needs a refresh/retest after that fix lands, or whether additional branch-local changes are needed. Run context: https://github.com/github/gh-aw/actions/runs/31206913072
|
|
🎉 This pull request is included in a new release. Release: |
Code scanning flagged
pkg/cli/project_command.gofor GraphQL injection risk: owner-scoped queries were assembled through CLI field interpolation, relying on manual escaping. This change moves those paths to parameterized GraphQL variables carried in JSON request bodies.Owner lookup/query execution path
{query, variables}viagh api graphql --input -.validateOwner,getOwnerNodeId, andgetStatusFieldto use variableized payloads for user-controlled values (login,number) instead of inline/interpolated arguments.Security posture improvement
variables, not embedded into GraphQL text.Focused test updates
$login/$numbervariables--input -with expected--jqbehavior.Run context: https://github.com/github/gh-aw/actions/runs/31206913072> Generated by 👨🍳 PR Sous Chef · gpt54 · 13 AIC · ⊞ 8.4K · ◷