anthropics / anthropics/claude-code-action

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

Ouverte
#1,764 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
bug p3
Langage dominant
TypeScript
Étoiles
8.9k
Forks
2.1k
Métriques de merge des PR
Métriques de PR en attente

Description

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

Guide de contribution

Ouvrir le guide de contribution

Évaluation

Cette issue n'a pas encore été évaluée.

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.