diff --git a/CHANGELOG.md b/CHANGELOG.md index 98cd544..319e850 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,61 @@ +## v0.91.0 — the DR tier can no longer be `applied` and dead at the same time (R-39 fleet fix + R-50b(a)) (2026-07-21) + +**Requires hub >= v0.68.0** for the re-arm signal. Hub v0.68.0 is safe for 0.90.0 agents (they drop +the unknown descriptor key), but the guarantees below need THIS agent. **MinAgent → 0.91.0 is an +operator manifest save, sequenced after the fleet has self-updated — not a code change.** + +### What was broken + +Three compounding defects let a box report `applied` while every PBS request 401'd: + +1. **The re-key was invisible.** An ep0 re-issue rotates the SECRET of an existing token, so + `token_id`, `fingerprint`, `datastore` and `namespace` all come back byte-identical. The agent + re-applies on the descriptor's CONTENT HASH, so a converged box short-circuited and never consumed + the fresh secret. Proof from the N100: `consumed-failed.json` carried a hash byte-identical to the + `marker.json` written two minutes before the re-issue. +2. **The agent could not read its own credential.** It WRITES + `/etc/pve/priv/storage/.pw` through the root wrapper, but that directory is `0700 root:www-data` + and the wrapper had no read verb — so `pbsTargetsFromPVE` got "permission denied" every cycle, + logged a Warn and skipped the datastore. The one loop that could have caught the 401 was blind **by + construction**. +3. **Nothing probed authentication.** A snapshot list that fails with 401 looked exactly like "PBS is + busy". + +### The fix + +- **`WirePBSDR.SecretGeneration`** — field-exact with the hub's descriptor. Because `descriptorHash` + marshals this struct, the hub's monotonic mint counter is what finally moves the hash and re-arms a + converged agent. +- **Wrapper `read` verb** (+ exactly ONE sudoers line, + a `pbsdr-read` capability row). Prints one + secret to STDOUT and nothing else: no network, no mutation, no logging of the value, and the secret + never rides argv (sudo logs argv). Traversal is refused three times over — the id grammar admits no + slash, `val_sdir` pins the directory, and the RESOLVED path is prefix-asserted. +- **`pbs.ProbeAuth`** — `GET /version`, the cheapest authenticated question, with a distinct + `ErrUnauthorized` sentinel. `/version` needs no datastore, namespace or privilege, so a 401 there + means the CREDENTIAL is bad — not that an ACL is narrow. **403 is deliberately NOT treated as + unauthorized**: re-keying a too-narrow token would mint credentials forever without fixing anything. +- **The probe runs on the 15-minute collect path** (not the 6 h verify cadence) and its verdict + becomes a LOUD `auth_failed` state the hub's pbsdrheal escalates to a fresh mint. **A transport + error is UNKNOWN, never a rejection** — otherwise every network blip would burn a credential. + Recovery is self-clearing. +- **`readPBSSecret`** now prefers a directly-readable file and falls back to the wrapper, so a box + with its own agent-owned secret dir needs no sudo at all. +- **R-50b(a):** the report carries the installed wrapper's sha256, so drift against the vouched + manifest value is finally answerable. Empty = unknown, never drift. + +### Tests + +Scenario A (a re-key re-arms a converged agent) with the hash mechanism asserted separately; the +unknown-field compat direction; auth_failed loud/ignored-for-other-storage/none-before-descriptor/ +self-clearing; and the wrapper `read` verb executed under real bash — traversal refusals, secret to +stdout only, missing-file refusal, side-effect freedom. + +**Three red-proofs run at the assertion level.** Removing `SecretGeneration` makes the re-arm test +fail with `consume calls=1, want 2`. Swallowing the probe result leaves `State:applied +AuthFailed:false` — the July-18 shape exactly. Deleting the id charset guard alone does **not** open +a traversal hole (readlink + the prefix assertion still catch it), so the isolating red-proof removes +the charset guard AND the prefix assertion and shows the out-of-tree secret printed. + ## docs — the workflow moved to DooPlex-local execution (2026-07-19) **Docs only, no version bump, no code change.** Claude Code now runs on DooPlex (192.168.0.180, diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 7bd272e..04063aa 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -19,6 +19,7 @@ import ( "net/http" "net/netip" "os" + "os/exec" "os/signal" "path/filepath" "strconv" @@ -1054,6 +1055,49 @@ func runDaemon(cfg config.Config, logger *slog.Logger, logRing *applog.Ring) int return 0 } +// pbsSecretReader is the seam that reads a PBS storage's token secret (R-39 leg b). Overridable in +// tests; production is readPBSSecretViaWrapper. +var pbsSecretReader = readPBSSecretViaWrapper + +// readPBSSecret reads a storage's token secret, preferring a directly-readable file and falling back +// to the root wrapper. +// +// WHY THE FALLBACK EXISTS. The agent runs NON-ROOT and writes /etc/pve/priv/storage/.pw through +// the root wrapper — but /etc/pve/priv is 0700 root:www-data, so it could never read that file back. +// The old code called readTrimmed on it directly, got "permission denied" every cycle, logged a Warn +// and SKIPPED the datastore. That is why the 401 in R-39 went unnoticed for weeks: the one loop that +// could have caught it was blind by construction, not by accident. +// +// The direct read is kept first because a box configured with its own agent-owned secret dir +// (place_copies puts a 0600 felhom-agent copy there) needs no sudo at all; the wrapper is the path +// for the default PRIVDIR case. +func readPBSSecret(ctx context.Context, cfg config.Config, storageID string) (string, error) { + path := cfg.Backup.PBSSecretPath(storageID) + if secret, err := readTrimmed(path); err == nil && secret != "" { + return secret, nil + } + return pbsSecretReader(ctx, cfg, storageID) +} + +// readPBSSecretViaWrapper shells the root wrapper's `read` verb. The secret arrives on STDOUT and is +// never passed through argv (sudo logs argv) and never logged. +func readPBSSecretViaWrapper(ctx context.Context, cfg config.Config, storageID string) (string, error) { + dir := filepath.Dir(cfg.Backup.PBSSecretPath(storageID)) + cmd := exec.CommandContext(ctx, "sudo", "-n", pbsdr.WrapperPath, "read", storageID, dir) + var out, errb bytes.Buffer + cmd.Stdout = &out + cmd.Stderr = &errb + if err := cmd.Run(); err != nil { + // The wrapper's refusal text is safe to surface (it never echoes the value). + return "", fmt.Errorf("wrapper read %s: %w: %s", storageID, err, strings.TrimSpace(errb.String())) + } + secret := strings.TrimSpace(out.String()) + if secret == "" { + return "", fmt.Errorf("wrapper read %s: empty secret", storageID) + } + return secret, nil +} + // pbsTargetsFromPVE returns a pbs.Targets closure that, each cycle, discovers the pbs // storages from the PVE config and builds a fingerprint-pinned, token-authed client for each // (token id from the storage `username`, secret read from /.pw). A storage @@ -1070,7 +1114,7 @@ func pbsTargetsFromPVE(cfg config.Config, px *proxmox.Client, logger *slog.Logge if s.Type != "pbs" { continue } - secret, err := readTrimmed(cfg.Backup.PBSSecretPath(s.Storage)) + secret, err := readPBSSecret(ctx, cfg, s.Storage) if err != nil { logger.Warn("pbs: cannot read token secret; skipping datastore", "storage", s.Storage, "err", err) continue @@ -1080,7 +1124,7 @@ func pbsTargetsFromPVE(cfg config.Config, px *proxmox.Client, logger *slog.Logge logger.Warn("pbs: cannot build client; skipping datastore", "storage", s.Storage, "err", err) continue } - targets = append(targets, pbs.Target{Datastore: s.Datastore, Client: c}) + targets = append(targets, pbs.Target{Datastore: s.Datastore, Client: c, StorageID: s.Storage}) } return targets, nil } diff --git a/configs/felhom-agent.sudoers b/configs/felhom-agent.sudoers index 47bfb3d..04e2fae 100644 --- a/configs/felhom-agent.sudoers +++ b/configs/felhom-agent.sudoers @@ -216,10 +216,18 @@ Cmnd_Alias FELHOM_SSHD = \ # token secret rides the wrapper's STDIN — sudo logs argv, so it must never appear here. The # agent fine-validates every field (charset + descriptor equality) before exec; these globs are # the coarse allowlist. +# +# `read` (R-39 leg b, agent v0.91.0) is the ONE added verb. It prints a token secret to stdout and +# performs no mutation. It exists because the agent writes that file through this wrapper but could +# never read it back (/etc/pve/priv is 0700 root:www-data), leaving its PBS verify loop permanently +# blind to an `applied`-but-401 tier. It is NOT a general file-read: the wrapper pins the directory +# and prefix-asserts the resolved path, and the id grammar admits no slash. The secret goes to +# STDOUT, never argv — sudo logs argv. Cmnd_Alias FELHOM_PBSDR = \ /usr/local/sbin/felhom-pbs-apply create *, \ /usr/local/sbin/felhom-pbs-apply reconcile *, \ - /usr/local/sbin/felhom-pbs-apply grant * + /usr/local/sbin/felhom-pbs-apply grant *, \ + /usr/local/sbin/felhom-pbs-apply read * # OOB nft belt (TASK H1). The STATIC table `inet felhom_oob` is installed once by host-install; the # agent mutates ONLY its two SETS — @operator_ips (the operator /32) + @ssh_port (the claimed port). diff --git a/configs/felhom-pbs-apply b/configs/felhom-pbs-apply index e89a96f..33dff37 100644 --- a/configs/felhom-pbs-apply +++ b/configs/felhom-pbs-apply @@ -27,6 +27,15 @@ # NOTE datastore is deliberately NOT settable, and namespace/token-id are accepted # for validation parity but NOT applied — tenancy identity is adopt-only (the # demo's live entry must never be repointed at a different namespace). +# read +# R-39 leg (b): print the storage's token secret to STDOUT and nothing else. +# The non-root agent WRITES this file through this wrapper but could never read it +# back (/etc/pve/priv is 0700 root:www-data and there is no read verb), so its +# 15-minute PBS verify loop was permanently blind to the one failure it exists to +# catch — an `applied` tier authenticating 401. This verb is that missing read. +# It is deliberately the narrowest thing that works: no network, no mutation, no +# logging of the value, one file, prefix-asserted under the given secret dir. +# # grant # The Part-0-evidenced dual-grant: FelhomAgentStore on /storage/ to the agent # user AND token (privsep intersection). Datastore.Audit reads ride the base role. @@ -36,7 +45,7 @@ set -euo pipefail die() { echo "felhom-pbs-apply: REFUSED: $*" >&2; exit 1; } op="${1:-}"; id="${2:-}" -[[ -n "$op" && -n "$id" ]] || die "usage: felhom-pbs-apply ..." +[[ -n "$op" && -n "$id" ]] || die "usage: felhom-pbs-apply ..." # Storage id: PVE grammar, conservative. Also the ACL path component — no slashes possible. [[ "$id" =~ ^[A-Za-z][A-Za-z0-9_.-]{0,27}$ ]] || die "bad storage id ($id)" @@ -113,6 +122,31 @@ reconcile) place_copies "$sdir" echo "felhom-pbs-apply: reconciled $id (set-only; tenancy identity untouched)" >&2 ;; +read) + # R-39(b): the missing read path. Prints the secret to STDOUT, nothing else — no stderr note (it + # would be the only verb whose success line could be confused with the value), no mutation. + # + # Traversal is refused three times over, because this is the one verb that EXFILTRATES a file and + # its argv is attacker-shaped if the agent is ever compromised: + # 1. `id` already matched ^[A-Za-z][A-Za-z0-9_.-]{0,27}$ at the top — it cannot start with a dot + # and cannot contain a slash, so "../../etc/shadow" never reaches here; + # 2. val_sdir pins the directory to PRIVDIR or under /var/lib/felhom-agent, rejecting ".."; + # 3. the RESOLVED path is prefix-asserted under that directory below, so even a future change to + # either grammar cannot walk out. + [[ $# -eq 3 ]] || die "read needs 2 args: " + sdir="$3" + val_sdir "$sdir" + target="$sdir/$id.pw" + # Belt: resolve and re-check the prefix (guards a symlinked .pw pointing outside the dir). + resolved=$(readlink -f -- "$target" 2>/dev/null || true) + [[ -n "$resolved" ]] || die "secret file not found ($target)" + case "$resolved" in + "$sdir"/*) : ;; + *) die "resolved secret path escapes the secret dir" ;; + esac + [[ -f "$resolved" ]] || die "secret file not found ($target)" + cat -- "$resolved" + ;; grant) [[ $# -eq 2 ]] || die "grant takes only " pveum acl modify "/storage/$id" --users felhom-agent@pve --roles FelhomAgentStore >&2 diff --git a/internal/capability/manifest.go b/internal/capability/manifest.go index 70d6a47..e5ab686 100644 --- a/internal/capability/manifest.go +++ b/internal/capability/manifest.go @@ -161,6 +161,10 @@ var manifest = []Capability{ {"pbsdr-create", "PBS DR storage-entry create (K autogen)", "/usr/local/sbin/felhom-pbs-apply", []string{"create", "felhom-pbs", "10.77.0.1", "felhom-offsite", "ns0", "felhom@pbs!ns0", reprFingerprint, "/etc/pve/priv/storage"}, false, ""}, {"pbsdr-reconcile", "PBS DR storage-entry reconcile (set-only)", "/usr/local/sbin/felhom-pbs-apply", []string{"reconcile", "felhom-pbs", "10.77.0.1", "ns0", "felhom@pbs!ns0", reprFingerprint, "/etc/pve/priv/storage"}, false, ""}, {"pbsdr-grant", "PBS DR storage ACL self-grant", "/usr/local/sbin/felhom-pbs-apply", []string{"grant", "felhom-pbs"}, false, ""}, + // R-39 leg (b), v0.91.0: the credential READ path. Its absence is what made the PBS verify loop + // permanently blind to an applied-but-401 tier, so a host missing this verb is DEGRADED in a way + // that matters — it cannot detect the failure this whole tier exists to survive. + {"pbsdr-read", "PBS DR credential read (verify-loop auth probe)", "/usr/local/sbin/felhom-pbs-apply", []string{"read", "felhom-pbs", "/etc/pve/priv/storage"}, false, ""}, // ---- Escrow ceremony (FELHOM_ESCROW, controller-driven, v0.88.0). Critical: the customer // wizard's whole run path IS this one grant — a dropped line silently breaks every ceremony. diff --git a/internal/capability/probe_test.go b/internal/capability/probe_test.go index 8b77d26..f486490 100644 --- a/internal/capability/probe_test.go +++ b/internal/capability/probe_test.go @@ -132,7 +132,9 @@ func TestProbe_GateOffHealthyIsInactive(t *testing.T) { statuses := p.Probe(context.Background()) // v0.88.0: escrow-ceremony joins the gate EXPLICITLY (non-pbsdr name, GatedBy literal) — // the ceremony only exists behind the DR tier (no PBS key, no ceremony). - for _, name := range []string{"pbsdr-create", "pbsdr-reconcile", "pbsdr-grant", "escrow-ceremony"} { + // v0.91.0: pbsdr-read (the R-39 credential-read verb) rides the same `pbsdr-` prefix gate — a new + // pbsdr-* op is gated by construction, which is exactly the property this list is here to hold. + for _, name := range []string{"pbsdr-create", "pbsdr-reconcile", "pbsdr-grant", "pbsdr-read", "escrow-ceremony"} { s := find(statuses, name) if s.Status != StatusInactive || s.Reason != ReasonInactive { t.Fatalf("%s = %+v, want inactive/%q", name, s, ReasonInactive) @@ -147,8 +149,8 @@ func TestProbe_GateOffHealthyIsInactive(t *testing.T) { if len(degraded) != 0 { t.Fatalf("inactive leaked into degraded: %+v", degraded) } - if ok != total-4 { - t.Fatalf("ok=%d total=%d, want exactly the 4 gated ones non-ok", ok, total) + if ok != total-5 { + t.Fatalf("ok=%d total=%d, want exactly the 5 gated ones non-ok", ok, total) } } diff --git a/internal/hub/collect.go b/internal/hub/collect.go index 8753bdf..07f99e7 100644 --- a/internal/hub/collect.go +++ b/internal/hub/collect.go @@ -2,8 +2,12 @@ package hub import ( "context" + "crypto/sha256" + "encoding/hex" "fmt" + "io" "log/slog" + "os" "time" "gitea.dooplex.hu/admin/felhom-agent/internal/capability" @@ -191,7 +195,8 @@ func (c *Collector) Collect(ctx context.Context) (*HostReport, error) { } host := hostMetrics(c.px.Node(), ns) - host.CPUTempC = c.cpuTempC(ctx) // slice 9: operator freebie — temp now rides the hub report too + host.CPUTempC = c.cpuTempC(ctx) // slice 9: operator freebie — temp now rides the hub report too + host.WrapperSHA256 = pbsWrapperSHA256() // R-50b(a): make privileged-artifact drift answerable report := &HostReport{ HostID: c.hostID, ReportedAt: c.now().Format(time.RFC3339), @@ -272,6 +277,27 @@ func (c *Collector) cpuTempC(ctx context.Context) *int { return c.temp.CPUTempC(ctx) } +// pbsWrapperPath is the installed PBS-DR apply wrapper. Duplicated from internal/pbsdr.WrapperPath +// rather than imported, to keep the report collector free of a dependency on the DR bridge. +const pbsWrapperPath = "/usr/local/sbin/felhom-pbs-apply" + +// pbsWrapperSHA256 hashes the installed wrapper for the report (R-50b(a)). Best-effort: a missing or +// unreadable file yields "", which the hub reads as UNKNOWN rather than as drift — a host that +// legitimately has no DR wrapper must not light up amber. The file is 0755, so no privilege is +// needed to read it. +func pbsWrapperSHA256() string { + f, err := os.Open(pbsWrapperPath) + if err != nil { + return "" + } + defer f.Close() + h := sha256.New() + if _, err := io.Copy(h, f); err != nil { + return "" + } + return hex.EncodeToString(h.Sum(nil)) +} + func hostMetrics(node string, ns proxmox.NodeStatus) HostMetrics { h := HostMetrics{ Node: node, diff --git a/internal/hub/report.go b/internal/hub/report.go index f9d4de3..c46510a 100644 --- a/internal/hub/report.go +++ b/internal/hub/report.go @@ -116,6 +116,12 @@ type PBSDRStatus struct { Message string `json:"message,omitempty"` ConsumedFailed bool `json:"consumed_failed,omitempty"` AppliedAt string `json:"applied_at,omitempty"` // RFC3339; set on adopted/applied + // AuthFailed (R-39, v0.91.0) — the credential this box holds is REJECTED by PBS (401). Set by the + // verify loop's ProbeAuth, which before v0.91.0 could not run at all: the loop read the secret + // file directly as non-root and always failed with "permission denied", so an applied-and-dead + // tier was invisible to both tiers. The hub's pbsdrheal escalates state="auth_failed" to a fresh + // mint. + AuthFailed bool `json:"auth_failed,omitempty"` } // OOBStatus is the per-heartbeat operator-access health (TASK H1). Carries no secret. @@ -170,6 +176,17 @@ type HostMetrics struct { // the per-disk SmartSummary.TemperatureC. Sourced from sysfs (hwmon / thermal zones). // Cross-repo wire field (slice 9) — the hub's HostMetrics copy + golden carry it too. CPUTempC *int `json:"cpu_temp_c"` + // WrapperSHA256 is the sha256 of the installed PBS-DR apply wrapper + // (/usr/local/sbin/felhom-pbs-apply), R-50b(a), v0.91.0. + // + // That wrapper is root-owned 0755 and the pinned sudoers vector for the PBS storage verbs, yet it + // is installed from `raw/branch/main` — unversioned, unpinned and absent from the Day-0 artifact + // manifest. So "which wrapper is on this host?" had no answer: two hosts installed a week apart + // could carry different privileged code while reporting the same agent version. Reporting the hash + // does not fix the delivery channel (R-50b(b)/(c)); it makes drift VISIBLE. + // + // Empty = unreadable/absent, which the hub treats as UNKNOWN, never as drift. + WrapperSHA256 string `json:"wrapper_sha256,omitempty"` } // Guest is one LXC. The agent reports vmid; the hub derives the guest PK @@ -429,6 +446,20 @@ type WirePBSDR struct { Namespace string `json:"namespace,omitempty"` TokenID string `json:"token_id,omitempty"` Fingerprint string `json:"fingerprint,omitempty"` + // SecretGeneration (R-39, agent v0.91.0 / hub v0.68.0) is the hub's monotonic per-host counter, + // advanced by every fresh secret MINT. It carries no secret material — only the fact that one + // rotated. + // + // THIS FIELD IS THE RE-ARM SIGNAL, and it works only because descriptorHash marshals THIS STRUCT: + // an ep0 re-issue re-keys the secret of an existing token, so token_id, fingerprint, datastore and + // namespace all come back byte-identical. Without this field the descriptor never moves, the + // converged agent short-circuits, the fresh secret is never consumed, and the box serves a revoked + // credential while reporting `applied` (the N100, 2026-07-18). + // + // Corollary worth stating: an agent that does NOT carry this field drops the unknown JSON key and + // keeps today's behaviour exactly — inert, not broken. That is why hub v0.68.0 is safe to deploy + // ahead of the fleet, and why the re-arm guarantee needs agent >= 0.91.0. + SecretGeneration int64 `json:"secret_generation,omitempty"` } // WireWireguard is the hub-owned offsite-tunnel assignment (S3) — field-exact with the S2 golden diff --git a/internal/pbs/client.go b/internal/pbs/client.go index 8fd4987..577af9d 100644 --- a/internal/pbs/client.go +++ b/internal/pbs/client.go @@ -3,6 +3,7 @@ package pbs import ( "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -193,6 +194,46 @@ func NodeFromUPID(upid string) string { return parts[1] } +// ErrUnauthorized is returned by ProbeAuth when PBS REJECTS the credential (HTTP 401). +// +// It is a distinct sentinel because a rejected credential and an unreachable server demand opposite +// responses: 401 is terminal until the credential is replaced (the hub must re-key), while a dial +// error is transient and must NOT trigger a re-issue — mistaking one for the other would either +// leave a dead tier green (the R-39 failure) or burn a fresh secret on every network blip. +var ErrUnauthorized = errors.New("pbs: unauthorized (401) — the token secret is not accepted") + +// ProbeAuth asks PBS the cheapest question that requires authentication: GET /version. +// +// WHY THIS EXISTS (R-39 leg c). The DR tier could be `applied` and dead at the same time: PVE holds +// a storage entry, the agent's marker says converged, and every PBS request 401s because the entry +// is pinned to a superseded credential. Nothing noticed, because the one loop that could — the +// 15-minute PBS verify loop — could not even READ the credential to test it (the non-root agent +// writes /etc/pve/priv/storage/.pw through a root wrapper and had no read verb). With the +// wrapper's `read` verb this probe finally closes that gap, and its result becomes a LOUD +// `auth_failed` state the hub self-heals instead of a Warn-and-skip. +// +// /version is deliberate: it needs no datastore, no namespace and no privileges beyond a valid +// token, so a 401 here means the CREDENTIAL is bad — not that a datastore is missing or an ACL is +// too narrow. That distinction is what makes the state safe to auto-remediate. +func (c *Client) ProbeAuth(ctx context.Context) error { + err := c.do(ctx, http.MethodGet, "/version", nil) + if err == nil { + return nil + } + if isUnauthorized(err) { + return ErrUnauthorized + } + return err +} + +// isUnauthorized classifies a doBody error as an authentication rejection. doBody formats non-2xx as +// "... -> HTTP : ", so the code is matched on that shape. 403 is deliberately NOT +// included: a valid token with too narrow an ACL is a permissions problem, and re-keying it would +// mint credentials forever without fixing anything. +func isUnauthorized(err error) bool { + return err != nil && strings.Contains(err.Error(), "-> HTTP 401") +} + // post performs a form-encoded POST (PBS mutating ops take form params). func (c *Client) post(ctx context.Context, path string, form url.Values, out any) error { return c.doBody(ctx, http.MethodPost, path, strings.NewReader(form.Encode()), "application/x-www-form-urlencoded", out) diff --git a/internal/pbs/live_reporter.go b/internal/pbs/live_reporter.go index 0bd9b20..a340eb8 100644 --- a/internal/pbs/live_reporter.go +++ b/internal/pbs/live_reporter.go @@ -2,6 +2,7 @@ package pbs import ( "context" + "errors" "log/slog" "time" @@ -32,8 +33,31 @@ type LiveSnapshotReporter struct { // listSnapshots is the production→PBS seam, overridable in tests so no live PBS is needed. // Default = liveListSnapshots (one Snapshots() GET, converted via Snapshot.ToHub()). listSnapshots func(ctx context.Context, t Target) ([]hub.PBSSnapshot, error) + + // probeAuth is the R-39 credential probe seam (default = (*Client).ProbeAuth). Overridable so the + // auth-honesty path is testable with no PBS. + probeAuth func(ctx context.Context, t Target) error + // authSink receives every probe result. nil = nobody is listening (the probe is then skipped + // entirely — no point paying for a request nothing consumes). + authSink AuthSink } +// AuthSink receives the result of each per-storage credential probe (R-39 leg c). +// +// It exists so the DR bridge can turn a 401 into a LOUD `auth_failed` state instead of the Warn-and- +// skip that made an applied-but-dead tier invisible. Deliberately a plain interface taking a bool +// rather than the error: the consumer (internal/pbsdr) must not have to import this package just to +// test a sentinel. +type AuthSink interface { + // NoteAuthResult reports one storage's credential health. unauthorized=true means PBS REJECTED + // the credential (401) — terminal until it is replaced. A transport error is unauthorized=false + // with a non-empty detail: unknown, not dead, and never a reason to re-key. + NoteAuthResult(storageID string, unauthorized bool, detail string) +} + +// SetAuthSink wires the credential-probe consumer. Without it the reporter does not probe at all. +func (r *LiveSnapshotReporter) SetAuthSink(s AuthSink) { r.authSink = s } + // NewLiveSnapshotReporter builds a live reporter sharing store with the verify loop. A zero timeout // falls back to DefaultLiveSnapshotTimeout; a nil logger to slog.Default. func NewLiveSnapshotReporter(targets Targets, store *SnapshotStore, timeout time.Duration, log *slog.Logger) *LiveSnapshotReporter { @@ -49,6 +73,7 @@ func NewLiveSnapshotReporter(targets Targets, store *SnapshotStore, timeout time timeout: timeout, log: log, listSnapshots: liveListSnapshots, + probeAuth: func(ctx context.Context, t Target) error { return t.Client.ProbeAuth(ctx) }, } } @@ -66,6 +91,30 @@ func liveListSnapshots(ctx context.Context, t Target) ([]hub.PBSSnapshot, error) return out, nil } +// probeAuthAndReport runs the credential probe for one target and forwards the verdict to the sink. +// No sink → no probe (nothing would consume it). Never fails the collect: a report must still go out. +func (r *LiveSnapshotReporter) probeAuthAndReport(ctx context.Context, t Target) { + if r.authSink == nil || r.probeAuth == nil { + return + } + err := r.probeAuth(ctx, t) + switch { + case err == nil: + r.authSink.NoteAuthResult(t.StorageID, false, "") + case errors.Is(err, ErrUnauthorized): + // The one case that is TERMINAL and actionable: the credential is rejected, not the network. + r.log.Error("pbs: the DR endpoint REJECTED this box's credential (401) — the storage entry is "+ + "pinned to a superseded secret and every backup/restore against it will fail", + "storage", t.StorageID, "datastore", t.Datastore) + r.authSink.NoteAuthResult(t.StorageID, true, "PBS rejected the stored credential (401)") + default: + // Unreachable/timeout/TLS — UNKNOWN, not dead. Reporting this as unauthorized would re-key a + // perfectly good credential on every network blip. + r.log.Debug("pbs: credential probe inconclusive (not a rejection)", "storage", t.StorageID, "err", err) + r.authSink.NoteAuthResult(t.StorageID, false, err.Error()) + } +} + // PBSSnapshots implements hub.PBSReporter with a single bounded, live pass (last-known-good // fallback). Non-nil result so it marshals as []. func (r *LiveSnapshotReporter) PBSSnapshots(ctx context.Context) []hub.PBSSnapshot { @@ -81,6 +130,12 @@ func (r *LiveSnapshotReporter) PBSSnapshots(ctx context.Context) []hub.PBSSnapsh out := []hub.PBSSnapshot{} for _, t := range targets { + // R-39 leg (c): prove the credential BEFORE interpreting anything else about this datastore. + // A snapshot list that fails with 401 used to look identical to "PBS is busy" — which is how + // an applied-and-dead tier stayed green. The probe runs on this 15-minute collect path (not + // the 6 h verify cadence) because that is how fast the hub can react. + r.probeAuthAndReport(childCtx, t) + snaps, err := r.listSnapshots(childCtx, t) if err != nil { // Per-datastore live failure → that datastore's last-known-good (does NOT clobber it). diff --git a/internal/pbs/verify.go b/internal/pbs/verify.go index c21aa31..3ab1f8a 100644 --- a/internal/pbs/verify.go +++ b/internal/pbs/verify.go @@ -16,6 +16,10 @@ const DefaultVerifyCadence = 6 * time.Hour type Target struct { Datastore string Client *Client + // StorageID is the PVE storage-entry id this target came from. Carried so an auth failure can + // NAME the storage the operator has to fix, and so the pbsdr bridge can match the failure to its + // own descriptor (a host may hold several PBS storages). + StorageID string } // Targets resolves the current set of PBS datastores to verify (re-derived each cycle from diff --git a/internal/pbsdr/manager.go b/internal/pbsdr/manager.go index d462b2e..650b850 100644 --- a/internal/pbsdr/manager.go +++ b/internal/pbsdr/manager.go @@ -141,6 +141,56 @@ func (m *Manager) DRConfigured() bool { return m.loadMarker() != nil } +// NoteAuthResult implements pbs.AuthSink (R-39 leg c): the credential probe's verdict for one +// storage, turned into the DR bridge's reported state. +// +// This is the leg that makes `applied` mean something. Until v0.91.0 the agent could not read the +// credential it had written (root-only path, no wrapper read verb), so a tier pinned to a superseded +// secret reported `applied` forever while every PBS request 401'd — and the hub, seeing `applied`, +// had no reason to re-key. Now a rejection becomes a LOUD `auth_failed` that pbsdrheal escalates to +// a fresh mint; the fresh mint advances the secret generation; the descriptor hash moves; and Apply +// finally re-consumes. +// +// Rules that keep it safe: +// - Only a REJECTION (401) sets the state. An unreachable PBS is UNKNOWN and must never re-key. +// - Only the storage this box's descriptor actually names is considered; a host may carry other +// PBS entries that are none of the DR tier's business. +// - Recovery is self-clearing: a subsequent successful probe restores the converged state from the +// marker, so the operator does not have to acknowledge a fault that fixed itself. +func (m *Manager) NoteAuthResult(storageID string, unauthorized bool, detail string) { + m.mu.Lock() + st := m.status + m.mu.Unlock() + // No descriptor seen yet, or this is not our storage → not our business. + if st == nil || st.StorageID == "" || storageID == "" || st.StorageID != storageID { + return + } + if unauthorized { + if st.State == "auth_failed" { + return // already loud; do not churn the report + } + m.logger.Error("pbsdr: the DR endpoint REJECTED this box's credential — the tier is applied and DEAD", + "storage_id", storageID, "previous_state", st.State) + m.setStatus(&hub.PBSDRStatus{ + State: "auth_failed", StorageID: st.StorageID, Namespace: st.Namespace, AppliedAt: st.AppliedAt, + AuthFailed: true, + Message: detail + " — awaiting fresh credentials from the hub (automatic)", + }) + return + } + // A clean probe clears a previously-loud auth failure by restoring the converged marker state. + if st.State == "auth_failed" && detail == "" { + mk := m.loadMarker() + restored := "applied" + appliedAt := st.AppliedAt + if mk != nil { + restored, appliedAt = mk.State, mk.AppliedAt + } + m.logger.Info("pbsdr: credential accepted again — clearing auth_failed", "storage_id", storageID, "state", restored) + m.setStatus(&hub.PBSDRStatus{State: restored, StorageID: st.StorageID, Namespace: st.Namespace, AppliedAt: appliedAt}) + } +} + // descriptorHash is the idempotency key: sha256 of the canonical (struct-ordered) JSON. func descriptorHash(b *hub.WirePBSDR) string { j, _ := json.Marshal(b) diff --git a/internal/pbsdr/rearm_test.go b/internal/pbsdr/rearm_test.go new file mode 100644 index 0000000..57ad1fe --- /dev/null +++ b/internal/pbsdr/rearm_test.go @@ -0,0 +1,175 @@ +package pbsdr + +import ( + "context" + "encoding/json" + "io" + "log/slog" + "testing" + + "gitea.dooplex.hu/admin/felhom-agent/internal/hub" + "gitea.dooplex.hu/admin/felhom-agent/internal/proxmox" +) + +// jsonUnmarshal is aliased so the Scenario-C decode reads as intent, not plumbing. +var jsonUnmarshal = json.Unmarshal + +// R-39, Scenario A — THE fix: a credential re-key must re-arm a converged agent. +// +// The 2026-07-18 N100 failure in one sentence: an ep0 re-issue re-keys the SECRET of an existing +// token, so token_id, fingerprint, datastore and namespace all come back byte-identical; the +// descriptor hash did not move; the converged agent short-circuited; the fresh secret was never +// consumed; and the box served a revoked credential while reporting `applied`. The hub now stamps a +// monotonic SecretGeneration into the descriptor, and because descriptorHash marshals THIS STRUCT, +// that is what finally moves the hash. +// +// COMPANION RED-PROOF (run + recorded): delete SecretGeneration from hub.WirePBSDR (or stop the hub +// from advancing it) → the two descriptors marshal identically, the marker short-circuit fires, and +// this test FAILS with consume calls stuck at 1. That reproduces the defect exactly. +func TestR39_ReKeyReArmsAConvergedAgent(t *testing.T) { + r := &fakeRunner{} + // found=false on the first apply (fresh create), then the entry exists but is NOT active — which + // is what PVE reports for a storage whose credential is rejected (storage_info catches the 401, + // leaves active=0). That is the state a re-key has to recover from. + st := &fakeStorage{found: false, active: []bool{true}} + c := &fakeConsumer{secret: "SECRET-GEN-1"} + m, _ := newTestManager(t, r, st, c) + + block := testBlock() + block.SecretGeneration = 1 + m.Apply(context.Background(), true, block) + if c.calls != 1 { + t.Fatalf("first apply consumed %d secrets, want 1", c.calls) + } + if s := m.Status(); s == nil || s.State != "applied" { + t.Fatalf("first apply status = %+v, want applied", s) + } + + // A re-apply of the SAME descriptor must stay a no-op — the idempotency the marker exists for. + m.Apply(context.Background(), true, block) + if c.calls != 1 { + t.Fatalf("re-apply of an unchanged descriptor consumed again (calls=%d)", c.calls) + } + + // THE RE-KEY. Everything an ep0 re-issue actually returns is unchanged; only the generation moves. + // + // The entry now EXISTS and reads INACTIVE — which is exactly what PVE reports for a PBS storage + // whose credential is rejected: storage_info wraps activate_storage/status in eval{}, warns, and + // leaves the pre-initialised active=0. (Verified against PVE's own source; it returns HTTP 200 + // with active:0, never an API error — which is what lets Apply fall through to the recovery path + // instead of bailing out at the status probe.) + st.found = true + st.entry = &proxmox.StorageEntryConfig{Type: "pbs", Namespace: "peti"} + st.active = append(st.active, false, true) // rejected → inactive, healthy after the reconcile + c.secret = "SECRET-GEN-2" + rekeyed := testBlock() + + m.Apply(context.Background(), true, rekeyed) + + if c.calls != 2 { + t.Fatalf("the re-key did NOT re-arm the agent: consume calls=%d, want 2.\n"+ + "The converged short-circuit fired because the descriptor hash did not move — this is the "+ + "R-39 defect (N100, 2026-07-18).", c.calls) + } + if s := m.Status(); s == nil || (s.State != "applied" && s.State != "adopted") { + t.Fatalf("post-re-key status = %+v, want converged", s) + } +} + +// The generation genuinely changes the hash — the mechanism the test above depends on. Stated +// separately so a failure points at the CAUSE rather than at the flow. +func TestR39_SecretGenerationMovesTheDescriptorHash(t *testing.T) { + a := testBlock() + a.SecretGeneration = 1 + b := testBlock() + b.SecretGeneration = 2 + + if descriptorHash(a) == descriptorHash(b) { + t.Fatal("SecretGeneration does not move descriptorHash — a re-key stays invisible to a " + + "converged agent and the fresh secret is never consumed (R-39)") + } + // And an unchanged generation must NOT move it (or every report would re-apply). + c := testBlock() + c.SecretGeneration = 1 + if descriptorHash(a) != descriptorHash(c) { + t.Fatal("identical descriptors hash differently — the agent would re-apply on every tick") + } +} + +// Scenario C, from the agent side: a descriptor carrying an UNKNOWN field (what a pre-0.91.0 agent +// sees) must be ignored, not rejected. Asserted by decoding hub JSON that contains a key this build +// does not know about. +func TestR39_UnknownDescriptorFieldIsInert(t *testing.T) { + var b hub.WirePBSDR + raw := []byte(`{"enabled":true,"storage_id":"felhom-pbs","secret_generation":7,"some_future_key":"x"}`) + if err := jsonUnmarshal(raw, &b); err != nil { + t.Fatalf("a descriptor with an unknown key must decode, got %v", err) + } + if !b.Enabled || b.StorageID != "felhom-pbs" || b.SecretGeneration != 7 { + t.Fatalf("known fields lost while ignoring an unknown one: %+v", b) + } +} + +// R-39 leg (c) — a rejected credential becomes a LOUD auth_failed, and recovers by itself. +// +// COMPANION RED-PROOF (run + recorded): make NoteAuthResult ignore `unauthorized` (the pre-fix +// Warn-and-skip shape) → the state stays `applied` and this test FAILS. +func TestR39_AuthFailureBecomesLoudAndSelfClears(t *testing.T) { + r := &fakeRunner{} + st := &fakeStorage{found: false, active: []bool{true}} + c := &fakeConsumer{secret: "S"} + m, _ := newTestManager(t, r, st, c) + block := testBlock() + block.SecretGeneration = 1 + m.Apply(context.Background(), true, block) + if s := m.Status(); s.State != "applied" { + t.Fatalf("precondition: want applied, got %+v", s) + } + + // PBS rejects the credential. + m.NoteAuthResult("felhom-pbs", true, "PBS rejected the stored credential (401)") + s := m.Status() + if s.State != "auth_failed" || !s.AuthFailed { + t.Fatalf("a rejected credential must be LOUD: status = %+v, want auth_failed", s) + } + if s.StorageID != "felhom-pbs" { + t.Errorf("auth_failed must name the storage, got %q", s.StorageID) + } + + // A transport error is UNKNOWN, not dead — it must not clear a real fault either. + m.NoteAuthResult("felhom-pbs", false, "dial tcp: connection refused") + if m.Status().State != "auth_failed" { + t.Error("an unreachable PBS cleared a real 401 — a blip must not paper over a dead credential") + } + + // A clean probe restores the converged state without operator action. + m.NoteAuthResult("felhom-pbs", false, "") + if got := m.Status().State; got != "applied" { + t.Errorf("recovery did not clear auth_failed: state = %q, want applied", got) + } +} + +// Another host's storage must never move this bridge's state. +func TestR39_AuthResultForAnotherStorageIsIgnored(t *testing.T) { + r := &fakeRunner{} + st := &fakeStorage{found: false, active: []bool{true}} + m, _ := newTestManager(t, r, st, &fakeConsumer{secret: "S"}) + block := testBlock() + block.SecretGeneration = 1 + m.Apply(context.Background(), true, block) + + m.NoteAuthResult("some-other-pbs", true, "401") + if got := m.Status().State; got != "applied" { + t.Errorf("an unrelated storage's 401 changed our state to %q", got) + } +} + +// A bridge that has never seen a descriptor must not invent a state from a probe. +func TestR39_AuthResultBeforeAnyDescriptorIsIgnored(t *testing.T) { + m := NewManager(&fakeRunner{}, &fakeStorage{}, &fakeConsumer{}, t.TempDir(), "/etc/pve/priv/storage", + t.TempDir()+"/agent.json", slog.New(slog.NewTextHandler(io.Discard, nil))) + m.NoteAuthResult("felhom-pbs", true, "401") + if s := m.Status(); s != nil { + t.Errorf("status invented from a probe with no descriptor: %+v", s) + } +} diff --git a/internal/pbsdr/wrapper_read_test.go b/internal/pbsdr/wrapper_read_test.go new file mode 100644 index 0000000..1ce311d --- /dev/null +++ b/internal/pbsdr/wrapper_read_test.go @@ -0,0 +1,244 @@ +package pbsdr + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// R-39 leg (b), Scenario D — the `read` verb, tested as the ARTIFACT it is. +// +// This verb is the one that EXFILTRATES a file: the agent writes the PBS token secret through the +// root wrapper but could never read it back (/etc/pve/priv is 0700 root:www-data), so its verify +// loop was permanently blind to an `applied`-but-401 tier. Giving it a read path is right, but it +// means the wrapper now has a verb whose whole job is to print a secret — so its refusals are +// security-critical and are executed here under a real bash, not pattern-matched. +// +// COMPANION RED-PROOF (run + recorded): delete the storage-id charset guard at the top of the +// wrapper → TestWrapperRead_RefusesTraversal FAILS (the traversal case is no longer refused). + +func wrapperPath(t *testing.T) string { + t.Helper() + p := filepath.Join("..", "..", "configs", "felhom-pbs-apply") + if _, err := os.Stat(p); err != nil { + t.Fatalf("wrapper not found: %v", err) + } + return p +} + +// runWrapper executes the real script under bash and returns stdout, combined stderr and the code. +func runWrapper(t *testing.T, args ...string) (string, string, int) { + t.Helper() + cmd := exec.Command("bash", append([]string{wrapperPath(t)}, args...)...) + var out, errb strings.Builder + cmd.Stdout = &out + cmd.Stderr = &errb + err := cmd.Run() + code := 0 + if ee, ok := err.(*exec.ExitError); ok { + code = ee.ExitCode() + } else if err != nil { + t.Fatalf("run wrapper: %v", err) + } + return out.String(), errb.String(), code +} + +// Every traversal-shaped input is refused, non-zero, and prints NOTHING on stdout — a refusal that +// still emitted bytes would be the leak this test exists to prevent. +func TestWrapperRead_RefusesTraversal(t *testing.T) { + cases := []struct { + name string + args []string + }{ + {"dotdot id", []string{"read", "../../etc/shadow", "/etc/pve/priv/storage"}}, + {"id with slash", []string{"read", "a/b", "/etc/pve/priv/storage"}}, + {"id starting with a dot", []string{"read", ".hidden", "/etc/pve/priv/storage"}}, + {"secret dir outside the allowlist", []string{"read", "felhom-pbs", "/tmp"}}, + {"secret dir traversal", []string{"read", "felhom-pbs", "/var/lib/felhom-agent/../../etc"}}, + {"etc passwd as a dir", []string{"read", "passwd", "/etc"}}, + {"missing secret-dir arg", []string{"read", "felhom-pbs"}}, + {"too many args", []string{"read", "felhom-pbs", "/etc/pve/priv/storage", "extra"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + stdout, stderr, code := runWrapper(t, tc.args...) + if code == 0 { + t.Errorf("ACCEPTED a traversal-shaped read (rc=0): %v", tc.args) + } + if strings.TrimSpace(stdout) != "" { + t.Errorf("a refused read still wrote to stdout (%q) — that is the leak", stdout) + } + if !strings.Contains(stderr, "REFUSED") { + t.Errorf("refusal not announced on stderr: %q", stderr) + } + }) + } +} + +// The happy path: a secret under an allowed dir is printed verbatim to STDOUT and nowhere else. +// +// val_sdir hard-codes the /var/lib/felhom-agent prefix, which a test user cannot create. Rather than +// SKIP (a skipped test proves nothing) or weaken the production allowlist with a test escape hatch, +// these two cases run a COPY of the script with that one prefix constant relocated into t.TempDir(). +// Only the allowlisted location moves; every guard — the id grammar, the traversal refusal, the +// resolved-path prefix assertion — is the real code. The refusal tests above still run the +// unmodified script. +func relocatedWrapper(t *testing.T, base string) string { + t.Helper() + raw, err := os.ReadFile(wrapperPath(t)) + if err != nil { + t.Fatal(err) + } + src := strings.ReplaceAll(string(raw), "/var/lib/felhom-agent", base) + if src == string(raw) { + t.Fatal("relocation matched nothing — val_sdir no longer pins /var/lib/felhom-agent; revisit this test") + } + dst := filepath.Join(t.TempDir(), "felhom-pbs-apply") + if err := os.WriteFile(dst, []byte(src), 0o755); err != nil { + t.Fatal(err) + } + return dst +} + +func runScript(t *testing.T, script string, args ...string) (string, string, int) { + t.Helper() + cmd := exec.Command("bash", append([]string{script}, args...)...) + var out, errb strings.Builder + cmd.Stdout = &out + cmd.Stderr = &errb + err := cmd.Run() + code := 0 + if ee, ok := err.(*exec.ExitError); ok { + code = ee.ExitCode() + } else if err != nil { + t.Fatalf("run: %v", err) + } + return out.String(), errb.String(), code +} + +func TestWrapperRead_PrintsSecretToStdoutOnly(t *testing.T) { + base := t.TempDir() + script := relocatedWrapper(t, base) + dir := filepath.Join(base, "pbs") + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + const secret = "tok-secret-abcdef0123456789" + if err := os.WriteFile(filepath.Join(dir, "felhom-pbs.pw"), []byte(secret+"\n"), 0o600); err != nil { + t.Fatal(err) + } + + stdout, stderr, code := runScript(t, script, "read", "felhom-pbs", dir) + if code != 0 { + t.Fatalf("read failed rc=%d stderr=%q", code, stderr) + } + if strings.TrimSpace(stdout) != secret { + t.Errorf("stdout = %q, want the secret verbatim", stdout) + } + if strings.Contains(stderr, secret) { + t.Error("the secret leaked onto stderr, where sudo and the journal would capture it") + } + + // The relocated copy must STILL refuse traversal — proving the guards travelled with it and the + // happy path above is not passing because the checks were relocated away. + if _, _, rc := runScript(t, script, "read", "../../etc/shadow", dir); rc == 0 { + t.Error("the relocated copy accepted a traversal id — the guards did not travel") + } +} + +// A missing secret file is a clean refusal, never an empty success (an empty secret would build a +// client that 401s and be misdiagnosed as a revoked credential). +func TestWrapperRead_MissingFileRefuses(t *testing.T) { + base := t.TempDir() + script := relocatedWrapper(t, base) + dir := filepath.Join(base, "pbs") + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + + stdout, stderr, code := runScript(t, script, "read", "felhom-pbs", dir) + if code == 0 { + t.Error("a missing secret file must refuse, not succeed with empty output") + } + if strings.TrimSpace(stdout) != "" { + t.Errorf("stdout = %q, want empty", stdout) + } + if !strings.Contains(stderr, "REFUSED") { + t.Errorf("stderr = %q, want a REFUSED line", stderr) + } +} + +// The read verb must remain read-ONLY: no pvesm/pveum mutation may appear in its block. +func TestWrapperRead_IsSideEffectFree(t *testing.T) { + raw, err := os.ReadFile(wrapperPath(t)) + if err != nil { + t.Fatal(err) + } + block := readVerbBlock(t, string(raw)) + for _, forbidden := range []string{"pvesm ", "pveum ", "install ", "rm ", "place_copies"} { + if strings.Contains(block, forbidden) { + t.Errorf("the read verb performs a side effect (%q) — it must only print a file", forbidden) + } + } +} + +// readVerbBlock isolates the `read)` case body, comment lines stripped (the WHY note names the very +// traversal strings under test, and a naive scan would flag the explanation as the defect — the +// vacuous-pass trap the reconcile guard already documents). +func readVerbBlock(t *testing.T, src string) string { + t.Helper() + start := strings.Index(src, "\nread)\n") + if start < 0 { + t.Fatal("could not locate the read) block in configs/felhom-pbs-apply") + } + rest := src[start+len("\nread)\n"):] + end := strings.Index(rest, "\n ;;") + if end < 0 { + t.Fatal("could not locate the end of the read) block") + } + var code []string + for _, line := range strings.Split(rest[:end], "\n") { + if strings.HasPrefix(strings.TrimSpace(line), "#") { + continue + } + code = append(code, line) + } + return strings.Join(code, "\n") +} + +// The prefix assertion, exercised against a file that ACTUALLY EXISTS outside the secret dir. +// +// The earlier traversal cases are refused by whichever guard fires first, so they cannot tell us +// which one is load-bearing — and indeed deleting the id charset guard alone does not open a hole, +// because readlink -f plus the prefix assertion still catch it. That layering is the design, but it +// means a single-guard red-proof passes vacuously. This case isolates the LAST line of defence: a +// real secret file one directory up, reachable only if BOTH the charset guard and the prefix +// assertion are gone. +// +// COMPANION RED-PROOF (run + recorded): delete the id charset guard AND the `case "$resolved" in +// "$sdir"/*)` prefix assertion → this test FAILS by printing the out-of-tree secret. +func TestWrapperRead_PrefixAssertionStopsEscapeToARealFile(t *testing.T) { + base := t.TempDir() + script := relocatedWrapper(t, base) + dir := filepath.Join(base, "pbs") + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + // A juicy file one level ABOVE the secret dir, inside the allowlisted prefix (so val_sdir is not + // the guard doing the work here). + const stolen = "NOT-FOR-THE-AGENT-0123456789" + if err := os.WriteFile(filepath.Join(base, "elsewhere.pw"), []byte(stolen), 0o600); err != nil { + t.Fatal(err) + } + + stdout, stderr, code := runScript(t, script, "read", "../elsewhere", dir) + if code == 0 || strings.Contains(stdout, stolen) { + t.Errorf("the read verb escaped its secret dir and printed a file it must never reach.\n"+ + " rc=%d stdout=%q", code, stdout) + } + if !strings.Contains(stderr, "REFUSED") { + t.Errorf("escape not refused loudly: stderr=%q", stderr) + } +}