Skip to content

レビュー指摘の3件を修正 - #49

Merged
Atsumi3 merged 1 commit into
mainfrom
claude/app-jar-release-output-m2pekn
Aug 24, 2026
Merged

レビュー指摘の3件を修正#49
Atsumi3 merged 1 commit into
mainfrom
claude/app-jar-release-output-m2pekn

Conversation

@Atsumi3

@Atsumi3 Atsumi3 commented Aug 24, 2026

Copy link
Copy Markdown
Member

懐疑的レビューで挙がった指摘のうち、対応すると決めた3件を修正しました。

時刻同期の巻き戻り防止

  • Raspberry Pi には RTC がなく、起動直後の時刻は /etc/fake-hwclock.data から復元される
  • このファイルは毎時の cron と正常シャットダウン時にしか保存されないため、同期した直後に電源が落ちると最大1時間巻き戻る
  • date -s の成功後に fake-hwclock save を実行して巻き戻りを防ぐ
  • 実行ファイルは複数の候補パスから探し、見つからない場合や保存に失敗した場合も同期そのものの成否には影響させない

なお、レビューで「範囲外の時刻が500になる」と指摘した点は誤検知でした。リクエストDTOに Min と Max のバリデーションが既にあり、400が返ります。

バーコードPDFのキャッシュ拡張

  • これまでキャッシュ対象は全商品のPDFのみで、選択商品のPDFは毎回同期生成していた
  • 選択内容の署名をキーにした容量8のLRUキャッシュを追加
  • ウォームアップの対象を枠線なし版だけでなく枠線あり版にも拡張
  • 商品の追加・更新・削除をイベントで受け取り、コミット後にバックグラウンドでキャッシュを作り直す
  • BarcodePdfService が ItemService に依存しているため、逆向きの直接参照を避けてイベント経由にした

Danger Zone パスワード照合のレート制限

  • 照合は PBKDF2 を10万回反復するため、連打されると推測される危険に加えて Raspberry Pi の CPU を占有し会計処理まで巻き添えになる
  • 接続元アドレスごとに連続失敗を数え、5回で60秒ブロックする
  • ブロック中は照合そのものを行わず、429 と Retry-After ヘッダーを返す
  • X-Forwarded-For は詐称できるため使わず remoteAddr のみを見る
  • パスワード未設定時の照合は失敗として数えない
  • 失敗した端末の分だけエントリが残るため、1000件を超えたら期限切れ分を掃除する

検証

  • 追加テスト: DangerZoneVerifyRateLimiterTest 9件、DangerZoneApiControllerTest に4件、BarcodePdfServiceTest と SystemClockUpdaterTest を拡張
  • ./gradlew build -x detekt が成功、ktlint も通過
  • ./gradlew bootRun で起動を確認
  • 実サーバーに対して curl で、5回失敗までは200、6回目以降は429と Retry-After が返り、60秒後に正しいパスワードで認証できることを確認

補足

  • api.yaml に429レスポンスを追記済み
  • Androidアプリ側は429時のメッセージ表示に未対応(別リポジトリで対応予定)

Generated by Claude Code

- 時刻同期後に fake-hwclock へ保存し、RTC のない Raspberry Pi で
  再起動時に時刻が巻き戻るのを防ぐ
- 選択商品のバーコード PDF もキャッシュ対象にし、枠線あり版も
  起動時にウォームアップ。商品の追加・更新・削除をイベントで受けて
  キャッシュを作り直す
- Danger Zone のパスワード照合に接続元ごとのレート制限を追加。
  連続失敗が上限に達している間は照合せず 429 と Retry-After を返す
@github-actions

Copy link
Copy Markdown
Contributor

🎨 Visual Regression Test Results

All visual regression tests passed!

All page screenshots match the expected baselines.

📊 Test Coverage

  • Desktop (1280x720)
  • Tablet (iPad)
  • Mobile (iPhone 13)

@Atsumi3
Atsumi3 merged commit daeda93 into main Aug 24, 2026
7 checks passed
@Atsumi3
Atsumi3 deleted the claude/app-jar-release-output-m2pekn branch August 24, 2026 14:35
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