Improve best-effort system proxy handling

Signed-off-by: Loren Eteval <loren.eteval@proton.me>

Furious/Controllers/AGENTS.md

@@ -13,7 +13,7 @@ transitions. Services own execution resources; existing prompts are presentation
   exposed during `Connecting`; successful runtime commit precedes System Proxy setup and `Connected`. Failure resets
   the active profile. Disconnect/reconnect cancels the exact in-flight generation and ignores stale completion.
 - Preserve state and signal ordering, interaction gating, the exact selected `ServerProfile`, runtime snapshots,
-  reconnect preference, and rollback after validation, runtime, TUN, System Proxy, cancellation, or unexpected-exit
+  reconnect preference, and rollback after validation, runtime, TUN, System Proxy exceptions, cancellation, or unexpected-exit
   failure. Worker/native callbacks cross to the controller’s Qt thread before transition.
 - The active live profile is not the prepared document used by an already-started runtime. Resolve identity and
   generation before changing state or host effects, and preserve typed runtime failures; cancellation and supersession
@@ -25,9 +25,9 @@ transitions. Services own execution resources; existing prompts are presentation
 - `SettingsController` is the shared policy path used by Home, Settings, tray, and platform integration. Startup
   registration persists only after host success; other preferences may apply immediately or on the next connection.
   Preserve each setting's actual application timing instead of imposing one transaction order on all preferences.
-- System Proxy helpers currently log some host failures without raising. Controller exception-path tests prove
-  recovery when an error reaches the controller, not that every OS failure is propagated. Keep desired proxy mode
-  distinct from observed host state when evolving this boundary.
+- System Proxy configuration is best effort. Its helper logs host failures; an explicit False return does not roll
+  back an otherwise usable committed runtime or prevent Connected. Preserve exception-path recovery separately.
+  Exercise actual helper results with mocked OS boundaries; Connected does not guarantee the OS proxy was applied.
 
 ## Verification and evolution
 

Furious/Controllers/ConnectionController.py

@@ -76,6 +76,8 @@ def validateProxyServer(server) -> bool:
         if int(port) < 0 or int(port) > 65535:
             raise ValueError
     except Exception:
+        # Any non-exit exceptions
+
         return False
 
     return True
@@ -382,6 +384,8 @@ class ConnectionController(QtCore.QObject):
         )
 
         try:
+            # System proxy setup is best effort; the helper logs host failures.
+            # Ignore even False so an otherwise usable core stays connected.
             SystemProxy.set(httpProxy, proxyServerBypass)
         except Exception as ex:
             # Any non-exit exceptions
@@ -515,6 +519,8 @@ class ConnectionController(QtCore.QObject):
             try:
                 self._actionQueue.get_nowait()
             except Exception:
+                # Any non-exit exceptions
+
                 pass
 
         Mixins.ConnectionAware.callDisconnectedCallback()

Furious/Frozenlib/AGENTS.md

@@ -14,10 +14,11 @@ for unrelated application orchestration to accumulate in a broad helper namespac
 - Keep proxy, DNS, routing, TUN, startup registration, session callbacks, external commands, and platform detection here
   or behind a runtime boundary so tests can replace them completely. Windows, macOS, Linux, Flatpak, AppImage, and older
   platform paths are distinct capabilities; never generalize from the current host.
-- Check each helper's real result contract. Startup registration and some routing helpers return Booleans; System
-  Proxy set/off currently log failures and return no success value. Script-mode startup registration intentionally
-  does nothing. Do not infer confirmed host state from absence of an exception or generalize one helper's semantics
-  to all. New mutation APIs should report actionable success/failure to the owning controller/service.
+- Check each helper's real result contract. System Proxy set/off/pac return True for reported host success, False for
+  failure, and None when policy deliberately leaves host settings unchanged. Startup registration and some routing
+  helpers return Booleans; script-mode startup registration intentionally does nothing. Preserve these distinctions
+  at callers instead of treating absence of an exception as confirmed host state. Check every native command result,
+  including each enabled macOS network service, and bound host-command waits at this boundary.
 - Prefer argument vectors over shell strings. Each caller owns any responsiveness/cleanup timeout appropriate to its
   context; build-time commands and GUI-time host mutation do not share one universal timeout policy.
 - Windows proxy calls, Linux desktop settings/host bridging, and macOS network-service operations are distinct

Furious/Frozenlib/SystemProxy.py

@@ -33,6 +33,8 @@ __all__ = ['SystemProxy']
 
 logger = logging.getLogger(__name__)
 
+HOST_PROXY_COMMAND_TIMEOUT = 5.0
+
 
 def handleAppSystemProxyMode() -> bool:
     """Handle app system proxy mode."""
@@ -68,6 +70,7 @@ def linuxProxyConfig(proxy_args, arg0, arg1):
         stdout=subprocess.DEVNULL,
         stderr=subprocess.DEVNULL,
         check=True,
+        timeout=HOST_PROXY_COMMAND_TIMEOUT,
     )
 
 
@@ -81,6 +84,7 @@ def darwinProxyConfig(operation, *args):
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
             check=True,
+            timeout=HOST_PROXY_COMMAND_TIMEOUT,
         )
 
         # Replace with command.stdout.decode('utf-8', 'replace')...?
@@ -89,13 +93,20 @@ def darwinProxyConfig(operation, *args):
         return service[1:]
 
     for serviceName in getNetworkServices():
+        if serviceName.startswith('*'):
+            continue
+
         runExternalCommand(
             [
                 'networksetup',
                 f'-{operation}',
                 serviceName,
                 *args,
-            ]
+            ],
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+            check=True,
+            timeout=HOST_PROXY_COMMAND_TIMEOUT,
         )
 
 
@@ -166,14 +177,18 @@ class _SystemProxy:
 
             return
 
-        if _pac():
+        success = bool(_pac())
+
+        if success:
             logger.info('set proxy PAC success')
         else:
             logger.error('set proxy PAC failed')
 
+        return success
+
     @staticmethod
     def set(server, bypass):
-        """Set data managed by the system proxy."""
+        """Return host success/failure, or None when policy deliberately ignores it."""
 
         def _set():
             """Return the set value used by the system proxy."""
@@ -226,11 +241,15 @@ class _SystemProxy:
 
             return
 
-        if _set():
+        success = bool(_set())
+
+        if success:
             logger.info(f'set proxy server {server} success')
         else:
             logger.error(f'set proxy server {server} failed')
 
+        return success
+
     @staticmethod
     def off():
         """Disable the system proxy."""
@@ -274,11 +293,15 @@ class _SystemProxy:
 
             return
 
-        if _off():
+        success = bool(_off())
+
+        if success:
             logger.info('turn off proxy success')
         else:
             logger.error('turn off proxy failed')
 
+        return success
+
     def daemonOn_(self):
         """Return the daemon on value used by the system proxy."""
 
@@ -309,6 +332,8 @@ class _SystemProxy:
                         try:
                             thread.start()
                         except Exception:
+                            # Any non-exit exceptions
+
                             if self._daemonThread is thread:
                                 self._daemonThread = None
 

tests/test_controllers.py

@@ -35,6 +35,7 @@ from PySide6 import QtCore
 from tests.support import application, isolatedSettings, processQtEvents
 
 import unittest
+import importlib
 
 from types import SimpleNamespace
 from unittest import mock
@@ -498,6 +499,51 @@ class ConnectionControllerTest(unittest.TestCase):
 
             controller.deleteLater()
 
+    def testActualProxyHelperFailurePreservesCommittedStartup(self):
+        """Keep a usable core connected when best-effort system proxy setup fails."""
+        proxyModule = importlib.import_module('Furious.Frozenlib.SystemProxy')
+
+        for asynchronous in (False, True):
+            with self.subTest(asynchronous=asynchronous), isolatedSettings():
+                core = (
+                    FixtureAsyncCoreManager() if asynchronous else FixtureCoreManager()
+                )
+                controller = ConnectionController(
+                    coreManager=core, updatesManager=FixtureUpdatesManager()
+                )
+                self.addCleanup(controller.deleteLater)
+
+                states = []
+                errors = []
+
+                controller.stateChanged.connect(states.append)
+                controller.errorOccurred.connect(errors.append)
+
+                with mock.patch.object(
+                    proxyModule, 'PLATFORM', 'Linux'
+                ), mock.patch.object(
+                    proxyModule, 'handleAppSystemProxyMode', return_value=True
+                ), mock.patch.object(
+                    proxyModule,
+                    'linuxProxyConfig',
+                    side_effect=OSError('host rejected operation'),
+                ), mock.patch.object(
+                    controller, '_runPostConnectTasksOnce'
+                ) as postConnect:
+                    self.assertTrue(controller.startConnection(self.profile))
+
+                    if asynchronous:
+                        core.operations[0][0].succeed()
+
+                    processQtEvents()
+
+                    self.assertEqual(controller.state, ConnectionState.Connected)
+                    self.assertEqual(states.count(ConnectionState.Connected), 1)
+                    self.assertIs(controller.activeProfile, self.profile)
+                    self.assertEqual(core.stopCalls, 0)
+                    self.assertEqual(errors, [])
+                    postConnect.assert_called_once()
+
     def testStartAndProxyExceptionsReturnToStableDisconnectedState(self):
         """Clean every partially acquired resource after injected failures."""
         for core, proxySideEffect in (

tests/test_frozenlib.py

@@ -431,6 +431,112 @@ class FrozenlibUtilityTest(unittest.TestCase):
 class MockedPlatformHelperTest(unittest.TestCase):
     """Exercise platform helpers without touching host startup or networking."""
 
+    def testProxyMutationsReturnHostFailureWithoutDisclosingSecrets(self):
+        """Preserve false results from each native boundary for callers to handle."""
+        proxy = SystemProxyModule._SystemProxy()
+        native = types.SimpleNamespace(
+            set=mock.Mock(return_value=False),
+            off=mock.Mock(return_value=False),
+            pac=mock.Mock(return_value=False),
+        )
+
+        for platform in ('Windows', 'Linux', 'Darwin'):
+            with self.subTest(platform=platform), mock.patch.object(
+                SystemProxyModule, 'PLATFORM', platform
+            ), mock.patch.object(
+                SystemProxyModule, 'handleAppSystemProxyMode', return_value=True
+            ), mock.patch.dict(
+                sys.modules, {'sysproxy': native}
+            ), mock.patch.object(
+                SystemProxyModule,
+                'linuxProxyConfig',
+                side_effect=OSError('private-token'),
+            ), mock.patch.object(
+                SystemProxyModule,
+                'darwinProxyConfig',
+                side_effect=OSError('private-token'),
+            ), self.assertLogs(
+                SystemProxyModule.logger, level='ERROR'
+            ) as captured:
+                self.assertIs(proxy.set('127.0.0.1:10809', 'localhost'), False)
+                self.assertIs(proxy.off(), False)
+                self.assertIs(proxy.pac('https://invalid.test/private-token'), False)
+
+            self.assertNotIn('private-token', '\n'.join(captured.output))
+
+    def testIgnoredProxyMutationsDoNotTouchHost(self):
+        """Do-not-change remains a deliberate no-op rather than a failed mutation."""
+        with mock.patch.object(
+            SystemProxyModule, 'handleAppSystemProxyMode', return_value=False
+        ), mock.patch.object(SystemProxyModule, 'runExternalCommand') as command:
+            self.assertIsNone(SystemProxyModule.SystemProxy.set('127.0.0.1:10809', ''))
+            self.assertIsNone(SystemProxyModule.SystemProxy.off())
+            self.assertIsNone(SystemProxyModule.SystemProxy.pac('https://invalid.test'))
+
+            command.assert_not_called()
+
+    def testDarwinProxyChecksEveryEnabledServiceCommand(self):
+        """Check enabled services and propagate even a later service rejection."""
+        for fail in (False, True):
+            commands = []
+
+            def run(command, **kwargs):
+                commands.append(command)
+
+                self.assertTrue(kwargs.get('check'))
+                self.assertEqual(kwargs.get('timeout'), 5.0)
+
+                if '-listallnetworkservices' in command:
+                    return subprocess.CompletedProcess(
+                        command,
+                        0,
+                        stdout=b'Header\nWi-Fi\n*Disabled service\nEthernet\n',
+                    )
+
+                if fail and 'Ethernet' in command:
+                    raise subprocess.CalledProcessError(1, command)
+
+                return subprocess.CompletedProcess(command, 0)
+
+            with self.subTest(fail=fail), mock.patch.object(
+                SystemProxyModule, 'runExternalCommand', side_effect=run
+            ):
+                if fail:
+                    with self.assertRaises(subprocess.CalledProcessError):
+                        SystemProxyModule.darwinProxyConfig(
+                            'setwebproxy', '127.0.0.1', '10809'
+                        )
+                else:
+                    SystemProxyModule.darwinProxyConfig(
+                        'setwebproxy', '127.0.0.1', '10809'
+                    )
+
+                self.assertEqual(
+                    [command[2] for command in commands[1:]], ['Wi-Fi', 'Ethernet']
+                )
+
+    def testLinuxProxyCommandTimeoutBecomesExplicitFailure(self):
+        """A hung host settings command has a deadline and cannot claim success."""
+        with (
+            mock.patch.object(SystemProxyModule, 'PLATFORM', 'Linux'),
+            mock.patch.object(
+                SystemProxyModule, 'handleAppSystemProxyMode', return_value=True
+            ),
+            mock.patch.object(
+                SystemProxyModule.SystemRuntime, 'flatpakID', return_value=''
+            ),
+            mock.patch.object(
+                SystemProxyModule,
+                'runExternalCommand',
+                side_effect=subprocess.TimeoutExpired('gsettings', 5.0),
+            ) as command,
+            self.assertLogs(SystemProxyModule.logger, level='ERROR'),
+        ):
+            self.assertIs(SystemProxyModule.SystemProxy.off(), False)
+
+            self.assertTrue(command.call_args.kwargs['check'])
+            self.assertEqual(command.call_args.kwargs['timeout'], 5.0)
+
     def testPacDiagnosticsDoNotExposeConfiguredUrl(self):
         """Avoid logging credential-bearing PAC URLs on any outcome."""
         proxy = SystemProxyModule._SystemProxy()