Skip to content

fix(sync): copy peer files from SAF dir into internal dir before pull #292

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
TimeToBuildBob wants to merge 1 commit into from
Closed

fix(sync): copy peer files from SAF dir into internal dir before pull #292

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions mobile/src/main/java/net/activitywatch/android/SyncInterface.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import androidx.documentfile.provider.DocumentFile
import org.json.JSONObject
import java.io.File
import java.io.FileInputStream
import java.io.FileOutputStream
import java.io.IOException
import java.util.concurrent.ExecutorService
import java.util.concurrent.Executors
Expand Down Expand Up @@ -256,6 +257,7 @@ class SyncInterface(context: Context) {
},
mirrorBeforeCallback
) {
copyPeerFilesFromSafDir()
syncBoth(BuildConfig.SERVER_PORT, hostname)
}
}
Expand Down Expand Up @@ -497,6 +499,89 @@ class SyncInterface(context: Context) {
}
}

/**
* Before each full sync, copy peer databases from the user-configured SAF
* directory into the internal sync directory so that [syncBoth] can find them.
*
* Syncthing (or any external file-sync tool) writes peer databases into the
* SAF-granted tree as `<hostname>/<device_id>/test.db`. The internal [syncDir]
* is app-private and invisible to those tools, so without this step `pull_all`
* inside `syncBoth` always finds zero peers — every run reports "pulled 0"
* regardless of how many peers have synced their data into the Syncthing folder.
*
* We skip the directory whose name matches our own hostname to avoid replacing
* live staging files with the one-cycle-stale SAF mirror. Errors for individual
* entries are logged and skipped so a partially-accessible SAF directory does not
* abort an otherwise healthy sync pass.
*/
private fun copyPeerFilesFromSafDir() {
val uriStr = AWPreferences(appContext).getSyncDirUri() ?: return
val safUri = Uri.parse(uriStr)
val safDir = DocumentFile.fromTreeUri(appContext, safUri) ?: return
if (!safDir.isDirectory) {
Log.w(TAG, "SAF peer pre-copy: configured URI is not a directory")
return
}

val ownHostname = getDeviceName()
val destRoot = File(syncDir)
var copied = 0
var errors = 0

for (hostDir in safDir.listFiles()) {
if (!hostDir.isDirectory) continue
val hostname = hostDir.name ?: continue
if (hostname == ownHostname) continue // own staging is authoritative in the internal dir

val localHostDir = File(destRoot, hostname)
localHostDir.mkdirs()
val (c, e) = copyFromSafDirectory(hostDir, localHostDir)

@greptile-apps greptile-apps Bot Sep 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Removed Peers Remain Active

The inbound copy creates and overwrites entries but never removes internal peer files that have disappeared from the SAF tree. Since syncDir persists between runs and remote discovery scans retained hostname and device databases, removing a peer or renaming its hostname leaves the old snapshot participating in later pulls and accumulating indefinitely. Reconcile copied peer directories with the current SAF tree while preserving the intentionally skipped local hostname.

copied += c
errors += e
}
Log.i(TAG, "SAF peer pre-copy: copied=$copied errors=$errors")
}

/**
* Recursively copy [safDir] (a SAF DocumentFile subtree) into [destDir] (a
* local File directory), skipping entries that cannot be read.
*
* Returns (copiedCount, errorCount).
*/
private fun copyFromSafDirectory(safDir: DocumentFile, destDir: File): Pair<Int, Int> {
var copied = 0
var errors = 0
for (entry in safDir.listFiles()) {
if (cancelRequested) break
val name = entry.name ?: continue
try {
if (entry.isDirectory) {
val subDest = File(destDir, name)
subDest.mkdirs()
val (c, e) = copyFromSafDirectory(entry, subDest)
copied += c
errors += e
} else {
val inp = appContext.contentResolver.openInputStream(entry.uri)
if (inp == null) {
Log.w(TAG, "SAF peer pre-copy: null input stream for $name")
errors++
continue
}
inp.use { FileOutputStream(File(destDir, name)).use { out -> it.copyTo(out) } }

@greptile-apps greptile-apps Bot Sep 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Partial Copies Replace Valid Data

If a SAF read fails or is interrupted, FileOutputStream has already truncated the existing peer database. The error handlers leave that partial file in place, and syncBoth immediately tries to consume it. A transient provider error or a file being updated by Syncthing can therefore replace the last valid peer snapshot with a corrupt SQLite database and cause the pull to fail. Copy to a temporary file and replace the destination only after the copy succeeds.

@greptile-apps greptile-apps Bot Sep 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security SAF Names Escape Sync Directory

Provider-controlled DocumentFile.name values are used directly in File paths at the hostname, directory, and file levels. Because a user-selected third-party document provider can return names containing .. or path separators, a malicious or nonconforming provider can make this write escape syncDir and overwrite other app files, including app-private files when the internal-storage fallback is active. Reject unsafe path components and verify that every canonical destination remains inside the intended root.

How this was verified: SAF display names flow unchanged through File(destDir, name) into a writable file sink, while .. resolves outside the internal sync directory.

copied++
}
} catch (e: IOException) {
Log.w(TAG, "SAF peer pre-copy: failed to copy $name: ${e.message}")
errors++
} catch (e: SecurityException) {
Log.w(TAG, "SAF peer pre-copy: permission denied for $name: ${e.message}")
errors++
}
}
return Pair(copied, errors)
}

fun getSyncDirectory(): String = syncDir

private fun migrateLegacySyncFolders(): SanitizedHostnameMigration.MigrationResult {
Expand Down
Loading