diff --git a/Furious/Application/DesktopApplication.py b/Furious/Application/DesktopApplication.py index 195a36b..836361b 100644 --- a/Furious/Application/DesktopApplication.py +++ b/Furious/Application/DesktopApplication.py @@ -839,7 +839,9 @@ class DesktopApplication(ApplicationRunner, SingletonApplication): logger.info(f'python version: {PLATFORM_PYTHON_VERSION}') logger.info(f'system version: {sys.version}') logger.info(f'sys.executable: {sys.executable}') - logger.info(f'sys.argv: {sys.argv}') + # Arguments may contain imported share links, subscription URLs, or + # plugin-defined commands. Their contents are not diagnostic metadata. + logger.info(f'command-line arguments: {max(len(sys.argv) - 1, 0)} provided') logger.info(f'appFilePath: {self.applicationFilePath()}') logger.info(f'isPythonw: {SystemRuntime.isPythonw()}') logger.info(f'system language is {SYSTEM_LANGUAGE}') diff --git a/Furious/Core/CoreProcessWorker.py b/Furious/Core/CoreProcessWorker.py index 996e1ab..c268e04 100644 --- a/Furious/Core/CoreProcessWorker.py +++ b/Furious/Core/CoreProcessWorker.py @@ -389,7 +389,10 @@ class CoreProcessWorker(CoreProcessMonitor, ABC): def startWithSpec(self, launchSpec: CoreLaunchSpec) -> bool: """Start and validate a child process from a launch specification.""" if not isinstance(launchSpec, CoreLaunchSpec): - logger.error(f'invalid launch spec for {self.name()}: {launchSpec}') + logger.error( + f'invalid launch spec type for {self.name()}: ' + f'{type(launchSpec).__name__}' + ) self.setState(CoreProcessState.Failed) @@ -397,7 +400,8 @@ class CoreProcessWorker(CoreProcessMonitor, ABC): if not callable(launchSpec.target): logger.error( - f'invalid launch target for {self.name()}: {launchSpec.target}' + f'invalid launch target type for {self.name()}: ' + f'{type(launchSpec.target).__name__}' ) self.setState(CoreProcessState.Failed) diff --git a/Furious/Service/SubscriptionManager.py b/Furious/Service/SubscriptionManager.py index 9c2f12a..ab14aee 100644 --- a/Furious/Service/SubscriptionManager.py +++ b/Furious/Service/SubscriptionManager.py @@ -494,6 +494,7 @@ class SubscriptionManager(HttpGetManager): if not self._isCurrentRequest(kwargs): return + unique = kwargs.get('unique', '') remark = kwargs.get('remark', '') webURL = kwargs.get('webURL', '') successArgs = kwargs.get('successArgs', list()) @@ -536,7 +537,7 @@ class SubscriptionManager(HttpGetManager): return logger.info( - f'update subs ({remark}, {webURL}) success. ' + f'update subscription ({remark}, {unique!r}) success. ' f'Got {len(result.profiles)} profiles from {result.decoderId!r}; ' f'rejected {result.rejectedItems}' ) @@ -550,13 +551,13 @@ class SubscriptionManager(HttpGetManager): if not self._isCurrentRequest(kwargs): return + unique = kwargs.get('unique', '') remark = kwargs.get('remark', '') - webURL = kwargs.get('webURL', '') failureArgs = kwargs.get('failureArgs', list()) error = networkReply.errorString() - logger.error(f'update subs ({remark}, {webURL}) failed: {error}') + logger.error(f'update subscription ({remark}, {unique!r}) failed: {error}') failureArgs.append({'error': error, **kwargs}) diff --git a/Furious/Widget/ServerTableView.py b/Furious/Widget/ServerTableView.py index 1e39014..1f43110 100644 --- a/Furious/Widget/ServerTableView.py +++ b/Furious/Widget/ServerTableView.py @@ -2403,8 +2403,6 @@ class ServerTableView( 'config': kwargs.pop('config', ''), 'subsId': kwargs.pop('subsId', ''), } - tostr = f'{model}' - factory = profileFromAny(model.pop('config', ''), **model) if factory.isValid(): @@ -2413,7 +2411,9 @@ class ServerTableView( if acceptInvalid: self.appendNewItemByFactory(factory) else: - logger.error(f'invalid item: {tostr}') + # The rejected input may be a complete JSON configuration or + # share URI containing credentials. + logger.error('invalid server profile input') def exportSelectedItemURI(self): """Export selected item URI.""" diff --git a/tests/test_architecture_refactors.py b/tests/test_architecture_refactors.py index c75c700..affa1e5 100644 --- a/tests/test_architecture_refactors.py +++ b/tests/test_architecture_refactors.py @@ -172,6 +172,91 @@ class ApplicationLifecycleTransactionTest(TestCase): finalExit.assert_called_once_with(23) + def testRuntimeInformationDoesNotLogCommandLinePayloads(self): + """Report argument count without retaining imported secrets in logs.""" + secret = 'token=do-not-log-this' + application = SimpleNamespace( + applicationFilePath=mock.Mock(return_value='Furious'), + customFontLoadMsg='custom font unavailable', + theme=mock.Mock(return_value='Light'), + ) + + with ( + mock.patch( + 'Furious.Application.DesktopApplication.sys.argv', + ['Furious', f'https://example.invalid/sub?{secret}', '--flag'], + ), + self.assertLogs( + 'Furious.Application.DesktopApplication', level='INFO' + ) as logs, + ): + DesktopApplication._logRuntimeInformation(application) + + output = '\n'.join(logs.output) + + self.assertIn('command-line arguments: 2 provided', output) + self.assertNotIn(secret, output) + + def testInvalidCoreLaunchDiagnosticsDoNotRenderPayloads(self): + """Describe invalid launch types without rendering secret arguments.""" + secret = 'password=do-not-log-this' + runtime = SimpleNamespace( + name=mock.Mock(return_value='Probe'), + setState=mock.Mock(), + ) + + class SecretLaunch: + def __repr__(self): + return secret + + with self.assertLogs('Furious.Core.CoreProcessWorker', level='ERROR') as logs: + self.assertFalse( + CoreProcessWorkerModule.CoreProcessWorker.startWithSpec( + runtime, + SecretLaunch(), + ) + ) + self.assertFalse( + CoreProcessWorkerModule.CoreProcessWorker.startWithSpec( + runtime, + CoreProcessWorkerModule.CoreLaunchSpec( + target=secret, + args=(secret,), + ), + ) + ) + + output = '\n'.join(logs.output) + + self.assertIn('SecretLaunch', output) + self.assertIn('str', output) + self.assertNotIn(secret, output) + + def testRejectedServerProfileDiagnosticsDoNotRenderInput(self): + """Reject an invalid profile without logging its complete configuration.""" + from Furious.Widget.ServerTableView import ServerTableView + + secret = 'password=do-not-log-this' + invalidProfile = SimpleNamespace(isValid=mock.Mock(return_value=False)) + + with ( + mock.patch( + 'Furious.Widget.ServerTableView.profileFromAny', + return_value=invalidProfile, + ), + self.assertLogs('Furious.Widget.ServerTableView', level='ERROR') as logs, + ): + ServerTableView.appendNewItem( + SimpleNamespace(), + config=f'{{"password": "{secret}"}}', + remark='Rejected profile', + ) + + output = '\n'.join(logs.output) + + self.assertIn('invalid server profile input', output) + self.assertNotIn(secret, output) + def testFirstInstanceClaimsEndpointWithoutRemovingSocket(self): """Serialize election, then continue only after the endpoint is owned.""" electionLock = mock.Mock() diff --git a/tests/test_subscription_manager.py b/tests/test_subscription_manager.py index 8df98a4..b5fb88b 100644 --- a/tests/test_subscription_manager.py +++ b/tests/test_subscription_manager.py @@ -169,8 +169,8 @@ class SubscriptionManagerTest(TestCase): self.assertFalse(hasattr(manager, 'table')) manager.deleteLater() - def testSubscriptionDiagnosticsIncludeConfiguredURL(self): - """Identify a request with its configured remark and URL.""" + def testSubscriptionDiagnosticsExcludeConfiguredURL(self): + """Identify a request without logging its credential-bearing URL.""" manager = self._manager() profile = SimpleNamespace(itemRemark='profile') manager.importer = SimpleNamespace( @@ -205,8 +205,12 @@ class SubscriptionManagerTest(TestCase): self.assertIn('Group A', infoLog.call_args.args[0]) self.assertIn('Group A', errorLog.call_args.args[0]) - self.assertIn(configuredURL, infoLog.call_args.args[0]) - self.assertIn(configuredURL, errorLog.call_args.args[0]) + self.assertIn('group-a', infoLog.call_args.args[0]) + self.assertIn('group-a', errorLog.call_args.args[0]) + self.assertNotIn(configuredURL, infoLog.call_args.args[0]) + self.assertNotIn(configuredURL, errorLog.call_args.args[0]) + self.assertNotIn('token=value', infoLog.call_args.args[0]) + self.assertNotIn('token=value', errorLog.call_args.args[0]) manager.deleteLater()