Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 18 additions & 10 deletions core/schemas/vault.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,25 +80,30 @@ var (
// RemoveOwnedVaultSecretVars best-effort deletes the vault secret for every
// SecretVar / *SecretVar field in model whose VaultRef starts with
// ownedPrefix+"/". Refs outside that prefix are user-provided and are left alone.
func RemoveOwnedVaultSecretVars(ctx context.Context, ownedPrefix string, model interface{}) {
// Returns one error per field whose deletion failed; callers should log these but
// must not treat them as fatal (the DB row is already deleted).
func RemoveOwnedVaultSecretVars(ctx context.Context, ownedPrefix string, model interface{}) []error {
if VaultRemoveHook == nil {
return
return nil
}
rv := reflect.ValueOf(model)
if rv.Kind() == reflect.Ptr {
rv = rv.Elem()
}
if rv.Kind() != reflect.Struct {
return
return nil
}
rt := rv.Type()
var errs []error
for i := 0; i < rt.NumField(); i++ {
fv := rv.Field(i)
if fv.Type() == secretVarMapType {
iter := fv.MapRange()
for iter.Next() {
e := iter.Value().Interface().(SecretVar)
removeOwnedVaultSecretVar(ctx, ownedPrefix, &e)
if err := removeOwnedVaultSecretVar(ctx, ownedPrefix, &e); err != nil {
errs = append(errs, err)
}
}
continue
}
Expand All @@ -111,25 +116,28 @@ func RemoveOwnedVaultSecretVars(ctx context.Context, ownedPrefix string, model i
field = fv.Interface().(*SecretVar)
}
}
removeOwnedVaultSecretVar(ctx, ownedPrefix, field)
if err := removeOwnedVaultSecretVar(ctx, ownedPrefix, field); err != nil {
errs = append(errs, err)
}
}
return errs
}

// removeOwnedVaultSecretVar removes a single SecretVar's vault secret if it is a
// vault-backed, non-fragment reference under ownedPrefix. Fragment refs (#key)
// point at shared, externally-managed secrets and are never auto-deleted.
func removeOwnedVaultSecretVar(ctx context.Context, ownedPrefix string, field *SecretVar) {
func removeOwnedVaultSecretVar(ctx context.Context, ownedPrefix string, field *SecretVar) error {
path := field.GetRef()
if path == "" {
return
return nil
}
if strings.IndexByte(path, '#') >= 0 {
return
return nil
}
if !strings.HasPrefix(path, ownedPrefix+"/") {
return
return nil
}
_ = VaultRemoveHook(ctx, path)
return VaultRemoveHook(ctx, path)
}

// StoreVaultSecretVar pushes a single plaintext SecretVar value into the vault at path
Expand Down
5 changes: 3 additions & 2 deletions framework/configstore/clientconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -863,8 +863,9 @@ func GenerateVirtualKeyHash(vk tables.TableVirtualKey) (string, error) {
hash.Write([]byte(vk.Name))
// Hash Description
hash.Write([]byte(vk.Description))
// Hash Value
hash.Write([]byte(vk.Value))
// Hash the resolved value so that secret rotation (vault/env change) is
// detected as a config change and triggers a re-sync.
hash.Write([]byte(vk.Value.GetValue()))
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// Hash IsActive (nil treated as DB default true)
if vk.IsActiveValue() {
hash.Write([]byte("isActive:true"))
Expand Down
4 changes: 2 additions & 2 deletions framework/configstore/encryption_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ func TestEncryptPlaintextVirtualKeys_EncryptsAndDecryptsCorrectly(t *testing.T)
// GORM hooks should decrypt on read
var found tables.TableVirtualKey
require.NoError(t, db.Where("id = ?", "vk-batch-1").First(&found).Error)
assert.Equal(t, "vk-batch-secret", found.Value)
assert.Equal(t, "vk-batch-secret", found.Value.GetValue())
}

func TestEncryptPlaintextOAuthConfigs_EncryptsAndDecryptsCorrectly(t *testing.T) {
Expand Down Expand Up @@ -1342,7 +1342,7 @@ func TestEncryptPlaintextRows_SkipsAlreadyEncryptedVirtualKeys(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: "vk-already-enc",
Name: "already-encrypted-vk",
Value: "vk-secret-already",
Value: *schemas.NewSecretVar("vk-secret-already"),
IsActive: bifrost.Ptr(true),
}
require.NoError(t, db.Create(vk).Error)
Expand Down
8 changes: 4 additions & 4 deletions framework/configstore/migrations_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1250,7 +1250,7 @@ func TestFullMigration_VirtualKeyCRUD(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: "vk-test-001",
Name: "test-virtual-key",
Value: "vk-secret-value-12345",
Value: *schemas.NewSecretVar("vk-secret-value-12345"),
IsActive: bifrost.Ptr(true),
CreatedAt: now,
UpdatedAt: now,
Expand All @@ -1266,7 +1266,7 @@ func TestFullMigration_VirtualKeyCRUD(t *testing.T) {

assert.Equal(t, "vk-test-001", vks[0].ID)
assert.Equal(t, "test-virtual-key", vks[0].Name)
assert.Equal(t, "vk-secret-value-12345", vks[0].Value) // AfterFind decrypts
assert.Equal(t, "vk-secret-value-12345", vks[0].Value.GetValue()) // AfterFind decrypts
assert.True(t, vks[0].IsActiveValue())

// Verify encryption at raw DB level
Expand Down Expand Up @@ -1396,7 +1396,7 @@ func TestFullMigration_EncryptPlaintextRows(t *testing.T) {
var vk tables.TableVirtualKey
err = db.Where("id = ?", "vk-plain-1").First(&vk).Error
require.NoError(t, err)
assert.Equal(t, "vk-plain-secret", vk.Value)
assert.Equal(t, "vk-plain-secret", vk.Value.Val)
}

func TestFullMigration_EndToEnd(t *testing.T) {
Expand Down Expand Up @@ -1438,7 +1438,7 @@ func TestFullMigration_EndToEnd(t *testing.T) {
{"vk-2", "vk-beta", "vk-beta-secret"},
} {
err := store.CreateVirtualKey(ctx, &tables.TableVirtualKey{
ID: vk.id, Name: vk.name, Value: vk.value,
ID: vk.id, Name: vk.name, Value: *schemas.NewSecretVar(vk.value),
IsActive: bifrost.Ptr(true), CreatedAt: now, UpdatedAt: now,
})
require.NoError(t, err, "CreateVirtualKey %s", vk.name)
Expand Down
6 changes: 3 additions & 3 deletions framework/configstore/rdb_deadlock_postgres_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func TestPostgresVirtualKeyBudgetConcurrentMutationsDoNotDeadlock(t *testing.T)
if err := store.UpdateVirtualKey(ctx, &tables.TableVirtualKey{
ID: vkID,
Name: "PG VK Budget",
Value: "pg-vk-budget-value",
Value: *schemas.NewSecretVar("pg-vk-budget-value"),
IsActive: schemas.Ptr(true),
}, tx); err != nil {
return err
Expand Down Expand Up @@ -247,7 +247,7 @@ func seedProviderGraph(ctx context.Context, store *RDBConfigStore) error {
if err := store.CreateVirtualKey(ctx, &tables.TableVirtualKey{
ID: "pg-vk",
Name: "PG VK",
Value: fmt.Sprintf("pg-vk-value-%d", time.Now().UnixNano()),
Value: *schemas.NewSecretVar(fmt.Sprintf("pg-vk-value-%d", time.Now().UnixNano())),
IsActive: schemas.Ptr(true),
}); err != nil && !isUniqueRace(err) {
return err
Expand All @@ -271,7 +271,7 @@ func seedVirtualKeyBudget(ctx context.Context, t *testing.T, store *RDBConfigSto
require.NoError(t, store.CreateVirtualKey(ctx, &tables.TableVirtualKey{
ID: vkID,
Name: "PG VK Budget",
Value: "pg-vk-budget-value",
Value: *schemas.NewSecretVar("pg-vk-budget-value"),
IsActive: schemas.Ptr(true),
}))
require.NoError(t, store.CreateBudget(ctx, &tables.TableBudget{
Expand Down
3 changes: 2 additions & 1 deletion framework/configstore/rdb_mcp_sessions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"
"time"

"github.com/maximhq/bifrost/core/schemas"
"github.com/maximhq/bifrost/framework/configstore/tables"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -39,7 +40,7 @@ func seedMCPSessionsFixture(t *testing.T, store *RDBConfigStore) {
vk := &tables.TableVirtualKey{
ID: "vk-alpha",
Name: "Alpha VK",
Value: "sk-bf-alpha",
Value: *schemas.NewSecretVar("sk-bf-alpha"),
}
require.NoError(t, store.DB().WithContext(ctx).Create(vk).Error)

Expand Down
5 changes: 3 additions & 2 deletions framework/configstore/rdb_oauth2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"testing"
"time"

"github.com/maximhq/bifrost/core/schemas"
"github.com/maximhq/bifrost/framework/configstore/tables"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -370,7 +371,7 @@ func TestListOAuth2Sessions_JoinsAndExcludesRevoked(t *testing.T) {
ID: "crow", ClientID: "client-1", ClientName: "Test Client",
RedirectURIs: []string{"http://127.0.0.1/cb"}, GrantTypes: []string{"authorization_code"}, CreatedAt: time.Now(),
}).Error)
require.NoError(t, s.DB().Create(&tables.TableVirtualKey{ID: "vk-1", Name: "Alpha VK", Value: "sk-bf-alpha"}).Error)
require.NoError(t, s.DB().Create(&tables.TableVirtualKey{ID: "vk-1", Name: "Alpha VK", Value: *schemas.NewSecretVar("sk-bf-alpha")}).Error)

vkTok := makeRefreshToken("rt-vk", "f1", "client-1", "h-vk")
vkTok.BfSub = "vk-1" // joins to governance_virtual_keys.id
Expand Down Expand Up @@ -425,7 +426,7 @@ func TestListOAuth2Sessions_FilterAndPaginate(t *testing.T) {
ID: "c2", ClientID: "client-2", ClientName: "Beta Server",
RedirectURIs: []string{"http://127.0.0.1/cb"}, GrantTypes: []string{"authorization_code"}, CreatedAt: time.Now(),
}).Error)
require.NoError(t, s.DB().Create(&tables.TableVirtualKey{ID: "vk-1", Name: "Alpha VK", Value: "sk-bf-alpha"}).Error)
require.NoError(t, s.DB().Create(&tables.TableVirtualKey{ID: "vk-1", Name: "Alpha VK", Value: *schemas.NewSecretVar("sk-bf-alpha")}).Error)

base := time.Now()
rtA := makeRefreshToken("rt-a", "fa", "client-1", "h-a") // vk mode, bf_sub vk-1 → display "Alpha VK"
Expand Down
36 changes: 18 additions & 18 deletions framework/configstore/rdb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,7 @@ func TestCreateVirtualKey(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: "vk-test",
Name: "Test Virtual Key",
Value: "vk-test-value-123",
Value: *schemas.NewSecretVar("vk-test-value-123"),
IsActive: schemas.Ptr(true),
}

Expand All @@ -877,7 +877,7 @@ func TestCreateVirtualKey(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "vk-test", result.ID)
assert.Equal(t, "Test Virtual Key", result.Name)
assert.Equal(t, "vk-test-value-123", result.Value)
assert.Equal(t, "vk-test-value-123", result.Value.Val)
assert.True(t, result.IsActiveValue())
}

Expand Down Expand Up @@ -911,7 +911,7 @@ func TestCreateVirtualKey_WithBudgetAndRateLimit(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: vkID,
Name: "VK With References",
Value: "vk-refs-value",
Value: *schemas.NewSecretVar("vk-refs-value"),
IsActive: schemas.Ptr(true),
RateLimitID: &rateLimitID,
}
Expand Down Expand Up @@ -939,7 +939,7 @@ func TestCreateVirtualKey_DuplicateName(t *testing.T) {
vk1 := &tables.TableVirtualKey{
ID: "vk-1",
Name: "Same Name",
Value: "vk-value-1",
Value: *schemas.NewSecretVar("vk-value-1"),
IsActive: schemas.Ptr(true),
}
err := store.CreateVirtualKey(ctx, vk1)
Expand All @@ -948,7 +948,7 @@ func TestCreateVirtualKey_DuplicateName(t *testing.T) {
vk2 := &tables.TableVirtualKey{
ID: "vk-2",
Name: "Same Name", // Duplicate name
Value: "vk-value-2",
Value: *schemas.NewSecretVar("vk-value-2"),
IsActive: schemas.Ptr(true),
}
err = store.CreateVirtualKey(ctx, vk2)
Expand All @@ -962,7 +962,7 @@ func TestGetVirtualKeyByValue(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: "vk-lookup",
Name: "Lookup Key",
Value: "vk-unique-value-xyz",
Value: *schemas.NewSecretVar("vk-unique-value-xyz"),
IsActive: schemas.Ptr(true),
}
err := store.CreateVirtualKey(ctx, vk)
Expand All @@ -980,7 +980,7 @@ func TestUpdateVirtualKey(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: "vk-update",
Name: "Original Name",
Value: "vk-update-value",
Value: *schemas.NewSecretVar("vk-update-value"),
IsActive: schemas.Ptr(true),
}
err := store.CreateVirtualKey(ctx, vk)
Expand All @@ -1005,7 +1005,7 @@ func TestDeleteVirtualKey(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: "vk-delete",
Name: "Delete Me",
Value: "vk-delete-value",
Value: *schemas.NewSecretVar("vk-delete-value"),
IsActive: schemas.Ptr(true),
}
err := store.CreateVirtualKey(ctx, vk)
Expand All @@ -1025,7 +1025,7 @@ func TestDeleteVirtualKey_RevokesInboundVKGrants(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: "vk-grant",
Name: "Grant VK",
Value: "vk-grant-value",
Value: *schemas.NewSecretVar("vk-grant-value"),
IsActive: schemas.Ptr(true),
}
require.NoError(t, store.CreateVirtualKey(ctx, vk))
Expand Down Expand Up @@ -1059,7 +1059,7 @@ func TestDeleteVirtualKey_CleansUpScopedModelConfigs(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: "vk-scoped",
Name: "Scoped VK",
Value: "vk-scoped-value",
Value: *schemas.NewSecretVar("vk-scoped-value"),
IsActive: schemas.Ptr(true),
}
require.NoError(t, store.CreateVirtualKey(ctx, vk))
Expand Down Expand Up @@ -1113,7 +1113,7 @@ func TestDeleteVirtualKey_CleansUpMultiBudgetScopedModelConfigs(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: "vk-multibudget",
Name: "MultiBudget VK",
Value: "vk-multibudget-value",
Value: *schemas.NewSecretVar("vk-multibudget-value"),
IsActive: schemas.Ptr(true),
}
require.NoError(t, store.CreateVirtualKey(ctx, vk))
Expand Down Expand Up @@ -1236,7 +1236,7 @@ func TestCreateVirtualKeyProviderConfig(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: "vk-for-pc",
Name: "VK For Provider Config",
Value: "vk-pc-value",
Value: *schemas.NewSecretVar("vk-pc-value"),
IsActive: schemas.Ptr(true),
}
err := store.CreateVirtualKey(ctx, vk)
Expand Down Expand Up @@ -1279,7 +1279,7 @@ func TestCreateVirtualKeyProviderConfig_WithKeys(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: "vk-with-keys",
Name: "VK With Keys",
Value: "vk-keys-value",
Value: *schemas.NewSecretVar("vk-keys-value"),
IsActive: schemas.Ptr(true),
}
err = store.CreateVirtualKey(ctx, vk)
Expand Down Expand Up @@ -1319,7 +1319,7 @@ func TestCreateVirtualKeyProviderConfig_UnresolvedKeys(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: "vk-unresolved",
Name: "VK Unresolved",
Value: "vk-unresolved-value",
Value: *schemas.NewSecretVar("vk-unresolved-value"),
IsActive: schemas.Ptr(true),
}
err := store.CreateVirtualKey(ctx, vk)
Expand Down Expand Up @@ -1361,7 +1361,7 @@ func TestUpdateProvider_RemovesStaleVirtualKeyProviderConfigKeyAssociations(t *t
vk := &tables.TableVirtualKey{
ID: "vk-update-provider-cleanup",
Name: "VK Update Provider Cleanup",
Value: "vk-update-provider-cleanup-value",
Value: *schemas.NewSecretVar("vk-update-provider-cleanup-value"),
IsActive: schemas.Ptr(true),
}
err = store.CreateVirtualKey(ctx, vk)
Expand Down Expand Up @@ -1410,7 +1410,7 @@ func TestDeleteProvider_RemovesVirtualKeyProviderConfigs(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: "vk-delete-provider-cleanup",
Name: "VK Delete Provider Cleanup",
Value: "vk-delete-provider-cleanup-value",
Value: *schemas.NewSecretVar("vk-delete-provider-cleanup-value"),
IsActive: schemas.Ptr(true),
}
err = store.CreateVirtualKey(ctx, vk)
Expand Down Expand Up @@ -1806,7 +1806,7 @@ func TestFullVirtualKeyFlow(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: integrationVKID,
Name: "Integration Virtual Key",
Value: "vk-integration-xyz",
Value: *schemas.NewSecretVar("vk-integration-xyz"),
IsActive: schemas.Ptr(true),
RateLimitID: &rateLimitID,
}
Expand Down Expand Up @@ -1857,7 +1857,7 @@ func TestGetVirtualKeysUsesInternalPagination(t *testing.T) {
vk := &tables.TableVirtualKey{
ID: fmt.Sprintf("vk-page-%04d", i),
Name: fmt.Sprintf("Virtual Key %04d", i),
Value: fmt.Sprintf("vk-value-%04d", i),
Value: *schemas.NewSecretVar(fmt.Sprintf("vk-value-%04d", i)),
IsActive: schemas.Ptr(true),
CreatedAt: createdAt,
UpdatedAt: createdAt,
Expand Down
Loading
Loading