From 02b15e78c9ecc08cfa5f2913f7334db9e96daa8f Mon Sep 17 00:00:00 2001 From: Lukasz Gryglicki Date: Tue, 9 Jun 2026 08:21:58 +0200 Subject: [PATCH 1/9] Followup SSS integration Signed-off-by: Lukasz Gryglicki Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot) Assisted by [Claude](https://claude.ai) --- cla-backend-go/company/projections.go | 1 + cla-backend-go/company/repository.go | 16 +++++++++ cla-backend-go/signatures/service.go | 19 ++++++++++ cla-backend-go/v2/gitlab-activity/service.go | 10 +++++- cla-backend-go/v2/sign/service.go | 35 ++++++++++++++++--- cla-backend-legacy/internal/api/handlers.go | 14 ++++++++ .../internal/store/companies.go | 27 ++++++++++++-- 7 files changed, 114 insertions(+), 8 deletions(-) diff --git a/cla-backend-go/company/projections.go b/cla-backend-go/company/projections.go index 32d54249f..ad40e95e9 100644 --- a/cla-backend-go/company/projections.go +++ b/cla-backend-go/company/projections.go @@ -19,6 +19,7 @@ func buildCompanyProjection() expression.ProjectionBuilder { expression.Name("date_modified"), expression.Name("note"), expression.Name("is_sanctioned"), + expression.Name("sanction_origin"), expression.Name("version"), ) } diff --git a/cla-backend-go/company/repository.go b/cla-backend-go/company/repository.go index fc53a7642..9e5ae36c7 100644 --- a/cla-backend-go/company/repository.go +++ b/cla-backend-go/company/repository.go @@ -1318,7 +1318,23 @@ func (repo repository) UpdateCompanySanctionStatus(ctx context.Context, companyI UpdateExpression: aws.String(updateExpr), } + // When SSS sets a block, never overwrite a manual/admin block (is_sanctioned=true + // with absent or non-"sss" origin). Only set the SSS flag when the company is + // currently unblocked or already SSS-blocked. A ConditionalCheckFailedException + // therefore means a manual/admin block is already in place and must be preserved. + sssSettingBlock := sanctioned && origin == "sss" + if sssSettingBlock { + values[":false"] = &dynamodb.AttributeValue{BOOL: aws.Bool(false)} + input.ConditionExpression = aws.String("attribute_not_exists(#S) OR #S = :false OR #O = :o") + } + if _, err := repo.dynamoDBClient.UpdateItem(input); err != nil { + if sssSettingBlock { + if aerr, ok := err.(awserr.Error); ok && aerr.Code() == dynamodb.ErrCodeConditionalCheckFailedException { + log.WithFields(f).Debugf("company %s already has a manual/admin sanction block; preserving it and not overwriting origin with sss", companyID) + return nil + } + } log.WithFields(f).Warnf("error updating company sanction status, error: %v", err) return err } diff --git a/cla-backend-go/signatures/service.go b/cla-backend-go/signatures/service.go index c9bf9b8db..c7bb70cf4 100644 --- a/cla-backend-go/signatures/service.go +++ b/cla-backend-go/signatures/service.go @@ -833,6 +833,14 @@ func (s service) CreateOrUpdateEmployeeSignature(ctx context.Context, claGroupMo "companyID": companyModel.CompanyID, } + // Sanctions gate: never auto-create employee (ECLA) signatures for a sanctioned + // company. is_sanctioned is the persisted gate (SSS origin="sss" or a manual/admin + // block); both must block ECLA creation (auto-create toggle and approval-list edits). + if companyModel.IsSanctioned { + log.WithFields(f).Warnf("company %s is sanctioned (origin=%q); refusing to auto-create employee (ECLA) signatures", companyModel.CompanyID, companyModel.SanctionOrigin) + return nil, fmt.Errorf("company %s is sanctioned; employee (ECLA) signatures cannot be created", companyModel.CompanyID) + } + // Most of the following business logic is all the same - however, we need to handle the different types of approval lists entries and process them in the same way // We build a list of users to process - this is a list of simple user models that contain the email, GitHub username, and GitLab username - typically only one of the values in the model will be set userList, userErr := s.createOrGetEmployeeModels(ctx, claGroupModel, companyModel, corporateSignatureModel) @@ -1517,6 +1525,17 @@ func (s service) ProcessEmployeeSignature(ctx context.Context, companyModel *mod "projectID": claGroupModel.ProjectID, "userID": user.UserID, } + + // Sanctions gate: a sanctioned company's employees are not authorized. is_sanctioned + // is the persisted gate (SSS origin="sss" or a manual/admin block); honor it here so + // employee-acknowledgement (ECLA) authorization fails for sanctioned companies on + // GitHub PR checks and authorization queries. + if companyModel.IsSanctioned { + log.WithFields(f).Warnf("company %s is sanctioned (origin=%q); employee acknowledgement not authorized", companyModel.CompanyID, companyModel.SanctionOrigin) + notSigned := false + return ¬Signed, nil + } + var wg sync.WaitGroup resultChannel := make(chan *EmployeeModel) errorChannel := make(chan error) diff --git a/cla-backend-go/v2/gitlab-activity/service.go b/cla-backend-go/v2/gitlab-activity/service.go index b75b23ebf..9302a2865 100644 --- a/cla-backend-go/v2/gitlab-activity/service.go +++ b/cla-backend-go/v2/gitlab-activity/service.go @@ -516,13 +516,21 @@ func (s *service) isSigned(ctx context.Context, userModel *models.User, claGroup } companyID := userModel.CompanyID - _, err = s.companyRepository.GetCompany(ctx, companyID) + companyModel, err := s.companyRepository.GetCompany(ctx, companyID) if err != nil { msg := fmt.Sprintf("can't load company record: %s for user: %s (%s), error: %v", companyID, userModel.Username, userModel.UserID, err) log.WithFields(f).Errorf("%s", msg) return false, fmt.Errorf("%s", msg) } + // Sanctions gate: a sanctioned company's employees are not authorized. Honor the + // persisted is_sanctioned gate (SSS origin="sss" or a manual/admin block) so GitLab + // MR checks fail for sanctioned companies. + if companyModel != nil && companyModel.IsSanctioned { + log.WithFields(f).Warnf("company %s is sanctioned (origin=%q); GitLab contributor not authorized", companyID, companyModel.SanctionOrigin) + return false, fmt.Errorf("company %s is sanctioned", companyID) + } + corporateSignature, err := s.signatureRepository.GetCorporateSignature(ctx, claGroupID, companyID, aws.Bool(true), aws.Bool(true)) if err != nil { msg := fmt.Sprintf("can't load company signature record for company: %s for user : %s (%s), error : %v", companyID, userModel.Username, userModel.UserID, err) diff --git a/cla-backend-go/v2/sign/service.go b/cla-backend-go/v2/sign/service.go index 8f444e7bd..6bdadb094 100644 --- a/cla-backend-go/v2/sign/service.go +++ b/cla-backend-go/v2/sign/service.go @@ -1149,6 +1149,17 @@ func (s *service) SignedCorporateCallback(ctx context.Context, payload []byte, c return err } + // Sanctions gate: re-screen the company before finalizing the CCLA. A company can + // become blocked (manual/admin or SSS) between the DocuSign request and this + // completion callback; do not finalize a corporate CLA for a sanctioned company. + if sanctioned, complianceErr := s.checkCompanyCompliance(ctx, companyModel); complianceErr != nil { + log.WithFields(f).WithError(complianceErr).Warnf("company compliance check failed in corporate callback for company %s; not finalizing CCLA", companyID) + return complianceErr + } else if sanctioned { + log.WithFields(f).Warnf("company %s is sanctioned; refusing to finalize corporate CLA in callback", companyID) + return fmt.Errorf("company %s is sanctioned; corporate CLA cannot be finalized", companyID) + } + // Assumme only one signature per company/project var signatureID string var signature *v1Models.Signature @@ -2970,12 +2981,19 @@ func (s *service) GetUserActiveSignature(ctx context.Context, userID string) (*m // checkCompanyCompliance queries the Sanctions Screening Service for the given company // and persists the result. Returns (sanctioned, error). func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models.Company) (bool, error) { + sssMode := "optional" + if s.sssRequired { + sssMode = "required" + } f := logrus.Fields{ - "functionName": "sign.checkCompanyCompliance", - utils.XREQUESTID: ctx.Value(utils.XREQUESTID), - "companyID": company.CompanyID, - "companyName": company.CompanyName, + "functionName": "sign.checkCompanyCompliance", + utils.XREQUESTID: ctx.Value(utils.XREQUESTID), + "companyID": company.CompanyID, + "companyName": company.CompanyName, + "companyExternalID": company.CompanyExternalID, + "sssMode": sssMode, } + log.WithFields(f).Debugf("starting company sanctions screening (mode=%s)", sssMode) // Short-circuit for manually/admin-set blocks (sanction_origin != "sss" or no origin). // SSS-origin blocks fall through so a now-clean result can clear them. @@ -3052,10 +3070,12 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. req.SFDCID = company.CompanyExternalID } + log.WithFields(f).Infof("calling SSS GetOrganizationStatus: domain=%s, orgName=%s, sfdcID=%q (mode=%s)", req.Domain, req.OrgName, req.SFDCID, sssMode) result, err := s.sssClient.GetOrganizationStatus(ctx, req) if err != nil { return s.handleSSSError(f, company.CompanyID, err) } + log.WithFields(f).Infof("SSS GetOrganizationStatus result for company %s: status=%q (domain=%s, mode=%s)", company.CompanyID, result.Status, req.Domain, sssMode) sanctioned := result.Status == sss.StatusFlagged @@ -3073,9 +3093,16 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. } } else { // Clear only when previously set by SSS; manual blocks are left untouched. + // ClearCompanySanctionStatusIfSSS treats a conditional-check failure (manual + // block / nothing to clear) as a no-op, so a non-nil error here is a real + // persistence failure. In required mode we fail closed rather than allow with a + // stale persisted sanction still in place. log.WithFields(f).Debugf("SSS returned clean status for company %s; attempting conditional clear", company.CompanyID) if clearErr := s.companyRepo.ClearCompanySanctionStatusIfSSS(ctx, company.CompanyID); clearErr != nil { log.WithFields(f).WithError(clearErr).Warnf("failed to conditionally clear sanction status for company %s", company.CompanyID) + if s.sssRequired { + return false, fmt.Errorf("checkCompanyCompliance: SSS returned clean but clearing the persisted sanction failed for company %s: %w", company.CompanyID, clearErr) + } } } diff --git a/cla-backend-legacy/internal/api/handlers.go b/cla-backend-legacy/internal/api/handlers.go index d4b274e2a..e4d7cd989 100644 --- a/cla-backend-legacy/internal/api/handlers.go +++ b/cla-backend-legacy/internal/api/handlers.go @@ -8896,6 +8896,12 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin companyName := getAttrString(company, "company_name") companyExternalID := getAttrString(company, "company_external_id") + sssMode := "optional" + if h.sssRequired { + sssMode = "required" + } + logging.Debugf("starting company sanctions screening for company %s (external_id=%s, mode=%s)", companyID, companyExternalID, sssMode) + // Short-circuit for manually/admin-set blocks (sanction_origin != "sss" or no origin). // SSS-origin blocks fall through so a now-clean result can clear them. isSanctioned := false @@ -8959,10 +8965,12 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin req.SFDCID = companyExternalID } + logging.Infof("calling SSS GetOrganizationStatus for company %s: domain=%s, orgName=%s, sfdcID=%q (mode=%s)", companyID, req.Domain, req.OrgName, req.SFDCID, sssMode) result, err := h.sssClient.GetOrganizationStatus(ctx, req) if err != nil { return h.handleSSSError(ctx, companyID, err) } + logging.Infof("SSS GetOrganizationStatus result for company %s: status=%q (domain=%s, mode=%s)", companyID, result.Status, req.Domain, sssMode) sanctioned := result.Status == sss.StatusFlagged @@ -8980,9 +8988,15 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin } } else { // Clear only when previously set by SSS; manual blocks are left untouched. + // ClearCompanySanctionStatusIfSSS treats a conditional-check failure as a no-op, + // so a non-nil error here is a real persistence failure. In required mode we fail + // closed rather than allow with a stale persisted sanction still in place. logging.Debugf("SSS returned clean status for company %s; attempting conditional clear", companyID) if clearErr := h.companies.ClearCompanySanctionStatusIfSSS(ctx, companyID); clearErr != nil { logging.Warnf("failed to conditionally clear sanction status for company %s: %v", companyID, clearErr) + if h.sssRequired { + return false, fmt.Errorf("checkCompanyCompliance: SSS returned clean but clearing the persisted sanction failed for company %s: %w", companyID, clearErr) + } } } diff --git a/cla-backend-legacy/internal/store/companies.go b/cla-backend-legacy/internal/store/companies.go index d52f1f2c0..0be1dc79c 100644 --- a/cla-backend-legacy/internal/store/companies.go +++ b/cla-backend-legacy/internal/store/companies.go @@ -166,7 +166,7 @@ func (s *CompaniesStore) UpdateCompanySanctionStatus(ctx context.Context, compan updateExpr += ", #O = :o" } - _, err := s.client.UpdateItem(ctx, &dynamodb.UpdateItemInput{ + input := &dynamodb.UpdateItemInput{ TableName: aws.String(s.table), Key: map[string]types.AttributeValue{ "company_id": &types.AttributeValueMemberS{Value: companyID}, @@ -174,8 +174,29 @@ func (s *CompaniesStore) UpdateCompanySanctionStatus(ctx context.Context, compan UpdateExpression: aws.String(updateExpr), ExpressionAttributeNames: names, ExpressionAttributeValues: values, - }) - return err + } + + // When SSS sets a block, never overwrite a manual/admin block (is_sanctioned=true + // with absent or non-"sss" origin). Only set the SSS flag when the company is + // currently unblocked or already SSS-blocked; a ConditionalCheckFailedException + // means a manual/admin block is already present and must be preserved. + sssSettingBlock := sanctioned && origin == "sss" + if sssSettingBlock { + values[":false"] = &types.AttributeValueMemberBOOL{Value: false} + input.ConditionExpression = aws.String("attribute_not_exists(#S) OR #S = :false OR #O = :o") + } + + _, err := s.client.UpdateItem(ctx, input) + if err != nil { + if sssSettingBlock { + var condErr *types.ConditionalCheckFailedException + if errors.As(err, &condErr) { + return nil // Preserve the existing manual/admin block + } + } + return err + } + return nil } // ClearCompanySanctionStatusIfSSS clears is_sanctioned only when sanction_origin="sss". From 5d15e538c99d90c6d4022999321c965aadfcb5a8 Mon Sep 17 00:00:00 2001 From: Lukasz Gryglicki Date: Tue, 9 Jun 2026 09:28:23 +0200 Subject: [PATCH 2/9] address AI feedback Signed-off-by: Lukasz Gryglicki Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot) Assisted by [Claude](https://claude.ai) --- cla-backend-go/company/repository.go | 5 +++++ cla-backend-go/v2/gitlab-activity/service.go | 4 ++-- .../v2/gitlab_organizations/service.go | 12 +++++++++--- cla-backend-go/v2/signatures/handlers.go | 8 ++++++++ cla-backend-legacy/internal/api/handlers.go | 17 ++++++++++++----- cla-backend-legacy/internal/store/companies.go | 5 +++++ 6 files changed, 41 insertions(+), 10 deletions(-) diff --git a/cla-backend-go/company/repository.go b/cla-backend-go/company/repository.go index 9e5ae36c7..a4b514e97 100644 --- a/cla-backend-go/company/repository.go +++ b/cla-backend-go/company/repository.go @@ -1306,6 +1306,11 @@ func (repo repository) UpdateCompanySanctionStatus(ctx context.Context, companyI names["#O"] = aws.String("sanction_origin") values[":o"] = &dynamodb.AttributeValue{S: aws.String(origin)} updateExpr += ", #O = :o" + } else { + // Manual/admin update: remove any stale SSS-set origin so the record becomes a + // sticky admin block (origin absent) that SSS will never auto-clear. + names["#O"] = aws.String("sanction_origin") + updateExpr += " REMOVE #O" } input := &dynamodb.UpdateItemInput{ diff --git a/cla-backend-go/v2/gitlab-activity/service.go b/cla-backend-go/v2/gitlab-activity/service.go index 9302a2865..ffc5df823 100644 --- a/cla-backend-go/v2/gitlab-activity/service.go +++ b/cla-backend-go/v2/gitlab-activity/service.go @@ -261,7 +261,7 @@ func (s *service) ProcessMergeActivity(ctx context.Context, secretToken string, log.WithFields(f).WithError(signedCheckErr).Warnf("problem checking if user : %s (%d) has signed - assuming not signed", gitlabUser.Username, gitlabUser.ID) missingUsers = append(missingUsers, &gatedGitlabUser{ User: gitlabUser, - err: err, + err: signedCheckErr, }) continue } @@ -273,7 +273,7 @@ func (s *service) ProcessMergeActivity(ctx context.Context, secretToken string, log.WithFields(f).Infof("gitlabUser: %s (%d) has NOT signed", gitlabUser.Username, gitlabUser.ID) missingUsers = append(missingUsers, &gatedGitlabUser{ User: gitlabUser, - err: err, + err: nil, }) } } diff --git a/cla-backend-go/v2/gitlab_organizations/service.go b/cla-backend-go/v2/gitlab_organizations/service.go index 1eb75811e..c1004cf63 100644 --- a/cla-backend-go/v2/gitlab_organizations/service.go +++ b/cla-backend-go/v2/gitlab_organizations/service.go @@ -858,13 +858,19 @@ func (s *Service) InitiateSignRequest(ctx context.Context, req *http.Request, gi } companyID := claUser.CompanyID - _, err = s.companyRepository.GetCompany(ctx, companyID) - if err != nil { - msg := fmt.Sprintf("can't load company record: %s for user: %s (%s), error: %v", companyID, claUser.Username, claUser.UserID, err) + companyModel, companyErr := s.companyRepository.GetCompany(ctx, companyID) + if companyErr != nil { + msg := fmt.Sprintf("can't load company record: %s for user: %s (%s), error: %v", companyID, claUser.Username, claUser.UserID, companyErr) log.WithFields(f).Errorf("%s", msg) return &consoleURL, nil } + // Sanctions gate: do not activate a corporate signature for a sanctioned company. + if companyModel != nil && companyModel.IsSanctioned { + log.WithFields(f).Warnf("company %s is sanctioned (origin=%q); not activating signature for GitLab user %s", companyID, companyModel.SanctionOrigin, claUser.UserID) + return &consoleURL, nil + } + corporateSignature, err := s.signatureRepo.GetCorporateSignature(ctx, gitlabRepo.RepositoryClaGroupID, companyID, aws.Bool(false), aws.Bool(true)) if err != nil { msg := fmt.Sprintf("can't load company signature record for company: %s for user : %s (%s), error : %v", companyID, claUser.Username, claUser.UserID, err) diff --git a/cla-backend-go/v2/signatures/handlers.go b/cla-backend-go/v2/signatures/handlers.go index 476b59ab8..9382b5784 100644 --- a/cla-backend-go/v2/signatures/handlers.go +++ b/cla-backend-go/v2/signatures/handlers.go @@ -1341,6 +1341,14 @@ func Configure(api *operations.EasyclaAPI, claGroupService service.Service, proj utils.ErrorResponseForbidden(reqID, msg)) } + // Sanctions gate: do not enable ECLA auto-create for a sanctioned company. + if eacp.Body.AutoCreateEcla && companyRecord.IsSanctioned { + msg := fmt.Sprintf("company %s is sanctioned (origin=%q); cannot enable ECLA auto-create", companyRecord.CompanyID, companyRecord.SanctionOrigin) + log.WithFields(f).Warn(msg) + return signatures.NewEclaAutoCreateBadRequest().WithXRequestID(reqID).WithPayload( + utils.ErrorResponseBadRequestWithError(reqID, msg, fmt.Errorf("company is sanctioned"))) + } + err = v2SignatureService.EclaAutoCreate(ctx, cclaSignature.SignatureID, eacp.Body.AutoCreateEcla) if err != nil { msg := "unable to update auto_create_ecla flag" diff --git a/cla-backend-legacy/internal/api/handlers.go b/cla-backend-legacy/internal/api/handlers.go index e4d7cd989..41a8751d0 100644 --- a/cla-backend-legacy/internal/api/handlers.go +++ b/cla-backend-legacy/internal/api/handlers.go @@ -5244,6 +5244,9 @@ func (h *Handlers) PutCompanyV1(w http.ResponseWriter, r *http.Request) { } if req.IsSanctioned != nil { item["is_sanctioned"] = &types.AttributeValueMemberBOOL{Value: *req.IsSanctioned} + // Manual/admin sanction change: drop any SSS-set origin so this becomes an + // admin-controlled state (sticky when true; never later auto-cleared by SSS). + delete(item, "sanction_origin") updateStr += fmt.Sprintf("The company is_sanctioned was updated to %t. ", *req.IsSanctioned) } @@ -8915,11 +8918,15 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin return true, nil } + // Optional mode never blocks in the fallbacks below: a manual/admin block was + // already handled by the short-circuit above, and optional mode only blocks on an + // explicit live "flagged" result (parity with cla-backend-go, and avoids a durable + // false-positive when a stale sss-origin record can't be re-screened). if h.sssClient == nil { if h.sssRequired { return false, fmt.Errorf("checkCompanyCompliance: SSS is required but the client is not configured") } - return isSanctioned, nil + return false, nil } if companyExternalID == "" { @@ -8927,7 +8934,7 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin if h.sssRequired { return false, fmt.Errorf("checkCompanyCompliance: company %s has no external ID (SFDC ID) required for screening", companyID) } - return isSanctioned, nil + return false, nil } // Fetch organization details from Salesforce to resolve the domain. @@ -8937,14 +8944,14 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin if h.sssRequired { return false, fmt.Errorf("checkCompanyCompliance: failed to get organization %s: %w", companyExternalID, err) } - return isSanctioned, nil + return false, nil } if org == nil { logging.Warnf("organization record is nil for %s", companyExternalID) if h.sssRequired { return false, fmt.Errorf("checkCompanyCompliance: organization record is nil for %s", companyExternalID) } - return isSanctioned, nil + return false, nil } domain := h.resolveDomain(org) @@ -8953,7 +8960,7 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin if h.sssRequired { return false, fmt.Errorf("checkCompanyCompliance: unable to resolve domain for organization %s", companyExternalID) } - return isSanctioned, nil + return false, nil } req := sss.OrganizationStatusRequest{ diff --git a/cla-backend-legacy/internal/store/companies.go b/cla-backend-legacy/internal/store/companies.go index 0be1dc79c..6d56dcd18 100644 --- a/cla-backend-legacy/internal/store/companies.go +++ b/cla-backend-legacy/internal/store/companies.go @@ -164,6 +164,11 @@ func (s *CompaniesStore) UpdateCompanySanctionStatus(ctx context.Context, compan names["#O"] = "sanction_origin" values[":o"] = &types.AttributeValueMemberS{Value: origin} updateExpr += ", #O = :o" + } else { + // Manual/admin update: remove any stale SSS-set origin so the record becomes a + // sticky admin block (origin absent) that SSS will never auto-clear. + names["#O"] = "sanction_origin" + updateExpr += " REMOVE #O" } input := &dynamodb.UpdateItemInput{ From d51af38631fc43ecfd5ab7b7c4647da680f0859c Mon Sep 17 00:00:00 2001 From: Lukasz Gryglicki Date: Tue, 9 Jun 2026 10:23:11 +0200 Subject: [PATCH 3/9] Address AI feedback Signed-off-by: Lukasz Gryglicki Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot) Assisted by [Claude](https://claude.ai) --- cla-backend-go/v2/sign/service.go | 52 ++++++++++++--------- cla-backend-legacy/internal/api/handlers.go | 19 ++++---- 2 files changed, 41 insertions(+), 30 deletions(-) diff --git a/cla-backend-go/v2/sign/service.go b/cla-backend-go/v2/sign/service.go index 6bdadb094..e6076c227 100644 --- a/cla-backend-go/v2/sign/service.go +++ b/cla-backend-go/v2/sign/service.go @@ -1149,17 +1149,6 @@ func (s *service) SignedCorporateCallback(ctx context.Context, payload []byte, c return err } - // Sanctions gate: re-screen the company before finalizing the CCLA. A company can - // become blocked (manual/admin or SSS) between the DocuSign request and this - // completion callback; do not finalize a corporate CLA for a sanctioned company. - if sanctioned, complianceErr := s.checkCompanyCompliance(ctx, companyModel); complianceErr != nil { - log.WithFields(f).WithError(complianceErr).Warnf("company compliance check failed in corporate callback for company %s; not finalizing CCLA", companyID) - return complianceErr - } else if sanctioned { - log.WithFields(f).Warnf("company %s is sanctioned; refusing to finalize corporate CLA in callback", companyID) - return fmt.Errorf("company %s is sanctioned; corporate CLA cannot be finalized", companyID) - } - // Assumme only one signature per company/project var signatureID string var signature *v1Models.Signature @@ -1227,6 +1216,17 @@ func (s *service) SignedCorporateCallback(ctx context.Context, payload []byte, c // Update the signature status if changed status := info.EnvelopeStatus.Status if status == DocusignCompleted && !signature.SignatureSigned { + // Sanctions gate: re-screen the company before finalizing the CCLA. A company can + // become blocked (manual/admin or SSS) between the DocuSign request and this + // completion callback; do not finalize a corporate CLA for a sanctioned company. + if sanctioned, complianceErr := s.checkCompanyCompliance(ctx, companyModel); complianceErr != nil { + log.WithFields(f).WithError(complianceErr).Warnf("company compliance check failed in corporate callback for company %s; not finalizing CCLA", companyID) + return complianceErr + } else if sanctioned { + log.WithFields(f).Warnf("company %s is sanctioned; refusing to finalize corporate CLA in callback", companyID) + return fmt.Errorf("company %s is sanctioned; corporate CLA cannot be finalized", companyID) + } + _, currentTime := utils.CurrentTime() updates := map[string]interface{}{ "signature_signed": true, @@ -3014,8 +3014,8 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. log.WithFields(f).WithError(resultErr).Error("SSS client not configured") return false, resultErr } - log.WithFields(f).Debug("SSS client not configured, skipping optional live compliance check") - return false, nil + log.WithFields(f).Debug("SSS client not configured; honoring persisted sanction state without a live compliance check") + return company.IsSanctioned, nil } // Fetch org from organization service to get the website/domain. @@ -3023,8 +3023,8 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. if orgClient == nil { resultErr := fmt.Errorf("checkCompanyCompliance: organization service client is not configured") if !s.sssRequired { - log.WithFields(f).WithError(resultErr).Warn("SSS is not required; continuing without live compliance result") - return false, nil + log.WithFields(f).WithError(resultErr).Warn("SSS is not required; honoring persisted sanction state without a live compliance result") + return company.IsSanctioned, nil } return false, resultErr } @@ -3033,8 +3033,8 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. log.WithFields(f).WithError(err).Warnf("failed to get organization %s for domain resolution", company.CompanyExternalID) resultErr := fmt.Errorf("checkCompanyCompliance: failed to get organization %s: %w", company.CompanyExternalID, err) if !s.sssRequired { - log.WithFields(f).WithError(resultErr).Warn("SSS is not required; continuing without live compliance result") - return false, nil + log.WithFields(f).WithError(resultErr).Warn("SSS is not required; honoring persisted sanction state without a live compliance result") + return company.IsSanctioned, nil } return false, resultErr } @@ -3042,8 +3042,8 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. log.WithFields(f).Warnf("organization record is nil for %s", company.CompanyExternalID) resultErr := fmt.Errorf("checkCompanyCompliance: organization record is nil for %s", company.CompanyExternalID) if !s.sssRequired { - log.WithFields(f).WithError(resultErr).Warn("SSS is not required; continuing without live compliance result") - return false, nil + log.WithFields(f).WithError(resultErr).Warn("SSS is not required; honoring persisted sanction state without a live compliance result") + return company.IsSanctioned, nil } return false, resultErr } @@ -3056,8 +3056,8 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. log.WithFields(f).WithError(resultErr).Error("unable to resolve domain for required SSS check") return false, resultErr } - log.WithFields(f).WithError(resultErr).Warn("SSS is not required; continuing without live compliance result") - return false, nil + log.WithFields(f).WithError(resultErr).Warn("SSS is not required; honoring persisted sanction state without a live compliance result") + return company.IsSanctioned, nil } log.WithFields(f).Debugf("resolved domain: %s for SSS check", domain) @@ -3106,6 +3106,16 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. } } + // Reflect the live SSS decision on the in-memory model so any downstream gate in + // this same request (e.g. ProcessEmployeeSignature) sees the just-updated state + // rather than the now-stale value that was loaded before this check ran. + company.IsSanctioned = sanctioned + if sanctioned { + company.SanctionOrigin = "sss" + } else { + company.SanctionOrigin = "" + } + s.setComplianceCache(cacheKey, sanctioned) return sanctioned, nil } diff --git a/cla-backend-legacy/internal/api/handlers.go b/cla-backend-legacy/internal/api/handlers.go index 41a8751d0..5fc4d277e 100644 --- a/cla-backend-legacy/internal/api/handlers.go +++ b/cla-backend-legacy/internal/api/handlers.go @@ -8918,15 +8918,16 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin return true, nil } - // Optional mode never blocks in the fallbacks below: a manual/admin block was - // already handled by the short-circuit above, and optional mode only blocks on an - // explicit live "flagged" result (parity with cla-backend-go, and avoids a durable - // false-positive when a stale sss-origin record can't be re-screened). + // In the fallbacks below (no live SSS check possible) we honor the persisted state: + // a manual/admin block was already handled by the short-circuit above, and a stale + // sss-origin block keeps blocking until a live "clean" can clear it. The mode only + // changes behavior for an UNflagged company: required blocks (cannot confirm clean), + // optional allows. (Parity with cla-backend-go.) if h.sssClient == nil { if h.sssRequired { return false, fmt.Errorf("checkCompanyCompliance: SSS is required but the client is not configured") } - return false, nil + return isSanctioned, nil } if companyExternalID == "" { @@ -8934,7 +8935,7 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin if h.sssRequired { return false, fmt.Errorf("checkCompanyCompliance: company %s has no external ID (SFDC ID) required for screening", companyID) } - return false, nil + return isSanctioned, nil } // Fetch organization details from Salesforce to resolve the domain. @@ -8944,14 +8945,14 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin if h.sssRequired { return false, fmt.Errorf("checkCompanyCompliance: failed to get organization %s: %w", companyExternalID, err) } - return false, nil + return isSanctioned, nil } if org == nil { logging.Warnf("organization record is nil for %s", companyExternalID) if h.sssRequired { return false, fmt.Errorf("checkCompanyCompliance: organization record is nil for %s", companyExternalID) } - return false, nil + return isSanctioned, nil } domain := h.resolveDomain(org) @@ -8960,7 +8961,7 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin if h.sssRequired { return false, fmt.Errorf("checkCompanyCompliance: unable to resolve domain for organization %s", companyExternalID) } - return false, nil + return isSanctioned, nil } req := sss.OrganizationStatusRequest{ From 938ba2a6e1d890b08975556a37c9813d3d1d103b Mon Sep 17 00:00:00 2001 From: Lukasz Gryglicki Date: Tue, 9 Jun 2026 10:31:38 +0200 Subject: [PATCH 4/9] Address AI feedback Signed-off-by: Lukasz Gryglicki Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot) Assisted by [Claude](https://claude.ai) --- cla-backend-legacy/internal/api/handlers.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/cla-backend-legacy/internal/api/handlers.go b/cla-backend-legacy/internal/api/handlers.go index 5fc4d277e..c5dc78c19 100644 --- a/cla-backend-legacy/internal/api/handlers.go +++ b/cla-backend-legacy/internal/api/handlers.go @@ -8938,6 +8938,16 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin return isSanctioned, nil } + // Defensive: the Salesforce/org-service client may be unconfigured (it is treated as + // nil-able elsewhere in this file). Mirror the org-fetch fallback instead of panicking. + if h.salesforce == nil { + logging.Warnf("salesforce/org-service client not configured, cannot resolve domain for %s", companyExternalID) + if h.sssRequired { + return false, fmt.Errorf("checkCompanyCompliance: salesforce client not configured for organization %s", companyExternalID) + } + return isSanctioned, nil + } + // Fetch organization details from Salesforce to resolve the domain. org, err := h.salesforce.GetOrganization(ctx, companyExternalID) if err != nil { From 8018705589ebb2eeb70cc1b1b0bad6e48176e6d8 Mon Sep 17 00:00:00 2001 From: Lukasz Gryglicki Date: Tue, 9 Jun 2026 10:38:18 +0200 Subject: [PATCH 5/9] Address AI feedback Signed-off-by: Lukasz Gryglicki Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot) Assisted by [Claude](https://claude.ai) --- cla-backend-go/v2/sign/service.go | 9 ++++++++- cla-backend-legacy/internal/api/handlers.go | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/cla-backend-go/v2/sign/service.go b/cla-backend-go/v2/sign/service.go index e6076c227..27ad0f7e1 100644 --- a/cla-backend-go/v2/sign/service.go +++ b/cla-backend-go/v2/sign/service.go @@ -3073,7 +3073,14 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. log.WithFields(f).Infof("calling SSS GetOrganizationStatus: domain=%s, orgName=%s, sfdcID=%q (mode=%s)", req.Domain, req.OrgName, req.SFDCID, sssMode) result, err := s.sssClient.GetOrganizationStatus(ctx, req) if err != nil { - return s.handleSSSError(f, company.CompanyID, err) + blocked, herr := s.handleSSSError(f, company.CompanyID, err) + // Optional mode allows on SSS errors, but a company already persisted as + // SSS-sanctioned must keep blocking until a live clean result can clear it. + if herr == nil && !blocked && company.IsSanctioned { + log.WithFields(f).Warnf("SSS call failed for company %s; honoring persisted sanction until a live clean result (mode=%s)", company.CompanyID, sssMode) + return true, nil + } + return blocked, herr } log.WithFields(f).Infof("SSS GetOrganizationStatus result for company %s: status=%q (domain=%s, mode=%s)", company.CompanyID, result.Status, req.Domain, sssMode) diff --git a/cla-backend-legacy/internal/api/handlers.go b/cla-backend-legacy/internal/api/handlers.go index c5dc78c19..5349fe672 100644 --- a/cla-backend-legacy/internal/api/handlers.go +++ b/cla-backend-legacy/internal/api/handlers.go @@ -8986,7 +8986,14 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin logging.Infof("calling SSS GetOrganizationStatus for company %s: domain=%s, orgName=%s, sfdcID=%q (mode=%s)", companyID, req.Domain, req.OrgName, req.SFDCID, sssMode) result, err := h.sssClient.GetOrganizationStatus(ctx, req) if err != nil { - return h.handleSSSError(ctx, companyID, err) + blocked, herr := h.handleSSSError(ctx, companyID, err) + // Optional mode allows on SSS errors, but a company already persisted as + // SSS-sanctioned must keep blocking until a live clean result can clear it. + if herr == nil && !blocked && isSanctioned { + logging.Warnf("SSS call failed for company %s; honoring persisted sanction until a live clean result (mode=%s)", companyID, sssMode) + return true, nil + } + return blocked, herr } logging.Infof("SSS GetOrganizationStatus result for company %s: status=%q (domain=%s, mode=%s)", companyID, result.Status, req.Domain, sssMode) From 7feba2a2b9fd41ecb1e3f4ea2553146617897213 Mon Sep 17 00:00:00 2001 From: Lukasz Gryglicki Date: Tue, 9 Jun 2026 11:40:33 +0200 Subject: [PATCH 6/9] Address AI feedback Signed-off-by: Lukasz Gryglicki Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot) Assisted by [Claude](https://claude.ai) --- cla-backend-go/company/mocks/mock_repo.go | 7 ++-- cla-backend-go/company/repository.go | 21 ++++++---- cla-backend-go/v2/sign/service.go | 50 ++++++++++++++++------- 3 files changed, 52 insertions(+), 26 deletions(-) diff --git a/cla-backend-go/company/mocks/mock_repo.go b/cla-backend-go/company/mocks/mock_repo.go index 7e1665cd3..c3d59cd2a 100644 --- a/cla-backend-go/company/mocks/mock_repo.go +++ b/cla-backend-go/company/mocks/mock_repo.go @@ -365,11 +365,12 @@ func (mr *MockIRepositoryMockRecorder) UpdateCompanySanctionStatus(ctx, companyI } // ClearCompanySanctionStatusIfSSS mocks base method. -func (m *MockIRepository) ClearCompanySanctionStatusIfSSS(ctx context.Context, companyID string) error { +func (m *MockIRepository) ClearCompanySanctionStatusIfSSS(ctx context.Context, companyID string) (bool, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ClearCompanySanctionStatusIfSSS", ctx, companyID) - ret0, _ := ret[0].(error) - return ret0 + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 } // ClearCompanySanctionStatusIfSSS indicates an expected call of ClearCompanySanctionStatusIfSSS. diff --git a/cla-backend-go/company/repository.go b/cla-backend-go/company/repository.go index a4b514e97..5b8e2fbc8 100644 --- a/cla-backend-go/company/repository.go +++ b/cla-backend-go/company/repository.go @@ -55,7 +55,7 @@ type IRepository interface { //nolint RejectCompanyAccessRequest(ctx context.Context, companyInviteID string) error UpdateCompanyAccessList(ctx context.Context, companyID string, companyACL []string) error UpdateCompanySanctionStatus(ctx context.Context, companyID string, sanctioned bool, origin string) error - ClearCompanySanctionStatusIfSSS(ctx context.Context, companyID string) error + ClearCompanySanctionStatusIfSSS(ctx context.Context, companyID string) (bool, error) IsCCLAEnabledForCompany(ctx context.Context, companyID string) (bool, error) } @@ -1279,6 +1279,9 @@ func (repo repository) UpdateCompanyAccessList(ctx context.Context, companyID st return nil } +// sanctionOriginSSS is the sanction_origin value written by the Sanctions Screening Service. +const sanctionOriginSSS = "sss" + // UpdateCompanySanctionStatus sets is_sanctioned and, when origin is non-empty, sanction_origin. // Pass origin="sss" when flagging via SSS; pass origin="" for manual admin updates. func (repo repository) UpdateCompanySanctionStatus(ctx context.Context, companyID string, sanctioned bool, origin string) error { @@ -1327,7 +1330,7 @@ func (repo repository) UpdateCompanySanctionStatus(ctx context.Context, companyI // with absent or non-"sss" origin). Only set the SSS flag when the company is // currently unblocked or already SSS-blocked. A ConditionalCheckFailedException // therefore means a manual/admin block is already in place and must be preserved. - sssSettingBlock := sanctioned && origin == "sss" + sssSettingBlock := sanctioned && origin == sanctionOriginSSS if sssSettingBlock { values[":false"] = &dynamodb.AttributeValue{BOOL: aws.Bool(false)} input.ConditionExpression = aws.String("attribute_not_exists(#S) OR #S = :false OR #O = :o") @@ -1347,8 +1350,10 @@ func (repo repository) UpdateCompanySanctionStatus(ctx context.Context, companyI } // ClearCompanySanctionStatusIfSSS clears is_sanctioned only when sanction_origin="sss". -// A ConditionalCheckFailedException (manual/absent origin) is silently ignored. -func (repo repository) ClearCompanySanctionStatusIfSSS(ctx context.Context, companyID string) error { +// It returns (cleared, err): cleared is true only when the conditional matched and the +// record was actually cleared; a ConditionalCheckFailedException (manual/absent origin) +// returns (false, nil) so callers can leave any manual/admin block in place. +func (repo repository) ClearCompanySanctionStatusIfSSS(ctx context.Context, companyID string) (bool, error) { f := logrus.Fields{ "functionName": "company.repository.ClearCompanySanctionStatusIfSSS", utils.XREQUESTID: ctx.Value(utils.XREQUESTID), @@ -1372,19 +1377,19 @@ func (repo repository) ClearCompanySanctionStatusIfSSS(ctx context.Context, comp ExpressionAttributeValues: map[string]*dynamodb.AttributeValue{ ":false": {BOOL: aws.Bool(false)}, ":m": {S: aws.String(now)}, - ":sss": {S: aws.String("sss")}, + ":sss": {S: aws.String(sanctionOriginSSS)}, }, } if _, err := repo.dynamoDBClient.UpdateItem(input); err != nil { if aerr, ok := err.(awserr.Error); ok && aerr.Code() == dynamodb.ErrCodeConditionalCheckFailedException { log.WithFields(f).Debugf("sanction_origin != sss for company %s; not auto-clearing (manual/admin block)", companyID) - return nil + return false, nil } log.WithFields(f).Warnf("error clearing company sanction status: %v", err) - return err + return false, err } - return nil + return true, nil } func (repo repository) CreateCompany(ctx context.Context, in *models.Company) (*models.Company, error) { diff --git a/cla-backend-go/v2/sign/service.go b/cla-backend-go/v2/sign/service.go index 27ad0f7e1..d0677a018 100644 --- a/cla-backend-go/v2/sign/service.go +++ b/cla-backend-go/v2/sign/service.go @@ -64,6 +64,7 @@ const ( DocusignCompleted = "Completed" complianceCacheTTL = 5 * time.Minute maxComplianceCacheSize = 1000 + sanctionOriginSSS = "sss" ) // errors @@ -2997,7 +2998,7 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. // Short-circuit for manually/admin-set blocks (sanction_origin != "sss" or no origin). // SSS-origin blocks fall through so a now-clean result can clear them. - if company.IsSanctioned && company.SanctionOrigin != "sss" { + if company.IsSanctioned && company.SanctionOrigin != sanctionOriginSSS { log.WithFields(f).Warnf("company has non-SSS sanction block (origin=%q), blocking without SSS call", company.SanctionOrigin) return true, nil } @@ -3005,6 +3006,10 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. cacheKey := s.complianceCacheKey(company) if cached, ok := s.getComplianceCache(cacheKey); ok { log.WithFields(f).Debugf("using cached compliance result for organization/company: %s", cacheKey) + // Mirror the cached decision onto the loaded model so downstream gates in this + // request stay consistent. Manual/admin blocks already short-circuited above, so a + // cached-clean result here cannot be masking such a block. + s.applyComplianceToModel(company, cached.sanctioned) return cached.sanctioned, nil } @@ -3091,40 +3096,55 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. return false, fmt.Errorf("checkCompanyCompliance: unexpected SSS status %q for company %s (required mode blocks on ambiguous results)", result.Status, company.CompanyID) } - // Persist result: set origin="sss" on flagged; conditionally clear on clean (only if sss-origin). + // Persist result and reflect it on the in-memory model so downstream gates in this + // same request (e.g. ProcessEmployeeSignature) see the just-updated state instead of + // the stale value loaded before this check ran. if sanctioned { log.WithFields(f).Warnf("SSS returned flagged status for company %s, persisting sanction with origin=sss", company.CompanyID) - if persistErr := s.companyRepo.UpdateCompanySanctionStatus(ctx, company.CompanyID, true, "sss"); persistErr != nil { + if persistErr := s.companyRepo.UpdateCompanySanctionStatus(ctx, company.CompanyID, true, sanctionOriginSSS); persistErr != nil { log.WithFields(f).WithError(persistErr).Warnf("failed to persist sanction status for company %s", company.CompanyID) return false, fmt.Errorf("failed to persist sanction status for company %s: %w", company.CompanyID, persistErr) } + // Flagged always blocks downstream, so reflecting it is safe even if a concurrent + // manual/admin block now owns the persisted record (that also blocks). + s.applyComplianceToModel(company, true) } else { // Clear only when previously set by SSS; manual blocks are left untouched. - // ClearCompanySanctionStatusIfSSS treats a conditional-check failure (manual - // block / nothing to clear) as a no-op, so a non-nil error here is a real - // persistence failure. In required mode we fail closed rather than allow with a - // stale persisted sanction still in place. + // ClearCompanySanctionStatusIfSSS reports whether it actually cleared; a non-nil + // error is a real persistence failure (not a benign conditional-check no-op), so in + // required mode we fail closed rather than allow with a stale persisted sanction. log.WithFields(f).Debugf("SSS returned clean status for company %s; attempting conditional clear", company.CompanyID) - if clearErr := s.companyRepo.ClearCompanySanctionStatusIfSSS(ctx, company.CompanyID); clearErr != nil { + cleared, clearErr := s.companyRepo.ClearCompanySanctionStatusIfSSS(ctx, company.CompanyID) + if clearErr != nil { log.WithFields(f).WithError(clearErr).Warnf("failed to conditionally clear sanction status for company %s", company.CompanyID) if s.sssRequired { return false, fmt.Errorf("checkCompanyCompliance: SSS returned clean but clearing the persisted sanction failed for company %s: %w", company.CompanyID, clearErr) } } + // Only mirror the clear on the in-memory model when it actually applied. If the + // conditional did not match — e.g. a concurrent manual/admin block now owns the + // record — leave the loaded state so downstream gates still honor that block. + if cleared { + s.applyComplianceToModel(company, false) + } } - // Reflect the live SSS decision on the in-memory model so any downstream gate in - // this same request (e.g. ProcessEmployeeSignature) sees the just-updated state - // rather than the now-stale value that was loaded before this check ran. + s.setComplianceCache(cacheKey, sanctioned) + return sanctioned, nil +} + +// applyComplianceToModel updates the in-memory company model to reflect a compliance +// decision so downstream gates in the same request stay consistent with this check. +func (s *service) applyComplianceToModel(company *v1Models.Company, sanctioned bool) { + if company == nil { + return + } company.IsSanctioned = sanctioned if sanctioned { - company.SanctionOrigin = "sss" + company.SanctionOrigin = sanctionOriginSSS } else { company.SanctionOrigin = "" } - - s.setComplianceCache(cacheKey, sanctioned) - return sanctioned, nil } func (s *service) complianceCacheKey(company *v1Models.Company) string { From 23307ed6321814e2405db27d2dd0abaa8686cd6a Mon Sep 17 00:00:00 2001 From: Lukasz Gryglicki Date: Tue, 9 Jun 2026 11:56:14 +0200 Subject: [PATCH 7/9] Address AI feedback Signed-off-by: Lukasz Gryglicki Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot) Assisted by [Claude](https://claude.ai) --- cla-backend-go/v2/sign/service_sss_test.go | 65 ++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/cla-backend-go/v2/sign/service_sss_test.go b/cla-backend-go/v2/sign/service_sss_test.go index b825d2794..01e92e14e 100644 --- a/cla-backend-go/v2/sign/service_sss_test.go +++ b/cla-backend-go/v2/sign/service_sss_test.go @@ -139,3 +139,68 @@ func TestComplianceCacheSkipsErrors(t *testing.T) { t.Fatal("expected cache entry to be stored") } } + +func TestCheckCompanyComplianceCacheHitMutatesModel(t *testing.T) { + // A cached "clean" result must clear the stale loaded model so downstream gates + // (e.g. ProcessEmployeeSignature) in the same request stay consistent. + svc := &service{ + complianceCache: map[string]complianceCacheEntry{ + "external-id": {sanctioned: false, expiresAt: time.Now().Add(time.Minute)}, + }, + complianceCacheMu: &sync.Mutex{}, + } + + company := &models.Company{ + CompanyID: "company-id", + CompanyExternalID: "external-id", + IsSanctioned: true, + SanctionOrigin: "sss", + } + + blocked, err := svc.checkCompanyCompliance(context.Background(), company) + if err != nil { + t.Fatalf("expected cached clean result to continue, got %v", err) + } + if blocked { + t.Fatal("expected cached clean result not to block") + } + if company.IsSanctioned || company.SanctionOrigin != "" { + t.Fatalf("expected in-memory model cleared on cache hit, got IsSanctioned=%v origin=%q", company.IsSanctioned, company.SanctionOrigin) + } +} + +func TestCheckCompanyComplianceOptionalHonorsPersistedSSSFlag(t *testing.T) { + // Optional mode with no SSS client: an already-persisted SSS-origin block must keep + // blocking until a live clean result can clear it (do not fail open on the flag). + svc := &service{sssRequired: false} + + blocked, err := svc.checkCompanyCompliance(context.Background(), &models.Company{ + CompanyID: "company-id", + CompanyName: "Company", + IsSanctioned: true, + SanctionOrigin: "sss", + }) + if err != nil { + t.Fatalf("expected optional persisted-sanction check to continue without error, got %v", err) + } + if !blocked { + t.Fatal("expected a persisted SSS sanction to keep blocking in optional mode when SSS is unavailable") + } +} + +func TestCheckCompanyComplianceAdminBlockAlwaysBlocks(t *testing.T) { + // A manual/admin block (is_sanctioned=true, no/!=sss origin) must short-circuit and + // block regardless of mode or SSS availability. + svc := &service{sssRequired: false} + + blocked, err := svc.checkCompanyCompliance(context.Background(), &models.Company{ + CompanyID: "company-id", + IsSanctioned: true, + }) + if err != nil { + t.Fatalf("unexpected error for admin block: %v", err) + } + if !blocked { + t.Fatal("expected a manual/admin sanction (no origin) to always block") + } +} From 12a93839941ded4aee9eb5da09fc266fbbf5d70b Mon Sep 17 00:00:00 2001 From: Lukasz Gryglicki Date: Tue, 9 Jun 2026 13:17:53 +0200 Subject: [PATCH 8/9] Address AI feedback Signed-off-by: Lukasz Gryglicki Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot) Assisted by [Claude](https://claude.ai) --- cla-backend-go/v2/sign/service.go | 11 ++++++++--- cla-backend-legacy/internal/api/handlers.go | 10 +++++++++- cla-backend-legacy/internal/store/companies.go | 10 +++++----- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/cla-backend-go/v2/sign/service.go b/cla-backend-go/v2/sign/service.go index d0677a018..b0b75efb4 100644 --- a/cla-backend-go/v2/sign/service.go +++ b/cla-backend-go/v2/sign/service.go @@ -3121,11 +3121,16 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. return false, fmt.Errorf("checkCompanyCompliance: SSS returned clean but clearing the persisted sanction failed for company %s: %w", company.CompanyID, clearErr) } } - // Only mirror the clear on the in-memory model when it actually applied. If the - // conditional did not match — e.g. a concurrent manual/admin block now owns the - // record — leave the loaded state so downstream gates still honor that block. + // Only mirror the clear on the in-memory model when it actually applied. If we + // loaded an SSS-origin block but the conditional did not match, the persisted + // record changed underneath us (e.g. a concurrent manual/admin block transition): + // the state is uncertain and may now be a manual block, so fail closed and do not + // cache the clean result. if cleared { s.applyComplianceToModel(company, false) + } else if company.IsSanctioned && company.SanctionOrigin == sanctionOriginSSS { + log.WithFields(f).Warnf("SSS returned clean for company %s but the persisted SSS block could not be cleared (record changed concurrently); blocking", company.CompanyID) + return true, nil } } diff --git a/cla-backend-legacy/internal/api/handlers.go b/cla-backend-legacy/internal/api/handlers.go index 5349fe672..df24c6f12 100644 --- a/cla-backend-legacy/internal/api/handlers.go +++ b/cla-backend-legacy/internal/api/handlers.go @@ -9017,12 +9017,20 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin // so a non-nil error here is a real persistence failure. In required mode we fail // closed rather than allow with a stale persisted sanction still in place. logging.Debugf("SSS returned clean status for company %s; attempting conditional clear", companyID) - if clearErr := h.companies.ClearCompanySanctionStatusIfSSS(ctx, companyID); clearErr != nil { + cleared, clearErr := h.companies.ClearCompanySanctionStatusIfSSS(ctx, companyID) + if clearErr != nil { logging.Warnf("failed to conditionally clear sanction status for company %s: %v", companyID, clearErr) if h.sssRequired { return false, fmt.Errorf("checkCompanyCompliance: SSS returned clean but clearing the persisted sanction failed for company %s: %w", companyID, clearErr) } } + if !cleared && isSanctioned && sanctionOrigin == "sss" { + // We loaded an SSS-origin block but the conditional clear did not apply, so the + // persisted record changed underneath us (e.g. a concurrent manual/admin block): + // the state is uncertain and may now be a manual block, so fail closed. + logging.Warnf("SSS returned clean for company %s but the persisted SSS block could not be cleared (record changed concurrently); blocking", companyID) + return true, nil + } } return sanctioned, nil diff --git a/cla-backend-legacy/internal/store/companies.go b/cla-backend-legacy/internal/store/companies.go index 6d56dcd18..f79e8efdb 100644 --- a/cla-backend-legacy/internal/store/companies.go +++ b/cla-backend-legacy/internal/store/companies.go @@ -205,9 +205,9 @@ func (s *CompaniesStore) UpdateCompanySanctionStatus(ctx context.Context, compan } // ClearCompanySanctionStatusIfSSS clears is_sanctioned only when sanction_origin="sss". -func (s *CompaniesStore) ClearCompanySanctionStatusIfSSS(ctx context.Context, companyID string) error { +func (s *CompaniesStore) ClearCompanySanctionStatusIfSSS(ctx context.Context, companyID string) (bool, error) { if s == nil || s.client == nil { - return nil + return false, nil } now := time.Now().UTC().Format("2006-01-02T15:04:05.000000-0700") @@ -233,9 +233,9 @@ func (s *CompaniesStore) ClearCompanySanctionStatusIfSSS(ctx context.Context, co if err != nil { var condErr *types.ConditionalCheckFailedException if errors.As(err, &condErr) { - return nil // Already manual/admin or not SSS-flagged + return false, nil // Already manual/admin or not SSS-flagged } - return err + return false, err } - return nil + return true, nil } From a305ab2b4ba2d5fc691c3107cc21819e9267161c Mon Sep 17 00:00:00 2001 From: Lukasz Gryglicki Date: Tue, 9 Jun 2026 14:42:26 +0200 Subject: [PATCH 9/9] Update comments Signed-off-by: Lukasz Gryglicki Assisted by [OpenAI](https://platform.openai.com/) Assisted by [GitHub Copilot](https://github.com/features/copilot) Assisted by [Claude](https://claude.ai) --- cla-backend-go/signatures/service.go | 7 ++++++- cla-backend-go/v2/gitlab-activity/service.go | 3 ++- cla-backend-go/v2/signatures/handlers.go | 4 +++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/cla-backend-go/signatures/service.go b/cla-backend-go/signatures/service.go index c7bb70cf4..b1500aec7 100644 --- a/cla-backend-go/signatures/service.go +++ b/cla-backend-go/signatures/service.go @@ -836,6 +836,9 @@ func (s service) CreateOrUpdateEmployeeSignature(ctx context.Context, claGroupMo // Sanctions gate: never auto-create employee (ECLA) signatures for a sanctioned // company. is_sanctioned is the persisted gate (SSS origin="sss" or a manual/admin // block); both must block ECLA creation (auto-create toggle and approval-list edits). + // By design this enforces the persisted flag, not a live SSS call: the live screen runs + // at the sign/request entry points (CCLA request + legacy ECLA precheck) and keeps this + // flag fresh; secondary paths intentionally honor the persisted state. if companyModel.IsSanctioned { log.WithFields(f).Warnf("company %s is sanctioned (origin=%q); refusing to auto-create employee (ECLA) signatures", companyModel.CompanyID, companyModel.SanctionOrigin) return nil, fmt.Errorf("company %s is sanctioned; employee (ECLA) signatures cannot be created", companyModel.CompanyID) @@ -1529,7 +1532,9 @@ func (s service) ProcessEmployeeSignature(ctx context.Context, companyModel *mod // Sanctions gate: a sanctioned company's employees are not authorized. is_sanctioned // is the persisted gate (SSS origin="sss" or a manual/admin block); honor it here so // employee-acknowledgement (ECLA) authorization fails for sanctioned companies on - // GitHub PR checks and authorization queries. + // GitHub PR checks and authorization queries. By design this enforces the persisted + // flag, not a live SSS call (the live screen at the sign/request entry points keeps it + // fresh). if companyModel.IsSanctioned { log.WithFields(f).Warnf("company %s is sanctioned (origin=%q); employee acknowledgement not authorized", companyModel.CompanyID, companyModel.SanctionOrigin) notSigned := false diff --git a/cla-backend-go/v2/gitlab-activity/service.go b/cla-backend-go/v2/gitlab-activity/service.go index ffc5df823..3538473d2 100644 --- a/cla-backend-go/v2/gitlab-activity/service.go +++ b/cla-backend-go/v2/gitlab-activity/service.go @@ -525,7 +525,8 @@ func (s *service) isSigned(ctx context.Context, userModel *models.User, claGroup // Sanctions gate: a sanctioned company's employees are not authorized. Honor the // persisted is_sanctioned gate (SSS origin="sss" or a manual/admin block) so GitLab - // MR checks fail for sanctioned companies. + // MR checks fail for sanctioned companies. By design this enforces the persisted flag, + // not a live SSS call (the live screen at the sign/request entry points keeps it fresh). if companyModel != nil && companyModel.IsSanctioned { log.WithFields(f).Warnf("company %s is sanctioned (origin=%q); GitLab contributor not authorized", companyID, companyModel.SanctionOrigin) return false, fmt.Errorf("company %s is sanctioned", companyID) diff --git a/cla-backend-go/v2/signatures/handlers.go b/cla-backend-go/v2/signatures/handlers.go index 9382b5784..86db493e7 100644 --- a/cla-backend-go/v2/signatures/handlers.go +++ b/cla-backend-go/v2/signatures/handlers.go @@ -1341,7 +1341,9 @@ func Configure(api *operations.EasyclaAPI, claGroupService service.Service, proj utils.ErrorResponseForbidden(reqID, msg)) } - // Sanctions gate: do not enable ECLA auto-create for a sanctioned company. + // Sanctions gate: do not enable ECLA auto-create for a sanctioned company. By design + // this enforces the persisted is_sanctioned flag, not a live SSS call (the live screen + // at the sign/request entry points keeps it fresh). if eacp.Body.AutoCreateEcla && companyRecord.IsSanctioned { msg := fmt.Sprintf("company %s is sanctioned (origin=%q); cannot enable ECLA auto-create", companyRecord.CompanyID, companyRecord.SanctionOrigin) log.WithFields(f).Warn(msg)