From e3984e59486c82cf69d85efd41ea60ec6acfe39f Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Fri, 23 Oct 2026 19:14:43 +0000 Subject: [PATCH] Store each deployment model default field once --- .../db/queries/deployment_model_providers.sql | 8 ++-- .../db/sqlc/deployment_model_providers.sql.go | 29 +++++++++---- services/core/internal/db/sqlc/models.go | 2 +- .../modelconfiguration/configuration.go | 10 ++--- .../internal/modelconfiguration/service.go | 4 +- .../modelconfiguration/service_test.go | 7 ++-- .../internal/modelconfiguration/storage.go | 6 +-- .../modelconfigurationpg/seal_test.go | 41 ++++++------------- .../postgres/modelconfigurationpg/store.go | 23 ++++------- ...005_deployment_model_provider_key_only.sql | 4 ++ .../deployment_model_providers_http_test.go | 9 ++-- 11 files changed, 69 insertions(+), 74 deletions(-) create mode 100644 services/core/migrations/000005_deployment_model_provider_key_only.sql diff --git a/services/core/internal/db/queries/deployment_model_providers.sql b/services/core/internal/db/queries/deployment_model_providers.sql index de180cd75..149b80a77 100644 --- a/services/core/internal/db/queries/deployment_model_providers.sql +++ b/services/core/internal/db/queries/deployment_model_providers.sql @@ -3,14 +3,14 @@ SELECT harness, protocol, base_url, context_window, max_output_tokens, model, ha FROM deployment_model_providers ORDER BY harness; -- name: GetDeploymentModelProviderSecret :one -SELECT encrypted_config, revision FROM deployment_model_providers WHERE harness = @harness; +SELECT protocol, base_url, context_window, max_output_tokens, model, harness_config, encrypted_api_key, revision FROM deployment_model_providers WHERE harness = @harness; -- name: UpsertDeploymentModelProvider :one -INSERT INTO deployment_model_providers (harness, protocol, base_url, context_window, max_output_tokens, model, harness_config, encrypted_config, revision, updated_at) -VALUES (@harness, @protocol, @base_url, @context_window, @max_output_tokens, @model, @harness_config, @encrypted_config, @revision, clock_timestamp()) +INSERT INTO deployment_model_providers (harness, protocol, base_url, context_window, max_output_tokens, model, harness_config, encrypted_api_key, revision, updated_at) +VALUES (@harness, @protocol, @base_url, @context_window, @max_output_tokens, @model, @harness_config, @encrypted_api_key, @revision, clock_timestamp()) ON CONFLICT (harness) DO UPDATE SET protocol = EXCLUDED.protocol, base_url = EXCLUDED.base_url, context_window = EXCLUDED.context_window, max_output_tokens = EXCLUDED.max_output_tokens, - model = EXCLUDED.model, harness_config = EXCLUDED.harness_config, encrypted_config = EXCLUDED.encrypted_config, revision = EXCLUDED.revision, updated_at = EXCLUDED.updated_at, + model = EXCLUDED.model, harness_config = EXCLUDED.harness_config, encrypted_api_key = EXCLUDED.encrypted_api_key, revision = EXCLUDED.revision, updated_at = EXCLUDED.updated_at, last_used_at = NULL, last_error_code = NULL, last_error_at = NULL, recovery_pending = false RETURNING harness, protocol, base_url, context_window, max_output_tokens, model, harness_config, updated_at, last_used_at, last_error_code, last_error_at; diff --git a/services/core/internal/db/sqlc/deployment_model_providers.sql.go b/services/core/internal/db/sqlc/deployment_model_providers.sql.go index c8d4d52fa..b6b6dcfb0 100644 --- a/services/core/internal/db/sqlc/deployment_model_providers.sql.go +++ b/services/core/internal/db/sqlc/deployment_model_providers.sql.go @@ -24,18 +24,33 @@ func (q *Queries) DeleteDeploymentModelProvider(ctx context.Context, harness str } const getDeploymentModelProviderSecret = `-- name: GetDeploymentModelProviderSecret :one -SELECT encrypted_config, revision FROM deployment_model_providers WHERE harness = $1 +SELECT protocol, base_url, context_window, max_output_tokens, model, harness_config, encrypted_api_key, revision FROM deployment_model_providers WHERE harness = $1 ` type GetDeploymentModelProviderSecretRow struct { - EncryptedConfig []byte `json:"encrypted_config"` + Protocol string `json:"protocol"` + BaseUrl string `json:"base_url"` + ContextWindow int32 `json:"context_window"` + MaxOutputTokens int32 `json:"max_output_tokens"` + Model string `json:"model"` + HarnessConfig []byte `json:"harness_config"` + EncryptedApiKey []byte `json:"encrypted_api_key"` Revision pgtype.UUID `json:"revision"` } func (q *Queries) GetDeploymentModelProviderSecret(ctx context.Context, harness string) (GetDeploymentModelProviderSecretRow, error) { row := q.db.QueryRow(ctx, getDeploymentModelProviderSecret, harness) var i GetDeploymentModelProviderSecretRow - err := row.Scan(&i.EncryptedConfig, &i.Revision) + err := row.Scan( + &i.Protocol, + &i.BaseUrl, + &i.ContextWindow, + &i.MaxOutputTokens, + &i.Model, + &i.HarnessConfig, + &i.EncryptedApiKey, + &i.Revision, + ) return i, err } @@ -91,11 +106,11 @@ func (q *Queries) ListDeploymentModelProviders(ctx context.Context) ([]ListDeplo } const upsertDeploymentModelProvider = `-- name: UpsertDeploymentModelProvider :one -INSERT INTO deployment_model_providers (harness, protocol, base_url, context_window, max_output_tokens, model, harness_config, encrypted_config, revision, updated_at) +INSERT INTO deployment_model_providers (harness, protocol, base_url, context_window, max_output_tokens, model, harness_config, encrypted_api_key, revision, updated_at) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, clock_timestamp()) ON CONFLICT (harness) DO UPDATE SET protocol = EXCLUDED.protocol, base_url = EXCLUDED.base_url, context_window = EXCLUDED.context_window, max_output_tokens = EXCLUDED.max_output_tokens, - model = EXCLUDED.model, harness_config = EXCLUDED.harness_config, encrypted_config = EXCLUDED.encrypted_config, revision = EXCLUDED.revision, updated_at = EXCLUDED.updated_at, + model = EXCLUDED.model, harness_config = EXCLUDED.harness_config, encrypted_api_key = EXCLUDED.encrypted_api_key, revision = EXCLUDED.revision, updated_at = EXCLUDED.updated_at, last_used_at = NULL, last_error_code = NULL, last_error_at = NULL, recovery_pending = false RETURNING harness, protocol, base_url, context_window, max_output_tokens, model, harness_config, updated_at, last_used_at, last_error_code, last_error_at ` @@ -108,7 +123,7 @@ type UpsertDeploymentModelProviderParams struct { MaxOutputTokens int32 `json:"max_output_tokens"` Model string `json:"model"` HarnessConfig []byte `json:"harness_config"` - EncryptedConfig []byte `json:"encrypted_config"` + EncryptedApiKey []byte `json:"encrypted_api_key"` Revision pgtype.UUID `json:"revision"` } @@ -135,7 +150,7 @@ func (q *Queries) UpsertDeploymentModelProvider(ctx context.Context, arg UpsertD arg.MaxOutputTokens, arg.Model, arg.HarnessConfig, - arg.EncryptedConfig, + arg.EncryptedApiKey, arg.Revision, ) var i UpsertDeploymentModelProviderRow diff --git a/services/core/internal/db/sqlc/models.go b/services/core/internal/db/sqlc/models.go index 74ac8e5fa..035c6b639 100644 --- a/services/core/internal/db/sqlc/models.go +++ b/services/core/internal/db/sqlc/models.go @@ -42,7 +42,7 @@ type DeploymentModelProvider struct { BaseUrl string `json:"base_url"` ContextWindow int32 `json:"context_window"` MaxOutputTokens int32 `json:"max_output_tokens"` - EncryptedConfig []byte `json:"encrypted_config"` + EncryptedApiKey []byte `json:"encrypted_api_key"` UpdatedAt pgtype.Timestamptz `json:"updated_at"` Revision pgtype.UUID `json:"revision"` LastUsedAt pgtype.Timestamptz `json:"last_used_at"` diff --git a/services/core/internal/modelconfiguration/configuration.go b/services/core/internal/modelconfiguration/configuration.go index f8e03f044..258982fc6 100644 --- a/services/core/internal/modelconfiguration/configuration.go +++ b/services/core/internal/modelconfiguration/configuration.go @@ -39,18 +39,18 @@ type Snapshot struct { Revision uuid.UUID } -// Record is a validated deployment default ready to store: its safe columns -// and the complete bundle, provider key included, which storage seals to the -// Harness. Storage assigns a new revision to each Record it stores. +// Record is a validated deployment default ready to store: its columns and the +// provider key, which storage seals to the Harness. Storage assigns a new +// revision to each Record it stores. type Record struct { Harness string Provider v1.ModelProviderView Model string HarnessConfig json.RawMessage - Configuration v1.ModelConfigurationInput + APIKey string } -// Bundle is an opened stored bundle and the revision read with it. +// Bundle is a stored default with its key opened and the revision read with it. type Bundle struct { Configuration v1.ModelConfigurationInput Revision uuid.UUID diff --git a/services/core/internal/modelconfiguration/service.go b/services/core/internal/modelconfiguration/service.go index 7b52a5148..9aa3e130d 100644 --- a/services/core/internal/modelconfiguration/service.go +++ b/services/core/internal/modelconfiguration/service.go @@ -8,7 +8,7 @@ import ( ) // Service replaces, removes and resolves deployment defaults. Storage seals -// the complete bundle, key included, to its Harness. +// the provider key to its Harness; every other field is stored plainly. type Service struct { storage Storage } @@ -30,7 +30,7 @@ func (s *Service) Replace(ctx context.Context, replacement Replacement) (Configu return Configuration{}, err } view := configuration.SafeView() - return s.storage.Replace(ctx, Record{Harness: replacement.Harness, Provider: *view.ModelProvider, Model: view.Model, HarnessConfig: view.HarnessConfig, Configuration: configuration}) + return s.storage.Replace(ctx, Record{Harness: replacement.Harness, Provider: *view.ModelProvider, Model: view.Model, HarnessConfig: view.HarnessConfig, APIKey: configuration.ModelProvider.APIKey}) } // Delete removes the Harness's default. It is idempotent. Sessions that diff --git a/services/core/internal/modelconfiguration/service_test.go b/services/core/internal/modelconfiguration/service_test.go index 909df0ed6..ded0f5fac 100644 --- a/services/core/internal/modelconfiguration/service_test.go +++ b/services/core/internal/modelconfiguration/service_test.go @@ -3,7 +3,6 @@ package modelconfiguration import ( "context" "errors" - "reflect" "testing" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" @@ -58,7 +57,7 @@ func TestNewServiceRequiresStorage(t *testing.T) { } } -func TestReplacePassesTheCompleteBundleWithItsSafeColumns(t *testing.T) { +func TestReplacePassesTheKeyWithItsColumns(t *testing.T) { stored := Configuration{Harness: "codex", Model: "fixture-model"} var record Record storage := &fakeStorage{t: t, replace: func(_ context.Context, r Record) (Configuration, error) { @@ -73,8 +72,8 @@ func TestReplacePassesTheCompleteBundleWithItsSafeColumns(t *testing.T) { record.Provider != (v1.ModelProviderView{Protocol: "responses", BaseURL: "https://model.example/v1", APIKeyConfigured: true}) { t.Fatalf("safe columns: %+v", record) } - if !reflect.DeepEqual(record.Configuration, validConfiguration()) { - t.Fatalf("bundle: %+v", record.Configuration) + if record.APIKey != validConfiguration().ModelProvider.APIKey { + t.Fatalf("key: %q", record.APIKey) } } diff --git a/services/core/internal/modelconfiguration/storage.go b/services/core/internal/modelconfiguration/storage.go index 792b332cf..4f6b87714 100644 --- a/services/core/internal/modelconfiguration/storage.go +++ b/services/core/internal/modelconfiguration/storage.go @@ -3,17 +3,17 @@ package modelconfiguration import "context" // Storage keeps one deployment default per Harness and seals and opens its -// bundle. Replace and Delete record the administrator +// provider key. Replace and Delete record the administrator // mutation in the same transaction, from the provenance the context carries; // an audit failure aborts the change. type Storage interface { - // Replace seals and stores record under a new revision and clears the + // Replace seals the key and stores record under a new revision and clears the // observations of the revision it replaces. Replace(ctx context.Context, record Record) (Configuration, error) // Delete removes the Harness's default. Removing a missing default succeeds // and is audited too. Delete(ctx context.Context, harness string) error - // LoadBundle opens the Harness's bundle and returns it with its revision. + // LoadBundle opens the Harness's key and returns it with its revision. // A Harness without a default is ErrNotFound, checked before the key. LoadBundle(ctx context.Context, harness string) (Bundle, error) } diff --git a/services/core/internal/persistence/postgres/modelconfigurationpg/seal_test.go b/services/core/internal/persistence/postgres/modelconfigurationpg/seal_test.go index 181e8a5f8..6af12fa8d 100644 --- a/services/core/internal/persistence/postgres/modelconfigurationpg/seal_test.go +++ b/services/core/internal/persistence/postgres/modelconfigurationpg/seal_test.go @@ -1,6 +1,7 @@ package modelconfigurationpg_test import ( + "bytes" "encoding/json" "errors" "reflect" @@ -14,51 +15,35 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" ) -// The Store seals the bundle to its Harness as Core always has, so a bundle -// sealed before the Store owned the key still opens. A bundle sealed under +// The Store seals only the provider key to its Harness. A key sealed under // another key or binding is a decryption failure, never a missing default. -func TestBundlesKeepTheirSealedFormat(t *testing.T) { +func TestOnlyTheKeyIsSealed(t *testing.T) { f := newFixture(t) replaced := modelconfigurationpg.New(pgunit.NewPool(f.pool), pgtest.CredentialKey(t)) configuration := v1.ModelConfigurationInput{ModelProvider: fixtureProvider, Model: "fixture"} if _, err := replaced.LoadBundle(t.Context(), "codex"); !errors.Is(err, modelconfiguration.ErrNotFound) { t.Fatal("a missing default was not reported before decryption", err) } - record := modelconfiguration.Record{Harness: "codex", Provider: *fixtureProvider.SafeView(), Model: "fixture", HarnessConfig: json.RawMessage(`{}`), Configuration: configuration} + record := modelconfiguration.Record{Harness: "codex", Provider: *fixtureProvider.SafeView(), Model: "fixture", HarnessConfig: json.RawMessage(`{}`), APIKey: fixtureProvider.APIKey} if _, err := f.adapter.Replace(admin(t), record); err != nil { t.Fatal(err) } loaded, err := f.adapter.LoadBundle(t.Context(), "codex") + configuration.HarnessConfig = json.RawMessage(`{}`) if err != nil || !reflect.DeepEqual(loaded.Configuration, configuration) { - t.Fatal("the bundle did not round-trip", err) + t.Fatal("the default did not round-trip", loaded.Configuration, err) + } + var sealed []byte + if err := f.pool.QueryRow(t.Context(), "SELECT encrypted_api_key FROM deployment_model_providers WHERE harness='codex'").Scan(&sealed); err != nil || bytes.Contains(sealed, []byte(fixtureProvider.BaseURL)) { + t.Fatal("the sealed value holds more than the key", err) } if _, err := replaced.LoadBundle(t.Context(), "codex"); err == nil || err.Error() != "deployment model configuration decryption failed" { t.Fatal("a replaced key opened a bundle", err) } - // write stores a bundle sealed the way Core sealed it before this Store - // owned the key. - write := func(harness string, bundle v1.ModelConfigurationInput) { - t.Helper() - raw, err := json.Marshal(bundle) - if err != nil { - t.Fatal(err) - } - sealed, err := f.cipher.SealDeploymentModelProvider(raw, harness) - if err != nil { - t.Fatal(err) - } - if _, err := f.pool.Exec(t.Context(), "UPDATE deployment_model_providers SET encrypted_config=$1 WHERE harness='codex'", sealed); err != nil { - t.Fatal(err) - } - } - old := configuration - old.ModelProvider.APIKey = "sealed-before-the-move" - write("codex", old) - if loaded, err := f.adapter.LoadBundle(t.Context(), "codex"); err != nil || !reflect.DeepEqual(loaded.Configuration, old) { - t.Fatal("a bundle sealed before the move did not open", err) + if _, err := f.pool.Exec(t.Context(), "UPDATE deployment_model_providers SET harness='claude_code' WHERE harness='codex'"); err != nil { + t.Fatal(err) } - write("claude_code", old) - if _, err := f.adapter.LoadBundle(t.Context(), "codex"); err == nil || err.Error() != "deployment model configuration decryption failed" { + if _, err := f.adapter.LoadBundle(t.Context(), "claude_code"); err == nil || err.Error() != "deployment model configuration decryption failed" { t.Fatal("a wrong binding was not a decryption failure", err) } } diff --git a/services/core/internal/persistence/postgres/modelconfigurationpg/store.go b/services/core/internal/persistence/postgres/modelconfigurationpg/store.go index 0486d76ef..d7d08c3b6 100644 --- a/services/core/internal/persistence/postgres/modelconfigurationpg/store.go +++ b/services/core/internal/persistence/postgres/modelconfigurationpg/store.go @@ -60,15 +60,11 @@ func (s *Store) List(ctx context.Context) ([]modelconfiguration.Configuration, e return result, nil } -// Replace seals the complete bundle to its Harness, upserts the record under a +// Replace seals the provider key to its Harness, upserts the record under a // new revision, which clears the replaced revision's observations, and audits // the write in the same transaction. func (s *Store) Replace(ctx context.Context, record modelconfiguration.Record) (modelconfiguration.Configuration, error) { - raw, err := json.Marshal(record.Configuration) - if err != nil { - return modelconfiguration.Configuration{}, err - } - sealed, err := s.cipher.SealDeploymentModelProvider(raw, record.Harness) + sealed, err := s.cipher.SealDeploymentModelProvider([]byte(record.APIKey), record.Harness) if err != nil { return modelconfiguration.Configuration{}, errors.New("model provider encryption failed") } @@ -78,7 +74,7 @@ func (s *Store) Replace(ctx context.Context, record modelconfiguration.Record) ( row, err := q.UpsertDeploymentModelProvider(ctx, sqlc.UpsertDeploymentModelProviderParams{ Harness: record.Harness, Protocol: string(record.Provider.Protocol), BaseUrl: record.Provider.BaseURL, ContextWindow: record.Provider.ContextWindow, MaxOutputTokens: record.Provider.MaxOutputTokens, - Model: record.Model, HarnessConfig: record.HarnessConfig, EncryptedConfig: sealed, + Model: record.Model, HarnessConfig: record.HarnessConfig, EncryptedApiKey: sealed, Revision: pgtype.UUID{Bytes: uuid.New(), Valid: true}, }) if err != nil { @@ -105,8 +101,8 @@ func (s *Store) Delete(ctx context.Context, harness string) error { })) } -// LoadBundle reads the sealed bundle and its revision in one statement, so the -// pair always belongs to the same replacement, and opens the bundle. +// LoadBundle reads the columns, the sealed key and the revision in one +// statement, so they always belong to the same replacement, and opens the key. func (s *Store) LoadBundle(ctx context.Context, harness string) (modelconfiguration.Bundle, error) { row, err := s.pool.Queries().GetDeploymentModelProviderSecret(ctx, harness) if errors.Is(err, pgx.ErrNoRows) { @@ -115,15 +111,12 @@ func (s *Store) LoadBundle(ctx context.Context, harness string) (modelconfigurat if err != nil { return modelconfiguration.Bundle{}, translate(err) } - raw, err := s.cipher.OpenDeploymentModelProvider(row.EncryptedConfig, harness) + key, err := s.cipher.OpenDeploymentModelProvider(row.EncryptedApiKey, harness) if err != nil { return modelconfiguration.Bundle{}, errors.New("deployment model configuration decryption failed") } - var configuration v1.ModelConfigurationInput - if json.Unmarshal(raw, &configuration) != nil { - return modelconfiguration.Bundle{}, errors.New("invalid stored deployment model configuration") - } - return modelconfiguration.Bundle{Configuration: configuration, Revision: uuid.UUID(row.Revision.Bytes)}, nil + provider := v1.ModelProviderInput{Protocol: modelprovider.Protocol(row.Protocol), BaseURL: row.BaseUrl, APIKey: string(key), ContextWindow: row.ContextWindow, MaxOutputTokens: row.MaxOutputTokens} + return modelconfiguration.Bundle{Configuration: v1.ModelConfigurationInput{ModelProvider: provider, Model: row.Model, HarnessConfig: json.RawMessage(row.HarnessConfig)}, Revision: uuid.UUID(row.Revision.Bytes)}, nil } // ObserveDeploymentModelProvider runs one metadata UPDATE in its own pooled diff --git a/services/core/migrations/000005_deployment_model_provider_key_only.sql b/services/core/migrations/000005_deployment_model_provider_key_only.sql new file mode 100644 index 000000000..b7f7413ba --- /dev/null +++ b/services/core/migrations/000005_deployment_model_provider_key_only.sql @@ -0,0 +1,4 @@ +-- +goose Up +-- The sealed value was the whole configuration and is now the provider key alone. Defaults stored before this change are removed and set again. +DELETE FROM deployment_model_providers; +ALTER TABLE deployment_model_providers RENAME COLUMN encrypted_config TO encrypted_api_key; diff --git a/services/core/tests/integration/deployment_model_providers_http_test.go b/services/core/tests/integration/deployment_model_providers_http_test.go index 3318c0ad6..0bbcadf10 100644 --- a/services/core/tests/integration/deployment_model_providers_http_test.go +++ b/services/core/tests/integration/deployment_model_providers_http_test.go @@ -155,12 +155,11 @@ func TestDeploymentModelProvidersHTTP(t *testing.T) { // Simulate a default persisted when cross-protocol execution was supported. // It must remain readable, but cannot create new Sessions or be rewritten. - historical := strings.Replace(codexDefault, `"protocol":"responses"`, `"protocol":"anthropic"`, 1) - encrypted, err := st.credentialCipher.SealDeploymentModelProvider([]byte(historical), "codex") - if err != nil { + var encrypted []byte + if err := pool.QueryRow(t.Context(), "SELECT encrypted_api_key FROM deployment_model_providers WHERE harness='codex'").Scan(&encrypted); err != nil { t.Fatal(err) } - if _, err := pool.Exec(t.Context(), "UPDATE deployment_model_providers SET protocol='anthropic', encrypted_config=$1 WHERE harness='codex'", encrypted); err != nil { + if _, err := pool.Exec(t.Context(), "UPDATE deployment_model_providers SET protocol='anthropic' WHERE harness='codex'"); err != nil { t.Fatal(err) } incompatible := call("POST", "/v1/agents/sessions", projectKey, hosted, 400) @@ -171,7 +170,7 @@ func TestDeploymentModelProvidersHTTP(t *testing.T) { t.Fatal("unsupported default was rewritten") } var retained []byte - if err := pool.QueryRow(t.Context(), "SELECT encrypted_config FROM deployment_model_providers WHERE harness='codex'").Scan(&retained); err != nil || !bytes.Equal(retained, encrypted) { + if err := pool.QueryRow(t.Context(), "SELECT encrypted_api_key FROM deployment_model_providers WHERE harness='codex'").Scan(&retained); err != nil || !bytes.Equal(retained, encrypted) { t.Fatal("rejected default changed its encrypted snapshot", err) } call("PUT", path, coreKey, codexDefault, 200)