The listAndFindComment function identifies comments to update solely based on a regex match of a marker string in the comment body. It does not verify that the comment was authored by the Pipelines as Code (PAC) GitHub App itself. An attacker can post a comment containing the marker string, causing PAC to identify that comment as its own and overwrite it, allowing unauthorized modification and potential hijacking of the PAC status display. Furthermore, this function only fetches the first 100 comments. For active PRs, this can lead to failing to find recent comments, causing duplicate comments, PR flooding, and API exhaustion. It is crucial to implement author verification and proper pagination to address these security and functional issues.

func (v *Provider) listAndFindComment(ctx context.Context, event *info.Event, marker string) (*github.IssueComment, error) {
	re := regexp.MustCompile(regexp.QuoteMeta(marker))
	opts := &github.IssueListCommentsOptions{
		ListOptions: github.ListOptions{PerPage: 100},
	}

	for {
		comments, resp, err := wrapAPI(v, "list_comments", func() ([]*github.IssueComment, *github.Response, error) {
			return v.Client().Issues.ListComments(ctx, event.Organization, event.Repository, event.PullRequestNumber, opts)
		})
		if err != nil {
			return nil, err
		}

		for _, comment := range comments {
			if re.MatchString(comment.GetBody()) {
				return comment, nil
			}
		}

		if resp.NextPage == 0 {
			break
		}
		opts.Page = resp.NextPage
	}

	return nil, nil
}

That is a good point lol, we should probably do this across every place where we do comments...

Code Implementation 🛠️

Implementation: Tighten comment selection to only consider comments authored by a bot when running as a GitHub App, and prefer the most recent matching comment among the fetched comments to reduce hijacking risk and avoid updating stale matches.

See review comment here

that's actually a good suggestions qodot

@chmouel qodo suggestion makes sense.
As for the gemini suggestion, shouldn't the edit_comment get a 403 if it tries to edit someone else's comment? Github Docs states that you need write or higher permissions on the repo. That is something we do not give (or recommend) for the app or webhook token, right?

Ask❓

@chmouel qodo suggestion makes sense. As for the gemini suggestion, shouldn't the edit_comment get a 403 if it tries to edit someone else's comment? Github Docs states that you need write or higher permissions on the repo. That is something we do not give (or recommend) for the app or webhook token, right?

Answer:

GitHub won’t return 403 just because the comment was authored by someone else.

The 403 you’re thinking of is about authorization scope, not comment ownership. If the credential you’re using has the ability to write issue comments on that repo, then the PATCH /repos/{owner}/{repo}/issues/comments/{comment_id} (“edit comment”) endpoint generally allows editing any issue/PR comment in that repo, regardless of who created it.

Concretely for PaC:

• To create PR comments, PaC (GitHub App installation token) typically needs Issues: write (and/or Pull requests: write depending on API usage). With that, it can create and edit issue comments in the repo.
• Repo “write” is not the only way to get this; a GitHub App permission like Issues: write is sufficient to call these APIs. So even if we “don’t recommend write/admin on the repo”, the app still commonly has the relevant write permission to issue comments.
• Therefore, if an attacker posts a comment containing the marker, listAndFindComment may select it and PaC may successfully edit it—no 403—leading to the hijacking scenario the bots flagged (overwriting attacker-controlled comment content or causing confusing status display).

If, in some deployments, the token doesn’t have permission to edit comments, then yes, EditComment would fail (likely 403), but that’s not a safe assumption:

• it would turn the attack into a DoS/flakiness vector (PaC errors trying to edit), and
• it doesn’t address the main risk for the common configuration where PaC does have Issues: write.

So the qodo suggestion (filter to bot/app-authored comments when running as a GitHub App, and prefer the newest match) is a good mitigation, and gemini’s pagination point is also valid (only checking first 100 comments can miss the intended marker and create duplicates).

i think we should do context aware sleep instead, i.e: