amadan
server: hibernating env files hand ownership to the service user (#660)
Found on the production canary's first wake: add-instance (root) writes the env file 0600 root-owned — fine for systemd, which reads EnvironmentFile as root before dropping privileges, but the activator reads it as the router's service user and got permission denied. matchOwner chowns the file to /data's owner (the service user per the box hardening) for hibernating registers and on hibernate --on flips of existing instances. Also fixes the vet break on main (handlers_test.go: resp2 used before its error check).
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
3 files changed,
+57
−1
control.go+32 −0control_test.go+21 −0handlers_test.go+4 −1
diff --git a/control.go b/control.go| index 01a75eb..e1ed366 100644 |
| --- a/control.go |
| +++ b/control.go |
| @@ -230,6 +230,16 @@ func materializeInstance(reg *Registry, route Route, token string, opts addInsta |
| if err := writeEnvFile(envPath, env); err != nil { |
| return nil, fmt.Errorf("write env file: %v", err) |
| } |
| + if route.Hibernate { |
| + // Systemd-managed instances never care who owns the env file — systemd |
| + // reads EnvironmentFile as root before dropping privileges. A hibernating |
| + // instance's env is read by the ROUTER at wake time, and the router runs |
| + // as the service user, so root-run add-instance must hand the file over |
| + // (found live: the canary's first wake died on permission denied). |
| + if err := matchOwner(envPath, instanceDataDir); err != nil { |
| + return nil, fmt.Errorf("env file ownership: %v", err) |
| + } |
| + } |
| // The instance's unit execs /opt/lchat/live/<host>; stage the running control |
| // binary and point that symlink at it, so ExecStart resolves on first start. |
| @@ -618,6 +628,12 @@ func runHibernate(args []string) { |
| // so the first wake's restore sees it as the warm cache and skips; replication |
| // under the fresh lease is what backfills S3. |
| func hibernateOn(reg *Registry, rt *Route, unitTmpl, routerUnit string) error { |
| + // Existing instances' env files were written for the systemd path (root |
| + // reads them); the router's activator reads them as the service user, so |
| + // hand the file over first — a flip that can't wake is worse than no flip. |
| + if err := matchOwner(filepath.Join(instanceDataDir, rt.Host+".env"), instanceDataDir); err != nil { |
| + return fmt.Errorf("env file ownership: %v", err) |
| + } |
| if err := reg.setHibernate(rt.Host, true); err != nil { |
| return fmt.Errorf("flag: %v", err) |
| } |
| @@ -1022,6 +1038,22 @@ var systemctl = func(args ...string) error { |
| return nil |
| } |
| +// matchOwner chowns path to whatever owns dir — the service user owns /data |
| +// (the box hardening), so this is "give the file to whoever runs the router" |
| +// without parsing systemd units for a user name. A no-op when both are |
| +// already the same (the single-user dev case). |
| +func matchOwner(path, dir string) error { |
| + st, err := os.Stat(dir) |
| + if err != nil { |
| + return err |
| + } |
| + sys, ok := st.Sys().(*syscall.Stat_t) |
| + if !ok { |
| + return nil // non-unix stat: nothing to match, nothing to fix |
| + } |
| + return os.Chown(path, int(sys.Uid), int(sys.Gid)) |
| +} |
| + |
| // writeEnvFile writes KEY=value lines (sorted for a stable file) at 0600 — it |
| // holds the admin token, so it's operator-secret, like the platform env it |
| // replaces. |
diff --git a/control_test.go b/control_test.go| index 9d5791e..2fa6732 100644 |
| --- a/control_test.go |
| +++ b/control_test.go |
| @@ -9,6 +9,7 @@ import ( |
| "path/filepath" |
| "reflect" |
| "strings" |
| + "syscall" |
| "testing" |
| "time" |
| ) |
| @@ -657,3 +658,23 @@ func TestWaitSocketQuiet(t *testing.T) { |
| t.Error("live listener reported quiet — the flip-off would start a unit against a live child") |
| } |
| } |
| + |
| +// matchOwner hands a root-written env file to the service user (the owner of |
| +// /data) so the router's activator can read it at wake time — the bug the |
| +// production canary's first wake hit. Unprivileged test: same-owner is a |
| +// clean no-op and the owners really do match afterwards. |
| +func TestMatchOwner(t *testing.T) { |
| + dir := t.TempDir() |
| + path := filepath.Join(dir, "x.env") |
| + if err := os.WriteFile(path, []byte("A=b\n"), 0o600); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := matchOwner(path, dir); err != nil { |
| + t.Fatalf("matchOwner: %v", err) |
| + } |
| + ds, _ := os.Stat(dir) |
| + fs, _ := os.Stat(path) |
| + if ds.Sys().(*syscall.Stat_t).Uid != fs.Sys().(*syscall.Stat_t).Uid { |
| + t.Fatalf("owner mismatch after matchOwner") |
| + } |
| +} |
diff --git a/handlers_test.go b/handlers_test.go| index 2076728..1fe9f9e 100644 |
| --- a/handlers_test.go |
| +++ b/handlers_test.go |
| @@ -2424,7 +2424,10 @@ func TestLevelGatedGroups(t *testing.T) { |
| if code, _ := post("/api/threads/"+gid+"/visibility", `{"token":"`+tokB+`","visibility":"open","open_tier":"`+tid+`"}`); code != 200 { |
| t.Fatalf("keyless reopen: code=%d", code) |
| } |
| - resp2, _ := http.Get(ts.URL + "/api/threads/open?token=" + tokB) |
| + resp2, err := http.Get(ts.URL + "/api/threads/open?token=" + tokB) |
| + if err != nil { |
| + t.Fatalf("open directory: %v", err) |
| + } |
| defer resp2.Body.Close() |
| var dir2 struct { |
| Threads []OpenThread `json:"threads"` |