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
7 changes: 7 additions & 0 deletions scripts/check_issue_consistency.py
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,13 @@ def main() -> int:
"retaining PR title issue reference.",
file=sys.stderr,
)
else:
print(
"Warning: PR title uses a bare # reference with no issue context; "
"falling back to commits/headers.",
file=sys.stderr,
)
pr_issue = None
if not pr_issue:
pr_issue, head_ambiguous = resolve_head_ref_issue_number(head_ref)
if head_ambiguous:
Expand Down
25 changes: 19 additions & 6 deletions scripts/generate_suppression_guard_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,15 @@
("marocchino/sticky-pull-request-comment", "sticky-pull-request-comment action"),
)

# Matches suppress_comments in a negation context so that an inverted guard
# like ``inputs.suppress_comments == true`` is NOT treated as a valid guard.
# Covers: != true, == false, !inputs.suppress_comments, !(inputs.suppress_comments)
_SUPPRESS_NEGATION_RE = re.compile(
r"suppress_comments\s*!=\s*true"
r"|suppress_comments\s*==\s*false"
r"|!\s*\(?\s*inputs\.suppress_comments"
)


def _normalise_keys(node: Any) -> Any:
if isinstance(node, dict):
Expand Down Expand Up @@ -112,8 +121,8 @@ def _iter_posting_steps(workflow: dict[str, Any]) -> list[tuple[str, str, list[s
guarded = (
"should_post_review" in job_if_str
or "should_post_review" in step_if_str
or "suppress_comments" in job_if_str
or "suppress_comments" in step_if_str
or bool(_SUPPRESS_NEGATION_RE.search(job_if_str))
or bool(_SUPPRESS_NEGATION_RE.search(step_if_str))
)
if hints and not guarded:
findings.append((str(job_id), str(name), hints))
Expand Down Expand Up @@ -142,11 +151,15 @@ def build_comment(workflows: Iterable[pathlib.Path], include_label: bool = False
if include_label:
lines.append("Label: needs-human")
lines.append(
"Blocked by workflow protection: update .github/workflows/keepalive.yml and "
".github/workflows/autofix.yml to add explicit `if:` guards on every step/job "
"that posts a PR comment or PR review so they cannot run when suppression is active. "
"Blocked by workflow protection: update "
".github/workflows/agents-keepalive-loop.yml, "
".github/workflows/autofix.yml, and "
".github/workflows/reusable-18-autofix.yml to add explicit `if:` guards "
"on every step/job that posts a PR comment or PR review so they cannot "
"run when suppression is active. "
"Use the suppression output key `should_post_review` (from "
".github/scripts/should-post-review.js) to gate the posting steps."
".github/scripts/should-post-review.js) or the `suppress_comments` "
"input to gate the posting steps."
)
lines.append("")

Expand Down
50 changes: 50 additions & 0 deletions tests/scripts/test_generate_suppression_guard_comment.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,56 @@ def test_build_comment_ignores_suppress_comments_guarded_steps(
assert "post / Post comment" not in comment


def test_build_comment_flags_inverted_suppress_comments_guard(
tmp_path: Path,
) -> None:
"""``suppress_comments == true`` allows posting during suppression."""
workflow_path = tmp_path / "inverted.yml"
_write_yaml(
workflow_path,
"""
name: Inverted Guard Workflow
jobs:
post:
runs-on: ubuntu-latest
steps:
- name: Post comment
if: inputs.suppress_comments == true
run: github.rest.issues.createComment
""",
)

comment = build_comment([workflow_path])

# Inverted guard should NOT be treated as properly guarded
assert "post / Post comment" in comment


def test_build_comment_ignores_parenthesized_negation_guard(
tmp_path: Path,
) -> None:
"""``!(inputs.suppress_comments)`` is a valid negation guard."""
workflow_path = tmp_path / "paren.yml"
_write_yaml(
workflow_path,
"""
name: Parenthesized Negation Workflow
jobs:
post:
runs-on: ubuntu-latest
steps:
- name: Post comment
if: ${{ !(inputs.suppress_comments) }}
run: github.rest.issues.createComment
""",
)

comment = build_comment([workflow_path])

assert "No unguarded PR comment/review posting steps detected" in comment
assert "post / Post comment" not in comment


def test_build_comment_detects_octokit_aliases(tmp_path: Path) -> None:
workflow_path = tmp_path / "octokit.yml"
_write_yaml(
Expand Down
25 changes: 25 additions & 0 deletions tests/scripts/test_issue_consistency_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,31 @@ def test_main_title_hash_prefers_head_ref(monkeypatch, capsys) -> None:
assert "Issue consistency check passed for #5678." in captured.out


def test_main_title_hash_without_head_ref_issue_falls_back(monkeypatch, capsys) -> None:
monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False)
monkeypatch.setenv("PR_TITLE", "Update docs (#1234)")
monkeypatch.setenv("HEAD_REF", "feature/no-issue")
monkeypatch.setenv("BASE_REF", "")
monkeypatch.setenv("BASE_SHA", "")
monkeypatch.setenv("BASE_REMOTE", "origin")
monkeypatch.setattr(sys, "argv", ["check_issue_consistency.py"])

monkeypatch.setattr(
check_issue_consistency,
"collect_commit_messages",
lambda *args, **kwargs: (["fix: resolve issue #5678"], False),
)
monkeypatch.setattr(
check_issue_consistency,
"collect_changed_files",
lambda *args, **kwargs: ([], False),
)

assert check_issue_consistency.main() == 0
captured = capsys.readouterr()
assert "Issue consistency check passed for #5678." in captured.out


def test_main_autofix_skips_mismatched_commits(monkeypatch, capsys) -> None:
monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False)
monkeypatch.setenv("PR_TITLE", "Autofix #1234")
Expand Down
Loading