From 0bc2edb8cf8981d20787121cce72d395c859ca47 Mon Sep 17 00:00:00 2001 From: Jesus Cabrera <96826623+jacsdev@users.noreply.github.com> Date: Fri, 24 Jul 2026 12:37:06 -0300 Subject: [PATCH 1/3] fix(cloud): allow issued_token key in bootstrap audit metadata `engram cloud bootstrap admin --issue-token` always failed with "sensitive auth audit metadata is not allowed: issued_token". The bootstrap completion audit records a boolean `issued_token` flag, but sensitiveAuthAuditKey rejected every key containing "token". Because the token and its audit are persisted atomically, the token was never minted, and since bootstrap refuses to run twice this left a token-less admin with no supported recovery path (all token-issuing routes require an existing managed-admin token). Whitelist `issued_token` next to the existing `token_prefix` exception: it is a non-secret boolean flag, not credential material. Add a regression test asserting the key is accepted. Fixes #597 Fixes #603 --- internal/cloud/cloudstore/identity.go | 7 ++++++- internal/cloud/cloudstore/identity_storage_test.go | 11 +++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/internal/cloud/cloudstore/identity.go b/internal/cloud/cloudstore/identity.go index ce80acc1..2b253927 100644 --- a/internal/cloud/cloudstore/identity.go +++ b/internal/cloud/cloudstore/identity.go @@ -955,7 +955,12 @@ func rejectSensitiveAuthAuditValue(value any) error { func sensitiveAuthAuditKey(key string) bool { key = strings.ToLower(strings.TrimSpace(key)) - if key == "token_prefix" { + // Known-safe keys whose names match the sensitive-fragment heuristic below + // but never carry a secret value: token_prefix is the short non-secret + // prefix, and issued_token is a boolean flag recorded by the bootstrap + // completion audit (see cloudBootstrapCompletionMetadata). + switch key { + case "token_prefix", "issued_token": return false } for _, fragment := range []string{"token", "authorization", "cookie", "secret", "hash", "password", "bearer"} { diff --git a/internal/cloud/cloudstore/identity_storage_test.go b/internal/cloud/cloudstore/identity_storage_test.go index 3e45a535..b91f53dd 100644 --- a/internal/cloud/cloudstore/identity_storage_test.go +++ b/internal/cloud/cloudstore/identity_storage_test.go @@ -522,6 +522,9 @@ func TestCloudstoreIdentityPureHelpers(t *testing.T) { if sensitiveAuthAuditKey("token_prefix") { t.Fatal("token_prefix is safe metadata and should not be rejected") } + if sensitiveAuthAuditKey("issued_token") { + t.Fatal("issued_token is a safe boolean flag and should not be rejected") + } for _, key := range []string{"raw_token", "authorization_header", "session_cookie", "token_hash", "password"} { if !sensitiveAuthAuditKey(key) { t.Fatalf("%s should be classified as sensitive audit metadata", key) @@ -536,6 +539,14 @@ func TestCloudstoreIdentityPureHelpers(t *testing.T) { if err := rejectSensitiveAuthAuditMetadata(map[string]any{"events": []map[string]any{{"raw_token": "secret"}}}); !errors.Is(err, ErrSensitiveAuditMetadata) { t.Fatalf("typed nested slice sensitive audit metadata must be rejected, got %v", err) } + // Regression: the bootstrap completion audit metadata (created by + // cloudBootstrapCompletionMetadata) carries the boolean flag "issued_token". + // It is not a secret and must be accepted, otherwise + // `engram cloud bootstrap admin --issue-token` fails atomically and no admin + // token is ever minted. + if err := rejectSensitiveAuthAuditMetadata(map[string]any{"issued_token": true, "username": "admin", "created_admin": true}); err != nil { + t.Fatalf("bootstrap completion metadata must be accepted, got %v", err) + } } func tableExists(t *testing.T, db *sql.DB, table string) bool { From 3bb4fb00489409fb25e4087994fb8cbf0a74c949 Mon Sep 17 00:00:00 2001 From: Jesus Cabrera <96826623+jacsdev@users.noreply.github.com> Date: Fri, 24 Jul 2026 12:59:55 -0300 Subject: [PATCH 2/3] fix(cloud): gate issued_token audit exemption on boolean value Address review feedback: the issued_token whitelist bypassed the sensitive-metadata filter by key name alone, so a non-boolean value could smuggle secret material under a trusted key. Exempt issued_token only when its value is actually a bool, at the top level and recursively; otherwise the key falls through to the normal sensitive-fragment heuristic. Adds rejection tests for string and nested non-boolean issued_token values. --- internal/cloud/cloudstore/identity.go | 31 +++++++++++++------ .../cloud/cloudstore/identity_storage_test.go | 15 +++++++-- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/internal/cloud/cloudstore/identity.go b/internal/cloud/cloudstore/identity.go index 2b253927..de246b30 100644 --- a/internal/cloud/cloudstore/identity.go +++ b/internal/cloud/cloudstore/identity.go @@ -906,9 +906,22 @@ func guardLastActiveAdminTx(ctx context.Context, tx *sql.Tx, principalID string, return nil } +// auditKeyValueExempt reports whether a (key, value) pair should bypass the +// sensitive-key heuristic based on its value. issued_token is a boolean flag +// written by the bootstrap completion audit (see cloudBootstrapCompletionMetadata); +// it is exempt ONLY when the value is actually a bool, so the exemption cannot +// be used to smuggle a secret string under a trusted key name. +func auditKeyValueExempt(key string, value any) bool { + if strings.EqualFold(strings.TrimSpace(key), "issued_token") { + _, ok := value.(bool) + return ok + } + return false +} + func rejectSensitiveAuthAuditMetadata(metadata map[string]any) error { for key, value := range metadata { - if sensitiveAuthAuditKey(key) { + if !auditKeyValueExempt(key, value) && sensitiveAuthAuditKey(key) { return fmt.Errorf("%w: %s", ErrSensitiveAuditMetadata, key) } if err := rejectSensitiveAuthAuditValue(value); err != nil { @@ -936,10 +949,11 @@ func rejectSensitiveAuthAuditValue(value any) error { } for _, key := range reflected.MapKeys() { keyText := key.String() - if sensitiveAuthAuditKey(keyText) { + entryValue := reflected.MapIndex(key).Interface() + if !auditKeyValueExempt(keyText, entryValue) && sensitiveAuthAuditKey(keyText) { return fmt.Errorf("%w: %s", ErrSensitiveAuditMetadata, keyText) } - if err := rejectSensitiveAuthAuditValue(reflected.MapIndex(key).Interface()); err != nil { + if err := rejectSensitiveAuthAuditValue(entryValue); err != nil { return err } } @@ -955,12 +969,11 @@ func rejectSensitiveAuthAuditValue(value any) error { func sensitiveAuthAuditKey(key string) bool { key = strings.ToLower(strings.TrimSpace(key)) - // Known-safe keys whose names match the sensitive-fragment heuristic below - // but never carry a secret value: token_prefix is the short non-secret - // prefix, and issued_token is a boolean flag recorded by the bootstrap - // completion audit (see cloudBootstrapCompletionMetadata). - switch key { - case "token_prefix", "issued_token": + // token_prefix is the short, non-secret token prefix; it matches the + // fragment heuristic below but never carries secret material. issued_token + // is handled by auditKeyValueExempt instead, because it is only safe when + // its value is the boolean flag the bootstrap completion audit records. + if key == "token_prefix" { return false } for _, fragment := range []string{"token", "authorization", "cookie", "secret", "hash", "password", "bearer"} { diff --git a/internal/cloud/cloudstore/identity_storage_test.go b/internal/cloud/cloudstore/identity_storage_test.go index b91f53dd..c0078c76 100644 --- a/internal/cloud/cloudstore/identity_storage_test.go +++ b/internal/cloud/cloudstore/identity_storage_test.go @@ -522,9 +522,6 @@ func TestCloudstoreIdentityPureHelpers(t *testing.T) { if sensitiveAuthAuditKey("token_prefix") { t.Fatal("token_prefix is safe metadata and should not be rejected") } - if sensitiveAuthAuditKey("issued_token") { - t.Fatal("issued_token is a safe boolean flag and should not be rejected") - } for _, key := range []string{"raw_token", "authorization_header", "session_cookie", "token_hash", "password"} { if !sensitiveAuthAuditKey(key) { t.Fatalf("%s should be classified as sensitive audit metadata", key) @@ -547,6 +544,18 @@ func TestCloudstoreIdentityPureHelpers(t *testing.T) { if err := rejectSensitiveAuthAuditMetadata(map[string]any{"issued_token": true, "username": "admin", "created_admin": true}); err != nil { t.Fatalf("bootstrap completion metadata must be accepted, got %v", err) } + // issued_token is exempt ONLY as a boolean flag. A non-boolean value (e.g. a + // string that could carry secret material) must NOT bypass the filter, at + // the top level or nested. + if err := rejectSensitiveAuthAuditMetadata(map[string]any{"issued_token": "egc_live_would_be_leaked"}); !errors.Is(err, ErrSensitiveAuditMetadata) { + t.Fatalf("non-boolean issued_token must be rejected, got %v", err) + } + if err := rejectSensitiveAuthAuditMetadata(map[string]any{"outer": map[string]any{"issued_token": "egc_live_would_be_leaked"}}); !errors.Is(err, ErrSensitiveAuditMetadata) { + t.Fatalf("nested non-boolean issued_token must be rejected, got %v", err) + } + if err := rejectSensitiveAuthAuditMetadata(map[string]any{"outer": map[string]any{"issued_token": true}}); err != nil { + t.Fatalf("nested boolean issued_token must be accepted, got %v", err) + } } func tableExists(t *testing.T, db *sql.DB, table string) bool { From 53a9b5ada5ddf3e22e2f2c3ab2863d67e36b17ea Mon Sep 17 00:00:00 2001 From: Jesus Cabrera <96826623+jacsdev@users.noreply.github.com> Date: Fri, 24 Jul 2026 13:06:34 -0300 Subject: [PATCH 3/3] test(cloud): cover issued_token=false in audit metadata exemption --- internal/cloud/cloudstore/identity_storage_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/cloud/cloudstore/identity_storage_test.go b/internal/cloud/cloudstore/identity_storage_test.go index c0078c76..79359416 100644 --- a/internal/cloud/cloudstore/identity_storage_test.go +++ b/internal/cloud/cloudstore/identity_storage_test.go @@ -544,6 +544,11 @@ func TestCloudstoreIdentityPureHelpers(t *testing.T) { if err := rejectSensitiveAuthAuditMetadata(map[string]any{"issued_token": true, "username": "admin", "created_admin": true}); err != nil { t.Fatalf("bootstrap completion metadata must be accepted, got %v", err) } + // issued_token is false for grant-only bootstraps (no --issue-token); both + // boolean values must be accepted. + if err := rejectSensitiveAuthAuditMetadata(map[string]any{"issued_token": false}); err != nil { + t.Fatalf("false issued_token must be accepted, got %v", err) + } // issued_token is exempt ONLY as a boolean flag. A non-boolean value (e.g. a // string that could carry secret material) must NOT bypass the filter, at // the top level or nested.