Merge ~pappacena/turnip:block-readonly-refs into turnip:master

Proposed by Thiago F. Pappacena
Status: Merged
Approved by: Thiago F. Pappacena
Approved revision: d508de4483522a9b0e63272239f798943323b9af
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~pappacena/turnip:block-readonly-refs
Merge into: turnip:master
Diff against target: 68 lines (+26/-0)
2 files modified
turnip/pack/hooks/hook.py (+10/-0)
turnip/pack/tests/test_hooks.py (+16/-0)
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Review via email: mp+390620@code.launchpad.net

Commit message

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

To post a comment you must log in.
Revision history for this message
Thiago F. Pappacena (pappacena) wrote :

Although there is a chain of MPs to make the `refs/merge/xxx` virtual refs work, this MP is independent and can be merged to master once it's reviewed.

Revision history for this message
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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
25GIT_OID_HEX_ZERO = '0'*4025GIT_OID_HEX_ZERO = '0'*40
2626
2727
28# Users cannot update references in this list.
29READONLY_REF_NAMESPACES = [
30 b'refs/merge/',
31]
32
33
28def check_ancestor(old, new):34def check_ancestor(old, new):
29 # This is a delete, setting the new ref.35 # This is a delete, setting the new ref.
30 if new == GIT_OID_HEX_ZERO:36 if new == GIT_OID_HEX_ZERO:
@@ -42,6 +48,8 @@ def is_default_branch(pushed_branch):
4248
4349
44def determine_permissions_outcome(old, ref, rule_lines):50def determine_permissions_outcome(old, ref, rule_lines):
51 if any(ref.startswith(i) for i in READONLY_REF_NAMESPACES):
52 return b"%s is in a read-only namespace." % ref
45 rule = rule_lines.get(ref, [])53 rule = rule_lines.get(ref, [])
46 if old == GIT_OID_HEX_ZERO:54 if old == GIT_OID_HEX_ZERO:
47 # We are creating a new ref55 # We are creating a new ref
@@ -88,6 +96,8 @@ def match_update_rules(rule_lines, ref_line):
88 the rule_lines to confirm that the user has permissions for that operation.96 the rule_lines to confirm that the user has permissions for that operation.
89 """97 """
90 ref, old, new = ref_line98 ref, old, new = ref_line
99 if any(ref.startswith(i) for i in READONLY_REF_NAMESPACES):
100 return [b"%s is in a read-only namespace." % ref]
91101
92 # If it's a create, the old ref doesn't exist102 # If it's a create, the old ref doesn't exist
93 if old == GIT_OID_HEX_ZERO:103 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):
265 b"You do not have permission to push to refs/heads/verboten.\n")265 b"You do not have permission to push to refs/heads/verboten.\n")
266266
267 @defer.inlineCallbacks267 @defer.inlineCallbacks
268 def test_rejected_readonly_namespaces(self):
269 # An read-only ref is rejected.
270 yield self.assertRejected(
271 [(b'refs/merge/123/heads', self.old_sha1, self.new_sha1)],
272 {b'refs/heads/master': ['push', 'force_push', 'create']},
273 b"refs/merge/123/heads is in a read-only namespace.\n")
274
275 @defer.inlineCallbacks
268 def test_rejected_multiple(self):276 def test_rejected_multiple(self):
269 # A combination of valid and invalid refs is still rejected.277 # A combination of valid and invalid refs is still rejected.
270 yield self.assertRejected(278 yield self.assertRejected(
@@ -441,6 +449,14 @@ class TestUpdateHook(TestCase):
441 self.assertEqual(449 self.assertEqual(
442 [b'You do not have permission to force-push to ref.'], output)450 [b'You do not have permission to force-push to ref.'], output)
443451
452 def test_read_only_ref(self):
453 # User does not have permission to force-push to a read-only namespace
454 output = hook.match_update_rules(
455 {'ref': ['create']},
456 [b'refs/merge/123/head', 'old', 'new'])
457 self.assertEqual(
458 [b'refs/merge/123/head is in a read-only namespace.'], output)
459
444460
445class TestDeterminePermissions(TestCase):461class TestDeterminePermissions(TestCase):
446462

Subscribers

People subscribed via source and target branches