fix(gateway): require usable API_SERVER_KEY to enroll the api_server platform (#36180 salvage) - #70166
Merged
Merged
Conversation
…platform at load time Salvaged from PR #36180 (commits 68dfeb4 and 86f4375 by arimu1), re-applied onto current main with the incidental black-reformat churn stripped out (~1,700 lines -> the semantic change + tests). Previously gateway/config.py enrolled the api_server platform on `api_server_enabled or api_server_key`, so API_SERVER_ENABLED=true with no key (or a weak/placeholder key) still loaded the platform: the adapter is instantiated (ResponseStore/SQLite opened in __init__), the reconnect watcher spins, and the startup guard refuses at connect() — logging errors forever. Now the platform is enrolled only when API_SERVER_KEY passes the same strength bar as the adapter's startup guard (has_usable_secret, min_length=16), via a shared _has_usable_api_server_key() helper. The no-op `lambda cfg: True` connected-checker for API_SERVER is also replaced with the same key check, so get_connected_platforms() only reports the platform "up" when it could actually start. Known limitation (intentionally out of scope): a YAML config with `platforms.api_server.enabled: true` and no key still loads the platform; this gate covers the env-override path only. Dropped from the original PR: EMAIL/SMS checker additions (scope creep beyond the PR title; absent on current main) and the wholesale black reformat of gateway/config.py and tests. Fixes #36111 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
૮ >ﻌ< ა ci reviewran on 8f3c691 all good! |
alt-glitch
added
type/bug
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Salvage of #36180 by @arimu1 (earliest PR in the #36111 cluster) — the api_server platform is only enrolled at load time when a usable key exists, closing the "loads, spins the reconnect watcher, and logs errors forever" path.
API_SERVER_ENABLED=truewith no (or a weak) key enrolled the platform anyway; the adapter's startup guard then refused every connect() while the watcher retried indefinitely. This gates enrollment with the same strength bar as the startup guard (has_usable_secret, min_length=16), and gives the platform an honest connected-checker.Changes
gateway/config.py:_has_usable_api_server_key()helper; env enrollment gated on it;Platform.API_SERVERconnected-checker upgraded fromlambda cfg: Trueto a key checktests/gateway/test_api_server.py,tests/gateway/test_platform_connected_checkers.py: no-key / weak-key / strong-key enrollment testsSalvage note: the original PR was ~1,700 lines, mostly incidental reformat churn of
gateway/config.py; landed here as the 72-line semantic change only. EMAIL/SMS checker additions (scope creep) dropped. Known partial: YAMLenabled: truewithout a key still loads — the connect-time guard covers that path.Validation
API_SERVER_ENABLED=true, no keyAuthorship preserved (@arimu1). Closes #36180.
Infographic