Skip to content

Commit 33ab2dc

Browse files
authored
Merge pull request #14 from CdeJong/KD-0887-v1-5-scrubber-hardening
feat(scrubber): v1.5 hardening — elfproef BSN, secret-class expansion, path/DB carrier-strip (KD-0887)
2 parents 25327c0 + 19a9c23 commit 33ab2dc

12 files changed

Lines changed: 474 additions & 66 deletions

File tree

CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,17 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and
66

77
## [Unreleased]
88

9+
### Added
10+
11+
- **API-key prefix and IPv4 scrubbing (KD-0887).** The Scrubber now redacts Stripe-style `sk_live_...` keys, AWS `AKIA...` access key IDs, and IPv4 addresses, in addition to the existing JWT / Bearer / BSN / email coverage.
12+
- **Database DSN password scrubbing (KD-0887).** Any `scheme://user:pass@host` credential — not just a fixed list of DB scheme names — has its password redacted (`scheme://user:[REDACTED:dsn-password]@host`), closing the most severe gap named in the KD-0885 audit debrief.
13+
- **`QueryException` / `PDOException` carrier-strip (KD-0887).** A `QueryException` message embeds the full SQL string with bound parameter values interpolated in — free-text data (a name, an address, a care-data note) that is not a regex-able secret shape and previously reached the payload unredacted on the most common database-error path (surfaced by the emmie annexation, which carries NEN 7510 / AVG care data). `ErrorTracker` now replaces any `PDOException`-family message with `class [SQLSTATE x] [driver code y]` before the Scrubber even runs, dropping the SQL and bindings entirely while preserving the fingerprint.
14+
- **`PathNormalizer` username redaction fallback (KD-0887, M-3).** A stack frame whose path does not start with `base_path()` (vendor installed outside the app root, a globally-installed tool) previously leaked the absolute path verbatim, including the OS username. A secondary pass now redacts the username segment of any `/home/<user>/` or `/Users/<user>/` shape in those frames.
15+
16+
### Fixed
17+
18+
- **BSN eleven-test (Dutch: elfproef) validation (KD-0887, M-1).** The KD-0885 fix widened the BSN pattern to any run of 9-or-more digits, which resolved the missed-detection leak but over-redacted legitimate 9+-digit IDs (order numbers, invoice IDs, timestamps) as `[REDACTED:bsn]`. BSN candidates — both grouped (`123.456.782`) and bare digit runs — are now validated against the eleven-test checksum before redaction, so only a number that is actually shaped like a real BSN redacts. A digit run is scanned for any contiguous 9-digit window that passes, so a real BSN embedded in a longer number is still caught without redacting the surrounding digits.
19+
920
## [0.1.1] — 2026-06-29
1021

1122
### Fixed

CLAUDE.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ The client targets the kendo error-events ingestion endpoint shipped by KD-0771:
2626

2727
| Class | Responsibility |
2828
|---|---|
29-
| `ErrorTracker` | Public surface. `report(\Throwable)`: builds the scrubbed/normalized payload synchronously, then sends inline (sync) or dispatches `ReportErrorJob` (async). `send(array)`: the HTTP POST, swallow-on-failure. Reads config live from the `Config` repository. |
30-
| `Scrubber` | Redacts JWT / Bearer / BSN / email from a string. |
29+
| `ErrorTracker` | Public surface. `report(\Throwable)`: builds the scrubbed/normalized payload synchronously, then sends inline (sync) or dispatches `ReportErrorJob` (async). Any `\PDOException` (including Laravel's `QueryException`, which extends it) gets a carrier-strip first — the message becomes `class [SQLSTATE x] [driver code y]`, dropping the SQL string and bound parameter values before the Scrubber even runs. `send(array)`: the HTTP POST, swallow-on-failure. Reads config live from the `Config` repository. |
30+
| `Scrubber` | Redacts JWT / Bearer / DSN password / API-key prefix / IPv4 / BSN (eleven-test validated) / email from a string. |
3131
| `PathNormalizer` | Exact `base_path()` prefix strip of every frame (mirrors `laravel/nightwatch`'s `Location::normalizeFile()`). |
3232
| `Jobs\ReportErrorJob` | Async carrier for the already-scrubbed payload. `$tries = 1` (0 retries); `failed()` logs to `error_log`, never requeues. |
3333
| `ErrorTrackerServiceProvider` | Auto-discovered. Merges + publishes config; binds `ErrorTracker` + `PathNormalizer` (wired to the app's `base_path()`). |
@@ -54,6 +54,6 @@ The client targets the kendo error-events ingestion endpoint shipped by KD-0771:
5454

5555
SemVer. Pre-1.0 (`0.x`): minor bumps are treated as breaking (Composer's `^0.x` caret locks at minor). `main` is always release-ready; PRs update `CHANGELOG.md` under `[Unreleased]`; a release PR moves it to a versioned heading and tags the merge commit (`v0.x.y`). Packagist's webhook picks up the tag; `release.yml` re-runs CI and creates the GitHub release.
5656

57-
## Out of scope (v1)
57+
## Out of scope
5858

59-
Client-side coalescing/debounce (server owns dedup + rate limit), per-project custom scrub rules (v1.5), a framework-agnostic core (Laravel-only), a JS/TS client (v1.5+), Sentry shim, context fields (schema-banned), API-key-prefix / IPv4 scrubbing (v1.5).
59+
Client-side coalescing/debounce (server owns dedup + rate limit), per-project custom scrub rules (v1.5), a framework-agnostic core (Laravel-only), a JS/TS client (v1.5+), Sentry shim, context fields (schema-banned), phone numbers / session IDs as scrub patterns (no shape distinct enough to redact without a high false-positive rate), a `RequestException` message-body carrier-strip (raised as a "consider" in the KD-0887 discussion, referencing the war-room Nightwatch-egress recon — no concrete shape/threshold was specified, so it's deferred pending that follow-up rather than guessed at here).

README.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,16 @@ Before send, the message and stack trace are scrubbed of the following patterns
118118
|---|---|
119119
| JWT | `eyJhbGc...` (three base64url segments) |
120120
| Bearer token | `Bearer <credential>` |
121-
| BSN (Dutch citizen service number) | a 9-digit run |
121+
| Database DSN password | `mysql://user:pass@host` — only the password is redacted |
122+
| API-key prefix | `sk_live_...`, `AKIA...` |
123+
| IPv4 address | `192.168.1.42` |
124+
| BSN (Dutch citizen service number) | a 9-digit run passing the eleven-test (Dutch: elfproef) checksum |
122125
| Email address | `user@example.com` |
123126

127+
BSN candidates are checksum-validated (the eleven-test) before redaction, so an arbitrary 9+-digit ID (an order number, invoice ID, or timestamp) is not falsely redacted, and a real BSN embedded in a longer digit run (e.g. a phone number) is still caught.
128+
129+
Free-text PII that isn't a fixed secret shape — a name, address, or care-data value embedded in a database error message — is not covered by pattern matching. `QueryException` and `PDOException` are instead handled by a per-exception-type carrier-strip: the message is replaced with just the exception class, SQLSTATE, and driver error code, dropping the SQL string and bound parameter values entirely.
130+
124131
## Path normalization
125132

126133
Each stack frame's absolute path has the app's own `base_path()` stripped (an **exact** prefix removal, mirroring `laravel/nightwatch`'s `Location::normalizeFile()`). The same exception thrown from `/var/www/html/app/Foo.php` and `/home/forge/app/Foo.php` normalizes to the identical `app/Foo.php`, so kendo fingerprints it once regardless of deploy root.

src/ErrorTracker.php

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Illuminate\Contracts\Config\Repository as Config;
99
use Illuminate\Contracts\Container\Container;
1010
use Illuminate\Http\Client\Factory as HttpFactory;
11+
use PDOException;
1112
use ScriptDevelopment\KendoErrorTracker\Jobs\ReportErrorJob;
1213
use Throwable;
1314

@@ -125,7 +126,7 @@ public function send(array $payload): void
125126
*/
126127
private function buildPayload(Throwable $throwable): array
127128
{
128-
$message = $this->scrubber->scrub($throwable->getMessage());
129+
$message = $this->scrubber->scrub($this->safeMessage($throwable));
129130
$stackTrace = $this->scrubber->scrub(
130131
$this->pathNormalizer->normalize($throwable->getTraceAsString()),
131132
);
@@ -145,6 +146,44 @@ private function buildPayload(Throwable $throwable): array
145146
return array_filter($payload, static fn(mixed $value): bool => $value !== null);
146147
}
147148

149+
/**
150+
* Resolve the message to send: a database-carrier strip for any
151+
* `PDOException` (including Laravel's `QueryException`, which extends
152+
* it), otherwise the exception's own message unchanged (still passed
153+
* through the Scrubber afterwards either way).
154+
*
155+
* A `QueryException` message embeds the full SQL string with bound
156+
* parameter values interpolated in — free-text data (a name, an address,
157+
* a care-data note) that is not a regex-able secret shape and would
158+
* otherwise leak on the most common database-error path. The fingerprint
159+
* (exception class + SQLSTATE + driver error code) survives; the bound
160+
* values do not.
161+
*/
162+
private function safeMessage(Throwable $throwable): string
163+
{
164+
return $throwable instanceof PDOException
165+
? $this->databaseCarrierMessage($throwable)
166+
: $throwable->getMessage();
167+
}
168+
169+
/**
170+
* Build the class + SQLSTATE + driver-error-code fingerprint that
171+
* replaces a database exception's message. `errorInfo` is PDO's
172+
* `[SQLSTATE, driver code, driver message]` triple; `QueryException`
173+
* copies it from its wrapped PDOException. Either piece may be absent
174+
* (mocked/manually-constructed exceptions, or a previous exception that
175+
* was not itself a PDOException), so both fall back to "unknown".
176+
*/
177+
private function databaseCarrierMessage(PDOException $throwable): string
178+
{
179+
$errorInfo = $throwable->errorInfo;
180+
181+
$sqlState = isset($errorInfo[0]) && is_scalar($errorInfo[0]) ? (string) $errorInfo[0] : 'unknown';
182+
$driverCode = isset($errorInfo[1]) && is_scalar($errorInfo[1]) ? (string) $errorInfo[1] : 'unknown';
183+
184+
return sprintf('%s [SQLSTATE %s] [driver code %s]', $throwable::class, $sqlState, $driverCode);
185+
}
186+
148187
/**
149188
* Read a string config value, narrowing the repository's mixed return.
150189
* Non-scalar / null values collapse to an empty string.

src/PathNormalizer.php

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use const DIRECTORY_SEPARATOR;
88

9+
use function preg_replace;
910
use function str_replace;
1011

1112
/**
@@ -18,9 +19,20 @@
1819
* mirroring `laravel/nightwatch`'s `Location::normalizeFile()` — not a guessed
1920
* list of common deploy-root prefixes (that is the server's best-effort fallback
2021
* for raw-HTTP callers, never the client's).
22+
*
23+
* A frame whose path does NOT start with `base_path()` (vendor installed
24+
* outside the app root, a globally-installed tool) is left otherwise
25+
* untouched by that strip, but still leaks the OS username via `/home/<user>/`
26+
* or `/Users/<user>/` (M-3). A secondary redaction pass replaces just the
27+
* username segment of those two shapes, everywhere in the trace, after the
28+
* exact-prefix strip runs.
2129
*/
2230
final readonly class PathNormalizer
2331
{
32+
private const string HOME_USERNAME = '#/home/[^/\s]+/#';
33+
34+
private const string MAC_USERNAME = '#/Users/[^/\s]+/#';
35+
2436
private string $prefix;
2537

2638
public function __construct(string $basePath)
@@ -31,16 +43,22 @@ public function __construct(string $basePath)
3143
}
3244

3345
/**
34-
* Replace every occurrence of the base-path prefix in the trace string.
46+
* Replace every occurrence of the base-path prefix in the trace string,
47+
* then redact the username segment of any remaining `/home/<user>/` or
48+
* `/Users/<user>/` path that did not start with the prefix.
3549
*
3650
* `getTraceAsString()` embeds absolute file paths inline (`#3 /abs/app/Foo.php(10): ...`),
3751
* so a global replace of the prefix normalizes every frame at once. Paths
3852
* that do not start with the prefix (vendor under a symlinked store, the
39-
* trailing `{main}` marker) are left untouched — exactly nightwatch's
40-
* "return unchanged when the prefix does not match" behavior.
53+
* trailing `{main}` marker) are left otherwise untouched — exactly
54+
* nightwatch's "return unchanged when the prefix does not match" behavior
55+
* — but still pass through the username-redaction fallback below.
4156
*/
4257
public function normalize(string $trace): string
4358
{
44-
return str_replace($this->prefix, '', $trace);
59+
$trace = str_replace($this->prefix, '', $trace);
60+
$trace = (string) preg_replace(self::HOME_USERNAME, '/home/[REDACTED:user]/', $trace);
61+
62+
return (string) preg_replace(self::MAC_USERNAME, '/Users/[REDACTED:user]/', $trace);
4563
}
4664
}

0 commit comments

Comments
 (0)