Page MenuHomePhabricator

Do not disclose hidden deployment commits
ClosedPublic

Authored by demon on Dec 1 2016, 7:46 PM.

Details

Summary

Instead of displaying the exact sha1 of deployment, use the last common ancestor with the remote we're tracking. This is what we do in git.info() for MediaWiki already so reuse that data in scap3.

While we're here, clean up the logic for the failure case when we're not tracking an upstream. This has never worked, as "origin" is not a valid reference to anything, it's just a remote. For get_disclosable_head() to have any chance at finding an ancestor, we need an upstream branch or tag so accept one now.

git.info() is generally more robust to failure now.

Test Plan

Ran this with scap deploy a bunch of times with all kinds of bogus output. The default case should be mostly unchanged for MW, only more robust if we break things.

Diff Detail

Repository
rMSCA Scap
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

demon retitled this revision from to Do not disclose hidden deployment commits.
demon updated this object.
demon edited the test plan for this revision. (Show Details)
demon added a reviewer: thcipriani.
thcipriani edited edge metadata.

Couple of inline nitpicks.

Overall this is much improved and needed -- accepted.

scap/git.py
123–124

You catch the exception for git remote in get_disclosable_head, but this will also raise if no remote is configured.

374

Could make except narrower: except subprocess.CalledProcessError:

This revision is now accepted and ready to land.Dec 1 2016, 8:20 PM
scap/git.py
123–124

It should be basically impossible for this to fail assuming we got a sha1 back from get_disclosable_head()

374

Typo, will fix.

scap/git.py
123–124

Duh, I misread you and was looking at the commit_date. You're right, will fix.

demon edited edge metadata.
  • Better exception handling
demon marked 5 inline comments as done.Dec 1 2016, 8:44 PM

All inlines done.

This revision was automatically updated to reflect the committed changes.