Skip to content

fix(client/android): check VPN consent in VpnServiceStarter and harden foreground notification - #2892

Open
sentry[bot] wants to merge 2 commits into
masterfrom
seer/fix/android-vpn-starter-foreground-deadline
Open

sentry[bot] wants to merge 2 commits into
masterfrom
seer/fix/android-vpn-starter-foreground-deadline

Conversation

@sentry

@sentry sentry Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Fixes Sentry issue OUTLINE-CLIENTS-NE2GP (RemoteServiceException$ForegroundServiceDidNotStartInTimeException: Context.startForegroundService() did not then call Service.startForeground()).

Problem

  • VpnServiceStarter called context.startForegroundService() whenever tunnelStore.getTunnelStatus() was CONNECTED. When the process was previously killed in the background (e.g. under low-memory conditions), that status was often stale and the device frequently no longer had VPN consent (e.g. another VPN app became active, consent was revoked, or the app ran under a secondary Android profile).
  • Once startForegroundService() is called, Android requires the service to call startForeground() within a strict deadline. Because VPN consent requires UI, VpnTunnelService could not prepare the VPN and aborted auto-connect before calling startForeground(), crashing the app.
  • Furthermore, any exception during notification creation (e.g. missing launcher activity or failure to locate small_icon) would fail startForeground() and trigger the same crash.

Changes

  1. VPN consent check on boot/update: In VpnServiceStarter, check VpnService.prepare(context) == null before launching the service. If consent is absent, mark the tunnel status as DISCONNECTED and exit early so the service is never started.
  2. Resilient notification builder: In VpnTunnelService, do not throw if the launcher intent cannot be resolved (falls back to a notification without a tap action), and fall back to android.R.drawable.ic_lock_lock if small_icon cannot be resolved.
  3. Handle foreground start failures: In VpnTunnelService.startForegroundWithNotification(), log the full exception on failure and return false. In startLastSuccessfulTunnel(), abort cleanly if moving to the foreground fails.
  4. AutoStartPolicy & Regression Tests: Extracted the auto-start decision logic and fallback icon selection to AutoStartPolicy so it can be covered by local JVM unit tests (AutoStartPolicyTest, 9 tests). Added instrumented tests (VpnServiceStarterTest, VpnTunnelServiceNotificationTest) covering boot without consent, missing launcher intent, and missing small icon fallback.

Fixes OUTLINE-CLIENTS-NE2GP

@sentry <feedback>: Autofix iterates on these changes
@sentry stop iterating: Autofix stops iterating on this run

This PR was automatically generated by Sentry at no cost. You can adjust this setting at any time.

@sentry
sentry Bot requested a review from a team as a code owner October 13, 2026 11:12
@greptile-apps

greptile-apps Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium impact] Adds VPN consent checks and hardens foreground service startup.

The PR appears safe to merge, with a non-blocking request for regression tests.

Findings

  1. P2 Startup fixes lack tests ▶

Summary

This PR checks VPN consent before starting the service at boot or after an update.

  • Boot and update auto-start checks VPN consent before launching the service.
  • VPN notifications still build without a launcher activity or app icon.
  • Auto-connect stops when foreground startup fails.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Boot or app update] --> B{Saved status connected?}
  B -->|No| X[Exit]
  B -->|Yes| C{VPN consent granted?}
  C -->|No| D[Save disconnected status]
  D --> X
  C -->|Yes| E[Start VPN service]
  E --> F[Build notification with fallbacks]
  F --> G{Foreground start succeeds?}
  G -->|No| H[Clear status and stop service]
  G -->|Yes| I[Continue saved tunnel startup]
Loading

Reviews (1) · Last reviewed commit: "fix(client/android): check VPN consent i..." · Reviewed by Greptile

Comment on lines +51 to +54
if (VpnService.prepare(context) != null) {
tunnelStore.setTunnelStatus(VpnTunnelService.TunnelStatus.DISCONNECTED);
return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Startup fixes lack tests

The new failure paths have no regression tests. Existing tests only cover saved tunnel data, so a later change could bring back the reported startup crash without a test failing.

Add tests showing that missing VPN consent clears the saved status without starting the service, missing launcher or icon resources still allow a notification, and a failed foreground start stops auto-connect.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@github-actions github-actions Bot added size/L and removed size/S labels Oct 13, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants