diff --git a/internal/apply/archive.go b/internal/apply/archive.go index 490891a..526e0bb 100644 --- a/internal/apply/archive.go +++ b/internal/apply/archive.go @@ -66,16 +66,10 @@ func applyArchive(ctx context.Context, r *declaration.Archive, previous store.Ap } } - if err := os.MkdirAll(r.Path, 0o755); err != nil { - return out, err - } - written, err := unpack(body, r.Path) + written, err := replaceWith(body, r.Path, r.Owner) if err != nil { return out, err } - if err := ownAll(r.Path, r.Owner); err != nil { - return out, err - } out.Action = "updated" if previous.Wrote == "" { out.Action = "created" @@ -84,6 +78,63 @@ func applyArchive(ctx context.Context, r *declaration.Archive, previous store.Ap return out, nil } +// replaceWith makes the directory exactly the archive (novox/hq issue 220). +// +// **The tree on disk is the archive and nothing else.** The digest is the whole identity of what +// is unpacked here, so a file the previous archive had and this one does not must go. Unpacked over +// the old tree, it stayed: a bundle rebuilt as one file per entrypoint kept the package directory +// of the version before, which code could still import, and a fix that removed a file worked on a +// fresh machine only. So the archive is unpacked into a fresh directory beside the old one, owned, +// and swapped in by rename. A running process keeps the files it has open, and the old tree is +// removed only once the new one is in place. A failed unpack leaves the old tree untouched. +func replaceWith(body []byte, path, owner string) (int, error) { + parent := filepath.Dir(path) + if err := os.MkdirAll(parent, 0o755); err != nil { + return 0, err + } + fresh := path + ".unpacking" + replaced := path + ".replaced" + // What an interrupted earlier attempt left beside the directory. + for _, leftover := range []string{fresh, replaced} { + if err := os.RemoveAll(leftover); err != nil { + return 0, err + } + } + if err := os.Mkdir(fresh, 0o755); err != nil { + return 0, err + } + written, err := unpack(body, fresh) + if err == nil { + err = ownAll(fresh, owner) + } + if err != nil { + os.RemoveAll(fresh) + return written, err + } + hadOne := true + if err := os.Rename(path, replaced); err != nil { + if !os.IsNotExist(err) { + os.RemoveAll(fresh) + return written, err + } + hadOne = false + } + if err := os.Rename(fresh, path); err != nil { + if hadOne { + // Put the old tree back rather than leave nothing at the path. + os.Rename(replaced, path) + } + os.RemoveAll(fresh) + return written, err + } + if hadOne { + if err := os.RemoveAll(replaced); err != nil { + return written, fmt.Errorf("%s is in place, and the tree it replaced could not be removed: %w", path, err) + } + } + return written, nil +} + func fetch(ctx context.Context, source string) ([]byte, error) { request, err := http.NewRequestWithContext(ctx, http.MethodGet, source, nil) if err != nil { diff --git a/internal/apply/archive_replace_test.go b/internal/apply/archive_replace_test.go new file mode 100644 index 0000000..2982f1f --- /dev/null +++ b/internal/apply/archive_replace_test.go @@ -0,0 +1,68 @@ +package apply + +import ( + "context" + "os" + "testing" + + "github.com/novox/mesh-host/internal/store" +) + +// novox/hq issue 220: the tree on disk is exactly the archive. A file the previous archive had and +// this one does not is gone, and nothing is left beside the directory. +func TestAnArchiveReplacesTheTreeItWasUnpackedOver(t *testing.T) { + dir := t.TempDir() + first, firstDigest := anArchive(t, map[string]string{"index.js": "old", "node_modules/dep/index.js": "dep"}) + d := declare(t, `{"id":"bundle-tools","type":"archive","source":"`+serving(t, first)+ + `","digest":"`+firstDigest+`","path":"`+dir+`/tools"}`) + _, state, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, noServices, nil, nil) + if err != nil { + t.Fatal(err) + } + + second, secondDigest := anArchive(t, map[string]string{"index.js": "one file"}) + d = declare(t, `{"id":"bundle-tools","type":"archive","source":"`+serving(t, second)+ + `","digest":"`+secondDigest+`","path":"`+dir+`/tools"}`) + if _, _, err := Apply(context.Background(), archHost(t), d, state, store.OriginCarried, noServices, nil, nil); err != nil { + t.Fatal(err) + } + if got, _ := os.ReadFile(dir + "/tools/index.js"); string(got) != "one file" { + t.Fatalf("index.js is %q", got) + } + if _, err := os.Stat(dir + "/tools/node_modules"); err == nil { + t.Fatal("the previous archive's directory is still there") + } + entries, _ := os.ReadDir(dir) + if len(entries) != 1 { + names := []string{} + for _, e := range entries { + names = append(names, e.Name()) + } + t.Fatalf("beside the tree: %v", names) + } +} + +// A tree is replaced only by one that unpacked whole: an archive refused halfway leaves the old +// tree as it was. +func TestARefusedArchiveLeavesTheTreeItWouldHaveReplaced(t *testing.T) { + dir := t.TempDir() + first, firstDigest := anArchive(t, map[string]string{"index.js": "old"}) + d := declare(t, `{"id":"bundle-tools","type":"archive","source":"`+serving(t, first)+ + `","digest":"`+firstDigest+`","path":"`+dir+`/tools"}`) + _, state, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, noServices, nil, nil) + if err != nil { + t.Fatal(err) + } + bad, badDigest := anArchive(t, map[string]string{"index.js": "new", "../../escaped": "no"}) + d = declare(t, `{"id":"bundle-tools","type":"archive","source":"`+serving(t, bad)+ + `","digest":"`+badDigest+`","path":"`+dir+`/tools"}`) + if _, _, err := Apply(context.Background(), archHost(t), d, state, store.OriginCarried, noServices, nil, nil); err == nil { + t.Fatal("an escaping archive was accepted") + } + if got, _ := os.ReadFile(dir + "/tools/index.js"); string(got) != "old" { + t.Fatalf("index.js is %q after a refused archive", got) + } + if entries, _ := os.ReadDir(dir); len(entries) != 1 { + t.Fatalf("%d entries beside the tree after a refused archive", len(entries)) + } +}