diff --git a/README.md b/README.md index ba610a2..9b2a83b 100644 --- a/README.md +++ b/README.md @@ -122,6 +122,14 @@ The following data can be extracted: | A list of files on the system, optionally including on-device hashes. | :white_check_mark: | `files.json` | | A copy of the files available in temp folders. | | `tmp/*` | | A bug report containing system and app-specific logs, with no private data included. | | `bugreport.zip` | +| Existing bugreports and companion files from `/bugreports/`, collected before generating a new report. | | `bugreports/` | + +Existing bugreport collection is limited to files accessible through `/bugreports/` +using the ADB shell's permissions. It follows that directory's symlink, but not +symlinks inside it. Exported copies in shared storage and vendor-specific locations +are not searched. Android may already have deleted older reports under its +retention policy, so an empty or missing directory does not mean no reports were +ever generated. Existing files are copied without deleting them from the device. Every acquisition also contains `acquisition.json`, `command.log` when log output was produced, and `hashes.csv`. The hash list records the SHA-256 digest of each diff --git a/modules/bugreport.go b/modules/bugreport.go index 975ce68..477b7a3 100644 --- a/modules/bugreport.go +++ b/modules/bugreport.go @@ -6,9 +6,13 @@ package modules import ( + "errors" "fmt" + "path" + "strings" "github.com/mvt-project/androidqf/acquisition" + "github.com/mvt-project/androidqf/adb" "github.com/mvt-project/androidqf/log" ) @@ -23,16 +27,51 @@ func (b *Bugreport) Name() string { } func (b *Bugreport) Run(acq *acquisition.Acquisition, opts *Options) error { + // Preserve existing reports before generating another report, which can + // trigger Android's retention cleanup. + collectionErr := collectExistingBugreports(acq) + if collectionErr != nil { + log.Warningf("Failed to collect some existing bugreports: %v", collectionErr) + } + log.Info( "Generating a bugreport for the device...", ) err := acq.StreamBugreportToZip("bugreport.zip") if err != nil { - return fmt.Errorf("failed to stream bugreport to archive: %v", err) + return errors.Join(collectionErr, fmt.Errorf("failed to stream bugreport to archive: %w", err)) } log.Debug("Bugreport completed!") - return nil + return partialCollectionError(collectionErr) +} + +func collectExistingBugreports(acq *acquisition.Acquisition) error { + log.Info("Collecting existing files from /bugreports/...") + // cd follows the /bugreports symlink without following symlinks inside it. + // NUL delimiters preserve filenames containing spaces or newlines. A missing + // directory is normal; an inaccessible directory remains a collection error. + out, err := adb.Client.Shell("if [ ! -e /bugreports ] && [ ! -L /bugreports ]; then exit 0; fi; cd /bugreports/ && find . -type f -print0") + var collectionErr error + if err != nil { + collectionErr = fmt.Errorf("listing /bugreports/: %w", err) + } + for _, name := range strings.Split(out, "\x00") { + if name == "" { + continue + } + // Validate before joining: path.Join would hide traversal components. + remotePath := "/bugreports/" + strings.TrimPrefix(name, "./") + rel, err := relativeDeviceChild("/bugreports/", remotePath) + if err != nil { + collectionErr = errors.Join(collectionErr, err) + continue + } + if err := acq.PullToZipStaged(remotePath, path.Join("bugreports", rel)); err != nil { + collectionErr = errors.Join(collectionErr, fmt.Errorf("collecting %s: %w", remotePath, err)) + } + } + return collectionErr } diff --git a/modules/bugreport_test.go b/modules/bugreport_test.go new file mode 100644 index 0000000..5ddb7c4 --- /dev/null +++ b/modules/bugreport_test.go @@ -0,0 +1,141 @@ +package modules + +import ( + "archive/zip" + "errors" + "io" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/mvt-project/androidqf/acquisition" + "github.com/mvt-project/androidqf/adb" +) + +func TestBugreportPreservesExistingFiles(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("shell fixture is Unix-specific") + } + for _, scenario := range []string{"reports", "missing", "empty", "listing-failed", "pull-failed", "generation-failed", "unsafe-path"} { + t.Run(scenario, func(t *testing.T) { + dir := t.TempDir() + t.Setenv("BUGREPORT_TEST_DIR", dir) + t.Setenv("BUGREPORT_TEST_SCENARIO", scenario) + fakeADB := filepath.Join(dir, "adb") + // Execute the real listing command against a local symlink fixture. + // Generating a new report removes the old source, enforcing ordering. + script := `#!/bin/sh +case "$1" in +shell) + shift + if [ "$1" = bugreportz ]; then + [ "$BUGREPORT_TEST_SCENARIO" = generation-failed ] && exit 1 + rm -f "$BUGREPORT_TEST_DIR/source/old report.zip" + printf 'OK:/fresh.zip\n' + elif [ "$1" = rm ]; then + exit 0 + else + case "$BUGREPORT_TEST_SCENARIO" in + listing-failed) exit 1 ;; + unsafe-path) printf './../../escape.zip\000'; exit 0 ;; + esac + command=$(printf '%s' "$*" | sed "s|/bugreports|$BUGREPORT_TEST_DIR/bugreports|g") + sh -c "$command" + fi ;; +exec-out) + case "$*" in + *fresh.zip*) printf 'fresh report' ;; + *failed.zip*) printf 'truncated'; exit 1 ;; + *) + shift + command=$(printf '%s' "$*" | sed "s|/bugreports|$BUGREPORT_TEST_DIR/bugreports|g") + sh -c "$command" ;; + esac ;; +*) exit 1 ;; +esac +` + if err := os.WriteFile(fakeADB, []byte(script), 0700); err != nil { + t.Fatal(err) + } + want := map[string]string{"bugreport.zip": "fresh report"} + if scenario != "missing" { + if err := os.Mkdir(filepath.Join(dir, "source"), 0700); err != nil { + t.Fatal(err) + } + if err := os.Symlink(filepath.Join(dir, "source"), filepath.Join(dir, "bugreports")); err != nil { + t.Fatal(err) + } + } + if scenario == "reports" || scenario == "pull-failed" || scenario == "generation-failed" { + for _, name := range []string{"old report.zip", "screenshot\n'1.png", "dumpstate_log.txt"} { + if err := os.WriteFile(filepath.Join(dir, "source", name), []byte(name), 0600); err != nil { + t.Fatal(err) + } + want["bugreports/"+name] = name + } + if err := os.Symlink(fakeADB, filepath.Join(dir, "source", "outside")); err != nil { + t.Fatal(err) + } + } + if scenario == "pull-failed" { + if err := os.WriteFile(filepath.Join(dir, "source", "failed.zip"), []byte("report"), 0600); err != nil { + t.Fatal(err) + } + } + oldClient := adb.Client + adb.Client = &adb.ADB{ExePath: fakeADB} + t.Cleanup(func() { adb.Client = oldClient }) + writer, err := acquisition.NewStreamingZipWriter("bugreport-test", dir) + if err != nil { + t.Fatal(err) + } + acq := &acquisition.Acquisition{ + ZipWriter: writer, StreamingMode: true, + StreamingPuller: acquisition.NewStreamingPuller(fakeADB, "", 1), + } + err = NewBugreport().Run(acq, &Options{}) + switch scenario { + case "listing-failed", "pull-failed", "unsafe-path": + if !errors.Is(err, ErrPartialCollection) { + t.Fatalf("Run() = %v, want partial collection", err) + } + case "generation-failed": + delete(want, "bugreport.zip") + if err == nil || !strings.Contains(err.Error(), "failed to stream bugreport") { + t.Fatalf("Run() = %v, want generation failure", err) + } + default: + if err != nil { + t.Fatal(err) + } + } + if err := writer.Close(); err != nil { + t.Fatal(err) + } + archive, err := zip.OpenReader(writer.GetOutputPath()) + if err != nil { + t.Fatal(err) + } + defer archive.Close() + if len(archive.File) != len(want) { + t.Fatalf("archive has %d entries, want %d", len(archive.File), len(want)) + } + for _, file := range archive.File { + r, err := file.Open() + if err != nil { + t.Fatal(err) + } + content, err := io.ReadAll(r) + r.Close() + if err != nil { + t.Fatal(err) + } + if expected, ok := want[file.Name]; !ok || string(content) != expected { + t.Fatalf("unexpected archive entry %q: %q", file.Name, content) + } + } + }) + } +}