v0.150.0 — green gate restored + the export link stops leaking the CSRF token
F7/R-53: app_export.html built the app's public URL as '<sub>.{{$.CSRFToken}}',
so the "Megnyitás" link was wrong for every app with a subdomain and a session
CSRF token was written into a URL. Template now uses {{$.Domain}}, and
exportPageHandler supplies the key — it builds its own data map instead of
going through baseData, which is where every other page gets it. The page's
real CSRF path (csrfH() reading the meta tag) is correct and untouched.
The 7 red internal/backup tests are green again, with no behaviour change.
TestTier2V2_* / TestSharesTier2* all failed for one environmental reason:
Tier-2's off-drive guard asks system.SamePhysicalDevice (st_dev equality)
whether a target is really a second disk, and every t.TempDir() here shares one
filesystem — so the guard correctly refused the fixture's "two drives" and the
tests never reached their subject ("nincs másik fizikai meghajtó").
Seam in the package's existing style: a nil-defaulted Manager.samePhysicalDevice
field + sameDevice wrapper, seven call sites routed through it. Nil resolves to
system.SamePhysicalDevice, so production is byte-for-byte unchanged; only the two
fixtures inject a fake modelling one drive per directory subtree. No assertion
weakened, nothing skipped/renamed/deleted; all 7 mutation-proved.
Also: the ssh->pct-exec ASCII-grep and heredoc-credential traps are now in
CLAUDE.md's live-validation section.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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 `<subdomain>.{{$.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)
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -90,7 +90,7 @@
|
||||
|
||||
<script>
|
||||
var stackName = '{{.Stack.Name}}';
|
||||
var domain = '{{.Stack.Meta.Subdomain}}' ? '{{.Stack.Meta.Subdomain}}.{{$.CSRFToken}}' : '';
|
||||
var domain = '{{.Stack.Meta.Subdomain}}' ? '{{.Stack.Meta.Subdomain}}.{{$.Domain}}' : '';
|
||||
var pollTimer = null;
|
||||
|
||||
function csrfH() {
|
||||
|
||||
Reference in New Issue
Block a user