registrystack / registrystack/registry-stack

BReg change requests: submit admits submitter targets twice, and admission blanks the target context instead of restoring it

Đang mở
#950 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
area:breg criticality:p2 rust triage:needs-implementation
Ngôn ngữ chính
Rust
Star
2
Fork
0
Merge trung bình
2 giờ 55 phút
Pull request đã merge (30 ngày)
130

Mô tả

Found in the review of PR #929 (2026-09-09). Line references are as of that branch; re-resolve after it merges. Both points are in `crates/registry-breg/src/mutation/request.rs`.

**1. A submit admits its submitter targets twice.** The generic request-action path admits them for Submit and Revise (lines 600-612), and the submit routine admits them again for the same claims (lines 1237-1255). Each call clears the change-request target context, reinstalls the target row boundaries, takes `LOCK TABLE ... IN SHARE MODE` on every target table and re-reads every target row (`admit_submitter_targets`, from line 2201). The second pass is idempotent, so the answer is right; the cost is two rounds of locks and one more place to reason about. Consolidate to one admission per mutation, or state in the code why the submit routine needs its own.

**2. `admit_submitter_targets` clears the target context instead of restoring it.** It sets `registry.change_request_target_context` to the empty string before its loop (lines 2243-2250) and leaves it empty on return. Today that is harmless: admission always runs before any target context is installed, and the coordinator uses the same clear-to-empty as "restore the ordinary request context" (lines 1042-1050). The hazard is latent. An admission that ever runs after `install_change_request_target_context` (`crates/registry-breg/src/postgres/context.rs:2213`) would silently blank a context a later statement depends on, and no test would notice, because clearing is indistinguishable from the ordinary state. Either save and restore the prior value, or assert on entry that the setting is empty so a future reordering fails loudly.

Neither point changes an authorization answer today, which is why they are a follow-up rather than a fix in #929.

Security-sensitive area (change-request admission and the settings the generated RLS policies read); needs explicit review notes when implemented.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

Bắt đầu trong crates/registry-breg/src/mutation/request.rs, tại đường dẫn Submit/Revise generic, routine submit và admit_submitter_targets; sau đó đọc phần xử lý context trong crates/registry-breg/src/postgres/context.rs. Xem lại các test admission và mutation hiện có, xác minh rằng mỗi mutation chỉ admit các target của submitter một lần, đồng thời xác nhận context của target trước đó được giữ nguyên hoặc entry không hợp lệ bị từ chối mà không thay đổi kết quả authorization.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
postgresql, rust
Lĩnh vực
backend, databases, security
Loại issue
Tái cấu trúc
Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức độ hoạt động
Sôi nổi
Độ rõ ràng
Khá rõ ràng
Mức phù hợp với người mới
48/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.