Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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()
logins := make([]string, 0)
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
89 changes: 80 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,27 @@ 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. The login is lowercased internally to
// match how UserKey stores it, so callers may pass either the original
// GitHub login or a pre-lowercased form. 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, login string) int {
loginLower := strings.ToLower(login)
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 +560,46 @@ func (c *ProjectUserCache) Clear() {

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

// InvalidateByProject removes every entry for the given project, regardless
// of user. Used after an approval-list mutation (UpdateApprovalList), since
// any cached signed/authorized decision under that project may now be
// stale: users newly added to email/domain/org/github approvals must flip
// red→green, and users removed must flip green→red. Cache misses for
// affected webhooks are then resolved against fresh DDB state on next read.
func (c *ProjectUserCache) InvalidateByProject(projectID string) int {
c.mu.Lock()
defer c.mu.Unlock()
n := 0
for k := range c.data {
if k[0] == projectID {
delete(c.data, k)
n++
}
}
return n
}

// InvalidateByUser removes every entry whose (projectID, id, login) prefix
// matches, regardless of the email component. The login is lowercased
// internally to match how ProjectUserKey stores it, so callers may pass
// either the original GitHub login or a pre-lowercased form. 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, login string) int {
loginLower := strings.ToLower(login)
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 +2084,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 +2111,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
141 changes: 141 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,143 @@ 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")
}

// TestInvalidateByProjectScopesToProject verifies that InvalidateByProject
// drops every entry for the given project regardless of user, but does not
// touch entries for other projects. Used after UpdateApprovalList because
// approval-list mutations may flip authorization in either direction for
// any user who has cached state under that project.
func TestInvalidateByProjectScopesToProject(t *testing.T) {
ModelProjectUserCache.Clear()
t.Cleanup(func() { ModelProjectUserCache.Clear() })

const (
targetProj = "01af041c-0000-0000-0000-000000000000"
otherProj = "ffffffff-1111-2222-3333-444444444444"
)

a := ProjectUserKey(targetProj, "1", "userA", "a@example.com")
b := ProjectUserKey(targetProj, "2", "userB", "b@example.com")
c := ProjectUserKey(otherProj, "3", "userC", "c@example.com")
ModelProjectUserCache.SetWithTTL(a, nil, false, false, NegativeCacheTTL)
ModelProjectUserCache.SetWithTTL(b, nil, true, true, NegativeCacheTTL)
ModelProjectUserCache.SetWithTTL(c, nil, true, true, NegativeCacheTTL)

n := ModelProjectUserCache.InvalidateByProject(targetProj)
assert.Equal(t, 2, n, "must invalidate exactly the target project's entries")

_, _, _, ok := ModelProjectUserCache.Get(a)
assert.False(t, ok, "target-project entry A must be gone")
_, _, _, ok = ModelProjectUserCache.Get(b)
assert.False(t, ok, "target-project entry B must be gone")
_, _, _, ok = ModelProjectUserCache.Get(c)
assert.True(t, ok, "other-project entry C must NOT be touched")
}

// TestInvalidateByUserNormalizesLoginCase verifies that InvalidateByUser
// lowercases the login internally and matches entries regardless of the
// caller's casing. UserKey/ProjectUserKey lowercase the login on insert,
// so callers passing a mixed-case login must still hit those entries.
func TestInvalidateByUserNormalizesLoginCase(t *testing.T) {
ModelProjectUserCache.Clear()
ModelUserCache.Clear()
t.Cleanup(func() {
ModelProjectUserCache.Clear()
ModelUserCache.Clear()
})

const (
projectID = "01af041c-0000-0000-0000-000000000000"
githubID = "12345"
mixedCase = "MixedCaseLogin"
email = "u@example.com"
)

// Insert via canonical keys (login is lowercased by the *Key helpers).
pKey := ProjectUserKey(projectID, githubID, mixedCase, email)
ModelProjectUserCache.SetWithTTL(pKey, nil, false, false, NegativeCacheTTL)
uKey := UserKey(githubID, mixedCase, email)
ModelUserCache.SetWithTTL(uKey, nil, NegativeCacheTTL)

// Caller passes the original mixed-case login (the common mistake).
pn := ModelProjectUserCache.InvalidateByUser(projectID, githubID, mixedCase)
un := ModelUserCache.InvalidateByUser(githubID, mixedCase)

assert.Equal(t, 1, pn, "project cache entry must be invalidated despite mixed-case login")
assert.Equal(t, 1, un, "user cache entry must be invalidated despite mixed-case login")

_, _, _, ok := ModelProjectUserCache.Get(pKey)
assert.False(t, ok, "project cache entry must be gone")
_, ok = ModelUserCache.Get(uKey)
assert.False(t, ok, "user cache entry must be gone")
}
Loading
Loading