Repository navigation
docs: schedule closing the role-grant escalation gap before U-3 - #74
Merged
Merged
Conversation
A review of U-1 found the new policies on member_roles trust the role the
application claims, and nothing else:
with check (current_setting('app.current_user_role', true) = 'admin')
members is not defended that way. It carries the enforce_member_workflow
trigger alongside its policies, which checks the claimed role against what
app_accounts actually says, so a compromised application layer cannot promote
itself. That is what constraint C-4 is for.
member_roles is where every permission now comes from — a higher-value target
than members — and has the weaker of the two defences. Nothing reads the table
until U-3 moves the checks across, so there is no exposure today, but U-3 is
exactly the point where this stops being theoretical. It becomes U-2.5, ordered
before it.
Also corrects two things in U-1 itself. Its requirement list claimed FR-4.1,
which it does not implement: there is no way to create or delete a role, only
select is granted, and role management is U-5's work. And the is_system comment
described an enforcement that does not exist, while the ON CONFLICT clause on
the admin backfill could never fire, since app_accounts.member_id is unique.
Co-Authored-By: Claude Opus 4.8 <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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
U-1(#73)のレビューで見つかった不足を、作業単位として記録します。あわせて U-1 自身の誤りを2点訂正します。
見つかった不足 — ロール付与が申告だけで通る
U-1 が入れたポリシーは、アプリが申告したロールだけを見ています。
membersはこの守り方をしていません。 RLS に加えてapp_private.enforce_member_workflowが、申告されたロールをapp_accountsの実値と突き合わせます。アプリ層が侵害されても権限を偽装できないようにするためで、これが制約 C-4 の趣旨です。member_rolesは全権限の源泉であり、membersより価値の高い標的でありながら、2つのうち弱いほうの防御しかありません。現時点の危険はありません。 U-3 で判定を移すまで誰もこのテーブルを読み書きしないためです。ただし U-3 がまさにその境目なので、U-2.5 として U-3 の前に置きます。
U-1 の訂正2点
① 要件範囲の過大申告
FR-4.1「ロールを作成・変更・削除できる」を実装したと書いていましたが、していません。
app_rolesにはselectしか付与しておらず、ロールを作る経路がありません。ロール管理は U-5 の範囲です。正しくは FR-4.2・4.3・4.6 です。② コメントが存在しない強制を主張していた
is_systemに「管理者が削除できない」と書いていましたが、強制していません。現状 delete 権限自体がないため実害はありませんが、コメントを実態に合わせ、U-5 で守るべきものとして書き直しました。あわせて、管理者バックフィルの
on conflict do nothingを削除しました。app_accounts.member_idが unique なので到達しません。前提が崩れた場合は黙って飛ばすより止まるほうが安全です。🤖 Generated with Claude Code