Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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).
> PR comments are skipped for pull requests from forked repositories. For more details, refer to issue [`#143`](https://github.com/commit-check/commit-check-action/issues/143).

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

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():
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

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).


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

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