Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

vole
Clone

Read only, no credentials:

https://rodent.software/git/vole.git

Write over SSH with a registered key:

git clone ssh://git@0.0.0.0:2222/vole.git

A free and open-source forge that lives in your Git repo.

Let the CLI do what the browser does for the same account

7e0b5950c190caf11089e1aaac10f18678074fed · Keenan Nemetz · 6d ago · parent 6e1fca51

Let the CLI do what the browser does for the same account

A contributor could file an issue, comment, manage the item it wrote, and open a
change from its own branch — but only in the browser, because a push to
`refs/heads/vole` required `maintain`. The role was not the reason: the hook had
no per-path authorization for that branch, so the role gate stood in for one.

The per-path checker is now applied to every session, not only to run
credentials. It asks the ACL the questions the browser's forms ask —
CanDiscuss, CanOpenRequest, CanTriage, CanApprove — so a contributor's key can
write what a contributor may write, and a maintainer's key is unchanged. A
non-maintainer's new file must name that account as its author, which is what the
browser does for it; a maintainer keeps writing an imported issue under its
original author, because that is a trust decision rather than a capability.

Creating the issues branch still needs a maintainer, as it does from the browser,
and an approval still cannot approve its author's own change — the tree validator
already refused that, and now every transport gets the same answer. The one rule
that moved is self-approval for a maintainer's push, which is now refused earlier
rather than only at the landing gate.

With the branch no longer bundled into `maintain`, CONTRIBUTING and the guides stop
telling contributors to use the web. Both transports are one authorization model.

Part of #168, stacked on #167, which built the checker this generalises.

server/hook/scope.go +60−38

@@ -1,17 +1,16 @@
1 1 package hook
2 2
3 // What a job may write.
3 // What a session may write to the issues branch.
4 4 //
5 // A run credential authenticates as the account the job acts as, but it is not that
6 // account's key: it exists for one job, in one repository, until the run ends. The
7 // role still decides what the account may do — the credential does not widen it — and
8 // this file is the one place that turns a metadata diff into that decision.
5 // There are two ways to write there: the browser, one action at a time through a
6 // handler that asks the account's capability, and a push, which is this file. They
7 // have to offer the same actions for the same account, so both ask the ACL the same
8 // questions — CanDiscuss, CanOpenRequest, CanTriage, CanApprove — and this is the
9 // push boundary's half of that: a whole diff, checked path by path.
9 10 //
10 // The rule is the browser's write surface, moved to the push boundary. The browser
11 // already lets a contributor comment, open a change from a branch they may push, and
12 // lets an eligible approver approve; everything else it does is a maintainer action.
13 // A job gets the same list and no more, which is what makes it safe to hand a bot a
14 // key without handing it the role that used to be the only way to write metadata.
11 // Before this existed, "writing the issues branch requires maintain" stood in for the
12 // per-path rules, which is why only the browser could offer a contributor an issue or
13 // a comment. The rules were always expressible; they just had not been asked here.
15 14
16 15 import (
17 16 "fmt"
@@ -49,27 +48,27 @@ func lookupRunCredential(cfg Config, fingerprint string) (*runs.RunCredential, e
49 48 return store.RunCredential(fingerprint, time.Now().UTC())
50 49 }
51 50
52 // scopedMetadata reports what a push to the issues branch may not contain when it
53 // came from a run credential.
51 // writableMetadata reports what a push to the issues branch may not contain. It is
52 // called for every session, key or run credential: the credential changes how a job
53 // authenticates, not what the account may do.
54 54 //
55 // The rule is the account's capabilities, asked the way the browser asks them. A
56 // credential does not make the account stronger or weaker: a maintainer may do what a
57 // maintainer may do, a contributor may file issues, comment, open a change from a
58 // branch it could push, and manage the items it wrote, and an account that may approve
59 // may approve. The credential replaces exactly one requirement — that writing the
60 // issues branch at all needs maintain — and adds no rule of its own.
61 func scopedMetadata(g *git.Repo, c change, acl *access.ACL, actor string) []string {
55 // A maintainer may do what a maintainer may do, which is every write the branch
56 // carries; the one rule that applies to everybody is that an approval must be
57 // somebody else's. A contributor may discuss, file an issue, manage the item it wrote,
58 // and open a change from a branch it could have pushed, which is exactly what the
59 // browser offers the same account.
60 func writableMetadata(g *git.Repo, c change, acl *access.ACL, actor string) []string {
62 61 if c.old == "" || c.old == git.ZeroSHA {
63 return []string{"a job cannot create the issues branch; it must already exist"}
64 }
65 // Triaging other people's items covers every write the issues branch carries, so a
66 // maintainer needs nothing further: what a key may do, a credential may do.
67 if acl.CanTriage(actor) {
68 return nil
62 // Nothing to compare against: the branch does not exist yet, and creating it is
63 // a maintainer's act, exactly as creating it from the browser is.
64 if acl.CanTriage(actor) {
65 return nil
66 }
67 return []string{"creating the issues branch needs a maintainer; it must already exist"}
69 68 }
70 69 status, err := g.DiffStatus(c.old, c.new)
71 70 if err != nil {
72 return []string{"cannot check what this job's push writes: " + err.Error()}
71 return []string{"cannot check what this push writes: " + err.Error()}
73 72 }
74 73 paths := make([]string, 0, len(status))
75 74 for path := range status {
@@ -80,6 +79,24 @@ func scopedMetadata(g *git.Repo, c change, acl *access.ACL, actor string) []stri
80 79 var problems []string
81 80 for _, path := range paths {
82 81 change := status[path]
82 // An approval is an account's statement about its own review, whether it arrives
83 // through the browser or a push. approvalOwnership already requires the file to
84 // name the actor, and ValidateRequests already refuses one that approves its
85 // author's own change; what this adds is the eligibility the browser's approve
86 // button asks for, which the push boundary used to get for free from "writing the
87 // issues branch requires maintain".
88 if metadata.ApprovalPath(path) != "" {
89 if change == "A" && !acl.CanApprove(actor) {
90 problems = append(problems, fmt.Sprintf(
91 "%s: %s may not approve changes here", path, actor))
92 }
93 continue
94 }
95 // Everything else on the branch is a maintainer's to write: triage covers
96 // issues, changes, and the files they carry.
97 if acl.CanTriage(actor) {
98 continue
99 }
83 100 switch {
84 101 case metadata.IsIssueCommentPath(path), metadata.IsRequestCommentPath(path):
85 102 // Adding a comment is discussing. Changing or removing one is the author's,
@@ -99,15 +116,28 @@ func scopedMetadata(g *git.Repo, c change, acl *access.ACL, actor string) []stri
99 116 }
100 117 if author != actor {
101 118 problems = append(problems, fmt.Sprintf(
102 "%s: a job writes as %s, and this comment says %s", path, actor, author))
119 "%s: this push writes as %s, and this comment says %s", path, actor, author))
103 120 }
104 121 case isIssueFile(path):
105 122 id, _ := metadata.IssueIDFromPath(path)
123 if change == "D" {
124 // An issue is closed, not removed; the browser offers no delete, and
125 // neither does the CLI.
126 problems = append(problems, fmt.Sprintf("%s: an issue cannot be removed", path))
127 continue
128 }
106 129 if change == "A" {
107 // Filing an issue is part of discussing.
130 // Filing an issue is part of discussing, and the author is this push:
131 // the browser fills that field in from the session, so a push has to
132 // put the same name there.
108 133 if !acl.CanDiscuss(actor) {
109 134 problems = append(problems, fmt.Sprintf(
110 135 "%s: %s may not file an issue in this repository", path, actor))
136 continue
137 }
138 if author, ok := authorAt(g, c.new, path); !ok || author != actor {
139 problems = append(problems, fmt.Sprintf(
140 "%s: this push files the issue as %s", path, actor))
111 141 }
112 142 continue
113 143 }
@@ -152,7 +182,7 @@ func scopedMetadata(g *git.Repo, c change, acl *access.ACL, actor string) []stri
152 182 }
153 183 if req.Author != actor {
154 184 problems = append(problems, fmt.Sprintf(
155 "%s: a job opens a change as %s, and this one says %s", path, actor, req.Author))
185 "%s: this push opens a change as %s, and this one says %s", path, actor, req.Author))
156 186 }
157 187 // A contributor proposes a branch it could have pushed itself. A bare
158 188 // commit is the maintainer form, and a contributor has no business
@@ -166,17 +196,9 @@ func scopedMetadata(g *git.Repo, c change, acl *access.ACL, actor string) []stri
166 196 "%s: %s may only propose a branch under %s",
167 197 path, actor, access.ContributorBranchPrefix(actor)))
168 198 }
169 case metadata.ApprovalPath(path) != "":
170 // approvalOwnership already requires the file to name the actor; this is
171 // the eligibility the browser checks, which the push boundary used to get
172 // for free from "writing the issues branch requires maintain".
173 if !acl.CanApprove(actor) {
174 problems = append(problems, fmt.Sprintf(
175 "%s: %s may not approve changes here", path, actor))
176 }
177 199 default:
178 200 problems = append(problems, fmt.Sprintf(
179 "%s: a job may write a comment, an issue, a change request, or an approval", path))
201 "%s: a contributor may write a comment, an issue, or a change request", path))
180 202 }
181 203 }
182 204 return problems