Opened 7 years ago

Closed 5 years ago

#8437 closed task (fixed)

Prevent invalid rules from being committed, or at leat warn when they are

Reported by: schoen Owned by: zyan
Priority: Medium Milestone:
Component: HTTPS Everywhere/EFF-HTTPS Everywhere Version:
Severity: Keywords:
Cc: Sebastian Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


Sometimes people commit rules that we can automatically detect are invalid, for example with utils/ It would be good to have a system to prevent these from being committed at all or to immediately warn someone when they have been, rather than having to wait until someone tries to build the extension later.

I've never used commit-hooks, so I don't know exactly what options are available in the git infrastructure to help with this. Another alternative is a daemon that periodically runs validation scripts across the current tree and generates e-mail if anything invalid is present.

Child Tickets

Change History (5)

comment:1 Changed 7 years ago by pde

Cc: Sebastian added

Sebastian please correct me if I'm wrong about this, but it seems as though the server side post-receive hook is the first time that the server can inspect the rulesets, and that's too late to stop the commit from going through.

Even aside from that, the most conceptually elegant way to audit rulesets via git might be in the client-side pre-commit hook. But I don't know if there's any practical way to get a hook script into hundreds of ruleset authors' local repos [*].

Maybe there's a hook that you, Mike, Dan and I could use on the client side that would prevent us from pushing broken stuff. But that's inherently less productive than telling the people who wrote the buggy rulesets that they were buggy.

[*] the Internet suggests the hack of cp'ing a git hook script into .git/hooks from somewhere like the build script. But I think that the ruleset authors who are failing to validate their rulesets probably aren't running the build script either.

comment:2 Changed 6 years ago by yan

Owner: changed from pde to yan
Status: newassigned

I'm working on a pre-commit hook for people who want to use it.

comment:3 Changed 6 years ago by zyan

Status: assignedneeds_review

After some reading, it seemed that the way to do this is to have a server-side update hook that runs the build script (which includes ruleset validation) in a temporary directory that includes the changes being pushed and rejects the push if the build fails. An update hook is similar to a pre-receive hook from what I understand, except it runs once per branch.

With much help from, I've made a draft of this script here:

It should go in .git/hooks.

comment:4 Changed 6 years ago by zyan

Owner: changed from yan to zyan
Status: needs_reviewassigned

(reassigning to myself since I changed usernames)

comment:5 Changed 5 years ago by jsha

Resolution: fixed
Status: assignedclosed

We wound up solving this by integrating Travis-CI, so we now get warned at code review time whether a branch is going to break the build

Note: See TracTickets for help on using tickets.