-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add first-party RedactSensitiveProcessor to mask secrets in logs (m…
#1983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| <?php | ||
| declare(strict_types=1); | ||
|
|
||
| namespace Monolog\Processor; | ||
|
|
||
| use Monolog\LogRecord; | ||
|
|
||
| final class RedactSensitiveProcessor implements ProcessorInterface | ||
| { | ||
| /** @var string[] */ | ||
| private array $sensitiveKeys; | ||
|
|
||
| /** @var string[] */ | ||
| private array $patterns; | ||
|
|
||
| private string $mask; | ||
|
|
||
| /** | ||
| * @param string[] $sensitiveKeys exact keys to redact in context/extra (case-insensitive) | ||
| * @param string[] $patterns PCRE regex patterns to mask inside string values (e.g. '/Bearer\\s+[A-Za-z0-9\\._-]+/') | ||
| * @param string $mask replacement token | ||
| */ | ||
| public function __construct(array $sensitiveKeys = ['password','passwd','pwd','secret','token','api_key','apikey','authorization','auth','cookie'], array $patterns = [], string $mask = 'REDACTED') | ||
| { | ||
| $this->sensitiveKeys = array_map('strtolower', $sensitiveKeys); | ||
| $this->patterns = $patterns; | ||
| $this->mask = $mask; | ||
| } | ||
|
|
||
| public function __invoke(LogRecord $record): LogRecord | ||
| { | ||
| $context = $this->sanitize($record->context); | ||
| $extra = $this->sanitize($record->extra); | ||
|
|
||
| $message = $record->message; | ||
| foreach ($this->patterns as $pattern) { | ||
| if (@preg_match($pattern, '') === false) { | ||
| continue; // ignore invalid pattern instead of throwing inside logging path | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure we want to silently ignore failures. If the pattern is invalid and ignored silently, nothing will tell the dev that his expected masking won't heppe.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for pointing that out — you're absolutely right. Silently ignoring an invalid regex could lead to unexpected behavior and make debugging harder. To improve transparency while preserving resilience, I’ll change the implementation to log a warning via error_log() whenever a pattern fails to compile, instead of failing silently. That way: Developers are notified of the issue (e.g., in error logs) The processor continues working safely without interrupting log flow Let me know if you’d prefer an alternative approach (like throwing on invalid regex only in a strict/dev mode).
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO the best would be to run this pattern check once in the constructor, and if anything returns false you simply throw an InvalidArgumentException. That is how we handle all other misconfigurations in Monolog. |
||
| } | ||
| $message = preg_replace($pattern, $this->mask, $message) ?? $message; | ||
| } | ||
|
|
||
| return $record->with( | ||
| message: $message, | ||
| context: $context, | ||
| extra: $extra | ||
| ); | ||
| } | ||
|
|
||
| /** @param mixed $value */ | ||
| private function sanitize(mixed $value): mixed | ||
| { | ||
| if (is_array($value)) { | ||
| $sanitized = []; | ||
| foreach ($value as $k => $v) { | ||
| $key = \is_string($k) ? strtolower($k) : $k; | ||
| if (is_string($k) && in_array($key, $this->sensitiveKeys, true)) { | ||
| $sanitized[$k] = $this->mask; | ||
| } else { | ||
| $sanitized[$k] = $this->sanitize($v); | ||
| } | ||
| } | ||
| return $sanitized; | ||
| } | ||
|
|
||
| if (is_string($value) && $this->patterns) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The check on |
||
| foreach ($this->patterns as $pattern) { | ||
| if (@preg_match($pattern, '') === false) { | ||
| continue; | ||
| } | ||
| $value = preg_replace($pattern, $this->mask, $value) ?? $value; | ||
| } | ||
| } | ||
|
|
||
| return $value; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,58 @@ | ||
| <?php | ||
| declare(strict_types=1); | ||
|
|
||
| namespace Monolog\Processor; | ||
|
|
||
| use Monolog\Level; | ||
| use Monolog\LogRecord; | ||
| use PHPUnit\Framework\TestCase; | ||
|
|
||
| final class RedactSensitiveProcessorTest extends TestCase | ||
| { | ||
| public function testRedactsContextAndExtraKeys(): void | ||
| { | ||
| $p = new RedactSensitiveProcessor(); | ||
|
|
||
| $rec = new LogRecord( | ||
| datetime: new \DateTimeImmutable('@0'), | ||
| channel: 'test', | ||
| level: Level::Info, | ||
| message: 'Login for {user}', | ||
| context: ['user' => 'rishi', 'password' => 'super-secret', 'nested' => ['api_key' => 'abc123']], | ||
| extra: ['token' => 't-456', 'irrelevant' => 'keep'] | ||
| ); | ||
|
|
||
| $out = $p($rec); | ||
|
|
||
| $this->assertSame('REDACTED', $out->context['password']); | ||
| $this->assertSame('REDACTED', $out->context['nested']['api_key']); | ||
| $this->assertSame('REDACTED', $out->extra['token']); | ||
| $this->assertSame('keep', $out->extra['irrelevant']); | ||
| } | ||
|
|
||
| public function testRedactsWithPatterns(): void | ||
| { | ||
| $p = new RedactSensitiveProcessor( | ||
| sensitiveKeys: [], | ||
| patterns: ['/(Bearer\\s+)[A-Za-z0-9\\._-]+/i', '/([\\w.%+-]+@[\\w.-]+\\.[A-Za-z]{2,})/'] | ||
| ); | ||
|
|
||
| $rec = new LogRecord( | ||
| new \DateTimeImmutable('@0'), 'test', Level::Info, | ||
| 'Auth {h}: Bearer abc.def-ghi and user [email protected]', | ||
| ['h' => 'header'], [] | ||
| ); | ||
|
|
||
| $out = $p($rec); | ||
| $this->assertSame('Auth {h}: REDACTED and user REDACTED', $out->message); | ||
| } | ||
|
|
||
| public function testIgnoresInvalidRegexSafely(): void | ||
| { | ||
| $p = new RedactSensitiveProcessor([], ['/[invalid/']); | ||
| $rec = new LogRecord(new \DateTimeImmutable('@0'), 'test', Level::Info, 'hello', [], []); | ||
| $out = $p($rec); | ||
| $this->assertSame('hello', $out->message); | ||
| } | ||
| } | ||
| ?> |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just an idea.. not sure if good or bad, but this should catch any long word of 30+ [a-z0-9] characters (optionally prefixed by foo_bar_ or the like, like many API tokens are), which is very likely to be a secret/token.. But of course it risks some collateral damage.