Repository navigation
Reject carried-forward receipts by default - #47
jamilahmadzai wants to merge 3 commits into
Conversation
AAH20
left a comment
There was a problem hiding this comment.
Clean and well-scoped. The core change — inserting a CARRIED_FORWARD check between the API response parse and the set_output calls — is exactly where it belongs.
Key observations:
-
Placement after
VALIDand before outputs — the guard at line ~375 flipsVALID=falseand setsPP_CARRY_FORWARD_DISABLEDonly when all three conditions hold (VALID=true,CARRIED_FORWARD=true, opt-in not enabled). This means a genuinely fresh receipt is never touched, and a failed/denied receipt is never double-errored. -
PP_CARRY_FORWARD_DISABLEDerror-code block — follows the same pattern asSIGNATURE_INVALID/KEY_REVOKED: prints details, callsupsert_pr_comment, exits1. Consistent with the rest of the action. -
README
## Carry-forward approvalssection — good call documenting the version boundary (v2.2.0) so users upgrading know when the behavior changed. -
One nit (non-blocking): the
CARRIED_FORWARDvariable is parsed from the API response with a fallback of"false". If the API ever returns the booleanfalserather than the string"false",jq -rrenders it asfalsewhich matches, so this is fine — just noting for posterity.
LGTM. Approving.
Permission Protocol: human approval requiredNo automatic clearance rule applies. What changes: This pull request changes product or deploy behavior in the areas described by the engineering team. Reviewers may see a different result in the affected workflow after this is released. Operators may need to watch the affected flow closely on the first deploy. Estimated risk: high. Sensitive executable, configuration or receipt-gate changes need review; production impact must be confirmed from the diff.
Inspect the supporting diff and all 4 changed files Path-based review areas: Infra, Scripts. These matches do not establish production behavior changes. Note: Structured review generation failed — deterministic fallback used for review bullets. Signing authorizes this commit and scope. It does not prove deployment; other GitHub and deployment requirements still apply. |
Summary
allow-carry-forwardopt-in for repos that knowingly want PR-level approval carry-forwardWhy
A carried-forward approval can keep the gate green after the PR head changes. Since the action cannot prove which head SHA the prior approval covered, the safer default is to fail closed and require a fresh approval for the current head unless carry-forward is explicitly enabled.
Related: #46
Bounty: #36
Validation
788dd7b) and retained its PR-title JSON encoding and UTF-16 limit behavior.bash test/run-tests.sh: 90 passed, 0 failed, including upstream title handling and carry-forward policy in both PR and deploy modes.git diff --checkpasses.Payout
PayPal: jamilurrehman722@gmail.com