From 0d7eb5cf7f4555047dcfc431c0e457410fec0977 Mon Sep 17 00:00:00 2001 From: Stephan Schuler Date: Fri, 18 Oct 2024 14:31:33 +0200 Subject: [PATCH 1/2] feat: Enhanced variable scrubbing and context vars Scrub frame variables manually in case scrubbing isn't already done by zend.exception_ignore_args. This allows for individual per-frame decissions to either drop certain variables or keep them. It doesn't change, however, how frame variables are transported to Sentry, which usually is very none-descriptive due to how Sentry handles variable repreesntation. To add more details to variables, specific frame variables can be analyzed and copied from frames to the event context befor being potentially scrubbed and stringified. --- Classes/Command/SentryCommandController.php | 5 + Classes/Integration/NetlogixIntegration.php | 53 +++++- Classes/Scope/Context/ContextProvider.php | 14 ++ .../Extra/VariablesFromStackProvider.php | 161 ++++++++++++++++++ Classes/Scope/ScopeProvider.php | 17 ++ Configuration/Settings.Providers.yaml | 1 + Configuration/Settings.VariableScrubbing.yaml | 38 +++++ README.md | 107 ++++++++++++ 8 files changed, 394 insertions(+), 2 deletions(-) create mode 100644 Classes/Scope/Context/ContextProvider.php create mode 100644 Classes/Scope/Extra/VariablesFromStackProvider.php create mode 100644 Configuration/Settings.VariableScrubbing.yaml diff --git a/Classes/Command/SentryCommandController.php b/Classes/Command/SentryCommandController.php index 5405953..bd033ab 100644 --- a/Classes/Command/SentryCommandController.php +++ b/Classes/Command/SentryCommandController.php @@ -45,6 +45,11 @@ public function showScopeCommand(): void $this->outputLine(); + $this->outputLine('Scope Extra:'); + \Neos\Flow\var_dump($this->scopeProvider->collectContexts()); + + $this->outputLine(); + $this->outputLine('Scope Release:'); \Neos\Flow\var_dump($this->scopeProvider->collectRelease()); diff --git a/Classes/Integration/NetlogixIntegration.php b/Classes/Integration/NetlogixIntegration.php index dca54bf..cac2f38 100644 --- a/Classes/Integration/NetlogixIntegration.php +++ b/Classes/Integration/NetlogixIntegration.php @@ -106,16 +106,17 @@ function ($exception) { private static function rewriteStacktraceAndFlagInApp(Stacktrace $stacktrace): Stacktrace { $frames = array_map(function ($frame) { + $functionName = self::replaceProxyClassName($frame->getFunctionName()); $classPathAndFilename = self::getOriginalClassPathAndFilename($frame->getFile()); return new Frame( - self::replaceProxyClassName($frame->getFunctionName()), + $functionName, $classPathAndFilename, $frame->getLine(), self::replaceProxyClassName($frame->getRawFunctionName()), $frame->getAbsoluteFilePath() ? Files::concatenatePaths([FLOW_PATH_ROOT, trim($classPathAndFilename, '/')]) : null, - $frame->getVars(), + self::scrubVariablesFromFrame((string)$functionName, $frame->getVars()), self::isInApp($classPathAndFilename) ); }, $stacktrace->getFrames()); @@ -159,6 +160,51 @@ private static function isInApp(string $path): bool return true; } + private static function scrubVariablesFromFrame($traceFunction, $frameVariables): array + { + if (!$frameVariables) { + return $frameVariables; + } + assert(is_array($frameVariables)); + + $config = Bootstrap::$staticObjectManager + ->get(ConfigurationManager::class) + ->getConfiguration( + ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, + 'Netlogix.Sentry.variableScrubbing' + ) ?? []; + + $scrubbing = (bool)($config['scrubbing'] ?? false); + if (!$scrubbing) { + return $frameVariables; + } + + $keep = $config['keepFromScrubbing'] ?? []; + if (!$keep) { + return []; + } + + $result = []; + $traceFunction = str_replace('_Original::', '::', $traceFunction); + foreach ($keep as $keepConfig) { + try { + ['className' => $className, 'methodName' => $methodName, 'arguments' => $arguments] = $keepConfig; + $configFunction = $className . '::' . $methodName; + if ($configFunction !== $traceFunction) { + continue; + } + foreach ($arguments as $argumentName) { + $result[$argumentName] = $frameVariables[$argumentName] ?? '👻'; + } + + } catch (\Exception $e) { + } + + } + + return $result; + } + private static function configureScopeForEvent(Event $event, EventHint $hint): void { try { @@ -170,6 +216,9 @@ private static function configureScopeForEvent(Event $event, EventHint $hint): v $configureEvent = function () use ($event, $scopeProvider) { $event->setEnvironment($scopeProvider->collectEnvironment()); $event->setExtra($scopeProvider->collectExtra()); + foreach ($scopeProvider->collectContexts() as $key => $value) { + $event->setContext($key, $value); + } $event->setRelease($scopeProvider->collectRelease()); $event->setTags($scopeProvider->collectTags()); $userData = $scopeProvider->collectUser(); diff --git a/Classes/Scope/Context/ContextProvider.php b/Classes/Scope/Context/ContextProvider.php new file mode 100644 index 0000000..6c66c89 --- /dev/null +++ b/Classes/Scope/Context/ContextProvider.php @@ -0,0 +1,14 @@ + + */ + public function getContexts(): array; + +} diff --git a/Classes/Scope/Extra/VariablesFromStackProvider.php b/Classes/Scope/Extra/VariablesFromStackProvider.php new file mode 100644 index 0000000..5411152 --- /dev/null +++ b/Classes/Scope/Extra/VariablesFromStackProvider.php @@ -0,0 +1,161 @@ +collectDataFromTraversables(), false); + if ($result) { + return ['Method Arguments' => $result]; + } else { + return []; + } + } + + private function collectDataFromTraversables(): Traversable + { + $throwable = $this->scopeProvider->getCurrentThrowable(); + while ($throwable instanceof Throwable) { + yield from $this->collectDataFromTraces($throwable); + $throwable = $throwable->getPrevious(); + } + } + + private function collectDataFromTraces(Throwable $throwable): Traversable + { + $traces = $throwable->getTrace(); + foreach ($traces as $trace) { + yield from $this->collectDataFromTrace($trace); + } + } + + private function collectDataFromTrace(array $trace): Traversable + { + $traceFunction = self::callablePattern($trace['class'] ?? '', $trace['function'] ?? ''); + + $settings = iterator_to_array($this->getSettings(), false); + foreach ($settings as ['className' => $className, 'methodName' => $methodName, 'argumentPaths' => $argumentPaths]) { + $configFunction = self::callablePattern($className, $methodName); + if ($traceFunction !== $configFunction) { + continue; + } + $values = []; + foreach ($argumentPaths as $argumentPathName => $argumentPathLookup) { + try { + $values[$argumentPathName] = $this->representationSerialize( + ObjectAccess::getPropertyPath($trace['args'], $argumentPathLookup) + ); + } catch (Throwable $t) { + $values[$argumentPathName] = '👻'; + } + } + yield [$configFunction => $values]; + } + } + + private function representationSerialize($value) + { + static $representationSerialize; + + if (!$representationSerialize) { + $client = SentrySdk::getCurrentHub()->getClient(); + if ($client) { + $serializer = new RepresentationSerializer($client->getOptions()); + $representationSerialize = function($value) use ($serializer) { + return $serializer->representationSerialize($value); + }; + } else { + $representationSerialize = function($value) { + return json_encode($value); + }; + } + } + + return $representationSerialize($value); + } + + private function getSettings(): Traversable + { + foreach ($this->settings as $config) { + $className = $config['className'] ?? null; + if (!$className || !class_exists($className)) { + continue; + } + + $methodName = $config['methodName'] ?? null; + if (!$methodName || !method_exists($className, $methodName)) { + continue; + } + + if (!is_array($config['arguments'])) { + continue; + } + + $argumentPaths = array_filter($config['arguments'] ?? [], function ($argumentPath) { + return is_string($argumentPath) && $argumentPath; + }); + $argumentPaths = array_combine($argumentPaths, $argumentPaths); + + $reflection = new MethodReflection($className, $methodName); + foreach ($reflection->getParameters() as $parameter) { + $search = sprintf('/^%s./', $parameter->getName()); + $replace = sprintf('%d.', $parameter->getPosition()); + $argumentPaths = preg_replace($search, $replace, $argumentPaths); + } + + yield [ + 'className' => $className, + 'methodName' => $methodName, + 'argumentPaths' => $argumentPaths + ]; + yield [ + 'className' => $className . '_Original', + 'methodName' => $methodName, + 'argumentPaths' => $argumentPaths + ]; + } + } + + private function callablePattern(string $className, string $methodName): string + { + return sprintf(self::FUNCTION_PATTERN, $className, $methodName); + } +} diff --git a/Classes/Scope/ScopeProvider.php b/Classes/Scope/ScopeProvider.php index 69c98d9..7943bed 100644 --- a/Classes/Scope/ScopeProvider.php +++ b/Classes/Scope/ScopeProvider.php @@ -7,6 +7,7 @@ use Neos\Flow\ObjectManagement\ObjectManagerInterface; use Neos\Utility\PositionalArraySorter; use Netlogix\Sentry\Exception\InvalidProviderType; +use Netlogix\Sentry\Scope\Context\ContextProvider; use Netlogix\Sentry\Scope\Environment\EnvironmentProvider; use Netlogix\Sentry\Scope\Extra\ExtraProvider; use Netlogix\Sentry\Scope\Release\ReleaseProvider; @@ -24,6 +25,7 @@ class ScopeProvider private const SCOPE_ENVIRONMENT = 'environment'; private const SCOPE_EXTRA = 'extra'; + private const SCOPE_CONTEXTS = 'contexts'; private const SCOPE_RELEASE = 'release'; private const SCOPE_TAGS = 'tags'; private const SCOPE_USER = 'user'; @@ -31,6 +33,7 @@ class ScopeProvider private const SCOPE_TYPE_MAPPING = [ self::SCOPE_ENVIRONMENT => EnvironmentProvider::class, self::SCOPE_EXTRA => ExtraProvider::class, + self::SCOPE_CONTEXTS => ContextProvider::class, self::SCOPE_RELEASE => ReleaseProvider::class, self::SCOPE_TAGS => TagProvider::class, self::SCOPE_USER => UserProvider::class, @@ -52,6 +55,7 @@ class ScopeProvider protected $providers = [ self::SCOPE_ENVIRONMENT => [], self::SCOPE_EXTRA => [], + self::SCOPE_CONTEXTS => [], self::SCOPE_RELEASE => [], self::SCOPE_TAGS => [], self::SCOPE_USER => [], @@ -98,6 +102,19 @@ public function collectExtra(): array return $extra; } + public function collectContexts(): array + { + $contexts = []; + + foreach ($this->providers[self::SCOPE_CONTEXTS] as $provider) { + assert($provider instanceof ContextProvider); + + $extra = array_merge_recursive($extra, $provider->getContexts()); + } + + return $contexts; + } + public function collectRelease(): ?string { $release = null; diff --git a/Configuration/Settings.Providers.yaml b/Configuration/Settings.Providers.yaml index d718c38..ad5caf8 100644 --- a/Configuration/Settings.Providers.yaml +++ b/Configuration/Settings.Providers.yaml @@ -8,6 +8,7 @@ Netlogix: extra: 'Netlogix\Sentry\Scope\Extra\ReferenceCodeProvider': true + 'Netlogix\Sentry\Scope\Extra\VariablesFromStackProvider': true release: # See Configuration/Settings.Release.yaml for settings diff --git a/Configuration/Settings.VariableScrubbing.yaml b/Configuration/Settings.VariableScrubbing.yaml new file mode 100644 index 0000000..5be2ef9 --- /dev/null +++ b/Configuration/Settings.VariableScrubbing.yaml @@ -0,0 +1,38 @@ +Netlogix: + Sentry: + + variableScrubbing: + + scrubbing: true + + keepFromScrubbing: + + 'Neos\ContentRepository\Search\Indexer\NodeIndexingManager::indexNode()': + className: 'Neos\ContentRepository\Search\Indexer\NodeIndexingManager' + methodName: 'indexNode' + arguments: + - 'node' + + 'Flowpack\ElasticSearch\ContentRepositoryAdaptor\Indexer\NodeIndexer::indexNode()': + className: 'Flowpack\ElasticSearch\ContentRepositoryAdaptor\Indexer\NodeIndexer' + methodName: 'indexNode' + arguments: + - 'node' + + contextDetails: + + 'Neos\ContentRepository\Search\Indexer\NodeIndexingManager::indexNode()': + className: 'Neos\ContentRepository\Search\Indexer\NodeIndexingManager' + methodName: 'indexNode' + arguments: + - 'node.path' + - 'node.identifier' + - 'node.name' + + 'Flowpack\ElasticSearch\ContentRepositoryAdaptor\Indexer\NodeIndexer::indexNode()': + className: 'Flowpack\ElasticSearch\ContentRepositoryAdaptor\Indexer\NodeIndexer' + methodName: 'indexNode' + arguments: + - 'node.path' + - 'node.identifier' + - 'node.name' diff --git a/README.md b/README.md index a43cd8c..e052555 100644 --- a/README.md +++ b/README.md @@ -266,3 +266,110 @@ Neos: patternOptions: controllerObjectNamePattern: 'Netlogix\Sentry\Controller\.*' ``` + +## Scrubbing variables or adding more details + +### Allow in php.ini config + +Context variables depend on zend not removing exception arguments. +Make sure zend is not configured to remove arguments from exceptions to +make use of fine-grained variable scrubbing and context-aware variable details. + +```ìni +[php] +zend.exception_ignore_args=0 +``` + +### Scrub all frame arguments + +Ignoring exception arguments via zend setting would just remove every exception +argument. +Scrubbing can be re-added manually in userspace code by config settings. +This might be helpful for either different environments or deployment targets. + +```yaml +Netlogix: + Sentry: + variableScrubbing: + scrubbing: true +``` + +### Prevent specific arguments from being scrubbed + +In contrast to zend.exception_ignore_args, having them scrubbed manually allows +for keeping very specific variables by naming them individually. + +```yaml +Netlogix: + Sentry: + + variableScrubbing: + + # Only works when manual scrubbing is enabled. Otherwise, all data is + # kept anyway. + + scrubbing: true + + # Keep certain frame variables even if scrubbing is enabled + # + # In contrast to using an individual RepresentationSerializer, this is context + # aware. Not every string should be displayed, but some might. + # + # Complex arguments might be obfuscated when being passed through the default + # Sentry RepresentationSerializerInterface::representationSerialize() and not very + # useful. Use our VariablesFromStackProvider and the "contextDetails" setting for + # more detailed logging. + + keepFromScrubbing: + + 'Neos\ContentRepository\Search\Indexer\NodeIndexingManager::indexNode()': + + # Class names cover "_Original" as well, but neither implementing + # interfaces nor extending classes. This only matches by exact string + # comparison. + + className: 'Neos\ContentRepository\Search\Indexer\NodeIndexingManager' + methodName: 'indexNode' + + + # Arguments can only be argument names, not argument paths. Preventing + # certain arguments form being scrubbed happens after the Sentry + # RepresentationSerializer converted them, so no actual object + # information is available for detailed matching. + + arguments: + - 'node' +``` + +### Add specific object information of frame variables to the sentry event context + +Data that is not ony "a representation of" a method argument but somewhere +within any given data structure can be extracted and adde to the sentry event. + +This covers way more details about any given event frame, as long as the +information is accessible by either public properties or a getter method. + +```yaml +Netlogix: + Sentry: + + variableScrubbing: + + # In contrast to scrubbing and keeping specific arguments from being + # scrubbed, this context vars are added "in full" to the event context. + + contextDetails: + + 'Neos\ContentRepository\Search\Indexer\NodeIndexingManager::indexNode()': + className: 'Neos\ContentRepository\Search\Indexer\NodeIndexingManager' + methodName: 'indexNode' + arguments: + + # It's necessary to specify exactly which property paths should be + # added to prevent loosing information. Data not being specified is + # just dropped. + + - 'node.path' + - 'node.identifier' + - 'node.name' +``` From 6c8c3efb85cd1aa9e205be39df638ac20dbe8e20 Mon Sep 17 00:00:00 2001 From: Stephan Schuler Date: Fri, 25 Oct 2024 15:51:41 +0200 Subject: [PATCH 2/2] fix: Missing typing in method arguments Co-authored-by: Lars Lauger --- Classes/Integration/NetlogixIntegration.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Classes/Integration/NetlogixIntegration.php b/Classes/Integration/NetlogixIntegration.php index cac2f38..48822cc 100644 --- a/Classes/Integration/NetlogixIntegration.php +++ b/Classes/Integration/NetlogixIntegration.php @@ -160,7 +160,7 @@ private static function isInApp(string $path): bool return true; } - private static function scrubVariablesFromFrame($traceFunction, $frameVariables): array + private static function scrubVariablesFromFrame(string $traceFunction, array $frameVariables): array { if (!$frameVariables) { return $frameVariables;