mirror of
https://github.com/LorenEteval/Furious.git
synced 2026-10-07 22:38:21 +03:00
fix: avoid logging credential-bearing inputs
Signed-off-by: Loren Eteval <loren.eteval@proton.me>
This commit is contained in:
@@ -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}')
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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})
|
||||
|
||||
|
||||
@@ -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."""
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user