This repository was archived by the owner on Aug 26, 2026. It is now read-only.
[Phase1] test/db/ RLS検証(最小8項目 + 権限マトリクス全項目) - #32
Merged
Merged
Conversation
docs/permissions.mdの権限マトリクスと最小検証セットに基づき、 実際にauth.signUpで作成したユーザーのクライアントでRLSを検証する。 service_roleキーは一切使わない。 - profiles: 本人のみ全カラムSELECT、profiles_publicビュー経由の列制限、 is_adminの書き換え拒否、DELETE不可 - events: 全員SELECT(未削除)、他人による編集不可、なりすまし登録不可、 削除ガード(公開/非公開どちらの参加者でも失敗)、削除後もオーナー/ 支出保持者は閲覧可、無関係者は閲覧不可 - event_participants: 自己登録、なりすまし不可、招待(visibility/ participation_state固定)、未参加者による招待不可、他人の行の 更新・削除不可、visibility別の可視性 - ticket_entries/expenses/budgets: 本人のみ全操作、他人からは不可視 - budgets: NULLS NOT DISTINCT制約の重複防止を回帰テストとして固定 CI(.github/workflows/supabase.yml)でsupabase status -o envの出力を 環境変数として渡し、supabase startしたローカルインスタンスに対して yarn test:dbを実行する。 Closes #26
クォート付きのまま渡すとAPI_URLが"http://..."となりsupabase-jsの URLバリデーションに失敗していた。
There was a problem hiding this comment.
Pull request overview
Issue #26 に対応し、docs/permissions.md の権限マトリクス/最小検証セットに基づいて test/db/ で実DBのRLS検証（否定側中心）を追加し、CIの db-test でSupabase接続情報（API_URL / ANON_KEY）を受け渡せるようにするPRです。
Changes:
test/db/helpers.tsを追加し、auth.signUpで作成した実セッションのクライアントでRLS検証できるようにした- 各テーブル（profiles/events/event_participants/ticket_entries/expenses/budgets）のRLS検証テストを
test/db/に追加 .github/workflows/supabase.ymlのdb-testにsupabase status -o envの出力をGITHUB_ENV経由で渡すステップを追加
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | @supabase/supabase-js 追加に伴うロック更新 |
| package.json | DBテスト用に @supabase/supabase-js を依存追加 |
| .github/workflows/supabase.yml | db-test 実行時に API_URL / ANON_KEY を環境変数へエクスポート |
| test/db/helpers.ts | auth.signUp で作成した認証済みクライアント/イベント作成ヘルパーを追加 |
| test/db/profiles.test.ts | profiles / profiles_public のRLS・カラム露出の否定側検証を追加 |
| test/db/events.test.ts | events の更新・削除ガード/削除後閲覧のRLS検証を追加 |
| test/db/event-participants.test.ts | 参加登録/招待/visibility/他人操作不可のRLS検証を追加 |
| test/db/ticket-entries.test.ts | ticket_entries の本人のみCRUD・他人不可のRLS検証を追加 |
| test/db/expenses.test.ts | expenses の本人のみCRUD・他人不可のRLS検証を追加 |
| test/db/budgets.test.ts | budgets の本人のみCRUD・他人不可 + NULLS NOT DISTINCT回帰検証を追加 |
| test/db/.gitkeep | test/db/ ディレクトリ維持用ファイル |
Suppressed comments (6)
test/db/budgets.test.ts:53
const id = created?.id ?? ""は、setupのinsertが失敗してcreatedがnullでもテストが進み、空IDに対するupdate/deleteが0件で通ってしまう可能性があります。setupのerror/dataを検証してからcreated.idを使ってください。
amount: 10000,
})
.select()
.single();
const id = created?.id ?? "";
test/db/budgets.test.ts:80
- 本人のDELETEは
error === nullだけだと「0件削除（実は権限が無い）」でも通ってしまいます。setupのinsert成功を検証した上で、deleteで返る行数も確認して「本当に削除できた」ことを担保してください。
const { error } = await user.client.from("budgets").delete().eq("id", created?.id ?? "");
expect(error).toBeNull();
test/db/expenses.test.ts:50
const id = created?.id ?? ""はsetupのinsert失敗時に空IDでupdate/deleteして0件となり、RLSの回帰を見逃す可能性があります。insertのerror/dataを検証してからcreated.idを使ってください。
.from("expenses")
.insert({ event_id: event.id, user_id: self.userId, category: "ticket" })
.select()
.single();
const id = created?.id ?? "";
test/db/expenses.test.ts:73
- 本人のDELETEを
error === nullだけで判定すると、0件削除でも通ってしまいます（RLSが効いていない/効きすぎている回帰を検出できない）。setupのinsert成功を検証し、deleteの返却行数も確認してください。
const { error } = await self.client.from("expenses").delete().eq("id", created?.id ?? "");
expect(error).toBeNull();
test/db/ticket-entries.test.ts:50
const id = created?.id ?? ""はsetupのinsert失敗時に空IDでupdate/deleteして0件となり、RLSの回帰を見逃す可能性があります。insertのerror/dataを検証してからcreated.idを使ってください。
.from("ticket_entries")
.insert({ event_id: event.id, user_id: self.userId, entry_type: "lottery" })
.select()
.single();
const id = created?.id ?? "";
test/db/ticket-entries.test.ts:80
- 本人のDELETEを
error === nullだけで判定すると、0件削除でも通ってしまいます。setupのinsert成功を検証し、deleteの返却行数も確認して「本当に削除できた」ことを担保してください。
const { error } = await self.client
.from("ticket_entries")
.delete()
.eq("id", created?.id ?? "");
expect(error).toBeNull();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+22
to
+38
| const { data: created } = await self.client | ||
| .from("budgets") | ||
| .insert({ | ||
| user_id: self.userId, | ||
| period_type: "monthly", | ||
| period_start: "2026-08-01", | ||
| amount: 10000, | ||
| }) | ||
| .select() | ||
| .single(); | ||
|
|
||
| const { data, error } = await stranger.client | ||
| .from("budgets") | ||
| .select() | ||
| .eq("id", created?.id ?? ""); | ||
| expect(error).toBeNull(); | ||
| expect(data).toHaveLength(0); |
Comment on lines
+24
to
+35
| const { data: created } = await self.client | ||
| .from("expenses") | ||
| .insert({ event_id: event.id, user_id: self.userId, category: "ticket" }) | ||
| .select() | ||
| .single(); | ||
|
|
||
| const { data, error } = await stranger.client | ||
| .from("expenses") | ||
| .select() | ||
| .eq("id", created?.id ?? ""); | ||
| expect(error).toBeNull(); | ||
| expect(data).toHaveLength(0); |
Comment on lines
+24
to
+35
| const { data: created } = await self.client | ||
| .from("ticket_entries") | ||
| .insert({ event_id: event.id, user_id: self.userId, entry_type: "lottery" }) | ||
| .select() | ||
| .single(); | ||
|
|
||
| const { data, error } = await stranger.client | ||
| .from("ticket_entries") | ||
| .select() | ||
| .eq("id", created?.id ?? ""); | ||
| expect(error).toBeNull(); | ||
| expect(data).toHaveLength(0); |
Comment on lines
+85
to
+90
| const { error } = await owner.client | ||
| .from("events") | ||
| .update({ deleted_at: new Date().toISOString() }) | ||
| .eq("id", event.id); | ||
| expect(error).toBeNull(); | ||
| }); |
Comment on lines
+95
to
+99
| await owner.client | ||
| .from("events") | ||
| .update({ deleted_at: new Date().toISOString() }) | ||
| .eq("id", event.id); | ||
|
|
Comment on lines
+127
to
+135
| await spender.client.from("expenses").insert({ | ||
| user_id: spender.userId, | ||
| event_id: event.id, | ||
| category: "ticket", | ||
| }); | ||
| await owner.client | ||
| .from("events") | ||
| .update({ deleted_at: new Date().toISOString() }) | ||
| .eq("id", event.id); |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Comment on lines
+16
to
+34
| test("無関係のユーザーは他人のイベントを編集できない", async () => { | ||
| const [owner, stranger] = await Promise.all([createTestUser(), createTestUser()]); | ||
| const event = await createEvent(owner); | ||
|
|
||
| const { data, error } = await stranger.client | ||
| .from("events") | ||
| .update({ title: "hijacked" }) | ||
| .eq("id", event.id) | ||
| .select(); | ||
| expect(error).toBeNull(); | ||
| expect(data).toHaveLength(0); | ||
|
|
||
| const { data: unchanged } = await owner.client | ||
| .from("events") | ||
| .select("title") | ||
| .eq("id", event.id) | ||
| .single(); | ||
| expect(unchanged?.title).toBe("test event"); | ||
| }); |
There was a problem hiding this comment.
docs/permissions.mdの「最小の検証セット」8項目のうち、「オーナー以外がイベントを削除 → 失敗」に対応するテストが見当たりません。このファイルには「オーナー以外がイベントを更新できない」テスト(16-34行目)はありますが、deleted_atを設定しようとする削除の試行はすべてowner.clientから行われており(47, 64, 81, 95, 111, 124行目)、無関係のユーザーやオーナー以外が削除を試みて弾かれることを確認するテストがありません。
events_update_owner_onlyポリシーはUPDATE全般(title変更もdeleted_at設定も)を同じUSING/WITH CHECK句でカバーしているため実質的には守られているはずですが、最小検証セットが明示的にこの項目を独立して挙げているのは、将来DELETE専用ポリシーや別のガード条件に分岐した場合の回帰を検出するためだと考えられます。ポリシーを一時的に無効化して赤くなることを確認する対象としても、独立したテストを追加した方が安全です。
Comment on lines
+57
to
+76
| test("招待時にvisibilityをpublicへ上書きしようとすると失敗する", async () => { | ||
| const [owner, inviter, invitee] = await Promise.all([ | ||
| createTestUser(), | ||
| createTestUser(), | ||
| createTestUser(), | ||
| ]); | ||
| const event = await createEvent(owner); | ||
| await inviter.client | ||
| .from("event_participants") | ||
| .insert({ event_id: event.id, user_id: inviter.userId, status: "considering" }); | ||
|
|
||
| const { error } = await inviter.client.from("event_participants").insert({ | ||
| event_id: event.id, | ||
| user_id: invitee.userId, | ||
| invited_by: inviter.userId, | ||
| status: "considering", | ||
| visibility: "public", | ||
| }); | ||
| expect(error).not.toBeNull(); | ||
| }); |
There was a problem hiding this comment.
PR概要には「招待経路でvisibility/participation_stateを上書きできないこと」を否定側で検証したとありますが、実際にこのファイルで明示的に失敗を確認しているのはvisibilityのみです(このテスト)。participation_stateについては31-55行目の正常系テストで挿入後の値が"joined"であることをアサートしているだけで、招待時にparticipation_stateを明示的に上書きしようとして失敗する、という否定側のテストがありません。
docs/testing.mdが「正常系だけのテストは、権限が全開放されていても通る。否定側が本体」と明記している通り、31-55行目のテストはpayloadにparticipation_stateを含めていないため、仮にwith check句からparticipation_state = 'joined'の制約が抜け落ちても(visibility同様に)このテストは通り続けてしまいます。57-76行目と対になるparticipation_stateの上書き失敗テストの追加をおすすめします。
| "gen:types": "supabase gen types typescript --local > supabase/types.ts" | ||
| }, | ||
| "dependencies": { | ||
| "@supabase/supabase-js": "^2", |
There was a problem hiding this comment.
nit: @supabase/supabase-jsは現時点でtest/db/helpers.tsからのみ参照されており(lib/はまだ.gitkeepのみ)、dependenciesではなくdevDependenciesの方が実態に合いそうです。lib/で実際にSupabaseクライアントを使う実装が入るタイミングでdependenciesに昇格させる形でも良いかもしれません。ブロッカーではありません。
レビュー総評
良い点
指摘した点(インラインコメント参照)
スコープ外として妥当と判断した点 1と2は「弾かれるはずの経路が本当に弾かれているか」を直接検証していない箇所で、 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.
Suppressed comments (16)
test/db/budgets.test.ts:2
- このファイル内で
Database型を参照してsatisfiesを使うため、生成型の import を追加してください(型アサーションasを避けられます)。
import { expect, test } from "vitest";
import { createTestUser } from "./helpers";
test/db/budgets.test.ts:90
as constによる型アサーションは避け、satisfiesで Insert 型に一致することを検証してください(テストコードでも型の握りつぶしを避けたいです)。
period_type: "monthly" as const,
period_start: "2026-09-01",
amount: 10000,
};
test/db/budgets.test.ts:26
- 作成用 insert の結果(特に error と created.id)を検証せずに
created?.id ?? ""で続行すると、前提作成が失敗してもテストが偽陽性で通る可能性があります。insert の error と作成結果を明示的に検証してください。
const { data: created } = await self.client
.from("budgets")
.insert({
user_id: self.userId,
period_type: "monthly",
test/db/budgets.test.ts:47
- 作成用 insert が失敗して
createdが null でもid = created?.id ?? ""だと update/delete が 0 件になり、意図と違う理由でテストが通る可能性があります。insert の error と created.id を先に検証してから id を使ってください。
const { data: created } = await self.client
.from("budgets")
.insert({
user_id: self.userId,
period_type: "monthly",
test/db/budgets.test.ts:72
- 前提となる budget 作成に失敗しても
created?.id ?? ""で delete してしまうと、削除が 0 件でも error が null のままになり偽陽性になる可能性があります。insert の error/id を検証し、確実に作成できた id で削除してください。
const { data: created } = await user.client
.from("budgets")
.insert({
user_id: user.userId,
period_type: "monthly",
test/db/ticket-entries.test.ts:28
- 作成用 insert の結果(特に error と created.id)を検証せずに
created?.id ?? ""で続行すると、前提作成が失敗してもテストが偽陽性で通る可能性があります。insert の error と作成結果を明示的に検証してください。
const { data: created } = await self.client
.from("ticket_entries")
.insert({ event_id: event.id, user_id: self.userId, entry_type: "lottery" })
.select()
.single();
test/db/ticket-entries.test.ts:50
- 前提となる ticket_entry 作成に失敗しても
id = created?.id ?? ""だと update/delete が 0 件になり、意図と違う理由でテストが通る可能性があります。insert の error/id を検証してから id を使ってください。
const { data: created } = await self.client
.from("ticket_entries")
.insert({ event_id: event.id, user_id: self.userId, entry_type: "lottery" })
.select()
.single();
const id = created?.id ?? "";
test/db/ticket-entries.test.ts:74
- 前提となる ticket_entry 作成に失敗しても
created?.id ?? ""で delete すると、削除 0 件でも error が null のままになり偽陽性になり得ます。insert の error/id を検証し、作成できた id で削除してください。
const { data: created } = await self.client
.from("ticket_entries")
.insert({ event_id: event.id, user_id: self.userId, entry_type: "lottery" })
.select()
.single();
test/db/expenses.test.ts:28
- 作成用 insert の結果(特に error と created.id)を検証せずに
created?.id ?? ""で続行すると、前提作成が失敗してもテストが偽陽性で通る可能性があります。insert の error と作成結果を明示的に検証してください。
const { data: created } = await self.client
.from("expenses")
.insert({ event_id: event.id, user_id: self.userId, category: "ticket" })
.select()
.single();
test/db/expenses.test.ts:50
- 前提となる expense 作成に失敗しても
id = created?.id ?? ""だと update/delete が 0 件になり、意図と違う理由でテストが通る可能性があります。insert の error/id を検証してから id を使ってください。
const { data: created } = await self.client
.from("expenses")
.insert({ event_id: event.id, user_id: self.userId, category: "ticket" })
.select()
.single();
const id = created?.id ?? "";
test/db/expenses.test.ts:70
- 前提となる expense 作成に失敗しても
created?.id ?? ""で delete すると、削除 0 件でも error が null のままになり偽陽性になり得ます。insert の error/id を検証し、作成できた id で削除してください。
const { data: created } = await self.client
.from("expenses")
.insert({ event_id: event.id, user_id: self.userId, category: "ticket" })
.select()
.single();
test/db/event-participants.test.ts:127
- 前提となる target の参加行作成に失敗しても、その後の update/delete が 0 件になりテストが偽陽性で通る可能性があります。前提 insert の成功(error=null)を検証してください。
createTestUser(),
]);
const event = await createEvent(owner);
test/db/event-participants.test.ts:153
- 前提となる参加登録 insert の error を無視すると、参加登録ができていない状態でも delete が 0 件で成功扱いになり、テストが偽陽性で通る可能性があります。前提 insert の成功を検証してください。
test("本人は自分の参加登録を取りやめられる", async () => {
const [owner, self] = await Promise.all([createTestUser(), createTestUser()]);
const event = await createEvent(owner);
test/db/event-participants.test.ts:179
- このテストは「非公開の参加行が他ユーザーから見えない」ことを検証していますが、前提 insert が失敗して行自体が存在しない場合でも同じ期待値(0件)になり偽陽性になります。前提 insert の成功(error=null)を検証してください。
createTestUser(),
]);
const event = await createEvent(owner);
package.json:19
- 現状
@supabase/supabase-jsはtest/db/helpers.tsでのみ使われており、プロダクション依存に入れる理由がありません。不要な本番依存を避けるためdevDependenciesへ移動してください。
"dependencies": {
"@supabase/supabase-js": "^2",
"next": "16.3.0",
"react": "19.2.8",
"react-dom": "19.2.8"
test/db/event-participants.test.ts:66
- このテストは「参加登録済みであること」を前提に visibility 上書きの拒否を検証しています。前提 insert の error を無視すると、前提が満たせていない(=招待権限がない)理由で失敗してもテストが通ってしまう可能性があります。前提 insert の成功を明示的に検証してください。
This issue also appears in the following locations of the same file:
- line 125
- line 151
- line 177
createTestUser(),
]);
const event = await createEvent(owner);
レビュー結果
主な指摘
良かった点
CI( |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated 1 comment.
Suppressed comments (10)
test/db/budgets.test.ts:87
as constの型アサーションは不要です(このリポジトリでは型アサーションを避けたい)。ここは文字列リテラルのままで型が通るので削除できます。
period_type: "monthly" as const,
test/db/ticket-entries.test.ts:33
- セットアップ用のinsertで
errorを検証しておらず、created?.id ?? ""のフォールバックにより「insert自体が失敗してcreatedがundefinedでも、空文字idでselectして0件になりテストが通る」状態になっています。セットアップが成功したことを明示的に検証してから、実IDで検証してください。
const { data: created } = await self.client
.from("ticket_entries")
.insert({ event_id: event.id, user_id: self.userId, entry_type: "lottery" })
.select()
.single();
const { data, error } = await stranger.client
.from("ticket_entries")
.select()
.eq("id", created?.id ?? "");
test/db/ticket-entries.test.ts:56
- セットアップ用のinsertで
errorを検証しておらず、created?.id ?? ""により、insert失敗時でも空文字idに対するupdate/deleteが0件扱いになってテストが通る可能性があります。セットアップ成功を検証し、実IDで更新/削除不可を確認してください。
const { data: created } = await self.client
.from("ticket_entries")
.insert({ event_id: event.id, user_id: self.userId, entry_type: "lottery" })
.select()
.single();
const id = created?.id ?? "";
const { data: updated } = await stranger.client
.from("ticket_entries")
.update({ provider: "hijacked" })
.eq("id", id)
.select();
test/db/expenses.test.ts:33
- セットアップ用のinsertで
errorを検証しておらず、created?.id ?? ""のフォールバックにより「insert失敗でも空文字idでselectして0件になりテストが通る」状態になっています。セットアップ成功を検証してから、実IDで0件になることを確認してください。
const { data: created } = await self.client
.from("expenses")
.insert({ event_id: event.id, user_id: self.userId, category: "ticket" })
.select()
.single();
const { data, error } = await stranger.client
.from("expenses")
.select()
.eq("id", created?.id ?? "");
test/db/expenses.test.ts:56
- セットアップ用のinsertで
errorを検証しておらず、created?.id ?? ""により、insert失敗時でも空文字idに対するupdate/deleteが0件扱いになってテストが通る可能性があります。セットアップ成功を検証し、実IDで更新/削除不可を確認してください。
const { data: created } = await self.client
.from("expenses")
.insert({ event_id: event.id, user_id: self.userId, category: "ticket" })
.select()
.single();
const id = created?.id ?? "";
const { data: updated } = await stranger.client
.from("expenses")
.update({ memo: "hijacked" })
.eq("id", id)
.select();
test/db/budgets.test.ts:37
- セットアップ用のinsertで
errorを検証しておらず、created?.id ?? ""のフォールバックにより「insert失敗でも空文字idでselectして0件になりテストが通る」状態になっています。セットアップ成功を検証してから、実IDで0件になることを確認してください。
const { data: created } = await self.client
.from("budgets")
.insert({
user_id: self.userId,
period_type: "monthly",
period_start: "2026-08-01",
amount: 10000,
})
.select()
.single();
const { data, error } = await stranger.client
.from("budgets")
.select()
.eq("id", created?.id ?? "");
expect(error).toBeNull();
test/db/budgets.test.ts:60
- セットアップ用のinsertで
errorを検証しておらず、created?.id ?? ""により、insert失敗時でも空文字idに対するupdate/deleteが0件扱いになってテストが通る可能性があります。セットアップ成功を検証し、実IDで更新/削除不可を確認してください。
const { data: created } = await self.client
.from("budgets")
.insert({
user_id: self.userId,
period_type: "monthly",
period_start: "2026-08-01",
amount: 10000,
})
.select()
.single();
const id = created?.id ?? "";
const { data: updated } = await stranger.client
.from("budgets")
.update({ amount: 1 })
.eq("id", id)
.select();
expect(updated).toHaveLength(0);
.github/workflows/supabase.yml:65
supabase status -o envはSUPABASE_SERVICE_ROLE_KEYなども出力するため、現状のパイプだと service role キーも含めて$GITHUB_ENVに書き込まれてしまいます。また--override-nameはCLIのヘルプ/ドキュメント上で一般的に確認できず、バージョンによっては失敗してdb-testが落ちるリスクがあります。必要な値だけを抽出してAPI_URL/ANON_KEYにマップする形にすると安全です。
supabase status -o env --override-name api.url=API_URL --override-name auth.anon_key=ANON_KEY \
| sed 's/^\([A-Z_]*\)="\(.*\)"$/\1=\2/' >> "$GITHUB_ENV"
package.json:20
@supabase/supabase-jsが現状test/db/helpers.tsでのみ使われているため、ランタイム依存ではなくdevDependenciesに置いた方が本番バンドル/インストール対象を増やさずに済みます。
"dependencies": {
"@supabase/supabase-js": "^2",
"next": "16.3.0",
"react": "19.2.8",
"react-dom": "19.2.8"
},
test/db/helpers.ts:37
createTestUser()がauth.signUpを多用していますが、supabase/config.tomlの[auth.rate_limit] sign_in_sign_ups = 30に対して、このPRのdbテスト全体ではサインアップ回数が30回を大きく超えるため、CIでレートリミットに当たりテストが不安定になるリスクがあります。テストスイート内でユーザーを再利用する(各ファイルでbeforeAllで必要人数だけ作って使い回す等)など、サインアップ回数を抑える構成にするのが安全です。
export const createTestUser = async (): Promise<TestUser> => {
const client = createAnonClient();
const email = `test-${crypto.randomUUID()}@example.com`;
const { data, error } = await client.auth.signUp({
email,
password: crypto.randomUUID(),
});
if (error || !data.user) {
throw new Error(`テストユーザー作成に失敗しました: ${error?.message ?? "unknown error"}`);
}
return { client, userId: data.user.id };
| @@ -0,0 +1,63 @@ | |||
| import { createClient, type SupabaseClient } from "@supabase/supabase-js"; | |||
| import type { Database } from "@/supabase/types"; | |||
event_participants_insert_self_or_inviteのEXISTSサブクエリが招待者自身の RLS越しに評価され、参加登録済みでも「参加登録していない」と誤判定されて 正当な招待が失敗するバグがCIのdb-testで発見された。SECURITY DEFINER関数 is_event_participant()でラップし、guard_event_deletion()と同じ方式で解消する。
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (12)
.github/workflows/supabase.yml:65
supabase status -o envに--override-nameを付けていますが、Supabase CLI ではそのフラグがサポートされていない可能性が高く、ここでジョブが失敗します。必要なキー(API_URL / ANON_KEY)だけをフィルタして $GITHUB_ENV に書き込む形にすると、安全に service_role も除外できます。
supabase status -o env --override-name api.url=API_URL --override-name auth.anon_key=ANON_KEY \
| sed 's/^\([A-Z_]*\)="\(.*\)"$/\1=\2/' >> "$GITHUB_ENV"
test/db/ticket-entries.test.ts:33
- このテストは
createdが作れなかった場合でも.eq("id", created?.id ?? "")が空文字で検索して 0 件になり、RLSが正しくても/壊れていてもテストが通ってしまいます。作成エラーを明示的に検証し、created.idを使って検索してください。
.eq("id", created?.id ?? "");
test/db/ticket-entries.test.ts:50
created?.id ?? ""で空文字を使うと、作成に失敗しても更新/削除が 0 件のままになりテストが意図せず通る可能性があります。作成のerrorを検証してからcreated.idを使ってください。
const id = created?.id ?? "";
test/db/helpers.ts:2
- Vitest(db project) 側で tsconfig の path alias(@/*) が解決されない構成だと、この
@/supabase/typesimport が実行時に解決できずテストが落ちます。テスト側は相対パス import にしておくと確実です。
import type { Database } from "@/supabase/types";
test/db/ticket-entries.test.ts:79
created?.id ?? ""だと、作成に失敗しても delete が空文字条件で 0 件のまま成功扱いになりテストが通ってしまいます。作成結果を検証してcreated.idを使ってください。
.eq("id", created?.id ?? "");
test/db/expenses.test.ts:33
- このテストは
createdが作れなかった場合でも.eq("id", created?.id ?? "")が空文字で検索して 0 件になり、テストが意図せず通る可能性があります。作成エラーを明示的に検証し、created.idを使って検索してください。
.eq("id", created?.id ?? "");
test/db/expenses.test.ts:50
created?.id ?? ""で空文字を使うと、作成に失敗しても update/delete が 0 件のままになりテストが意図せず通る可能性があります。作成のerrorを検証してからcreated.idを使ってください。
const id = created?.id ?? "";
test/db/expenses.test.ts:72
created?.id ?? ""だと、作成に失敗しても delete が空文字条件で 0 件のまま成功扱いになりテストが通ってしまいます。作成結果を検証してcreated.idを使ってください。
const { error } = await self.client.from("expenses").delete().eq("id", created?.id ?? "");
test/db/budgets.test.ts:36
- このテストは
createdが作れなかった場合でも.eq("id", created?.id ?? "")が空文字で検索して 0 件になり、RLSが壊れていてもテストが通ってしまいます。作成エラーを明示的に検証し、created.idを使って検索してください。
.eq("id", created?.id ?? "");
test/db/budgets.test.ts:53
created?.id ?? ""で空文字を使うと、作成に失敗しても update/delete が 0 件のままになりテストが意図せず通る可能性があります。作成のerrorを検証してからcreated.idを使ってください。
const id = created?.id ?? "";
test/db/budgets.test.ts:79
created?.id ?? ""だと、作成に失敗しても delete が空文字条件で 0 件のまま成功扱いになりテストが通ってしまいます。作成結果を検証してcreated.idを使ってください。
const { error } = await user.client.from("budgets").delete().eq("id", created?.id ?? "");
test/db/budgets.test.ts:87
- このリポジトリでは
asキャスト禁止なので、"monthly" as constは避けてください。文字列リテラル型の変数を用意すればキャスト無しで同じ意図を表現できます。
period_type: "monthly" as const,
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (13)
test/db/helpers.ts:2
@/*の tsconfig paths は Vitest/Vite の解決ではデフォルトで効かないため、ここがCannot find module '@/supabase/types'で落ちる可能性が高いです（vitest.config.ts に alias 設定や tsconfig-paths 系プラグインが無い）。テスト側は相対パスで import するか、別途 Vitest 側で alias を設定してください。
import { createClient, type SupabaseClient } from "@supabase/supabase-js";
import type { Database } from "@/supabase/types";
.github/workflows/supabase.yml:60
supabase status -o envは通常SERVICE_ROLE_KEYも含め多数の値を出力します。このまま>> $GITHUB_ENVすると service_role キーも環境変数として流れ込み、コメントの「エクスポートしない」と矛盾します。必要なAPI_URL/ANON_KEYのみに絞って書き込むようにしてください。
# test/db/ が認証済みクライアントでRLSを検証するために接続情報を渡す。
# service_roleキーはエクスポートしない(test/db/では使わない。docs/permissions.md)。
- name: Export local Supabase connection info
if: steps.check.outputs.initialized == 'true'
# supabase status -o env は値をダブルクォートで囲んで出力するが、
test/db/ticket-entries.test.ts:28
- このテストは insert の
errorを検証していないため、作成に失敗してcreatedが null の場合でも.eq("id", "")になってしまい、後続の「0件」を満たして誤ってパスする可能性があります。セットアップ(insert)が成功したことを先に assert してください。
const { data: created } = await self.client
.from("ticket_entries")
.insert({ event_id: event.id, user_id: self.userId, entry_type: "lottery" })
.select()
.single();
test/db/ticket-entries.test.ts:51
- このテストも insert の失敗を検知できず、
idが空文字になって update/delete が「0件」で通ってしまう可能性があります。作成が成功したことを assert してからidを使ってください。
const { data: created } = await self.client
.from("ticket_entries")
.insert({ event_id: event.id, user_id: self.userId, entry_type: "lottery" })
.select()
.single();
const id = created?.id ?? "";
test/db/ticket-entries.test.ts:79
- delete 対象の insert 失敗時に
created?.id ?? ""になると、削除が何も起きずerror=nullのままテストが誤ってパスし得ます。事前に insert 成功と id を確定させてください。
const { data: created } = await self.client
.from("ticket_entries")
.insert({ event_id: event.id, user_id: self.userId, entry_type: "lottery" })
.select()
.single();
const { error } = await self.client
.from("ticket_entries")
.delete()
.eq("id", created?.id ?? "");
test/db/expenses.test.ts:28
- このテストは insert の
errorを検証していないため、作成に失敗してcreatedが null の場合でも.eq("id", "")になり、後続の「0件」で誤ってパスする可能性があります。セットアップ(insert)の成功を先に assert してください。
const { data: created } = await self.client
.from("expenses")
.insert({ event_id: event.id, user_id: self.userId, category: "ticket" })
.select()
.single();
test/db/expenses.test.ts:51
- このテストも insert 失敗時に
idが空文字になって update/delete が「0件」で通ってしまう可能性があります。作成が成功したことを assert してからidを使ってください。
const { data: created } = await self.client
.from("expenses")
.insert({ event_id: event.id, user_id: self.userId, category: "ticket" })
.select()
.single();
const id = created?.id ?? "";
test/db/expenses.test.ts:73
- delete 対象の insert 失敗時に
created?.id ?? ""になると、削除が何も起きずerror=nullのままテストが誤ってパスし得ます。事前に insert 成功と id を確定させてください。
const { data: created } = await self.client
.from("expenses")
.insert({ event_id: event.id, user_id: self.userId, category: "ticket" })
.select()
.single();
const { error } = await self.client.from("expenses").delete().eq("id", created?.id ?? "");
expect(error).toBeNull();
test/db/budgets.test.ts:31
- このテストは insert の
errorを検証していないため、作成に失敗してcreatedが null の場合でも.eq("id", "")になり、後続の「0件」で誤ってパスする可能性があります。セットアップ(insert)の成功を先に assert してください。
const { data: created } = await self.client
.from("budgets")
.insert({
user_id: self.userId,
period_type: "monthly",
period_start: "2026-08-01",
amount: 10000,
})
.select()
.single();
test/db/budgets.test.ts:54
- このテストも insert 失敗時に
idが空文字になって update/delete が「0件」で通ってしまう可能性があります。作成が成功したことを assert してからidを使ってください。
const { data: created } = await self.client
.from("budgets")
.insert({
user_id: self.userId,
period_type: "monthly",
period_start: "2026-08-01",
amount: 10000,
})
.select()
.single();
const id = created?.id ?? "";
test/db/budgets.test.ts:80
- delete 対象の insert 失敗時に
created?.id ?? ""になると、削除が何も起きずerror=nullのままテストが誤ってパスし得ます。事前に insert 成功と id を確定させてください。
const { data: created } = await user.client
.from("budgets")
.insert({
user_id: user.userId,
period_type: "monthly",
period_start: "2026-08-01",
amount: 10000,
})
.select()
.single();
const { error } = await user.client.from("budgets").delete().eq("id", created?.id ?? "");
expect(error).toBeNull();
test/db/budgets.test.ts:89
- このリポジトリでは
asキャスト禁止です（CLAUDE.md 29行目）。ここはas constを使わず、型注釈やsatisfiesなどでリテラル型を維持してください。
const payload = {
user_id: user.userId,
period_type: "monthly" as const,
period_start: "2026-09-01",
amount: 10000,
supabase/migrations/20260806045042_fix_invite_check_rls_recursion.sql:11
- この
SECURITY DEFINER関数は (event_id, user_id) を任意に渡せるため、RPC 経由で「特定ユーザーが特定イベントに参加しているか」を第三者が推測できる経路になります（RLSで event_participants を隠しても回避されうる）。少なくともp_user_idを受け取らずauth.uid()のみに固定した関数にし、REVOKE/GRANT EXECUTEでanonからの呼び出しも遮断する設計に寄せるのが安全です。
create function public.is_event_participant(p_event_id uuid, p_user_id uuid)
returns boolean
language sql
security definer
set search_path = public
- events.test.ts: テスト名が「編集・削除できない」なのに削除 (deleted_at更新)を試していなかったため追加 - events.test.ts / event-participants.test.ts: 残っていたセットアップ insertのerror未チェック箇所を修正 - budgets/expenses/ticket-entries.test.ts: events/event_participants にはある「他人になりすまして作成できない」テストを対称性のため追加
| expect(error).not.toBeNull(); | ||
| }); | ||
|
|
||
| test("本人は自分の参加行のvisibilityを変更できる", async () => { |
There was a problem hiding this comment.
nit: docs/permissions.md の権限マトリクスには「自分のステータス変更 ○」の行がありますが、これを直接検証する正常系テスト(UPDATEでstatusを変更できること)は無いようです。visibilityのUPDATE成功例はここにありますが、statusのUPDATE成功例はinsert時の初期値でしか確認していません。必須ではありませんが、マトリクスの行をそのままテストに写す方針(docs/permissions.md「3. マトリクスを表のままテストに写す」)に沿えば、statusのUPDATE成功ケースも1つ足しておくと対称性が取れそうです。
レビュー総評
確認した内容
軽微な指摘(inline)
確認しておきたい点(コード上は判断できないため質問)
以上、テスト内容自体はスキーマ・RLSポリシーと整合しており、否定側中心の網羅も十分だと判断しました。 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (4)
test/db/budgets.test.ts:119
period_type: "monthly" as constのasキャストは、このリポジトリのルール(例: CLAUDE.md:29)に反します。ここは値を直接.insert()に渡してコンテキスト型付けに任せるか、型注釈でリテラル型を保持してください。
const payload = {
user_id: user.userId,
period_type: "monthly" as const,
period_start: "2026-09-01",
amount: 10000,
test/db/events.test.ts:152
- このテストは
.update(...).eq(...)のerror === nullしか見ておらず、RLSのUSING句で0件更新になっても成功扱いで通ってしまいます。RETURNINGか再SELECTでdeleted_atが実際にセットされたことまで検証してください。
const { error } = await owner.client
.from("events")
.update({ deleted_at: new Date().toISOString() })
.eq("id", event.id);
expect(error).toBeNull();
test/db/events.test.ts:201
- このケースも削除(update)の結果を
errorだけで判定しているため、RLSで0件更新になっても「削除済みイベントが支出保持者には見える」を検証できません。deleted_atが非NULLになったことを確認してから閲覧可否をテストしてください。
const deleteResult = await owner.client
.from("events")
.update({ deleted_at: new Date().toISOString() })
.eq("id", event.id);
expect(deleteResult.error).toBeNull();
test/db/events.test.ts:162
- 削除後の可視性をテストしていますが、削除(update)自体が0件更新で失敗しても
error === nullで通ってしまい、意図した前提(削除済み)が保証されません。deleted_atが実際にセットされたことを.select()等で確認してください。
This issue also appears on line 197 of the same file.
const deleteResult = await owner.client
.from("events")
.update({ deleted_at: new Date().toISOString() })
.eq("id", event.id);
expect(deleteResult.error).toBeNull();
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (1)
test/db/budgets.test.ts:117
as constの型アサーションが入っていますが、このリポジトリではasキャストを使わない運用になっています（CLAUDE.md に明記）。ここは生成型でもperiod_type: stringのためアサーション不要なので削除してください。
period_type: "monthly" as const,
| const user = await createTestUser(); | ||
| const payload = { | ||
| user_id: user.userId, | ||
| period_type: "monthly" as const, |
There was a problem hiding this comment.
nit: as const はCLAUDE.mdの「as によるキャストを使わない」に抵触します。しかも period_type は生成型上 string(supabase/types.ts)で、check 制約由来のリテラル型ではないため、この as const は型エラー回避としても不要に見えます。実際、同PR内の ticket_entries.test.ts の entry_type: "lottery" や expenses.test.ts の category: "ticket" は同じ構造(CHECK制約付きtext列)でキャスト無しの素のリテラルを渡しており、一貫性のためにもここも as const を外して問題ないはずです。
レビュー総評
良い点
指摘
確認できなかった点
総じて、権限マトリクスと最小検証セットに対する網羅性・否定側テストの質は方針に沿っており、致命的な問題は見つかりませんでした。上記1件はnitpickです。 |
共有payloadオブジェクトをやめ、2回とも直接リテラルを書く形にして 型アサーションを不要にした。
Owner
Author
レビュー指摘の総括分類(このPR全体を通して)多数のラウンドで指摘があったため、最終まとめとして分類する。 本物の修正(実装したRLSポリシー自体の不具合をこのPRのテストで発見)
妥当な指摘(テストカバレッジ・堅牢性)→ 全て対応済み
見送り(誤検知/対応不要と判断)
CI(lint/typecheck/unit-test/db-test/types-check/claude-review/codex-review)は最終コミットで全てgreen。db-test は50件のRLSテストが全てpassしている。 |
レビュー総評
CLAUDE.md絶対ルールとの照合
CI
1点、既出だが確認した内容
以上、独立した確認でも新たなブロッキング事項は見つからなかった。方針への準拠度は高く、承認できる水準。 |
This was referenced Aug 6, 2026
reitojike
added a commit
that referenced
this pull request
Aug 8, 2026
判断ポイントは1 Issueに3個まで(5個超で分割)、PR差分は300行を目安とする。 記事の実測値をそのまま採らず、main にマージ済みのPR 26本(中央値135行、 300行以下65%)で裏を取ってから採用した。行数の数え方から生成物・ロック ファイル・権限マトリクスを写したテスト表を除外する根拠も、超過した PR #16 / #32 の実態から示した。 機械的ゲートにしない旨と、3回ルール(モデルを上げる) / PO確認(判断を 下せる層に上げる) / 粒度超過(Issueを分ける)の対処の違いを表で整理。 docs/roadmap.md はポインタ1行に留め、根拠は1箇所にだけ置く。 Refs #44
reitojike
added a commit
that referenced
this pull request
Aug 8, 2026
PR #68 のClaude Reviewの指摘に対応。指摘は正しかった。 誤り: 「300行を超えた5本」は事実と異なり、素の差分では9本だった (#31 307 / #59 365 / #60 380 / #30 441 / #29 617 / #18 679 / #41 698 / #32 1063 / #16 3528)。同じ本文が書いていた「300行以下が65%(17/26)」は 超過9本を含意しており、記述が自己矛盾していた。 原因: 母集団の統計(中央値・65%)は素の差分で数え、外れ値の説明だけに 除外規則を適用していた。数え方を混在させたうえ、超過リストを上位5本で 打ち切って全件確認しなかった。 訂正: 26本すべてを git show --numstat で数え直し、除外の段階ごとに 表で示す。素の差分(中央値135行 / 65% / 超過9本)、パス名で機械的に 判定できる除外まで(中央値128行 / 85% / 超過4本)、除外規則を最後まで 適用(超過2本 = #59・#60)。落ちる7本の内訳も明記した。 あわせて、3段階目の中央値を出さない理由を書いた。supabase/config.toml は #29 では supabase init の出力(416行)、#56 では根拠コメント付きで手で 直した6行で、同じパスでも扱いが逆になる。パス名では決まらないことが、 この節が機械的ゲートになり得ない理由そのものなので、「lintではない」の 段落の根拠もこの実測に差し替えた。 Refs #44
reitojike
added a commit
that referenced
this pull request
Aug 8, 2026
reitojike
added a commit
that referenced
this pull request
Aug 8, 2026
* docs: Issueの粒度とPR差分サイズの目安をCLAUDE.mdに明文化 判断ポイントは1 Issueに3個まで(5個超で分割)、PR差分は300行を目安とする。 記事の実測値をそのまま採らず、main にマージ済みのPR 26本(中央値135行、 300行以下65%)で裏を取ってから採用した。行数の数え方から生成物・ロック ファイル・権限マトリクスを写したテスト表を除外する根拠も、超過した PR #16 / #32 の実態から示した。 機械的ゲートにしない旨と、3回ルール(モデルを上げる) / PO確認(判断を 下せる層に上げる) / 粒度超過(Issueを分ける)の対処の違いを表で整理。 docs/roadmap.md はポインタ1行に留め、根拠は1箇所にだけ置く。 Refs #44 * docs: 判断ポイント数の境界を一本化し、裏取り済みの数値と外部実測を書き分ける PR #68 のClaude Reviewの指摘2件に対応。 指摘1: 「3個まで、5個を超えるなら分割」で4個の扱いが未定義だった。 閾値を「3個まで。4個目が出てきたら分ける」に一本化する。機械的ゲートに しない方針である以上、「検討」と「必ず」の二段構えは実効性のない false precisionになるため、緩衝域を作らず単一の線にした。 指摘2: 裏取り済みの300行と、外部実測のままの3個が同じ文脈に並んでいた。 「2つの数字は裏付けの強さが違う」として段落を分け、300行はこのリポジトリの 実測(PR 26本、中央値135行/300行以下65%)で検証済み、3個は外部実測のみを 根拠とする未検証のヒューリスティックであると明示した。過去Issueの判断数は 記録がなく後から数え直せないため、このリポジトリでの裏取りが今はできない 理由も併記。採用の根拠はコストの非対称性に置いた。 あわせて、外部実測に対応値のない4個/5個を推定して線を引いていないことと、 実績が溜まったら見直す旨を記載した。 Refs #44 * docs: PR実測の集計を数え直し、超過本数の誤りを訂正する PR #68 のClaude Reviewの指摘に対応。指摘は正しかった。 誤り: 「300行を超えた5本」は事実と異なり、素の差分では9本だった (#31 307 / #59 365 / #60 380 / #30 441 / #29 617 / #18 679 / #41 698 / #32 1063 / #16 3528)。同じ本文が書いていた「300行以下が65%(17/26)」は 超過9本を含意しており、記述が自己矛盾していた。 原因: 母集団の統計(中央値・65%)は素の差分で数え、外れ値の説明だけに 除外規則を適用していた。数え方を混在させたうえ、超過リストを上位5本で 打ち切って全件確認しなかった。 訂正: 26本すべてを git show --numstat で数え直し、除外の段階ごとに 表で示す。素の差分(中央値135行 / 65% / 超過9本)、パス名で機械的に 判定できる除外まで(中央値128行 / 85% / 超過4本)、除外規則を最後まで 適用(超過2本 = #59・#60)。落ちる7本の内訳も明記した。 あわせて、3段階目の中央値を出さない理由を書いた。supabase/config.toml は #29 では supabase init の出力(416行)、#56 では根拠コメント付きで手で 直した6行で、同じパスでも扱いが逆になる。パス名では決まらないことが、 この節が機械的ゲートになり得ない理由そのものなので、「lintではない」の 段落の根拠もこの実測に差し替えた。 Refs #44 * docs: PR #29/#32の除外理由が2段階なのに1段階しか書いていなかった記述漏れを修正 Claude Reviewの指摘どおり、#29(617→22)はsupabase/config.toml(416行) だけでなくsupabase/types.ts(179行)も、#32(1063→79)はテスト表(924行) だけでなくyarn.lock(60行)も除外して初めて数字が再現できる。 片方しか書いていなかったため、追試すると数値が合わなかった。 Refs #44
This was referenced Aug 9, 2026
Merged
reitojike
added a commit
that referenced
this pull request
Aug 9, 2026
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
reitojike
added a commit
that referenced
this pull request
Aug 9, 2026
* レビュー指摘の重複による修正ラリーを減らす 同一根拠の指摘がファイルごとに個別投稿され、修正→push→再指摘のラウンドが かさむ問題(PR #32, #60, #63, #73)への対処。指摘を直す側には横展開確認の 規律を、レビューボット側には同一根拠の指摘を1件に集約する指示を追加する。 Codexレビュー本格導入前の準備。 Closes #79 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Copilotの指摘反映: PR #32/#60の例が読点で連結され読みにくい問題を修正 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
Issue #26 の対応。
docs/permissions.mdの権限マトリクスと最小検証セットに基づき、test/db/に実DBに対するRLS検証テストを実装する。実装内容
test/db/helpers.ts—auth.signUpで実際のテストユーザー(認証済みクライアント)を作成するヘルパー。service_roleキーは一切使わないprofiles.test.ts/events.test.ts/event-participants.test.ts/ticket-entries.test.ts/expenses.test.ts/budgets.test.ts— 各テーブルのRLSポリシーを否定側中心に検証.github/workflows/supabase.ymlのdb-testジョブにsupabase status -o envの出力を環境変数として渡すステップを追加(API_URL/ANON_KEYの2行のみに絞ってGITHUB_ENVへ)特に否定側で検証している内容
visibility/participation_stateを上書きできないことprofiles_public経由ではemail/is_adminが漏れないことis_adminを書き換えられないこと、profilesをDELETEできないことbudgetsのNULLS NOT DISTINCT制約(全ジャンル合算枠の重複禁止)の回帰確認Test plan
db-testジョブで全テスト(45件)がCIでpassすることを確認済み