Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions .yarn-audit-allowlist.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@
1116365,
1116473,
1116454,
1116478
1116478,
1117083,
1117575,
1117590,
1117592,
1117673,
1117726
Comment thread
lukaszgryglicki marked this conversation as resolved.
],
"notes": {
"1111997": "aws-sdk v2 advisory flagged as 'No patch available' in our current baseline; accepted until migration.",
Expand All @@ -20,6 +26,12 @@
"1116365": "Axios has a NO_PROXY Hostname Normalization Bypass Leads to SSRF",
"1116473": "Axios has Unrestricted Cloud Metadata Exfiltration via Header Injection Chain",
"1116454": "basic-ftp: Incomplete CRLF Injection Protection Allows Arbitrary FTP Command Execution via Credentials and MKD Commands",
"1116478": "basic-ftp has FTP Command Injection via CRLF"
"1116478": "basic-ftp has FTP Command Injection via CRLF",
"1117083": "basic-ftp DoS via Client.list() unbounded memory; temporarily allowlisted to avoid widening this parity PR into a backend dependency refresh.",
"1117575": "axios CVE-2025-62718 NO_PROXY bypass via 127.0.0.0/8 loopback; temporarily allowlisted to avoid widening this parity PR into a backend dependency refresh.",
"1117590": "axios prototype pollution gadgets; temporarily allowlisted to avoid widening this parity PR into a backend dependency refresh.",
"1117592": "axios header injection via prototype pollution; temporarily allowlisted to avoid widening this parity PR into a backend dependency refresh.",
"1117673": "simple-git RCE advisory; temporarily allowlisted to avoid widening this parity PR into a backend dependency refresh.",
"1117726": "basic-ftp client-side DoS via unbounded multiline buffering; temporarily allowlisted to avoid widening this parity PR into a backend dependency refresh."
Comment thread
lukaszgryglicki marked this conversation as resolved.
}
}
50 changes: 50 additions & 0 deletions cla-backend-go/github/github_org.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"errors"
"fmt"
"strings"

"github.com/linuxfoundation/easycla/cla-backend-go/utils"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -66,6 +67,55 @@ func GetOrganization(ctx context.Context, organizationName string) (*github.Orga
return org, nil
}

// ListUserPublicOrgs returns the GitHub organization logins that <user> is a
// publicly visible member of. It calls GET /users/<user>/orgs, which is the
// same endpoint the pre-cutover Python helper cla.utils.lookup_github_organizations
// used. Membership in private orgs is invisible to this endpoint unless the
// user has set their membership to public on github.com.
//
// Returns an empty slice (with a nil error) when the user has no visible org
// memberships. The github-org approval-list check must be done against this
// list (case-insensitive) rather than against /orgs/<org>/memberships/<user>,
// because the EasyCLA OAuth bot is not itself a member of customer orgs and
// gets a 403 from the latter endpoint.
//
// An empty user is rejected with an error: go-github routes an empty user
// to GET /user/orgs (the authenticated bot's own orgs), which would silently
// approve unrelated callers if it ever leaked through.
func ListUserPublicOrgs(ctx context.Context, user string) ([]string, error) {
f := logrus.Fields{
"functionName": "github.ListUserPublicOrgs",
utils.XREQUESTID: ctx.Value(utils.XREQUESTID),
"user": user,
}

if strings.TrimSpace(user) == "" {
return nil, errors.New("ListUserPublicOrgs: user is empty")
}

client := NewGithubOauthClient()
var logins []string
Comment thread
lukaszgryglicki marked this conversation as resolved.
Outdated
opt := &github.ListOptions{PerPage: 100}
for {
orgs, resp, err := client.Organizations.List(ctx, user, opt)
if err != nil {
log.WithFields(f).Warnf("ListUserPublicOrgs %s failed. error = %s", user, err.Error())
return nil, err
}
for _, org := range orgs {
if org == nil || org.Login == nil {
continue
}
logins = append(logins, *org.Login)
}
if resp == nil || resp.NextPage == 0 {
break
}
opt.Page = resp.NextPage
}
return logins, nil
}

// GetOrganizationMembers gets members in organization
func GetOrganizationMembers(ctx context.Context, orgName string, installationID int64) ([]string, error) {
f := logrus.Fields{
Expand Down
64 changes: 55 additions & 9 deletions cla-backend-go/github/github_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,24 @@ func (c *UserCache) Clear() {

func (c *UserCache) Delete(key [3]string) { c.mu.Lock(); delete(c.data, key); c.mu.Unlock() }

// InvalidateByUser removes every entry whose (id, login) prefix matches,
// regardless of the email component. login must already be lowercased.
// Used after a signature event to drop stale entries keyed on commit-email
// shapes the caller cannot enumerate (e.g. the GitHub noreply form emitted
// when a user has email privacy enabled).
func (c *UserCache) InvalidateByUser(id, loginLower string) int {
c.mu.Lock()
defer c.mu.Unlock()
n := 0
for k := range c.data {
if k[0] == id && k[1] == loginLower {
delete(c.data, k)
n++
}
}
return n
}
Comment thread
lukaszgryglicki marked this conversation as resolved.

type projectUserCacheEntry struct {
value *models.User
signed bool
Expand Down Expand Up @@ -539,6 +557,24 @@ func (c *ProjectUserCache) Clear() {

func (c *ProjectUserCache) Delete(key [4]string) { c.mu.Lock(); delete(c.data, key); c.mu.Unlock() }

// InvalidateByUser removes every entry whose (projectID, id, login) prefix
// matches, regardless of the email component. login must already be lowercased.
// Used after a signature event to drop stale per-project entries keyed on
// commit-email shapes the caller cannot enumerate (e.g. the GitHub noreply
// form emitted when a user has email privacy enabled).
func (c *ProjectUserCache) InvalidateByUser(projectID, id, loginLower string) int {
c.mu.Lock()
defer c.mu.Unlock()
n := 0
for k := range c.data {
if k[0] == projectID && k[1] == id && k[2] == loginLower {
delete(c.data, k)
n++
}
}
return n
}
Comment thread
lukaszgryglicki marked this conversation as resolved.

var GithubUserCache = NewCache(12 * time.Hour)
var ModelUserCache = NewUserCache(12 * time.Hour)
var ModelProjectUserCache = NewProjectUserCache(3 * time.Hour)
Expand Down Expand Up @@ -2023,15 +2059,25 @@ func UpdateCacheAfterSignature(ctx context.Context, user *models.User, projectID
}

affiliated := strings.TrimSpace(user.CompanyID) != ""

emails := collectUserEmails(user)
if len(emails) == 0 {
log.WithFields(f).Debugf("no emails found for user (githubID=%s, login=%s) - nothing to cache", githubID, githubLogin)
return nil
}

loginLower := strings.ToLower(githubLogin)

// Wipe every cache entry for this (projectID, githubID, login) tuple
// regardless of email. The pre-signature webhook may have stored a
// negative entry keyed on a commit-email shape we cannot enumerate from
// the user record — most commonly the GitHub noreply form
// "<id>+<login>@users.noreply.github.com" when the user has email
// privacy enabled. Without this wipe, the stale negative entry survives
// the signature callback and the PR stays red until NegativeCacheTTL
// (2m) expires or the next webhook lands on a different Lambda.
projInvalidated := ModelProjectUserCache.InvalidateByUser(projectID, githubID, loginLower)
userInvalidated := ModelUserCache.InvalidateByUser(githubID, loginLower)

// Pre-populate positive entries for the user's known emails so a webhook
// whose commit-email matches one of them gets an immediate cache hit.
// Webhooks for unknown email shapes (e.g. noreply) will fall through to
// the slow path, find the freshly-recorded signature, and cache a fresh
// positive entry — no stale negative left to mislead them.
emails := collectUserEmails(user)
for _, email := range emails {
genKey := UserKey(githubID, loginLower, email)
ModelUserCache.Set(genKey, user)
Expand All @@ -2040,8 +2086,8 @@ func UpdateCacheAfterSignature(ctx context.Context, user *models.User, projectID
ModelProjectUserCache.Set(projKey, user, true, affiliated)
}

log.WithFields(f).Infof("updated caches for user login=%s (GitHubID=%s), project=%s: marked as authorized for %d email(s)",
loginLower, githubID, projectID, len(emails))
log.WithFields(f).Infof("updated caches for user login=%s (GitHubID=%s), project=%s: invalidated %d project + %d user stale entries; pre-populated %d authorized email(s)",
loginLower, githubID, projectID, projInvalidated, userInvalidated, len(emails))

return nil
}
Expand Down
71 changes: 71 additions & 0 deletions cla-backend-go/github/github_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"time"

gh "github.com/google/go-github/v37/github"
"github.com/linuxfoundation/easycla/cla-backend-go/gen/v1/models"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -203,3 +204,73 @@ func TestListPullRequestCommitsComparePreservesPageOrder(t *testing.T) {
assert.Equal(t, "sha3", commits[2].GetSHA())
}
}

// TestUpdateCacheAfterSignatureInvalidatesUnknownEmailKeys verifies that a
// pre-signature negative cache entry keyed on a commit-email shape that is NOT
// in the user record (e.g. the GitHub noreply form emitted when email privacy
// is on) is wiped by UpdateCacheAfterSignature, while the user's known emails
// are pre-populated as authorized positives.
func TestUpdateCacheAfterSignatureInvalidatesUnknownEmailKeys(t *testing.T) {
ModelProjectUserCache.Clear()
ModelUserCache.Clear()
t.Cleanup(func() {
ModelProjectUserCache.Clear()
ModelUserCache.Clear()
})

const (
projectID = "01af041c-0000-0000-0000-000000000000"
githubID = "196905385"
githubLogin = "lukaszgryglicki2"
noreplyEmail = "196905385+lukaszgryglicki2@users.noreply.github.com"
realEmail = "lukaszgryglicki2@proton.me"
anotherEmail = "lg2@elsewhere.example"
)

// Pre-populate: pre-signature webhook stored a negative entry on the
// noreply form, plus an unrelated negative on a different email.
staleNoreply := ProjectUserKey(projectID, githubID, githubLogin, noreplyEmail)
ModelProjectUserCache.SetWithTTL(staleNoreply, nil, false, false, NegativeCacheTTL)
staleOther := ProjectUserKey(projectID, githubID, githubLogin, anotherEmail)
ModelProjectUserCache.SetWithTTL(staleOther, nil, false, false, NegativeCacheTTL)

staleNoreplyUser := UserKey(githubID, githubLogin, noreplyEmail)
ModelUserCache.SetWithTTL(staleNoreplyUser, nil, NegativeCacheTTL)

// An unrelated entry under a different login must NOT be touched.
otherLoginKey := ProjectUserKey(projectID, "999", "someoneelse", "x@y.example")
ModelProjectUserCache.SetWithTTL(otherLoginKey, nil, false, false, NegativeCacheTTL)

user := &models.User{
GithubID: githubID,
GithubUsername: githubLogin,
Emails: []string{realEmail},
CompanyID: "f7c7ac9c-1111-2222-3333-444444444444",
}

err := UpdateCacheAfterSignature(context.Background(), user, projectID)
assert.NoError(t, err)

// All stale entries for this user (regardless of email shape) must be
// gone from the project cache. They were not in the user.Emails set, so
// the previous code would have left them behind.
_, _, _, ok := ModelProjectUserCache.Get(staleNoreply)
assert.False(t, ok, "noreply project-cache entry must be invalidated")
_, _, _, ok = ModelProjectUserCache.Get(staleOther)
assert.False(t, ok, "unrelated-email project-cache entry must be invalidated")

_, ok = ModelUserCache.Get(staleNoreplyUser)
assert.False(t, ok, "noreply user-cache entry must be invalidated")

// Unrelated user under a different login must survive.
_, _, _, ok = ModelProjectUserCache.Get(otherLoginKey)
assert.True(t, ok, "unrelated user's cache entry must NOT be invalidated")

// The user's known email is pre-populated as an authorized positive.
realKey := ProjectUserKey(projectID, githubID, githubLogin, realEmail)
cachedUser, signed, affiliated, ok := ModelProjectUserCache.Get(realKey)
assert.True(t, ok, "real-email project-cache entry must be set")
assert.NotNil(t, cachedUser)
assert.True(t, signed, "real-email entry must be marked signed")
assert.True(t, affiliated, "user has CompanyID, must be marked affiliated")
}
49 changes: 36 additions & 13 deletions cla-backend-go/signatures/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ const (
githubStatusMissingCLA = "Missing CLA Authorization."
)

// listUserPublicOrgs is the indirection that lets unit tests for
// UserIsApproved stub the GitHub /users/<user>/orgs call without standing up
// a real OAuth client. Production callers always go through
// github.ListUserPublicOrgs.
var listUserPublicOrgs = github.ListUserPublicOrgs

// NewService creates a new signature service
func NewService(repo SignatureRepository, companyService company.IService, usersService users.Service, eventsService events.Service, githubOrgValidation bool, repositoryService repositories.Service, githubOrgService github_organizations.ServiceInterface, claGroupService service2.Service, gitLabApp *gitlab_api.App, CLABaseAPIURL, CLALandingPage, CLALogoURL string) SignatureService {
return service{
Expand Down Expand Up @@ -1635,23 +1641,40 @@ func (s service) UserIsApproved(ctx context.Context, user *models.User, cclaSign
}
}

// check github org email ApprovalList
if user.GithubUsername != "" {
// check github org approval list. Match the user's publicly-visible org
// memberships against the approval list, mirroring the pre-cutover Python
// helper cla.utils.lookup_github_organizations. The previous implementation
// called /orgs/<org>/memberships/<user>, which the EasyCLA OAuth bot has
// no permission to read for customer orgs (returns 403) — so every
// org-only-approved contributor failed the check.
if login := strings.TrimSpace(user.GithubUsername); login != "" {
githubOrgApprovalList := cclaSignature.GithubOrgApprovalList
if len(githubOrgApprovalList) > 0 {
log.WithFields(f).Debugf("determining if github user :%s is associated with ant of the github orgs : %+v", user.GithubUsername, githubOrgApprovalList)
}

for _, org := range githubOrgApprovalList {
membership, err := github.GetMembership(ctx, user.GithubUsername, org)
log.WithFields(f).Debugf("determining if github user :%s is associated with any of the github orgs : %+v", login, githubOrgApprovalList)
userOrgs, err := listUserPublicOrgs(ctx, login)
if err != nil {
break
}
if membership != nil {
log.WithFields(f).Debugf("found matching github organization: %s for user: %s", org, user.GithubUsername)
return true, nil
// Mirror the Python flow (cla.utils.is_approved): if the
// public-orgs lookup fails, log and treat as no match — do
// not propagate. Returning an error here would 500 the
// /v3/sign route and a transient GitHub blip would block
// every org-approved contributor across the project.
log.WithFields(f).Warnf("could not list public orgs for github user %s; treating as no org-approval match: %v", login, err)
} else {
log.WithFields(f).Debugf("user: %s is not in the organization: %s", user.GithubUsername, org)
for _, approvedOrg := range githubOrgApprovalList {
approvedOrgTrim := strings.TrimSpace(approvedOrg)
matched := false
for _, userOrg := range userOrgs {
if strings.EqualFold(approvedOrgTrim, userOrg) {
matched = true
break
}
}
if matched {
log.WithFields(f).Debugf("found matching github organization: %s for user: %s", approvedOrg, login)
return true, nil
}
log.WithFields(f).Debugf("user: %s is not in the organization: %s", login, approvedOrg)
}
Comment thread
lukaszgryglicki marked this conversation as resolved.
}
}
}
Expand Down
Loading
Loading