Page MenuHomePhabricator

Restricting Push not working
Closed, InvalidPublic

Assigned To
Authored By
Jan 24 2017, 11:08 AM
Referenced Files
F2477045: pasted_file
Jan 24 2017, 12:22 PM
F2477040: pasted_file
Jan 24 2017, 12:22 PM


we had used Herald to block un-reviewed differential commit on a hosted repository

Rule Type     Global
Applies To    Commit Hook: Commit Content

When all of these conditions are met:
 - Accepted Differential revision does not exist 
 - Repository is any of [a list of review-required repositories]

it had worked perfectly, pushing unaccepted diffs is blocked by evil dragon bureaucrats

we realize that the rule is not working anymore, as recently there are some unreviewed pushes to the same repository

Step to reproduce:

  1. Create the herald rule like above that applies to a Diffusion, rX
  2. Do some modifications in rX
  3. Commit the change
  4. Push the commit to remote

Expected Result:

  • the push is rejected, an Evil Dragon Bureaucrats ASCII art appears

Actual Result:

  • the push succeeds




    9d10727f651fd4b359c4893f1886b7e436faad91 (Sat, Dec 31) (branched from 3fedc8c299f79e3dc3d389ba8bf0cc4b95b1a307 on origin) 
    e8b580d2e5e740071b25d087a0aca33f0b0dd691 (Sat, Dec 31) (branched from c243cbbd9fc701ca9555672ea5c1666ea154898f on origin) 
    0ae0cc00acb1413c22bfe3384fd6086ade4cc206 (Dec 10 2016) (branched from 5ac2ca1214890d865bc57fab2715a322fdf02ab6 on origin)

Event Timeline

avivey added a subscriber: avivey.

I couldn't reproduce this.

  • Got instance on Phacility
  • Created a repository and a herald rule as described:

pasted_file (374×418 px, 28 KB)

  • made an unauthorized push, got a dragon:

pasted_file (799×756 px, 86 KB)

Your code doesn't appear to be forked, but you are out of date (And also appears to be out of sync? your phutil is 3 weeks older than the rest). Try Upgrading Phabricator.

Also, double check that the rule is defined the way you think it is, and doesn't have exceptions such as "pusher is salvian" or something.

your phutil is 3 weeks older than the rest

This is probably just because we didn't make any changes to libphutil during those weeks -- we don't currently promote an empty commit to stable if a repository has no changes in the most recent period. But maybe we should, since having commits that appear to be out of sync is probably more confusing than having occasional empty commits on stable would be. We could also special-case this and make the commit message say "No changes" or something.

Okay, we will try to upgrade phabricator in our next weekly maintenance window tomorrow, I'll update you with our result later

We haven't received any more information about this in a little more than two weeks, and don't know how to reproduce it so we can't move forward. Feel free to file a new task if you're able to come up with reproduction steps which work in a clean environment.

We migrated phabricator last months just realized somehow "Push Logs" are not generated anymore (the last push log was created just before the migration)
and I guess commit content hooks are somehow related on push logs (?)
May I ask what can I check if the push logs aren't being created?

update: We used default mount options in /etc/fstab for the disk containing repositories


we fixes the push logs and herald commit content hooks issues by changing it to