ハコヅミを作る #48:2つのモデルにバグとセキュリティを見てもらうと、見つけるものが違った
前回(#47)で、セカンドオピニオンのレビューの計画を立てました。今回は、その結果の前半です。
- バグの観点:2つのモデルの報告を、比べる
- セキュリティの観点:守りのテストが、空振りしていた
- 報告を、どう確かめたか
進め方
新しいセッションを開いて、頼み方の文を貼ります。報告が出たら、もとのセッション(ここまで一緒に作ってきた Claude Code)に貼って、1件ずつ、コードを読んで確かめてもらいます。
確かめるのを、もとのセッションに任せるのは、矛盾して聞こえるかもしれません。ただ、やることが違います。新しいセッションの仕事は、「疑って探す」こと。もとのセッションの仕事は、「指された場所を読んで、本当にそう動くかを見る」ことです。指された場所があれば、思い込みは入りにくくなります。
先に結論を書くと、4回の報告(2つの観点 × 2つのモデル)の指摘に、思い違いは1つもありませんでした。直すかどうかの判断は分かれましたが、「コードがそう動く」という点は、どれも合っていました。
バグの観点
重なったもの、片方だけのもの
| 中身 | Opus | Sonnet |
|---|---|---|
| 古い編集画面から保存すると、チェックが全部消える | ○ | ○ |
| 登録のボタンを2回押すと、2件できる | ○ | ○ |
| チェックの送信が失敗すると、画面全体がエラーの画面になる | ○ | ○ |
| コンテナが 100 個以上あると、画面が開けない | ○ | ○ |
| チェックを続けて2回押すと、意図と逆に保存される | ○ | — |
| ログイン後の戻り先を、タブ文字入りの値がすり抜ける | ○ | — |
| 期間で、開始日だけ選ぶと「日帰り」になる | — | ○ |
| 上限は、同時に送った数だけ超えられる | — | ○ |
| プランの一覧のギアの数に、廃棄済みのギアも入る | — | ○ |
重なる指摘が多いのは、予想どおりです。面白いのは、片方だけが見つけたものが、どちらにもあったことでした。
続けて押すと、逆に保存される
Opus が、いちばん重いとして挙げたものです。
持ち物チェックのチェックボックスは、押すとすぐ保存します。このとき送っていた値は、「最後にサーバーから読んだ状態の、逆」でした。
未チェックの項目を押します。応答が返る前に、押しまちがいに気づいて、もう一度押します。すると、2回とも「チェック済みにする」が送られます。取り消したつもりが、「積んだ」で保存されます。
電波の弱いキャンプ場ほど、応答は遅くなります。ハコヅミは、忘れ物をなくすためのアプリです。積んでいないものが「積んだ」になるのは、いちばん起きてほしくないことでした。
片方が「問題なし」としたところ
この同じ場所を、Sonnet は「問題なし」と報告していました。
読むと、理由が書いてあります。「送る値が、反転ではなく絶対値なので、2つの端末で同じ項目を切り替えても、最後の書き込みに収束する」。
Sonnet が見ていたのは、2つの端末で押す場合でした。これは、確かに問題ありません。Opus が見つけたのは、同じ画面で、応答の前にもう一度押す場合です。同じコードの、別の場面でした。
前回、「問題が見つからなかったところも、何を見たかを書いてもらう」と決めておいたのが、ここで効きました。「問題なし」の一言だけだったら、2つの報告が食い違って見えて、どちらを信じるかで迷ったはずです。
戻り先のすり抜け
ログインしていない人が画面を開くと、ログインの画面に移します。ログインのあとは、もとの画面に戻ります。この「戻り先」は、#27 で、アプリの中のパスだけを受け付けるようにしていました。//example.com のような、別のサイトに飛ぶ形は断っています。ユニットテストも書きました。
抜けていたのは、タブ文字です。
ブラウザは、URL の中のタブや改行を、取り除いてから読みます。/<タブ>/example.com は、//example.com として扱われます。今までの確認は、先頭の2文字だけを見ていたので、すり抜けていました。
本物のドメインのリンクから、偽のサイトへ誘導できることになります。外から悪用できるものは、今回の見直しでは、これだけでした。
「ここは悪用されやすいところ」と分かっていて、テストまで書いた場所です。それでも抜けていました。
セキュリティの観点
見つからなかったもの
2つのモデルとも、結論は同じでした。ほかの人のデータを、読む・変える・消す方法は、見つからなかった。権限や会員区分を、本人が書き換える方法もありません。
全部の画面の処理と、データベースを触る関数を読んだうえでの結論です。いちばん心配していたところなので、ほっとしました。
守りのテストが、空振りしていた
Sonnet だけが挙げた指摘が、今回いちばん価値のあるものでした。
#35 で、「どの入口でも、ほかの人のデータを変えられない」ことを確かめる E2E テストを書いています。ほかの人のギアを編集しようとする、ほかの人のキャンプのチェックを付けようとする。どれも断られる(404 が返る)ことを見ています。
このテストの一部が、何も確かめていませんでした。
| テストが送っていたもの | 実際 | 起きていたこと |
|---|---|---|
| ギアの編集(処理の名前なし) | 処理の名前は「update」と「delete」だけ | 名前が合わないので、誰が送っても 404 |
| キャンプのチェック(名前は「check」) | 「check」という処理はない | 同じく、必ず 404 |
| 自分のプランに、他人のギアを入れる | 名前が合っていない | 処理に届いていない。「入っていない」のは当たり前 |
ほかの人のものだから断られたのではなく、送り先がまちがっていたから断られていました。持ち主が送っても、同じ 404 です。
コードそのものは、正しく守れています。2つのモデルが全部読んで、そう判断しています。問題は、守りの証拠だと思っていたテストが、証拠になっていなかったことです。「404 が返る」ことだけを見て、「なぜ 404 か」を確かめていませんでした。
テストは通っていました。CI も緑でした。通っているテストを、疑う理由はありません。だから、書いた本人(Claude Code)にも、受入で見た私にも、見つけられませんでした。
入口が、広すぎた
2つのモデルが、そろって挙げたものもあります。
ログインには、Better Auth というライブラリを使っています。このライブラリは、ログインのほかにも、たくさんの入口(URL)を持っています。その1つに、「自分の名前とプロフィールの画像を、書き換える」入口がありました。
権限や会員区分は、本人が書き換えられないように守っていました。名前と画像は、ライブラリにもとからある欄なので、守りの外でした。長さの上限もありません。数メガバイトの文字を、名前に入れられます。
害は大きくありませんが、「使っていない入口が、開いている」こと自体が問題です。直し方は、指摘より一歩広げました。ライブラリの入口のうち、使っているものだけを通す。使っているのは、2つだけでした。
調べたら、問題がなかったもの
Sonnet は、「開発環境と本番の、予備の URL が、開いた設定のままになっている。手前の守り(Cloudflare Access)が、そこまで効いているかは、コードからは分からない」とも指摘しました。
これは、ダッシュボードを見るしかありません。見てみると、どの URL にも、守りが付いていました。
コードを読むだけでは分からないことを、「分からない」と書いてくれたので、確かめに行けました。
直さない、と決めたもの
指摘のすべてを、直すわけではありません。
- 「招待した人だけ」を守る仕組みが、コードにない:守っているのは、Google のログインの設定(登録した人だけがログインできる)です。コードの外にあるので、コードだけ読むと見えません。ただ、「人が順番どおりに設定することだけが頼り」という指摘は正しいので、手順書を厚くします
- 退会が途中で失敗すると、画像だけ消えたアカウントが残る:順番を逆にすると、「退会した人の画像だけが残る」ことになります。そちらのほうが悪いので、今のままにします
- 上限は、同時に送った数だけ超えられる:コードは直しません。ただ、説明の文に「1件だけ超えることがある」と書いていたのは、事実と違いました。文を直します
直さないものも、理由と一緒に、Issue に残しました。次に同じ指摘を受けたときに、もう一度考えずに済みます。
おわりに
- 2つのモデルに別々に読んでもらうと、重なる指摘が多い。それでも、片方だけが見つけるものが、どちらにもある
- 片方が「問題なし」としたところを、もう片方が見つける。「何を見て、問題なしとしたか」が書いてあれば、食い違いではなく、場面の違いだと分かる
- 悪用されやすいと分かっていて、テストまで書いた場所でも、抜ける
- 「断られること」だけを見るテストは、断られた理由まで確かめる。同じ送り方で、正しい人なら通ることも、一緒に見る
- 通っているテストは、疑われない。だから、外の目が要る
次回は、残りの3つの観点(規約、データの出入り、テスト)です。