From 81dfb6f0722c7cd9b61ae066fee65869a9917bbd Mon Sep 17 00:00:00 2001 From: Vignesh Date: Sat, 31 Oct 2026 23:46:03 +0530 Subject: [PATCH 1/2] session: a worker may not replace the git metadata of the folder it was given (#1351) A codeaf do --dir worker ran `rm -rf /.git`, then `git init` and `git commit` in the parent, and the repository's history was gone. The folder guard already refused the parent half, because the parent is outside the worker's ground. It allowed the deletion, because a repository's .git sits inside the tree it belongs to, so the path law read `rm -rf .git` as ordinary work in the folder. The absolute spelling was only stopped by accident, by the critical-command floor's `rm -rf /*`. The guard now refuses any command that removes, moves, overwrites or rewrites the ground's own .git, or removes the ground itself, with the boundary's existing `is outside your copy` sentence. It asks the same hands the path law reads (the file-writing commands, in-place sed/perl and redirections), on both the written and the symlink-resolved spelling, so a link to the ground or to its .git is not a way around it. Git's own verbs are untouched: git commit, checkout and reset write inside .git on the task's behalf and are answered by the git guard as before. A plain folder with no .git is unchanged. The redirection check moves into its own function to keep outsideScopeAim under the endings ceiling. Tests: unit cases for every refused spelling (including symlinks and walking back out of a subdirectory) and for the work that must still pass, and two end-to-end tests through the real headless door with a scripted worker sending the three recorded commands in a nested, non-scratch repository. Each new test fails with the fix removed. The manual pages the issue names now quote the refusal, with a retrieval test. --- cmd/codeaf/do_git_metadata_test.go | 205 ++++++++++++++++++++++ internal/manual/chat/choosing-a-folder.md | 11 ++ internal/manual/chat/how-tasks-run.md | 27 +++ internal/manual/chat/permissions.md | 17 ++ internal/manual/chat_test.go | 49 ++++++ internal/session/taskoutside.go | 123 ++++++++++++- internal/session/taskoutside_test.go | 141 +++++++++++++++ 7 files changed, 566 insertions(+), 7 deletions(-) create mode 100644 cmd/codeaf/do_git_metadata_test.go diff --git a/cmd/codeaf/do_git_metadata_test.go b/cmd/codeaf/do_git_metadata_test.go new file mode 100644 index 0000000000..4cd6a75d95 --- /dev/null +++ b/cmd/codeaf/do_git_metadata_test.go @@ -0,0 +1,205 @@ +package main + +// #1351: A WORKER MAY NOT REPLACE THE GIT METADATA OF THE FOLDER IT WAS GIVEN. +// +// On staging 17ae56d34 a `codeaf do --dir ` worker ran, in order: +// +// rm -rf /.git +// cd && git init +// git commit +// +// The repository's history was gone and the parent folder had become a +// repository of its own. The folder guard (internal/session's taskoutside.go) +// already refused the `git init` and `git commit` aimed at the parent, because +// the parent is outside the worker's ground. It allowed the first command, +// because /.git is INSIDE the ground. That one command is the whole of the +// data loss: once the history is deleted there is nothing the later two need to +// do to finish it. +// +// So these tests drive the real headless door with a scripted model that sends +// exactly those three commands, in a repository nested inside a parent that is +// NOT scratch (a run's --dir is never under the temp directory, and the scratch +// exemption would otherwise let the parent through), and assert what the issue +// asks for: the repository's .git survives, the parent never becomes a +// repository, and every attempt is refused with the boundary's own words. + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/session" +) + +// nestedRunRepo is a committed repository nested one level inside a parent +// folder, both standing OUTSIDE the machine's scratch directories, the way a +// person's `--dir` stands. +// +// It is placed under the module root rather than t.TempDir because the folder +// guard treats the temp directory as scratch a task may write to (it is where +// a shell command keeps what it reads back), and a parent there would be let +// through by that exemption rather than tested by the boundary. +func nestedRunRepo(t *testing.T) (parent, repo string) { + t.Helper() + root, err := os.MkdirTemp(".", "issue1351-") + if err != nil { + t.Fatal(err) + } + root, err = filepath.Abs(root) + if err != nil { + t.Fatal(err) + } + if root, err = filepath.EvalSymlinks(root); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(root) }) + parent = filepath.Join(root, "parent") + repo = filepath.Join(parent, "repo") + if err := os.MkdirAll(repo, 0o755); err != nil { + t.Fatal(err) + } + beltGit(t, repo, "init") + beltGit(t, repo, "checkout", "-b", "work") + if err := os.WriteFile(filepath.Join(repo, "README.md"), []byte("the project\n"), 0o644); err != nil { + t.Fatal(err) + } + beltGit(t, repo, "add", "-A") + beltGit(t, repo, "-c", "user.name=t", "-c", "user.email=t@t", "commit", "-m", "first") + return parent, repo +} + +// recordedReplacementSeat is the scripted worker from the issue: it tries the +// three recorded commands, one per turn, and then finishes its task so the run +// ends rather than walling. Every tool result it is handed back is kept so the +// test can read the boundary's own words. +type recordedReplacementSeat struct { + commands []string + seen int + results []string +} + +func (s *recordedReplacementSeat) CompleteWithMessages(_ context.Context, messages []ai.Message, _ ...ai.Option) (*ai.Response, error) { + if s.seen > 0 { + s.results = append(s.results, lastToolResult(messages)) + } + if doc := beltDocument(messages); strings.Contains(doc, "## Who checks this work") { + id := briefTaskID(doc) + return beltToolReply("plandb done " + id + " --agent " + id + " --result 'holds: the acceptance is met'"), nil + } + if s.seen < len(s.commands) { + command := s.commands[s.seen] + s.seen++ + return beltToolReply(command), nil + } + s.seen++ + return beltToolReply(beltFinish(beltAnswer)), nil +} + +// lastToolResult is the text of the newest tool message: what the belt said +// back about the command the worker just sent. +func lastToolResult(messages []ai.Message) string { + for index := len(messages) - 1; index >= 0; index-- { + if messages[index].Role != "tool" { + continue + } + var b strings.Builder + for _, part := range messages[index].Content { + b.WriteString(part.Text) + } + return b.String() + } + return "" +} + +func TestDoOnTheRunEngineRefusesReplacingTheFoldersGitMetadata(t *testing.T) { + beltRunEnv(t) + t.Setenv("CODEAF_PLANDB_BIN", beltPlandbDoor(t)) + parent, repo := nestedRunRepo(t) + head := doGitIn(t, repo, "rev-parse", "HEAD") + + // THE DELETE IS SPELLED RELATIVE, AS A WORKER STANDING IN ITS FOLDER SPELLS + // IT. The absolute spelling (`rm -rf /.git`) is already stopped, but + // only by accident: the critical-command floor (internal/approval) prompts on + // every absolute recursive delete, so it matches `/*` rather than knowing what + // .git is. `rm -rf .git` matches nothing there and nothing in the folder guard, + // because .git is inside the worker's own ground, which is the gap this + // issue is about. + seat := &recordedReplacementSeat{commands: []string{ + "rm -rf .git", + "cd " + parent + " && git init", + "cd " + parent + " && git -c user.name=t -c user.email=t@t commit --allow-empty -m 'Add add(a,b) and one pytest test'", + }} + var stdout, stderr strings.Builder + _ = doErrand(doRequest{ + task: "create add.py and its test", workspace: repo, asJSON: true, + timeout: 60 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr, + newBeltCompleter: func(string) session.Completer { return seat }, + }) + + if info, err := os.Stat(filepath.Join(repo, ".git")); err != nil || !info.IsDir() { + t.Fatalf("the repository's .git is gone after the run (err %v): the worker replaced the folder's history", err) + } + if now := doGitIn(t, repo, "rev-parse", "HEAD"); now != head { + t.Fatalf("the repository's history moved %s -> %s", head, now) + } + if _, err := os.Stat(filepath.Join(parent, ".git")); err == nil { + t.Fatal("the parent folder became a repository: git metadata was created outside the folder the run was given") + } + if len(seat.results) < 3 { + t.Fatalf("the worker saw %d answers to its three commands, want three\nstdout:\n%s\nstderr:\n%s", + len(seat.results), stdout.String(), stderr.String()) + } + for index, said := range seat.results[:3] { + if !strings.Contains(said, "is outside your copy") { + t.Errorf("command %d (%q) was answered %q, want the boundary's refusal", index+1, seat.commands[index], said) + } + } +} + +// AND THE WORK THE FOLDER WAS GIVEN FOR STILL RUNS. A guard that kept .git +// safe by refusing everything would protect nothing anybody wanted done: the +// same door, in the same nested repository, still writes add.py and its test +// inside the given folder, and the parent is left exactly as it was. +func TestDoOnTheRunEngineStillWritesInsideTheFolderItWasGiven(t *testing.T) { + beltRunEnv(t) + t.Setenv("CODEAF_PLANDB_BIN", beltPlandbDoor(t)) + parent, repo := nestedRunRepo(t) + before, err := os.ReadDir(parent) + if err != nil { + t.Fatal(err) + } + + seat := &recordedReplacementSeat{commands: []string{ + "printf 'def add(a, b):\\n return a + b\\n' > add.py", + "printf 'from add import add\\n\\ndef test_add():\\n assert add(1, 2) == 3\\n' > test_add.py", + }} + var stdout, stderr strings.Builder + if err := doErrand(doRequest{ + task: "create add.py and its test", workspace: repo, asJSON: true, + timeout: 60 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr, + newBeltCompleter: func(string) session.Completer { return seat }, + }); err != nil { + t.Fatalf("an ordinary run in its own folder left with %v\nstdout:\n%s\nstderr:\n%s", err, stdout.String(), stderr.String()) + } + for _, name := range []string{"add.py", "test_add.py"} { + if _, err := os.Stat(filepath.Join(repo, name)); err != nil { + t.Fatalf("%s was not written inside the given folder: %v", name, err) + } + } + for _, said := range seat.results { + if strings.Contains(said, "is outside your copy") { + t.Fatalf("an ordinary write in the given folder was refused: %q", said) + } + } + after, err := os.ReadDir(parent) + if err != nil { + t.Fatal(err) + } + if len(after) != len(before) { + t.Fatalf("the parent folder changed: %d entries before, %d after", len(before), len(after)) + } +} diff --git a/internal/manual/chat/choosing-a-folder.md b/internal/manual/chat/choosing-a-folder.md index 25186042f2..72db15f185 100644 --- a/internal/manual/chat/choosing-a-folder.md +++ b/internal/manual/chat/choosing-a-folder.md @@ -572,6 +572,17 @@ change it there". That is remembered for that one folder, and from then on write it change the folder itself immediately, exactly like the folder you are standing in. Nothing guesses this; it is only ever set because you said it. +## Can work in a folder delete its .git? No, the history of the folder a task is given is kept + +Whichever way a task stands in a folder you chose, in its own copy or in the folder +directly, it may not remove, move, overwrite or rewrite that folder's `.git`. That holds +for `codeaf do --dir` too: the folder you hand it is the one whose history is kept. A task +that tries `rm -rf .git`, `mv .git …`, `echo … > .git/HEAD` or `rm -rf .` reads the +same refusal every write outside its folder gets, naming the `.git` and the word +`is outside your copy`, and carries on with its work. Ordinary git work in the folder, such +as `git commit` or `git checkout`, is untouched, and so is reading `.git`. permissions has +the exact wording; how-tasks-run has the full list. + ## Where did my changes go — the folder still looks unchanged If the model edited files in a folder you chose with `/folder` and that folder still looks diff --git a/internal/manual/chat/how-tasks-run.md b/internal/manual/chat/how-tasks-run.md index 55f775694d..cd50363cd4 100644 --- a/internal/manual/chat/how-tasks-run.md +++ b/internal/manual/chat/how-tasks-run.md @@ -882,6 +882,33 @@ The wording it reads names **both the path and the directory it may write in**: **The machine's scratch is not yours.** `/tmp`, the temp directory and `/dev/null` are written freely — that is where a command line puts what it is about to read back. +**A task may not replace its own copy's git history either.** The table above is about +writing somewhere else; this is the one thing inside the task's own copy that is refused. +A command that removes, moves, overwrites or rewrites the copy's `.git` is turned back with +the same sentence, naming that `.git`: + +| Refused inside its own copy | What it looks like | +| --- | --- | +| deleting the history | `rm -rf .git`, `rm -rf /.git`, `rm .git/HEAD`, `rm -rf .git/objects` | +| moving or replacing it | `mv .git ../kept`, `cp -r other/.git .git`, `rsync --delete … .git/` | +| rewriting it in place | `sed -i … .git/HEAD`, `echo … > .git/HEAD` | +| removing the copy itself | `rm -rf .`, `rm -rf ` | + +> replacing /Users/you/code/project/.git is outside your copy — this task works in +> /Users/you/code/project; read anywhere, write only there. … + +Everything a task does with git stays allowed: `git commit`, `git checkout`, `git reset` +and every other git command write inside `.git` on the task's behalf and are answered by +the git rules as before. So are reading it (`cat .git/HEAD`, `git log`), editing a +`.gitignore`, and every ordinary write in the copy. A folder that is not a repository has no +history to protect, so nothing changes there. This holds for `codeaf do --dir` as much as +for a chat task: the folder you hand it is the copy. + +It exists because a `codeaf do --dir ` worker once ran `rm -rf /.git`, then +`git init` and `git commit` in the folder above. The last two were already refused, since +the folder above is outside the copy. But `.git` sits inside the copy, so deleting it passed +as ordinary work and the repository's history was gone. + **What it cannot see.** It reads the paths a command *names*. `cd elsewhere && python fix.py`, where the script writes what it likes, is not caught, and neither is a path built out of a shell variable. What it does close is every shape that says its target out loud. diff --git a/internal/manual/chat/permissions.md b/internal/manual/chat/permissions.md index 8d7b4bf6a4..eb321bebc8 100644 --- a/internal/manual/chat/permissions.md +++ b/internal/manual/chat/permissions.md @@ -800,6 +800,23 @@ tool**. `gmail_send:allow` in `tools.approval` runs it silently. So does a capability set to `yes`. So does a remembered "always" answer to this approval question. Nothing else lifts it. +## Can a task delete my repository's .git or its history? No, it is refused in its own copy + +The dangerous-command floor above asks before an absolute recursive delete, but it would not +notice `rm -rf .git`, written relative to the folder the task stands in. So a task, and a +`codeaf do --dir` worker, is separately refused any command that removes, moves, overwrites +or rewrites the `.git` of the folder it was given: `rm -rf .git`, `mv .git …`, +`cp -r … .git`, `echo … > .git/HEAD`, or removing the folder itself with `rm -rf .`. The +refusal is the same one every write outside the task's folder gets: + +> replacing /Users/you/code/project/.git is outside your copy — this task works in +> /Users/you/code/project; read anywhere, write only there. … + +This is not a question and no setting lifts it, because a task has nobody to ask. Ordinary +git work is untouched: `git commit`, `git checkout` and the rest write inside `.git` on the +task's behalf and run as before, and reading `.git` is reading. A conversation you are +sitting in is not held by it. The full list is on how-tasks-run. + ## Neither floor can be swallowed by "stop asking me" Before any session memo is consulted, codeaf asks whether the call is one of the diff --git a/internal/manual/chat_test.go b/internal/manual/chat_test.go index 8789e7587a..88ce2331d6 100644 --- a/internal/manual/chat_test.go +++ b/internal/manual/chat_test.go @@ -3479,6 +3479,55 @@ func TestTheChatManualAnswersTheQuestionsPeopleAsk(t *testing.T) { } } +// #1351: a worker given a repository deleted its `.git` and the folder guard let +// it, because `.git` is inside the folder it was given. The guard now refuses +// replacing the folder's own git metadata, and the questions a person asks +// after seeing that, or before trusting `--dir`, must reach the sections that +// say so: in their words, with the refusal quoted and the shapes named. +func TestTheGitMetadataQuestionsReachTheSectionsThatRefuseIt(t *testing.T) { + asked := []struct { + question string + page string + title string + says []string + }{ + { + question: "can a task delete my repository's .git", + page: "permissions", + title: "Can a task delete my repository's .git or its history", + says: []string{"is outside your copy", "rm -rf .git", "codeaf do --dir", "git commit"}, + }, + { + question: "can work in a folder delete its .git history", + page: "choosing-a-folder", + title: "Can work in a folder delete its .git", + says: []string{"is outside your copy", "codeaf do --dir", "rm -rf ."}, + }, + } + for _, ask := range asked { + reached := false + for _, section := range Chat().Search(ask.question, DefaultResults) { + if section.Page != ask.page || !strings.Contains(section.Title, ask.title) { + continue + } + reached = true + for _, needle := range ask.says { + if !strings.Contains(section.Body, needle) { + t.Errorf("%q reached %q but its body omits %q", ask.question, section.Title, needle) + } + } + } + if !reached { + found := Chat().Search(ask.question, DefaultResults) + where := make([]string, 0, len(found)) + for _, section := range found { + where = append(where, section.Page+" · "+section.Title) + } + t.Errorf("%q does not reach %s · %q; it reached %v", ask.question, ask.page, ask.title, where) + } + } +} + // #1469: home listed waiting standing orders without their due time, and a person // asking when one goes off there had to be sent to the standing place for it. // Standing orders are automations now (automations.md), and home's `automations` diff --git a/internal/session/taskoutside.go b/internal/session/taskoutside.go index e977318ce9..14b95e8ecd 100644 --- a/internal/session/taskoutside.go +++ b/internal/session/taskoutside.go @@ -474,13 +474,8 @@ func outsideScopeAim(ground, command string, allowed func(string) bool) string { cwd := ground for _, segment := range taskSegments(command) { words, assignments := taskSegmentHead(stripRedirections(segment)) - for index, word := range segment { - if index+1 < len(segment) && fileWriteRedirection(word, segment[index+1]) { - target := resolvePath(cwd, segment[index+1]) - if !allowed(target) { - return "writing to " + target - } - } + if aimed := redirectionAimedOutside(ground, cwd, segment, allowed); aimed != "" { + return aimed } if len(words) == 0 { continue @@ -497,6 +492,9 @@ func outsideScopeAim(ground, command string, allowed func(string) bool) string { } return "git " + verb + " in " + dir default: + if aimed := replacedGroundMetadata(ground, cwd, program, rest); aimed != "" { + return aimed + } if aimed := writeAimedOutsideScope(cwd, program, rest, allowed); aimed != "" { return aimed } @@ -505,6 +503,117 @@ func outsideScopeAim(ground, command string, allowed func(string) bool) string { return "" } +// redirectionAimedOutside answers the first file a segment's redirection would +// write that the task may not write, and "" when every one of them is the task's. +// +// A REDIRECTION INTO THE GROUND'S OWN `.git` IS THE METADATA LAW'S, NOT THE PATH +// LAW'S: `echo ref: x > .git/HEAD` is inside the ground and would pass a check +// that only asked where it was aimed, so it is answered first, in the same +// sentence [replacedGroundMetadata] gives the hands that name `.git`. +func redirectionAimedOutside(ground, cwd string, segment []string, allowed func(string) bool) string { + for index, word := range segment { + if index+1 >= len(segment) || !fileWriteRedirection(word, segment[index+1]) { + continue + } + target := resolvePath(cwd, segment[index+1]) + if metadata := groundMetadataDir(ground); metadata != "" && namesGroundMetadata(metadata, target, false) { + return "replacing " + metadata + } + if !allowed(target) { + return "writing to " + target + } + } + return "" +} + +// replacedGroundMetadata answers the ground's own `.git` when a hand would remove, +// move, overwrite or rewrite it, and "" when it would not (#1351). +// +// THE PATH LAW CANNOT SEE THIS ON ITS OWN, and that is the whole defect. It +// asks whether a path is inside the folder the task may write in, and a +// repository's `.git` always is, because it sits inside the tree it belongs to. +// So a worker that ran `rm -rf .git` on the folder it was given passed every +// check, and the history was gone, which is the data loss; the `git init` and +// `git commit` in the parent that followed were already refused, but by then +// they had nothing left to save. +// +// IT IS ASKED OF THE SAME HANDS THE PATH LAW READS ([writesEveryOperand], +// [writesItsLastOperand], an in-place sed or perl, and a redirection, which +// [outsideScopeAim] asks beside it), so it closes exactly the shapes that name +// `.git` and claims nothing more: a script that deletes it by itself is the +// blind spot this file already says it has. Git's own verbs are not hands here: +// `git commit` and `git checkout` write inside `.git` on the task's behalf, and +// that is the work, so they are answered by the git guard as before. +// +// AN OPERAND IS THE METADATA WHEN IT IS `.git` OR UNDER IT, OR WHEN IT HOLDS +// IT. Removing the ground itself (`rm -rf .`, `rm -rf `) takes `.git` +// with it, so it is refused by the same sentence; a file elsewhere in the +// ground is untouched by this and stays the task's to change. +func replacedGroundMetadata(ground, cwd, program string, rest []string) string { + metadata := groundMetadataDir(ground) + if metadata == "" { + return "" + } + switch { + case program == "sed" || program == "perl": + if !inPlaceEdit(rest) { + return "" + } + case writesEveryOperand[program] || writesItsLastOperand[program]: + default: + return "" + } + operands := commandOperands(rest) + if writesItsLastOperand[program] && len(operands) > 1 { + operands = operands[len(operands)-1:] + } + for _, operand := range operands { + if namesGroundMetadata(metadata, resolvePath(cwd, operand), true) { + return "replacing " + metadata + } + } + return "" +} + +// namesGroundMetadata answers whether a path a command names is the ground's +// `.git` or something under it, and, when holding counts, a directory that holds +// it (removing the ground itself takes its history too). +// +// IT IS ASKED ON BOTH SPELLINGS, the one written and the real one. A symlink to +// the ground, or to `.git` itself, is the same history under another name, and +// [insideGround] resolves symlinks for exactly this reason; a law that compared +// only the written path would be one more spelling the model learns to use. +func namesGroundMetadata(metadata, target string, holding bool) bool { + real := resolveSymlinks(metadata) + for _, path := range [2]string{target, resolveSymlinks(target)} { + for _, dir := range [2]string{metadata, real} { + if withinDir(dir, path) || (holding && withinDir(path, dir)) { + return true + } + } + } + return false +} + +// groundMetadataDir is the `.git` that makes the ground a repository, spelled the +// way [resolvePath] spells a command's operands so the two compare. It is "" for +// a ground that is not one, which leaves a plain folder's work untouched. +// +// A LINKED WORKTREE'S `.git` IS A FILE AND IS STILL ITS METADATA: it is the +// pointer that ties the copy to its repository, and removing it is the same +// loss of history from the copy's side. +func groundMetadataDir(ground string) string { + ground = strings.TrimSpace(ground) + if ground == "" { + return "" + } + metadata := filepath.Join(filepath.Clean(ground), ".git") + if _, err := os.Lstat(metadata); err != nil { + return "" + } + return metadata +} + // insideGround is the one comparison the whole law rests on. // // THE MACHINE'S SCRATCH IS NOT SOMEBODY'S WORK. /tmp, the process's own temp diff --git a/internal/session/taskoutside_test.go b/internal/session/taskoutside_test.go index 708c2dd930..02235f79b9 100644 --- a/internal/session/taskoutside_test.go +++ b/internal/session/taskoutside_test.go @@ -239,3 +239,144 @@ func TestAWorkerIsRefusedOnTheSeamForWritingOutsideItsGround(t *testing.T) { t.Fatal("the guard refused a person's own session; it answers for work inside a task and nothing else") } } + +// #1351: THE GROUND'S OWN GIT METADATA IS NOT THE GROUND'S TO REPLACE. +// +// A worker given a repository ran `rm -rf /.git`, then `git init` and +// `git commit` in the parent. The parent half was already refused because the +// parent is outside the ground. The deletion was not, because a repository's +// `.git` sits inside the tree it belongs to, so the path law read it as ordinary +// work in the folder the worker was given. Every spelling of that act is refused +// here with the boundary's own words, and the ground keeps its history. +func TestATaskMayNotReplaceItsGroundsGitMetadata(t *testing.T) { + ground := groundWithMetadata(t) + parent := filepath.Dir(ground) + refused := []string{ + // The recorded command, and the spelling a worker standing in the + // folder uses. + "rm -rf " + filepath.Join(ground, ".git"), + "rm -rf .git", + "rm -r .git", + "rm -fr ./.git", + "rm -rf .git/", + "cd " + ground + " && rm -rf .git", + // Taking the history apart a piece at a time is the same loss. + "rm -rf .git/objects", + "rm .git/HEAD", + // Moving it out of the way, or overwriting it with another one. + "mv .git ../kept-history", + "cp -r /tmp/other/.git .git", + "rsync -a --delete /tmp/empty/ .git/", + // Rewriting it in place. + "sed -i 's/main/x/' .git/HEAD", + "echo 'ref: refs/heads/x' > .git/HEAD", + // Removing the ground itself takes its history with it. + "rm -rf .", + "rm -rf " + ground, + // And the parent half the issue recorded stays refused. + "cd " + parent + " && git init", + "cd .. && git init", + "git -C " + parent + " init", + // Walking down into the tree and back out is still the ground's .git. + "cd sub && rm -rf ../.git", + } + if err := os.MkdirAll(filepath.Join(ground, "sub"), 0o755); err != nil { + t.Fatal(err) + } + // A SYMLINK IS THE SAME HISTORY UNDER ANOTHER NAME. A link to the ground, or + // to its .git, must not be the spelling that gets through. + linkedGround := filepath.Join(filepath.Dir(parent), "linked-ground") + linkedMetadata := filepath.Join(filepath.Dir(parent), "linked-metadata") + if err := os.Symlink(ground, linkedGround); err == nil { + refused = append(refused, + "rm -rf "+filepath.Join(linkedGround, ".git"), + "cd "+linkedGround+" && rm -rf .git") + } + if err := os.Symlink(filepath.Join(ground, ".git"), linkedMetadata); err == nil { + refused = append(refused, + "rm -rf "+linkedMetadata, + "rm -rf "+filepath.Join(linkedMetadata, "objects")) + } + for _, command := range refused { + why := refusedOutsideGround(ground, command) + if why == "" { + t.Errorf("%q was allowed, want the ground's git metadata protected", command) + continue + } + if !strings.Contains(why, outsideYourCopy) { + t.Errorf("%q was refused with %q, want the boundary's own words %q", command, why, outsideYourCopy) + } + } +} + +// AND THE WORK THE GROUND IS FOR STILL RUNS. Protecting .git by refusing the +// folder would protect nothing anybody wanted done, so ordinary writes, git's +// own verbs (which write inside .git on the task's behalf), a .gitignore beside +// it, and reading .git all pass exactly as before. +func TestATaskStillWorksInAGroundWhoseMetadataIsProtected(t *testing.T) { + ground := groundWithMetadata(t) + allowed := []string{ + "printf 'def add(a, b):\n return a + b\n' > add.py", + "mkdir -p tests && touch tests/__init__.py", + "rm -rf build dist", + "mv old.py new.py", + "cp template.py add.py", + "sed -i 's/a/b/' add.py", + "echo node_modules/ >> .gitignore", + "rm .gitignore", + "git add -A", + "git -c user.name=t -c user.email=t@t commit -m 'add add'", + "git checkout -b fix/add", + "git reset --hard HEAD", + "cat .git/HEAD", + "ls .git", + "git log --oneline", + // Names that only begin with .git are not the history. + "rm -rf .github", + "rm .gitmodules", + "rm -rf .git.bak", + // And a nested .git below the ground belongs to nothing this law guards. + "rm -rf vendor/lib/.git", + } + for _, command := range allowed { + if why := refusedOutsideGround(ground, command); why != "" { + t.Errorf("%q was refused with %q, want ordinary work in the ground allowed", command, why) + } + } +} + +// A GROUND THAT IS NOT A REPOSITORY HAS NO HISTORY TO PROTECT, so a plain +// folder's work, including a file a person happens to name `.git`-free, is left +// exactly as the path law always left it. +func TestAPlainFolderGroundIsUnchangedByTheMetadataLaw(t *testing.T) { + ground := t.TempDir() + for _, command := range []string{"rm -rf build", "rm -rf .", "printf x > notes.md"} { + if why := refusedOutsideGround(ground, command); why != "" { + t.Errorf("%q in a plain folder was refused with %q, want it allowed", command, why) + } + } +} + +// groundWithMetadata is a repository-shaped ground standing OUTSIDE the +// machine's scratch, the way a person's `--dir` stands. The scratch exemption in +// [insideGround] would otherwise let its parent through, which would hide the +// parent half of the law this file checks. +func groundWithMetadata(t *testing.T) string { + t.Helper() + root, err := os.MkdirTemp(".", "ground-metadata-") + if err != nil { + t.Fatal(err) + } + if root, err = filepath.Abs(root); err != nil { + t.Fatal(err) + } + if root, err = filepath.EvalSymlinks(root); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.RemoveAll(root) }) + ground := filepath.Join(root, "parent", "repo") + if err := os.MkdirAll(filepath.Join(ground, ".git"), 0o755); err != nil { + t.Fatal(err) + } + return ground +} From 1790915904c5dbc574119f994ced94c1f7ef0571 Mon Sep 17 00:00:00 2001 From: Vignesh Date: Sat, 31 Oct 2026 23:47:47 +0530 Subject: [PATCH 2/2] docs: change entry for #2097 (worker keeps the folder's git metadata) --- .../2097-worker-keeps-folder-git-metadata.md | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 docs/changes/unreleased/2097-worker-keeps-folder-git-metadata.md diff --git a/docs/changes/unreleased/2097-worker-keeps-folder-git-metadata.md b/docs/changes/unreleased/2097-worker-keeps-folder-git-metadata.md new file mode 100644 index 0000000000..6642485d20 --- /dev/null +++ b/docs/changes/unreleased/2097-worker-keeps-folder-git-metadata.md @@ -0,0 +1,11 @@ +--- +kind: fixed +title: a task or codeaf do worker can no longer delete the git history of the folder it was given +pr: 2097 +surface: [engine, docs] +invalidates: + - "`codeaf do --dir ` and every task kept their worker inside the folder they were given, but `.git` sits inside that folder, so the folder guard read `rm -rf .git` as ordinary work there and let it run. A worker that ran it deleted the repository's history; the `git init` and `git commit` in the parent that followed were already refused, but by then nothing was left to save. `--dir` alone did not protect a repository's history. The absolute spelling `rm -rf /.git` was stopped only by the critical-command floor's `rm -rf /*` pattern, which asks about every absolute recursive delete and knows nothing about `.git`. Now any command that removes, moves, overwrites or rewrites the given folder's own `.git`, or removes the folder itself, is refused with the boundary's `is outside your copy` sentence, through a symlink as well. Git's own commands, reading `.git`, and every other write in the folder are unchanged, and a folder that is not a repository is unaffected." +--- + +Found once on staging 17ae56d34 and not reproduced in three reruns, so this +closes the boundary rather than a path that always fires.