fix(R-1187 a,b,d,f,g): .fab — links carried and restored safely, FIFOs refused, owners kept, busy and bad-path refusals, postgis + ON_ERROR_STOP

Red-proved (6 tests). Commit only.
2026-10-26 20:26:02 +01:00
parent 1170b48e59
commit dfd0f8ec53
8 changed files with 321 additions and 31 deletions
@@ -5,6 +5,7 @@ import (
 	"compress/gzip"
 	"context"
 	"fmt"
+	"gitea.dooplex.hu/admin/felhom-controller/internal/appbackup"
 	"gitea.dooplex.hu/admin/felhom-controller/internal/dockerexec"
 	"gitea.dooplex.hu/admin/felhom-controller/internal/util"
 	"io"
@@ -184,6 +185,16 @@ func (e *Exporter) StartImport(req ImportRequest) error {
 		e.debugf("StartImport rejected: another job is already running")
 		return fmt.Errorf("export or import already in progress")
 	}
+	// R-1187 (d): another operation holds the app-stop marker (a restore, a put-back, a stop-first export) — an import
+	// now would stop and rewrite an app that operation is moving. Refused before anything is read. Pinned by
+	// TestR1187d_AnImportWaitsForARestore.
+	if e.stopGuard != nil {
+		if busy := e.stopGuard.BusyWith(); busy != "" {
+			e.mu.Unlock()
+			e.logger.Printf("[WARN] Import: refused — another operation holds the box (%s) (R-1187 d)", busy)
+			return util.MsgError("err.appexport.import_foglalt", busy)
+		}
+	}

 	// Validate file exists
 	info, err := os.Stat(req.FABPath)
@@ -674,9 +685,14 @@ func (e *Exporter) restoreHDDData(tmpDir string, manifest *Manifest, composePath

 		e.logger.Printf("[INFO] Import: extracting HDD data %s → %s", subdir, destPath)
 		extractStart := time.Now()
-		if err := extractTar(tarPath, destPath); err != nil {
+		refused, err := extractTar(tarPath, destPath)
+		if err != nil {
 			return fmt.Errorf("extracting %s: %w", subdir, err)
 		}
+		if len(refused) > 0 {
+			e.logger.Printf("[WARN] Import: %s — %d link(s) pointing outside the app's folder were NOT restored (R-1187 b): %s",
+				subdir, len(refused), strings.Join(refused, ", "))
+		}
 		e.debugf("restoreHDDData: extracted %s in %v", subdir, time.Since(extractStart))
 	}

@@ -726,6 +742,16 @@ func resolveHDDMounts(composePath string, env map[string]string) []string {
 // validateBundleData asserts every manifest-claimed data tar exists non-empty in the extracted
 // bundle (v0.125.0, scenario C). Pure read — called before ANY destructive import step.
 func validateBundleData(tmpDir string, manifest *Manifest) error {
+	// R-1187 (f): drive data needs an ABSOLUTE, clean HDD_PATH from the bundle's app.yaml — an empty one made the drive
+	// tars extract into a relative path (the controller's working directory). Refused before anything is touched.
+	// Pinned by TestR1187f_AnEmptyOrRelativeHDDPathIsRefused.
+	if len(manifest.HDDSubdirs) > 0 {
+		env, _ := readPlaintextAppYaml(filepath.Join(tmpDir, "config", "app.yaml"))
+		hdd := strings.TrimSpace(env["HDD_PATH"])
+		if hdd == "" || !filepath.IsAbs(hdd) || filepath.Clean(hdd) != hdd || strings.Contains(hdd, "..") {
+			return util.MsgError("err.appexport.hdd_path_hibas", hdd)
+		}
+	}
 	for _, v := range manifest.VolumeNames {
 		if err := ValidateSegment("volume_name", v); err != nil {
 			return err
@@ -967,11 +993,12 @@ func findDBServiceInCompose(composePath string) *dbServiceInfo {
 }

 func classifyDBImage(service, image, container string) *dbServiceInfo {
-	img := strings.ToLower(image)
-	if strings.Contains(img, "postgres") {
+	// R-1187 (g): the backup side's predicate (appbackup.DBTypeForImage, R-484) — postgis / pgvector / timescaledb are
+	// PostgreSQL too. The import used its own narrower list and skipped adventurelog's and dawarich's database.
+	switch t, ok := appbackup.DBTypeForImage(image); {
+	case ok && t == appbackup.DBTypePostgres:
 		return &dbServiceInfo{ServiceName: service, DBType: "postgres", ContainerName: container}
-	}
-	if strings.Contains(img, "mariadb") || strings.Contains(img, "mysql") {
+	case ok && t == appbackup.DBTypeMariaDB:
 		return &dbServiceInfo{ServiceName: service, DBType: "mariadb", ContainerName: container}
 	}
 	return nil
@@ -1056,6 +1083,13 @@ func waitForDB(containerID, dbType string, env map[string]string) error {
 	}
 }

+// psqlImportArgs is the `docker` argv that loads a Postgres dump.
+func psqlImportArgs(containerID, user, dbName string) []string {
+	// R-1187 (g): stop on the first error — the dump is --clean --if-exists, so a failing statement is a real failure,
+	// and without ON_ERROR_STOP psql went on and the import reported success over a half-loaded database.
+	return []string{"exec", "-i", containerID, "psql", "-v", "ON_ERROR_STOP=1", "-U", user, "-d", dbName}
+}
+
 // importDBDump imports a (possibly gzipped) SQL dump into the running database container.
 func importDBDump(containerID, dumpPath, dbType string, env map[string]string) error {
 	// Open the dump file
@@ -1090,8 +1124,7 @@ func importDBDump(containerID, dumpPath, dbType string, env map[string]string) e
 		if dbName == "" {
 			dbName = user
 		}
-		cmd = dockerexec.CommandContext(ctx, "docker", "exec", "-i", containerID,
-			"psql", "-U", user, "-d", dbName)
+		cmd = dockerexec.CommandContext(ctx, "docker", psqlImportArgs(containerID, user, dbName)...)
 	case "mariadb":
 		password := env["MYSQL_ROOT_PASSWORD"]
 		if password == "" {
@@ -1170,50 +1203,80 @@ func extractTarGz(tgzPath, destDir string) error {
 	}
 }

-// extractTar extracts a plain tar archive to a directory.
-func extractTar(tarPath, destDir string) error {
+// lchownFn is the owner seam (a test records it; production is os.Lchown).
+var lchownFn = os.Lchown
+
+// extractTar extracts a plain tar archive (one drive folder) into destDir and returns the entries it REFUSED.
+//
+// R-1187 (b): owners and modes are kept (Lchown — the controller runs as root on a box), and a symlink is restored when
+// it stays inside destDir; a link pointing outside (absolute, or climbing out with ..) is refused and listed — never
+// followed, never created. Only folders, regular files and links exist in a drive tar (the export refuses the rest).
+// Pinned by TestR1187b_TheImportKeepsOwnersModesAndInsideLinks.
+func extractTar(tarPath, destDir string) ([]string, error) {
 	f, err := os.Open(tarPath)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	defer f.Close()
-
+	root := filepath.Clean(destDir)
+	var refused []string
 	tr := tar.NewReader(f)
 	for {
 		hdr, err := tr.Next()
 		if err == io.EOF {
-			return nil
+			return refused, nil
 		}
 		if err != nil {
-			return err
+			return refused, err
 		}
-
-		target := filepath.Join(destDir, hdr.Name)
-
-		// Security: prevent path traversal
-		if !strings.HasPrefix(filepath.Clean(target), filepath.Clean(destDir)+string(os.PathSeparator)) {
-			continue
+		target := filepath.Join(root, hdr.Name)
+		if !strings.HasPrefix(filepath.Clean(target), root+string(os.PathSeparator)) {
+			continue // path traversal
 		}
-
 		switch hdr.Typeflag {
 		case tar.TypeDir:
-			if err := os.MkdirAll(target, os.FileMode(hdr.Mode)); err != nil {
-				return err
+			if err := os.MkdirAll(target, os.FileMode(hdr.Mode).Perm()); err != nil {
+				return refused, err
 			}
+			os.Chmod(target, os.FileMode(hdr.Mode).Perm())
 		case tar.TypeReg:
 			if err := os.MkdirAll(filepath.Dir(target), 0755); err != nil {
-				return err
+				return refused, err
 			}
 			outFile, err := os.Create(target)
 			if err != nil {
-				return err
+				return refused, err
 			}
 			if _, err := io.Copy(outFile, tr); err != nil {
 				outFile.Close()
-				return err
+				return refused, err
 			}
 			outFile.Close()
-			os.Chmod(target, os.FileMode(hdr.Mode))
+			os.Chmod(target, os.FileMode(hdr.Mode).Perm())
+		case tar.TypeSymlink:
+			dst := hdr.Linkname
+			if filepath.IsAbs(dst) {
+				refused = append(refused, hdr.Name)
+				continue
+			}
+			resolved := filepath.Clean(filepath.Join(filepath.Dir(target), dst))
+			if resolved != root && !strings.HasPrefix(resolved, root+string(os.PathSeparator)) {
+				refused = append(refused, hdr.Name)
+				continue
+			}
+			if err := os.MkdirAll(filepath.Dir(target), 0755); err != nil {
+				return refused, err
+			}
+			os.Remove(target)
+			if err := os.Symlink(dst, target); err != nil {
+				return refused, err
+			}
+		default:
+			refused = append(refused, hdr.Name)
+			continue
+		}
+		if err := lchownFn(target, hdr.Uid, hdr.Gid); err != nil && !os.IsPermission(err) {
+			return refused, fmt.Errorf("setting the owner of %s: %w", hdr.Name, err)
 		}
 	}
 }