Repository navigation
fix(sse): fail closed on background token-refresh for dead proxy pools (#13470) - #13793
Merged
diegosouzapw merged 2 commits intoSep 16, 2026
Merged
diegosouzapw merged 2 commits into
diegosouzapw merged 2 commits into
Conversation
#13470) resolveProxyForCredentials (src/sse/services/tokenRefresh.ts, shared by all 7 exported refresh functions) and tokenHealthCheck.ts's two direct resolveProxyForConnection call sites resolved a connection with a fully dead assigned proxy pool to bare direct egress, unlike the interactive chat/executor path (safeResolveProxy) which already fails closed per the #6246 guard. This routes both background paths through hasBlockingProxyAssignment, mirroring the #6246 policy (PROXY_FAIL_OPEN escape hatch included) so refresh-token traffic never leaks onto a connection's real IP when its proxy pool goes dark. Regression test: tests/unit/issue-13470-token-refresh-proxy-bypass.test.ts
…ypasses-dead (base-red fix #13747)
muhamadgalihsaputra
pushed a commit
to niyatna/NiyatnaRoute
that referenced
this pull request
Sep 27, 2026
diegosouzapw#13470) (diegosouzapw#13793) Merged in the 2026-09-16 sweep of the maintainer's own open PRs, at the owner's explicit instruction. No push was made to the PR branch: the merge took the head as the owning session left it (verified OPEN, non-draft and MERGEABLE against the release tip immediately before merging).
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
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.
Closes #13470
Root cause (short)
The interactive chat/executor path fails closed via
safeResolveProxy+hasBlockingProxyAssignment(src/sse/handlers/chatHelpers.ts) when a connection'sassigned proxy pool has gone fully dead (the #6246 guard). The background OAuth
token-refresh path never consulted that guard:
resolveProxyForCredentials(src/sse/services/tokenRefresh.ts), the shared helperbehind all 7 exported refresh functions (
refreshAccessToken,refreshClaudeOAuthToken,refreshGoogleToken,refreshCodexToken,refreshQoderToken,refreshGitHubToken,refreshCopilotToken), calledresolveProxyForConnectiondirectly and fell through toresolveProxyForProvideron any non-alive result — including a dead-pool result —without ever checking
hasBlockingProxyAssignment.src/lib/tokenHealthCheck.ts's two health-check sweep call sites(
resolveProxyForConnection+extractResolvedProxyConfig) had the same gap.Net effect: a connection whose assigned proxy pool is entirely dead resolves to
{ proxy: null, level: "direct" }at the DB layer, and only the guarded chat pathtreated that as an error. The background refresh paths silently proceeded, sending
the refresh-token exchange out on
resolveEnvProxyUrl's fallback or a truly directconnection — the same class of IP-provenance leak #6246 closed, on a more sensitive
payload (the refresh token itself).
Fix
resolveProxyForCredentialsnow callshasBlockingProxyAssignmentwhen the DBresolution has no live proxy, and throws a
PROXY_ASSIGNED_UNAVAILABLE-coded errorinstead of falling through to
resolveProxyForProvider— mirroringsafeResolveProxy's#6246contract (including thePROXY_FAIL_OPENescapehatch).
decideProxyResolutionFailurecouldn't be imported fromchatHelpers.tswithout creating an import cycle (chatHelpers.ts already importsupdateProviderCredentialsfrom this module), so the same policy is duplicatedverbatim as
decideTokenRefreshProxyFailure.tokenHealthCheck.tscall sites are replaced with a new sharedresolveGuardedProxyConfighelper (extracted to a new leaf module,src/lib/tokenHealthCheckProxyGuard.ts, to stay under the frozen file-size cap ontokenHealthCheck.ts). On a blocked resolution it skips the connection'srefresh cycle for the current tick (log + return) instead of throwing — this is a
scheduler sweep over many connections, so one blocked connection must not abort the
rest.
Not covered here
DIRECT_PROXY_CONTEXTsentinel suggestion for the legitimatedirect case (no assignment at all) is not included — the guard added here only
changes behavior for the dead-pool case the issue is about; a connection with no
proxy assignment continues to resolve exactly as before.
Regression test (path + RED output excerpt on unfixed code + GREEN excerpt)
tests/unit/issue-13470-token-refresh-proxy-bypass.test.ts— the plan-file's originalrepro asserted the buggy asymmetry as a passing test (proving the leak existed); it
is inverted here to assert the fixed contract instead, run against
resolveProxyForCredentialsand the newresolveGuardedProxyConfigdirectly (ratherthan through
refreshAccessToken's network path, since that helper never throws onan HTTP-level failure — testing at the network boundary would have hit a real OAuth
endpoint in the test and produced a false negative).
RED (on unfixed
origin/release/v3.8.51):GREEN (after the fix):
Gates run
npx eslint --suppressions-location config/quality/eslint-suppressions.json <changed files>— clean, 0 new warnings.npm run typecheck:core— clean (0 errors).node scripts/check/check-file-size.mjs— clean on touched files (the newresolveGuardedProxyConfigguard was extracted to its own leaf module,src/lib/tokenHealthCheckProxyGuard.ts, sotokenHealthCheck.tsstays under itsfrozen 1218-line cap: 1214 lines after the fix).
open-sse/utils/stream.tsisflagged by this gate but is untouched by this PR — pre-existing base drift
(3115 lines vs frozen 3098 on
origin/release/v3.8.51, confirmed withgit diff --statshowing no changes to that file).node scripts/check/check-complexity.mjs— OK, 2824 violations (baseline 3218).node scripts/check/check-cognitive-complexity.mjs— OK, 1276 violations (baseline 1437).node scripts/check/check-test-discovery.mjs— OK, new test file discovered, no new orphans.Existing tests aligned
None needed alignment —
tests/unit/proxy-assigned-unavailable-6246.test.ts(the purehasBlockingProxyAssignmentpredicate test) and a focused batch of 11 existingtokenHealthCheck/token-refresh test files (60 tests total) all pass unchangedagainst the fixed code.
Credit
Thanks to @elielsousa-pathbit for the precise write-up in #13470 — the root-cause
citations (line numbers in
tokenRefresh.ts,tokenHealthCheck.ts,proxies/rotation.ts,proxies/guards.ts) all checked out exactly as reported.