Skip to content

Commit 445e6f9

Browse files
committed
Add Library Registry access control system
Background ---------- The Arduino Library Registry repository receives thousands of pull requests from a large number of community contributors. The great majority of these contributors behave in a responsible manner. Unfortunately this repository is regularly the subject of irresponsible behavior. The small number of people who behave irresponsibly consume a significant amount of the finite maintenance resources available for maintenance of Arduino's repositories. Communication is always the first measure taken in these cases. This is done automatically by the bot, and then by the registry maintainer when it becomes clear that the user has disregarded the comments from the bot. Unfortunately it is regularly the case that the user simply disregards all communication and continues their pattern of irresponsible behavior unchecked. Alternatives ------------ GitHub provides tools for dealing with harmful behavior: - Report user - Block user Reporting a user is the appropriate measure in cases of malicious behavior, and the account is usually banned from the site relatively quickly after a legitimate report is made. However, the irresponsible behavior in the registry repository is not overtly malicious and so reporting the user in these cases would not be appropriate or effective. At first glance, the block feature seems ideal. However, it can only be done at an organization-wide level, and by an organization administrator. The repository maintainer is not an organization administrator, so this makes the feature inconvenient to use. There is no sign of these users interacting with other repositories in the `arduino` organization, and so there is no benefit to blocking them at organization scope. In addition, in order to make it more difficult to circumvent the access restriction, we need the ability to block requests for libraries owned by an entity who has established a pattern of irresponsible behavior, regardless of which user submits the request. So the tools provided by GitHub are not suitable and a bespoke system must be implemented. Access Levels ------------- Allow: the user may submit requests for any library, even if registry privileges have been revoked for the owner of the library's repository. This access level will only be granted to registry maintainers, in order to allow them to make exceptions for specific libraries owned by an entity whose privileges have been revoked. Default: the user may submit requests for any library, unless registry privileges have been revoked for the owner of the library's repository. Deny: the user may not submit requests. Requests from users with "default" access level for any library repository owned by the entity (user or organization) are denied.
1 parent 77e2d18 commit 445e6f9

File tree

34 files changed

+305
-24
lines changed

34 files changed

+305
-24
lines changed

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ require (
77
github.com/arduino/go-properties-orderedmap v1.8.1
88
github.com/sourcegraph/go-diff v0.7.0
99
github.com/stretchr/testify v1.10.0
10+
gopkg.in/yaml.v3 v3.0.1
1011
)
1112

1213
require (
1314
github.com/davecgh/go-spew v1.1.1 // indirect
1415
github.com/pmezard/go-difflib v1.0.0 // indirect
15-
gopkg.in/yaml.v3 v3.0.1 // indirect
1616
)

main.go

Lines changed: 107 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"strings"
3535

3636
"github.com/sourcegraph/go-diff/diff"
37+
"gopkg.in/yaml.v3"
3738

3839
"github.com/arduino/go-paths-helper"
3940
properties "github.com/arduino/go-properties-orderedmap"
@@ -67,6 +68,29 @@ var recommendedOrganizations []string = []string{
6768
"github.com/adafruit",
6869
}
6970

71+
// accessType is the type of the access control level.
72+
type accessType string
73+
74+
const (
75+
// Allow allows unrestricted access. The entity can make requests even for library repositories whose owner is denied
76+
// access.
77+
Allow accessType = "allow"
78+
// Default gives default access. The entity can make requests as long as the owner of the library's repository is not
79+
// denied access.
80+
Default = "default"
81+
// Deny denies all access. The entity is not permitted to make requests, and registration of libraries from
82+
// repositories they own is not permitted.
83+
Deny = "deny"
84+
)
85+
86+
// accessDataType is the type of the access control data.
87+
type accessDataType struct {
88+
Access accessType `yaml:"access"` // Access level.
89+
Host string `yaml:"host"` // Account host (e.g., `github.com`).
90+
Name string `yaml:"name"` // User or organization account name.
91+
Reference string `yaml:"reference"` // URL that provides additional information about the access control entry.
92+
}
93+
7094
// request is the type of the request data.
7195
type request struct {
7296
Type string `json:"type"` // Request type.
@@ -89,14 +113,23 @@ type submissionType struct {
89113
}
90114

91115
// Command line flags.
116+
// Path of the access control file, relative to repopath.
117+
var accesslistArgument = flag.String("accesslist", "", "")
92118
var diffPathArgument = flag.String("diffpath", "", "")
93119
var repoPathArgument = flag.String("repopath", "", "")
94120
var listNameArgument = flag.String("listname", "", "")
95121

122+
// GitHub username of the user making the submission.
123+
var submitterArgument = flag.String("submitter", "", "")
124+
96125
func main() {
97126
// Validate flag input.
98127
flag.Parse()
99128

129+
if *accesslistArgument == "" {
130+
errorExit("--accesslist flag is required")
131+
}
132+
100133
if *diffPathArgument == "" {
101134
errorExit("--diffpath flag is required")
102135
}
@@ -109,8 +142,18 @@ func main() {
109142
errorExit("--listname flag is required")
110143
}
111144

145+
if *submitterArgument == "" {
146+
errorExit("--submitter flag is required")
147+
}
148+
149+
accesslistPath := paths.New(*repoPathArgument, *accesslistArgument)
150+
exist, err := accesslistPath.ExistCheck()
151+
if !exist {
152+
errorExit("Access control file not found")
153+
}
154+
112155
diffPath := paths.New(*diffPathArgument)
113-
exist, err := diffPath.ExistCheck()
156+
exist, err = diffPath.ExistCheck()
114157
if !exist {
115158
errorExit("diff file not found")
116159
}
@@ -121,23 +164,59 @@ func main() {
121164
errorExit(fmt.Sprintf("list file %s not found", listPath))
122165
}
123166

124-
// Parse the PR diff.
125-
rawDiff, err := diffPath.ReadFile()
167+
rawAccessList, err := accesslistPath.ReadFile()
126168
if err != nil {
127169
panic(err)
128170
}
171+
172+
// Unmarshal access control file.
173+
var accessList []accessDataType
174+
err = yaml.Unmarshal(rawAccessList, &accessList)
175+
if err != nil {
176+
errorExit(fmt.Sprintf("Access control file has invalid format:\n\n%s", err))
177+
}
178+
129179
var req request
130180
var submissionURLs []string
131-
req.Type, req.Error, req.ArduinoLintLibraryManagerSetting, submissionURLs = parseDiff(rawDiff, *listNameArgument)
181+
182+
// Determine access level of submitter.
183+
var submitterAccess accessType = Default
184+
for _, accessData := range accessList {
185+
if accessData.Host == "github.com" && *submitterArgument == accessData.Name {
186+
submitterAccess = accessData.Access
187+
if submitterAccess == Deny {
188+
req.Type = "declined"
189+
req.Error = fmt.Sprintf("Library registry privileges for @%s have been revoked.%%0ASee: %s", *submitterArgument, accessData.Reference)
190+
}
191+
break
192+
}
193+
}
194+
195+
if req.Error == "" {
196+
// Parse the PR diff.
197+
rawDiff, err := diffPath.ReadFile()
198+
if err != nil {
199+
panic(err)
200+
}
201+
req.Type, req.Error, req.ArduinoLintLibraryManagerSetting, submissionURLs = parseDiff(rawDiff, *listNameArgument)
202+
}
132203

133204
// Process the submissions.
134205
var indexEntries []string
135206
var indexerLogsURLs []string
207+
allowedSubmissions := false
136208
for _, submissionURL := range submissionURLs {
137-
submission, indexEntry := populateSubmission(submissionURL, listPath)
209+
submission, indexEntry, allowed := populateSubmission(submissionURL, listPath, accessList, submitterAccess)
138210
req.Submissions = append(req.Submissions, submission)
139211
indexEntries = append(indexEntries, indexEntry)
140212
indexerLogsURLs = append(indexerLogsURLs, indexerLogsURL(submission.NormalizedURL))
213+
if allowed {
214+
allowedSubmissions = true
215+
}
216+
}
217+
if len(submissionURLs) > 0 && !allowedSubmissions {
218+
// If none of the submissions are allowed, decline the request.
219+
req.Type = "declined"
141220
}
142221

143222
// Check for duplicates within the submission itself.
@@ -240,7 +319,7 @@ func parseDiff(rawDiff []byte, listName string) (string, string, string, []strin
240319
}
241320

242321
// populateSubmission does the checks on the submission that aren't provided by Arduino Lint and gathers the necessary data on it.
243-
func populateSubmission(submissionURL string, listPath *paths.Path) (submissionType, string) {
322+
func populateSubmission(submissionURL string, listPath *paths.Path, accessList []accessDataType, submitterAccess accessType) (submissionType, string, bool) {
244323
indexSourceSeparator := "|"
245324
var submission submissionType
246325

@@ -250,37 +329,48 @@ func populateSubmission(submissionURL string, listPath *paths.Path) (submissionT
250329
submissionURLObject, err := url.Parse(submission.SubmissionURL)
251330
if err != nil {
252331
submission.Error = fmt.Sprintf("Invalid submission URL (%s)", err)
253-
return submission, ""
332+
return submission, "", true
254333
}
255334

256335
// Check if URL is accessible.
257336
httpResponse, err := http.Get(submissionURLObject.String())
258337
if err != nil {
259338
submission.Error = fmt.Sprintf("Unable to load submission URL: %s", err)
260-
return submission, ""
339+
return submission, "", true
261340
}
262341
if httpResponse.StatusCode != http.StatusOK {
263342
submission.Error = "Unable to load submission URL. Is the repository public?"
264-
return submission, ""
343+
return submission, "", true
265344
}
266345

267346
// Resolve redirects and normalize.
268347
normalizedURLObject := normalizeURL(httpResponse.Request.URL)
269348

270349
submission.NormalizedURL = normalizedURLObject.String()
271350

351+
if submitterAccess != Allow {
352+
// Check library repository owner access.
353+
for _, accessData := range accessList {
354+
ownerSlug := fmt.Sprintf("%s/%s", accessData.Host, accessData.Name)
355+
if accessData.Access == Deny && uRLIsUnder(normalizedURLObject, []string{ownerSlug}) {
356+
submission.Error = fmt.Sprintf("Library registry privileges for library repository owner `%s` have been revoked.%%0ASee: %s", ownerSlug, accessData.Reference)
357+
return submission, "", false
358+
}
359+
}
360+
}
361+
272362
// Check if URL is from a supported Git host.
273363
if !uRLIsUnder(normalizedURLObject, supportedHosts) {
274364
submission.Error = fmt.Sprintf("`%s` is not currently supported as a Git hosting website for Library Manager.%%0A%%0ASee: https://github.com/arduino/library-registry/blob/main/FAQ.md#what-are-the-requirements-for-a-library-to-be-added-to-library-manager", normalizedURLObject.Host)
275-
return submission, ""
365+
return submission, "", true
276366
}
277367

278368
// Check if URL is a Git repository
279369
err = exec.Command("git", "ls-remote", normalizedURLObject.String()).Run()
280370
if err != nil {
281371
if _, ok := err.(*exec.ExitError); ok {
282372
submission.Error = "Submission URL is not a Git clone URL (e.g., `https://github.com/arduino-libraries/Servo`)."
283-
return submission, ""
373+
return submission, "", true
284374
}
285375

286376
panic(err)
@@ -299,7 +389,7 @@ func populateSubmission(submissionURL string, listPath *paths.Path) (submissionT
299389
normalizedListURLObject := normalizeURL(listURLObject)
300390
if normalizedListURLObject.String() == normalizedURLObject.String() {
301391
submission.Error = "Submission URL is already in the Library Manager index."
302-
return submission, ""
392+
return submission, "", true
303393
}
304394
}
305395

@@ -344,7 +434,7 @@ func populateSubmission(submissionURL string, listPath *paths.Path) (submissionT
344434
}
345435
if string(tagList) == "" {
346436
submission.Error = "The repository has no tags. You need to create a [release](https://docs.github.com/en/github/administering-a-repository/managing-releases-in-a-repository) or [tag](https://git-scm.com/docs/git-tag) that matches the `version` value in the library's library.properties file."
347-
return submission, ""
437+
return submission, "", true
348438
}
349439
latestTag, err := exec.Command("git", "describe", "--tags", strings.TrimSpace(string(tagList))).Output()
350440
if err != nil {
@@ -362,18 +452,18 @@ func populateSubmission(submissionURL string, listPath *paths.Path) (submissionT
362452
libraryPropertiesPath := submissionClonePath.Join("library.properties")
363453
if !libraryPropertiesPath.Exist() {
364454
submission.Error = "Library is missing a library.properties metadata file.%0A%0ASee: https://arduino.github.io/arduino-cli/latest/library-specification/#library-metadata"
365-
return submission, ""
455+
return submission, "", true
366456
}
367457
libraryProperties, err := properties.LoadFromPath(libraryPropertiesPath)
368458
if err != nil {
369459
submission.Error = fmt.Sprintf("Invalid library.properties file: %s%%0A%%0ASee: https://arduino.github.io/arduino-cli/latest/library-specification/#library-metadata", err)
370-
return submission, ""
460+
return submission, "", true
371461
}
372462
var ok bool
373463
submission.Name, ok = libraryProperties.GetOk("name")
374464
if !ok {
375465
submission.Error = "library.properties is missing a name field.%0A%0ASee: https://arduino.github.io/arduino-cli/latest/library-specification/#library-metadata"
376-
return submission, ""
466+
return submission, "", true
377467
}
378468

379469
// Assemble Library Manager index source entry string
@@ -386,7 +476,7 @@ func populateSubmission(submissionURL string, listPath *paths.Path) (submissionT
386476
indexSourceSeparator,
387477
)
388478

389-
return submission, indexEntry
479+
return submission, indexEntry, true
390480
}
391481

392482
// normalizeURL converts the URL into the standardized format used in the index.

0 commit comments

Comments
 (0)