58b598b697
The agent cannot create a PVE storage (Datastore.Allocate at /storage) or grant an ACL (Permissions.Modify) -- it holds neither by design, and widening the role would trade the whole blast-radius containment model for one feature. The privileged half therefore lives in a new fenced shim behind a literal FELHOM_BACKUPTARGET sudoers alias, following the mkfs/pbs-apply pattern. The wrapper enforces the two laws E-1 paid for on live hardware so no caller can forget them: F-1 the path must BE the drive's own mountpoint, F-2 is_mountpoint 1 is hardcoded rather than a caller flag. It refuses a root-device target, has NO storage-removal path of any kind (the pbs-apply no-delete law, grep-assertable), is idempotent for the same path, and REFUSES to repoint an existing id. POST /backup/target drives it in a fixed order: create -> grant -> config. Reversed, a config pointing at an ungranted storage 403s every backup on first run -- exactly E-1 finding F-3. A failed grant leaves the config untouched. It deliberately does NOT restart the agent: restarting with a backup in flight cancels the wait and records a spurious tier failure for a backup that actually succeeded (E-1 did this to a real felhom-pbs run). It returns restart_required and the caller restarts behind its own immediate in-flight check. Config rewrite preserves unknown keys verbatim and writes in place, since /etc/felhom-agent is root-owned while agent.json is agent-owned 0600. Green gate: build + vet + test rc=0 (29 packages), run separately from this commit.
175 lines
6.7 KiB
Go
175 lines
6.7 KiB
Go
package localapi
|
|
|
|
import (
|
|
"context"
|
|
"encoding/json"
|
|
"io"
|
|
"log/slog"
|
|
"net/http"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
|
|
"gitea.dooplex.hu/admin/felhom-agent/internal/storage"
|
|
)
|
|
|
|
// wrapperRunner captures the wrapper vector without ever running sudo. The wrapper IS the security
|
|
// boundary, so tests substitute it rather than bypassing it — what is asserted here is the ORDER and
|
|
// the ARGUMENTS the agent sends, which is the agent's half of the contract.
|
|
type wrapperRunner struct {
|
|
calls [][]string
|
|
failOn string // verb to fail, "" = all succeed
|
|
}
|
|
|
|
func (r *wrapperRunner) Run(_ context.Context, name string, args ...string) ([]byte, []byte, error) {
|
|
r.calls = append(r.calls, append([]string{name}, args...))
|
|
if len(args) > 0 && args[0] == r.failOn {
|
|
return nil, []byte("felhom-backup-target-apply: REFUSED: synthetic " + r.failOn + " failure\n"),
|
|
io.ErrUnexpectedEOF
|
|
}
|
|
return nil, nil, nil
|
|
}
|
|
|
|
func (r *wrapperRunner) verbs() []string {
|
|
var out []string
|
|
for _, c := range r.calls {
|
|
if len(c) > 1 {
|
|
out = append(out, c[1])
|
|
}
|
|
}
|
|
return out
|
|
}
|
|
|
|
// moveServer builds a server with /mnt/data mounted on its own device and / on another, plus a
|
|
// throwaway agent.json the move can rewrite.
|
|
func moveServer(t *testing.T, run *wrapperRunner) (http.Handler, string) {
|
|
t.Helper()
|
|
dir := t.TempDir()
|
|
cfgPath := filepath.Join(dir, "agent.json")
|
|
// An UNKNOWN key is deliberately present: the rewrite must preserve it verbatim.
|
|
seed := `{"backup":{"local_backup_target":"local","local_backup_retention":3},"some_future_key":{"keep":"me"}}`
|
|
if err := os.WriteFile(cfgPath, []byte(seed), 0o600); err != nil {
|
|
t.Fatalf("seed config: %v", err)
|
|
}
|
|
hr := fakeHostReader{mounts: []storage.Mount{
|
|
{Device: "/dev/sda1", MountPoint: "/"},
|
|
{Device: "/dev/sdb1", MountPoint: "/mnt/data"},
|
|
}}
|
|
srv, err := NewServer(Options{
|
|
ListenAddr: "127.0.0.1:0",
|
|
Guests: &fakeGuests{}, Backups: &fakeBackups{}, Store: &fakeStore{}, Storage: fakeStorage{},
|
|
Tokens: staticTokens{"A": 8200}, HostReader: hr,
|
|
Privileged: run, ConfigPath: cfgPath, StateDir: dir,
|
|
Logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("new server: %v", err)
|
|
}
|
|
srv.baseCtx = context.Background()
|
|
return srv.Handler(), cfgPath
|
|
}
|
|
|
|
// THE ORDER IS THE CONTRACT: create → grant → config. Reversed, a config pointing at an ungranted
|
|
// storage makes every backup 403 on first run, which is precisely what E-1 hit (finding F-3).
|
|
func TestBackupTargetMoveOrdersCreateThenGrantThenConfig(t *testing.T) {
|
|
run := &wrapperRunner{}
|
|
h, cfgPath := moveServer(t, run)
|
|
|
|
rr := do(t, h, http.MethodPost, "/backup/target", "A", `{"vmid":8200,"where":"/mnt/data"}`)
|
|
if rr.Code != http.StatusOK {
|
|
t.Fatalf("move = %d, body %s", rr.Code, rr.Body.String())
|
|
}
|
|
got := strings.Join(run.verbs(), ",")
|
|
if got != "create,grant" {
|
|
t.Fatalf("wrapper verbs = %q, want create,grant (in that order)", got)
|
|
}
|
|
// The config must have been written only AFTER both wrapper calls succeeded.
|
|
raw, _ := os.ReadFile(cfgPath)
|
|
var doc map[string]json.RawMessage
|
|
if err := json.Unmarshal(raw, &doc); err != nil {
|
|
t.Fatalf("config unreadable after move: %v", err)
|
|
}
|
|
var bk map[string]any
|
|
_ = json.Unmarshal(doc["backup"], &bk)
|
|
if bk["local_backup_target"] != "felhom-backup" {
|
|
t.Errorf("local_backup_target = %v, want felhom-backup", bk["local_backup_target"])
|
|
}
|
|
// Unknown keys preserved verbatim — the property that made E-1's hand edit safe.
|
|
if _, ok := doc["some_future_key"]; !ok {
|
|
t.Error("the rewrite DROPPED an unknown top-level key — a typed round-trip would do this " +
|
|
"and silently discard config this build does not know about")
|
|
}
|
|
// Sibling keys inside `backup` survive too.
|
|
if bk["local_backup_retention"] == nil {
|
|
t.Error("the rewrite dropped local_backup_retention from the backup section")
|
|
}
|
|
}
|
|
|
|
// A FAILED GRANT MUST NOT LEAVE THE CONFIG POINTING AT THE NEW STORAGE. That state is exactly E-1's
|
|
// 403-on-every-backup: the tier looks configured and cannot write.
|
|
func TestBackupTargetMoveDoesNotRepointWhenTheGrantFails(t *testing.T) {
|
|
run := &wrapperRunner{failOn: "grant"}
|
|
h, cfgPath := moveServer(t, run)
|
|
|
|
rr := do(t, h, http.MethodPost, "/backup/target", "A", `{"vmid":8200,"where":"/mnt/data"}`)
|
|
if rr.Code == http.StatusOK {
|
|
t.Fatalf("move SUCCEEDED despite a failed grant (%d)", rr.Code)
|
|
}
|
|
if !strings.Contains(rr.Body.String(), "403") {
|
|
t.Errorf("the error should name the consequence (backups would 403); got %s", rr.Body.String())
|
|
}
|
|
raw, _ := os.ReadFile(cfgPath)
|
|
if strings.Contains(string(raw), "felhom-backup") {
|
|
t.Fatal("the config was repointed at a storage the agent cannot write to — every backup " +
|
|
"would 403 while the tier reported as configured")
|
|
}
|
|
}
|
|
|
|
// It must NOT restart the agent itself. Restarting with a backup in flight cancels the wait and
|
|
// records a spurious tier failure for a backup that actually succeeded — E-1 did exactly that to a
|
|
// felhom-pbs run. Only the caller can re-check in-flight work immediately before restarting.
|
|
func TestBackupTargetMoveReportsRestartRequiredRatherThanRestarting(t *testing.T) {
|
|
run := &wrapperRunner{}
|
|
h, _ := moveServer(t, run)
|
|
rr := do(t, h, http.MethodPost, "/backup/target", "A", `{"vmid":8200,"where":"/mnt/data"}`)
|
|
if !strings.Contains(rr.Body.String(), `"restart_required":true`) {
|
|
t.Errorf("response must tell the caller a restart is required; got %s", rr.Body.String())
|
|
}
|
|
for _, c := range run.calls {
|
|
joined := strings.Join(c, " ")
|
|
if strings.Contains(joined, "systemctl") || strings.Contains(joined, "restart") {
|
|
t.Fatalf("the handler restarted the agent itself: %q — the caller must do it behind its "+
|
|
"own in-flight check", joined)
|
|
}
|
|
}
|
|
}
|
|
|
|
// A path that is not a mountpoint is refused BEFORE sudo is reached (F-1): a subdirectory target
|
|
// reports disconnected forever, and an unmounted path silently retargets onto the system drive.
|
|
func TestBackupTargetMoveRefusesANonMountpoint(t *testing.T) {
|
|
run := &wrapperRunner{}
|
|
h, _ := moveServer(t, run)
|
|
rr := do(t, h, http.MethodPost, "/backup/target", "A", `{"vmid":8200,"where":"/mnt/data/sub"}`)
|
|
if rr.Code == http.StatusOK {
|
|
t.Fatal("a non-mountpoint was accepted as the backup target")
|
|
}
|
|
if len(run.calls) != 0 {
|
|
t.Errorf("a refused request still reached the privileged wrapper: %v", run.calls)
|
|
}
|
|
}
|
|
|
|
// The system disk is refused: a target there protects against corruption only, never drive loss —
|
|
// which is the entire point of the move.
|
|
func TestBackupTargetMoveRefusesTheSystemDisk(t *testing.T) {
|
|
run := &wrapperRunner{}
|
|
h, _ := moveServer(t, run)
|
|
rr := do(t, h, http.MethodPost, "/backup/target", "A", `{"vmid":8200,"where":"/"}`)
|
|
if rr.Code == http.StatusOK {
|
|
t.Fatal("the system disk was accepted as the backup target")
|
|
}
|
|
if len(run.calls) != 0 {
|
|
t.Errorf("a refused request still reached the privileged wrapper: %v", run.calls)
|
|
}
|
|
}
|