feat: rename the checks CLI to effect and enforce the admin-only write rule
commit
7bd4db7feat: rename the checks CLI to effect and enforce the admin-only write rule
git-ents-core, git-ents-server, and the checks/effect execution engine were already shrunk to their target shape in earlier phases; the two remaining gaps abstractions.adoc called out were the CLI surface and the admin-only refs/meta/effects/* write rule, both closed here.
feat: rename git ents checks to git ents effect (list/add/remove/debug/log) feat: reject a push to refs/meta/effects/* from a non-admin-registered member docs: update cli.adoc, checks.adoc, meta-ref.adoc, conformance.adoc for the rename and the new admin-only rule
git effect run/show (local host-direct execution sharing git-effect’s sandboxed path) and the standalone git store/anchor/effect plumbing binaries remain out of scope, per abstractions.adoc’s own note that they are deferred follow-up work rather than part of this crate boundary refactor.
Assisted-by: Claude:claude-sonnet-5
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
docs/spec/checks.adoc
@@ -33,6 +33,17 @@
migration.
--
+[role="requirement", id="checks.admin-only"]
+.Effect Definitions Are Admin-Only
+--
+`pre-receive` MUST reject a push to `refs/meta/effects/*` from a member
+whose provenance is not admin-registered, regardless of `refs/meta/config`'s
+role rules: authoring an effect schedules code execution, which needs more
+trust than an ordinary branch push, so this rule MUST be enforced
+explicitly rather than left to a role a repository may or may not have
+configured.
+--
+
[role="requirement", id="checks.toolchains"]
.Check Toolchains
--
@@ -123,7 +134,7 @@
.Sprite Debug Shell
--
A signed-in member MUST be able to open an interactive, read-write shell in
-a repository's checks Sprite (`git ents checks debug`), brokered by the
+a repository's effects Sprite (`git ents effect debug`), brokered by the
server over a WebSocket at the reserved path `/_debug/<repo>`.
The server holds the only Fly credential (`SPRITES_TOKEN`); a member MUST
NOT need one of their own, and the broker MUST refuse the connection
docs/spec/cli.adoc
@@ -13,7 +13,7 @@
named remote (defaulting to `origin`), load the typed document, apply the
change, store it, and push the updated ref back — signed per the client's
git config, through the same `pre-receive` gate a content push traverses.
-A read-only command (`members list`, `members check`, `checks list`,
+A read-only command (`members list`, `members check`, `effect list`,
`comment list`/`show`) MUST only fetch, never push.
--
@@ -60,13 +60,13 @@
--
[role="requirement", id="cli.account-checks"]
-.Account and Check Commands
+.Account and Effect Commands
--
`git ents account create` MUST write or update `refs/meta/account`
(<<account.ref>>) and print the account's genesis identity
(<<account.genesis>>).
-`git ents checks` MUST provide `list`, `add`, and `remove` over the
-`refs/meta/effects/<name>` set (<<checks.definition>>).
+`git ents effect` MUST provide `list`, `add`, `remove`, `debug`, and `log`
+over the `refs/meta/effects/<name>` set (<<checks.definition>>).
--
[role="requirement", id="cli.toolchains"]
docs/spec/conformance.adoc
@@ -47,13 +47,15 @@
|`checks.worker` |`git-effect/src/engine.rs` (`worker`, `pending_jobs`, `drain_repo`, `process_job`, `derive_composite`) |`engine::tests::pending_jobs_groups_by_repo_and_drops_malformed`, `composite_status_derives_from_its_dependencies` _(dependency-ordered execution requires a live Sprite; `order` is unit-tested in `git-effect`)_
|`checks.sandbox` |`git-effect/src/engine.rs` (`ensure_auth`, `ensure_sprite`, `sync_tree`) | _(requires a live Sprite; not covered by unit tests)_
|`checks.outcomes` |`git-effect/src/results.rs` (`Status`, `record`/`update_run`); `git-effect/src/engine.rs` (`CHECK_TIMEOUT`, `finalize_error`) |`results::tests::update_run_advances_in_place_rather_than_appending`, `round_trips_an_outcomes_duration_and_recording`
-|`checks.debug` |`git-ents-server/src/web/debug.rs` (`handshake`, broker); `git-ents/src/main.rs` (`checks debug`) | _(requires a live Sprite; manually verified)_
+|`checks.debug` |`git-ents-server/src/web/debug.rs` (`handshake`, broker); `git-ents/src/main.rs` (`effect debug`) | _(requires a live Sprite; manually verified)_
+|`checks.admin-only` |`git-signed-push/src/lib.rs` (`pre_receive`, `EFFECTS_NS_PREFIX`) |`git-ents-server::pre_receive rejects_a_push_to_effects_from_a_self_attested_member`
|`issues.ref` |`git-ents/src/issues.rs` (`Issue`, `State`) |`issues::store_then_load_round_trips_an_issue`
|`issues.id` |`git-ents/src/issues.rs` (`new_id`, `promote`) |`issues::new_id_hashes_its_own_content_with_no_origin`, `promotion_assigns_a_number_and_advances_the_counter_without_renaming_the_ref`
|`comments.ref` |`git-comment/src/lib.rs` (`Comment`, `COMMENTS_NS`, `new_id`) |`git-comment::store_then_load_round_trips_a_comment`, `loads_the_on_disk_comment_format`, `new_id_hashes_its_own_content_with_no_origin`
|`comments.authorship` |`git-store/src/lib.rs` (`store_item_authored`, `provenance`); `git-comment/src/lib.rs` (`provenance`) |`git-store::provenance_recovers_the_creating_and_updating_authors`, `git-comment::provenance_comes_from_the_commits_not_the_document`
|`comments.anchor` |`git-anchor/src/lib.rs` (`Anchor`, `capture`, `snippet`) |`git-anchor::capture_records_the_commit_and_blob_and_snippet_derives_the_text`, `capture_rejects_a_missing_path_and_an_oversized_range`
|`comments.projection` |`git-anchor/src/lib.rs` (`project`, `Projection`) |`git-anchor::unchanged_file_projects_as_current`, `a_pure_rename_relocates_with_the_same_lines`, `an_edit_inside_the_range_is_outdated`, `a_deleted_file_projects_as_deleted`, `projection_works_backwards_onto_an_ancestor`
+|`anchor.reachability` |`git-store/src/lib.rs` (`store_with_parents`, `store_authored_with_parents`, `CHAIN_PARENT_HEADER`); `git-comment/src/lib.rs` (`store`) |`git-comment::a_stored_comment_carries_its_anchored_commit_as_a_second_parent`
|`web.render-registry` |`git-ents-server/src/render.rs` (`to_html`/`to_text`, `mime_for_name`) | _(passthrough fallback exercised by web/CLI render paths)_
|`web.server-rendered` |`git-ents-server/src/web/*.rs` (Askama/maud templates, no client JS required) | _(manual UI verification)_
|`web.index` |`git-ents-server/src/web/mod.rs` (`index`) | _(manual UI verification)_
docs/spec/meta-ref.adoc
@@ -56,12 +56,12 @@
Key strategy::
Every document is one of three shapes.
- A *singleton* lives on one fixed ref (`config`, `account`, `checks`,
+ A *singleton* lives on one fixed ref (`config`, `account`,
`revoked`, `issue-number`): `Store::load`/`store`.
A *named collection* is one-ref-per-item under a namespace prefix
- (`member/<username>`, `toolchains/<name>`) or a scalar-keyed map on a
- single ref (`checks/<name>`, `revoked/<fingerprint>`,
- `runs/<commit>/results/<name>`): `Store::load_item`/`store_item`,
+ (`member/<username>`, `toolchains/<name>`, `effects/<name>`,
+ `results/<effect>/<short-oid>`) or a scalar-keyed map on a single ref
+ (`revoked/<fingerprint>`): `Store::load_item`/`store_item`,
`load_map`/`store_map`.
A *content-addressed* collection is keyed by the hash of its own content
(`issues/<id>`, `comments/<id>`) via the shared `git_store::new_id`
crates/git-ents-server/tests/pre_receive.rs
@@ -205,12 +205,17 @@
/// Attempt a push, returning whether it succeeded.
fn push(work: &Path, server: &Path, signed: bool) -> bool {
+ push_ref(work, server, signed, "main:refs/heads/main")
+}
+
+/// Attempt a push of `refspec`, returning whether it succeeded.
+fn push_ref(work: &Path, server: &Path, signed: bool, refspec: &str) -> bool {
let url = format!("file://{}", server.display());
let mut args = vec!["push"];
if signed {
args.push("--signed");
}
- args.extend_from_slice(&[url.as_str(), "main:refs/heads/main"]);
+ args.extend_from_slice(&[url.as_str(), refspec]);
git_env(work, "git", &args)
.stdin(Stdio::null())
.stdout(Stdio::null())
@@ -348,6 +353,51 @@
std::fs::remove_dir_all(&base).ok();
}
+// @relation(checks.admin-only, role=Verifies)
+#[test]
+fn rejects_a_push_to_effects_from_a_self_attested_member() {
+ // A self-attested (non-admin-registered) member can still sign an
+ // ordinary content push, but is refused for `refs/meta/effects/*`
+ // regardless of any role rule: authoring an effect schedules code
+ // execution, which the admin-only rule guards unconditionally.
+ let base = unique_dir("effects-admin");
+ let pubkey = keygen(&base, "id");
+ let server = server_repo(&base, &[]);
+
+ let mut keys = std::collections::BTreeMap::new();
+ keys.insert("key".to_owned(), std::fs::read_to_string(&pubkey).unwrap());
+ let member = git_member::members::Member {
+ principal: "self-attested".to_owned(),
+ valid_after: None,
+ valid_before: None,
+ trust: git_member::members::Trust::Keys(keys),
+ provenance: git_member::members::Provenance::SelfAttestedWeb,
+ account: None,
+ role: None,
+ };
+ git_member::members::store(&server, &member).unwrap();
+
+ let work = work_repo(&base, Some(&pubkey));
+ let tree = ok(&work, "git", &["write-tree"]);
+ let commit = ok(&work, "git", &["commit-tree", &tree, "-m", "effect"]);
+ ok(
+ &work,
+ "git",
+ &["update-ref", "refs/heads/effect-tmp", &commit],
+ );
+
+ assert!(
+ !push_ref(
+ &work,
+ &server,
+ true,
+ "refs/heads/effect-tmp:refs/meta/effects/demo",
+ ),
+ "self-attested member was allowed to push to refs/meta/effects/*"
+ );
+ std::fs::remove_dir_all(&base).ok();
+}
+
// @relation(auth.bootstrap, role=Verifies)
#[test]
fn accepts_any_push_before_signers_are_configured() {
crates/git-ents/src/interactive.rs
@@ -1,7 +1,7 @@
//! Prompting for `add` commands left with unset fields.
//!
//! An omitted field is filled interactively when the terminal supports it,
-//! so `git ents checks add` alone walks a user through every field; a script
+//! so `git ents effect add` alone walks a user through every field; a script
//! or CI invocation without a TTY gets a clear error instead of a hang.
use std::io::IsTerminal as _;
crates/git-ents/src/main.rs
@@ -2,7 +2,7 @@
//!
//! It carries `git ents members` for managing the repository members recorded
//! one-ref-per-person at `refs/meta/member/<username>`, `git ents account` for
-//! the account identity at `refs/meta/account`, `git ents checks` for the
+//! the account identity at `refs/meta/account`, `git ents effect` for the
//! effect set at one-ref-per-effect `refs/meta/effects/<name>`, `git ents
//! toolchain` for the toolchains stored as git trees at
//! `refs/meta/toolchains/<name>` (`git-toolchain`), `git ents comment` for the
@@ -59,10 +59,10 @@
#[facet(args::subcommand)]
action: AccountAction,
},
- /// Manage the configured checks at `refs/meta/effects/<name>`.
- Checks {
+ /// Manage the configured effects at `refs/meta/effects/<name>`.
+ Effect {
#[facet(args::subcommand)]
- action: ChecksAction,
+ action: EffectAction,
},
/// Manage the toolchains stored as git trees at
/// `refs/meta/toolchains/<name>`.
@@ -78,7 +78,7 @@
},
/// Sign in to a remote's server the same way the web UI does — sign a
/// server-issued challenge with your key — so this machine can also open a
- /// debug session (`checks debug`).
+ /// debug session (`effect debug`).
Login {
/// Key to sign in with; defaults to `user.signingkey`.
#[facet(args::named)]
@@ -199,44 +199,44 @@
/// @relation(cli.account-checks)
#[derive(Facet)]
#[repr(u8)]
-enum ChecksAction {
- /// List the checks configured on a remote.
+enum EffectAction {
+ /// List the effects configured on a remote.
List,
- /// Add (or replace) a check on a remote's set and push the update.
+ /// Add (or replace) an effect on a remote's set and push the update.
/// Prompts for any field left unset when run at an interactive terminal.
Add {
- /// Name to record the check under (`checks/<name>`).
+ /// Name to record the effect under (`effects/<name>`).
#[facet(args::positional, default)]
name: Option<String>,
- /// Command the check runs (e.g. `cargo fmt --check`); omit for a
- /// composite check that only aggregates its dependencies.
+ /// Command the effect runs (e.g. `cargo fmt --check`); omit for a
+ /// composite effect that only aggregates its dependencies.
#[facet(args::positional, default)]
command: Option<String>,
/// Sandbox image the command runs in (reserved: the Sprite sandbox
/// does not honor an image yet, so setting one is rejected).
#[facet(args::named)]
image: Option<String>,
- /// Check that must pass before this one runs (repeatable).
- #[facet(args::named, args::label = "CHECK", default)]
+ /// Effect that must pass before this one runs (repeatable).
+ #[facet(args::named, args::label = "EFFECT", default)]
depends: Vec<String>,
/// Toolchain (`refs/meta/toolchains/<name>`) to activate on `PATH`
/// before the command runs (repeatable).
#[facet(args::named, args::label = "TOOLCHAIN", default)]
toolchains: Vec<String>,
},
- /// Remove a check from a remote's set and push the update.
+ /// Remove an effect from a remote's set and push the update.
Remove {
- /// Name (`checks/<name>`) to drop.
+ /// Name (`effects/<name>`) to drop.
#[facet(args::positional)]
name: String,
},
- /// Open an interactive, read-write shell in `remote`'s persistent checks
- /// Sprite — the same sandbox its check runs execute in. Requires
+ /// Open an interactive, read-write shell in `remote`'s persistent effects
+ /// Sprite — the same sandbox its effect runs execute in. Requires
/// `git ents login <remote>` first.
Debug,
- /// Show recorded check runs (queued/running/pass/fail/error) from
+ /// Show recorded effect runs (queued/running/pass/fail/error) from
/// `refs/meta/results/*` on a remote, newest first.
- Runs,
+ Log,
}
/// ## Requirements
@@ -403,7 +403,7 @@
match cli.command {
Top::Members { action } => exit_code(run_members(action, &remote)),
Top::Account { action } => exit_code(run_account(action, &remote)),
- Top::Checks { action } => exit_code(run_checks(action, &remote)),
+ Top::Effect { action } => exit_code(run_effect(action, &remote)),
Top::Toolchain { action } => exit_code(run_toolchain(action, &remote)),
Top::Comment { action } => exit_code(run_comment(action, &remote)),
Top::Login { key } => exit_code(login(&remote, key.as_deref())),
@@ -474,30 +474,30 @@
/// ## Requirements
///
/// @relation(cli.account-checks)
-fn run_checks(action: ChecksAction, remote: &str) -> Result<(), String> {
+fn run_effect(action: EffectAction, remote: &str) -> Result<(), String> {
match action {
- ChecksAction::List => effect_list(remote),
- ChecksAction::Add {
+ EffectAction::List => effect_list(remote),
+ EffectAction::Add {
name,
command,
image,
depends,
toolchains,
- } => add_check(name, command, image, depends, toolchains, remote),
- ChecksAction::Remove { name } => effect_remove(&name, remote),
- ChecksAction::Debug => checks_debug(remote),
- ChecksAction::Runs => checks_runs(remote),
+ } => effect_add(name, command, image, depends, toolchains, remote),
+ EffectAction::Remove { name } => effect_remove(&name, remote),
+ EffectAction::Debug => effect_debug(remote),
+ EffectAction::Log => effect_log(remote),
}
}
/// Print the latest recorded status of every checked commit on `remote`,
-/// newest commit first, as `<commit> <when> <check>=<status> …`.
-fn checks_runs(remote: &str) -> Result<(), String> {
+/// newest commit first, as `<commit> <when> <effect>=<status> …`.
+fn effect_log(remote: &str) -> Result<(), String> {
let repo = repo()?;
sync_namespace(remote, git_effect::RESULTS_NS)?;
let commits = git_effect::runs(&repo).map_err(|error| error.to_string())?;
if commits.is_empty() {
- println!("no check runs on {remote}");
+ println!("no effect runs on {remote}");
return Ok(());
}
for commit_runs in commits {
@@ -977,7 +977,7 @@
sync_namespace(remote, git_effect::EFFECTS_NS)?;
let mut effects = git_effect::load_all(&repo).map_err(|error| error.to_string())?;
if effects.is_empty() {
- println!("no checks configured on {remote}");
+ println!("no effects configured on {remote}");
return Ok(());
}
effects.sort_by(|a, b| a.name.cmp(&b.name));
@@ -992,7 +992,7 @@
fn effect_remove(name: &str, remote: &str) -> Result<(), String> {
let refname = git_effect::effect_ref(name);
let expected =
- sync(remote, &refname)?.ok_or_else(|| format!("no check named {name} on {remote}"))?;
+ sync(remote, &refname)?.ok_or_else(|| format!("no effect named {name} on {remote}"))?;
push_delete(remote, &refname, &expected)?;
println!("removed {name}");
Ok(())
@@ -1008,7 +1008,7 @@
/// ## Requirements
///
/// @relation(cli.account-checks)
-fn add_check(
+fn effect_add(
name: Option<String>,
command: Option<String>,
image: Option<String>,
@@ -1016,7 +1016,7 @@
toolchains: Vec<String>,
remote: &str,
) -> Result<(), String> {
- let name = interactive::text_or(name, "Check name")?;
+ let name = interactive::text_or(name, "Effect name")?;
let command = interactive::optional_text_or(command, "Command (empty for a composite)")?;
let depends = if depends.is_empty() {
parse_names(interactive::optional_text_or(
@@ -1051,7 +1051,7 @@
let _ordered = git_effect::order(&effects)?;
git_effect::store(&repo, &effect).map_err(|error| error.to_string())?;
push_signed(remote, &refname, expected.as_deref())?;
- println!("recorded check {name}");
+ println!("recorded effect {name}");
Ok(())
}
@@ -1517,7 +1517,7 @@
/// Sign in to `remote`'s server: fetch its one-time challenge, sign it locally
/// with `key` (never handing the private key anywhere), and post the
/// signature back — the same proof the browser login page collects by hand.
-/// The returned session token is stored locally so `checks_debug` can reuse
+/// The returned session token is stored locally so `effect_debug` can reuse
/// it.
///
/// ## Requirements
@@ -1542,14 +1542,14 @@
Ok(())
}
-/// Open an interactive, read-write shell in `remote`'s persistent checks
+/// Open an interactive, read-write shell in `remote`'s persistent effects
/// Sprite, brokered by the server over a WebSocket using the session
/// `login` stored.
///
/// ## Requirements
///
/// @relation(checks.debug)
-fn checks_debug(remote: &str) -> Result<(), String> {
+fn effect_debug(remote: &str) -> Result<(), String> {
let (base, repo_path) = remote_http_base(remote)?;
let host = host_of(&base)?;
let token = load_session(&host)?
crates/git-signed-push/src/lib.rs
@@ -15,9 +15,15 @@
use std::process::{Command, Stdio};
use git_ents_core::config;
-use git_member::members::{self, Member};
+use git_member::members::{self, Member, Provenance};
use git_member::revocations;
+/// Authoring an effect schedules code execution, which needs more trust than
+/// an ordinary branch push — this admin-only rule is explicit and unaffected
+/// by `refs/meta/config`'s role rules, per `abstractions.adoc`'s "this rule
+/// must exist explicitly; it is not the default."
+const EFFECTS_NS_PREFIX: &str = "refs/meta/effects/";
+
/// Verify the push git is about to apply, returning `Ok(())` to accept it or
/// `Err(reason)` to reject it. The push certificate is read from the
/// environment git populates for the hook.
@@ -68,6 +74,15 @@
member.principal, member.role
));
}
+ // @relation(checks.admin-only)
+ if ref_name.starts_with(EFFECTS_NS_PREFIX)
+ && member.provenance != Provenance::AdminRegistered
+ {
+ return Err(format!(
+ "{} is not admin-registered and so cannot push to {ref_name:?}: authoring an effect requires an admin",
+ member.principal
+ ));
+ }
}
}
Ok(())
crates/git-ents-server/src/web/pages.rs
@@ -890,7 +890,7 @@
}
/// The Checks tab. The effect set lives one ref per effect under
-/// `refs/meta/effects` (managed with `git ents checks`); each push queues them
+/// `refs/meta/effects` (managed with `git ents effect`); each push queues them
/// and a worker runs them in a Sprite. "Checks on HEAD" mirrors a GitHub PR
/// checks list — one row per configured effect, its latest status against the
/// current commit, linked to its recorded terminal session when it has one;
@@ -925,7 +925,7 @@
div.page-header { h1.page-title { "Checks" } }
p.shell-note {
"Checks are configured on " code { "refs/meta/effects/<name>" }
- " (" code { "git ents checks list" } ") and run in a Sprite after each push; "
+ " (" code { "git ents effect list" } ") and run in a Sprite after each push; "
"each run is recorded under " code { "refs/meta/results/<effect>/<commit>" } "."
}
div.card {
@@ -1253,7 +1253,7 @@
p.shell-note {
"Commands on " code { "refs/meta/effects/*" } " run against each push "
- "(" code { "git ents checks list" } ")."
+ "(" code { "git ents effect list" } ")."
}
(component::card(&checks))