Skip to content

認証の境界を1か所に決める (#472) - #479

Merged
kojira merged 2 commits into
mainfrom
fix/duplicate-require-auth-472
Sep 4, 2026
Merged

認証の境界を1か所に決める (#472)#479
kojira merged 2 commits into
mainfrom
fix/duplicate-require-auth-472

Conversation

@kojira

@kojira kojira commented Sep 4, 2026

Copy link
Copy Markdown
Member

Fixes #472

境界の規則

/api/events/* の認証は routes/events.tsuse("*", requireAuth) だけが持つ。worker.tsapi.route("/events", eventRoutes)/events への登録の1本目なので、後ろに並べた配下ルート(scoring・photos・chat …22本)は必ずこれを通る。配下のサブアプリは自前の requireAuth を持たない。

認証なしで通したい経路は api.route("/events", eventRoutes) より前に登録する(既存の公開画像・公開コメント一覧などと同じ形)。/api/events 以外の接頭辞(/api/me/api/communities など)は、従来どおり各ルートファイルが自分の境界を持つ。

なぜ「二重」では済まなかったか

Hono のミドルウェアはパターン一致で積まれる。/api/events/*use("*") を持つサブアプリを22本並べると、どのサブアプリのハンドラを叩いても、並べた全部の requireAuth が順に走る。最も後ろに並ぶ /api/events/:id/staff-invites では23回。requireAuth 1回につきセッションとユーザーで2クエリなので、1リクエストで D1 を46回引いていた。

requireAuth の実行回数 D1 クエリ
GET /api/events/:id/staff-invites (最悪) 23 → 1 46 → 2
GET /api/events/:id/todos 16 → 1 32 → 2
GET /api/events/:id/bingo 14 → 1 28 → 2
GET /api/events/api/me など 1 → 1 変化なし
認証必須448本の延べ実行回数 2893 → 448

ルート表の差分

app.routes を歩き、各ルートのパスを実際のルーター(app.router.match)に引かせて、そのハンドラに届くまでに通る requireAuth を数えた。読んだだけの推測ではなく、Hono 自身の解決結果。

  • 登録ルート 533本、うち認証必須 448本、未認証で通る 85本
  • 認証の有無は1本も変わらない(533本の AUTH/OPEN 分類が前後で完全一致)
  • 変わったのは実行回数だけ。変更後は448本すべてが authN=1

変異テスト

test/auth-boundary.test.ts が本当に効くことを、共有ツリーとは別の使い捨てコピーで確認した。各変異はファイルの内容ハッシュで「当たったこと」と「戻ったこと」を検証している。

変異 ハッシュ (前→変異→復元) 結果
events.tsuse("*", requireAuth) を削除 9fa1bd4ea864ffe7d6d3b50e9fa1bd4ea864 落ちた(未認証の検査・events の検査)
境界の外に api.get("/leak") を追加 a0398b7ec7de183fad3751dda0398b7ec7de 落ちた(未認証の検査)
worker.tsscoringRouteseventRoutes より前へ a0398b7ec7defeee55fb7caaa0398b7ec7de 落ちた(未認証の検査・events の検査)
eventBingo.tsrequireAuth を重ねる 2ddef3d0d160114267aae5682ddef3d0d160 落ちた(1回だけの検査・events の検査)

未認証で通る85本は表に列挙してあり、認証の要らない経路を足すときは表にも足さないと落ちる。逆に、認証必須のはずの経路を境界の外に生やしても落ちる。

検証

  • pnpm -r typecheck 通過
  • サーバー全件: 86ファイル 1150件 通過(11シャード)
  • web 全件: 880件 通過
  • 触ったファイルはすべて800行未満(最大 worker.ts 653行)

docs

docs/design.md §3.1 に「配下は自前の requireAuth を持たない」根拠と実測、テストが見張る範囲を追記。docs/staff-todo.md §6.1・docs/staff-roles.md §6.1 のコード片も実装後の姿に合わせた。

🤖 Generated with Claude Code

https://claude.ai/code/session_01SLkuubF8cHzSnnsv4fN9DA

kojira and others added 2 commits September 4, 2026 22:20
`/api/events/*` の認証は `routes/events.ts` の `use("*", requireAuth)` だけが
持つ。`worker.ts` で `api.route("/events", eventRoutes)` が `/events` への登録の
1本目なので、後ろに並べた配下ルートは全部これを通る。にもかかわらず配下の
サブアプリ22本が各自 `use("*", requireAuth)` を持っていた。

Hono のミドルウェアはパターン一致で積まれるため、これは「2回」では済まない。
`/api/events/*` に `use("*")` を持つサブアプリを並べると、どのサブアプリの
ハンドラを叩いても並べた全部の requireAuth が順に走る。最も後ろに並ぶ
`/api/events/:id/staff-invites` では 23回、requireAuth 1回あたりセッションと
ユーザーで2クエリなので、1リクエストで D1 を46回引いていた。

22本の重複を落とし、登録済み533本を実際のルーターに引かせて、認証の有無が
1本も変わらないことを確認した(requireAuth の実行回数は延べ 2893 → 448、
認証必須の448本すべてがちょうど1回)。

戻りを防ぐため test/auth-boundary.test.ts を追加した。登録済みルートを1本ずつ
歩いて、requireAuth がちょうど1回であること・未認証で通る経路が表のとおりで
あることを見張る。境界の削除・境界の外への新ルート・worker.ts の並び替え・
重複の復活の4つの変異でそれぞれ落ちることを確認済み。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SLkuubF8cHzSnnsv4fN9DA
## OPEN_ROUTES はこの検査の唯一の抜け道

境界の外へ経路を1本出しても、その鍵を表に足せば検査は通る(サブアプリ丸ごとは
全パスぶん足すことになるので通らないが、1本なら通る)。守りが「表への追加を
レビューが認可の変更として読むこと」に載っているので、それを前口上に命令形で
書いた。「テストを通すために足す」は禁止、と明記している。

事故で足せないよう EXPECTED_OPEN_COUNT も突き合わせる。表を1行足すと必ずこの数も
動かすことになるので、こっそり1本開ける差分が必ずレビューに現れる。
表そのものも用途ごとの塊に並べ替えて、足すときにどの塊かを選ばせる。

## 歩けない2つの形を塞ぐ

これまで ALL メソッドとワイルドカードを含むパスを一律で飛ばしていた。今は
`api.all("/x", h)` のような ALL の終端ハンドラも歩く(ワイルドカードでなければ
代表パスを作れる)。これで5本(broadcasts・todos・staffing・name-cards・
timetable/editing の requireEventRole)も検査対象に入り、歩く数は 533 → 538。

ワイルドカードのパスは代表パスを作れないので依然歩けないが、そこに入るのは
ミドルウェア登録で、ミドルウェアは終端ではないため穴にならない。穴になるのは
ワイルドカードに終端ハンドラを載せた場合だけなので、next を受け取るか
(引数の数)で見分けて、終端ハンドラが worker.ts 末尾の ASSETS フォールバック
1本だけであることを固定した。

## 変異

新しく3つ確認した(いずれも内容ハッシュで当たりと復元を検証、使い捨てコピーで実行):

- `api.all("/leak-all", h)` を境界の外に追加 → 落ちた(旧実装では見えなかった)
- `api.get("/leak-wild/*", h)` を境界の外に追加 → 落ちた(同上)
- 表にだけ1行足して件数は据え置き → 落ちた

既存の4つ(境界の削除・境界の外への新ルート・worker.ts の並び替え・重複の復活)
も引き続き落ちることを再確認した。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SLkuubF8cHzSnnsv4fN9DA
@kojira

kojira commented Sep 4, 2026

Copy link
Copy Markdown
Member Author

レビューの2点に対応した(8c9dc7f)。

1. 表への追加は認可の変更である、と命令形で書いた。 前口上に「OPEN_ROUTES への追加は『この経路を未ログインに開く』という宣言として読み、認可の変更としてレビューすること」「テストを通すために足すのは禁止」を明記。事故で足せないよう EXPECTED_OPEN_COUNT(79)とも突き合わせるようにしたので、1行足すと必ず数も動き、差分に現れる。表そのものも用途ごとの塊(ログイン前・死活・cron・メール・公開の読み取り・SPA/フィード等)に並べ替えて、足すときにどの塊かを選ばせている。

2. 歩けない2つの形を塞いだ。 api.all("/x", h) のような ALL メソッドの終端ハンドラは、ワイルドカードでなければ代表パスを作れるので普通に歩く対象に入れた。これで requireEventRole の5本(broadcasts・todos・staffing・name-cards・timetable/editing)も検査対象になり、歩く数は 533 → 538。

ワイルドカードのパスは代表パスを作れず依然歩けないが、そこに入るのはミドルウェア登録で、ミドルウェアは終端ではないので穴にならない。穴になるのはワイルドカードに終端ハンドラを載せた場合だけなので、next を受け取るか(引数の数)で見分け、終端ハンドラが worker.ts:625 の ASSETS フォールバック1本だけであることを固定した。現状 arity=1 が1本、arity=2 が44本ときれいに割れている。

追加の変異(いずれも内容ハッシュで当たり/復元を検証、使い捨てコピーで実行)

変異 ハッシュ (前→変異→復元) 結果
api.all("/leak-all", h) を境界の外に追加 a0398b7ec7de5e94184cd309a0398b7ec7de 落ちた(未認証の検査)。旧実装では見えなかった
api.get("/leak-wild/*", h) を境界の外に追加 a0398b7ec7de371c69811acca0398b7ec7de 落ちた(ワイルドカード終端の検査)。同上
表にだけ1行足して件数は据え置き 3eb7908830a5c6fc8f5b67473eb7908830a5 落ちた(件数の検査)

既存の4つ(境界の削除・境界の外への新ルート・worker.ts の並び替え・重複の復活)も引き続き落ちることを再確認した。

検証

pnpm -r typecheck 通過。サーバー全件 86ファイル 1152件 通過(11シャード、auth-boundary は 5→7件)。web 全件 880件 通過。auth-boundary.test.ts は272行。

docs/staff-todo.md の801行は把握した(main でも801行)。このPRでは触らず、別issueに回す。

@kojira
kojira merged commit 53203e5 into main Sep 4, 2026
1 check passed
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.

requireAuth がイベント配下のルートで二重に走る

1 participant