chore: update baton-github to v0.4.0 - #26
Conversation
Picks up repository grant-replacement handling for AddCollaborator (reports the previous role as replaced instead of silently overwriting it) and a fail-closed fix for org default_repository_permission when the field is empty/unreadable (previously assumed "read", now "none"). Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| collaborator, resp, err := o.client.Repositories.IsCollaborator(ctx, repo.GetOwner().GetLogin(), repo.GetName(), user.GetLogin()) | ||
| if err != nil { | ||
| return nil, wrapGitHubError(err, resp, "github-connector: failed to check if user is a collaborator") | ||
| } | ||
|
|
||
| var replacedGrantID string | ||
| if collaborator { | ||
| permLevel, resp, err := o.client.Repositories.GetPermissionLevel(ctx, repo.GetOwner().GetLogin(), repo.GetName(), user.GetLogin()) | ||
| if err != nil { | ||
| return nil, wrapGitHubError(err, resp, "github-connector: failed to get user's repository permission") | ||
| } | ||
|
|
||
| prevPermission := roleNameToRepoPermission(permLevel.GetRoleName()) | ||
| if prevPermission == "" { | ||
| // Custom repository role: fall back to the coarse permission (read/write/admin). | ||
| prevPermission = roleNameToRepoPermission(permLevel.GetPermission()) | ||
| } | ||
|
|
||
| switch prevPermission { | ||
| case permission: | ||
| return annotations.New(&v2.GrantAlreadyExists{}), nil |
There was a problem hiding this comment.
🟠 Bug: IsCollaborator and GetPermissionLevel report effective access — GitHub counts org members with access via a team or via default_repository_permission as collaborators. So a user whose only push access comes from a team makes prevPermission == permission, and this returns GrantAlreadyExists without ever calling AddCollaborator. C1 records the grant as fulfilled, but no direct collaborator record is created, so removing the user from the team silently drops the access. This is a regression versus v0.3.10, where AddCollaborator always ran; it is most visible with direct-collaborators-only, where the sync (affiliation direct) will keep showing no grant at all. Fix belongs upstream in baton-github: restrict the short-circuit to direct collaborators (e.g. ListCollaborators with Affiliation: "direct", or check permLevel.GetUser().GetRoleName() against the direct-collaborator source) rather than effective permission. (confidence: medium-high)
| // AddCollaborator overwrites the user's existing role, so report the | ||
| // old role's grant as replaced. GitHub permissions are cumulative; | ||
| // grants for other implied flags are reconciled at the next sync. | ||
| replacedGrantID = grant.NewGrantID(principal, &v2.Entitlement{ | ||
| Id: entitlement.NewEntitlementID(en.Resource, prevPermission), | ||
| }) | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: with the default direct-collaborators-only=false, Grants() emits user-principal grants for team- and org-base-derived access too (ListCollaborators with affiliation all, line 233). prevPermission is the effective permission, so this can emit GrantReplaced for a grant that was never replaced — e.g. pull inherited from the org base permission survives AddCollaborator(push), but C1 drops it until the next sync reconciles. Worth scoping the GrantReplaced to the direct-collaborator permission upstream, or at least noting the transient inconsistency in the PR description. (confidence: medium)
| if perm == "" { | ||
| perm = "read" // GitHub default | ||
| l.Debug( | ||
| "baton-github: org default_repository_permission missing or empty; skipping org-member repo expansion (treating as none). "+ | ||
| "Grant the credential org-owner visibility (admin:org / Organization Administration) to sync base-permission grants accurately.", | ||
| zap.String("org", orgName), | ||
| zap.String("org_id", orgResourceID.Resource), | ||
| ) | ||
| perm = "none" |
There was a problem hiding this comment.
🟡 Suggestion: this fail-closed change is default-on and removes grants for existing installs. docs/connector.mdx:77 currently presents read:org as an acceptable alternative to admin:org, but default_repository_permission is only returned to org owners / admin:org, so a read:org install running with direct-collaborators-only will lose every org-member-derived repo grant on the next sync — and the only signal is a Debug log. Update docs/connector.mdx to state that admin:org is required for accurate base-permission grants (BP3/D3), and consider surfacing this at Warn since it reflects a credential-scope gap (L1) rather than a routinely-absent field. (confidence: high on the docs gap, medium on the log level)
Connector PR Review: chore: update baton-github to v0.4.0Blocking Issues: 1 | Suggestions: 2 | Threads Resolved: 0 Review SummaryScanned the full PR diff for security and correctness: Security IssuesNone found. Correctness Issues
Suggestions
Prompt for AI agents |
2 participants
Summary
github.com/conductorone/baton-githubfrom v0.3.10 to v0.4.0 and re-vendors.Grant(AddCollaborator) now detects an existing collaborator role and returns aGrantReplacedannotation instead of silently overwriting the prior permission grant.getOrgBasePermissionnow treats a missing/emptydefault_repository_permissionas"none"(fail closed) instead of assuming GitHub's create-time default of"read", which previously could invent pull grants for every org member when the credential lacked org-owner visibility.Test plan
go build ./...go test ./...go vet ./...vendor/github.com/conductorone/baton-github/pkg/connector/repository.go) — only file with content changes besidesgo.mod/go.sum/vendor/modules.txt