docs(plan,spec): align enforce_owner_only_elevation to shipped parent-role authority
The plan's pre-receive-hook pseudocode judged owner-elevation authority on the post-change `signer.role` (so a self-promoting Admin reads as Owner in the same commit and self-authorizes the promotion — the exact escalation the gate exists to stop).f249395had fixed only the skip-predicate, leaving this final check vulnerable. Align the plan's `enforce_owner_only_elevation` to the SHIPPED fix (relicario-server/src/main.rs,aace6f1): derive `signer_may_manage_owners` from `signer_parent = parent_role(signer.member_id)` (the signer's PRE-commit role; None -> reject; genesis allowed) and gate on that, never the post-change role. The spec was already policy-correct in prose ("a member-role-change granting owner/admin must be signed by an owner") and did NOT carry the vulnerable implementation detail; strengthened it with an explicit pre-commit-role note so the design record pins the property and no one re-derives the vulnerable form. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TJo44YM3UbBjro2fG6NrKy
This commit is contained in:
@@ -4546,6 +4546,14 @@ fn verify_org_commit(commit: &str) -> Result<()> {
|
|||||||
/// role elevated up to Owner/Admin). On genesis (root), the sole bootstrap
|
/// role elevated up to Owner/Admin). On genesis (root), the sole bootstrap
|
||||||
/// owner the commit introduces is allowed (it has no parent baseline).
|
/// owner the commit introduces is allowed (it has no parent baseline).
|
||||||
///
|
///
|
||||||
|
/// CRITICAL: the signer's authority is judged on their role in the PARENT
|
||||||
|
/// commit (`parent_role(signer)`), NOT the post-change `signer.role` carried in
|
||||||
|
/// the commit under verification. Reading `signer.role` would let an Admin
|
||||||
|
/// self-promote to Owner in the same commit and then self-authorize that very
|
||||||
|
/// promotion (the gate would see the already-elevated role and pass) — the
|
||||||
|
/// exact escalation this exists to stop. A signer absent from the parent
|
||||||
|
/// (`None`) has no prior authority and is rejected.
|
||||||
|
///
|
||||||
/// `git_show_parent` is defined in Task C2 (same file, same crate).
|
/// `git_show_parent` is defined in Task C2 (same file, same crate).
|
||||||
fn enforce_owner_only_elevation(
|
fn enforce_owner_only_elevation(
|
||||||
commit: &str,
|
commit: &str,
|
||||||
@@ -4579,6 +4587,12 @@ fn enforce_owner_only_elevation(
|
|||||||
parent_members.iter().find(|(mid, _)| mid == id).map(|(_, r)| *r)
|
parent_members.iter().find(|(mid, _)| mid == id).map(|(_, r)| *r)
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// The signer's authority = their PARENT role. A member absent from the parent
|
||||||
|
// (brand new) has no prior authority and cannot mint owners/admins. This is
|
||||||
|
// judged BEFORE the loop and never reads the post-change `signer.role`.
|
||||||
|
let signer_parent = parent_role(signer.member_id.as_str());
|
||||||
|
let signer_may_manage_owners = signer_parent.map_or(false, |r| r.can_manage_owners());
|
||||||
|
|
||||||
for m in &new_members.members {
|
for m in &new_members.members {
|
||||||
if !is_privileged(m.role) {
|
if !is_privileged(m.role) {
|
||||||
continue;
|
continue;
|
||||||
@@ -4592,12 +4606,14 @@ fn enforce_owner_only_elevation(
|
|||||||
if parent_role(m.member_id.as_str()) == Some(m.role) {
|
if parent_role(m.member_id.as_str()) == Some(m.role) {
|
||||||
continue; // unchanged role — not an introduction or elevation
|
continue; // unchanged role — not an introduction or elevation
|
||||||
}
|
}
|
||||||
// A new owner/admin, or a member elevated to owner/admin → owner-only.
|
// A new owner/admin, or a member elevated to owner/admin → owner-only,
|
||||||
if !signer.role.can_manage_owners() {
|
// judged by the signer's PRE-commit (parent) authority — never the
|
||||||
|
// post-change `signer.role`.
|
||||||
|
if !signer_may_manage_owners {
|
||||||
eprintln!(
|
eprintln!(
|
||||||
"REJECT: org commit {commit} — member '{}' (role {:?}) may not introduce or \
|
"REJECT: org commit {commit} — member '{}' (parent role {:?}) may not introduce or \
|
||||||
elevate owner/admin '{}' to {:?}; only an owner may",
|
elevate owner/admin '{}' to {:?}; only an owner may",
|
||||||
signer.display_name, signer.role, m.display_name, m.role
|
signer.display_name, signer_parent, m.display_name, m.role
|
||||||
);
|
);
|
||||||
std::process::exit(1);
|
std::process::exit(1);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -271,7 +271,7 @@ Parses `git log` (record separator `%x1e`, field separator `%x1f` to survive mul
|
|||||||
|
|
||||||
1. **Verifies the signature** by building a temporary `allowed_signers` from `members.json` ed25519 keys, injecting `gpg.ssh.allowedSignersFile` via `GIT_CONFIG_*`, running `git verify-commit --raw`, and parsing the `SHA256:` fingerprint from stderr — the same mechanism the existing `verify-commit` uses. A commit with no good signature, or whose signer is not a current member, is rejected. (Bare `git %GF` is **not** used — it returns empty without an allowed-signers file.)
|
1. **Verifies the signature** by building a temporary `allowed_signers` from `members.json` ed25519 keys, injecting `gpg.ssh.allowedSignersFile` via `GIT_CONFIG_*`, running `git verify-commit --raw`, and parsing the `SHA256:` fingerprint from stderr — the same mechanism the existing `verify-commit` uses. A commit with no good signature, or whose signer is not a current member, is rejected. (Bare `git %GF` is **not** used — it returns empty without an allowed-signers file.)
|
||||||
2. **Authorizes the change** by inspecting `git diff-tree` paths:
|
2. **Authorizes the change** by inspecting `git diff-tree` paths:
|
||||||
- `members.json` / `collections.json` / `org.json` → signer must be owner/admin; a `member-role-change` granting owner/admin must be signed by an owner.
|
- `members.json` / `collections.json` / `org.json` → signer must be owner/admin; a `member-role-change` granting owner/admin must be signed by an owner. The signer's authority here is judged on their role in the **parent** commit (their pre-change role), never the post-change role carried in the commit under verification — otherwise an Admin could self-promote to Owner in one commit and have the gate read the already-elevated role and self-authorize. A signer absent from the parent has no prior authority and is rejected. (Genesis is the sole exception — see §4 below.)
|
||||||
- `items/<slug>/<id>.enc` → `<slug>` must be in the signing member's grants.
|
- `items/<slug>/<id>.enc` → `<slug>` must be in the signing member's grants.
|
||||||
3. **Validates schema** — `schema_version` must not decrease for any of the three JSON files (compared against `{commit}^:<file>`), and `members.json`/`collections.json` must pass `validate()`.
|
3. **Validates schema** — `schema_version` must not decrease for any of the three JSON files (compared against `{commit}^:<file>`), and `members.json`/`collections.json` must pass `validate()`.
|
||||||
4. **Handles genesis and merges** — the root commit (no parent) is the org-init genesis: it is allowed if signed by the sole owner it introduces. Merge commits are rejected (org history is linear) to avoid first-parent-only diff blind spots.
|
4. **Handles genesis and merges** — the root commit (no parent) is the org-init genesis: it is allowed if signed by the sole owner it introduces. Merge commits are rejected (org history is linear) to avoid first-parent-only diff blind spots.
|
||||||
|
|||||||
Reference in New Issue
Block a user