diff --git a/design/agent-publish.md b/design/agent-publish.md index ea8266d3..8f814476 100644 --- a/design/agent-publish.md +++ b/design/agent-publish.md @@ -36,6 +36,7 @@ The endpoint performs one push: stored commit. Trust the complete history verified at ingestion and startup. 2. If E is non-null, require E to be a stored ancestor of H. If H contains E, CAOS already has E. A missing E or non-fast-forward is a rejection. + Reject H if its tree contains paths matched by its own .gitignore rules. 3. Push H to the destination branch with --force-with-lease=refs/heads/:, disabling tag following. Empty E requires creation. The ancestry check prevents history rewrites; @@ -45,8 +46,8 @@ The endpoint performs one push: per-ref receiver rejections are definite failures. Unconfirmed transport failures are uncertain. The CLI preserves these results for llm-step. -The endpoint does not fetch, import, merge, rebase, resolve source policy, or -perform a follow-up remote lookup. Objects transfer directly from CAOS to the +The endpoint does not fetch, import, merge, rebase, rewrite commits, or perform +a follow-up remote lookup. Objects transfer directly from CAOS to the destination through Git. Reuse import authentication: token-file option, sensitive header and repository-scoped credential helper. @@ -63,19 +64,28 @@ reason through the CLI to the tool result. Importing and integration are never hidden inside a push. A remote can accept a push before the connection drops. Git's HTTP retry can -then report a stale lease. After a conflict or uncertain result, llm-step reads -the branch: H confirms completion. Otherwise it preserves a definite rejection; +then report a stale lease. After a receiver conflict or uncertain result, +llm-step reads the branch: H confirms completion. Otherwise it preserves a definite rejection; for uncertainty, another value than E is a conflict, while E or a failed lookup remains uncertain because a push may still be running. The endpoint itself does no recovery. A success receipt records the original push even if the branch later advances. -Publication transfers the exact commit, including its tracked files. It does -not apply .gitignore or remove .caos content. Local Git staging respects -.gitignore for untracked files, and host path ingestion uses git ls-files. -Remote imports preserve their existing commit. The agent's bash tool, however, -stores every real file left in its working tree through caos put; that path -currently does not apply .gitignore. Ignore handling belongs in source capture -as a follow-up, before a commit is formed, not in publication. +Publication transfers the exact commit. Before pushing, the server checks H's +tree against its versioned .gitignore files, including nested rules and +negations. A match returns HTTP 422 with code `ignored-files`; the CLI and +agent retain this as a definite rejection without remote reconciliation. Git +performs the check using a private index; no source files are checked out. + +This is deliberately stricter than ordinary Git: a tracked file matching an +ignore rule is rejected too. Global excludes and .git/info/exclude do not +apply. This checks the requested snapshot only, not earlier commits; a file +added and deleted in its history is outside this check. The server never +strips files or rewrites history. + +Local Git staging still respects .gitignore for untracked files. Imports keep +their exact commits, and agent tools continue to capture files as they do +today. Ignored scratch files can therefore remain in a source during work; +the agent must remove them or adjust the rules before publication. Merge conflicts currently create a tracked .caos/conflicts ledger inside a source tree, including conflicts without inline markers. llm-step must resolve diff --git a/rust/crates/git-locator/src/publish.rs b/rust/crates/git-locator/src/publish.rs index bbfc966b..ca38fd8d 100644 --- a/rust/crates/git-locator/src/publish.rs +++ b/rust/crates/git-locator/src/publish.rs @@ -5,6 +5,7 @@ pub fn diagnostic(code: &str) -> Option<&'static str> { "missing-commit" => "The server does not hold the source commit. Import it before publishing.", "missing-expected" => "The server does not hold the expected remote head. Import it before publishing.", "not-fast-forward" => "The update is not a fast-forward. Import and merge the remote head before publishing.", + "ignored-files" => "The source commit contains files matched by its .gitignore rules. Remove those files or adjust the ignore rules before publishing; no push was attempted.", "validation-failed" => "The server could not validate the source commit; no push was attempted.", "lease-rejected" => "The remote head does not match the pinned lease. Inspect it before importing and integrating changes.", "hook-declined" => "The remote rejected the push: a receive hook declined it. Check repository rules and branch protection.", diff --git a/rust/crates/server/src/push.rs b/rust/crates/server/src/push.rs index cd98184c..fc5e4f0c 100644 --- a/rust/crates/server/src/push.rs +++ b/rust/crates/server/src/push.rs @@ -84,7 +84,7 @@ pub(crate) fn endpoint( .truncate(false) .read(true) .write(true) - .open(locks.join(key))?; + .open(locks.join(&key))?; lock.lock()?; let receipt = @@ -110,6 +110,16 @@ pub(crate) fn endpoint( _ => return Err(reject("validation-failed")), } } + if ignored_files( + &input.destination, + &config.git_dir, + &input.commit, + &locks.join(format!("{key}.check")), + ) + .map_err(|_| reject("validation-failed"))? + { + return Err(reject("ignored-files")); + } let lease = format!( "--force-with-lease={refname}:{}", input.expected.as_deref().unwrap_or("") @@ -161,6 +171,85 @@ pub(crate) fn endpoint( } } +// The destination lock owns this scratch directory, including leftovers after +// a crash. Only an index and path list are written: skip-worktree lets Git read +// nested .gitignore blobs from the index without checking out source files. +fn ignored_files( + destination: &str, + git_dir: &str, + commit: &str, + directory: &std::path::Path, +) -> Result { + use std::fs::{self, File}; + if directory.exists() { + fs::remove_dir_all(directory)?; + } + fs::create_dir_all(directory.join("work"))?; + let directory = fs::canonicalize(directory)?; + let result = (|| { + let invalid = || HttpError::new(422, "Git ignore validation failed"); + let run = |args: &[&str], stdin: Option| -> Result, HttpError> { + let mut command = git_locator::import::git(destination, None).map_err(|_| invalid())?; + command + .args(["--git-dir", git_dir, "-c", "core.bare=false"]) + .args(["-c", "core.sparseCheckout=false", "--work-tree"]) + .arg(directory.join("work")) + .env("GIT_INDEX_FILE", directory.join("index")) + .args(args); + if let Some(input) = stdin { + command.stdin(input); + } + let output = command.output().map_err(|_| invalid())?; + if !output.status.success() { + return Err(invalid()); + } + Ok(output.stdout) + }; + run(&["read-tree", commit], None)?; + // Only regular ignore files may supply patterns. Marking a symlink + // skip-worktree would make Git's index fallback parse its link target. + let mut patterns = Vec::new(); + for entry in run(&["ls-files", "--stage", "-z"], None)? + .split(|b| *b == 0) + .filter(|entry| entry.starts_with(b"100644 ") || entry.starts_with(b"100755 ")) + { + let path = entry + .splitn(2, |b| *b == b'\t') + .nth(1) + .ok_or_else(invalid)?; + if path == b".gitignore" || path.ends_with(b"/.gitignore") { + patterns.extend_from_slice(path); + patterns.push(0); + } + } + if patterns.is_empty() { + return Ok(false); + } + let paths = directory.join("paths"); + fs::write(&paths, patterns)?; + run( + &["update-index", "--skip-worktree", "-z", "--stdin"], + Some(File::open(paths)?), + )?; + // Explicit per-directory rules exclude host/global/info/exclude policy. + // --cached deliberately checks tracked entries too: this is a publication + // rule, stricter than Git's ordinary admission of untracked files. + Ok(!run( + &[ + "ls-files", + "--cached", + "--ignored", + "--exclude-per-directory=.gitignore", + "-z", + ], + None, + )? + .is_empty()) + })(); + fs::remove_dir_all(directory)?; + result +} + // Only a per-ref porcelain rejection proves the receiver refused this update. // Missing status (including authentication/transport failures) remains uncertain. fn rejection(stdout: &[u8], refname: &str) -> Option<&'static str> { diff --git a/std/llm-step/src/main.rs b/std/llm-step/src/main.rs index 86a32b68..7243fc5d 100644 --- a/std/llm-step/src/main.rs +++ b/std/llm-step/src/main.rs @@ -4401,11 +4401,15 @@ mod tests { .unwrap(); assert_eq!(recovered, pending); let outcome = if rejected { - conversation_protocol::v3::publication::Outcome::new( - PublicationStatus::Conflict, - "validation-rejected", - Some("Invalid source".into()), - None, + publish_source::reconcile( + &recovered, + conversation_protocol::v3::publication::Outcome::new( + PublicationStatus::Conflict, + "validation-rejected", + Some("Source contains ignored files".into()), + None, + ), + || panic!("validation rejection must not observe the remote"), ) } else { // Git may resend after a lost acknowledgement and report a @@ -4418,7 +4422,7 @@ mod tests { None, None, ), - Ok(Some(commit.clone())), + || Ok(Some(commit.clone())), ) }; publish_source::finish(&mut state, &site, &pending, Some(outcome)).unwrap(); diff --git a/std/llm-step/src/publish_source.rs b/std/llm-step/src/publish_source.rs index 7766b72a..708264c3 100644 --- a/std/llm-step/src/publish_source.rs +++ b/std/llm-step/src/publish_source.rs @@ -3,7 +3,7 @@ use super::*; use conversation_protocol::v3::publication::Outcome; use conversation_protocol::v3::{Descriptor, PublicationRecord, PublicationStatus}; -pub(super) const HELP: &str = "Publish the exact selected source commit to an HTTPS Git repository branch, preserving its history. Test and inspect the intended PR diff first. Resolve merge conflicts and clear .caos/conflicts before publishing. The endpoint pushes the commit unchanged; it does not filter files or apply .gitignore. This does not create a PR or change the source gitlink. Only fast-forward updates are supported: import and merge remote changes before retrying a conflict. A receipt names the exact published commit even if the source later changes. On uncertainty, inspect the remote before taking another action. +pub(super) const HELP: &str = "Publish the exact selected source commit to an HTTPS Git repository branch, preserving its history. Test and inspect the intended PR diff first. Resolve merge conflicts and clear .caos/conflicts before publishing. The endpoint rejects files matched by the source commit's .gitignore rules, including tracked files. Remove those files or adjust the rules before publishing. It never strips files or rewrites commits. This does not create a PR or change the source gitlink. Only fast-forward updates are supported: import and merge remote changes before retrying a conflict. A receipt names the exact published commit even if the source later changes. On uncertainty, inspect the remote before taking another action. @param repository HTTPS Git repository URL, without credentials. @param branch Destination branch name (without refs/heads/)."; @@ -172,11 +172,7 @@ pub(super) fn execute(state: &mut progress::State, site: &CallSite<'_>) -> Resul value["diagnostic"].as_str().map(str::to_owned), observed, ); - if status == PublicationStatus::Complete { - outcome - } else { - reconcile(&pending, outcome, observe()) - } + reconcile(&pending, outcome, observe) } Ok(output) if output.status.code() == Some(1) => Outcome::new( PublicationStatus::Conflict, @@ -203,9 +199,16 @@ pub(super) fn invocation(conversation: &str, site: &CallSite<'_>) -> Result, String>, + observe: impl FnOnce() -> Result, String>, ) -> Outcome { - match observed { + // A local/server validation refusal means no push was attempted. A remote + // head that already matches must not hide the rejection or its diagnostic. + if outcome.status == PublicationStatus::Complete + || outcome.evidence.kind == "validation-rejected" + { + return outcome; + } + match observe() { Ok(head) if head.as_ref() == Some(&pending.planned_head) => { Outcome::new(PublicationStatus::Complete, "ref-converged", None, head) } diff --git a/tests/git-import/fixture.py b/tests/git-import/fixture.py index 62ec575d..868e9bc7 100644 --- a/tests/git-import/fixture.py +++ b/tests/git-import/fixture.py @@ -6,6 +6,7 @@ """ import base64 import concurrent.futures +import gzip import hashlib import http.server import json @@ -59,7 +60,7 @@ def main(): git_bin.mkdir() git_shim = git_bin / "git" git_shim.write_text("#!/bin/sh\nfor arg do\n" - 'case "$arg" in ls-remote|fetch|cat-file|rev-list|show-index|pack-objects|index-pack)\n' + 'case "$arg" in ls-remote|fetch|cat-file|rev-list|show-index|pack-objects|index-pack|push)\n' "printf '%s\\n' \"$arg\" >> " + shlex.quote(str(git_commands)) + "; break;;\n" "esac\ndone\nexec " + shlex.quote(shutil.which("git")) + ' "$@"\n') git_shim.chmod(0o755) @@ -103,6 +104,8 @@ def do_POST(self): def serve(self): path, _, query = self.path.partition("?") body = self.rfile.read(int(self.headers.get("Content-Length", 0))) + if self.headers.get("Content-Encoding") == "gzip": + body = gzip.decompress(body) if path.startswith("/surplus.git/"): if self.command == "GET": payload = (packet(b"# service=git-upload-pack\n") + b"0000" @@ -547,6 +550,83 @@ def source_commit(tree, parent=None): head = run(*args, env=env) call(payload(head)) return head + # Publication checks the exact snapshot against its own ignore rules. + # Files here are intentionally tracked; ordinary git-add admission + # would not catch them. The server must neither rewrite nor push them. + def ignore_snapshot(files, parent=first[0]): + index_env = dict(env, GIT_INDEX_FILE=str(root / "ignore-index")) + run("git", "--git-dir", str(origin), "read-tree", "--empty", env=index_env) + entries = b"" + for name, content in files.items(): + mode = "100644" + if isinstance(content, tuple): + mode, content = content + blob = run("git", "--git-dir", str(origin), "hash-object", "-w", "--stdin", + input=content.encode()) + entries += f"{mode} {blob}\t{name}\0".encode() + run("git", "--git-dir", str(origin), "update-index", "-z", "--index-info", + input=entries, env=index_env) + tree = run("git", "--git-dir", str(origin), "write-tree", env=index_env) + return source_commit(tree, parent) + + cases = [ + ({"results/out": "generated", ".gitignore": "results/\n"}, True), + ({".gitignore": "*.log\n!keep.log\n", "keep.log": "kept"}, False), + ({".gitignore": "*.log\n", "sub/.gitignore": "!keep.log\n", + "sub/keep.log": "kept"}, False), + ({"sub/.gitignore": "*.tmp\n", "sub/a.tmp": "generated"}, True), + ({".gitignore": "sub/\n", "sub/.gitignore": "!keep\n", + "sub/keep": "parent exclusion wins"}, True), + ({".gitignore": "*.log\n", "a space/line\nbreak.log": "generated"}, True), + ({".gitignore": "*.log\n", "link.log": ("120000", "elsewhere")}, True), + ({".gitignore": "cache/\n", "cache": ("120000", "elsewhere")}, False), + ({".gitignore": ("120000", "rules"), "rules": "*.log\n", "file.log": "kept"}, False), + ({".gitignore": "/root-only\n", "sub/root-only": "kept"}, False), + ({".gitignore": "*.tmp\n", "ordinary.txt": "kept"}, False), + ] + # Host policy and the server's index must not influence the check. + (odb / "info").mkdir(exist_ok=True) + (odb / "info/exclude").write_text("ordinary.txt\n") + global_excludes = root / "server-excludes" + global_excludes.write_text("ordinary.txt\n") + run("git", "--git-dir", str(odb), "config", "core.excludesFile", str(global_excludes)) + (odb / "index").write_bytes(b"server index must remain untouched") + ignored_head = None + for number, (files, ignored) in enumerate(cases): + head = ignore_snapshot(files) + branch = f"ignore-{number}" + git_commands.write_text("") + if ignored: + ignored_head = head + assert json.loads(push(head, branch=branch, expected=422))["code"] == "ignored-files" + assert "push" not in git_commands.read_text().splitlines() + assert subprocess.run(["git", "--git-dir", str(published), "show-ref", + "--verify", "--quiet", "refs/heads/" + branch]).returncode == 1 + else: + try: + assert push(head, branch=branch)["status"] == "complete" + except AssertionError as error: + raise AssertionError(f"ignore case {number}: {files!r}") from error + assert remote_head(branch) == head + assert (odb / "index").read_bytes() == b"server index must remain untouched" + assert not list((odb / "caos-pushes").glob("*.check")) + # Rejected updates leave an existing branch and exact lease intact. + assert push(first[0], branch="ignore-update")["status"] == "complete" + assert json.loads(push(ignored_head, branch="ignore-update", old=first[0], + expected=422))["code"] == "ignored-files" + assert remote_head("ignore-update") == first[0] + if cli: + rejected = json.loads(run(cli, "push-git", destination, ignored_head, "ignore-cli", + "--expected=absent", env=cli_env)) + assert rejected["kind"] == "validation-rejected" + assert ".gitignore" in rejected["diagnostic"] + assert "no push was attempted" in rejected["diagnostic"] + # This is a tip-tree policy, not a history scrub. + cleaned = ignore_snapshot({"ordinary.txt": "kept"}, parent=ignored_head) + assert push(cleaned, branch="ignore-cleaned")["status"] == "complete" + (odb / "index").unlink() + (odb / "info/exclude").unlink() + run("git", "--git-dir", str(odb), "config", "--unset", "core.excludesFile") empty = run("git", "--git-dir", str(origin), "mktree", input=b"") reserved = run("git", "--git-dir", str(origin), "mktree", input=f"040000 tree {empty}\t.caos\n".encode()) assert push(source_commit(reserved), branch="reserved")["status"] == "complete" @@ -555,7 +635,7 @@ def source_commit(tree, parent=None): marked = source_commit(marked_tree, first[0]) assert push(marked, branch="markers")["status"] == "complete" assert remote_head("markers") == marked - # Source policy belongs to the agent; the endpoint transfers exact commits. + # Conflict resolution belongs to the agent; the endpoint transfers exact commits. clean = source_commit(first[1], marked) assert push(clean, branch="resolved")["status"] == "complete" genesis_env = dict(env, GIT_AUTHOR_NAME="caos", GIT_AUTHOR_EMAIL="caos@caos",