anthropics / anthropics/claude-code-action

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

Đang mở
#1,764 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
bug p3
Ngôn ngữ chính
TypeScript
Star
8.9k
Fork
2.1k
Chỉ số merge pull request
Chỉ số pull request đang chờ

Mô tả

**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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.