Repository navigation
Reject empty OTLP API keys - #20363
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 20363Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 20363" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The JSON null integration test must be reconciled with .NET 10 configuration binding behavior.
Review effort: Balanced
Findings: None
What changed in this PR
Hardens OTLP API-key authentication by rejecting empty configured keys and empty request headers.
Changes:
- Validates non-empty primary and secondary keys while allowing an unset secondary key.
- Rejects empty headers before fixed-time comparison.
- Adds unit and integration coverage.
| File | Review |
|---|---|
tests/Aspire.Dashboard.Tests/OtlpApiKeyAuthenticationHandlerTests.cs |
Covers handler rejection behavior. |
tests/Aspire.Dashboard.Tests/Integration/OtlpHttpServiceTests.cs |
The explicit JSON null test conflicts with .NET 10 binding behavior and will fail; update its expectation or validation logic. |
tests/Aspire.Dashboard.Tests/DashboardOptionsTests.cs |
Covers API-key option validation. |
src/Aspire.Dashboard/Configuration/ValidateDashboardOptions.cs |
Validates configured OTLP API keys. |
src/Aspire.Dashboard/Authentication/OtlpApiKey/OtlpApiKeyAuthenticationHandler.cs |
Rejects empty incoming API-key headers. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Jose Perez Rodriguez (joperezr)
left a comment
Jose Perez Rodriguez (joperezr)
left a comment
There was a problem hiding this comment.
Changes look good to me.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Accept a null optional secondary key while asserting that an explicitly empty key fails dashboard options validation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Tests selector13 / 99 PR test projects · 2 PR jobs, from 5 changed files. Selected PR test projects (13 / 99)
Selected PR jobs (2)
How these were chosen — grouped by what changed
🔧 📦 affected project 🧪 🧪 🧪 Job reasons
Selection computed for commit |
Documents PR microsoft/aspire#20363, which rejects empty OTLP primary/secondary API keys and empty incoming x-otlp-api-key headers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Pull request created: #1720
|
|
📝 Documentation has been drafted in microsoft/aspire.dev#1720 targeting docs_optional path: no deterministic signal fired (signal_count=0), but this matches the 'user-visible behavior change to an already-documented feature' positive trigger. The diff (OtlpApiKeyAuthenticationHandler.cs) shows the handler now rejects an empty Note This draft PR needs human review before merging. |
|
/backport to release/13.6 |
|
Started backporting to |
Backport of #20363 to release/13.6 /cc @JamesNK ## Customer Impact An explicitly empty secondary OTLP API key could allow a request with an empty `x-otlp-api-key` header to authenticate. This prevents that unintended access while still allowing an unset optional secondary key. ## Testing Dashboard test CI passed on Windows and Linux for this backport; Ubuntu x64 native-AOT Dashboard validation passed. The source PR adds configuration, authentication-handler, and OTLP HTTP endpoint coverage for empty keys and a null optional secondary key. ## Risk Medium. This changes OTLP authentication and startup validation: configurations with explicitly empty keys that were previously accepted will now fail validation; no public API changes. ## Regression? Unknown — please confirm. --------- Co-authored-by: James Newton-King <james@newtonking.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Backport of #20363 to release/13.6 /cc @JamesNK ## Customer Impact An explicitly empty secondary OTLP API key could allow a request with an empty `x-otlp-api-key` header to authenticate. This prevents that unintended access while still allowing an unset optional secondary key. ## Testing Dashboard test CI passed on Windows and Linux for this backport; Ubuntu x64 native-AOT Dashboard validation passed. The source PR adds configuration, authentication-handler, and OTLP HTTP endpoint coverage for empty keys and a null optional secondary key. ## Risk Medium. This changes OTLP authentication and startup validation: configurations with explicitly empty keys that were previously accepted will now fail validation; no public API changes. ## Regression? Unknown — please confirm. --------- Co-authored-by: James Newton-King <james@newtonking.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Reject empty OTLP API keys so an explicitly empty secondary key cannot authenticate a request carrying an empty
x-otlp-api-keyheader.This change validates that configured primary and secondary OTLP API keys are non-empty when API-key authentication is enabled. The secondary key remains optional when it is unset. The authentication handler also rejects an empty incoming header before performing either fixed-time key comparison.
The tests cover empty primary and secondary configuration, an explicit JSON
nullsecondary key, direct handler behavior, and the OTLP HTTP endpoint response.Checklist
<remarks />and<code />elements on your triple slash comments?