From aa04ba4c0fca35ddd2fb31bcb842231ee8dbe6b2 Mon Sep 17 00:00:00 2001 From: Dirk Petersen Date: Mon, 7 Sep 2026 17:06:39 -0700 Subject: [PATCH] maps: validate tile archives before extracting, and never publish a partial file Fixes the failure that left a device unable to read its own map tiles ("could not unmarshal offline data: unexpected EOF") and silently re-downloading ~286 MB of OSM data over cellular to repair itself. Four independent defects in the offline-tile download path, all of which end with corrupt data at a LIVE tile path: 1. No validation before extraction. The archive was extracted straight over live tiles; a truncated download was only discovered later, at read time, after it had already overwritten good data. ValidateArchive now reads the whole stream first and the extract is gated on it. The subtle part: it is not enough to walk the tar. tar.Next returns io.EOF at the tar end-of-archive marker, which sits BEFORE gzip's 8-byte CRC32+ISIZE trailer -- so a tail truncation (the last bytes lost as a connection dies) validates clean if you stop there. ValidateArchive drains the gzip stream to its end, which is what actually checks the checksum and length. My first version had this bug; the 99%-truncation case in the tests found it. 2. Downloads were written directly to their final path via os.Create. A process killed mid-transfer -- power cut, reboot, OOM -- left a truncated file that is indistinguishable from a complete one. Now staged as .part and renamed only after the transfer is verified, so the destination either does not exist or is complete. TestDestinationDoesNotExistWhileTransferIsInFlight asserts this from inside the server handler, mid-copy. 3. Extraction opened targets with os.O_CREATE|os.O_RDWR and NO os.O_TRUNC. Writing a SHORTER file over a longer existing one left the old tail attached, producing a corrupt hybrid that SURVIVED re-downloading -- which is why a bad tile could not be repaired by fetching it again. Each file is now written via .part + rename, replacing it wholesale. 4. Every extraction error was logged and ignored: a failed os.Open produced a nil file that was handed to gzip.NewReader, a failed gzip parse produced a nil reader handed to tar.NewReader, and a failed io.Copy left a half-written file in place -- after all of which the tile was still counted as successfully downloaded. Extraction now returns an error and the tile is not counted. Also adds a tar-slip guard (safeJoin): entries were joined to the base path with no check that they stay under it, so "../../.." in an archive could write anywhere. The source is a trusted HTTPS host, so this is hardening, not an observed exploit. Honest scope notes, both verified by mutation rather than assumed: * The Content-Length check is defence-in-depth, NOT the primary guard -- Go's HTTP client already reports a short body against a declared length as io.ErrUnexpectedEOF. Removing the check alone does not let truncation through. * O_TRUNC is likewise belt-and-braces. The RENAME is what fixes the stale-tail bug; removing O_TRUNC does not fail the tests. Tests: 15 new in settings/download_validate_test.go, covering intact/truncated (4 truncation points)/garbage/empty/no-file archives, the mid-transfer crash window, stale .part contamination, HTTP errors, the stale-tail overwrite, tar slip, and that a failed validation leaves live tiles untouched. go build, go vet and go test ./... all clean. Built for arm64 via native CI and running on the owner's comma 3X since 2026-09-03. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FwdRXRST5H2Hoft9yDkyU3 --- settings/download.go | 283 +++++++++++++++++++------ settings/download_validate_test.go | 324 +++++++++++++++++++++++++++++ 2 files changed, 539 insertions(+), 68 deletions(-) create mode 100644 settings/download_validate_test.go diff --git a/settings/download.go b/settings/download.go index 823da93..28c80d8 100644 --- a/settings/download.go +++ b/settings/download.go @@ -46,37 +46,224 @@ func GetDownloadMenu() (menu DownloadMenu) { return } -func DownloadFile(url string, filepath string) (err error) { +// DownloadFile fetches url into destPath. +// +// It writes to destPath+".part" and only renames into place once the transfer has been verified, +// so an interrupted download (process killed, power cut, connection dropped) can never leave a +// TRUNCATED file sitting at the real path looking like a complete one. That failure mode is not +// hypothetical: a device that lost power mid-download was later unable to read its own map tiles +// ("could not unmarshal offline data: unexpected EOF") and silently re-downloaded ~286MB over +// cellular to repair itself. +// +// The transfer is also checked against Content-Length when the server provides one. This is +// defence-in-depth rather than the primary guard: Go's HTTP client already surfaces a body that +// ends short of a declared length as io.ErrUnexpectedEOF, so removing this check does not by itself +// let a truncated download through (verified by mutation). It costs nothing and covers the case +// where a future transport or a proxy is less strict. +func DownloadFile(url string, destPath string) (err error) { slog.Info("Downloading", "url", url) - // Create the file - out, err := os.Create(filepath) + + partPath := destPath + ".part" + // Remove any leftover .part from a previous interrupted attempt before starting. + _ = os.Remove(partPath) + + out, err := os.Create(partPath) if err != nil { return errors.Wrap(err, "could not create file for download") } - defer out.Close() + // On any failure below, drop the partial file rather than leaving it to be mistaken for good + // data later. Named return + defer so every early return is covered. + defer func() { + out.Close() + if err != nil { + _ = os.Remove(partPath) + } + }() - // Get the data resp, err := http.Get(url) if err != nil { return errors.Wrap(err, "could not download the file data") } defer resp.Body.Close() - // Check server response if resp.StatusCode != http.StatusOK { return errors.Errorf("download received bad status: %s", resp.Status) } - // Writer the body to file - _, err = io.Copy(out, resp.Body) + written, err := io.Copy(out, resp.Body) if err != nil { return errors.Wrap(err, "could not write download data to file") } - err = out.Sync() - if err != nil { + // ContentLength is -1 when the server doesn't declare one (chunked); only assert when known. + if resp.ContentLength >= 0 && written != resp.ContentLength { + return errors.Errorf("truncated download: got %d bytes, expected %d", written, resp.ContentLength) + } + if err = out.Sync(); err != nil { return errors.Wrap(err, "could not fsync downloaded file") } + if err = out.Close(); err != nil { + return errors.Wrap(err, "could not close downloaded file") + } + // Atomic publish: either the complete file appears at destPath, or nothing does. + if err = os.Rename(partPath, destPath); err != nil { + return errors.Wrap(err, "could not move downloaded file into place") + } + return nil +} + +// ValidateArchive reads the whole gzip+tar stream and discards it, purely to prove the archive is +// intact BEFORE any live map tile is touched. +// +// This is the load-bearing check. gzip carries a CRC32 and length trailer that is only verified once +// the stream is read to the end, so a truncated archive is indistinguishable from a good one until +// something reads all of it. Extracting first and discovering the problem afterwards is exactly how +// half-written tiles reach disk. +func ValidateArchive(path string) error { + file, err := os.Open(path) + if err != nil { + return errors.Wrap(err, "could not open archive for validation") + } + defer file.Close() + + reader, err := gzip.NewReader(file) + if err != nil { + return errors.Wrap(err, "could not parse archive gzip header") + } + defer reader.Close() + + tr := tar.NewReader(reader) + entries := 0 + for { + header, err := tr.Next() + if err == io.EOF { + break // clean end of archive + } + if err != nil { + return errors.Wrap(err, "archive is corrupt or truncated") + } + if header == nil { + continue + } + // Read the entry body so gzip's CRC/length trailer is actually checked. + if _, err := io.Copy(io.Discard, tr); err != nil { + return errors.Wrap(err, "archive entry is corrupt or truncated") + } + entries++ + } + if entries == 0 { + return errors.Errorf("archive contains no files") + } + + // Drain whatever is left of the GZIP stream. This is not redundant: tar.Next returns io.EOF at + // the tar end-of-archive marker, which sits BEFORE gzip's 8-byte CRC32+ISIZE trailer, so + // stopping at the tar level never checks the trailer at all. A small tail truncation -- the + // last few bytes lost as a connection dies -- otherwise validates clean. Reading to the end of + // the gzip stream is what actually verifies the checksum and the uncompressed length. + if _, err := io.Copy(io.Discard, reader); err != nil { + return errors.Wrap(err, "archive gzip stream is corrupt or truncated") + } + return nil +} + +// safeJoin resolves a tar entry name under base, refusing anything that would escape it. +// Standard tar-slip guard: an entry named "../../etc/whatever" must never be written. +func safeJoin(base string, name string) (string, error) { + target := filepath.Join(base, name) + cleanBase := filepath.Clean(base) + string(os.PathSeparator) + if !strings.HasPrefix(filepath.Clean(target)+string(os.PathSeparator), cleanBase) { + return "", errors.Errorf("archive entry escapes destination: %s", name) + } + return target, nil +} + +// writeFileAtomic writes r to target via target+".part" then renames, so a crash mid-extract leaves +// a stray .part rather than a half-written map tile at a live path. +// +// It also fixes a subtler bug in the original extractor, which opened targets with +// os.O_CREATE|os.O_RDWR and NO os.O_TRUNC: writing a SHORTER file over a longer existing one left +// the tail of the old file attached, producing a corrupt hybrid that survived re-downloading. +// The RENAME is what fixes that -- it replaces the file wholesale, so length cannot carry over. +// O_TRUNC below is belt-and-braces for the .part itself. +func writeFileAtomic(target string, mode os.FileMode, r io.Reader) (err error) { + partPath := target + ".part" + _ = os.Remove(partPath) + f, err := os.OpenFile(partPath, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, mode) + if err != nil { + return errors.Wrap(err, "could not open extract target") + } + defer func() { + f.Close() + if err != nil { + _ = os.Remove(partPath) + } + }() + if _, err = io.Copy(f, r); err != nil { + return errors.Wrap(err, "could not write extract target") + } + if err = f.Sync(); err != nil { + return errors.Wrap(err, "could not fsync extract target") + } + if err = f.Close(); err != nil { + return errors.Wrap(err, "could not close extract target") + } + return errors.Wrap(os.Rename(partPath, target), "could not move extracted file into place") +} + +// extractArchive unpacks a VALIDATED archive into the base path. +// +// Unlike the original inline extractor this returns an error instead of logging and pressing on: +// every one of those old warn-and-continue paths (open failed, gzip parse failed, copy failed) left +// a partially-written file at a LIVE tile path and still counted the tile as successfully +// downloaded. Each regular file is written atomically, so an interrupted extract leaves a stray +// .part rather than a corrupt tile. +func extractArchive(archivePath string) error { + file, err := os.Open(archivePath) + if err != nil { + return errors.Wrap(err, "could not open archive") + } + defer file.Close() + + reader, err := gzip.NewReader(file) + if err != nil { + return errors.Wrap(err, "could not parse archive gzip") + } + defer reader.Close() + + base := params.GetBaseOpPath() + tr := tar.NewReader(reader) + for { + header, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + return errors.Wrap(err, "could not read archive entry") + } + if header == nil { + continue + } + target, err := safeJoin(base, header.Name) + if err != nil { + return err + } + switch header.Typeflag { + case tar.TypeDir: + if _, statErr := os.Stat(target); statErr != nil { + if err := os.MkdirAll(target, 0o755); err != nil { + return errors.Wrap(err, "could not create directory from archive") + } + } + case tar.TypeReg: + // The parent dir may not exist if the archive omits explicit dir entries. + if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { + return errors.Wrap(err, "could not create parent directory for archive entry") + } + if err := writeFileAtomic(target, os.FileMode(header.Mode), tr); err != nil { + return err + } + } + } return nil } @@ -207,69 +394,29 @@ func (d *download) downloadBounds(bounds Bounds, locationName string) (err error slog.Warn("failed to download file, continuing to next", "error", err, "url", url, "file", outputName) continue } - file, err := os.Open(outputName) - if err != nil { - slog.Warn("failed to open downloaded file", "error", err, "file", outputName) - } - reader, err := gzip.NewReader(file) - if err != nil { - slog.Warn("failed to parse gzip downloaded file", "error", err, "file", outputName) - } - tr := tar.NewReader(reader) - for { - header, err := tr.Next() - if err != nil { - break - } - // if the header is nil, just skip it (not sure how this happens) - if header == nil { - continue + // VALIDATE BEFORE EXTRACTING. The archive is read end-to-end (which is what actually + // verifies gzip's CRC/length trailer) before a single live tile is touched. A truncated + // or corrupt download is discarded here instead of being half-written over good map + // data -- the failure that left this device unable to read its own tiles. + if err = ValidateArchive(outputName); err != nil { + slog.Warn("downloaded archive failed validation, discarding", "error", err, "url", url, "file", outputName) + if rmErr := os.Remove(outputName); rmErr != nil { + slog.Warn("could not delete invalid archive", "error", rmErr, "file", outputName) } - // the target location where the dir/file should be created - target := filepath.Join(params.GetBaseOpPath(), header.Name) - // check the file type - switch header.Typeflag { - - // if its a dir and it doesn't exist create it - case tar.TypeDir: - if _, err := os.Stat(target); err != nil { - err := os.MkdirAll(target, 0o755) - if err != nil { - slog.Warn("could not create directory from downloaded gzip", "error", err, "file", outputName, "directory", target) - } - } - - // if it's a file create it - case tar.TypeReg: - f, err := os.OpenFile(target, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode)) - if err != nil { - slog.Warn("could not open file target from downloaded gzip", "error", err, "file", outputName, "targetFile", target) - } - - _, err = io.Copy(f, tr) - if err != nil { - slog.Warn("could not write data to file target from downloaded gzip", "error", err, "file", outputName, "targetFile", target) - } - - err = f.Sync() - if err != nil { - slog.Warn("could not fsync file target from downloaded gzip", "error", err, "file", outputName, "targetFile", target) - } - f.Close() - } - } - err = reader.Close() - if err != nil { - slog.Warn("could not close gzip reader", "error", err) + continue } - err = file.Close() - if err != nil { - slog.Warn("could not close downloaded file", "error", err) + + if err = extractArchive(outputName); err != nil { + // Do NOT count this tile as downloaded -- it isn't. + slog.Warn("failed to extract archive", "error", err, "file", outputName) + if rmErr := os.Remove(outputName); rmErr != nil { + slog.Warn("could not delete archive", "error", rmErr, "file", outputName) + } + continue } - err = os.Remove(outputName) - if err != nil { + if err = os.Remove(outputName); err != nil { slog.Warn("could not delete downloaded gzip file", "error", err) } diff --git a/settings/download_validate_test.go b/settings/download_validate_test.go new file mode 100644 index 0000000..236721a --- /dev/null +++ b/settings/download_validate_test.go @@ -0,0 +1,324 @@ +package settings + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +// buildArchive returns a gzip+tar archive containing the given name->content entries. +func buildArchive(t *testing.T, files map[string]string) []byte { + t.Helper() + var buf bytes.Buffer + gz := gzip.NewWriter(&buf) + tw := tar.NewWriter(gz) + for name, content := range files { + if err := tw.WriteHeader(&tar.Header{ + Name: name, Mode: 0o644, Size: int64(len(content)), Typeflag: tar.TypeReg, + }); err != nil { + t.Fatal(err) + } + if _, err := tw.Write([]byte(content)); err != nil { + t.Fatal(err) + } + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + if err := gz.Close(); err != nil { + t.Fatal(err) + } + return buf.Bytes() +} + +func writeTemp(t *testing.T, dir, name string, data []byte) string { + t.Helper() + p := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, data, 0o644); err != nil { + t.Fatal(err) + } + return p +} + +// --- ValidateArchive ------------------------------------------------------------------ + +func TestValidateAcceptsIntactArchive(t *testing.T) { + dir := t.TempDir() + p := writeTemp(t, dir, "good.tar.gz", buildArchive(t, map[string]string{"offline/46/-124/data": "hello"})) + if err := ValidateArchive(p); err != nil { + t.Fatalf("intact archive rejected: %v", err) + } +} + +// This is THE case that broke the device: the file looks like a gzip, has a valid header, and only +// reveals itself as short when the stream is read to its CRC/length trailer. +func TestValidateRejectsTruncatedArchive(t *testing.T) { + dir := t.TempDir() + full := buildArchive(t, map[string]string{"offline/46/-124/data": strings.Repeat("x", 40000)}) + // 99% matters most: it is only a couple of bytes short, which is what a connection dying at the + // very end looks like. That case slipped through until ValidateArchive drained the gzip trailer. + for _, frac := range []int{10, 50, 90, 99} { + cut := len(full) * frac / 100 + p := writeTemp(t, dir, "trunc.tar.gz", full[:cut]) + if err := ValidateArchive(p); err == nil { + t.Fatalf("truncated archive (%d%% of %d bytes) passed validation", frac, len(full)) + } + } +} + +func TestValidateRejectsGarbageAndEmpty(t *testing.T) { + dir := t.TempDir() + for name, data := range map[string][]byte{ + "garbage.tar.gz": []byte("this is not a gzip stream at all"), + "empty.tar.gz": {}, + } { + if err := ValidateArchive(writeTemp(t, dir, name, data)); err == nil { + t.Fatalf("%s passed validation", name) + } + } +} + +func TestValidateRejectsArchiveWithNoFiles(t *testing.T) { + dir := t.TempDir() + p := writeTemp(t, dir, "nofiles.tar.gz", buildArchive(t, map[string]string{})) + if err := ValidateArchive(p); err == nil { + t.Fatal("empty-but-well-formed archive passed validation") + } +} + +func TestValidateRejectsMissingFile(t *testing.T) { + if err := ValidateArchive(filepath.Join(t.TempDir(), "nope.tar.gz")); err == nil { + t.Fatal("missing file passed validation") + } +} + +// --- DownloadFile --------------------------------------------------------------------- + +func TestDownloadWritesCompleteFileAndLeavesNoPart(t *testing.T) { + body := []byte("complete payload") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write(body) + })) + defer srv.Close() + + dest := filepath.Join(t.TempDir(), "out.bin") + if err := DownloadFile(srv.URL, dest); err != nil { + t.Fatalf("download failed: %v", err) + } + got, err := os.ReadFile(dest) + if err != nil || !bytes.Equal(got, body) { + t.Fatalf("content mismatch: %q vs %q (%v)", got, body, err) + } + if _, err := os.Stat(dest + ".part"); !os.IsNotExist(err) { + t.Fatal(".part file left behind after a successful download") + } +} + +// The server declares a Content-Length it does not honour -- the shape of a connection that drops +// mid-body. Without the length check this lands as a short file that reads as a clean EOF. +func TestDownloadRejectsShortBodyAgainstContentLength(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Length", "1000") + w.WriteHeader(http.StatusOK) + w.Write([]byte("only a few bytes")) + if f, ok := w.(http.Flusher); ok { + f.Flush() + } + })) + defer srv.Close() + + dest := filepath.Join(t.TempDir(), "short.bin") + if err := DownloadFile(srv.URL, dest); err == nil { + t.Fatal("short body accepted despite Content-Length mismatch") + } + if _, err := os.Stat(dest); !os.IsNotExist(err) { + t.Fatal("a truncated download was published to the destination path") + } +} + +func TestDownloadDoesNotPublishOnHTTPError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Error(w, "nope", http.StatusInternalServerError) + })) + defer srv.Close() + + dest := filepath.Join(t.TempDir(), "err.bin") + if err := DownloadFile(srv.URL, dest); err == nil { + t.Fatal("HTTP 500 accepted") + } + for _, p := range []string{dest, dest + ".part"} { + if _, err := os.Stat(p); !os.IsNotExist(err) { + t.Fatalf("%s should not exist after a failed download", p) + } + } +} + +// A leftover .part from a previous interrupted attempt must not contaminate the next one. +func TestDownloadOverwritesStalePartFile(t *testing.T) { + body := []byte("fresh") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write(body) + })) + defer srv.Close() + + dir := t.TempDir() + dest := filepath.Join(dir, "out.bin") + writeTemp(t, dir, "out.bin.part", []byte("stale garbage from a killed run")) + + if err := DownloadFile(srv.URL, dest); err != nil { + t.Fatalf("download failed: %v", err) + } + got, _ := os.ReadFile(dest) + if !bytes.Equal(got, body) { + t.Fatalf("stale .part contaminated the result: %q", got) + } +} + +// The POINT of the .part indirection is crash safety: while bytes are in flight, the destination +// path must not exist, so a process killed mid-transfer leaves no file that a later run could +// mistake for a complete tile. Asserted from inside the server handler, which runs while +// DownloadFile is still copying. +func TestDestinationDoesNotExistWhileTransferIsInFlight(t *testing.T) { + dir := t.TempDir() + dest := filepath.Join(dir, "inflight.bin") + + var midDestExists, midPartExists bool + checked := make(chan struct{}) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write(bytes.Repeat([]byte("a"), 4096)) + if f, ok := w.(http.Flusher); ok { + f.Flush() + } + // The client is now mid-copy; look at what is on disk. + _, destErr := os.Stat(dest) + _, partErr := os.Stat(dest + ".part") + midDestExists = destErr == nil + midPartExists = partErr == nil + close(checked) + w.Write(bytes.Repeat([]byte("b"), 4096)) + })) + defer srv.Close() + + if err := DownloadFile(srv.URL, dest); err != nil { + t.Fatalf("download failed: %v", err) + } + <-checked + if midDestExists { + t.Fatal("destination path existed mid-transfer: a crash here would leave a truncated tile") + } + if !midPartExists { + t.Fatal("no .part file during transfer -- bytes are not being staged") + } + if _, err := os.Stat(dest); err != nil { + t.Fatalf("destination missing after a successful download: %v", err) + } +} + +// --- writeFileAtomic ------------------------------------------------------------------ + +// The original extractor opened targets with O_CREATE|O_RDWR and no O_TRUNC. Writing a SHORTER file +// over a longer one left the old tail attached, producing a corrupt hybrid that SURVIVED +// re-downloading -- which is why repeated repair attempts could not fix a bad tile. +// +// NOTE on what actually fixes this: the temp-file + rename does. Writing into a freshly-created +// .part and renaming replaces the target wholesale, so length can never carry over regardless of +// the open flags. O_TRUNC is belt-and-braces (it matters only if the .part itself survives), and +// removing it does NOT make this test fail -- verified by mutation. The test is kept because it +// pins the observable property (no stale tail) rather than the mechanism. +func TestExtractReplacesLongerFileWholesale(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "tile") + old := strings.Repeat("O", 500) + if err := os.WriteFile(target, []byte(old), 0o644); err != nil { + t.Fatal(err) + } + + newContent := "short new content" + if err := writeFileAtomic(target, 0o644, strings.NewReader(newContent)); err != nil { + t.Fatalf("atomic write failed: %v", err) + } + got, err := os.ReadFile(target) + if err != nil { + t.Fatal(err) + } + if string(got) != newContent { + t.Fatalf("stale tail survived the overwrite: got %d bytes %q, want %d", len(got), got, len(newContent)) + } +} + +func TestExtractLeavesNoPartOnSuccess(t *testing.T) { + target := filepath.Join(t.TempDir(), "tile") + if err := writeFileAtomic(target, 0o644, strings.NewReader("x")); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(target + ".part"); !os.IsNotExist(err) { + t.Fatal(".part left behind after a successful extract") + } +} + +// --- safeJoin: tar-slip --------------------------------------------------------------- + +func TestSafeJoinRejectsEscapingEntries(t *testing.T) { + base := "/data/media/0/osm" + // NB: an ABSOLUTE entry name like "/etc/shadow" is NOT in this list. filepath.Join neutralizes + // it into base+"/etc/shadow", which stays inside the destination -- it is ugly but not an + // escape. Only ".." traversal actually gets out. + for _, name := range []string{ + "../../../etc/passwd", + "offline/../../../../tmp/evil", + "../offline/46/-124/data", + } { + if _, err := safeJoin(base, name); err == nil { + t.Fatalf("entry %q was allowed to escape the destination", name) + } + } +} + +func TestSafeJoinAllowsNormalEntries(t *testing.T) { + base := "/data/media/0/osm" + for _, name := range []string{"offline/46/-124/data", "offline/44/-122/bounds", "/etc/shadow"} { + got, err := safeJoin(base, name) + if err != nil { + t.Fatalf("legitimate entry %q rejected: %v", name, err) + } + if !strings.HasPrefix(got, base) { + t.Fatalf("unexpected target %q", got) + } + } +} + +// --- end to end ----------------------------------------------------------------------- + +// A truncated archive must leave existing good tiles untouched: validation runs before extraction. +func TestTruncatedArchiveNeverTouchesLiveTiles(t *testing.T) { + dir := t.TempDir() + live := filepath.Join(dir, "existing_tile") + good := strings.Repeat("G", 300) + if err := os.WriteFile(live, []byte(good), 0o644); err != nil { + t.Fatal(err) + } + + full := buildArchive(t, map[string]string{"existing_tile": "replacement"}) + p := writeTemp(t, dir, "bad.tar.gz", full[:len(full)*60/100]) + + if err := ValidateArchive(p); err == nil { + t.Fatal("truncated archive passed validation") + } + // extraction is gated on validation, so the live tile is still the original + got, _ := os.ReadFile(live) + if string(got) != good { + t.Fatal("a live tile was modified by a download that failed validation") + } +} + +var _ = io.Discard