fix(sdk): scope endpoint base-url state per instance to harden jwks host - #118
Conversation
Endpoint base URLs live in static class state that DescopeSDK overwrites for every instance in the process, so a later instance could redirect an earlier one's JWKS fetch or route a management request (carrying projectId:mgmtKey) to an attacker-controlled host. Move the security-critical host resolution off the shared statics: - Add EndpointsV1::resolveBaseUrl(), a pure helper that derives a base URL without mutating static state. - SDKConfig builds the JWKS URL from the instance baseUrl instead of the EndpointsV2 static, so a sibling instance can no longer hijack it. - API resolves and pins its own baseUrl; bearer credentials (management key or refresh token) are never sent to a host other than the configured one. Bare-projectId requests stay unrestricted. - Extractor validates the JWT iss against the configured project ID. - Add a two-instance regression test covering JWKS host isolation and the credential-host guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🐕 Review complete — View session on Shuni Portal 🐾 |
🐕 Suggested ReviewersI prioritized recent contributors who have worked across different key files involved in the change, ensuring coverage of core SDK components and testing to validate the security fix effectively.
Suggested by Shuni based on git history and PR context. Names are not @-mentioned to avoid notifying anyone — request a review from whoever fits best. |
![shuni-bot-dev[bot]](https://avatars.githubusercontent.com/u/97479186?s=60&v=4){kind=small}
There was a problem hiding this comment.
🐕 Shuni's Review
Scopes endpoint base-URL resolution per instance: JWKS fetch now uses the instance baseUrl, API pins its host and refuses bearer credentials to foreign hosts, and Extractor validates the JWT iss against the configured project.
No blocking issues found — good bones! 🦴
Traced all three new guards: the JWKS URL is byte-equivalent to the removed EndpointsV2::getPublicKeyPath().'/'.$projectId, the credential-host check is applied consistently across doGet/doPost/doDelete, and the issuer extraction mirrors the existing adjustProperties logic. The acknowledged tradeoff (multi-instance management calls fail-closed since EndpointsV1/MgmtV1 paths stay static) is the safe direction and doesn't affect single-instance usage. Woof!
2 participants
Fixes descope/etc#16631
View workflow run
Done. Fixed the JWKS-host-hijack / credential-routing hardening in
descope/descope-php.Root cause: endpoint base URLs live in static class state that
DescopeSDK::__constructoverwrites per instance, so a later instance could redirect an earlier one's JWKS fetch or route a management bearer to an attacker host.Change (
f17a21b, +96/−5):EndpointsV1::resolveBaseUrl()— pure helper, no static mutation.SDKConfig::fetchJWKSets()now builds the JWKS URL from the instance baseUrl, not theEndpointsV2static → sibling instances can't hijack it, and the project-scoped cache (PR feat(security): NullCache fallback disables JWKS caching entirely when APCu unavailable #86) becomes effective.APIresolves and pins its own baseUrl; bearer secrets (mgmt key / refresh token) are refused to any host other than the configured one. Bare-projectId requests stay unrestricted.Extractorvalidates JWTissagainst the configured project ID.StaticStateIsolationTest— two-instance regression (JWKS host isolation + credential-host guard).Tests: 54/54 + 29/29 green. No new lint.
Scoped, not the full per-instance rewrite — skipped: moving every
EndpointsV1/MgmtV1auth-path static to instance state (large, risky, and the non-JWKS vectors need the same unrealistic attacker-controls-baseUrlprecondition). The static auth paths are left intact since the credential-host pin inAPIneutralizes the actual leak regardless. Add the full migration when an SDK consumer genuinely needs concurrent multi-cluster instances in one process.Created by Shuni 🐕