Skip to content

Commit a5a2df5

Browse files
turegjorupclaude
turegjorup
and
claude
committed
feat(logging): redact URL query in outbound HTTP logs
SensitiveDataProcessor scrubs by context key name only, so a credential in a URL value (?api_key=…, ?token=…, or user:pass@host) bypassed the backstop. LoggingHttpClient now sanitises url.full at the source: the query string is redacted wholesale (host/path?[redacted]) and userinfo/fragment dropped, while the inner client still receives the full URL. Documents the key-name-only limitation in docs/logging.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7e01531 commit a5a2df5

3 files changed

Lines changed: 143 additions & 2 deletions

File tree

‎docs/logging.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,4 +130,11 @@ The two stay in separate bags, so there is no key collision.
130130
`apikey`, `token`, `credential` or `bearer` is replaced with `[redacted]`, at any depth
131131
in `context`/`extra`.
132132

133+
`SensitiveDataProcessor` matches on the context **key name only** — it never inspects
134+
*values*. A secret carried inside a value under an innocuous key (most commonly a URL with
135+
`?api_key=…` in its query string) is therefore **not** caught by the backstop and must be
136+
sanitised at the source. The outbound HTTP client does this: `LoggingHttpClient` redacts the
137+
query string of `url.full` wholesale (`https://host/path?[redacted]`) and drops any userinfo
138+
(`user:pass@`) before logging, so credentials in an outbound URL never reach the log.
139+
133140
Still: do not put credentials or token strings into log context in the first place.

‎src/HttpClient/LoggingHttpClient.php‎

Lines changed: 48 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111

1212
class LoggingHttpClient implements HttpClientInterface
1313
{
14+
private const REDACTED = '[redacted]';
15+
1416
public function __construct(
1517
private HttpClientInterface $client,
1618
private readonly LoggerInterface $logger,
@@ -22,6 +24,10 @@ public function request(string $method, string $url, array $options = []): Respo
2224

2325
$response = $this->client->request($method, $url, $options);
2426

27+
// The actual request uses the full $url; only the logged value is
28+
// sanitised (see sanitizeUrl).
29+
$loggedUrl = $this->sanitizeUrl($url);
30+
2531
try {
2632
$statusCode = $response->getStatusCode();
2733
} catch (\Throwable $throwable) {
@@ -30,7 +36,7 @@ public function request(string $method, string $url, array $options = []): Respo
3036
// serialises it (see docs/logging.md).
3137
$this->logger->error('{http.request.method} {url.full} failed', [
3238
'http.request.method' => $method,
33-
'url.full' => $url,
39+
'url.full' => $loggedUrl,
3440
'http.client.request.duration' => $this->durationSeconds($startTime),
3541
'exception' => $throwable,
3642
]);
@@ -43,14 +49,54 @@ public function request(string $method, string $url, array $options = []): Respo
4349
// threshold (LOG_LEVEL_OUTBOUND_HTTP), consistent with every channel.
4450
$this->logger->info('{http.request.method} {url.full} {http.response.status_code} ({http.client.request.duration}s)', [
4551
'http.request.method' => $method,
46-
'url.full' => $url,
52+
'url.full' => $loggedUrl,
4753
'http.response.status_code' => $statusCode,
4854
'http.client.request.duration' => $this->durationSeconds($startTime),
4955
]);
5056

5157
return $response;
5258
}
5359

60+
/**
61+
* Sanitises a URL for logging: the query string is replaced wholesale with
62+
* a redaction marker (`?[redacted]`), and the fragment and any userinfo
63+
* (`user:pass@`) are dropped, keeping scheme + host + port + path.
64+
*
65+
* The {@see \App\Logger\Processor\SensitiveDataProcessor} backstop only
66+
* redacts by context *key* name; a credential carried in a URL *value*
67+
* (`?api_key=…`, `?token=…`, or `https://user:pass@host/…`) would otherwise
68+
* pass through unredacted. URL values must therefore be sanitised here, at
69+
* the source. The query is where secrets live, so its contents are redacted
70+
* rather than dropped — the `?[redacted]` marker preserves the signal that
71+
* a query was present without leaking any of its values.
72+
*/
73+
private function sanitizeUrl(string $url): string
74+
{
75+
$parts = parse_url($url);
76+
if (false === $parts) {
77+
// Unparseable: redact from the first query delimiter onward rather
78+
// than risk emitting a credential-bearing tail.
79+
$base = explode('#', explode('?', $url, 2)[0], 2)[0];
80+
81+
return str_contains($url, '?') ? $base.'?'.self::REDACTED : $base;
82+
}
83+
84+
$scheme = isset($parts['scheme']) ? $parts['scheme'].'://' : '';
85+
$host = $parts['host'] ?? '';
86+
$port = isset($parts['port']) ? ':'.$parts['port'] : '';
87+
$path = $parts['path'] ?? '';
88+
// Show that a query existed, but never its contents.
89+
$query = isset($parts['query']) ? '?'.self::REDACTED : '';
90+
91+
// No host (e.g. a relative URL resolved against the client's base_uri):
92+
// keep just the path, still with the query redacted.
93+
if ('' === $host) {
94+
return $path.$query;
95+
}
96+
97+
return $scheme.$host.$port.$path.$query;
98+
}
99+
54100
/**
55101
* Elapsed time since $startTime in seconds (OTel
56102
* `http.client.request.duration` unit), rounded to 0.1 ms.

‎tests/HttpClient/LoggingHttpClientTest.php‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,94 @@ public function testRequestLogsErrorOnTransportException(): void
6868
$client->request('POST', 'https://example.com/fail');
6969
}
7070

71+
/**
72+
* The inner client must receive the full URL (query intact) while the
73+
* logged `url.full` has its query redacted wholesale.
74+
*
75+
* @dataProvider urlSanitizationProvider
76+
*/
77+
public function testUrlQueryIsRedactedInLogsButNotInRequest(string $requestUrl, string $expectedLoggedUrl): void
78+
{
79+
$response = $this->createMock(ResponseInterface::class);
80+
$response->method('getStatusCode')->willReturn(200);
81+
82+
$inner = $this->createMock(HttpClientInterface::class);
83+
// The real request keeps the full, unredacted URL.
84+
$inner->expects($this->once())
85+
->method('request')
86+
->with('GET', $requestUrl)
87+
->willReturn($response);
88+
89+
$logger = $this->createMock(LoggerInterface::class);
90+
$logger->expects($this->once())
91+
->method('info')
92+
->with(
93+
$this->anything(),
94+
$this->callback(fn (array $context) => $expectedLoggedUrl === $context['url.full'])
95+
);
96+
97+
$client = new LoggingHttpClient($inner, $logger);
98+
$client->request('GET', $requestUrl);
99+
}
100+
101+
/**
102+
* @return iterable<string, array{string, string}>
103+
*/
104+
public static function urlSanitizationProvider(): iterable
105+
{
106+
yield 'query with secrets is redacted wholesale' => [
107+
'https://example.com/api?api_key=SECRET&token=abc123',
108+
'https://example.com/api?[redacted]',
109+
];
110+
yield 'a single query param is still redacted' => [
111+
'https://example.com/api?token=abc123',
112+
'https://example.com/api?[redacted]',
113+
];
114+
yield 'no query is left untouched' => [
115+
'https://example.com/api',
116+
'https://example.com/api',
117+
];
118+
yield 'fragment is dropped' => [
119+
'https://example.com/api#section',
120+
'https://example.com/api',
121+
];
122+
yield 'userinfo credentials are dropped' => [
123+
'https://user:pass@example.com/api?token=abc',
124+
'https://example.com/api?[redacted]',
125+
];
126+
yield 'port is preserved' => [
127+
'https://example.com:8443/api?token=abc',
128+
'https://example.com:8443/api?[redacted]',
129+
];
130+
yield 'relative url keeps path with query redacted' => [
131+
'/api/items?token=abc',
132+
'/api/items?[redacted]',
133+
];
134+
}
135+
136+
public function testErrorPathAlsoRedactsTheQuery(): void
137+
{
138+
$inner = $this->createMock(HttpClientInterface::class);
139+
$response = $this->createMock(ResponseInterface::class);
140+
$response->method('getStatusCode')
141+
->willThrowException(new \RuntimeException('Connection refused'));
142+
$inner->method('request')->willReturn($response);
143+
144+
$logger = $this->createMock(LoggerInterface::class);
145+
$logger->expects($this->once())
146+
->method('error')
147+
->with(
148+
'{http.request.method} {url.full} failed',
149+
$this->callback(fn (array $context) => 'https://example.com/fail?api_key=SECRET' !== $context['url.full']
150+
&& 'https://example.com/fail?[redacted]' === $context['url.full'])
151+
);
152+
153+
$client = new LoggingHttpClient($inner, $logger);
154+
155+
$this->expectException(\RuntimeException::class);
156+
$client->request('POST', 'https://example.com/fail?api_key=SECRET');
157+
}
158+
71159
public function testStreamDelegatesToInnerClient(): void
72160
{
73161
$response = $this->createMock(ResponseInterface::class);

0 commit comments

Comments
 (0)