Harden Android resets and disable ADB key expiry

src/clusterfuzz/_internal/bot/init_scripts/android.py

@@ -50,6 +50,11 @@ def run():
   logs.info('Init: wait_until_fully_booted.')
   android.adb.wait_until_fully_booted()
 
+  # Disable Android's 7-day automatic ADB key revocation so engine fuzzer bots
+  # and bots experiencing build setup outages do not become ADB unauthorized.
+  android.settings.set_content_setting('settings/global',
+                                       'adb_allowed_connection_time', 0)
+
   # Wait until battery charges to a minimum level and temperature threshold.
   android.battery.wait_until_good_state()
 

src/clusterfuzz/_internal/platforms/android/adb.py

@@ -116,8 +116,25 @@ def directory_exists(directory_path):
   return result == expected
 
 
-def execute_command(cmd, timeout=None, on_cuttlefish_host=False):
+def execute_command(cmd: str,
+                    timeout: int | None = None,
+                    on_cuttlefish_host: bool = False) -> str | None:
   """Spawns a subprocess to run the given shell command."""
+  _, output = execute_command_with_return_code(
+      cmd, timeout=timeout, on_cuttlefish_host=on_cuttlefish_host)
+  return output
+
+
+def execute_command_with_return_code(cmd: str,
+                                     timeout: int | None = None,
+                                     on_cuttlefish_host: bool = False
+                                    ) -> tuple[int | None, str | None]:
+  """Spawns a subprocess to run the given shell command.
+
+  Returns:
+    A (return_code, output) tuple. Both values are None if the command timed
+    out.
+  """
   if on_cuttlefish_host and environment.is_android_cuttlefish():
     # Auto accept key fingerprint for ssh command.
     cmd = ('ssh -o StrictHostKeyChecking=no '
@@ -162,10 +179,10 @@ def run():
       # Can't kill a dead process.
       pass
 
-    return None
+    return None, None
 
   bytes_output = b''.join(so)
-  return bytes_output.strip().decode('utf-8', errors='ignore')
+  return pipe.returncode, bytes_output.strip().decode('utf-8', errors='ignore')
 
 
 def copy_to_cuttlefish(src_path, dest_path, timeout=None):
@@ -190,6 +207,13 @@ def factory_reset():
   # Reverting that can bring a device back to good state.
   revert_asan_device_setup_if_needed()
 
+  if not environment.is_android_emulator():
+    # Don't factory reset physical devices: MASTER_CLEAR deletes the stored
+    # ADB keys (/data/misc/adb/adb_keys), leaving the phone "unauthorized" so
+    # no ADB command (including reflashing) works until someone accepts the
+    # prompt on the device. Exit instead, as for Cuttlefish above.
+    bad_state_reached()
+
   run_as_root()
   run_shell_command([
       'am', 'broadcast', '-a', 'android.intent.action.MASTER_CLEAR', '-n',
@@ -358,22 +382,38 @@ def get_property(property_name):
   return run_shell_command(['getprop', property_name])
 
 
+def _try_graceful_reboot() -> bool:
+  """Attempts a graceful `adb reboot` and returns whether it succeeded."""
+  soft_reset_cmd = get_adb_command_line('reboot')
+  return_code, reboot_output = execute_command_with_return_code(
+      soft_reset_cmd, timeout=RECOVERY_CMD_TIMEOUT)
+  # Successful `adb reboot` prints nothing from the device, while device-side
+  # errors (e.g. "reboot: Connection timed out") still return 0. Ignore benign
+  # host "* daemon ..." startup lines and treat any other output as a failure.
+  has_reboot_error = any(line.strip() and not line.startswith('*')
+                         for line in (reboot_output or '').splitlines())
+  return return_code == 0 and not has_reboot_error
+
+
 def hard_reset():
   """Perform a hard reset of the device."""
   if environment.is_android_cuttlefish():
     # There is no recovery step at this point for a cuttlefish bot, so just exit
     # and wait for reimage on next iteration.
     bad_state_reached()
 
-  # For physical device.
-  # Try hard-reset via sysrq-trigger (requires root).
-  hard_reset_sysrq_cmd = get_adb_command_line(
-      'shell echo b \\> /proc/sysrq-trigger')
-  execute_command(hard_reset_sysrq_cmd, timeout=RECOVERY_CMD_TIMEOUT)
+  # Flush filesystem buffers on a best-effort basis before rebooting.
+  sync_cmd = get_adb_command_line('shell sync')
+  execute_command(sync_cmd, timeout=GET_DEVICE_STATE_TIMEOUT)
 
-  # Try soft-reset now (does not require root).
-  soft_reset_cmd = get_adb_command_line('reboot')
-  execute_command(soft_reset_cmd, timeout=RECOVERY_CMD_TIMEOUT)
+  # Try graceful reboot first (unmounts /data cleanly and does not require
+  # root). Only fall back to sysrq-trigger if graceful reboot fails or hangs.
+  if not _try_graceful_reboot():
+    run_shell_command(
+        'echo b > /proc/sysrq-trigger',
+        root=True,
+        timeout=RECOVERY_CMD_TIMEOUT,
+        recover=False)
 
   if environment.is_android_emulator():
     logs.info('Platform ANDROID_EMULATOR detected.')
@@ -406,7 +446,8 @@ def read_data_from_file(file_path):
 
 
 def reboot():
-  """Reboots device."""
+  """Flushes filesystem buffers and reboots device."""
+  run_shell_command('sync', timeout=RECOVERY_CMD_TIMEOUT, recover=False)
   run_command('reboot')
 
 

src/clusterfuzz/_internal/platforms/android/device.py

@@ -143,6 +143,8 @@ def configure_device_settings():
   # Write our test environment settings in content database.
   settings.set_content_setting('com.google.settings/partner',
                                'use_location_for_services', 0)
+  settings.set_content_setting('settings/global', 'adb_allowed_connection_time',
+                               0)
   settings.set_content_setting('settings/global', 'assisted_gps_enabled', 0)
   settings.set_content_setting('settings/global',
                                'development_settings_enabled', 0)

src/clusterfuzz/_internal/tests/core/platforms/android/adb_test.py

@@ -15,6 +15,8 @@
 
 import os
 import tempfile
+import unittest
+from unittest import mock
 
 from clusterfuzz._internal.base import utils
 from clusterfuzz._internal.platforms.android import adb
@@ -216,3 +218,88 @@ def setUp(self):
   def test_with_device(self):
     """Tests reset_usb with a connected device if available."""
     self.assertTrue(adb.reset_usb())
+
+
+class ResetSafetyTest(unittest.TestCase):
+  """Unit tests for hard_reset and factory_reset."""
+
+  def setUp(self):
+    super().setUp()
+    test_helpers.patch_environ(self)
+    test_helpers.patch(self, [
+        'clusterfuzz._internal.platforms.android.adb.bad_state_reached',
+        'clusterfuzz._internal.platforms.android.adb.execute_command',
+        'clusterfuzz._internal.platforms.android.adb.execute_command_with_return_code',
+        'clusterfuzz._internal.platforms.android.adb.get_adb_command_line',
+        'clusterfuzz._internal.platforms.android.adb.revert_asan_device_setup_if_needed',
+        'clusterfuzz._internal.platforms.android.adb.run_as_root',
+        'clusterfuzz._internal.platforms.android.adb.run_shell_command',
+        'clusterfuzz._internal.system.environment.is_android_cuttlefish',
+        'clusterfuzz._internal.system.environment.is_android_emulator',
+        'time.sleep',
+    ])
+    self.mock.bad_state_reached.side_effect = SystemExit
+    self.mock.is_android_cuttlefish.return_value = False
+    self.mock.is_android_emulator.return_value = False
+    self.mock.get_adb_command_line.side_effect = lambda cmd: f'adb -s ser {cmd}'
+
+  def test_hard_reset_skips_sysrq_when_graceful_reboot_succeeds(self):
+    """Test that hard_reset syncs then reboots and skips sysrq-trigger on 0."""
+    self.mock.execute_command_with_return_code.return_value = (
+        0, '* daemon not running\n* daemon started successfully')
+    call_order = mock.Mock()
+    call_order.attach_mock(self.mock.execute_command, 'execute_command')
+    call_order.attach_mock(self.mock.execute_command_with_return_code,
+                           'execute_command_with_return_code')
+
+    adb.hard_reset()
+
+    self.assertEqual(call_order.mock_calls, [
+        mock.call.execute_command(
+            'adb -s ser shell sync', timeout=adb.GET_DEVICE_STATE_TIMEOUT),
+        mock.call.execute_command_with_return_code(
+            'adb -s ser reboot', timeout=adb.RECOVERY_CMD_TIMEOUT),
+    ])
+    self.mock.run_shell_command.assert_not_called()
+
+  def test_hard_reset_falls_back_to_root_sysrq_when_reboot_fails(self):
+    """Test fallback to root sysrq-trigger on non-zero exit, timeout, or error."""
+    failure_results = [
+        (1, 'error: device offline'),
+        (None, None),
+        (0, 'reboot (adb) failed'),
+        (0, 'reboot: Connection timed out'),
+    ]
+    for result in failure_results:
+      with self.subTest(result=result):
+        self.mock.run_shell_command.reset_mock()
+        self.mock.execute_command_with_return_code.return_value = result
+
+        adb.hard_reset()
+
+        self.mock.run_shell_command.assert_called_once_with(
+            'echo b > /proc/sysrq-trigger',
+            root=True,
+            timeout=adb.RECOVERY_CMD_TIMEOUT,
+            recover=False)
+
+  def test_factory_reset_avoids_master_clear_on_physical_device(self):
+    """Test that factory_reset calls bad_state_reached on physical devices."""
+    with self.assertRaises(SystemExit):
+      adb.factory_reset()
+
+    self.mock.revert_asan_device_setup_if_needed.assert_called_once()
+    self.mock.bad_state_reached.assert_called_once()
+    self.mock.run_shell_command.assert_not_called()
+
+  def test_factory_reset_runs_master_clear_on_emulator(self):
+    """Test that factory_reset still broadcasts MASTER_CLEAR on emulators."""
+    self.mock.is_android_emulator.return_value = True
+
+    adb.factory_reset()
+
+    self.mock.bad_state_reached.assert_not_called()
+    self.mock.run_as_root.assert_called_once()
+    self.mock.run_shell_command.assert_called_once()
+    self.assertIn('android.intent.action.MASTER_CLEAR',
+                  self.mock.run_shell_command.call_args[0][0])