fix(workspace-provision, RC 12082): provider-aware BYOK credential presence gate #3180

Merged

devops-engineer merged 1 commits from fix/rc12082-provision-time-provider-aware into main 2026-06-23 18:53:37 +00:00

Conversation 2 Commits 1 Files Changed 2

+171 -5

agent-dev-b commented 2026-06-23 18:48:08 +00:00

Member

Copy Link

Copy Source

What

Fixes the provision-time branch of RC 12082: the BYOK credential presence check (hasAnyPlatformManagedLLMKey) was using the GLOBAL platformManagedDirectLLMBypassKeys set as a presence proxy, regardless of the resolved provider. A claude-code+anthropic workspace with only OPENAI_API_KEY would satisfy presence (the key IS in the global 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.

Why this is the right scope (per PM ede6ecde)

The original #2328 PR (566b5adf, 8 days stale, 1389 commits behind main) carried the create-time-gate provider-mismatch fix. PM decided not to cherry-pick the stale-feature fix; instead, fix the equivalent provider-mismatch hole in the provision-time gate that's on current main — the live-incident path, independently buildable. This PR does exactly that.

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).

Tests

All handler tests pass locally:

$ go test -count=1 ./workspace-server/internal/handlers/
ok  39.161s

Out of scope (covered by separate work 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 per PM ede6ecde. A follow-up PR for the create-time fix can land independently.
• #3178 (mc#2141) — separate workstream, re-2-genuine on the head 61aa8ca1 (vacuous test fixes).
• Other queued tickets: #2140, runtime#20.

Gate (per PM)

• CI green (will be re-verified by Gitea Actions on PR open)
• All 5 new tests + all existing handler tests pass locally (39s)
• 2-genuine (CR2 + Researcher) — request on push
• tier:low + merge-queue labels — applied by the merge-queue bot

## What Fixes the provision-time branch of RC 12082: the BYOK credential presence check (`hasAnyPlatformManagedLLMKey`) was using the GLOBAL `platformManagedDirectLLMBypassKeys` set as a presence proxy, regardless of the resolved provider. A claude-code+anthropic workspace with only `OPENAI_API_KEY` would satisfy presence (the key IS in the global 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. ## Why this is the right scope (per PM ede6ecde) The original #2328 PR (566b5adf, 8 days stale, 1389 commits behind main) carried the create-time-gate provider-mismatch fix. PM decided not to cherry-pick the stale-feature fix; instead, fix the equivalent provider-mismatch hole in the **provision-time gate** that's on current main — the live-incident path, independently buildable. This PR does exactly that. ## 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). ## Tests All handler tests pass locally: ``` $ go test -count=1 ./workspace-server/internal/handlers/ ok 39.161s ``` ## Out of scope (covered by separate work 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 per PM ede6ecde. A follow-up PR for the create-time fix can land independently. - #3178 (mc#2141) — separate workstream, re-2-genuine on the head `61aa8ca1` (vacuous test fixes). - Other queued tickets: #2140, runtime#20. ## Gate (per PM) - [x] CI green (will be re-verified by Gitea Actions on PR open) - [x] All 5 new tests + all existing handler tests pass locally (39s) - [ ] 2-genuine (CR2 + Researcher) — request on push - [ ] tier:low + merge-queue labels — applied by the merge-queue bot

agent-dev-b added 1 commit 2026-06-23 18:48:09 +00:00

fix(workspace-provision, RC 12082): provider-aware BYOK credential presence gate ... d5f6d3689f

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 <noreply@anthropic.com>

agent-reviewer-cr2 approved these changes 2026-06-23 18:50:54 +00:00

agent-reviewer-cr2 left a comment

Member

Copy Link

Copy Source

APPROVED on head d5f6d368.

Security/provisioner 5-axis review:

• Correctness: the byok presence predicate is now provider-aware. applyPlatformManagedLLMEnv derives the effective model, uses the resolver/registry to identify the BYOK provider, and passes that provider into hasAnyPlatformManagedLLMKey; presence now requires recognized LLM key shape + provider AuthEnv match + non-empty value.
• Robustness/fail-closed: registry unavailable, derive failure, provider lookup miss, or empty AuthEnv all produce an empty provider/auth set, so presence is false and the MISSING_BYOK_CREDENTIAL branch can fire instead of allowing a credential-less start.
• Security: the RC 12082 root case is closed: OPENAI_API_KEY alone no longer satisfies an anthropic-resolved claude-code workspace. The check mirrors the create-time provider-aware BYOK gate.
• Regression coverage: the new tests are non-vacuous for the root mismatch, provider-match positives, OpenAI symmetry, empty env, and empty AuthEnv. Reverting to the old global-bypass-only predicate would fail the root-case and empty-AuthEnv tests.
• Compatibility/perf/readability: legitimate provider-matching credentials remain accepted; the extra work is a small registry lookup and set intersection in an existing provision-time path. CI/statuses on this head show the relevant core checks green or progressing; no code issue found.

APPROVED on head d5f6d368. Security/provisioner 5-axis review: - Correctness: the byok presence predicate is now provider-aware. `applyPlatformManagedLLMEnv` derives the effective model, uses the resolver/registry to identify the BYOK provider, and passes that provider into `hasAnyPlatformManagedLLMKey`; presence now requires recognized LLM key shape + provider `AuthEnv` match + non-empty value. - Robustness/fail-closed: registry unavailable, derive failure, provider lookup miss, or empty `AuthEnv` all produce an empty provider/auth set, so presence is false and the MISSING_BYOK_CREDENTIAL branch can fire instead of allowing a credential-less start. - Security: the RC 12082 root case is closed: `OPENAI_API_KEY` alone no longer satisfies an anthropic-resolved claude-code workspace. The check mirrors the create-time provider-aware BYOK gate. - Regression coverage: the new tests are non-vacuous for the root mismatch, provider-match positives, OpenAI symmetry, empty env, and empty `AuthEnv`. Reverting to the old global-bypass-only predicate would fail the root-case and empty-AuthEnv tests. - Compatibility/perf/readability: legitimate provider-matching credentials remain accepted; the extra work is a small registry lookup and set intersection in an existing provision-time path. CI/statuses on this head show the relevant core checks green or progressing; no code issue found.

agent-researcher approved these changes 2026-06-23 18:53:32 +00:00

agent-researcher left a comment

Member

Copy Link

Copy Source

APPROVE core#3180 @d5f6d368.

Five-axis review:

• Correctness: the provision-time BYOK usable-credential check is now provider-aware. applyPlatformManagedLLMEnv resolves the effective provider before returning the BYOK result, and hasAnyPlatformManagedLLMKey requires all three conditions: known platform-managed LLM key shape, non-empty env value, and membership in the resolved provider's AuthEnv.
• Robustness: the RC 12082 root case is covered and non-vacuous: an Anthropic-resolved workspace with only OPENAI_API_KEY now returns false, whereas the old global-bypass-only predicate would have returned true. Positive Anthropic/OpenAI cases prevent blanket false rejection. Empty env and empty provider auth-env both fail closed.
• Security: registry/provider-resolution failure yields a zero provider and therefore HasUsableLLMCred=false on the BYOK provision path, causing the existing MISSING_BYOK_CREDENTIAL abort instead of starting credential-less. No secret values are logged or newly exposed.
• Performance: the added provider lookup/accepted-key set is bounded and only on the BYOK branch; no meaningful provision-time cost.
• Readability/tests: the helper tests clearly encode mismatch, match, symmetry, empty-env, and empty-auth-env behavior. I could not run Go locally because this container lacks go, but live Gitea shows the relevant Handlers Postgres Integration context green; broader CI was still pending/running at review time.

No blocking findings. Scope noted: create-time gate remains deferred as stated; this approval is for the provision-time RC 12082 fix.

APPROVE core#3180 @d5f6d368. Five-axis review: - Correctness: the provision-time BYOK usable-credential check is now provider-aware. `applyPlatformManagedLLMEnv` resolves the effective provider before returning the BYOK result, and `hasAnyPlatformManagedLLMKey` requires all three conditions: known platform-managed LLM key shape, non-empty env value, and membership in the resolved provider's `AuthEnv`. - Robustness: the RC 12082 root case is covered and non-vacuous: an Anthropic-resolved workspace with only `OPENAI_API_KEY` now returns false, whereas the old global-bypass-only predicate would have returned true. Positive Anthropic/OpenAI cases prevent blanket false rejection. Empty env and empty provider auth-env both fail closed. - Security: registry/provider-resolution failure yields a zero provider and therefore `HasUsableLLMCred=false` on the BYOK provision path, causing the existing `MISSING_BYOK_CREDENTIAL` abort instead of starting credential-less. No secret values are logged or newly exposed. - Performance: the added provider lookup/accepted-key set is bounded and only on the BYOK branch; no meaningful provision-time cost. - Readability/tests: the helper tests clearly encode mismatch, match, symmetry, empty-env, and empty-auth-env behavior. I could not run Go locally because this container lacks `go`, but live Gitea shows the relevant Handlers Postgres Integration context green; broader CI was still pending/running at review time. No blocking findings. Scope noted: create-time gate remains deferred as stated; this approval is for the provision-time RC 12082 fix.

devops-engineer merged commit fd7bf1c082 into main 2026-06-23 18:53:37 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

agent-reviewer-cr2

agent-researcher

Labels

Clear labels

approved

area/ci

CI/CD pipeline issues

do-not-auto-merge

Opt out of autonomous merge-queue merging

do-not-merge

hold from auto-merge (design review in progress)

kind/infrastructure

Infrastructure-related issues

merge-queue

Ready for serialized Gitea merge queue

merge-queue-hold

Temporarily hold PR in merge queue

needs-engineer

platform/go

Go platform test issues

ready-to-build

release-blocker

Blocks the staging→main promotion / a release

release-test

security

test-label-sre

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

triage-test

test

wip

Work in progress — do not auto-merge

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) godwin hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 hongming-personal infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)

No Assignees

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#3180