fix(R-1187): the import's compose runs get the controller's path variables; a compose warning is never read as the container id
gates / gates (push) Successful in 50s

Found live on 9202 (grimmory import timed out on the DB step and left the app down). Red-proved.
This commit is contained in:
2026-10-26 21:38:46 +01:00
parent a1c65b3d81
commit a3ed63ee9a
3 changed files with 81 additions and 5 deletions
+6
View File
@@ -47,6 +47,12 @@ hub answers 400 to both and nothing else changes.
A third review finding: a link TARGET through a link already in the folder („x" → „evil/y", evil pointing out). The rule
is now strict and simple: a restored link points only DOWNWARD — relative, no „..", no link on its path or in its
target; everything else is refused and listed (`safeDownwardLink`; `TestR1187b_NoLinkTargetThroughALink`, red-proved).
**Found live on 9202 the same night (grimmory), fixed:** the import ran `compose` without the controller's own path
variables (`USERDATA_PATH`, `IMPORT_PATH` — `stacks.policyEnv` adds them at every install and start); compose printed
„The USERDATA_PATH variable is not set" and the DB step read that WARNING as the container id, waited 60 s and left the
app removed and not started. Now `withImportPathVars` gives the import's compose runs the same derivation (never a
bundle's value) and `containerIDFrom` takes only a line that IS an id. Tests `TestR1187_TheContainerIDIsNeverAComposeWarning`,
`TestR1187_TheImportEnvCarriesTheControllersPathVars`, red-proved.
- **R-1298:** the hold report skips an install's hold (`unhealthy_stop`, `update_failed`, `putback_pending`) of an app that
is not installed (`holdreport.Reporter.SetInstalled`: the app's `app.yaml` exists — an unreadable one still counts,
R-1220) and reports it cleared if it was reported; a restore hold is always reported (R-379). 9202's four orphan holds
@@ -0,0 +1,40 @@
package appexport
import (
"testing"
"gitea.dooplex.hu/admin/felhom-controller/internal/appbackup"
)
// R-1187, found live on 9202 2026-10-26 night (grimmory): the import ran `compose` WITHOUT the two path variables the
// controller adds at every install and start (USERDATA_PATH, IMPORT_PATH — stacks.policyEnv). Compose then printed
// „The USERDATA_PATH variable is not set" on its combined output, the DB step read that WARNING as the container id,
// waited 60 s for a container that does not exist, and left the app removed and not started. Both halves pinned here.
// RED-PROOF: with the old helpers (the first output line; the env unchanged) both tests fail.
func TestR1187_TheContainerIDIsNeverAComposeWarning(t *testing.T) {
out := []byte("time=\"2026-10-26T20:18:06Z\" level=warning msg=\"The \\\"USERDATA_PATH\\\" variable is not set. Defaulting to a blank string.\"\n" +
"1047c698c778ab12cd34ef56ab78cd90ef12ab34cd56ef78ab90cd12ef34ab56\n")
if got := containerIDFrom(out); got != "1047c698c778ab12cd34ef56ab78cd90ef12ab34cd56ef78ab90cd12ef34ab56" {
t.Fatalf("container id = %q", got)
}
if got := containerIDFrom([]byte("time=x level=warning msg=y\n")); got != "" {
t.Fatalf("a warning alone must give no id, got %q", got)
}
}
func TestR1187_TheImportEnvCarriesTheControllersPathVars(t *testing.T) {
env := map[string]string{"HDD_PATH": "/mnt/felhom-drives/d", "USERDATA_PATH": "/mnt/felhom-drives/d/appdata/other", "X": "1"}
got := withImportPathVars(env, "/mnt/felhom-drives/d", "/mnt/sys_drive/felhom-data/userdata/import")
if got["USERDATA_PATH"] != appbackup.UserdataDir("/mnt/felhom-drives/d") {
t.Fatalf("USERDATA_PATH = %q — must be the controller's own derivation, never the bundle's", got["USERDATA_PATH"])
}
if got["IMPORT_PATH"] != "/mnt/sys_drive/felhom-data/userdata/import" || got["X"] != "1" || got["HDD_PATH"] != "/mnt/felhom-drives/d" {
t.Fatalf("env = %v", got)
}
if env["USERDATA_PATH"] != "/mnt/felhom-drives/d/appdata/other" {
t.Fatal("the caller's map was changed")
}
if g := withImportPathVars(map[string]string{"IMPORT_PATH": "/x"}, "", ""); g["USERDATA_PATH"] != "" || g["IMPORT_PATH"] != "" {
t.Fatalf("no namespace / no import root must leave neither variable: %v", g)
}
}
+35 -5
View File
@@ -442,6 +442,8 @@ func (e *Exporter) executeImport(req ImportRequest, job *Job) {
e.failJob(job, step, fmt.Sprintf("Konfiguráció visszaállítása sikertelen: %v", err))
return
}
// R-1187 (found live): compose runs below see the controller's path variables, as at every install and start.
env = withImportPathVars(env, e.provider.GetStackNamespaceRoot(manifest.AppName), e.provider.GetImportRoot())
e.debugf("config restored: %d env vars", len(env))
if e.debug {
for k := range env {
@@ -1029,17 +1031,45 @@ func getComposeContainerID(stackDir string, env map[string]string, service strin
if err != nil {
return "", fmt.Errorf("compose ps: %w", err)
}
id := strings.TrimSpace(string(out))
id := containerIDFrom(out)
if id == "" {
return "", fmt.Errorf("no container found for service %s", service)
}
// Take the first line in case of multiple
if idx := strings.Index(id, "\n"); idx > 0 {
id = id[:idx]
}
return id, nil
}
// containerIDFrom picks the container id out of `compose ps -q` output: the first line that IS an id (12–64 hex). The
// output is combined, so a compose WARNING can come first — it was read as the id once (R-1187, found live on 9202).
func containerIDFrom(out []byte) string {
for _, line := range strings.Split(string(out), "\n") {
line = strings.TrimSpace(line)
if n := len(line); n >= 12 && n <= 64 && strings.Trim(line, "0123456789abcdef") == "" {
return line
}
}
return ""
}
// withImportPathVars is the import's env as `compose` must see it: the bundle's values plus the two path variables the
// controller itself adds at every install and start (stacks.policyEnv) — USERDATA_PATH always the controller's own
// derivation from the namespace root (never a bundle's value), IMPORT_PATH the drop-zone root. Without them compose
// printed a warning the DB step read as the container id (R-1187, live on 9202). Returns a new map.
func withImportPathVars(env map[string]string, nsRoot, importRoot string) map[string]string {
out := make(map[string]string, len(env)+2)
for k, v := range env {
out[k] = v
}
delete(out, "USERDATA_PATH")
delete(out, "IMPORT_PATH")
if nsRoot != "" {
out["USERDATA_PATH"] = appbackup.UserdataDir(nsRoot)
}
if importRoot != "" {
out["IMPORT_PATH"] = importRoot
}
return out
}
// waitForDB polls until the database is ready to accept connections.
func waitForDB(containerID, dbType string, env map[string]string) error {
timeout := 60 * time.Second