From b96ea558ee822632cf540ad395597e7a0d4b47be Mon Sep 17 00:00:00 2001 From: eyeveil <64274427+eyeveil@users.noreply.github.com> Date: Thu, 9 Jul 2026 20:52:29 +0200 Subject: [PATCH] [fix]: configstore - repair bare wildcard allowed_models rows that break admin PUTs Legacy v1.5.10 rows persisted allowed_models as the bare string * in a serializer:json column; loading them aborts with invalid character '*' and poisons every subsequent PUT /api/providers for that provider. Adds a data-repair migration rewriting bare * to canonical ["*"] for both allowed_models and blacklisted_models (current write path already round-trips correctly; only legacy rows are affected). Affected packages: - framework/configstore/migrations.go Fixes #4318 --- framework/configstore/migrations.go | 46 +++++++++++++ framework/configstore/migrations_test.go | 88 ++++++++++++++++++++++++ 2 files changed, 134 insertions(+) diff --git a/framework/configstore/migrations.go b/framework/configstore/migrations.go index 151132928ca..642fdda6672 100644 --- a/framework/configstore/migrations.go +++ b/framework/configstore/migrations.go @@ -441,6 +441,7 @@ var configstoreMigrationSteps = []migrationStep{ {IDs: []string{"add_sidekiq_kind_status_created_index"}, run: migrationAddSidekiqKindStatusCreatedIndex}, {IDs: []string{"add_fast_mode_cache_pricing_columns"}, run: migrationAddFastModeCachePricingColumns}, {IDs: []string{"add_inference_geo_multiplier_column"}, run: migrationAddInferenceGeoMultiplierColumn}, + {IDs: []string{"repair_bare_wildcard_allowed_models"}, run: migrationRepairBareWildcardAllowedModels}, } // quoteSQLiteIdentifier quotes a SQLite identifier, escaping any double quotes. @@ -6442,6 +6443,51 @@ func migrationBackfillAllowedModelsWildcard(ctx context.Context, db *gorm.DB, lo return nil } +// migrationRepairBareWildcardAllowedModels repairs governance_virtual_key_provider_configs +// rows whose allowed_models / blacklisted_models column holds the bare one-character +// string '*' instead of the JSON array '["*"]'. Such rows abort the GORM json +// deserializer ("invalid character '*' ...") when the VK is loaded, which poisons the +// whole provider admin surface (see issue #4318). The repair rewrites the column to the +// canonical '["*"]' form the serializer:json tag is supposed to produce; the intended +// value at write time was already the WhiteList ["*"], so the config_hash — computed +// from the in-memory slice — stays consistent and needs no recomputation. +func migrationRepairBareWildcardAllowedModels(ctx context.Context, db *gorm.DB, logger schemas.Logger) error { + migrationName := "repair_bare_wildcard_allowed_models" + logger.Info("[configstore] starting migration %s", migrationName) + defer logger.Info("[configstore] finished migration %s", migrationName) + m := migrator.New(db, migrator.DefaultOptions, []*migrator.Migration{{ + ID: migrationName, + Migrate: func(tx *gorm.DB) error { + tx = tx.WithContext(ctx) + + // Match the documented manual workaround exactly: bare '*' → '["*"]'. + // Covers both whitelist columns on the provider config, which share the + // serializer:json tag and the same corruption class. + for _, column := range []string{"allowed_models", "blacklisted_models"} { + res := tx.Model(&tables.TableVirtualKeyProviderConfig{}). + Where(column+" = ?", "*"). + Update(column, `["*"]`) + if res.Error != nil { + return fmt.Errorf("failed to repair bare wildcard %s: %w", column, res.Error) + } + if res.RowsAffected > 0 { + logger.Info("[configstore] %s: repaired %d rows with bare wildcard %s", migrationName, res.RowsAffected, column) + } + } + return nil + }, + Rollback: func(tx *gorm.DB) error { + // Rollback is a no-op: reverting '["*"]' back to '*' would re-introduce + // the value that breaks the deserializer. + return nil + }, + }}) + if err := m.Migrate(); err != nil { + return fmt.Errorf("error running %s migration: %s", migrationName, err.Error()) + } + return nil +} + // migrationAddMCPClientAllowedExtraHeadersJSONColumn adds the allowed_extra_headers_json column to the mcp_client table func migrationAddMCPClientAllowedExtraHeadersJSONColumn(ctx context.Context, db *gorm.DB, logger schemas.Logger) error { migrationName := "add_mcp_client_allowed_extra_headers_json_column" diff --git a/framework/configstore/migrations_test.go b/framework/configstore/migrations_test.go index 71da431a22e..60f23ad9ab9 100644 --- a/framework/configstore/migrations_test.go +++ b/framework/configstore/migrations_test.go @@ -1904,6 +1904,94 @@ func TestMigrationBackfillAllowedModelsWildcard(t *testing.T) { assert.NotEmpty(t, keyHash, "key config_hash should be recomputed") } +// TestProviderConfigWildcardRoundTrip verifies the current write path persists a +// WhiteList/BlackList wildcard as the JSON array '["*"]' (never the bare byte '*') +// and reads it back intact — the round-trip that issue #4318's corrupted rows broke. +func TestProviderConfigWildcardRoundTrip(t *testing.T) { + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{ + Logger: logger.Default.LogMode(logger.Silent), + }) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate(&tables.TableVirtualKeyProviderConfig{})) + + pc := tables.TableVirtualKeyProviderConfig{ + VirtualKeyID: "vk-roundtrip", + Provider: "zai", + AllowedModels: schemas.WhiteList{"*"}, + BlacklistedModels: schemas.BlackList{"*"}, + } + require.NoError(t, db.Create(&pc).Error) + + // Raw column bytes must be the JSON array, not the bare character. + var allowedRaw, blacklistedRaw string + require.NoError(t, db.Table("governance_virtual_key_provider_configs"). + Select("allowed_models").Where("virtual_key_id = ?", "vk-roundtrip").Scan(&allowedRaw).Error) + require.NoError(t, db.Table("governance_virtual_key_provider_configs"). + Select("blacklisted_models").Where("virtual_key_id = ?", "vk-roundtrip").Scan(&blacklistedRaw).Error) + assert.Equal(t, `["*"]`, allowedRaw) + assert.Equal(t, `["*"]`, blacklistedRaw) + + // Model read must deserialize cleanly back to the wildcard slice. + var got tables.TableVirtualKeyProviderConfig + require.NoError(t, db.Where("virtual_key_id = ?", "vk-roundtrip").First(&got).Error) + assert.True(t, got.AllowedModels.IsUnrestricted()) + assert.True(t, got.BlacklistedModels.IsBlockAll()) +} + +// TestMigrationRepairBareWildcardAllowedModels verifies the repair migration heals +// legacy rows whose allowed_models / blacklisted_models column holds the bare byte +// '*' (issue #4318). Without the repair, loading such a row aborts the GORM json +// deserializer and poisons the provider admin surface. +func TestMigrationRepairBareWildcardAllowedModels(t *testing.T) { + _, db := setupFullMigrationDB(t) + ctx := context.Background() + now := time.Now() + + // Clear migration tracking so it runs again against the seeded rows. + db.Exec(`DELETE FROM migrations WHERE id = 'repair_bare_wildcard_allowed_models'`) + + err := db.Exec(`INSERT INTO governance_virtual_keys (id, name, value, is_active, encryption_status, created_at, updated_at) + VALUES ('vk-bare-1', 'bare-vk', 'vk-val', true, 'plain_text', ?, ?)`, now, now).Error + require.NoError(t, err) + + // Row A: corrupted allowed_models, valid blacklisted_models. + err = db.Exec(`INSERT INTO governance_virtual_key_provider_configs (virtual_key_id, provider, allowed_models, blacklisted_models, allow_all_keys) + VALUES ('vk-bare-1', 'zai', '*', '[]', true)`).Error + require.NoError(t, err) + // Row B: valid allowed_models, corrupted blacklisted_models. + err = db.Exec(`INSERT INTO governance_virtual_key_provider_configs (virtual_key_id, provider, allowed_models, blacklisted_models, allow_all_keys) + VALUES ('vk-bare-1', 'openai', '["*"]', '*', true)`).Error + require.NoError(t, err) + + // Pre-condition: the bare '*' rows cannot be loaded through the GORM model. + var pre []tables.TableVirtualKeyProviderConfig + preErr := db.Where("virtual_key_id = ?", "vk-bare-1").Find(&pre).Error + require.Error(t, preErr, "bare '*' rows should fail to deserialize before repair") + + require.NoError(t, migrationRepairBareWildcardAllowedModels(ctx, db, testMigrationLogger)) + + // Both columns are now canonical JSON arrays. + var allowedA, blacklistedB string + require.NoError(t, db.Table("governance_virtual_key_provider_configs"). + Select("allowed_models").Where("virtual_key_id = ? AND provider = ?", "vk-bare-1", "zai").Scan(&allowedA).Error) + require.NoError(t, db.Table("governance_virtual_key_provider_configs"). + Select("blacklisted_models").Where("virtual_key_id = ? AND provider = ?", "vk-bare-1", "openai").Scan(&blacklistedB).Error) + assert.Equal(t, `["*"]`, allowedA, "bare '*' allowed_models should be repaired to wildcard array") + assert.Equal(t, `["*"]`, blacklistedB, "bare '*' blacklisted_models should be repaired to wildcard array") + + // Post-condition: the rows now load cleanly through the GORM model. + var post []tables.TableVirtualKeyProviderConfig + require.NoError(t, db.Where("virtual_key_id = ?", "vk-bare-1").Find(&post).Error, + "repaired rows should deserialize without error") + require.Len(t, post, 2) + byProvider := map[string]tables.TableVirtualKeyProviderConfig{} + for _, pc := range post { + byProvider[pc.Provider] = pc + } + assert.True(t, byProvider["zai"].AllowedModels.IsUnrestricted(), "repaired allowed_models should be unrestricted") + assert.True(t, byProvider["openai"].BlacklistedModels.IsBlockAll(), "repaired blacklisted_models should block all") +} + func TestMigrationRemoveServerPrefixFromMCPTools(t *testing.T) { _, db := setupFullMigrationDB(t) ctx := context.Background()