Page MenuHomePhabricator
Create Task
Maniphest T12150

Restricting Push not working
Closed, InvalidPublic
Actions

Assigned To
None
Authored By
salvian
Jan 24 2017, 11:08 AM
Tags
Referenced Files
F2477045: pasted_file
Jan 24 2017, 12:22 PM
F2477040: pasted_file
Jan 24 2017, 12:22 PM
Subscribers
avivey
epriestley
salvian

Description

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

Regards,

Salvian

Versions:

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

Event Timeline

salvian created this task.Jan 24 2017, 11:08 AM
avivey edited projects, added Bug Report (Needs Information); removed Bug Report.Jan 24 2017, 12:22 PM
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.

epriestley added a subscriber: epriestley.Jan 24 2017, 2:39 PM

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.

salvian added a comment.Jan 25 2017, 5:01 AM

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

epriestley closed this task as Invalid.Feb 10 2017, 5:10 PM

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.

salvian added a comment.Feb 16 2017, 10:44 AM

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?

salvian added a comment.Feb 17 2017, 3:36 AM

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

noexec,noatime,nodiratime

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

noatime,nodiratime

thanks!

Phabricator · Built by Phacility