v0.91.0 — the DR tier can no longer be applied and dead at the same time (R-39 + R-50b(a))
Closes the agent half of R-39's fleet fix. Requires hub >=0.68.0 for the re-arm signal; that hub is safe for 0.90.0 agents (unknown key dropped), so it deploys first. 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/namespace come back byte-identical and the descriptor content hash never moved — the converged agent short-circuited and never consumed the fresh secret. WirePBSDR.SecretGeneration (field-exact with the hub) is what moves the hash now, because descriptorHash marshals this struct. 2. The agent could not READ its own credential. It writes /etc/pve/priv/storage/<id>.pw through the root wrapper, but that dir is 0700 root:www-data and the wrapper had no read verb — so the target resolver got "permission denied" every cycle, warned, and skipped. The one loop that could have caught the 401 was blind BY CONSTRUCTION. Adds a narrow `read` verb (+ exactly one sudoers line, + a pbsdr-read capability row): one secret to stdout, no network, no mutation, never in argv (sudo logs argv), traversal refused by the id grammar, the dir allowlist AND a resolved-path prefix assertion. 3. Nothing probed AUTHENTICATION. pbs.ProbeAuth (GET /version + an ErrUnauthorized sentinel) runs on the 15-minute collect path and its verdict becomes a loud `auth_failed` the hub escalates to a fresh mint. /version needs no datastore, namespace or privilege, so a 401 means the CREDENTIAL is bad; 403 is deliberately NOT treated as unauthorized, since re-keying a too-narrow token would mint forever without fixing anything. A transport error is UNKNOWN, never a rejection — otherwise every network blip burns a credential. Recovery self-clears. R-50b(a): the report now carries the installed wrapper's sha256 so drift against the vouched manifest value is answerable. Empty = unknown, never drift. Three red-proofs, all at the assertion level. Removing SecretGeneration fails the re-arm test with "consume calls=1, want 2". Swallowing the probe result leaves State:applied AuthFailed:false — the July-18 shape exactly. Notably, deleting the wrapper's id charset guard alone does NOT open a traversal hole (readlink + the prefix assertion still catch it), so the isolating red-proof removes BOTH and shows the out-of-tree secret printed — the layering is real, and a single-guard red-proof would have passed vacuously.
This commit is contained in:
@@ -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/<id>.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,
|
||||
|
||||
@@ -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/<id>.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 <PBSSecretDir>/<id>.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
|
||||
}
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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 <id> <secret-dir>
|
||||
# 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 <id>
|
||||
# The Part-0-evidenced dual-grant: FelhomAgentStore on /storage/<id> 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 <create|reconcile|grant> <storage-id> ..."
|
||||
[[ -n "$op" && -n "$id" ]] || die "usage: felhom-pbs-apply <create|reconcile|grant|read> <storage-id> ..."
|
||||
|
||||
# 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: <id> <secret-dir>"
|
||||
sdir="$3"
|
||||
val_sdir "$sdir"
|
||||
target="$sdir/$id.pw"
|
||||
# Belt: resolve and re-check the prefix (guards a symlinked <id>.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 <id>"
|
||||
pveum acl modify "/storage/$id" --users felhom-agent@pve --roles FelhomAgentStore >&2
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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"
|
||||
@@ -192,6 +196,7 @@ 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.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,
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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/<id>.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 <code>: <body>", 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)
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user