diff --git a/cla-backend-go/v2/sign/service.go b/cla-backend-go/v2/sign/service.go index 838ce03ed..aba2e11d9 100644 --- a/cla-backend-go/v2/sign/service.go +++ b/cla-backend-go/v2/sign/service.go @@ -3074,13 +3074,15 @@ func (s *service) checkCompanyCompliance(ctx context.Context, company *v1Models. } 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 - - // In required mode, only an explicit "clean" is acceptable — any other status blocks. - if s.sssRequired && result.Status != sss.StatusClean && result.Status != sss.StatusFlagged { - return false, fmt.Errorf("checkCompanyCompliance: unexpected SSS status %q for company %s (required mode blocks on ambiguous results)", result.Status, company.CompanyID) + // Only an explicit clean/flagged is actionable. Any other status is ambiguous: block + // when required; otherwise honor the persisted sanction state without clearing or + // caching (never auto-clear an SSS-origin block on an unknown status). + if !sssStatusActionable(result.Status) { + return s.complianceUnavailable(f, company, fmt.Errorf("checkCompanyCompliance: unexpected SSS status %q for company %s", result.Status, company.CompanyID)) } + sanctioned := result.Status == sss.StatusFlagged + // 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. @@ -3135,6 +3137,13 @@ func (s *service) complianceUnavailable(f logrus.Fields, company *v1Models.Compa return company.IsSanctioned, nil } +// sssStatusActionable reports whether an SSS status is one checkCompanyCompliance can act +// on directly (clean or flagged). Any other (ambiguous/unknown) status must be treated as +// "no live result" rather than silently as clean. +func sssStatusActionable(status string) bool { + return status == sss.StatusClean || status == sss.StatusFlagged +} + // 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) { diff --git a/cla-backend-go/v2/sign/service_sss_test.go b/cla-backend-go/v2/sign/service_sss_test.go index 98820c279..a465f9dc8 100644 --- a/cla-backend-go/v2/sign/service_sss_test.go +++ b/cla-backend-go/v2/sign/service_sss_test.go @@ -5,6 +5,7 @@ package sign import ( "context" + "errors" "sync" "testing" "time" @@ -82,6 +83,63 @@ func newTestSSSClient(t *testing.T) *sss.Client { return client } +// checkCompanyCompliance routes an unexpected (non-clean/non-flagged) SSS status through +// complianceUnavailable; these cover its decision so an ambiguous status never auto-clears +// or caches a sanction. +func TestComplianceUnavailableRequiredBlocks(t *testing.T) { + svc := &service{sssRequired: true} + blocked, err := svc.complianceUnavailable(logrus.Fields{}, &models.Company{CompanyID: "company-id"}, errors.New(`unexpected SSS status "weird"`)) + if err == nil { + t.Fatal("expected required mode to block (error) on an ambiguous result") + } + if blocked { + t.Fatal("expected blocked=false alongside the error in required mode") + } +} + +func TestComplianceUnavailableOptionalAllowsUnsanctioned(t *testing.T) { + svc := &service{sssRequired: false} + blocked, err := svc.complianceUnavailable(logrus.Fields{}, &models.Company{CompanyID: "company-id", IsSanctioned: false}, errors.New(`unexpected SSS status "weird"`)) + if err != nil { + t.Fatalf("optional mode should not error on an ambiguous result, got %v", err) + } + if blocked { + t.Fatal("optional mode with no persisted sanction should allow") + } +} + +func TestComplianceUnavailableOptionalHonorsPersistedSanction(t *testing.T) { + svc := &service{sssRequired: false} + blocked, err := svc.complianceUnavailable(logrus.Fields{}, &models.Company{CompanyID: "company-id", IsSanctioned: true, SanctionOrigin: "sss"}, errors.New(`unexpected SSS status "weird"`)) + if err != nil { + t.Fatalf("optional mode should not error on an ambiguous result, got %v", err) + } + if !blocked { + t.Fatal("optional mode must keep blocking a persisted sanction (no auto-clear) on an ambiguous result") + } +} + +// sssStatusActionable is the guard checkCompanyCompliance uses to route ambiguous statuses +// to complianceUnavailable; flipping it is the regression that would let an unknown status +// fall through to the clean/clear path. +func TestSSSStatusActionable(t *testing.T) { + cases := []struct { + status string + want bool + }{ + {sss.StatusClean, true}, + {sss.StatusFlagged, true}, + {"pending", false}, + {"PENDING", false}, + {"", false}, + } + for _, tc := range cases { + if got := sssStatusActionable(tc.status); got != tc.want { + t.Errorf("sssStatusActionable(%q) = %v, want %v", tc.status, got, tc.want) + } + } +} + func TestCheckCompanyComplianceRequiredBlocksMissingExternalID(t *testing.T) { svc := &service{sssRequired: true, sssClient: newTestSSSClient(t)} diff --git a/cla-backend-legacy/internal/api/handlers.go b/cla-backend-legacy/internal/api/handlers.go index df24c6f12..6e1851b0f 100644 --- a/cla-backend-legacy/internal/api/handlers.go +++ b/cla-backend-legacy/internal/api/handlers.go @@ -8997,13 +8997,19 @@ func (h *Handlers) checkCompanyCompliance(ctx context.Context, company map[strin } 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 - - // In required mode, only an explicit "clean" is acceptable — any other status blocks. - if h.sssRequired && result.Status != sss.StatusClean && result.Status != sss.StatusFlagged { - return false, fmt.Errorf("checkCompanyCompliance: unexpected SSS status %q for company %s (required mode blocks on ambiguous results)", result.Status, companyID) + // Only an explicit clean/flagged is actionable. Any other status is ambiguous: block + // when required; otherwise honor the persisted sanction state without clearing (never + // auto-clear an SSS-origin block on an unknown status). + if result.Status != sss.StatusClean && result.Status != sss.StatusFlagged { + if h.sssRequired { + return false, fmt.Errorf("checkCompanyCompliance: unexpected SSS status %q for company %s", result.Status, companyID) + } + logging.Warnf("unexpected SSS status %q for company %s; SSS is not required, honoring persisted sanction state", result.Status, companyID) + return isSanctioned, nil } + sanctioned := result.Status == sss.StatusFlagged + // Persist result: set origin="sss" on flagged; conditionally clear on clean (only if sss-origin). if sanctioned { logging.Warnf("SSS returned flagged status for company %s, persisting sanction with origin=sss", companyID)