Skip to content

fix: セキュリティレビュー指摘の対応 - #2

Closed
laiso wants to merge 1 commit into
mainfrom
fix/security-review-followup
Closed

fix: セキュリティレビュー指摘の対応#2
laiso wants to merge 1 commit into
mainfrom
fix/security-review-followup

Conversation

@laiso

@laiso laiso commented May 1, 2026

Copy link
Copy Markdown
Owner

外部セキュリティレビューで上がった妥当な 5 件を 1 PR にまとめる。CI 通過確認用の intra-fork PR。

修正内容

Webhook constant-time 比較 (HIGH)

`server/index.js` で `!==` を `crypto.timingSafeEqual` に置換。長さ違いを先に弾いて等長保証。

iOS Bundle ID typo 修正 (MEDIUM)

`com.exmaple.` → `com.example.` を `project.pbxproj` の 4 箇所 (Debug/Release × app/tests) で置換。

CORS 既定の警告ログ (MEDIUM)

`ALLOWED_ORIGINS` 未設定時に起動ログで警告。動作は維持。

quantity 入力バリデーション (LOW)

整数 / 1-100 範囲チェックを追加。負数 / 小数 / 巨大値が PAY.JP API に素通りしないように。

README 整合 (LOW)

Node.js 18 → 22 (CI 検証バージョン)、CORS 既定挙動の注意書きを追加。

レビュー指摘から除外したもの

詳しくは plan に記載。要点:

  • iOS deployment target 26.1 → 2026-05 時点で iOS 26 は実在 (誤検出)
  • `sk_test_*` を Critical 扱い → テストキーで実害なし
  • `local.properties` tracked → 既に `.gitignore` でカバー済み

Test plan

  • `node --check server/index.js` 通過
  • CORS 警告が起動時に出る
  • webhook 異トークン (異長 / 同長) / ヘッダ無し → 401, 正トークン → 200
  • quantity 0 / -1 / 1.5 / 999 → 400, 1 → API 到達
  • iOS `exmaple` の grep 結果 0 件
  • CI 全 5 ジョブ green

外部レビューで上がった妥当な 5 件をまとめて対応:

- server: webhook トークン比較を crypto.timingSafeEqual に変更
  (タイミング攻撃耐性)
- server: quantity に整数 / 1-100 範囲のバリデーションを追加
- server: ALLOWED_ORIGINS 未設定時に警告ログを出力
- ios: Bundle ID typo "exmaple" を "example" に修正 (4 箇所)
- README: Node.js 版数を CI 整合 (18 → 22)、CORS 既定の注意書きを追加

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@laiso

laiso commented May 1, 2026

Copy link
Copy Markdown
Owner Author

upstream PR を作成のため close。

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.

1 participant