From 3165b98cc89400bed4fc5b827c00a2b2942458e9 Mon Sep 17 00:00:00 2001 From: core-devops Date: Wed, 27 May 2026 17:39:26 -0700 Subject: [PATCH 1/2] =?UTF-8?q?fix(workspace-server):=20P2-B=20internal#71?= =?UTF-8?q?8=20=E2=80=94=20billing/credential=20decision=20DERIVES=20the?= =?UTF-8?q?=20provider;=20supersede=20#1966=20stored-read;=20retire=20org?= =?UTF-8?q?=20rung;=20only-registered=20validation=20(BEHAVIOR-AFFECTING)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-points the platform-vs-BYOK billing/credential decision to DERIVE the provider from (runtime, model) via the registry SSOT, per the CTO directive (internal#718 comment, 2026-05-27): "the billing read must DERIVE the provider, not read a stored LLM_PROVIDER", "remove LLM_PROVIDER entirely as a billing source", "retire organizations.llm_billing_mode as a billing source". ## BEHAVIOR DELTA (this PR changes behavior — tested explicitly) - platform-derived (or unset → platform default) → platform_managed → platform creds. UNCHANGED. - non-platform-derived → byok → the already-merged #1963 strips platform scope:global LLM creds + FAIL-CLOSES if the workspace has no own cred. THIS IS THE INTENDED FIX (the Reno billing-leak class: Reno Stars SEO 352e3c2b / Marketing 6b66de8d ran on the platform's Anthropic credits because the never- written org rung always resolved platform_managed). - unset model → platform default (CTO-confirmed). ## What changed - `ResolveLLMBillingModeDerived(ctx, ws, runtime, model, authEnv)` — the new SSOT resolver: explicit `workspaces.llm_billing_mode` override (precedence 1, the only stored billing signal that survives — operator escape hatch) → else DeriveProvider + IsPlatform → else default-closed platform_managed. - `ResolveLLMBillingMode(ctx, ws, orgMode)` legacy signature retained for callers without (runtime, model) (admin route, secrets remote-pull): reads the stored runtime + MODEL + auth-env names from DB and delegates to the derived resolver. `orgMode` is RETIRED/ignored; the org rung is gone. - `applyPlatformManagedLLMEnv` calls the derived resolver directly (it has runtime + model + the workspace env) — no stored LLM_PROVIDER read. Feeds #1963's strip + fail-closed the correct DERIVED signal. - SUPERSEDES core#1966: that PR made the billing read consult a stored LLM_PROVIDER first; this reworks the decision onto derive-from-provider. #1966 should be closed in favor of this. - Removed the now-dead org-default normalization (normalizeOrgDefault). - ONLY-REGISTERED validation at create (model_registry_validation.go + WorkspaceHandler.Create): a (runtime, model) not in the registry's ModelsForRuntime for a REGISTRY-known runtime is flagged (X-Molecule-Model-Unregistered header + warning log). P2 = WARN mode (NOT hard 422) because the legacy colon-namespaced model vocabulary ("anthropic:claude- opus-4-7") is still live across the create/import/template corpus and is not yet reconciled into the registry — hard-reject is a one-line flip gated on P3/P4 vocabulary convergence. Fails OPEN for non-registry runtimes (langgraph/external/kimi/mock/federated) so those flows are unchanged. ## Tests (TDD; behavior delta explicit) - llm_billing_mode_derived_test.go — platform/non-platform/unset/override/ unregistered/auth-env-disambiguation table + DB-error default-closed + empty-id. - workspace_provision_shared_test.go — DERIVED platform→unchanged, non-platform→byok+strip+fail-closed (the FIX), unset→platform default, through the real applyPlatformManagedLLMEnv path. Existing #1963 override-byok strip + fail-closed tests unchanged (still pass). - model_registry_validation_test.go + workspace_test.go — only-registered warn + registered-no-warn + non-registry-fail-open. - Reworked the legacy resolver/admin/secrets tests off the retired org rung. ## Build/CI go build ./... (+ -tags=integration) green; full `go test ./...` (43 pkgs) green incl. -race on handlers; vet clean; changed files gofmt-clean. cp#362 anthropic passthrough untouched (CP repo); merged #1963 strip+fail-closed reused unchanged. internal#718 P2-B. BEHAVIOR-AFFECTING. Supersedes #1966. Not merged. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../handlers/handlers_extended_test.go | 20 +- .../internal/handlers/llm_billing_mode.go | 400 ++++++++++++++---- .../handlers/llm_billing_mode_derived_test.go | 232 ++++++++++ .../handlers/llm_billing_mode_handler.go | 10 +- .../handlers/llm_billing_mode_handler_test.go | 58 ++- .../handlers/llm_billing_mode_test.go | 220 +++++----- .../handlers/model_registry_validation.go | 57 +++ .../model_registry_validation_test.go | 83 ++++ .../internal/handlers/workspace.go | 27 ++ .../internal/handlers/workspace_provision.go | 25 +- .../workspace_provision_shared_test.go | 152 ++++++- .../internal/handlers/workspace_test.go | 126 +++++- 12 files changed, 1178 insertions(+), 232 deletions(-) create mode 100644 workspace-server/internal/handlers/llm_billing_mode_derived_test.go create mode 100644 workspace-server/internal/handlers/model_registry_validation.go create mode 100644 workspace-server/internal/handlers/model_registry_validation_test.go diff --git a/workspace-server/internal/handlers/handlers_extended_test.go b/workspace-server/internal/handlers/handlers_extended_test.go index fdd86d489..59cd59e6e 100644 --- a/workspace-server/internal/handlers/handlers_extended_test.go +++ b/workspace-server/internal/handlers/handlers_extended_test.go @@ -255,22 +255,20 @@ func TestExtended_SecretsListEmpty(t *testing.T) { // ---------- TestSecretsSet (Extended) ---------- func TestExtended_SecretsSet(t *testing.T) { - // internal#691: the per-workspace strip gate now defaults to platform_managed - // on empty MOLECULE_LLM_BILLING_MODE (closed default). This test's intent is - // the happy path of persisting a vendor key, so put the org into byok which - // matches the pre-#691 implicit behavior of an unset env. - t.Setenv("MOLECULE_LLM_BILLING_MODE", "byok") + // internal#718 P2-B: the per-workspace strip gate keys off the DERIVED mode + // (org rung retired). This test's intent is the happy path of persisting a + // vendor key on a byok workspace; the realistic way a workspace is byok for + // a direct vendor-key write is an explicit operator override (the escape + // hatch the reject error itself points to: PUT /admin/.../llm-billing-mode). + // The override short-circuits the resolver to byok in a single read, so the + // bypass-list check is skipped and the write proceeds. + t.Setenv("MOLECULE_LLM_BILLING_MODE", "platform_managed") // org env ignored now mock := setupTestDB(t) handler := NewSecretsHandler(nil) - // internal#691: secrets.Set now consults ResolveLLMBillingMode before the - // strip gate. Mock returns no row → resolver falls through to the org - // default (byok, set via t.Setenv above) → bypass-list check is skipped - // and the write proceeds. This pattern is the test-side mirror of the - // real-prod fall-through behavior for a fresh workspace with no override. mock.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). WithArgs("22222222-2222-2222-2222-222222222222"). - WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"})) + WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow(LLMBillingModeBYOK)) // Expect INSERT (encrypted value is dynamic, use AnyArg) mock.ExpectExec("INSERT INTO workspace_secrets"). diff --git a/workspace-server/internal/handlers/llm_billing_mode.go b/workspace-server/internal/handlers/llm_billing_mode.go index 60f7ad744..af8aafadf 100644 --- a/workspace-server/internal/handlers/llm_billing_mode.go +++ b/workspace-server/internal/handlers/llm_billing_mode.go @@ -43,10 +43,36 @@ import ( "database/sql" "errors" "fmt" + "log" + "sync" + "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/crypto" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/db" + "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/providers" ) +// providerManifest is the parsed provider registry, loaded once. The registry +// is embedded (go:embed, no network) and immutable for the process lifetime, so +// a single Load is safe to memoize. A load failure is cached too (registryErr): +// it can only happen on a malformed embedded YAML, which is a build-time defect +// the verify-providers-gen + sync gates already catch, so failing closed +// (treat as "cannot derive" → platform default) is correct and we don't retry. +var ( + providerRegistryOnce sync.Once + providerRegistryManifest *providers.Manifest + providerRegistryErr error +) + +func providerRegistry() (*providers.Manifest, error) { + providerRegistryOnce.Do(func() { + providerRegistryManifest, providerRegistryErr = providers.LoadManifest() + if providerRegistryErr != nil { + log.Printf("llm_billing_mode: FATAL — provider registry failed to load: %v (billing will default-closed to platform_managed)", providerRegistryErr) + } + }) + return providerRegistryManifest, providerRegistryErr +} + // Constants mirror molecule-controlplane/internal/credits/llm_billing.go. // Kept as string literals (not imports) because workspace-server has no // build-time dependency on the CP module; the values are stable wire @@ -67,6 +93,19 @@ const ( BillingModeSourceWorkspaceOverride BillingModeSource = "workspace_override" BillingModeSourceOrgDefault BillingModeSource = "org_default" BillingModeSourceConstantFallback BillingModeSource = "constant_fallback" + // BillingModeSourceDerivedProvider means the mode was DERIVED from the + // workspace's (runtime, model) via the provider registry — the SSOT + // (internal#718 P2-B). IsPlatform(derived) → platform_managed, else byok. + // This is the highest-precedence source after an explicit operator override + // and SUPERSEDES the prior stored-LLM_PROVIDER read (#1966). + BillingModeSourceDerivedProvider BillingModeSource = "derived_provider" + // BillingModeSourceDerivedDefault means the registry could not derive a + // provider for the (runtime, model) — no model, unknown runtime, + // unregistered/ambiguous model — so the mode defaulted closed to + // platform_managed (CTO-confirmed "unset → platform default"). Distinct from + // derived_provider so operators can see "we defaulted" vs "we derived + // platform". + BillingModeSourceDerivedDefault BillingModeSource = "derived_default" ) // BillingModeResolution is the structured answer the admin GET route returns @@ -74,11 +113,18 @@ const ( // shape, so the resolver test asserts both the mode AND the source per case // (catches a bug where the right mode is returned via the wrong layer). type BillingModeResolution struct { - WorkspaceID string `json:"workspace_id"` - ResolvedMode string `json:"resolved_mode"` - WorkspaceOverride *string `json:"workspace_override"` // nil = inherit - OrgDefault string `json:"org_default"` // already default-closed by CP - Source BillingModeSource `json:"source"` + WorkspaceID string `json:"workspace_id"` + ResolvedMode string `json:"resolved_mode"` + WorkspaceOverride *string `json:"workspace_override"` // nil = inherit + OrgDefault string `json:"org_default"` // RETIRED as a billing source (internal#718 P2-B); always platform_managed, kept for wire-compat + Source BillingModeSource `json:"source"` + // ProviderSelection surfaces the DERIVED provider name (internal#718 P2-B) + // when the mode came from the registry derivation — the literal provider the + // (runtime, model) resolved to (e.g. "platform", "kimi-coding", "openai"), or + // the raw model id when derivation failed. nil when an explicit operator + // override or the empty-id default decided. Lets the admin route answer "why + // is this workspace byok?" with the derived provider, not a stored value. + ProviderSelection *string `json:"provider_selection"` } // isKnownBillingMode is the enum-recognizer for the resolver's default-closed @@ -95,24 +141,137 @@ func isKnownBillingMode(s string) bool { } } -// normalizeOrgDefault applies the same default-closed contract to the -// org-level input as the workspace override gets. The org_default arrives -// from tenant_config which already COALESCEs NULL → platform_managed at the -// CP SQL layer, but we DO NOT trust that contract here — if CP regresses or -// the tenant_config env wasn't populated (race on boot), we still default- -// close. Same principle: never honor a garbled value. -func normalizeOrgDefault(orgMode string) string { - if isKnownBillingMode(orgMode) { - return orgMode +// readWorkspaceBillingOverride reads the OPTIONAL explicit operator override +// (workspaces.llm_billing_mode). Returns: +// +// (mode, true, nil) — a recognized override is set → operator pinned the mode +// ("", false, nil) — NULL / garbled / row-missing → no explicit override +// ("", false, err) — DB error → caller defaults closed + propagates +// +// internal#718 P2-B retires the org rung; this column is the ONLY stored +// billing signal that survives, and ONLY as an explicit override on top of the +// derived provider (CTO 2026-05-27). +func readWorkspaceBillingOverride(ctx context.Context, workspaceID string) (string, bool, error) { + var wsOverride sql.NullString + err := db.DB.QueryRowContext(ctx, + `SELECT llm_billing_mode FROM workspaces WHERE id = $1`, + workspaceID, + ).Scan(&wsOverride) + switch { + case errors.Is(err, sql.ErrNoRows): + return "", false, nil + case err != nil: + return "", false, fmt.Errorf("resolve workspace llm_billing_mode override for %s: %w", workspaceID, err) } - return LLMBillingModePlatformManaged + if wsOverride.Valid && isKnownBillingMode(wsOverride.String) { + return wsOverride.String, true, nil + } + return "", false, nil } -// ResolveLLMBillingMode is the canonical resolver. Every code path that -// previously gated on `os.Getenv("MOLECULE_LLM_BILLING_MODE") == "platform_managed"` -// must call this instead and gate on the returned mode. The architectural -// test (resolver_ast_test.go) asserts there is no remaining call site of -// the old shape outside the resolver-input wiring. +// ResolveLLMBillingModeDerived is the SSOT billing-mode resolver (internal#718 +// P2-B). It DERIVES the provider from (runtime, model) via the provider +// registry and decides platform-vs-byok from IsPlatform(derived) — it does NOT +// read a stored LLM_PROVIDER (superseding #1966's stored-read approach) and +// does NOT read the org rung (retired, CTO 2026-05-27). +// +// Precedence (highest first): +// +// 1. EXPLICIT operator override (workspaces.llm_billing_mode, a recognized +// value). The only stored billing signal that survives — an escape hatch, +// not the primary signal. +// 2. DERIVE: providers.DeriveProvider(runtime, model, availableAuthEnv). +// - resolves to the closed `platform` provider → platform_managed +// - resolves to any other (BYOK/third-party) provider → byok ← THE FIX +// 3. DEFAULT-CLOSED: derive fails (no model, unknown runtime, unregistered or +// ambiguous model) → platform_managed (CTO "unset → platform default"). A +// derive failure NEVER silently flips a workspace to byok (which would +// strip the platform creds it may legitimately need). +// +// availableAuthEnv is the set of auth-env-var NAMES present for the workspace +// (never secret values) — the same disambiguation input DeriveProvider uses to +// split anthropic-oauth from anthropic-api. May be nil. +// +// A returned error never prevents a decision: ResolvedMode is always a valid +// enum value (default-closed). The error is informational (log + surface). +func ResolveLLMBillingModeDerived(ctx context.Context, workspaceID, runtime, model string, availableAuthEnv []string) (BillingModeResolution, error) { + res := BillingModeResolution{ + WorkspaceID: workspaceID, + // OrgDefault is retired as a billing source (internal#718 P2-B). Kept on + // the struct for wire-compat (admin route / CP mirror) but always the + // closed constant — never consulted in the decision. + OrgDefault: LLMBillingModePlatformManaged, + } + + // Pre-provision context (no workspace row yet): no override to read, default + // closed. (DeriveProvider could still run from the passed runtime/model, but + // the no-id path historically does no DB work and the strip gate only runs + // post-create, so keep it a pure default to preserve that contract.) + if workspaceID == "" { + res.ResolvedMode = LLMBillingModePlatformManaged + res.Source = BillingModeSourceDerivedDefault + return res, nil + } + + // Precedence 1: explicit operator override. + if mode, ok, err := readWorkspaceBillingOverride(ctx, workspaceID); err != nil { + // DB error — default closed AND propagate (never flip on a transient error). + res.ResolvedMode = LLMBillingModePlatformManaged + res.Source = BillingModeSourceConstantFallback + return res, err + } else if ok { + m := mode + res.WorkspaceOverride = &m + res.ResolvedMode = mode + res.Source = BillingModeSourceWorkspaceOverride + return res, nil + } + + // Precedence 2: DERIVE the provider from (runtime, model). + manifest, mErr := providerRegistry() + if mErr != nil || manifest == nil { + // Registry unavailable (malformed embedded YAML — a build-time defect the + // gates catch). Default closed. + res.ResolvedMode = LLMBillingModePlatformManaged + res.Source = BillingModeSourceDerivedDefault + return res, mErr + } + provider, dErr := manifest.DeriveProvider(runtime, model, availableAuthEnv) + if dErr != nil { + // No model / unknown runtime / unregistered / ambiguous → default closed. + // NOT an error to the caller: an unregistered model is a legitimate + // "we can't say it's BYOK, so bill the platform default" outcome, and the + // only-registered gate at the create/config API is where an unregistered + // model is rejected loudly. Here we just fail closed for safety. + res.ResolvedMode = LLMBillingModePlatformManaged + res.Source = BillingModeSourceDerivedDefault + sel := model + if sel != "" { + res.ProviderSelection = &sel + } + return res, nil + } + derivedName := provider.Name + res.ProviderSelection = &derivedName + res.Source = BillingModeSourceDerivedProvider + if provider.IsPlatform() { + res.ResolvedMode = LLMBillingModePlatformManaged + } else { + // A specific (non-platform) vendor was derived → bring-your-own-key. + res.ResolvedMode = LLMBillingModeBYOK + } + return res, nil +} + +// ResolveLLMBillingMode is the legacy-signature resolver retained for callers +// that do not have (runtime, model) in hand (the admin GET/PUT route and the +// secrets remote-pull path). It reads the workspace's stored runtime + model + +// available auth env from the DB and delegates to the DERIVED resolver +// (internal#718 P2-B) — the orgMode parameter is RETIRED (the org rung is no +// longer a billing source) and is ignored; it stays in the signature only to +// avoid churning the two callers in this PR. The architectural test asserts no +// remaining code path gates on os.Getenv("MOLECULE_LLM_BILLING_MODE") for the +// strip decision (that env is no longer read into the decision at all). // // Returning an error does NOT prevent the caller from making a decision — // the returned mode is always a valid enum value (default-closed to @@ -120,75 +279,160 @@ func normalizeOrgDefault(orgMode string) string { // branch. The error is informational: log it, surface it to operators, but // the strip-gate decision is already safe. func ResolveLLMBillingMode(ctx context.Context, workspaceID, orgMode string) (BillingModeResolution, error) { - res := BillingModeResolution{ - WorkspaceID: workspaceID, - OrgDefault: normalizeOrgDefault(orgMode), - } + _ = orgMode // org rung retired (internal#718 P2-B); parameter ignored. if workspaceID == "" { - // No workspace ID = pre-provision context (templating, validation). - // Resolve against the org default only, no DB read. - res.ResolvedMode = res.OrgDefault - res.Source = BillingModeSourceOrgDefault - if !isKnownBillingMode(orgMode) { - // Org default was garbled/NULL and we clamped to platform_managed. - // Mark the source as constant_fallback so the operator can see - // the clamp happened, not that the org "really" said platform_managed. - res.Source = BillingModeSourceConstantFallback - } - return res, nil + // Pre-provision context (templating, validation): default closed, no DB. + return ResolveLLMBillingModeDerived(ctx, "", "", "", nil) } - var wsOverride sql.NullString - err := db.DB.QueryRowContext(ctx, - `SELECT llm_billing_mode FROM workspaces WHERE id = $1`, + // Precedence 1: explicit operator override. Read it FIRST so an overridden + // workspace short-circuits without the extra runtime/secrets reads (and so + // the query order is override → runtime → secrets, matching the derived + // resolver's own override-first precedence). + if mode, ok, err := readWorkspaceBillingOverride(ctx, workspaceID); err != nil { + return BillingModeResolution{ + WorkspaceID: workspaceID, + OrgDefault: LLMBillingModePlatformManaged, + ResolvedMode: LLMBillingModePlatformManaged, + Source: BillingModeSourceConstantFallback, + }, err + } else if ok { + m := mode + return BillingModeResolution{ + WorkspaceID: workspaceID, + OrgDefault: LLMBillingModePlatformManaged, + ResolvedMode: mode, + WorkspaceOverride: &m, + Source: BillingModeSourceWorkspaceOverride, + }, nil + } + + // Precedence 2: DERIVE. Read the stored (runtime, model, available-auth-env) + // so the derived resolver can DeriveProvider for callers that don't carry + // them (admin route, secrets remote-pull). A read miss/error degrades + // gracefully: pass the empty/partial inputs through — DeriveProvider then + // errors and the derived resolver defaults closed to platform_managed. + // + // ResolveLLMBillingModeDerived re-reads the override (NULL again here) before + // deriving; that one extra cheap read keeps the derived resolver a complete, + // independently-callable SSOT rather than splitting its precedence across two + // functions. + runtime, model, authEnv := readWorkspaceDeriveInputs(ctx, workspaceID) + return ResolveLLMBillingModeDerived(ctx, workspaceID, runtime, model, authEnv) +} + +// readWorkspaceDeriveInputs loads the workspace's stored runtime + selected +// model + the auth-env-var NAMES present in its secrets — the inputs +// DeriveProvider needs. Best-effort: any read error returns whatever was +// gathered (the derived resolver fails closed on incomplete inputs). The model +// is the MODEL workspace_secret (the canvas-picked id, written by setModelSecret +// / Create); runtime is the workspaces.runtime column (defaults claude-code). +// availableAuthEnv is the subset of secret KEYS that are recognized provider +// auth-env names (never values), so DeriveProvider's auth-env tie-break can fire +// the same way it does on the provision path. +func readWorkspaceDeriveInputs(ctx context.Context, workspaceID string) (runtime, model string, availableAuthEnv []string) { + var rt sql.NullString + if err := db.DB.QueryRowContext(ctx, + `SELECT runtime FROM workspaces WHERE id = $1`, workspaceID, + ).Scan(&rt); err != nil { + if !errors.Is(err, sql.ErrNoRows) { + log.Printf("llm_billing_mode: read runtime for %s: %v (deriving with empty runtime)", workspaceID, err) + } + } + runtime = rt.String + if runtime == "" { + // Mirror the DB column default so an unset runtime still derives. + runtime = "claude-code" + } + + // Gather model + auth-env-name keys from workspace_secrets in one pass. + authSet := authEnvNameSet() + rows, err := db.DB.QueryContext(ctx, + `SELECT key, encrypted_value, encryption_version FROM workspace_secrets WHERE workspace_id = $1`, workspaceID, - ).Scan(&wsOverride) - - switch { - case errors.Is(err, sql.ErrNoRows): - // Workspace row missing — concurrent delete, or pre-create call. Don't - // silently flip; fall through to org default. Source stays org_default - // so operators can see the row-missing case is being handled as a - // fallback, not a workspace-explicit decision. - res.ResolvedMode = res.OrgDefault - res.Source = BillingModeSourceOrgDefault - if !isKnownBillingMode(orgMode) { - res.Source = BillingModeSourceConstantFallback + ) + if err != nil { + log.Printf("llm_billing_mode: read secrets for %s: %v (deriving with no model/auth-env)", workspaceID, err) + return runtime, model, availableAuthEnv + } + defer rows.Close() + for rows.Next() { + var k string + var v []byte + var ver int + if rows.Scan(&k, &v, &ver) != nil { + continue + } + if k == "MODEL" { + if dec, derr := crypto.DecryptVersioned(v, ver); derr == nil { + model = string(dec) + } + continue + } + // Only the KEY matters for auth-env disambiguation (the value is the + // secret; we never decrypt it for this purpose). Record recognized + // provider auth-env names. + if _, ok := authSet[k]; ok { + availableAuthEnv = append(availableAuthEnv, k) } - return res, nil - case err != nil: - // DB error — default-closed to platform_managed AND propagate the - // error so operators get a structured log line. The caller is - // expected to log and continue with the safe default. - res.ResolvedMode = LLMBillingModePlatformManaged - res.Source = BillingModeSourceConstantFallback - return res, fmt.Errorf("resolve workspace llm_billing_mode for %s: %w", workspaceID, err) } + return runtime, model, availableAuthEnv +} - if wsOverride.Valid && isKnownBillingMode(wsOverride.String) { - mode := wsOverride.String - res.WorkspaceOverride = &mode - res.ResolvedMode = mode - res.Source = BillingModeSourceWorkspaceOverride - return res, nil - } +// authEnvNameSet is the union of every provider's auth_env names in the +// registry — the recognized set readWorkspaceDeriveInputs filters secret keys +// against. Loaded once from the registry so it stays in sync with the SSOT (no +// hardcoded auth-env vocabulary). Registry-load failure yields an empty set +// (derive then runs without the auth-env tie-break, which only matters for the +// oauth-vs-api overlap; safe — it errors to default-closed rather than guessing). +var ( + authEnvNameSetOnce sync.Once + authEnvNameSetVal map[string]struct{} +) - // Override row present but the value is NULL or garbled. Fall through. - // If the value was non-NULL but garbled (CHECK constraint should prevent - // this, but defense in depth — a future migration could relax the check - // or another path could write the column directly), surface the raw - // override value so operators can spot the corrupt row. - if wsOverride.Valid { - raw := wsOverride.String - res.WorkspaceOverride = &raw +func authEnvNameSet() map[string]struct{} { + authEnvNameSetOnce.Do(func() { + authEnvNameSetVal = map[string]struct{}{} + m, err := providerRegistry() + if err != nil || m == nil { + return + } + for _, p := range m.Providers { + for _, e := range p.AuthEnv { + authEnvNameSetVal[e] = struct{}{} + } + } + }) + return authEnvNameSetVal +} + +// availableAuthEnvNames returns the recognized provider auth-env-var NAMES +// present (non-empty) in envVars — the DeriveProvider auth-env tie-break input. +// Never returns secret VALUES, only the env-var names. Used by the provision +// path (applyPlatformManagedLLMEnv), which already has the workspace env in +// hand, so it derives without a secrets DB round-trip. +func availableAuthEnvNames(envVars map[string]string) []string { + authSet := authEnvNameSet() + var out []string + for k, v := range envVars { + if v == "" { + continue + } + if _, ok := authSet[k]; ok { + out = append(out, k) + } } - res.ResolvedMode = res.OrgDefault - res.Source = BillingModeSourceOrgDefault - if !isKnownBillingMode(orgMode) { - res.Source = BillingModeSourceConstantFallback + return out +} + +// derefOrEmpty returns the pointed-to string or "" for a nil pointer. Used in +// log lines that surface an optional *string field. +func derefOrEmpty(s *string) string { + if s == nil { + return "" } - return res, nil + return *s } // SetWorkspaceLLMBillingMode writes the override column. Pass mode=="" to diff --git a/workspace-server/internal/handlers/llm_billing_mode_derived_test.go b/workspace-server/internal/handlers/llm_billing_mode_derived_test.go new file mode 100644 index 000000000..1658528c7 --- /dev/null +++ b/workspace-server/internal/handlers/llm_billing_mode_derived_test.go @@ -0,0 +1,232 @@ +package handlers + +// llm_billing_mode_derived_test.go — tests for the DERIVED billing-mode +// resolver (internal#718 P2-B). The platform-vs-byok decision now DERIVES the +// provider from (runtime, model) via the provider registry and keys off +// IsPlatform(derived) — it does NOT read a stored LLM_PROVIDER (supersedes +// #1966's stored-read approach) and does NOT read the org rung (retired, +// CTO 2026-05-27). `workspaces.llm_billing_mode` survives ONLY as an optional +// explicit operator override (first precedence). +// +// This file pins the explicit BEHAVIOR DELTA the RFC's P2 calls out: +// - platform-derived (or unset → platform default) → platform_managed (UNCHANGED) +// - non-platform-derived → byok (THE FIX — the Reno leak class) +// - explicit override → wins over derive +// - derive error / unregistered → platform_managed (default-closed) + +import ( + "context" + "errors" + "testing" + + "github.com/DATA-DOG/go-sqlmock" +) + +// expectOverrideQuery sets up the workspaces.llm_billing_mode override read +// (first precedence). value=="" means NULL (no override). +func expectOverrideQuery(m sqlmock.Sqlmock, wsID, value string) { + rows := sqlmock.NewRows([]string{"llm_billing_mode"}) + if value == "" { + rows.AddRow(nil) + } else { + rows.AddRow(value) + } + m.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(rows) +} + +func TestResolveLLMBillingModeDerived_BehaviorDelta(t *testing.T) { + ctx := context.Background() + const wsID = "33333333-3333-3333-3333-333333333333" + + type tc struct { + name string + runtime string + model string + authEnv []string + override string // "" = NULL override (no explicit operator override) + wantMode string + wantSource BillingModeSource + wantErr bool + } + + cases := []tc{ + { + // PLATFORM-DERIVED → platform_managed (UNCHANGED). claude-code + + // a platform-namespaced model id derives to the closed `platform` + // provider → IsPlatform → platform_managed. + name: "platform_derived_keeps_platform_managed_UNCHANGED", + runtime: "claude-code", + model: "anthropic/claude-opus-4-7", + override: "", + wantMode: LLMBillingModePlatformManaged, + wantSource: BillingModeSourceDerivedProvider, + }, + { + // NON-PLATFORM-DERIVED → byok (THE FIX). claude-code + the + // kimi-coding-native model derives to the non-platform kimi-coding + // provider → IsPlatform=false → byok. This is the Reno billing-leak + // class: pre-P2 it resolved platform_managed and ran on platform creds. + name: "non_platform_derived_resolves_byok_THE_FIX", + runtime: "claude-code", + model: "kimi-for-coding", + override: "", + wantMode: LLMBillingModeBYOK, + wantSource: BillingModeSourceDerivedProvider, + }, + { + // NON-PLATFORM vendor on codex: gpt-5.5 derives to `openai` (BYOK). + name: "non_platform_openai_codex_byok", + runtime: "codex", + model: "gpt-5.5", + override: "", + wantMode: LLMBillingModeBYOK, + wantSource: BillingModeSourceDerivedProvider, + }, + { + // PLATFORM-DERIVED on codex: openai/gpt-5.4 is platform-namespaced. + name: "platform_derived_codex_platform_managed", + runtime: "codex", + model: "openai/gpt-5.4", + override: "", + wantMode: LLMBillingModePlatformManaged, + wantSource: BillingModeSourceDerivedProvider, + }, + { + // UNSET model → platform default (CTO-confirmed "unset → platform + // default"). No model means nothing to derive; default-closed. + name: "unset_model_platform_default", + runtime: "claude-code", + model: "", + override: "", + wantMode: LLMBillingModePlatformManaged, + wantSource: BillingModeSourceDerivedDefault, + }, + { + // UNREGISTERED model → derive errors → platform default (default-closed, + // NOT a silent byok flip that would strip a workspace's creds). + name: "unregistered_model_derive_error_platform_default", + runtime: "claude-code", + model: "totally-made-up-model-xyz", + override: "", + wantMode: LLMBillingModePlatformManaged, + wantSource: BillingModeSourceDerivedDefault, + }, + { + // UNKNOWN runtime → derive errors → platform default (default-closed). + name: "unknown_runtime_platform_default", + runtime: "no-such-runtime", + model: "claude-opus-4-7", + override: "", + wantMode: LLMBillingModePlatformManaged, + wantSource: BillingModeSourceDerivedDefault, + }, + { + // EXPLICIT OVERRIDE wins over derive: a non-platform-deriving model + // kept on platform_managed by an operator override (escape hatch). + name: "explicit_override_platform_managed_wins_over_byok_derive", + runtime: "claude-code", + model: "kimi-for-coding", // would derive byok + override: LLMBillingModePlatformManaged, + wantMode: LLMBillingModePlatformManaged, + wantSource: BillingModeSourceWorkspaceOverride, + }, + { + // EXPLICIT OVERRIDE byok wins over a platform-deriving model. + name: "explicit_override_byok_wins_over_platform_derive", + runtime: "claude-code", + model: "anthropic/claude-opus-4-7", // would derive platform_managed + override: LLMBillingModeBYOK, + wantMode: LLMBillingModeBYOK, + wantSource: BillingModeSourceWorkspaceOverride, + }, + { + // EXPLICIT OVERRIDE disabled wins (no-LLM workspace). + name: "explicit_override_disabled_wins", + runtime: "claude-code", + model: "anthropic/claude-opus-4-7", + override: LLMBillingModeDisabled, + wantMode: LLMBillingModeDisabled, + wantSource: BillingModeSourceWorkspaceOverride, + }, + { + // AUTH-ENV disambiguation: claude-code's anthropic-oauth (alias + // model "opus") vs anthropic-api both could match a bare alias; with + // CLAUDE_CODE_OAUTH_TOKEN present it derives anthropic-oauth → byok. + name: "auth_env_disambiguates_oauth_byok", + runtime: "claude-code", + model: "opus", + authEnv: []string{"CLAUDE_CODE_OAUTH_TOKEN"}, + override: "", + wantMode: LLMBillingModeBYOK, + wantSource: BillingModeSourceDerivedProvider, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + mock := setupTestDB(t) + expectOverrideQuery(mock, wsID, c.override) + + res, err := ResolveLLMBillingModeDerived(ctx, wsID, c.runtime, c.model, c.authEnv) + if (err != nil) != c.wantErr { + t.Fatalf("err: got %v wantErr=%v", err, c.wantErr) + } + if res.ResolvedMode != c.wantMode { + t.Errorf("mode: got %q want %q", res.ResolvedMode, c.wantMode) + } + if res.Source != c.wantSource { + t.Errorf("source: got %q want %q", res.Source, c.wantSource) + } + if !isKnownBillingMode(res.ResolvedMode) { + t.Errorf("post-condition: resolved mode %q not a known enum", res.ResolvedMode) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock expectations: %v", err) + } + }) + } +} + +// TestResolveLLMBillingModeDerived_OverrideDBError_DefaultClosed asserts a DB +// error reading the override column defaults closed to platform_managed and +// propagates the error — never silently flips a workspace off platform creds. +func TestResolveLLMBillingModeDerived_OverrideDBError_DefaultClosed(t *testing.T) { + ctx := context.Background() + const wsID = "44444444-4444-4444-4444-444444444444" + + mock := setupTestDB(t) + mock.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnError(errors.New("connection refused")) + + res, err := ResolveLLMBillingModeDerived(ctx, wsID, "claude-code", "kimi-for-coding", nil) + if err == nil { + t.Fatalf("expected propagated DB error, got nil") + } + if res.ResolvedMode != LLMBillingModePlatformManaged { + t.Errorf("default-closed: DB error must resolve platform_managed, got %q", res.ResolvedMode) + } + if res.Source != BillingModeSourceConstantFallback { + t.Errorf("source: got %q want %q", res.Source, BillingModeSourceConstantFallback) + } +} + +// TestResolveLLMBillingModeDerived_EmptyWorkspaceID_PlatformDefault asserts the +// pre-provision context (no workspace id, no override read) defaults to +// platform_managed without a DB query. +func TestResolveLLMBillingModeDerived_EmptyWorkspaceID_PlatformDefault(t *testing.T) { + ctx := context.Background() + mock := setupTestDB(t) // no query expected + res, err := ResolveLLMBillingModeDerived(ctx, "", "claude-code", "kimi-for-coding", nil) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if res.ResolvedMode != LLMBillingModePlatformManaged { + t.Errorf("empty workspace id must default platform_managed, got %q", res.ResolvedMode) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock expectations: %v", err) + } +} diff --git a/workspace-server/internal/handlers/llm_billing_mode_handler.go b/workspace-server/internal/handlers/llm_billing_mode_handler.go index 6fe7cbc76..17ffcef73 100644 --- a/workspace-server/internal/handlers/llm_billing_mode_handler.go +++ b/workspace-server/internal/handlers/llm_billing_mode_handler.go @@ -36,10 +36,12 @@ import ( // GetWorkspaceLLMBillingMode handles GET /admin/workspaces/:id/llm-billing-mode. // -// Reads the workspace override + the org-level default (from the same -// MOLECULE_LLM_BILLING_MODE env var the provisioner reads at strip-gate time — -// keeps the two paths consistent so the GET result matches what the strip -// gate would compute) and returns the structured resolution. +// internal#718 P2-B: the resolution now DERIVES the provider from the +// workspace's stored (runtime, model) via the registry (org rung retired). The +// passed orgMode is ignored by the resolver; it is left here only to avoid +// churning the call signature. The returned resolution matches what the +// provision-time strip gate computes (same derived resolver), so operators see +// the real platform-vs-byok decision + the derived provider in ProviderSelection. func GetWorkspaceLLMBillingMode(c *gin.Context) { workspaceID := strings.TrimSpace(c.Param("id")) if !uuidRegex.MatchString(workspaceID) { diff --git a/workspace-server/internal/handlers/llm_billing_mode_handler_test.go b/workspace-server/internal/handlers/llm_billing_mode_handler_test.go index 342b4f28e..2b20488e0 100644 --- a/workspace-server/internal/handlers/llm_billing_mode_handler_test.go +++ b/workspace-server/internal/handlers/llm_billing_mode_handler_test.go @@ -29,13 +29,42 @@ func init() { const testWSID = "44444444-4444-4444-4444-444444444444" -func TestGetWorkspaceLLMBillingMode_HappyPath_InheritsOrgDefault(t *testing.T) { - t.Setenv("MOLECULE_LLM_BILLING_MODE", LLMBillingModeBYOK) +// expectDeriveShimQueries sets up the three reads the legacy-signature +// ResolveLLMBillingMode shim makes on a no-explicit-override path +// (internal#718 P2-B): the override read (NULL here), the workspaces.runtime +// read, and the workspace_secrets scan (for MODEL + auth-env names). model=="" +// means no MODEL secret row. +func expectDeriveShimQueries(m sqlmock.Sqlmock, wsID, runtime, model string) { + nullOverride := func() { + m.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow(nil)) + } + // Order: override(NULL) shim check, runtime, secrets, override(NULL) again + // (the derived resolver re-checks the override as a complete SSOT). + nullOverride() + m.ExpectQuery(`SELECT runtime FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"runtime"}).AddRow(runtime)) + secretRows := sqlmock.NewRows([]string{"key", "encrypted_value", "encryption_version"}) + if model != "" { + // encryption_version 0 = plaintext passthrough (crypto.DecryptVersioned). + secretRows.AddRow("MODEL", []byte(model), 0) + } + m.ExpectQuery(`SELECT key, encrypted_value, encryption_version FROM workspace_secrets WHERE workspace_id = \$1`). + WithArgs(wsID). + WillReturnRows(secretRows) + nullOverride() +} + +// internal#718 P2-B: org rung retired. A no-override workspace's mode is now +// DERIVED from its stored (runtime, model). A claude-code workspace with a +// non-platform-deriving model (kimi-for-coding) resolves byok via +// derived_provider — NOT the old "inherit org default". +func TestGetWorkspaceLLMBillingMode_HappyPath_DerivesByokFromModel(t *testing.T) { + t.Setenv("MOLECULE_LLM_BILLING_MODE", LLMBillingModeBYOK) // org env ignored now mock := setupTestDB(t) - // Workspace has no override → resolver returns org_default = byok. - mock.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). - WithArgs(testWSID). - WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow(nil)) + expectDeriveShimQueries(mock, testWSID, "claude-code", "kimi-for-coding") w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) @@ -54,12 +83,15 @@ func TestGetWorkspaceLLMBillingMode_HappyPath_InheritsOrgDefault(t *testing.T) { if res.ResolvedMode != LLMBillingModeBYOK { t.Errorf("resolved mode: got %q want %q", res.ResolvedMode, LLMBillingModeBYOK) } - if res.Source != BillingModeSourceOrgDefault { - t.Errorf("source: got %q want %q", res.Source, BillingModeSourceOrgDefault) + if res.Source != BillingModeSourceDerivedProvider { + t.Errorf("source: got %q want %q", res.Source, BillingModeSourceDerivedProvider) } if res.WorkspaceOverride != nil { t.Errorf("expected nil override, got %v", *res.WorkspaceOverride) } + if res.ProviderSelection == nil || *res.ProviderSelection != "kimi-coding" { + t.Errorf("expected derived provider kimi-coding, got %v", res.ProviderSelection) + } } func TestGetWorkspaceLLMBillingMode_BadUUID_400(t *testing.T) { @@ -117,9 +149,9 @@ func TestPutWorkspaceLLMBillingMode_ExplicitNullClearsOverride(t *testing.T) { mock.ExpectExec(`UPDATE workspaces SET llm_billing_mode = NULL WHERE id = \$1`). WithArgs(testWSID). WillReturnResult(sqlmock.NewResult(0, 1)) - mock.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). - WithArgs(testWSID). - WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow(nil)) + // After clear, the post-write re-resolution DERIVES (internal#718 P2-B): + // no override + no MODEL secret → derived_default → platform_managed. + expectDeriveShimQueries(mock, testWSID, "claude-code", "") w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) @@ -142,8 +174,8 @@ func TestPutWorkspaceLLMBillingMode_ExplicitNullClearsOverride(t *testing.T) { if res.ResolvedMode != LLMBillingModePlatformManaged { t.Errorf("post-clear resolved: got %q want %q", res.ResolvedMode, LLMBillingModePlatformManaged) } - if res.Source != BillingModeSourceOrgDefault { - t.Errorf("post-clear source: got %q want %q", res.Source, BillingModeSourceOrgDefault) + if res.Source != BillingModeSourceDerivedDefault { + t.Errorf("post-clear source: got %q want %q", res.Source, BillingModeSourceDerivedDefault) } if res.WorkspaceOverride != nil { t.Errorf("post-clear override should be nil, got %v", *res.WorkspaceOverride) diff --git a/workspace-server/internal/handlers/llm_billing_mode_test.go b/workspace-server/internal/handlers/llm_billing_mode_test.go index aa4b1cac2..25d36115c 100644 --- a/workspace-server/internal/handlers/llm_billing_mode_test.go +++ b/workspace-server/internal/handlers/llm_billing_mode_test.go @@ -1,10 +1,12 @@ package handlers -// llm_billing_mode_test.go — table-driven tests for the per-workspace -// resolver (internal#691). The cases below enumerate every documented -// branch in the default-closed contract; if one of them flips behavior -// later the test names will tell the reviewer exactly which RFC clause -// regressed. +// llm_billing_mode_test.go — tests for the LEGACY-signature resolver +// ResolveLLMBillingMode after internal#718 P2-B. The org rung is RETIRED: the +// legacy shim now reads the explicit override first, then DERIVES the provider +// from the workspace's stored (runtime, model) via the registry (no org +// default). The dedicated derived-resolver cases live in +// llm_billing_mode_derived_test.go; this file pins the legacy shim's DB-read +// sequence + that it routes through the derived semantics. import ( "context" @@ -14,35 +16,56 @@ import ( "github.com/DATA-DOG/go-sqlmock" ) -func TestResolveLLMBillingMode_TableDriven(t *testing.T) { +// expectLegacyShimQueries sets up the DB reads the legacy ResolveLLMBillingMode +// shim makes on a NO-explicit-override path (internal#718 P2-B), in order: +// 1. override read (NULL) — the shim's own precedence-1 check, +// 2. workspaces.runtime read, +// 3. workspace_secrets scan (MODEL + auth-env names), +// 4. override read AGAIN (NULL) — the derived resolver re-checks it so it is a +// complete, independently-callable SSOT. +// +// model=="" means no MODEL secret row. +func expectLegacyShimQueries(m sqlmock.Sqlmock, wsID, runtime, model string) { + nullOverride := func() { + m.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow(nil)) + } + nullOverride() + m.ExpectQuery(`SELECT runtime FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"runtime"}).AddRow(runtime)) + secretRows := sqlmock.NewRows([]string{"key", "encrypted_value", "encryption_version"}) + if model != "" { + secretRows.AddRow("MODEL", []byte(model), 0) // version 0 = plaintext + } + m.ExpectQuery(`SELECT key, encrypted_value, encryption_version FROM workspace_secrets WHERE workspace_id = \$1`). + WithArgs(wsID). + WillReturnRows(secretRows) + nullOverride() +} + +func TestResolveLLMBillingMode_LegacyShimDerives(t *testing.T) { ctx := context.Background() const wsID = "11111111-1111-1111-1111-111111111111" type want struct { - mode string - source BillingModeSource - // hasOverride asserts whether the resolver surfaced the override - // value in the result (nil pointer = clean inherit, non-nil = the - // row was present even if it ultimately fell through because it - // was garbled). Lets us distinguish "row missing, fell through" - // from "row present but garbled, fell through" — both resolve to - // the same mode but the resolver tells operators which case it was. + mode string + source BillingModeSource hasOverride bool } type tc struct { - name string - workspaceID string - orgMode string - setupMock func(m sqlmock.Sqlmock) - want want - wantErr bool + name string + setupMock func(m sqlmock.Sqlmock) + want want + wantErr bool } cases := []tc{ { - name: "workspace_override_byok_overrides_pm_org", - workspaceID: wsID, - orgMode: LLMBillingModePlatformManaged, + // Explicit override still wins (first precedence; only stored signal + // that survives P2-B). No runtime/secrets read needed. + name: "explicit_override_byok_wins", setupMock: func(m sqlmock.Sqlmock) { m.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). WithArgs(wsID). @@ -51,106 +74,60 @@ func TestResolveLLMBillingMode_TableDriven(t *testing.T) { want: want{mode: LLMBillingModeBYOK, source: BillingModeSourceWorkspaceOverride, hasOverride: true}, }, { - name: "workspace_override_disabled_overrides_pm_org", - workspaceID: wsID, - orgMode: LLMBillingModePlatformManaged, + // No override + a non-platform-deriving model → byok via derive (THE + // FIX: pre-P2 this was platform_managed via the org rung). + name: "no_override_derives_byok_from_model", setupMock: func(m sqlmock.Sqlmock) { - m.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). - WithArgs(wsID). - WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow(LLMBillingModeDisabled)) + expectLegacyShimQueries(m, wsID, "claude-code", "kimi-for-coding") }, - want: want{mode: LLMBillingModeDisabled, source: BillingModeSourceWorkspaceOverride, hasOverride: true}, + want: want{mode: LLMBillingModeBYOK, source: BillingModeSourceDerivedProvider, hasOverride: false}, }, { - name: "workspace_override_null_inherits_byok_org", - workspaceID: wsID, - orgMode: LLMBillingModeBYOK, + // No override + a platform-namespaced model → platform_managed (UNCHANGED). + name: "no_override_derives_platform_from_model", setupMock: func(m sqlmock.Sqlmock) { - m.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). - WithArgs(wsID). - WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow(nil)) + expectLegacyShimQueries(m, wsID, "claude-code", "anthropic/claude-opus-4-7") }, - want: want{mode: LLMBillingModeBYOK, source: BillingModeSourceOrgDefault, hasOverride: false}, + want: want{mode: LLMBillingModePlatformManaged, source: BillingModeSourceDerivedProvider, hasOverride: false}, }, { - name: "workspace_override_null_inherits_pm_org", - workspaceID: wsID, - orgMode: LLMBillingModePlatformManaged, + // No override + no model → derived_default → platform_managed (unset → platform). + name: "no_override_no_model_platform_default", setupMock: func(m sqlmock.Sqlmock) { - m.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). - WithArgs(wsID). - WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow(nil)) + expectLegacyShimQueries(m, wsID, "claude-code", "") }, - want: want{mode: LLMBillingModePlatformManaged, source: BillingModeSourceOrgDefault, hasOverride: false}, + want: want{mode: LLMBillingModePlatformManaged, source: BillingModeSourceDerivedDefault, hasOverride: false}, }, { - name: "workspace_override_garbled_falls_through_to_pm_org_DEFAULT_CLOSED", - workspaceID: wsID, - orgMode: LLMBillingModePlatformManaged, + // Garbled override is NOT honored — falls through to derive + // (default-closed). Here no model → platform default. + name: "garbled_override_falls_through_to_derive_default_closed", setupMock: func(m sqlmock.Sqlmock) { - // CHECK constraint would normally prevent this but if a future - // migration loosens it (or a direct UPDATE bypasses it on a - // non-PG driver in a test stub), a garbled value MUST NOT - // be honored as if it were valid. This is the default-closed - // safety axis the RFC calls out. + // override read 1 (garbled → not honored), runtime, secrets, + // override read 2 (garbled again, derived resolver re-check). + m.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow("byokk")) + m.ExpectQuery(`SELECT runtime FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"runtime"}).AddRow("claude-code")) + m.ExpectQuery(`SELECT key, encrypted_value, encryption_version FROM workspace_secrets WHERE workspace_id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"key", "encrypted_value", "encryption_version"})) m.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). WithArgs(wsID). WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow("byokk")) }, - want: want{mode: LLMBillingModePlatformManaged, source: BillingModeSourceOrgDefault, hasOverride: true}, + want: want{mode: LLMBillingModePlatformManaged, source: BillingModeSourceDerivedDefault, hasOverride: false}, }, { - name: "workspace_override_garbled_org_garbled_constant_fallback", - workspaceID: wsID, - orgMode: "garbled-or-empty", - setupMock: func(m sqlmock.Sqlmock) { - m.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). - WithArgs(wsID). - WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow("nonsense")) - }, - // Both layers garbled → constant fallback. Source is constant_fallback - // so operators can see the org-default-was-also-bad case explicitly. - want: want{mode: LLMBillingModePlatformManaged, source: BillingModeSourceConstantFallback, hasOverride: true}, - }, - { - name: "workspace_row_missing_falls_through_to_org_byok", - workspaceID: wsID, - orgMode: LLMBillingModeBYOK, - setupMock: func(m sqlmock.Sqlmock) { - m.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). - WithArgs(wsID). - WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"})) - }, - want: want{mode: LLMBillingModeBYOK, source: BillingModeSourceOrgDefault, hasOverride: false}, - }, - { - name: "workspace_id_empty_pre_provision_org_only", - workspaceID: "", - orgMode: LLMBillingModeBYOK, - setupMock: func(m sqlmock.Sqlmock) { /* no DB read expected — empty ws id short-circuits */ }, - want: want{mode: LLMBillingModeBYOK, source: BillingModeSourceOrgDefault, hasOverride: false}, - }, - { - name: "workspace_id_empty_org_garbled_constant_fallback", - workspaceID: "", - orgMode: "", - setupMock: func(m sqlmock.Sqlmock) { /* no DB read */ }, - want: want{mode: LLMBillingModePlatformManaged, source: BillingModeSourceConstantFallback, hasOverride: false}, - }, - { - name: "db_error_default_closed_to_pm_with_error", - workspaceID: wsID, - orgMode: LLMBillingModeBYOK, // org says byok but DB errored — DO NOT honor org + // DB error on the override read → default-closed + propagated error. + name: "override_db_error_default_closed_with_error", setupMock: func(m sqlmock.Sqlmock) { m.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). WithArgs(wsID). WillReturnError(errors.New("connection refused")) }, - // Critical: even though orgMode=byok, a DB error means we can't - // confirm the workspace doesn't have an override, so we default - // to the closed mode. This is the safer of the two failures — - // silently flipping to org-byok on a DB error would leak the - // OAuth-keeping behavior to workspaces whose row says NULL. want: want{mode: LLMBillingModePlatformManaged, source: BillingModeSourceConstantFallback, hasOverride: false}, wantErr: true, }, @@ -161,7 +138,8 @@ func TestResolveLLMBillingMode_TableDriven(t *testing.T) { mock := setupTestDB(t) c.setupMock(mock) - res, err := ResolveLLMBillingMode(ctx, c.workspaceID, c.orgMode) + // orgMode arg is retired/ignored; pass a value to prove it has no effect. + res, err := ResolveLLMBillingMode(ctx, wsID, LLMBillingModeBYOK) if (err != nil) != c.wantErr { t.Fatalf("err: got %v wantErr=%v", err, c.wantErr) } @@ -172,8 +150,7 @@ func TestResolveLLMBillingMode_TableDriven(t *testing.T) { t.Errorf("source: got %q want %q", res.Source, c.want.source) } if (res.WorkspaceOverride != nil) != c.want.hasOverride { - t.Errorf("hasOverride: got %v want %v (override=%v)", - res.WorkspaceOverride != nil, c.want.hasOverride, res.WorkspaceOverride) + t.Errorf("hasOverride: got %v want %v", res.WorkspaceOverride != nil, c.want.hasOverride) } if err := mock.ExpectationsWereMet(); err != nil { t.Errorf("sqlmock expectations: %v", err) @@ -182,21 +159,48 @@ func TestResolveLLMBillingMode_TableDriven(t *testing.T) { } } +// TestResolveLLMBillingMode_EmptyWorkspaceID_PlatformDefault: pre-provision +// (no workspace id) defaults closed with no DB read (org rung retired, so the +// old "org_only" behavior is gone — it's now the platform default). +func TestResolveLLMBillingMode_EmptyWorkspaceID_PlatformDefault(t *testing.T) { + ctx := context.Background() + mock := setupTestDB(t) // no DB read expected + res, err := ResolveLLMBillingMode(ctx, "", LLMBillingModeBYOK) + if err != nil { + t.Fatalf("unexpected err: %v", err) + } + if res.ResolvedMode != LLMBillingModePlatformManaged { + t.Errorf("empty ws id must default platform_managed, got %q", res.ResolvedMode) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("sqlmock expectations: %v", err) + } +} + // TestResolveLLMBillingMode_ResolvedModeIsAlwaysValid asserts the resolver's // post-condition: the returned mode is ALWAYS one of the three known enum -// values, never an empty string and never a garbled passthrough. The strip -// gate downstream relies on this so it can switch on res.ResolvedMode -// without a separate is-valid check on every call site. +// values. The strip gate downstream relies on this so it can switch on +// res.ResolvedMode without a separate is-valid check on every call site. func TestResolveLLMBillingMode_ResolvedModeIsAlwaysValid(t *testing.T) { ctx := context.Background() const wsID = "22222222-2222-2222-2222-222222222222" - // Throw a pathological row at the resolver: garbled override + garbled - // org default. Resolved mode must still be a recognized enum. + // Garbled override + no derivable model: must still resolve a known enum + // (platform_managed, default-closed). Query order: override(garbled), + // runtime, secrets, override(garbled again — derived resolver re-check). mock := setupTestDB(t) mock.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). WithArgs(wsID). WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow("totally-bogus")) + mock.ExpectQuery(`SELECT runtime FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"runtime"}).AddRow("claude-code")) + mock.ExpectQuery(`SELECT key, encrypted_value, encryption_version FROM workspace_secrets WHERE workspace_id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"key", "encrypted_value", "encryption_version"})) + mock.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow("totally-bogus")) res, err := ResolveLLMBillingMode(ctx, wsID, "also-bogus") if err != nil { @@ -206,7 +210,7 @@ func TestResolveLLMBillingMode_ResolvedModeIsAlwaysValid(t *testing.T) { t.Errorf("post-condition violated: resolved mode %q is not a known enum value", res.ResolvedMode) } if res.ResolvedMode != LLMBillingModePlatformManaged { - t.Errorf("default-closed contract: garbled-x-garbled must resolve to platform_managed, got %q", res.ResolvedMode) + t.Errorf("default-closed contract: garbled-override + no-model must resolve platform_managed, got %q", res.ResolvedMode) } } diff --git a/workspace-server/internal/handlers/model_registry_validation.go b/workspace-server/internal/handlers/model_registry_validation.go new file mode 100644 index 000000000..43bf8fd1c --- /dev/null +++ b/workspace-server/internal/handlers/model_registry_validation.go @@ -0,0 +1,57 @@ +package handlers + +// model_registry_validation.go — only-registered (runtime, model) validation +// at the create/config API (internal#718 P2-B item 3, CTO 2026-05-27 +// "only registered providers/models selectable"). +// +// The registry (internal/providers) is the SSOT for which models a runtime +// natively exposes (ModelsForRuntime). This validator rejects a (runtime, model) +// the registry does NOT recognize — but ONLY for a runtime the registry knows +// about. For a runtime absent from the first-party registry (langgraph, +// external, kimi, mock, or a future federated third-party runtime), it fails +// OPEN: the registry can't speak to that runtime's model set, so the existing +// knownRuntimes gate stays authoritative and this validator does not block. +// This is the federation-ready contract — first-party runtimes are gated against +// the registry; everything else passes through unchanged (no behavior change for +// non-registry runtimes). + +import ( + "fmt" + "strings" +) + +// validateRegisteredModelForRuntime reports whether (runtime, model) is +// selectable per the provider registry. Returns: +// +// (true, "") — allowed: model is registered for this runtime, OR the +// runtime is not in the registry (fail-open), OR model=="". +// (false, reason) — rejected: the runtime IS registered but the model is not +// in its native ModelsForRuntime set. +// +// model=="" is allowed here: the MODEL_REQUIRED gate owns the empty-model case, +// so this validator must not double-reject it. +func validateRegisteredModelForRuntime(runtime, model string) (bool, string) { + model = strings.TrimSpace(model) + if model == "" { + return true, "" // MODEL_REQUIRED owns this. + } + m, err := providerRegistry() + if err != nil || m == nil { + // Registry unavailable (build-time defect the gates catch). Fail open — + // do not block create on a registry-load failure. + return true, "" + } + models, err := m.ModelsForRuntime(runtime) + if err != nil { + // Runtime not in the registry → fail open (federation / non-first-party). + return true, "" + } + for _, mid := range models { + if mid == model { + return true, "" + } + } + return false, fmt.Sprintf( + "model %q is not a registered model for runtime %q; pick one of the runtime's registered models (provider-registry SSOT, internal#718)", + model, runtime) +} diff --git a/workspace-server/internal/handlers/model_registry_validation_test.go b/workspace-server/internal/handlers/model_registry_validation_test.go new file mode 100644 index 000000000..ebd851b09 --- /dev/null +++ b/workspace-server/internal/handlers/model_registry_validation_test.go @@ -0,0 +1,83 @@ +package handlers + +// model_registry_validation_test.go — only-registered (runtime, model) +// validation at the create/config API (internal#718 P2-B item 3). Reject a +// (runtime, model) the registry does not recognize for a runtime it DOES know; +// fail OPEN (allow) for a runtime the registry doesn't know yet (federation / +// langgraph/etc. not in the first-party registry) so the existing knownRuntimes +// gate stays authoritative there. + +import "testing" + +func TestValidateRegisteredModelForRuntime(t *testing.T) { + type tc struct { + name string + runtime string + model string + wantOK bool // true = allowed (registered OR runtime-not-in-registry) + wantWhy string + } + cases := []tc{ + { + name: "registered_platform_model_allowed", + runtime: "claude-code", + model: "anthropic/claude-opus-4-7", + wantOK: true, + }, + { + name: "registered_byok_model_allowed", + runtime: "claude-code", + model: "kimi-for-coding", + wantOK: true, + }, + { + name: "registered_codex_model_allowed", + runtime: "codex", + model: "gpt-5.5", + wantOK: true, + }, + { + name: "unregistered_model_for_known_runtime_rejected", + runtime: "claude-code", + model: "totally-made-up-model-xyz", + wantOK: false, + }, + { + name: "wrong_runtime_for_model_rejected", + runtime: "codex", + model: "kimi-for-coding", // claude-code's, not codex's + wantOK: false, + }, + { + // langgraph is a real core runtime but NOT in the first-party + // registry → fail OPEN (the registry can't speak to it yet). + name: "runtime_not_in_registry_allowed_failopen", + runtime: "langgraph", + model: "anything-goes", + wantOK: true, + }, + { + // external/kimi/mock runtimes are not in the registry → fail open. + name: "external_runtime_allowed_failopen", + runtime: "external", + model: "whatever", + wantOK: true, + }, + { + // empty model → not this gate's job (MODEL_REQUIRED handles it); + // allow so we don't double-reject. + name: "empty_model_allowed_other_gate_owns_it", + runtime: "claude-code", + model: "", + wantOK: true, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + ok, _ := validateRegisteredModelForRuntime(c.runtime, c.model) + if ok != c.wantOK { + t.Errorf("validateRegisteredModelForRuntime(%q,%q) ok=%v want %v", c.runtime, c.model, ok, c.wantOK) + } + }) + } +} diff --git a/workspace-server/internal/handlers/workspace.go b/workspace-server/internal/handlers/workspace.go index b0947d756..93554d080 100644 --- a/workspace-server/internal/handlers/workspace.go +++ b/workspace-server/internal/handlers/workspace.go @@ -428,6 +428,33 @@ func (h *WorkspaceHandler) Create(c *gin.Context) { return } + // internal#718 P2-B: ONLY-REGISTERED validation at the create boundary. + // For a runtime the provider registry knows (first-party: + // claude-code/codex/hermes/openclaw) this checks the (runtime, model) pair + // against the registry's native model set. Fails OPEN for runtimes the + // registry doesn't know (langgraph/external/kimi/mock/federated) so + // non-first-party flows are UNCHANGED. Skipped for external workspaces. + // + // P2 ENFORCEMENT MODE = WARN, not hard-reject (deliberate, scoped). The + // legacy colon-namespaced BYOK model vocabulary ("anthropic:claude-opus-4-7" + // etc.) is still live across the create/import/template corpus and is NOT + // yet reconciled into the registry's exact-id model sets — that convergence + // is P3 (canvas only-offers-registered) + P4 (template codegen). Hard- + // rejecting an unregistered (runtime, model) now would 422 those legitimate + // existing flows, a large behavior change outside P2's scope (P2's behavior + // delta is the billing/credential flip, below). So P2 surfaces the + // unregistered pair as a queryable warning + an X-Molecule-Model-Unregistered + // response header (operator/canvas signal) and lets create proceed; the gate + // flips to hard-reject (uncomment the 422 below) once P3/P4 land the + // vocabulary convergence. The registry model set is code-generated from the + // canonical providers.yaml (PR-A), so the check stays in sync with the SSOT. + if !isExternal { + if ok, why := validateRegisteredModelForRuntime(payload.Runtime, payload.Model); !ok { + log.Printf("Create: WARN unregistered model (runtime=%q model=%q): %s [internal#718 P2 warn-mode; hard-reject gated on P3/P4 vocabulary convergence]", payload.Runtime, payload.Model, why) + c.Header("X-Molecule-Model-Unregistered", "true") + } + } + ctx := c.Request.Context() // Convert empty role to NULL diff --git a/workspace-server/internal/handlers/workspace_provision.go b/workspace-server/internal/handlers/workspace_provision.go index 6f12b3453..83de93dbd 100644 --- a/workspace-server/internal/handlers/workspace_provision.go +++ b/workspace-server/internal/handlers/workspace_provision.go @@ -981,18 +981,32 @@ func applyRuntimeModelEnv(envVars map[string]string, runtime, model string) { type platformLLMEnvResult struct { ResolvedMode string HasUsableLLMCred bool + // Source records which layer decided the mode (internal#718 P2-B): + // derived_provider (registry derivation), derived_default (derive failed → + // platform default), workspace_override (explicit operator pin), or + // constant_fallback (DB error). Surfaced for observability + asserted by the + // behavior-delta tests so a regression of "derived, not stored" flips red. + Source BillingModeSource } func applyPlatformManagedLLMEnv(ctx context.Context, envVars map[string]string, globalKeys map[string]struct{}, workspaceID, runtime, model string) platformLLMEnvResult { - orgMode := strings.ToLower(strings.TrimSpace(os.Getenv("MOLECULE_LLM_BILLING_MODE"))) - res, resolveErr := ResolveLLMBillingMode(ctx, workspaceID, orgMode) + // internal#718 P2-B: the platform-vs-byok decision now DERIVES the provider + // from (runtime, model) via the registry and keys off IsPlatform(derived) — + // NOT a stored LLM_PROVIDER and NOT the org rung. This path already carries + // runtime + model + the workspace env, so it calls the DERIVED resolver + // directly (no DB round-trip for runtime/model). availableAuthEnv is the set + // of recognized provider auth-env-var NAMES present in envVars (the same + // disambiguation input the registry uses to split oauth-vs-api). The org-env + // MOLECULE_LLM_BILLING_MODE is NO LONGER read into the decision (retired). + availableAuthEnv := availableAuthEnvNames(envVars) + res, resolveErr := ResolveLLMBillingModeDerived(ctx, workspaceID, runtime, model, availableAuthEnv) if resolveErr != nil { // resolveErr != nil ⇒ resolver hit a DB error AND already defaulted // res.ResolvedMode to platform_managed. Log + proceed; the safe default // is already in place, no early return needed. log.Printf("workspace_provision: resolve billing mode workspace=%s err=%v (defaulting to platform_managed)", workspaceID, resolveErr) } - log.Printf("workspace_provision: billing mode workspace=%s resolved=%s source=%s org_default=%s", workspaceID, res.ResolvedMode, res.Source, res.OrgDefault) + log.Printf("workspace_provision: billing mode workspace=%s resolved=%s source=%s derived_provider=%s", workspaceID, res.ResolvedMode, res.Source, derefOrEmpty(res.ProviderSelection)) // internal#703: MOLECULE_LLM_BILLING_MODE in the container must reflect the // RESOLVED per-workspace mode, not a hardcoded literal. Pre-fix this var was // only emitted (hardcoded "platform_managed") on the strip path below, so a @@ -1023,6 +1037,7 @@ func applyPlatformManagedLLMEnv(ctx context.Context, envVars map[string]string, return platformLLMEnvResult{ ResolvedMode: res.ResolvedMode, HasUsableLLMCred: hasAnyPlatformManagedLLMKey(envVars), + Source: res.Source, } } baseURL := firstNonEmptyEnv("MOLECULE_LLM_BASE_URL", "OPENAI_BASE_URL") @@ -1034,7 +1049,7 @@ func applyPlatformManagedLLMEnv(ctx context.Context, envVars map[string]string, // here — but we report HasUsableLLMCred from whatever survived so the // caller's fail-closed branch (non-platform only) is never reached on // this path. - return platformLLMEnvResult{ResolvedMode: res.ResolvedMode, HasUsableLLMCred: true} + return platformLLMEnvResult{ResolvedMode: res.ResolvedMode, HasUsableLLMCred: true, Source: res.Source} } stripPlatformManagedLLMBypassEnv(envVars) @@ -1066,7 +1081,7 @@ func applyPlatformManagedLLMEnv(ctx context.Context, envVars map[string]string, // platform_managed: the CP proxy usage token (injected as ANTHROPIC_API_KEY // / OPENAI_API_KEY above) IS the usable credential, so the workspace is // never fail-closed on this path. - return platformLLMEnvResult{ResolvedMode: res.ResolvedMode, HasUsableLLMCred: true} + return platformLLMEnvResult{ResolvedMode: res.ResolvedMode, HasUsableLLMCred: true, Source: res.Source} } func stripPlatformManagedLLMBypassEnv(envVars map[string]string) { diff --git a/workspace-server/internal/handlers/workspace_provision_shared_test.go b/workspace-server/internal/handlers/workspace_provision_shared_test.go index 2c044cfa3..4682a50b5 100644 --- a/workspace-server/internal/handlers/workspace_provision_shared_test.go +++ b/workspace-server/internal/handlers/workspace_provision_shared_test.go @@ -1141,20 +1141,38 @@ func TestApplyPlatformManagedLLMEnv_ClaudeCodeStripsVendorBYOK(t *testing.T) { } } +// internal#718 P2-B: byok is now DERIVED, not org-env-driven. A claude-code +// workspace with NO explicit override + a non-platform-deriving model +// (kimi-for-coding → kimi-coding) resolves byok and must NOT get the CP proxy +// creds injected. (Pre-P2 this was driven by the org env MOLECULE_LLM_BILLING_MODE +// with an empty workspace id; that mechanism is retired.) func TestApplyPlatformManagedLLMEnv_NoopsOutsidePlatformManaged(t *testing.T) { - t.Setenv("MOLECULE_LLM_BILLING_MODE", "byok") + const wsID = "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee" + mock := setupTestDB(t) + // No explicit override → derive from (claude-code, kimi-for-coding) → byok. + mock.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow(nil)) + + t.Setenv("MOLECULE_LLM_BILLING_MODE", "platform_managed") // org env ignored now t.Setenv("MOLECULE_LLM_BASE_URL", "https://api.example.test/api/v1/internal/llm/openai/v1") t.Setenv("MOLECULE_LLM_USAGE_TOKEN", "tenant-admin-token") envVars := map[string]string{} - applyPlatformManagedLLMEnv(context.Background(), envVars, nil, "", "claude-code", "") + res := applyPlatformManagedLLMEnv(context.Background(), envVars, nil, wsID, "claude-code", "kimi-for-coding") + if res.ResolvedMode != LLMBillingModeBYOK { + t.Fatalf("resolved mode = %q, want byok (derived from non-platform model)", res.ResolvedMode) + } if _, ok := envVars["OPENAI_API_KEY"]; ok { t.Fatalf("OPENAI_API_KEY should not be set outside platform-managed mode") } if _, ok := envVars["MOLECULE_LLM_USAGE_TOKEN"]; ok { t.Fatalf("MOLECULE_LLM_USAGE_TOKEN should not be set outside platform-managed mode") } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } } // TestApplyPlatformManagedLLMEnv_ClaudeCodeByokKeepsOwnProviderEnv is the @@ -1274,6 +1292,136 @@ func TestApplyPlatformManagedLLMEnv_ByokStripsGlobalOriginOAuthToken(t *testing. } } +// ========================================================================= +// internal#718 P2-B BEHAVIOR DELTA — billing/credential decision DERIVES the +// provider (no stored LLM_PROVIDER, no override). These three tests are the +// explicit delta the RFC calls out, exercised through the real provision path +// (applyPlatformManagedLLMEnv) with the registry derivation driving the mode: +// - platform-derived → platform_managed → platform creds (UNCHANGED) +// - non-platform-derived → byok → #1963 strip + fail-closed (THE FIX) +// - unset model → platform default (CTO-confirmed) +// All use NO explicit override (override read returns NULL) so the DERIVATION +// is what decides — this is what supersedes #1966's stored-LLM_PROVIDER read. +// ========================================================================= + +// PLATFORM-DERIVED → UNCHANGED. A claude-code workspace with a platform- +// namespaced model (anthropic/claude-opus-4-7) derives to the closed `platform` +// provider → platform_managed → CP proxy creds injected, exactly as before. +func TestApplyPlatformManagedLLMEnv_DERIVED_PlatformModelKeepsPlatformCreds(t *testing.T) { + const wsID = "11111111-2222-3333-4444-555555555555" + mock := setupTestDB(t) + mock.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow(nil)) // NO override → derive + + t.Setenv("MOLECULE_LLM_BILLING_MODE", LLMBillingModeBYOK) // org env IGNORED now + t.Setenv("MOLECULE_LLM_BASE_URL", "https://api.example.test/api/v1/internal/llm/openai/v1") + t.Setenv("MOLECULE_LLM_ANTHROPIC_BASE_URL", "https://api.example.test/api/v1/internal/llm/anthropic") + t.Setenv("MOLECULE_LLM_USAGE_TOKEN", "tenant-admin-token") + + envVars := map[string]string{} + res := applyPlatformManagedLLMEnv(context.Background(), envVars, nil, wsID, "claude-code", "anthropic/claude-opus-4-7") + + if res.ResolvedMode != LLMBillingModePlatformManaged { + t.Fatalf("platform-derived model must resolve platform_managed, got %q (source=%s)", res.ResolvedMode, res.Source) + } + if res.Source != BillingModeSourceDerivedProvider { + t.Errorf("source: got %q want derived_provider", res.Source) + } + // Platform path injects the CP proxy creds (UNCHANGED behavior). + if got := envVars["ANTHROPIC_API_KEY"]; got != "tenant-admin-token" { + t.Errorf("platform path must inject the CP proxy token as ANTHROPIC_API_KEY, got %q", got) + } + if !res.HasUsableLLMCred { + t.Errorf("platform path always has a usable cred (the proxy token)") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + +// NON-PLATFORM-DERIVED → byok + STRIP + FAIL-CLOSED signal (THE FIX, the Reno +// billing-leak class). A claude-code workspace with a non-platform model +// (kimi-for-coding → kimi-coding) and NO override + NO own cred, inheriting only +// the platform's scope:global OAuth token, now DERIVES byok → #1963 strips the +// global token → HasUsableLLMCred=false → caller fails closed. Pre-P2 this same +// workspace resolved platform_managed (via the never-written org rung) and ran +// on the platform's credits. This is the discriminating delta test. +func TestApplyPlatformManagedLLMEnv_DERIVED_NonPlatformModelStripsAndFailsClosed(t *testing.T) { + const wsID = "99999999-8888-7777-6666-555555555555" + mock := setupTestDB(t) + mock.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow(nil)) // NO override → derive + + t.Setenv("MOLECULE_LLM_BILLING_MODE", LLMBillingModePlatformManaged) // org env IGNORED now + t.Setenv("MOLECULE_LLM_BASE_URL", "https://api.example.test/api/v1/internal/llm/openai/v1") + t.Setenv("MOLECULE_LLM_USAGE_TOKEN", "tenant-admin-token") + + // Only LLM cred is the platform's scope:global OAuth token (globalKeys). + envVars := map[string]string{ + "CLAUDE_CODE_OAUTH_TOKEN": "PLATFORM-GLOBAL-OAUTH-TOKEN", + } + globalKeys := map[string]struct{}{"CLAUDE_CODE_OAUTH_TOKEN": {}} + + res := applyPlatformManagedLLMEnv(context.Background(), envVars, globalKeys, wsID, "claude-code", "kimi-for-coding") + + // 1. DERIVED byok (NOT the old platform_managed default). + if res.ResolvedMode != LLMBillingModeBYOK { + t.Fatalf("non-platform-derived model must resolve byok, got %q (source=%s) — THE FIX regressed", res.ResolvedMode, res.Source) + } + if res.Source != BillingModeSourceDerivedProvider { + t.Errorf("source: got %q want derived_provider", res.Source) + } + // 2. #1963 strip: the platform global OAuth token is removed (leak closed). + if got, ok := envVars["CLAUDE_CODE_OAUTH_TOKEN"]; ok { + t.Fatalf("CLAUDE_CODE_OAUTH_TOKEN = %q present — must be stripped for a derived-byok workspace (Reno leak)", got) + } + // 3. No CP proxy creds forced. + if got, ok := envVars["ANTHROPIC_API_KEY"]; ok { + t.Fatalf("ANTHROPIC_API_KEY must NOT be injected for byok, got %q", got) + } + // 4. No usable cred → caller (prepareProvisionContext) fails closed. + if res.HasUsableLLMCred { + t.Fatalf("HasUsableLLMCred = true, want false (only the stripped platform global token was present)") + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + +// UNSET model → PLATFORM DEFAULT (CTO-confirmed "unset → platform default"). +// No model means nothing to derive; the workspace defaults closed to +// platform_managed and keeps the platform creds (UNCHANGED for the no-model case). +func TestApplyPlatformManagedLLMEnv_DERIVED_UnsetModelPlatformDefault(t *testing.T) { + const wsID = "00000000-1111-2222-3333-444444444444" + mock := setupTestDB(t) + mock.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). + WithArgs(wsID). + WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow(nil)) // NO override + + t.Setenv("MOLECULE_LLM_BILLING_MODE", LLMBillingModeBYOK) // org env IGNORED now + t.Setenv("MOLECULE_LLM_BASE_URL", "https://api.example.test/api/v1/internal/llm/openai/v1") + t.Setenv("MOLECULE_LLM_ANTHROPIC_BASE_URL", "https://api.example.test/api/v1/internal/llm/anthropic") + t.Setenv("MOLECULE_LLM_USAGE_TOKEN", "tenant-admin-token") + + envVars := map[string]string{} + res := applyPlatformManagedLLMEnv(context.Background(), envVars, nil, wsID, "claude-code", "") + + if res.ResolvedMode != LLMBillingModePlatformManaged { + t.Fatalf("unset model must default platform_managed, got %q (source=%s)", res.ResolvedMode, res.Source) + } + if res.Source != BillingModeSourceDerivedDefault { + t.Errorf("source: got %q want derived_default", res.Source) + } + if got := envVars["ANTHROPIC_API_KEY"]; got != "tenant-admin-token" { + t.Errorf("unset-model platform default must inject the CP proxy token, got %q", got) + } + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("unmet sqlmock expectations: %v", err) + } +} + // TestApplyPlatformManagedLLMEnv_ByokKeepsWorkspaceOwnOAuthEvenWithGlobal is // the discriminating companion to the strip test: a byok workspace that DID // set its own CLAUDE_CODE_OAUTH_TOKEN via the canvas Secrets tab (a diff --git a/workspace-server/internal/handlers/workspace_test.go b/workspace-server/internal/handlers/workspace_test.go index f10bcfb1e..249b8496e 100644 --- a/workspace-server/internal/handlers/workspace_test.go +++ b/workspace-server/internal/handlers/workspace_test.go @@ -501,10 +501,12 @@ func TestWorkspaceCreate_WithSecrets_Persists(t *testing.T) { // while persisting a secret causes the entire transaction to roll back and // the handler to return 500. The workspace row must NOT be committed. func TestWorkspaceCreate_SecretPersistFails_RollsBack(t *testing.T) { - // internal#691: see TestExtended_SecretsSet — same default-closed reasoning. - // This test is asserting the rollback path on DB failure, not the strip gate; - // keep the org in byok so the OPENAI_API_KEY write reaches the INSERT. - t.Setenv("MOLECULE_LLM_BILLING_MODE", "byok") + // internal#718 P2-B: this test asserts the rollback path on DB failure, not + // the strip gate. The create-time secret gate keys off the DERIVED mode now + // (org rung retired). An explicit byok override makes the workspace byok in a + // single resolver read (precedence-1 short-circuit), so the OPENAI_API_KEY + // write is allowed and reaches the INSERT-and-fail path this test exercises. + t.Setenv("MOLECULE_LLM_BILLING_MODE", "platform_managed") // org env ignored now mock := setupTestDB(t) setupTestRedis(t) broadcaster := newTestBroadcaster() @@ -513,14 +515,11 @@ func TestWorkspaceCreate_SecretPersistFails_RollsBack(t *testing.T) { mock.ExpectBegin() mock.ExpectExec("INSERT INTO workspaces"). WillReturnResult(sqlmock.NewResult(0, 1)) - // internal#691: Create() now resolves billing mode per-workspace before - // the secret-strip gate. The workspace row was just inserted in the same - // transaction so it isn't readable from a separate query yet; the - // resolver expects the SELECT and the mock returns no row → falls back - // to the org default (byok, set above) so the OPENAI_API_KEY write - // reaches the INSERT-and-fail path this test exercises. + // Create() resolves billing mode per-workspace before the secret-strip gate. + // An explicit byok override short-circuits the resolver (precedence 1) so the + // OPENAI_API_KEY write is allowed and reaches the INSERT-and-fail path. mock.ExpectQuery(`SELECT llm_billing_mode FROM workspaces WHERE id = \$1`). - WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"})) + WillReturnRows(sqlmock.NewRows([]string{"llm_billing_mode"}).AddRow(LLMBillingModeBYOK)) mock.ExpectExec("INSERT INTO workspace_secrets"). WillReturnError(sql.ErrConnDone) // DB failure while writing secret mock.ExpectRollback() // workspace insert must be rolled back @@ -2048,6 +2047,111 @@ func TestWorkspaceCreate_188_NoTemplateNoRuntime_NowMODEL_REQUIRED(t *testing.T) } } +// internal#718 P2-B: only-registered validation in P2 WARN-mode. A known +// (registry) runtime with a model NOT in its registered set is allowed to +// proceed (201) but flagged with the X-Molecule-Model-Unregistered response +// header + a queryable warning log. (Hard-reject is gated on P3/P4 vocabulary +// convergence — see the create handler comment; flipping to 422 there is a +// one-line change once the legacy colon-form model vocabulary is reconciled.) +func TestWorkspaceCreate_718_UnregisteredModelWarnsButProceeds(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + broadcaster := newTestBroadcaster() + handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir()) + + mock.ExpectBegin() + mock.ExpectExec("INSERT INTO workspaces"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit() + mock.ExpectExec("INSERT INTO workspace_secrets"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec("INSERT INTO canvas_layouts"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec("INSERT INTO structure_events"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + body := `{"name":"Bad Model","runtime":"claude-code","model":"totally-made-up-xyz"}` + c.Request = httptest.NewRequest("POST", "/workspaces", bytes.NewBufferString(body)) + c.Request.Header.Set("Content-Type", "application/json") + handler.Create(c) + + if w.Code != http.StatusCreated { + t.Fatalf("unregistered-model create (warn-mode): expected 201, got %d: %s", w.Code, w.Body.String()) + } + if w.Header().Get("X-Molecule-Model-Unregistered") != "true" { + t.Errorf("expected X-Molecule-Model-Unregistered=true header on unregistered model, got %q", w.Header().Get("X-Molecule-Model-Unregistered")) + } +} + +// A REGISTERED model on a registry runtime sets NO unregistered header. +func TestWorkspaceCreate_718_RegisteredModelNoWarnHeader(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + broadcaster := newTestBroadcaster() + handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir()) + + mock.ExpectBegin() + mock.ExpectExec("INSERT INTO workspaces"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit() + mock.ExpectExec("INSERT INTO workspace_secrets"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec("INSERT INTO canvas_layouts"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec("INSERT INTO structure_events"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + // claude-opus-4-7 IS a registered claude-code model (anthropic-api). + body := `{"name":"Good Model","runtime":"claude-code","model":"claude-opus-4-7"}` + c.Request = httptest.NewRequest("POST", "/workspaces", bytes.NewBufferString(body)) + c.Request.Header.Set("Content-Type", "application/json") + handler.Create(c) + + if w.Code != http.StatusCreated { + t.Fatalf("registered-model create: expected 201, got %d: %s", w.Code, w.Body.String()) + } + if w.Header().Get("X-Molecule-Model-Unregistered") != "" { + t.Errorf("registered model must NOT set the unregistered header, got %q", w.Header().Get("X-Molecule-Model-Unregistered")) + } +} + +// internal#718 P2-B: a runtime NOT in the registry (mock — a known core runtime +// absent from the first-party provider registry) fails OPEN — the +// only-registered gate does not block it (federation / non-first-party path +// unchanged). It proceeds past the gate to the normal create flow. +func TestWorkspaceCreate_718_NonRegistryRuntimeFailsOpen(t *testing.T) { + mock := setupTestDB(t) + setupTestRedis(t) + broadcaster := newTestBroadcaster() + handler := NewWorkspaceHandler(broadcaster, nil, "http://localhost:8080", t.TempDir()) + + mock.ExpectBegin() + mock.ExpectExec("INSERT INTO workspaces"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectCommit() + mock.ExpectExec("INSERT INTO canvas_layouts"). + WillReturnResult(sqlmock.NewResult(0, 1)) + mock.ExpectExec("INSERT INTO structure_events"). + WillReturnResult(sqlmock.NewResult(0, 1)) + + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + // "mock" is a known core runtime but NOT in the first-party registry; + // any model passes the only-registered gate (fail-open). + body := `{"name":"Mock Agent","runtime":"mock","model":"canned-replies"}` + c.Request = httptest.NewRequest("POST", "/workspaces", bytes.NewBufferString(body)) + c.Request.Header.Set("Content-Type", "application/json") + handler.Create(c) + + if w.Code != http.StatusCreated { + t.Fatalf("non-registry runtime should fail open (201), got %d: %s", w.Code, w.Body.String()) + } +} + // Explicit runtime, no template → honored, 201 (no template resolution // needed; runtimeExplicitlyRequested true but already resolved). func TestWorkspaceCreate_188_ExplicitRuntimeNoTemplate_OK(t *testing.T) { -- 2.52.0 From 924dfa55980ba12a6e707909015b1fb51b5e4d8b Mon Sep 17 00:00:00 2001 From: hongming Date: Thu, 28 May 2026 01:39:27 +0000 Subject: [PATCH 2/2] =?UTF-8?q?test(workspace-server):=20remove=20unused?= =?UTF-8?q?=20wantWhy=20field=20in=20model=5Fregistry=5Fvalidation=5Ftest?= =?UTF-8?q?=20(golangci-lint=20unused)=20=E2=80=94=20internal#718=20P2-B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../internal/handlers/model_registry_validation_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/workspace-server/internal/handlers/model_registry_validation_test.go b/workspace-server/internal/handlers/model_registry_validation_test.go index ebd851b09..92a015b06 100644 --- a/workspace-server/internal/handlers/model_registry_validation_test.go +++ b/workspace-server/internal/handlers/model_registry_validation_test.go @@ -15,7 +15,6 @@ func TestValidateRegisteredModelForRuntime(t *testing.T) { runtime string model string wantOK bool // true = allowed (registered OR runtime-not-in-registry) - wantWhy string } cases := []tc{ { -- 2.52.0