From 5f7a5b2639b790c78c0fb77bd8f69208c09e23fd Mon Sep 17 00:00:00 2001 From: Parfii-bot Date: Thu, 23 Apr 2026 20:54:59 +0800 Subject: [PATCH] fix(wave18): 8 HIGH audit findings closed + three-role pipeline actually built MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../output/merge-result/capability.toml | 35 +++ _capabilities/output/merge-result/text.md | 47 ++++ _capabilities/output/verdict/capability.toml | 35 +++ _capabilities/output/verdict/text.md | 45 ++++ .../policy/git-ops-scope/capability.toml | 35 +++ _capabilities/policy/git-ops-scope/text.md | 39 ++++ _capabilities/scope/read-only/capability.toml | 35 +++ _capabilities/scope/read-only/text.md | 26 +++ .../verify/fork-audit/capability.toml | 35 +++ _capabilities/verify/fork-audit/text.md | 44 ++++ _primitives/_rust/Cargo.lock | 1 + _primitives/_rust/kei-fork/src/collect.rs | 85 +++++--- _primitives/_rust/kei-fork/src/create.rs | 46 +++- _primitives/_rust/kei-fork/src/error.rs | 4 + _primitives/_rust/kei-fork/src/git.rs | 148 +++++++++++-- .../_rust/kei-fork/tests/fork_integration.rs | 202 ++++++++++++++++-- .../_rust/kei-ledger-sign/src/keypair.rs | 28 ++- .../_rust/kei-ledger-sign/src/perms.rs | 39 +++- .../kei-ledger-sign/tests/integration.rs | 36 ++++ _primitives/_rust/kei-spawn/Cargo.toml | 1 + _primitives/_rust/kei-spawn/src/drive_http.rs | 44 +++- _primitives/_rust/kei-spawn/src/ledger_sh.rs | 92 ++++++++ _primitives/_rust/kei-spawn/src/lib.rs | 8 +- _primitives/_rust/kei-spawn/src/main.rs | 36 +++- _primitives/_rust/kei-spawn/src/pipeline.rs | 171 +++++++++++++++ _primitives/_rust/kei-spawn/src/precedent.rs | 120 +++++++++++ _primitives/_rust/kei-spawn/src/spawn.rs | 63 +++++- .../_rust/kei-spawn/tests/http_driver.rs | 63 ++++++ .../_rust/kei-spawn/tests/pipeline_smoke.rs | 179 ++++++++++++++++ .../_rust/kei-spawn/tests/pipeline_unit.rs | 139 ++++++++++++ _roles/auditor.toml | 28 +++ _roles/merger.toml | 28 +++ 32 files changed, 1843 insertions(+), 94 deletions(-) create mode 100644 _capabilities/output/merge-result/capability.toml create mode 100644 _capabilities/output/merge-result/text.md create mode 100644 _capabilities/output/verdict/capability.toml create mode 100644 _capabilities/output/verdict/text.md create mode 100644 _capabilities/policy/git-ops-scope/capability.toml create mode 100644 _capabilities/policy/git-ops-scope/text.md create mode 100644 _capabilities/scope/read-only/capability.toml create mode 100644 _capabilities/scope/read-only/text.md create mode 100644 _capabilities/verify/fork-audit/capability.toml create mode 100644 _capabilities/verify/fork-audit/text.md create mode 100644 _primitives/_rust/kei-spawn/src/pipeline.rs create mode 100644 _primitives/_rust/kei-spawn/src/precedent.rs create mode 100644 _primitives/_rust/kei-spawn/tests/pipeline_smoke.rs create mode 100644 _primitives/_rust/kei-spawn/tests/pipeline_unit.rs create mode 100644 _roles/auditor.toml create mode 100644 _roles/merger.toml diff --git a/_capabilities/output/merge-result/capability.toml b/_capabilities/output/merge-result/capability.toml new file mode 100644 index 0000000..007254b --- /dev/null +++ b/_capabilities/output/merge-result/capability.toml @@ -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" diff --git a/_capabilities/output/merge-result/text.md b/_capabilities/output/merge-result/text.md new file mode 100644 index 0000000..c6c0243 --- /dev/null +++ b/_capabilities/output/merge-result/text.md @@ -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: + 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 `` + 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 ` 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. diff --git a/_capabilities/output/verdict/capability.toml b/_capabilities/output/verdict/capability.toml new file mode 100644 index 0000000..6c6e2e3 --- /dev/null +++ b/_capabilities/output/verdict/capability.toml @@ -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" diff --git a/_capabilities/output/verdict/text.md b/_capabilities/output/verdict/text.md new file mode 100644 index 0000000..33a5d2d --- /dev/null +++ b/_capabilities/output/verdict/text.md @@ -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: + audited-agent: + + 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: + audited-agent: + +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 ` (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. diff --git a/_capabilities/policy/git-ops-scope/capability.toml b/_capabilities/policy/git-ops-scope/capability.toml new file mode 100644 index 0000000..3570583 --- /dev/null +++ b/_capabilities/policy/git-ops-scope/capability.toml @@ -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" diff --git a/_capabilities/policy/git-ops-scope/text.md b/_capabilities/policy/git-ops-scope/text.md new file mode 100644 index 0000000..1c69461 --- /dev/null +++ b/_capabilities/policy/git-ops-scope/text.md @@ -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. diff --git a/_capabilities/scope/read-only/capability.toml b/_capabilities/scope/read-only/capability.toml new file mode 100644 index 0000000..dd39aa9 --- /dev/null +++ b/_capabilities/scope/read-only/capability.toml @@ -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" diff --git a/_capabilities/scope/read-only/text.md b/_capabilities/scope/read-only/text.md new file mode 100644 index 0000000..4ac4d6d --- /dev/null +++ b/_capabilities/scope/read-only/text.md @@ -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. diff --git a/_capabilities/verify/fork-audit/capability.toml b/_capabilities/verify/fork-audit/capability.toml new file mode 100644 index 0000000..743d18d --- /dev/null +++ b/_capabilities/verify/fork-audit/capability.toml @@ -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" diff --git a/_capabilities/verify/fork-audit/text.md b/_capabilities/verify/fork-audit/text.md new file mode 100644 index 0000000..268615d --- /dev/null +++ b/_capabilities/verify/fork-audit/text.md @@ -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. diff --git a/_primitives/_rust/Cargo.lock b/_primitives/_rust/Cargo.lock index 64e39ed..155f9d6 100644 --- a/_primitives/_rust/Cargo.lock +++ b/_primitives/_rust/Cargo.lock @@ -2845,6 +2845,7 @@ dependencies = [ "serde_json", "sha2 0.10.9", "tempfile", + "toml", ] [[package]] diff --git a/_primitives/_rust/kei-fork/src/collect.rs b/_primitives/_rust/kei-fork/src/collect.rs index 61ee210..60b8ef2 100644 --- a/_primitives/_rust/kei-fork/src/collect.rs +++ b/_primitives/_rust/kei-fork/src/collect.rs @@ -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 ` + `git commit` //! 3. Capture commit SHA, then `git merge --no-ff fork/` in kit_root //! 4. Move worktree to `_archive/forks/YYYY-MM-DD//` (preserving //! the agent's artefacts for post-hoc review / rescue) //! 5. `git worktree prune && git branch -D fork/` to clean up refs //! 6. `kei-ledger done ` unless `KEI_FORK_SKIP_LEDGER=1` //! -//! On SUCCESS: `_forks//` 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 { 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 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, Error> { + let untracked = git::ls_untracked(worktree_abs)?; + let modified = git::ls_modified(worktree_abs)?; + let mut combined: Vec = 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")); + } +} diff --git a/_primitives/_rust/kei-fork/src/create.rs b/_primitives/_rust/kei-fork/src/create.rs index f9295a5..3086d0e 100644 --- a/_primitives/_rust/kei-fork/src/create.rs +++ b/_primitives/_rust/kei-fork/src/create.rs @@ -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 Result 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(()); } diff --git a/_primitives/_rust/kei-fork/src/error.rs b/_primitives/_rust/kei-fork/src/error.rs index 0127495..b91e4b5 100644 --- a/_primitives/_rust/kei-fork/src/error.rs +++ b/_primitives/_rust/kei-fork/src/error.rs @@ -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), diff --git a/_primitives/_rust/kei-fork/src/git.rs b/_primitives/_rust/kei-fork/src/git.rs index 679d849..9034142 100644 --- a/_primitives/_rust/kei-fork/src/git.rs +++ b/_primitives/_rust/kei-fork/src/git.rs @@ -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 { 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 { 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, 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, 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::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 { - 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) } diff --git a/_primitives/_rust/kei-fork/tests/fork_integration.rs b/_primitives/_rust/kei-fork/tests/fork_integration.rs index 43cdd8e..1197d07 100644 --- a/_primitives/_rust/kei-fork/tests/fork_integration.rs +++ b/_primitives/_rust/kei-fork/tests/fork_integration.rs @@ -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}" + ); +} diff --git a/_primitives/_rust/kei-ledger-sign/src/keypair.rs b/_primitives/_rust/kei-ledger-sign/src/keypair.rs index 2059de9..d639537 100644 --- a/_primitives/_rust/kei-ledger-sign/src/keypair.rs +++ b/_primitives/_rust/kei-ledger-sign/src/keypair.rs @@ -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 .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 { let text = std::fs::read_to_string(path)?; let file: KeyFile = serde_json::from_str(&text)?; diff --git a/_primitives/_rust/kei-ledger-sign/src/perms.rs b/_primitives/_rust/kei-ledger-sign/src/perms.rs index 22dd5a6..869497a 100644 --- a/_primitives/_rust/kei-ledger-sign/src/perms.rs +++ b/_primitives/_rust/kei-ledger-sign/src/perms.rs @@ -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 { + 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 { + // 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))] diff --git a/_primitives/_rust/kei-ledger-sign/tests/integration.rs b/_primitives/_rust/kei-ledger-sign/tests/integration.rs index be0b689..56d6896 100644 --- a/_primitives/_rust/kei-ledger-sign/tests/integration.rs +++ b/_primitives/_rust/kei-ledger-sign/tests/integration.rs @@ -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 .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"); diff --git a/_primitives/_rust/kei-spawn/Cargo.toml b/_primitives/_rust/kei-spawn/Cargo.toml index 3ab82a3..b8ada83 100644 --- a/_primitives/_rust/kei-spawn/Cargo.toml +++ b/_primitives/_rust/kei-spawn/Cargo.toml @@ -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] diff --git a/_primitives/_rust/kei-spawn/src/drive_http.rs b/_primitives/_rust/kei-spawn/src/drive_http.rs index f71da4d..fc1d036 100644 --- a/_primitives/_rust/kei-spawn/src/drive_http.rs +++ b/_primitives/_rust/kei-spawn/src/drive_http.rs @@ -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 { + 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}"), diff --git a/_primitives/_rust/kei-spawn/src/ledger_sh.rs b/_primitives/_rust/kei-spawn/src/ledger_sh.rs index 38935c6..381cd71 100644 --- a/_primitives/_rust/kei-spawn/src/ledger_sh.rs +++ b/_primitives/_rust/kei-spawn/src/ledger_sh.rs @@ -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(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"); + }, + ); + } +} diff --git a/_primitives/_rust/kei-spawn/src/lib.rs b/_primitives/_rust/kei-spawn/src/lib.rs index 9384d2e..4ff8f43 100644 --- a/_primitives/_rust/kei-spawn/src/lib.rs +++ b/_primitives/_rust/kei-spawn/src/lib.rs @@ -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}; diff --git a/_primitives/_rust/kei-spawn/src/main.rs b/_primitives/_rust/kei-spawn/src/main.rs index b46bce8..d74cb7c 100644 --- a/_primitives/_rust/kei-spawn/src/main.rs +++ b/_primitives/_rust/kei-spawn/src/main.rs @@ -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, + /// 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) -> ExitCode { +#[derive(Serialize)] +struct SpawnWithPipelineJson<'a> { + #[serde(flatten)] + spawn: &'a SpawnOutput, + pipeline: &'a PipelineChain, +} + +fn run_spawn(task: PathBuf, kit_root: Option, 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), + } } } diff --git a/_primitives/_rust/kei-spawn/src/pipeline.rs b/_primitives/_rust/kei-spawn/src/pipeline.rs new file mode 100644 index 0000000..9f9fe66 --- /dev/null +++ b/_primitives/_rust/kei-spawn/src/pipeline.rs @@ -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, +} + +/// Raw on-disk shape of `_roles/.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, +} + +/// Read `_roles/.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> { + 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 +/// `-` so ledger rows remain unique + parent-linked. +pub fn derive_steps(writer_id: &str, handoff_roles: &[String]) -> Vec { + 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//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//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//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 ` 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 { + 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") +} diff --git a/_primitives/_rust/kei-spawn/src/precedent.rs b/_primitives/_rust/kei-spawn/src/precedent.rs new file mode 100644 index 0000000..4fc4f78 --- /dev/null +++ b/_primitives/_rust/kei-spawn/src/precedent.rs @@ -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 `; 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 { + 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 { + 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::(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); + } +} diff --git a/_primitives/_rust/kei-spawn/src/spawn.rs b/_primitives/_rust/kei-spawn/src/spawn.rs index f4d9d7c..c49b3a3 100644 --- a/_primitives/_rust/kei-spawn/src/spawn.rs +++ b/_primitives/_rust/kei-spawn/src/spawn.rs @@ -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 { 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 .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( diff --git a/_primitives/_rust/kei-spawn/tests/http_driver.rs b/_primitives/_rust/kei-spawn/tests/http_driver.rs index 0d2c06c..73cc142 100644 --- a/_primitives/_rust/kei-spawn/tests/http_driver.rs +++ b/_primitives/_rust/kei-spawn/tests/http_driver.rs @@ -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}"), + } +} diff --git a/_primitives/_rust/kei-spawn/tests/pipeline_smoke.rs b/_primitives/_rust/kei-spawn/tests/pipeline_smoke.rs new file mode 100644 index 0000000..f61092c --- /dev/null +++ b/_primitives/_rust/kei-spawn/tests/pipeline_smoke.rs @@ -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))); +} diff --git a/_primitives/_rust/kei-spawn/tests/pipeline_unit.rs b/_primitives/_rust/kei-spawn/tests/pipeline_unit.rs new file mode 100644 index 0000000..4bada0e --- /dev/null +++ b/_primitives/_rust/kei-spawn/tests/pipeline_unit.rs @@ -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"); +} diff --git a/_roles/auditor.toml b/_roles/auditor.toml new file mode 100644 index 0000000..5d7d164 --- /dev/null +++ b/_roles/auditor.toml @@ -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"] diff --git a/_roles/merger.toml b/_roles/merger.toml new file mode 100644 index 0000000..330d4c3 --- /dev/null +++ b/_roles/merger.toml @@ -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 = []