launchpad-reviewers team mailing list archive

["Thiago F. Pappacena" <pappacena@xxxxxxxxx>]

Thiago F. Pappacena has proposed merging ~pappacena/turnip:block-readonly-refs into turnip:master.

Commit message:
Blocking pushes to read-only ref namespaces, like refs/merge/

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/turnip/+git/turnip/+merge/390620
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/turnip:block-readonly-refs into turnip:master.

diff --git a/turnip/pack/hooks/hook.py b/turnip/pack/hooks/hook.py
index 0dba4dd..4e492ea 100755
--- a/turnip/pack/hooks/hook.py
+++ b/turnip/pack/hooks/hook.py
@@ -25,6 +25,12 @@ from turnip.compat.files import fd_buffer
 GIT_OID_HEX_ZERO = '0'*40
 
 
+# Users cannot update references in this list.
+READONLY_REF_NAMESPACES = [
+    b'refs/merge/',
+]
+
+
 def check_ancestor(old, new):
     # This is a delete, setting the new ref.
     if new == GIT_OID_HEX_ZERO:
@@ -42,6 +48,8 @@ def is_default_branch(pushed_branch):
 
 
 def determine_permissions_outcome(old, ref, rule_lines):
+    if any(ref.startswith(i) for i in READONLY_REF_NAMESPACES):
+        return b"%s is in a read-only namespace." % ref
     rule = rule_lines.get(ref, [])
     if old == GIT_OID_HEX_ZERO:
         # We are creating a new ref
@@ -88,6 +96,8 @@ def match_update_rules(rule_lines, ref_line):
     the rule_lines to confirm that the user has permissions for that operation.
     """
     ref, old, new = ref_line
+    if any(ref.startswith(i) for i in READONLY_REF_NAMESPACES):
+        return [b"%s is in a read-only namespace." % ref]
 
     # If it's a create, the old ref doesn't exist
     if old == GIT_OID_HEX_ZERO:
diff --git a/turnip/pack/tests/test_hooks.py b/turnip/pack/tests/test_hooks.py
index 22c31c7..374097f 100644
--- a/turnip/pack/tests/test_hooks.py
+++ b/turnip/pack/tests/test_hooks.py
@@ -265,6 +265,14 @@ class TestPreReceiveHook(HookTestMixin, TestCase):
             b"You do not have permission to push to refs/heads/verboten.\n")
 
     @defer.inlineCallbacks
+    def test_rejected_readonly_namespaces(self):
+        # An read-only ref is rejected.
+        yield self.assertRejected(
+            [(b'refs/merge/123/heads', self.old_sha1, self.new_sha1)],
+            {b'refs/heads/master': ['push', 'force_push', 'create']},
+            b"refs/merge/123/heads is in a read-only namespace.\n")
+
+    @defer.inlineCallbacks
     def test_rejected_multiple(self):
         # A combination of valid and invalid refs is still rejected.
         yield self.assertRejected(
@@ -441,6 +449,14 @@ class TestUpdateHook(TestCase):
         self.assertEqual(
             [b'You do not have permission to force-push to ref.'], output)
 
+    def test_read_only_ref(self):
+        # User does not have permission to force-push to a read-only namespace
+        output = hook.match_update_rules(
+            {'ref': ['create']},
+            [b'refs/merge/123/head', 'old', 'new'])
+        self.assertEqual(
+            [b'refs/merge/123/head is in a read-only namespace.'], output)
+
 
 class TestDeterminePermissions(TestCase):