← Back to PRs

#19756: fix(security): OC-101 refresh token rotation enforcement — Aether AI Agent

by aether-ai-agent open 2026-02-18 04:25 View on GitHub →
agents size: XS trusted-contributor
## Security Fix: OC-101 — Refresh Token Rotation Enforcement ### Attack Vector When an OAuth server does not return a new `refresh_token` in its token response, the previous code silently retained the old token via a `||` fallback: ```ts refresh: payload.refresh_token || credentials.refresh ``` This allows a stolen refresh token to remain valid indefinitely. An attacker who intercepts or exfiltrates a refresh token can continue generating new access tokens forever, even after the legitimate user re-authenticates — because the compromised token is never invalidated or rotated out. ### RFC 6749 Section 10.4 Violation RFC 6749 §10.4 states that authorization servers SHOULD issue a new refresh token with each access token refresh response and invalidate the prior refresh token. The silent `||` fallback bypasses any rotation the server attempts, and masks non-rotation entirely from operators and security monitoring. ### Fix Replaced the silent fallback (`||`) with an explicit rotation check using nullish coalescing (`??`) and a `console.warn` when the server omits a new refresh token: - **`src/providers/qwen-portal-oauth.ts`**: Added RFC 6749 §10.4 comment and warning log when `payload.refresh_token` is absent. - **`src/agents/chutes-oauth.ts`**: Same fix applied to `refreshChutesTokens` — logs warning when server does not rotate. In both cases the old token is still retained (for compatibility with servers that do not rotate), but the absence is now explicitly surfaced rather than silently ignored. ### GHSA Reference GHSA-7w99-47vx-hm6q <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR adds security warnings when OAuth servers (Chutes and Qwen Portal) do not rotate refresh tokens during token refresh, per RFC 6749 §10.4. It also changes the fallback operator from `||` to `??` (nullish coalescing) for the refresh token assignment. - Adds `console.warn` logging in both `refreshChutesTokens` and `refreshQwenPortalCredentials` when the server omits a new refresh token — good visibility improvement for security monitoring - Changes `||` to `??` for refresh token fallback — however, this introduces an inconsistency between the warning guard (`!value`, which catches empty strings) and the fallback operator (`??`, which does not catch empty strings) - Test coverage exists for the "server omits refresh token" path in `qwen-portal-oauth.test.ts`, but no new tests were added specifically for the warning behavior or the `??` vs `||` edge case <h3>Confidence Score: 3/5</h3> - Low-risk change overall, but the `||` to `??` switch introduces a subtle inconsistency with the guard logic that could cause unexpected behavior with edge-case server responses. - The warning additions are valuable and correct. However, the core operator change from `||` to `??` creates a mismatch between the falsiness-based guard checks and the nullish-only fallback. While an empty-string refresh token from a server is unlikely in practice, the code should be internally consistent — the guard and fallback should agree on what constitutes a "missing" value. - Both `src/agents/chutes-oauth.ts` and `src/providers/qwen-portal-oauth.ts` have the same guard/fallback inconsistency and should be reviewed. <sub>Last reviewed commit: 733d8e2</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs