diff options
| -rw-r--r-- | internal/apply/apply.go | 20 | ||||
| -rw-r--r-- | internal/apply/swap_test.go | 101 |
2 files changed, 117 insertions, 4 deletions
diff --git a/internal/apply/apply.go b/internal/apply/apply.go index f177a17..713913e 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -77,17 +77,29 @@ func Chain(c plan.Chain) []StepResult { } // checkUnchanged is the guard that matters most: before every step, the -// source is stat'd and compared against the size and mtime the plan -// recorded for the whole file. A file rewritten or replaced between -// planning and applying must never be acted on. +// source is Lstat'd and compared against what the plan recorded for the +// whole file. A file rewritten or replaced between planning and applying +// must never be acted on (spec ยง15.1): its size and mtime must match, it +// must still be a regular file - not a symlink put in its place - and, at +// its planned path, it must be the same inode. The inode is not compared +// once an earlier step has moved the file: a move across filesystems +// copies it to a new inode, and the chain is still following its own file. func checkUnchanged(src string, f scan.File) error { - fi, err := os.Stat(src) + fi, err := os.Lstat(src) if err != nil { return fmt.Errorf("changed since plan: %w", err) } + if !fi.Mode().IsRegular() { + return errors.New("changed since plan: no longer a regular file") + } if fi.Size() != f.Size || !fi.ModTime().Equal(f.ModTime) { return errors.New("changed since plan") } + if src == f.Path && f.Ino != 0 { + if now := scan.NewFile(src, f.Rel, fi); now.Dev != f.Dev || now.Ino != f.Ino { + return errors.New("changed since plan: another file is in its place") + } + } return nil } diff --git a/internal/apply/swap_test.go b/internal/apply/swap_test.go new file mode 100644 index 0000000..fe7542e --- /dev/null +++ b/internal/apply/swap_test.go @@ -0,0 +1,101 @@ +// SPDX-License-Identifier: GPL-3.0-or-later + +package apply + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "krino/internal/plan" + "krino/internal/scan" +) + +// planned writes body to root/rel and returns the chain a plan would build +// for it, identity included, with steps. +func planned(t *testing.T, root, rel, body string, steps ...plan.Step) plan.Chain { + t.Helper() + p := filepath.Join(root, rel) + if err := os.WriteFile(p, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + info, err := os.Lstat(p) + if err != nil { + t.Fatal(err) + } + return plan.Chain{File: scan.NewFile(p, rel, info), Steps: steps} +} + +// TestChainRefusesSourceSwappedForSymlink: a file replaced by a symlink +// between plan and apply - even to a file of the same size and modification +// time - is not acted on, so a copy never reads through the link. +func TestChainRefusesSourceSwappedForSymlink(t *testing.T) { + root := t.TempDir() + p := filepath.Join(root, "a.txt") + out := filepath.Join(root, "Out", "a.txt") + c := planned(t, root, "a.txt", "12345", plan.Step{Kind: plan.Copy, Src: p, Dst: out}) + target := filepath.Join(root, "secret.txt") + if err := os.WriteFile(target, []byte("54321"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(target, c.File.ModTime, c.File.ModTime); err != nil { + t.Fatal(err) + } + if err := os.Remove(p); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, p); err != nil { + t.Fatal(err) + } + res := Chain(c) + if res[0].Status != "failed" || !strings.HasPrefix(res[0].Detail, "changed since plan") { + t.Fatalf("step = %s %q, want failed: changed since plan", res[0].Status, res[0].Detail) + } + if _, err := os.Lstat(out); !os.IsNotExist(err) { + t.Errorf("the copy was made through the symlink: %v", err) + } +} + +// TestChainRefusesSourceReplacedByAnotherFile: another file renamed into +// the planned path, with the same size and modification time, has another +// inode: it is not the file that was planned. +func TestChainRefusesSourceReplacedByAnotherFile(t *testing.T) { + root := t.TempDir() + p := filepath.Join(root, "a.txt") + c := planned(t, root, "a.txt", "12345", plan.Step{Kind: plan.Move, Src: p, Dst: filepath.Join(root, "Out", "a.txt")}) + other := filepath.Join(root, "other.txt") + if err := os.WriteFile(other, []byte("54321"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Chtimes(other, c.File.ModTime, c.File.ModTime); err != nil { + t.Fatal(err) + } + if err := os.Rename(other, p); err != nil { + t.Fatal(err) + } + res := Chain(c) + if res[0].Status != "failed" || !strings.HasPrefix(res[0].Detail, "changed since plan") { + t.Fatalf("step = %s %q, want failed: changed since plan", res[0].Status, res[0].Detail) + } + if b, err := os.ReadFile(p); err != nil || string(b) != "54321" { + t.Errorf("the replacement was moved: %q, %v", b, err) + } +} + +// TestChainFollowsItsOwnFile: the file's identity check does not stop a +// chain that renames and then moves the planned file itself. +func TestChainFollowsItsOwnFile(t *testing.T) { + root := t.TempDir() + p := filepath.Join(root, "a.txt") + b := filepath.Join(root, "b.txt") + c := planned(t, root, "a.txt", "12345", + plan.Step{Kind: plan.Rename, Src: p, Dst: b}, + plan.Step{Kind: plan.Move, Src: b, Dst: filepath.Join(root, "Out", "b.txt")}, + ) + for i, r := range Chain(c) { + if r.Status != "ok" { + t.Errorf("step %d: %s %q", i+1, r.Status, r.Detail) + } + } +} |