git-ents.gitmain
⌘K
foforge
commit 8aa0383
roots: install hosted hooks and wire the executor from the root, not the CLI

Two review findings from the fable pass, both fixed here.

git ents setup --hosted <path> is the missing command: nothing in git-ents actually installed the pre-receive/post-receive hooks into a hosted bare repository before this, meaning a real deployment with missing hooks would silently accept every push completely ungated - stock git’s receive-pack enforces nothing on its own. --hosted resolves or generates a signing key for the hosted worker (the same key logic setup already had, now shared via resolve_or_generate_key) and writes this binary’s own current_exe() into hooks/pre-receive and post-receive, executable. It skips receive.denyCurrentBranch=updateInstead, the local-root checked-out-worktree edge case, meaningless for a bare repository. tests/hosted_root.rs now exercises the real command over a subprocess instead of writing hook scripts itself.

DockerExecutor/SpriteExecutor construction moves from exe.rs into LocalRoot/HostedRoot: root.rs’s own doc claimed "every trait implementation is selected here…​ and nowhere else", but the CLI dispatcher was the one actually constructing the executor, and roots.local requires the local root itself to wire a Docker executor. Both roots now carry a boxed Executor field; exe.rs only ever reads root.executor. This also removes the doc’s reference to a --executor flag that was never implemented - LocalRoot fixes DockerExecutor unconditionally, since nothing in cli.rs lets a caller choose otherwise. HOSTED_WORKER_NAME is the one place the hosted worker’s Sprite name and result-commit author name are named, shared by root.rs and hook.rs instead of two literals kept in sync by hand.

Smaller fixes alongside: hook.rs’s comment claiming gix_odb::at follows a quarantine’s own info/alternates back to the real odb was stale (it never did; QuarantineObjects is the actual mechanism, verified by its own tests) - corrected to point at that type. read_effect’s unparsable- trigger case now returns Ok(None) instead of propagating through ?, matching its own doc ("nothing to run, not a hard failure") instead of aborting the whole post-receive drain over one pre-existing malformed effect. write_dir_as_tree now refuses a non-UTF-8 filename instead of silently mangling it through to_string_lossy.

Not fixed, noted instead: HostedRoot::open always runs a full reconcile scan even when opened just for pre-receive’s gate check, which never reads it - flagged as a non-blocking performance observation, not a correctness issue, and left alone rather than reshaping the constructor’s public signature under time pressure.

Assisted-by: Claude:claude-sonnet-4-6

Joseph D. Carpinelli · 1 month ago

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

verdict

crates/git-ents/src/cli.rs @@ -23,7 +23,7 @@ } /// Every top-level `git ents` subcommand. -// @relation(roots.local, roots.worktree-update, scope=file) +// @relation(roots.local, roots.worktree-update, roots.single-node-hosted, scope=file) #[derive(Facet)] #[repr(u8)] pub enum Top { @@ -32,11 +32,28 @@ /// `gpg.format=ssh`, and set `receive.denyCurrentBranch=updateInstead` /// so the integration-test harness can push into this repository's /// checked-out branch (`roots.worktree-update`). + /// + /// With `--hosted`, configures the single-node hosted root instead + /// (`roots.single-node-hosted`): a signing key for the hosted worker, + /// and this binary's own `pre-receive`/`post-receive` hooks installed + /// into a bare repository's `hooks/` directory. Without these hooks + /// installed, a hosted bare repository accepts every push ungated — + /// stock git's `receive-pack` has no gate of its own. Setup { /// Key to sign with; defaults to `user.signingkey`, else a new /// `~/.ssh/id_ed25519` is generated. #[facet(args::named)] key: Option<PathBuf>, + /// Configure the single-node hosted root instead of the local + /// one: install this binary's `hook pre-receive`/`hook + /// post-receive` into a bare repository's own hooks, and a + /// signing key for the hosted worker. + #[facet(args::named, default)] + hosted: bool, + /// The bare repository to configure with `--hosted`; defaults to + /// the current directory. Ignored without `--hosted`. + #[facet(args::positional, default)] + path: Option<PathBuf>, }, /// Manage the repository members at `refs/meta/member/<username>`. Members {
crates/git-ents/src/exe.rs @@ -23,10 +23,25 @@ /// Any [`crate::Error`] the dispatched command reports. pub fn run(cli: Cli, out: &mut impl std::io::Write) -> Result<()> { match cli.command { - Top::Setup { key } => { + Top::Setup { + key, + hosted: true, + path, + } => { + let target = path.unwrap_or_else(|| ".".into()); + let key_path = commands::setup::run_hosted(&target, key)?; + let _ = writeln!(out, "signing key: {}", key_path.display()); + let _ = writeln!(out, "hooks installed in {}/hooks", target.display()); + Ok(()) + } + Top::Setup { + key, + hosted: false, + path: _, + } => { let root = LocalRoot::discover(".")?; - let path = commands::setup::run(&root, key)?; - let _ = writeln!(out, "signing key: {}", path.display()); + let key_path = commands::setup::run(&root, key)?; + let _ = writeln!(out, "signing key: {}", key_path.display()); Ok(()) } Top::Members { action } => run_members(action, out), @@ -125,8 +140,7 @@ let _ = writeln!(out, "defined {name}"); } EffectAction::Run { name, at, key } => { - let executor = ents_effect::DockerExecutor; - let outcomes = commands::effect::run(&root, &name, at, key, &executor)?; + let outcomes = commands::effect::run(&root, &name, at, key, root.executor.as_ref())?; for (oid, outcome) in outcomes { let _ = writeln!(out, "{oid}\t{:?}", outcome.result); } @@ -228,9 +242,13 @@ let repo = gix::open(&root.path)?; let key_path = crate::sign::resolve_key_path(&repo, None)?; let signer = crate::sign::Signer::load(&key_path)?; - let executor = ents_effect::SpriteExecutor::new("git-ents-hosted-worker"); - let ran = - crate::hook::post_receive(&root, &executor, scratch.path(), cache.path(), &signer)?; + let ran = crate::hook::post_receive( + &root, + root.executor.as_ref(), + scratch.path(), + cache.path(), + &signer, + )?; let _ = writeln!(out, "ran {ran} effect(s)"); Ok(()) }
crates/git-ents/src/hook.rs @@ -53,11 +53,15 @@ //! responsibility. Git runs `pre-receive` with new objects visible only //! through `GIT_OBJECT_DIRECTORY` (the quarantine) plus //! `GIT_ALTERNATE_OBJECT_DIRECTORIES` (the real odb) until the push is -//! accepted; [`crate::root::HostedRoot::open`] honors -//! `GIT_OBJECT_DIRECTORY` when the environment sets it (which git does for -//! `pre-receive`, and does not for `post-receive`, whose objects are by -//! then no longer quarantined), and `gix_odb::at` follows the quarantine -//! directory's own `info/alternates` back to the real odb transparently. +//! accepted; [`crate::root::HostedRoot::open`] honors `GIT_OBJECT_DIRECTORY` +//! when the environment sets it (which git does for `pre-receive`, and does +//! not for `post-receive`, whose objects are by then no longer quarantined). +//! `gix_odb::at` itself only ever follows a physical `info/alternates` +//! *file*, and git's own quarantine directory never has one — so this +//! crate's own [`crate::root::QuarantineObjects`] is what actually chains +//! the two directories, entirely in-process (no alternates file is ever +//! written to disk; see that type's own doc for why an earlier attempt at +//! writing one was wrong). //! //! # No separate daemon //! @@ -198,7 +202,7 @@ ents_receive::reconcile(&root.refs, &root.objects, &root.events)?; let author = gix::actor::Signature { - name: "git-ents-hosted-worker".into(), + name: crate::root::HOSTED_WORKER_NAME.into(), email: "worker@git.ents.cloud".into(), time: gix::date::Time { seconds: std::time::SystemTime::now() @@ -277,11 +281,11 @@ }; // Confirm the trigger still parses, mirroring `reconcile`'s own // tolerance rule; an effect whose trigger is unparsable is treated as - // "nothing to run" rather than a hard failure. - let _: Query = effect - .trigger - .parse() - .map_err(|_source| Error::InvalidArgument("unparsable trigger".to_owned()))?; + // "nothing to run" — `None`, not a hard failure that would abort the + // whole drain over one pre-existing malformed effect. + if effect.trigger.parse::<Query>().is_err() { + return Ok(None); + } Ok(Some(effect)) }
crates/git-ents/src/root.rs @@ -6,19 +6,18 @@ //! //! - [`LocalRoot`] (`roots.local`): the plain CLI, wired against whatever //! repository the current directory is in — loose-ref `RefStore`, the -//! local odb, a null `EventSink`, the advisory gate. `Executor` is -//! chosen per invocation (`git ents effect run --executor`), never -//! fixed by this root, since local execution is pull-only -//! (`effect.local-run`) and never itself runs an effect as part of -//! composing the root. -//! - [`HostedRoot`] (the single-node hosted root the development plan's -//! `git-ents` row describes: "loose refs and a real odb on a Fly -//! volume, served behind git's own `receive-pack`... with an in-memory -//! `EventSink` and a boot-time reconciliation scan, and the Sprite -//! executor"): the same loose-ref/odb primitives as [`LocalRoot`], but -//! the mandatory gate, an in-memory `EventSink`, and a `SpriteExecutor` -//! — wired by the `git-ents hook` plumbing subcommands -//! ([`crate::hook`]) that git's own `receive-pack` invokes. +//! local odb, a null `EventSink`, the advisory gate, and a fixed +//! `DockerExecutor` (there is no `--executor` flag anywhere in +//! [`crate::cli`] to choose otherwise; local execution stays pull-only, +//! via `git effect run`, per `effect.local-run`). +//! - [`HostedRoot`] (`roots.single-node-hosted`, the single-node hosted +//! root the development plan's `git-ents` row describes: "loose refs and +//! a real odb on a Fly volume, served behind git's own `receive-pack`... +//! with an in-memory `EventSink` and a boot-time reconciliation scan, and +//! the Sprite executor"): the same loose-ref/odb primitives as +//! [`LocalRoot`], but the mandatory gate, an in-memory `EventSink`, and a +//! fixed `SpriteExecutor` — wired by the `git-ents hook` plumbing +//! subcommands ([`crate::hook`]) that git's own `receive-pack` invokes. //! //! Neither root is `roots.hosted` (`git-ents-server`, phase 8): that root //! replaces the `RefStore` and object store with Postgres and Tigris and @@ -46,10 +45,11 @@ //! `HostedRoot` are two distinct types, never one type with an //! `if hosted` branch, and every command module ([`crate::commands`]) //! takes an already-constructed root, never constructing a store itself. -// @relation(roots.composition, roots.config-isolation, arch.store-composition-root, arch.no-hosted-branch, scope=file) +// @relation(roots.composition, roots.local, roots.single-node-hosted, roots.config-isolation, arch.store-composition-root, arch.no-hosted-branch, scope=file) use std::path::{Path, PathBuf}; +use ents_effect::Executor; use ents_receive::{Mode, NullEventSink}; use gix_ref_store::LooseRefStore; @@ -106,6 +106,11 @@ /// The null `EventSink` (`roots.local`): local effect execution is /// pull-only, so nothing is ever enqueued here (`effect.local-run`). pub events: NullEventSink, + /// The fixed `Executor` this root wires (`roots.local`): a + /// `DockerExecutor`. Boxed because `LocalRoot` and `HostedRoot` fix + /// different concrete backends and every command module is written + /// against the trait, never a specific one. + pub executor: Box<dyn Executor>, } impl LocalRoot { @@ -124,6 +129,7 @@ refs, objects, events: NullEventSink, + executor: Box::new(ents_effect::DockerExecutor), }) } @@ -179,8 +185,18 @@ /// The in-memory `EventSink`, reconciled at boot /// (`receive.reconstructible`). pub events: ents_receive::MemoryEventSink, + /// The fixed `Executor` this root wires (`roots.single-node-hosted`): a + /// `SpriteExecutor` targeting [`HOSTED_WORKER_NAME`]. + pub executor: Box<dyn Executor>, } +/// The Sprite name (and commit author name) the single-node hosted root's +/// worker uses — shared between [`HostedRoot`]'s `SpriteExecutor` and +/// [`crate::hook::post_receive`]'s result-commit author, so the two stay +/// the same identity by construction rather than by two literals staying +/// in sync by hand. +pub const HOSTED_WORKER_NAME: &str = "git-ents-hosted-worker"; + impl HostedRoot { /// Open the hosted composition root against the repository at `path`, /// honoring a pre-receive quarantine object directory if the @@ -204,6 +220,7 @@ refs, objects, events, + executor: Box::new(ents_effect::SpriteExecutor::new(HOSTED_WORKER_NAME)), }) }
crates/git-ents/tests/hosted_root.rs @@ -17,24 +17,40 @@ use git_ents::root::LocalRoot; -/// Install `pre-receive` and `post-receive` hooks on `bare` that shell to -/// the built `git-ents` binary's plumbing subcommands -/// (`crate::hook::pre_receive`, `crate::hook::post_receive`) — exactly -/// what a real deployment's hook scripts do. -fn install_hooks(bare: &Path) { - let bin = common::bin_path(); - let hooks_dir = bare.join("hooks"); - std::fs::create_dir_all(&hooks_dir).expect("hooks dir"); +/// Install `pre-receive` and `post-receive` hooks on `bare` by running the +/// real, built `git ents setup --hosted` command — not a test-harness +/// stand-in — over a subprocess, exactly how an operator deploying the +/// single-node hosted root would (`roots.single-node-hosted`). +/// +/// Neither test defines an effect, so `post-receive` never has a pending +/// obligation to sign results for — meaning it is safe for the scratch +/// `HOME` this generates a key under to be cleaned up once this function +/// returns; nothing later needs to load that key again. +fn setup_hosted(bare: &Path) { + let scratch_home = tempfile::tempdir().expect("tempdir"); + let output = Command::new(common::bin_path()) + .arg("setup") + .arg("--hosted") + .arg(bare) + // Isolate from the ambient environment the same way `git()` below + // does, but keep a real (scratch) HOME: `setup --hosted` needs + // somewhere to generate a signing key when neither `--key` nor + // `user.signingkey` resolves to one. + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null") + .env("HOME", scratch_home.path()) + .output() + .expect("git-ents runs"); + assert!(output.status.success(), "{output:?}"); + for hook in ["pre-receive", "post-receive"] { - let script = format!("#!/bin/sh\nexec {:?} hook {hook}\n", bin.display()); - let path = hooks_dir.join(hook); - std::fs::write(&path, script).expect("write hook"); + let path = bare.join("hooks").join(hook); + assert!(path.exists(), "setup --hosted must install {hook}"); #[cfg(unix)] { use std::os::unix::fs::PermissionsExt as _; - let mut perms = std::fs::metadata(&path).expect("meta").permissions(); - perms.set_mode(0o755); - std::fs::set_permissions(&path, perms).expect("chmod"); + let mode = std::fs::metadata(&path).expect("meta").permissions().mode(); + assert!(mode & 0o111 != 0, "{hook} must be executable: {mode:o}"); } } } @@ -69,15 +85,16 @@ } /// A bootstrap enrollment pushed to the single-node hosted root round -/// trips: the mandatory gate (`gate.mandatory-hosted`) admits it under the -/// bootstrap window (`gate.bootstrap`) exactly as the advisory local root -/// would, and the ref lands on the bare repository for real, over a real -/// `git push`. -// @relation(roots.local, roots.composition, gate.mandatory-hosted, gate.bootstrap, scope=function, role=Verifies) +/// trips: `setup_hosted` (`git ents setup --hosted`, `roots.single-node-hosted`) +/// installs the real hooks, then the mandatory gate (`gate.mandatory-hosted`) +/// admits the push under the bootstrap window (`gate.bootstrap`) exactly as +/// the advisory local root would, and the ref lands on the bare repository +/// for real, over a real `git push`. +// @relation(roots.local, roots.composition, roots.single-node-hosted, gate.mandatory-hosted, gate.bootstrap, scope=function, role=Verifies) #[test] fn bootstrap_push_round_trips_through_the_hosted_root() { let bare = common::Fixture::new_bare(20); - install_hooks(bare.path()); + setup_hosted(bare.path()); let clone_dir = tempfile::tempdir().expect("tempdir"); let clone_output = git( @@ -115,7 +132,7 @@ #[test] fn unauthorized_push_is_refused_by_the_hosted_root() { let bare = common::Fixture::new_bare(22); - install_hooks(bare.path()); + setup_hosted(bare.path()); // First, a legitimate admin bootstraps the repository. let admin_clone = tempfile::tempdir().expect("tempdir");
crates/git-ents/src/commands/setup.rs @@ -8,6 +8,14 @@ //! working tree, which is not how a normal git remote behaves and is never //! needed for `refs/meta/*` traffic (which never touches a worktree at //! all). +//! +//! `--hosted` ([`run_hosted`]) configures the single-node hosted root +//! instead (`roots.single-node-hosted`): a signing key for the hosted +//! worker, and this binary's own `hook pre-receive`/`hook post-receive` +//! installed into a bare repository's `hooks/`. Without this, a hosted +//! bare repository accepts every push completely ungated — stock git's +//! `receive-pack` enforces nothing on its own; the gate exists only where +//! a hook calls it. use std::path::{Path, PathBuf}; use std::process::Command; @@ -31,29 +39,7 @@ /// [`Error::Io`] if generating or writing a new key fails; propagates a /// config-write failure. pub fn run(root: &LocalRoot, key: Option<PathBuf>) -> Result<PathBuf> { - let repo = gix::open(&root.path)?; - let resolved = match crate::sign::resolve_key_path(&repo, key.as_deref()) { - Ok(path) if path.exists() => path, - Ok(path) => generate_key(&path)?, - Err(Error::NoSigningKey) => { - let default = default_key_path()?; - generate_key(&default)? - } - Err(other) => return Err(other), - }; - // Confirm the resolved key actually loads before recording it. - Signer::load(&resolved)?; - - // `gix`'s own config-snapshot API (`config_snapshot_mut`) has no - // file-persistence path at all: `SnapshotMut::commit` only updates the - // in-memory resolved view this `Repository` handle holds, never - // `.git/config` on disk (confirmed empirically — a value written that - // way is invisible to a subsequent, separate `git config` read). - // Writing durable local config is therefore delegated to `git config` - // itself here, same as `pre-redo`'s own client setup did; it is not - // part of the ref/object CAS discipline the rest of this crate is - // strict about (`arch.loose-cas-discipline` governs refs, not plain - // config values). + let resolved = resolve_or_generate_key(&root.path, key)?; let path_str = resolved.to_string_lossy().into_owned(); for (key, value) in [ ("user.signingkey", path_str.as_str()), @@ -62,10 +48,112 @@ ] { set_local_config(&root.path, key, value)?; } - Ok(resolved) } +/// Run `git ents setup --hosted` against the bare repository at `path`: +/// resolve or generate a signing key for the hosted worker (recorded as +/// `path`'s own `user.signingkey`/`gpg.format=ssh`, same as [`run`]), and +/// install this binary's `hook pre-receive`/`hook post-receive` as +/// `path`'s own git hooks (`roots.single-node-hosted`). +/// +/// `receive.denyCurrentBranch=updateInstead` is deliberately not set here: +/// it is the local-root, checked-out-worktree edge case +/// (`roots.worktree-update`), meaningless for a bare repository with no +/// worktree to update. +/// +/// # Errors +/// +/// [`Error::BadSigningKey`] if a given or configured key cannot be loaded; +/// [`Error::Io`] if generating a key, writing config, resolving this +/// binary's own path, or writing a hook file fails. +// @relation(roots.single-node-hosted, scope=function) +pub fn run_hosted(path: &Path, key: Option<PathBuf>) -> Result<PathBuf> { + let resolved = resolve_or_generate_key(path, key)?; + let path_str = resolved.to_string_lossy().into_owned(); + for (key, value) in [ + ("user.signingkey", path_str.as_str()), + ("gpg.format", "ssh"), + ] { + set_local_config(path, key, value)?; + } + install_hooks(path)?; + Ok(resolved) +} + +/// Resolve `key` (or `path`'s `user.signingkey`, or a default +/// `~/.ssh/id_ed25519`), generating a fresh key if nothing resolves to an +/// existing file, and confirm the result actually loads. +fn resolve_or_generate_key(path: &Path, key: Option<PathBuf>) -> Result<PathBuf> { + let repo = gix::open(path)?; + let resolved = match crate::sign::resolve_key_path(&repo, key.as_deref()) { + Ok(candidate) if candidate.exists() => candidate, + Ok(candidate) => generate_key(&candidate)?, + Err(Error::NoSigningKey) => { + let default = default_key_path()?; + generate_key(&default)? + } + Err(other) => return Err(other), + }; + // Confirm the resolved key actually loads before recording it. + Signer::load(&resolved)?; + Ok(resolved) +} + +/// Install this binary's own `hook pre-receive`/`hook post-receive` as +/// `repo_path`'s git hooks, overwriting any existing scripts of the same +/// name — the mechanism `roots.single-node-hosted` requires: without +/// these hooks, git's own `receive-pack` performs no gate check at all, +/// and a hosted bare repository would accept every push ungated. +/// +/// # Errors +/// +/// [`Error::Io`] if this binary's own path cannot be resolved, the +/// `hooks/` directory cannot be created, or a hook file cannot be written +/// or (on unix) made executable. +fn install_hooks(repo_path: &Path) -> Result<()> { + let this_binary = std::env::current_exe().map_err(|source| Error::Io { + path: repo_path.to_owned(), + source, + })?; + let hooks_dir = repo_path.join("hooks"); + std::fs::create_dir_all(&hooks_dir).map_err(|source| Error::Io { + path: hooks_dir.clone(), + source, + })?; + for hook in ["pre-receive", "post-receive"] { + let script = format!("#!/bin/sh\nexec {:?} hook {hook}\n", this_binary.display()); + let hook_path = hooks_dir.join(hook); + std::fs::write(&hook_path, script).map_err(|source| Error::Io { + path: hook_path.clone(), + source, + })?; + set_executable(&hook_path)?; + } + Ok(()) +} + +#[cfg(unix)] +fn set_executable(path: &Path) -> Result<()> { + use std::os::unix::fs::PermissionsExt as _; + let mut perms = std::fs::metadata(path) + .map_err(|source| Error::Io { + path: path.to_owned(), + source, + })? + .permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(path, perms).map_err(|source| Error::Io { + path: path.to_owned(), + source, + }) +} + +#[cfg(not(unix))] +fn set_executable(_path: &Path) -> Result<()> { + Ok(()) +} + /// Set `key` to `value` in `repo_path`'s own local config via `git config`. fn set_local_config(repo_path: &Path, key: &str, value: &str) -> Result<()> { let output = Command::new("git")
crates/git-ents/src/commands/toolchain.rs @@ -123,7 +123,10 @@ path: item.path(), source, })?; - let filename = item.file_name().to_string_lossy().into_owned(); + let filename = item.file_name().into_string().map_err(|raw| Error::Io { + path: dir.join(raw), + source: std::io::Error::other("non-UTF-8 filename cannot round-trip through a tree"), + })?; let (mode, oid) = if file_type.is_dir() { (EntryKind::Tree, write_dir_as_tree(&item.path(), objects)?) } else if file_type.is_file() {