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
1diff --git a/turnip/pack/hooks/hook.py b/turnip/pack/hooks/hook.py
2index 0dba4dd..4e492ea 100755
3--- a/turnip/pack/hooks/hook.py
4+++ b/turnip/pack/hooks/hook.py
5@@ -25,6 +25,12 @@ from turnip.compat.files import fd_buffer
6 GIT_OID_HEX_ZERO = '0'*40
7
8
9+# Users cannot update references in this list.
10+READONLY_REF_NAMESPACES = [
11+ b'refs/merge/',
12+]
13+
14+
15 def check_ancestor(old, new):
16 # This is a delete, setting the new ref.
17 if new == GIT_OID_HEX_ZERO:
18@@ -42,6 +48,8 @@ def is_default_branch(pushed_branch):
19
20
21 def determine_permissions_outcome(old, ref, rule_lines):
22+ if any(ref.startswith(i) for i in READONLY_REF_NAMESPACES):
23+ return b"%s is in a read-only namespace." % ref
24 rule = rule_lines.get(ref, [])
25 if old == GIT_OID_HEX_ZERO:
26 # We are creating a new ref
27@@ -88,6 +96,8 @@ def match_update_rules(rule_lines, ref_line):
28 the rule_lines to confirm that the user has permissions for that operation.
29 """
30 ref, old, new = ref_line
31+ if any(ref.startswith(i) for i in READONLY_REF_NAMESPACES):
32+ return [b"%s is in a read-only namespace." % ref]
33
34 # If it's a create, the old ref doesn't exist
35 if old == GIT_OID_HEX_ZERO:
36diff --git a/turnip/pack/tests/test_hooks.py b/turnip/pack/tests/test_hooks.py
37index 22c31c7..374097f 100644
38--- a/turnip/pack/tests/test_hooks.py
39+++ b/turnip/pack/tests/test_hooks.py
40@@ -265,6 +265,14 @@ class TestPreReceiveHook(HookTestMixin, TestCase):
41 b"You do not have permission to push to refs/heads/verboten.\n")
42
43 @defer.inlineCallbacks
44+ def test_rejected_readonly_namespaces(self):
45+ # An read-only ref is rejected.
46+ yield self.assertRejected(
47+ [(b'refs/merge/123/heads', self.old_sha1, self.new_sha1)],
48+ {b'refs/heads/master': ['push', 'force_push', 'create']},
49+ b"refs/merge/123/heads is in a read-only namespace.\n")
50+
51+ @defer.inlineCallbacks
52 def test_rejected_multiple(self):
53 # A combination of valid and invalid refs is still rejected.
54 yield self.assertRejected(
55@@ -441,6 +449,14 @@ class TestUpdateHook(TestCase):
56 self.assertEqual(
57 [b'You do not have permission to force-push to ref.'], output)
58
59+ def test_read_only_ref(self):
60+ # User does not have permission to force-push to a read-only namespace
61+ output = hook.match_update_rules(
62+ {'ref': ['create']},
63+ [b'refs/merge/123/head', 'old', 'new'])
64+ self.assertEqual(
65+ [b'refs/merge/123/head is in a read-only namespace.'], output)
66+
67
68 class TestDeterminePermissions(TestCase):
69

Subscribers

People subscribed via source and target branches