From b78e47bb5d44a4a2c29e7437ac8faef8d3e77d07 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 07:39:54 +0000 Subject: [PATCH 1/2] fix: use GraphQL variables in getOwnerNodeId to prevent injection 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=) 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> --- pkg/cli/project_command.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cli/project_command.go b/pkg/cli/project_command.go index 72413ed93d1..eb7c36ade1c 100644 --- a/pkg/cli/project_command.go +++ b/pkg/cli/project_command.go @@ -266,14 +266,14 @@ func getOwnerNodeId(ctx context.Context, ownerType, owner string, verbose bool) var query string var jqPath string if ownerType == "org" { - query = fmt.Sprintf(`query { organization(login: "%s") { id } }`, escapeGraphQLString(owner)) + query = `query($login: String!) { organization(login: $login) { id } }` jqPath = ".data.organization.id" } else { - query = fmt.Sprintf(`query { user(login: "%s") { id } }`, escapeGraphQLString(owner)) + query = `query($login: String!) { user(login: $login) { id } }` jqPath = ".data.user.id" } - 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) if err != nil { return "", fmt.Errorf("failed to get owner node ID: %w", err) } From b99be13dfb467948907098405cb84c053eba1634 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 25 Jul 2026 08:28:12 +0000 Subject: [PATCH 2/2] test: cover getOwnerNodeId GraphQL variable path Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/project_command.go | 2 +- pkg/cli/project_command_test.go | 65 +++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/pkg/cli/project_command.go b/pkg/cli/project_command.go index eb7c36ade1c..606d2c470f6 100644 --- a/pkg/cli/project_command.go +++ b/pkg/cli/project_command.go @@ -273,7 +273,7 @@ func getOwnerNodeId(ctx context.Context, ownerType, owner string, verbose bool) jqPath = ".data.user.id" } - output, err := workflow.RunGH("Getting owner ID...", "api", "graphql", "-f", "query="+query, "-f", "login="+owner, "--jq", jqPath) + output, err := projectCommandRunGH("Getting owner ID...", "api", "graphql", "-f", "query="+query, "-f", "login="+owner, "--jq", jqPath) if err != nil { return "", fmt.Errorf("failed to get owner node ID: %w", err) } diff --git a/pkg/cli/project_command_test.go b/pkg/cli/project_command_test.go index ab568334b67..7efed380f87 100644 --- a/pkg/cli/project_command_test.go +++ b/pkg/cli/project_command_test.go @@ -589,6 +589,71 @@ func TestValidateOwnerUsesStringLoginField(t *testing.T) { } } +func TestGetOwnerNodeIdUsesStringLoginField(t *testing.T) { + oldRunGH := projectCommandRunGH + defer func() { projectCommandRunGH = oldRunGH }() + + tests := []struct { + name string + ownerType string + owner string + wantJQ string + wantQuery string + }{ + { + name: "organization login false stays string", + ownerType: "org", + owner: "false", + wantJQ: ".data.organization.id", + wantQuery: `query($login: String!) { organization(login: $login) { id } }`, + }, + { + name: "user login null stays string", + ownerType: "user", + owner: "null", + wantJQ: ".data.user.id", + wantQuery: `query($login: String!) { user(login: $login) { id } }`, + }, + { + name: "special characters are passed via login field", + ownerType: "org", + owner: `octo"cat\team`, + wantJQ: ".data.organization.id", + wantQuery: `query($login: String!) { organization(login: $login) { id } }`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var captured []string + projectCommandRunGH = func(spinnerMessage string, args ...string) ([]byte, error) { + captured = append([]string(nil), args...) + return []byte("NODE_ID_123"), nil + } + + nodeID, err := getOwnerNodeId(context.Background(), tt.ownerType, tt.owner, false) + require.NoError(t, err) + assert.Equal(t, "NODE_ID_123", nodeID) + + queryArg := "query=" + tt.wantQuery + require.Contains(t, captured, queryArg) + queryIndex := slices.Index(captured, queryArg) + require.Positive(t, queryIndex) + assert.Equal(t, "-f", captured[queryIndex-1], "query must be passed with -f") + + require.Contains(t, captured, "login="+tt.owner) + loginIndex := slices.Index(captured, "login="+tt.owner) + require.Positive(t, loginIndex) + assert.Equal(t, "-f", captured[loginIndex-1], "login must be passed with -f so gh keeps String! values as strings") + + jqIndex := slices.Index(captured, "--jq") + require.Positive(t, jqIndex) + require.Less(t, jqIndex, len(captured)-1) + assert.Equal(t, tt.wantJQ, captured[jqIndex+1]) + }) + } +} + func TestGetStatusFieldUsesStringLoginAndIntNumberFields(t *testing.T) { oldRunGH := projectCommandRunGH defer func() { projectCommandRunGH = oldRunGH }()