Summary
Users with the SELF_CERTIFY role were able to bypass the push review queue entirely — pushes were forwarded upstream without ever entering the approval workflow. This is incorrect: self-certify means a user may approve their own push in the dashboard, not skip the queue.
A secondary side effect was a hidden commits false positive: because the bypass skipped the forwarding path that updates the local clone ref, subsequent incremental pushes from the same user were falsely rejected by CheckHiddenCommitsFilter.
Root cause
PushFinalizerFilter and ApprovalPreReceiveHook both called isBypassReviewAllowed() and immediately set result=ALLOWED, forwarding the push without enqueueing it for review. Introduced in 3ed0d4a (feat: add SELF_CERTIFY operation) and first shipped in v1.0.0-beta.2.
Fix
Fixed in #148 (shipped in v1.0.0-beta.4):
- Removed both bypass blocks; all pushes now enter the review queue regardless of self-certify status
ApprovalPreReceiveHook adds a verifySelfApprovalEntitled() defence-in-depth check after waitForApproval returns APPROVEDCheckHiddenCommitsFilter: always mark commitFrom as uninteresting before walking (fixes stale-ref false positive)LocalRepositoryCache: always call refreshIfStale() on cache hits
Summary
Users with the
SELF_CERTIFYrole were able to bypass the push review queue entirely — pushes were forwarded upstream without ever entering the approval workflow. This is incorrect: self-certify means a user may approve their own push in the dashboard, not skip the queue.A secondary side effect was a hidden commits false positive: because the bypass skipped the forwarding path that updates the local clone ref, subsequent incremental pushes from the same user were falsely rejected by
CheckHiddenCommitsFilter.Root cause
PushFinalizerFilterandApprovalPreReceiveHookboth calledisBypassReviewAllowed()and immediately setresult=ALLOWED, forwarding the push without enqueueing it for review. Introduced in 3ed0d4a (feat: add SELF_CERTIFY operation) and first shipped in v1.0.0-beta.2.Fix
Fixed in #148 (shipped in v1.0.0-beta.4):
ApprovalPreReceiveHookadds averifySelfApprovalEntitled()defence-in-depth check afterwaitForApprovalreturnsAPPROVEDCheckHiddenCommitsFilter: always markcommitFromas uninteresting before walking (fixes stale-ref false positive)LocalRepositoryCache: always callrefreshIfStale()on cache hits