feat: enforce the self-attested member's limited web trust
commit
59c3519feat: enforce the self-attested member's limited web trust
The settings-edit path is the concrete enforcement point the members.provenance requirement already promised: pre_receive cannot gate this (purely key-based, and a self-attested member may have no push key at all), so the web write path now refuses a SelfAttestedWeb member’s config edit before it ever reaches stage_and_push. Config editing is the only web write path implemented so far; issues/comments (the actually-allowed set) aren’t wired to the browser yet, so there is nothing further to gate today.
Also adds members::load_all_indexed(_with), the principal-keyed batch path Phase 4d prepares, and annotates the existing O(m×k) key-to-member scan as acceptable at current scale rather than indexing it with a bi-map (a member may legitimately hold more than one key).
feat: add members::load_all_indexed/load_all_indexed_with feat: refuse a SelfAttestedWeb member’s settings edit in the web write path test: cover a self-attested member being refused a settings edit docs: annotate the O(m×k) key-to-member scan as a deferred index Assisted-by: Claude:claude-sonnet-5
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
crates/git-ents-server/tests/web_edit.rs
@@ -104,6 +104,45 @@
);
}
+#[test]
+fn a_self_attested_member_is_refused_a_settings_edit() {
+ let env = Server::start();
+ let bare = env.create_repo("repo.git");
+ env.add_server_member(&bare);
+ let alice = keygen(env.scratch(), "alice");
+ env.add_self_attested_member(&bare, "alice", &pubkey(&alice));
+
+ let cookie = env.sign_in(&alice);
+ let page = env.get("/repo.git/settings", &cookie);
+ assert!(
+ page.body.contains("name=\"csrf\""),
+ "the edit form should still render for a self-attested member"
+ );
+ let csrf = page.field("csrf").unwrap();
+
+ let edit = env.post(
+ "/repo.git/settings",
+ &cookie,
+ &form(&[
+ ("csrf", &csrf),
+ ("description", "should not land"),
+ ("homepage", ""),
+ ("topics", ""),
+ ]),
+ );
+ assert_eq!(
+ edit.status, 200,
+ "a self-attested member's edit should not redirect: {}",
+ edit.body
+ );
+ assert!(
+ !env.get("/repo.git/settings", &cookie)
+ .body
+ .contains("should not land"),
+ "the description must be unchanged"
+ );
+}
+
#[test]
fn a_non_member_is_not_offered_an_edit_form() {
let env = Server::start();
@@ -279,6 +318,38 @@
.unwrap();
}
+ /// Like [`Server::add_member`], but with `provenance/SelfAttestedWeb` —
+ /// the shape a member self-onboarded through the browser carries, still
+ /// resting on a leaf key so the challenge-response sign-in flow works.
+ fn add_self_attested_member(&self, bare: &Path, username: &str, public_key: &str) {
+ let principal = hash_object(bare, username.as_bytes());
+ let key_blob = hash_object(bare, public_key.as_bytes());
+ let keys = mktree(bare, &format!("100644 blob {key_blob}\tkey\n"));
+ let trust = mktree(bare, &format!("040000 tree {keys}\tKeys\n"));
+ let empty = mktree(bare, "");
+ let provenance = mktree(bare, &format!("040000 tree {empty}\tSelfAttestedWeb\n"));
+ let root = mktree(
+ bare,
+ &format!(
+ "100644 blob {principal}\tprincipal\n\
+ 040000 tree {empty}\tvalid_after\n\
+ 040000 tree {empty}\tvalid_before\n\
+ 040000 tree {trust}\ttrust\n\
+ 040000 tree {provenance}\tprovenance\n"
+ ),
+ );
+ let commit = git(bare, &["commit-tree", &root, "-m", "member"]).unwrap();
+ git(
+ bare,
+ &[
+ "update-ref",
+ &format!("refs/meta/member/{username}"),
+ &commit,
+ ],
+ )
+ .unwrap();
+ }
+
fn get(&self, path: &str, cookie: &str) -> Http {
let headers: Vec<(&str, &str)> = if cookie.is_empty() {
vec![]
crates/git-ents/src/members.rs
@@ -221,6 +221,30 @@
load_all_with(&git_store::Store::open(repo)?)
}
+/// Load every member recorded under [`MEMBER_NS`] from an already-open
+/// `store`, keyed by principal.
+///
+/// Prepares the batch path for lookups keyed by principal directly (there is
+/// exactly one member per principal, unlike a signing key, which a member may
+/// legitimately hold several of — that is why this indexes principals rather
+/// than a `Trust::Keys` bi-map). Not yet wired to any caller: the web layer's
+/// public-key lookup needs a different index (key → member), an O(m×k) linear
+/// scan that stays fine at current scale (see its own doc comment).
+pub fn load_all_indexed_with(
+ store: &git_store::Store,
+) -> Result<BTreeMap<String, Member>, git_store::Error> {
+ Ok(load_all_with(store)?
+ .into_iter()
+ .map(|member| (member.principal.clone(), member))
+ .collect())
+}
+
+/// Load every member recorded under [`MEMBER_NS`] in `repo`, keyed by
+/// principal. See [`load_all_indexed_with`].
+pub fn load_all_indexed(repo: &Path) -> Result<BTreeMap<String, Member>, git_store::Error> {
+ load_all_indexed_with(&git_store::Store::open(repo)?)
+}
+
/// Write `member` to its `refs/meta/member/<principal>` ref, replacing any
/// prior value, as a new commit, through an already-open `store`.
pub fn store_with(store: &git_store::Store, member: &Member) -> Result<(), git_store::Error> {
crates/git-ents-server/src/web/write.rs
@@ -171,6 +171,26 @@
}
}
+/// Refuse `username` unless they are [`Provenance::AdminRegistered`]: a
+/// self-attested web member gets limited trust and may not edit settings
+/// (outside the allowed set of issues/comments) until an admin promotes them.
+/// `pre_receive` cannot enforce this — it is purely key-based, and a
+/// self-attested member typically has no push key to gate — so the web write
+/// path is the enforcement point.
+fn require_admin_registered(store: &git_store::Store, username: &str) -> Result<(), String> {
+ use git_ents::members::Provenance;
+ let member = git_ents::members::load_with(store, username)
+ .map_err(|e| format!("could not read member: {e}"))?
+ .ok_or_else(|| "your web key is not a member of this repository".to_owned())?;
+ match member.provenance {
+ Provenance::AdminRegistered => Ok(()),
+ Provenance::SelfAttestedWeb => Err(
+ "self-attested members may only edit issues and comments until an admin promotes them"
+ .to_owned(),
+ ),
+ }
+}
+
/// Land a configuration change: stage it on a throwaway ref authored by the
/// signed-in member, then push it onto `refs/meta/config` signed with the
/// server's key, through the `pre-receive` gate. Returns `Ok` only when the gate
@@ -205,6 +225,7 @@
let store = git_store::Store::open(repo).map_err(|e| format!("cannot open store: {e}"))?;
let username = member_for_public_key_with(&store, &public_key)
.ok_or_else(|| "your web key is not a member of this repository".to_owned())?;
+ require_admin_registered(&store, &username)?;
let mut config =
git_ents::config::load_with(&store).map_err(|e| format!("could not read config: {e}"))?;
@@ -319,6 +340,13 @@
/// The username of the member whose web key matches `public_key`, if any, from
/// an already-open `store`. The match is on the key type and body, ignoring
/// any trailing comment.
+///
+/// O(m×k): loads every member and scans each one's keys. Acceptable at
+/// current scale; a batch path exists on the other axis
+/// (`members::load_all_indexed`, principal → member) but this lookup goes the
+/// other way (key → member), which would need its own index — deferred until
+/// measured, since a member legitimately holds more than one key, ruling out
+/// a simple bi-map.
pub(super) fn member_for_public_key_with(
store: &git_store::Store,
public_key: &str,