Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 28 additions & 5 deletions open-sse/services/tokenRefresh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,22 @@ function getRefreshCacheKey(provider, refreshToken) {
return `${provider}:${tokenHash}`;
}

function buildClaudeRefreshRequestMeta(refreshToken, headers, body, hasProxyConfig) {
return {
method: "POST",
url: OAUTH_ENDPOINTS.anthropic.token,
contentType: headers["Content-Type"],
accept: headers.Accept,
anthropicBeta: headers["anthropic-beta"] || null,
bodyFormat:
headers["Content-Type"] === "application/x-www-form-urlencoded" ? "form-urlencoded" : "json",
bodyKeys: body instanceof URLSearchParams ? Array.from(body.keys()) : Object.keys(body || {}),
clientId: PROVIDERS.claude.clientId || null,
refreshTokenLength: typeof refreshToken === "string" ? refreshToken.length : 0,
hasProxyConfig: hasProxyConfig,

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasProxyConfig is populated with the full proxyConfig object (passed as the hasProxyConfig argument). Proxy config objects can include username/password, so this risks leaking proxy credentials into logs and also doesn’t match the field name (implies a boolean). Consider logging a boolean (e.g. !!proxyConfig) or a redacted proxy label (host:port only) instead of the raw object.

Suggested change
hasProxyConfig: hasProxyConfig,
hasProxyConfig: !!hasProxyConfig,

Copilot uses AI. Check for mistakes.
};
}

/**
* Refresh OAuth access token using refresh token
*/
Expand Down Expand Up @@ -222,20 +238,27 @@ export async function refreshKimiCodingToken(refreshToken, log, proxyConfig = nu
*/
export async function refreshClaudeOAuthToken(refreshToken, log, proxyConfig = null) {
try {
const headers = {
"Content-Type": "application/x-www-form-urlencoded",
Accept: "application/json",
"anthropic-beta": "oauth-2025-04-20",
};
const params = new URLSearchParams({
grant_type: "refresh_token",
refresh_token: refreshToken,
client_id: PROVIDERS.claude.clientId,
});

log?.warn?.(
"TOKEN_REFRESH",
"Claude OAuth refresh request details",
buildClaudeRefreshRequestMeta(refreshToken, headers, params, proxyConfig)
);
Comment on lines +252 to +256

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logs Claude refresh request details at warn level on every refresh attempt, including successful refreshes. That will likely create warning noise and may trigger alerting; it also makes it hard to distinguish actual warning conditions. Consider downgrading this to debug/info, and/or gating it behind an env flag so it can be enabled when diagnosing contract issues.

Copilot uses AI. Check for mistakes.

const response = await runWithProxyContext(proxyConfig, () =>
fetch(OAUTH_ENDPOINTS.anthropic.token, {
method: "POST",
headers: {
"Content-Type": "application/x-www-form-urlencoded",
Accept: "application/json",
"anthropic-beta": "oauth-2025-04-20",
},
headers,
body: params.toString(),
})
);
Expand Down
20 changes: 18 additions & 2 deletions tests/unit/token-refresh-service.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ test("refreshKimiCodingToken adds provider-specific headers and fields", async (
assert.match(bodyToString(calls[0].options.body), /grant_type=refresh_token/);
});

test("refreshClaudeOAuthToken matches upstream Claude refresh contract", async () => {
test("refreshClaudeOAuthToken matches OmniRoute Claude refresh contract and emits redacted request details", async () => {
const log = createLog();
const calls = [];

Expand Down Expand Up @@ -310,6 +310,22 @@ test("refreshClaudeOAuthToken matches upstream Claude refresh contract", async (
bodyToString(calls[0].options.body),
`grant_type=refresh_token&refresh_token=claude-refresh&client_id=${encodeURIComponent(PROVIDERS.claude.clientId)}`
);

const detailLog = log.entries.find(
(entry) => entry.level === "warn" && entry.message === "Claude OAuth refresh request details"
);
assert.deepEqual(detailLog?.meta, {
method: "POST",
url: OAUTH_ENDPOINTS.anthropic.token,
contentType: "application/x-www-form-urlencoded",
accept: "application/json",
anthropicBeta: "oauth-2025-04-20",
bodyFormat: "form-urlencoded",
bodyKeys: ["grant_type", "refresh_token", "client_id"],
clientId: PROVIDERS.claude.clientId || null,
refreshTokenLength: "claude-refresh".length,
hasProxyConfig: null,
});
});

test("refreshClaudeOAuthToken returns null on invalid Claude refresh contract rejection", async () => {
Expand Down Expand Up @@ -338,7 +354,7 @@ test("refreshClaudeOAuthToken returns null on invalid Claude refresh contract re
assert.equal(calls[0].options.headers["anthropic-beta"], "oauth-2025-04-20");
assert.equal(
log.entries.some((entry) => entry.level === "warn"),

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion log.entries.some((entry) => entry.level === "warn") === true is very broad and now effectively couples this test to “some warning happened”, which can become brittle as other warnings are added. Prefer asserting the specific expected warning entry (e.g., message "Claude OAuth refresh request details") rather than any warn-level log.

Suggested change
log.entries.some((entry) => entry.level === "warn"),
log.entries.some(
(entry) => entry.level === "warn" && entry.message === "Claude OAuth refresh request details"
),

Copilot uses AI. Check for mistakes.
false
true
);
assert.equal(
log.entries.some(
Expand Down
Loading