feat: 取得ボディにサイズ上限とタイムアウトを設ける（#56）

CLAUDE.md

@@ -46,7 +46,7 @@ GitHub Actions runs the bundled `dist/index.js` directly (see `action.yml` → `
 Key design seams to preserve when editing:
 
 - **`ReleaseApi` interface (`version.ts`)** is the network seam. `version.ts` holds only pure, testable resolution logic; `github.ts` (`createReleaseApi`) is the **only** module that performs HTTP. Keep network access out of `version.ts` so its tests stay HTTP-free.
-- **Reliability core (`github.ts` `fetchJson`)**: rejects non-JSON bodies as errors. This is the project's reason for existing — unauthenticated GitHub returns HTML rate-limit pages that broke `arduino/setup-task`. Treat that case as *transient* (retryable) and surface a message pointing at `repo-token`.
+- **Reliability core (`github.ts` `fetchJson`)**: rejects non-JSON bodies as errors. This is the project's reason for existing — unauthenticated GitHub returns HTML rate-limit pages that broke `arduino/setup-task`. Treat that case as *transient* (retryable) and surface a message pointing at `repo-token`. Bodies are read via `readCappedText` (byte cap, `MAX_RESPONSE_BYTES`) and every request carries an `AbortSignal.timeout` (`REQUEST_TIMEOUT_MS`) so a hijacked/hung trusted host can't exhaust memory or wedge the action (NFR-1) — a size overrun is permanent, a timeout transient.
 - **Retry vs. permanent (`download.ts` `withRetry` + `errors.ts` `PermanentError`)**: `withRetry` does exponential backoff, but `PermanentError` (and HTTP 404, including tool-cache's `httpStatusCode === 404`) bail out immediately with no retry. When adding a new failure mode, decide deliberately which side it falls on — checksum mismatches and 404s are permanent; everything network-ish is transient.
 - **Platform mapping (`platform.ts`)**: maps Node `process.platform`/`process.arch` to go-task's asset naming (`task_<os>_<arch>.<ext>`) and validates against a hardcoded `SUPPORTED` matrix mirroring go-task's published assets. Update `SUPPORTED` if upstream adds/drops a target.
 - **`constants.ts`** centralizes the upstream repo (`go-task/task`), the `task_checksums.txt` asset name, URL builders, and retry tuning.

TODO.md

@@ -53,7 +53,6 @@
 | [#41](https://github.com/yk-lab/setup-task/issues/41) | `[test]` platform.test.ts を §9 全 os/arch 組合せに拡張 | `P3: low` | `test` |
 | [#44](https://github.com/yk-lab/setup-task/issues/44) | `[enhancement]` フォールバックソースをサポート（FR-11） | `P3: low` | `enhancement` |
 | [#45](https://github.com/yk-lab/setup-task/issues/45) | `[chore]` Biome 導入を評価 | `P3: low` | `chore` |
-| [#56](https://github.com/yk-lab/setup-task/issues/56) | `[security]` 取得ボディにサイズ上限/タイムアウトを設ける | `P3: low` | `security` | |
 
 ### 未 Issue 化
 
@@ -81,3 +80,4 @@ v1.0.0 実装中に作成・解決済みの Issue（参考）。
 | [#54](https://github.com/yk-lab/setup-task/issues/54) | `[enhancement]` HTTP(S) proxy 環境で全 fetch を proxy 経由にする | #57 |
 | [#55](https://github.com/yk-lab/setup-task/issues/55) | `[documentation]` テスト規約（CLAUDE.md）を stub-fetch unit test の実態に合わせる | #58 |
 | [#59](https://github.com/yk-lab/setup-task/issues/59) | `[ci]` paths-filter が root 直下の .md を docs 扱いせずフル CI が走る | #60 |
+| [#56](https://github.com/yk-lab/setup-task/issues/56) | `[security]` 取得ボディにサイズ上限/タイムアウトを設ける | #61 |

src/constants.ts

@@ -18,6 +18,14 @@ export const DEFAULT_RETRY_BASE_MS = 1000;
 /** Cap on redirects followed while validating each hop's host (NFR-1). */
 export const MAX_REDIRECTS = 5;
 
+/**
+ * Per-request timeout and response-body size cap (NFR-1): bound how long a fetch
+ * may hang and how much a (possibly hijacked) trusted host may stream. Both are
+ * generous — the API pages and `task_checksums.txt` sit far below the cap.
+ */
+export const REQUEST_TIMEOUT_MS = 30_000;
+export const MAX_RESPONSE_BYTES = 16 * 1024 * 1024;
+
 /**
  * Build the download URL for a release asset.
  * Tags on go-task are `v`-prefixed (e.g. v3.51.1).

src/download.ts

@@ -59,20 +59,28 @@ export async function withRetry<T>(fn: () => Promise<T>, opts: RetryOptions = {}
 export async function downloadAsset(url: string, token?: string): Promise<string> {
   // Vet the redirect chain's hosts before tool-cache (which follows redirects
   // opaquely) fetches the body (NFR-1). Only an untrusted host (PermanentError)
-  // is fatal; a transient network failure in the preflight falls through to the
+  // is fatal; a preflight that simply couldn't complete falls through to the
   // checksum-verified tool-cache download rather than blocking it.
   try {
     await assertRedirectTrusted(url, token);
   } catch (err) {
     if (err instanceof PermanentError) {
       throw err;
     }
-    // Only a failed fetch (TypeError) is tolerated; anything else is unexpected.
-    if (!(err instanceof TypeError)) {
+    // "Couldn't run" = a failed fetch (TypeError) or a timeout (DOMException);
+    // anything else is unexpected.
+    const couldNotRun =
+      err instanceof TypeError ||
+      (err instanceof DOMException && (err.name === 'AbortError' || err.name === 'TimeoutError'));
+    if (!couldNotRun) {
       throw err;
     }
-    core.debug(`Redirect preflight skipped (network/proxy failure): ${errorMessage(err)}`);
+    core.debug(`Redirect preflight skipped (${errorMessage(err)}); proceeding to tool-cache.`);
   }
+  // tool-cache's downloader exposes no AbortSignal/size limit, and we keep it for
+  // its proxy support (#54); the binary transfer is bounded only by the job
+  // timeout (hangs) and runner disk (size), and SHA256 verification (FR-5)
+  // rejects a tampered/oversized payload before it is cached.
   const auth = token ? `Bearer ${token}` : undefined;
   return tc.downloadTool(url, undefined, auth);
 }

src/github.ts

@@ -1,5 +1,12 @@
 import * as semver from 'semver';
-import { GITHUB_API, MAX_REDIRECTS, REPO_NAME, REPO_OWNER } from './constants';
+import {
+  GITHUB_API,
+  MAX_REDIRECTS,
+  MAX_RESPONSE_BYTES,
+  REPO_NAME,
+  REPO_OWNER,
+  REQUEST_TIMEOUT_MS,
+} from './constants';
 import { PermanentError } from './errors';
 import { fetch } from './fetch';
 import { assertAllowedHost } from './url-guard';
@@ -41,10 +48,14 @@ function withoutAuth(headers: Record<string, string>): Record<string, string> {
 export async function secureFetch(url: string, headers: Record<string, string>): Promise<Response> {
   let currentUrl = url;
   let currentHeaders = headers;
+  // One deadline for the whole operation — redirects plus the final body read
+  // (the returned response's body stays bound to this signal). A hang aborts
+  // and surfaces as a transient error withRetry can retry (NFR-1).
+  const signal = AbortSignal.timeout(REQUEST_TIMEOUT_MS);
 
   for (let hop = 0; hop <= MAX_REDIRECTS; hop++) {
     assertAllowedHost(currentUrl, 'request');
-    const resp = await fetch(currentUrl, { headers: currentHeaders, redirect: 'manual' });
+    const resp = await fetch(currentUrl, { headers: currentHeaders, redirect: 'manual', signal });
 
     if (!REDIRECT_STATUSES.has(resp.status)) {
       return resp;
@@ -121,18 +132,51 @@ function guardContentType(resp: Response, url: string, isExpected: (contentType:
   }
 }
 
+/**
+ * Read a response body as UTF-8 text, refusing bodies over `maxBytes` (NFR-1):
+ * `resp.text()` buffers an unbounded stream, so a hijacked trusted host could
+ * exhaust memory. Count bytes as they arrive and bail (PermanentError — retrying
+ * just re-streams the same oversized body).
+ */
+export async function readCappedText(
+  resp: Response,
+  url: string,
+  maxBytes: number = MAX_RESPONSE_BYTES,
+): Promise<string> {
+  if (!resp.body) {
+    return '';
+  }
+  const reader = resp.body.getReader();
+  const chunks: Uint8Array[] = [];
+  let total = 0;
+  for (;;) {
+    const { done, value } = await reader.read();
+    if (done) {
+      break;
+    }
+    total += value.byteLength;
+    if (total > maxBytes) {
+      await reader.cancel().catch(() => undefined); // don't let a cancel error mask the limit
+      throw new PermanentError(`Response from ${url} exceeded the ${maxBytes}-byte limit.`);
+    }
+    chunks.push(value);
+  }
+  return Buffer.concat(chunks).toString('utf-8');
+}
+
 /** Fetch and parse JSON, requiring a JSON content-type (FR-3). */
 export async function fetchJson<T>(url: string, token?: string): Promise<T> {
   const resp = await fetchOk(url, token);
   guardContentType(resp, url, (contentType) => contentType.includes('json'));
-  return (await resp.json()) as T;
+  // Strip a leading BOM the way resp.json() did — JSON.parse rejects it.
+  return JSON.parse((await readCappedText(resp, url)).replace(/^\uFEFF/, '')) as T;
 }
 
 /** Fetch a text body (the checksums file), rejecting HTML error pages (FR-3). */
 export async function fetchText(url: string, token?: string): Promise<string> {
   const resp = await fetchOk(url, token);
   guardContentType(resp, url, (contentType) => !contentType.includes('html'));
-  return resp.text();
+  return readCappedText(resp, url);
 }
 
 /** GitHub-backed ReleaseApi implementation. */

tests/download.test.ts

@@ -104,6 +104,16 @@ describe('downloadAsset (redirect preflight wiring, NFR-1)', () => {
     expect(tc.downloadTool).toHaveBeenCalledOnce();
   });
 
+  it('falls through to tool-cache on a preflight timeout (DOMException)', async () => {
+    // AbortSignal.timeout rejects with a DOMException, not a TypeError — it must
+    // also be treated as "preflight couldn't run", not block the download.
+    vi.mocked(assertRedirectTrusted).mockRejectedValueOnce(
+      new DOMException('The operation timed out', 'TimeoutError'),
+    );
+    await expect(downloadAsset(ASSET_URL, 'tok')).resolves.toBe('/tmp/task-archive');
+    expect(tc.downloadTool).toHaveBeenCalledOnce();
+  });
+
   it('propagates an unexpected (non-network) preflight error', async () => {
     vi.mocked(assertRedirectTrusted).mockRejectedValueOnce(new Error('unexpected'));
     await expect(downloadAsset(ASSET_URL, 'tok')).rejects.toThrow('unexpected');

tests/github.test.ts

@@ -6,6 +6,7 @@ import {
   createReleaseApi,
   fetchJson,
   fetchText,
+  readCappedText,
   secureFetch,
 } from '../src/github';
 
@@ -48,6 +49,11 @@ describe('fetchJson (content-type guard, FR-3)', () => {
     await expect(fetchJson(API_URL)).resolves.toEqual({ tag_name: 'v3.51.1' });
   });
 
+  it('strips a leading BOM before parsing (as resp.json() did)', async () => {
+    stubFetch(`\uFEFF${JSON.stringify({ tag_name: 'v3.51.1' })}`, { contentType: 'application/json' });
+    await expect(fetchJson(API_URL)).resolves.toEqual({ tag_name: 'v3.51.1' });
+  });
+
   it('rejects an HTML rate-limit page as a retryable Error pointing at repo-token', async () => {
     // 200 OK but non-JSON body = the "Unicorn" page. Must throw (transient),
     // not be parsed, so withRetry() can retry it.
@@ -166,6 +172,28 @@ describe('secureFetch (redirect host validation, NFR-1)', () => {
     mockedFetch.mockImplementation(async () => redirectTo('https://evil.example.com/asset'));
     await expect(assertRedirectTrusted(ASSET_URL, 'tok')).rejects.toBeInstanceOf(PermanentError);
   });
+
+  it('passes an abort signal (per-request timeout) to fetch (NFR-1)', async () => {
+    mockedFetch.mockResolvedValueOnce(new Response('ok', { status: 200 }));
+    await secureFetch(CHECKSUMS_URL, {});
+    expect(mockedFetch).toHaveBeenCalledWith(
+      CHECKSUMS_URL,
+      expect.objectContaining({ signal: expect.any(AbortSignal) }),
+    );
+  });
+});
+
+describe('readCappedText (response size cap, NFR-1)', () => {
+  it('returns the body when under the cap', async () => {
+    await expect(readCappedText(new Response('hello'), CHECKSUMS_URL, 100)).resolves.toBe('hello');
+  });
+
+  it('throws PermanentError when the streamed body exceeds the cap', async () => {
+    // The streamed byte counter is the defence against an unbounded/hijacked body.
+    await expect(
+      readCappedText(new Response('abcdefghij'), CHECKSUMS_URL, 4),
+    ).rejects.toBeInstanceOf(PermanentError);
+  });
 });
 
 describe('createReleaseApi', () => {