diff --git a/CHANGELOG.md b/CHANGELOG.md index 68fc234..cd06d73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,32 @@ ## Changelog +### v0.150.0 — green gate restored + the export link stops leaking the CSRF token (2026-07-20) + +**F7 / R-53 — `app_export.html` built the app's public URL from the CSRF token.** The line read +`var domain = '.{{$.CSRFToken}}'`, so the „Megnyitás" link was wrong for every app with a +subdomain and a session CSRF token was written into a URL (history, referrers, logs). Two-part fix: +the template token becomes `{{$.Domain}}`, and `exportPageHandler` supplies `Domain` — that handler +builds its own data map instead of going through `baseData`, which is where every other page gets +the key, so the template had nothing to read. The page's real CSRF path (the `csrfH()` helper +reading the meta tag) is correct and untouched. Render tests assert the joined `.` and +that the token appears nowhere on that line; red-proofed against the pre-fix template. + +**The 7 red `internal/backup` tests are green again — no behaviour change.** `TestTier2V2_*` and +`TestSharesTier2*` had been failing on DooPlex since before v0.149.0. Root cause is environmental, +one class for all seven: Tier-2's off-drive guard asks `system.SamePhysicalDevice` (st_dev equality) +whether a candidate target is really a *second* disk, and on a host where every `t.TempDir()` lands +on one filesystem the fixture's "two drives" are indistinguishable — so the guard correctly refused +the target and the tests could never reach their subject. The failure message said so outright: +`nincs másik fizikai meghajtó`. + +Fixed with one behaviour-preserving seam in the package's existing style: a nil-defaulted +`Manager.samePhysicalDevice` field plus a `sameDevice` wrapper, with the seven call sites routed +through it. **Nil → `system.SamePhysicalDevice`, so production is byte-for-byte unchanged**; only +the two test fixtures install a fake that models one drive per directory subtree. No assertion was +weakened, no test skipped, renamed or deleted, and every one of the seven was mutation-proved: the +defect each guards was re-introduced one at a time and each test failed, including the notifier +test's own documented red-proof (`_shares` reaching Hungarian copy). + ### v0.149.0 — the dashboard tells the truth about the last backup (2026-07-20) Closes **F3** from `felhom.eu/documentation/audits/AUDIT-vacation-remote-ops-2026-07-20.md`. diff --git a/CLAUDE.md b/CLAUDE.md index 229f956..fce091d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -110,6 +110,15 @@ state). **`claude-in-chrome` is NOT available in the DooPlex environment** — t endpoint-level: invoke the exact endpoint the UI invokes (no server logic is skipped, only rendering) and say which method was used. Strict end-to-end UI coverage is a manual click-through. +Two traps in that method, both from the 2026-07-20 remediation: +- **Grep the fetched page with ASCII-only substrings.** Accented Hungarian patterns get mangled + through the `ssh → pct exec → bash -c` chain and return a false `0` — which reads exactly like the + banner/string being gone. Use `kezel`, `Utols`, `Biztons`; never let an accented pattern gate a + conclusion (it nearly produced a wrong "banner cleared" claim). +- **Credentials with `!` or `'` break in heredoc-built helper scripts** (history expansion eats + `!!`). Use the proven inline `-d "password=$PW"` form for authed curl, and delete any + credential-bearing helper from `/tmp` (host AND guest) when done. + ## Environment & access Claude Code runs **on DooPlex (192.168.0.180, Debian 13, user `kisfenyo`)**; repos in diff --git a/CONTEXT.md b/CONTEXT.md index 41a9ad9..9aecf71 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,7 +7,21 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" -Last updated: 2026-07-20 (v0.149.0 — F3 backup-card fix + remote-site remediation) +Last updated: 2026-07-20 (v0.150.0 — green gate restored + F7 export-link fix) + +> **2026-07-20 — v0.150.0.** **`go test ./...` on DooPlex is fully green again (23/23 packages, run +> twice) — no surviving `t.Skip`s, no weakened assertions, no deleted tests.** The 7 red +> `TestTier2V2_*` / `TestSharesTier2*` were all one environmental class: Tier-2's off-drive guard +> uses `system.SamePhysicalDevice` (st_dev equality), and every `t.TempDir()` here shares one +> filesystem, so the fixture's "two drives" looked identical and the guard correctly refused the +> target (`nincs másik fizikai meghajtó`). Fixed with a nil-defaulted `Manager.samePhysicalDevice` +> seam + `sameDevice` wrapper — **production behaviour is byte-for-byte unchanged** (nil → the real +> check); only the two fixtures inject a fake. All 7 mutation-proved. **F7/R-53 shipped:** +> `app_export.html` built the app URL from `{{$.CSRFToken}}`; now `{{$.Domain}}`, with +> `exportPageHandler` supplying the key (it bypasses `baseData`). Host: the orphaned `dhclient` on +> the non-existent `eth0` was killed and did not respawn. Still open from the arc: R-50 (durable F1, +> spike-first — its ROADMAP note about needing a new cert SAN was **corrected**: the pin is a raw +> leaf-DER SHA-256 with `InsecureSkipVerify`, so SAN never enters it), R-51, R-52, R-39(b)/F6. > **2026-07-20 — remote-site remediation + v0.149.0.** **F1 is MITIGATED FOR THE WINDOW, not durably > fixed:** `vmbr0` on the demo host is now **static `192.168.0.162/24`** (was DHCP; the remote router diff --git a/controller/internal/backup/backup.go b/controller/internal/backup/backup.go index 6e1b611..adb9e59 100644 --- a/controller/internal/backup/backup.go +++ b/controller/internal/backup/backup.go @@ -127,6 +127,13 @@ type Manager struct { // real tier2FitsSystemDrive. tier2SSDFits func(sys string, sizeBytes int64) bool + // samePhysicalDevice — the off-drive identity predicate behind every Tier-2 "is this really a + // SECOND disk?" guard, overridable in tests. The real check is `st_dev` equality, so on a host + // where every `t.TempDir()` lands on one filesystem the fixture's "two drives" are indistinguishable + // and Tier-2 correctly refuses them — which makes the off-drive tests unrunnable rather than wrong. + // Nil → the real system.SamePhysicalDevice (production always takes this path). + samePhysicalDevice func(a, b string) bool + // migrationRunning, if set, reports whether a data migration is in progress. The scheduled // backup paths skip when it returns true (Change 3 — backup ↔ migration mutual exclusion), so a // nightly dump/Tier-2 can't race a migration copy/cleanup on the same drive. @@ -991,6 +998,15 @@ func (m *Manager) GetFullStatus(nextDBDump time.Time) *FullBackupStatus { return status } +// sameDevice reports whether two paths sit on the same physical device, through the test seam when +// one is installed. Nil seam → system.SamePhysicalDevice, i.e. byte-for-byte the previous behaviour. +func (m *Manager) sameDevice(a, b string) bool { + if m.samePhysicalDevice != nil { + return m.samePhysicalDevice(a, b) + } + return system.SamePhysicalDevice(a, b) +} + // hasOffDriveTarget reports whether any registered, schedulable storage path lives on a physical disk // OTHER than the system drive — i.e. whether a genuine off-drive (tier-2) copy is possible at all. // When false the box is single-drive: tier-1 is the ONLY local copy and 3-2-1 needs a 2nd drive or @@ -1000,7 +1016,7 @@ func (m *Manager) hasOffDriveTarget() bool { return false } for _, sp := range m.settings.GetSchedulableStoragePaths() { - if sp.Path == m.systemDataPath || system.SamePhysicalDevice(m.systemDataPath, sp.Path) { + if sp.Path == m.systemDataPath || m.sameDevice(m.systemDataPath, sp.Path) { continue } return true diff --git a/controller/internal/backup/device_seam_test.go b/controller/internal/backup/device_seam_test.go new file mode 100644 index 0000000..d0bc716 --- /dev/null +++ b/controller/internal/backup/device_seam_test.go @@ -0,0 +1,28 @@ +package backup + +import ( + "path/filepath" + "strings" +) + +// oneDrivePerSubtree is the test stand-in for system.SamePhysicalDevice (st_dev equality). +// +// Why it exists: the real predicate asks "are these two paths on the same physical disk?", and +// Tier-2's whole purpose is to refuse a target that is. On a host where every t.TempDir() lands on +// one filesystem — DooPlex, and any CI box with a single volume — a fixture's "usb" and "flash" +// dirs share one st_dev, so the guard correctly refuses them and the off-drive tests can never +// exercise their subject. This models what the fixture is actually depicting: one drive per +// directory subtree, so two paths share a device only when one contains the other (a path inside a +// drive IS on that drive). Unrelated subtrees are distinct devices, exactly as real mountpoints are. +// +// It does NOT relax any assertion — the guard still runs, still refuses same-device targets (see +// TestSharesTier2NeverTargetsItsOwnSourceDrive, which passes under this seam), and production keeps +// using the real st_dev check because the seam is nil there. +func oneDrivePerSubtree(a, b string) bool { + a, b = filepath.Clean(a), filepath.Clean(b) + if a == b { + return true + } + sep := string(filepath.Separator) + return strings.HasPrefix(a, b+sep) || strings.HasPrefix(b, a+sep) +} diff --git a/controller/internal/backup/shares_test.go b/controller/internal/backup/shares_test.go index 85d91c8..01c36e7 100644 --- a/controller/internal/backup/shares_test.go +++ b/controller/internal/backup/shares_test.go @@ -68,6 +68,9 @@ func newSharesEnv(t *testing.T, driveNames ...string) *sharesEnv { return copyTreeForTest(src, dst) }, sharesPassdbCapture: func() ([]byte, error) { return []byte("FAKE-PASSDB-TAR"), nil }, + // Each drive dir is its own "device" — otherwise hdd_1/hdd_2 share one st_dev here and the + // off-drive guard refuses the target, so the mirror under test never runs. + samePhysicalDevice: oneDrivePerSubtree, } return env } diff --git a/controller/internal/backup/tier2.go b/controller/internal/backup/tier2.go index 2646dd3..05d3f3e 100644 --- a/controller/internal/backup/tier2.go +++ b/controller/internal/backup/tier2.go @@ -115,7 +115,7 @@ func (m *Manager) selectTier2TargetFrom(stackName, sourceDrive string, fullSize, sawNetworkCandidate = true break } - if sp.Path == sourceDrive || system.SamePhysicalDevice(sourceDrive, sp.Path) { + if sp.Path == sourceDrive || m.sameDevice(sourceDrive, sp.Path) { break // pinned target is on the same physical disk — not off-drive; fall through } label := sp.Label @@ -134,7 +134,7 @@ func (m *Manager) selectTier2TargetFrom(stackName, sourceDrive string, fullSize, // 1. Prefer another registered user-data drive on a DIFFERENT physical disk, NON-network. if m.settings != nil { for _, sp := range m.settings.GetSchedulableStoragePaths() { - if sp.Path == sourceDrive || system.SamePhysicalDevice(sourceDrive, sp.Path) { + if sp.Path == sourceDrive || m.sameDevice(sourceDrive, sp.Path) { continue } if sp.IsNetwork() { @@ -155,7 +155,7 @@ func (m *Manager) selectTier2TargetFrom(stackName, sourceDrive string, fullSize, // 2. Fall back to the internal SSD (system data path) — STATE-ONLY set only. sys := m.systemDataPath - if sys == "" || system.SamePhysicalDevice(sourceDrive, sys) { + if sys == "" || m.sameDevice(sourceDrive, sys) { if sawNetworkCandidate { return nil, errTier2NetworkOnly // the only off-disk candidate was a NAS } @@ -324,7 +324,7 @@ func (m *Manager) RunTier2(stackName string) error { return nil } // Defense-in-depth off-drive guard (selection already enforced it). - if system.SamePhysicalDevice(sourceDrive, target.NamespaceRoot) { + if m.sameDevice(sourceDrive, target.NamespaceRoot) { m.recordTier2NoTarget(stackName, "a kiválasztott cél ugyanazon a fizikai lemezen van") return nil } @@ -490,7 +490,7 @@ func (m *Manager) Tier2Info(stackName string) Tier2Info { } // Eligible alternative drives: registered, schedulable, on a DIFFERENT physical disk. for _, sp := range m.settings.GetSchedulableStoragePaths() { - if sp.Path == source || system.SamePhysicalDevice(source, sp.Path) { + if sp.Path == source || m.sameDevice(source, sp.Path) { continue } label := sp.Label diff --git a/controller/internal/backup/tier2_shares.go b/controller/internal/backup/tier2_shares.go index c342fef..b84a420 100644 --- a/controller/internal/backup/tier2_shares.go +++ b/controller/internal/backup/tier2_shares.go @@ -9,7 +9,6 @@ import ( "time" "gitea.dooplex.hu/admin/felhom-controller/internal/settings" - "gitea.dooplex.hu/admin/felhom-controller/internal/system" ) // Tier-2 shares job — R-7b Part 2, the local cross-drive leg of Model B′. @@ -178,7 +177,7 @@ func (m *Manager) RunSharesTier2() error { } // Defense-in-depth off-drive guard (selection already enforced it): a leg's target may never // be that leg's own source drive — that would be a same-disk "copy" pretending to be tier 2. - if system.SamePhysicalDevice(g.sourceDrive, target.NamespaceRoot) { + if m.sameDevice(g.sourceDrive, target.NamespaceRoot) { noTargetWhy = append(noTargetWhy, fmt.Sprintf("%s: a kiválasztott cél ugyanazon a fizikai lemezen van", g.sourceDrive)) continue } diff --git a/controller/internal/backup/tier2_v2_test.go b/controller/internal/backup/tier2_v2_test.go index 7520b04..56531fb 100644 --- a/controller/internal/backup/tier2_v2_test.go +++ b/controller/internal/backup/tier2_v2_test.go @@ -61,6 +61,7 @@ func newTier2V2(t *testing.T, stack string) (m *Manager, src, target string, pro m.systemDataPath = sys m.tier2Mirror = copyTree m.tier2SSDFits = func(string, int64) bool { return true } + m.samePhysicalDevice = oneDrivePerSubtree // src/target/sys are separate "drives" on one filesystem return m, src, target, prov } diff --git a/controller/internal/web/app_export_domain_test.go b/controller/internal/web/app_export_domain_test.go new file mode 100644 index 0000000..1c9a524 --- /dev/null +++ b/controller/internal/web/app_export_domain_test.go @@ -0,0 +1,76 @@ +package web + +import ( + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/stacks" +) + +// R-53 / F7: app_export.html built the app's public URL as `.{{$.CSRFToken}}` — the +// session CSRF token substituted where the customer domain belongs. That is two defects in one +// token: the „open in browser" link is wrong for every app that has a subdomain, and a CSRF token +// lands in a URL (history, referrers, logs). The correct CSRF usage on this page is the csrfH() +// helper reading the meta tag, which is untouched. + +const exportTestToken = "deadbeefcafebabedeadbeefcafebabedeadbeefcafebabedeadbeefcafebabe" + +// exportScriptLine returns the `var domain = …` line, so an assertion cannot accidentally match the +// token where it legitimately appears (the meta tag) elsewhere on the page. +func exportScriptLine(t *testing.T, html string) string { + t.Helper() + for _, ln := range strings.Split(html, "\n") { + if strings.Contains(ln, "var domain =") { + return ln + } + } + t.Fatalf("no `var domain =` line in the rendered export page") + return "" +} + +func renderExport(t *testing.T, subdomain string) string { + t.Helper() + return renderBackupPage(t, "app_export", map[string]interface{}{ + "Stack": stacks.Stack{ + Name: "immich", Deployed: true, + Meta: stacks.Metadata{Slug: "immich", DisplayName: "Immich", Subdomain: subdomain}, + }, + "Drives": nil, + "Domain": "demo-felhom.eu", + "CSRFToken": exportTestToken, + "CSRFField": "", + "Page": "apps", + "Title": "Export", + "ExportPage": true, + }) +} + +// Scenario C — the domain is joined from the CUSTOMER DOMAIN, and the CSRF token appears nowhere in +// that line. COMPANION red-proof: restore `{{$.CSRFToken}}` in app_export.html's `var domain` line +// → both assertions FAIL. Run → fail → revert (recorded in REPORT). +func TestAppExportDomainUsesCustomerDomainNotCSRFToken(t *testing.T) { + line := exportScriptLine(t, renderExport(t, "photos")) + + if !strings.Contains(line, "'photos.demo-felhom.eu'") { + t.Errorf("export link must be built from the customer domain, got: %s", line) + } + if strings.Contains(line, exportTestToken) { + t.Errorf("the CSRF token must NEVER appear in the export URL, got: %s", line) + } +} + +// The empty-subdomain branch still yields '' — an app without a subdomain must not get a link to +// a bare domain (the truthiness guard is what produces that, and the fix must not disturb it). +func TestAppExportDomainEmptyWithoutSubdomain(t *testing.T) { + line := exportScriptLine(t, renderExport(t, "")) + + if !strings.Contains(line, "''") { + t.Errorf("no subdomain must yield an empty domain, got: %s", line) + } + if strings.Contains(line, "demo-felhom.eu'") && !strings.Contains(line, "'' ? ") { + t.Errorf("no subdomain must not produce a bare-domain link, got: %s", line) + } + if strings.Contains(line, exportTestToken) { + t.Errorf("the CSRF token must NEVER appear in the export URL, got: %s", line) + } +} diff --git a/controller/internal/web/handler_export.go b/controller/internal/web/handler_export.go index 16b1bb2..2204cb8 100644 --- a/controller/internal/web/handler_export.go +++ b/controller/internal/web/handler_export.go @@ -103,6 +103,10 @@ func (s *Server) exportPageHandler(w http.ResponseWriter, r *http.Request, name data := map[string]interface{}{ "Stack": stack, "Drives": drives, + // R-53: the page builds the app's public URL from the customer domain. This handler does not + // go through baseData (which is where every other page gets "Domain"), so it must supply the + // key itself — the template used to substitute the CSRF token here instead. + "Domain": s.cfg.Customer.Domain, } s.executeTemplate(w, r, "app_export", data) } diff --git a/controller/internal/web/templates/app_export.html b/controller/internal/web/templates/app_export.html index c9100af..c3a1d8d 100644 --- a/controller/internal/web/templates/app_export.html +++ b/controller/internal/web/templates/app_export.html @@ -90,7 +90,7 @@