git ents: effect, cli: per-member BYOK credentials and the redaction audit (phase 6)

crates/cli/git-ents/src/agent_worker.rs @@ -55,6 +55,22 @@ //! branch's tree reflects genuine sandbox output; this is called out again //! in the module-level doc of `ents-effect`'s `sprite` module as a known //! gap, not silently assumed away here. +//! +//! # The BYOK credential seam (`roots.config-isolation`, Phase 6) +//! +//! A session's own `meta.member` names whose credential is injected: BYOK +//! means each member's runs are billed and rate-limited against their own +//! Anthropic account, never a shared one. [`credentials`] is looked up +//! *after* the claim succeeds (so the claim itself — "this worker owns this +//! session now" — is unconditional) but *before* anything else about the +//! run happens: no workdir is checked out, no plan file written, nothing +//! executed. A member with no configured credential is not retried as an +//! infrastructure hiccup — which member has which credential is a fixed +//! property of this deployment, not a transient failure, so retrying the +//! exact same run changes nothing (`effect.result-taxonomy`'s own +//! pass/fail split: this is a genuine, deterministic `fail`, landed via the +//! ordinary finalize with no result branch, since no sandbox ever ran to +//! produce one). use std::path::{Path, PathBuf}; @@ -71,6 +87,7 @@ use gix_ref_store::RefStore; use crate::commands::commit_tree; +use crate::credentials::CredentialStore; use crate::error::{Error, Result}; /// `agent-exec`'s own effect name — re-exported so a caller deciding @@ -140,7 +157,7 @@ /// session stays `Running`, and the queue's own retry policy is what /// revisits it (`effect.result-taxonomy`) — or building and sending the /// finalize proposal. -// @relation(effect.execution, effect.results-writeback, effect.result-taxonomy, receive.multi-ref-atomicity, scope=function) +// @relation(effect.execution, effect.results-writeback, effect.result-taxonomy, receive.multi-ref-atomicity, roots.config-isolation, scope=function) #[expect( clippy::too_many_arguments, reason = "one input per materialization/identity step, mirrors ents_effect::run::run_one's \ @@ -160,6 +177,7 @@ author: &gix::actor::Signature, sign: &dyn Fn(&[u8]) -> String, mode: Mode, + credentials: &CredentialStore, ) -> Result<AgentRunOutcome> where O: Find + Write, @@ -204,6 +222,31 @@ return Ok(AgentRunOutcome::ClaimLost); } + // BYOK: this session's own member must have a configured credential + // before anything else about the run happens — no workdir, no plan + // file, no sandbox launch. Missing is a deterministic property of this + // deployment, not a transient hiccup a retry would fix, so it lands as + // an ordinary completed `fail`, exactly like the sandbox reporting a + // nonzero exit below, just without ever having produced a result + // branch (`roots.config-isolation`, `effect.result-taxonomy`). + let Some(credential) = credentials.get(&session.meta.member) else { + return finish_without_running( + refs, + objects, + events, + &results_ref, + oid, + &id, + format!( + "no credential configured for member {}", + session.meta.member + ), + &identity, + mode, + ); + }; + let env = [(credential.var.clone(), credential.secret.clone())]; + // 3. Run the sandbox against the declared base ref's tip. let base_ref: gix::refs::FullName = session @@ -240,6 +283,7 @@ workdir: &workdir, toolchains, command, + env: &env, }; // An `Err` here is an infrastructure failure the sandbox itself never // turned into a completed pass/fail (`effect.result-taxonomy`): it @@ -260,6 +304,18 @@ let _ = std::fs::remove_file(&plan_path); let output_tree = ents_effect::materialize::write_tree(objects, &workdir)?; + // The workdir's job is done once its tree is captured: wipe it rather + // than leave it sitting on disk under `scratch` for the rest of this + // process's life (a BYOK credential may have reached it via the + // sandbox's own env, `SandboxInputs::env`, or a file the command + // wrote). Best-effort, like the plan-file removal above. + #[expect( + clippy::let_underscore_must_use, + reason = "best-effort cleanup; a failure here is not actionable and must not fail an \ + otherwise-successful run" + )] + let _ = std::fs::remove_dir_all(&workdir); + let branch_name = result_branch_name(&session.meta.member, &id); let branch_ref: gix::refs::FullName = format!("refs/heads/{branch_name}") @@ -323,6 +379,60 @@ Ok(AgentRunOutcome::Finished { id, outcome }) } +/// Finalize a claimed session as `Failed` *without ever running the +/// sandbox at all* — the BYOK missing-credential path +/// (`roots.config-isolation`): no workdir is checked out, so there is no +/// output tree and no result branch (`FinishAgentSession::result_branch` +/// stays `None` — nothing ran to produce one). Lands the session's +/// terminal state and this effect's own `fail` result atomically, exactly +/// like [`run_agent_exec`]'s own finalize minus the branch transition. +#[expect( + clippy::too_many_arguments, + reason = "one input per identity/write step, mirrors run_agent_exec's own finalize shape" +)] +fn finish_without_running<O: Find + Write>( + refs: &dyn RefStore, + objects: &O, + events: &dyn EventSink, + results_ref: &gix::refs::FullName, + oid: ObjectId, + id: &str, + detail: String, + identity: &Identity<'_>, + mode: Mode, +) -> Result<AgentRunOutcome> { + let finish = FinishAgentSession { + outcome: FinishOutcome::Failed(detail.clone()), + result_branch: None, + thread: vec![detail.into_bytes()], + }; + let (finish_transition, finish_tip) = + ents_forge::agent::finish_transition(refs, objects, id, finish, identity)?; + + let record = ResultRecord::new(AGENT_EXEC_NAME, oid, ResultStatus::Fail); + let (result_transition, result_tip) = ents_receive::entity_transition( + refs, + objects, + results_ref, + &record, + identity, + "Record agent-exec result", + )?; + + // Finalize = one atomic multi-ref proposal, same as the ordinary case, + // just two transitions instead of three (`receive.multi-ref-atomicity`). + let proposal = Proposal { + transitions: vec![finish_transition, result_transition], + objects: vec![finish_tip, result_tip], + auth: None, + }; + let outcome = ents_receive::receive(refs, objects, events, &proposal, mode)?; + Ok(AgentRunOutcome::Finished { + id: id.to_owned(), + outcome, + }) +} + /// Record a cheap `pass` for `oid` on the canonical `agent-exec` results /// ref — the no-op path both [`Dispatch::NoOp`] and a lost claim race take. #[expect( @@ -586,6 +696,19 @@ &*self.sign } + /// A credential store configured for `queued_session`'s own member + /// (`jdc`) — every test that expects the run to actually reach the + /// sandbox uses this rather than [`CredentialStore::empty`]. + fn credentials(&self) -> CredentialStore { + CredentialStore::from_pairs([( + MemberId::new("jdc"), + crate::credentials::Credential { + var: "ANTHROPIC_API_KEY".to_owned(), + secret: "sk-ant-test-token".to_owned(), + }, + )]) + } + /// A brand-new, queued session (ready, plan confirmed) — the only /// precondition [`super::run_agent_exec`]'s `Dispatch::Claim` arm /// runs against. @@ -686,6 +809,7 @@ &author, fixture.sign_fn(), Mode::Advisory, + &CredentialStore::empty(), ) .expect("dispatch never touches the sandbox for a no-op"); assert!(matches!(run, AgentRunOutcome::NoOp)); @@ -739,6 +863,7 @@ &author, fixture.sign_fn(), Mode::Advisory, + &CredentialStore::empty(), ) .expect("a lost race is not an error"); assert!(matches!(run, AgentRunOutcome::ClaimLost)); @@ -777,6 +902,7 @@ &author, fixture.sign_fn(), Mode::Advisory, + &fixture.credentials(), ) .expect("runs and finalizes"); let AgentRunOutcome::Finished { @@ -840,6 +966,15 @@ !checkout_dir.path().join(AGENT_PLAN_FILE).exists(), "the sideband plan file must never leak into the pushed branch" ); + + // (d) the scratch workdir this run used is cleaned up afterward — + // nothing about a completed run should sit on disk for the rest of + // this process's life (see `run_agent_exec`'s own `remove_dir_all` + // after `write_tree`). + assert!( + !scratch.path().join(oid.to_string()).exists(), + "the run's own scratch workdir must not survive the run" + ); } #[rstest] @@ -868,6 +1003,7 @@ &author, fixture.sign_fn(), Mode::Advisory, + &fixture.credentials(), ) .expect("runs and finalizes"); @@ -890,6 +1026,76 @@ assert_eq!(record.status, ResultStatus::Fail); } + /// A member with no configured credential (`roots.config-isolation`) + /// gets a completed, deterministic `fail` — never a retryable + /// infrastructure error, and the sandbox is never even invoked + /// (`StubExecutor::run` would panic-visibly via its mutation if + /// called; instead this test hands a `panic`-on-run executor to prove + /// it never runs). + #[rstest] + // @relation(roots.config-isolation, effect.result-taxonomy, scope=function, role=Verifies) + fn a_member_with_no_configured_credential_fails_without_ever_running_the_sandbox() { + struct PanicsIfRun; + impl Executor for PanicsIfRun { + #[expect( + clippy::panic_in_result_fn, + reason = "the panic itself is this test's assertion: the sandbox must never run" + )] + fn run( + &self, + _inputs: &SandboxInputs<'_>, + ) -> ents_effect::Result<ents_effect::RunOutput> { + panic!("the sandbox must never launch without a configured credential"); + } + } + + let fixture = Fixture::new(); + let (oid, id) = fixture.queued_session(); + + let scratch = tempfile::tempdir().expect("tempdir"); + let author = fixture.author(); + let run = run_agent_exec( + &fixture.refs, + &fixture.objects, + &NullEventSink, + &PanicsIfRun, + scratch.path(), + &[], + "true", + oid, + MemberId::new("worker"), + "sprite-1".to_owned(), + &author, + fixture.sign_fn(), + Mode::Advisory, + &CredentialStore::empty(), + ) + .expect("a missing credential is a completed finalize, not an error"); + assert!(matches!(run, AgentRunOutcome::Finished { .. })); + + let session = ents_forge::agent::show(&fixture.refs, &fixture.objects, &id).expect("shows"); + assert!( + matches!(session.meta.status, ents_forge::agent::Status::Failed(_)), + "a missing credential must land the session as Failed, not leave it Running" + ); + assert!( + session.meta.result_branch.is_none(), + "no sandbox ran, so there is nothing for a result branch to carry" + ); + + let results_ref = + ents_model::namespace::result_ref(AGENT_EXEC_NAME, &short_oid(oid)).expect("valid"); + let result_tip = fixture + .refs + .get(results_ref.as_ref()) + .expect("readable") + .expect("result landed"); + let result_tree = commit_tree(&fixture.objects, result_tip).expect("tree"); + let record: ResultRecord = + facet_git_tree::deserialize(&result_tree, &fixture.objects).expect("decodes"); + assert_eq!(record.status, ResultStatus::Fail); + } + #[rstest] // @relation(effect.result-taxonomy, scope=function, role=Verifies) fn a_sandbox_infrastructure_failure_publishes_nothing_and_leaves_the_session_running() { @@ -912,6 +1118,7 @@ &author, fixture.sign_fn(), Mode::Advisory, + &fixture.credentials(), ) .expect_err("an infra failure must propagate, not silently finalize"); assert!(matches!(error, Error::Effect(_)));

crates/cli/git-ents/src/hook.rs @@ -260,6 +260,7 @@ &author, &|payload| signer.sign(payload), Mode::Mandatory, + &root.credentials, )?; } else if effect_name == crate::plan_worker::AGENT_PLAN_NAME { // The `agent-plan` effect (`docs/agent-sessions-plan.adoc`'s @@ -279,6 +280,7 @@ &author, &|payload| signer.sign(payload), Mode::Mandatory, + &root.credentials, )?; } else if effect_name == crate::review_worker::AGENT_REVIEW_NAME { // The `agent-review` effect (`docs/agent-sessions-plan.adoc`'s

crates/cli/git-ents/src/lib.rs @@ -76,6 +76,7 @@ pub mod agent_worker; pub mod cli; pub mod commands; +pub mod credentials; pub mod error; pub mod exe; pub mod hook;

crates/cli/git-ents/src/plan_worker.rs @@ -43,6 +43,18 @@ //! drafting is read-only context gathering, not a code change, so there is //! no result branch to push and nothing to clean up before an object write //! that never happens. +//! +//! # The BYOK credential seam (`roots.config-isolation`, Phase 6) +//! +//! Drafting a plan is itself an agent run against the session's own member +//! (`AgentSession::meta::member`), so it needs that member's credential +//! injected exactly like `agent_worker::run_agent_exec` does — looked up +//! *before* anything about the drafting attempt happens (no workdir, no +//! prompt file written). A member with no configured credential is not an +//! infrastructure hiccup (`effect.result-taxonomy`): it is recorded through +//! the same [`record_result`] path a failed drafting attempt already takes, +//! `Fail` on this effect's own results ref, the session left untouched and +//! still dispatchable once the deployment gets a credential configured. use std::path::{Path, PathBuf}; @@ -57,6 +69,7 @@ use gix_ref_store::RefStore; use crate::commands::commit_tree; +use crate::credentials::CredentialStore; use crate::error::{Error, Result}; /// `agent-plan`'s own effect name — re-exported so a caller deciding which @@ -121,7 +134,7 @@ /// untouched, and the queue's own retry policy is what revisits it, /// `effect.result-taxonomy`), reading back the drafted plan file, or /// building and sending the finalize proposal. -// @relation(effect.execution, effect.results-writeback, effect.result-taxonomy, receive.multi-ref-atomicity, scope=function) +// @relation(effect.execution, effect.results-writeback, effect.result-taxonomy, receive.multi-ref-atomicity, roots.config-isolation, scope=function) #[expect( clippy::too_many_arguments, reason = "one input per materialization/identity step, mirrors run_agent_exec's own shape" @@ -138,6 +151,7 @@ author: &gix::actor::Signature, sign: &dyn Fn(&[u8]) -> String, mode: Mode, + credentials: &CredentialStore, ) -> Result<AgentPlanOutcome> where O: Find + Write, @@ -168,6 +182,27 @@ sign, }; + // BYOK: this session's own member must have a configured credential + // before a drafting attempt happens at all — no workdir, no prompt + // file. Missing is a deterministic deployment property, not a + // transient hiccup, so it is recorded exactly like a drafting command + // that ran and reported failure (`effect.result-taxonomy`). + let Some(credential) = credentials.get(&session.meta.member) else { + record_result( + refs, + objects, + events, + &results_ref, + oid, + ResultStatus::Fail, + author, + sign, + mode, + )?; + return Ok(AgentPlanOutcome::DraftFailed); + }; + let env = [(credential.var.clone(), credential.secret.clone())]; + // Read-only context gathering against the declared base ref: no output // tree is ever captured back from this checkout (unlike `agent-exec`'s // run), since drafting a plan is never a code change. @@ -209,6 +244,7 @@ workdir: &workdir, toolchains, command, + env: &env, }; // An `Err` here is an infrastructure failure the sandbox itself never // turned into a completed pass/fail: it propagates as-is, past every @@ -223,6 +259,12 @@ // ever moving it toward a terminal `Status::Failed` — that variant // names a session's own run failing, not a drafting attempt that // may simply be retried on a future commit. + #[expect( + clippy::let_underscore_must_use, + reason = "best-effort cleanup; a failure here is not actionable and must not fail an \ + otherwise-recorded result" + )] + let _ = std::fs::remove_dir_all(&workdir); record_result( refs, objects, @@ -243,6 +285,18 @@ source, })?; + // The workdir's job is done once the draft is read back: wipe it + // rather than leave it (and whatever the sandbox's env or the + // command's own output wrote into it) sitting on disk for the rest of + // this process's life. Best-effort, like `agent_worker`'s identical + // cleanup. + #[expect( + clippy::let_underscore_must_use, + reason = "best-effort cleanup; a failure here is not actionable and must not fail an \ + otherwise-successful draft" + )] + let _ = std::fs::remove_dir_all(&workdir); + let (draft_transition, draft_tip) = match ents_forge::agent::draft_plan_transition( refs, objects, @@ -523,6 +577,19 @@ &*self.sign } + /// A credential store configured for `planning_session`'s own + /// member (`jdc`) — every test that expects drafting to actually + /// reach the sandbox uses this rather than [`CredentialStore::empty`]. + fn credentials(&self) -> CredentialStore { + CredentialStore::from_pairs([( + MemberId::new("jdc"), + crate::credentials::Credential { + var: "ANTHROPIC_API_KEY".to_owned(), + secret: "sk-ant-test-token".to_owned(), + }, + )]) + } + /// A brand-new, `planning` session with a prompt and no plan — the /// only precondition [`super::run_agent_plan`]'s /// [`PlanDispatch::Draft`] arm runs against. @@ -593,6 +660,7 @@ &author, fixture.sign_fn(), Mode::Advisory, + &CredentialStore::empty(), ) .expect("dispatch never touches the sandbox for a no-op"); assert!(matches!(run, AgentPlanOutcome::NoOp)); @@ -639,6 +707,7 @@ &author, fixture.sign_fn(), Mode::Advisory, + &fixture.credentials(), ) .expect("drafts and finalizes"); let AgentPlanOutcome::Drafted { @@ -675,6 +744,77 @@ facet_git_tree::deserialize(&result_tree, &fixture.objects).expect("decodes"); assert_eq!(record.status, ResultStatus::Pass); assert_eq!(record.target(), oid); + + // The scratch workdir this drafting run used is cleaned up + // afterward, mirroring `agent_worker::run_agent_exec`'s identical + // guarantee. + assert!( + !scratch.path().join(oid.to_string()).exists(), + "the drafting run's own scratch workdir must not survive the run" + ); + } + + /// A member with no configured credential (`roots.config-isolation`) + /// never reaches the sandbox at all: recorded as a completed `fail` on + /// this effect's own results ref, the session left untouched — the same + /// outcome shape [`AgentPlanOutcome::DraftFailed`] already covers for a + /// drafting command that ran and failed, since a missing credential is + /// exactly as deterministic and exactly as retry-proof. + #[rstest] + // @relation(roots.config-isolation, effect.result-taxonomy, scope=function, role=Verifies) + fn a_member_with_no_configured_credential_never_runs_the_sandbox() { + struct PanicsIfRun; + impl Executor for PanicsIfRun { + #[expect( + clippy::panic_in_result_fn, + reason = "the panic itself is this test's assertion: drafting must never run a \ + sandbox" + )] + fn run( + &self, + _inputs: &SandboxInputs<'_>, + ) -> ents_effect::Result<ents_effect::RunOutput> { + panic!("drafting must never launch a sandbox without a configured credential"); + } + } + + let fixture = Fixture::new(); + let (oid, id) = fixture.planning_session(); + + let scratch = tempfile::tempdir().expect("tempdir"); + let author = fixture.author(); + let run = run_agent_plan( + &fixture.refs, + &fixture.objects, + &NullEventSink, + &PanicsIfRun, + scratch.path(), + &[], + "true", + oid, + &author, + fixture.sign_fn(), + Mode::Advisory, + &CredentialStore::empty(), + ) + .expect("a missing credential is a completed result, not an error"); + assert!(matches!(run, AgentPlanOutcome::DraftFailed)); + + let session = ents_forge::agent::show(&fixture.refs, &fixture.objects, &id).expect("shows"); + assert_eq!(session.meta.status, ents_forge::agent::Status::Planning); + assert!(session.plan.is_none()); + + let results_ref = + ents_model::namespace::result_ref(AGENT_PLAN_NAME, &short_oid(oid)).expect("valid"); + let result_tip = fixture + .refs + .get(results_ref.as_ref()) + .expect("readable") + .expect("result landed"); + let result_tree = commit_tree(&fixture.objects, result_tip).expect("tree"); + let record: ResultRecord = + facet_git_tree::deserialize(&result_tree, &fixture.objects).expect("decodes"); + assert_eq!(record.status, ResultStatus::Fail); } #[rstest] @@ -701,6 +841,7 @@ &author, fixture.sign_fn(), Mode::Advisory, + &fixture.credentials(), ) .expect("runs"); assert!(matches!(run, AgentPlanOutcome::DraftFailed)); @@ -742,6 +883,7 @@ &author, fixture.sign_fn(), Mode::Advisory, + &fixture.credentials(), ) .expect_err("an infra failure must propagate, not silently finalize"); assert!(matches!(error, Error::Effect(_))); @@ -800,6 +942,7 @@ &author, fixture.sign_fn(), Mode::Advisory, + &fixture.credentials(), ) .expect("a lost race is not an error"); assert!(matches!(run, AgentPlanOutcome::DraftLost));

crates/cli/git-ents/src/root.rs @@ -53,6 +53,7 @@ use ents_receive::{Mode, NullEventSink}; use gix_ref_store::LooseRefStore; +use crate::credentials::CredentialStore; use crate::error::{Error, Result}; /// A real, on-disk object store: the repository's own odb, opened for @@ -188,6 +189,12 @@ /// The fixed `Executor` this root wires (`roots.single-node-hosted`): a /// `SpriteExecutor` targeting [`HOSTED_WORKER_NAME`]. pub executor: Box<dyn Executor>, + /// Per-member BYOK credentials (`roots.config-isolation`), read once + /// from [`crate::credentials::CREDENTIALS_FILE_VAR`] — the seam + /// [`crate::agent_worker::run_agent_exec`]/ + /// [`crate::plan_worker::run_agent_plan`] resolve a session's own + /// member's credential from before injecting it into a sandbox launch. + pub credentials: CredentialStore, } /// The Sprite name (and commit author name) the single-node hosted root's @@ -208,19 +215,23 @@ /// /// [`Error::Repo`] or [`Error::Refs`] if `path` is not a git /// repository; [`Error::Receive`] if the reconciliation scan itself - /// fails to read repository state. + /// fails to read repository state; [`Error::Io`]/[`Error::InvalidArgument`] + /// if [`crate::credentials::CREDENTIALS_FILE_VAR`] names a credentials + /// file that cannot be read or is malformed. pub fn open(path: impl AsRef<Path>) -> Result<Self> { let path = path.as_ref().to_owned(); let refs = LooseRefStore::open(&path)?; let objects = QuarantineObjects::open(&path)?; let events = ents_receive::MemoryEventSink::default(); ents_receive::reconcile(&refs, &objects, &events)?; + let credentials = CredentialStore::from_env()?; Ok(Self { path, refs, objects, events, executor: Box::new(ents_effect::SpriteExecutor::new(HOSTED_WORKER_NAME)), + credentials, }) }

crates/cli/git-ents/tests/agent.rs @@ -322,6 +322,17 @@ assert_eq!(planning.meta.status, Status::Planning); assert!(planning.plan.is_none()); + // BYOK (`roots.config-isolation`): both the headless draft and the + // exec run below need this session's own member's credential + // configured, or they fail closed rather than run. + let credentials = git_ents::credentials::CredentialStore::from_pairs([( + planning.meta.member.clone(), + git_ents::credentials::Credential { + var: "ANTHROPIC_API_KEY".to_owned(), + secret: "sk-ant-test-token".to_owned(), + }, + )]); + // 2. The `agent-plan` effect drafts headlessly. let session_ref = ents_model::namespace::agent_session_ref(&id).expect("valid"); let tip = root @@ -351,6 +362,7 @@ &worker_author, &|payload| signer.sign(payload), root.mode(), + &credentials, ) .expect("drafts"); assert!(matches!( @@ -397,6 +409,7 @@ &worker_author, &|payload| signer.sign(payload), Mode::Advisory, + &credentials, ) .expect("claims and runs"); assert!(matches!(

crates/kernel/ents-effect/src/docker.rs @@ -73,20 +73,34 @@ /// use ents_effect::executor::EXIT_MARKER; /// use std::path::Path; /// -/// let args = run_args(Path::new("/tmp/s/work"), &[], "cargo test"); +/// let args = run_args(Path::new("/tmp/s/work"), &[], "cargo test", &[]); /// assert!(args.contains(&"/tmp/s/work:/work".to_owned())); /// assert!(args.contains(&"debian:stable-slim".to_owned())); /// let script = args.last().expect("has a script"); /// assert!(script.contains("cargo test") && script.contains(EXIT_MARKER)); /// ``` #[must_use] -pub fn run_args(workdir: &Path, toolchains: &[(String, PathBuf)], command: &str) -> Vec<String> { +pub fn run_args( + workdir: &Path, + toolchains: &[(String, PathBuf)], + command: &str, + env: &[(String, String)], +) -> Vec<String> { let mut args = vec![ "run".to_owned(), "--rm".to_owned(), "-v".to_owned(), format!("{}:{WORKDIR}", workdir.display()), ]; + // Passed as literal argv entries, never through a shell — `docker run` + // itself sets these on the container's environment, so a secret (a + // BYOK credential, `SandboxInputs::env`) needs no shell quoting at all + // here (unlike `crate::sprite::SpriteExecutor`, which ships the whole + // command as one remote shell-script string). + for (var, secret) in env { + args.push("-e".to_owned()); + args.push(format!("{var}={secret}")); + } let mut sandbox_dirs = Vec::with_capacity(toolchains.len()); for (name, host_dir) in toolchains { let sandbox_dir = format!("{TOOLCHAINS_DIR}/{name}/bin"); @@ -115,7 +129,12 @@ // @relation(effect.result-taxonomy, scope=function) fn run(&self, inputs: &SandboxInputs<'_>) -> Result<RunOutput> { ensure_docker()?; - let args = run_args(inputs.workdir, inputs.toolchains, inputs.command); + let args = run_args( + inputs.workdir, + inputs.toolchains, + inputs.command, + inputs.env, + ); let output = Command::new("docker") .args(&args) .output() @@ -150,7 +169,7 @@ #[rstest] // @relation(effect.execution, scope=function, role=Verifies) fn run_args_binds_the_workdir() { - let args = run_args(Path::new("/tmp/s/work"), &[], "cargo test"); + let args = run_args(Path::new("/tmp/s/work"), &[], "cargo test", &[]); assert_eq!( args, vec![ @@ -171,7 +190,7 @@ #[rstest] // @relation(effect.result-taxonomy, scope=function, role=Verifies) fn run_args_script_completes_with_the_exit_marker() { - let args = run_args(Path::new("/w"), &[], "true"); + let args = run_args(Path::new("/w"), &[], "true", &[]); let script = args.last().expect("has a script"); assert!( script.contains(crate::executor::EXIT_MARKER), @@ -184,7 +203,7 @@ // @relation(effect.execution, effect.toolchains, scope=function, role=Verifies) fn run_args_binds_each_toolchain_read_only_and_activates_it() { let toolchains = vec![("rust".to_owned(), PathBuf::from("/cache/rust/bin"))]; - let args = run_args(Path::new("/w"), &toolchains, "cargo test"); + let args = run_args(Path::new("/w"), &toolchains, "cargo test", &[]); assert!(args.contains(&"/cache/rust/bin:/toolchains/rust/bin:ro".to_owned())); let last = args.last().expect("has a command"); assert!(last.contains("export PATH=/toolchains/rust/bin:$PATH; cargo test")); @@ -193,10 +212,23 @@ #[rstest] // @relation(effect.execution, scope=function, role=Verifies) fn run_args_uses_the_minimal_base_image() { - let args = run_args(Path::new("/w"), &[], "true"); + let args = run_args(Path::new("/w"), &[], "true", &[]); assert_eq!( args.get(args.len().saturating_sub(4)).map(String::as_str), Some(IMAGE) ); } + + #[rstest] + // @relation(roots.config-isolation, scope=function, role=Verifies) + fn run_args_passes_env_as_literal_docker_flags_not_shell_text() { + let env = vec![("ANTHROPIC_API_KEY".to_owned(), "sk-ant-abc".to_owned())]; + let args = run_args(Path::new("/w"), &[], "true", &env); + assert!(args.contains(&"-e".to_owned())); + assert!(args.contains(&"ANTHROPIC_API_KEY=sk-ant-abc".to_owned())); + // Never folded into the shell script itself — that's the sprite + // backend's own quoting concern, not docker's. + let script = args.last().expect("has a script"); + assert!(!script.contains("sk-ant-abc")); + } }

crates/kernel/ents-effect/src/executor.rs @@ -29,6 +29,14 @@ /// The run command, exactly as stored on the effect definition /// (`model.effect-definition`). pub command: &'a str, + /// Extra environment variables to inject into the launched command, + /// deployment state a composition root resolves (a per-member BYOK + /// credential, `roots.config-isolation`) and hands down for exactly + /// this one run — never read from repository data, never written back + /// to it. Every backend injects these the same way it launches the + /// command at all; empty for every ordinary effect run, which has + /// nothing to inject. + pub env: &'a [(String, String)], } /// What a completed run reported. Only `Pass` or `Fail` @@ -83,7 +91,7 @@ /// } /// /// let dir = tempfile::tempdir().expect("tempdir"); -/// let inputs = SandboxInputs { workdir: dir.path(), toolchains: &[], command: "true" }; +/// let inputs = SandboxInputs { workdir: dir.path(), toolchains: &[], command: "true", env: &[] }; /// let output = AlwaysPass.run(&inputs).expect("infallible"); /// assert_eq!(output.status, RunStatus::Pass); /// ``` @@ -125,6 +133,49 @@ format!("export PATH={path}:$PATH; {command}") } +/// Prefix `command` with an `export` for each of `env`'s pairs, single-quoted +/// so an arbitrary secret value (a BYOK credential, +/// [`SandboxInputs::env`]) round-trips through a `sh -c` script unmodified +/// regardless of its own content — used by every backend that ships the +/// command as one shell-script string to a remote or containerized shell +/// ([`crate::sprite::SpriteExecutor`], [`crate::docker::DockerExecutor`]). +/// The unsandboxed backend needs no such quoting: it sets the child +/// process's environment directly via `Command::envs`, never folding a +/// secret into shell source at all. +/// +/// # Examples +/// +/// ``` +/// use ents_effect::executor::inject_env; +/// +/// let env = vec![("ANTHROPIC_API_KEY".to_owned(), "sk-ant-abc".to_owned())]; +/// assert_eq!( +/// inject_env("run-agent", &env), +/// "export ANTHROPIC_API_KEY='sk-ant-abc'; run-agent" +/// ); +/// assert_eq!(inject_env("run-agent", &[]), "run-agent"); +/// ``` +#[must_use] +pub fn inject_env(command: &str, env: &[(String, String)]) -> String { + if env.is_empty() { + return command.to_owned(); + } + let exports = env + .iter() + .map(|(var, secret)| format!("export {var}={}", shell_single_quote(secret))) + .collect::<Vec<_>>() + .join("; "); + format!("{exports}; {command}") +} + +/// Single-quote `value` for a POSIX shell, escaping any embedded `'` by +/// closing the quote, emitting an escaped literal quote, and reopening it — +/// the standard `'\''` trick, so a credential containing an apostrophe (or +/// any other shell metacharacter) still round-trips as one literal string. +fn shell_single_quote(value: &str) -> String { + format!("'{}'", value.replace('\'', r"'\''")) +} + /// The sentinel [`wrap_exit_marker`] appends after the wrapped command, so /// a CLI-driven backend can tell "the command completed and exited with /// this status" apart from "the CLI or its transport failed" — the @@ -227,6 +278,38 @@ assert_eq!(activate("run", &[]), "run"); } + #[rstest] + // @relation(roots.config-isolation, scope=function, role=Verifies) + fn inject_env_is_identity_with_no_env() { + assert_eq!(inject_env("run", &[]), "run"); + } + + #[rstest] + // @relation(roots.config-isolation, scope=function, role=Verifies) + fn inject_env_exports_every_pair_before_the_command() { + let env = vec![ + ("A".to_owned(), "one".to_owned()), + ("B".to_owned(), "two".to_owned()), + ]; + assert_eq!( + inject_env("run", &env), + "export A='one'; export B='two'; run" + ); + } + + #[rstest] + // @relation(roots.config-isolation, scope=function, role=Verifies) + fn inject_env_round_trips_a_secret_with_an_embedded_quote_through_a_real_shell() { + let env = vec![("SECRET".to_owned(), "it's a secret".to_owned())]; + let script = inject_env("printf '%s' \"$SECRET\"", &env); + let output = std::process::Command::new("sh") + .arg("-c") + .arg(&script) + .output() + .expect("sh runs"); + assert_eq!(String::from_utf8_lossy(&output.stdout), "it's a secret"); + } + #[rstest] #[case::pass("out\n__ENTS_EFFECT_EXIT=0\n", Some((RunStatus::Pass, "out")))] #[case::fail("out\n__ENTS_EFFECT_EXIT=1\n", Some((RunStatus::Fail, "out")))]

crates/kernel/ents-effect/src/run.rs @@ -170,6 +170,11 @@ workdir: &workdir, toolchains, command: &effect.run, + // An ordinary effect run has no per-member credential to inject + // (`roots.config-isolation`'s BYOK seam is `agent_worker`'s and + // `plan_worker`'s own concern, in `git-ents`); every backend still + // accepts the field uniformly, it is simply empty here. + env: &[], }; let output = executor.run(&inputs)?; let status = match output.status {

crates/kernel/ents-effect/src/sprite.rs @@ -25,7 +25,7 @@ use crate::error::{Error, Result}; use crate::executor::{ - Executor, RunOutput, SandboxInputs, activate, parse_exit_marker, wrap_exit_marker, + Executor, RunOutput, SandboxInputs, activate, inject_env, parse_exit_marker, wrap_exit_marker, }; /// Where the workdir is unpacked inside the Sprite. @@ -273,13 +273,19 @@ } /// The in-Sprite script for one run: enter the synced workdir, run the -/// activated command wrapped by [`wrap_exit_marker`]. A `cd` failure (the -/// workdir sync silently lost) exits before the marker can print, so it -/// surfaces as infrastructure, not as a recorded `fail` — the same -/// discrimination the marker gives a dying transport -/// (`effect.result-taxonomy`). -fn run_script(activated: &str) -> String { - format!("cd {WORKDIR} || exit 70\n{}", wrap_exit_marker(activated)) +/// activated command — with `env`'s pairs exported first +/// ([`inject_env`], `roots.config-isolation`: a per-member BYOK credential +/// the composition root resolved, injected only here, at sandbox launch, +/// never written to any tree this crate builds) — wrapped by +/// [`wrap_exit_marker`]. A `cd` failure (the workdir sync silently lost) +/// exits before the marker can print, so it surfaces as infrastructure, not +/// as a recorded `fail` — the same discrimination the marker gives a dying +/// transport (`effect.result-taxonomy`). +fn run_script(activated: &str, env: &[(String, String)]) -> String { + format!( + "cd {WORKDIR} || exit 70\n{}", + wrap_exit_marker(&inject_env(activated, env)) + ) } impl Executor for SpriteExecutor { @@ -296,7 +302,7 @@ sandbox_dirs.push((toolchain_name.clone(), sandbox_dir)); } - let script = run_script(&activate(inputs.command, &sandbox_dirs)); + let script = run_script(&activate(inputs.command, &sandbox_dirs), inputs.env); let output = Command::new("sprite") .args(["exec", "-s", &self.name, "--", "sh", "-c", &script]) .output() @@ -340,7 +346,7 @@ #[rstest] // @relation(effect.result-taxonomy, scope=function, role=Verifies) fn run_script_gates_the_exit_marker_on_a_successful_cd() { - let script = run_script("cargo test"); + let script = run_script("cargo test", &[]); // A failed cd exits before the marker can print, so a lost workdir // sync surfaces as infrastructure, never as a recorded fail. assert!(script.starts_with("cd /work || exit 70\n")); @@ -348,6 +354,14 @@ assert!(script.contains("cargo test")); } + #[rstest] + // @relation(roots.config-isolation, scope=function, role=Verifies) + fn run_script_exports_env_before_the_activated_command() { + let env = vec![("ANTHROPIC_API_KEY".to_owned(), "sk-ant-abc".to_owned())]; + let script = run_script("cargo test", &env); + assert!(script.contains("export ANTHROPIC_API_KEY='sk-ant-abc'; cargo test")); + } + #[rstest] // @relation(effect.execution, scope=function, role=Verifies) fn unpack_script_kills_orphans_before_wiping_the_destination() {

crates/kernel/ents-effect/src/unsandboxed.rs @@ -31,6 +31,13 @@ .arg("-c") .arg(activate(inputs.command, &dirs)) .current_dir(inputs.workdir) + // Set directly on the child's environment rather than folded + // into the shell script `activate` built above: a secret (a + // BYOK credential, `SandboxInputs::env`) never needs shell + // quoting at all this way, unlike the sandboxed backends that + // must ship the whole command as one script string + // (`crate::executor::inject_env`). + .envs(inputs.env.iter().cloned()) .output() .map_err(|e| Error::Spawn { program: "sh".to_owned(), @@ -65,6 +72,7 @@ workdir: dir.path(), toolchains: &[], command: "true", + env: &[], }; let output = UnsandboxedExecutor.run(&inputs).expect("runs"); assert_eq!(output.status, RunStatus::Pass); @@ -78,6 +86,7 @@ workdir: dir.path(), toolchains: &[], command: "false", + env: &[], }; let output = UnsandboxedExecutor .run(&inputs) @@ -96,9 +105,25 @@ workdir: dir.path(), toolchains: &toolchains, command: "echo $PATH", + env: &[], }; let output = UnsandboxedExecutor.run(&inputs).expect("runs"); assert!(output.log.contains("bin")); let _: Vec<(String, PathBuf)> = toolchains; } + + #[rstest] + // @relation(roots.config-isolation, scope=function, role=Verifies) + fn unsandboxed_injects_env_directly_on_the_child_process() { + let dir = tempfile::tempdir().expect("tempdir"); + let env = vec![("ENTS_TEST_CREDENTIAL".to_owned(), "sk-ant-test".to_owned())]; + let inputs = SandboxInputs { + workdir: dir.path(), + toolchains: &[], + command: "printf '%s' \"$ENTS_TEST_CREDENTIAL\"", + env: &env, + }; + let output = UnsandboxedExecutor.run(&inputs).expect("runs"); + assert_eq!(output.log, "sk-ant-test"); + } }

crates/cli/git-ents/src/commands/serve.rs @@ -233,6 +233,7 @@ objects, events, executor: _, + credentials: _, } = root; Ok(Arc::new( AppState::new(

crates/cli/git-ents/src/credentials.rs @@ -1,0 +1,263 @@ +//! Per-member BYOK credentials (`roots.config-isolation`): read from the +//! hosted composition root's own deployment config, injected into a +//! sandbox's environment at launch ([`crate::agent_worker::run_agent_exec`], +//! [`crate::plan_worker::run_agent_plan`]) — never repository data +//! (`effect.deployment-property`), never written to any tree this crate +//! builds. +//! +//! # Shape +//! +//! A member's credential is a secret value plus the environment variable +//! name a sandboxed agent command expects it under (an Anthropic API key or +//! subscription token, e.g. `ANTHROPIC_API_KEY`) — every member may use +//! their own credential and their own variable name, since BYOK means each +//! member's runs are billed and rate-limited against their own account. +//! +//! # Where it comes from +//! +//! [`CredentialStore::from_env`] mirrors the two deployment-config +//! conventions this crate already has for the hosted root: an env var +//! naming a *path* the process reads once at startup +//! ([`crate::sign::resolve_key_path`]'s own `user.signingkey`-or-default +//! shape for the worker's signing key), and an env var carrying a +//! deployment secret directly ([`ents_effect::sprite::SPRITES_TOKEN_VAR`], +//! read once by [`ents_effect::sprite::ensure_auth`]). Since one member's +//! credential is itself secret material like the Sprite token, and there +//! may be many members, this store takes the file-path shape: +//! [`CREDENTIALS_FILE_VAR`] names a file the deployment operator provisions +//! (one line per member), read once when a [`crate::root::HostedRoot`] +//! opens and handed down by reference from there — never re-read per run, +//! never read by any code but this composition root +//! (`roots.config-isolation`: "Configuration MUST select trait +//! implementations only at the composition root and MUST NOT leak past +//! it"). + +use std::collections::HashMap; +use std::path::Path; + +use ents_model::MemberId; + +use crate::error::{Error, Result}; + +/// The env var naming the credentials file's path on the hosted deployment. +/// Unset means "no member has a credential" — the common case for a +/// repository with no agent sessions configured at all, and for every +/// non-hosted root ([`crate::root::LocalRoot`] never runs `agent-exec`/ +/// `agent-plan` today, so it never constructs a [`CredentialStore`]). +pub const CREDENTIALS_FILE_VAR: &str = "GIT_ENTS_CREDENTIALS_FILE"; + +/// One member's injected credential: the secret value, and the environment +/// variable name a sandbox's launched command expects it under. +#[derive(Debug, Clone)] +pub struct Credential { + /// The environment variable name to inject the secret as, e.g. + /// `ANTHROPIC_API_KEY`. + pub var: String, + /// The secret value itself — a member's own Anthropic API key or + /// subscription token (BYOK). + pub secret: String, +} + +/// Per-member credentials, keyed by [`MemberId`] — deployment state +/// (`effect.deployment-property`), constructed only at a composition root +/// ([`crate::root::HostedRoot::open`]) and handed by reference into +/// [`crate::agent_worker::run_agent_exec`]/[`crate::plan_worker::run_agent_plan`], +/// never persisted to any git object. +#[derive(Debug, Clone, Default)] +pub struct CredentialStore { + by_member: HashMap<MemberId, Credential>, +} + +impl CredentialStore { + /// An empty store: no member has a credential configured. + #[must_use] + pub fn empty() -> Self { + Self::default() + } + + /// Build a store directly from `(member, credential)` pairs — the shape + /// a test fixture uses; production code reaches a [`CredentialStore`] + /// only through [`Self::from_env`]. + #[must_use] + pub fn from_pairs(entries: impl IntoIterator<Item = (MemberId, Credential)>) -> Self { + Self { + by_member: entries.into_iter().collect(), + } + } + + /// Load from [`CREDENTIALS_FILE_VAR`]'s named file, or [`Self::empty`] + /// if that env var is unset. + /// + /// # Errors + /// + /// See [`Self::load`]. + pub fn from_env() -> Result<Self> { + match std::env::var_os(CREDENTIALS_FILE_VAR) { + Some(path) => Self::load(Path::new(&path)), + None => Ok(Self::empty()), + } + } + + /// Parse the store from `path`'s contents: one + /// `<member-id>\t<var-name>\t<secret>` line per member, blank lines and + /// `#`-prefixed lines ignored. + /// + /// # Errors + /// + /// [`Error::Io`] if `path` cannot be read; [`Error::InvalidArgument`] for + /// a line that is not exactly three tab-separated fields, or whose + /// var-name field is not a POSIX environment variable name + /// (`[A-Za-z_][A-Za-z0-9_]*`) — the name is folded verbatim into the + /// `export` statement [`ents_effect::executor::inject_env`] builds for + /// shell-script backends, so anything else would corrupt the sandbox + /// script (only the secret *value* is quote-escaped there, on the + /// grounds that the operator-authored name, unlike the member-supplied + /// secret, has no reason to ever contain metacharacters). + pub fn load(path: &Path) -> Result<Self> { + let text = std::fs::read_to_string(path).map_err(|source| Error::Io { + path: path.to_owned(), + source, + })?; + let mut by_member = HashMap::new(); + for (number, line) in text.lines().enumerate() { + let line = line.trim(); + if line.is_empty() || line.starts_with('#') { + continue; + } + let mut fields = line.splitn(3, '\t'); + let (Some(member), Some(var), Some(secret)) = + (fields.next(), fields.next(), fields.next()) + else { + return Err(Error::InvalidArgument(format!( + "{}:{}: malformed credential line (expected \ + <member-id>\\t<var-name>\\t<secret>)", + path.display(), + number.saturating_add(1) + ))); + }; + if !is_env_var_name(var) { + return Err(Error::InvalidArgument(format!( + "{}:{}: {var:?} is not a valid environment variable name \ + ([A-Za-z_][A-Za-z0-9_]*)", + path.display(), + number.saturating_add(1) + ))); + } + by_member.insert( + MemberId::new(member), + Credential { + var: var.to_owned(), + secret: secret.to_owned(), + }, + ); + } + Ok(Self { by_member }) + } + + /// `member`'s configured credential, if the deployment has one. + #[must_use] + pub fn get(&self, member: &MemberId) -> Option<&Credential> { + self.by_member.get(member) + } +} + +/// Whether `name` is a POSIX environment variable name: +/// `[A-Za-z_][A-Za-z0-9_]*` (see [`CredentialStore::load`]'s own doc for +/// why anything else is refused at parse time). +fn is_env_var_name(name: &str) -> bool { + let mut chars = name.chars(); + chars + .next() + .is_some_and(|first| first.is_ascii_alphabetic() || first == '_') + && chars.all(|c| c.is_ascii_alphanumeric() || c == '_') +} + +#[cfg(test)] +mod tests { + #![allow(clippy::expect_used, reason = "unit test")] + + use rstest::rstest; + + use super::*; + + #[rstest] + // @relation(roots.config-isolation, scope=function, role=Verifies) + fn empty_store_has_no_credential_for_anyone() { + let store = CredentialStore::empty(); + assert!(store.get(&MemberId::new("jdc")).is_none()); + } + + #[rstest] + // @relation(roots.config-isolation, scope=function, role=Verifies) + fn from_pairs_looks_up_by_member() { + let store = CredentialStore::from_pairs([( + MemberId::new("jdc"), + Credential { + var: "ANTHROPIC_API_KEY".to_owned(), + secret: "sk-ant-abc".to_owned(), + }, + )]); + let credential = store.get(&MemberId::new("jdc")).expect("configured"); + assert_eq!(credential.var, "ANTHROPIC_API_KEY"); + assert_eq!(credential.secret, "sk-ant-abc"); + assert!(store.get(&MemberId::new("someone-else")).is_none()); + } + + #[rstest] + // @relation(roots.config-isolation, scope=function, role=Verifies) + fn load_parses_one_credential_per_line_and_skips_comments_and_blanks() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("credentials"); + std::fs::write( + &path, + "# deployment credentials\n\njdc\tANTHROPIC_API_KEY\tsk-ant-abc\n\nmallory\tANTHROPIC_API_KEY\tsk-ant-xyz\n", + ) + .expect("write"); + + let store = CredentialStore::load(&path).expect("parses"); + assert_eq!( + store.get(&MemberId::new("jdc")).expect("configured").secret, + "sk-ant-abc" + ); + assert_eq!( + store + .get(&MemberId::new("mallory")) + .expect("configured") + .secret, + "sk-ant-xyz" + ); + } + + #[rstest] + // @relation(roots.config-isolation, scope=function, role=Verifies) + fn load_rejects_a_malformed_line() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("credentials"); + std::fs::write(&path, "jdc\tANTHROPIC_API_KEY\n").expect("write"); + + let error = CredentialStore::load(&path).expect_err("missing the secret field"); + assert!(matches!(error, Error::InvalidArgument(_))); + } + + #[rstest] + #[case::shell_metacharacters("KEY; rm -rf /")] + #[case::leading_digit("1KEY")] + #[case::empty("")] + // @relation(roots.config-isolation, scope=function, role=Verifies) + fn load_rejects_a_var_name_that_is_not_a_posix_env_name(#[case] var: &str) { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("credentials"); + std::fs::write(&path, format!("jdc\t{var}\tsk-ant-abc\n")).expect("write"); + + let error = CredentialStore::load(&path).expect_err("the var name is folded into shell"); + assert!(matches!(error, Error::InvalidArgument(_))); + } + + // `from_env`'s own body is a two-arm match over `CREDENTIALS_FILE_VAR` + // delegating straight to `load` (covered above) or `empty` (covered + // above); it is deliberately not exercised here via + // `std::env::set_var`/`remove_var`, since mutating process-global env + // state is unsound to race against other tests in the same process + // (`std::env`'s own safety docs) — the two branches it can take are + // both already proven correct on their own. +}

crates/cli/git-ents/tests/agent_credentials.rs @@ -1,0 +1,539 @@ +//! Phase 6's redaction audit (`docs/agent-sessions-plan.adoc`): a per-member +//! BYOK credential (`roots.config-isolation`) is injected into a sandbox's +//! environment at launch (`SandboxInputs::env`) and must never be written +//! to repository data — every persisted artifact of a completed session +//! must be free of it. +//! +//! Two tests, deliberately asymmetric, spell out exactly what this system +//! guarantees and what it does not: +//! +//! - [`a_well_behaved_command_never_leaks_its_injected_credential`] proves +//! the actual guarantee: the worker-side machinery +//! (`git_ents::plan_worker::run_agent_plan`, +//! `git_ents::agent_worker::run_agent_exec`) never itself writes the +//! credential into any persisted artifact — every git object reachable +//! from the session ref, both effects' own result refs, and the result +//! branch, plus the on-disk scratch workdir left behind, are all swept +//! for the sentinel and found clean. +//! - [`a_malicious_command_can_still_exfiltrate_its_own_env`] documents, +//! honestly, the boundary that guarantee stops at: a command that +//! deliberately echoes its own environment into its own log output or a +//! file inside the workdir it controls gets that byte range faithfully +//! recorded onto the result branch and the session's transcript — the +//! system's job is injecting the credential at launch and never touching +//! repository data with it itself, not sanitizing an adversarial +//! command's own reported output. This test does not claim the system +//! prevents that; it demonstrates the boundary so nobody has to take the +//! first test's guarantee on faith beyond what it actually covers. + +#![allow(clippy::expect_used, reason = "integration test")] + +mod common; + +use std::collections::HashSet; +use std::sync::Mutex; + +use ents_effect::executor::SandboxInputs; +use ents_effect::run::short_oid; +use ents_effect::{Executor, RunOutput, RunStatus}; +use ents_forge::agent::Status; +use ents_model::MemberId; +use ents_receive::Mode; +use git_ents::commands::agent; +use git_ents::credentials::{Credential, CredentialStore}; +use git_ents::root::LocalRoot; +use git_ents::{agent_worker, plan_worker}; +use gix_hash::ObjectId; +use gix_object::{Commit, CommitRef, Find, Kind, TreeRef}; +use gix_ref_store::{Expected, RefEdit, RefStore, RefStoreRead as _}; + +/// The sentinel credential value: distinctive enough that any accidental +/// match is meaningful, shaped like a real Anthropic API key so this test +/// exercises the actual string shape a BYOK credential would have. +const SENTINEL: &str = "sk-ant-SENTINEL-0000000000000000-do-not-persist"; +const VAR: &str = "ANTHROPIC_API_KEY"; + +/// Write an empty-tree commit and move `refname` to it directly through the +/// ref store — mirrors `tests/agent.rs`'s own identical helper (this +/// crate's own convention for a small per-file duplicate rather than a +/// shared `tests/common` addition for a single-use helper). +fn advance_branch( + refs: &dyn RefStore, + objects: &impl gix_object::Write, + refname: &str, + seconds: i64, +) -> ObjectId { + let empty_tree = objects.write(&gix_object::Tree::empty()).expect("tree"); + let actor = gix::actor::Signature { + name: "test".into(), + email: "test@ents.test".into(), + time: gix::date::Time { seconds, offset: 0 }, + }; + let commit = Commit { + tree: empty_tree, + parents: Default::default(), + author: actor.clone(), + committer: actor, + encoding: None, + message: "advance".into(), + extra_headers: Vec::new(), + }; + let mut raw = Vec::new(); + gix_object::WriteTo::write_to(&commit, &mut raw).expect("serialize"); + let oid = objects.write_buf(Kind::Commit, &raw).expect("write"); + + let name: gix::refs::FullName = refname.try_into().expect("valid refname"); + refs.transaction(&[RefEdit { + name, + expected: Expected::Any, + new: Some(oid), + }]) + .expect("moves the ref"); + oid +} + +/// Recursively collect every blob's bytes and every commit's message, +/// reachable from `start` by following *every* parent (not just the first) +/// and every tree entry — a full reachability sweep of one git object +/// graph, not a single-tip spot check. +fn reachable_text( + objects: &impl Find, + start: ObjectId, + seen: &mut HashSet<ObjectId>, + out: &mut Vec<Vec<u8>>, +) { + if !seen.insert(start) { + return; + } + let mut buf = Vec::new(); + let Some(data) = Find::try_find(objects, &start, &mut buf).expect("object store reads") else { + return; + }; + match data.kind { + Kind::Commit => { + let commit = CommitRef::from_bytes(data.data, start.kind()).expect("commit parses"); + out.push(commit.message.to_vec()); + let tree = commit.tree(); + let parents: Vec<ObjectId> = commit.parents().collect(); + drop(commit); + reachable_text(objects, tree, seen, out); + for parent in parents { + reachable_text(objects, parent, seen, out); + } + } + Kind::Tree => { + let tree = TreeRef::from_bytes(data.data, start.kind()).expect("tree parses"); + let children: Vec<ObjectId> = tree + .entries + .iter() + .map(|entry| entry.oid.to_owned()) + .collect(); + drop(tree); + for child in children { + reachable_text(objects, child, seen, out); + } + } + Kind::Blob => out.push(data.data.to_vec()), + Kind::Tag => {} + } +} + +/// Whether `bytes` contains the sentinel credential, as a raw byte +/// substring (not a UTF-8 string comparison) so this catches the sentinel +/// regardless of what else surrounds it in a blob. +fn contains_sentinel(bytes: &[u8]) -> bool { + let needle = SENTINEL.as_bytes(); + bytes.windows(needle.len()).any(|window| window == needle) +} + +/// Assert that every object reachable from `tip` (including `tip` itself) +/// is free of the sentinel — `label` names what ref this tip came from, for +/// a legible failure. +fn assert_no_sentinel(objects: &impl Find, tip: ObjectId, label: &str) { + let mut seen = HashSet::new(); + let mut blobs = Vec::new(); + reachable_text(objects, tip, &mut seen, &mut blobs); + for blob in &blobs { + assert!( + !contains_sentinel(blob), + "{label}: the sentinel credential leaked into a persisted git object reachable from {tip}" + ); + } +} + +/// A stub `Executor` for both `agent-plan` and `agent-exec`: records every +/// `env` pair it was launched with (so the test can confirm the credential +/// really was injected), and either behaves like an ordinary well-behaved +/// agent command (never echoes its own env anywhere) or, when `leak` is +/// set, deliberately writes its own env into a file in the workdir and +/// into its own log output — standing in for a buggy or malicious command, +/// never for the ordinary case. +struct RecordingExecutor { + seen_env: Mutex<Vec<(String, String)>>, + leak: bool, + /// When set, write `plan_worker::AGENT_PLAN_DRAFT_FILE` so this stub + /// also satisfies `run_agent_plan`'s own contract. + drafts: bool, +} + +impl Executor for RecordingExecutor { + #[expect( + clippy::unwrap_in_result, + reason = "test fixture: a poisoned mutex or a workdir write failure here is a broken \ + test, not a condition under test" + )] + fn run(&self, inputs: &SandboxInputs<'_>) -> ents_effect::Result<RunOutput> { + self.seen_env + .lock() + .expect("uncontended in this test") + .extend(inputs.env.iter().cloned()); + if self.drafts { + std::fs::write( + inputs.workdir.join(plan_worker::AGENT_PLAN_DRAFT_FILE), + "1. reproduce\n2. fix\n3. verify", + ) + .expect("write draft"); + } + if self.leak { + // Deliberately misbehaves: writes its own env into a file it + // controls, and echoes it into its own reported output. + let logged = inputs + .env + .iter() + .map(|(k, v)| format!("{k}={v}")) + .collect::<Vec<_>>() + .join("\n"); + std::fs::write(inputs.workdir.join("leaked-by-the-command.txt"), &logged) + .expect("write"); + return Ok(RunOutput { + status: RunStatus::Pass, + log: logged, + }); + } + Ok(RunOutput { + status: RunStatus::Pass, + log: "ordinary agent output, no secrets here".to_owned(), + }) + } +} + +/// Fixture: a fresh session, drafted and confirmed via a non-leaking +/// `RecordingExecutor`, with a `CredentialStore` configured for the +/// session's own member under [`SENTINEL`] — everything both tests share +/// before diverging on the `agent-exec` executor's own behavior. +struct Drafted { + // Kept alive for this fixture's whole lifetime: `root` only borrows this + // repository's path, it does not own the `TempDir` -- dropping `fixture` + // early would delete the on-disk repository out from under `root`. + _fixture: common::Fixture, + root: LocalRoot, + id: String, + credentials: CredentialStore, + worker_author: gix::actor::Signature, + signer: git_ents::sign::Signer, + scratch: tempfile::TempDir, + /// The dequeued `planning` oid the headless draft ran against -- + /// `agent-plan`'s own results ref is keyed by this, not by `queued_tip`. + planning_tip: ObjectId, + queued_tip: ObjectId, +} + +fn draft_and_confirm(seed: u8) -> Drafted { + let fixture = common::Fixture::new(seed); + let root = LocalRoot::open(fixture.path()).expect("opens"); + advance_branch(&root.refs, &root.objects, "refs/heads/main", 100); + + let id = agent::new( + &root, + "fix the flaky test".to_owned(), + "claude-sonnet-5".to_owned(), + vec![], + "refs/heads/main".to_owned(), + "manual".to_owned(), + None, + Some(fixture.key_path.clone()), + ) + .expect("starts a session"); + let member = agent::show(&root, &id).expect("shows").meta.member; + + let credentials = CredentialStore::from_pairs([( + member, + Credential { + var: VAR.to_owned(), + secret: SENTINEL.to_owned(), + }, + )]); + + let session_ref = ents_model::namespace::agent_session_ref(&id).expect("valid"); + let planning_tip = root + .refs + .get(session_ref.as_ref()) + .expect("readable") + .expect("exists"); + let worker_author = gix::actor::Signature { + name: "worker".into(), + email: "worker@ents.test".into(), + time: gix::date::Time { + seconds: 200, + offset: 0, + }, + }; + let signer = git_ents::sign::Signer::load(&fixture.key_path).expect("loads"); + let scratch = tempfile::tempdir().expect("tempdir"); + + // Drafting is itself a credentialed run: a non-leaking executor here + // keeps this fixture's own drafting step out of the redaction sweep's + // way (its own credential handling is proven independently below). + let draft_executor = RecordingExecutor { + seen_env: Mutex::new(Vec::new()), + leak: false, + drafts: true, + }; + let plan_run = plan_worker::run_agent_plan( + &root.refs, + &root.objects, + &root.events, + &draft_executor, + scratch.path(), + &[], + "true", + planning_tip, + &worker_author, + &|payload| signer.sign(payload), + root.mode(), + &credentials, + ) + .expect("drafts"); + assert!(matches!( + plan_run, + plan_worker::AgentPlanOutcome::Drafted { .. } + )); + assert!( + draft_executor + .seen_env + .lock() + .expect("uncontended in this test") + .iter() + .any(|(k, v)| k == VAR && v == SENTINEL), + "drafting must receive the injected credential too" + ); + + agent::confirm(&root, &id, None, Some(fixture.key_path.clone())).expect("confirms"); + let queued_ref = ents_model::namespace::agent_session_ref(&id).expect("valid"); + let queued_tip = root + .refs + .get(queued_ref.as_ref()) + .expect("readable") + .expect("exists"); + + Drafted { + _fixture: fixture, + root, + id, + credentials, + worker_author, + signer, + scratch, + planning_tip, + queued_tip, + } +} + +/// The system's actual guarantee: worker-side machinery never itself writes +/// the injected BYOK credential into any persisted artifact. Sweeps the +/// session ref, both effects' own result refs, the result branch, and the +/// on-disk scratch remnants after the run. +// @relation(roots.config-isolation, effect.deployment-property, scope=function, role=Verifies) +#[test] +fn a_well_behaved_command_never_leaks_its_injected_credential() { + let d = draft_and_confirm(60); + + let executor = RecordingExecutor { + seen_env: Mutex::new(Vec::new()), + leak: false, + drafts: false, + }; + let run = agent_worker::run_agent_exec( + &d.root.refs, + &d.root.objects, + &d.root.events, + &executor, + d.scratch.path(), + &[], + "true", + d.queued_tip, + MemberId::new("worker"), + "sprite-1".to_owned(), + &d.worker_author, + &|payload| d.signer.sign(payload), + Mode::Advisory, + &d.credentials, + ) + .expect("claims and runs"); + assert!(matches!( + run, + agent_worker::AgentRunOutcome::Finished { .. } + )); + assert!( + executor + .seen_env + .lock() + .expect("uncontended in this test") + .iter() + .any(|(k, v)| k == VAR && v == SENTINEL), + "the agent-exec run must actually receive the injected credential -- otherwise this \ + test would trivially pass by never exercising the seam at all" + ); + + let session = agent::show(&d.root, &d.id).expect("shows"); + assert_eq!(session.meta.status, Status::Done); + + // (a) the session ref: meta + plan + confirm + thread, all in the + // tip's own tree (this design's typed-entity trees are rewritten whole + // each commit, so the tip already carries the full thread history) -- + // plus every ancestor commit back to genesis, for a full sweep. + let session_ref = ents_model::namespace::agent_session_ref(&d.id).expect("valid"); + let session_tip = d + .root + .refs + .get(session_ref.as_ref()) + .expect("readable") + .expect("exists"); + assert_no_sentinel(&d.root.objects, session_tip, "the session ref"); + + // (b) the agent-exec effect's own result ref. + let exec_results_ref = + ents_model::namespace::result_ref(agent_worker::AGENT_EXEC_NAME, &short_oid(d.queued_tip)) + .expect("valid"); + let exec_results_tip = d + .root + .refs + .get(exec_results_ref.as_ref()) + .expect("readable") + .expect("exists"); + assert_no_sentinel( + &d.root.objects, + exec_results_tip, + "the agent-exec result ref", + ); + + // (c) the agent-plan effect's own result ref, from the drafting step. + let plan_results_ref = + ents_model::namespace::result_ref(plan_worker::AGENT_PLAN_NAME, &short_oid(d.planning_tip)) + .expect("valid"); + let plan_results_tip = d + .root + .refs + .get(plan_results_ref.as_ref()) + .expect("readable") + .expect("the drafting step recorded a result"); + assert_no_sentinel( + &d.root.objects, + plan_results_tip, + "the agent-plan result ref", + ); + + // (d) the result branch: the full tree contents the sandbox's own + // output was captured into. + let branch_name = session + .meta + .result_branch + .clone() + .expect("a result branch was recorded"); + let branch_ref: gix::refs::FullName = format!("refs/heads/{branch_name}") + .try_into() + .expect("valid"); + let branch_tip = d + .root + .refs + .get(branch_ref.as_ref()) + .expect("readable") + .expect("exists"); + assert_no_sentinel(&d.root.objects, branch_tip, "the result branch"); + + // (e) the on-disk scratch workdir remnants: `run_agent_exec` cleans its + // own workdir up after capturing the output tree, so nothing should be + // left under `scratch/<oid>` at all -- belt and suspenders alongside + // the git-object sweep above. + assert!( + !d.scratch.path().join(d.queued_tip.to_string()).exists(), + "the run's own scratch workdir must not survive the run" + ); +} + +/// Documents the honest boundary the guarantee above stops at: a command +/// that deliberately echoes its own environment into its own reported +/// output, or into a file inside the workdir it controls, gets that byte +/// range faithfully recorded onto the result branch and the session's own +/// transcript. This is not a bug the system fails to prevent -- the +/// transcript *is* the command's own stdout/stderr, and the result branch +/// *is* whatever the command left in its workdir; scrubbing either would +/// mean not recording what actually ran. The system's actual guarantee +/// (proven above) is narrower and does not cover this: it never injects the +/// credential into repository data itself, but it cannot stop an +/// adversarial command from exfiltrating its own environment through its +/// own reported output. +// @relation(roots.config-isolation, effect.deployment-property, scope=function, role=Verifies) +#[test] +fn a_malicious_command_can_still_exfiltrate_its_own_env() { + let d = draft_and_confirm(61); + + let executor = RecordingExecutor { + seen_env: Mutex::new(Vec::new()), + leak: true, + drafts: false, + }; + let run = agent_worker::run_agent_exec( + &d.root.refs, + &d.root.objects, + &d.root.events, + &executor, + d.scratch.path(), + &[], + "true", + d.queued_tip, + MemberId::new("worker"), + "sprite-1".to_owned(), + &d.worker_author, + &|payload| d.signer.sign(payload), + Mode::Advisory, + &d.credentials, + ) + .expect("claims and runs"); + assert!(matches!( + run, + agent_worker::AgentRunOutcome::Finished { .. } + )); + + let session = agent::show(&d.root, &d.id).expect("shows"); + assert!( + session.thread.iter().any(|blob| contains_sentinel(blob)), + "the leaking command's own echoed log line lands in the transcript verbatim -- the \ + transcript is the command's own reported output, which this system faithfully records" + ); + + let branch_name = session + .meta + .result_branch + .clone() + .expect("a result branch was recorded"); + let branch_ref: gix::refs::FullName = format!("refs/heads/{branch_name}") + .try_into() + .expect("valid"); + let branch_tip = d + .root + .refs + .get(branch_ref.as_ref()) + .expect("readable") + .expect("exists"); + let mut seen = HashSet::new(); + let mut blobs = Vec::new(); + reachable_text(&d.root.objects, branch_tip, &mut seen, &mut blobs); + assert!( + blobs.iter().any(|blob| contains_sentinel(blob)), + "the leaking command's own written file shows up on the result branch -- that file is \ + the command's own workdir output, which this system faithfully captures; it is not \ + something the worker-side machinery could have scrubbed without also discarding \ + genuine run output" + ); +}

effect, cli: per-member BYOK credentials and the redaction audit (phase 6)

Reviews

Start a review