amadan

carlos / platform Public

hostfiles: restore refuses an entry whose object, hash or size is not one this package could have written, before touching anything

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Paul Campbell pushed by claude 6c8571d176dc3cb8a79a69dc4991d0eba8d98a09 parent 55af287
2 files changed, +173 −0
  • internal/hostfiles/hostfiles_test.go +140 −0
  • internal/hostfiles/restore.go +33 −0
diff --git a/internal/hostfiles/hostfiles_test.go b/internal/hostfiles/hostfiles_test.go
index 4e905c139..ca1135232 100644
--- a/internal/hostfiles/hostfiles_test.go
+++ b/internal/hostfiles/hostfiles_test.go
@@ -680,3 +680,143 @@ func TestTwoFailedCommitsThenAStageRecognisesTheAppliedOne(t *testing.T) {
t.Fatalf("third commit = %d, %v; want 3, nil", seq, err)
}
}
+
+// putHostile publishes a good manifest with one file "a", lets bend alter
+// the stored manifest, and returns a files directory that already holds a
+// file Restore would remove or replace if it acted.
+func putHostile(t *testing.T, bend func(m *Manifest)) (release.Store, string) {
+ t.Helper()
+ ctx, s := context.Background(), release.NewDirStore(t.TempDir())
+ p := open(t, s)
+ stage(t, p, "a", "good")
+ stage(t, p, "gone", "soon")
+ commit(t, p)
+ if err := p.StageDelete("gone"); err != nil {
+ t.Fatal(err)
+ }
+ if err := p.StagePlatform(ctx, "item", []byte("plat")); err != nil {
+ t.Fatal(err)
+ }
+ commit(t, p)
+ m, etag, _, err := load(ctx, s, pfx)
+ if err != nil {
+ t.Fatal(err)
+ }
+ bend(&m)
+ body, _ := json.Marshal(m)
+ if _, err := s.Put(ctx, ManifestKey(pfx), body, etag); err != nil {
+ t.Fatal(err)
+ }
+ dir := t.TempDir()
+ if err := os.WriteFile(filepath.Join(dir, "gone"), []byte("still here"), 0o600); err != nil {
+ t.Fatal(err)
+ }
+ return s, dir
+}
+
+func TestRestoreRefusesAnEntryWhoseObjectOrHashIsMalformedAndTouchesNothing(t *testing.T) {
+ cases := map[string]func(m *Manifest){
+ "uppercase sha": func(m *Manifest) {
+ e := m.Entries["a"]
+ e.SHA256 = strings.ToUpper(e.SHA256)
+ e.Object = objectKey(e.Seq, e.SHA256)
+ m.Entries["a"] = e
+ },
+ "short sha": func(m *Manifest) {
+ e := m.Entries["a"]
+ e.SHA256, e.Object = "aa", objectKey(e.Seq, "aa")
+ m.Entries["a"] = e
+ },
+ "object elsewhere": func(m *Manifest) {
+ e := m.Entries["a"]
+ e.Object = "../../other/files/g1/" + e.Object
+ m.Entries["a"] = e
+ },
+ "object with another seq": func(m *Manifest) {
+ e := m.Entries["a"]
+ e.Object = objectKey(e.Seq+1, e.SHA256)
+ m.Entries["a"] = e
+ },
+ "platform object elsewhere": func(m *Manifest) {
+ e := m.Platform["item"]
+ e.Object = "objects/elsewhere"
+ m.Platform["item"] = e
+ },
+ "platform sha not hex": func(m *Manifest) {
+ e := m.Platform["item"]
+ e.SHA256 = strings.Repeat("g", 64)
+ e.Object = objectKey(e.Seq, e.SHA256)
+ m.Platform["item"] = e
+ },
+ }
+ for name, bend := range cases {
+ s, dir := putHostile(t, bend)
+ if _, _, err := Restore(context.Background(), s, pfx, dir); err == nil {
+ t.Fatalf("%s: Restore accepted the manifest", name)
+ }
+ if got := read(t, dir, "gone"); got != "still here" {
+ t.Fatalf("%s: gone = %q, want it untouched", name, got)
+ }
+ if _, err := os.Stat(filepath.Join(dir, "a")); !os.IsNotExist(err) {
+ t.Fatalf("%s: a was written", name)
+ }
+ }
+}
+
+func TestReadPlatformRefusesAMalformedEntry(t *testing.T) {
+ s, _ := putHostile(t, func(m *Manifest) {
+ e := m.Platform["item"]
+ e.Object = "objects/elsewhere"
+ m.Platform["item"] = e
+ })
+ ctx := context.Background()
+ // The right bytes at the wrong key: only the key check can refuse it.
+ if _, err := s.Put(ctx, pfx+"objects/elsewhere", []byte("plat"), ""); err != nil {
+ t.Fatal(err)
+ }
+ m, _, err := Load(ctx, s, pfx)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, err := ReadPlatform(ctx, s, pfx, m, "item"); err == nil {
+ t.Fatal("ReadPlatform accepted an object key that is not its own")
+ }
+}
+
+func TestRestoreRefusesAnEntryWhoseSizeIsOutOfRangeAndTouchesNothing(t *testing.T) {
+ for _, size := range []int64{-1, MaxFileBytes + 1} {
+ s, dir := putHostile(t, func(m *Manifest) {
+ e := m.Entries["a"]
+ e.Size = size
+ m.Entries["a"] = e
+ })
+ if _, _, err := Restore(context.Background(), s, pfx, dir); err == nil {
+ t.Fatalf("size %d: Restore accepted the manifest", size)
+ }
+ if got := read(t, dir, "gone"); got != "still here" {
+ t.Fatalf("size %d: gone = %q, want it untouched", size, got)
+ }
+ }
+}
+
+func TestFetchRefusesAnObjectWhoseLengthIsNotTheRecordedSize(t *testing.T) {
+ s, dir := putHostile(t, func(m *Manifest) {
+ e := m.Entries["a"]
+ e.Size = 3 // the object is four bytes, and its hash is right
+ m.Entries["a"] = e
+ p := m.Platform["item"]
+ p.Size = 5
+ m.Platform["item"] = p
+ })
+ ctx := context.Background()
+ if _, _, err := Restore(ctx, s, pfx, dir); err == nil || !strings.Contains(err.Error(), "size") {
+ t.Fatalf("Restore err = %v, want a size mismatch", err)
+ }
+ m, _, err := Load(ctx, s, pfx)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if _, err := ReadPlatform(ctx, s, pfx, m, "item"); err == nil || !strings.Contains(err.Error(), "size") {
+ t.Fatalf("ReadPlatform err = %v, want a size mismatch", err)
+ }
+}
diff --git a/internal/hostfiles/restore.go b/internal/hostfiles/restore.go
index 5582687ad..03da182ce 100644
--- a/internal/hostfiles/restore.go
+++ b/internal/hostfiles/restore.go
@@ -55,10 +55,18 @@ func Restore(ctx context.Context, s release.Store, prefix, dir string) (Manifest
return Manifest{}, st, err
}
var live, dead []string
+ for name, e := range m.Platform {
+ if err := checkEntry(e); err != nil {
+ return Manifest{}, st, fmt.Errorf("hostfiles: platform item %q: %w", name, err)
+ }
+ }
for p, e := range m.Entries {
if _, err := CleanPath(p); err != nil {
return Manifest{}, st, err
}
+ if err := checkEntry(e); err != nil {
+ return Manifest{}, st, fmt.Errorf("hostfiles: %s: %w", p, err)
+ }
if e.Deleted {
dead = append(dead, p)
} else {
@@ -192,7 +200,29 @@ func ReadPlatform(ctx context.Context, s release.Store, prefix string, m Manifes
return fetch(ctx, s, prefix, e)
}
+// checkEntry refuses a published entry whose object is not the one this
+// package would have written for it, or whose size it could never have
+// published. Tombstones and skipped entries name no object.
+func checkEntry(e Entry) error {
+ if e.Deleted || e.Skipped != "" {
+ return nil
+ }
+ if len(e.SHA256) != 64 || strings.Trim(e.SHA256, "0123456789abcdef") != "" {
+ return fmt.Errorf("sha256 %q is not 64 lowercase hex characters", e.SHA256)
+ }
+ if want := objectKey(e.Seq, e.SHA256); e.Object != want {
+ return fmt.Errorf("object %q is not %q", e.Object, want)
+ }
+ if e.Size < 0 || e.Size > MaxFileBytes {
+ return fmt.Errorf("size %d is outside 0 to %d", e.Size, MaxFileBytes)
+ }
+ return nil
+}
+
func fetch(ctx context.Context, s release.Store, prefix string, e Entry) ([]byte, error) {
+ if err := checkEntry(e); err != nil {
+ return nil, err
+ }
body, _, err := s.Get(ctx, prefix+e.Object)
if err != nil {
return nil, err
@@ -200,6 +230,9 @@ func fetch(ctx context.Context, s release.Store, prefix string, e Entry) ([]byte
if got := sum(body); got != e.SHA256 {
return nil, fmt.Errorf("object %s has sha256 %s, the manifest says %s", e.Object, got, e.SHA256)
}
+ if int64(len(body)) != e.Size {
+ return nil, fmt.Errorf("object %s has size %d, the manifest says %d", e.Object, len(body), e.Size)
+ }
return body, nil
}