Repository navigation
fix(guardrail): detect direct pushes using merged PR records - #54
greenc-FNAL wants to merge 2 commits into
Conversation
Walkthrough
Merge Risk:
|
There was a problem hiding this comment.
🟢 Approval recommended
The main-branch guardrail update has no active review issues.
No open issues 🎉
What changed in this PR
Updates the repository’s main-branch guardrail to distinguish PR merges from direct pushes using GitHub’s merged-PR records rather than committer names.
Changes:
- Recognizes PR merges, including rebase merges, by matching the pushed SHA.
- Treats forced updates as alert-worthy and removes the Dependabot exemption.
- Adds PR-read permission, preserves safe Slack JSON encoding, and fails on API, missing-webhook, or delivery errors.
| File | Description |
|---|---|
.github/workflows/guardrail-audit-alert.yaml |
Replaces committer-based detection with PR-record checks and strengthens Slack alert error handling. |
Review effort: Balanced
Summary
mainusing GitHub's merged-PR records, including rebase merges, rather than trusting committer names or exempting bot pushes.repository.pushed_at, accepting epoch or UTC ISO timestamps within a documented 60-second tolerance. This avoids runner-queue and rerun timing errors and alerts on stale merge restorations.Scope and relationships
Changes are limited to
.github/workflows/guardrail-audit-alert.yaml,.github/workflows/ci.yaml, andtests/test_guardrail_audit_alert.py. Action interfaces, releases, dependencies, and existing Python tooling are unchanged. The guardrail workflow and test file are identical across all fifteen action repositories; other CI content and pins are preserved.This addresses the concern reviewed in action-detect-act-env#53, without using the unsupported
github.run_started_atcontext from Copilot's corrective commit. The guardrail correction can merge independently of the actionlint-pinning PR.Validation
Known limitation
This is best-effort temporal correlation, not complete event-provenance tracking. A rewind and restoration within 60 seconds of the original PR merge may still suppress the restoration alert. That bounded limitation is intentional; exact detection of every replay is outside this change's scope.
mainby matching the push SHA against merged-PR records and correlatingmerged_atwith the repository push timestamp within 60 seconds. Forced pushes do not receive the merge exemption. Missing or invalid timestamps produce a warning and send an alert.pull-requests: read. Checks webhook configuration before merge detection, safely builds the Slack JSON payload withjq, and fails on API errors, curl errors, or any Slack status other than HTTP 200.guardrail-testjob that runs the workflow’s shell logic tests after setup succeeds, without a pull-request change filter. The tests cover timestamp boundaries, PR identity and pagination, force pushes, failure paths, payload escaping, and event context. Test execution results were not provided.