From 473faf725e1bca094c56da20d2728061a56b97b4 Mon Sep 17 00:00:00 2001 From: Emirhan Date: Mon, 14 Sep 2026 17:59:54 +0300 Subject: [PATCH 1/7] fix: require confirmation for background NFC rules --- README.md | 2 +- README.tr.md | 2 +- app/src/main/AndroidManifest.xml | 2 +- .../java/com/flowpilot/app/MainActivity.kt | 108 ++++++++++-------- .../engine/NfcBackgroundConfirmationGate.kt | 46 ++++++++ .../flowpilot/app/engine/NfcIntentSession.kt | 15 +++ .../com/flowpilot/app/engine/NfcTagHandoff.kt | 22 ++-- .../com/flowpilot/app/engine/NfcTagUtils.kt | 2 +- app/src/main/res/values-tr/strings.xml | 3 + app/src/main/res/values/strings.xml | 3 + .../app/engine/NfcIntentTrustBoundaryTest.kt | 90 +++++++++++++++ .../flowpilot/app/engine/RuleEvaluatorTest.kt | 8 +- docs/IMPLEMENTATION.md | 4 +- docs/ROADMAP.md | 12 +- docs/STATUS.md | 2 +- 15 files changed, 245 insertions(+), 76 deletions(-) create mode 100644 app/src/main/java/com/flowpilot/app/engine/NfcBackgroundConfirmationGate.kt create mode 100644 app/src/main/java/com/flowpilot/app/engine/NfcIntentSession.kt create mode 100644 app/src/test/java/com/flowpilot/app/engine/NfcIntentTrustBoundaryTest.kt diff --git a/README.md b/README.md index 8120730..6f39e91 100644 --- a/README.md +++ b/README.md @@ -166,7 +166,7 @@ FlowPilot listens to a rich spectrum of hardware, radio, and system events: - **Shake:** Firm shake detection with configurable sensitivity slider. - **Ambient Light:** Lux drops below or rises above target threshold. - 📍 **Hardware Geofencing:** Enter or exit defined geographical zones using Google Play Services `GeofencingClient`. Zero idle battery drain, up to 50 persistent queued events across engine restarts, and coordinate reuse for template variables. -- 🏷️ **NFC Tags:** Instant hex UID matching on physical tag scan. +- 🏷️ **NFC Tags:** Instant hex UID matching on physical scans. Foreground ReaderMode scans run matching rules automatically; background Android discovery opens FlowPilot and requires explicit confirmation before any matching NFC automation runs. - 📞 **Phone & SMS:** Call ringing, answered, outgoing dialed, call ended; SMS received with keyword, prefix, regex, or exact sender matching. - 🔔 **Notifications:** Incoming notifications from selected apps with keyword filtering. diff --git a/README.tr.md b/README.tr.md index 825278b..b461643 100644 --- a/README.tr.md +++ b/README.tr.md @@ -166,7 +166,7 @@ FlowPilot zengin bir donanım, radyo ve sistem olayı yelpazesini dinler: - **Sallama:** Hassasiyet ayarlı telefon sallama algılaması. - **Ortam Işığı:** Lüks değerinin belirlenen sınırın altına düşmesi veya üstüne çıkması. - 📍 **Donanım Coğrafi Çit (Geofence):** Google Play Services `GeofencingClient` ile belirlenen alana giriş/çıkış. Boşta sıfır pil tüketimi, yeniden başlatmada kaybolmayan 50 olaylık kalıcı kuyruk ve şablon değişkenlerinde doğrudan koordinat kullanımı. -- 🏷️ **NFC Etiketleri:** Fiziksel etiket okutulduğunda anında hex UID eşleşmesi. +- 🏷️ **NFC Etiketleri:** Fiziksel taramalarda anında hex UID eşleşmesi. Ön plandaki ReaderMode taramaları eşleşen kuralları otomatik çalıştırır; Android arka plan keşfi FlowPilot'ı açar ve eşleşen NFC otomasyonu çalışmadan önce açık onay ister. - 📞 **Arama & SMS:** Gelen arama çalıyor, yanıtlandı, giden arama başladı, arama bitti; SMS gönderen numaraya ve kelime, önek veya regex kalıbına göre tetikleme. - 🔔 **Bildirimler:** Seçili uygulamalardan gelen bildirimler ve anahtar kelime filtreleme. diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 3364b41..79445fd 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -115,7 +115,7 @@ - + diff --git a/app/src/main/java/com/flowpilot/app/MainActivity.kt b/app/src/main/java/com/flowpilot/app/MainActivity.kt index 0504135..2c2c5ff 100644 --- a/app/src/main/java/com/flowpilot/app/MainActivity.kt +++ b/app/src/main/java/com/flowpilot/app/MainActivity.kt @@ -1,10 +1,8 @@ package com.flowpilot.app -import android.app.PendingIntent -import android.content.Intent import android.nfc.NfcAdapter import android.nfc.Tag -import android.os.Build +import android.content.Intent import android.os.Bundle import android.content.pm.PackageManager import android.util.Log @@ -15,27 +13,34 @@ import androidx.activity.enableEdgeToEdge import com.flowpilot.app.actions.ShizukuPermissionBridge import com.flowpilot.app.actions.ShizukuShell import com.flowpilot.app.engine.NfcTagHandoff +import com.flowpilot.app.engine.NfcBackgroundConfirmationGate +import com.flowpilot.app.engine.NfcIntentSession import com.flowpilot.app.ui.FlowPilotRoot import com.flowpilot.app.ui.theme.FlowPilotTheme import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue +import androidx.compose.material3.AlertDialog +import androidx.compose.material3.Text +import androidx.compose.material3.TextButton +import androidx.compose.ui.res.stringResource import rikka.shizuku.Shizuku class MainActivity : ComponentActivity() { private var nfcAdapter: NfcAdapter? = null - private var pendingIntent: PendingIntent? = null + private val nfcIntentSession = NfcIntentSession() override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) enableEdgeToEdge() Shizuku.addRequestPermissionResultListener(requestListener) - initNfcForegroundDispatch() - handleNfcIntent(intent) + initNfcReaderMode() + handleNfcDiscoveryIntent(intent) setContent { val vm: com.flowpilot.app.ui.AppViewModel = androidx.lifecycle.viewmodel.compose.viewModel() val appLanguage by vm.appLanguage.collectAsState() val appTheme by vm.appTheme.collectAsState() + val hasPendingNfcConfirmation by NfcBackgroundConfirmationGate.hasPendingConfirmation.collectAsState() val isDark = when (appTheme.lowercase()) { "light" -> false @@ -46,6 +51,23 @@ class MainActivity : ComponentActivity() { com.flowpilot.app.ui.util.AppLocaleProvider(appLanguage) { FlowPilotTheme(darkTheme = isDark) { FlowPilotRoot(vm) + if (hasPendingNfcConfirmation) { + AlertDialog( + onDismissRequest = NfcBackgroundConfirmationGate::dismiss, + title = { Text(stringResource(R.string.nfc_background_confirm_title)) }, + text = { Text(stringResource(R.string.nfc_background_confirm_desc)) }, + confirmButton = { + TextButton(onClick = { NfcBackgroundConfirmationGate.confirm() }) { + Text(stringResource(R.string.nfc_background_confirm_action)) + } + }, + dismissButton = { + TextButton(onClick = NfcBackgroundConfirmationGate::dismiss) { + Text(stringResource(R.string.btn_cancel)) + } + }, + ) + } } } } @@ -53,70 +75,57 @@ class MainActivity : ComponentActivity() { override fun onResume() { super.onResume() - enableNfcForegroundDispatch() + if (nfcIntentSession.readerModeAllowed) enableNfcReaderMode() } override fun onPause() { - disableNfcForegroundDispatch() + disableNfcReaderMode() super.onPause() } - private fun initNfcForegroundDispatch() { - val manager = getSystemService(android.nfc.NfcManager::class.java) - nfcAdapter = manager?.defaultAdapter ?: NfcAdapter.getDefaultAdapter(this) - val flags = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { - PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_MUTABLE - } else { - PendingIntent.FLAG_UPDATE_CURRENT - } - val explicitIntent = Intent(this, javaClass).apply { - addFlags(Intent.FLAG_ACTIVITY_SINGLE_TOP) - setPackage(packageName) + override fun onNewIntent(intent: Intent) { + super.onNewIntent(intent) + setIntent(intent) + handleNfcDiscoveryIntent(intent) + } + + private fun handleNfcDiscoveryIntent(intent: Intent?) { + // Manifest NFC intents are caller-spoofable. They only open this user-confirmed gate. + if (NfcBackgroundConfirmationGate.requestConfirmation(intent)) { + nfcIntentSession.markIntentOriginated() + disableNfcReaderMode() } - pendingIntent = PendingIntent.getActivity(this, 0, explicitIntent, flags) + } + private fun initNfcReaderMode() { + val manager = getSystemService(android.nfc.NfcManager::class.java) + nfcAdapter = manager?.defaultAdapter ?: NfcAdapter.getDefaultAdapter(this) } - private fun enableNfcForegroundDispatch() { + private fun enableNfcReaderMode() { val adapter = nfcAdapter ?: return - val pi = pendingIntent ?: return if (adapter.isEnabled) { try { - // Null filters/tech lists capture every supported tag while this activity is foreground. - // Background delivery stays limited to manifest TAG/TECH filters. - adapter.enableForegroundDispatch(this, pi, null, null) + adapter.enableReaderMode(this, nfcReaderCallback, NFC_READER_FLAGS, null) } catch (e: IllegalStateException) { - Log.w(TAG, "NFC foreground dispatch unavailable while activity is not resumed", e) + Log.w(TAG, "NFC reader mode unavailable while activity is not resumed", e) } } } - private fun disableNfcForegroundDispatch() { + private fun disableNfcReaderMode() { val adapter = nfcAdapter ?: return try { - adapter.disableForegroundDispatch(this) + adapter.disableReaderMode(this) } catch (e: IllegalStateException) { - Log.w(TAG, "NFC foreground dispatch already disabled", e) + Log.w(TAG, "NFC reader mode already disabled", e) } } - private fun handleNfcIntent(intent: Intent?) { - intent ?: return - val action = intent.action ?: return - if (action == NfcAdapter.ACTION_NDEF_DISCOVERED || - action == NfcAdapter.ACTION_TECH_DISCOVERED || - action == NfcAdapter.ACTION_TAG_DISCOVERED - ) { - val rawId: ByteArray? = intent.getByteArrayExtra(NfcAdapter.EXTRA_ID) - ?: if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { - intent.getParcelableExtra(NfcAdapter.EXTRA_TAG, Tag::class.java)?.id - } else { - @Suppress("DEPRECATION") - (intent.getParcelableExtra(NfcAdapter.EXTRA_TAG) as? Tag)?.id - } - - if (rawId != null && rawId.isNotEmpty()) { - NfcTagHandoff.emitTagScanned(rawId) + private val nfcReaderCallback = NfcAdapter.ReaderCallback { tag: Tag -> + // ReaderMode invokes this only for a tag discovered by Android's NFC stack. Intent extras are forgeable. + if (nfcIntentSession.emitReaderModeTag(tag.id, NfcTagHandoff::emitTagScanned)) { + runOnUiThread { Toast.makeText(this, "NFC tag scanned", Toast.LENGTH_SHORT).show() } } @@ -142,5 +151,12 @@ class MainActivity : ComponentActivity() { private companion object { const val TAG = "FlowPilotMainActivity" + const val NFC_READER_FLAGS = + NfcAdapter.FLAG_READER_NFC_A or + NfcAdapter.FLAG_READER_NFC_B or + NfcAdapter.FLAG_READER_NFC_F or + NfcAdapter.FLAG_READER_NFC_V or + NfcAdapter.FLAG_READER_NFC_BARCODE or + NfcAdapter.FLAG_READER_SKIP_NDEF_CHECK } } diff --git a/app/src/main/java/com/flowpilot/app/engine/NfcBackgroundConfirmationGate.kt b/app/src/main/java/com/flowpilot/app/engine/NfcBackgroundConfirmationGate.kt new file mode 100644 index 0000000..553593c --- /dev/null +++ b/app/src/main/java/com/flowpilot/app/engine/NfcBackgroundConfirmationGate.kt @@ -0,0 +1,46 @@ +package com.flowpilot.app.engine + +import android.content.Intent +import android.nfc.NfcAdapter +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow + +/** Holds an untrusted NFC discovery request until a visible user confirmation authorizes it. */ +object NfcBackgroundConfirmationGate { + private val _hasPendingConfirmation = MutableStateFlow(false) + val hasPendingConfirmation: StateFlow = _hasPendingConfirmation.asStateFlow() + + private var pendingTagId: ByteArray? = null + + @Synchronized + fun requestConfirmation(intent: Intent?): Boolean { + val tagId = intent?.takeIf { it.action in DISCOVERY_ACTIONS } + ?.getByteArrayExtra(NfcAdapter.EXTRA_ID) + ?.takeIf { it.isNotEmpty() } + ?.copyOf() + ?: return false + pendingTagId = tagId + _hasPendingConfirmation.value = true + return true + } + + @Synchronized + fun confirm(emit: (ByteArray?) -> Boolean = NfcTagHandoff::emitTagScanned): Boolean { + val tagId = pendingTagId ?: return false + pendingTagId = null + _hasPendingConfirmation.value = false + return emit(tagId) + } + + @Synchronized + fun dismiss() { + pendingTagId = null + _hasPendingConfirmation.value = false + } + + private val DISCOVERY_ACTIONS = setOf( + NfcAdapter.ACTION_TECH_DISCOVERED, + NfcAdapter.ACTION_TAG_DISCOVERED, + ) +} diff --git a/app/src/main/java/com/flowpilot/app/engine/NfcIntentSession.kt b/app/src/main/java/com/flowpilot/app/engine/NfcIntentSession.kt new file mode 100644 index 0000000..c547d4f --- /dev/null +++ b/app/src/main/java/com/flowpilot/app/engine/NfcIntentSession.kt @@ -0,0 +1,15 @@ +package com.flowpilot.app.engine + +/** Prevents an NFC intent from being re-delivered through automatic ReaderMode in same activity session. */ +class NfcIntentSession { + private var intentOriginated = false + + fun markIntentOriginated() { + intentOriginated = true + } + + val readerModeAllowed: Boolean get() = !intentOriginated + + fun emitReaderModeTag(rawId: ByteArray?, emit: (ByteArray?) -> Boolean): Boolean = + readerModeAllowed && emit(rawId) +} diff --git a/app/src/main/java/com/flowpilot/app/engine/NfcTagHandoff.kt b/app/src/main/java/com/flowpilot/app/engine/NfcTagHandoff.kt index f9271da..187e0a1 100644 --- a/app/src/main/java/com/flowpilot/app/engine/NfcTagHandoff.kt +++ b/app/src/main/java/com/flowpilot/app/engine/NfcTagHandoff.kt @@ -19,20 +19,16 @@ object NfcTagHandoff { private val _latestScannedTagId = MutableStateFlow(null) val latestScannedTagId: StateFlow = _latestScannedTagId.asStateFlow() - fun emitTagScanned(rawId: ByteArray?) { + /** + * Accepts IDs only from Android's [android.nfc.NfcAdapter.ReaderCallback]. + * NFC discovery intents are not a trusted source because another app can send matching intent data. + */ + fun emitTagScanned(rawId: ByteArray?): Boolean { val tagId = NfcTagUtils.formatTagId(rawId) - if (tagId.isNotEmpty()) { - queue.add(NfcTagScannedEvent(tagId = tagId)) - _latestScannedTagId.value = tagId - } - } - - fun emitTagId(tagId: String) { - val normalized = NfcTagUtils.normalizeTagId(tagId) - if (normalized.isNotEmpty()) { - queue.add(NfcTagScannedEvent(tagId = normalized)) - _latestScannedTagId.value = normalized - } + if (tagId.isEmpty()) return false + queue.add(NfcTagScannedEvent(tagId = tagId)) + _latestScannedTagId.value = tagId + return true } fun drainEvents(): List = buildList { diff --git a/app/src/main/java/com/flowpilot/app/engine/NfcTagUtils.kt b/app/src/main/java/com/flowpilot/app/engine/NfcTagUtils.kt index 105c5ab..1a33585 100644 --- a/app/src/main/java/com/flowpilot/app/engine/NfcTagUtils.kt +++ b/app/src/main/java/com/flowpilot/app/engine/NfcTagUtils.kt @@ -9,7 +9,7 @@ import java.util.Locale object NfcTagUtils { /** - * Converts a raw byte array from tag discovery intent (NfcAdapter.EXTRA_ID) into normalized hex. + * Converts a raw byte array from Android's NFC reader callback into normalized hex. */ fun formatTagId(rawId: ByteArray?): String { if (rawId == null || rawId.isEmpty()) return "" diff --git a/app/src/main/res/values-tr/strings.xml b/app/src/main/res/values-tr/strings.xml index 2fd0fad..d8aaaba 100644 --- a/app/src/main/res/values-tr/strings.xml +++ b/app/src/main/res/values-tr/strings.xml @@ -29,6 +29,9 @@ Kapat Test Şimdi Çalıştır + NFC otomasyonları çalıştırılsın mı? + Bir NFC keşif isteği bu uygulamayla eşleşti. Eşleşen NFC otomasyonları çalıştırılsın mı? + Otomasyonları çalıştır Kullan Eylem ekle Koşul ekle diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 8b0870f..ba31037 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -29,6 +29,9 @@ Close Test Run now + Run NFC automations? + An NFC discovery request matched this app. Run matching NFC automations? + Run automations Use Add action Add condition diff --git a/app/src/test/java/com/flowpilot/app/engine/NfcIntentTrustBoundaryTest.kt b/app/src/test/java/com/flowpilot/app/engine/NfcIntentTrustBoundaryTest.kt new file mode 100644 index 0000000..3890e59 --- /dev/null +++ b/app/src/test/java/com/flowpilot/app/engine/NfcIntentTrustBoundaryTest.kt @@ -0,0 +1,90 @@ +package com.flowpilot.app.engine + +import android.content.Intent +import android.nfc.NfcAdapter +import com.flowpilot.app.data.model.ActionType +import com.flowpilot.app.data.model.Automation +import com.flowpilot.app.data.model.TriggerEvent +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class NfcIntentTrustBoundaryTest { + + @After + fun clearNfcState() { + NfcBackgroundConfirmationGate.dismiss() + NfcTagHandoff.clear() + } + + @Test + fun forgedBackgroundDiscoveryIntentCannotEmitBeforeConfirmation() { + NfcTagHandoff.clear() + val forgedIntent = Intent(NfcAdapter.ACTION_TAG_DISCOVERED) + .putExtra(NfcAdapter.EXTRA_ID, configuredId) + + assertThat(NfcBackgroundConfirmationGate.requestConfirmation(forgedIntent)).isTrue() + assertThat(NfcBackgroundConfirmationGate.hasPendingConfirmation.value).isTrue() + assertThat(NfcTagHandoff.drainEvents()).isEmpty() + + NfcBackgroundConfirmationGate.dismiss() + assertThat(NfcBackgroundConfirmationGate.hasPendingConfirmation.value).isFalse() + assertThat(NfcTagHandoff.drainEvents()).isEmpty() + } + + @Test + fun confirmedBackgroundDiscoveryEmitsAndMatchesConfiguredRule() { + NfcTagHandoff.clear() + val rule = Automation( + id = "nfc-rule", + name = "NFC rule", + triggerEvent = TriggerEvent.NFC_TAG_SCANNED, + nfcTagId = "04A1B21F", + action = ActionType.VIBRATE, + createdAt = 1L, + ) + + val backgroundDiscovery = Intent(NfcAdapter.ACTION_TECH_DISCOVERED) + .putExtra(NfcAdapter.EXTRA_ID, configuredId) + assertThat(NfcBackgroundConfirmationGate.requestConfirmation(backgroundDiscovery)).isTrue() + assertThat(NfcTagHandoff.drainEvents()).isEmpty() + assertThat(NfcBackgroundConfirmationGate.confirm()).isTrue() + + assertThat(RuleEvaluator.evaluateNfcTag(listOf(rule), NfcTagHandoff.drainEvents().single())) + .containsExactly(rule) + } + + @Test + fun intentOriginatedSessionKeepsReaderModeDisabledBeforeAndAfterConfirmation() { + val session = NfcIntentSession() + val emitted = mutableListOf() + val discoveryIntent = Intent(NfcAdapter.ACTION_TAG_DISCOVERED) + .putExtra(NfcAdapter.EXTRA_ID, configuredId) + + assertThat(session.readerModeAllowed).isTrue() + assertThat(NfcBackgroundConfirmationGate.requestConfirmation(discoveryIntent)).isTrue() + session.markIntentOriginated() + assertThat(session.readerModeAllowed).isFalse() + assertThat(session.emitReaderModeTag(configuredId, emitted::add)).isFalse() + assertThat(NfcTagHandoff.drainEvents()).isEmpty() + assertThat(emitted).isEmpty() + + assertThat(NfcBackgroundConfirmationGate.confirm()).isTrue() + assertThat(session.readerModeAllowed).isFalse() + assertThat(session.emitReaderModeTag(configuredId, emitted::add)).isFalse() + + NfcBackgroundConfirmationGate.dismiss() + assertThat(session.readerModeAllowed).isFalse() + assertThat(session.emitReaderModeTag(configuredId, emitted::add)).isFalse() + assertThat(emitted).isEmpty() + } + + private companion object { + val configuredId = byteArrayOf(0x04, 0xA1.toByte(), 0xB2.toByte(), 0x1F) + } +} diff --git a/app/src/test/java/com/flowpilot/app/engine/RuleEvaluatorTest.kt b/app/src/test/java/com/flowpilot/app/engine/RuleEvaluatorTest.kt index be54167..e7a9da3 100644 --- a/app/src/test/java/com/flowpilot/app/engine/RuleEvaluatorTest.kt +++ b/app/src/test/java/com/flowpilot/app/engine/RuleEvaluatorTest.kt @@ -322,10 +322,10 @@ class RuleEvaluatorTest { NfcTagHandoff.clear() assertThat(NfcTagHandoff.drainEvents()).isEmpty() - NfcTagHandoff.emitTagScanned(byteArrayOf(0x04, 0x12, 0x34)) - NfcTagHandoff.emitTagId("04:ab:cd:ef") - NfcTagHandoff.emitTagId("") - NfcTagHandoff.emitTagScanned(null) + assertThat(NfcTagHandoff.emitTagScanned(byteArrayOf(0x04, 0x12, 0x34))).isTrue() + assertThat(NfcTagHandoff.emitTagScanned(byteArrayOf(0x04, 0xAB.toByte(), 0xCD.toByte(), 0xEF.toByte()))).isTrue() + assertThat(NfcTagHandoff.emitTagScanned(byteArrayOf())).isFalse() + assertThat(NfcTagHandoff.emitTagScanned(null)).isFalse() val events = NfcTagHandoff.drainEvents() assertThat(events).hasSize(2) diff --git a/docs/IMPLEMENTATION.md b/docs/IMPLEMENTATION.md index 1daca22..c9f3f29 100644 --- a/docs/IMPLEMENTATION.md +++ b/docs/IMPLEMENTATION.md @@ -97,7 +97,7 @@ app/src/main/java/com/flowpilot/app/ BluetoothDeviceTracker.kt bonded-device ACL broadcasts + per-device transition reducer DeviceFlipState.kt pure flip orientation models and debounce state reducer DeviceFlipTracker.kt motion sensor listener with dynamic lifecycle and battery-saving unregistering - NfcTagHandoff.kt transient tag UID intent-to-engine queue and UI capture state + NfcTagHandoff.kt transient platform-reader tag UID queue and UI capture state NfcTagUtils.kt pure tag UID normalization and validation FlowPilotNotificationListener.kt transient notification listener, dedupe, and engine watchdog GeofenceState.kt pure geofence models, config validation, registration diff, and prerequisites evaluator @@ -169,7 +169,7 @@ BluetoothDeviceTracker dynamically receives public ACL connection broadcasts onl BluetoothExecutor runs only exact allowlisted `svc bluetooth enable` or `svc bluetooth disable` through Shizuku. It returns failure when adapter, `BLUETOOTH_CONNECT`, Shizuku, command, or state readback is unavailable/mismatched, polling adapter state for up to 5 seconds after command completion. Xiaomi 15T Pro / HyperOS 3 smoke testing confirmed Bluetooth turns on and off successfully. -MainActivity receives NFC tag/tech discovery intents, extracts only tag UID, and hands normalized UID to the running engine through in-memory NfcTagHandoff. Rule matching uses selected UID only. No NDEF payload or technology data is retained. Create/Edit screens can capture a tag UID while open. Unit/build and configured-tag Xiaomi 15T Pro / HyperOS 3 smoke testing passed. +MainActivity enables `NfcAdapter.ReaderMode` while resumed. Its Android NFC-stack `ReaderCallback` extracts only a physical tag UID and hands its normalized value to in-memory `NfcTagHandoff`, preserving foreground automatic NFC rules. Manifest `TAG_DISCOVERED` and `TECH_DISCOVERED` filters keep Android background tag discovery available, but their intent data is untrusted: `NfcBackgroundConfirmationGate` holds only the UID until the visible Compose confirmation is accepted. Dismissal drops it; no intent-derived UID reaches the engine before confirmation. `NfcIntentSession` blocks ReaderMode registration and callback emission for that NFC-intent activity session, so confirmation, dismissal, or a queued callback cannot bypass the gate after resume. Rule matching uses selected UID only. No NDEF payload or technology data is retained. Create/Edit screens can capture a tag UID while open. Phone call triggers (`CALL_RINGING`, `CALL_ANSWERED`, `CALL_OUTGOING`, `CALL_ENDED`) evaluate state transitions without phone-number filtering. Android 12+ / HyperOS does not expose outgoing numbers to apps without the default-dialer role; call triggers match every call of that state. Legacy filter-configured rules operate as state-only / any-number rules. Device validation for this removal has not been run on device. Direct call and dial actions (`CALL_NUMBER`, `DIAL_NUMBER`) preserve phone number inputs and normalization/masking safeguards. diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 97ae685..9b257d1 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -64,9 +64,9 @@ Do not bundle unrelated features. One feature family at a time. - Android public ACL broadcasts only while engine runs; no discovery, pairing, scan history, or startup replay - Android 12+ `BLUETOOTH_CONNECT` runtime permission required 9. **NFC tag scanned** (complete; Xiaomi configured-tag smoke test passed) - - Selected normalized tag UID matching, with no NDEF payload or tag-tech persistence - - Tag UID capture in Create/Edit while FlowPilot is open - - Tag/tech discovery intent handoff evaluates only while engine runs + - Selected normalized tag UID matching, with no NDEF payload or tag-tech persistence + - Tag UID capture in Create/Edit while FlowPilot is open + - Foreground `NfcAdapter.ReaderCallback` handoff evaluates automatically; background discovery opens explicit confirmation before any matching rule runs 10. **Phone call triggers** (implementation complete; Xiaomi device smoke test pending) - Incoming call ringing (`CALL_RINGING`), answered (`CALL_ANSWERED`), outgoing call placed (`CALL_OUTGOING`), and call ended (`CALL_ENDED`). @@ -212,9 +212,9 @@ Each must expose its required permission or Shizuku state. Do not show success u - Unpair selected device; verify no crash and no false match. - Stop/deny Shizuku and verify Bluetooth on/off failures remain explicit. 6. **NFC tag and action delay** - - Scan different tag UID with engine running; verify it does not fire. - - Scan with engine stopped and NFC disabled; verify no action or false success. - - Add a visible action after 5 seconds; verify timing, order, stop cancellation, and history. + - Scan different tag UID with engine running; verify it does not fire. + - Send forged `TAG_DISCOVERED` and `TECH_DISCOVERED` intents with configured UIDs; verify no action before confirmation and no action after dismissal. Confirm a configured background scan; verify matching rule fires. Scan configured physical tag while FlowPilot is foreground; verify it fires automatically. + - Add a visible action after 5 seconds; verify timing, order, stop cancellation, and history. 7. **Action reordering and delay sequence validation** - Add multiple actions with distinct delays (e.g. Action A with 3s delay, Action B with 2s delay). - Use Move Up / Move Down controls to swap orders; verify execution timing proceeds strictly sequentially in configured order (A runs at 3s, then B runs at 5s total elapsed). diff --git a/docs/STATUS.md b/docs/STATUS.md index 82cd9d5..2302dd6 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -93,7 +93,7 @@ Last updated: 2026-09-14 - Battery Saver needs `WRITE_SECURE_SETTINGS` or Shizuku. - Webhook secrets are Android Keystore AES-256-GCM encrypted at rest; Android backups are disabled. - Bluetooth triggers use selected bonded devices, public ACL broadcasts, and `BLUETOOTH_CONNECT` on Android 12+; no discovery, pairing, scan history, or startup replay. Bluetooth device/profile behavior can still differ on other OEMs. -- NFC tag rules match a persisted tag UID, not tag payload. UID is identifier only, not authentication; cloned tags can match. +- NFC tag rules match a persisted tag UID, not tag payload. UID is identifier only, not authentication; cloned tags can match. Foreground `NfcAdapter.ReaderCallback` scans remain automatic. Background Android discovery intents are untrusted and can only open a visible confirmation; their UID reaches the engine after the user confirms, never automatically. ReaderMode stays disabled for that NFC-intent activity session, preventing its scan from bypassing confirmation after resume. - Per-action delay is bounded to 300 seconds in UI. Engine cancellation during delay creates failed run-history record. - Rule cooldown begins only after successful automatic execution, applies to every automatic trigger, and is bypassed by manual test runs. - Xiaomi 15T Pro maps Sound profile Vibrate and Silent to the same observed ringer behavior; other devices can differ. From b0a7ae8ae2816be2db1063da38177e22d051115c Mon Sep 17 00:00:00 2001 From: Emirhan Date: Mon, 14 Sep 2026 17:59:54 +0300 Subject: [PATCH 2/7] fix: pin webhook connections to validated IPs --- .../flowpilot/app/actions/WebhookExecutor.kt | 250 +++++-- .../app/actions/WebhookExecutorTest.kt | 670 +++++------------- .../actions/WebhookPrivacyRegressionTest.kt | 11 +- 3 files changed, 363 insertions(+), 568 deletions(-) diff --git a/app/src/main/java/com/flowpilot/app/actions/WebhookExecutor.kt b/app/src/main/java/com/flowpilot/app/actions/WebhookExecutor.kt index e305267..90fa532 100644 --- a/app/src/main/java/com/flowpilot/app/actions/WebhookExecutor.kt +++ b/app/src/main/java/com/flowpilot/app/actions/WebhookExecutor.kt @@ -2,67 +2,148 @@ package com.flowpilot.app.actions import android.util.Log import com.flowpilot.app.data.model.ActionType -import java.nio.charset.StandardCharsets -import java.net.HttpURLConnection +import java.io.EOFException +import java.io.InputStream import java.net.Inet6Address import java.net.InetAddress -import javax.net.ssl.HttpsURLConnection -import javax.net.ssl.SSLSocketFactory import java.net.InetSocketAddress +import java.net.ProtocolException import java.net.Socket +import javax.net.ssl.HostnameVerifier +import javax.net.ssl.HttpsURLConnection +import javax.net.ssl.SSLSocket +import javax.net.ssl.SSLSocketFactory import java.net.URI import java.net.URL +import java.nio.charset.StandardCharsets -// This checks the TLS peer, not the initial TCP destination: Android's HTTP stack -// connects its raw socket before calling the layered SSLSocketFactory overload. -// Do not treat this factory as verified transport-level DNS-rebinding protection. -internal class PinnedSocketFactory( - private val delegate: SSLSocketFactory, +/** HTTPS transport whose first TCP connection targets only prevalidated [address]. */ +internal class PinnedHttpsTransport( + private val url: URL, private val address: InetAddress, - private val hostname: String, -) : SSLSocketFactory() { - private fun connect(port: Int, localAddress: InetAddress? = null, localPort: Int = 0): Socket = - delegate.createSocket().apply { - if (localAddress != null) bind(InetSocketAddress(localAddress, localPort)) - connect(InetSocketAddress(address, port)) + private val rawSocketFactory: () -> Socket = ::Socket, + private val sslSocketFactory: SSLSocketFactory = HttpsURLConnection.getDefaultSSLSocketFactory(), + private val verifier: HostnameVerifier = HttpsURLConnection.getDefaultHostnameVerifier(), +) : WebhookTransport { + private var socket: SSLSocket? = null + + override fun execute(method: String, headers: Map, body: ByteArray, timeoutMs: Int): Int { + val tlsSocket = connect(timeoutMs) + val requestTarget = url.file.takeIf { it.isNotEmpty() } ?: "/" + val host = hostHeader(url) + val request = buildString { + append("$method $requestTarget HTTP/1.1\r\n") + append("Host: $host\r\n") + append("Connection: close\r\n") + headers.forEach { (name, value) -> append("$name: $value\r\n") } + if (method in WebhookExecutor.METHODS_WITH_BODY) append("Content-Length: ${body.size}\r\n") + append("\r\n") } + tlsSocket.outputStream.write(request.toByteArray(StandardCharsets.ISO_8859_1)) + if (body.isNotEmpty()) tlsSocket.outputStream.write(body) + tlsSocket.outputStream.flush() + return readResponseCode(tlsSocket.inputStream) + } - private fun layer(socket: Socket, port: Int, autoClose: Boolean = true): Socket = - delegate.createSocket(socket, hostname, port, autoClose) + override fun close() { + try { + socket?.close() + } finally { + socket = null + } + } - override fun createSocket(host: String, port: Int): Socket = layer(connect(port), port) + private fun connect(timeoutMs: Int): SSLSocket { + socket?.let { return it } + val port = if (url.port == -1) DEFAULT_HTTPS_PORT else url.port + val tcpSocket = rawSocketFactory() + try { + tcpSocket.connect(InetSocketAddress(address, port), timeoutMs) + tcpSocket.soTimeout = timeoutMs + val tlsSocket = sslSocketFactory.createSocket(tcpSocket, url.host, port, true) as SSLSocket + tlsSocket.soTimeout = timeoutMs + tlsSocket.startHandshake() + if (!verifier.verify(url.host, tlsSocket.session)) { + throw javax.net.ssl.SSLPeerUnverifiedException("HTTPS hostname verification failed") + } + socket = tlsSocket + return tlsSocket + } catch (e: Exception) { + try { + tcpSocket.close() + } catch (_: Exception) { + } + throw e + } + } - override fun createSocket(host: String, port: Int, localHost: InetAddress, localPort: Int): Socket = - layer(connect(port, localHost, localPort), port) + private fun readResponseCode(input: InputStream): Int { + repeat(MAX_INTERIM_RESPONSES) { + val statusLine = readLine(input) + val parts = statusLine.split(' ', limit = 3) + val status = parts.getOrNull(1)?.toIntOrNull() + if (parts.size < 2 || !parts[0].startsWith("HTTP/") || status == null || status !in 100..599) { + throw ProtocolException("Invalid HTTPS response status") + } + consumeHeaders(input) + if (status !in 100..199) return status + if (status == SWITCHING_PROTOCOLS) throw ProtocolException("HTTPS protocol upgrade is not supported") + } + throw ProtocolException("Too many interim HTTPS responses") + } - override fun createSocket(address: InetAddress, port: Int): Socket = layer(connect(port), port) + private fun consumeHeaders(input: InputStream) { + var consumed = 0 + while (true) { + val line = readLine(input) + consumed += line.length + CRLF_BYTES + if (consumed > MAX_RESPONSE_HEADER_BYTES) throw ProtocolException("HTTPS response headers are too large") + if (line.isEmpty()) return + } + } - override fun createSocket(address: InetAddress, port: Int, localAddress: InetAddress, localPort: Int): Socket = - layer(connect(port, localAddress, localPort), port) + private fun readLine(input: InputStream): String { + val bytes = ArrayList(MAX_RESPONSE_LINE_BYTES) + while (true) { + val value = input.read() + if (value == -1) throw EOFException("HTTPS response ended before headers completed") + if (value == '\n'.code) { + if (bytes.lastOrNull()?.toInt() != '\r'.code) throw ProtocolException("Invalid HTTPS response line ending") + bytes.removeAt(bytes.lastIndex) + return String(bytes.toByteArray(), StandardCharsets.ISO_8859_1) + } + if (bytes.size >= MAX_RESPONSE_LINE_BYTES) throw ProtocolException("HTTPS response line is too large") + bytes += value.toByte() + } + } - override fun createSocket(socket: Socket, host: String, port: Int, autoClose: Boolean): Socket { - if (socket.isConnected && socket.inetAddress != address) { - throw SecurityException("Socket is not connected to validated address") + private companion object { + const val DEFAULT_HTTPS_PORT = 443 + const val CRLF_BYTES = 2 + const val MAX_RESPONSE_LINE_BYTES = 8 * 1024 + const val MAX_RESPONSE_HEADER_BYTES = 32 * 1024 + const val MAX_INTERIM_RESPONSES = 8 + const val SWITCHING_PROTOCOLS = 101 + + fun hostHeader(url: URL): String { + val host = url.host.let { if (':' in it && !it.startsWith("[")) "[$it]" else it } + return host + if (url.port != -1 && url.port != DEFAULT_HTTPS_PORT) ":${url.port}" else "" } - if (!socket.isConnected) socket.connect(InetSocketAddress(address, port)) - return layer(socket, port, autoClose) } +} - override fun getDefaultCipherSuites(): Array = delegate.defaultCipherSuites - override fun getSupportedCipherSuites(): Array = delegate.supportedCipherSuites +internal interface WebhookTransport { + fun execute(method: String, headers: Map, body: ByteArray, timeoutMs: Int): Int + fun close() } /** - * Executes outbound HTTP/HTTPS requests (webhooks) using standard HttpURLConnection. + * Executes outbound HTTPS webhook requests through a validated-address transport. * Validates URLs, bounds timeouts, handles headers/bodies, redacts secrets in logs and failure messages. */ -class WebhookExecutor( - private val connectionFactory: (URL, InetAddress) -> HttpURLConnection = { url, address -> - (url.openConnection() as HttpURLConnection).apply { - if (this is HttpsURLConnection) { - sslSocketFactory = PinnedSocketFactory(sslSocketFactory, address, url.host) - } - } +class WebhookExecutor internal constructor( + private val transportFactory: (URL, InetAddress) -> WebhookTransport = { url, address -> + PinnedHttpsTransport(url, address) }, private val addressLookup: (String) -> Array = InetAddress::getAllByName, ) : ActionExecutor { @@ -82,43 +163,26 @@ class WebhookExecutor( val rawUrl = parameters.webhookUrl.trim() val method = parameters.webhookMethod.trim().uppercase() val timeoutMs = parameters.webhookTimeoutSeconds.coerceIn(MIN_TIMEOUT_SECONDS, MAX_TIMEOUT_SECONDS) * 1000 - val renderedHeaders = WebhookTemplateRenderer.render(parameters.webhookHeaders, parameters.webhookTemplateContext) - val headers = parseHeaders(renderedHeaders) + val headers = try { + renderHeaders(parameters.webhookHeaders, parameters.webhookTemplateContext) + } catch (e: IllegalArgumentException) { + return ActionResult(false, e.message ?: "Invalid rendered webhook headers") + } val body = WebhookTemplateRenderer.render(parameters.webhookBody, parameters.webhookTemplateContext) Log.i(TAG, "Dispatching HTTP Webhook: method=$method") - var connection: HttpURLConnection? = null + var transport: WebhookTransport? = null return try { val url = URI(rawUrl).toURL() val address = validateResolvedAddresses(url.host) - connection = connectionFactory(url, address).apply { - requestMethod = method - connectTimeout = timeoutMs - readTimeout = timeoutMs - instanceFollowRedirects = false - useCaches = false - doInput = true - - headers.forEach { (name, value) -> - setRequestProperty(name, value) - } - - if (method in METHODS_WITH_BODY && body.isNotEmpty()) { - doOutput = true - val bytes = body.toByteArray(StandardCharsets.UTF_8) - setFixedLengthStreamingMode(bytes.size) - outputStream.use { os -> - os.write(bytes) - os.flush() - } - } else if (method in METHODS_WITH_BODY && body.isEmpty()) { - // For POST/PUT/PATCH with empty body, ensure Content-Length is 0 if no output stream written - setFixedLengthStreamingMode(0) - } - } - - val statusCode = connection.responseCode + transport = transportFactory(url, address) + val statusCode = transport.execute( + method = method, + headers = headers, + body = if (method in METHODS_WITH_BODY) body.toByteArray(StandardCharsets.UTF_8) else ByteArray(0), + timeoutMs = timeoutMs, + ) val isSuccess = statusCode in 200..299 val message = if (isSuccess) { "HTTP Webhook delivered: status $statusCode" @@ -138,7 +202,7 @@ class WebhookExecutor( ActionResult(false, "HTTP request failed: $safeMessage") } finally { try { - connection?.disconnect() + transport?.close() } catch (_: Throwable) {} } } @@ -234,7 +298,12 @@ class WebhookExecutor( val method = parameters.webhookMethod.trim().uppercase() if (method !in ALLOWED_METHODS) { - return "Unsupported HTTP method: $method. Allowed: ${ALLOWED_METHODS.joinToString(", ")}" + val allowedMethods = ALLOWED_METHODS.joinToString(separator = ", ") + return "Unsupported HTTP method: $method. Allowed: $allowedMethods" + } + + if (method !in METHODS_WITH_BODY && parameters.webhookBody.isNotEmpty()) { + return "HTTP method $method does not support a webhook body" } val headerError = validateHeaders(parameters.webhookHeaders) @@ -265,6 +334,12 @@ class WebhookExecutor( if (name.isEmpty()) { return "Invalid header name on line ${index + 1}: name cannot be empty" } + if (!name.all { it in HEADER_NAME_CHARS }) { + return "Invalid header name on line ${index + 1}" + } + if (name.lowercase() in FORBIDDEN_REQUEST_HEADERS) { + return "Forbidden header name on line ${index + 1}" + } if (name.any { it.isISOControl() } || value.any { it.isISOControl() }) { return "Invalid header on line ${index + 1}: header cannot contain control characters" } @@ -291,6 +366,41 @@ class WebhookExecutor( return result } + /** Renders configured header lines independently; replacements may not add header lines. */ + fun renderHeaders(rawHeaders: String, context: WebhookTemplateContext?): Map { + val validationError = validateHeaders(rawHeaders) + require(validationError == null) { validationError ?: "Invalid webhook headers" } + if (rawHeaders.isBlank()) return emptyMap() + + return buildMap { + rawHeaders.split("\r\n", "\n", "\r").forEachIndexed { index, line -> + val configuredLine = line.trim() + if (configuredLine.isEmpty() || configuredLine.startsWith("#")) return@forEachIndexed + val renderedLine = WebhookTemplateRenderer.render(configuredLine, context) + if (renderedLine.any { it == '\r' || it == '\n' || it.isISOControl() }) { + throw IllegalArgumentException("Invalid rendered header on line ${index + 1}: header cannot contain control characters") + } + val colonIndex = renderedLine.indexOf(':') + val name = renderedLine.substring(0, colonIndex).trim() + val value = renderedLine.substring(colonIndex + 1).trim() + if (!name.all { it in HEADER_NAME_CHARS }) { + throw IllegalArgumentException("Invalid rendered header name on line ${index + 1}") + } + if (name.lowercase() in FORBIDDEN_REQUEST_HEADERS) { + throw IllegalArgumentException("Forbidden rendered header name on line ${index + 1}") + } + put(name, value) + } + } + } + + private const val HEADER_NAME_CHARS = "!#$%&'*+-.^_`|~0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ" + private val FORBIDDEN_REQUEST_HEADERS = setOf( + "connection", "content-length", "host", "keep-alive", "proxy-connection", + "expect", "proxy-authenticate", "proxy-authorization", "te", "trailer", + "transfer-encoding", "upgrade", + ) + fun sanitizeUrlForLogging(rawUrl: String): String { if (rawUrl.isBlank()) return rawUrl return try { diff --git a/app/src/test/java/com/flowpilot/app/actions/WebhookExecutorTest.kt b/app/src/test/java/com/flowpilot/app/actions/WebhookExecutorTest.kt index f2b9b37..1497245 100644 --- a/app/src/test/java/com/flowpilot/app/actions/WebhookExecutorTest.kt +++ b/app/src/test/java/com/flowpilot/app/actions/WebhookExecutorTest.kt @@ -6,559 +6,247 @@ import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config -import org.robolectric.shadows.ShadowLog +import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream -import java.io.IOException -import java.net.HttpURLConnection import java.net.InetAddress +import java.net.ProtocolException import java.net.Socket import java.net.URL import java.nio.charset.StandardCharsets +import java.lang.reflect.Proxy +import javax.net.ssl.HandshakeCompletedListener +import javax.net.ssl.HostnameVerifier +import javax.net.ssl.SSLPeerUnverifiedException +import javax.net.ssl.SSLSession +import javax.net.ssl.SSLSocket +import javax.net.ssl.SSLSocketFactory @RunWith(RobolectricTestRunner::class) @Config(sdk = [34]) class WebhookExecutorTest { @Test - fun execute_unsupportedAction_returnsFailure() { - val executor = WebhookExecutor() - val result = executor.execute(ActionType.VIBRATE) - - assertThat(result.success).isFalse() - assertThat(result.message).contains("Unsupported action for Webhook") - } - - @Test - fun execute_emptyUrl_returnsFailure() { - val executor = WebhookExecutor() - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters(webhookUrl = ""), - ) - - assertThat(result.success).isFalse() - assertThat(result.message).contains("Webhook URL cannot be empty") - } - - @Test - fun execute_invalidUrlScheme_returnsFailure() { - val executor = WebhookExecutor() - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters(webhookUrl = "ftp://example.com/api"), - ) - - assertThat(result.success).isFalse() - assertThat(result.message).contains("must use HTTPS scheme") - } - - @Test - fun execute_httpUrl_returnsFailureBeforeConnection() { - var connectionFactoryCalled = false - val executor = WebhookExecutor(connectionFactory = { _, _ -> - connectionFactoryCalled = true - error("HTTP webhook must not open connection") - }) - - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters(webhookUrl = "http://example.com/api"), - ) - - assertThat(result.success).isFalse() - assertThat(result.message).contains("must use HTTPS scheme") - assertThat(connectionFactoryCalled).isFalse() - } - - @Test - fun execute_localhost_privateAndMetadataTargets_areRejectedBeforeConnection() { - listOf("127.0.0.1", "10.0.0.1", "169.254.169.254").forEach { addressText -> - var connectionFactoryCalled = false - val executor = WebhookExecutor( - connectionFactory = { _, _ -> - connectionFactoryCalled = true - error("SSRF target must not open connection") - }, - addressLookup = { arrayOf(InetAddress.getByName(addressText)) }, - ) - - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters(webhookUrl = "https://target.example/webhook"), - ) - - assertThat(result.success).isFalse() - assertThat(result.message).contains("non-public address") - assertThat(connectionFactoryCalled).isFalse() - } - } - - @Test - fun execute_mappedLoopbackPrivateAndMetadataTargets_areRejectedBeforeConnection() { - listOf("::ffff:127.0.0.1", "::ffff:10.0.0.1", "::ffff:169.254.169.254").forEach { addressText -> - var connectionFactoryCalled = false - val executor = WebhookExecutor( - connectionFactory = { _, _ -> - connectionFactoryCalled = true - error("SSRF target must not open connection") - }, - addressLookup = { arrayOf(InetAddress.getByName(addressText)) }, - ) - - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters(webhookUrl = "https://target.example/webhook"), - ) - - assertThat(result.success).isFalse() - assertThat(result.message).contains("non-public address") - assertThat(connectionFactoryCalled).isFalse() - } - } - - @Test - fun execute_mappedPublicTarget_allowsConnection() { - val mockConnection = FakeHttpURLConnection(URL("https://target.example/webhook"), 204) - val executor = WebhookExecutor( - connectionFactory = { _, _ -> mockConnection }, - addressLookup = { arrayOf(InetAddress.getByName("::ffff:93.184.216.34")) }, - ) - - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters(webhookUrl = "https://target.example/webhook"), - ) - - assertThat(result.success).isTrue() - assertThat(result.message).contains("status 204") - } - - @Test - fun execute_publicHttpsTarget_allowsConnection() { - val mockConnection = FakeHttpURLConnection(URL("https://target.example/webhook"), 204) - val executor = WebhookExecutor( - connectionFactory = { _, _ -> mockConnection }, - addressLookup = { arrayOf(InetAddress.getByName("93.184.216.34")) }, - ) - - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters(webhookUrl = "https://target.example/webhook"), - ) - - assertThat(result.success).isTrue() - assertThat(result.message).contains("status 204") - } - - @Test - fun execute_pinsConnectionToValidatedAddress_withoutSecondDnsLookup() { - val validatedAddress = InetAddress.getByName("93.184.216.34") - val redirectedAddress = InetAddress.getByName("10.0.0.1") - var lookupCount = 0 - var connectedAddress: InetAddress? = null - val mockConnection = FakeHttpURLConnection(URL("https://target.example/webhook"), 204) - val executor = WebhookExecutor( - connectionFactory = { _, address -> - connectedAddress = address - mockConnection - }, - addressLookup = { - lookupCount++ - if (lookupCount == 1) arrayOf(validatedAddress) else arrayOf(redirectedAddress) - }, - ) - - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters(webhookUrl = "https://target.example/webhook"), - ) - - assertThat(result.success).isTrue() - assertThat(lookupCount).isEqualTo(1) - assertThat(connectedAddress).isEqualTo(validatedAddress) - } - - @Test - fun pinnedSocketFactory_allOverloadsConnectValidatedAddressAndPreserveTlsHost() { + fun execute_pinsSingleValidatedAddress_andClosesTransport() { val validated = InetAddress.getByName("93.184.216.34") - val delegate = RecordingSslSocketFactory() - val factory = PinnedSocketFactory(delegate, validated, "original.example") - - factory.createSocket("original.example", 443) - factory.createSocket("original.example", 443, InetAddress.getByName("192.0.2.1"), 0) - factory.createSocket(InetAddress.getByName("10.0.0.1"), 443) - factory.createSocket(InetAddress.getByName("10.0.0.1"), 443, InetAddress.getByName("192.0.2.1"), 0) - factory.createSocket(RecordingSocket(delegate.connectedAddresses), "original.example", 443, true) + val transport = FakeTransport(204) + var lookups = 0 + var suppliedAddress: InetAddress? = null + val result = WebhookExecutor( + transportFactory = { _, address -> suppliedAddress = address; transport }, + addressLookup = { if (++lookups == 1) arrayOf(validated) else arrayOf(InetAddress.getByName("10.0.0.1")) }, + ).execute(ActionType.HTTP_WEBHOOK, ActionParameters(webhookUrl = "https://target.example/hook")) - assertThat(delegate.connectedAddresses).containsExactlyElementsIn(List(5) { validated }).inOrder() - assertThat(delegate.layeredHosts).containsExactly("original.example", "original.example", "original.example", "original.example", "original.example").inOrder() + assertThat(result.success).isTrue() + assertThat(lookups).isEqualTo(1) + assertThat(suppliedAddress).isEqualTo(validated) + assertThat(transport.closed).isTrue() } @Test - fun pinnedSocketFactory_rejectsAlreadyConnectedSocketToOtherAddress() { - val validated = InetAddress.getByName("93.184.216.34") - val server = java.net.ServerSocket(0) - val socket = Socket("127.0.0.1", server.localPort) - val factory = PinnedSocketFactory(RecordingSslSocketFactory(), validated, "original.example") + fun execute_rejectsNonPublicAddressAndRenderedHeaderInjection_beforeTransport() { + var opened = false + val privateTarget = WebhookExecutor( + transportFactory = { _, _ -> opened = true; error("must not connect") }, + addressLookup = { arrayOf(InetAddress.getByName("127.0.0.1")) }, + ).execute(ActionType.HTTP_WEBHOOK, ActionParameters(webhookUrl = "https://target.example/hook")) + assertThat(privateTarget.success).isFalse() + assertThat(opened).isFalse() - try { - factory.createSocket(socket, "original.example", 443, true) - throw AssertionError("connected socket must be rejected") - } catch (expected: SecurityException) { - assertThat(expected).hasMessageThat().contains("validated address") - } finally { - socket.close() - } + val injected = WebhookExecutor( + transportFactory = { _, _ -> opened = true; error("must not connect") }, + addressLookup = { arrayOf(InetAddress.getByName("93.184.216.34")) }, + ).execute(ActionType.HTTP_WEBHOOK, ActionParameters( + webhookUrl = "https://target.example/hook", + webhookHeaders = "X-Trigger: \${trigger}", + webhookTemplateContext = WebhookTemplateContext(trigger = "safe\r\nAuthorization: attacker"), + )) + assertThat(injected.success).isFalse() + assertThat(injected.message).doesNotContain("attacker") + assertThat(opened).isFalse() } @Test - fun execute_allKnownNonGlobalSpecialUseRanges_areRejected() { + fun execute_rejectsReservedHeadersAndBodiesForBodylessMethods() { listOf( - "0.1.2.3", "100.64.0.1", "192.0.0.1", "192.0.2.1", "192.88.99.1", - "198.18.0.1", "198.51.100.1", "203.0.113.1", "224.0.0.1", "240.0.0.1", - "::", "::1", "2001:db8::1", "2001:10::1", "2001:20::1", "2001:0000::1", - "64:ff9b::1", "100::1", "2002::1", "fc00::1", "fe80::1", "ff02::1", - "::ffff:192.0.2.1", - ).forEach { addressText -> - var opened = false - val result = WebhookExecutor( - connectionFactory = { _, _ -> opened = true; error("special-use target opened") }, - addressLookup = { arrayOf(InetAddress.getByName(addressText)) }, - ).execute(ActionType.HTTP_WEBHOOK, ActionParameters(webhookUrl = "https://target.example/webhook")) - assertThat(result.success).isFalse() - assertThat(opened).isFalse() + "Host: attacker.example", "Transfer-Encoding: chunked", "Connection: keep-alive", + "Expect: 100-continue", "Proxy-Authorization: Basic attacker", + ).forEach { + assertThat(WebhookExecutor.validateHeaders(it)).contains("Forbidden header") } - } - - @Test - fun execute_globalAddress_remainsAllowed() { - val connection = FakeHttpURLConnection(URL("https://target.example/webhook"), 204) - val result = WebhookExecutor( - connectionFactory = { _, _ -> connection }, - addressLookup = { arrayOf(InetAddress.getByName("93.184.216.34")) }, - ).execute(ActionType.HTTP_WEBHOOK, ActionParameters(webhookUrl = "https://target.example/webhook")) - assertThat(result.success).isTrue() - } - - @Test - fun execute_unsupportedMethod_returnsFailure() { - val executor = WebhookExecutor() - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters( - webhookUrl = "https://example.com/api", - webhookMethod = "INVALID_METHOD", - ), - ) - + val result = WebhookExecutor().execute(ActionType.HTTP_WEBHOOK, ActionParameters( + webhookUrl = "https://target.example/hook", + webhookMethod = "GET", + webhookBody = "must-not-send", + )) assertThat(result.success).isFalse() - assertThat(result.message).contains("Unsupported HTTP method") - } - - @Test - fun execute_rendersTemplateVariablesInHeadersAndBody() { - val mockConnection = FakeHttpURLConnection(URL("https://example.com/webhook"), 200) - val executor = WebhookExecutor( - connectionFactory = { _, _ -> mockConnection }, - addressLookup = { arrayOf(InetAddress.getByName("93.184.216.34")) }, - ) - val templateContext = WebhookTemplateContext( - trigger = "CHARGER_CONNECTED", - timestamp = 1700000000000L, - timeProvider = { "2023-11-14T22:13:20Z" }, - batteryPercent = 90, - isCharging = true, - wifiSsid = "OfficeNet", - ) - - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters( - webhookUrl = "https://example.com/webhook", - webhookMethod = "POST", - webhookHeaders = "X-Trigger: \${trigger}\nX-Battery: \${batteryPercent}\nContent-Type: application/json", - webhookBody = "{\"event\": \"\${trigger}\", \"time\": \"\${time}\", \"wifi\": \"\${wifiSsid}\", \"charging\": \${isCharging}}", - webhookTemplateContext = templateContext, - ), - ) - - assertThat(result.success).isTrue() - assertThat(mockConnection.recordedRequestProperties["X-Trigger"]).isEqualTo("CHARGER_CONNECTED") - assertThat(mockConnection.recordedRequestProperties["X-Battery"]).isEqualTo("90") - assertThat(mockConnection.recordedRequestProperties["Content-Type"]).isEqualTo("application/json") - assertThat(mockConnection.writtenBody()).isEqualTo("{\"event\": \"CHARGER_CONNECTED\", \"time\": \"2023-11-14T22:13:20Z\", \"wifi\": \"OfficeNet\", \"charging\": true}") - } - - @Test - fun execute_successful200Response_returnsSuccess() { - val mockConnection = FakeHttpURLConnection(URL("https://example.com/webhook"), 200) - val executor = WebhookExecutor( - connectionFactory = { _, _ -> mockConnection }, - addressLookup = { arrayOf(InetAddress.getByName("93.184.216.34")) }, - ) - - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters( - webhookUrl = "https://example.com/webhook", - webhookMethod = "POST", - webhookHeaders = "Content-Type: application/json\nAuthorization: Bearer secret_token_123", - webhookBody = "{\"state\": \"on\"}", - webhookTimeoutSeconds = 5, - ), - ) - - assertThat(result.success).isTrue() - assertThat(result.message).contains("status 200") - assertThat(mockConnection.requestMethod).isEqualTo("POST") - assertThat(mockConnection.connectTimeout).isEqualTo(5000) - assertThat(mockConnection.readTimeout).isEqualTo(5000) - assertThat(mockConnection.recordedRequestProperties["Content-Type"]).isEqualTo("application/json") - assertThat(mockConnection.recordedRequestProperties["Authorization"]).isEqualTo("Bearer secret_token_123") - assertThat(mockConnection.writtenBody()).isEqualTo("{\"state\": \"on\"}") - assertThat(mockConnection.isDisconnected).isTrue() + assertThat(result.message).contains("does not support a webhook body") } @Test - fun execute_204NoContent_returnsSuccess() { - val mockConnection = FakeHttpURLConnection(URL("https://example.com/webhook"), 204) - val executor = WebhookExecutor( - connectionFactory = { _, _ -> mockConnection }, + fun execute_sendsPostBodyHeadersAndTimeout_toTransport() { + val transport = FakeTransport(200) + val result = WebhookExecutor( + transportFactory = { _, _ -> transport }, addressLookup = { arrayOf(InetAddress.getByName("93.184.216.34")) }, - ) - - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters( - webhookUrl = "https://example.com/webhook", - webhookMethod = "GET", - ), - ) + ).execute(ActionType.HTTP_WEBHOOK, ActionParameters( + webhookUrl = "https://target.example/hook", + webhookMethod = "POST", + webhookHeaders = "Content-Type: application/json\nX-Event: \${trigger}", + webhookBody = "{\"event\":\"\${trigger}\"}", + webhookTimeoutSeconds = 5, + webhookTemplateContext = WebhookTemplateContext(trigger = "CHARGER_CONNECTED"), + )) assertThat(result.success).isTrue() - assertThat(result.message).contains("status 204") + assertThat(transport.method).isEqualTo("POST") + assertThat(transport.headers).containsEntry("X-Event", "CHARGER_CONNECTED") + assertThat(String(transport.body, StandardCharsets.UTF_8)).isEqualTo("{\"event\":\"CHARGER_CONNECTED\"}") + assertThat(transport.timeoutMs).isEqualTo(5_000) + assertThat(transport.closed).isTrue() } @Test - fun execute_http400Response_returnsFailure() { - val mockConnection = FakeHttpURLConnection(URL("https://example.com/webhook"), 400) - val executor = WebhookExecutor( - connectionFactory = { _, _ -> mockConnection }, + fun execute_closesTransportWhenDispatchFails() { + val transport = object : WebhookTransport { + var closed = false + override fun execute(method: String, headers: Map, body: ByteArray, timeoutMs: Int): Int = + throw ProtocolException("synthetic") + override fun close() { closed = true } + } + val result = WebhookExecutor( + transportFactory = { _, _ -> transport }, addressLookup = { arrayOf(InetAddress.getByName("93.184.216.34")) }, - ) - - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters( - webhookUrl = "https://example.com/webhook", - webhookMethod = "POST", - ), - ) + ).execute(ActionType.HTTP_WEBHOOK, ActionParameters(webhookUrl = "https://target.example/hook")) assertThat(result.success).isFalse() - assertThat(result.message).contains("status 400") + assertThat(transport.closed).isTrue() } @Test - fun execute_http500Response_returnsFailure() { - val mockConnection = FakeHttpURLConnection(URL("https://example.com/webhook"), 500) - val executor = WebhookExecutor( - connectionFactory = { _, _ -> mockConnection }, - addressLookup = { arrayOf(InetAddress.getByName("93.184.216.34")) }, + fun pinnedTransport_serializesFixedLengthRequest_andSkipsInterimResponse() { + val rawSocket = RecordingSocket() + val tlsSocket = RecordingTlsSocket("HTTP/1.1 100 Continue\r\nX-Interim: yes\r\n\r\nHTTP/1.1 204 No Content\r\nX-Final: yes\r\n\r\n") + val tlsFactory = RecordingSslSocketFactory(tlsSocket) + val transport = PinnedHttpsTransport( + URL("https://original.example:8443/hook?x=1"), + InetAddress.getByName("93.184.216.34"), + rawSocketFactory = { rawSocket }, + sslSocketFactory = tlsFactory, + verifier = HostnameVerifier { host, _ -> host == "original.example" }, ) - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters( - webhookUrl = "https://example.com/webhook", - webhookMethod = "POST", - ), - ) + val status = transport.execute("POST", mapOf("X-Test" to "yes"), "body".toByteArray(), 5_000) - assertThat(result.success).isFalse() - assertThat(result.message).contains("status 500") + assertThat(status).isEqualTo(204) + assertThat(rawSocket.address).isEqualTo(InetAddress.getByName("93.184.216.34")) + assertThat(rawSocket.port).isEqualTo(8443) + assertThat(tlsFactory.layeredHost).isEqualTo("original.example") + assertThat(tlsSocket.written.toString(StandardCharsets.ISO_8859_1.name())).isEqualTo( + "POST /hook?x=1 HTTP/1.1\r\nHost: original.example:8443\r\nConnection: close\r\nX-Test: yes\r\nContent-Length: 4\r\n\r\nbody", + ) + transport.close() + assertThat(tlsSocket.closed).isTrue() } @Test - fun execute_networkExceptionWithSensitiveAuth_redactsAuthInFailureMessage() { - val bearer = "SYNTHETIC_BEARER_DO_NOT_LOG_123" - ShadowLog.clear() - val executor = WebhookExecutor( - connectionFactory = { _, _ -> - throw IOException("Failed to connect with Authorization: Bearer $bearer and key=secret123") - }, - addressLookup = { arrayOf(InetAddress.getByName("93.184.216.34")) }, + fun pinnedTransport_rejectsMalformedResponseLine() { + val transport = PinnedHttpsTransport( + URL("https://original.example/hook"), + InetAddress.getByName("93.184.216.34"), + rawSocketFactory = { RecordingSocket() }, + sslSocketFactory = RecordingSslSocketFactory(RecordingTlsSocket("not-http\n")), + verifier = HostnameVerifier { _, _ -> true }, ) - - val result = executor.execute( - ActionType.HTTP_WEBHOOK, - ActionParameters( - webhookUrl = "https://example.com/webhook", - webhookMethod = "POST", - ), - ) - - assertThat(result.success).isFalse() - assertThat(result.message).doesNotContain(bearer) - assertThat(result.message).doesNotContain("secret123") - assertThat(result.message).contains("[REDACTED]") - val logs = ShadowLog.getLogsForTag(WebhookExecutor.TAG) - assertThat(logs).isNotEmpty() - logs.forEach { log -> - assertThat(log.msg).doesNotContain(bearer) - assertThat(log.msg).doesNotContain("secret123") - assertThat(log.throwable).isNull() + try { + transport.execute("GET", emptyMap(), ByteArray(0), 5_000) + throw AssertionError("malformed response must fail") + } catch (_: ProtocolException) { } } @Test - fun parseHeaders_handlesValidLinesAndComments() { - val raw = """ - Content-Type: application/json - # Comment line - Authorization: Bearer test_token - X-Custom-Header: value with : colon - EmptyValue: - """.trimIndent() - - val headers = WebhookExecutor.parseHeaders(raw) - assertThat(headers).hasSize(4) - assertThat(headers["Content-Type"]).isEqualTo("application/json") - assertThat(headers["Authorization"]).isEqualTo("Bearer test_token") - assertThat(headers["X-Custom-Header"]).isEqualTo("value with : colon") - assertThat(headers["EmptyValue"]).isEqualTo("") - } - - @Test - fun sanitizeHeadersForLogging_redactsSensitiveHeaders() { - val headers = mapOf( - "Content-Type" to "application/json", - "Authorization" to "Bearer sensitive_token", - "X-Api-Key" to "api_key_value", - "Cookie" to "session=abc", - "User-Agent" to "FlowPilot", + fun pinnedTransport_rejectsHostnameMismatch_andClosesTcpSocket() { + val rawSocket = RecordingSocket() + val transport = PinnedHttpsTransport( + URL("https://original.example/hook"), + InetAddress.getByName("93.184.216.34"), + rawSocketFactory = { rawSocket }, + sslSocketFactory = RecordingSslSocketFactory(RecordingTlsSocket("")), + verifier = HostnameVerifier { _, _ -> false }, ) - val sanitized = WebhookExecutor.sanitizeHeadersForLogging(headers) - - assertThat(sanitized["Content-Type"]).isEqualTo("application/json") - assertThat(sanitized["User-Agent"]).isEqualTo("FlowPilot") - assertThat(sanitized["Authorization"]).isEqualTo("[REDACTED]") - assertThat(sanitized["X-Api-Key"]).isEqualTo("[REDACTED]") - assertThat(sanitized["Cookie"]).isEqualTo("[REDACTED]") - } - - @Test - fun redactSensitiveText_replacesTokensAndPasswords() { - val text = "Error: Bearer 1234567890abcdef failed. Param token=sensitive_tok&user=bob, password=mypassword https://api.example.com/v1/trigger?token=secret123&user=admin" - val redacted = WebhookExecutor.redactSensitiveText(text) - - assertThat(redacted).doesNotContain("1234567890abcdef") - assertThat(redacted).doesNotContain("sensitive_tok") - assertThat(redacted).doesNotContain("mypassword") - assertThat(redacted).doesNotContain("secret123") - assertThat(redacted).doesNotContain("admin") - assertThat(redacted).contains("Bearer [REDACTED]") - assertThat(redacted).contains("token=[REDACTED]") - assertThat(redacted).contains("password=[REDACTED]") - assertThat(redacted).contains("https://api.example.com/v1/trigger?token=[REDACTED]&user=[REDACTED]") - } - - @Test - fun sanitizeUrlForLogging_redactsQueryParamsAndUserInfo() { - val url = "https://user:pass123@api.example.com/v1/webhook?apiKey=xyz789&action=alert" - val sanitized = WebhookExecutor.sanitizeUrlForLogging(url) - - assertThat(sanitized).doesNotContain("pass123") - assertThat(sanitized).doesNotContain("xyz789") - assertThat(sanitized).contains("https://[REDACTED]@api.example.com/v1/webhook?apiKey=[REDACTED]&action=[REDACTED]") - } - - @Test - fun validateParameters_rejectsMalformedHeaders() { - val invalidHeaderParam = ActionParameters( - webhookUrl = "https://example.com/webhook", - webhookHeaders = "InvalidHeaderWithoutColon", - ) - val error = WebhookExecutor.validateParameters(invalidHeaderParam) - assertThat(error).isNotNull() - assertThat(error).contains("Invalid header format on line 1") - // Ensure failure message does not echo raw header value - assertThat(error).doesNotContain("InvalidHeaderWithoutColon") + try { + transport.execute("GET", emptyMap(), ByteArray(0), 5_000) + throw AssertionError("hostname mismatch must fail") + } catch (_: SSLPeerUnverifiedException) { + assertThat(rawSocket.closed).isTrue() + } } - @Test - fun validateParameters_rejectsCrlfInHeader() { - val crlfHeaderParam = ActionParameters( - webhookUrl = "https://example.com/webhook", - webhookHeaders = "X-Bad-Header: value\u0000injection", - ) - val error = WebhookExecutor.validateParameters(crlfHeaderParam) - assertThat(error).isNotNull() - assertThat(error).contains("header cannot contain control characters") - assertThat(error).doesNotContain("value\u0000injection") + private class FakeTransport(private val responseCode: Int) : WebhookTransport { + var method = "" + var headers = emptyMap() + var body = ByteArray(0) + var timeoutMs = 0 + var closed = false + override fun execute(method: String, headers: Map, body: ByteArray, timeoutMs: Int): Int { + this.method = method; this.headers = headers; this.body = body; this.timeoutMs = timeoutMs + return responseCode + } + override fun close() { closed = true } } - @Test - fun validateHeaders_validatesCorrectly() { - assertThat(WebhookExecutor.validateHeaders("")).isNull() - assertThat(WebhookExecutor.validateHeaders(" \n ")).isNull() - assertThat(WebhookExecutor.validateHeaders("# Just comment\nContent-Type: application/json")).isNull() - - val emptyNameError = WebhookExecutor.validateHeaders(": value_without_name") - assertThat(emptyNameError).contains("Invalid header format on line 1") - - val malformedError = WebhookExecutor.validateHeaders("MalformedHeader") - assertThat(malformedError).contains("Invalid header format on line 1") + private class RecordingSocket : Socket() { + var address: InetAddress? = null + var port: Int? = null + var closed = false + override fun connect(endpoint: java.net.SocketAddress?, timeout: Int) { + (endpoint as java.net.InetSocketAddress).also { address = it.address; port = it.port } + } + override fun setSoTimeout(timeout: Int) = Unit + override fun close() { closed = true } } - private class RecordingSslSocketFactory : javax.net.ssl.SSLSocketFactory() { - val connectedAddresses = mutableListOf() - val layeredHosts = mutableListOf() - - override fun createSocket(): Socket = RecordingSocket(connectedAddresses) + private class RecordingSslSocketFactory(private val socket: SSLSocket) : SSLSocketFactory() { + var layeredHost: String? = null override fun createSocket(socket: Socket, host: String, port: Int, autoClose: Boolean): Socket { - layeredHosts += host - return socket + layeredHost = host + return this.socket } - override fun createSocket(host: String, port: Int): Socket = error("unexpected direct delegate call") - override fun createSocket(host: String, port: Int, localHost: InetAddress, localPort: Int): Socket = error("unexpected direct delegate call") - override fun createSocket(address: InetAddress, port: Int): Socket = error("unexpected direct delegate call") - override fun createSocket(address: InetAddress, port: Int, localAddress: InetAddress, localPort: Int): Socket = error("unexpected direct delegate call") + override fun createSocket(): Socket = error("unexpected") + override fun createSocket(host: String, port: Int): Socket = error("unexpected") + override fun createSocket(host: String, port: Int, localHost: InetAddress, localPort: Int): Socket = error("unexpected") + override fun createSocket(address: InetAddress, port: Int): Socket = error("unexpected") + override fun createSocket(address: InetAddress, port: Int, localAddress: InetAddress, localPort: Int): Socket = error("unexpected") override fun getDefaultCipherSuites(): Array = emptyArray() override fun getSupportedCipherSuites(): Array = emptyArray() } - private class RecordingSocket(private val connectedAddresses: MutableList) : Socket() { - override fun connect(endpoint: java.net.SocketAddress?) { - connectedAddresses += (endpoint as java.net.InetSocketAddress).address - } - override fun connect(endpoint: java.net.SocketAddress?, timeout: Int) = connect(endpoint) - override fun bind(endpoint: java.net.SocketAddress?) {} - override fun isConnected(): Boolean = false - } - - private class FakeHttpURLConnection(url: URL, private val responseCodeStub: Int) : HttpURLConnection(url) { - val recordedRequestProperties = mutableMapOf() - private val outputStreamBuffer = ByteArrayOutputStream() - var isDisconnected = false - - override fun setRequestProperty(key: String, value: String) { - recordedRequestProperties[key] = value - } - - override fun getOutputStream(): java.io.OutputStream = outputStreamBuffer - - override fun getResponseCode(): Int = responseCodeStub - - override fun connect() {} - - override fun disconnect() { - isDisconnected = true - } - - override fun usingProxy(): Boolean = false - - fun writtenBody(): String = outputStreamBuffer.toString(StandardCharsets.UTF_8.name()) + private class RecordingTlsSocket(response: String) : SSLSocket() { + val written = ByteArrayOutputStream() + val input = ByteArrayInputStream(response.toByteArray(StandardCharsets.ISO_8859_1)) + var closed = false + override fun getOutputStream() = written + override fun getInputStream() = input + override fun close() { closed = true } + override fun setSoTimeout(timeout: Int) = Unit + override fun getSupportedCipherSuites(): Array = emptyArray() + override fun getEnabledCipherSuites(): Array = emptyArray() + override fun setEnabledCipherSuites(suites: Array?) = Unit + override fun getSupportedProtocols(): Array = emptyArray() + override fun getEnabledProtocols(): Array = emptyArray() + override fun setEnabledProtocols(protocols: Array?) = Unit + override fun getSession(): SSLSession = Proxy.newProxyInstance(SSLSession::class.java.classLoader, arrayOf(SSLSession::class.java)) { _, _, _ -> null } as SSLSession + override fun addHandshakeCompletedListener(listener: HandshakeCompletedListener?) = Unit + override fun removeHandshakeCompletedListener(listener: HandshakeCompletedListener?) = Unit + override fun startHandshake() = Unit + override fun setUseClientMode(mode: Boolean) = Unit + override fun getUseClientMode(): Boolean = true + override fun setNeedClientAuth(need: Boolean) = Unit + override fun getNeedClientAuth(): Boolean = false + override fun setWantClientAuth(want: Boolean) = Unit + override fun getWantClientAuth(): Boolean = false + override fun setEnableSessionCreation(flag: Boolean) = Unit + override fun getEnableSessionCreation(): Boolean = true } } diff --git a/app/src/test/java/com/flowpilot/app/actions/WebhookPrivacyRegressionTest.kt b/app/src/test/java/com/flowpilot/app/actions/WebhookPrivacyRegressionTest.kt index d9e454f..9fda66a 100644 --- a/app/src/test/java/com/flowpilot/app/actions/WebhookPrivacyRegressionTest.kt +++ b/app/src/test/java/com/flowpilot/app/actions/WebhookPrivacyRegressionTest.kt @@ -8,7 +8,6 @@ import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config import org.robolectric.shadows.ShadowLog import java.io.IOException -import java.net.HttpURLConnection import java.net.InetAddress @RunWith(RobolectricTestRunner::class) @@ -18,11 +17,9 @@ class WebhookPrivacyRegressionTest { fun dispatchAndResultLogsContainOnlyMethodAndStatus() { ShadowLog.clear() val executor = WebhookExecutor( - connectionFactory = { url, _ -> object : HttpURLConnection(url) { - override fun connect() = Unit - override fun disconnect() = Unit - override fun usingProxy() = false - override fun getResponseCode() = 204 + transportFactory = { _, _ -> object : WebhookTransport { + override fun execute(method: String, headers: Map, body: ByteArray, timeoutMs: Int) = 204 + override fun close() = Unit } }, addressLookup = { arrayOf(InetAddress.getByName("93.184.216.34")) }, ) @@ -43,7 +40,7 @@ class WebhookPrivacyRegressionTest { ShadowLog.clear() val secret = "https://private-host.example/path-secret?bare-secret X-Custom: header-secret" val executor = WebhookExecutor( - connectionFactory = { _, _ -> throw IOException(secret) }, + transportFactory = { _, _ -> throw IOException(secret) }, addressLookup = { arrayOf(InetAddress.getByName("93.184.216.34")) }, ) val result = executor.execute(ActionType.HTTP_WEBHOOK, ActionParameters( From 2b32e79792815d49f696937736104523273ef3f2 Mon Sep 17 00:00:00 2001 From: Emirhan Date: Mon, 14 Sep 2026 17:59:54 +0300 Subject: [PATCH 3/7] fix: gate sensitive event execution --- .../flowpilot/app/engine/AutomationEngine.kt | 93 ++++++++++------ .../flowpilot/app/engine/AutomationService.kt | 52 +++++++++ .../app/engine/EventExecutionAuthorization.kt | 23 ++++ .../engine/FlowPilotNotificationListener.kt | 98 +++++++++++----- .../flowpilot/app/engine/SmsEventTracker.kt | 61 ++++++---- .../com/flowpilot/app/engine/SmsReceiver.kt | 34 +++--- .../app/engine/SensitiveEventQueueTest.kt | 105 ++++++++++++++++++ 7 files changed, 367 insertions(+), 99 deletions(-) create mode 100644 app/src/main/java/com/flowpilot/app/engine/EventExecutionAuthorization.kt create mode 100644 app/src/test/java/com/flowpilot/app/engine/SensitiveEventQueueTest.kt diff --git a/app/src/main/java/com/flowpilot/app/engine/AutomationEngine.kt b/app/src/main/java/com/flowpilot/app/engine/AutomationEngine.kt index 93875e5..557bd16 100644 --- a/app/src/main/java/com/flowpilot/app/engine/AutomationEngine.kt +++ b/app/src/main/java/com/flowpilot/app/engine/AutomationEngine.kt @@ -306,12 +306,18 @@ class AutomationEngine( private suspend fun pollNotificationEvents(liveState: LiveSystemState) { val events = FlowPilotNotificationListener.drainEvents() if (events.isEmpty()) return + val authorization = AutomationService.authorizeEventExecution(appContext) ?: return val rules = repository.automations.first() for (event in events) { val matches = RuleEvaluator.evaluateNotification(rules, event, liveState) if (matches.isNotEmpty()) { Log.i(TAG, "Executing notification rules for ${event.packageName} (${matches.size} rule(s))") - executeAll(matches, trigger = TriggerEvent.NOTIFICATION_RECEIVED, liveState = liveState) + executeAll( + matches, + trigger = TriggerEvent.NOTIFICATION_RECEIVED, + liveState = liveState, + eventAuthorization = authorization, + ) } } } @@ -450,6 +456,7 @@ class AutomationEngine( private suspend fun pollSmsEvents(liveState: LiveSystemState) { val events = SmsEventTracker.drainEvents() if (events.isEmpty()) return + val authorization = AutomationService.authorizeEventExecution(appContext) ?: return val rules = repository.automations.first() for (event in events) { val matches = RuleEvaluator.evaluateSms(rules, event, liveState) @@ -462,6 +469,7 @@ class AutomationEngine( liveState = liveState, smsSender = event.sender, smsBody = event.body, + eventAuthorization = authorization, ) } } @@ -496,6 +504,7 @@ class AutomationEngine( smsSender: String? = null, smsBody: String? = null, eventCoordinates: Pair? = null, + eventAuthorization: EventExecutionAuthorization.Token? = null, ) { val coords = resolveExecutionCoordinates( requiresLocation = rules.any { it.requiresLocation() }, @@ -535,42 +544,15 @@ class AutomationEngine( } currentCoroutineContext().ensureActive() - val result = dispatcher.execute( - action, - com.flowpilot.app.actions.ActionParameters( - notificationTitle = rule.notificationTitle, - notificationBody = rule.notificationBody, - vibrationPattern = rule.vibrationPattern, - vibrationDurationMs = rule.vibrationDurationMs, - vibrationAmplitude = rule.vibrationAmplitude, - mediaVolumePercent = rule.mediaVolumePercent, - soundPreset = rule.soundPreset, - soundUri = rule.soundUri, - soundDurationMs = rule.soundDurationMs, - launchPackage = rule.launchPackage, - url = rule.url, - ttsText = rule.ttsText, - ttsVoiceName = rule.ttsVoiceName, - ttsSpeechRate = rule.ttsSpeechRate, - ttsAudioFileName = rule.ttsAudioFileName, - alarmHour = rule.alarmHour, - alarmMinute = rule.alarmMinute, - alarmMessage = rule.alarmMessage, - timerDurationSeconds = rule.timerDurationSeconds, - timerMessage = rule.timerMessage, - webhookMethod = rule.webhookMethod, - webhookUrl = rule.webhookUrl, - webhookHeaders = rule.webhookHeaders, - webhookBody = rule.webhookBody, - webhookTimeoutSeconds = rule.webhookTimeoutSeconds, - webhookTemplateContext = templateContext, - phoneNumber = rule.phoneNumber, - screenBrightnessPercent = rule.screenBrightnessPercent, - forceStopPackage = rule.forceStopPackage, - smsRecipient = rule.smsRecipient, - smsMessage = rule.smsMessage, - ), - ) + val result = eventAuthorization?.let { authorization -> + AutomationService.executeIfEventAuthorized(authorization) { + dispatcher.execute(action, actionParameters(rule, templateContext)) + } + } ?: if (eventAuthorization == null) { + dispatcher.execute(action, actionParameters(rule, templateContext)) + } else { + return@withContext + } Log.i(TAG, "Rule action result: action=${action.name}, success=${result.success}") if (result.success) { anySuccess = true @@ -612,6 +594,43 @@ class AutomationEngine( } } + private fun actionParameters( + rule: com.flowpilot.app.data.model.Automation, + templateContext: com.flowpilot.app.actions.WebhookTemplateContext, + ) = com.flowpilot.app.actions.ActionParameters( + notificationTitle = rule.notificationTitle, + notificationBody = rule.notificationBody, + vibrationPattern = rule.vibrationPattern, + vibrationDurationMs = rule.vibrationDurationMs, + vibrationAmplitude = rule.vibrationAmplitude, + mediaVolumePercent = rule.mediaVolumePercent, + soundPreset = rule.soundPreset, + soundUri = rule.soundUri, + soundDurationMs = rule.soundDurationMs, + launchPackage = rule.launchPackage, + url = rule.url, + ttsText = rule.ttsText, + ttsVoiceName = rule.ttsVoiceName, + ttsSpeechRate = rule.ttsSpeechRate, + ttsAudioFileName = rule.ttsAudioFileName, + alarmHour = rule.alarmHour, + alarmMinute = rule.alarmMinute, + alarmMessage = rule.alarmMessage, + timerDurationSeconds = rule.timerDurationSeconds, + timerMessage = rule.timerMessage, + webhookMethod = rule.webhookMethod, + webhookUrl = rule.webhookUrl, + webhookHeaders = rule.webhookHeaders, + webhookBody = rule.webhookBody, + webhookTimeoutSeconds = rule.webhookTimeoutSeconds, + webhookTemplateContext = templateContext, + phoneNumber = rule.phoneNumber, + screenBrightnessPercent = rule.screenBrightnessPercent, + forceStopPackage = rule.forceStopPackage, + smsRecipient = rule.smsRecipient, + smsMessage = rule.smsMessage, + ) + private companion object { val engineLifetime = Mutex() const val TAG = "FlowPilotEngine" diff --git a/app/src/main/java/com/flowpilot/app/engine/AutomationService.kt b/app/src/main/java/com/flowpilot/app/engine/AutomationService.kt index aa971aa..a148146 100644 --- a/app/src/main/java/com/flowpilot/app/engine/AutomationService.kt +++ b/app/src/main/java/com/flowpilot/app/engine/AutomationService.kt @@ -183,6 +183,7 @@ class AutomationService : Service() { companion object { private val controlMutex = Mutex() + private val eventExecutionAuthorization = EventExecutionAuthorization() private var activeService: AutomationService? = null private val mutableRunning = MutableStateFlow(false) val running = mutableRunning.asStateFlow() @@ -211,6 +212,50 @@ class AutomationService : Service() { suspend fun reconcileEnabled(context: Context) = control(context) { isEngineEnabled.first() } + /** Authorizes a just-drained transient event batch against current persisted engine state. */ + internal suspend fun authorizeEventExecution(context: Context): EventExecutionAuthorization.Token? = controlMutex.withLock { + eventExecutionAuthorization.authorize( + AutomationRepository(context.applicationContext).isEngineEnabled.first(), + ) + } + + /** + * Serializes each sensitive event action with disabling. The action begins while this lock is held, + * so disable cannot complete between authorization and dispatch. + */ + internal suspend fun executeIfEventAuthorized( + authorization: EventExecutionAuthorization.Token, + execute: () -> T, + ): T? = controlMutex.withLock { + eventExecutionAuthorization.executeIfAuthorized(authorization, execute) + } + + /** Serializes event intake with enabled-state changes, preventing disabled-state queue replay. */ + suspend fun enqueueSmsIfEngineEnabled( + context: Context, + sender: String, + body: String, + timestamp: Long, + ): Boolean = enqueueIfEngineEnabled(context) { + SmsEventTracker.enqueueIfEnabled(true, sender, body, timestamp) + } + + /** Serializes event intake with enabled-state changes, preventing disabled-state queue replay. */ + suspend fun enqueueNotificationIfEngineEnabled( + context: Context, + event: TransientNotificationEvent, + ): Boolean = enqueueIfEngineEnabled(context) { + FlowPilotNotificationListener.enqueueIfEnabled(true, event) + } + + private suspend fun enqueueIfEngineEnabled( + context: Context, + enqueue: () -> Boolean, + ): Boolean = controlMutex.withLock { + if (!AutomationRepository(context.applicationContext).isEngineEnabled.first()) return@withLock false + enqueue() + } + // Never acquire this lock from engineLifetime: controls only enqueue Android commands. private suspend fun control( context: Context, @@ -220,6 +265,8 @@ class AutomationService : Service() { try { val enabled = AutomationRepository(context.applicationContext).preference() if (enabled) start(context) else { + eventExecutionAuthorization.invalidate() + clearTransientEventState() stop(context) clearFailure(context) com.flowpilot.app.widget.FlowPilotWidgetProvider.updateAllWidgets(context) @@ -314,5 +361,10 @@ class AutomationService : Service() { private fun stop(context: Context) { context.stopService(Intent(context, AutomationService::class.java)) } + + private fun clearTransientEventState() { + SmsEventTracker.clear() + FlowPilotNotificationListener.clearTransientState() + } } } diff --git a/app/src/main/java/com/flowpilot/app/engine/EventExecutionAuthorization.kt b/app/src/main/java/com/flowpilot/app/engine/EventExecutionAuthorization.kt new file mode 100644 index 0000000..66a6375 --- /dev/null +++ b/app/src/main/java/com/flowpilot/app/engine/EventExecutionAuthorization.kt @@ -0,0 +1,23 @@ +package com.flowpilot.app.engine + +/** Process-local generation gate for transient events accepted while engine is enabled. */ +internal class EventExecutionAuthorization { + private var generation = 0L + + @Synchronized + fun authorize(engineEnabled: Boolean): Token? = if (engineEnabled) Token(generation) else null + + @Synchronized + fun invalidate() { + generation += 1 + } + + @Synchronized + fun isAuthorized(token: Token): Boolean = token.generation == generation + + @Synchronized + fun executeIfAuthorized(token: Token, execute: () -> T): T? = + if (token.generation == generation) execute() else null + + internal data class Token internal constructor(private val generation: Long) +} diff --git a/app/src/main/java/com/flowpilot/app/engine/FlowPilotNotificationListener.kt b/app/src/main/java/com/flowpilot/app/engine/FlowPilotNotificationListener.kt index 2d25e92..43505f5 100644 --- a/app/src/main/java/com/flowpilot/app/engine/FlowPilotNotificationListener.kt +++ b/app/src/main/java/com/flowpilot/app/engine/FlowPilotNotificationListener.kt @@ -2,13 +2,15 @@ package com.flowpilot.app.engine import android.service.notification.NotificationListenerService import android.service.notification.StatusBarNotification -import com.flowpilot.app.data.AutomationRepository +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.flow.first +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancel +import kotlinx.coroutines.cancelChildren import kotlinx.coroutines.launch -import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.ConcurrentLinkedQueue +import java.util.ArrayDeque +import java.util.LinkedHashMap /** * Transient notification event abstraction. Never persists raw text/title/body. @@ -25,34 +27,41 @@ data class TransientNotificationEvent( * Pure deduplication helper for notifications to ensure unit-testability without Android framework classes. */ class NotificationDeduplicator(private val ttlMs: Long = 60_000L, private val maxEntries: Int = 200) { - private val recentKeys = ConcurrentHashMap() + private val recentKeys = LinkedHashMap(maxEntries) + @Synchronized fun shouldProcess(key: String, postTime: Long, currentTime: Long = System.currentTimeMillis()): Boolean { + trim(currentTime) val lastSeen = recentKeys[key] - if (lastSeen != null && postTime <= lastSeen) { + if (lastSeen != null && postTime <= lastSeen.postTime) { return false } - recentKeys[key] = postTime - trim(currentTime) + recentKeys[key] = Entry(postTime = postTime, seenAt = currentTime) + while (recentKeys.size > maxEntries) { + recentKeys.entries.iterator().apply { + next() + remove() + } + } return true } + @Synchronized fun clear() { recentKeys.clear() } private fun trim(currentTime: Long) { - if (recentKeys.size > maxEntries) { - val oldestAllowed = currentTime - ttlMs - val iterator = recentKeys.entries.iterator() - while (iterator.hasNext()) { - val entry = iterator.next() - if (entry.value < oldestAllowed) { - iterator.remove() - } + val oldestAllowed = currentTime - ttlMs + val iterator = recentKeys.entries.iterator() + while (iterator.hasNext()) { + if (iterator.next().value.seenAt < oldestAllowed) { + iterator.remove() } } } + + private data class Entry(val postTime: Long, val seenAt: Long) } /** @@ -62,9 +71,11 @@ class NotificationDeduplicator(private val ttlMs: Long = 60_000L, private val ma class FlowPilotNotificationListener : NotificationListenerService() { private var lastWatchdogCheckMs = 0L + private val intakeScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) override fun onNotificationPosted(sbn: StatusBarNotification?) { checkWatchdog() + if (!isConnected) return sbn ?: return val pkg = sbn.packageName ?: return // Ignore own notifications to prevent loops @@ -73,10 +84,6 @@ class FlowPilotNotificationListener : NotificationListenerService() { val key = sbn.key ?: "${pkg}_${sbn.id}_${sbn.postTime}" val postTime = sbn.postTime - if (!deduplicator.shouldProcess(key, postTime)) { - return - } - val extras = sbn.notification?.extras val title = extras?.getCharSequence(android.app.Notification.EXTRA_TITLE)?.toString().orEmpty() val text = extras?.getCharSequence(android.app.Notification.EXTRA_TEXT)?.toString().orEmpty() @@ -91,7 +98,16 @@ class FlowPilotNotificationListener : NotificationListenerService() { text = combinedText, ) - eventQueue.add(event) + intakeScope.launch { + try { + if (AutomationService.enqueueNotificationIfEngineEnabled(applicationContext, event)) { + AutomationService.reconcileEnabled(applicationContext) + } + } catch (e: CancellationException) { + throw e + } catch (_: Throwable) { + } + } } override fun onListenerConnected() { @@ -103,15 +119,23 @@ class FlowPilotNotificationListener : NotificationListenerService() { override fun onListenerDisconnected() { super.onListenerDisconnected() isConnected = false + intakeScope.coroutineContext.cancelChildren() + } + + override fun onDestroy() { + intakeScope.cancel() + super.onDestroy() } private fun checkWatchdog(force: Boolean = false) { val now = System.currentTimeMillis() if (force || now - lastWatchdogCheckMs > 60_000L) { lastWatchdogCheckMs = now - CoroutineScope(Dispatchers.IO).launch { + intakeScope.launch { try { AutomationService.reconcileEnabled(applicationContext) + } catch (e: CancellationException) { + throw e } catch (_: Throwable) {} } } @@ -122,11 +146,35 @@ class FlowPilotNotificationListener : NotificationListenerService() { var isConnected: Boolean = false private set - private val eventQueue = ConcurrentLinkedQueue() + internal const val MAX_EVENT_AGE_MS = 2 * 60 * 1000L + internal const val MAX_QUEUE_SIZE = 100 + + private val queueLock = Any() + private val eventQueue = ArrayDeque(MAX_QUEUE_SIZE) val deduplicator = NotificationDeduplicator() - fun drainEvents(): List = buildList { - while (true) add(eventQueue.poll() ?: break) + fun enqueueIfEnabled(engineEnabled: Boolean, event: TransientNotificationEvent): Boolean = synchronized(queueLock) { + if (!engineEnabled || eventQueue.size >= MAX_QUEUE_SIZE || !deduplicator.shouldProcess(event.key, event.postTime)) { + return false + } + eventQueue.addLast(event) + true + } + + fun drainEvents(currentTime: Long = System.currentTimeMillis()): List = synchronized(queueLock) { + buildList { + while (eventQueue.isNotEmpty()) { + val event = eventQueue.removeFirst() + if (event.postTime in (currentTime - MAX_EVENT_AGE_MS)..currentTime) { + add(event) + } + } + } + } + + fun clearTransientState() = synchronized(queueLock) { + eventQueue.clear() + deduplicator.clear() } } } diff --git a/app/src/main/java/com/flowpilot/app/engine/SmsEventTracker.kt b/app/src/main/java/com/flowpilot/app/engine/SmsEventTracker.kt index c5f71bb..792f179 100644 --- a/app/src/main/java/com/flowpilot/app/engine/SmsEventTracker.kt +++ b/app/src/main/java/com/flowpilot/app/engine/SmsEventTracker.kt @@ -1,7 +1,7 @@ package com.flowpilot.app.engine -import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.ConcurrentLinkedQueue +import java.util.ArrayDeque +import java.util.LinkedHashMap /** * Event generated when an incoming SMS message is received and reassembled. @@ -17,52 +17,69 @@ data class SmsReceivedEvent( */ object SmsEventTracker { - private val eventQueue = ConcurrentLinkedQueue() - private val recentDedupeKeys = ConcurrentHashMap() + internal const val MAX_EVENT_AGE_MS = 2 * 60 * 1000L + internal const val MAX_QUEUE_SIZE = 100 + internal const val MAX_DEDUPE_ENTRIES = 200 private const val DEDUPE_TTL_MS = 60_000L - private const val MAX_DEDUPE_ENTRIES = 200 + + private val lock = Any() + private val eventQueue = ArrayDeque(MAX_QUEUE_SIZE) + private val recentDedupeKeys = LinkedHashMap(MAX_DEDUPE_ENTRIES) /** - * Enqueues an SMS received event if it has not been processed within the deduplication window. + * Accepts an SMS only after AutomationService has checked persisted engine state under its control lock. */ - fun enqueue(sender: String, body: String, timestamp: Long = System.currentTimeMillis()): Boolean { + fun enqueueIfEnabled( + engineEnabled: Boolean, + sender: String, + body: String, + timestamp: Long = System.currentTimeMillis(), + ): Boolean = synchronized(lock) { + if (!engineEnabled || eventQueue.size >= MAX_QUEUE_SIZE) return false val dedupeKey = "${PhoneNumberUtils.normalize(sender)}_${body.hashCode()}_${timestamp / 5000}" val now = System.currentTimeMillis() + trimOldKeys(now) val lastSeen = recentDedupeKeys[dedupeKey] if (lastSeen != null && now - lastSeen < DEDUPE_TTL_MS) { return false } recentDedupeKeys[dedupeKey] = now - trimOldKeys(now) - - eventQueue.add(SmsReceivedEvent(sender = sender, body = body, timestamp = timestamp)) + while (recentDedupeKeys.size > MAX_DEDUPE_ENTRIES) { + recentDedupeKeys.entries.iterator().apply { + next() + remove() + } + } + eventQueue.addLast(SmsReceivedEvent(sender = sender, body = body, timestamp = timestamp)) return true } /** * Drains all pending SMS received events in FIFO order. */ - fun drainEvents(): List = buildList { - while (true) { - add(eventQueue.poll() ?: break) + fun drainEvents(currentTime: Long = System.currentTimeMillis()): List = synchronized(lock) { + buildList { + while (eventQueue.isNotEmpty()) { + val event = eventQueue.removeFirst() + if (event.timestamp in (currentTime - MAX_EVENT_AGE_MS)..currentTime) { + add(event) + } + } } } - fun clear() { + fun clear() = synchronized(lock) { eventQueue.clear() recentDedupeKeys.clear() } private fun trimOldKeys(currentTime: Long) { - if (recentDedupeKeys.size > MAX_DEDUPE_ENTRIES) { - val oldestAllowed = currentTime - DEDUPE_TTL_MS - val it = recentDedupeKeys.entries.iterator() - while (it.hasNext()) { - val entry = it.next() - if (entry.value < oldestAllowed) { - it.remove() - } + val oldestAllowed = currentTime - DEDUPE_TTL_MS + val it = recentDedupeKeys.entries.iterator() + while (it.hasNext()) { + if (it.next().value < oldestAllowed) { + it.remove() } } } diff --git a/app/src/main/java/com/flowpilot/app/engine/SmsReceiver.kt b/app/src/main/java/com/flowpilot/app/engine/SmsReceiver.kt index 1cfbc1d..3c7aedf 100644 --- a/app/src/main/java/com/flowpilot/app/engine/SmsReceiver.kt +++ b/app/src/main/java/com/flowpilot/app/engine/SmsReceiver.kt @@ -5,10 +5,8 @@ import android.content.Context import android.content.Intent import android.provider.Telephony import android.util.Log -import com.flowpilot.app.data.AutomationRepository import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch /** @@ -39,11 +37,25 @@ class SmsReceiver : BroadcastReceiver() { val timestamp = if (firstMessage.timestampMillis > 0L) firstMessage.timestampMillis else System.currentTimeMillis() if (sender.isNotBlank() || fullBody.isNotBlank()) { - val enqueued = SmsEventTracker.enqueue(sender = sender, body = fullBody, timestamp = timestamp) - if (enqueued) { - val masked = PhoneNumberUtils.mask(sender) - Log.i(TAG, "Incoming SMS queued from $masked (len=${fullBody.length})") - ensureEngineRunning(context.applicationContext) + val pendingResult = goAsync() + CoroutineScope(Dispatchers.IO).launch { + try { + val enqueued = AutomationService.enqueueSmsIfEngineEnabled( + context = context.applicationContext, + sender = sender, + body = fullBody, + timestamp = timestamp, + ) + if (enqueued) { + val masked = PhoneNumberUtils.mask(sender) + Log.i(TAG, "Incoming SMS queued from $masked (len=${fullBody.length})") + AutomationService.reconcileEnabled(context.applicationContext) + } + } catch (e: Exception) { + Log.w(TAG, "Error queuing incoming SMS: ${e.javaClass.simpleName}") + } finally { + pendingResult.finish() + } } } } catch (e: Exception) { @@ -51,14 +63,6 @@ class SmsReceiver : BroadcastReceiver() { } } - private fun ensureEngineRunning(appContext: Context) { - CoroutineScope(Dispatchers.IO).launch { - try { - AutomationService.reconcileEnabled(appContext) - } catch (_: Throwable) {} - } - } - companion object { private const val TAG = "SmsReceiver" } diff --git a/app/src/test/java/com/flowpilot/app/engine/SensitiveEventQueueTest.kt b/app/src/test/java/com/flowpilot/app/engine/SensitiveEventQueueTest.kt new file mode 100644 index 0000000..6fe68c4 --- /dev/null +++ b/app/src/test/java/com/flowpilot/app/engine/SensitiveEventQueueTest.kt @@ -0,0 +1,105 @@ +package com.flowpilot.app.engine + +import com.google.common.truth.Truth.assertThat +import org.junit.After +import org.junit.Test + +class SensitiveEventQueueTest { + private val now = 1_000_000L + + @After + fun clearTransientState() { + SmsEventTracker.clear() + FlowPilotNotificationListener.clearTransientState() + } + + @Test + fun smsRejectsDisabledIntakeAndStaleEventsAtDrain() { + assertThat(SmsEventTracker.enqueueIfEnabled(false, "one", "body", now)).isFalse() + assertThat(SmsEventTracker.enqueueIfEnabled(true, "one", "body", now - SmsEventTracker.MAX_EVENT_AGE_MS - 1)).isTrue() + + assertThat(SmsEventTracker.drainEvents(now)).isEmpty() + } + + @Test + fun smsQueueIsBoundedFifoAndDedupeRetentionIsBounded() { + repeat(SmsEventTracker.MAX_QUEUE_SIZE + 1) { index -> + assertThat(SmsEventTracker.enqueueIfEnabled(true, "sender-$index", "body", now)).isEqualTo(index < SmsEventTracker.MAX_QUEUE_SIZE) + } + assertThat(SmsEventTracker.drainEvents(now).map { it.sender }) + .containsExactlyElementsIn((0 until SmsEventTracker.MAX_QUEUE_SIZE).map { "sender-$it" }).inOrder() + + SmsEventTracker.clear() + assertThat(SmsEventTracker.enqueueIfEnabled(true, "oldest", "body", now)).isTrue() + SmsEventTracker.drainEvents(now) + repeat(SmsEventTracker.MAX_DEDUPE_ENTRIES) { index -> + assertThat(SmsEventTracker.enqueueIfEnabled(true, "dedupe-$index", "body", now)).isTrue() + SmsEventTracker.drainEvents(now) + } + assertThat(SmsEventTracker.enqueueIfEnabled(true, "oldest", "body", now)).isTrue() + } + + @Test + fun smsClearResetsQueueAndDedupe() { + assertThat(SmsEventTracker.enqueueIfEnabled(true, "sender", "body", now)).isTrue() + SmsEventTracker.clear() + + assertThat(SmsEventTracker.drainEvents(now)).isEmpty() + assertThat(SmsEventTracker.enqueueIfEnabled(true, "sender", "body", now)).isTrue() + } + + @Test + fun notificationRejectsDisabledIntakeAndStaleEventsAtDrain() { + val event = notification("disabled", now) + assertThat(FlowPilotNotificationListener.enqueueIfEnabled(false, event)).isFalse() + assertThat(FlowPilotNotificationListener.enqueueIfEnabled(true, notification("stale", now - FlowPilotNotificationListener.MAX_EVENT_AGE_MS - 1))).isTrue() + + assertThat(FlowPilotNotificationListener.drainEvents(now)).isEmpty() + } + + @Test + fun notificationQueueIsBoundedFifoAndClearResetsDedupe() { + repeat(FlowPilotNotificationListener.MAX_QUEUE_SIZE + 1) { index -> + assertThat(FlowPilotNotificationListener.enqueueIfEnabled(true, notification("$index", now))).isEqualTo(index < FlowPilotNotificationListener.MAX_QUEUE_SIZE) + } + assertThat(FlowPilotNotificationListener.drainEvents(now).map { it.key }) + .containsExactlyElementsIn((0 until FlowPilotNotificationListener.MAX_QUEUE_SIZE).map { "key-$it" }).inOrder() + + val replay = notification("replay", now) + assertThat(FlowPilotNotificationListener.enqueueIfEnabled(true, replay)).isTrue() + FlowPilotNotificationListener.clearTransientState() + assertThat(FlowPilotNotificationListener.enqueueIfEnabled(true, replay)).isTrue() + } + + @Test + fun notificationDedupeRetentionIsBounded() { + val dedupe = NotificationDeduplicator(ttlMs = Long.MAX_VALUE, maxEntries = 2) + + assertThat(dedupe.shouldProcess("first", 1, now)).isTrue() + assertThat(dedupe.shouldProcess("second", 1, now)).isTrue() + assertThat(dedupe.shouldProcess("third", 1, now)).isTrue() + assertThat(dedupe.shouldProcess("first", 1, now)).isTrue() + } + + @Test + fun executionAuthorizationRejectsDrainedBatchAfterDisable() { + val authorization = EventExecutionAuthorization() + val token = authorization.authorize(engineEnabled = true) + + assertThat(token).isNotNull() + assertThat(authorization.isAuthorized(token!!)).isTrue() + assertThat(authorization.executeIfAuthorized(token) { "ran" }).isEqualTo("ran") + authorization.invalidate() + assertThat(authorization.isAuthorized(token)).isFalse() + assertThat(authorization.executeIfAuthorized(token) { "ran" }).isNull() + assertThat(authorization.authorize(engineEnabled = false)).isNull() + } + + private fun notification(id: String, postTime: Long) = TransientNotificationEvent( + packageName = "com.example.$id", + postTime = postTime, + key = "key-$id", + title = "title", + text = "text", + ) +} From d187d0ae975e25f0d4aa34d768f649c1f67aafb2 Mon Sep 17 00:00:00 2001 From: Emirhan Date: Mon, 14 Sep 2026 17:59:55 +0300 Subject: [PATCH 4/7] fix: reserve and revoke queued rule execution --- .../app/data/AutomationRepository.kt | 208 ++++++++++++++++- .../flowpilot/app/data/model/Automation.kt | 5 + .../flowpilot/app/engine/AutomationEngine.kt | 210 ++++++++++++------ .../data/AutomationRepositoryCryptoTest.kt | 6 +- ...ationRepositoryExecutionReservationTest.kt | 203 +++++++++++++++++ 5 files changed, 548 insertions(+), 84 deletions(-) create mode 100644 app/src/test/java/com/flowpilot/app/data/AutomationRepositoryExecutionReservationTest.kt diff --git a/app/src/main/java/com/flowpilot/app/data/AutomationRepository.kt b/app/src/main/java/com/flowpilot/app/data/AutomationRepository.kt index 751d9be..f3a3dbe 100644 --- a/app/src/main/java/com/flowpilot/app/data/AutomationRepository.kt +++ b/app/src/main/java/com/flowpilot/app/data/AutomationRepository.kt @@ -5,6 +5,7 @@ import androidx.datastore.core.DataStore import androidx.datastore.preferences.core.Preferences import androidx.datastore.preferences.core.MutablePreferences import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.longPreferencesKey import androidx.datastore.preferences.core.stringPreferencesKey import androidx.datastore.preferences.preferencesDataStore import com.flowpilot.app.data.model.Automation @@ -20,6 +21,8 @@ import java.util.UUID import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.serialization.builtins.ListSerializer import kotlinx.serialization.json.Json @@ -28,6 +31,13 @@ private val Context.dataStore: DataStore by preferencesDataStore(na /** Persists automation rules as JSON in a single DataStore preferences key. */ class AutomationRepository(private val context: Context) { + data class ExecutionReservation( + val rule: Automation, + val revision: Long, + val token: String, + val previousLastTriggeredAt: Long, + ) + internal val rawDataStore: DataStore get() = context.dataStore @@ -41,6 +51,7 @@ class AutomationRepository(private val context: Context) { private val geofenceDiagnosticListSerializer = ListSerializer(GeofenceDiagnostic.serializer()) private val key = stringPreferencesKey("rules") + private val executionRevisionKey = longPreferencesKey("execution_revision") private val historyKey = stringPreferencesKey("execution_history") private val geofenceQueueKey = stringPreferencesKey("geofence_event_queue") private val geofenceDiagnosticsKey = stringPreferencesKey("geofence_diagnostics") @@ -339,7 +350,7 @@ class AutomationRepository(private val context: Context) { geofenceName = geofenceName, geofenceRadiusMeters = geofenceRadiusMeters, ) - val rule = Automation( + var rule = Automation( id = id, name = name.ifBlank { automaticName }, triggerEvent = triggerEvent, @@ -404,13 +415,17 @@ class AutomationRepository(private val context: Context) { flipScreenOffDetection = flipScreenOffDetection, createdAt = System.currentTimeMillis(), ) + executionStateMutex.withLock { context.dataStore.edit { prefs -> migrateHistory(prefs) val current = prefs[key]?.let { safeDecode(it) } ?: emptyList() + rule = rule.copy(executionRevision = nextExecutionRevision(prefs)) val encryptedRule = rule.withEncryptedSecrets() + check(current.none { it.id == rule.id }) { "Duplicate automation ID" } val updated = current.map { it.withEncryptedSecrets() } + encryptedRule prefs[key] = json.encodeToString(listSerializer, updated) } + } cleanupOrphanTtsFiles() notifyWidgets() return rule @@ -426,6 +441,7 @@ class AutomationRepository(private val context: Context) { var clone: Automation? = null var createdCloneTtsFile: java.io.File? = null try { + executionStateMutex.withLock { context.dataStore.edit { prefs -> migrateHistory(prefs) val current = prefs[key]?.let { safeDecode(it) } ?: return@edit @@ -454,18 +470,21 @@ class AutomationRepository(private val context: Context) { } }.orEmpty() clone = source.copy( - id = newId, + id = newId, name = copyName, enabled = false, ttsAudioFileName = cloneTtsFileName, createdAt = createdAt, lastTriggeredAt = 0L, - ) + executionRevision = nextExecutionRevision(prefs), + ) + check(current.none { it.id == newId }) { "Duplicate automation ID" } prefs[key] = json.encodeToString( listSerializer, current.map { it.withEncryptedSecrets() } + clone!!.withEncryptedSecrets(), ) } + } } catch (error: Throwable) { createdCloneTtsFile?.delete() throw error @@ -478,15 +497,22 @@ class AutomationRepository(private val context: Context) { } suspend fun update(rule: Automation) { + executionStateMutex.withLock { context.dataStore.edit { prefs -> migrateHistory(prefs) val current = prefs[key]?.let { safeDecode(it) } ?: emptyList() - val encryptedRule = rule.copy(name = rule.normalizedName).withEncryptedSecrets() + val revision = nextExecutionRevision(prefs) val updated = current.map { - if (it.id == rule.id) encryptedRule else it.withEncryptedSecrets() + if (it.id == rule.id) { + revokePendingExecution(rule.copy( + name = rule.normalizedName, + executionRevision = revision, + )).withEncryptedSecrets() + } else it.withEncryptedSecrets() } prefs[key] = json.encodeToString(listSerializer, updated) } + } cleanupOrphanTtsFiles() notifyWidgets() } @@ -503,38 +529,147 @@ class AutomationRepository(private val context: Context) { } } + /** Atomically claims an automatic run before any action side effect. */ + suspend fun reserveExecution( + id: String, + expectedRevision: Long, + at: Long = System.currentTimeMillis(), + ): ExecutionReservation? { + var reservation: ExecutionReservation? = null + executionStateMutex.withLock { + context.dataStore.edit { prefs -> + val current = prefs[key]?.let { safeDecode(it) } ?: return@edit + val index = current.indexOfFirst { it.id == id } + if (index < 0) return@edit + val stored = current[index] + val rule = stored.withDecryptedSecrets() + if (!rule.enabled || rule.executionRevision != expectedRevision || + rule.isCoolingDown(at) || rule.executionLeaseExpiresAt > at + ) { + return@edit + } + val token = UUID.randomUUID().toString() + val updated = current.toMutableList() + updated[index] = stored.copy( + lastTriggeredAt = at, + executionLeaseToken = token, + executionLeaseExpiresAt = at + EXECUTION_LEASE_MS, + ) + prefs[key] = json.encodeToString(listSerializer, updated) + reservation = ExecutionReservation( + rule = rule.copy( + lastTriggeredAt = at, + executionLeaseToken = token, + executionLeaseExpiresAt = at + EXECUTION_LEASE_MS, + ), + revision = rule.executionRevision, + token = token, + previousLastTriggeredAt = rule.lastTriggeredAt, + ) + } + } + return reservation + } + + /** Releases a reservation only when its exact durable lease token still owns the rule. */ + suspend fun releaseExecutionReservation(reservation: ExecutionReservation, successful: Boolean) { + executionStateMutex.withLock { + context.dataStore.edit { prefs -> + val current = prefs[key]?.let { safeDecode(it) } ?: return@edit + val updated = current.map { stored -> + if (stored.id == reservation.rule.id && + stored.executionRevision == reservation.revision && + stored.executionLeaseToken == reservation.token + ) { + stored.copy( + lastTriggeredAt = if (successful) stored.lastTriggeredAt else reservation.previousLastTriggeredAt, + executionLeaseToken = "", + executionLeaseExpiresAt = 0L, + ) + } else stored + } + prefs[key] = json.encodeToString(listSerializer, updated) + } + } + } + + /** Extends a live lease before a bounded action delay; stale owners cannot renew. */ + suspend fun renewExecutionReservation( + reservation: ExecutionReservation, + at: Long = System.currentTimeMillis(), + ): Boolean { + var renewed = false + executionStateMutex.withLock { + context.dataStore.edit { prefs -> + val current = prefs[key]?.let { safeDecode(it) } ?: return@edit + val updated = current.map { stored -> + if (stored.id == reservation.rule.id && + stored.enabled && + stored.executionRevision == reservation.revision && + stored.executionLeaseToken == reservation.token + ) { + renewed = true + stored.copy(executionLeaseExpiresAt = at + EXECUTION_LEASE_MS) + } else stored + } + prefs[key] = json.encodeToString(listSerializer, updated) + } + } + return renewed + } + + /** Valid only while rule still exists, is enabled, and has unchanged execution config. */ + suspend fun isExecutionAuthorized(id: String, revision: Long): Boolean = + rawDataStore.data.first()[key] + ?.let { safeDecode(it) } + .orEmpty() + .firstOrNull { it.id == id } + ?.let { it.enabled && it.executionRevision == revision } + ?: false + suspend fun setEnabled(id: String, enabled: Boolean) { + executionStateMutex.withLock { context.dataStore.edit { prefs -> migrateHistory(prefs) val current = prefs[key]?.let { safeDecode(it) } ?: return@edit + val revision = if (current.any { it.id == id && it.enabled != enabled }) { + nextExecutionRevision(prefs) + } else null val updated = current.map { - val base = if (it.id == id) it.copy(enabled = enabled) else it + val base = if (it.id == id && it.enabled != enabled) { + revokePendingExecution(it.copy(enabled = enabled, executionRevision = checkNotNull(revision))) + } else it base.withEncryptedSecrets() } prefs[key] = json.encodeToString(listSerializer, updated) } + } notifyWidgets() } suspend fun delete(id: String) { + executionStateMutex.withLock { context.dataStore.edit { prefs -> migrateHistory(prefs) val current = prefs[key]?.let { safeDecode(it) } ?: return@edit val updated = current.filterNot { it.id == id }.map { it.withEncryptedSecrets() } prefs[key] = json.encodeToString(listSerializer, updated) } + } cleanupOrphanTtsFiles() notifyWidgets() } suspend fun deleteMany(ids: Set) { if (ids.isEmpty()) return + executionStateMutex.withLock { context.dataStore.edit { prefs -> migrateHistory(prefs) val current = prefs[key]?.let { safeDecode(it) } ?: return@edit val updated = current.filterNot { it.id in ids }.map { it.withEncryptedSecrets() } prefs[key] = json.encodeToString(listSerializer, updated) } + } cleanupOrphanTtsFiles() notifyWidgets() } @@ -544,6 +679,7 @@ class AutomationRepository(private val context: Context) { strategy: com.flowpilot.app.data.backup.ImportStrategy, ): Int { if (imported.isEmpty()) return 0 + executionStateMutex.withLock { context.dataStore.edit { prefs -> migrateHistory(prefs) val current = prefs[key]?.let { safeDecode(it) } ?: emptyList() @@ -554,27 +690,44 @@ class AutomationRepository(private val context: Context) { id = UUID.randomUUID().toString(), createdAt = System.currentTimeMillis(), name = rule.normalizedName, + executionRevision = nextExecutionRevision(prefs), ).withEncryptedSecrets() } + requireUniqueIds(current + remapped) current.map { it.withEncryptedSecrets() } + remapped } com.flowpilot.app.data.backup.ImportStrategy.REPLACE_ALL -> { - imported.map { it.copy(name = it.normalizedName).withEncryptedSecrets() } + requireUniqueIds(imported) + imported.map { rule -> + revokePendingExecution(rule.copy( + name = rule.normalizedName, + executionRevision = nextExecutionRevision(prefs), + )).withEncryptedSecrets() + } } } prefs[key] = json.encodeToString(listSerializer, finalRules) } + } cleanupOrphanTtsFiles() notifyWidgets() return imported.size } suspend fun replaceAll(rules: List) { + requireUniqueIds(rules) + executionStateMutex.withLock { context.dataStore.edit { prefs -> migrateHistory(prefs) - val encrypted = rules.map { it.copy(name = it.normalizedName).withEncryptedSecrets() } + val encrypted = rules.map { rule -> + revokePendingExecution(rule.copy( + name = rule.normalizedName, + executionRevision = nextExecutionRevision(prefs), + )).withEncryptedSecrets() + } prefs[key] = json.encodeToString(listSerializer, encrypted) } + } cleanupOrphanTtsFiles() notifyWidgets() } @@ -586,11 +739,46 @@ class AutomationRepository(private val context: Context) { } private fun safeDecode(raw: String): List = try { - json.decodeFromString(listSerializer, raw) + recoverDuplicateIds(json.decodeFromString(listSerializer, raw)) } catch (_: Exception) { emptyList() } + private fun nextExecutionRevision(prefs: MutablePreferences): Long { + val next = (prefs[executionRevisionKey] ?: 0L) + 1L + prefs[executionRevisionKey] = next + return next + } + + private fun revokePendingExecution(rule: Automation): Automation = rule.copy( + executionLeaseToken = "", + executionLeaseExpiresAt = 0L, + ) + + private fun requireUniqueIds(rules: List) { + require(rules.map { it.id }.toSet().size == rules.size) { "Duplicate automation ID" } + } + + private fun recoverDuplicateIds(rules: List): List = + rules.distinctBy { it.id } + + /** Serializes in-process rule mutation with final authorization and dispatch. */ + suspend fun dispatchIfAuthorized( + reservation: ExecutionReservation, + block: suspend () -> T, + ): T? = executionStateMutex.withLock { + val stored = rawDataStore.data.first()[key] + ?.let(::safeDecode) + ?.firstOrNull { it.id == reservation.rule.id } + if (stored?.enabled == true && + stored.executionRevision == reservation.revision && + stored.executionLeaseToken == reservation.token && + stored.executionLeaseExpiresAt > System.currentTimeMillis() + ) { + block() + } else null + } + private fun migrateHistory(prefs: MutablePreferences) { val raw = prefs[historyKey] ?: return val history = safeDecodeHistory(raw) @@ -645,6 +833,8 @@ class AutomationRepository(private val context: Context) { const val MAX_HISTORY_ENTRIES = 100 const val GEOFENCE_RECEIVER_DIAGNOSTIC_ID = "__geofence_receiver__" private const val MAX_GEOFENCE_ERROR_LENGTH = 300 + private const val EXECUTION_LEASE_MS = 10 * 60_000L + private val executionStateMutex = Mutex() } suspend fun migrateLegacySecretsIfNeeded() { diff --git a/app/src/main/java/com/flowpilot/app/data/model/Automation.kt b/app/src/main/java/com/flowpilot/app/data/model/Automation.kt index 9b51937..625acc9 100644 --- a/app/src/main/java/com/flowpilot/app/data/model/Automation.kt +++ b/app/src/main/java/com/flowpilot/app/data/model/Automation.kt @@ -301,6 +301,11 @@ data class Automation( val actionDelays: List = emptyList(), /** Cooldown duration in minutes (0 means disabled, stored values clamp to 1440). Blocks automatic trigger evaluation when (now - lastTriggeredAt) < cooldown. */ val cooldownMinutes: Int = 0, + /** Increments for each saved rule change, invalidating pending executions from older snapshots. */ + val executionRevision: Long = 0L, + /** Durable automatic-execution lease; never contains action or webhook data. */ + val executionLeaseToken: String = "", + val executionLeaseExpiresAt: Long = 0L, /** Whether motion/flip triggers should listen and evaluate even when the device screen is off. */ val flipScreenOffDetection: Boolean = false, /** Threshold in lux for LIGHT_BELOW / LIGHT_ABOVE triggers. */ diff --git a/app/src/main/java/com/flowpilot/app/engine/AutomationEngine.kt b/app/src/main/java/com/flowpilot/app/engine/AutomationEngine.kt index 557bd16..4b295a6 100644 --- a/app/src/main/java/com/flowpilot/app/engine/AutomationEngine.kt +++ b/app/src/main/java/com/flowpilot/app/engine/AutomationEngine.kt @@ -506,90 +506,144 @@ class AutomationEngine( eventCoordinates: Pair? = null, eventAuthorization: EventExecutionAuthorization.Token? = null, ) { - val coords = resolveExecutionCoordinates( - requiresLocation = rules.any { it.requiresLocation() }, - eventCoordinates = eventCoordinates, - freshLocationProvider = { - LocationFetcher.getCoordinates(appContext, isBackgroundExecution = true) - }, - ) - val templateContext = com.flowpilot.app.actions.WebhookTemplateContext( - trigger = trigger?.name ?: "", - timestamp = System.currentTimeMillis(), - batteryPercent = liveState.batteryPercent, - isCharging = liveState.isChargerConnected, - wifiSsid = liveState.connectedWifiSsid, - smsSender = smsSender, - smsBody = smsBody, - locationLat = coords?.first, - locationLng = coords?.second, - ) - for (rule in rules) { - withContext(Dispatchers.IO) { + for (candidate in rules.distinctBy { it.id }) { + if (!reservePendingExecution(candidate.id)) continue + try { + val reservation = repository.reserveExecution( + id = candidate.id, + expectedRevision = candidate.executionRevision, + ) ?: continue + val rule = reservation.rule var anySuccess = false - val actionRecords = mutableListOf() - val actions = rule.effectiveActions - val delays = rule.effectiveActionDelays - var currentAction: com.flowpilot.app.data.model.ActionType? = null - + var cancelled = false + var actionRecords = mutableListOf() try { - for (i in actions.indices) { - currentCoroutineContext().ensureActive() - val action = actions[i] - currentAction = action - val delaySec = delays.getOrElse(i) { 0 } - - if (delaySec > 0) { - delay(delaySec * 1000L) - } - - currentCoroutineContext().ensureActive() - val result = eventAuthorization?.let { authorization -> - AutomationService.executeIfEventAuthorized(authorization) { - dispatcher.execute(action, actionParameters(rule, templateContext)) + val coords = resolveExecutionCoordinates( + requiresLocation = rule.requiresLocation(), + eventCoordinates = eventCoordinates, + freshLocationProvider = { + LocationFetcher.getCoordinates(appContext, isBackgroundExecution = true) + }, + ) + val templateContext = com.flowpilot.app.actions.WebhookTemplateContext( + trigger = trigger?.name ?: "", + timestamp = System.currentTimeMillis(), + batteryPercent = liveState.batteryPercent, + isCharging = liveState.isChargerConnected, + wifiSsid = liveState.connectedWifiSsid, + smsSender = smsSender, + smsBody = smsBody, + locationLat = coords?.first, + locationLng = coords?.second, + ) + withContext(Dispatchers.IO) { + actionRecords = mutableListOf() + val actions = rule.effectiveActions + val delays = rule.effectiveActionDelays + var currentAction: com.flowpilot.app.data.model.ActionType? = null + + try { + for (i in actions.indices) { + currentCoroutineContext().ensureActive() + val action = actions[i] + currentAction = action + val delaySec = delays.getOrElse(i) { 0 } + + if (delaySec > 0) { + if (!repository.renewExecutionReservation(reservation)) { + actionRecords.add( + ActionExecutionRecord.create( + actionType = action, + success = false, + message = "Execution revoked before dispatch", + resultCode = com.flowpilot.app.actions.ActionResultCode.EXECUTION_CANCELLED, + ) + ) + currentAction = null + break + } + delay(delaySec * 1000L) + } + + currentCoroutineContext().ensureActive() + val result = repository.dispatchIfAuthorized(reservation) { + eventAuthorization?.let { authorization -> + AutomationService.executeIfEventAuthorized(authorization) { + dispatcher.execute(action, actionParameters(rule, templateContext)) + } + } ?: dispatcher.execute(action, actionParameters(rule, templateContext)) + } + if (result == null) { + actionRecords.add( + ActionExecutionRecord.create( + actionType = action, + success = false, + message = "Execution revoked before dispatch", + resultCode = com.flowpilot.app.actions.ActionResultCode.EXECUTION_CANCELLED, + ) + ) + currentAction = null + break + } + Log.i(TAG, "Rule action result: action=${action.name}, success=${result.success}") + anySuccess = anySuccess || result.success + actionRecords.add( + ActionExecutionRecord.create(action, result) + ) + currentAction = null } - } ?: if (eventAuthorization == null) { - dispatcher.execute(action, actionParameters(rule, templateContext)) - } else { - return@withContext - } - Log.i(TAG, "Rule action result: action=${action.name}, success=${result.success}") - if (result.success) { - anySuccess = true + } catch (ce: CancellationException) { + currentAction?.let { action -> + actionRecords.add( + ActionExecutionRecord.create( + actionType = action, + success = false, + message = "Execution cancelled", + resultCode = com.flowpilot.app.actions.ActionResultCode.EXECUTION_CANCELLED, + ) + ) + } + throw ce } - actionRecords.add( - ActionExecutionRecord.create(action, result) - ) - currentAction = null } } catch (ce: CancellationException) { - currentAction?.let { action -> - actionRecords.add( - ActionExecutionRecord.create( - actionType = action, - success = false, - message = "Execution cancelled", - resultCode = com.flowpilot.app.actions.ActionResultCode.EXECUTION_CANCELLED, - ) - ) - } + cancelled = true throw ce } finally { - val historyEntry = ExecutionHistoryEntry.create( - ruleId = rule.id, - ruleName = rule.normalizedName, - trigger = trigger?.name ?: rule.triggerEvent.name, - timestamp = System.currentTimeMillis(), - actions = actionRecords, - ) withContext(NonCancellable) { - repository.appendHistory(historyEntry) + repository.appendHistory( + ExecutionHistoryEntry.create( + ruleId = rule.id, + ruleName = rule.normalizedName, + trigger = trigger?.name ?: rule.triggerEvent.name, + timestamp = System.currentTimeMillis(), + actions = actionRecords.ifEmpty { + listOf( + ActionExecutionRecord.create( + actionType = rule.effectiveActions.first(), + success = false, + message = if (cancelled) "Execution cancelled" else "Execution failed before dispatch", + resultCode = if (cancelled) { + com.flowpilot.app.actions.ActionResultCode.EXECUTION_CANCELLED + } else null, + ) + ) + }, + ) + ) + } + if (!anySuccess) { + withContext(NonCancellable) { + repository.releaseExecutionReservation(reservation, successful = false) + } + } else { + withContext(NonCancellable) { + repository.releaseExecutionReservation(reservation, successful = true) + } } } - - if (anySuccess) { - repository.patchLastTriggeredAt(rule.id, System.currentTimeMillis()) - } + } finally { + releasePendingExecution(candidate.id) } } } @@ -631,8 +685,18 @@ class AutomationEngine( smsMessage = rule.smsMessage, ) + private suspend fun reservePendingExecution(id: String): Boolean = pendingExecutionMutex.withLock { + pendingRuleIds.add(id) + } + + private suspend fun releasePendingExecution(id: String) { + pendingExecutionMutex.withLock { pendingRuleIds.remove(id) } + } + private companion object { val engineLifetime = Mutex() + val pendingExecutionMutex = Mutex() + val pendingRuleIds = mutableSetOf() const val TAG = "FlowPilotEngine" const val BLUETOOTH_TAG = "FlowPilotBluetooth" const val POLL_INTERVAL_MS = 500L diff --git a/app/src/test/java/com/flowpilot/app/data/AutomationRepositoryCryptoTest.kt b/app/src/test/java/com/flowpilot/app/data/AutomationRepositoryCryptoTest.kt index 74f950d..8329d7e 100644 --- a/app/src/test/java/com/flowpilot/app/data/AutomationRepositoryCryptoTest.kt +++ b/app/src/test/java/com/flowpilot/app/data/AutomationRepositoryCryptoTest.kt @@ -207,12 +207,12 @@ class AutomationRepositoryCryptoTest { repository.recordGeofenceRegistration(listOf(source.id), at = 8_888L) val persistedSource = repository.automations.first().single() - val clone = repository.duplicate( + val clone = requireNotNull(repository.duplicate( sourceId = source.id, copyName = "Morning (copy)", newId = "clone-id", createdAt = 12_345L, - ) + )) assertThat(clone).isEqualTo( persistedSource.copy( @@ -221,8 +221,10 @@ class AutomationRepositoryCryptoTest { enabled = false, createdAt = 12_345L, lastTriggeredAt = 0L, + executionRevision = clone.executionRevision, ), ) + assertThat(clone.executionRevision).isGreaterThan(persistedSource.executionRevision) assertThat(repository.geofenceDiagnostics.first()["clone-id"]).isNull() assertThat(repository.automations.first().first { it.id == source.id }).isEqualTo(persistedSource) } diff --git a/app/src/test/java/com/flowpilot/app/data/AutomationRepositoryExecutionReservationTest.kt b/app/src/test/java/com/flowpilot/app/data/AutomationRepositoryExecutionReservationTest.kt new file mode 100644 index 0000000..56cc4ad --- /dev/null +++ b/app/src/test/java/com/flowpilot/app/data/AutomationRepositoryExecutionReservationTest.kt @@ -0,0 +1,203 @@ +package com.flowpilot.app.data + +import android.content.Context +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.stringPreferencesKey +import com.flowpilot.app.data.model.ActionType +import com.flowpilot.app.data.model.ActionExecutionRecord +import com.flowpilot.app.data.model.Automation +import com.flowpilot.app.data.model.ExecutionHistoryEntry +import com.flowpilot.app.data.model.TriggerEvent +import com.flowpilot.app.actions.ActionResultCode +import com.flowpilot.app.data.security.SecretCipher +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runTest +import kotlinx.serialization.builtins.ListSerializer +import kotlinx.serialization.json.Json +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.annotation.Config +import javax.crypto.KeyGenerator +import javax.crypto.SecretKey + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class AutomationRepositoryExecutionReservationTest { + private lateinit var context: Context + private lateinit var repository: AutomationRepository + private lateinit var testSecretKey: SecretKey + private val rulesKey = stringPreferencesKey("rules") + private val rulesSerializer = ListSerializer(Automation.serializer()) + private val json = Json { encodeDefaults = true } + + @Before + fun setup() { + context = RuntimeEnvironment.getApplication() + testSecretKey = KeyGenerator.getInstance("AES").apply { init(256) }.generateKey() + SecretCipher.secretKeyProvider = { testSecretKey } + repository = AutomationRepository(context) + } + + @After + fun tearDown() = runTest { + repository.rawDataStore.edit { it.clear() } + SecretCipher.secretKeyProvider = null + } + + @Test + fun reserveExecution_claimsCooldownAtomically() = runTest { + val rule = addRule(cooldownMinutes = 5) + val snapshot = repository.automations.first().single() + val gate = CompletableDeferred() + + val reservations = (1..2).map { + async(Dispatchers.IO) { + gate.await() + repository.reserveExecution(snapshot.id, snapshot.executionRevision, at = 10_000L) + } + } + gate.complete(Unit) + + assertThat(reservations.awaitAll().filterNotNull()).hasSize(1) + assertThat(repository.automations.first().single().lastTriggeredAt).isEqualTo(10_000L) + assertThat(repository.executionHistory.first()).isEmpty() + } + + @Test + fun reserveExecution_zeroCooldownStillDeniesConcurrentLease_andReleaseRequiresOwnerToken() = runTest { + val rule = addRule(cooldownMinutes = 0) + val snapshot = repository.automations.first().single() + val first = requireNotNull(repository.reserveExecution(snapshot.id, snapshot.executionRevision, at = 10_000L)) + + assertThat(repository.reserveExecution(snapshot.id, snapshot.executionRevision, at = 10_001L)).isNull() + repository.releaseExecutionReservation(first.copy(token = "wrong"), successful = false) + assertThat(repository.reserveExecution(snapshot.id, snapshot.executionRevision, at = 10_002L)).isNull() + repository.releaseExecutionReservation(first, successful = false) + assertThat(repository.reserveExecution(snapshot.id, snapshot.executionRevision, at = 10_003L)).isNotNull() + } + + @Test + fun cancelledReservation_restoresPriorCooldown_andPersistsCancelledHistory() = runTest { + val rule = addRule(cooldownMinutes = 5) + repository.patchLastTriggeredAt(rule.id, 1_000L) + val snapshot = repository.automations.first().single() + val reservation = requireNotNull(repository.reserveExecution(snapshot.id, snapshot.executionRevision, at = 400_000L)) + + repository.releaseExecutionReservation(reservation, successful = false) + repository.appendHistory( + ExecutionHistoryEntry.create( + ruleId = rule.id, + ruleName = rule.name, + trigger = rule.triggerEvent.name, + actions = listOf( + ActionExecutionRecord.create( + actionType = ActionType.SHOW_NOTIFICATION, + success = false, + message = "Execution cancelled", + resultCode = ActionResultCode.EXECUTION_CANCELLED, + ) + ), + ) + ) + + assertThat(repository.automations.first().single().lastTriggeredAt).isEqualTo(1_000L) + assertThat(repository.executionHistory.first().single().actions.single().resultCode) + .isEqualTo(ActionResultCode.EXECUTION_CANCELLED) + } + + @Test + fun executionAuthorization_revokesDisabledDeletedAndChangedRules() = runTest { + val disabled = addRule(id = "disabled") + val disabledRevision = reserve(disabled) + repository.setEnabled(disabled.id, false) + assertThat(repository.isExecutionAuthorized(disabled.id, disabledRevision)).isFalse() + + val changed = addRule(id = "changed") + val changedRevision = reserve(changed) + repository.update(changed.copy(notificationTitle = "Changed")) + assertThat(repository.isExecutionAuthorized(changed.id, changedRevision)).isFalse() + assertThat(repository.reserveExecution(changed.id, changedRevision, at = 20_000L)).isNull() + + val deleted = addRule(id = "deleted") + val deletedRevision = reserve(deleted) + repository.delete(deleted.id) + assertThat(repository.isExecutionAuthorized(deleted.id, deletedRevision)).isFalse() + } + + @Test + fun dispatchAuthorization_serializesMutationAndRejectsRevokedLease() = runTest { + val rule = addRule() + val snapshot = repository.automations.first().single() + val reservation = requireNotNull(repository.reserveExecution(snapshot.id, snapshot.executionRevision)) + + repository.setEnabled(rule.id, false) + + assertThat(repository.dispatchIfAuthorized(reservation) { "side effect" }).isNull() + } + + @Test + fun replaceAll_revokesSameIdReservationAndKeepsWebhookSecretEncrypted() = runTest { + val rule = repository.add( + name = "Webhook", + triggerEvent = TriggerEvent.CHARGER_CONNECTED, + appPackage = "", + appName = "", + actions = listOf(ActionType.HTTP_WEBHOOK), + webhookUrl = "https://example.test/hook?token=secret", + id = "replacement", + ) + val revision = reserve(rule) + val storedAfterReservation = json.decodeFromString( + rulesSerializer, + repository.rawDataStore.data.first()[rulesKey]!!, + ).single() + assertThat(storedAfterReservation.webhookUrl).startsWith("enc:v1:") + assertThat(storedAfterReservation.webhookUrl).doesNotContain("secret") + + repository.replaceAll(listOf(rule.copy(notificationTitle = "Replacement"))) + + assertThat(repository.isExecutionAuthorized(rule.id, revision)).isFalse() + val stored = json.decodeFromString(rulesSerializer, repository.rawDataStore.data.first()[rulesKey]!!).single() + assertThat(stored.executionRevision).isGreaterThan(revision) + assertThat(stored.webhookUrl).startsWith("enc:v1:") + assertThat(stored.webhookUrl).doesNotContain("secret") + } + + @Test + fun persistenceRejectsDuplicateIds_andLegacyDuplicateReadsFailClosedToFirstRule() = runTest { + val rule = addRule(id = "duplicate") + val duplicate = rule.copy(name = "Second") + repository.rawDataStore.edit { prefs -> + prefs[rulesKey] = json.encodeToString(rulesSerializer, listOf(rule, duplicate)) + } + + assertThat(repository.automations.first()).containsExactly(rule) + assertThat(runCatching { repository.replaceAll(listOf(rule, duplicate)) }.isFailure).isTrue() + assertThat(runCatching { addRule(id = "duplicate") }.isFailure).isTrue() + } + + private suspend fun reserve(rule: Automation): Long { + val snapshot = repository.automations.first().first { it.id == rule.id } + assertThat(repository.reserveExecution(snapshot.id, snapshot.executionRevision, at = 10_000L)).isNotNull() + return snapshot.executionRevision + } + + private suspend fun addRule(id: String = "rule", cooldownMinutes: Int = 0): Automation = repository.add( + name = "Rule $id", + triggerEvent = TriggerEvent.CHARGER_CONNECTED, + appPackage = "", + appName = "", + actions = listOf(ActionType.SHOW_NOTIFICATION), + cooldownMinutes = cooldownMinutes, + id = id, + ) +} From 8371ecb180b11fc44356e4a42e33403199c62121 Mon Sep 17 00:00:00 2001 From: Emirhan Date: Mon, 14 Sep 2026 17:59:54 +0300 Subject: [PATCH 5/7] ci: harden release provenance gate --- .github/workflows/release.yml | 198 +++++++++++++++++++++++++++++++--- docs/RELEASE_SECURITY.md | 16 +++ 2 files changed, 197 insertions(+), 17 deletions(-) create mode 100644 docs/RELEASE_SECURITY.md diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 904a50a..1b0dba2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -10,25 +10,125 @@ on: - 'v*' permissions: + actions: read contents: write jobs: - release: - name: Build & Publish Release APK + validate: + name: Validate Release Provenance timeout-minutes: 20 runs-on: ubuntu-latest + permissions: + actions: read + contents: read + outputs: + release_commit: ${{ steps.provenance.outputs.release_commit }} steps: - name: Checkout repository uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + with: + ref: ${{ github.ref }} + fetch-depth: 0 - - name: Validate release tag + - name: Validate release provenance + id: provenance + env: + GH_TOKEN: ${{ github.token }} run: | + set -euo pipefail + if [[ "$GITHUB_REF_TYPE" != "tag" || ! "$GITHUB_REF_NAME" =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then echo "ERROR: release requires vX.Y.Z tag; got '$GITHUB_REF_NAME' ($GITHUB_REF_TYPE)" exit 1 fi + event_ref=$(jq --raw-output '.ref' "$GITHUB_EVENT_PATH") + event_after=$(jq --raw-output '.after' "$GITHUB_EVENT_PATH") + tag_object=$(git rev-parse "$GITHUB_REF") + release_commit=$(git rev-parse "${GITHUB_REF}^{commit}") + if [[ "$event_ref" != "$GITHUB_REF" || ( "$event_after" != "$tag_object" && "$event_after" != "$release_commit" ) ]]; then + echo "ERROR: release event ref does not match checked-out tag object" + exit 1 + fi + + git fetch --no-tags --force origin +refs/heads/main:refs/remotes/origin/main + main_commit=$(git rev-parse origin/main) + if [[ "$release_commit" != "$main_commit" ]]; then + echo "ERROR: tag commit $release_commit is not current origin/main commit $main_commit" + exit 1 + fi + git checkout --detach "$release_commit" + echo "release_commit=$release_commit" >> "$GITHUB_OUTPUT" + + version_name=$(sed -nE 's/^[[:space:]]*versionName[[:space:]]*=[[:space:]]*"([^"]+)".*/\1/p' app/build.gradle.kts) + if [[ $(printf '%s\n' "$version_name" | sed '/^$/d' | wc -l) -ne 1 || "$version_name" != "${GITHUB_REF_NAME#v}" ]]; then + echo "ERROR: app versionName '$version_name' does not match tag '$GITHUB_REF_NAME'" + exit 1 + fi + + ci_runs_url="https://api.github.com/repos/${GITHUB_REPOSITORY}/actions/workflows/ci.yml/runs?head_sha=${release_commit}&event=push&status=completed&per_page=100" + curl --fail --silent --show-error \ + --header 'Accept: application/vnd.github+json' \ + --header "Authorization: Bearer $GH_TOKEN" \ + --header 'X-GitHub-Api-Version: 2022-11-28' \ + "$ci_runs_url" > "$RUNNER_TEMP/ci-runs.json" + ci_run_id=$(jq --exit-status --raw-output --arg commit "$release_commit" ' + [.workflow_runs[] | select(.head_sha == $commit and .event == "push" and .head_branch == "main" and .status == "completed")] + | max_by(.run_started_at) | select(.conclusion == "success") | .id + ' "$RUNNER_TEMP/ci-runs.json") || { + echo "ERROR: Android CI push run for $release_commit on main has not completed successfully" + exit 1 + } + curl --fail --silent --show-error \ + --header 'Accept: application/vnd.github+json' \ + --header "Authorization: Bearer $GH_TOKEN" \ + --header 'X-GitHub-Api-Version: 2022-11-28' \ + "https://api.github.com/repos/${GITHUB_REPOSITORY}/actions/runs/${ci_run_id}/jobs?filter=latest&per_page=100" > "$RUNNER_TEMP/ci-jobs.json" + if ! jq --exit-status ' + any(.jobs[]; .name == "Build & Test" and .status == "completed" and .conclusion == "success") + ' "$RUNNER_TEMP/ci-jobs.json" > /dev/null; then + echo "ERROR: Android CI Build & Test job for $release_commit has not completed successfully" + exit 1 + fi + + release_url="https://api.github.com/repos/${GITHUB_REPOSITORY}/releases/tags/${GITHUB_REF_NAME}" + release_status=$(curl --silent --show-error --output "$RUNNER_TEMP/existing-release.json" --write-out '%{http_code}' \ + --header 'Authorization: Bearer $GH_TOKEN' \ + --header 'Accept: application/vnd.github+json' \ + --header 'X-GitHub-Api-Version: 2022-11-28' \ + "$release_url") + case "$release_status" in + 404) ;; + 200) + jq --exit-status --raw-output '.body | strings' "$RUNNER_TEMP/existing-release.json" > "$RUNNER_TEMP/existing-release-body.md" + echo "ERROR: release for $GITHUB_REF_NAME already exists; refusing to overwrite its notes or assets" + exit 1 + ;; + *) + echo "ERROR: unable to query existing release for $GITHUB_REF_NAME (HTTP $release_status)" + exit 1 + ;; + esac + + release: + name: Build & Publish Release APK + needs: validate + timeout-minutes: 20 + runs-on: ubuntu-latest + environment: release-signing + permissions: + contents: write + env: + RELEASE_COMMIT: ${{ needs.validate.outputs.release_commit }} + + steps: + - name: Checkout validated release commit + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 + with: + ref: ${{ needs.validate.outputs.release_commit }} + fetch-depth: 1 + - name: Set up JDK 17 uses: actions/setup-java@cf277c60eb25467037889841efdb72551f06f6c3 # v4 with: @@ -78,27 +178,91 @@ jobs: KEY_ALIAS: ${{ secrets.KEY_ALIAS }} KEY_PASSWORD: ${{ secrets.KEY_PASSWORD }} - - name: Rename APK & Generate Checksum + - name: Package release assets and notes + id: release_assets run: | - VERSION_TAG=$GITHUB_REF_NAME - cd app/build/outputs/apk/release/ - RELEASE_APK=$(ls -1 app-*-release.apk app-release.apk 2>/dev/null | head -n 1) - if [ -n "$RELEASE_APK" ]; then - FINAL_NAME="FlowPilot-${VERSION_TAG}.apk" - mv "$RELEASE_APK" "$FINAL_NAME" - sha256sum "$FINAL_NAME" > "${FINAL_NAME}.sha256" - echo "RELEASE_APK_NAME=$FINAL_NAME" >> $GITHUB_ENV - else - echo "ERROR: Release APK not found!" && exit 1 + set -euo pipefail + shopt -s nullglob + release_dir=app/build/outputs/apk/release + candidates=("$release_dir"/app-*-release.apk "$release_dir"/app-release.apk) + if [[ ${#candidates[@]} -ne 1 ]]; then + echo "ERROR: expected exactly one release APK; found ${#candidates[@]}" + printf ' %s\n' "${candidates[@]}" + exit 1 fi + release_apk=${candidates[0]} + release_apk_name="FlowPilot-${GITHUB_REF_NAME}.apk" + release_apk_path="$release_dir/$release_apk_name" + mv "$release_apk" "$release_apk_path" + release_checksum_path="${release_apk_path}.sha256" + sha256sum "$release_apk_path" > "$release_checksum_path" + sha256sum --check "$release_checksum_path" + release_sha256=$(awk '{print $1}' "$release_checksum_path") + + release_title="FlowPilot ${GITHUB_REF_NAME}" + release_notes_path="$RUNNER_TEMP/release-notes.md" + cat > "$release_notes_path" <> "$GITHUB_OUTPUT" + - name: Create GitHub Release & Upload APK uses: softprops/action-gh-release@3bb12739c298aeb8a4eeaf626c5b8d85266b0e65 # v2 with: files: | - app/build/outputs/apk/release/FlowPilot-*.apk - app/build/outputs/apk/release/FlowPilot-*.apk.sha256 - generate_release_notes: true + ${{ steps.release_assets.outputs.apk_path }} + ${{ steps.release_assets.outputs.checksum_path }} + name: ${{ steps.release_assets.outputs.release_title }} + body_path: ${{ steps.release_assets.outputs.notes_path }} draft: false prerelease: false env: diff --git a/docs/RELEASE_SECURITY.md b/docs/RELEASE_SECURITY.md new file mode 100644 index 0000000..58f1940 --- /dev/null +++ b/docs/RELEASE_SECURITY.md @@ -0,0 +1,16 @@ +# Release tag security + +`release.yml` only accepts `vX.Y.Z` tags. Its unprivileged validation job verifies push event tag object (annotated or lightweight), peels it to a commit, and requires that commit equal fresh `origin/main`. It also requires matching `app` `versionName`, successful `Android CI` push workflow and `Build & Test` job for exact commit, and no existing GitHub Release for tag. Only then can the environment-protected signing job run. + +## Required GitHub repository setup + +GitHub rulesets cannot live in repository. Configure in **Settings > Rules > Rulesets**: + +1. Create active **Tag** ruleset targeting `v*`. +2. Enable **Restrict creations**, **Restrict updates**, **Restrict deletions**, and **Require signed tags**. This rejects unsigned lightweight tags; workflow still validates either tag object form defensively. +3. Add only dedicated trusted release maintainers to bypass list. Do not grant `GitHub Actions`, broad write roles, or administrators bypass unless separately justified and audited. +4. Create or retain active **Branch** ruleset for `main`: require pull requests, require `Android CI / Build & Test`, require branch up to date before merge, restrict force pushes and deletions, and limit bypass list to trusted maintainers. +5. Create environment `release-signing`. Move `KEYSTORE_BASE64`, `KEYSTORE_PASSWORD`, `KEY_ALIAS`, and `KEY_PASSWORD` from repository secrets into that environment, then delete repository-level copies. Require at least one trusted release maintainer approval and prohibit self-review. Historical tag workflow revisions cannot read environment-only secrets because they do not reference this environment. +6. Set default `GITHUB_TOKEN` workflow permissions to read-only. `release.yml` explicitly grants only `actions: read` and `contents: write`; do not allow actions to create or approve pull requests. + +Tag rules prevent post-validation tag moves or deletion/recreation. Branch rules keep qualifying commits reviewed and CI-gated before tagging. Environment-scoped secrets prevent historical tag workflow revisions from reaching signing material. GitHub rulesets, environment protection, token defaults, secret migration, and bypass membership require repository-admin configuration and cannot be enforced from this repository. From c5e2b33f69952c04e79061b38bf34e7dc8b1d818 Mon Sep 17 00:00:00 2001 From: Emirhan Date: Mon, 14 Sep 2026 18:07:10 +0300 Subject: [PATCH 6/7] fix: expose event authorization token --- .../com/flowpilot/app/engine/EventExecutionAuthorization.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/com/flowpilot/app/engine/EventExecutionAuthorization.kt b/app/src/main/java/com/flowpilot/app/engine/EventExecutionAuthorization.kt index 66a6375..6c161fd 100644 --- a/app/src/main/java/com/flowpilot/app/engine/EventExecutionAuthorization.kt +++ b/app/src/main/java/com/flowpilot/app/engine/EventExecutionAuthorization.kt @@ -19,5 +19,5 @@ internal class EventExecutionAuthorization { fun executeIfAuthorized(token: Token, execute: () -> T): T? = if (token.generation == generation) execute() else null - internal data class Token internal constructor(private val generation: Long) + internal data class Token internal constructor(val generation: Long) } From f34aeae5d0c838d26f299d684d1342c08f269360 Mon Sep 17 00:00:00 2001 From: Emirhan Date: Mon, 14 Sep 2026 18:12:30 +0300 Subject: [PATCH 7/7] docs: record advisory remediation --- CHANGELOG.md | 5 +++++ docs/STATUS.md | 1 + 2 files changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95bad44..6978fb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ Changes completed after `1.0.2` and intended for the next release. ### Security +- Background NFC discovery now opens a confirmation gate before it can run matching automations; only foreground Android ReaderMode scans execute automatically. +- Webhooks pin initial TCP connections to prevalidated public IP addresses, preserve TLS hostname verification, reject unsafe rendered headers, and use bounded HTTP/1.1 parsing. +- Sensitive SMS and notification events are accepted only while the engine is enabled, bounded and freshness-limited, and reauthorized immediately before execution. +- Automatic rule runs now use durable execution leases and revision checks, preventing cooldown bypasses and revoking queued work after rule changes. +- Release workflow now requires a current `main` commit, exact successful CI, matching version tag, and protected signing environment before signing. - Enabled Dependabot vulnerability alerts and security update pull requests. - Enabled secret scanning, push protection, and private vulnerability reporting for the public repository. - Protected `main`: pull requests, a current successful `Build & Test` check, and resolved review conversations are required; force-push and branch deletion are disabled. diff --git a/docs/STATUS.md b/docs/STATUS.md index 2302dd6..e296416 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -4,6 +4,7 @@ Last updated: 2026-09-14 ## Build state +- Security advisory remediation passed local release gate on 2026-09-14: `testDebugUnitTest lintDebug assembleDebug assembleRelease -PreleaseSigningRequired=false`. Debug APK device install remains blocked because wireless ADB target is offline. - Debug/release builds, unit tests, and lint passed: `.\gradlew.bat testDebugUnitTest lintDebug assembleDebug assembleRelease -PreleaseSigningRequired=false`. - Resource contract test passed: `python scripts/test_lint_resource_contracts.py`. - Latest debug APK was installed and launched on Xiaomi (2506BPN68G) / HyperOS (Android 16).