amadan
hostfiles: restore never blocks on a swapped-in FIFO, absence on tenant ground is decided beneath one held directory descriptor, and walks and manifests are capped at PATH_MAX and 256 components.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
14 files changed,
+603
−63
internal/host/relocationflush.go+13 −22internal/host/relocationflush_saferead_linux_test.go+52 −0internal/host/relocationinstall.go+3 −1internal/hostfiles/dir_other.go+6 −1internal/hostfiles/dir_unix.go+5 −0internal/hostfiles/fix3_linux_test.go+231 −0internal/hostfiles/present.go+23 −31internal/hostfiles/restore.go+10 −1internal/hostfiles/sync.go+25 −1internal/hostfiles/walk_unix.go+21 −4internal/instancefs/absent_linux_test.go+136 −0internal/instancefs/instancefs.go+24 −1internal/instancefs/instancefs_linux.go+47 −1internal/instancefs/instancefs_other.go+7 −0
CI cancelled — superseded by a newer push — run details
diff --git a/internal/host/relocationflush.go b/internal/host/relocationflush.go| index dc0aa56b9..461cf9c69 100644 |
| --- a/internal/host/relocationflush.go |
| +++ b/internal/host/relocationflush.go |
| @@ -165,30 +165,21 @@ func syncTenantDir(dir string) error { |
| } |
| // readInstanceFile reads name in dir, an instance directory the tenant can |
| -// write, through main's one safe read for that ground: the same |
| -// instancefs.ReadBeneathLimit the activator's instance-key and adoption |
| -// code and appconfig's nested render use for these very files. It refuses |
| -// a symlink at any component of dir or name (openat2 RESOLVE_NO_SYMLINKS), |
| -// opens O_NONBLOCK so a FIFO cannot hang the agent, accepts only a regular |
| -// file, and never reads more than max bytes. Any of those refusals, and |
| -// any other failure to read, is an error. |
| +// write, through main's one safe read for that ground: instancefs, the |
| +// package the activator's instance-key and adoption code and appconfig's |
| +// nested render read these very files through. It refuses a symlink at |
| +// any component of dir or name (openat2 RESOLVE_NO_SYMLINKS), opens |
| +// O_NONBLOCK so a FIFO cannot hang the agent, accepts only a regular file, |
| +// and never reads more than max bytes. Any of those refusals, and any other |
| +// failure to read, is an error. |
| // |
| -// absent is true only on positive evidence: dir itself opened as a real |
| -// directory with no symlink anywhere in its path (instancefs.StatDir), and |
| -// the name was then not in it. A missing directory is not that evidence: |
| -// it is an error, never "no file". |
| +// absent is true only on positive evidence, decided in one place |
| +// (instancefs.ReadBeneathLimitOrAbsent): dir opened once as a real |
| +// directory, and name was not found beneath that same descriptor. A |
| +// directory that is missing, or that vanishes between a check and a read, |
| +// is an error, never "no file". |
| func readInstanceFile(dir, name string, max int64) (raw []byte, absent bool, err error) { |
| - if _, err := instancefs.StatDir(dir); err != nil { |
| - return nil, false, err |
| - } |
| - raw, err = instancefs.ReadBeneathLimit(dir, name, max) |
| - if errors.Is(err, os.ErrNotExist) { |
| - return nil, true, nil |
| - } |
| - if err != nil { |
| - return nil, false, err |
| - } |
| - return raw, false, nil |
| + return instancefs.ReadBeneathLimitOrAbsent(dir, name, max) |
| } |
| // releasePosition is one recorded release on disk: a versioned |
diff --git a/internal/host/relocationflush_saferead_linux_test.go b/internal/host/relocationflush_saferead_linux_test.go| index 78f222aea..4f0982aa3 100644 |
| --- a/internal/host/relocationflush_saferead_linux_test.go |
| +++ b/internal/host/relocationflush_saferead_linux_test.go |
| @@ -8,6 +8,8 @@ import ( |
| "errors" |
| "os" |
| "path/filepath" |
| + "strings" |
| + "sync/atomic" |
| "syscall" |
| "testing" |
| "time" |
| @@ -351,3 +353,53 @@ func TestReleaseFlushPublishesTheDirectoryItOpenedNotALinkSwappedInAfter(t *test |
| t.Fatalf("the flush did not publish the directory it opened: %v", m.Entries) |
| } |
| } |
| + |
| +// Astra P2 / Opus M4: a directory renamed away and back while |
| +// readInstanceFile looks is an error or the file, never "absent": a flush |
| +// must never stage the deletion of an env file that is there. |
| +func TestReadInstanceFileNeverReadsAVanishingDirectoryAsAbsent(t *testing.T) { |
| + dir := filepath.Join(t.TempDir(), "inst") |
| + if err := os.Mkdir(dir, 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.WriteFile(filepath.Join(dir, "shop.example.env"), []byte("K=v\n"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + var stop atomic.Bool |
| + flapped := make(chan struct{}) |
| + go func() { |
| + defer close(flapped) |
| + for !stop.Load() { |
| + os.Rename(dir, dir+".away") |
| + os.Rename(dir+".away", dir) |
| + } |
| + }() |
| + defer func() { stop.Store(true); <-flapped }() |
| + deadline := time.Now().Add(1500 * time.Millisecond) |
| + for time.Now().Before(deadline) { |
| + if _, absent, _ := readInstanceFile(dir, "shop.example.env", 64); absent { |
| + t.Fatal("a directory that vanished between the steps read as an absent file") |
| + } |
| + } |
| +} |
| + |
| +// Opus I1: a files tree past the walk's caps refuses the flush whole; |
| +// nothing is committed. |
| +func TestReleaseFlushRefusesATreePastTheWalksCaps(t *testing.T) { |
| + x := newFlushFixture(t) |
| + root, err := os.OpenRoot(x.filesDir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer root.Close() |
| + if err := root.MkdirAll(strings.TrimSuffix(strings.Repeat("d/", hostfiles.MaxDepth+1), "/"), 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, err = x.flush(t) |
| + if !errors.Is(err, hostfiles.ErrTooDeep) { |
| + t.Fatalf("flush of a tree past the caps: %v", err) |
| + } |
| + if _, ok, err := hostfiles.Load(context.Background(), x.data, x.prefix); ok || err != nil { |
| + t.Fatalf("a refused flush committed a manifest: %v %v", ok, err) |
| + } |
| +} |
diff --git a/internal/host/relocationinstall.go b/internal/host/relocationinstall.go| index 4864a14f4..f9c682fca 100644 |
| --- a/internal/host/relocationinstall.go |
| +++ b/internal/host/relocationinstall.go |
| @@ -497,7 +497,9 @@ func (a ClaimAssembly) relocationInstall(cfg RelocationAssemblyConfig) func(cont |
| defer root.Close() |
| tenantRootSeam("opened") |
| if m, _, err = hostfiles.RestoreRoot(ctx, cfg.Data, prefix, root, origins); err != nil { |
| - return err |
| + // The root is named by its /proc/self/fd link, closed by |
| + // the time anyone reads this: name the files directory. |
| + return fmt.Errorf("hostfiles: restore into %s: %w", paths.FilesDir, err) |
| } |
| journal.Steps["files"] = strconv.FormatInt(m.Seq, 10) |
| // O3: the env file is P's EnvItem byte for byte; nothing is |
diff --git a/internal/hostfiles/dir_other.go b/internal/hostfiles/dir_other.go| index df73c2d5b..9d6f8db63 100644 |
| --- a/internal/hostfiles/dir_other.go |
| +++ b/internal/hostfiles/dir_other.go |
| @@ -3,6 +3,7 @@ |
| package hostfiles |
| import ( |
| + "context" |
| "errors" |
| "os" |
| ) |
| @@ -12,6 +13,10 @@ import ( |
| // package builds, restores and reads manifests, and does not publish. |
| var errNoPublish = errors.New("hostfiles: publishing a directory is not supported on this OS") |
| +// restoreDirFlags is Restore's directory open off unix, where there is no |
| +// FIFO to wait on. |
| +const restoreDirFlags = os.O_RDONLY |
| + |
| type dirHandle struct{ f *os.File } |
| func openTree(path string) (*dirHandle, error) { return nil, errNoPublish } |
| @@ -19,7 +24,7 @@ func handleOf(f *os.File) (*dirHandle, error) { return nil, errNoPublish } |
| func (d *dirHandle) Close() error { return nil } |
| func (d *dirHandle) Name() string { return "" } |
| -func syncTree(d *dirHandle, p *Publisher, skip func(string, bool) bool, visit func(*dirHandle, string, string) error) error { |
| +func syncTree(ctx context.Context, d *dirHandle, p *Publisher, skip func(string, bool) bool, visit func(*dirHandle, string, string) error) error { |
| return errNoPublish |
| } |
diff --git a/internal/hostfiles/dir_unix.go b/internal/hostfiles/dir_unix.go| index 9ada71af4..4148ab92d 100644 |
| --- a/internal/hostfiles/dir_unix.go |
| +++ b/internal/hostfiles/dir_unix.go |
| @@ -13,6 +13,11 @@ import ( |
| "golang.org/x/sys/unix" |
| ) |
| +// restoreDirFlags is how Restore opens a directory it is about to sync: |
| +// read-only, a directory only, never through a link at the last |
| +// component, and never waiting on a FIFO. |
| +const restoreDirFlags = unix.O_RDONLY | unix.O_DIRECTORY | unix.O_NONBLOCK | unix.O_NOFOLLOW |
| + |
| // dirHandle is a directory held open by descriptor. Everything beneath it |
| // is reached one path component at a time with openat and O_NOFOLLOW, so |
| // no symlink is ever followed, inside the tree or out of it, and a |
diff --git a/internal/hostfiles/fix3_linux_test.go b/internal/hostfiles/fix3_linux_test.go| new file mode 100644 |
| index 000000000..f48b518c4 |
| --- /dev/null |
| +++ b/internal/hostfiles/fix3_linux_test.go |
| @@ -0,0 +1,231 @@ |
| +//go:build linux |
| + |
| +package hostfiles |
| + |
| +import ( |
| + "context" |
| + "encoding/json" |
| + "errors" |
| + "os" |
| + "path/filepath" |
| + "runtime" |
| + "strings" |
| + "sync/atomic" |
| + "syscall" |
| + "testing" |
| + "time" |
| + |
| + "github.com/carlosframework/platform/internal/release" |
| +) |
| + |
| +// Astra P1 / Opus M1: a FIFO swapped in at a directory Restore is about to |
| +// sync (after realParents looked) is refused at once, never waited on. |
| +func TestRestoreRefusesAFIFOSwappedInAtADirectoryItSyncs(t *testing.T) { |
| + ctx := context.Background() |
| + s := release.NewDirStore(t.TempDir()) |
| + p := open(t, s) |
| + stage(t, p, "a/b.txt", "restored") |
| + commit(t, p) |
| + dir := t.TempDir() |
| + prev := syncDir |
| + syncDir = func(root *os.Root, rel string) error { |
| + if rel == "a" { |
| + if err := os.Rename(filepath.Join(dir, "a"), filepath.Join(dir, "a.moved")); err != nil { |
| + t.Error(err) |
| + } |
| + if err := syscall.Mkfifo(filepath.Join(dir, "a"), 0o600); err != nil { |
| + t.Error(err) |
| + } |
| + } |
| + return prev(root, rel) |
| + } |
| + t.Cleanup(func() { syncDir = prev }) |
| + done := make(chan error, 1) |
| + go func() { _, _, err := Restore(ctx, s, pfx, dir, nil); done <- err }() |
| + select { |
| + case err := <-done: |
| + if err == nil { |
| + t.Fatal("Restore synced a FIFO as a directory") |
| + } |
| + case <-time.After(20 * time.Second): |
| + t.Fatal("Restore blocked on a FIFO swapped in at a directory it syncs") |
| + } |
| +} |
| + |
| +// Astra P2 / Opus M4: with the caller's evidence of no env file, a |
| +// directory renamed away and back while Present looks is never read as |
| +// "no env file there" when one is. |
| +func TestPresentNeverReadsAVanishingDirectoryAsNoEnvFile(t *testing.T) { |
| + none := Manifest{V: ManifestVersion, Entries: map[string]Entry{}, Platform: map[string]Entry{}} |
| + dir := filepath.Join(t.TempDir(), "inst") |
| + if err := os.Mkdir(dir, 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + env := filepath.Join(dir, "shop.example.env") |
| + if err := os.WriteFile(env, []byte("X=1\n"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + var stop atomic.Bool |
| + flapped := make(chan struct{}) |
| + go func() { |
| + defer close(flapped) |
| + for !stop.Load() { |
| + os.Rename(dir, dir+".away") |
| + os.Rename(dir+".away", dir) |
| + } |
| + }() |
| + deadline := time.Now().Add(1500 * time.Millisecond) |
| + for time.Now().Before(deadline) { |
| + if ok, _ := Present(t.TempDir(), env, none, true); ok { |
| + stop.Store(true) |
| + <-flapped |
| + t.Fatal("Present read a directory that vanished between its steps as no env file") |
| + } |
| + } |
| + stop.Store(true) |
| + <-flapped |
| +} |
| + |
| +// Opus I1: the walk refuses a path past MaxDepth components or |
| +// MaxPathBytes bytes, at the boundary exactly, and a chain far past it is |
| +// refused quickly with bounded allocation. |
| +func TestSyncDirRefusesATreePastItsCaps(t *testing.T) { |
| + build := func(t *testing.T, comps []string, file bool) string { |
| + t.Helper() |
| + dir := t.TempDir() |
| + root, err := os.OpenRoot(dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer root.Close() |
| + dirs := comps |
| + if file { |
| + dirs = comps[:len(comps)-1] |
| + } |
| + if len(dirs) > 0 { |
| + if err := root.MkdirAll(strings.Join(dirs, "/"), 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + if file { |
| + if err := root.WriteFile(strings.Join(comps, "/"), []byte("x"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + return dir |
| + } |
| + repeat := func(n int, c string) []string { |
| + out := make([]string, n) |
| + for i := range out { |
| + out[i] = c |
| + } |
| + return out |
| + } |
| + sync := func(t *testing.T, ctx context.Context, dir string) error { |
| + t.Helper() |
| + _, err := SyncDir(ctx, open(t, release.NewDirStore(t.TempDir())), dir, nil) |
| + return err |
| + } |
| + for _, tc := range []struct { |
| + name string |
| + comps []string |
| + file bool |
| + ok bool |
| + }{ |
| + {"a file at MaxDepth components", repeat(MaxDepth, "d"), true, true}, |
| + {"a file at MaxDepth+1 components", repeat(MaxDepth+1, "d"), true, false}, |
| + {"directories MaxDepth+1 deep, no file", repeat(MaxDepth+1, "d"), false, false}, |
| + {"a path of MaxPathBytes", strings.Split(pathOfBytes(MaxPathBytes), "/"), true, true}, |
| + {"a path of MaxPathBytes+1", strings.Split(pathOfBytes(MaxPathBytes+1), "/"), true, false}, |
| + } { |
| + t.Run(tc.name, func(t *testing.T) { |
| + if got := len(strings.Join(tc.comps, "/")); strings.Contains(tc.name, "Bytes") && got != MaxPathBytes && got != MaxPathBytes+1 { |
| + t.Fatalf("built a %d-byte path", got) |
| + } |
| + err := sync(t, context.Background(), build(t, tc.comps, tc.file)) |
| + if tc.ok && err != nil { |
| + t.Fatalf("a tree within the caps: %v", err) |
| + } |
| + if !tc.ok && !errors.Is(err, ErrTooDeep) { |
| + t.Fatalf("a tree past the caps: %v; want ErrTooDeep", err) |
| + } |
| + }) |
| + } |
| + t.Run("a chain far past the cap", func(t *testing.T) { |
| + dir := build(t, repeat(4000, "d"), true) |
| + var before, after runtime.MemStats |
| + runtime.GC() |
| + runtime.ReadMemStats(&before) |
| + start := time.Now() |
| + err := sync(t, context.Background(), dir) |
| + runtime.ReadMemStats(&after) |
| + if !errors.Is(err, ErrTooDeep) { |
| + t.Fatalf("got %v, want ErrTooDeep", err) |
| + } |
| + if d := time.Since(start); d > 5*time.Second { |
| + t.Fatalf("refusing took %v", d) |
| + } |
| + if alloc := after.TotalAlloc - before.TotalAlloc; alloc > 16<<20 { |
| + t.Fatalf("refusing allocated %d bytes", alloc) |
| + } |
| + }) |
| + t.Run("cancellation stops a walk with no files", func(t *testing.T) { |
| + ctx, cancel := context.WithCancel(context.Background()) |
| + cancel() |
| + if err := sync(t, ctx, build(t, repeat(20, "d"), false)); !errors.Is(err, context.Canceled) { |
| + t.Fatalf("got %v, want context.Canceled", err) |
| + } |
| + }) |
| +} |
| + |
| +// checkManifest holds a manifest's paths to the walk's caps, so Restore |
| +// refuses one a walk could not have published before touching anything. |
| +func TestRestoreRefusesAManifestPathPastTheCaps(t *testing.T) { |
| + sha := strings.Repeat("a", 64) |
| + entry := Entry{Object: pfx + objectKey(1, sha), SHA256: sha, Seq: 1, Size: 1} |
| + man := func(p string) Manifest { |
| + return Manifest{V: ManifestVersion, Seq: 1, Entries: map[string]Entry{p: entry}, Platform: map[string]Entry{}} |
| + } |
| + deep := func(n int) string { return strings.TrimSuffix(strings.Repeat("d/", n), "/") } |
| + for _, tc := range []struct { |
| + path string |
| + ok bool |
| + }{ |
| + {deep(MaxDepth), true}, {deep(MaxDepth + 1), false}, |
| + {pathOfBytes(MaxPathBytes), true}, |
| + {pathOfBytes(MaxPathBytes + 1), false}, |
| + } { |
| + err := checkManifest(man(tc.path), []string{pfx}) |
| + if tc.ok && err != nil || !tc.ok && !errors.Is(err, ErrTooDeep) { |
| + t.Fatalf("%d bytes, %d components: %v (ok %v)", len(tc.path), strings.Count(tc.path, "/")+1, err, tc.ok) |
| + } |
| + } |
| + ctx, s := context.Background(), release.NewDirStore(t.TempDir()) |
| + body, err := json.Marshal(man(deep(MaxDepth + 1))) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := s.Put(ctx, ManifestKey(pfx), body, ""); err != nil { |
| + t.Fatal(err) |
| + } |
| + dir := t.TempDir() |
| + if _, _, err := Restore(ctx, s, pfx, dir, nil); !errors.Is(err, ErrTooDeep) { |
| + t.Fatalf("Restore: %v; want ErrTooDeep", err) |
| + } |
| + if names, _ := os.ReadDir(dir); len(names) != 0 { |
| + t.Fatalf("a refused restore touched the directory: %v", names) |
| + } |
| +} |
| + |
| +// pathOfBytes is a relative path of exactly n bytes in components of at |
| +// most 200 bytes, so its depth stays far under MaxDepth. |
| +func pathOfBytes(n int) string { |
| + var b strings.Builder |
| + for b.Len() < n { |
| + if b.Len() > 0 { |
| + b.WriteByte('/') |
| + } |
| + b.WriteString(strings.Repeat("x", min(200, n-b.Len()))) |
| + } |
| + return b.String() |
| +} |
diff --git a/internal/hostfiles/present.go b/internal/hostfiles/present.go| index 90db56b8b..7d28043e9 100644 |
| --- a/internal/hostfiles/present.go |
| +++ b/internal/hostfiles/present.go |
| @@ -51,38 +51,29 @@ func Present(dir, envPath string, m Manifest, envAbsent bool) (bool, error) { |
| case !envAbsent: |
| return false, nil |
| } |
| - // No env file is present only on positive evidence: the directory |
| - // opened with no symlink at any component, and the name is not in it. |
| - // A missing directory is not that evidence. |
| - if err := presentRoot(envDir); err != nil { |
| + // No env file is present only on positive evidence, decided in one |
| + // place (instancefs.OpenBeneathOrAbsent): the directory opened once, |
| + // with no symlink at any component, and the name was not found beneath |
| + // that same descriptor. A directory that is missing or vanishes in |
| + // between is not that evidence. |
| + f, absent, err := instancefs.OpenBeneathOrAbsent(envDir, envName) |
| + switch { |
| + case absent: |
| + return true, nil |
| + case err == nil: |
| + f.Close() |
| + return false, nil |
| + case errors.Is(err, instancefs.ErrDirUnopened): |
| if errors.Is(err, os.ErrNotExist) { |
| return false, nil |
| } |
| - return false, err |
| - } |
| - f, err := instancefs.OpenBeneath(envDir, envName, os.O_RDONLY, 0) |
| - if err == nil { |
| - f.Close() |
| - return false, nil |
| - } |
| - if errors.Is(err, os.ErrNotExist) { |
| - return true, nil |
| - } |
| - if notThere(err) { |
| + return false, err // a linked directory: refused, never read through |
| + case notThere(err): |
| return false, nil // something that is not a regular file: not "none" |
| } |
| return false, err |
| } |
| -// presentRoot is dir opened as a real directory with no symlink at any |
| -// component. dir is an instance directory or the files directory in it, |
| -// both the tenant's to write, so a link there is refused as an error, |
| -// never read through and never read as "not present". |
| -func presentRoot(dir string) error { |
| - _, err := instancefs.StatDir(dir) |
| - return err |
| -} |
| - |
| // notThere is what instancefs.OpenBeneath answers for a name that holds |
| // something other than a regular file reached without a link: a symlink |
| // at the name or beneath dir, a directory, a FIFO or a device. None of |
| @@ -95,20 +86,21 @@ func notThere(err error) bool { |
| // presentFile reports whether rel beneath dir is a regular file of size |
| // bytes whose sha256 is sum. Both dir and everything beneath it are the |
| // tenant's to write, so it opens through main's one safe open for that |
| -// ground (instancefs.OpenBeneath: openat2 with no symlink at any |
| +// ground (instancefs.OpenBeneathOrAbsent: openat2 with no symlink at any |
| // component, O_NONBLOCK, a regular file only) and reads no more than size |
| // bytes plus one, so a file grown or swapped after the look cannot make it |
| // read without bound. |
| func presentFile(dir, rel string, size int64, sum string) (bool, error) { |
| - if err := presentRoot(dir); err != nil { |
| + f, absent, err := instancefs.OpenBeneathOrAbsent(dir, rel) |
| + switch { |
| + case absent: |
| + return false, nil |
| + case errors.Is(err, instancefs.ErrDirUnopened): |
| if errors.Is(err, os.ErrNotExist) { |
| return false, nil // nothing restored yet |
| } |
| - return false, err |
| - } |
| - f, err := instancefs.OpenBeneath(dir, rel, os.O_RDONLY, 0) |
| - switch { |
| - case errors.Is(err, os.ErrNotExist), notThere(err): |
| + return false, err // a linked root: refused, never read through |
| + case notThere(err): |
| return false, nil |
| case err != nil: |
| return false, err |
diff --git a/internal/hostfiles/restore.go b/internal/hostfiles/restore.go| index aea39a15e..acf477529 100644 |
| --- a/internal/hostfiles/restore.go |
| +++ b/internal/hostfiles/restore.go |
| @@ -18,8 +18,14 @@ import ( |
| ) |
| // syncDir makes a directory's entries durable. A test seam. |
| +// |
| +// The open is descriptor-relative (os.Root) with restoreDirFlags: |
| +// O_DIRECTORY|O_NONBLOCK|O_NOFOLLOW on unix, so whatever was swapped in at |
| +// rel since realParents looked (a FIFO, a file) is refused at once rather |
| +// than waited on: a plain open of a FIFO with no writer never returns, and |
| +// no context can interrupt it. |
| var syncDir = func(root *os.Root, rel string) error { |
| - d, err := root.Open(filepath.FromSlash(rel)) |
| + d, err := root.OpenFile(filepath.FromSlash(rel), restoreDirFlags, 0) |
| if err != nil { |
| return err |
| } |
| @@ -262,6 +268,9 @@ func checkManifest(m Manifest, origins []string) error { |
| if _, err := CleanPath(p); err != nil { |
| return err |
| } |
| + if err := withinTreeCap(p); err != nil { |
| + return err // no restore could recreate it: refused before anything is touched |
| + } |
| if err := checkEntry(e, origins); err != nil { |
| return fmt.Errorf("hostfiles: %s: %w", p, err) |
| } |
diff --git a/internal/hostfiles/sync.go b/internal/hostfiles/sync.go| index eda413ff0..acf3cb425 100644 |
| --- a/internal/hostfiles/sync.go |
| +++ b/internal/hostfiles/sync.go |
| @@ -2,11 +2,13 @@ package hostfiles |
| import ( |
| "context" |
| + "errors" |
| "fmt" |
| "io" |
| "os" |
| "path" |
| "sort" |
| + "strings" |
| ) |
| type fileState int |
| @@ -73,6 +75,28 @@ func SyncDir(ctx context.Context, p *Publisher, dir string, skip func(rel string |
| return SyncDirAt(ctx, p, root.f, skip) |
| } |
| +// MaxPathBytes caps a path in a files tree, relative to its root: PATH_MAX |
| +// (4096) less its terminating NUL. Every path a walk publishes can then be |
| +// opened by one openat2 beneath the restored root (Present), and no |
| +// manifest a restore accepts names a longer one. |
| +const MaxPathBytes = 4095 |
| + |
| +// MaxDepth caps the components in such a path. With MaxPathBytes it |
| +// bounds a walk's open directories and its path strings. |
| +const MaxDepth = 256 |
| + |
| +// ErrTooDeep is a path past MaxPathBytes or MaxDepth: the walk that met |
| +// it, or the manifest that names it, is refused whole. |
| +var ErrTooDeep = errors.New("hostfiles: a path is past the tree's length or depth cap") |
| + |
| +// withinTreeCap is the one cap a walk and checkManifest share. |
| +func withinTreeCap(rel string) error { |
| + if n, d := len(rel), strings.Count(rel, "/")+1; n > MaxPathBytes || d > MaxDepth { |
| + return fmt.Errorf("%w: %d bytes in %d components (caps %d and %d)", ErrTooDeep, n, d, MaxPathBytes, MaxDepth) |
| + } |
| + return nil |
| +} |
| + |
| // SyncDirAt is SyncDir over a directory the caller has already opened, and |
| // does not close. SyncDir opens its root by path and follows a link there |
| // (the path is the platform's own); a caller syncing ground someone else |
| @@ -91,7 +115,7 @@ func SyncDirAt(ctx context.Context, p *Publisher, dir *os.File, skip func(rel st |
| func syncRoot(ctx context.Context, p *Publisher, root *dirHandle, skip func(rel string, isDir bool) bool) (SyncStats, error) { |
| var st SyncStats |
| seen := map[string]bool{} |
| - err := syncTree(root, p, skip, func(d *dirHandle, name, rel string) error { |
| + err := syncTree(ctx, root, p, skip, func(d *dirHandle, name, rel string) error { |
| if err := ctx.Err(); err != nil { |
| return err |
| } |
diff --git a/internal/hostfiles/walk_unix.go b/internal/hostfiles/walk_unix.go| index 861075714..33afcde26 100644 |
| --- a/internal/hostfiles/walk_unix.go |
| +++ b/internal/hostfiles/walk_unix.go |
| @@ -3,6 +3,7 @@ |
| package hostfiles |
| import ( |
| + "context" |
| "errors" |
| "golang.org/x/sys/unix" |
| @@ -13,11 +14,21 @@ import ( |
| // it, its name there, and its path relative to root. It never follows a |
| // symlink and never blocks on a FIFO: a directory is entered only through |
| // openDir. |
| -func syncTree(root *dirHandle, p *Publisher, skip func(string, bool) bool, visit func(d *dirHandle, name, rel string) error) error { |
| - return walkDir(root, "", skip, visit) |
| +// |
| +// The walk is bounded (withinTreeCap): a path past MaxPathBytes or |
| +// MaxDepth components refuses the whole walk with ErrTooDeep, never a |
| +// silent skip, so a tree no restore could recreate is never published as |
| +// complete, and a chain of directories cannot make the walk's memory grow |
| +// with the square of its depth. ctx is checked on entering every |
| +// directory, so a deep tree with no files still stops when cancelled. |
| +func syncTree(ctx context.Context, root *dirHandle, p *Publisher, skip func(string, bool) bool, visit func(d *dirHandle, name, rel string) error) error { |
| + return walkDir(ctx, root, "", skip, visit) |
| } |
| -func walkDir(d *dirHandle, prefix string, skip func(string, bool) bool, visit func(d *dirHandle, name, rel string) error) error { |
| +func walkDir(ctx context.Context, d *dirHandle, prefix string, skip func(string, bool) bool, visit func(d *dirHandle, name, rel string) error) error { |
| + if err := ctx.Err(); err != nil { |
| + return err |
| + } |
| names, err := d.names() |
| if err != nil { |
| return err |
| @@ -38,6 +49,9 @@ func walkDir(d *dirHandle, prefix string, skip func(string, bool) bool, visit fu |
| if skipped(skip, rel, false) { |
| continue |
| } |
| + if err := withinTreeCap(rel); err != nil { |
| + return err |
| + } |
| if err := visit(d, name, rel); err != nil { |
| return err |
| } |
| @@ -46,6 +60,9 @@ func walkDir(d *dirHandle, prefix string, skip func(string, bool) bool, visit fu |
| if skipped(skip, rel, true) { |
| continue |
| } |
| + if err := withinTreeCap(rel); err != nil { |
| + return err |
| + } |
| betweenLookAndOpen() |
| sub, err := d.openDir(name) |
| if err != nil { |
| @@ -63,7 +80,7 @@ func walkDir(d *dirHandle, prefix string, skip func(string, bool) bool, visit fu |
| } |
| return err |
| } |
| - err = walkDir(sub, rel, skip, visit) |
| + err = walkDir(ctx, sub, rel, skip, visit) |
| sub.Close() |
| if err != nil { |
| return err |
diff --git a/internal/instancefs/absent_linux_test.go b/internal/instancefs/absent_linux_test.go| new file mode 100644 |
| index 000000000..3d2627e03 |
| --- /dev/null |
| +++ b/internal/instancefs/absent_linux_test.go |
| @@ -0,0 +1,136 @@ |
| +//go:build linux |
| + |
| +package instancefs |
| + |
| +import ( |
| + "errors" |
| + "os" |
| + "path/filepath" |
| + "syscall" |
| + "testing" |
| + "time" |
| +) |
| + |
| +// OpenBeneathOrAbsent is the one place absence is decided: absent only |
| +// when the name is not found beneath the descriptor dir was opened as. |
| +func TestOpenBeneathOrAbsentDecidesAbsenceBeneathOneDescriptor(t *testing.T) { |
| + setup := func(t *testing.T) (dir string) { |
| + t.Helper() |
| + dir = filepath.Join(t.TempDir(), "inst") |
| + if err := os.Mkdir(dir, 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := os.WriteFile(filepath.Join(dir, "f"), []byte("bytes"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + return dir |
| + } |
| + open := func(t *testing.T, dir, name string) (string, bool, error) { |
| + t.Helper() |
| + type res struct { |
| + b string |
| + absent bool |
| + err error |
| + } |
| + done := make(chan res, 1) |
| + go func() { |
| + f, absent, err := OpenBeneathOrAbsent(dir, name) |
| + var b []byte |
| + if f != nil { |
| + b, _ = readLimit(f, name, 1<<10) |
| + f.Close() |
| + } |
| + done <- res{string(b), absent, err} |
| + }() |
| + select { |
| + case r := <-done: |
| + return r.b, r.absent, r.err |
| + case <-time.After(20 * time.Second): |
| + t.Fatal("OpenBeneathOrAbsent blocked") |
| + return "", false, nil |
| + } |
| + } |
| + t.Run("present", func(t *testing.T) { |
| + if b, absent, err := open(t, setup(t), "f"); err != nil || absent || b != "bytes" { |
| + t.Fatalf("got %q absent=%v %v", b, absent, err) |
| + } |
| + }) |
| + t.Run("absent beneath a directory that opened", func(t *testing.T) { |
| + if _, absent, err := open(t, setup(t), "g"); err != nil || !absent { |
| + t.Fatalf("absent=%v %v", absent, err) |
| + } |
| + }) |
| + t.Run("a missing directory is an error", func(t *testing.T) { |
| + _, absent, err := open(t, filepath.Join(t.TempDir(), "gone"), "f") |
| + if absent || !errors.Is(err, ErrDirUnopened) || !errors.Is(err, os.ErrNotExist) { |
| + t.Fatalf("absent=%v %v", absent, err) |
| + } |
| + }) |
| + t.Run("a symlinked directory is an error", func(t *testing.T) { |
| + dir := setup(t) |
| + if err := os.Symlink(dir, dir+".link"); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, absent, err := open(t, dir+".link", "f") |
| + if absent || !errors.Is(err, ErrDirUnopened) || !errors.Is(err, ErrSymlink) { |
| + t.Fatalf("absent=%v %v", absent, err) |
| + } |
| + }) |
| + t.Run("a symlink at the name is the name's, not the directory's", func(t *testing.T) { |
| + dir := setup(t) |
| + if err := os.Symlink(filepath.Join(dir, "f"), filepath.Join(dir, "l")); err != nil { |
| + t.Fatal(err) |
| + } |
| + _, absent, err := open(t, dir, "l") |
| + if absent || !errors.Is(err, ErrSymlink) || errors.Is(err, ErrDirUnopened) { |
| + t.Fatalf("absent=%v %v", absent, err) |
| + } |
| + }) |
| + t.Run("a FIFO", func(t *testing.T) { |
| + dir := setup(t) |
| + if err := syscall.Mkfifo(filepath.Join(dir, "p"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, absent, err := open(t, dir, "p"); absent || !errors.Is(err, ErrNotRegular) { |
| + t.Fatalf("absent=%v %v", absent, err) |
| + } |
| + }) |
| + // Astra P2: the directory renamed away after it was opened, and an |
| + // empty one put at its path, is still the directory looked in. A |
| + // lookup that reopened the path would read the file as absent. |
| + t.Run("a directory renamed away after the open", func(t *testing.T) { |
| + dir := setup(t) |
| + prev := betweenDirAndLookup |
| + betweenDirAndLookup = func() { |
| + if err := os.Rename(dir, dir+".away"); err != nil { |
| + t.Error(err) |
| + } |
| + if err := os.Mkdir(dir, 0o700); err != nil { |
| + t.Error(err) |
| + } |
| + } |
| + t.Cleanup(func() { betweenDirAndLookup = prev }) |
| + if b, absent, err := open(t, dir, "f"); err != nil || absent || b != "bytes" { |
| + t.Fatalf("got %q absent=%v %v; want the file the opened directory holds", b, absent, err) |
| + } |
| + }) |
| +} |
| + |
| +// Opus M3: without /proc, OpenRootBeneath is an error, never a fallback to |
| +// opening the path. |
| +func TestOpenRootBeneathWithoutProcIsAnError(t *testing.T) { |
| + parent := t.TempDir() |
| + if err := os.Mkdir(filepath.Join(parent, "d"), 0o700); err != nil { |
| + t.Fatal(err) |
| + } |
| + prev := procSelfFD |
| + procSelfFD = filepath.Join(t.TempDir(), "no-proc") + "/" |
| + t.Cleanup(func() { procSelfFD = prev }) |
| + r, err := OpenRootBeneath(parent, "d") |
| + if err == nil || r != nil { |
| + if r != nil { |
| + r.Close() |
| + } |
| + t.Fatalf("OpenRootBeneath without /proc: root %v, %v; want an error", r != nil, err) |
| + } |
| +} |
diff --git a/internal/instancefs/instancefs.go b/internal/instancefs/instancefs.go| index 875d5aac7..aca341bd0 100644 |
| --- a/internal/instancefs/instancefs.go |
| +++ b/internal/instancefs/instancefs.go |
| @@ -40,6 +40,10 @@ var ErrEscapes = errors.New("instancefs: path escapes its directory") |
| // the files this package reads on a tenant's behalf. |
| var ErrNotRegular = errors.New("instancefs: not a regular file") |
| +// ErrDirUnopened wraps whatever kept OpenBeneathOrAbsent from opening its |
| +// directory: never evidence that the name is absent. |
| +var ErrDirUnopened = errors.New("instancefs: the directory could not be opened") |
| + |
| // ErrNotDir is returned when OpenDirBeneath resolves to anything other |
| // than a directory: a file, or a FIFO, which it never waits on. |
| var ErrNotDir = errors.New("instancefs: not a directory") |
| @@ -103,12 +107,31 @@ func ReadBeneathLimit(dir, name string, max int64) ([]byte, error) { |
| return nil, err |
| } |
| defer f.Close() |
| + return readLimit(f, filepath.Join(dir, name), max) |
| +} |
| + |
| +// ReadBeneathLimitOrAbsent is ReadBeneathLimit with absence decided by |
| +// OpenBeneathOrAbsent: absent only when name was not found beneath the one |
| +// descriptor dir was opened as. Every other failure, a dir that would not |
| +// open included, is an error. |
| +func ReadBeneathLimitOrAbsent(dir, name string, max int64) ([]byte, bool, error) { |
| + f, absent, err := OpenBeneathOrAbsent(dir, name) |
| + if err != nil || absent { |
| + return nil, absent, err |
| + } |
| + defer f.Close() |
| + b, err := readLimit(f, filepath.Join(dir, name), max) |
| + return b, false, err |
| +} |
| + |
| +// readLimit reads f whole, refusing more than max bytes. |
| +func readLimit(f *os.File, path string, max int64) ([]byte, error) { |
| b, err := io.ReadAll(io.LimitReader(f, max+1)) |
| if err != nil { |
| return nil, err |
| } |
| if int64(len(b)) > max { |
| - return nil, fmt.Errorf("%w: %s over %d bytes", ErrTooLarge, filepath.Join(dir, name), max) |
| + return nil, fmt.Errorf("%w: %s over %d bytes", ErrTooLarge, path, max) |
| } |
| return b, nil |
| } |
diff --git a/internal/instancefs/instancefs_linux.go b/internal/instancefs/instancefs_linux.go| index 99459f356..1b2a29cfd 100644 |
| --- a/internal/instancefs/instancefs_linux.go |
| +++ b/internal/instancefs/instancefs_linux.go |
| @@ -111,7 +111,7 @@ func OpenRootBeneath(dir, name string) (*os.Root, error) { |
| } |
| defer f.Close() |
| betweenOpenAndRoot() |
| - r, err := os.OpenRoot("/proc/self/fd/" + strconv.Itoa(int(f.Fd()))) |
| + r, err := os.OpenRoot(procSelfFD + strconv.Itoa(int(f.Fd()))) |
| if err != nil { |
| return nil, fmt.Errorf("instancefs: open %s as a root: %w", filepath.Join(dir, name), err) |
| } |
| @@ -122,6 +122,44 @@ func OpenRootBeneath(dir, name string) (*os.Root, error) { |
| // would race a root opened by path. |
| var betweenOpenAndRoot = func() {} |
| +// procSelfFD is where OpenRootBeneath finds its descriptor's magic link. A |
| +// test seam: a test points it at a missing directory to pin that no /proc |
| +// is an error, never a fallback to the path. |
| +var procSelfFD = "/proc/self/fd/" |
| + |
| +// OpenBeneathOrAbsent opens name under dir read-only as OpenBeneath does, |
| +// and is the one place "absent" is decided on ground someone else can |
| +// write. It opens dir once and keeps that descriptor: absent is true only |
| +// when the lookup of name beneath that very descriptor answers ENOENT. A |
| +// dir that cannot be opened (missing, a symlink, not a directory) is an |
| +// error wrapped in ErrDirUnopened, never absence: closing a check's |
| +// descriptor and opening the path again would read a directory renamed |
| +// away in between as a missing file. |
| +func OpenBeneathOrAbsent(dir, name string) (*os.File, bool, error) { |
| + rel, err := cleanRel(name) |
| + if err != nil { |
| + return nil, false, err |
| + } |
| + dfd, err := openDir(dir) |
| + if err != nil { |
| + return nil, false, fmt.Errorf("%w: %w", ErrDirUnopened, err) |
| + } |
| + defer unix.Close(dfd) |
| + betweenDirAndLookup() |
| + f, err := openAt(dfd, dir, rel, os.O_RDONLY, 0, unix.S_IFREG) |
| + if errors.Is(err, unix.ENOENT) { |
| + return nil, true, nil |
| + } |
| + if err != nil { |
| + return nil, false, err |
| + } |
| + return f, false, nil |
| +} |
| + |
| +// betweenDirAndLookup is a test seam: after OpenBeneathOrAbsent has opened |
| +// dir and before it looks name up beneath it. |
| +var betweenDirAndLookup = func() {} |
| + |
| // openBeneathAs is OpenBeneath's open, accepting only the file type want |
| // (S_IFREG or S_IFDIR) from fstat. |
| func openBeneathAs(dir, name string, flag int, perm os.FileMode, want uint32) (*os.File, error) { |
| @@ -134,6 +172,14 @@ func openBeneathAs(dir, name string, flag int, perm os.FileMode, want uint32) (* |
| return nil, err |
| } |
| defer unix.Close(dfd) |
| + return openAt(dfd, dir, rel, flag, perm, want) |
| +} |
| + |
| +// openAt is the open every function here shares: rel beneath the |
| +// directory descriptor dfd (which names dir, for errors), openat2 with no |
| +// symlink at any component, O_NONBLOCK, and then fstat, accepting only the |
| +// file type want. |
| +func openAt(dfd int, dir, rel string, flag int, perm os.FileMode, want uint32) (*os.File, error) { |
| how := unix.OpenHow{ |
| Flags: uint64(flag) | unix.O_NONBLOCK | unix.O_CLOEXEC, |
| Mode: uint64(perm.Perm()), |
diff --git a/internal/instancefs/instancefs_other.go b/internal/instancefs/instancefs_other.go| index 74aaf44ea..9c91ff7ee 100644 |
| --- a/internal/instancefs/instancefs_other.go |
| +++ b/internal/instancefs/instancefs_other.go |
| @@ -14,6 +14,13 @@ func OpenBeneath(dir, name string, flag int, perm os.FileMode) (*os.File, error) |
| return nil, errors.ErrUnsupported |
| } |
| +func OpenBeneathOrAbsent(dir, name string) (*os.File, bool, error) { |
| + if _, err := cleanRel(name); err != nil { |
| + return nil, false, err |
| + } |
| + return nil, false, errors.ErrUnsupported |
| +} |
| + |
| func OpenDirBeneath(dir, name string) (*os.File, error) { |
| if _, err := cleanRel(name); err != nil { |
| return nil, err |