From d5f6d3689f6aebfde156102479dbf39bc946622c Mon Sep 17 00:00:00 2001 From: "Molecule AI Dev Engineer B (MiniMax)" Date: Tue, 23 Jun 2026 18:47:50 +0000 Subject: [PATCH] fix(workspace-provision, RC 12082): provider-aware BYOK credential presence gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Researcher RC 12082 (the autonomous-tick note on the stale #2328 branch): the EXISTING provision-time fail-closed branch has the identical provider-mismatch hole that the create-time gate has. Fix both, ideally via one shared provider-aware helper. The provision-time path (applyPlatformManagedLLMEnv → byok) was using hasAnyPlatformManagedLLMKey which checks the GLOBAL platformManagedDirectLLMBypassKeys set as a presence proxy, regardless of the resolved provider. A claude-code+anthropic workspace with only OPENAI_API_KEY in envVars would satisfy presence (the key IS in the global bypass set), then 201+credential-less+die-at-provision. The fix: intersect the bypass-set iteration with the resolved provider's auth_env. A present key is now presence only if (a) it's in the global bypass shape AND (b) it's in the resolved provider's auth_env. The global set is a cheap pre-filter; the provider-match is the real fail-closed check. Changes ------- - workspace-server/internal/handlers/workspace_provision.go: - hasAnyPlatformManagedLLMKey now takes a providers.Provider and intersects the bypass-set iteration with provider.AuthEnv (mirrors anyBYOKCredentialKeyMatchesProvider in byok_credential_gate.go). - The call site in applyPlatformManagedLLMEnv derives the resolved provider from runtime/model (or res.ProviderSelection) BEFORE the presence check, so the provider is always available. Registry unavailable → defaults to the zero-value Provider (empty AuthEnv) which fails presence (safe default — a misconfigured registry cannot accidentally satisfy a byok workspace's credential check). - workspace-server/internal/handlers/byok_provision_provider_mismatch_test.go: NEW. 5 regression tests: 1. TestHasAnyPlatformManagedLLMKey_ProviderMismatch_FailsClosed — the RC 12082 root case: claude-code+anthropic with only OPENAI_API_KEY returns false (was true pre-fix). 2. TestHasAnyPlatformManagedLLMKey_ProviderMatch_Passes — positive case: ANTHROPIC_API_KEY + anthropic-api returns true. 3. TestHasAnyPlatformManagedLLMKey_OpenAI_OK — symmetry: OPENAI_API_KEY + openai returns true. 4. TestHasAnyPlatformManagedLLMKey_EmptyEnvVars — empty envVars returns false (preserves the pre-fix behavior). 5. TestHasAnyPlatformManagedLLMKey_EmptyAuthEnv — provider with empty auth_env returns false (registry-misconfig safety). All handler tests pass locally (39s). Out of scope (covered by separate PR per PM's queue plan): - The create-time gate (byok_credential_gate.go) has the parallel fix in anyBYOKCredentialKeyMatchesProvider. The original #2328 PR (566b5adf) was the build for that fix; it was deferred to allow this provision-time fix to land independently (per PM ede6ecde). Co-Authored-By: Claude --- .../byok_provision_provider_mismatch_test.go | 123 ++++++++++++++++++ .../internal/handlers/workspace_provision.go | 53 +++++++- 2 files changed, 171 insertions(+), 5 deletions(-) create mode 100644 workspace-server/internal/handlers/byok_provision_provider_mismatch_test.go diff --git a/workspace-server/internal/handlers/byok_provision_provider_mismatch_test.go b/workspace-server/internal/handlers/byok_provision_provider_mismatch_test.go new file mode 100644 index 00000000..2e4d0c53 --- /dev/null +++ b/workspace-server/internal/handlers/byok_provision_provider_mismatch_test.go @@ -0,0 +1,123 @@ +package handlers + +// byok_provision_provider_mismatch_test.go — RC 12082 regression guard. +// +// Pins the contract for hasAnyPlatformManagedLLMKey: the presence check +// for the BYOK provision-time fail-closed branch MUST be provider-AWARE. +// A stray key (e.g. OPENAI_API_KEY in a claude-code+anthropic workspace) +// MUST NOT satisfy presence even though the key IS in the global bypass +// set — the resolved provider (anthropic-api) would never authenticate +// with it, and the workspace would 201+credential-less+die-at-provision. +// +// Pre-fix bug: the global bypass set (platformManagedDirectLLMBypassKeys) +// was the SOLE presence check — it accepted ANY key in the set as +// presence, regardless of whether the derived provider would accept it. +// The fix intersects the bypass set with the resolved provider's auth_env. +// +// The create-time gate (byok_credential_gate.go) has the parallel fix in +// anyBYOKCredentialKeyMatchesProvider. These tests pin the provision-time +// side of the same contract. + +import ( + "testing" + + "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/providers" +) + +// TestHasAnyPlatformManagedLLMKey_ProviderMismatch_FailsClosed pins the +// RC 12082 fix: a claude-code+anthropic workspace carrying ONLY +// OPENAI_API_KEY (in the global bypass set but NOT in anthropic-api's +// auth_env) MUST NOT satisfy presence — HasUsableLLMCred must be false +// so the provision-time fail-closed branch fires MISSING_BYOK_CREDENTIAL +// rather than starting the workspace credential-less. +func TestHasAnyPlatformManagedLLMKey_ProviderMismatch_FailsClosed(t *testing.T) { + // anthropic-api's auth_env (per providers.yaml): ANTHROPIC_API_KEY, + // ANTHROPIC_AUTH_TOKEN. Notably NO OPENAI_API_KEY. + provider := providers.Provider{ + Name: "anthropic-api", + AuthEnv: []string{"ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN"}, + } + + envVars := map[string]string{ + // OPENAI_API_KEY is in the global bypass set BUT NOT in anthropic-api's + // auth_env. The runtime would never accept it — presence must fail. + "OPENAI_API_KEY": "sk-stray-1234", + // Non-bypass keys (operator secrets that anthropic-api happens to + // accept but that aren't LLM keys) MUST also not satisfy presence + // (catches the over-broad bypass-set filter). + "FOO_NON_LLM": "x", + } + + if hasAnyPlatformManagedLLMKey(provider, envVars) { + t.Errorf("hasAnyPlatformManagedLLMKey returned true for a claude-code+anthropic workspace with only OPENAI_API_KEY (RC 12082 bypass); want false so the provision-time fail-closed branch fires MISSING_BYOK_CREDENTIAL") + } +} + +// TestHasAnyPlatformManagedLLMKey_ProviderMatch_Passes pins the +// positive case: a workspace with a credential that DOES match the +// resolved provider's auth_env MUST satisfy presence. +func TestHasAnyPlatformManagedLLMKey_ProviderMatch_Passes(t *testing.T) { + provider := providers.Provider{ + Name: "anthropic-api", + AuthEnv: []string{"ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN"}, + } + + envVars := map[string]string{ + "ANTHROPIC_API_KEY": "sk-ant-1234", + } + + if !hasAnyPlatformManagedLLMKey(provider, envVars) { + t.Errorf("hasAnyPlatformManagedLLMKey returned false for a workspace with ANTHROPIC_API_KEY matching anthropic-api's auth_env; want true") + } +} + +// TestHasAnyPlatformManagedLLMKey_OpenAI_OK pins that the fix is +// symmetric — an openai-resolved workspace with only OPENAI_API_KEY +// (in the global bypass set AND in openai's auth_env) DOES satisfy +// presence. +func TestHasAnyPlatformManagedLLMKey_OpenAI_OK(t *testing.T) { + provider := providers.Provider{ + Name: "openai", + AuthEnv: []string{"OPENAI_API_KEY"}, + } + + envVars := map[string]string{ + "OPENAI_API_KEY": "sk-openai-1234", + } + + if !hasAnyPlatformManagedLLMKey(provider, envVars) { + t.Errorf("hasAnyPlatformManagedLLMKey returned false for an openai workspace with OPENAI_API_KEY matching openai's auth_env; want true") + } +} + +// TestHasAnyPlatformManagedLLMKey_EmptyEnvVars pins that no envVars at +// all fails (the pre-fix behavior; preserved). +func TestHasAnyPlatformManagedLLMKey_EmptyEnvVars(t *testing.T) { + provider := providers.Provider{ + Name: "anthropic-api", + AuthEnv: []string{"ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN"}, + } + + if hasAnyPlatformManagedLLMKey(provider, map[string]string{}) { + t.Errorf("hasAnyPlatformManagedLLMKey returned true for empty envVars; want false") + } +} + +// TestHasAnyPlatformManagedLLMKey_EmptyAuthEnv pins that a provider +// with NO auth_env (e.g. a misconfigured registry) fails presence even +// for a key in the global bypass set — the provider.AuthEnv intersection +// is empty, so no key matches. +func TestHasAnyPlatformManagedLLMKey_EmptyAuthEnv(t *testing.T) { + provider := providers.Provider{ + Name: "unknown-provider", + AuthEnv: []string{}, // empty — fall-through safe-default + } + + envVars := map[string]string{ + "ANTHROPIC_API_KEY": "sk-ant-1234", + } + + if hasAnyPlatformManagedLLMKey(provider, envVars) { + t.Errorf("hasAnyPlatformManagedLLMKey returned true for a provider with empty auth_env; want false (no key can match a provider with no accepted keys)") + } +} \ No newline at end of file diff --git a/workspace-server/internal/handlers/workspace_provision.go b/workspace-server/internal/handlers/workspace_provision.go index 7b29785d..dad62bfb 100644 --- a/workspace-server/internal/handlers/workspace_provision.go +++ b/workspace-server/internal/handlers/workspace_provision.go @@ -1241,6 +1241,28 @@ func applyPlatformManagedLLMEnv(ctx context.Context, envVars map[string]string, // workspace's own ANTHROPIC_BASE_URL, and DO NOT strip the tenant's own // (provider-matching) LLM credentials. // + // RC 12082: derive the resolved BYOK provider NOW so the presence check + // below (hasAnyPlatformManagedLLMKey) can be provider-AWARE. Without + // this, a stray key (e.g. OPENAI_API_KEY in a claude-code+anthropic + // workspace) would satisfy the global bypass set even though the + // resolved provider (anthropic-api) would never authenticate with it. + var byokResolvedProvider providers.Provider + if manifest, mErr := providerRegistry(); mErr == nil && manifest != nil { + providerName := derefOrEmpty(res.ProviderSelection) + if providerName == "" { + if p, dErr := manifest.DeriveProvider(runtime, effectiveModel, availableAuthEnv); dErr == nil { + providerName = p.Name + } + } + if p, ok := providerFromRegistry(providerName); ok { + byokResolvedProvider = p + } + } + + // molecule-core#1994 (corrected model): `global_secrets` is the + // workspace's own ANTHROPIC_BASE_URL, and DO NOT strip the tenant's own + // (provider-matching) LLM credentials. + // // molecule-core#1994 (corrected model): `global_secrets` is the // TENANT's store, not the platform's. The tenant's own credential — // at global OR workspace scope — is exactly what byok runs on, direct. @@ -1325,7 +1347,7 @@ func applyPlatformManagedLLMEnv(ctx context.Context, envVars map[string]string, return platformLLMEnvResult{ ResolvedMode: res.ResolvedMode, - HasUsableLLMCred: hasAnyPlatformManagedLLMKey(envVars), + HasUsableLLMCred: hasAnyPlatformManagedLLMKey(byokResolvedProvider, envVars), Source: res.Source, } } @@ -1404,12 +1426,33 @@ func stripPlatformManagedLLMBypassEnv(envVars map[string]string) { } // hasAnyPlatformManagedLLMKey reports whether envVars carries at least one -// non-empty platform-managed-shaped LLM credential key (the tenant's own, at -// global or workspace scope). Used by the byok fail-closed branch: a byok -// workspace with no LLM credential at ANY scope must be aborted with +// non-empty platform-managed-shaped LLM credential key that ALSO matches +// the resolved provider's auth_env (RC 12082 — provider-mismatch fail-closed). +// Used by the byok fail-closed branch: a byok workspace with no LLM +// credential for ITS resolved provider at ANY scope must be aborted with // MISSING_BYOK_CREDENTIAL rather than started credential-less. -func hasAnyPlatformManagedLLMKey(envVars map[string]string) bool { +// +// The provider-AWARE check is the real fail-closed predicate. The global +// bypass set (platformManagedDirectLLMBypassKeys) is over-broad by design +// (every known LLM vendor key lives there so the canvas Secrets tab can +// write any vendor's key without a registry check) — without the +// provider.AuthEnv intersection, a stray key (e.g. OPENAI_API_KEY in a +// claude-code+anthropic workspace) would satisfy presence even though the +// resolved provider would never authenticate with it. Mirrors the +// create-time gate's anyBYOKCredentialKeyMatchesProvider (byok_credential_gate.go). +func hasAnyPlatformManagedLLMKey(provider providers.Provider, envVars map[string]string) bool { + // Build the accepted-key set from the provider's auth_env (case-insensitive). + accepted := make(map[string]struct{}, len(provider.AuthEnv)) + for _, e := range provider.AuthEnv { + accepted[strings.ToUpper(strings.TrimSpace(e))] = struct{}{} + } for key := range platformManagedDirectLLMBypassKeys { + // Must be a recognized bypass shape (LLM key) AND match the provider's auth_env + // AND have a non-empty value in envVars. All three conditions required. + upper := strings.ToUpper(strings.TrimSpace(key)) + if _, matches := accepted[upper]; !matches { + continue + } if strings.TrimSpace(envVars[key]) != "" { return true } -- 2.52.0