[code-scanning-fix] Fix GraphQL injection in getOwnerNodeId (alerts #651 and #652) by github-actions[bot] · Pull Request #47952 · github/gh-aw

github-actions · 2026-07-25T07:39:56Z

Summary

Fixes GraphQL injection vulnerabilities in getOwnerNodeId (code scanning alerts #651 and #652). The owner login value was previously interpolated directly into GraphQL query strings using fmt.Sprintf, bypassing the escapeGraphQLString helper in some edge cases and still remaining vulnerable to injection. This PR replaces string interpolation with parameterized GraphQL variables, passing the login value via -f login=<owner> so gh api graphql transmits it as a typed String! variable.

Changes

pkg/cli/project_command.go

Replaced fmt.Sprintf-based query construction in getOwnerNodeId with static parameterized queries using $login: String!.
Switched from workflow.RunGH to projectCommandRunGH and added -f login=<owner> as an explicit variable argument.
Removed the escapeGraphQLString call — no longer needed since the value is passed out-of-band as a variable.

pkg/cli/project_command_test.go

Added TestGetOwnerNodeIdUsesStringLoginField covering org and user owner types.
Test cases include edge-case login values (false, null, special characters) that would previously risk injection or type coercion.
Asserts that both query= and login= args are passed with -f, and that the correct --jq path is forwarded.

Security Impact

Previously, a crafted owner value could escape the query string and inject arbitrary GraphQL. With parameterized variables, the login is always transmitted as a string and never interpreted as query syntax.

Testing

New unit tests in TestGetOwnerNodeIdUsesStringLoginField verify the parameterized variable path for both org and user owner types, including special-character inputs.

Generated by PR Description Updater for #47952 · sonnet46 · 26.3 AIC · ⌖ 7.42 AIC · ⊞ 4.8K · ◷

Replace fmt.Sprintf string interpolation with GraphQL query variables in getOwnerNodeId() to eliminate GraphQL injection risk. The owner login is now passed as a variable (-f login=<owner>) instead of being embedded directly into the query string. This matches the pattern already used in validateOwner() and eliminates alerts #651 and #652 (workflow-go-graphql-injection-sprintf). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-07-25T07:50:19Z

✅ 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 (only 3 new lines detected in default business directories).

github-actions · 2026-07-25T07:50:29Z

✅ PR Code Quality Reviewer completed the code quality review.

github-actions · 2026-07-25T07:50:33Z

✅ Test Quality Sentinel completed test quality analysis.

No test files were added or modified in this PR. Test Quality Sentinel skipped.

github-actions · 2026-07-25T07:50:33Z

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

Copilot

Pull request overview

Replaces unsafe GraphQL interpolation in owner ID lookup with query variables.

Changes:

Uses static parameterized queries for organization and user lookups.
Passes owner login via gh api graphql -f.

Show a summary per file

File	Description
`pkg/cli/project_command.go`	Parameterizes owner ID GraphQL queries.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

 	}

-	output, err := workflow.RunGH("Getting owner ID...", "api", "graphql", "-f", "query="+query, "--jq", jqPath)
+	output, err := workflow.RunGH("Getting owner ID...", "api", "graphql", "-f", "query="+query, "-f", "login="+owner, "--jq", jqPath)


github-actions

Security Fix Review

The fix correctly replaces brittle fmt.Sprintf string interpolation with parameterized GraphQL variables, eliminating the injection risk flagged by alerts #651 and #652.

What was changed:

Both organization and user queries now use query($login: String!) { ... } with a $login variable
owner is passed safely via -f login=<owner> to gh api graphql
This matches the pattern already used in validateOwner() in the same file

Assessment: The fix is correct, minimal, and consistent with existing safe patterns in the codebase. No blocking issues found.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 11.5 AIC · ⌖ 4.52 AIC · ⊞ 5K

github-actions

Approved

Clean, minimal security fix. Replacing fmt.Sprintf + escapeGraphQLString with parameterized GraphQL variables and -f login=owner is the correct approach — it eliminates the injection class entirely. The fix matches the pattern already used in validateOwner() in the same file.

🔎 Code quality review by PR Code Quality Reviewer · sonnet46 · 10.8 AIC · ⌖ 4.49 AIC · ⊞ 5.7K
_{Comment /review to run again}

github-actions

Skills-Based Review 🧠

Applied /diagnosing-bugs — the fix is correct and consistent with existing patterns; one comment on test coverage.

📋 Key Themes & Highlights

Key Themes

Correct fix: replacing fmt.Sprintf + escapeGraphQLString with GraphQL variables (-f login=...) is the right approach and matches validateOwner() on line 241.
Missing regression test: no test was added to lock in the parameterized pattern for special-character owner names.

Positive Highlights

✅ Fix is minimal, surgical, and consistent with the existing codebase pattern
✅ Both org and user branches fixed symmetrically
✅ escapeGraphQLString dead code is removed implicitly (the brittle manual escaping is no longer needed for this path)

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

github-actions · 2026-07-25T07:55:48Z

 	}

-	output, err := workflow.RunGH("Getting owner ID...", "api", "graphql", "-f", "query="+query, "--jq", jqPath)
+	output, err := workflow.RunGH("Getting owner ID...", "api", "graphql", "-f", "query="+query, "-f", "login="+owner, "--jq", jqPath)


[/diagnosing-bugs] No regression test covers getOwnerNodeId with special-character owner names — the exact scenario this injection fix guards against.

Without a test, a future refactor could silently reintroduce string interpolation and the code scanner would be the only safety net.

💡 Suggested test skeleton

func TestGetOwnerNodeId_SpecialChars(t *testing.T) { // Swap projectCommandRunGH with a mock that captures args. // Assert query contains "\$login" (not a literal owner string), // and that "-f", "login=<special-char-owner>" appears in args. }

This locks in the parameterized-variable pattern and prevents regression.

@copilot please address this.

…25bccd7f4885a0e3

gh-aw-bot · 2026-07-25T08:14:07Z

@copilot please run the pr-finisher skill, address any unresolved review feedback, fix any failing checks listed below, and rerun checks once the branch is up to date.

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

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

Copilot · 2026-07-25T08:28:24Z

@copilot please run the pr-finisher skill, address any unresolved review feedback, fix any failing checks listed below, and rerun checks once the branch is up to date.

...

Addressed the unresolved review feedback in commit b99be13 by routing getOwnerNodeId through projectCommandRunGH and adding regression tests for org/user $login query usage with -f (including false, null, and special-character owner values). I also ran local validation (make fmt, make lint, TEST_UNIT_RUN_FULL=1 make test-unit, make test) and parallel validation (Code Review + CodeQL). I can’t re-run GitHub checks from the cloud agent; CI on this new HEAD needs a maintainer re-trigger (close/reopen PR, workflow_dispatch, or a maintainer push).

github-actions · 2026-07-25T11:49:45Z

🎉 This pull request is included in a new release.

Release: v0.83.3

github-actions Bot added agentic-campaign automated-fix security z_campaign_security-alert-burndown labels Jul 25, 2026

pelikhan marked this pull request as ready for review July 25, 2026 07:49

Copilot AI review requested due to automatic review settings July 25, 2026 07:49

Copilot started reviewing on behalf of pelikhan July 25, 2026 07:49 View session

Copilot AI reviewed Jul 25, 2026

View reviewed changes

Comment thread pkg/cli/project_command.go Outdated

}

output, err := workflow.RunGH("Getting owner ID...", "api", "graphql", "-f", "query="+query, "--jq", jqPath)

output, err := workflow.RunGH("Getting owner ID...", "api", "graphql", "-f", "query="+query, "-f", "login="+owner, "--jq", jqPath)

github-actions Bot commented Jul 25, 2026

View reviewed changes

Merge branch 'main' into fix/code-scanning-651-652-graphql-injection-…

51c9ee7

…25bccd7f4885a0e3

github-actions Bot mentioned this pull request Jul 25, 2026

[pr-sous-chef] PR Sous Chef run report - 2026-07-25T00:16:35Z #47879

Closed

Copilot started work on behalf of gh-aw-bot July 25, 2026 08:14 View session

test: cover getOwnerNodeId GraphQL variable path

b99be13

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

Copilot finished work on behalf of gh-aw-bot July 25, 2026 08:29

Copilot AI requested a review from gh-aw-bot July 25, 2026 08:29

pelikhan merged commit 51939c8 into main Jul 25, 2026
23 checks passed

pelikhan deleted the fix/code-scanning-651-652-graphql-injection-25bccd7f4885a0e3 branch July 25, 2026 08:37

github-actions Bot mentioned this pull request Jul 25, 2026

Smoke Test: Copilot - 30151690498 #47964

Closed

github-actions Bot mentioned this pull request Jul 25, 2026

Smoke Test: MAI-Code-1-Flash - 30162378433 #48022

Closed

Uh oh!

Conversation

github-actions Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Security Impact

Testing

Uh oh!

github-actions Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Security Fix Review

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Approved

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Key Themes

Positive Highlights

Uh oh!

github-actions Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

gh-aw-bot commented Jul 25, 2026

Uh oh!

Copilot AI commented Jul 25, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jul 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Jul 25, 2026 •

edited

Loading

github-actions Bot commented Jul 25, 2026 •

edited

Loading

github-actions Bot commented Jul 25, 2026 •

edited

Loading

github-actions Bot commented Jul 25, 2026 •

edited

Loading

github-actions Bot commented Jul 25, 2026 •

edited

Loading