Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ jobs:
> [!IMPORTANT]
> `pr-comments` is an experimental feature. By default, it's disabled.
>
> This feature currently doesn’t work with forked repositories. For more details, refer to issue [#77](https://github.com/commit-check/commit-check-action/issues/77).
> For forked repositories skip PR comment. For more details, refer to issue [#77](https://github.com/commit-check/commit-check-action/issues/77).
Comment thread
shenxianpeng marked this conversation as resolved.
Outdated

Note: the default rule of above inputs is following [this configuration](https://github.com/commit-check/commit-check-action/blob/main/commit-check.toml). If you want to customize, just add your [`commit-check.toml`](https://commit-check.github.io/commit-check/configuration.html) config file under your repository root directory.

Expand Down
80 changes: 55 additions & 25 deletions main.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
#!/usr/bin/env python3
import json
import os
import sys
import subprocess
import re
from github import Github, Auth # type: ignore
from github import Github, Auth, GithubException # type: ignore


# Constants for message titles
Expand Down Expand Up @@ -96,73 +97,102 @@ def add_job_summary() -> int:
return 0 if result_text is None else 1


def is_fork_pr() -> bool:
"""Returns True when the triggering PR originates from a forked repository."""
event_path = os.getenv("GITHUB_EVENT_PATH")
if not event_path:
return False
try:
with open(event_path, "r") as f:
event = json.load(f)
pr = event.get("pull_request", {})
head_full_name = pr.get("head", {}).get("repo", {}).get("full_name", "")
base_full_name = pr.get("base", {}).get("repo", {}).get("full_name", "")
return bool(
head_full_name and base_full_name and head_full_name != base_full_name
)
except Exception:
return False
Comment on lines +114 to +115
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Narrow blind exception handlers to avoid masking real failures.

Line 112 and Line 191 both catch Exception broadly. This hides parse/runtime defects and makes behavior harder to diagnose in CI.

💡 Proposed tightening
@@
-    except Exception:
-        return False
+    except (OSError, json.JSONDecodeError) as e:
+        print(f"::warning::Unable to parse GITHUB_EVENT_PATH: {e}", file=sys.stderr)
+        return False
@@
-    except Exception as e:
-        print(f"Error posting PR comment: {e}", file=sys.stderr)
-        return 0
+    except Exception as e:
+        print(f"Unexpected error posting PR comment: {e}", file=sys.stderr)
+        raise

Also applies to: 191-193

🧰 Tools
🪛 Ruff (0.15.2)

[warning] 112-112: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@main.py` around lines 112 - 113, The broad "except Exception:" handlers (the
exact "except Exception:" at the shown location and the similar block at lines
191-193) should be narrowed to only the exceptions you expect from the guarded
code (for example json.JSONDecodeError / ValueError for parsing, KeyError for
dict access, TypeError for bad types, or specific library errors) and capture
the exception as a variable (e.g., "except (ValueError, json.JSONDecodeError) as
e:") so you can log e before returning False; for truly unexpected errors
re-raise them instead of swallowing (or let them propagate) so CI can surface
real failures. Ensure you replace each "except Exception:" with the specific
exception tuple relevant to the surrounding parse/validation logic and add
logging of the exception object.



def add_pr_comments() -> int:
"""Posts the commit check result as a comment on the pull request."""
if PR_COMMENTS == "false":
return 0

# Fork PRs triggered by the pull_request event receive a read-only token;
# the GitHub API will always reject comment writes with 403.
if is_fork_pr():
Comment thread
shenxianpeng marked this conversation as resolved.
print(
"::warning::Skipping PR comment: pull requests from forked repositories "
"cannot write comments via the pull_request event (GITHUB_TOKEN is "
"read-only for forks). Use the pull_request_target event or the "
"two-workflow artifact pattern instead. "
"See https://github.com/commit-check/commit-check-action/issues/77"
)
return 0
Comment on lines +123 to +133
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fork guard currently blocks valid pull_request_target comment flows.

Line 123 skips all fork PRs, including cases where the workflow runs on pull_request_target and has write permissions. That prevents the intended supported path.

✅ Proposed fix
-    if is_fork_pr():
+    event_name = os.getenv("GITHUB_EVENT_NAME", "")
+    if event_name == "pull_request" and is_fork_pr():
         print(
             "::warning::Skipping PR comment: pull requests from forked repositories "
             "cannot write comments via the pull_request event (GITHUB_TOKEN is "
             "read-only for forks). Use the pull_request_target event or the "
             "two-workflow artifact pattern instead. "
             "See https://github.com/commit-check/commit-check-action/issues/77"
         )
         return 0
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@main.py` around lines 121 - 131, The current fork guard unconditionally
returns for any fork PR (is_fork_pr()), which also blocks valid
pull_request_target workflows; change the conditional so it only skips when the
event is a fork PR AND the workflow is NOT running under pull_request_target
(e.g., check os.getenv("GITHUB_EVENT_NAME") != "pull_request_target" or
equivalent), leaving the warning and return in the is_fork_pr() block but
permitting comment writes when the event is pull_request_target (optionally also
guard by an explicit writable token/permission check if available).

Comment thread
shenxianpeng marked this conversation as resolved.

try:
token = os.getenv("GITHUB_TOKEN")
repo_name = os.getenv("GITHUB_REPOSITORY")
pr_number = os.getenv("GITHUB_REF")
if pr_number is not None:
pr_number = pr_number.split("/")[-2]
else:
# Handle the case where GITHUB_REF is not set
raise ValueError("GITHUB_REF environment variable is not set")

# Initialize GitHub client
# Use new Auth API to avoid deprecation warning
if not token:
raise ValueError("GITHUB_TOKEN is not set")

g = Github(auth=Auth.Token(token))
repo = g.get_repo(repo_name)
pull_request = repo.get_issue(int(pr_number))

# Prepare comment content
result_text = read_result_file()
pr_comments = (
pr_comment_body = (
SUCCESS_TITLE
if result_text is None
else f"{FAILURE_TITLE}\n```\n{result_text}\n```"
)

# Fetch all existing comments on the PR
comments = pull_request.get_comments()
matching_comments = [
c
for c in comments
if c.body.startswith(SUCCESS_TITLE) or c.body.startswith(FAILURE_TITLE)
]

# Track if we found a matching comment
matching_comments = []
last_comment = None

for comment in comments:
if comment.body.startswith(SUCCESS_TITLE) or comment.body.startswith(
FAILURE_TITLE
):
matching_comments.append(comment)
if matching_comments:
last_comment = matching_comments[-1]

if last_comment.body == pr_comments:
if last_comment.body == pr_comment_body:
print(f"PR comment already up-to-date for PR #{pr_number}.")
return 0
else:
# If the last comment doesn't match, update it
print(f"Updating the last comment on PR #{pr_number}.")
last_comment.edit(pr_comments)

# Delete all older matching comments
print(f"Updating the last comment on PR #{pr_number}.")
last_comment.edit(pr_comment_body)
for comment in matching_comments[:-1]:
print(f"Deleting an old comment on PR #{pr_number}.")
comment.delete()
Comment on lines +161 to 176
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Filter managed comments by author before edit/delete.

Line 161-176 identifies “managed” comments only by body prefix, then edits/deletes them. This can mutate or delete human comments that happen to start with the same header.

✅ Suggested fix
         comments = pull_request.get_comments()
+        bot_login = repo.owner.login
         matching_comments = [
             c
             for c in comments
-            if c.body.startswith(SUCCESS_TITLE) or c.body.startswith(FAILURE_TITLE)
+            if (
+                c.user
+                and c.user.login == bot_login
+                and (
+                    c.body.startswith(SUCCESS_TITLE)
+                    or c.body.startswith(FAILURE_TITLE)
+                )
+            )
         ]
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@main.py` around lines 161 - 176, The current logic builds matching_comments
by filtering comments only by body prefixes (using SUCCESS_TITLE/FAILURE_TITLE)
and then edits/deletes them; change this to also filter by the bot/automation
author before editing or deleting to avoid touching human comments: when
creating matching_comments, add an author check against the expected actor
(e.g., compare comment.user.login or comment.author to the bot/github app
identity) so only comments authored by your automation are included, then
continue using last_comment, pr_comment_body and the deletion loop safely for
those filtered results.

else:
# No matching comments, create a new one
print(f"Creating a new comment on PR #{pr_number}.")
pull_request.create_comment(body=pr_comments)
pull_request.create_comment(body=pr_comment_body)

return 0 if result_text is None else 1
except GithubException as e:
if e.status == 403:
print(
"::warning::Unable to post PR comment (403 Forbidden). "
"Ensure your workflow grants 'issues: write' permission. "
f"Error: {e.data.get('message', str(e))}",
file=sys.stderr,
)
return 0
print(f"Error posting PR comment: {e}", file=sys.stderr)
return 0
except Exception as e:
print(f"Error posting PR comment: {e}", file=sys.stderr)
return 1
return 0


def log_error_and_exit(
Expand Down
Loading