fix(background-review): preserve profile-scoped credentials

[lightcloud00]

Summary

• recognize the live parent route through both requested and resolved provider identities
• inherit the parent runtime credential for same-provider/model review forks
• resolve distinct-route key_env credentials through the active profile secret scope
• preserve requested-provider identity on the spawned review agent

Security invariant

A multiplexed profile must never forward a process-global credential captured from another profile. Named custom-provider aliases that resolve to custom must inherit the live parent credential when they identify the same route, while explicit separate providers remain independently routed.

Verification

• 41 focused and neighboring background-review tests passed
• real temp-profile config and custom-provider resolution passed with a conflicting process-global value and scoped profile value
• repository-wide Ruff passed
• compileall, diff check, and staged gitleaks scan passed

The full repository suite was attempted, but the fallback repository venv lacks pytest-asyncio and unrelated async test files fail before this change is exercised.

[andrexibiza]

Reviewed exact head 29244b7c2e3dc5680780533b1c5f7eaa21b8b43a against exact/current main fcbd1076a93841fa88855acce810e342a5b78101 (the PR is one commit ahead, zero behind). The direction is right: carrying requested_provider, recognizing named custom aliases that resolve to runtime custom, and re-resolving key_env through the active secret_scope close a real multiplex credential-routing hole. The background-review worker already receives the parent ContextVars through propagate_context_to_thread, so scoped lookup is available at this boundary.

I found two authority gaps that still block the stated security invariant.

1. api_key without key_env still forwards the already-expanded process-global credential

The new scoped re-resolution only runs when task_key_env is present. Otherwise _resolve_review_runtime() still does:

task_api_key = str(task.get("api_key", "")).strip() or None

and then passes that value as explicit_api_key to resolve_runtime_provider().

That is the same stale-provenance value this PR is trying to stop trusting. load_config_readonly() can already have expanded api_key: ${LITELLM_MASTER_KEY} from the process environment before the secondary profile scope was installed. The new real-resolution regression demonstrates exactly that process-global/scoped mismatch, but includes both api_key and key_env, so the new branch replaces the contaminated value and the inverse case is untested.

Concrete witness: profile A's process env has LITELLM_MASTER_KEY=A; active multiplex profile B's scope has LITELLM_MASTER_KEY=B; auxiliary.background_review config contains a distinct provider/model and only api_key: ${LITELLM_MASTER_KEY}. This head forwards A explicitly to the review route. That directly violates the PR's invariant that a multiplexed profile must never forward a process-global credential captured from another profile.

Please make credential provenance authoritative for every supported auxiliary credential form, not only the key_env form. If an already-expanded inline api_key cannot be proven to belong to the active profile under multiplexing, it must not outrank scoped/provider resolution. Add the no-key_env adversarial regression with conflicting A/B values.

2. "same route" currently means provider alias + model, but base_url is already part of this task's route contract

The early inheritance check runs before task_base_url is considered:

if task_model == agent.model and _review_provider_matches_parent(...):
    return parent

So a named custom parent litellm-local at endpoint A and a background-review block with the same alias/model but an explicit endpoint B is treated as the parent route. The configured base_url (and any credential meant for B) is silently ignored. This is newly relevant because the alias matcher intentionally broadens what counts as "same provider" for named-custom routes.

The route identity used to decide whether inheriting the parent's live credential is safe should include the effective endpoint (and any other authority-bearing runtime identity needed to distinguish routes), not just provider spelling + model. Add a witness where alias/model match but explicit base_url differs and prove the review resolves B instead of collapsing to A.

Interlocks / ownership

• #62081 is landed substrate for carrying the complete resolved runtime bundle into background-review/curator/delegation forks; it salvaged #61567 while preserving @infinitycrew39's authorship. #91509 should remain a narrow credential-provenance correction on top of that, not recreate runtime propagation.
• #74099 by @okbexx is overlapping/open requested-provider identity work and also touches agent/background_review.py; coordinate the identity semantics/merge order so named-custom route identity does not diverge between timeout lookup and background-review credential routing.
• #78573 by @web-wyf is adjacent/open background-review custom-provider model-routing work; it establishes that the configured auxiliary route must retain the caller's explicit model rather than silently fall back to provider defaults. The endpoint side needs the same exactness here.
• #77592 by @lesterlxt and #83007 by @wz-heng (for #82936, reported by @neo-wanderer) are adjacent multiplex-isolation work at dotenv/subprocess boundaries. They are not duplicates of this API-client boundary, but they establish the same governing rule: profile-scoped authority must beat ambient process state, and missing scoped authority must not inherit another profile's credential.

Verification state

The focused tests added here cover the positive key_env path and alias inheritance well, and the PR reports 41 focused/neighboring tests plus Ruff/compile/diff/gitleaks locally. Hosted exact-head workflows are currently action_required, so there is no hosted green receipt yet for 29244b7....

Once the two inverse cases above are closed, this is the right place to land the background-review side of the defect class.

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

Right fix at the right seam. The bug is exactly the multiplex-leak shape the repo's secret-scope contract warns about: auxiliary.background_review.api_key interpolated at config-load time captures the default profile's process-global value, and the old same-route check (task_provider == agent.provider) missed named custom aliases because the agent keeps requested_provider="litellm-local" while provider collapses to "custom" — so a same-route fork was treated as distinct routing and forwarded the stale expanded key verbatim. The dual-identity match in _review_provider_matches_parent plus refusing to treat bare custom as equivalent to every named endpoint is the correct discrimination, and making key_env authoritative over the already-expanded copy routes the read through secret_scope.get_secret, whose multiplex-miss-returns-default-no-os.environ-fallthrough semantics are exactly what this path needs. The real-resolver test (test_distinct_review_provider_real_resolution_uses_profile_scope) exercises the actual resolve_runtime_provider chain against a temp HERMES_HOME with a conflicting process-global value — the E2E standard this class of change should be held to. Downstream stays leak-safe too: runtime_provider._getenv is scope-aware, so the explicit_api_key=None fallback cannot reintroduce the leak through resolution.

Points:

1. Missing negative test for the invariant the docstring states: task_provider="custom" (bare) against a named-alias parent must NOT match and must remain routed. The parametrized cases cover the positive alias forms (litellm-local, custom:litellm-local); pinning the bare-custom collapse refusal protects the exact regression this helper exists to prevent.
2. Behavior note worth a docs line: when an auxiliary block sets both api_key and key_env, key_env now silently wins. Deliberate and right (the expanded copy is untrustworthy under multiplexing), but users who set both with different values should learn that from configuration docs, not from debugging.
3. Nit: _review_provider_matches_parent normalizes case and spaces-to-dashes but not underscores/dots; consistent with custom_provider_aliases()' own normalization, so fine today — just noting any future alias-normalization change now has two coordinated places.
4. Nice detail: propagating requested_provider onto the spawned review agent in _finish_request_phase — without it the child's own aux resolutions would re-collapse to custom and reproduce the same misroute class one level down.