From 18057c24ad318bba28b4f4c3e0e2aba2dcc1964c Mon Sep 17 00:00:00 2001 From: safayavatsal Date: Tue, 28 Apr 2026 17:52:34 +0530 Subject: [PATCH 1/2] =?UTF-8?q?fix:=20enforce=20SPIFFE=202048-byte=20ID=20?= =?UTF-8?q?cap=20at=20URI=20construction=20(=C2=A72.4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BuildWIMSEURI was a Sprintf with no length check, so the SPIFFE §2.4 cap was unenforced. Today's varchar(255) schema bounds the URI to about 1080 bytes, so this isn't reachable through the API — but the invariant belongs at the construction site so a future schema relaxation can't silently mint non-conformant SPIFFE IDs. Changes BuildWIMSEURI to return (string, error), introduces MaxSPIFFEIDBytes = 2048, and propagates the error from RegisterIdentity. Domain tests cover rejection, happy path, and the inclusive boundary at exactly 2048 bytes. Note: signature change to a public domain function. No external consumers in the repo (pkg/authjwt does not call it). Fixes #48 --- domain/identity.go | 17 +++++++++--- domain/identity_test.go | 54 ++++++++++++++++++++++++++++++++++++ internal/service/identity.go | 7 ++++- 3 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 domain/identity_test.go diff --git a/domain/identity.go b/domain/identity.go index 209d8df..915028a 100644 --- a/domain/identity.go +++ b/domain/identity.go @@ -340,8 +340,17 @@ func GetIdentitySchema() *IdentitySchema { } } -// BuildWIMSEURI constructs the WIMSE URI for an identity. -// Format: spiffe://{domain}/{account_id}/{project_id}/{identity_type}/{external_id} -func BuildWIMSEURI(wimseDomain, accountID, projectID string, identityType IdentityType, externalID string) string { - return fmt.Sprintf("spiffe://%s/%s/%s/%s/%s", wimseDomain, accountID, projectID, identityType, externalID) +// MaxSPIFFEIDBytes is the SPIFFE §2.4 hard cap. Spec says MUST NOT exceed. +const MaxSPIFFEIDBytes = 2048 + +// BuildWIMSEURI constructs the WIMSE URI for an identity: +// spiffe://{domain}/{account_id}/{project_id}/{identity_type}/{external_id}. +// Returns an error if the result exceeds MaxSPIFFEIDBytes — once persisted, +// every downstream system inherits a non-conformant subject claim. +func BuildWIMSEURI(wimseDomain, accountID, projectID string, identityType IdentityType, externalID string) (string, error) { + uri := fmt.Sprintf("spiffe://%s/%s/%s/%s/%s", wimseDomain, accountID, projectID, identityType, externalID) + if n := len(uri); n > MaxSPIFFEIDBytes { + return "", fmt.Errorf("SPIFFE ID exceeds %d bytes (got %d): %s…", MaxSPIFFEIDBytes, n, uri[:64]) + } + return uri, nil } diff --git a/domain/identity_test.go b/domain/identity_test.go new file mode 100644 index 0000000..f54dcf2 --- /dev/null +++ b/domain/identity_test.go @@ -0,0 +1,54 @@ +package domain + +import ( + "strings" + "testing" +) + +// TestBuildWIMSEURIRejectsOver2048Bytes locks in SPIFFE §2.4. Today's +// varchar(255) schema makes this unreachable through the API surface — the +// test exists so a future schema bump can't silently mint over-cap IDs. +func TestBuildWIMSEURIRejectsOver2048Bytes(t *testing.T) { + // 2200-byte external_id forces the assembled URI past 2048 bytes + // regardless of any other field. + tooLong := strings.Repeat("a", 2200) + + _, err := BuildWIMSEURI("highflame.ai", "acct", "proj", IdentityTypeAgent, tooLong) + if err == nil { + t.Fatalf("expected error for SPIFFE ID > %d bytes", MaxSPIFFEIDBytes) + } + if !strings.Contains(err.Error(), "exceeds 2048 bytes") { + t.Fatalf("error must name the cap so callers can act on it; got %q", err.Error()) + } +} + +// TestBuildWIMSEURIAcceptsTypicalSize covers the happy path so we'd notice +// if a future change tightened the cap by accident. +func TestBuildWIMSEURIAcceptsTypicalSize(t *testing.T) { + uri, err := BuildWIMSEURI("highflame.ai", "acct-001", "proj-001", IdentityTypeAgent, "agent-1") + if err != nil { + t.Fatalf("unexpected error for short URI: %v", err) + } + want := "spiffe://highflame.ai/acct-001/proj-001/agent/agent-1" + if uri != want { + t.Fatalf("URI shape changed: got %q, want %q", uri, want) + } +} + +// TestBuildWIMSEURIBoundary checks the inclusive boundary at exactly the +// cap. 2048 bytes must succeed; 2049 must fail. +func TestBuildWIMSEURIBoundary(t *testing.T) { + // Prefix length: "spiffe://" + "d" + "/" + "a" + "/" + "p" + "/" + + // "agent" + "/" = 9 + 1 + 1 + 1 + 1 + 1 + 1 + 5 + 1 = 21. + prefixLen := len("spiffe://d/a/p/agent/") + atCap := strings.Repeat("a", MaxSPIFFEIDBytes-prefixLen) + + if _, err := BuildWIMSEURI("d", "a", "p", IdentityTypeAgent, atCap); err != nil { + t.Fatalf("URI of exactly %d bytes should be allowed: %v", MaxSPIFFEIDBytes, err) + } + + overCap := atCap + "a" + if _, err := BuildWIMSEURI("d", "a", "p", IdentityTypeAgent, overCap); err == nil { + t.Fatalf("URI of %d bytes should be rejected", MaxSPIFFEIDBytes+1) + } +} diff --git a/internal/service/identity.go b/internal/service/identity.go index ed8a290..6bc27e9 100644 --- a/internal/service/identity.go +++ b/internal/service/identity.go @@ -157,13 +157,18 @@ func (s *IdentityService) RegisterIdentity(ctx context.Context, req RegisterIden return nil, err } + wimseURI, err := domain.BuildWIMSEURI(s.wimseDomain, req.AccountID, req.ProjectID, req.IdentityType, req.ExternalID) + if err != nil { + return nil, err + } + identity := &domain.Identity{ ID: uuid.New().String(), AccountID: req.AccountID, ProjectID: req.ProjectID, ExternalID: req.ExternalID, Name: req.Name, - WIMSEURI: domain.BuildWIMSEURI(s.wimseDomain, req.AccountID, req.ProjectID, req.IdentityType, req.ExternalID), + WIMSEURI: wimseURI, IdentityType: req.IdentityType, SubType: req.SubType, TrustLevel: req.TrustLevel, From c53387262b1eada942045dac76cfb81364916a1d Mon Sep 17 00:00:00 2001 From: safayavatsal Date: Thu, 7 May 2026 13:42:12 +0530 Subject: [PATCH 2/2] Add ErrSPIFFEIDTooLong sentinel and use errors.Is in tests - Introduce domain.ErrSPIFFEIDTooLong wrapped by BuildWIMSEURI via %w - Switch boundary tests to errors.Is for typed error checks --- domain/identity.go | 8 +++++++- domain/identity_test.go | 11 ++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/domain/identity.go b/domain/identity.go index af3039e..9a3418a 100644 --- a/domain/identity.go +++ b/domain/identity.go @@ -4,12 +4,18 @@ package domain import ( "encoding/json" + "errors" "fmt" "time" "github.com/uptrace/bun" ) +// ErrSPIFFEIDTooLong is returned by BuildWIMSEURI when the assembled URI +// exceeds MaxSPIFFEIDBytes. Callers can branch on this with errors.Is to +// distinguish the cap-exceeded case from generic build failures. +var ErrSPIFFEIDTooLong = errors.New("SPIFFE ID exceeds maximum length") + // ────────────────────────────────────────────────────────────────────────────── // Trust Level // ────────────────────────────────────────────────────────────────────────────── @@ -418,7 +424,7 @@ const MaxSPIFFEIDBytes = 2048 func BuildWIMSEURI(wimseDomain, accountID, projectID string, identityType IdentityType, externalID string) (string, error) { uri := fmt.Sprintf("spiffe://%s/%s/%s/%s/%s", wimseDomain, accountID, projectID, identityType, externalID) if n := len(uri); n > MaxSPIFFEIDBytes { - return "", fmt.Errorf("SPIFFE ID exceeds %d bytes (got %d): %s…", MaxSPIFFEIDBytes, n, uri[:64]) + return "", fmt.Errorf("%w: got %d bytes, max %d: %.64q", ErrSPIFFEIDTooLong, n, MaxSPIFFEIDBytes, uri) } return uri, nil } diff --git a/domain/identity_test.go b/domain/identity_test.go index f54dcf2..b65fd53 100644 --- a/domain/identity_test.go +++ b/domain/identity_test.go @@ -1,6 +1,7 @@ package domain import ( + "errors" "strings" "testing" ) @@ -17,8 +18,8 @@ func TestBuildWIMSEURIRejectsOver2048Bytes(t *testing.T) { if err == nil { t.Fatalf("expected error for SPIFFE ID > %d bytes", MaxSPIFFEIDBytes) } - if !strings.Contains(err.Error(), "exceeds 2048 bytes") { - t.Fatalf("error must name the cap so callers can act on it; got %q", err.Error()) + if !errors.Is(err, ErrSPIFFEIDTooLong) { + t.Fatalf("error must wrap ErrSPIFFEIDTooLong so callers can branch on it; got %q", err.Error()) } } @@ -48,7 +49,11 @@ func TestBuildWIMSEURIBoundary(t *testing.T) { } overCap := atCap + "a" - if _, err := BuildWIMSEURI("d", "a", "p", IdentityTypeAgent, overCap); err == nil { + _, err := BuildWIMSEURI("d", "a", "p", IdentityTypeAgent, overCap) + if err == nil { t.Fatalf("URI of %d bytes should be rejected", MaxSPIFFEIDBytes+1) } + if !errors.Is(err, ErrSPIFFEIDTooLong) { + t.Fatalf("error must wrap ErrSPIFFEIDTooLong so callers can branch on it; got %q", err.Error()) + } }