feat(logging): add PII scrubbing middleware with custom Winston trans…#242
Open
habnark wants to merge 1 commit into
Open
feat(logging): add PII scrubbing middleware with custom Winston trans…#242habnark wants to merge 1 commit into
habnark wants to merge 1 commit into
Conversation
…port
Implement a two-layer log redaction system to ensure sensitive data is
stripped from all log output before reaching external storage or the
console.
Layer 1 — redactFormat (logMasker.ts + winstonLogger.ts):
A custom Winston format applied globally at the logger level so every
entry is scrubbed before any transport receives it.
Layer 2 — RedactingTransport (redactingTransport.ts):
A new custom Winston Transport class that wraps DailyRotateFile (the
external-storage transport). It applies scrubLogInfo() a second time
as a defense-in-depth measure, catching any data injected after format
processing.
New patterns added to logMasker.ts:
- Private / internal IPv4 ranges (RFC 1918: 10.x, 172.16-31.x,
192.168.x; loopback: 127.x; link-local: 169.254.x) → [INTERNAL_IP]
- IPv6 loopback (::1) and ULA (fd00::/8) → [INTERNAL_IP]
- JWT tokens (three base64url segments) → [REDACTED]
- Discord and Slack webhook URLs → [REDACTED_URL]
- Generic key=value API key assignments → [REDACTED]
- Object keys matching admin/cluster/node_ip/server_ip → [REDACTED]
- Improved Stellar secret key pattern (exactly 56 chars)
- Regex lastIndex reset so global patterns work on repeated calls
New exports from logMasker.ts:
- scrubLogInfo(info): sanitises a Winston log info object, preserving
level/timestamp and all Symbol-keyed Winston internals unchanged.
winstonLogger.ts:
- Imports redactFormat and RedactingTransport
- Wraps DailyRotateFile with RedactingTransport
- Respects LOG_LEVEL env var (falls back to 'info')
- Keeps fetcherError convenience method with proper TypeScript typing
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@habnark Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #210
Log Redaction & PII Scrubbing Middleware — Full Description
Problem Being Solved
The existing
logMasker.tsonly patchedconsole.*methods. The Winston logger itself — which writes to rotating log files (external storage) — had no scrubbing at all. Any secret, private IP, JWT, or webhook URL passed tologger.info()/logger.error()was written verbatim to disk and any downstream log aggregator. This is a data exposure risk for admin credentials, internal network topology, and third-party API keys.Architecture: Two Independent Scrubbing Layers
File 1:
src/utils/logMasker.ts(expanded)New regex patterns added:
Pattern | What it catches | Replacement -- | -- | -- 10.x.x.x | RFC 1918 class-A private IPs | [INTERNAL_IP] 172.16-31.x.x | RFC 1918 class-B private IPs | [INTERNAL_IP] 192.168.x.x | RFC 1918 class-C private IPs | [INTERNAL_IP] 127.x.x.x | Loopback addresses | [INTERNAL_IP] 169.254.x.x | Link-local addresses | [INTERNAL_IP] ::1 | IPv6 loopback | [INTERNAL_IP] fd00::/8 | IPv6 Unique Local Addresses | [INTERNAL_IP] eyXXX.XXX.XXX | JWT tokens (3 base64url segments) | [REDACTED] discord.com/api/webhooks/… | Discord webhook URLs | [REDACTED_URL] hooks.slack.com/services/… | Slack webhook URLs | [REDACTED_URL] api_key=xxxxx | Generic key=value assignments | [REDACTED] Object key admin, cluster, node_ip, server_ip | Admin/internal fields in objects | [REDACTED]Fixed bugs in the existing code:
lastIndexbefore eachreplace()call — previously, stateful regexes silently skipped every second match when called on different stringsNew export —
scrubLogInfo(info):messageand all string/object metadata fieldslevel,timestamp, and allSymbol-keyed Winston internals (Symbol(level),Symbol(splat)) completely untouched — altering these breaks Winston's internal pipelineFile 2:
src/utils/redactingTransport.ts(new file)A custom Winston
Transportclass — the "transport layer for scrubbing" the spec requires.Key design decisions:
DailyRotateFile— stays generic so any transport (HTTP, S3, Datadog, etc.) can be wrapped by passing it asinnerinner.logdoesn't exist (graceful fallback)'logged'after the inner transport confirms the write — correct backpressure handling so Winston doesn't fill its internal buffer under high log volumeFile 3:
src/utils/winstonLogger.ts(updated)Changes:
redactFormat— aformat(info => scrubLogInfo(info))()custom Winston format applied at the logger level, meaning it runs before the entry reaches either transportDailyRotateFileis now instantiated privately and wrapped innew RedactingTransport({ inner: dailyRotateFileTransport })— callers only ever interact with the scrubbing wrapperLOG_LEVELenvironment variable now respected (process.env.LOG_LEVEL ?? 'info') so log verbosity can be adjusted per environment without code changesfetcherErrorconvenience method now has correct TypeScript typing (was previously typed asany)handleExceptions: true/handleRejections: trueflags so unhandled errors and promise rejections continue to be captured