18181599

Refuse a settings push that leaves a roster with no rules

a73x 2026-08-11 18:35

Commit message

Refuse a settings push that leaves a roster with no rules

`keydir/` without `conf/access.conf` is not a half-written configuration,
it is an exposure. An absent `conf/access.conf` is an authoritative
absence, so the server stays ungoverned — and an ungoverned server lists
and serves that repository like any other, publishing who has access to
the forge and how many of them there are. It is also exactly the
bootstrap sequence a first-time operator follows, since `keydir/` is the
thing you have before you have rules.

Closed at the source: the push that would create the state is refused,
so the mistake is reported where it is made. That keeps the
closed-by-default posture as the one mechanism — no repository singled
out by name on the way out — and it composes with the validation that
already rejects a config nobody could push again.

The check is on the resulting tree, not the diff, so a push that deletes
`conf/access.conf` and leaves the roster standing is refused by the same
rule and with the same message. The message names the fix, because an
operator hitting this is mid-bootstrap and has no other clue.

Rule 3 is unaffected: no `settings.git` at all, and a `settings.git` with
neither file, both stay ungoverned and behave exactly as before. Only a
roster *without* rules is refused.

A repository already in this state is on disk where no hook can reach
it, so it is reported once at startup and nothing more. Hiding it would
reinstate the second mechanism, and refusing to start would take down a
server over a repository it has been serving all along.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Patch: 1beeb03e3f61a0e5b7ab6b154b1554e30eb492d5

src/server/governance/hook.rs

Old		New
@@ -210,6 +210,15 @@ fn validate_settings_push(
210	.tree()	210	.tree()
211	.map_err(\|e\| format!("{refname}: cannot read the pushed tree: {e}"))?;	211	.map_err(\|e\| format!("{refname}: cannot read the pushed tree: {e}"))?;
212		212
		213	// A roster with no rules is refused, whether it arrives by pushing
		214	// `keydir/` first or by deleting `conf/access.conf` and leaving the roster
		215	// standing. The resulting tree is what matters, not the diff: both leave a
		216	// repository that governs nothing while naming everyone who would be
		217	// governed, and an ungoverned server publishes it like any other.
		218	if let Err(reason) = super::check_roster_has_rules(&tree) {
		219	return Err(format!("{refname}: {reason}"));
		220	}
		221
213	if !live_is_config && !has_access_conf(&tree) {	222	if !live_is_config && !has_access_conf(&tree) {
214	return Ok(());	223	return Ok(());
215	}	224	}

src/server/governance/mod.rs

@@ @@ -288,6 +288,53 @@ pub fn validate_settings_tree( @@
+    Ok(governance)
+/// Refuse a settings tree that holds a roster but no rules.
+///
+/// `keydir/` without `conf/access.conf` is not a half-written configuration,
+/// it is an exposure: the repository governs nothing — an absent
+/// `conf/access.conf` is an authoritative absence, so the server stays
+/// ungoverned — while naming every operator who has access to the forge and
+/// how many of them there are. On an ungoverned server that repository is
+/// listed and clonable like any other, so the roster is published to anyone.
+///
+/// It is refused at the push that would create it rather than hidden at the
+/// request that would serve it: the mistake is reported where it is made, and
+/// the closed-by-default posture stays the one mechanism, with no repository
+/// singled out by name on the way out.
+///
+/// This is deliberately *not* the same thing as an empty `settings.git`. A
+/// repository with neither file is an ordinary repository that happens to be
+/// called `settings`, and stays as pushable as it ever was.
+pub fn check_roster_has_rules(tree: &git2::Tree<'_>) -> Result<(), String> {
+    let has_rules = tree.get_path(Path::new(ACCESS_CONF_PATH)).is_ok();
+    let has_roster = tree.get_path(Path::new(KEYDIR)).is_ok();
+    if has_roster && !has_rules {
+        return Err(format!(
+            "{KEYDIR}/ without {ACCESS_CONF_PATH}: a roster with no rules governs nothing, \
+             so this repository would be served like any other and would publish who has \
+             access to this server.\n  \
+             Push both together, or push {ACCESS_CONF_PATH} first."
+        ));
+    }
+    Ok(())
+}
+/// The same check against what is on disk right now, for a repository that
+/// reached this state before the check existed. Returns the reason if the
+/// server's settings repository is holding a roster with no rules.
+///
+/// A hook cannot fix an existing repository — it is already there — so this is
+/// reported at startup and nothing more: hiding it would be the second
+/// mechanism the closed-by-default posture exists to avoid, and refusing to
+/// start would take a running server down over a repository it has been
+/// serving all along.
+pub fn unruled_roster(repos_dir: &Path) -> Option<String> {
+    let path = settings_repo_path(repos_dir);
+    let repo = git2::Repository::open_bare(&path).ok()?;
+    let tree = repo.head().ok()?.peel_to_commit().ok()?.tree().ok()?;
+    check_roster_has_rules(&tree).err()
+}
+/// Record which principal created a repository, for `CREATOR` rules.
+pub fn record_creator(repo_path: &Path, name: &str) -> std::io::Result<()> {
+    let dir = repo_path.join(".collab");
@@ @@ -348,6 +395,86 @@ mod tests { @@
+        assert!(matches!(load(tmp.path()), GovernanceState::Absent));
+    /// Build a settings tree from `path -> content` pairs and return it.
+    fn tree_of<'a>(repo: &'a git2::Repository, files: &[(&str, &str)]) -> git2::Tree<'a> {
+        let mut index = repo.index().unwrap();
+        for (path, content) in files {
+            let blob = repo.blob(content.as_bytes()).unwrap();
+            let mut entry = git2::IndexEntry {
+                ctime: git2::IndexTime::new(0, 0),
+                mtime: git2::IndexTime::new(0, 0),
+                dev: 0,
+                ino: 0,
+                mode: 0o100644,
+                uid: 0,
+                gid: 0,
+                file_size: 0,
+                id: blob,
+                flags: 0,
+                flags_extended: 0,
+                path: path.as_bytes().to_vec(),
+            };
+            entry.file_size = content.len() as u32;
+            index.add(&entry).unwrap();
+        }
+        let oid = index.write_tree().unwrap();
+        repo.find_tree(oid).unwrap()
+    }
+    #[test]
+    fn a_roster_without_rules_is_refused_and_anything_else_is_not() {
+        let tmp = tempfile::TempDir::new().unwrap();
+        let repo = git2::Repository::init_bare(tmp.path().join("settings.git")).unwrap();
+        let roster_only = tree_of(&repo, &[("keydir/alex.pub", "ssh-ed25519 AAAA alex")]);
+        let reason = check_roster_has_rules(&roster_only)
+            .expect_err("a roster with no rules must be refused");
+        assert!(reason.contains(KEYDIR) && reason.contains(ACCESS_CONF_PATH));
+        assert!(
+            reason.contains("Push both together"),
+            "the refusal must name the fix; got {reason}"
+        );
+        // Both halves: the configuration this exists to get to.
+        assert!(check_roster_has_rules(&tree_of(
+            &repo,
+            &[
+                ("conf/access.conf", "repo settings\n    RW+ = alex\n"),
+                ("keydir/alex.pub", "ssh-ed25519 AAAA alex"),
+            ],
+        ))
+        .is_ok());
+        // Neither half: an ordinary repository that happens to be called
+        // `settings`, and none of this business.
+        assert!(check_roster_has_rules(&tree_of(&repo, &[("README.md", "hi")])).is_ok());
+        // Rules without a roster is a different error, caught elsewhere by
+        // the lockout check rather than here.
+        assert!(check_roster_has_rules(&tree_of(
+            &repo,
+            &[("conf/access.conf", "repo settings\n    RW+ = alex\n")]
+        ))
+        .is_ok());
+    }
+    #[test]
+    fn an_existing_unruled_roster_is_reported_and_an_absent_one_is_not() {
+        let tmp = tempfile::TempDir::new().unwrap();
+        // Nothing there at all.
+        assert_eq!(unruled_roster(tmp.path()), None);
+        let repo = git2::Repository::init_bare(tmp.path().join("settings.git")).unwrap();
+        // There, but empty: no commit, so nothing to report.
+        assert_eq!(unruled_roster(tmp.path()), None);
+        let tree = tree_of(&repo, &[("keydir/alex.pub", "ssh-ed25519 AAAA alex")]);
+        let who = git2::Signature::now("Ops", "ops@example.com").unwrap();
+        repo.commit(Some("HEAD"), &who, &who, "roster", &tree, &[])
+            .unwrap();
+        assert!(unruled_roster(tmp.path()).is_some());
+    }
+    #[test]
+    fn creator_round_trips_and_is_absent_by_default() {
+        let tmp = tempfile::TempDir::new().unwrap();

src/server/main.rs

Old		New
@@ -87,6 +87,16 @@ async fn main() {
87	info!("Site title: {}", config.site_title);	87	info!("Site title: {}", config.site_title);
88	info!("SSH host key fingerprint: {}", fingerprint);	88	info!("SSH host key fingerprint: {}", fingerprint);
89		89
		90	// A repository that reached this state before the push check existed is
		91	// already on disk, where no hook can reach it. Said out loud once, at the
		92	// only moment an operator is looking, rather than silently worked around.
		93	if let Some(reason) = governance::unruled_roster(&config.repos_dir) {
		94	tracing::warn!(
		95	"{:?}: {reason}",
		96	config.repos_dir.join("settings.git").display()
		97	);
		98	}
		99
90	let app_state = http::AppState {	100	let app_state = http::AppState {
91	repos_dir: config.repos_dir.clone(),	101	repos_dir: config.repos_dir.clone(),
92	site_title: config.site_title.clone(),	102	site_title: config.site_title.clone(),

tests/common/mod.rs

@@ @@ -933,6 +933,44 @@ impl ServerHarness { @@
+        git(&work, &["commit", "-q", "--allow-empty", "-m", "settings"]);
+    /// Stage `keydir/` entries with no `conf/access.conf` at all — the
+    /// bootstrap half-state, where the repository names everyone who has
+    /// access without holding any rule about it.
+    ///
+    /// Removes the rules if they are already staged, so the same helper
+    /// produces the state by either route: never written, or deleted.
+    pub fn stage_keydir_only(&self, keys: &[(&str, &str)]) {
+        self.ensure_settings_repos();
+        let work = self.settings_work();
+        let conf_dir = work.join("conf");
+        if conf_dir.exists() {
+            std::fs::remove_dir_all(&conf_dir).unwrap();
+        }
+        for (rel, key_name) in keys {
+            let dest = work.join("keydir").join(rel);
+            std::fs::create_dir_all(dest.parent().unwrap()).unwrap();
+            let pubkey =
+                std::fs::read_to_string(self.named_key(key_name).with_extension("pub")).unwrap();
+            std::fs::write(dest, pubkey).unwrap();
+        }
+        git(&work, &["add", "-A"]);
+        git(&work, &["commit", "-q", "--allow-empty", "-m", "keydir"]);
+    }
+    /// Whether anything has landed in `settings.git` at all: whether its HEAD
+    /// resolves to a commit.
+    pub fn settings_has_content(&self) -> bool {
+        Command::new("git")
+            .args(["rev-parse", "-q", "--verify", "HEAD"])
+            .current_dir(self.settings_bare())
+            .output()
+            .expect("failed to run git rev-parse")
+            .status
+            .success()
+    }
+    /// The `conf/access.conf` the server would actually read right now, from
+    /// the tip of `settings.git`'s live branch.
+    pub fn live_access_conf(&self) -> String {

tests/governance_test.rs

@@ @@ -588,6 +588,116 @@ fn release_downloads_follow_the_anonymous_read_grant() { @@
+    assert_eq!(body, b"payload");
+// ---- The bootstrap half-state -------------------------------------------
+//
+// The closed-by-default posture is a property of the *governed* world, and a
+// `settings.git` holding `keydir/` but no `conf/access.conf` governs nothing:
+// it is an ordinary repository on an ungoverned server, listed and clonable,
+// publishing who has access to the forge and how many of them there are. That
+// window is closed at the source — the push that would create it is refused —
+// rather than by special-casing the repository on the way out.
+/// The mistake a first-time operator makes, because `keydir/` is the thing you
+/// have before you have rules.
+#[test]
+fn a_roster_pushed_without_rules_is_refused_and_nothing_lands() {
+    let harness = ServerHarness::new("alpha");
+    let key = harness.ssh_client_key();
+    harness.stage_keydir_only(&[("alex.pub", "alex")]);
+    let refused = harness.push_settings_over_ssh(&key);
+    assert_refused(&refused, "a keydir with no access.conf");
+    // The operator is mid-bootstrap and has no idea why this bounced, so the
+    // message has to name the fix.
+    let message = stderr(&refused);
+    assert!(
+        message.contains("conf/access.conf") && message.contains("keydir/"),
+        "the refusal must name both halves; got:\n{message}"
+    );
+    assert!(
+        message.contains("push both") || message.contains("first"),
+        "the refusal must say what to do instead; got:\n{message}"
+    );
+    // Nothing landed, so there is no roster on the server to publish.
+    assert!(
+        !harness.settings_has_content(),
+        "the refused push must not have updated the settings repository"
+    );
+    // And the server is still ungoverned: the repository it hosts is
+    // untouched by any of this.
+    assert!(readable_anonymously(&harness, "alpha"));
+}
+/// The same exposure by the other route: the resulting tree is what matters,
+/// not the diff that produced it.
+#[test]
+fn a_push_that_deletes_the_rules_and_keeps_the_roster_is_refused() {
+    let harness = ServerHarness::new("governed");
+    harness.bootstrap_settings(ACCESS_CONF, KEYS);
+    let admin = harness.named_key("alex");
+    harness.stage_keydir_only(KEYS);
+    let refused = harness.push_settings_over_ssh(&admin);
+    assert_refused(&refused, "a push that removes access.conf but keeps keydir");
+    // Specifically the roster check, not the generic "you removed the rules"
+    // one: both routes leave the same tree and must say the same thing.
+    assert!(
+        stderr(&refused).contains("keydir/ without conf/access.conf"),
+        "got:\n{}",
+        stderr(&refused)
+    );
+    // The previously-live config still governs.
+    assert!(harness.live_access_conf().contains("repo settings"));
+    assert!(!readable_anonymously(&harness, "settings"));
+}
+/// The fix the message tells the operator to apply: push both together.
+#[test]
+fn pushing_the_roster_and_the_rules_together_is_accepted() {
+    let harness = ServerHarness::new("alpha");
+    let key = harness.ssh_client_key();
+    harness.stage_settings(ACCESS_CONF, KEYS);
+    assert_accepted(
+        &harness.push_settings_over_ssh(&key),
+        "the first config push, roster and rules together",
+    );
+    assert!(harness.live_access_conf().contains("repo settings"));
+    // And from that moment the server is governed, so the repository holding
+    // the roster is closed by the default.
+    assert!(!readable_anonymously(&harness, "settings"));
+    assert!(!listed_anonymously(&harness, "settings"));
+}
+/// Rule 3, at the boundary: a `settings.git` with neither file governs
+/// nothing and is refused nothing. Only a roster *without* rules is.
+#[test]
+fn a_settings_repo_with_neither_file_is_an_ordinary_repository() {
+    let harness = ServerHarness::new("alpha");
+    let key = harness.ssh_client_key();
+    harness
+        .work_repo()
+        .commit_file("readme.md", "hello", "docs");
+    let pushed = harness.ssh_push_from(
+        harness.work_repo().dir.path(),
+        &key,
+        "settings",
+        "main:main",
+    );
+    assert_accepted(
+        &pushed,
+        "an ordinary push to a repository that happens to be called settings",
+    );
+    assert!(harness.settings_has_content());
+    assert!(readable_anonymously(&harness, "settings"));
+}
+/// Creating `settings.git` must not silently publish the key roster and the
+/// access rules to an internet-facing web UI. This used to be a hand-coded
+/// special case; it is now just the default with no rule to lift it.