Skip to content

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

Merged
yk-lab merged 3 commits into
mainfrom
security/fetch-limits
Jun 28, 2026
Merged

feat: 取得ボディにサイズ上限とタイムアウトを設ける(#56)#61
yk-lab merged 3 commits into
mainfrom
security/fetch-limits

Conversation

@yk-lab

@yk-lab yk-lab commented Jun 28, 2026

Copy link
Copy Markdown
Owner

概要

fetchText / fetchJson は許可ホストのレスポンスを上限なしで読み切り、per-request タイムアウトも無かった。許可ホストでも侵害時に巨大/無限ストリームでメモリ枯渇やハングを招き得るため、fetch 経路に上限を入れる(#56 / NFR-1)。

変更内容

  • constants.ts: REQUEST_TIMEOUT_MS(30s) / MAX_RESPONSE_BYTES(16 MiB)
  • github.ts
    • secureFetch: AbortSignal.timeout を全 hop + 最終 body 読込にかける(タイムアウトは transient で withRetry が再試行)
    • readCappedText: ストリームしながらバイト数を数え、上限超過は PermanentErrorfetchJson/fetchTextresp.json()/resp.text() の代わりに使用
    • fetchJson: 先頭 BOM を除去(resp.json() の挙動を維持)
  • download.ts: preflight の fall-through が timeout(DOMException) も許容するよう拡張。tool-cache のバイナリ転送が未制限な理由(proxy 維持・job/disk backstop・SHA256 検証)をコメント化

レビュー(push 前にローカル実施)

CodeRabbit / codex / opencode / comment-analyzer + /simplify 4エージェントで多面レビューし、以下を反映:

  • Content-Length 事前チェック削除(body cancel 漏れ=ソケットリーク / gzip 不整合を一掃)
  • reader.cancel() を catch(PermanentError の masking 防止)
  • BOM 除去(resp.json() 退行の修正)
  • preflight fall-through に DOMException 追加([enhancement] HTTP(S) proxy 環境で全 fetch を proxy 経由にする #54 の挙動維持)
  • コメント slim 化 + rot 解消

検証

  • pnpm run all ✅(typecheck / lint / 79 tests / build)/ typos ✅ / actionlint
  • codex セカンドオピニオン:correctness 指摘ゼロ

参照

🤖 Generated with Claude Code


Summary by cubic

Add a per-request timeout and response body size cap to all GitHub fetches to prevent hangs and memory exhaustion from trusted hosts. Applies a 30s timeout and a 16 MiB cap; timeouts are retried, size overruns fail permanently. Closes #56.

  • New Features

    • Added REQUEST_TIMEOUT_MS (30s) and MAX_RESPONSE_BYTES (16 MiB) in constants.ts.
    • secureFetch now uses a single AbortSignal.timeout across redirects and body read; retries on timeout via withRetry.
    • Introduced readCappedText to stream and cap bytes; used by fetchJson and fetchText.
  • Bug Fixes

    • fetchJson strips a leading BOM before JSON.parse to match resp.json() behavior.
    • Redirect preflight in download.ts now falls through on DOMException timeouts/aborts (still blocks on untrusted hosts).

Written for commit e0988c9. Summary will update on new commits.

Review in cubic

yk-lab and others added 3 commits June 28, 2026 12:26
許可ホストでも侵害時に巨大/無限ストリームでメモリ枯渇やハングを招き得るため、
fetch 経路に上限を入れる。

- constants.ts: REQUEST_TIMEOUT_MS(30s) / MAX_RESPONSE_BYTES(16MiB)
- github.ts:
  - secureFetch に AbortSignal.timeout を付与(hop + 最終 body 読込をカバー、
    タイムアウトは transient で withRetry が再試行)
  - readCappedText: Content-Length 事前チェック + ストリームしながらバイト数を
    数え、上限超過は PermanentError(再試行しても同じ巨大 body を引くため)。
    fetchJson/fetchText が resp.json()/text() の代わりに使用
- tests: size cap(上限内/超過)+ timeout signal の配線を検証

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AECzAv2vcdATecuvLjBSa6
- CLAUDE.md: Reliability core に readCappedText / AbortSignal.timeout を追記
- TODO.md: #56 を解決済み(#61)へ

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AECzAv2vcdATecuvLjBSa6
- github.ts: Content-Length 事前チェックを削除(streaming のバイト数が真の
  防御。事前チェックは body を cancel せずソケットリーク[CodeRabbit]、gzip 時に
  圧縮長と非圧縮カウントが不整合[opencode]、Number(null) の曖昧さ、と難あり)
- github.ts: over-cap の reader.cancel() を .catch(() => undefined) し、cancel の
  失敗が PermanentError を masking しないように[opencode]
- github.ts: fetchJson で先頭 BOM を除去(resp.json() は除去していたが JSON.parse
  は失敗=退行)[opencode]
- github.ts: readCappedText の "Exported for testing" コメントを実態に修正
- download.ts: preflight の fall-through が TypeError のみで、AbortSignal.timeout
  の DOMException を取りこぼし #54 の挙動を壊していたため timeout/abort も許容
  [opencode]。tool-cache のバイナリ転送が未制限な理由(proxy 維持・job/ディスク
  backstop・SHA256 検証)をコメント化[codex/altitude]
- tests: BOM 除去 / preflight timeout フォールスルー を追加

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AECzAv2vcdATecuvLjBSa6
@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: dd4d4e9b-7227-43f0-8cbd-6673816d6173

📥 Commits

Reviewing files that changed from the base of the PR and between 33f0ea3 and e0988c9.

📒 Files selected for processing (7)
  • CLAUDE.md
  • TODO.md
  • src/constants.ts
  • src/download.ts
  • src/github.ts
  • tests/download.test.ts
  • tests/github.test.ts

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • ダウンロードやGitHub API取得で、タイムアウトと取得サイズの上限が適用されるようになりました。
    • JSON/テキスト応答の読み取りが制限付きになり、過大なレスポンスで処理が止まりにくくなりました。
  • Bug Fixes

    • レート制限ページや一時的な中断時でも、再試行や代替ダウンロードに正しく切り替わるようになりました。
    • JSON先頭のBOMを含むレスポンスも正しく読み取れるようになりました。

ウォークスルー

src/constants.tsREQUEST_TIMEOUT_MS(30秒)と MAX_RESPONSE_BYTES(16MiB)を追加し、secureFetchAbortSignal.timeout を導入。新関数 readCappedText でストリーミング上限付き読み取りを実装し、fetchJson/fetchText に適用。download.ts の preflight フォールスルー条件を DOMException まで拡張。

変更点

NFR-1: タイムアウトとボディサイズ上限の実装

Layer / File(s) Summary
新定数 REQUEST_TIMEOUT_MS / MAX_RESPONSE_BYTES の追加
src/constants.ts, src/github.ts
NFR-1 制約として2つの定数を追加し、github.ts のインポートを複数行デストラクチャ形式へ変更。
secureFetch への AbortSignal.timeout 導入
src/github.ts, tests/github.test.ts
リダイレクト追跡ループ全体を覆う AbortSignal.timeout(REQUEST_TIMEOUT_MS) を生成し、各 fetch 呼び出しに signal を渡すよう変更。テストで AbortSignalfetch に渡ることを検証。
readCappedText と fetchJson/fetchText の上限付き読み取り
src/github.ts, tests/github.test.ts
readCappedText をストリームリーダーでバイト上限付き読み取りとして追加し、上限超過時は PermanentErrorfetchJson は BOM除去付き JSON.parsefetchTextreadCappedText へ切り替え。テストでキャップ超過・BOM除去の挙動を検証。
download.ts preflight の DOMException フォールスルー拡張
src/download.ts, tests/download.test.ts
assertRedirectTrustedcatch でフォールスルーを許容するエラー種別を TypeError から DOMExceptionAbortError / TimeoutError)まで拡張。タイムアウト時も downloadTool へフォールスルーされることをテストで検証。
CLAUDE.md / TODO.md の更新
CLAUDE.md, TODO.md
Architecture セクションに readCappedTextAbortSignal.timeout の挙動説明を追記し、Issue #56 を Backlog から解決済みへ移動。

推定コードレビュー労力

🎯 3 (Moderate) | ⏱️ ~20 minutes

関連する可能性のある PR

  • yk-lab/setup-task#26: fetchJson の content-type ガード(HTMLエラーページ判定)を tests/github.test.ts で検証しており、本 PR が改修する fetchJson の読み取りパスと同一機能を対象としている。
  • yk-lab/setup-task#53: secureFetch のリダイレクト追跡実装と assertRedirectTrusted preflight を改修しており、本 PR が変更する同一領域を対象としている。

🐇 ウサギの歌

でっかいボディを読みすぎないよ、
タイムアウトでハングも防ぐよ。
MAX_RESPONSE_BYTES でキャップを張って、
AbortSignal がコッソリ見張って、
安全・安心、バニーの番! 🎉

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed タイトルはサイズ上限とタイムアウト追加という主変更を正しく要約しています。
Description check ✅ Passed 概要・関連 issue・検証が含まれており、テンプレートに対して大筋で十分です。
Linked Issues check ✅ Passed #56 の要件どおり、ボディ上限・per-attempt タイムアウト・再試行可能な失敗扱いが実装されています。
Out of Scope Changes check ✅ Passed docs、tests、download.ts の変更は本題の fetch 制限と整合しており、明確な逸脱は見当たりません。
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/fetch-limits

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jun 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.80%. Comparing base (33f0ea3) to head (e0988c9).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/github.ts 90.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #61      +/-   ##
==========================================
+ Coverage   70.32%   71.80%   +1.47%     
==========================================
  Files          13       13              
  Lines         246      266      +20     
  Branches       62       67       +5     
==========================================
+ Hits          173      191      +18     
- Misses         64       65       +1     
- Partials        9       10       +1     
Files with missing lines Coverage Δ
src/constants.ts 100.00% <100.00%> (ø)
src/download.ts 90.00% <100.00%> (+0.34%) ⬆️
src/github.ts 91.56% <90.00%> (-0.86%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 7 files

Re-trigger cubic

@yk-lab yk-lab merged commit 91de08a into main Jun 28, 2026
21 checks passed
@yk-lab yk-lab deleted the security/fetch-limits branch June 28, 2026 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[security] 取得ボディにサイズ上限/タイムアウトを設ける

1 participant