0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

AIが書いたコードのレビューで見る5つの穴 認可漏れ・握りつぶした例外・存在しないAPIをPythonとTypeScriptで再現する

0
Posted at

AIが書いたコードのレビューで、テストが全部通っていても見落としやすい5つの穴を、小さなコードで再現しました。認可のチェック漏れ、握りつぶされた例外、存在しない(または意味が違う)API、ログに出る個人情報、依頼範囲外の変更です。

テストの質と、AIが提案するパッケージ名の確認は、前に別の記事(「AIでテストを書く QAチームのための4ステップ検証ループ」「AIが提案したパッケージ名を、インストール前にレジストリで確かめる」)で書いたので、ここでは扱いません。気になる節だけ拾ってもらえれば十分です。

どれも人が書いたコードでも起きます。ただ、AIが書いたコードは見た目が整っていて、コメントも型もそろっているので、読む側が「ちゃんとしている」と思い込みやすいです。各節は「書きがちな形、動かすと何が起きるか、レビューで聞くこと、直し方」の順にしています。

環境

動作確認は2026年10月17日、macOSです。

  • Python 3.14.6、pytest 9.1.1
  • Node.js v22.22.3(--experimental-strip-types でTypeScriptを直接実行)
  • TypeScript 5.9.3(型チェックのみ)

コードは説明用の最小例です。実際のフレームワークの認可やロギングの仕組みの代わりにはなりません。

この記事をざっくり図解

1 認可      画面では隠れていても、APIは他人のデータを返す
2 例外      失敗を空の値に変えて、成功のように見せる
3 API       存在しないメソッド / 存在するが意味が違う引数
4 ログ      デバッグ用の出力に個人情報が入る
5 範囲      頼んでいないファイルまで書き換わる

1. 認可のチェック漏れ

ログイン済みかどうかしか見ていないハンドラは、AIが書きがちな形です。画面では自分の注文しか出ないので、動かしても気づきません。

# orders.py
from dataclasses import dataclass

@dataclass(frozen=True)
class User:
    id: int
    role: str  # "customer" or "admin"

ORDERS = {
    1: {"id": 1, "owner_id": 10, "total": 12000},
    2: {"id": 2, "owner_id": 20, "total": 98000},
}

# 書きがちな版: ログイン済みかどうかしか見ていない
def get_order_unsafe(user: User, order_id: int) -> dict:
    return ORDERS[order_id]

def list_orders_unsafe(user: User) -> list[dict]:
    return list(ORDERS.values())

# 修正版: 所有者か管理者のみ
class Forbidden(Exception):
    pass

def get_order(user: User, order_id: int) -> dict:
    order = ORDERS[order_id]
    if user.role != "admin" and order["owner_id"] != user.id:
        raise Forbidden(f"order {order_id}")
    return order

def list_orders(user: User) -> list[dict]:
    if user.role == "admin":
        return list(ORDERS.values())
    return [o for o in ORDERS.values() if o["owner_id"] == user.id]
# test_orders.py
import pytest
from orders import (User, Forbidden, get_order_unsafe, list_orders_unsafe,
                    get_order, list_orders)

alice = User(id=10, role="customer")

def test_unsafe_returns_other_users_order():
    # 他人の注文(id=2, owner_id=20)が読めてしまう
    assert get_order_unsafe(alice, 2)["owner_id"] == 20
    assert len(list_orders_unsafe(alice)) == 2

def test_fixed_blocks_other_users_order():
    with pytest.raises(Forbidden):
        get_order(alice, 2)
    assert [o["id"] for o in list_orders(alice)] == [1]
    assert len(list_orders(User(id=1, role="admin"))) == 2

1つ目のテストは、バグを再現するためのものです。alice(id=10)が、他人の注文(id=2)を読めてしまうことを確かめています。実行結果は次のとおりです。

test_orders.py::test_unsafe_returns_other_users_order PASSED
test_orders.py::test_fixed_blocks_other_users_order PASSED

レビューで聞くのは一つです。「別のユーザーのIDで直接呼ぶと、どうなるか」。ボタンが出ない、メニューに出ない、は認可ではありません。一覧も同じで、list_orders_unsafe は全件を返しています。個別の取得だけ直して一覧を忘れる、という直し漏れもよくあります。

直すときは、チェックを画面側ではなく、データを取る関数の近くに置きます。テストには「他人のIDを渡したら拒否される」を必ず1本入れます。

2. 握りつぶした例外

失敗を空の配列やデフォルト値に変えてしまう形です。呼び出し側からは、成功して0件だったのか、失敗したのか区別できません。

// fetch_users.ts
type User = { id: number; name: string };

// 書きがちな版: 失敗を空配列にして返す
export async function fetchUsersUnsafe(fetcher: (url: string) => Promise<Response>): Promise<User[]> {
  try {
    const res = await fetcher("/api/users");
    return (await res.json()) as User[];
  } catch {
    return [];
  }
}

// 修正版: 失敗は呼び出し側に伝える。HTTPエラーも失敗として扱う
export async function fetchUsers(fetcher: (url: string) => Promise<Response>): Promise<User[]> {
  const res = await fetcher("/api/users");
  if (!res.ok) {
    throw new Error(`GET /api/users failed: ${res.status}`);
  }
  return (await res.json()) as User[];
}
// fetch_users.test.ts
import { test } from "node:test";
import assert from "node:assert/strict";
import { fetchUsersUnsafe, fetchUsers } from "./fetch_users.ts";

const down = async (_url: string): Promise<Response> => { throw new Error("network down"); };
const serverError = async (_url: string): Promise<Response> =>
  new Response("oops", { status: 500 });

test("unsafe: network failure looks like 'no users'", async () => {
  assert.deepEqual(await fetchUsersUnsafe(down), []);
});

test("unsafe: 500 with non-JSON body also looks like 'no users'", async () => {
  assert.deepEqual(await fetchUsersUnsafe(serverError), []);
});

test("fixed: failures reach the caller", async () => {
  await assert.rejects(() => fetchUsers(down), /network down/);
  await assert.rejects(() => fetchUsers(serverError), /failed: 500/);
});
node --experimental-strip-types --test fetch_users.test.ts
ok 1 - unsafe: network failure looks like 'no users'
ok 2 - unsafe: 500 with non-JSON body also looks like 'no users'
ok 3 - fixed: failures reach the caller
# pass 3
# fail 0

ネットワーク断でも500でも、画面には「ユーザーがいません」と出ます。障害が運用で気づかれず、「データが消えた」という問い合わせになる形です。

レビューで聞くのは「この catch に入ったとき、呼び出し側はそれを知る手段があるか」です。catch の中が return []、return null、return 0、ログだけ、のいずれかなら、まず疑います。直し方は、握りつぶさずに上げるか、上げないなら「失敗した」という値(結果型やエラー付きの戻り値)にして、呼び出し側が区別できるようにします。

3. 存在しないAPIと、存在するが意味が違うAPI

3-1. 存在しないメソッド

Array.prototype.groupBy は、標準にはありません。標準にあるのは Object.groupBy と Map.groupBy です。名前がもっともらしいので、目で読むと通ってしまいます。

// invented.ts(抜粋)
type Order = { id: number; status: string };

// 存在しないメソッド(Array.prototype.groupByは標準にない)
export function countByStatusInvented(list: Order[]) {
  // @ts-expect-error 型チェックで検出される
  return list.groupBy((o: Order) => o.status);
}

// 標準にあるのは Object.groupBy / Map.groupBy
export function countByStatus(list: Order[]) {
  const grouped = Object.groupBy(list, (o) => o.status);
  return Object.fromEntries(Object.entries(grouped).map(([k, v]) => [k, v?.length ?? 0]));
}

実行すると、実行時にだけ失敗します。

invented: TypeError: list.groupBy is not a function
fixed: { paid: 2, shipped: 1 }

型チェックを通していれば、実行前に止まります。@ts-expect-error を外したコードを tsc --noEmit --strict --target es2024 --lib es2024 にかけた結果です。

t.ts(3,15): error TS2339: Property 'groupBy' does not exist on type 'Order[]'.

つまり、AIが any や // @ts-ignore を足して型エラーを消していないか、が見どころです。レビューでは「これを実際に動かしたか、型チェックは通したか」と、「公式ドキュメントのどこに載っているか」を聞きます。

3-2. 存在するが、意味が違う

こちらのほうが気づきにくいです。エラーにならず、動いて、結果だけが違います。

# resub.py
import re

# 「大文字小文字を無視して置換」のつもり
def mask_unsafe(text: str) -> str:
    return re.sub("secret", "***", text, re.IGNORECASE)

def mask(text: str) -> str:
    return re.sub("secret", "***", text, flags=re.IGNORECASE)

if __name__ == "__main__":
    s = "Secret and SECRET and secret"
    print("unsafe:", mask_unsafe(s))
    print("fixed :", mask(s))
    print("re.IGNORECASE as int =", int(re.IGNORECASE))
unsafe: Secret and SECRET and ***
fixed : *** and *** and ***
re.IGNORECASE as int = 2

re.sub の4番目の位置引数は flags ではなく count です。re.IGNORECASE は整数の2なので、「最大2回置換」と解釈されます。しかも大文字小文字は区別されたままなので、小文字の secret だけが置換されました。例外は出ません。Python 3.14.6では DeprecationWarning: 'count' is passed as positional argument が出ましたが、警告を見ていなければ気づきません。

もう一つ、[10, 2, 1].sort() は [1, 10, 2] になります(TypeScript/JavaScriptの既定は文字列順)。数値は .sort((a, b) => a - b) と書きます。

レビューで聞くのは「引数の順序と意味を、公式のシグネチャで確かめたか」です。位置引数が3つ以上ある呼び出しは、キーワード引数で書かれているかを見ます。警告を見逃さないよう、CIで警告をエラーにする(Pythonなら -W error)設定も有効です。

4. ログに出る個人情報

「デバッグしやすいように」入力を丸ごとログに出す形です。

# pii_log.py
import logging
logger = logging.getLogger("signup")

def signup_unsafe(user: dict) -> None:
    # 「デバッグしやすいように」入力を丸ごと出す
    logger.info("signup request: %s", user)

def signup(user: dict) -> None:
    logger.info("signup request: user_id=%s plan=%s", user["id"], user["plan"])
# test_pii_log.py
import logging
from pii_log import signup_unsafe, signup

USER = {"id": 7, "plan": "free", "email": "taro@example.com", "phone": "090-0000-0000"}

def test_unsafe_logs_email(caplog):
    with caplog.at_level(logging.INFO, logger="signup"):
        signup_unsafe(USER)
    assert "taro@example.com" in caplog.text

def test_fixed_does_not_log_email(caplog):
    with caplog.at_level(logging.INFO, logger="signup"):
        signup(USER)
    assert "taro@example.com" not in caplog.text
    assert "user_id=7" in caplog.text
test_pii_log.py::test_unsafe_logs_email PASSED
test_pii_log.py::test_fixed_does_not_log_email PASSED

ログは、本番のデータよりも広い範囲の人が読めることがあります。レビューでは、logger.* と print の引数に、リクエスト全体やユーザーオブジェクトがそのまま渡っていないかを見ます。直し方は、出す項目を明示すること(許可リスト方式)です。「個人情報を消す」より「出してよいものを選ぶ」ほうが、項目が増えたときに漏れにくいです。

5. 依頼範囲外の変更

「請求書の税計算を直して」と頼んだのに、ログイン処理やロックファイルまで変わっているケースです。差分が大きいと、読む側は流し読みになります。

人が目で追う前に、機械で確かめられます。変更ファイルが許可した範囲に収まっているかを調べる小さなスクリプトです。

# scope_check.py
"""変更ファイルが、依頼で許可した範囲に収まっているかを確かめる。"""
import fnmatch
import subprocess
import sys

def changed_files(base: str) -> list[str]:
    out = subprocess.run(["git", "diff", "--name-only", f"{base}...HEAD"],
                         capture_output=True, text=True, check=True).stdout
    return [line for line in out.splitlines() if line]

def out_of_scope(files: list[str], allowed: list[str]) -> list[str]:
    return [f for f in files if not any(fnmatch.fnmatch(f, p) for p in allowed)]

if __name__ == "__main__":
    base, *allowed = sys.argv[1:]
    bad = out_of_scope(changed_files(base), allowed)
    for f in bad:
        print(f"範囲外: {f}")
    sys.exit(1 if bad else 0)

一時的なgitリポジトリ(src/invoice/、src/auth/、package-lock.json を変更したブランチ)で、src/invoice/* だけを許可して実行しました。

python scope_check.py main "src/invoice/*"
範囲外: package-lock.json
範囲外: src/auth/login.py

終了コードは1でした。fnmatch の * は / も含めて一致するので、src/invoice/* は src/invoice/ 配下の深い階層にも一致します。厳密に階層を区切りたい場合は、pathlib.PurePath.match や pathspec などに置き換えてください。

範囲外が出たら、レビューでは「これは別のPRにできないか」を聞きます。ロックファイルの変更は、依存を足していないのに変わっていたら、必ず理由を確かめます。依存パッケージが増えていた場合は、前述の別の記事の方法で名前を確かめます。

何が分からないか

  • 上のコードは、穴の形を再現するための最小例です。実際のバグは、フレームワークの認可機構、ORM、ミドルウェアの中に現れます。形が違っても、聞く質問は同じです。
  • 5つの穴は、全体の一部です。業務ルールの誤り(端数処理、締め日、顧客ごとの例外など)は、コードを動かしても分かりません。要件を読んだ人にしか判断できません。
  • 静的解析ツールで拾えるものもあります(空の catch、未使用の変数など)。ここでは試していません。ツールの設定は、どこまでを機械に任せるかを決めてからのほうがうまくいきます。
  • 動作確認はPython 3.14.6、Node.js v22.22.3、TypeScript 5.9.3です。re.sub の警告や Object.groupBy の有無は、バージョンで変わります。

最後に

1つだけ試すなら、1の「他人のIDで呼ぶ」です。自分のプロジェクトの取得系エンドポイントを3つ選んで、別のユーザーのIDで実際に叩いてみてください。画面を通した確認では見えないものが、直接呼ぶと見えることがあります。

参考文献

0
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
0
0

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?