Skip to content

Commit 7764560

Browse files
norberttechnorberttech
authored
Merge pull request #2695 from flow-php/telemetry-url-redaction
fix(flow-php/telemetry): redact secrets from HTTP URLs and stack traces
2 parents 740cc36 + 4d857c7 commit 7764560

53 files changed

Lines changed: 1235 additions & 110 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎benchmarks/src/Service/Seal/SealEngine.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,16 @@ public static function open(SealSchema $schema): EngineInterface
3838
public static function recreateIndex(EngineInterface $engine, string $index): void
3939
{
4040
if ($engine->existIndex($index)) {
41-
$engine->dropIndex($index, ['return_slow_promise_result' => true])?->wait();
41+
$engine->dropIndex($index, ['return_slow_promise_result' => true])->wait();
4242
}
4343

44-
$engine->createIndex($index, ['return_slow_promise_result' => true])?->wait();
44+
$engine->createIndex($index, ['return_slow_promise_result' => true])->wait();
4545
}
4646

4747
public static function dropIndex(EngineInterface $engine, string $index): void
4848
{
4949
if ($engine->existIndex($index)) {
50-
$engine->dropIndex($index, ['return_slow_promise_result' => true])?->wait();
50+
$engine->dropIndex($index, ['return_slow_promise_result' => true])->wait();
5151
}
5252
}
5353
}

‎documentation/components/bridges/psr18-telemetry-bridge.md‎

Lines changed: 28 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,23 +57,41 @@ $request = new Request('GET', 'https://api.example.com/users');
5757
$response = $traceableClient->sendRequest($request);
5858

5959
// A span is automatically created with:
60-
// - Name: "GET api.example.com"
60+
// - Name: "GET"
6161
// - Kind: CLIENT
62-
// - Attributes: http.method, http.url, http.scheme, http.host, http.status_code
63-
// - Status: OK for 2xx/3xx, ERROR for 4xx/5xx
62+
// - Attributes: http.request.method, url.full, url.scheme, server.address, server.port, http.response.status_code
63+
// - Status: unset for 1xx-3xx, ERROR for 4xx/5xx
6464
```
6565

6666
### Span Attributes
6767

6868
The traceable client automatically records the following span attributes:
6969

70-
| Attribute | Description | Example |
71-
|-----------|-------------|---------|
72-
| `http.method` | HTTP method | `GET`, `POST` |
73-
| `http.url` | Full request URL | `https://api.example.com/users` |
74-
| `http.scheme` | URL scheme | `https` |
75-
| `http.host` | Target host | `api.example.com` |
76-
| `http.status_code` | Response status code | `200`, `404`, `500` |
70+
| Attribute | Description | Example |
71+
|-----------------------------|-----------------------------------------------------------|----------------------------------------------|
72+
| `http.request.method` | HTTP method | `GET`, `POST` |
73+
| `url.full` | Full request URL, redacted | `https://api.example.com/users?sig=REDACTED` |
74+
| `url.scheme` | URL scheme | `https` |
75+
| `server.address` | Target host | `api.example.com` |
76+
| `server.port` | Target port, the scheme default when the URL has none | `443` |
77+
| `http.response.status_code` | Response status code | `200`, `404`, `500` |
78+
| `error.type` | Status code of a 4xx/5xx response, or the exception class | `404`, `RuntimeException` |
79+
80+
`url.full` and `exception.message` carry the URL with userinfo and the values of sensitive query parameters replaced by
81+
`REDACTED` (see [URL sanitization](/documentation/components/libs/telemetry.md#url-sanitization)):
82+
83+
```php
84+
use Flow\Telemetry\Sanitizer\UrlSanitizer;
85+
86+
use function Flow\Bridge\Psr18\Telemetry\DSL\psr18_traceable_client;
87+
use function Flow\Telemetry\DSL\url_sanitizer;
88+
89+
$traceableClient = psr18_traceable_client(
90+
$httpClient,
91+
$telemetry,
92+
url_sanitizer([...UrlSanitizer::DEFAULT_SENSITIVE_QUERY_PARAMETERS, 'fb_exchange_token']),
93+
);
94+
```
7795

7896
### Error Handling
7997

‎documentation/components/bridges/symfony-telemetry-bundle.md‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,6 +1435,9 @@ flow_telemetry:
14351435
- '/^debug\..*/'
14361436
```
14371437

1438+
`url.full`, `url.scheme`, `server.address` and `server.port` describe the effective URL the client sends
1439+
(`$response->getInfo('url')`), so `base_uri` and the `query` option are included.
1440+
14381441
#### PSR-18 Client
14391442

14401443
Traces PSR-18 HTTP client requests.
@@ -1448,6 +1451,34 @@ flow_telemetry:
14481451
- 'app.internal_client'
14491452
```
14501453

1454+
#### Sensitive query parameters
1455+
1456+
`http_kernel` (`url.query`), `http_client` and `psr18_client` (`url.full`, `exception.message`) replace userinfo and
1457+
the values of listed query parameters with `REDACTED`. The default is `UrlSanitizer::DEFAULT_SENSITIVE_QUERY_PARAMETERS`
1458+
(OpenTelemetry, OAuth 2.0 / OpenID Connect, API key and credential keys, see
1459+
[URL sanitization](/documentation/components/libs/telemetry.md#url-sanitization)).
1460+
1461+
A configured list **replaces** the default. Keys are case-sensitive. To add keys, extend the constant in a PHP config
1462+
file:
1463+
1464+
```php
1465+
// config/packages/flow_telemetry.php
1466+
use Flow\Telemetry\Sanitizer\UrlSanitizer;
1467+
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
1468+
1469+
return static function (ContainerConfigurator $container): void {
1470+
$container->extension('flow_telemetry', [
1471+
'instrumentation' => [
1472+
'sanitization' => [
1473+
'url' => [
1474+
'sensitive_query_parameters' => [...UrlSanitizer::DEFAULT_SENSITIVE_QUERY_PARAMETERS, 'fb_exchange_token'],
1475+
],
1476+
],
1477+
],
1478+
]);
1479+
};
1480+
```
1481+
14511482
#### Doctrine DBAL
14521483

14531484
Traces database queries.

‎documentation/components/libs/telemetry.md‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,40 @@ try {
349349
Use `withoutSuppressedTracing()` to lift suppression for a nested region (e.g. to trace real work inside an otherwise
350350
suppressed loop).
351351

352+
### URL sanitization
353+
354+
HTTP instrumentations (PSR-18 client, Symfony HttpClient, Symfony HttpKernel) write `url.full`, `url.query` and
355+
exception messages through a `UrlSanitizer`: userinfo and the values of sensitive query parameters become `REDACTED`,
356+
keys stay.
357+
358+
```php
359+
<?php
360+
361+
use Flow\Telemetry\Sanitizer\UrlSanitizer;
362+
363+
use function Flow\Telemetry\DSL\url_sanitizer;
364+
365+
url_sanitizer()->url('https://u:p@graph.facebook.com/me?access_token=T&fields=id');
366+
// https://REDACTED:REDACTED@graph.facebook.com/me?access_token=REDACTED&fields=id
367+
368+
url_sanitizer([...UrlSanitizer::DEFAULT_SENSITIVE_QUERY_PARAMETERS, 'fb_exchange_token'])->url('https://h/me?fb_exchange_token=T');
369+
// https://h/me?fb_exchange_token=REDACTED
370+
```
371+
372+
`UrlSanitizer::DEFAULT_SENSITIVE_QUERY_PARAMETERS` holds:
373+
374+
| Source | Keys |
375+
|--------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
376+
| OpenTelemetry semantic conventions | `AWSAccessKeyId`, `Signature`, `sig`, `X-Amz-Signature`, `X-Amz-Credential`, `X-Amz-Security-Token`, `X-Goog-Signature` |
377+
| OAuth 2.0 (RFC 6749, 6750, 7009, 7521, 7636, 8628, 8693), OpenID Connect | `access_token`, `refresh_token`, `id_token`, `id_token_hint`, `logout_token`, `client_secret`, `client_assertion`, `assertion`, `code`, `code_verifier`, `device_code`, `subject_token`, `actor_token`, `token` |
378+
| API keys, credentials, sessions (Sentry and Datadog scrubbing defaults) | `api_key`, `apikey`, `api-key`, `key`, `access_key`, `secret_key`, `private_key`, `secret`, `signature`, `password`, `passwd`, `pwd`, `auth`, `jwt`, `session`, `sessionid`, `PHPSESSID` |
379+
380+
The list goes beyond the OpenTelemetry default, which covers only signed cloud-storage URLs. Keys are case-sensitive.
381+
A given list replaces the default, it does not extend it; `[]` keeps only the userinfo redaction.
382+
383+
`exception.stacktrace` (spans, log records, `Throwable` attribute values) never carries function argument values,
384+
whatever `zend.exception_ignore_args` is set to.
385+
352386
### Metrics
353387

354388
Metrics are numerical measurements that track the state and performance of your application over time. Flow Telemetry

‎documentation/upgrading.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,14 @@ public function scan(FlowContext $context, FileScan $scan): Generator
130130
}
131131
```
132132

133+
### 8) `flow-php/telemetry`, `flow-php/psr18-telemetry-bridge`, `flow-php/symfony-telemetry-bundle` - HTTP URLs are redacted, stack traces carry no argument values
134+
135+
| Before | After |
136+
|---------------------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
137+
| `url.full` (PSR-18 and Symfony HttpClient), `url.query` (HttpKernel), `exception.message` and the client span status description held the raw URL | userinfo and the values of `UrlSanitizer::DEFAULT_SENSITIVE_QUERY_PARAMETERS` (OpenTelemetry, OAuth 2.0 / OpenID Connect, API key and credential keys) are `REDACTED`; configure the list with `url_sanitizer([...])` or `flow_telemetry.instrumentation.sanitization.url.sensitive_query_parameters` (replaces the default) |
138+
| Symfony HttpClient `url.full`, `url.scheme`, `server.address`, `server.port` came from the `$url` argument (`/me`, `unknown` with `base_uri`) | they come from the effective URL, `base_uri` and the `query` option included |
139+
| `exception.stacktrace` was `getTraceAsString()`, with argument values when `zend.exception_ignore_args` is off | `StackTrace::format()`: every frame renders `()`, whatever `zend.exception_ignore_args` is |
140+
133141
---
134142

135143
## Upgrading from 0.45.x to 0.46.x

‎src/adapter/etl-adapter-seal/tests/Flow/ETL/Adapter/Seal/Tests/Context/SealContext.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public function dropIndexes(): void
2929
{
3030
foreach ($this->indexes as [$engine, $index]) {
3131
if ($engine->existIndex($index)) {
32-
$engine->dropIndex($index, ['return_slow_promise_result' => true])?->wait();
32+
$engine->dropIndex($index, ['return_slow_promise_result' => true])->wait();
3333
}
3434
}
3535

@@ -42,10 +42,10 @@ public function engine(Schema $schema): EngineInterface
4242

4343
foreach ($schema->indexes as $index => $_value) {
4444
if ($engine->existIndex($index)) {
45-
$engine->dropIndex($index, ['return_slow_promise_result' => true])?->wait();
45+
$engine->dropIndex($index, ['return_slow_promise_result' => true])->wait();
4646
}
4747

48-
$engine->createIndex($index, ['return_slow_promise_result' => true])?->wait();
48+
$engine->createIndex($index, ['return_slow_promise_result' => true])->wait();
4949

5050
$this->indexes[] = [$engine, $index];
5151
}

‎src/bridge/psr18/telemetry/src/Flow/Bridge/Psr18/Telemetry/DSL/functions.php‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,15 @@
88
use Flow\Documentation\Attribute\DocumentationDSL;
99
use Flow\Documentation\Attribute\Module;
1010
use Flow\Documentation\Attribute\Type as DSLType;
11+
use Flow\Telemetry\Sanitizer\UrlSanitizer;
1112
use Flow\Telemetry\Telemetry;
1213
use Psr\Http\Client\ClientInterface;
1314

1415
#[DocumentationDSL(module: Module::PSR18_TELEMETRY_BRIDGE, type: DSLType::HELPER)]
15-
function psr18_traceable_client(ClientInterface $client, Telemetry $telemetry): PSR18TraceableClient
16-
{
17-
return new PSR18TraceableClient($client, $telemetry);
16+
function psr18_traceable_client(
17+
ClientInterface $client,
18+
Telemetry $telemetry,
19+
UrlSanitizer $urlSanitizer = new UrlSanitizer(),
20+
): PSR18TraceableClient {
21+
return new PSR18TraceableClient($client, $telemetry, $urlSanitizer);
1822
}

‎src/bridge/psr18/telemetry/src/Flow/Bridge/Psr18/Telemetry/PSR18TraceableClient.php‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
namespace Flow\Bridge\Psr18\Telemetry;
66

77
use DateTimeImmutable;
8+
use Flow\Telemetry\ClientUrlAttributes;
89
use Flow\Telemetry\PackageVersion;
10+
use Flow\Telemetry\Sanitizer\UrlSanitizer;
911
use Flow\Telemetry\SemConvAttributes;
1012
use Flow\Telemetry\Telemetry;
1113
use Flow\Telemetry\Tracer\SpanKind;
@@ -18,12 +20,16 @@
1820

1921
final readonly class PSR18TraceableClient implements ClientInterface
2022
{
23+
private ClientUrlAttributes $clientUrlAttributes;
24+
2125
private Tracer $tracer;
2226

2327
public function __construct(
2428
private ClientInterface $client,
2529
Telemetry $telemetry,
30+
private UrlSanitizer $urlSanitizer = new UrlSanitizer(),
2631
) {
32+
$this->clientUrlAttributes = new ClientUrlAttributes($urlSanitizer);
2733
$this->tracer = $telemetry->tracer(
2834
'flow.psr18.http_client',
2935
PackageVersion::get('flow-php/psr18-telemetry-bridge'),
@@ -32,22 +38,9 @@ public function __construct(
3238

3339
public function sendRequest(RequestInterface $request): ResponseInterface
3440
{
35-
$uri = $request->getUri();
3641
$method = $request->getMethod();
37-
$scheme = $uri->getScheme() ?: 'http';
38-
$host = $uri->getHost() ?: 'unknown';
39-
$port = $uri->getPort();
40-
$url = (string) $uri;
41-
42-
$attributes = [
43-
SemConvAttributes::HTTP_REQUEST_METHOD => $method,
44-
SemConvAttributes::URL_FULL => $url,
45-
SemConvAttributes::URL_SCHEME => $scheme,
46-
SemConvAttributes::SERVER_ADDRESS => $host,
47-
// OTEL HTTP semconv: server.port is Required on client spans; fall back to the
48-
// scheme default when the URI carries no explicit port.
49-
SemConvAttributes::SERVER_PORT => $port ?? ($scheme === 'https' ? 443 : 80),
50-
];
42+
$url = (string) $request->getUri();
43+
$attributes = [SemConvAttributes::HTTP_REQUEST_METHOD => $method] + $this->clientUrlAttributes->of($url);
5144

5245
// OTEL HTTP semconv: client span name is "{method}" - host would be per-host cardinality.
5346
$span = $this->tracer->span($method, SpanKind::CLIENT, $attributes);
@@ -67,9 +60,11 @@ public function sendRequest(RequestInterface $request): ResponseInterface
6760

6861
return $response;
6962
} catch (Throwable $exception) {
70-
$span->recordException($exception, new DateTimeImmutable());
63+
$message = $this->urlSanitizer->message($exception->getMessage(), $url);
64+
65+
$span->recordException($exception, new DateTimeImmutable(), ['exception.message' => $message]);
7166
$span->setAttribute(SemConvAttributes::ERROR_TYPE, $exception::class);
72-
$span->setStatus(SpanStatus::error($exception->getMessage()));
67+
$span->setStatus(SpanStatus::error($message));
7368

7469
throw $exception;
7570
} finally {

‎src/bridge/psr18/telemetry/tests/Flow/Bridge/Psr18/Telemetry/Tests/Unit/PSR18TraceableClientTest.php‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,47 @@
1515
use RuntimeException;
1616

1717
use function Flow\Bridge\Psr18\Telemetry\DSL\psr18_traceable_client;
18+
use function Flow\Telemetry\DSL\url_sanitizer;
1819

1920
final class PSR18TraceableClientTest extends TestCase
2021
{
22+
public function test_custom_sanitizer_redacts_configured_query_parameters_and_userinfo(): void
23+
{
24+
$spanProcessor = new MemorySpanProcessor(new VoidExporter());
25+
26+
$client = $this->createStub(ClientInterface::class);
27+
$client->method('sendRequest')->willReturn(new Response(200));
28+
29+
psr18_traceable_client(
30+
$client,
31+
TelemetryMother::withSpanProcessor($spanProcessor),
32+
url_sanitizer(['access_token']),
33+
)->sendRequest(new Request('GET', 'https://u:p@graph.facebook.com/me?access_token=SECRET&fields=id'));
34+
35+
static::assertSame(
36+
'https://REDACTED:REDACTED@graph.facebook.com/me?access_token=REDACTED&fields=id',
37+
$spanProcessor->endedSpans()[0]->attributes()['url.full'],
38+
);
39+
}
40+
41+
public function test_default_sanitizer_redacts_sensitive_query_parameters(): void
42+
{
43+
$spanProcessor = new MemorySpanProcessor(new VoidExporter());
44+
45+
$client = $this->createStub(ClientInterface::class);
46+
$client->method('sendRequest')->willReturn(new Response(200));
47+
48+
psr18_traceable_client($client, TelemetryMother::withSpanProcessor($spanProcessor))->sendRequest(new Request(
49+
'GET',
50+
'https://bucket.s3.amazonaws.com/file?X-Amz-Signature=SECRET&access_token=SECRET&fields=id',
51+
));
52+
53+
static::assertSame(
54+
'https://bucket.s3.amazonaws.com/file?X-Amz-Signature=REDACTED&access_token=REDACTED&fields=id',
55+
$spanProcessor->endedSpans()[0]->attributes()['url.full'],
56+
);
57+
}
58+
2159
public function test_emits_no_spans_when_tracing_is_suppressed(): void
2260
{
2361
$spanProcessor = new MemorySpanProcessor(new VoidExporter());
@@ -35,6 +73,38 @@ public function test_emits_no_spans_when_tracing_is_suppressed(): void
3573
);
3674
}
3775

76+
public function test_exception_message_and_status_description_hold_redacted_url(): void
77+
{
78+
$spanProcessor = new MemorySpanProcessor(new VoidExporter());
79+
80+
$client = $this->createStub(ClientInterface::class);
81+
$client
82+
->method('sendRequest')
83+
->willThrowException(
84+
new RuntimeException('Could not reach "https://graph.facebook.com/me?access_token=SECRET".'),
85+
);
86+
87+
$this->expectException(RuntimeException::class);
88+
89+
try {
90+
psr18_traceable_client(
91+
$client,
92+
TelemetryMother::withSpanProcessor($spanProcessor),
93+
url_sanitizer(['access_token']),
94+
)->sendRequest(new Request('GET', 'https://graph.facebook.com/me?access_token=SECRET'));
95+
} finally {
96+
$span = $spanProcessor->endedSpans()[0];
97+
static::assertSame(
98+
'Could not reach "https://graph.facebook.com/me?access_token=REDACTED".',
99+
$span->events()[0]->attributes()['exception.message'],
100+
);
101+
static::assertSame(
102+
'Could not reach "https://graph.facebook.com/me?access_token=REDACTED".',
103+
$span->status()?->description,
104+
);
105+
}
106+
}
107+
38108
public function test_exception_is_recorded_and_rethrown(): void
39109
{
40110
$spanProcessor = new MemorySpanProcessor(new VoidExporter());

‎src/bridge/symfony/telemetry-bundle/composer.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
"flow-php/telemetry-otlp-bridge": "self.version",
3636
"nyholm/psr7": "^1.8",
3737
"symfony/framework-bundle": "^6.4 || ^7.4 || ^8.0",
38+
"symfony/http-client": "^6.4 || ^7.4 || ^8.0",
3839
"symfony/messenger": "^6.4 || ^7.4 || ^8.0",
3940
"symfony/routing": "^6.4 || ^7.4 || ^8.0",
4041
"symfony/security-bundle": "^6.4 || ^7.4 || ^8.0",

0 commit comments

Comments
 (0)