fix(auth): prevent synchronous re-entrant recursion in compareOAuthExpiry - #2719
fix(auth): prevent synchronous re-entrant recursion in compareOAuthExpiry#2719cs-raj wants to merge 3 commits into
Conversation
…piry
The oauthRefreshInFlight mutex was assigned via an async IIFE:
this.oauthRefreshInFlight = (async () => { ... })();
In JavaScript the IIFE body executes synchronously before the assignment
completes. The full chain compareOAuthExpiry → refreshToken → initSDK →
managementSDKClient → createAPIClient → compareOAuthExpiry has no await
yields, so the second call sees oauthRefreshInFlight as null and starts
another refresh cycle — leading to unbounded recursion and a RangeError:
Maximum call stack size exceeded.
Fix 1: replace the IIFE assignment with new Promise so the guard is set
synchronously before any nested code can re-enter compareOAuthExpiry.
Fix 2: add skipTokenValidity: true in initSDK so createAPIClient does not
call compareOAuthExpiry again (which would deadlock on the in-flight
promise while that promise is waiting for initSDK to complete).
The outer management client used by commands is created after
compareOAuthExpiry resolves, so it always receives the fresh access token.
Concurrent async callers and the login flow are unaffected.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
🔒 Security Scan Results
⏱️ SLA Breach Summary
ℹ️ Vulnerabilities Without Available Fixes (Informational Only)The following vulnerabilities were detected but do not have fixes available (no upgrade or patch). These are excluded from failure thresholds:
Consider reviewing these vulnerabilities when fixes become available. |
🔒 Security Scan Results
⏱️ SLA Breach Summary
ℹ️ Vulnerabilities Without Available Fixes (Informational Only)The following vulnerabilities were detected but do not have fixes available (no upgrade or patch). These are excluded from failure thresholds:
Consider reviewing these vulnerabilities when fixes become available. |
The 401 branch was recursing with the same stale error and an unincremented counter — allowing unbounded retries. Add the same maxRetryCount guard used by the 429/408 branch: attempt one token refresh, then print a clear error and exit if the 401 persists. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🔒 Security Scan Results
⏱️ SLA Breach Summary
ℹ️ Vulnerabilities Without Available Fixes (Informational Only)The following vulnerabilities were detected but do not have fixes available (no upgrade or patch). These are excluded from failure thresholds:
Consider reviewing these vulnerabilities when fixes become available. |
✅ QA Verification — all test scenarios passedTested this PR end-to-end on a local monorepo build (
Scenarios 1, 4, and 5 are the ones that crashed with Also confirmed the mechanism in code: the Good from QA side 👍 |
3 participants
Problem
When a user is authenticated via OAuth and their access token has expired (after 59+ minutes idle), running any `csdx` command that initialises the management SDK triggers an unbounded recursive loop that crashes the process with `RangeError: Maximum call stack size exceeded`.
The call chain is entirely synchronous:
```
compareOAuthExpiry()
→ refreshToken()
→ initSDK()
→ managementSDKClient()
→ createAPIClient()
→ compareOAuthExpiry() ← re-enters before guard is set
→ refreshToken()
→ ... ← infinite recursion → stack overflow
```
Root cause
`compareOAuthExpiry` used a mutex (`oauthRefreshInFlight`) to prevent duplicate refreshes, but the guard was assigned using:
```typescript
this.oauthRefreshInFlight = (async () => {
// ...
})();
```
In JavaScript, the async IIFE's synchronous body executes before the assignment completes. The entire call chain from `compareOAuthExpiry → refreshToken → initSDK → managementSDKClient → createAPIClient → compareOAuthExpiry` is synchronous — zero `await` points yield before the second call. By the time the second `compareOAuthExpiry` checks the guard, it is still `null`. The mutex never fires.
Fix
Three changes in this PR:
1. Set the guard before the async work starts (`auth-handler.ts`)
Replace the IIFE self-assignment with a `new Promise` whose executor runs synchronously, assigning `oauthRefreshInFlight` before any nested code can re-enter:
```typescript
let _resolve: () => void;
let _reject: (err: unknown) => void;
this.oauthRefreshInFlight = new Promise((res, rej) => {
_resolve = res;
_reject = rej;
});
(async () => {
try {
await this.refreshToken();
_resolve();
} catch (error) {
_reject(error);
} finally {
this.oauthRefreshInFlight = null;
}
})();
```
2. Skip token validity check inside `initSDK` (`auth-handler.ts`)
`initSDK` calls `managementSDKClient` only to obtain an `oauthHandler` instance — it does not need a valid access token at construction time. Adding `skipTokenValidity: true` prevents `createAPIClient` from calling `compareOAuthExpiry` again, which would otherwise deadlock (the inner call would await the same in-flight promise that is waiting for `initSDK` to complete).
```typescript
this.managementAPIClient = await managementSDKClient({ host, skipTokenValidity: true });
```
The outer management client created by the command is built after `compareOAuthExpiry` resolves, so it always receives the fresh token.
3. Cap 401 retry count in `refreshAccessToken` (`authentication-handler.ts`)
The 401 branch was recursing with the same stale error object and an unincremented counter, allowing unbounded retries. Added the same `maxRetryCount` guard used by the 429/408 branch: attempt one token refresh, then print a clear error and exit if the 401 persists.
What is not affected
Tests
Added a regression test that simulates the synchronous re-entrant call pattern — `refreshToken` stub calls `compareOAuthExpiry` synchronously, verifying the guard is already set and that `refreshToken` and the "Token expired" print each happen exactly once.