From b973e58d9eb92ecb8d802a8b6f51e58a181a05ce Mon Sep 17 00:00:00 2001 From: Friday Date: Fri, 5 Jun 2026 02:15:23 +0000 Subject: [PATCH] Atomic transfers + signed-release CI Sync engine / providers: - LocalAccessor: replace createOutputStream with writeAtomically (temp sibling + rename/commit) for both JavaFile and SAF backends, so an interrupted download no longer truncates the destination file. - SyncEngine: use writeAtomically for DOWNLOAD and propagate downloadFile failures via getOrThrow (was silently swallowed -> false success + state). - WebDavProvider (covers Nextcloud/ownCloud): PUT to hidden temp then MOVE onto destination, so a failed upload can't leave a truncated remote file. - SftpProvider: upload to temp then rename onto destination. Build / CI: - compileSdk 34 -> 35 (was below targetSdk 35). - Release signing reads keystore from local.properties or env (CI), with a debug-key fallback so builds still succeed without secrets. - Disable R8/minify for release (never exercised by CI; keeps signed release behaving like the debug builds in use today). - CI: run unit tests on every push/PR, build assembleRelease (signed when KEYSTORE_BASE64 present), publish APK only on v* tags. --- .gitea/workflows/release.yml | 28 +++++- app/build.gradle.kts | 31 +++++-- .../data/providers/sftp/SftpProvider.kt | 17 +++- .../data/providers/webdav/WebDavProvider.kt | 17 +++- .../com/syncflow/domain/sync/LocalAccessor.kt | 87 +++++++++++++++---- .../com/syncflow/domain/sync/SyncEngine.kt | 4 +- 6 files changed, 153 insertions(+), 31 deletions(-) diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index 86ef475..c2d3b13 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -2,8 +2,10 @@ name: Build & Release APK on: push: + branches: ['**'] tags: - 'v*' + pull_request: jobs: build: @@ -20,10 +22,29 @@ jobs: - uses: android-actions/setup-android@v3 - - name: Build debug APK + - name: Run unit tests run: | chmod +x gradlew - ./gradlew assembleDebug --no-daemon + ./gradlew testDebugUnitTest --no-daemon + + - name: Decode release keystore + env: + KEYSTORE_BASE64: ${{ secrets.KEYSTORE_BASE64 }} + run: | + if [ -n "$KEYSTORE_BASE64" ]; then + echo "$KEYSTORE_BASE64" | base64 -d > "$RUNNER_TEMP/release.keystore" + echo "KEYSTORE_PATH=$RUNNER_TEMP/release.keystore" >> "$GITHUB_ENV" + echo "Release keystore decoded — building signed release." + else + echo "::warning::KEYSTORE_BASE64 secret not set — release APK will be debug-signed." + fi + + - name: Build release APK + env: + KEYSTORE_PASSWORD: ${{ secrets.KEYSTORE_PASSWORD }} + KEY_ALIAS: ${{ secrets.KEY_ALIAS }} + KEY_PASSWORD: ${{ secrets.KEY_PASSWORD }} + run: ./gradlew assembleRelease --no-daemon - name: Get version name id: ver @@ -32,10 +53,11 @@ jobs: - name: Rename APK run: | mkdir dist - cp app/build/outputs/apk/debug/app-debug.apk \ + cp app/build/outputs/apk/release/app-release.apk \ dist/SyncFlow-v${{ steps.ver.outputs.name }}.apk - name: Attach APK to release + if: startsWith(github.ref, 'refs/tags/v') env: TOKEN: ${{ secrets.GITHUB_TOKEN }} TAG: ${{ github.ref_name }} diff --git a/app/build.gradle.kts b/app/build.gradle.kts index bd7488b..3f7d2a0 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -18,9 +18,15 @@ val localProps = Properties().apply { if (f.exists()) load(f.inputStream()) } +// Release signing is read from local.properties (local builds) or environment variables +// (CI). When no keystore is available the release build falls back to the debug key so the +// build still succeeds — it just isn't a distributable, properly-signed APK. +val keystorePath = (localProps["KEYSTORE_PATH"] as String?) ?: System.getenv("KEYSTORE_PATH") +val hasReleaseKeystore = keystorePath != null && file(keystorePath).exists() + android { namespace = "com.syncflow" - compileSdk = 34 + compileSdk = 35 defaultConfig { applicationId = "com.syncflow" @@ -38,19 +44,28 @@ android { signingConfigs { create("release") { - storeFile = localProps["KEYSTORE_PATH"]?.toString()?.let { file(it) } - storePassword = localProps["KEYSTORE_PASSWORD"]?.toString() - keyAlias = localProps["KEY_ALIAS"]?.toString() - keyPassword = localProps["KEY_PASSWORD"]?.toString() + if (hasReleaseKeystore) { + storeFile = file(keystorePath!!) + storePassword = (localProps["KEYSTORE_PASSWORD"] as String?) ?: System.getenv("KEYSTORE_PASSWORD") + keyAlias = (localProps["KEY_ALIAS"] as String?) ?: System.getenv("KEY_ALIAS") + keyPassword = (localProps["KEY_PASSWORD"] as String?) ?: System.getenv("KEY_PASSWORD") + } } } buildTypes { release { - isMinifyEnabled = true - isShrinkResources = true + // R8/minify has never been exercised by CI (it only built debug), so leave it off + // to keep the signed release behaving identically to the debug builds in use today. + // Re-enable with proper keep rules and an on-device smoke test if APK size matters. + isMinifyEnabled = false + isShrinkResources = false proguardFiles(getDefaultProguardFile("proguard-android-optimize.txt"), "proguard-rules.pro") - signingConfig = signingConfigs.getByName("release") + signingConfig = if (hasReleaseKeystore) { + signingConfigs.getByName("release") + } else { + signingConfigs.getByName("debug") + } } } diff --git a/app/src/main/kotlin/com/syncflow/data/providers/sftp/SftpProvider.kt b/app/src/main/kotlin/com/syncflow/data/providers/sftp/SftpProvider.kt index ebd8c00..a02df52 100644 --- a/app/src/main/kotlin/com/syncflow/data/providers/sftp/SftpProvider.kt +++ b/app/src/main/kotlin/com/syncflow/data/providers/sftp/SftpProvider.kt @@ -63,12 +63,25 @@ class SftpProvider(private val account: CloudAccount, private val credentialStor sizeBytes: Long, onProgress: (Long) -> Unit, ): Result = runCatching { + // Upload to a hidden temp sibling, then rename onto the destination so an interrupted + // transfer never leaves a truncated file at the real path. + val dir = remotePath.substringBeforeLast('/', "") + val name = remotePath.substringAfterLast('/') + val tmpPath = if (dir.isEmpty()) ".$name.sfpart" else "$dir/.$name.sfpart" withSftp { sftp -> sftp.put(object : InMemorySourceFile() { - override fun getName() = remotePath.substringAfterLast('/') + override fun getName() = name override fun getLength() = sizeBytes override fun getInputStream() = localStream - }, remotePath) + }, tmpPath) + // SFTP rename fails if the target exists on servers without the POSIX-rename + // extension, so fall back to removing the destination first. + try { + sftp.rename(tmpPath, remotePath) + } catch (e: Exception) { + runCatching { sftp.rm(remotePath) } + sftp.rename(tmpPath, remotePath) + } } getFileMetadata(remotePath).getOrThrow() } diff --git a/app/src/main/kotlin/com/syncflow/data/providers/webdav/WebDavProvider.kt b/app/src/main/kotlin/com/syncflow/data/providers/webdav/WebDavProvider.kt index 73bc075..9851d5e 100644 --- a/app/src/main/kotlin/com/syncflow/data/providers/webdav/WebDavProvider.kt +++ b/app/src/main/kotlin/com/syncflow/data/providers/webdav/WebDavProvider.kt @@ -93,15 +93,30 @@ open class WebDavProvider(protected val account: CloudAccount) : CloudProvider { localStream.source().use { source -> sink.writeAll(source) } } } - val req = Request.Builder().url(url(remotePath)).put(body).build() + // Upload to a hidden temp sibling first, then MOVE it onto the destination. A + // failed PUT leaves the real file untouched instead of overwriting it with a + // truncated body; the MOVE is a server-side atomic-ish swap. + val tmpPath = tempPathFor(remotePath) + val req = Request.Builder().url(url(tmpPath)).put(body).build() client.newCall(req).execute().use { resp -> if (!resp.isSuccessful) throw Exception("Upload HTTP ${resp.code}") } + moveFile(tmpPath, remotePath).getOrElse { e -> + runCatching { deleteFile(tmpPath) } + throw e + } onProgress(sizeBytes) getFileMetadata(remotePath).getOrThrow() } } + private fun tempPathFor(remotePath: String): String { + val dir = remotePath.substringBeforeLast('/', "") + val name = remotePath.substringAfterLast('/') + val tmp = ".$name.sfpart" + return if (dir.isEmpty()) tmp else "$dir/$tmp" + } + override suspend fun downloadFile(remotePath: String, destination: OutputStream, onProgress: (Long) -> Unit): Result = runCatching { withContext(Dispatchers.IO) { val req = Request.Builder().url(url(remotePath)).get().build() diff --git a/app/src/main/kotlin/com/syncflow/domain/sync/LocalAccessor.kt b/app/src/main/kotlin/com/syncflow/domain/sync/LocalAccessor.kt index 1146922..273a438 100644 --- a/app/src/main/kotlin/com/syncflow/domain/sync/LocalAccessor.kt +++ b/app/src/main/kotlin/com/syncflow/domain/sync/LocalAccessor.kt @@ -5,6 +5,8 @@ import android.net.Uri import android.provider.DocumentsContract import com.syncflow.domain.model.SyncPair import java.io.File +import java.io.FileOutputStream +import java.io.IOException import java.io.InputStream import java.io.OutputStream @@ -14,10 +16,17 @@ sealed class LocalAccessor { abstract fun walkFiles(pair: SyncPair): Map abstract fun openInputStream(relativePath: String): InputStream? - abstract fun createOutputStream(relativePath: String): OutputStream? abstract fun delete(relativePath: String): Boolean abstract fun lastModifiedMs(relativePath: String): Long + /** + * Write [relativePath] atomically: stream into a temp sibling first, then swap it into + * place only after [write] completes without throwing. An interrupted transfer (network + * drop, process death) leaves the existing destination untouched instead of truncating it. + * On failure the temp is removed and the exception is rethrown. + */ + abstract suspend fun writeAtomically(relativePath: String, write: suspend (OutputStream) -> Unit) + // ── java.io.File backend (regular /storage/... paths) ──────────────────── class JavaFile(private val root: File) : LocalAccessor() { @@ -48,10 +57,30 @@ sealed class LocalAccessor { override fun openInputStream(relativePath: String): InputStream = File(root, relativePath).inputStream() - override fun createOutputStream(relativePath: String): OutputStream { + override suspend fun writeAtomically(relativePath: String, write: suspend (OutputStream) -> Unit) { val dest = File(root, relativePath) dest.parentFile?.mkdirs() - return dest.outputStream() + val tmp = File(dest.parentFile, ".${dest.name}.sfpart") + try { + FileOutputStream(tmp).use { os -> + write(os) + os.flush() + os.fd.sync() // durably persist bytes before the rename swaps the file in + } + } catch (e: Throwable) { + tmp.delete() + throw e + } + // Same-directory rename is atomic on POSIX/Android and replaces the destination. + if (!tmp.renameTo(dest)) { + try { + tmp.copyTo(dest, overwrite = true) + } catch (e: Throwable) { + tmp.delete() + throw e + } + tmp.delete() + } } override fun delete(relativePath: String): Boolean = File(root, relativePath).delete() @@ -131,7 +160,7 @@ sealed class LocalAccessor { return resolver.openInputStream(docUri) } - override fun createOutputStream(relativePath: String): OutputStream? { + override suspend fun writeAtomically(relativePath: String, write: suspend (OutputStream) -> Unit) { val parts = relativePath.replace('\\', '/').split('/') var currentId = DocumentsContract.getTreeDocumentId(treeUri) @@ -141,7 +170,7 @@ sealed class LocalAccessor { val parentUri = DocumentsContract.buildDocumentUriUsingTree(treeUri, currentId) val newDir = DocumentsContract.createDocument( resolver, parentUri, DocumentsContract.Document.MIME_TYPE_DIR, parts[i] - ) ?: return null + ) ?: throw IOException("Cannot create directory ${parts[i]} for $relativePath") DocumentsContract.getDocumentId(newDir) } } @@ -149,19 +178,47 @@ sealed class LocalAccessor { val fileName = parts.last() val childrenUri = DocumentsContract.buildChildDocumentsUriUsingTree(treeUri, currentId) val parentUri = DocumentsContract.buildDocumentUriUsingTree(treeUri, currentId) + val tmpName = ".$fileName.sfpart" - // Delete existing to allow overwrite - findChildId(childrenUri, fileName)?.let { existingId -> - DocumentsContract.deleteDocument( - resolver, - DocumentsContract.buildDocumentUriUsingTree(treeUri, existingId) - ) + // Clear any leftover temp document from a previously interrupted write. + findChildId(childrenUri, tmpName)?.let { staleId -> + runCatching { + DocumentsContract.deleteDocument( + resolver, DocumentsContract.buildDocumentUriUsingTree(treeUri, staleId) + ) + } } - val newUri = DocumentsContract.createDocument( - resolver, parentUri, "application/octet-stream", fileName - ) ?: return null - return resolver.openOutputStream(newUri) + val tmpUri = DocumentsContract.createDocument( + resolver, parentUri, "application/octet-stream", tmpName + ) ?: throw IOException("Cannot create temp document for $relativePath") + + try { + (resolver.openOutputStream(tmpUri) + ?: throw IOException("Cannot open temp stream for $relativePath")).use { os -> + write(os) + os.flush() + } + } catch (e: Throwable) { + runCatching { DocumentsContract.deleteDocument(resolver, tmpUri) } + throw e + } + + // Commit: remove the existing destination, then rename the fully-written temp into + // place. If interrupted between the two steps the temp still holds the complete data + // (recoverable by hand), which is strictly safer than truncating the destination. + findChildId(childrenUri, fileName)?.let { existingId -> + DocumentsContract.deleteDocument( + resolver, DocumentsContract.buildDocumentUriUsingTree(treeUri, existingId) + ) + } + val renamed = DocumentsContract.renameDocument(resolver, tmpUri, fileName) + if (renamed == null) { + runCatching { DocumentsContract.deleteDocument(resolver, tmpUri) } + throw IOException("Cannot finalize $relativePath") + } + // Drop the stale cache entry so the next read re-resolves the new document id. + docIdCache.remove(relativePath) } override fun delete(relativePath: String): Boolean { diff --git a/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt b/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt index 339f382..22358aa 100644 --- a/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt +++ b/app/src/main/kotlin/com/syncflow/domain/sync/SyncEngine.kt @@ -142,8 +142,8 @@ class SyncEngine @Inject constructor( } SyncDecision.DOWNLOAD -> { val bytes = runCatching { - accessor.createOutputStream(rel)?.use { stream -> - provider.downloadFile("${pair.remotePath}/$rel", stream) { } + accessor.writeAtomically(rel) { stream -> + provider.downloadFile("${pair.remotePath}/$rel", stream) { }.getOrThrow() } remote!!.sizeBytes }.getOrElse { e ->