綴Photo の勉強ノートに戻る
コードレビュー学習2026.04.15

サービス層関数の owner フィルタ漏れ

コードレビューDjangoセキュリティ

コードレビューで見つかった問題を、勉強のためにまとめたものです(修正済み)。


この記事でわかること

  • サービス層関数で owner フィルタを書き忘れたときの「将来的なリスク」
  • Defense in Depth(多層防御) という考え方
  • AI 生成コードのレビューで「つなぎ目」を見るクセ

概念

このバグは AI に書いてもらったサービス層関数で見つかった。「レイヤーをまたぐ暗黙の前提」 は AI も人間も同じくらい見落とす種類のミスで、特にビュー層 → サービス層の境界は黙ると静かに穴が開きます。

サービス層の関数は 「呼び出し元を信じない」 で実装するのが原則。

セキュリティの世界では 「Defense in Depth(多層防御)」 と呼ぶ考え方がある。どこか1箇所が抜けても大丈夫なように、各層で独立して守る設計。「View 層が owner チェックしてるから、サービス層は気にしなくていい」は、その原則と逆方向の発想になる。

修正前のコード

python
# 修正前
meaning = (
    Meaning.objects
    .filter(photo=photo)  # ← owner フィルタなし
    .order_by("-updated_at", "-created_at")
    .first()
)
# → 他ユーザーの Meaning を誤って上書きできる経路が残る

なぜ危険か

ビュー側で「リクエストしたユーザーが写真の owner か」を確認していれば、いまの呼び出し方では一見安全に見えます。

置き場所は PostgreSQL の photos_meaning。owner 列は行に付く。入口と確認はメモリで保存しない。今の画面HTTP のリクエストメモリ・保存しないビューが owner 確認サービスは photo だけphotos_meaning(PostgreSQL)自分の行だけ当たるビューの絞り込みのおかげバッチ・管理コマンド別画面のビューも含むメモリ・保存しないowner 確認なし同じサービス photo だけphotos_meaning(PostgreSQL)他人の行も結果に出るfirst が掴んで上書きする

「いま安全」と「将来も安全」は別物で、呼び出し元に守りを依存している関数は構造変更で簡単に穴が開きます。

対策(1 行)

python
# 修正後
meaning = (
    Meaning.objects
    .filter(photo=photo, owner=user)  # ← owner フィルタ追加
    .order_by("-updated_at", "-created_at")
    .first()
)

owner=user を 1 引数足すだけ。動作は変わりませんし書き味も変わりません。なのに「将来呼び出し方が増えても破綻しない」という耐性だけ上がる、コスパの高い修正。

学びポイント

  • サービス層は「呼ばれ方」に依存せず、単体で安全に動くよう書きます(Defense in Depth)
  • フィルタ条件を関数の中に埋め込んでおくと、呼び出し元が増えても破綻しません
  • AI が書いたコードも、人間が書いたコードと同じ種類のバグを持ちます。「つなぎ目」は自分の目で見る 習慣が効きます


おわりに

このバグは 1 引数足すだけ で直ります。にもかかわらず、別タスクの集中を切らしたくないとか「修正済み」ラベルを貼るほどの大きさじゃないとかの心理で 後回しになりがちです。「あとで」が来ないまま、コンテキストが薄れて直す気が失せていきます ─ セキュリティ修正の典型的な腐り方。

発見した日の最初のコミットで片付けるのが、結局いちばん安全で速いです。


この記事は2026年4月時点の自分の理解で書いています。Django のサービス層パターンや Defense in Depth の考え方は時代を超えて使えるが、最新の設計トレンドも合わせて参照してください。