anthropics / anthropics/claude-code-action

exchangeForAppToken parses error responses as JSON, masking non-JSON failures and skipping workflow-validation skip detection

オープン
#1,764 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る
bug p3
主要言語
TypeScript
スター
8.9k
フォーク
2.1k
PR マージ指標
PR 指標を取得中

説明

**Type:** bug (error handling)
**Severity:** medium
**Area:** `src/github/token.ts`
**Effort:** trivial

## Summary

On a non-OK response the token exchange unconditionally parses the body as JSON.
If the body is not JSON - an HTML error page from a corporate proxy, an empty
`502`/`504`, a rate-limit page - `response.json()` throws a `SyntaxError`. That
`SyntaxError` replaces the real failure, so the HTTP status is never reported and
the retry/skip classification never runs.

## Affected code

`src/github/token.ts:125-138`

```ts
if (!response.ok) {
const responseJson =
(await response.json()) as AppTokenExchangeErrorResponse; // <-- can throw

if (isWorkflowValidationError(response.status, responseJson)) { ... }

const message = getAppTokenExchangeErrorMessage(responseJson);
console.error(
`App token exchange failed: ${response.status} ${response.statusText} - ${message}`,
);
throw new Error(message);
}
```

## Impact

1. **Diagnostics are lost.** The user sees
`Unexpected token '<', "..." is not valid JSON` instead of
`App token exchange failed: 502 Bad Gateway`. The status code and
`statusText` - the only actionable information - are never logged.
2. **`WorkflowValidationSkipError` detection is skipped.** `isWorkflowValidationError`
never gets a chance to run, so a workflow-validation `401` served with a
non-JSON body is treated as a hard failure instead of the documented graceful
skip (`run.ts` sets `skipped_due_to_workflow_validation_mismatch`).
3. **Retry semantics change.** `retryWithBackoff`'s `shouldRetry` only
special-cases `WorkflowValidationSkipError`, so the `SyntaxError` is retried
three times with backoff against an endpoint that will keep returning HTML.

## Suggested fix

Read the body once as text and parse defensively:

```ts
if (!response.ok) {
const rawBody = await response.text();
let responseJson: AppTokenExchangeErrorResponse = {};
try {
responseJson = JSON.parse(rawBody) as AppTokenExchangeErrorResponse;
} catch {
// Non-JSON error body (proxy HTML page, empty 5xx). Fall back to the
// status line, which is the only thing worth reporting.
responseJson = { message: rawBody.slice(0, 500) || response.statusText };
}

if (isWorkflowValidationError(response.status, responseJson)) { ... }
...
}
```

Also consider including `response.status` in the thrown `Error` message - right
now only the `console.error` line carries it, and that line is skipped whenever
the parse throws.

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。