監査ログのRLS policyがactor_user_id=NULLを許すと、乗っ取られたアカウントが操作痕跡を消せる

結論

監査ログテーブルへのINSERTを許すRLS policyが、actor_user_id IS NULL を無条件に許可していた。乗っ取られたテナント内アカウントは、この抜け道を使って自分の操作をactor=NULLとして記録し、監査ログから自分を消せる状態だった。

• actor_user_id IS NULL は、どのロールでも通る条件になっていた
• 本来NULLを許してよいのは、cross-tenantの内部操作を行うsystem_adminだけ
• テナント内ロール(school_admin / teacherなど)は、自分自身のuser_idへの完全一致以外は拒否すべきだった

policyをロールで分岐させ、テナント内ロールのNULL指定を拒否する形に直した。

症状

監査ログテーブルへのINSERTは、次のようなpolicyで許可されていた。

CREATE POLICY audit_log_insert ON audit_log FOR INSERT
  WITH CHECK (
    (
      school_id IS NULL
      OR school_id = NULLIF(current_setting('app.current_school_id', true), '')::uuid
      OR current_setting('app.current_user_role', true) = 'system_admin'
    )
    AND (
      -- actor_user_id 詐称防止: 自分自身 or system_admin or null のみ許可
      actor_user_id IS NULL
      OR actor_user_id = NULLIF(current_setting('app.current_user_id', true), '')::uuid
      OR current_setting('app.current_user_role', true) = 'system_admin'
    )
  );

コメントには「詐称防止」と書かれている。実際、actor_user_idを他人のuuidに書き換えて成りすます経路は塞がれていた。だが**actor_user_id IS NULLはロールを問わず常に通る**条件として残っていた。

レビューで指摘されたのはここだ。テナント内のアカウント(school_adminやteacher)が乗っ取られた場合、攻撃者は自分のuser_idではなくNULLを指定して監査ログにINSERTできる。監査ログは「誰が何をしたか」を残すためのテーブルなので、actorがNULLの操作は実質、誰の仕業か特定できなくなる。これは否認(Repudiation)のリスクとして扱われた。

原因

actor_user_id IS NULLの許可は、cross-tenantの内部集計処理やmigrator経由のINSERTなど、特定のテナントユーザーに紐づかない正当な書き込みを通すために入れられたものだった。

しかしpolicyの条件式はロールで場合分けされておらず、「NULLなら通す」という一枚岩の条件になっていた。想定していたのはsystem_adminが行う内部操作だったが、WITH CHECKの条件式自体はどのロールで実行されたセッションにも等しく適用される。意図はロール限定だったのに、実装はロールを見ていなかった。

直す

actor_user_id側の条件を、ロールによる場合分けに書き換えた。

DROP POLICY IF EXISTS audit_log_insert ON audit_log;

CREATE POLICY audit_log_insert ON audit_log FOR INSERT
  WITH CHECK (
    (
      school_id IS NULL
      OR school_id = NULLIF(current_setting('app.current_school_id', true), '')::uuid
      OR current_setting('app.current_user_role', true) = 'system_admin'
    )
    AND (
      -- actor_user_id 側:
      --   system_admin は NULL / 任意 uuid どちらも許可 (cross-tenant 内部操作)
      --   それ以外のロールは自分自身の user_id に完全一致のみ (NULL 拒否、詐称防止)
      current_setting('app.current_user_role', true) = 'system_admin'
      OR actor_user_id = NULLIF(current_setting('app.current_user_id', true), '')::uuid
    )
  );

system_adminのときだけ最初のORで無条件に通り、それ以外のロールは2つ目の条件、つまりSET LOCALされた自分自身のuser_idとの完全一致でしか通らない。SQLの三値論理ではNULL = uuidの評価結果はNULL(真にならない)なので、テナント内ロールがactor_user_idにNULLを指定した場合、この条件は自動的に偽になる。「NULLなら通す」という一枚岩の条件を、ロール分岐の中に埋め込み直した格好だ。

副次的に見つかった検証漏れ

このpolicyを追加した直後のPRで、CIのテストが想定通りに落ちなかった。

テストDBを初期化するセットアップ処理は、適用するmigrationファイルを次のように定数で列挙していた。

const RLS_ENABLE_SQL = join(packageRoot, "migrations", "0001_enable_rls.sql");
const RLS_POLICIES_SQL = join(packageRoot, "migrations", "0002_rls_policies.sql");
const AUDIT_TRIGGER_SQL = join(packageRoot, "migrations", "0003_audit_trigger.sql");
const AUDIT_FK_SQL = join(packageRoot, "migrations", "0004_audit_fk.sql");

新しいpolicyを追加したmigrationファイルはリポジトリに存在していたが、この列挙には含まれていなかった。テストDBには旧policy(NULL許可)がそのまま残り、新policyを検証するはずのテストケース(「テナント内ロールがNULLを指定したら拒否される」)は、実際には旧policyに対して実行されていた。テストコード自体は正しく書かれていたのに、テスト対象のDBが新しいmigrationを反映していなかったため、検証したいはずの挙動が一度も検証されない状態だった。

修正は、新しいmigrationを列挙に追加し、セットアップの実行順に組み込むことだった。

const AUDIT_LOG_ACTOR_NULL_SQL = join(
  packageRoot,
  "migrations",
  "0005_audit_log_actor_null_school_admin.sql",
);

これでCI上のテストDBにも新policyが適用され、「テナント内ロールのNULL指定が拒否されること」が実際に検証されるようになった。

再発しにくい形

RLS policyの条件式を書くときは、「意図した対象」と「実装が指している対象」が一致しているかを、条件式そのものを読んで確かめる必要がある。actor_user_id IS NULLという一行は、コメントの意図(system_adminの内部操作)を読まなければ、どのロールにも等しく適用される一枚岩の許可に見えてしまう。

そしてpolicyを直しただけでは終わらない。テストDBがmigrationを反映しているかどうかは、migrationファイルの存在とは別に確認する対象になる。新しいmigrationを追加したら、テストのセットアップ処理にそれを実行対象として登録したかまで含めて、変更をワンセットで見る必要がある。