Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 59 additions & 42 deletions app/src/main/java/com/tunnelguard/app/TunnelGuardVpnService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,13 @@ class TunnelGuardVpnService : VpnService() {
var pendingWarningId: String? = null

val stateLock = Any()
val warningLock = Any()

fun shouldPostFallbackWarning(warningId: String): Boolean {
synchronized(warningLock) {
return pendingWarningId == warningId
}
}

@Volatile
var currentServiceState = ServiceState.NO_VPN
Expand Down Expand Up @@ -233,49 +240,59 @@ class TunnelGuardVpnService : VpnService() {
config.addLog("Could not start VpnWarningActivity directly from background: ${e.message}")
}

val options = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
android.app.ActivityOptions.makeBasic().setPendingIntentCreatorBackgroundActivityStartMode(
android.app.ActivityOptions.MODE_BACKGROUND_ACTIVITY_START_ALLOWED
).toBundle()
} else {
null
serviceScope.launch {
delay(1000)
synchronized(warningLock) {
if (shouldPostFallbackWarning(warningId)) {
config.addLog("VpnWarningActivity did not launch in time. Posting fallback warning notification.")
val options = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) {
android.app.ActivityOptions.makeBasic().setPendingIntentCreatorBackgroundActivityStartMode(
android.app.ActivityOptions.MODE_BACKGROUND_ACTIVITY_START_ALLOWED
).toBundle()
} else {
null
}

val pendingIntent = PendingIntent.getActivity(
this@TunnelGuardVpnService,
1002,
warningIntent,
PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT,
options
)

var appLabel = currentApp
try {
val pm = packageManager
val appInfo = pm.getApplicationInfo(currentApp, 0)
appLabel = pm.getApplicationLabel(appInfo).toString()
} catch (e: Exception) {
// Ignore
}

val warningNotificationBuilder = NotificationCompat.Builder(this@TunnelGuardVpnService, ALERT_CHANNEL_ID)
.setContentTitle("Security Warning")
.setContentText("$appLabel opened without an active VPN connection!")
.setSmallIcon(android.R.drawable.ic_dialog_alert)
.setPriority(NotificationCompat.PRIORITY_HIGH)
.setCategory(NotificationCompat.CATEGORY_ALARM)
.setAutoCancel(true)
.setContentIntent(pendingIntent)

// Only call setFullScreenIntent when permission is available according to NotificationManagerCompat
val managerCompat = androidx.core.app.NotificationManagerCompat.from(this@TunnelGuardVpnService)
if (managerCompat.canUseFullScreenIntent()) {
warningNotificationBuilder.setFullScreenIntent(pendingIntent, true)
}

val warningNotification = warningNotificationBuilder.build()
val manager = getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager
manager.notify(1002, warningNotification)
} else {
config.addLog("VpnWarningActivity launched successfully. Skipping fallback notification.")
}
}
Comment on lines +243 to +294

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Synchronize warning acknowledgement with fallback posting.

@Volatile makes pendingWarningId visible. It does not make the check at Line 238 and manager.notify at Line 282 atomic.

VpnWarningActivity.kt:37-45 can clear the matching ID and cancel notification 1002 after this check but before this notification post. In that order, the cancellation occurs first and the fallback notification remains posted.

Use one shared warning lock in both files. Hold it while the activity validates and clears the ID and cancels notification 1002. Hold the same lock while this coroutine checks the ID and posts the notification.

🧰 Tools
🪛 detekt (1.23.8)

[warning] 261-261: The caught exception is swallowed. The original exception could be lost.

(detekt.exceptions.SwallowedException)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/java/com/tunnelguard/app/TunnelGuardVpnService.kt` around lines
236 - 285, Add a shared warning lock accessible to both TunnelGuardVpnService
and VpnWarningActivity, and synchronize the fallback block around the
pendingWarningId check through manager.notify(1002, warningNotification). Update
the activity’s matching-ID validation, clearing of pendingWarningId, and
cancellation of notification 1002 to use the same lock, preserving the existing
behavior while preventing cancellation and fallback posting from interleaving.

}

val pendingIntent = PendingIntent.getActivity(
this@TunnelGuardVpnService,
1002,
warningIntent,
PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT,
options
)

var appLabel = currentApp
try {
val pm = packageManager
val appInfo = pm.getApplicationInfo(currentApp, 0)
appLabel = pm.getApplicationLabel(appInfo).toString()
} catch (e: Exception) {
// Ignore
}

val warningNotificationBuilder = NotificationCompat.Builder(this@TunnelGuardVpnService, ALERT_CHANNEL_ID)
.setContentTitle("Security Warning")
.setContentText("$appLabel opened without an active VPN connection!")
.setSmallIcon(android.R.drawable.ic_dialog_alert)
.setPriority(NotificationCompat.PRIORITY_HIGH)
.setCategory(NotificationCompat.CATEGORY_ALARM)
.setAutoCancel(true)
.setContentIntent(pendingIntent)

// Only call setFullScreenIntent when permission is available according to NotificationManagerCompat
val managerCompat = androidx.core.app.NotificationManagerCompat.from(this@TunnelGuardVpnService)
if (managerCompat.canUseFullScreenIntent()) {
warningNotificationBuilder.setFullScreenIntent(pendingIntent, true)
}

val warningNotification = warningNotificationBuilder.build()
val manager = getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager
manager.notify(1002, warningNotification)
}
}

Expand Down
36 changes: 20 additions & 16 deletions app/src/main/java/com/tunnelguard/app/VpnWarningActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -34,27 +34,31 @@ class VpnWarningActivity : AppCompatActivity() {

// Handle the per-warning one-shot state and cancel notification on entry
val incomingWarningId = intent.getStringExtra("warning_id")
val activePendingId = TunnelGuardVpnService.pendingWarningId

if (incomingWarningId != null) {
if (incomingWarningId == activePendingId) {
// Acknowledge the event
TunnelGuardVpnService.pendingWarningId = null
config.addLog("VpnWarningActivity acknowledged warning ID: $incomingWarningId")

// Cancel notification 1002 immediately on entry
synchronized(TunnelGuardVpnService.warningLock) {
val activePendingId = TunnelGuardVpnService.pendingWarningId
if (incomingWarningId == activePendingId) {
// Acknowledge the event
TunnelGuardVpnService.pendingWarningId = null
config.addLog("VpnWarningActivity acknowledged warning ID: $incomingWarningId")

// Cancel notification 1002 immediately on entry
val manager = getSystemService(android.content.Context.NOTIFICATION_SERVICE) as android.app.NotificationManager
manager.cancel(1002)
} else {
// Discard stale or duplicate event to prevent duplicate countdowns
config.addLog("VpnWarningActivity discarded duplicate/stale warning ID: $incomingWarningId (pending: $activePendingId)")
finish()
return
}
}
} else {
synchronized(TunnelGuardVpnService.warningLock) {
// Cancel notification 1002 on entry as well
val manager = getSystemService(android.content.Context.NOTIFICATION_SERVICE) as android.app.NotificationManager
manager.cancel(1002)
} else {
// Discard stale or duplicate event to prevent duplicate countdowns
config.addLog("VpnWarningActivity discarded duplicate/stale warning ID: $incomingWarningId (pending: $activePendingId)")
finish()
return
}
} else {
// Cancel notification 1002 on entry as well
val manager = getSystemService(android.content.Context.NOTIFICATION_SERVICE) as android.app.NotificationManager
manager.cancel(1002)
}

setContentView(R.layout.activity_vpn_warning)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,108 @@ class TunnelGuardVpnServiceWarningTest {
isSuppressed = true
))
}

@org.junit.After
fun tearDown() {
TunnelGuardVpnService.pendingWarningId = null
}

@Test
fun testFallbackSkippedAfterSuccessfulActivityLaunch() {
// Case 1: Fallback is skipped after successful activity launch
val knownId = "warning-123"
TunnelGuardVpnService.pendingWarningId = knownId

// Simulate VpnWarningActivity acknowledging the warning (by clearing pendingWarningId)
synchronized(TunnelGuardVpnService.warningLock) {
if (TunnelGuardVpnService.pendingWarningId == knownId) {
TunnelGuardVpnService.pendingWarningId = null
}
}

// Verify that the delayed fallback condition no longer matches the original warning ID
val shouldPost = TunnelGuardVpnService.shouldPostFallbackWarning(knownId)
assertFalse(shouldPost)
}

@Test
fun testFallbackOccursWhenActivityDoesNotLaunch() {
// Case 2: Fallback occurs when activity does not launch
val knownId = "warning-456"
TunnelGuardVpnService.pendingWarningId = knownId

// Do not clear it. Verify that the fallback condition still matches the original warning ID
val shouldPost = TunnelGuardVpnService.shouldPostFallbackWarning(knownId)
assertTrue(shouldPost)
}

@Test
fun testStaleWarningIdsAreIgnored() {
// Case 3: Stale warning IDs are ignored
TunnelGuardVpnService.pendingWarningId = "current-warning"

// Check the fallback using an old/stale warning ID
val shouldPost = TunnelGuardVpnService.shouldPostFallbackWarning("old-warning")
assertFalse(shouldPost)
}

@Test
fun testWarningIdAcknowledgementIsAtomic() {
// Case 4: Warning ID acknowledgement is atomic
val knownId = "warning-atomic"
TunnelGuardVpnService.pendingWarningId = knownId

val threadCheckedBeforeClearing: Boolean
val threadCheckedAfterClearing: Boolean

// Start checking in a synchronized block representing the warning/acknowledgement transaction
synchronized(TunnelGuardVpnService.warningLock) {
// Check fallback inside the lock
threadCheckedBeforeClearing = TunnelGuardVpnService.shouldPostFallbackWarning(knownId)

// Acknowledge/clear it inside the lock
TunnelGuardVpnService.pendingWarningId = null

// Check fallback again under lock after clearing
threadCheckedAfterClearing = TunnelGuardVpnService.shouldPostFallbackWarning(knownId)
}

assertTrue(threadCheckedBeforeClearing)
assertFalse(threadCheckedAfterClearing)
}

@Test
fun testDuplicateOrStaleActivityLaunches() {
// Case 5: Verify that an activity receiving an old warning ID does not clear the current warning ID
val currentId = "current-active-warning"
val oldId = "stale-old-warning"

TunnelGuardVpnService.pendingWarningId = currentId

// Simulate an activity starting with an old/stale warning ID and attempting to acknowledge
synchronized(TunnelGuardVpnService.warningLock) {
if (oldId == TunnelGuardVpnService.pendingWarningId) {
TunnelGuardVpnService.pendingWarningId = null
}
}

// The current warning ID should remain untouched/active
assertEquals(currentId, TunnelGuardVpnService.pendingWarningId)
}

@Test
fun testMultipleWarningIds() {
// Case 6: Set warning ID A, replace it with warning ID B, verify A's delayed fallback does not post, while B remains valid
val warningIdA = "warning-A"
val warningIdB = "warning-B"

TunnelGuardVpnService.pendingWarningId = warningIdA
// Replace with B
TunnelGuardVpnService.pendingWarningId = warningIdB

// A's delayed fallback check should return false
assertFalse(TunnelGuardVpnService.shouldPostFallbackWarning(warningIdA))
// B's delayed fallback check should return true
assertTrue(TunnelGuardVpnService.shouldPostFallbackWarning(warningIdB))
}
}
Loading