ハコヅミを作る #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つの観点(規約、データの出入り、テスト)です。

← トップページに戻る