amadan
Merge branch 'fix-adopt-registry-ownership' into main
6 files changed,
+687
−1
internal/host/agent.go+13 −1internal/host/bootstrap.go+39 −0internal/host/fileowner_other.go+10 −0internal/host/fileowner_unix.go+20 −0internal/host/registryowner.go+237 −0internal/host/registryowner_test.go+368 −0
CI passed — run details
diff --git a/internal/host/agent.go b/internal/host/agent.go| index 395fa43ab..d2da8db09 100644 |
| --- a/internal/host/agent.go |
| +++ b/internal/host/agent.go |
| @@ -198,6 +198,9 @@ func RunAgent(args []string) { |
| // one feeds the activator's due-time index and the adopt loop. |
| reg, err := registry.Open(regPath) |
| if err != nil { |
| + if hint := registryOpenHint(regPath, err); hint != "" { |
| + log.Fatalf("agent: open registry %s: %v\nagent: %s", regPath, err, hint) |
| + } |
| log.Fatalf("agent: open registry %s: %v", regPath, err) |
| } |
| @@ -2811,8 +2814,17 @@ func RunAdopt(args []string) { |
| restartOnly := fs.Bool("restart-only", false, "cycle the routes of apps whose restart generation moved, and nothing else: the agent-requested pass (carlos-restart.service)") |
| fs.Usage = cliflag.Usage(fs, "") |
| _ = fs.Parse(args) |
| - reg, err := registry.Open(*regPath) |
| + // openServiceRegistry, not registry.Open: this pass runs as root |
| + // (carlos-adopt/-stage/-restart/-config-sync have no User=), opening the |
| + // registry is what creates its -wal/-shm, and the helper hands them back |
| + // to the user the edge runs as as part of the open. That matters most for |
| + // -restart-only, which returns a dozen lines below — see registryowner.go |
| + // for why a converge on every pass is what it takes. |
| + reg, err := openServiceRegistry(*regPath) |
| if err != nil { |
| + if hint := registryOpenHint(*regPath, err); hint != "" { |
| + log.Fatalf("adopt: open registry %s: %v\nadopt: %s", *regPath, err, hint) |
| + } |
| log.Fatalf("adopt: open registry %s: %v", *regPath, err) |
| } |
| ctx := context.Background() |
diff --git a/internal/host/bootstrap.go b/internal/host/bootstrap.go| index 7611bde29..a34b66e08 100644 |
| --- a/internal/host/bootstrap.go |
| +++ b/internal/host/bootstrap.go |
| @@ -234,6 +234,45 @@ func bootstrap(root string, consumers []string) error { |
| } |
| } |
| + // The registry file, created here and by nothing else on a fresh box. |
| + // |
| + // It has to exist BEFORE the units below are armed, because four of them |
| + // (carlos-adopt, -stage, -restart, -config-sync) run `carlos adopt` as |
| + // root and open this same file — and on a fresh box adopt's .path and |
| + // .timer can fire before the edge has ever started. Whoever creates the |
| + // file owns it, and an edge running as serviceUser cannot write a |
| + // root-owned registry: that is discussion #8, three region boxes left |
| + // unstartable on 2026-08-31. Creating it here puts it inside the |
| + // `chown -R /data` that privilegedPlan already runs, so it is |
| + // serviceUser's before anything can race for it. |
| + // |
| + // An empty file is a valid empty SQLite database — the first opener |
| + // (edge or adopt) runs the migrations into it. Deliberately NOT opened |
| + // as a registry here: on a re-run over a live box this must not touch |
| + // the schema of a database the running edge holds open, and "the file |
| + // exists, owned by the right user" is the whole job. |
| + // |
| + // One constraint this puts on the deploy side: a STAGING run takes this |
| + // branch too. `carlos bootstrap -root <dir>` is how a live box refreshes |
| + // its unit files (pinfra's update-platform-binary.sh stages into |
| + // /tmp/carlos-unitstage), so a staging tree now contains a 0-byte |
| + // data/registry.db. Harmless while that script copies only etc/ out of |
| + // the tree — and destructive the day anything rsyncs the whole tree over |
| + // /, because a 0-byte file would land on top of the live registry and |
| + // empty it. The script lives in the private platform-infrastructure repo; |
| + // whoever widens what it copies has to exclude data/. |
| + regPath := filepath.Join(dataDir, "registry.db") |
| + if _, err := os.Stat(regPath); os.IsNotExist(err) { |
| + f, err := os.OpenFile(regPath, os.O_CREATE|os.O_WRONLY, 0o640) |
| + if err != nil { |
| + return fmt.Errorf("create %s: %v", regPath, err) |
| + } |
| + if err := f.Close(); err != nil { |
| + return fmt.Errorf("create %s: %v", regPath, err) |
| + } |
| + fmt.Printf(" created %s\n", regPath) |
| + } |
| + |
| names, err := unitNames() |
| if err != nil { |
| return err |
diff --git a/internal/host/fileowner_other.go b/internal/host/fileowner_other.go| new file mode 100644 |
| index 000000000..9ab9347f4 |
| --- /dev/null |
| +++ b/internal/host/fileowner_other.go |
| @@ -0,0 +1,10 @@ |
| +//go:build !unix |
| + |
| +package host |
| + |
| +import "io/fs" |
| + |
| +// fileOwner has no answer off unix. Every caller treats !ok as "say nothing |
| +// about ownership" rather than as a failure, so the CLI still builds and runs |
| +// on Windows; the box side this serves is Linux-only anyway. |
| +func fileOwner(fs.FileInfo) (uid, gid int, ok bool) { return 0, 0, false } |
diff --git a/internal/host/fileowner_unix.go b/internal/host/fileowner_unix.go| new file mode 100644 |
| index 000000000..1a8067ba6 |
| --- /dev/null |
| +++ b/internal/host/fileowner_unix.go |
| @@ -0,0 +1,20 @@ |
| +//go:build unix |
| + |
| +package host |
| + |
| +import ( |
| + "io/fs" |
| + "syscall" |
| +) |
| + |
| +// fileOwner reports a file's owning uid and gid. Split by build tag because |
| +// internal/host is linked into the CLI, which people run on Macs and Windows, |
| +// and syscall.Stat_t does not exist everywhere — the gate cross-compiles all |
| +// five targets for exactly this reason. |
| +func fileOwner(fi fs.FileInfo) (uid, gid int, ok bool) { |
| + st, ok := fi.Sys().(*syscall.Stat_t) |
| + if !ok { |
| + return 0, 0, false |
| + } |
| + return int(st.Uid), int(st.Gid), true |
| +} |
diff --git a/internal/host/registryowner.go b/internal/host/registryowner.go| new file mode 100644 |
| index 000000000..756b9e9d0 |
| --- /dev/null |
| +++ b/internal/host/registryowner.go |
| @@ -0,0 +1,237 @@ |
| +package host |
| + |
| +import ( |
| + "fmt" |
| + "log" |
| + "os" |
| + "os/user" |
| + "path/filepath" |
| + "strconv" |
| + "strings" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| +) |
| + |
| +// The registry is written by TWO users. `carlos agent` (carlos-edge.service) |
| +// runs as serviceUser; every adoption-shaped pass — carlos-adopt, |
| +// carlos-stage, carlos-restart, carlos-config-sync — runs as root, because a |
| +// pass writes /opt/carlos and cycles units via systemctl. Both open |
| +// /data/registry.db, and SQLite creates the database and its -wal/-shm |
| +// sidecars owned by whoever got there first. |
| +// |
| +// On an established box the edge got there first, the files are carlos-owned, |
| +// and root writing to them is fine. On a FRESH box it is a race that root can |
| +// win: `carlos bootstrap` arms adopt's .path and .timer before the edge has |
| +// ever started, so adopt can create the registry as root and leave the edge |
| +// unable to start at all: |
| +// |
| +// agent: open registry /data/registry.db: attempt to write a readonly database (8) |
| +// |
| +// systemd gives up after five restarts in three seconds, so the box then sits |
| +// failed with no further attempts. All three region boxes brought up on |
| +// 2026-08-31 landed exactly there (discussion #8). |
| +// |
| +// Two fences, because one is not enough: |
| +// |
| +// 1. bootstrap creates the file itself, before the units that could race for |
| +// it are armed (see bootstrap.go) — so root never gets to create it. |
| +// 2. every root pass converges the ownership afterwards, below. Chowning by |
| +// hand once is NOT enough on a live box and the report says so: the armed |
| +// path/timer units fire again and re-create the sidecars as root in |
| +// between. Two of the three boxes needed the chown twice. |
| + |
| +// openServiceRegistry opens the registry for a root-side pass and converges |
| +// its ownership in the same breath. |
| +// |
| +// One function rather than two statements, because the two must not drift. |
| +// Opening the database is what CREATES the -wal/-shm, so the converge has to |
| +// follow the open — and `carlos adopt -restart-only` returns from RunAdopt a |
| +// few lines after it. A converge written as a separate statement that ever |
| +// slid below that early return would silently stop self-healing the restart |
| +// path, which is the pass that fires most often on a live box (carlos-restart |
| +// .path is per-request). Callers get the registry only through here, so there |
| +// is no ordering left to get wrong. |
| +func openServiceRegistry(path string) (*registry.Registry, error) { |
| + return openRegistryConverging(os.Geteuid(), serviceUser, path) |
| +} |
| + |
| +// openRegistryConverging is openServiceRegistry with the two facts it reads |
| +// from the process passed in; see chownRegistryFiles. |
| +func openRegistryConverging(euid int, svcUser, path string) (*registry.Registry, error) { |
| + reg, err := registry.Open(path) |
| + if err != nil { |
| + return nil, err |
| + } |
| + chownRegistryFiles(euid, svcUser, path) |
| + return reg, nil |
| +} |
| + |
| +// chownRegistryToService leaves the registry and its WAL sidecars owned by |
| +// serviceUser. It is a no-op unless this process is root: a pass running as |
| +// the service user already creates everything with the right owner. |
| +// |
| +// Failures are logged, never fatal. An adoption pass that can read the |
| +// registry has real work to do — releases to stage, units to cycle — and |
| +// refusing to do it because a chown failed would turn a cosmetic problem into |
| +// an outage. The next pass tries again. |
| +func chownRegistryToService(path string) { |
| + // Geteuid reports -1 on platforms without the concept (the CLI |
| + // cross-compiles to windows), which takes the same early exit. |
| + chownRegistryFiles(os.Geteuid(), serviceUser, path) |
| +} |
| + |
| +// chownRegistryFiles is chownRegistryToService with the process facts — the |
| +// effective uid and the service user's name — made arguments, so a test can |
| +// drive the root arm, and both of its failure arms, from a test binary that |
| +// is not root on a box that has no carlos user. |
| +func chownRegistryFiles(euid int, svcUser, path string) { |
| + if euid != 0 { |
| + return |
| + } |
| + u, err := user.Lookup(svcUser) |
| + if err != nil { |
| + log.Printf("adopt: registry ownership: no %s user: %v", svcUser, err) |
| + return |
| + } |
| + uid, err := strconv.Atoi(u.Uid) |
| + if err != nil { |
| + return |
| + } |
| + gid, err := strconv.Atoi(u.Gid) |
| + if err != nil { |
| + return |
| + } |
| + for _, p := range registryFiles(path) { |
| + fi, err := os.Stat(p) |
| + if err != nil { |
| + // The -wal/-shm sidecars only exist while some connection is |
| + // open. Their absence is the normal state, not a problem. |
| + continue |
| + } |
| + if fuid, fgid, ok := fileOwner(fi); ok && fuid == uid && fgid == gid { |
| + continue |
| + } |
| + if err := os.Chown(p, uid, gid); err != nil { |
| + // Logged and stepped over, never returned: the files after this |
| + // one may well be fixable, and none of it is worth the pass. |
| + log.Printf("adopt: chown %s to %s: %v", p, svcUser, err) |
| + continue |
| + } |
| + // Worth a line every time it fires: on a healthy box it never does, |
| + // so a run of these is the box telling you something armed root is |
| + // creating the registry ahead of the edge. |
| + log.Printf("adopt: %s was not owned by %s — fixed; the edge runs as %s and could not have written it", p, svcUser, svcUser) |
| + } |
| +} |
| + |
| +// registryFiles is the database and the two sidecars SQLite keeps beside it |
| +// in WAL mode. Ownership has to cover all three: a carlos-owned database with |
| +// a root-owned -wal is just as unwritable as a root-owned database. |
| +func registryFiles(path string) []string { |
| + return []string{path, path + "-wal", path + "-shm"} |
| +} |
| + |
| +// registryOpenHint turns SQLite's words into the box's words. |
| +// |
| +// "attempt to write a readonly database (8)" is what the edge printed on all |
| +// three boxes, and it sent the operator looking at disk space and mount |
| +// options before file ownership. The hint exists so the next person does not |
| +// lose that time — which means it has to be RIGHT and not merely helpful: a |
| +// hint that names ownership where ownership is not the cause sends them |
| +// somewhere just as wrong, with more confidence behind it. |
| +// |
| +// So it says one of three things, and the process's own euid picks which: |
| +// |
| +// - As ROOT, ownership is never the answer. The root units drop no |
| +// capabilities, so root holds CAP_DAC_OVERRIDE and writes the file |
| +// whatever its owner and mode; a chown is the one action that cannot |
| +// help. What remains possible for root is a read-only remount, an |
| +// immutable attribute, a hot journal, or a MAC denial — and those are |
| +// what it names. |
| +// - Off root, holding a file this process does not own, ownership IS the |
| +// answer and it gives the chown. |
| +// - Off root with everything already owned correctly, ownership is not the |
| +// answer either, so it points at the mount and the unit sandbox rather |
| +// than repeating advice the operator has already taken. |
| +// |
| +// It stats all three of registryFiles rather than the database alone. A |
| +// carlos-owned database with a root-owned -wal reports the identical error, |
| +// and that is exactly the shape a hand chown of the database alone leaves |
| +// behind — the second failure in #8, and why two of the three boxes needed |
| +// the chown twice. Reporting the database alone printed "owned by carlos … |
| +// must be owned by carlos" there: a self-contradiction, on the very case that |
| +// motivated the hint. |
| +// |
| +// It returns "" for any other error, so a genuinely full disk still reads as |
| +// itself. |
| +func registryOpenHint(path string, err error) string { |
| + return registryOpenHintFor(os.Geteuid(), path, err) |
| +} |
| + |
| +// registryOpenHintFor is registryOpenHint with the effective uid passed in, |
| +// so the root and non-root wordings are both reachable from one test binary. |
| +func registryOpenHintFor(euid int, path string, err error) string { |
| + if err == nil || !strings.Contains(strings.ToLower(err.Error()), "readonly database") { |
| + return "" |
| + } |
| + var b strings.Builder |
| + fmt.Fprintf(&b, "the registry is not writable by this process (uid %d).", euid) |
| + |
| + // present counts the files that exist; mine, the ones this process owns. |
| + // They are equal only when there is nothing left for a chown to fix. |
| + var present, mine int |
| + for _, p := range registryFiles(path) { |
| + fi, statErr := os.Stat(p) |
| + if statErr != nil { |
| + // A missing -wal/-shm is the normal state between connections, |
| + // not a finding to report. |
| + continue |
| + } |
| + present++ |
| + uid, gid, ok := fileOwner(fi) |
| + if !ok { |
| + // Off unix there is no owner to read. Counted as present but |
| + // never as mine, so the hint never claims ownership is fine on |
| + // evidence it does not have. |
| + fmt.Fprintf(&b, " %s exists (owner unreadable on this platform).", p) |
| + continue |
| + } |
| + if uid == euid { |
| + mine++ |
| + } |
| + fmt.Fprintf(&b, " %s is owned by %s (uid %d, gid %d).", p, ownerName(uid), uid, gid) |
| + } |
| + |
| + dir := filepath.Dir(path) |
| + switch { |
| + case euid == 0: |
| + fmt.Fprintf(&b, " Running as root, this is NOT an ownership problem: the root units drop"+ |
| + " no capabilities, so root holds CAP_DAC_OVERRIDE and writes the file whatever its"+ |
| + " owner and mode — changing the owner would change nothing. What can still produce"+ |
| + " it for root: the filesystem remounted read-only after an I/O error"+ |
| + " (`dmesg -T | tail`, `findmnt -no OPTIONS %s`), an immutable attribute on one of"+ |
| + " the files (`lsattr %s*`), a hot journal or -wal left by a crash that SQLite"+ |
| + " cannot replay, or a MAC denial (SELinux/AppArmor) on the write.", dir, path) |
| + case present > 0 && present == mine: |
| + fmt.Fprintf(&b, " Ownership is already correct — every file that exists is owned by this"+ |
| + " uid — so setting it again will not help. Look instead at the mount (a read-only"+ |
| + " remount after an I/O error, or a read-only %s: `findmnt -no OPTIONS %s`) and at"+ |
| + " this unit's sandbox (ProtectSystem= or ReadOnlyPaths=, or %s missing from"+ |
| + " ReadWritePaths=).", dir, dir, dir) |
| + default: |
| + fmt.Fprintf(&b, " It and its -wal/-shm sidecars must be owned by %s, the user"+ |
| + " carlos-edge.service runs as. A root `carlos adopt` pass that reached the file"+ |
| + " before the edge did is the usual cause. Fix: chown %s:%s %s*", |
| + serviceUser, serviceUser, serviceUser, path) |
| + } |
| + return b.String() |
| +} |
| + |
| +// ownerName resolves a uid to a username, falling back to the number itself. |
| +// A box whose passwd lookup is broken still gets a usable hint. |
| +func ownerName(uid int) string { |
| + if u, err := user.LookupId(strconv.Itoa(uid)); err == nil && u.Username != "" { |
| + return u.Username |
| + } |
| + return strconv.Itoa(uid) |
| +} |
diff --git a/internal/host/registryowner_test.go b/internal/host/registryowner_test.go| new file mode 100644 |
| index 000000000..10a0d43d6 |
| --- /dev/null |
| +++ b/internal/host/registryowner_test.go |
| @@ -0,0 +1,368 @@ |
| +package host |
| + |
| +import ( |
| + "errors" |
| + "os" |
| + "path/filepath" |
| + "strings" |
| + "testing" |
| + |
| + "github.com/carlosframework/platform/internal/registry" |
| +) |
| + |
| +// TestBootstrapCreatesRegistryBeforeArmingUnits is the fence for discussion |
| +// #8. Four of the units bootstrap arms run `carlos adopt` as ROOT and open |
| +// /data/registry.db; the edge that also opens it runs as the service user. |
| +// Whoever creates the file owns it, so on a fresh box the file must exist |
| +// before anything is armed — created inside the /data tree that |
| +// privilegedPlan then chowns to the service user. |
| +func TestBootstrapCreatesRegistryBeforeArmingUnits(t *testing.T) { |
| + root := t.TempDir() |
| + if err := bootstrap(root, nil); err != nil { |
| + t.Fatalf("bootstrap: %v", err) |
| + } |
| + reg := filepath.Join(root, "data", "registry.db") |
| + fi, err := os.Stat(reg) |
| + if err != nil { |
| + t.Fatalf("bootstrap did not create the registry: %v", err) |
| + } |
| + if fi.IsDir() { |
| + t.Fatalf("%s is a directory", reg) |
| + } |
| + // It is inside the tree privilegedPlan hands to the service user — the |
| + // step that makes creating it here worth anything at all. |
| + var chowned bool |
| + for _, args := range privilegedPlan(nil) { |
| + if args[0] == "chown" && args[len(args)-1] == "/data" { |
| + chowned = true |
| + } |
| + } |
| + if !chowned { |
| + t.Error("privilegedPlan no longer chowns /data — the registry bootstrap creates would stay root-owned") |
| + } |
| +} |
| + |
| +// TestBootstrapRegistryIsAnOpenableRegistry: the file bootstrap leaves behind |
| +// is an empty SQLite database, not a placeholder the first real opener has to |
| +// delete. Zero length is a valid empty database; migrations run into it. |
| +func TestBootstrapRegistryIsAnOpenableRegistry(t *testing.T) { |
| + root := t.TempDir() |
| + if err := bootstrap(root, nil); err != nil { |
| + t.Fatalf("bootstrap: %v", err) |
| + } |
| + reg, err := registry.Open(filepath.Join(root, "data", "registry.db")) |
| + if err != nil { |
| + t.Fatalf("open the registry bootstrap created: %v", err) |
| + } |
| + defer reg.Close() |
| + if _, err := reg.List(); err != nil { |
| + t.Fatalf("list routes: %v", err) |
| + } |
| +} |
| + |
| +// TestBootstrapKeepsAnExistingRegistry: bootstrap re-runs over live boxes |
| +// (that is how a unit refresh happens), and the running edge holds this file |
| +// open. A re-run must not truncate it or touch its schema. |
| +func TestBootstrapKeepsAnExistingRegistry(t *testing.T) { |
| + root := t.TempDir() |
| + if err := bootstrap(root, nil); err != nil { |
| + t.Fatalf("first bootstrap: %v", err) |
| + } |
| + regPath := filepath.Join(root, "data", "registry.db") |
| + reg, err := registry.Open(regPath) |
| + if err != nil { |
| + t.Fatalf("open: %v", err) |
| + } |
| + if err := reg.Upsert(registry.Route{Host: "app.example.com", Kind: "static"}); err != nil { |
| + t.Fatalf("upsert: %v", err) |
| + } |
| + if err := reg.Close(); err != nil { |
| + t.Fatalf("close: %v", err) |
| + } |
| + |
| + if err := bootstrap(root, nil); err != nil { |
| + t.Fatalf("second bootstrap: %v", err) |
| + } |
| + |
| + reg2, err := registry.Open(regPath) |
| + if err != nil { |
| + t.Fatalf("reopen: %v", err) |
| + } |
| + defer reg2.Close() |
| + routes, err := reg2.List() |
| + if err != nil { |
| + t.Fatalf("list: %v", err) |
| + } |
| + if len(routes) != 1 || routes[0].Host != "app.example.com" { |
| + t.Fatalf("re-running bootstrap lost the registry contents: %+v", routes) |
| + } |
| +} |
| + |
| +// TestChownRegistryToServiceIsANoopOffRoot: the helper runs on every adoption |
| +// pass, and the tests (and any developer box) are not root. It must do |
| +// nothing at all there rather than log or fail. |
| +func TestChownRegistryToServiceIsANoopOffRoot(t *testing.T) { |
| + if os.Geteuid() == 0 { |
| + t.Skip("running as root: this test pins the non-root path") |
| + } |
| + path := filepath.Join(t.TempDir(), "registry.db") |
| + if err := os.WriteFile(path, []byte("x"), 0o640); err != nil { |
| + t.Fatal(err) |
| + } |
| + before, err := os.Stat(path) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + logged := captureLog(t) |
| + chownRegistryToService(path) // must not panic, must not change anything |
| + after, err := os.Stat(path) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + buid, bgid, ok := fileOwner(before) |
| + auid, agid, ok2 := fileOwner(after) |
| + if ok != ok2 || buid != auid || bgid != agid { |
| + t.Errorf("ownership moved off root: %d:%d -> %d:%d", buid, bgid, auid, agid) |
| + } |
| + // Silent, too: a laptop or a CI runner must not be told about a chown it |
| + // was never going to do. |
| + if out := logged(); out != "" { |
| + t.Errorf("off root the helper logged:\n%s", out) |
| + } |
| +} |
| + |
| +// TestRegistryFilesCoversTheWALSidecars: a carlos-owned database with a |
| +// root-owned -wal is exactly as unwritable as a root-owned database, and the |
| +// sidecars are what a root pass re-creates after a hand chown. Two of the |
| +// three boxes in #8 needed the chown twice for this reason. |
| +func TestRegistryFilesCoversTheWALSidecars(t *testing.T) { |
| + got := registryFiles("/data/registry.db") |
| + want := []string{"/data/registry.db", "/data/registry.db-wal", "/data/registry.db-shm"} |
| + if len(got) != len(want) { |
| + t.Fatalf("registryFiles = %v, want %v", got, want) |
| + } |
| + for i := range want { |
| + if got[i] != want[i] { |
| + t.Errorf("registryFiles[%d] = %q, want %q", i, got[i], want[i]) |
| + } |
| + } |
| +} |
| + |
| +// foreignUID is a uid that is certainly not the one owning a file this test |
| +// process just created, and certainly not root. It is how the "somebody else |
| +// owns the registry" shape is reached from a test binary that cannot chown. |
| +func foreignUID() int { return os.Geteuid() + 1 } |
| + |
| +// TestRegistryOpenHintNamesOwnership is the cheap fence #8 asked for: |
| +// "attempt to write a readonly database (8)" is SQLite's words, and they sent |
| +// the operator looking at disk space and mount options. Where ownership IS |
| +// the cause — a non-root process holding a file it does not own — the hint |
| +// has to name the owner it found, the service user, and the fixing command. |
| +func TestRegistryOpenHintNamesOwnership(t *testing.T) { |
| + path := filepath.Join(t.TempDir(), "registry.db") |
| + if err := os.WriteFile(path, nil, 0o640); err != nil { |
| + t.Fatal(err) |
| + } |
| + hint := registryOpenHintFor(foreignUID(), path, errors.New("attempt to write a readonly database (8)")) |
| + if hint == "" { |
| + t.Fatal("no hint for the readonly-database error") |
| + } |
| + for _, want := range []string{serviceUser, "chown", path, "owned by"} { |
| + if !strings.Contains(hint, want) { |
| + t.Errorf("hint does not mention %q:\n%s", want, hint) |
| + } |
| + } |
| +} |
| + |
| +// TestRegistryOpenHintStatsTheSidecars: the hint used to stat the database |
| +// alone, which is why the reporter's SECOND failure — database owned by |
| +// carlos, -wal still owned by root — printed "owned by carlos … must be owned |
| +// by carlos". Every file that exists has to be reported, or the hint |
| +// contradicts itself on the exact case that motivated it. |
| +func TestRegistryOpenHintStatsTheSidecars(t *testing.T) { |
| + dir := t.TempDir() |
| + path := filepath.Join(dir, "registry.db") |
| + for _, p := range registryFiles(path) { |
| + if err := os.WriteFile(p, nil, 0o640); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + hint := registryOpenHintFor(foreignUID(), path, errors.New("attempt to write a readonly database (8)")) |
| + for _, p := range registryFiles(path) { |
| + if !strings.Contains(hint, p) { |
| + t.Errorf("hint says nothing about %s:\n%s", p, hint) |
| + } |
| + } |
| +} |
| + |
| +// TestRegistryOpenHintDoesNotBlameOwnershipAsRoot is the negative fence. The |
| +// root passes drop no capabilities, so root holds CAP_DAC_OVERRIDE and writes |
| +// a file whatever its owner — a "readonly database" seen by root can NEVER be |
| +// an ownership problem, and telling root to chown is pointing at the one |
| +// action that cannot help. Note the file here is owned by somebody other than |
| +// root as far as the hint is concerned: even with a mismatch in front of it, |
| +// the root wording must hold. |
| +func TestRegistryOpenHintDoesNotBlameOwnershipAsRoot(t *testing.T) { |
| + path := filepath.Join(t.TempDir(), "registry.db") |
| + if err := os.WriteFile(path, nil, 0o640); err != nil { |
| + t.Fatal(err) |
| + } |
| + hint := registryOpenHintFor(0, path, errors.New("attempt to write a readonly database (8)")) |
| + if hint == "" { |
| + t.Fatal("no hint for the readonly-database error") |
| + } |
| + if strings.Contains(hint, "chown") { |
| + t.Errorf("the root hint tells the operator to chown, which cannot help root:\n%s", hint) |
| + } |
| + // And it names causes that ARE possible for root. |
| + for _, want := range []string{"CAP_DAC_OVERRIDE", "read-only", "immutable", "SELinux"} { |
| + if !strings.Contains(hint, want) { |
| + t.Errorf("the root hint does not mention %q:\n%s", want, hint) |
| + } |
| + } |
| +} |
| + |
| +// TestRegistryOpenHintSaysNothingIsWrongWithOwnership: off root, with every |
| +// file that exists already owned by this process, ownership is not the cause |
| +// either. Repeating the chown is exactly the advice the reporter had already |
| +// followed; the hint has to move on to the mount and the unit sandbox. |
| +func TestRegistryOpenHintSaysNothingIsWrongWithOwnership(t *testing.T) { |
| + if os.Geteuid() == 0 { |
| + t.Skip("running as root: the root wording is pinned by its own test") |
| + } |
| + dir := t.TempDir() |
| + path := filepath.Join(dir, "registry.db") |
| + for _, p := range registryFiles(path) { |
| + if err := os.WriteFile(p, nil, 0o640); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + hint := registryOpenHintFor(os.Geteuid(), path, errors.New("attempt to write a readonly database (8)")) |
| + if hint == "" { |
| + t.Fatal("no hint for the readonly-database error") |
| + } |
| + if strings.Contains(hint, "chown") { |
| + t.Errorf("ownership is already correct and the hint still asks for a chown:\n%s", hint) |
| + } |
| + for _, want := range []string{"already correct", "mount", "ReadWritePaths"} { |
| + if !strings.Contains(hint, want) { |
| + t.Errorf("hint does not mention %q:\n%s", want, hint) |
| + } |
| + } |
| +} |
| + |
| +// TestRegistryOpenHintSilentOnOtherErrors: a genuinely full disk or a missing |
| +// directory must still read as itself. The hint is only for the one error it |
| +// can actually explain. |
| +func TestRegistryOpenHintSilentOnOtherErrors(t *testing.T) { |
| + path := filepath.Join(t.TempDir(), "registry.db") |
| + for _, err := range []error{ |
| + nil, |
| + errors.New("database or disk is full (13)"), |
| + errors.New("unable to open database file"), |
| + } { |
| + if hint := registryOpenHint(path, err); hint != "" { |
| + t.Errorf("registryOpenHint(%v) = %q, want no hint", err, hint) |
| + } |
| + // Both wordings that DO fire have to stay off these errors too. |
| + if hint := registryOpenHintFor(0, path, err); hint != "" { |
| + t.Errorf("registryOpenHintFor(root, %v) = %q, want no hint", err, hint) |
| + } |
| + } |
| +} |
| + |
| +// TestOpenServiceRegistryConvergesOwnership pins the two halves as one |
| +// operation. `carlos adopt -restart-only` returns from RunAdopt within a few |
| +// lines of the open, so a converge that ever slid below that early return |
| +// would silently stop self-healing the pass that fires most often. Opening |
| +// through this helper is what makes the ordering unloseable — the test drives |
| +// the root arm with a service user that cannot resolve, so the one thing it |
| +// observes is that the open reached the converge at all. |
| +func TestOpenServiceRegistryConvergesOwnership(t *testing.T) { |
| + path := filepath.Join(t.TempDir(), "registry.db") |
| + logged := captureLog(t) |
| + const noSuchUser = "carlos-no-such-service-user-8f21" |
| + reg, err := openRegistryConverging(0, noSuchUser, path) |
| + if err != nil { |
| + t.Fatalf("open: %v", err) |
| + } |
| + defer reg.Close() |
| + if _, err := reg.List(); err != nil { |
| + t.Fatalf("the helper returned an unusable registry: %v", err) |
| + } |
| + if !strings.Contains(logged(), noSuchUser) { |
| + t.Errorf("opening the registry did not run the ownership converge; log was:\n%s", logged()) |
| + } |
| +} |
| + |
| +// TestRunAdoptOpensTheRegistryThroughTheHelper is the other half of the same |
| +// fence, and the reason it reads source rather than asserting an order: an |
| +// ordering assertion rots the moment the function is rearranged, while "the |
| +// root pass never calls registry.Open itself" stays true for as long as the |
| +// invariant does. |
| +func TestRunAdoptOpensTheRegistryThroughTheHelper(t *testing.T) { |
| + src, err := os.ReadFile("agent.go") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + i := strings.Index(string(src), "\nfunc RunAdopt(") |
| + if i < 0 { |
| + t.Fatal("RunAdopt is no longer in agent.go — move this test with it") |
| + } |
| + rest := string(src)[i+1:] |
| + j := strings.Index(rest, "\n}\n") |
| + if j < 0 { |
| + t.Fatal("could not find the end of RunAdopt") |
| + } |
| + body := rest[:j] |
| + if !strings.Contains(body, "openServiceRegistry(") { |
| + t.Error("RunAdopt no longer opens the registry through openServiceRegistry — the root pass has stopped converging ownership") |
| + } |
| + if strings.Contains(body, "registry.Open(") { |
| + t.Error("RunAdopt opens the registry directly; use openServiceRegistry so the chown cannot be lost ahead of the -restart-only early return") |
| + } |
| +} |
| + |
| +// TestChownRegistryFailureIsLoggedNotFatal: an adoption pass that can read the |
| +// registry has releases to stage and units to cycle. A chown it cannot do is |
| +// a cosmetic problem, and dying over it would turn that into an outage — so |
| +// every failure arm logs and carries on. A test that reaches its own end is |
| +// the assertion: log.Fatalf would take the process with it. |
| +func TestChownRegistryFailureIsLoggedNotFatal(t *testing.T) { |
| + t.Run("service user does not resolve", func(t *testing.T) { |
| + path := filepath.Join(t.TempDir(), "registry.db") |
| + if err := os.WriteFile(path, nil, 0o640); err != nil { |
| + t.Fatal(err) |
| + } |
| + logged := captureLog(t) |
| + chownRegistryFiles(0, "carlos-no-such-service-user-8f21", path) |
| + if !strings.Contains(logged(), "carlos-no-such-service-user-8f21") { |
| + t.Errorf("the failed lookup was not logged:\n%s", logged()) |
| + } |
| + }) |
| + |
| + t.Run("the chown itself is refused", func(t *testing.T) { |
| + if os.Geteuid() == 0 { |
| + t.Skip("running as root: chown would succeed, so there is no refusal to observe") |
| + } |
| + dir := t.TempDir() |
| + path := filepath.Join(dir, "registry.db") |
| + // Database and -wal both present and both owned by this (non-root) |
| + // user: chowning them to root is refused for each in turn. |
| + for _, p := range []string{path, path + "-wal"} { |
| + if err := os.WriteFile(p, nil, 0o640); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + logged := captureLog(t) |
| + chownRegistryFiles(0, "root", path) |
| + out := logged() |
| + // Both files reported, which is the "continue, do not return" half: |
| + // the loop stepped over the first failure and tried the second. |
| + for _, p := range []string{path, path + "-wal"} { |
| + if !strings.Contains(out, "chown "+p+" to root") { |
| + t.Errorf("no logged failure for %s; log was:\n%s", p, out) |
| + } |
| + } |
| + }) |
| +} |