34beb1b38 · carlos/platform

diff --git a/internal/activator/activator_test.go b/internal/activator/activator_test.go

index 54217d827..ede10d3a2 100644

--- a/internal/activator/activator_test.go

+++ b/internal/activator/activator_test.go

@@ -30,7 +30,10 @@ import (

"testing"

"time"

+ "golang.org/x/sys/unix"

+

"github.com/carlosframework/platform/internal/edge"

+ "github.com/carlosframework/platform/internal/provision"

"github.com/carlosframework/platform/internal/registry"

_ "modernc.org/sqlite"

@@ -2731,6 +2734,35 @@ func TestExecBackendNextDue(t *testing.T) {

}

+// TestExecBackendNextDueRefusesAFIFOEnvFile pins NextDue's nested dispatch:

+// its env file lives on tenant-writable ground exactly like instanceEnv's

+// does, so a FIFO planted there must fail the read fast rather than block

+// the probe forever.

+func TestExecBackendNextDueRefusesAFIFOEnvFile(t *testing.T) {

+ b, rt := nestedBackend(t, &fakeControl{}, &fakeUnit{})

+ instDir := provision.InstanceDir(b.DataDir, rt.Host)

+ if err := os.MkdirAll(instDir, 0o755); err != nil {

+ t.Fatalf("mkdir instance dir: %v", err)

+ }

+ envPath := filepath.Join(instDir, rt.Host+".env")

+ if err := unix.Mkfifo(envPath, 0o600); err != nil {

+ t.Fatalf("Mkfifo: %v", err)

+ }

+ done := make(chan error, 1)

+ go func() {

+ _, err := b.NextDue(context.Background(), rt)

+ done <- err

+ }()

+ select {

+ case err := <-done:

+ if err == nil {

+ t.Fatal("NextDue against a FIFO env file succeeded, want an error")

+ }

+ case <-time.After(2 * time.Second):

+ t.Fatal("NextDue blocked on a FIFO env file instead of refusing it")

+ }

+}

+

// HasSidecar is derived from the filesystem — the /data/<host>-sidecar.env

// convention — no registry column to drift from what's actually staged.

func TestExecBackendHasSidecar(t *testing.T) {

diff --git a/internal/activator/admintoken.go b/internal/activator/admintoken.go

index 77af6d60f..c92e23dbd 100644

--- a/internal/activator/admintoken.go

+++ b/internal/activator/admintoken.go

@@ -23,12 +23,16 @@ package activator

import (

"crypto/rand"

"encoding/hex"

+ "errors"

"fmt"

"log"

"os"

+ "path/filepath"

"strings"

"sync"

"time"

+

+ "github.com/carlosframework/platform/internal/instancefs"

)

// adminTokenMu serializes minting across the process. Two callers can find

@@ -109,12 +113,46 @@ func wellFormedAdminToken(tok string) bool {

// shape-checked, and a value that isn't the shape this mints is reported and

// left exactly as it is.

func EnsureAdminToken(path string) error {

+ return ensureAdminToken(filepath.Dir(path), filepath.Base(path), false)

+}

+

+// EnsureAdminTokenBeneath is EnsureAdminToken for a nested row's own env

+// file (design §3.2): the file lives on tenant-writable ground, so the read

+// and the append both go through instancefs.OpenBeneath instead of a plain

+// path-based open, refusing anything a tenant could plant there that is not

+// the regular file this contract expects (a FIFO the plain calls would

+// otherwise block on forever).

+//

+// EnsureAdminToken is expressed as a call into the same body (below) rather

+// than as a second copy of the mint-and-parse logic, because its own

+// os.ReadFile/os.OpenFile calls are byte-identical in behavior to going

+// through instancefs for a path no tenant can ever reach: nothing on the

+// flat path is ever a symlink or a special file in practice, and

+// instancefs adds only refusals, never a different outcome, for an

+// ordinary regular file. Splitting path into dir/name here, once, is what

+// lets both callers share every line after the read.

+func EnsureAdminTokenBeneath(dir, name string) error {

+ return ensureAdminToken(dir, name, true)

+}

+

+// ensureAdminToken is EnsureAdminToken's and EnsureAdminTokenBeneath's

+// shared body. beneath selects which pair of I/O primitives the read and

+// the append use; everything else, the parse, the mode check, the shape

+// check, the mint, is one piece of logic so the two callers cannot drift.

+func ensureAdminToken(dir, name string, beneath bool) error {

+ path := filepath.Join(dir, name)

adminTokenMu.Lock()

defer adminTokenMu.Unlock()

// Read the raw bytes once: they answer both questions this needs — is a

// token already there, and does the file end on a line boundary.

- raw, err := os.ReadFile(path)

- if err != nil && !os.IsNotExist(err) {

+ var raw []byte

+ var err error

+ if beneath {

+ raw, err = instancefs.ReadBeneathLimit(dir, name, 1<<20)

+ } else {

+ raw, err = os.ReadFile(path)

+ }

+ if err != nil && !errors.Is(err, os.ErrNotExist) {

return fmt.Errorf("read %s: %w", path, err)

}

if tok := parseEnvBytes(raw)["CARLOS_ADMIN_TOKEN"]; tok != "" {

@@ -170,7 +208,12 @@ func EnsureAdminToken(path string) error {

}

// Append rather than rewrite: the instance owns this file, and a

// read-modify-write would race anything else editing it.

- f, err := os.OpenFile(path, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0o600)

+ var f *os.File

+ if beneath {

+ f, err = instancefs.OpenBeneath(dir, name, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0o600)

+ } else {

+ f, err = os.OpenFile(path, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0o600)

+ }

if err != nil {

return fmt.Errorf("open %s: %w", path, err)

}

diff --git a/internal/activator/backend_exec.go b/internal/activator/backend_exec.go

index 8e78e615e..9c477de38 100644

--- a/internal/activator/backend_exec.go

+++ b/internal/activator/backend_exec.go

@@ -715,7 +715,18 @@ func (b *ExecBackend) waitSeedCgroupEmpty(ctx context.Context, rt registry.Route

// same env file the spawn path hands the child (the activator holds it either

// way).

func (b *ExecBackend) NextDue(ctx context.Context, rt registry.Route) (int64, error) {

- fileEnv, err := readEnvFile(b.pathsFor(rt).envFile)

+ paths := b.pathsFor(rt)

+ // A nested host's env file lives on tenant-writable ground (design

+ // §3.2), the same as instanceEnv's read of it: go through instancefs

+ // rather than a plain read, so a name planted there cannot be anything

+ // but a regular file, on pain of an error instead of a hang.

+ read := readEnvFile

+ if paths.layout == LayoutNested {

+ read = func(path string) (map[string]string, error) {

+ return readEnvFileBeneath(filepath.Dir(path), filepath.Base(path))

+ }

+ fileEnv, err := read(paths.envFile)

if err != nil {

return 0, fmt.Errorf("read env file: %w", err)

}

diff --git a/internal/activator/loglistener_test.go b/internal/activator/loglistener_test.go

index e9e1a7e4f..9d5e41578 100644

--- a/internal/activator/loglistener_test.go

+++ b/internal/activator/loglistener_test.go

@@ -66,7 +66,7 @@ func TestLogListenerTeesOneConnectionAndRefusesASecond(t *testing.T) {

// Close must shut the accepted connection and wait for the tee: a line

// written after Close returns must not reach the sink, and the tee

-// goroutine must not outlive the listener (task 3 review, item 1).

+// goroutine must not outlive the listener.

func TestLogListenerCloseShutsTheConnectionAndWaitsForTheTee(t *testing.T) {

dir, err := os.MkdirTemp("", "ll")

if err != nil {

diff --git a/internal/activator/prepare.go b/internal/activator/prepare.go

index c776ae26d..e3a9f4205 100644

--- a/internal/activator/prepare.go

+++ b/internal/activator/prepare.go

@@ -43,7 +43,10 @@ func (b *ExecBackend) prepare(rt registry.Route, role instancectl.Role) (instanc

}

var env map[string]string

if role == instancectl.RoleRun || role == instancectl.RoleSeed {

- if err := EnsureAdminToken(p.envFile); err != nil {

+ // p.dataDir is the instance directory here, tenant-writable ground

+ // (design §3.2), so the token file is minted and read through

+ // instancefs rather than a plain path-based open.

+ if err := EnsureAdminTokenBeneath(p.dataDir, rt.Host+".env"); err != nil {

return p, nil, nil, nil, fmt.Errorf("admin token for %s: %w", rt.Host, err)

}

var err error

diff --git a/internal/activator/unitproc_test.go b/internal/activator/unitproc_test.go

index 3c445dac1..9c9d89297 100644

--- a/internal/activator/unitproc_test.go

+++ b/internal/activator/unitproc_test.go

@@ -13,6 +13,8 @@ import (

"testing"

"time"

+ "golang.org/x/sys/unix"

+

"github.com/carlosframework/platform/internal/instancectl"

"github.com/carlosframework/platform/internal/provision"

"github.com/carlosframework/platform/internal/registry"

@@ -524,11 +526,10 @@ func TestSeedUnitFailureStopsBeforeCleanupAndKeepsTheDBWhenProcessesRemain(t *te

}

-// TestPrepareRemovesAStaleSocketFile covers the controller ruling that

-// restored prepare's stale-socket removal (fix round 1, item 2): a file

-// left behind at rt.Socket by an earlier, SIGKILLed invocation must not

-// survive into the next one, or every later wake's health check would fail

-// with nothing but a dial timeout.

+// TestPrepareRemovesAStaleSocketFile covers prepare's stale-socket removal:

+// a file left behind at rt.Socket by an earlier, SIGKILLed invocation must

+// not survive into the next one, or every later wake's health check would

+// fail with nothing but a dial timeout.

func TestPrepareRemovesAStaleSocketFile(t *testing.T) {

ctl := &fakeControl{inv: "inv1"}

unit := &fakeUnit{}

@@ -552,6 +553,37 @@ func TestPrepareRemovesAStaleSocketFile(t *testing.T) {

}

+// TestPrepareRefusesAFIFOAtTheAdminTokenPath pins EnsureAdminTokenBeneath's

+// use from the nested prepare: the token file lives on the same

+// tenant-writable ground as everything else prepare touches, so a FIFO

+// planted at that name must fail prepare fast rather than hang it forever

+// behind the token mint's own read.

+func TestPrepareRefusesAFIFOAtTheAdminTokenPath(t *testing.T) {

+ ctl := &fakeControl{inv: "inv1"}

+ unit := &fakeUnit{}

+ b, rt := nestedBackend(t, ctl, unit)

+ instDir := provision.InstanceDir(b.DataDir, rt.Host)

+ if err := os.MkdirAll(instDir, 0o700); err != nil {

+ t.Fatal(err)

+ }

+ if err := unix.Mkfifo(filepath.Join(instDir, rt.Host+".env"), 0o600); err != nil {

+ t.Fatalf("Mkfifo: %v", err)

+ }

+ done := make(chan error, 1)

+ go func() {

+ _, _, _, _, err := b.prepare(rt, instancectl.RoleRun)

+ done <- err

+ }()

+ select {

+ case err := <-done:

+ if err == nil {

+ t.Fatal("prepare against a FIFO'd admin token file succeeded, want an error")

+ }

+ case <-time.After(2 * time.Second):

+ t.Fatal("prepare blocked on a FIFO'd admin token file instead of refusing it")

+ }

+}

+

// serveOK binds a unix socket at path and answers 200 to every request, so

// waitHealthyCtx passes without a real unit.

func serveOK(t *testing.T, path string) func() {

diff --git a/internal/host/movespass.go b/internal/host/movespass.go

index 865479cbf..ad66f0ffe 100644

--- a/internal/host/movespass.go

+++ b/internal/host/movespass.go

@@ -5,6 +5,7 @@ import (

"encoding/json"

"errors"

"fmt"

+ "io/fs"

"log"

"os"

"path/filepath"

@@ -287,8 +288,12 @@ func runMoveRound(ctx context.Context, s release.Store, w *movesWorker, rec prov

// §8.2) is fenced here exactly as Classify fences it for a wake: its

// DB may be mid-copy between the flat and nested paths, and neither

// location is safe to checkpoint or purge until the journal clears.

- // Retried next pass, like every other not-yet-ready state above.

- if _, err := os.Stat(provision.LayoutJournalPath(cfg.DataDir, rec.Host)); err == nil {

+ // Retried next pass, like every other not-yet-ready state above. A

+ // journal that cannot be read is not proof there isn't one, so only

+ // a definite ErrNotExist counts as "no journal": anything else

+ // fences rather than risks the move against a migration this box

+ // simply failed to see.

+ if _, err := os.Stat(provision.LayoutJournalPath(cfg.DataDir, rec.Host)); !errors.Is(err, fs.ErrNotExist) {

log.Printf("moves: %s: layout migration in flight; leaving the move for the next pass", rec.Host)

return nil

}

diff --git a/internal/host/movespass_test.go b/internal/host/movespass_test.go

index 17b0f1d7a..19fd26b2f 100644

--- a/internal/host/movespass_test.go

+++ b/internal/host/movespass_test.go

@@ -272,6 +272,33 @@ func TestMovesPassCutFencedByLayoutJournal(t *testing.T) {

}

+// TestMovesPassCutFencedWhenTheJournalCannotBeRead pins the fail-closed

+// fix: a journal directory os.Stat cannot even resolve (ENOTDIR here, a

+// plain file sitting where "layout" should be a directory) is not proof

+// there is no journal, and must fence the cut exactly as a readable

+// journal does, not fail open and let it proceed.

+func TestMovesPassCutFencedWhenTheJournalCannotBeRead(t *testing.T) {

+ h := newMoveHarness(t)

+ host := "a.go.example.com"

+ h.record(t, host, provision.MoveRequest("uk", time.Now()), provision.MovePromoteReady(time.Now()), provision.MoveMarkCutting())

+ h.f.put("data-ie", "instances/"+host+"/generations/g/snapshot", "e1")

+ // A file where the journal's own parent directory should be: any stat

+ // under it fails with ENOTDIR, never ENOENT.

+ if err := writeFileForTest(filepath.Join(h.cfg.DataDir, "layout"), "not a directory"); err != nil {

+ t.Fatal(err)

+ }

+ h.quiet = true

+ if err := movesPass(context.Background(), h.s, h.worker()); err != nil {

+ t.Fatal(err)

+ }

+ if len(h.f.ops) != 0 {

+ t.Fatalf("cut proceeded past an unreadable layout fence: %v", h.f.ops)

+ }

+ if got := h.read(t, host); got.Region != "ie" || got.Move.State != provision.MoveCutting {

+ t.Fatalf("cut state advanced past an unreadable layout fence: %+v", got)

+ }

+}

+

// A cutting move for a nested row checkpoints the row's OWN DBPath, not the

// legacy <DataDir>/<host>.db guess: a nested row's DB lives under

// provision.InstanceDir and the flat guess names a file that was never

diff --git a/internal/instancefs/instancefs_linux.go b/internal/instancefs/instancefs_linux.go

index 333d8a089..738d011c8 100644

--- a/internal/instancefs/instancefs_linux.go

+++ b/internal/instancefs/instancefs_linux.go

@@ -43,14 +43,21 @@ func openDir(dir string) (int, error) {

}

// OpenBeneath opens name under dir with openat2, following no symlink at

-// any component and never leaving dir. A read-only open also carries

-// O_NONBLOCK on the wire and is then checked with fstat: a FIFO with no

-// writer blocks open(2) indefinitely, and a tenant can plant one anywhere

-// under an instance directory the agent reads by contract (its own run

-// directory, its own env files). O_NONBLOCK is a no-op for opening a

-// regular file, so this changes nothing for the read paths that are

-// already the common case; a non-regular target is refused with

-// ErrNotRegular instead of hanging the caller forever.

+// any component and never leaving dir. Every open, whatever its flags,

+// carries O_NONBLOCK on the wire and is then checked with fstat: a FIFO

+// with no reader on the other end blocks a write-open indefinitely, and

+// one with no writer blocks a read-open indefinitely, and a tenant can

+// plant one anywhere under an instance directory the agent reads OR

+// writes by contract (its run directory, its env files, its admin token

+// file). O_NONBLOCK changes the two blocking cases without changing the

+// non-blocking ones: opening a regular file (the common case, and every

+// O_CREAT|O_EXCL caller's own fresh file) behaves identically either way,

+// a read-open of a FIFO with no writer now returns at once instead of

+// blocking, and a write-open of a FIFO with no reader answers ENXIO at

+// once instead of blocking. Whatever DOES come back with a valid fd (a

+// FIFO opened for read, or for write against an existing reader) is then

+// refused by the fstat check unless it is S_IFREG, so nothing non-regular

+// can be read or written through this function at all.

func OpenBeneath(dir, name string, flag int, perm os.FileMode) (*os.File, error) {

rel, err := cleanRel(name)

if err != nil {

@@ -61,13 +68,8 @@ func OpenBeneath(dir, name string, flag int, perm os.FileMode) (*os.File, error)

return nil, err

}

defer unix.Close(dfd)

- wireFlags := uint64(flag)

- readOnly := flag == os.O_RDONLY

- if readOnly {

- wireFlags |= unix.O_NONBLOCK

- }

how := unix.OpenHow{

- Flags: wireFlags | unix.O_CLOEXEC,

+ Flags: uint64(flag) | unix.O_NONBLOCK | unix.O_CLOEXEC,

Mode: uint64(perm.Perm()),

Resolve: resolveFlags,

}

@@ -82,16 +84,14 @@ func OpenBeneath(dir, name string, flag int, perm os.FileMode) (*os.File, error)

return nil, &os.PathError{Op: "openat2", Path: filepath.Join(dir, rel), Err: err}

}

path := filepath.Join(dir, rel)

- if readOnly {

- var st unix.Stat_t

- if err := unix.Fstat(fd, &st); err != nil {

- unix.Close(fd)

- return nil, &os.PathError{Op: "fstat", Path: path, Err: err}

- }

- if st.Mode&unix.S_IFMT != unix.S_IFREG {

- unix.Close(fd)

- return nil, fmt.Errorf("%w: %s", ErrNotRegular, path)

- }

+ var st unix.Stat_t

+ if err := unix.Fstat(fd, &st); err != nil {

+ unix.Close(fd)

+ return nil, &os.PathError{Op: "fstat", Path: path, Err: err}

+ }

+ if st.Mode&unix.S_IFMT != unix.S_IFREG {

+ unix.Close(fd)

+ return nil, fmt.Errorf("%w: %s", ErrNotRegular, path)

}

return os.NewFile(uintptr(fd), path), nil

}

diff --git a/internal/instancefs/instancefs_linux_test.go b/internal/instancefs/instancefs_linux_test.go

index 158f05a2e..c29d21681 100644

--- a/internal/instancefs/instancefs_linux_test.go

+++ b/internal/instancefs/instancefs_linux_test.go

@@ -222,6 +222,31 @@ func TestOpenBeneathRefusesAFIFOInsteadOfBlocking(t *testing.T) {

}

+// TestOpenBeneathRefusesAWriteOpenOfAFIFO pins the write-side half of the

+// same fix: a write-open of a FIFO with no reader on the other end blocks

+// open(2) indefinitely too, and the admin token file this backs an append

+// onto is on the same tenant-writable ground as anything OpenBeneath reads.

+// O_NONBLOCK turns that into an immediate ENXIO instead.

+func TestOpenBeneathRefusesAWriteOpenOfAFIFO(t *testing.T) {

+ dir := t.TempDir()

+ if err := unix.Mkfifo(filepath.Join(dir, "token"), 0o600); err != nil {

+ t.Fatalf("Mkfifo: %v", err)

+ }

+ done := make(chan error, 1)

+ go func() {

+ _, err := OpenBeneath(dir, "token", os.O_WRONLY|os.O_APPEND, 0)

+ done <- err

+ }()

+ select {

+ case err := <-done:

+ if err == nil {

+ t.Fatal("write-open of a FIFO with no reader succeeded, want an error")

+ }

+ case <-time.After(2 * time.Second):

+ t.Fatal("write-open blocked on a FIFO with no reader instead of refusing it")

+ }

+}

+

func TestReadBeneathLimitRefusesALargeFile(t *testing.T) {

dir := t.TempDir()

if err := os.WriteFile(filepath.Join(dir, "big"), make([]byte, 4096), 0o600); err != nil {

carlos / platform Public

isolated exec: every open on tenant ground refuses a special file; NextDue and the admin token read beneath; the moves fence fails closed (plan 3 final fix round 2)