diff --git a/hub/CHANGELOG.md b/hub/CHANGELOG.md index 7ffe44c..ff2ce49 100644 --- a/hub/CHANGELOG.md +++ b/hub/CHANGELOG.md @@ -1,5 +1,28 @@ # Felhom Hub — Changelog +## v0.39.0 — offsite hardening: F4 credential re-issue + F2 scan retry + F5 save UX (2026-07-09) + +Part of the offsite-provisioning hardening bundle (pairs with controller v0.107.0 + agent v0.78.0); the +sharp edges from the live e2e (`documentation/audits/VALIDATION-offsite-provisioning-e2e-2026-07-09.md`). + +- **F4 (pilot-gating) — "Re-issue offsite credentials":** `Provisioner.ReissueCredentials` — the EXPLICIT + operator recovery for a consumed-password dead-end (fresh-guest DR; consumed-but-failed install). Resets + the customer's sub-account password (`ResetSubaccountPassword`) or dedicated-box password (new + `ResetBoxPassword` in `hetznerapi`, client+interface+fake) → stores a FRESH one-time secret → the handler + re-saves the config unchanged so `ConfigVersion` bumps and the stuck guest's next refresh re-runs the + bridge. **Hard-scoped:** targets ONLY the resource labelled `felhom-customer=`; refuses unless the + label lookup finds exactly 1 (ambiguity = refuse, no reset, no secret) **+ companion red-proof** (dropped + the exactly-1 guard → ambiguous lookup proceeded → test FAILED). NOT implicit rotation — `ProvisionOffsite` + never calls it. UI: a confirm-gated button on the config form (shown only when provisioned), route + `POST /configs/{id}/offsite-reissue` (CSRF rides the parent form). The password value is never logged. +- **F2 — host-key scan retry-with-backoff:** a fresh sub-account's DNS lags creation, so the FIRST save + 502'd (`no such host`, live). `scanWithRetry` retries on failure (default ladder 2/4/8/16/30s ≈ 60s total, + inside applyOffsite's 3-min detached ctx; ctx-abortable; fail-closed past the budget) **+ companion + red-proof** (disabled the retry loop → DNS-lag save failed → test FAILED). `Provisioner.ScanBackoff` + injectable for tests. +- **F5 — save UX:** the config form disables its submit buttons and shows an in-flight notice on submit + (the ~25–60s spinner-less save was the re-click bait that caused F1 live). + ## v0.38.1 — offsite provisioning: detach from the client's request context (live finding F1) (2026-07-09) Found in the first supervised live run: the offsite save takes ~25s (create + wait + host-key scan) with no diff --git a/hub/internal/hetznerapi/fake.go b/hub/internal/hetznerapi/fake.go index 051f82c..caf43cb 100644 --- a/hub/internal/hetznerapi/fake.go +++ b/hub/internal/hetznerapi/fake.go @@ -20,6 +20,7 @@ type Fake struct { CreatedSubaccounts int CreatedBoxes int ResetCalls int + BoxResetCalls int DeletedSubaccounts int DeletedBoxes int @@ -175,6 +176,13 @@ func (f *Fake) ChangeType(_ context.Context, _ int64, _ string) (Action, error) return f.newAction("change_type"), nil } +func (f *Fake) ResetBoxPassword(_ context.Context, _ int64, _ string) (Action, error) { + f.mu.Lock() + defer f.mu.Unlock() + f.BoxResetCalls++ + return f.newAction("reset_password"), nil +} + func (f *Fake) DeleteStorageBox(_ context.Context, boxID int64) (Action, error) { f.mu.Lock() defer f.mu.Unlock() diff --git a/hub/internal/hetznerapi/hetznerapi.go b/hub/internal/hetznerapi/hetznerapi.go index c6237ec..0aae2fc 100644 --- a/hub/internal/hetznerapi/hetznerapi.go +++ b/hub/internal/hetznerapi/hetznerapi.go @@ -96,6 +96,7 @@ type CloudAPI interface { GetStorageBox(ctx context.Context, boxID int64) (StorageBox, error) CreateStorageBox(ctx context.Context, req CreateBoxRequest) (createdID int64, action Action, err error) ChangeType(ctx context.Context, boxID int64, boxType string) (Action, error) + ResetBoxPassword(ctx context.Context, boxID int64, password string) (Action, error) DeleteStorageBox(ctx context.Context, boxID int64) (Action, error) // WaitAction polls the action to "success" (bounded); errors on "error" or timeout. @@ -270,6 +271,16 @@ func (c *Client) ChangeType(ctx context.Context, boxID int64, boxType string) (A return out.Action, err } +// ResetBoxPassword resets a dedicated box's (main-account) password. The password value is only ever in +// the request body — never logged by the caller (F4 re-issue). +func (c *Client) ResetBoxPassword(ctx context.Context, boxID int64, password string) (Action, error) { + var out struct { + Action Action `json:"action"` + } + err := c.do(ctx, http.MethodPost, fmt.Sprintf("/storage_boxes/%d/actions/reset_password", boxID), map[string]string{"password": password}, &out) + return out.Action, err +} + func (c *Client) DeleteStorageBox(ctx context.Context, boxID int64) (Action, error) { var out struct { Action Action `json:"action"` diff --git a/hub/internal/offsite/offsite.go b/hub/internal/offsite/offsite.go index aef97cb..bc42bb9 100644 --- a/hub/internal/offsite/offsite.go +++ b/hub/internal/offsite/offsite.go @@ -16,6 +16,7 @@ import ( "log" "math/big" "strings" + "time" "gitea.dooplex.hu/admin/felhom-hub/internal/hetznerapi" "gitea.dooplex.hu/admin/felhom-hub/internal/store" @@ -61,8 +62,15 @@ type Provisioner struct { PoolBoxID int64 // the shared-pool storage-box id (e.g. 611421) Location string // dedicated-box location, e.g. "fsn1" Logger *log.Logger + // ScanBackoff is the retry schedule for the host-key scan (F2: a fresh sub-account's DNS name lags + // creation by seconds-to-a-minute, so the first scan typically fails with "no such host"). nil → the + // default ~60s ladder. Tests inject zeros. The total must fit inside applyOffsite's 3-min detached ctx. + ScanBackoff []time.Duration } +// defaultScanBackoff: 5 retries, ~60s total — sized to the observed DNS propagation lag. +var defaultScanBackoff = []time.Duration{2 * time.Second, 4 * time.Second, 8 * time.Second, 16 * time.Second, 30 * time.Second} + func (p *Provisioner) logf(f string, a ...any) { if p.Logger != nil { p.Logger.Printf(f, a...) @@ -106,7 +114,7 @@ func (p *Provisioner) ProvisionOffsite(ctx context.Context, customerID string, i if port == 0 { port = sftpPort } - fp, err := p.Scanner.Fingerprint(ctx, d.Host, port) + fp, err := p.scanWithRetry(ctx, d.Host, port) if err != nil { return nil, fmt.Errorf("offsite: host-key scan %s: %w", d.Host, err) } @@ -114,6 +122,81 @@ func (p *Provisioner) ProvisionOffsite(ctx context.Context, customerID string, i return d, nil } +// scanWithRetry retries the host-key scan on failure (F2: fresh-resource DNS lag). Fail-closed past the +// budget; ctx cancellation aborts between attempts. +func (p *Provisioner) scanWithRetry(ctx context.Context, host string, port int) (string, error) { + backoff := p.ScanBackoff + if backoff == nil { + backoff = defaultScanBackoff + } + fp, err := p.Scanner.Fingerprint(ctx, host, port) + for i := 0; err != nil && i < len(backoff); i++ { + p.logf("[offsite] host-key scan %s failed (attempt %d/%d, retrying in %s): %v", host, i+1, len(backoff)+1, backoff[i], err) + select { + case <-ctx.Done(): + return "", ctx.Err() + case <-time.After(backoff[i]): + } + fp, err = p.Scanner.Fingerprint(ctx, host, port) + } + return fp, err +} + +// ReissueCredentials resets the customer's offsite credential and stores a FRESH one-time password — the +// EXPLICIT operator recovery for a consumed-password dead-end (a fresh guest at DR, or a +// consumed-but-failed install). It is NOT implicit rotation: ProvisionOffsite never calls this. Scoped +// hard: the reset targets ONLY the resource labelled `felhom-customer=`, and refuses unless the label +// lookup finds exactly one. The password value is never logged (the action is). +func (p *Provisioner) ReissueCredentials(ctx context.Context, customerID, typ string) error { + pw, err := genPassword() + if err != nil { + return err + } + switch typ { + case "shared": + if p.PoolBoxID == 0 { + return fmt.Errorf("offsite: no shared pool box configured") + } + subs, err := p.API.ListSubaccounts(ctx, p.PoolBoxID, customerSelector(customerID)) + if err != nil { + return fmt.Errorf("offsite: reissue lookup: %w", err) + } + if len(subs) != 1 { + return fmt.Errorf("offsite: reissue needs exactly 1 sub-account labelled for %s, found %d — refusing", customerID, len(subs)) + } + act, err := p.API.ResetSubaccountPassword(ctx, p.PoolBoxID, subs[0].ID, pw) + if err != nil { + return fmt.Errorf("offsite: reset sub-account password: %w", err) + } + if err := p.API.WaitAction(ctx, act); err != nil { + return fmt.Errorf("offsite: reset action: %w", err) + } + p.logf("[offsite] re-issued shared credentials for %s (subaccount %d)", customerID, subs[0].ID) + case "dedicated": + boxes, err := p.API.ListStorageBoxes(ctx, customerSelector(customerID)) + if err != nil { + return fmt.Errorf("offsite: reissue lookup: %w", err) + } + if len(boxes) != 1 { + return fmt.Errorf("offsite: reissue needs exactly 1 box labelled for %s, found %d — refusing", customerID, len(boxes)) + } + act, err := p.API.ResetBoxPassword(ctx, boxes[0].ID, pw) + if err != nil { + return fmt.Errorf("offsite: reset box password: %w", err) + } + if err := p.API.WaitAction(ctx, act); err != nil { + return fmt.Errorf("offsite: reset action: %w", err) + } + p.logf("[offsite] re-issued dedicated credentials for %s (box %d)", customerID, boxes[0].ID) + default: + return fmt.Errorf("offsite: reissue: unknown type %q (want shared|dedicated)", typ) + } + if err := p.Store.SaveOneTimeSecret(customerID, pw); err != nil { + return fmt.Errorf("offsite: store re-issued one-time password: %w", err) + } + return nil +} + func (p *Provisioner) provisionShared(ctx context.Context, customerID string, in Input) (*Descriptor, error) { if p.PoolBoxID == 0 { return nil, fmt.Errorf("offsite: no shared pool box configured") diff --git a/hub/internal/offsite/offsite_test.go b/hub/internal/offsite/offsite_test.go index 18292d2..e9c49ce 100644 --- a/hub/internal/offsite/offsite_test.go +++ b/hub/internal/offsite/offsite_test.go @@ -10,6 +10,7 @@ import ( "path/filepath" "strings" "testing" + "time" "gitea.dooplex.hu/admin/felhom-hub/internal/hetznerapi" "gitea.dooplex.hu/admin/felhom-hub/internal/store" @@ -23,7 +24,24 @@ func newTestProvisioner(t *testing.T) (*Provisioner, *hetznerapi.Fake, *store.St } t.Cleanup(func() { st.Close() }) fake := hetznerapi.NewFake() - return &Provisioner{API: fake, Store: st, Scanner: &fakeScanner{fp: "SHA256:testfp"}, PoolBoxID: 611421, Location: "fsn1", Logger: log.New(io.Discard, "", 0)}, fake, st + // ScanBackoff: empty (non-nil) → single scan attempt, no retries — tests that want the F2 retry set + // their own schedule (nil would select the ~60s production default and stall the suite). + return &Provisioner{API: fake, Store: st, Scanner: &fakeScanner{fp: "SHA256:testfp"}, PoolBoxID: 611421, Location: "fsn1", Logger: log.New(io.Discard, "", 0), ScanBackoff: []time.Duration{}}, fake, st +} + +// flakyScanner fails the first `failures` calls (fresh-resource DNS lag), then succeeds. +type flakyScanner struct { + failures int + fp string + calls int +} + +func (f *flakyScanner) Fingerprint(_ context.Context, _ string, _ int) (string, error) { + f.calls++ + if f.calls <= f.failures { + return "", errors.New("dial tcp: lookup fresh.your-storagebox.de: no such host") + } + return f.fp, nil } // fakeScanner returns a fixed fingerprint (or an error) — no live SSH in tests. @@ -96,6 +114,103 @@ func TestProvision_ScanFailClosed(t *testing.T) { _ = st } +// Scenario D (F2) — a fresh sub-account's DNS lags creation: the scan is retried and the FIRST save +// serves the descriptor. +func TestProvision_ScanRetriesThroughDNSLag(t *testing.T) { + p, _, _ := newTestProvisioner(t) + p.ScanBackoff = []time.Duration{0, 0, 0} // instant retries in tests + sc := &flakyScanner{failures: 2, fp: "SHA256:late"} + p.Scanner = sc + d, err := p.ProvisionOffsite(context.Background(), "cust-dns", Input{Enabled: true, Type: "shared", QuotaGB: 10}) + if err != nil { + t.Fatalf("the first save must survive DNS lag via scan retries, got: %v", err) + } + if d.HostFingerprint != "SHA256:late" || sc.calls != 3 { + t.Fatalf("want fingerprint after 2 failed + 1 good scan, got fp=%q calls=%d", d.HostFingerprint, sc.calls) + } +} + +// Scenario D (F2) — a scan that keeps failing past the budget still fails CLOSED (no descriptor). +func TestProvision_ScanExhaustedFailsClosed(t *testing.T) { + p, _, _ := newTestProvisioner(t) + p.ScanBackoff = []time.Duration{0, 0} + sc := &flakyScanner{failures: 99, fp: "SHA256:never"} + p.Scanner = sc + d, err := p.ProvisionOffsite(context.Background(), "cust-dns2", Input{Enabled: true, Type: "shared", QuotaGB: 10}) + if err == nil || d != nil { + t.Fatalf("an exhausted scan budget must fail closed, got d=%+v err=%v", d, err) + } + if sc.calls != 3 { // 1 initial + 2 retries + t.Fatalf("want 3 attempts (1 + 2 retries), got %d", sc.calls) + } +} + +// Scenario A (F4) — re-issue resets the labelled sub-account's password and stores a FRESH one-time +// secret (the consumed-password dead-end recovery). +func TestReissue_SharedFreshSecret(t *testing.T) { + p, fake, st := newTestProvisioner(t) + if _, err := p.ProvisionOffsite(context.Background(), "cust-r", Input{Enabled: true, Type: "shared", QuotaGB: 10}); err != nil { + t.Fatal(err) + } + pw1, err := st.ConsumeOneTimeSecret("cust-r") // the original is spent (the dead-end premise) + if err != nil || pw1 == "" { + t.Fatal("harness: no initial secret") + } + if err := p.ReissueCredentials(context.Background(), "cust-r", "shared"); err != nil { + t.Fatalf("reissue: %v", err) + } + if fake.ResetCalls != 1 { + t.Fatalf("want exactly 1 sub-account password reset, got %d", fake.ResetCalls) + } + pw2, err := st.ConsumeOneTimeSecret("cust-r") + if err != nil || pw2 == "" { + t.Fatal("a FRESH one-time secret must be stored after reissue") + } + if pw2 == pw1 { + t.Fatal("the re-issued password must differ from the spent one") + } +} + +// Scenario A (F4) — the dedicated path resets the labelled box's password. +func TestReissue_DedicatedFreshSecret(t *testing.T) { + p, fake, st := newTestProvisioner(t) + if _, err := p.ProvisionOffsite(context.Background(), "cust-rd", Input{Enabled: true, Type: "dedicated", BoxType: "bx11"}); err != nil { + t.Fatal(err) + } + _, _ = st.ConsumeOneTimeSecret("cust-rd") + if err := p.ReissueCredentials(context.Background(), "cust-rd", "dedicated"); err != nil { + t.Fatalf("reissue dedicated: %v", err) + } + if fake.BoxResetCalls != 1 { + t.Fatalf("want exactly 1 box password reset, got %d", fake.BoxResetCalls) + } + if pw, err := st.ConsumeOneTimeSecret("cust-rd"); err != nil || pw == "" { + t.Fatal("fresh secret must be stored after a dedicated reissue") + } +} + +// Scenario A WRONG-guard (F4) — an ambiguous label lookup (≠1 resource) must REFUSE: no reset, no secret. +func TestReissue_RefusesAmbiguousLookup(t *testing.T) { + p, fake, st := newTestProvisioner(t) + // two sub-accounts labelled for the same customer (should never happen — refuse rather than guess) + fake.Subaccounts[1] = hetznerapi.Subaccount{ID: 1, StorageBox: 611421, Username: "u-sub1", Labels: map[string]string{"felhom-customer": "cust-amb"}} + fake.Subaccounts[2] = hetznerapi.Subaccount{ID: 2, StorageBox: 611421, Username: "u-sub2", Labels: map[string]string{"felhom-customer": "cust-amb"}} + err := p.ReissueCredentials(context.Background(), "cust-amb", "shared") + if err == nil || !strings.Contains(err.Error(), "exactly 1") { + t.Fatalf("ambiguous lookup must refuse, got %v", err) + } + if fake.ResetCalls != 0 { + t.Fatal("NO reset may run on an ambiguous lookup") + } + if _, cerr := st.ConsumeOneTimeSecret("cust-amb"); cerr == nil { + t.Fatal("NO secret may be stored on a refused reissue") + } + // zero resources → also refuse (nothing provisioned) + if err := p.ReissueCredentials(context.Background(), "cust-none", "shared"); err == nil { + t.Fatal("reissue with nothing provisioned must refuse") + } +} + // Scenario B — enable dedicated → box provisioned. func TestProvision_Dedicated(t *testing.T) { p, fake, st := newTestProvisioner(t) diff --git a/hub/internal/web/configs.go b/hub/internal/web/configs.go index bedcee4..12019a2 100644 --- a/hub/internal/web/configs.go +++ b/hub/internal/web/configs.go @@ -537,6 +537,50 @@ func (s *Server) handleConfigUpdate(w http.ResponseWriter, r *http.Request, cust http.Redirect(w, r, "/customers/"+customerID+"?flash=updated", http.StatusSeeOther) } +// handleOffsiteReissue (F4) resets the customer's offsite credential and stores a fresh one-time password — +// the explicit operator recovery for a consumed-password dead-end (fresh-guest DR, consumed-but-failed +// install). Scoped to the resource labelled for THIS customer (the provisioner refuses unless exactly one). +// The config is re-saved unchanged so ConfigVersion bumps → the stuck guest's next refresh re-runs the +// bridge, which consumes the fresh password. The password value is never logged or rendered. +func (s *Server) handleOffsiteReissue(w http.ResponseWriter, r *http.Request, customerID string) { + if s.offsite == nil { + http.Error(w, "Offsite provisioning is not configured on this hub", http.StatusBadGateway) + return + } + cfg, err := s.store.GetCustomerConfig(customerID) + if err != nil || cfg == nil { + http.NotFound(w, r) + return + } + var overrides struct { + Offsite struct { + Enabled bool `json:"enabled"` + Type string `json:"type"` + } `json:"offsite"` + } + _ = json.Unmarshal([]byte(cfg.ConfigJSON), &overrides) + if !overrides.Offsite.Enabled || overrides.Offsite.Type == "" { + http.Error(w, "No provisioned offsite tier for this customer", http.StatusBadRequest) + return + } + // Same detached-ctx discipline as applyOffsite (F1): once the reset starts, reset→store must complete. + ctx, cancel := context.WithTimeout(context.WithoutCancel(r.Context()), 3*time.Minute) + defer cancel() + if err := s.offsite.ReissueCredentials(ctx, customerID, overrides.Offsite.Type); err != nil { + s.logger.Printf("[ERROR] offsite reissue for %s: %v", customerID, err) + http.Error(w, "Offsite credential re-issue failed: "+err.Error(), http.StatusBadGateway) + return + } + // Re-save unchanged → ConfigVersion bump → the customer's controller re-pulls + re-runs the bridge. + if err := s.store.SaveCustomerConfig(cfg); err != nil { + s.logger.Printf("[ERROR] offsite reissue for %s: config bump failed: %v", customerID, err) + http.Error(w, "Credential re-issued but the config bump failed — save the config once to trigger the pickup", http.StatusInternalServerError) + return + } + s.logger.Printf("[INFO] offsite credentials re-issued for %s (fresh one-time password stored; ConfigVersion bumped)", customerID) + http.Redirect(w, r, "/customers/"+customerID+"?flash=offsite_reissued", http.StatusSeeOther) +} + // handleConfigDelete deletes a customer config. func (s *Server) handleConfigDelete(w http.ResponseWriter, r *http.Request, customerID string) { if err := s.store.DeleteCustomerConfig(customerID); err != nil { diff --git a/hub/internal/web/server.go b/hub/internal/web/server.go index 180d0f5..facd202 100644 --- a/hub/internal/web/server.go +++ b/hub/internal/web/server.go @@ -343,6 +343,14 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { } else { s.handleConfigEditForm(w, r, customerID) } + case strings.HasPrefix(path, "/configs/") && strings.HasSuffix(path, "/offsite-reissue"): + customerID := strings.TrimPrefix(path, "/configs/") + customerID = strings.TrimSuffix(customerID, "/offsite-reissue") + if r.Method == http.MethodPost { + s.handleOffsiteReissue(w, r, customerID) + } else { + http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) + } case strings.HasPrefix(path, "/configs/") && strings.HasSuffix(path, "/preview"): customerID := strings.TrimPrefix(path, "/configs/") customerID = strings.TrimSuffix(customerID, "/preview") diff --git a/hub/internal/web/templates/config_form.html b/hub/internal/web/templates/config_form.html index 40f94bc..ee0b566 100644 --- a/hub/internal/web/templates/config_form.html +++ b/hub/internal/web/templates/config_form.html @@ -134,14 +134,39 @@ {{with .Overrides}}{{with index . "offsite"}}{{if index . "host"}}

Provisioned: {{index . "user"}}@{{index . "host"}}:{{index . "repo_path"}} — the transient password is delivered to the controller once (never shown here).

+ + {{end}}{{end}}{{end}} -
+
Cancel +
+