fix(wave18): 8 HIGH audit findings closed + three-role pipeline actually built

47 crates, 801 tests green (up from 771 at v0.34.0). Wave 18 audit
found 8 HIGH findings across architect/critic/security/validator. All
closed. Three-role pipeline REBUILT after validator discovered Wave 16
commit was a half-commit (files claimed but never tracked).

## A. Three-role pipeline (REBUILD — was missing from v0.33.0 despite
      CHANGELOG claim)

Files validator flagged absent: _roles/auditor.toml + merger.toml,
4 _capabilities/{policy/git-ops-scope,output/verdict,output/merge-result,
verify/fork-audit}/text.md, kei-spawn/src/{pipeline,precedent}.rs,
pipeline_smoke.rs + pipeline_unit.rs tests. ALL NOW REAL (verified by
git log --all and `ls`).

- auditor role: claude-subagent-type=critic, handoff=[merger]
- merger role: git-ops scope, claude-subagent-type=infra-implementer,
  leaf (empty handoff)
- 5 capability text.md (+ capability.toml for each) defining contracts
- kei-spawn pipeline.rs (171 LOC): pipeline_from_role, derive_steps,
  emit_pipeline_json, scaffold_downstream_tasks
- kei-spawn precedent.rs (118 LOC): env-gated advisory shell-out
- --pipeline flag on spawn subcommand
- +11 tests (pipeline_smoke + pipeline_unit)

## B. kei-fork — 4 HIGH fixes (Critic F1+F7a, Security #3+#4)

- `git add -A` → explicit path list from ls-untracked + ls-modified,
  with exclusion filter for .DONE / .KEI_FORK_META.toml / _archive/ /
  _forks/. No more merge bleed. +1 regression test.
- create() rollback: on write_meta or ledger_fork failure, worktree
  + branch cleaned. +1 test via KEI_FORK_FORCE_LEDGER_FAIL=1.
- worktree_add arg injection: added `--` sentinel + is_safe_refname()
  validator (refuses dash-leading, NUL, ..). +3 tests.
- PATH hijack: KEI_FORK_GIT_BIN env override for all Command::new(git).
  +1 test.

## C. kei-spawn — 2 HIGH fixes (Security #1+#2)

- HTTP body unbounded DoS: MAX_BODY_BYTES=10MiB + content-length
  pre-check + streamed cap (io::Read::take) for chunked encoding.
  +2 feature-gated tests.
- PATH hijack: KEI_LEDGER_BIN env override already existed at
  ledger_sh.rs:15; documented precedence + added 4 regression tests
  locking the 3-tier lookup order.

## D. kei-ledger-sign — 1 HIGH fix (Security #2)

- save_keypair atomic POSIX open(2) O_CREAT|O_EXCL + mode 0o600 +
  rename(2) into place. No race window where key is world-readable.
  +2 tests.

## E. spawn_from_task rollback (Critic F7b)

- register_in_ledger helper: on ledger fork failure, rollback_task_dir
  before error propagation. +1 test spawn_rolls_back_task_dir_on_ledger_fail.

## Audit summary

- architect: GO conditional (taxonomy 19% — defer)
- critic: HIGH closed, MEDIUM debt logged
- security: 4 HIGH closed; MEDIUM (tar symlink, watcher symlink) tracked
- validator: CHANGELOG no longer lies — three-role pipeline is real
- patent-compliance: GO / LOW risk unchanged

All 8 HIGH blockers from Wave 18 consolidated audit → GREEN.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Parfii-bot 2026-04-23 20:54:59 +08:00
parent 9622d41bb6
commit 5f7a5b2639
32 changed files with 1843 additions and 94 deletions

View file

@ -0,0 +1,35 @@
[capability]
name = "output::merge-result"
category = "output"
version = "1.0"
description = "Require the merger agent's return to contain COMMIT_SHA and LEDGER_STATUS — the two facts the orchestrator needs to confirm the fork was merged and the ledger row closed."
rationale = "Merger outcome is binary: either the fork is on main with a visible commit sha + the ledger row is `done`, or something broke and the fork is still alive. Structured output lets the orchestrator decide next step (proceed / rescue / re-spawn) without re-running git log."
[restricts]
tool-patterns = []
tools-denied = []
[parameterized]
accepts = []
[text]
path = "text.md"
[verify]
rust-module = "verifies::output_merge_result"
run-mode = "worktree"
when = "on-return"
[taxonomy]
kingdom = "capability"
mechanism = "verify"
domain = "output"
layer = "agent-substrate"
stage = "runtime"
stability = "stable"
language = "rust"
[lineage]
parents = []
creator = "ag-orchestrator-human"
created = "2026-04-23"

View file

@ -0,0 +1,47 @@
## Merge result output
Your return report MUST contain the following fields, each on its
own line, with exact key names:
- `COMMIT_SHA:` — the SHA-1 of the new commit on `main` (40 hex
chars). If the merge produced multiple commits (e.g. squash vs
merge-commit), report the tip of `main` after your work.
- `LEDGER_STATUS:` — exactly one of `done`, `failed`, or
`still-running`. Reflects the ledger row for the fork you merged.
- `FORK_AGENT_ID:` — the agent-id of the writer whose fork you
merged (or attempted to merge).
- `MERGE_METHOD:` — exactly one of `merge-no-ff`, `squash`,
`rebase`, or `cherry-pick`. Whatever strategy you actually used.
Skeleton — success:
COMMIT_SHA: e8b37c92d4a1f0...
LEDGER_STATUS: done
FORK_AGENT_ID: ag-edit-local-20260423-142033
MERGE_METHOD: merge-no-ff
blockers: none
next: none
Skeleton — failure (fork diff did not apply):
COMMIT_SHA: <none>
LEDGER_STATUS: failed
FORK_AGENT_ID: ag-edit-local-20260423-142033
MERGE_METHOD: merge-no-ff
blockers:
- "3-way merge reported conflict in src/pipeline.rs line 42"
next: "Orchestrator re-spawns writer with conflict hint"
Rules:
- `COMMIT_SHA:` — 40 hex chars on success, literal string `<none>`
on failure. Do not paraphrase ("merged but no sha recorded" → FAIL).
- `LEDGER_STATUS:` — must match the actual ledger row. Cross-check
with `kei-ledger show <agent-id>` before emitting.
- Merger MUST NOT close the ledger row if the merge failed; the
`still-running` state is legitimate when the merge is deferred.
- If you had to rescue a half-merged state (`merge --abort` + retry),
document the rescue in `blockers:` with the original sha + rescue
sha, even on eventual success.

View file

@ -0,0 +1,35 @@
[capability]
name = "output::verdict"
category = "output"
version = "1.0"
description = "Require the agent's return to contain a structured PASS/FAIL/INCONCLUSIVE verdict with findings, suitable for automated pipeline routing."
rationale = "Auditor-style roles emit a categorical verdict the merger consumes. Free-text 'looks good' returns force the orchestrator to re-parse intent. A typed verdict makes the pipeline's next step (merge, defer, reject) mechanical."
[restricts]
tool-patterns = []
tools-denied = []
[parameterized]
accepts = []
[text]
path = "text.md"
[verify]
rust-module = "verifies::output_verdict"
run-mode = "worktree"
when = "on-return"
[taxonomy]
kingdom = "capability"
mechanism = "verify"
domain = "output"
layer = "agent-substrate"
stage = "runtime"
stability = "stable"
language = "rust"
[lineage]
parents = []
creator = "ag-orchestrator-human"
created = "2026-04-23"

View file

@ -0,0 +1,45 @@
## Verdict output format
Your return report MUST contain a single `verdict:` line, followed by
a `findings:` block. The verdict value MUST be exactly one of:
- `PASS` — every audited point passes. No blocking issues. Merger
may proceed to integrate the fork into main.
- `FAIL` — at least one audited point fails. Merger MUST NOT merge.
Each failure MUST have a remediation entry under `findings:`.
- `INCONCLUSIVE` — a required audit point could not be evaluated
(e.g. tests failed to run, diff unavailable). Merger MUST NOT
merge; orchestrator re-spawns the writer or the auditor.
Skeleton:
verdict: PASS
findings: none
body-sha: <sha256 of the fork diff, 64 hex chars>
audited-agent: <writer agent-id being reviewed>
verdict: FAIL
findings:
- point: 2
file: _primitives/_rust/kei-spawn/src/pipeline.rs
evidence: "No `cargo-test:` line in writer's return"
remediation: "Re-run `cargo test -p kei-spawn` and paste stdout"
- point: 5
file: _primitives/_rust/kei-spawn/src/pipeline.rs
evidence: "File is 243 LOC (limit 200)"
remediation: "Split pipeline.rs into pipeline.rs + pipeline_io.rs"
body-sha: <sha256>
audited-agent: <writer agent-id>
Rules:
- `verdict:` must be on its own line with no surrounding prose.
- `findings:` is a YAML-style block even for PASS (use `findings: none`).
- `body-sha:` is the SHA-256 of the concatenated fork diff as reported
by `kei-fork body-sha <agent-id>` (or equivalent).
- `audited-agent:` is the agent-id of the writer under review — not
your own id.
The merger role reads these four fields mechanically. Missing field
or malformed verdict value → merger refuses to proceed, orchestrator
re-spawns.

View file

@ -0,0 +1,35 @@
[capability]
name = "policy::git-ops-scope"
category = "policy"
version = "1.0"
description = "Allow ONLY the merger's required git and kei-fork / kei-ledger shell patterns. Every other shell command is denied."
rationale = "The merger role needs real git state-changing access (git merge, git push, git tag) that RULE 0.13 forbids from every other role. policy::git-ops-scope is the narrow exception: allow precisely the git/kei-fork/kei-ledger subcommands the merger needs, deny everything else. Less than unrestricted, more than read-only."
[restricts]
tool-patterns = []
tools-denied = []
[parameterized]
accepts = []
[text]
path = "text.md"
[gate]
rust-module = "gates::policy_git_ops_scope"
event = "PreToolUse:Bash"
severity = "enforce"
[taxonomy]
kingdom = "capability"
mechanism = "gate"
domain = "policy"
layer = "agent-substrate"
stage = "runtime"
stability = "stable"
language = "rust"
[lineage]
parents = []
creator = "ag-orchestrator-human"
created = "2026-04-23"

View file

@ -0,0 +1,39 @@
## Git-ops scope (merger-only)
You ARE permitted to invoke the following shell commands. Every other
command is denied by the `policy::git-ops-scope` gate:
- `git` — any subcommand (merge, fetch, push, tag, log, show, diff,
branch, reset, revert, rebase, cherry-pick). Used to integrate
the writer's fork into `main`.
- `kei-fork` — any subcommand (`collect`, `gc`, `rescue`, `list`,
`body-sha`). The managed-worktree primitive. Use `kei-fork collect`
as the preferred merge path; it enforces the safety envelope the
orchestrator expects.
- `kei-ledger` — any subcommand (`done`, `fail`, `list`, `show`).
Close the ledger row for the fork you merged. MUST be consistent
with actual commit state.
Explicitly denied (will be blocked by the gate):
- `rm`, `mv`, `cp` — no raw filesystem mutations.
- `curl`, `wget`, `nc` — no network fetches. If you need to push to
a remote, use `git push` (which is in scope).
- `cargo run`, `./script.sh`, `python` — no arbitrary program
execution. Use `git` / `kei-fork` / `kei-ledger` only.
- `sudo`, `ssh` — no privilege escalation, no remote hosts.
- `cat > file`, `echo > file`, redirection to files — the `Edit`
and `Write` tools are denied for this role by `scope::read-only`
semantics (see your role's `tools` allowlist).
The merger role deliberately does NOT include `Edit` or `Write` in
its tool allowlist. If a merge reveals a code fix is required, your
correct action is to set `LEDGER_STATUS: failed` with a blocker
entry and let the orchestrator re-spawn a writer. Merger repairs
code only via git operations (revert, cherry-pick, reset) — never
via source edits.
Gate severity: `enforce`. A denied command will error and you must
revise, not retry. Repeated attempts indicate the task is miscoped
and you should return `INCONCLUSIVE` with a blocker describing the
mismatch.

View file

@ -0,0 +1,35 @@
[capability]
name = "scope::read-only"
category = "scope"
version = "1.0"
description = "Forbid all mutating tools (Edit, Write, NotebookEdit). Agent can only read, grep, and — if allowed — run read-only shell commands."
rationale = "Audit-style roles need to inspect the writer's work without the possibility of re-editing it. Write capability to a writer's worktree would defeat the review purpose. See _roles/auditor.toml + RULE 0.13 (orchestrator-branch-first.md)."
[restricts]
tool-patterns = []
tools-denied = ["Edit", "Write", "NotebookEdit"]
[parameterized]
accepts = []
[text]
path = "text.md"
[gate]
rust-module = "gates::scope_read_only"
event = "PreToolUse:Edit|Write|NotebookEdit"
severity = "block"
[taxonomy]
kingdom = "capability"
mechanism = "gate"
domain = "scope"
layer = "agent-substrate"
stage = "runtime"
stability = "stable"
language = "rust"
[lineage]
parents = []
creator = "ag-orchestrator-human"
created = "2026-04-23"

View file

@ -0,0 +1,26 @@
## Read-only scope
You MUST NOT invoke any tool that mutates the filesystem. Specifically,
the following tools are denied for this role:
- `Edit` — no in-place edits
- `Write` — no new files, no file replacement
- `NotebookEdit` — no notebook cell mutation
You MAY use `Read`, `Glob`, `Grep`, and — where the role allows it —
`Bash` for read-only shell commands (`cargo check --dry-run` is fine,
`git diff` / `git log` / `git show` are fine, `cargo test` is fine
because it does not mutate source; destructive commands and any
shell redirection to files are blocked by other capabilities).
Your task is inspection, not repair. If you find a defect, describe
it precisely in your return report — include file path, line number,
evidence, severity. The orchestrator (or a follow-up writer agent)
will act on your findings. Do NOT attempt to apply the fix yourself
— that is out of scope for a read-only role and indicates you should
return an ESCALATE verdict instead of a direct action.
Rationale: audit-style roles (e.g. `auditor`) review a writer's work.
Granting the reviewer write access would blur responsibility and
defeat the review — the reviewer would re-become an author, bypassing
the sign-off ceremony the pipeline is designed to enforce.

View file

@ -0,0 +1,35 @@
[capability]
name = "verify::fork-audit"
category = "verify"
version = "1.0"
description = "Require the agent's return report to satisfy the 6-point fork-audit checklist (diff coverage, test evidence, scope adherence, capability enforcement, constructor-pattern LOC limits, blocker disclosure)."
rationale = "Codifies the minimum review a writer's fork must receive before merger proceeds. A drive-by 'looks good' return is insufficient — the 6 points are each independently falsifiable from the fork diff."
[restricts]
tool-patterns = []
tools-denied = []
[parameterized]
accepts = ["fork-audit-target"]
[text]
path = "text.md"
[verify]
rust-module = "verifies::verify_fork_audit"
run-mode = "worktree"
when = "on-return"
[taxonomy]
kingdom = "capability"
mechanism = "verify"
domain = "verify"
layer = "agent-substrate"
stage = "runtime"
stability = "stable"
language = "rust"
[lineage]
parents = []
creator = "ag-orchestrator-human"
created = "2026-04-23"

View file

@ -0,0 +1,44 @@
## Fork audit — 6-point checklist
When reviewing a writer's fork diff, your return MUST address each of
the six points below. Each point is independently falsifiable from
the diff — "looks fine" without point-by-point evidence is not a
valid audit.
1. **Diff coverage.** Every file in the diff must correspond to a
file declared in the writer's task whitelist. Orphan writes
(outside whitelist) → FAIL. Include the exact path of any orphan
in your verdict.
2. **Test evidence.** The writer's return MUST include a real
`cargo-test:` (or equivalent) output line with a visible pass
count. "Tested mentally" / "tests should pass" / any paraphrase
→ FAIL. Cross-check the test count matches new test files in
the diff.
3. **Scope adherence.** No edits outside the writer's declared
whitelist. Adjacent-file refactors, drive-by typo fixes, or
unasked re-formatting → FAIL (RULE: Surgical Changes).
4. **Capability enforcement.** If the writer's role required
capabilities (e.g. `output::report-format`), every required field
must be present and non-empty in the return. Missing field → FAIL.
5. **Constructor-pattern LOC limits.** Any new `.rs` file must be
≤200 LOC; any function ≤30 LOC. Larger files → FAIL unless the
writer has an explicit documented exception (file-level comment).
6. **Blocker disclosure.** The writer's return must contain a
`blockers:` field — either empty (list) or an enumerated list.
Silent dropping of known issues → FAIL. Silence = FAIL, not PASS.
For each of the six points, cite the exact path / line / excerpt
from the diff that establishes PASS or FAIL. The verdict is derived
from these six points:
- **PASS** — all 6 points evidence PASS.
- **FAIL** — any point evidence FAIL. Include remediation suggestion
per failed point (file, line, exact edit the writer should make).
- **INCONCLUSIVE** — point N cannot be evaluated from the available
diff (e.g. tests didn't run, CI output missing). State which point
and what would make it evaluable.

View file

@ -2845,6 +2845,7 @@ dependencies = [
"serde_json",
"sha2 0.10.9",
"tempfile",
"toml",
]
[[package]]

View file

@ -2,16 +2,18 @@
//!
//! Contract:
//! 1. `.DONE` must exist inside the worktree, else `Error::NotDone`
//! 2. `git add -A && git commit` inside the worktree
//! 2. Compute an EXPLICIT path list (untracked + modified), minus the
//! reserved exclusion set, then `git add <paths>` + `git commit`
//! 3. Capture commit SHA, then `git merge --no-ff fork/<id>` in kit_root
//! 4. Move worktree to `_archive/forks/YYYY-MM-DD/<id>/` (preserving
//! the agent's artefacts for post-hoc review / rescue)
//! 5. `git worktree prune && git branch -D fork/<id>` to clean up refs
//! 6. `kei-ledger done <id>` unless `KEI_FORK_SKIP_LEDGER=1`
//!
//! On SUCCESS: `_forks/<id>/` is gone, archive exists, merge
//! commit is on HEAD of kit_root. Return value carries the SHA and
//! count of files added by the agent.
//! HIGH #1 mitigation: the earlier `git add -A` was replaced by an
//! explicit path list. Reserved names (`.DONE`, `.KEI_FORK_META.toml`,
//! `_archive/**`, `_forks/**`) are stripped before staging so they
//! never land in the merge commit even if an agent wrote them.
use crate::error::Error;
use crate::git;
@ -28,15 +30,22 @@ pub struct CollectReport {
pub archive_path: PathBuf,
}
/// Paths that never belong in the merged history.
const EXCLUDED_NAMES: &[&str] = &[".DONE", ".KEI_FORK_META.toml"];
/// Path prefixes (relative to worktree root) that are kit-internal.
const EXCLUDED_PREFIXES: &[&str] = &["_archive/", "_forks/"];
pub fn collect(agent_id: &str, commit_msg: &str, kit_root: &Path) -> Result<CollectReport, Error> {
let worktree_abs = kit_root.join("_forks").join(agent_id);
if !worktree_abs.join(".DONE").exists() {
return Err(Error::NotDone(agent_id.to_string()));
}
let files_added = count_tracked_files(&worktree_abs);
let stage_list = compute_stage_list(&worktree_abs)?;
let files_added = stage_list.len();
let branch = format!("fork/{agent_id}");
git::add_all(&worktree_abs)?;
git::add_paths(&worktree_abs, &stage_list)?;
git::commit(&worktree_abs, commit_msg)?;
let commit_sha = git::rev_parse_head(&worktree_abs)?;
@ -59,28 +68,26 @@ pub fn collect(agent_id: &str, commit_msg: &str, kit_root: &Path) -> Result<Coll
})
}
fn count_tracked_files(worktree_abs: &Path) -> usize {
// Cheap approximation — walk the worktree, skip `.git*` and the
// KEI_FORK meta file. Used for the report only, not for decisions.
fn walk(dir: &Path) -> usize {
let mut n = 0;
let Ok(rd) = fs::read_dir(dir) else { return 0 };
for e in rd.flatten() {
let p = e.path();
let name = e.file_name();
let s = name.to_string_lossy();
if s.starts_with(".git") {
continue;
}
if p.is_dir() {
n += walk(&p);
} else {
n += 1;
}
}
n
/// Union of (untracked, exclude-standard) + (modified-tracked),
/// minus any path that matches the reserved exclusion set.
fn compute_stage_list(worktree_abs: &Path) -> Result<Vec<String>, Error> {
let untracked = git::ls_untracked(worktree_abs)?;
let modified = git::ls_modified(worktree_abs)?;
let mut combined: Vec<String> = untracked.into_iter().chain(modified).collect();
combined.sort();
combined.dedup();
combined.retain(|p| !is_excluded(p));
Ok(combined)
}
fn is_excluded(path: &str) -> bool {
if EXCLUDED_NAMES.contains(&path) {
return true;
}
walk(worktree_abs)
if EXCLUDED_PREFIXES.iter().any(|p| path.starts_with(*p)) {
return true;
}
false
}
fn archive_worktree(
@ -116,3 +123,27 @@ fn ledger_done(agent_id: &str) -> Result<(), Error> {
Err(e) => Err(Error::Ledger(format!("kei-ledger not runnable: {e}"))),
}
}
#[cfg(test)]
mod tests {
use super::is_excluded;
#[test]
fn excludes_reserved_names() {
assert!(is_excluded(".DONE"));
assert!(is_excluded(".KEI_FORK_META.toml"));
}
#[test]
fn excludes_kit_prefixes() {
assert!(is_excluded("_archive/forks/2026-04-23/x/y"));
assert!(is_excluded("_forks/other/file.txt"));
}
#[test]
fn admits_regular_files() {
assert!(!is_excluded("src/main.rs"));
assert!(!is_excluded("hello.txt"));
assert!(!is_excluded("sub/.DONE.txt"));
}
}

View file

@ -7,6 +7,15 @@
//! 4. Write `.KEI_FORK_META.toml` with agent_id + started_ts + base_branch + ledger_id
//! 5. `kei-ledger fork` unless env `KEI_FORK_SKIP_LEDGER=1`
//!
//! HIGH #2 mitigation: after the worktree exists, any failure in
//! steps 4 or 5 triggers a rollback — the worktree is force-removed
//! and the branch is deleted — so `create()` is either fully-committed
//! or leaves no trace. Callers can retry safely.
//!
//! Test hook: if env `KEI_FORK_FORCE_LEDGER_FAIL=1` is set, the ledger
//! call returns `Error::Ledger` unconditionally (regardless of
//! `KEI_FORK_SKIP_LEDGER`). Used by rollback regression tests.
//!
//! Worktree path is indexed by `agent_id`, not UUID, so `rescue()` /
//! `collect()` can be resolved from a human-readable CLI arg.
@ -32,10 +41,20 @@ pub fn create(agent_id: &str, base_branch: &str, kit_root: &Path) -> Result<Fork
fs::create_dir_all(parent)?;
}
git::worktree_add(kit_root, &worktree_rel, &branch, base_branch)?;
// From here on, worktree + branch exist on disk. If any step fails,
// we MUST roll them back so the caller sees a clean "no fork" state.
let started_ts = unix_now();
let meta = build_meta(agent_id, base_branch, started_ts);
write_meta(&worktree_abs, &meta)?;
ledger_fork(agent_id, &branch, base_branch)?;
if let Err(e) = write_meta(&worktree_abs, &meta) {
rollback(kit_root, &worktree_abs, &branch);
return Err(e);
}
if let Err(e) = ledger_fork(agent_id, &branch, base_branch) {
rollback(kit_root, &worktree_abs, &branch);
return Err(e);
}
Ok(ForkHandle {
agent_id: agent_id.to_string(),
worktree: worktree_abs,
@ -45,6 +64,20 @@ pub fn create(agent_id: &str, base_branch: &str, kit_root: &Path) -> Result<Fork
})
}
/// Best-effort cleanup after a partial failure. Errors from the
/// individual commands are intentionally swallowed — the outer error
/// is the real cause; a follow-up `gc` can clean any residue.
fn rollback(kit_root: &Path, worktree_abs: &Path, branch: &str) {
let _ = git::worktree_remove_force(kit_root, worktree_abs);
let _ = git::branch_delete(kit_root, branch);
// If `worktree remove` failed (e.g. git's ref db is out of sync),
// also clear the directory directly so the next `create` sees a
// clean slate.
if worktree_abs.exists() {
let _ = fs::remove_dir_all(worktree_abs);
}
}
fn build_meta(agent_id: &str, base_branch: &str, started_ts: i64) -> ForkMeta {
ForkMeta {
agent_id: agent_id.to_string(),
@ -65,7 +98,16 @@ fn ledger_skipped() -> bool {
std::env::var("KEI_FORK_SKIP_LEDGER").ok().as_deref() == Some("1")
}
fn ledger_force_fail() -> bool {
std::env::var("KEI_FORK_FORCE_LEDGER_FAIL").ok().as_deref() == Some("1")
}
fn ledger_fork(agent_id: &str, branch: &str, base: &str) -> Result<(), Error> {
if ledger_force_fail() {
return Err(Error::Ledger(
"forced failure via KEI_FORK_FORCE_LEDGER_FAIL (test hook)".to_string(),
));
}
if ledger_skipped() {
return Ok(());
}

View file

@ -5,6 +5,7 @@
//! - `Duplicate` — worktree/branch for this agent-id already exists
//! - `NotDone` — collect() called before the agent wrote `.DONE`
//! - `Gone` — rescue() could not find the worktree (live or archived)
//! - `InvalidRef` — branch / base-branch string rejected by refname guard
//! - `Io` / `Git` / `Ledger` / `Meta` — subsystem failures
use thiserror::Error;
@ -23,6 +24,9 @@ pub enum Error {
#[error("no live or archived worktree found for agent-id '{0}'")]
Gone(String),
#[error("invalid ref name '{0}' (arg-injection guard)")]
InvalidRef(String),
#[error("io error: {0}")]
Io(#[from] std::io::Error),

View file

@ -1,13 +1,26 @@
//! Thin `Command::new("git")` wrappers.
//! Thin `Command::new(git_bin())` wrappers.
//!
//! Every helper runs `git` in `kit_root` (or a specified worktree),
//! captures stdout/stderr, and returns `Error::Git` on non-zero exit.
//! No parsing beyond `trim()` on stdout — callers interpret the string.
//!
//! PATH hijack mitigation (HIGH #4): the git binary is resolved via
//! `git_bin()`, which honours `KEI_FORK_GIT_BIN` if set. Ops can pin to
//! an absolute path (e.g. `/usr/bin/git`) in trusted environments.
//!
//! Arg-injection mitigation (HIGH #3): `worktree_add` uses the `--`
//! sentinel before the base commit-ish and validates the refname shape.
use crate::error::Error;
use std::ffi::OsString;
use std::path::Path;
use std::process::{Command, Output};
/// Resolve the `git` binary. Honours `KEI_FORK_GIT_BIN` for hardening.
pub fn git_bin() -> OsString {
std::env::var_os("KEI_FORK_GIT_BIN").unwrap_or_else(|| OsString::from("git"))
}
fn run(cmd_desc: &str, c: &mut Command) -> Result<Output, Error> {
let out = c.output().map_err(Error::Io)?;
if !out.status.success() {
@ -19,68 +32,153 @@ fn run(cmd_desc: &str, c: &mut Command) -> Result<Output, Error> {
Ok(out)
}
/// Conservative git refname validator. Accepts the subset we emit and
/// the subset a caller may reasonably pass as `base`. Rejects leading
/// `-` (option injection), NUL, newline, and characters outside a
/// deliberately narrow allowlist.
pub fn is_safe_refname(s: &str) -> bool {
if s.is_empty() || s.len() > 255 {
return false;
}
let first = s.as_bytes()[0];
if first == b'-' || first == b'.' || first == b'/' {
return false;
}
for b in s.bytes() {
let ok = b.is_ascii_alphanumeric()
|| matches!(b, b'_' | b'-' | b'.' | b'/');
if !ok {
return false;
}
}
// No consecutive dots, no `..` traversal, no trailing `.lock`.
if s.contains("..") || s.ends_with(".lock") || s.ends_with('/') {
return false;
}
true
}
pub fn worktree_add(
kit_root: &Path,
worktree_rel: &Path,
new_branch: &str,
base: &str,
) -> Result<(), Error> {
let mut c = Command::new("git");
c.current_dir(kit_root)
.args(["worktree", "add", worktree_rel.to_str().unwrap_or("."), "-b", new_branch, base]);
if !is_safe_refname(new_branch) {
return Err(Error::InvalidRef(new_branch.to_string()));
}
if !is_safe_refname(base) {
return Err(Error::InvalidRef(base.to_string()));
}
let path_str = worktree_rel.to_str().ok_or_else(|| {
Error::Validate("worktree path is not valid UTF-8".to_string())
})?;
let mut c = Command::new(git_bin());
// `--` sentinel: everything after is positional. The branch name is
// pinned by `-b` (we already refname-validated it). The `base` is
// the commit-ish; placing it after `--` stops any accidental
// promotion to a flag if a future git version reorders parsing.
c.current_dir(kit_root).args([
"worktree",
"add",
"-b",
new_branch,
"--",
path_str,
base,
]);
run("git worktree add", &mut c)?;
Ok(())
}
pub fn add_all(cwd: &Path) -> Result<(), Error> {
let mut c = Command::new("git");
c.current_dir(cwd).args(["add", "-A"]);
run("git add -A", &mut c)?;
/// Stage an explicit list of paths. Replacement for `add -A` which
/// bled unwanted markers (`.DONE`, meta files) into commits.
pub fn add_paths(cwd: &Path, paths: &[String]) -> Result<(), Error> {
if paths.is_empty() {
return Ok(());
}
let mut c = Command::new(git_bin());
c.current_dir(cwd).arg("add").arg("--");
for p in paths {
c.arg(p);
}
run("git add", &mut c)?;
Ok(())
}
/// List untracked files (respects `.gitignore`) relative to `cwd`.
pub fn ls_untracked(cwd: &Path) -> Result<Vec<String>, Error> {
let mut c = Command::new(git_bin());
c.current_dir(cwd)
.args(["ls-files", "-o", "--exclude-standard"]);
let out = run("git ls-files -o", &mut c)?;
Ok(split_lines(&out.stdout))
}
/// List modified tracked files relative to `cwd`.
pub fn ls_modified(cwd: &Path) -> Result<Vec<String>, Error> {
let mut c = Command::new(git_bin());
c.current_dir(cwd).args(["diff", "--name-only"]);
let out = run("git diff --name-only", &mut c)?;
Ok(split_lines(&out.stdout))
}
fn split_lines(stdout: &[u8]) -> Vec<String> {
String::from_utf8_lossy(stdout)
.lines()
.filter(|l| !l.is_empty())
.map(|l| l.to_string())
.collect()
}
pub fn commit(cwd: &Path, msg: &str) -> Result<(), Error> {
let mut c = Command::new("git");
let mut c = Command::new(git_bin());
c.current_dir(cwd).args(["commit", "--allow-empty", "-m", msg]);
run("git commit", &mut c)?;
Ok(())
}
pub fn rev_parse_head(cwd: &Path) -> Result<String, Error> {
let mut c = Command::new("git");
let mut c = Command::new(git_bin());
c.current_dir(cwd).args(["rev-parse", "HEAD"]);
let out = run("git rev-parse HEAD", &mut c)?;
Ok(String::from_utf8_lossy(&out.stdout).trim().to_string())
}
pub fn merge_no_ff(kit_root: &Path, branch: &str, msg: &str) -> Result<(), Error> {
let mut c = Command::new("git");
c.current_dir(kit_root).args(["merge", "--no-ff", branch, "-m", msg]);
if !is_safe_refname(branch) {
return Err(Error::InvalidRef(branch.to_string()));
}
let mut c = Command::new(git_bin());
c.current_dir(kit_root)
.args(["merge", "--no-ff", branch, "-m", msg]);
run("git merge --no-ff", &mut c)?;
Ok(())
}
pub fn worktree_prune(kit_root: &Path) -> Result<(), Error> {
let mut c = Command::new("git");
let mut c = Command::new(git_bin());
c.current_dir(kit_root).args(["worktree", "prune"]);
run("git worktree prune", &mut c)?;
Ok(())
}
pub fn worktree_remove_force(kit_root: &Path, worktree_abs: &Path) -> Result<(), Error> {
let mut c = Command::new("git");
c.current_dir(kit_root).args([
"worktree",
"remove",
"--force",
worktree_abs.to_str().unwrap_or("."),
]);
let path_str = worktree_abs.to_str().ok_or_else(|| {
Error::Validate("worktree path is not valid UTF-8".to_string())
})?;
let mut c = Command::new(git_bin());
c.current_dir(kit_root)
.args(["worktree", "remove", "--force", "--", path_str]);
run("git worktree remove --force", &mut c)?;
Ok(())
}
pub fn branch_delete(kit_root: &Path, branch: &str) -> Result<(), Error> {
let mut c = Command::new("git");
if !is_safe_refname(branch) {
return Err(Error::InvalidRef(branch.to_string()));
}
let mut c = Command::new(git_bin());
c.current_dir(kit_root).args(["branch", "-D", branch]);
run("git branch -D", &mut c)?;
Ok(())
@ -89,8 +187,12 @@ pub fn branch_delete(kit_root: &Path, branch: &str) -> Result<(), Error> {
/// Check whether `branch` exists. `git show-ref` exits 0 if the ref is
/// present, non-zero otherwise — we treat both as valid data, no error.
pub fn branch_exists(kit_root: &Path, branch: &str) -> bool {
if !is_safe_refname(branch) {
return false;
}
let full = format!("refs/heads/{branch}");
let mut c = Command::new("git");
c.current_dir(kit_root).args(["show-ref", "--verify", "--quiet", &full]);
let mut c = Command::new(git_bin());
c.current_dir(kit_root)
.args(["show-ref", "--verify", "--quiet", &full]);
c.status().map(|s| s.success()).unwrap_or(false)
}

View file

@ -6,14 +6,34 @@
//!
//! NOTE: `KEI_FORK_SKIP_LEDGER` is process-wide. Tests set it once in
//! `setup_kit()` — do not unset mid-test.
//!
//! Tests that mutate *other* env vars (`KEI_FORK_FORCE_LEDGER_FAIL`,
//! `KEI_FORK_GIT_BIN`) serialize against all other tests via the
//! `ENV_LOCK` mutex below — cargo test runs in parallel by default and
//! leaking a binary override into a peer test would be catastrophic.
use kei_fork::{collect, create, gc, list, rescue, ForkStatus};
use std::fs;
use std::path::{Path, PathBuf};
use std::process::Command;
use std::sync::{Mutex, MutexGuard};
use tempfile::TempDir;
fn setup_kit() -> (TempDir, PathBuf) {
/// Serializes every test in this binary. Cargo runs tests in parallel
/// by default; two parallel tests with different `KEI_FORK_GIT_BIN` or
/// `KEI_FORK_FORCE_LEDGER_FAIL` settings would corrupt each other.
/// `setup_kit()` returns the guard — its lifetime is the test body.
static ENV_LOCK: Mutex<()> = Mutex::new(());
type KitGuard = MutexGuard<'static, ()>;
fn setup_kit() -> (TempDir, PathBuf, KitGuard) {
// `lock()` returns a poisoned guard if a previous test panicked —
// we still want to run, so recover the guard unconditionally.
let guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner());
// Defensive: clear any env the previous test may have left behind.
std::env::remove_var("KEI_FORK_FORCE_LEDGER_FAIL");
std::env::remove_var("KEI_FORK_GIT_BIN");
std::env::set_var("KEI_FORK_SKIP_LEDGER", "1");
let td = TempDir::new().expect("tempdir");
let root = td.path().to_path_buf();
@ -24,7 +44,7 @@ fn setup_kit() -> (TempDir, PathBuf) {
fs::write(root.join("README.md"), "hi").unwrap();
run_git(&root, &["add", "README.md"]);
run_git(&root, &["commit", "-q", "-m", "init"]);
(td, root)
(td, root, guard)
}
fn run_git(cwd: &Path, args: &[&str]) {
@ -48,7 +68,7 @@ fn mark_done(worktree: &Path) {
#[test]
fn create_produces_worktree_and_branch() {
let (_td, root) = setup_kit();
let (_td, root, _g) = setup_kit();
let h = create("ag-one", "main", &root).expect("create ok");
assert_eq!(h.agent_id, "ag-one");
assert_eq!(h.branch, "fork/ag-one");
@ -64,7 +84,7 @@ fn create_produces_worktree_and_branch() {
#[test]
fn create_rejects_invalid_agent_id() {
let (_td, root) = setup_kit();
let (_td, root, _g) = setup_kit();
let err = create("../evil", "main", &root).unwrap_err();
let msg = err.to_string();
assert!(msg.contains("invalid agent-id"), "got: {msg}");
@ -72,7 +92,7 @@ fn create_rejects_invalid_agent_id() {
#[test]
fn create_rejects_duplicate_agent_id() {
let (_td, root) = setup_kit();
let (_td, root, _g) = setup_kit();
create("ag-dup", "main", &root).expect("first create");
let err = create("ag-dup", "main", &root).unwrap_err();
assert!(err.to_string().contains("already exists"));
@ -80,7 +100,7 @@ fn create_rejects_duplicate_agent_id() {
#[test]
fn create_writes_meta_toml() {
let (_td, root) = setup_kit();
let (_td, root, _g) = setup_kit();
let h = create("ag-meta", "main", &root).expect("create ok");
let raw = fs::read_to_string(h.worktree.join(".KEI_FORK_META.toml")).unwrap();
let parsed: toml::Value = toml::from_str(&raw).unwrap();
@ -92,7 +112,7 @@ fn create_writes_meta_toml() {
#[test]
fn collect_without_done_fails() {
let (_td, root) = setup_kit();
let (_td, root, _g) = setup_kit();
create("ag-nodone", "main", &root).unwrap();
let err = collect("ag-nodone", "msg", &root).unwrap_err();
assert!(err.to_string().contains(".DONE"));
@ -100,7 +120,7 @@ fn collect_without_done_fails() {
#[test]
fn collect_with_done_produces_merge_commit() {
let (_td, root) = setup_kit();
let (_td, root, _g) = setup_kit();
let h = create("ag-merge", "main", &root).unwrap();
mark_done(&h.worktree);
let report = collect("ag-merge", "feat: agent work", &root).expect("collect ok");
@ -121,7 +141,7 @@ fn collect_with_done_produces_merge_commit() {
#[test]
fn collect_archives_worktree() {
let (_td, root) = setup_kit();
let (_td, root, _g) = setup_kit();
let h = create("ag-arch", "main", &root).unwrap();
mark_done(&h.worktree);
let report = collect("ag-arch", "msg", &root).expect("collect ok");
@ -132,7 +152,7 @@ fn collect_archives_worktree() {
#[test]
fn collect_removes_live_worktree() {
let (_td, root) = setup_kit();
let (_td, root, _g) = setup_kit();
let h = create("ag-gone", "main", &root).unwrap();
mark_done(&h.worktree);
collect("ag-gone", "msg", &root).expect("collect ok");
@ -141,7 +161,7 @@ fn collect_removes_live_worktree() {
#[test]
fn list_filters_by_status() {
let (_td, root) = setup_kit();
let (_td, root, _g) = setup_kit();
// Active
create("ag-active", "main", &root).unwrap();
// Done (mark .DONE but do not collect)
@ -170,7 +190,7 @@ fn list_filters_by_status() {
#[test]
fn gc_prunes_stale() {
let (_td, root) = setup_kit();
let (_td, root, _g) = setup_kit();
let h = create("ag-stale", "main", &root).unwrap();
// Backdate meta.started_ts by 48h — no .DONE → STALE under 24h threshold.
let raw = fs::read_to_string(h.worktree.join(".KEI_FORK_META.toml")).unwrap();
@ -193,7 +213,7 @@ fn gc_prunes_stale() {
#[test]
fn rescue_copies_live_files() {
let (_td, root) = setup_kit();
let (_td, root, _g) = setup_kit();
let h = create("ag-rescue-live", "main", &root).unwrap();
fs::write(h.worktree.join("note.txt"), "payload").unwrap();
fs::create_dir_all(h.worktree.join("sub")).unwrap();
@ -214,7 +234,7 @@ fn rescue_copies_live_files() {
#[test]
fn rescue_extracts_archived() {
let (_td, root) = setup_kit();
let (_td, root, _g) = setup_kit();
let h = create("ag-rescue-arch", "main", &root).unwrap();
mark_done(&h.worktree);
fs::write(h.worktree.join("artefact.md"), "# hi").unwrap();
@ -228,7 +248,159 @@ fn rescue_extracts_archived() {
#[test]
fn rescue_missing_agent_errors() {
let (_td, root) = setup_kit();
let (_td, root, _g) = setup_kit();
let err = rescue("ag-nope", &root, &root.join("x")).unwrap_err();
assert!(err.to_string().contains("no live or archived"));
}
// ---------------------------------------------------------------------
// Regression tests for HIGH findings (Critic F1 / F7a, Security #3 / #4).
// One test per finding + helpers shared via `setup_kit()` above.
// ---------------------------------------------------------------------
/// HIGH #1 (Critic F1): `git add -A` used to bleed `.DONE` and
/// `.KEI_FORK_META.toml` into every fork commit. The explicit-path
/// staging strategy must exclude them.
#[test]
fn collect_does_not_stage_done_or_meta() {
let (_td, root, _g) = setup_kit();
let h = create("ag-no-bleed", "main", &root).unwrap();
mark_done(&h.worktree);
let report = collect("ag-no-bleed", "feat: agent work", &root).expect("collect ok");
// Snapshot files introduced by the merge commit itself (second parent).
let out = Command::new("git")
.current_dir(&root)
.args(["show", "--name-only", "--pretty=", &report.commit_sha])
.output()
.unwrap();
let listing = String::from_utf8_lossy(&out.stdout).into_owned();
assert!(
listing.contains("hello.txt"),
"expected agent artefact in commit, got: {listing}"
);
assert!(
!listing.contains(".DONE"),
".DONE must NOT be staged, got: {listing}"
);
assert!(
!listing.contains(".KEI_FORK_META.toml"),
"meta must NOT be staged, got: {listing}"
);
}
/// HIGH #1 secondary: kit-internal prefixes (`_forks/`, `_archive/`)
/// must also be filtered out even if the agent writes into them.
#[test]
fn collect_does_not_stage_kit_internal_prefixes() {
let (_td, root, _g) = setup_kit();
let h = create("ag-no-kit", "main", &root).unwrap();
mark_done(&h.worktree);
// Agent accidentally writes into a kit-internal path.
fs::create_dir_all(h.worktree.join("_archive/forks/oops")).unwrap();
fs::write(h.worktree.join("_archive/forks/oops/evil.txt"), "nope").unwrap();
let report = collect("ag-no-kit", "msg", &root).expect("collect ok");
let out = Command::new("git")
.current_dir(&root)
.args(["show", "--name-only", "--pretty=", &report.commit_sha])
.output()
.unwrap();
let listing = String::from_utf8_lossy(&out.stdout).into_owned();
assert!(
!listing.contains("_archive/"),
"_archive/** must be excluded, got: {listing}"
);
}
/// HIGH #2 (Critic F7a): `create()` must roll back the worktree and
/// branch if a post-worktree-add step fails. We force a ledger error
/// via `KEI_FORK_FORCE_LEDGER_FAIL=1` and assert there is no residue.
#[test]
fn create_rolls_back_on_ledger_failure() {
let (_td, root, _g) = setup_kit();
std::env::set_var("KEI_FORK_FORCE_LEDGER_FAIL", "1");
let err = create("ag-rollback", "main", &root).unwrap_err();
std::env::remove_var("KEI_FORK_FORCE_LEDGER_FAIL");
assert!(
err.to_string().contains("ledger"),
"expected ledger error, got: {err}"
);
// Worktree dir is gone.
assert!(
!root.join("_forks/ag-rollback").exists(),
"worktree should be rolled back"
);
// Branch is gone.
let br = Command::new("git")
.current_dir(&root)
.args(["branch", "--list", "fork/ag-rollback"])
.output()
.unwrap();
assert!(
String::from_utf8_lossy(&br.stdout).trim().is_empty(),
"branch should be deleted, got: {}",
String::from_utf8_lossy(&br.stdout)
);
// And a retry MUST now succeed (no Duplicate error).
create("ag-rollback", "main", &root).expect("retry after rollback");
}
/// HIGH #3 (Security #3): refname validator must reject a
/// base-branch that starts with `-` (would be parsed as an option).
#[test]
fn create_rejects_base_branch_starting_with_dash() {
let (_td, root, _g) = setup_kit();
let err = create("ag-dash", "--evil-flag", &root).unwrap_err();
let msg = err.to_string();
assert!(
msg.contains("invalid ref name"),
"expected InvalidRef error, got: {msg}"
);
}
/// HIGH #3: NUL byte inside the refname must be rejected before git
/// sees it.
#[test]
fn create_rejects_base_branch_with_nul() {
let (_td, root, _g) = setup_kit();
let err = create("ag-nul", "main\0evil", &root).unwrap_err();
assert!(err.to_string().contains("invalid ref name"));
}
/// HIGH #3: dotty traversal also rejected.
#[test]
fn create_rejects_base_branch_with_dot_dot() {
let (_td, root, _g) = setup_kit();
let err = create("ag-dots", "foo..bar", &root).unwrap_err();
assert!(err.to_string().contains("invalid ref name"));
}
/// HIGH #4 (Security #4): the `git` binary MUST be resolvable via the
/// `KEI_FORK_GIT_BIN` env var. Point it at `false(1)` and confirm that
/// `create()` fails at the first git call (proving the env was
/// honoured). Any residue is cleaned up by `setup_kit()` of the next
/// test via the `ENV_LOCK` + defensive `remove_var`.
#[test]
fn custom_git_bin_env_respected() {
let (_td, root, _g) = setup_kit();
// Portable: macOS has /usr/bin/false, Linux usually has /bin/false.
let false_bin = ["/usr/bin/false", "/bin/false"]
.iter()
.find(|p| Path::new(p).exists())
.copied()
.expect("false(1) not found at either /bin/false or /usr/bin/false");
std::env::set_var("KEI_FORK_GIT_BIN", false_bin);
let err = create("ag-custombin", "main", &root).unwrap_err();
std::env::remove_var("KEI_FORK_GIT_BIN");
// false(1) exits 1 with no stdout — our wrapper surfaces it as
// Error::Git. That proves the override was invoked (otherwise the
// real `git` would have succeeded, and `create` would have
// returned Ok).
let msg = err.to_string();
assert!(
msg.contains("git command failed") || msg.contains("git"),
"expected git error, got: {msg}"
);
}

View file

@ -1,8 +1,9 @@
use crate::error::{Error, Result};
use crate::perms::chmod_600;
use crate::perms::open_600_write;
use ed25519_dalek::{SigningKey, VerifyingKey, SECRET_KEY_LENGTH};
use rand_core::{OsRng, RngCore};
use serde::{Deserialize, Serialize};
use std::io::Write;
use std::path::Path;
#[derive(Debug, Serialize, Deserialize)]
@ -36,11 +37,32 @@ pub fn save_keypair(kp: &KeyPair, path: &Path) -> Result<()> {
public_hex: hex::encode(kp.verifying().to_bytes()),
};
let text = serde_json::to_string_pretty(&file)?;
std::fs::write(path, text)?;
chmod_600(path)?;
// Atomic save: write to <path>.tmp with mode 0o600 from the first byte,
// then rename over the destination. rename(2) preserves the 0o600 mode
// and is atomic on POSIX, so no other process can ever observe the
// private key at mode 0o644.
let tmp = tmp_path(path);
let _ = std::fs::remove_file(&tmp);
let mut f = open_600_write(&tmp)?;
f.write_all(text.as_bytes())?;
f.sync_all()?;
drop(f);
std::fs::rename(&tmp, path)?;
Ok(())
}
fn tmp_path(path: &Path) -> std::path::PathBuf {
let mut name = path
.file_name()
.map(|s| s.to_os_string())
.unwrap_or_default();
name.push(".tmp");
match path.parent() {
Some(parent) if !parent.as_os_str().is_empty() => parent.join(name),
_ => std::path::PathBuf::from(name),
}
}
pub fn load_keypair(path: &Path) -> Result<KeyPair> {
let text = std::fs::read_to_string(path)?;
let file: KeyFile = serde_json::from_str(&text)?;

View file

@ -1,11 +1,46 @@
use std::fs;
use std::io;
use std::path::Path;
/// Create a new file for writing with mode 0o600 atomically.
///
/// Uses `O_CREAT | O_EXCL` (`create_new`) + `O_WRONLY` + mode 0o600 in a
/// single `open(2)` syscall on Unix. This closes the TOCTOU window where a
/// two-step `write` + `chmod` sequence leaves the file world-readable for
/// an instant.
///
/// Fails if `path` already exists (caller must remove first for overwrite
/// semantics — see `save_keypair` for the atomic rename-into-place pattern).
#[cfg(unix)]
pub fn open_600_write(path: &Path) -> io::Result<fs::File> {
use std::os::unix::fs::OpenOptionsExt;
fs::OpenOptions::new()
.write(true)
.create_new(true)
.mode(0o600)
.open(path)
}
#[cfg(not(unix))]
pub fn open_600_write(path: &Path) -> io::Result<fs::File> {
// Non-unix: mode bits are not applicable. Best effort — create_new
// still prevents opening a pre-existing attacker-placed file.
fs::OpenOptions::new()
.write(true)
.create_new(true)
.open(path)
}
/// Tighten permissions on an existing file to 0o600.
///
/// Retained for backward-compatibility and for callers that need to fix
/// permissions on a file they didn't create. New call sites should prefer
/// `open_600_write` which avoids the race entirely.
#[cfg(unix)]
pub fn chmod_600(path: &Path) -> io::Result<()> {
use std::os::unix::fs::PermissionsExt;
let perms = std::fs::Permissions::from_mode(0o600);
std::fs::set_permissions(path, perms)
let perms = fs::Permissions::from_mode(0o600);
fs::set_permissions(path, perms)
}
#[cfg(not(unix))]

View file

@ -74,6 +74,42 @@ fn save_keypair_sets_600_perms() {
assert_eq!(mode, 0o600, "expected 0o600, got {:o}", mode);
}
#[cfg(unix)]
#[test]
fn save_keypair_atomic_no_race_window() {
// Regression: save_keypair MUST NOT leave an intermediate world-readable
// file between write and chmod. With the rename-into-place fix, the
// final file is mode 0o600 from the first byte and the <path>.tmp
// sidecar is cleaned up by rename(2).
use std::os::unix::fs::PermissionsExt;
let dir = tempdir().unwrap();
let path = dir.path().join("keys.json");
let kp = generate_keypair();
save_keypair(&kp, &path).unwrap();
let meta = std::fs::metadata(&path).unwrap();
assert_eq!(meta.permissions().mode() & 0o777, 0o600);
let tmp = dir.path().join("keys.json.tmp");
assert!(!tmp.exists(), "tmp sidecar must be renamed away, found {:?}", tmp);
}
#[test]
fn save_keypair_overwrites_existing_file() {
// Overwrite semantics must survive the atomic-rename refactor:
// a second save_keypair on the same path replaces the prior content.
let dir = tempdir().unwrap();
let path = dir.path().join("keys.json");
let kp1 = generate_keypair();
save_keypair(&kp1, &path).unwrap();
let kp2 = generate_keypair();
save_keypair(&kp2, &path).unwrap();
let loaded = load_keypair(&path).unwrap();
assert_eq!(
loaded.verifying().to_bytes(),
kp2.verifying().to_bytes(),
"second save must replace first"
);
}
#[test]
fn canonical_message_rejects_pipe_in_fields() {
let err = canonical_message("dna|bad", "sha", "creator").expect_err("pipe in dna must fail");

View file

@ -26,6 +26,7 @@ serde = { version = "1", features = ["derive"] }
serde_json = "1"
anyhow = "1"
sha2 = { workspace = true }
toml = { workspace = true }
reqwest = { version = "0.12", default-features = false, features = ["json", "blocking", "rustls-tls"], optional = true }
[dev-dependencies]

View file

@ -11,6 +11,7 @@
#![cfg(feature = "http-driver")]
use std::io::Read as _;
use std::time::Duration;
use crate::drive::{AgentResult, AnthropicDriver, DriveError};
@ -27,6 +28,10 @@ const TIMEOUT_TOTAL: Duration = Duration::from_secs(300);
// "60s read" intent — request-body read is bounded by the 300s total).
const TIMEOUT_CONNECT: Duration = Duration::from_secs(60);
const ERR_BODY_EXCERPT: usize = 512;
/// Cap response body at 10 MiB to mitigate memory-DoS from an oversized
/// or malicious upstream (CWE-400). Anthropic `messages` responses are
/// well under 1 MiB in practice; 10 MiB leaves ample headroom.
const MAX_BODY_BYTES: usize = 10 * 1024 * 1024;
/// Real Anthropic-backed driver. Zero-state: key + endpoint read per call.
pub struct HttpDriver;
@ -95,9 +100,7 @@ fn send_and_parse(
.send()
.map_err(map_network_error)?;
let status = resp.status();
let text = resp.text().map_err(|e| DriveError::Transport {
message: format!("read response body: {e}"),
})?;
let text = read_body_bounded(resp)?;
if status.is_success() {
parse_response(&text)
} else {
@ -105,6 +108,41 @@ fn send_and_parse(
}
}
/// Read response body with a hard cap of `MAX_BODY_BYTES`.
///
/// Two defences layered:
/// 1. `content-length` pre-check rejects declared-huge bodies without
/// allocating (saves memory on honest servers advertising the size).
/// 2. `io::Read::take(MAX + 1)` caps the actual bytes consumed from the
/// wire — covers chunked-encoding where `content_length()` is `None`.
/// If the reader yields MAX+1 bytes we reject as oversize.
fn read_body_bounded(resp: reqwest::blocking::Response) -> Result<String, DriveError> {
if let Some(len) = resp.content_length() {
if len > MAX_BODY_BYTES as u64 {
return Err(DriveError::Transport {
message: format!(
"response body {len} bytes exceeds {MAX_BODY_BYTES}-byte limit (content-length)"
),
});
}
}
let mut buf = Vec::with_capacity(8192);
let mut limited = resp.take(MAX_BODY_BYTES as u64 + 1);
limited
.read_to_end(&mut buf)
.map_err(|e| DriveError::Transport {
message: format!("read response body: {e}"),
})?;
if buf.len() > MAX_BODY_BYTES {
return Err(DriveError::Transport {
message: format!("response body exceeds {MAX_BODY_BYTES}-byte limit (streamed)"),
});
}
String::from_utf8(buf).map_err(|e| DriveError::Transport {
message: format!("response body not utf-8: {e}"),
})
}
fn map_network_error(e: reqwest::Error) -> DriveError {
DriveError::Transport {
message: format!("network error: {e}"),

View file

@ -6,11 +6,36 @@
//!
//! Every call surfaces stderr on failure so orchestrator sees the real
//! ledger error (branch too long, duplicate id, etc.), not a wrapped one.
//!
//! # Security — `$PATH` hijack (CWE-427)
//!
//! The final fallback in [`ledger_bin`] is the bare name `"kei-ledger"`,
//! which `std::process::Command` resolves by walking `$PATH`. On a shared
//! or compromised machine an attacker-controlled directory earlier on
//! `$PATH` can provide a rogue `kei-ledger` that silently captures ledger
//! writes. To mitigate:
//!
//! * Set `KEI_LEDGER_BIN` to an **absolute path** in production / CI
//! (e.g. `/usr/local/bin/kei-ledger` or the cargo-install path), or
//! * Run integration tests via `cargo test` which populates the
//! `CARGO_BIN_EXE_kei-ledger` env var with the workspace-built binary.
//!
//! The env-override path is checked first in [`ledger_bin`] precisely so
//! a trusted operator can pin the binary and sidestep `$PATH` resolution.
use anyhow::{anyhow, Result};
use std::process::Command;
/// Resolve `kei-ledger` executable. Env override → CARGO env (tests) → PATH.
///
/// Lookup order:
/// 1. `KEI_LEDGER_BIN` — operator-pinned absolute path (recommended for
/// production; mitigates `$PATH` hijack per CWE-427, see module docs).
/// 2. `CARGO_BIN_EXE_kei-ledger` — set by `cargo test` for the workspace
/// binary under integration testing.
/// 3. `"kei-ledger"` — last-resort bare name; resolved via `$PATH` by
/// `std::process::Command`. Acceptable on single-user dev machines;
/// pin via `KEI_LEDGER_BIN` in any multi-user or CI context.
pub fn ledger_bin() -> String {
if let Ok(b) = std::env::var("KEI_LEDGER_BIN") {
return b;
@ -105,3 +130,70 @@ fn run(cmd: &mut Command, stage: &str) -> Result<()> {
let stderr = String::from_utf8_lossy(&out.stderr).to_string();
Err(anyhow!("kei-ledger {stage} failed: {stderr}"))
}
#[cfg(test)]
mod tests {
//! Regression coverage for [`ledger_bin`] lookup precedence.
//!
//! Env vars are process-global; serialize with a local mutex so
//! parallel cargo-test workers don't trample each other.
use super::*;
use std::sync::Mutex;
static ENV_LOCK: Mutex<()> = Mutex::new(());
fn with_env<F: FnOnce()>(kei_bin: Option<&str>, cargo_bin: Option<&str>, f: F) {
let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner());
let prev_kei = std::env::var("KEI_LEDGER_BIN").ok();
let prev_cargo = std::env::var("CARGO_BIN_EXE_kei-ledger").ok();
match kei_bin {
Some(v) => std::env::set_var("KEI_LEDGER_BIN", v),
None => std::env::remove_var("KEI_LEDGER_BIN"),
}
match cargo_bin {
Some(v) => std::env::set_var("CARGO_BIN_EXE_kei-ledger", v),
None => std::env::remove_var("CARGO_BIN_EXE_kei-ledger"),
}
f();
match prev_kei {
Some(v) => std::env::set_var("KEI_LEDGER_BIN", v),
None => std::env::remove_var("KEI_LEDGER_BIN"),
}
match prev_cargo {
Some(v) => std::env::set_var("CARGO_BIN_EXE_kei-ledger", v),
None => std::env::remove_var("CARGO_BIN_EXE_kei-ledger"),
}
}
#[test]
fn ledger_bin_env_overrides_default() {
with_env(Some("/opt/pinned/kei-ledger"), None, || {
assert_eq!(ledger_bin(), "/opt/pinned/kei-ledger");
});
}
#[test]
fn ledger_bin_cargo_env_used_when_kei_unset() {
with_env(None, Some("/tmp/cargo-built/kei-ledger"), || {
assert_eq!(ledger_bin(), "/tmp/cargo-built/kei-ledger");
});
}
#[test]
fn ledger_bin_falls_back_to_bare_name() {
with_env(None, None, || {
assert_eq!(ledger_bin(), "kei-ledger");
});
}
#[test]
fn ledger_bin_env_wins_over_cargo_env() {
with_env(
Some("/opt/pinned/kei-ledger"),
Some("/tmp/cargo-built/kei-ledger"),
|| {
assert_eq!(ledger_bin(), "/opt/pinned/kei-ledger");
},
);
}
}

View file

@ -32,6 +32,8 @@ pub mod drive_http;
#[cfg(feature = "http-driver")]
pub mod drive_http_parse;
pub mod ledger_sh;
pub mod pipeline;
pub mod precedent;
pub mod spawn;
pub mod verify;
@ -39,5 +41,9 @@ pub use drive::{
drive_with, not_implemented_message, AgentResult, AnthropicDriver, DriveError, HttpDriver,
ManualDriver,
};
pub use spawn::{spawn_from_task, SpawnOutput};
pub use pipeline::{
derive_chain_from_role, derive_steps, emit_pipeline_json, pipeline_from_role,
pipeline_json_path, scaffold_downstream_tasks, PipelineChain, PipelineStep,
};
pub use spawn::{spawn_from_task, spawn_with_pipeline, SpawnOutput};
pub use verify::{verify_agent, VerifyOutput};

View file

@ -18,9 +18,10 @@ use std::path::PathBuf;
use std::process::ExitCode;
use kei_spawn::{
drive_with, ledger_sh, not_implemented_message, spawn_from_task, verify_agent, DriveError,
ManualDriver, SpawnOutput,
drive_with, ledger_sh, not_implemented_message, spawn_from_task, spawn_with_pipeline,
verify_agent, DriveError, ManualDriver, PipelineChain, SpawnOutput,
};
use serde::Serialize;
#[derive(Parser)]
#[command(
@ -42,6 +43,10 @@ enum Cmd {
/// kit root (default: cwd).
#[arg(long)]
kit_root: Option<PathBuf>,
/// Also derive downstream handoff chain from role's `[pipeline]`
/// section + scaffold stub task files for each step.
#[arg(long)]
pipeline: bool,
},
/// Spawn + invoke driver (v0.1: stub — emits SpawnOutput then exit 64).
Drive {
@ -67,7 +72,7 @@ enum Cmd {
fn main() -> ExitCode {
let cli = Cli::parse();
match cli.cmd {
Cmd::Spawn { task, kit_root } => run_spawn(task, kit_root),
Cmd::Spawn { task, kit_root, pipeline } => run_spawn(task, kit_root, pipeline),
Cmd::Drive { task, kit_root } => run_drive(task, kit_root),
Cmd::Verify { agent_id, worktree, kit_root } => {
run_verify(agent_id, worktree, kit_root)
@ -76,11 +81,28 @@ fn main() -> ExitCode {
}
}
fn run_spawn(task: PathBuf, kit_root: Option<PathBuf>) -> ExitCode {
#[derive(Serialize)]
struct SpawnWithPipelineJson<'a> {
#[serde(flatten)]
spawn: &'a SpawnOutput,
pipeline: &'a PipelineChain,
}
fn run_spawn(task: PathBuf, kit_root: Option<PathBuf>, pipeline: bool) -> ExitCode {
let kit = kit_root_or_cwd(kit_root);
match spawn_from_task(&task, &kit) {
Ok(out) => emit_json(&out),
Err(e) => err("spawn", e),
if pipeline {
match spawn_with_pipeline(&task, &kit) {
Ok((out, chain)) => emit_json(&SpawnWithPipelineJson {
spawn: &out,
pipeline: &chain,
}),
Err(e) => err("spawn --pipeline", e),
}
} else {
match spawn_from_task(&task, &kit) {
Ok(out) => emit_json(&out),
Err(e) => err("spawn", e),
}
}
}

View file

@ -0,0 +1,171 @@
//! pipeline — derive downstream handoff chain from a writer's role.
//!
//! When a spawn is invoked with `--pipeline`, the orchestrator wants to know:
//! 1. Which downstream roles does the writer's role declare in
//! `[pipeline].handoff`? (e.g. `edit-local` → `["auditor"]`)
//! 2. What agent-ids should those downstream steps use?
//! 3. Where should the pipeline.json chain artefact be written?
//! 4. What skeleton task.toml should be scaffolded for each step?
//!
//! This module answers all four. It reads the writer's role file, parses
//! the optional `[pipeline]` section, and emits a `PipelineChain` the
//! caller can serialise + use to pre-create per-step task directories.
//!
//! Constructor Pattern: one module = one responsibility (pipeline derivation
//! only). No git, no shell, no ledger. Pure filesystem + TOML parsing.
//! No I/O beyond `std::fs::read_to_string` for role lookup and
//! `std::fs::write` / `create_dir_all` for scaffolding.
use anyhow::{anyhow, Context, Result};
use serde::{Deserialize, Serialize};
use std::path::{Path, PathBuf};
/// One step in a downstream handoff chain.
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct PipelineStep {
pub role: String,
pub agent_id: String,
}
/// Ordered chain of handoff steps derived from a writer's role.
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct PipelineChain {
pub steps: Vec<PipelineStep>,
}
/// Raw on-disk shape of `_roles/<name>.toml`'s `[pipeline]` section.
/// Tolerates absence — default = empty handoff = no downstream steps.
#[derive(Debug, Default, Deserialize)]
struct RolePipelineRaw {
#[serde(default)]
pipeline: PipelineSectionRaw,
}
#[derive(Debug, Default, Deserialize)]
struct PipelineSectionRaw {
#[serde(default)]
handoff: Vec<String>,
}
/// Read `_roles/<role>.toml` and return its `[pipeline].handoff` list.
/// Missing file → error. Missing `[pipeline]` section → empty Vec (OK).
pub fn pipeline_from_role(kit_root: &Path, role: &str) -> Result<Vec<String>> {
let path = kit_root.join("_roles").join(format!("{role}.toml"));
let text = std::fs::read_to_string(&path)
.with_context(|| format!("read role file {}", path.display()))?;
let raw: RolePipelineRaw = toml::from_str(&text)
.with_context(|| format!("parse role TOML {}", path.display()))?;
Ok(raw.pipeline.handoff)
}
/// Derive concrete `PipelineStep`s from a writer agent-id + handoff role
/// list. Each downstream step gets a distinct agent-id of the form
/// `<writer_id>-<role>` so ledger rows remain unique + parent-linked.
pub fn derive_steps(writer_id: &str, handoff_roles: &[String]) -> Vec<PipelineStep> {
let mut steps = Vec::with_capacity(handoff_roles.len());
for role in handoff_roles {
let role_trimmed = role.trim();
if role_trimmed.is_empty() {
continue;
}
steps.push(PipelineStep {
role: role_trimmed.to_string(),
agent_id: format!("{writer_id}-{role_trimmed}"),
});
}
steps
}
/// Serialise `chain` as pretty JSON into `out_path`. Creates parent dirs
/// if missing so callers can point at `tasks/<writer>/pipeline.json`
/// before the parent dir exists (unlikely in practice, but cheap).
pub fn emit_pipeline_json(out_path: &Path, chain: &PipelineChain) -> Result<()> {
if let Some(parent) = out_path.parent() {
if !parent.as_os_str().is_empty() {
std::fs::create_dir_all(parent)
.with_context(|| format!("create parent dir {}", parent.display()))?;
}
}
let json = serde_json::to_string_pretty(chain).context("serialise pipeline chain")?;
std::fs::write(out_path, json)
.with_context(|| format!("write pipeline json {}", out_path.display()))?;
Ok(())
}
/// For each step in `chain`, scaffold a stub `tasks/<step.agent_id>/task.toml`
/// that the orchestrator can later enrich + hand to `kei-spawn spawn`.
///
/// We deliberately stop short of calling `prepare_agent` / `kei-ledger fork`
/// for downstream steps — those are the orchestrator's responsibility to
/// invoke in order (writer succeeds → auditor spawns → auditor PASS →
/// merger spawns). Scaffolding a stub makes that sequence mechanical.
///
/// Also emits `tasks/<writer_id>/pipeline.json` so the orchestrator can
/// inspect the full chain in one read.
pub fn scaffold_downstream_tasks(
kit_root: &Path,
writer_id: &str,
chain: &PipelineChain,
) -> Result<()> {
let writer_dir = kit_root.join("tasks").join(writer_id);
std::fs::create_dir_all(&writer_dir)
.with_context(|| format!("create writer tasks dir {}", writer_dir.display()))?;
emit_pipeline_json(&writer_dir.join("pipeline.json"), chain)?;
for step in &chain.steps {
scaffold_one_step(kit_root, writer_id, step)?;
}
Ok(())
}
fn scaffold_one_step(kit_root: &Path, writer_id: &str, step: &PipelineStep) -> Result<()> {
let step_dir = kit_root.join("tasks").join(&step.agent_id);
std::fs::create_dir_all(&step_dir)
.with_context(|| format!("create step dir {}", step_dir.display()))?;
let stub_path = step_dir.join("task.stub.toml");
let stub = build_task_stub(writer_id, step);
std::fs::write(&stub_path, stub)
.with_context(|| format!("write task stub {}", stub_path.display()))?;
Ok(())
}
fn build_task_stub(writer_id: &str, step: &PipelineStep) -> String {
format!(
concat!(
"# Auto-scaffolded pipeline stub. Enrich `[body].text` and run\n",
"# `kei-spawn spawn <this file>` once the upstream agent returns.\n",
"\n",
"[task]\n",
"role = \"{role}\"\n",
"agent-id = \"{agent_id}\"\n",
"parent-agent = \"{parent}\"\n",
"\n",
"[body]\n",
"text = \"TODO: fill handoff body for {role} step\"\n",
),
role = step.role,
agent_id = step.agent_id,
parent = writer_id,
)
}
/// Convenience wrapper: read role, derive steps, return chain. Used by
/// spawn.rs when `--pipeline` is set at the CLI layer.
pub fn derive_chain_from_role(
kit_root: &Path,
writer_role: &str,
writer_id: &str,
) -> Result<PipelineChain> {
if writer_role.is_empty() {
return Err(anyhow!("writer_role is empty"));
}
let handoff = pipeline_from_role(kit_root, writer_role)?;
let steps = derive_steps(writer_id, &handoff);
Ok(PipelineChain { steps })
}
/// Public helper to compute the path where pipeline.json will be written.
/// Exposed so tests + orchestrator can compare without duplicating the
/// layout convention.
pub fn pipeline_json_path(kit_root: &Path, writer_id: &str) -> PathBuf {
kit_root.join("tasks").join(writer_id).join("pipeline.json")
}

View file

@ -0,0 +1,120 @@
//! precedent — env-gated advisory check against `kei-dna-index`.
//!
//! When `KEI_SPAWN_PRECEDENT_CHECK=1`, spawn.rs calls `run_advisory` after
//! it has composed the body but before `kei-ledger fork`. We shell out
//! to `kei-dna-index precedent --body <sha>`; if the primitive returns
//! a non-empty JSON array of prior-agent matches, we eprintln a WARN
//! and return the hit count. We never block the spawn — this is a
//! human-facing signal, not a gate.
//!
//! Constructor Pattern: one module = one responsibility (precedent
//! advisory only). No git, no ledger write, no filesystem mutation.
use anyhow::{anyhow, Result};
use std::process::Command;
/// Env flag that enables this advisory. Absent → `run_advisory` is a
/// silent no-op returning `Ok(0)`.
pub const ENABLE_ENV: &str = "KEI_SPAWN_PRECEDENT_CHECK";
/// Env override for the `kei-dna-index` binary path. Default = PATH lookup.
pub const BIN_ENV: &str = "KEI_DNA_INDEX_BIN";
/// Run the advisory. Returns the number of prior-agent hits reported
/// by `kei-dna-index precedent`. When the env flag is absent, returns
/// `Ok(0)` without invoking anything — keep fast path fast.
pub fn run_advisory(body_sha: &str) -> Result<usize> {
if std::env::var(ENABLE_ENV).is_err() {
return Ok(0);
}
if body_sha.is_empty() {
return Err(anyhow!("precedent advisory called with empty body_sha"));
}
let bin = dna_index_bin();
let stdout = match capture_stdout(&bin, body_sha) {
Some(s) => s,
None => return Ok(0),
};
let hits = parse_hits(&stdout);
warn_if_hits(hits, body_sha);
Ok(hits)
}
/// Shell to kei-dna-index and capture stdout on success. Any failure
/// (binary missing, non-zero exit) is logged + returns `None` so the
/// caller short-circuits to `Ok(0)` (best-effort advisory).
fn capture_stdout(bin: &str, body_sha: &str) -> Option<String> {
let mut cmd = Command::new(bin);
cmd.args(["precedent", "--body", body_sha]);
let output = match cmd.output() {
Ok(o) => o,
Err(e) => {
eprintln!("kei-spawn precedent: skipped ({bin} not runnable: {e})");
return None;
}
};
if !output.status.success() {
eprintln!(
"kei-spawn precedent: skipped (exit {}: {})",
output.status,
String::from_utf8_lossy(&output.stderr).trim()
);
return None;
}
Some(String::from_utf8_lossy(&output.stdout).to_string())
}
fn warn_if_hits(hits: usize, body_sha: &str) {
if hits > 0 {
eprintln!(
"kei-spawn precedent WARN: {hits} prior agent(s) share body_sha={body_sha} — review before spawn",
);
}
}
/// Resolve the kei-dna-index binary, env override first.
fn dna_index_bin() -> String {
if let Ok(b) = std::env::var(BIN_ENV) {
return b;
}
"kei-dna-index".into()
}
/// Minimal JSON-array-length parser. kei-dna-index `precedent` emits a
/// JSON array. We only need the element count — avoid adding serde_json
/// coupling to this single call site by counting top-level `{`. On
/// malformed output, return 0 (advisory is best-effort).
fn parse_hits(stdout: &str) -> usize {
let trimmed = stdout.trim();
if trimmed.is_empty() || trimmed == "[]" {
return 0;
}
serde_json::from_str::<serde_json::Value>(trimmed)
.ok()
.and_then(|v| v.as_array().map(|a| a.len()))
.unwrap_or(0)
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn parse_empty_array() {
assert_eq!(parse_hits("[]"), 0);
assert_eq!(parse_hits(""), 0);
assert_eq!(parse_hits(" "), 0);
}
#[test]
fn parse_two_element_array() {
let n = parse_hits(r#"[{"id":"a"},{"id":"b"}]"#);
assert_eq!(n, 2);
}
#[test]
fn parse_malformed_returns_zero() {
assert_eq!(parse_hits("not json"), 0);
assert_eq!(parse_hits("{}"), 0);
}
}

View file

@ -22,6 +22,8 @@ use sha2::{Digest, Sha256};
use std::path::{Path, PathBuf};
use crate::ledger_sh;
use crate::pipeline::{self, PipelineChain};
use crate::precedent;
/// The bundle orchestrator hands to Claude Code's Agent tool.
#[derive(Debug, Clone, Serialize)]
@ -43,6 +45,11 @@ pub struct SpawnOutput {
}
/// Main spawn entry. See module doc for the 6-step pipeline.
///
/// On `kei-ledger fork` failure, the task directory created at step 3 is
/// removed atomically so callers can retry without a half-created leftover
/// (HIGH fix #4). Ledger-fork failure is the first observable checkpoint —
/// earlier steps are pure composition and do not touch shared state.
pub fn spawn_from_task(task_path: &Path, kit_root: &Path) -> Result<SpawnOutput> {
let mut task = runtime_spawn::load_task(task_path)
.with_context(|| format!("load task {}", task_path.display()))?;
@ -55,21 +62,55 @@ pub fn spawn_from_task(task_path: &Path, kit_root: &Path) -> Result<SpawnOutput>
.with_context(|| format!("read written task {}", prepared.task_path.display()))?;
let spec_sha = sha256_hex(&task_bytes);
let branch = format!("agent/{}", inv.agent_id);
let parent = task
.task
.parent_agent
.as_deref()
.filter(|s| !s.is_empty());
ledger_sh::fork(
let parent = task.task.parent_agent.as_deref().filter(|s| !s.is_empty());
// Advisory precedent check — env-gated, never blocks.
let _ = precedent::run_advisory(&spec_sha);
register_in_ledger(&inv, &branch, parent, &spec_sha, &prepared)?;
Ok(build_output(inv, prepared, spec_sha, branch))
}
/// Call `kei-ledger fork`; on failure, remove the prepared task dir so
/// the spawn attempt leaves no half-created state for retry.
fn register_in_ledger(
inv: &prepare::AgentInvocation,
branch: &str,
parent: Option<&str>,
spec_sha: &str,
prepared: &runtime_spawn::PreparedAgent,
) -> Result<()> {
if let Err(e) = ledger_sh::fork(
&inv.agent_id,
&branch,
branch,
parent,
&spec_sha,
spec_sha,
prepared.dir.to_str(),
Some(&inv.dna),
)
.context("kei-ledger fork")?;
Ok(build_output(inv, prepared, spec_sha, branch))
) {
rollback_task_dir(&prepared.dir);
return Err(e.context("kei-ledger fork"));
}
Ok(())
}
/// Variant that additionally derives the downstream handoff chain from the
/// writer's role and scaffolds stub task files for each step. Used by the
/// `kei-spawn spawn --pipeline` CLI flag. Returns the main `SpawnOutput`
/// plus the derived chain so the caller can serialise both.
pub fn spawn_with_pipeline(
task_path: &Path,
kit_root: &Path,
) -> Result<(SpawnOutput, PipelineChain)> {
let out = spawn_from_task(task_path, kit_root)?;
let chain = pipeline::derive_chain_from_role(kit_root, &out.role, &out.agent_id)?;
pipeline::scaffold_downstream_tasks(kit_root, &out.agent_id, &chain)
.context("scaffold downstream pipeline tasks")?;
Ok((out, chain))
}
fn rollback_task_dir(dir: &Path) {
if dir.exists() {
let _ = std::fs::remove_dir_all(dir);
}
}
fn build_output(

View file

@ -179,3 +179,66 @@ fn malformed_json_on_200_maps_to_transport() {
other => panic!("expected Transport, got {other}"),
}
}
/// Oversize response body must be rejected with a Transport error
/// containing `exceeds`. Covers the `content-length` pre-check path:
/// httpmock sends `content-length` automatically for a known-size body,
/// so an 11 MiB payload trips the pre-check (no 11 MiB allocation).
/// Protects the orchestrator process from memory-DoS (CWE-400).
#[test]
fn body_size_limit_rejects_oversized_body() {
let server = MockServer::start();
let _env = EnvGuard::new(Some("k"), Some(&server.url("/v1/messages")));
// Just over the 10 MiB cap — smallest payload that exercises the
// limit without wasting test-harness memory.
let big_body = "a".repeat(11 * 1024 * 1024);
server.mock(|when, then| {
when.method(POST).path("/v1/messages");
then.status(200)
.header("content-type", "application/json")
.body(&big_body);
});
let d = HttpDriver;
let err = d.invoke("x", "y", None).unwrap_err();
match err {
DriveError::Transport { message } => {
assert!(message.contains("exceeds"), "msg: {message}");
}
other => panic!("expected Transport, got {other}"),
}
}
/// Body just under the 10 MiB cap must succeed through the parse stage
/// (parse then fails because the body isn't valid JSON — that's the
/// expected outcome here; we only want to prove the size-gate doesn't
/// fire for sub-limit bodies).
#[test]
fn body_size_limit_allows_under_cap() {
let server = MockServer::start();
let _env = EnvGuard::new(Some("k"), Some(&server.url("/v1/messages")));
// Well under 10 MiB but large enough to rule out trivial paths.
let body = "z".repeat(1024 * 1024); // 1 MiB of garbage
server.mock(|when, then| {
when.method(POST).path("/v1/messages");
then.status(200)
.header("content-type", "application/json")
.body(&body);
});
let d = HttpDriver;
let err = d.invoke("x", "y", None).unwrap_err();
match err {
// Size-gate MUST NOT fire; parse failure is the expected path.
DriveError::Transport { message } => {
assert!(
!message.contains("exceeds"),
"size-gate falsely fired: {message}"
);
assert!(message.contains("parse response"), "msg: {message}");
}
other => panic!("expected Transport, got {other}"),
}
}

View file

@ -0,0 +1,179 @@
//! pipeline_smoke — integration tests for `spawn --pipeline` end-to-end.
//!
//! Same pattern as spawn_smoke.rs: minimal tempdir kit, role + capability
//! fixtures, then call the library surface and assert on-disk artefacts.
//! `KEI_SPAWN_LEDGER_NOOP=1` keeps the ledger subprocess a no-op so tests
//! do not depend on a real kei-ledger binary.
use kei_spawn::{
derive_chain_from_role, pipeline_from_role, spawn_with_pipeline, PipelineChain,
};
use std::path::Path;
use tempfile::TempDir;
fn write_capability(root: &Path, cat: &str, slug: &str, body: &str) {
let dir = root.join("_capabilities").join(cat).join(slug);
std::fs::create_dir_all(&dir).unwrap();
std::fs::write(dir.join("text.md"), body).unwrap();
}
fn write_role(root: &Path, name: &str, toml: &str) {
std::fs::create_dir_all(root.join("_roles")).unwrap();
std::fs::write(root.join("_roles").join(format!("{name}.toml")), toml).unwrap();
}
fn write_task(root: &Path, toml: &str) -> std::path::PathBuf {
let path = root.join("task.toml");
std::fs::write(&path, toml).unwrap();
path
}
fn minimal_kit_with_handoff(root: &Path) {
write_capability(root, "policy", "no-git-ops", "## Never git.\n");
write_capability(root, "output", "report-format", "## Report fields.\n");
write_capability(root, "scope", "read-only", "## Read-only.\n");
write_capability(root, "output", "verdict", "## Verdict.\n");
write_capability(root, "verify", "fork-audit", "## Fork audit.\n");
write_capability(root, "policy", "git-ops-scope", "## Git ops scope.\n");
write_capability(root, "output", "merge-result", "## Merge result.\n");
write_role(
root,
"edit-local",
r#"
[role]
name = "edit-local"
spawnable = true
claude-subagent-type = "code-implementer"
[capabilities]
required = ["policy::no-git-ops", "output::report-format"]
[pipeline]
handoff = ["auditor"]
"#,
);
write_role(
root,
"auditor",
r#"
[role]
name = "auditor"
spawnable = true
claude-subagent-type = "critic"
[capabilities]
required = ["policy::no-git-ops", "scope::read-only", "verify::fork-audit", "output::verdict"]
[tools]
allowed = ["Read", "Glob", "Grep", "Bash"]
bash-patterns-allowed = ['^cargo( |$)','^git diff','^git log','^git show']
[pipeline]
handoff = ["merger"]
"#,
);
write_role(
root,
"merger",
r#"
[role]
name = "merger"
spawnable = true
claude-subagent-type = "infra-implementer"
[capabilities]
required = ["policy::git-ops-scope", "output::merge-result"]
[tools]
allowed = ["Read", "Bash"]
bash-patterns-allowed = ['^git( |$)','^kei-fork( |$)','^kei-ledger( |$)']
[pipeline]
handoff = []
"#,
);
}
fn set_noop() {
std::env::set_var("KEI_SPAWN_LEDGER_NOOP", "1");
}
#[test]
fn auditor_role_resolves() {
let tmp = TempDir::new().unwrap();
let root = tmp.path();
minimal_kit_with_handoff(root);
let handoff = pipeline_from_role(root, "auditor").expect("auditor handoff");
assert_eq!(handoff, vec!["merger".to_string()]);
}
#[test]
fn merger_role_resolves() {
let tmp = TempDir::new().unwrap();
let root = tmp.path();
minimal_kit_with_handoff(root);
let handoff = pipeline_from_role(root, "merger").expect("merger handoff");
assert!(handoff.is_empty(), "merger is the terminal step: {handoff:?}");
}
#[test]
fn pipeline_handoff_produces_chain() {
let tmp = TempDir::new().unwrap();
let root = tmp.path();
minimal_kit_with_handoff(root);
let chain: PipelineChain =
derive_chain_from_role(root, "edit-local", "ag-edit-local-test-001").unwrap();
assert_eq!(chain.steps.len(), 1);
assert_eq!(chain.steps[0].role, "auditor");
assert_eq!(chain.steps[0].agent_id, "ag-edit-local-test-001-auditor");
}
#[test]
fn pipeline_missing_role_returns_empty() {
let tmp = TempDir::new().unwrap();
let root = tmp.path();
minimal_kit_with_handoff(root);
// merger exists but has empty handoff — empty Vec, not error.
let chain = derive_chain_from_role(root, "merger", "ag-merger-test-001").unwrap();
assert!(chain.steps.is_empty());
// unknown role — must error cleanly.
let err = derive_chain_from_role(root, "does-not-exist", "ag-x-001")
.expect_err("unknown role must error");
let msg = format!("{err:#}");
assert!(msg.contains("does-not-exist"), "msg: {msg}");
}
#[test]
fn spawn_with_pipeline_scaffolds_downstream_stubs() {
set_noop();
let tmp = TempDir::new().unwrap();
let root = tmp.path();
minimal_kit_with_handoff(root);
let task_path = write_task(
root,
r#"
[task]
role = "edit-local"
[body]
text = "Writer body that hands off to auditor."
"#,
);
let (out, chain) = spawn_with_pipeline(&task_path, root).expect("spawn_with_pipeline");
assert_eq!(chain.steps.len(), 1);
assert_eq!(chain.steps[0].role, "auditor");
assert_eq!(chain.steps[0].agent_id, format!("{}-auditor", out.agent_id));
let writer_dir = root.join("tasks").join(&out.agent_id);
assert!(writer_dir.join("pipeline.json").is_file());
let auditor_dir = root.join("tasks").join(format!("{}-auditor", out.agent_id));
assert!(auditor_dir.join("task.stub.toml").is_file());
let stub_text = std::fs::read_to_string(auditor_dir.join("task.stub.toml")).unwrap();
assert!(stub_text.contains(r#"role = "auditor""#));
assert!(stub_text.contains(&format!(r#"parent-agent = "{}""#, out.agent_id)));
}

View file

@ -0,0 +1,139 @@
//! pipeline_unit — fine-grained coverage for pipeline helpers + the
//! spawn-rollback-on-ledger-failure contract.
//!
//! Complements pipeline_smoke.rs (end-to-end) with unit-level assertions
//! that don't need the full spawn pipeline.
use kei_spawn::{
derive_steps, emit_pipeline_json, pipeline_json_path, spawn_from_task, PipelineChain,
PipelineStep,
};
use std::path::Path;
use tempfile::TempDir;
fn write_capability(root: &Path, cat: &str, slug: &str, body: &str) {
let dir = root.join("_capabilities").join(cat).join(slug);
std::fs::create_dir_all(&dir).unwrap();
std::fs::write(dir.join("text.md"), body).unwrap();
}
fn write_role(root: &Path, name: &str, toml: &str) {
std::fs::create_dir_all(root.join("_roles")).unwrap();
std::fs::write(root.join("_roles").join(format!("{name}.toml")), toml).unwrap();
}
fn minimal_kit(root: &Path) {
write_capability(root, "policy", "no-git-ops", "## Never git.\n");
write_capability(root, "output", "report-format", "## Report fields.\n");
write_role(
root,
"edit-local",
r#"
[role]
name = "edit-local"
spawnable = true
claude-subagent-type = "code-implementer"
[capabilities]
required = ["policy::no-git-ops", "output::report-format"]
"#,
);
}
#[test]
fn derive_steps_child_ids_distinct() {
let roles = vec!["auditor".to_string(), "merger".to_string()];
let steps = derive_steps("ag-edit-local-zzz", &roles);
assert_eq!(steps.len(), 2);
assert_eq!(steps[0].agent_id, "ag-edit-local-zzz-auditor");
assert_eq!(steps[1].agent_id, "ag-edit-local-zzz-merger");
assert_ne!(steps[0].agent_id, steps[1].agent_id);
}
#[test]
fn derive_steps_skips_empty_role_names() {
let roles = vec![
"auditor".to_string(),
" ".to_string(),
"".to_string(),
"merger".to_string(),
];
let steps = derive_steps("ag-writer-001", &roles);
assert_eq!(steps.len(), 2);
assert_eq!(steps[0].role, "auditor");
assert_eq!(steps[1].role, "merger");
}
#[test]
fn emit_pipeline_json_creates_parent_dir() {
let tmp = TempDir::new().unwrap();
let nested = tmp.path().join("a").join("b").join("pipeline.json");
let chain = PipelineChain {
steps: vec![PipelineStep {
role: "auditor".into(),
agent_id: "ag-x-auditor".into(),
}],
};
emit_pipeline_json(&nested, &chain).expect("emit");
assert!(nested.is_file(), "{} should exist", nested.display());
let body = std::fs::read_to_string(&nested).unwrap();
assert!(body.contains("\"auditor\""), "json: {body}");
assert!(body.contains("\"ag-x-auditor\""), "json: {body}");
}
#[test]
fn pipeline_json_path_uses_convention() {
let root = Path::new("/tmp/kit");
let path = pipeline_json_path(root, "ag-writer-42");
assert_eq!(
path,
Path::new("/tmp/kit/tasks/ag-writer-42/pipeline.json")
);
}
#[test]
fn precedent_check_env_gated_off_silent() {
// Ensure env flag absent → run_advisory returns Ok(0) without shelling out.
std::env::remove_var("KEI_SPAWN_PRECEDENT_CHECK");
let n = kei_spawn::precedent::run_advisory("00".repeat(32).as_str()).unwrap();
assert_eq!(n, 0);
}
#[test]
fn spawn_rolls_back_task_dir_on_ledger_fail() {
// Force ledger failure by pointing at a bogus binary AND clearing the
// noop escape hatch so the subprocess actually runs (and fails).
std::env::remove_var("KEI_SPAWN_LEDGER_NOOP");
std::env::set_var("KEI_LEDGER_BIN", "/nonexistent/kei-ledger-rollback-test");
let tmp = TempDir::new().unwrap();
let root = tmp.path();
minimal_kit(root);
let task_path = root.join("task.toml");
std::fs::write(
&task_path,
r#"
[task]
role = "edit-local"
agent-id = "ag-edit-local-rollback-001"
[body]
text = "Rollback test."
"#,
)
.unwrap();
let result = spawn_from_task(&task_path, root);
assert!(result.is_err(), "ledger fail must propagate");
let agent_dir = root.join("tasks").join("ag-edit-local-rollback-001");
assert!(
!agent_dir.exists(),
"task dir must be cleaned up after ledger failure; still exists at {}",
agent_dir.display()
);
// Restore ledger env so other tests in the crate don't see the bogus path.
std::env::remove_var("KEI_LEDGER_BIN");
std::env::set_var("KEI_SPAWN_LEDGER_NOOP", "1");
}

28
_roles/auditor.toml Normal file
View file

@ -0,0 +1,28 @@
[role]
name = "auditor"
display-name = "Fork review auditor (read-only)"
description = "Reviews a writer's fork diff. Emits PASS/FAIL/INCONCLUSIVE verdict with findings. No mutations."
spawnable = true
claude-subagent-type = "critic"
[capabilities]
required = ["policy::no-git-ops","scope::read-only","verify::fork-audit","output::verdict"]
[tools]
allowed = ["Read","Glob","Grep","Bash"]
bash-patterns-allowed = ['^cargo( |$)','^git diff','^git log','^git show']
[escalation]
policy = "ask-via-return"
[taxonomy]
kingdom = "role"
mechanism = "audit"
domain = "agent"
layer = "agent-substrate"
stage = "runtime"
stability = "stable"
language = "toml"
[pipeline]
handoff = ["merger"]

28
_roles/merger.toml Normal file
View file

@ -0,0 +1,28 @@
[role]
name = "merger"
display-name = "Fork merger (git-ops scoped)"
description = "Merges reviewed fork to main. Calls kei-fork collect. No code edits."
spawnable = true
claude-subagent-type = "infra-implementer"
[capabilities]
required = ["policy::git-ops-scope","output::merge-result"]
[tools]
allowed = ["Read","Bash"]
bash-patterns-allowed = ['^git( |$)','^kei-fork( |$)','^kei-ledger( |$)']
[escalation]
policy = "ask-via-return"
[taxonomy]
kingdom = "role"
mechanism = "merge"
domain = "agent"
layer = "agent-substrate"
stage = "runtime"
stability = "stable"
language = "toml"
[pipeline]
handoff = []