From 05dc83f55fa4d1843bd26ad9f1b1b4a829809b01 Mon Sep 17 00:00:00 2001
From: gustavo-sec <289506718+gustavo-sec@users.noreply.github.com>
Date: Wed, 17 Jun 2026 00:00:59 -0300
Subject: [PATCH] Validate add_comment_to_pending_review required params before
 dispatch

add_comment_to_pending_review decoded its arguments with mapstructure.WeakDecode
and never checked the required ones, so an omitted required argument (e.g. owner)
was sent to the GraphQL API and surfaced as a confusing downstream error such as
"Could not resolve to a Repository with the name '/<repo>'" instead of a clear
missing-parameter message.

Validate owner, repo, pullNumber, path, body, and subjectType up front (matching
how the other pull request tools use RequiredParam/RequiredInt) so a missing
required argument is reported as "missing required parameter: <name>".
---
 pkg/github/pullrequests.go      | 14 ++++++++++++++
 pkg/github/pullrequests_test.go | 13 +++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go
index 07ff6a87f..c1084af0c 100644
--- a/pkg/github/pullrequests.go
+++ b/pkg/github/pullrequests.go
@@ -2293,6 +2293,20 @@ func AddCommentToPendingReview(t translations.TranslationHelperFunc) inventory.S
 				return utils.NewToolResultError(err.Error()), nil, nil
 			}
 
+			// WeakDecode leaves an omitted field as its zero value, so without an
+			// explicit check a missing required argument (e.g. owner) would be
+			// sent to the GraphQL API and surface as a confusing downstream error
+			// ("Could not resolve to a Repository ...") instead of a clear
+			// missing-parameter message. Validate the required arguments up front.
+			for _, p := range []string{"owner", "repo", "path", "body", "subjectType"} {
+				if _, err := RequiredParam[string](args, p); err != nil {
+					return utils.NewToolResultError(err.Error()), nil, nil
+				}
+			}
+			if _, err := RequiredInt(args, "pullNumber"); err != nil {
+				return utils.NewToolResultError(err.Error()), nil, nil
+			}
+
 			client, err := deps.GetGQLClient(ctx)
 			if err != nil {
 				return utils.NewToolResultErrorFromErr("failed to get GitHub GQL client", err), nil, nil
diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go
index 0f372519e..9092c1b8c 100644
--- a/pkg/github/pullrequests_test.go
+++ b/pkg/github/pullrequests_test.go
@@ -3575,6 +3575,19 @@ func TestAddPullRequestReviewCommentToPendingReview(t *testing.T) {
 			expectToolError:    true,
 			expectedToolErrMsg: "Failed to add comment to pending review",
 		},
+		{
+			name:         "missing required owner is reported instead of running the query",
+			mockedClient: githubv4mock.NewMockedHTTPClient(),
+			requestArgs: map[string]any{
+				"repo":        "repo",
+				"pullNumber":  float64(42),
+				"path":        "file.go",
+				"body":        "This is a test comment",
+				"subjectType": "LINE",
+			},
+			expectToolError:    true,
+			expectedToolErrMsg: "missing required parameter: owner",
+		},
 	}
 
 	for _, tc := range tests {