fix(proxy): resolve registry assignments for combo and key levels - #3048
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the proxy settings API route to consolidate and simplify proxy assignment retrieval across different levels (global, provider, combo, and key) using helper functions, and adds corresponding unit tests for combo and key registry assignments. Feedback was provided to address a potential prototype lookup vulnerability in the getRegistryScopeForLevel function by safely checking if the key exists on the mapping object before performing the lookup.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function getRegistryScopeForLevel(level: string) { | ||
| return PROXY_LEVEL_TO_REGISTRY_SCOPE[ | ||
| level as keyof typeof PROXY_LEVEL_TO_REGISTRY_SCOPE | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Using a direct lookup on PROXY_LEVEL_TO_REGISTRY_SCOPE with level as keyof typeof PROXY_LEVEL_TO_REGISTRY_SCOPE can lead to prototype lookup issues if level matches built-in object properties (e.g., "toString", "constructor", "valueOf"). If a client passes ?level=toString, the lookup will return a function instead of undefined, which could cause runtime errors or unexpected behavior when passed to downstream database queries.
To prevent this, safely check if the key exists on the object using Object.prototype.hasOwnProperty.call before performing the lookup.
function getRegistryScopeForLevel(level: string): "global" | "provider" | "combo" | "account" | undefined {
if (Object.prototype.hasOwnProperty.call(PROXY_LEVEL_TO_REGISTRY_SCOPE, level)) {
return PROXY_LEVEL_TO_REGISTRY_SCOPE[
level as keyof typeof PROXY_LEVEL_TO_REGISTRY_SCOPE
];
}
return undefined;
}
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The PR refactors the proxy settings route to use the proxy registry for combo and key level proxy resolution, with proper legacy fallback. The changes are well-structured and maintain backward compatibility. Files Reviewed (2 files)
OverviewChanges:
Security & Error Handling:
Test Coverage:
Reviewed by laguna-m.1-20260312:free · 2,823,953 tokens |
|
Validation rerun:
Result: 16 pass, 0 fail. |
…add contributor hall Audited all 687 commits / 60 release/v3.8.8 PRs since v3.8.7 against the CHANGELOG: - Added 5 missing Fixed entries: #3052 (heap-pressure auto-calibration), #3051/#3048 (proxy fail-closed + registry assignments, @terence71-glitch), #3049/#3046 (session-pool fingerprint rotation + claude-web cf_clearance, @oyi77). - Credited previously-uncredited contributors: @branben (#2958 scope fix, #2959 Notion context source) and @JxnLexn (per-API-key stream default mode). - Added an Added entry for the per-API-key stream default mode feature. - Added the "🏆 Contributors" hall (24 contributors), matching the v3.8.6 format. Maintainer fix-PRs (#2966–#3030) are intentionally referenced by their original issue numbers in the body rather than the fix-PR number; @diegosouzapw is in the hall.
…egosouzapw#3048) * fix(proxy): resolve registry assignments for combo and key levels * fix(proxy): guard registry scope level lookup
…add contributor hall Audited all 687 commits / 60 release/v3.8.8 PRs since v3.8.7 against the CHANGELOG: - Added 5 missing Fixed entries: diegosouzapw#3052 (heap-pressure auto-calibration), diegosouzapw#3051/diegosouzapw#3048 (proxy fail-closed + registry assignments, @terence71-glitch), diegosouzapw#3049/diegosouzapw#3046 (session-pool fingerprint rotation + claude-web cf_clearance, @oyi77). - Credited previously-uncredited contributors: @branben (diegosouzapw#2958 scope fix, diegosouzapw#2959 Notion context source) and @JxnLexn (per-API-key stream default mode). - Added an Added entry for the per-API-key stream default mode feature. - Added the "🏆 Contributors" hall (24 contributors), matching the v3.8.6 format. Maintainer fix-PRs (diegosouzapw#2966–diegosouzapw#3030) are intentionally referenced by their original issue numbers in the body rather than the fix-PR number; @diegosouzapw is in the hall.
…egosouzapw#3048) * fix(proxy): resolve registry assignments for combo and key levels * fix(proxy): guard registry scope level lookup
…add contributor hall Audited all 687 commits / 60 release/v3.8.8 PRs since v3.8.7 against the CHANGELOG: - Added 5 missing Fixed entries: diegosouzapw#3052 (heap-pressure auto-calibration), diegosouzapw#3051/diegosouzapw#3048 (proxy fail-closed + registry assignments, @terence71-glitch), diegosouzapw#3049/diegosouzapw#3046 (session-pool fingerprint rotation + claude-web cf_clearance, @oyi77). - Credited previously-uncredited contributors: @branben (diegosouzapw#2958 scope fix, diegosouzapw#2959 Notion context source) and @JxnLexn (per-API-key stream default mode). - Added an Added entry for the per-API-key stream default mode feature. - Added the "🏆 Contributors" hall (24 contributors), matching the v3.8.6 format. Maintainer fix-PRs (diegosouzapw#2966–diegosouzapw#3030) are intentionally referenced by their original issue numbers in the body rather than the fix-PR number; @diegosouzapw is in the hall.
…egosouzapw#3048) * fix(proxy): resolve registry assignments for combo and key levels * fix(proxy): guard registry scope level lookup
…add contributor hall Audited all 687 commits / 60 release/v3.8.8 PRs since v3.8.7 against the CHANGELOG: - Added 5 missing Fixed entries: diegosouzapw#3052 (heap-pressure auto-calibration), diegosouzapw#3051/diegosouzapw#3048 (proxy fail-closed + registry assignments, @terence71-glitch), diegosouzapw#3049/diegosouzapw#3046 (session-pool fingerprint rotation + claude-web cf_clearance, @oyi77). - Credited previously-uncredited contributors: @branben (diegosouzapw#2958 scope fix, diegosouzapw#2959 Notion context source) and @JxnLexn (per-API-key stream default mode). - Added an Added entry for the per-API-key stream default mode feature. - Added the "🏆 Contributors" hall (24 contributors), matching the v3.8.6 format. Maintainer fix-PRs (diegosouzapw#2966–diegosouzapw#3030) are intentionally referenced by their original issue numbers in the body rather than the fix-PR number; @diegosouzapw is in the hall.
Summary
GET /api/settings/proxy?level=...&id=...previously consulted the proxyregistry only for
globalandproviderlevels. Requests forcomboandkeyfell straight through to the legacygetProxyForLevel()path, so aproxy assigned through the registry UI was invisible to per-level reads at
those two levels — the response showed the old
proxyConfig.combos/proxyConfig.keysvalue (ornull) instead of the registry assignment.This PR routes all four levels through a single registry-lookup helper, with
the legacy store kept as a fallback when no registry assignment exists.
Changes
src/app/api/settings/proxy/route.tsPROXY_LEVEL_TO_REGISTRY_SCOPEmap translating the route's publiclevel names to the canonical
ProxyScopeused bysrc/lib/db/proxies.ts:key -> accountentry mirrorsnormalizeScope()inproxies.ts:157,where
accountis the canonical internal scope andkeyis the legacypublic-API alias.
getRegistryScopeForLevel(level)— typed map lookup.getRegistryProxyForLevel(level, id)— fetches the assignment for thescope, returns the underlying
proxy_registryrow (with secrets), ornullwhen there is no assignment or the level is unknown.toProxyConfig(proxyData)— extracts{ type, host, port, username, password }from a registry row. Previously inlined twice.GEThandler collapses the four per-level branches into one:toProxyConfig()for the provider loop,removing the last inline duplicate.
tests/unit/route-edge-coverage.test.tssettings proxy route resolves combo and key registry assignments with legacy fallback:combos.comboAandkeys.accountAproxies — asserts bothGETs return the legacy values (legacy fallback path).
localDb.createProxy, assigns one to(combo, comboA)and one to(account, accountA)viaassignProxyToScope.proxy hostnames, usernames, and decrypted passwords (registry path).
Behavior contract preserved
global: registry usesassignments[0](there is only ever one row,keyed by
__global__); response forcesid: null.assignment.scopeId === idmatch.mapAssignmentRowonly strips
__global__for the global scope, so combo and accountscopeIds round-trip unchanged.Naming note
The route accepts only
key(notaccount) as the level name on thepublic surface. This matches the docstring, the legacy
proxyConfig.keysmap, and the PUT body's
keysfield. If a caller passes?level=account, the level is unknown to the route, the registry helperreturns
null, and the request falls through to legacygetProxyForLevel("account", ...). This is intentional — keeping thepublic-vs-internal naming boundary intact.
Test plan
node --import tsx/esm --test tests/unit/route-edge-coverage.test.ts— new test passes; existing
prefers proxy registry assignments…test still passes (no regression on global/provider paths).
npm run typecheck:core— clean.npm run lint— 0 errors.