[Agentless] Protect API keys on agentless HTTP transports - #9064
Draft
dudikeleti wants to merge 1 commit into
Draft
[Agentless] Protect API keys on agentless HTTP transports#9064dudikeleti wants to merge 1 commit into
dudikeleti wants to merge 1 commit into
Conversation
Require factory-owned API-key headers, reject unsafe redirects/proxies/endpoints, and disable affected uploaders after terminal transport failures.
Contributor
Author
|
/codex review |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR introduces a guardrail against sending DD-API-KEY over unsafe HTTP transports (e.g., non-HTTPS endpoints and redirect/proxy scenarios), and updates multiple pipelines (telemetry, logs, debugger uploads, CI/Test Optimization) to treat unsafe API-key transport as a fatal condition that disables subsequent sends.
Changes:
- Add
ApiKeyHttpTransportGuard+ApiKeyHttpTransportExceptionand enforce safety checks inHttpClient/HttpWebRequesttransports. - Update API-key-bearing clients to fail fast on unsafe transport and stop retrying/future attempts after rejection.
- Add/adjust tests across telemetry, logs, debugger symbol upload, exception replay, CI, and transport layers to validate the new behavior.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tracer/src/Datadog.Trace/Agent/Transports/ApiKeyHttpTransportGuard.cs | Adds centralized safety rules for API-key transport (HTTPS/loopback + redirect/proxy constraints). |
| tracer/src/Datadog.Trace/Agent/Transports/ApiKeyHttpTransportException.cs | Introduces a dedicated exception to signal unsafe API-key transport. |
| tracer/src/Datadog.Trace/Agent/Transports/HttpClientRequestFactory.cs | Detects API-key default header and configures protected handler behavior (no redirects, special proxy handling). |
| tracer/src/Datadog.Trace/Agent/Transports/HttpClientRequest.cs | Enforces runtime safety checks before sending requests and blocks late API key header injection. |
| tracer/src/Datadog.Trace/Agent/Transports/ApiWebRequestFactory.cs | Tracks presence of API key header to enable guarded behavior in ApiWebRequest. |
| tracer/src/Datadog.Trace/Agent/Transports/ApiWebRequest.cs | Configures and validates safe transport when API key header is present; blocks late header injection. |
| tracer/src/Datadog.Trace/Agent/Transports/HttpStreamRequest.cs | Blocks adding API key header after request factory construction. |
| tracer/src/Datadog.Trace/Telemetry/Transports/JsonTelemetryTransport.cs | Disables future telemetry pushes after unsafe API-key transport is detected. |
| tracer/src/Datadog.Trace/Logging/DirectSubmission/Sink/LogsApi.cs | Removes per-request API key header addition; disables future log submissions after unsafe transport detection. |
| tracer/src/Datadog.Trace/Logging/DirectSubmission/LogsTransportStrategy.cs | Moves API key to request-factory default headers so it’s guarded at the transport layer. |
| tracer/src/Datadog.Trace/Logging/DirectSubmission/DirectLogSubmissionManager.cs | Updates LogsApi construction to rely on guarded request factory (no explicit API key parameter). |
| tracer/src/Datadog.Trace/Debugger/DebuggerTransportStrategy.cs | Standardizes debugger transport creation by injecting API key via guarded default headers. |
| tracer/src/Datadog.Trace/Debugger/ExceptionAutoInstrumentation/ExceptionReplayTransportFactory.cs | Rejects unsafe agentless override URL early (disables feature instead of attempting sends). |
| tracer/src/Datadog.Trace/Debugger/Upload/SymbolUploadApi.cs | Disables future symbol uploads after unsafe API-key transport is detected. |
| tracer/src/Datadog.Trace/Debugger/Sink/BatchUploader.cs | Disables future batch uploads after unsafe API-key transport is detected. |
| tracer/src/Datadog.Trace/Ci/TestOptimizationTracerManagement.cs | Adds agentless-mode default API key header and updates HttpClientRequestFactory creation. |
| tracer/src/Datadog.Trace/Ci/Net/TestOptimizationClient.cs | Stops retries and permanently disables agentless requests after unsafe API-key transport rejection. |
| tracer/src/Datadog.Trace/Ci/Agent/CIWriterHttpSender.cs | Stops retries and permanently disables agentless uploads after unsafe API-key transport rejection. |
| tracer/test/Datadog.Trace.Tests/Agent/Transports/ApiKeyHttpTransportGuardTests.cs | New unit tests for guard behavior across endpoints, redirects, proxy constraints, and factories. |
| tracer/test/Datadog.Trace.Tests/Agent/Transports/ApiWebRequestFactoryTests.cs | Adds tests validating unsafe API key transport rejection and proxy/redirect constraints in HttpWebRequest path. |
| tracer/test/Datadog.Trace.Tests/Telemetry/Transports/JsonTelemetryTransportTests.cs | Adds test ensuring unsafe API-key transport disables subsequent telemetry pushes. |
| tracer/test/Datadog.Trace.Tests/Logging/DirectSubmission/Sink/LogsApiTests.cs | Updates LogsApi usage + adds tests for unsafe URL rejection and “no retry after rejection” behavior. |
| tracer/test/Datadog.Trace.Tests/Debugger/SymbolsTests/SymbolUploadApiTests.cs | Adds test ensuring unsafe API-key transport disables subsequent symbol uploads. |
| tracer/test/Datadog.Trace.Tests/Debugger/ExceptionReplayTransportFactoryTests.cs | Adds test that unsafe override URL disables agentless transport creation. |
| tracer/test/Datadog.Trace.Tests/Debugger/BatchUploaderTests.cs | Adds test ensuring unsafe API-key transport disables subsequent batch uploads. |
| tracer/test/Datadog.Trace.Tests/Ci/TestOptimizationClientTests.cs | Adds test ensuring unsafe API-key transport stops retries and disables future requests. |
| tracer/test/Datadog.Trace.Tests/Ci/CIWriterHttpSenderTests.cs | New tests ensuring CI writer does not retry after unsafe API-key transport rejection. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9064) and master.
|
|||||||||||||||||||||||||||||||||||
| Metric | Master (Mean ± 95% CI) | Current (Mean ± 95% CI) | Change | Status |
|---|---|---|---|---|
| .NET Framework 4.8 - Baseline | ||||
| duration | 190.35 ± (190.91 - 191.69) ms | 215.00 ± (214.56 - 215.43) ms | +13.0% | ❌⬆️ |
| .NET Framework 4.8 - Bailout | ||||
| duration | 194.19 ± (194.09 - 194.56) ms | 218.57 ± (217.95 - 218.65) ms | +12.6% | ❌⬆️ |
| .NET Framework 4.8 - CallTarget+Inlining+NGEN | ||||
| duration | 1151.33 ± (1150.44 - 1155.97) ms | 1265.07 ± (1263.37 - 1269.86) ms | +9.9% | ❌⬆️ |
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.
Summary of changes
DD-API-KEYto be configured in request-factory default headers; per-request API-key additions are rejected.Reason for change
Implementation details
DD-API-KEYin their default headers and configure the transport:HttpClientRequestFactoryowns its protectedHttpClientHandler, disables redirects, and disables proxy use for plaintext loopback.ApiWebRequestdisables redirects and clears the proxy for plaintext loopback.AddHeader("DD-API-KEY", ...)is rejected by allIApiRequestimplementations. This makes construction-time ownership the only supported API-key path.TestOptimizationTracerManagement. Agent/EVP mode remains keyless and continues adding only the EVP routing header.Volatile.ReadandInterlocked.Exchangeto stop future attempts and log the first failure once.HttpWebRequest.Test coverage
Other details