diff --git a/docs/development/Configuration/AddingConfigurationKeys.md b/docs/development/Configuration/AddingConfigurationKeys.md index cbd99e0b19cb..caf8e9ade1fc 100644 --- a/docs/development/Configuration/AddingConfigurationKeys.md +++ b/docs/development/Configuration/AddingConfigurationKeys.md @@ -57,6 +57,7 @@ ConfigurationKeys.ProductName.cs. Without a product name, the keys will go in th - `product`: Groups the key into a product-specific partial class (e.g., `OpenTelemetry`) - `aliases`: A list of fallback environment variable names checked in order when the primary key is not found - `const_name`: Overrides the auto-generated constant name (useful for backward compatibility). For `managed`, the default is PascalCase. +- `sensitive`: Marks a credential-bearing value. Use `true` only when the configuration value itself must never be recorded in telemetry. The value must be `true` or `false` (case-insensitive); any other value is a YAML parse error. These fields are mandatory to keep the configuration registry complete and to ensure consistent behavior and documentation across products. @@ -181,9 +182,23 @@ The codebase includes Roslyn analyzers that enforce the use of configuration key - **`ConfigurationBuilderWithKeysAnalyzer`** - Enforces that `ConfigurationBuilder.WithKeys()` method calls only accept string constants from `ConfigurationKeys` or `PlatformKeys` classes, not hardcoded strings or variables. +For keys marked `sensitive: true`, the analyzer also enforces telemetry redaction at compile time. Read a sensitive string through `AsRedactedString()` or `AsRedactedStringResult()`. Use `AsRedactedDictionaryResult()` for dictionary-valued settings, or `AsStringResult(..., recordValue: false)` when the explicit `false` is a compile-time constant. Other accessors and storing the intermediate `WithKeys()` result are rejected because they could record the value. + +```csharp +var apiKey = config.WithKeys(ConfigurationKeys.ApiKey).AsRedactedString(); +var result = config.WithKeys(ConfigurationKeys.ApiKey) + .AsStringResult(validator: null, converter: null, recordValue: false); +var headers = config.WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpLogsHeaders) + .AsRedactedDictionaryResult(separator: '='); +``` + +Aliases use the normal `WithKeys()` fallback chain. When a sensitive primary key falls back to an alias, the selected value remains redacted because the redacted accessor records no value in telemetry. + ##### Diagnostic rules: - **DD0007**: Triggers when hardcoded string literals are used instead of configuration key constants - **DD0008**: Triggers when variables or expressions are used instead of configuration key constants +- **DD0015**: Triggers when a sensitive configuration key is not read through `AsRedactedString`, `AsRedactedStringResult`, `AsRedactedDictionaryResult`, or `AsStringResult` with compile-time `recordValue: false` +- **DD0016**: Triggers when the analyzer cannot load exactly one valid `supported-configurations.yaml` additional file #### 2. EnvironmentGetEnvironmentVariableAnalyzer diff --git a/docs/superpowers/plans/2026-07-31-sensitive-configuration-analyzer.md b/docs/superpowers/plans/2026-07-31-sensitive-configuration-analyzer.md new file mode 100644 index 000000000000..32a3bd5bb29b --- /dev/null +++ b/docs/superpowers/plans/2026-07-31-sensitive-configuration-analyzer.md @@ -0,0 +1,381 @@ +# Sensitive Configuration Analyzer Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Enforce at compile time that configuration keys marked `sensitive: true` in `supported-configurations.yaml` are read only through telemetry-redacting accessors, and migrate OTLP headers to those accessors. + +**Architecture:** The existing shared YAML reader will expose a `Sensitive` flag. `tracer/src/Directory.Build.props` will provide the registry to every analyzer consumer. `ConfigurationBuilderWithKeysAnalyzer` will require and parse that YAML additional file once per compilation, resolve each `WithKeys()` constant, and reject sensitive keys unless the immediately chained accessor proves `recordValue` is false. Runtime configuration code will use redacted string and dictionary accessors, so no startup-path sensitivity lookup is added. + +**Tech Stack:** C# 12, Roslyn analyzers, incremental source-generator support types, xUnit, FluentAssertions, .NET 10 test target. + +## Global Constraints + +- All changes stay in the current task worktree and branch. +- `supported-configurations.yaml` remains the single source of truth; do not generate runtime sensitivity metadata. +- Preserve alias fallback, OTLP header parsing, and public APIs. +- `DD_API_KEY` and the four `OTEL_EXPORTER_OTLP*_HEADERS` keys are sensitive. +- Do not mark AppSec's HTML template path sensitive; it is redacted for payload-size reasons. +- Every production behavior change follows a witnessed red-green TDD cycle. +- Before push, run `pre-push-review`, amend fixes into the relevant existing commit, and rerun verification. +- Create the PR as a draft with the `AI Generated` label and the repository PR template. + +--- + +### Task 1: Parse sensitivity metadata from YAML + +**Files:** +- Modify: `tracer/build/_build/NativeValidation/YamlReader.cs` +- Create: `tracer/test/Datadog.Trace.SourceGenerators.Tests/YamlReaderTests.cs` + +**Interfaces:** +- Consumes: `YamlReader.ParseSupportedConfigurations(string)` and `ConfigurationEntry`. +- Produces: `ConfigurationEntry.Sensitive` as a `bool`, defaulting to `false` and reset for every new configuration entry. + +- [x] **Step 1: Write failing parser tests** + +Add three behavior assertions using a hand-written YAML fixture: + +```csharp +[Fact] +public void ParsesSensitiveMetadata() +{ + var parsed = YamlReader.ParseSupportedConfigurations(YamlWithSensitiveAndOrdinaryEntries); + + parsed.Configurations["DD_API_KEY"].Sensitive.Should().BeTrue(); + parsed.Configurations["DD_TRACE_ENABLED"].Sensitive.Should().BeFalse(); + parsed.Configurations["DD_SERVICE"].Sensitive.Should().BeFalse(); +} +``` + +The fixture places an ordinary entry after a sensitive entry so the final assertion catches failure to reset parser state. + +- [x] **Step 2: Run the parser test and verify RED** + +Run: + +```bash +dotnet test tracer/test/Datadog.Trace.SourceGenerators.Tests/Datadog.Trace.SourceGenerators.Tests.csproj -c Release -f net10.0 --filter FullyQualifiedName~YamlReaderTests --disable-build-servers -m:1 +``` + +Expected: compilation fails because `ConfigurationEntry` has no `Sensitive` property. + +- [x] **Step 3: Implement minimal YAML parsing** + +In `YamlReader.ParseSupportedConfigurations`: + +```csharp +var currentSensitive = false; +``` + +Recognize `sensitive` as a property that terminates documentation, accept only case-insensitive `true` and `false`, pass it into every `ConfigurationEntry` construction, and reset it when a new key begins. Other values throw so the source generator reports `DDSG0007`: + +```csharp +case "sensitive": + if (propValue.Equals("true", StringComparison.OrdinalIgnoreCase)) + { + currentSensitive = true; + } + else if (propValue.Equals("false", StringComparison.OrdinalIgnoreCase)) + { + currentSensitive = false; + } + else + { + throw new InvalidOperationException(...); + } + + break; +``` + +Extend the entry model and its equality contract: + +```csharp +public ConfigurationEntry( + string key, + string? product, + string? documentation, + string? constName, + string[]? scope, + string[]? aliases = null, + bool sensitive = false) +{ + Sensitive = sensitive; +} + +public bool Sensitive { get; } +``` + +Include `Sensitive` in `Equals()` and `GetHashCode()` so incremental-generator caching notices registry changes. + +- [x] **Step 4: Run parser and source-generator suites and verify GREEN** + +Run the filtered command from Step 2, followed by: + +```bash +dotnet test tracer/test/Datadog.Trace.SourceGenerators.Tests/Datadog.Trace.SourceGenerators.Tests.csproj -c Release -f net10.0 --disable-build-servers -m:1 +``` + +Expected: all source-generator tests pass with no warnings. + +- [x] **Step 5: Commit parser support** + +```bash +git add tracer/build/_build/NativeValidation/YamlReader.cs tracer/test/Datadog.Trace.SourceGenerators.Tests/YamlReaderTests.cs +git commit -m "[Configuration] Parse sensitive config metadata" +``` + +### Task 2: Enforce sensitive reads in the analyzer + +**Files:** +- Modify: `tracer/src/Datadog.Trace.Tools.Analyzers/Datadog.Trace.Tools.Analyzers.csproj` +- Modify: `tracer/src/Datadog.Trace.Tools.Analyzers/ConfigurationAnalyzers/ConfigurationBuilderWithKeysAnalyzer.cs` +- Modify: `tracer/test/Datadog.Trace.Tools.Analyzers.Tests/ConfigurationAnalyzers/AnalyzerTestHelper.cs` +- Modify: `tracer/test/Datadog.Trace.Tools.Analyzers.Tests/ConfigurationAnalyzers/ConfigurationBuilderWithKeysAnalyzerTests.cs` + +**Interfaces:** +- Consumes: `ConfigurationEntry.Sensitive`, Roslyn `AdditionalText`, and direct `ConfigurationKeys` constants passed to `ConfigurationBuilder.WithKeys(string)`. +- Produces: diagnostic `DD0015` when a sensitive key is not immediately consumed by a provably redacted accessor. + +- [x] **Step 1: Link the shared parser into the analyzer project** + +Add linked compile items for the shared parser and its dependencies: + +```xml + + + +``` + +Build the analyzer project once to prove the shared types compile under `netstandard2.0`. + +- [x] **Step 2: Add failing analyzer tests for unsafe reads** + +Teach `AnalyzerTestHelper` to attach a literal `supported-configurations.yaml` additional file. Add focused tests where `DD_API_KEY` is marked sensitive and `DD_SERVICE` is not: + +```csharp +builder.WithKeys({|#0:ConfigurationKeys.ApiKey|}).AsString(); +builder.WithKeys(ConfigurationKeys.ServiceName).AsString(); +``` + +Expect `DD0015` only at marker `#0`. Add separate failing cases for: + +```csharp +builder.WithKeys({|#0:ConfigurationKeys.ApiKey|}).AsDictionaryResult(); +builder.WithKeys({|#0:ConfigurationKeys.ApiKey|}).AsStringResult(null, null, recordValue: true); +var sensitive = builder.WithKeys({|#0:ConfigurationKeys.ApiKey|}); +``` + +Each test names the unsafe branch it catches; do not combine unrelated failures into one assertion. + +- [x] **Step 3: Run unsafe-read tests and verify RED** + +Run: + +```bash +dotnet test tracer/test/Datadog.Trace.Tools.Analyzers.Tests/Datadog.Trace.Tools.Analyzers.Tests.csproj -c Release -f net10.0 --filter FullyQualifiedName~ConfigurationBuilderWithKeysAnalyzerTests --disable-build-servers -m:1 +``` + +Expected: the new tests fail because `DD0015` is not reported. + +- [x] **Step 4: Implement YAML loading and the unsafe-read rule** + +At compilation start, locate `supported-configurations.yaml`, parse it once, and collect canonical sensitive keys: + +```csharp +private static ImmutableHashSet GetSensitiveKeys(AnalyzerOptions options, CancellationToken cancellationToken) +{ + var file = options.AdditionalFiles.FirstOrDefault( + x => Path.GetFileName(x.Path).Equals(SupportedConfigurationsFileName, StringComparison.OrdinalIgnoreCase)); + var content = file?.GetText(cancellationToken)?.ToString(); + if (string.IsNullOrEmpty(content)) + { + return ImmutableHashSet.Empty; + } + + try + { + return YamlReader.ParseSupportedConfigurations(content!) + .Configurations + .Where(x => x.Value.Sensitive) + .Select(x => x.Key) + .ToImmutableHashSet(StringComparer.Ordinal); + } + catch + { + return ImmutableHashSet.Empty; + } +} +``` + +Add `DD0015` as an error diagnostic. After the existing constant validation, get the field's constant string value. Use `IInvocationOperation` and bound method symbols so parentheses and conversions around `WithKeys()` are transparent, while same-named extension methods remain rejected. For sensitive values, accept only an instance method on `ConfigurationBuilder.HasKeys` named: + +```csharp +AsRedactedString +AsRedactedStringResult +AsRedactedDictionaryResult +``` + +For `AsStringResult`, obtain `IInvocationOperation`, find the argument whose bound parameter is named `recordValue`, and require `argument.Value.ConstantValue` to equal `false`. Reject all other accessors and non-chained/stored `HasKeys` values. + +Require exactly one readable and valid YAML additional file. Report `DD0016` at compilation end when metadata is missing, ambiguous, unreadable, or invalid. Propagate `OperationCanceledException` instead of converting cancellation into missing metadata. + +- [x] **Step 5: Run unsafe-read tests and verify GREEN** + +Run the filtered analyzer command from Step 3. + +Expected: all `ConfigurationBuilderWithKeysAnalyzerTests` pass. + +- [x] **Step 6: Add passing tests for allowed redacted reads** + +Add separate no-diagnostic tests for: + +```csharp +builder.WithKeys(ConfigurationKeys.ApiKey).AsRedactedString(); +builder.WithKeys(ConfigurationKeys.ApiKey).AsRedactedStringResult(); +builder.WithKeys(ConfigurationKeys.ApiKey).AsStringResult(null, null, recordValue: false); +``` + +Also prove malformed or missing YAML produces `DD0016`, and prove a non-`Datadog.Trace` consumer assembly still enforces `DD0015` when the shared additional file is present. + +- [x] **Step 7: Run the entire analyzer suite** + +```bash +dotnet test tracer/test/Datadog.Trace.Tools.Analyzers.Tests/Datadog.Trace.Tools.Analyzers.Tests.csproj -c Release -f net10.0 --disable-build-servers -m:1 +``` + +Expected: 0 failures and no new warnings. + +- [x] **Step 8: Commit analyzer enforcement** + +```bash +git add tracer/src/Datadog.Trace.Tools.Analyzers tracer/test/Datadog.Trace.Tools.Analyzers.Tests/ConfigurationAnalyzers +git commit -m "[Configuration] Enforce redaction for sensitive keys" +``` + +### Task 3: Mark and redact credential-bearing configuration + +**Files:** +- Modify: `tracer/src/Datadog.Trace/Configuration/supported-configurations.yaml` +- Modify: `tracer/src/Datadog.Trace/Configuration/ExporterSettings.cs` +- Modify: `tracer/src/Datadog.Trace/Configuration/TracerSettings.cs` +- Modify: `tracer/test/Datadog.Trace.Tests/Configuration/ExporterSettingsTests.cs` +- Modify: `tracer/test/Datadog.Trace.Tests/Configuration/TracerSettingsTests.cs` + +**Interfaces:** +- Consumes: `AsRedactedString()`, `AsRedactedDictionaryResult(char)`, and existing configuration alias fallback. +- Produces: redacted telemetry for `DD_API_KEY` and all OTLP header settings, while preserving parsed header values for exporters. + +- [x] **Step 1: Add failing telemetry regression tests** + +In `ExporterSettingsTests`, configure distinct sentinel secrets for the general, metrics, and traces header keys. Construct `ExporterSettings` with a real `ConfigurationTelemetry` and assert each matching entry is `Redacted` with a null `StringValue`. Assert the endpoint entry remains a normal string entry. + +In `TracerSettingsTests`, configure a logs-header sentinel and assert: + +```csharp +settings.OtlpLogsHeaders.Should().Contain(new KeyValuePair("dd-api-key", logsSentinel)); +entries.Where(x => x.Key == ConfigurationKeys.OpenTelemetry.ExporterOtlpLogsHeaders) + .Should() + .OnlyContain(x => x.Type == ConfigurationTelemetryEntryType.Redacted && x.StringValue is null); +``` + +- [x] **Step 2: Run affected tracer tests and verify RED** + +```bash +dotnet test tracer/test/Datadog.Trace.Tests/Datadog.Trace.Tests.csproj -c Release -f net10.0 --filter "FullyQualifiedName~ExporterSettingsTests|FullyQualifiedName~TracerSettingsTests" --disable-build-servers -m:1 +``` + +Expected: sentinel values are recorded as string telemetry, so the new assertions fail. + +- [x] **Step 3: Mark sensitive YAML entries** + +Add `sensitive: true` to `DD_API_KEY` and the four OTLP header entries. Do not add sensitivity metadata to other header-named settings. + +- [x] **Step 4: Convert OTLP runtime reads to redacted accessors** + +In `ExporterSettings.RawSettings`, replace the three OTLP header `AsString()` calls with `AsRedactedString()`. + +Remove `TracerSettings.OtlpMetricsHeaders`, its duplicate parse block, and its now-redundant parsing test. `ExporterSettings` remains the metrics-header owner. + +For log headers, preserve `JsonConfigurationSource` object and array handling while redacting every telemetry path. Keep the public `IConfigurationSource` contract unchanged. Pass the existing separator-based dictionary overload an internal telemetry decorator that forces string-value records to use `recordValue: false` and forwards every other operation unchanged. Then retain the existing default, filter, and trim flow: + +```csharp +OtlpLogsHeaders = config + .WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpLogsHeaders) + .AsRedactedDictionaryResult(separator: '=') + .WithDefault(new DefaultResult>(new Dictionary(), "[]")) + .Where(kvp => !string.IsNullOrWhiteSpace(kvp.Key)) + .ToDictionary(kvp => kvp.Key.Trim(), kvp => kvp.Value?.Trim() ?? string.Empty); +``` + +- [x] **Step 5: Run affected tracer tests and verify GREEN** + +Run the filtered command from Step 2. Also run the existing OTLP parsing theories without filters broad enough to skip them. + +Expected: all affected tests pass, sentinels never appear in string telemetry, and parsing/fallback results are unchanged. + +- [x] **Step 6: Build Datadog.Trace with the analyzer enabled** + +```bash +dotnet build tracer/src/Datadog.Trace/Datadog.Trace.csproj -c Release --disable-build-servers -m:1 +``` + +Expected: all target frameworks compile with no `DD0015` violations. Existing `DD_API_KEY` reads already use redacted accessors. + +- [x] **Step 7: Commit runtime migration** + +```bash +git add tracer/src/Datadog.Trace/Configuration tracer/test/Datadog.Trace.Tests/Configuration +git commit -m "[Configuration] Redact sensitive configuration reads" +``` + +### Task 4: Document and verify the complete change + +**Files:** +- Modify: `docs/development/Configuration/AddingConfigurationKeys.md` +- Modify: `docs/superpowers/plans/2026-07-31-sensitive-configuration-analyzer.md` only to check completed steps during execution. + +**Interfaces:** +- Consumes: the implemented YAML property and analyzer behavior. +- Produces: contributor guidance and fresh verification evidence for publishing. + +- [x] **Step 1: Update contributor documentation** + +Document that `sensitive: true` marks credential-bearing values, that aliases inherit redaction through normal fallback, and that sensitive keys must use a redacted string or dictionary accessor, or an explicit compile-time `recordValue: false` path. + +- [x] **Step 2: Run focused suites** + +```bash +dotnet test tracer/test/Datadog.Trace.SourceGenerators.Tests/Datadog.Trace.SourceGenerators.Tests.csproj -c Release -f net10.0 --disable-build-servers -m:1 +dotnet test tracer/test/Datadog.Trace.Tools.Analyzers.Tests/Datadog.Trace.Tools.Analyzers.Tests.csproj -c Release -f net10.0 --disable-build-servers -m:1 +dotnet test tracer/test/Datadog.Trace.Tests/Datadog.Trace.Tests.csproj -c Release -f net10.0 --filter "FullyQualifiedName~ExporterSettingsTests|FullyQualifiedName~TracerSettingsTests" --disable-build-servers -m:1 +``` + +Expected: 0 failures in all three commands. + +- [x] **Step 3: Run the full tracer build and repository checks** + +```bash +dotnet build tracer/src/Datadog.Trace/Datadog.Trace.csproj -c Release --disable-build-servers -m:1 +git diff --check master...HEAD +``` + +Expected: build exit code 0 for every target framework and no whitespace errors. + +- [x] **Step 4: Commit documentation and plan completion** + +```bash +git add docs/development/Configuration/AddingConfigurationKeys.md docs/superpowers/plans/2026-07-31-sensitive-configuration-analyzer.md +git commit -m "[Configuration] Document sensitive config enforcement" +``` + +- [ ] **Step 5: Run mandatory pre-push review** + +Invoke the repository's `pre-push-review` skill. Apply valid findings, amend them into the commit that introduced the issue, and rerun every affected verification command. + +- [ ] **Step 6: Publish a draft PR** + +Read `.github/pull_request_template.md`, push with an authorized public-repository account, and create a draft PR against `master` with the `AI Generated` label. The description must explain the compile-time design, lack of runtime lookup, and exact test evidence. + +- [ ] **Step 7: Babysit CI** + +Invoke `dd:pr-babysit` and monitor until every real correctness check is green. Ignore `devflow/mergegate` and any aggregator blocked only by that gate. diff --git a/docs/superpowers/specs/2026-07-31-sensitive-configuration-analyzer-design.md b/docs/superpowers/specs/2026-07-31-sensitive-configuration-analyzer-design.md new file mode 100644 index 000000000000..c60125f3251f --- /dev/null +++ b/docs/superpowers/specs/2026-07-31-sensitive-configuration-analyzer-design.md @@ -0,0 +1,89 @@ +# Sensitive Configuration Analyzer Design + +## Context + +Configuration telemetry records most configuration values to help diagnose tracer setup. Some settings, including `DD_API_KEY` and OpenTelemetry exporter headers, contain credentials and must only produce redacted telemetry entries. + +The existing configuration API already supports redacted string reads through `AsRedactedString()` and `AsRedactedStringResult()`. Adding a runtime sensitivity lookup to every telemetry write would protect the values, but it would also add work to the tracer startup hot path. This design moves enforcement to compile time instead. + +## Goals + +- Keep `supported-configurations.yaml` as the single source of truth for sensitive configuration keys. +- Report a build error when tracer code reads a sensitive key through an accessor that records its value. +- Redact the four OTLP header settings without a runtime sensitivity lookup. +- Mark existing credential-bearing settings, currently `DD_API_KEY`, as sensitive. +- Preserve alias fallback, parsing behavior, and public APIs. + +## Non-goals + +- Runtime enforcement for third-party callers or reflection-based configuration reads. +- Data-flow analysis across variables that store `ConfigurationBuilder.HasKeys` values. +- Treating every deliberately redacted value as sensitive. For example, AppSec's HTML template path is redacted because its value can be large, not because it contains a credential. +- Adding a code fix in this change. + +## Architecture + +### YAML metadata + +Add an optional `sensitive: true` property to configuration entries. The shared `YamlReader` will parse and expose the property on `ConfigurationEntry`; omitted values default to `false`. Values are case-insensitive `true` or `false`. Other tokens are parse errors reported by the source generator as `DDSG0007` and by the analyzer as `DD0016`. + +The analyzer project will link the existing shared YAML parser and its helper types. `tracer/src/Directory.Build.props` will supply `supported-configurations.yaml` to every project that receives the analyzer. During compilation start, the analyzer will require exactly one readable additional file, parse it once, and build an immutable set of canonical sensitive keys. Missing, ambiguous, unreadable, or invalid metadata produces `DD0016` instead of disabling the rule. + +### Analyzer rule + +Extend `ConfigurationBuilderWithKeysAnalyzer` with a new error diagnostic for unsafe sensitive-key reads. + +For each `ConfigurationBuilder.WithKeys(...)` invocation, the analyzer will continue enforcing that the argument is a direct `ConfigurationKeys` or `PlatformKeys` constant. When the constant value is in the sensitive-key set, the analyzer will use Roslyn operations and symbols to inspect the immediately chained accessor. Parentheses and conversions around `WithKeys(...)` are transparent, but the accessor must resolve to an instance method on the returned `ConfigurationBuilder.HasKeys` type: + +- `AsRedactedString(...)` is allowed. +- `AsRedactedStringResult(...)` is allowed. +- `AsRedactedDictionaryResult(...)` is allowed. +- `AsStringResult(..., recordValue: false)` is allowed when Roslyn can prove the argument is the constant `false`. +- Any accessor that records values, an unrecognized accessor, or storing the intermediate `HasKeys` value is rejected because the analyzer cannot prove that telemetry is redacted. + +The diagnostic will be reported on the sensitive configuration-key argument and explain which redacted accessors are permitted. + +This deliberately targets the fluent `ConfigurationBuilder` path already governed by the analyzer. Direct `IConfigurationSource` calls are outside this rule and are already intended to be restricted separately. + +### Tracer configuration reads + +Mark these registry entries as sensitive: + +- `DD_API_KEY` +- `OTEL_EXPORTER_OTLP_HEADERS` +- `OTEL_EXPORTER_OTLP_LOGS_HEADERS` +- `OTEL_EXPORTER_OTLP_METRICS_HEADERS` +- `OTEL_EXPORTER_OTLP_TRACES_HEADERS` + +Change the OTLP string reads in `ExporterSettings` to `AsRedactedString()`. Remove the duplicate metrics-header parsing in `TracerSettings`, leaving `ExporterSettings` as its owner. + +`TracerSettings` still owns OTLP log headers as a parsed dictionary. It will use `AsRedactedDictionaryResult()` so `JsonConfigurationSource` keeps its object and array handling. The accessor passes an internal telemetry decorator to the existing dictionary API. The decorator forces string-value records to use `recordValue: false`, including parse and unexpected-type errors, and forwards every other operation unchanged. The existing default, filtering, and trimming flow remains in place. Alias fallback remains in `ConfigurationBuilder`, so the general header setting continues to work as the logs fallback. + +No runtime `IsSensitive()` method or sensitivity collection will be generated. + +## Data Flow + +1. MSBuild supplies `supported-configurations.yaml` as a Roslyn additional file to every analyzer consumer under `tracer/src`. +2. The analyzer parses the registry once at compilation start and collects keys marked `sensitive: true`. +3. A syntax action resolves each `WithKeys()` argument to its constant value. +4. Sensitive constants must flow directly into an accessor that records telemetry with `recordValue: false`. +5. At runtime, redacted string accessors pass `recordValue: false` to configuration sources. The redacted dictionary accessor wraps telemetry internally and uses the existing dictionary interface. Both paths record redacted entries without a key lookup. + +## Testing + +Follow red-green TDD for each behavior: + +- YAML parser tests prove mixed-case boolean values are captured, omitted values default to false, parser state resets between entries, and invalid tokens produce `DDSG0007`. +- Analyzer tests prove ordinary keys may use normal accessors. +- Analyzer tests prove sensitive keys may use both redacted accessors and an explicit constant `recordValue: false`. +- Analyzer tests prove sensitive keys fail with normal string/dictionary accessors, `recordValue: true`, non-constant values, missing accessors, and stored intermediate values. +- Analyzer tests prove missing or invalid sensitivity metadata fails closed with `DD0016`, including in a consumer assembly. +- Configuration tests prove all OTLP header values produce non-empty redacted telemetry while a nearby non-sensitive setting still records normally. Log-header coverage includes JSON objects, primary and alias strings, and unexpected object types. +- Existing API-key call sites compile under the new rule because they already use redacted accessors. +- Existing OTLP header parsing and fallback tests continue to pass after ownership consolidation. + +Run the analyzer and source-generator test suites, affected tracer configuration tests, and a full `Datadog.Trace` build before publishing. + +## Documentation + +Update the configuration-key development guide to document `sensitive`, its compile-time enforcement, and the accepted redacted access patterns. diff --git a/tracer/build/_build/NativeValidation/YamlReader.cs b/tracer/build/_build/NativeValidation/YamlReader.cs index d25dae88b7d2..065e39b25f27 100644 --- a/tracer/build/_build/NativeValidation/YamlReader.cs +++ b/tracer/build/_build/NativeValidation/YamlReader.cs @@ -32,6 +32,7 @@ public static ParsedConfigurationData ParseSupportedConfigurations(string yamlCo string? currentConstName = null; string[]? currentScope = null; var currentAliases = new List(); + var currentSensitive = false; var inDocumentation = false; var inAliases = false; var documentationBuilder = new StringBuilder(); @@ -65,7 +66,7 @@ public static ParsedConfigurationData ParseSupportedConfigurations(string yamlCo if (currentConfigKey != null) { var doc = inDocumentation ? documentationBuilder.ToString().TrimEnd() : currentDocumentation; - configurations[currentConfigKey] = new ConfigurationEntry(currentConfigKey, currentProduct ?? string.Empty, doc, currentConstName, currentScope, currentAliases.Count > 0 ? currentAliases.ToArray() : null); + configurations[currentConfigKey] = new ConfigurationEntry(currentConfigKey, currentProduct ?? string.Empty, doc, currentConstName, currentScope, currentAliases.Count > 0 ? currentAliases.ToArray() : null, currentSensitive); } inSupportedConfigurations = false; @@ -118,7 +119,7 @@ public static ParsedConfigurationData ParseSupportedConfigurations(string yamlCo if (currentConfigKey != null) { var doc = inDocumentation ? documentationBuilder.ToString().TrimEnd() : currentDocumentation; - configurations[currentConfigKey] = new ConfigurationEntry(currentConfigKey, currentProduct ?? string.Empty, doc, currentConstName, currentScope, currentAliases.Count > 0 ? currentAliases.ToArray() : null); + configurations[currentConfigKey] = new ConfigurationEntry(currentConfigKey, currentProduct ?? string.Empty, doc, currentConstName, currentScope, currentAliases.Count > 0 ? currentAliases.ToArray() : null, currentSensitive); } currentConfigKey = potentialKey; @@ -127,6 +128,7 @@ public static ParsedConfigurationData ParseSupportedConfigurations(string yamlCo currentConstName = null; currentScope = null; currentAliases.Clear(); + currentSensitive = false; inDocumentation = false; inAliases = false; documentationBuilder.Clear(); @@ -144,7 +146,7 @@ public static ParsedConfigurationData ParseSupportedConfigurations(string yamlCo if (propColonIdx > 0) { var propName = trimmedLine.Substring(0, propColonIdx); - if (propName is "const_name" or "product" or "implementation" or "type" or "default" or "aliases" or "deprecation_message" or "scope") + if (propName is "const_name" or "product" or "implementation" or "type" or "default" or "aliases" or "deprecation_message" or "scope" or "sensitive") { // End of documentation, process this property inDocumentation = false; @@ -259,6 +261,21 @@ public static ParsedConfigurationData ParseSupportedConfigurations(string yamlCo break; case "aliases": inAliases = true; + break; + case "sensitive": + if (propValue.Equals("true", StringComparison.OrdinalIgnoreCase)) + { + currentSensitive = true; + } + else if (propValue.Equals("false", StringComparison.OrdinalIgnoreCase)) + { + currentSensitive = false; + } + else + { + throw new InvalidOperationException($"Invalid sensitive value on line {lineNumber}: '{propValue}'. Expected true or false."); + } + break; case "documentation": if (propValue == "|-" || propValue == "|") @@ -303,7 +320,7 @@ public static ParsedConfigurationData ParseSupportedConfigurations(string yamlCo if (currentConfigKey != null) { var doc = inDocumentation ? documentationBuilder.ToString().TrimEnd() : currentDocumentation; - configurations[currentConfigKey] = new ConfigurationEntry(currentConfigKey, currentProduct ?? string.Empty, doc, currentConstName, currentScope, currentAliases.Count > 0 ? currentAliases.ToArray() : null); + configurations[currentConfigKey] = new ConfigurationEntry(currentConfigKey, currentProduct ?? string.Empty, doc, currentConstName, currentScope, currentAliases.Count > 0 ? currentAliases.ToArray() : null, currentSensitive); } return new ParsedConfigurationData(configurations, deprecations); @@ -371,7 +388,7 @@ public bool MoveNext() /// internal readonly struct ConfigurationEntry : IEquatable { - public ConfigurationEntry(string key, string? product, string? documentation, string? constName, string[]? scope, string[]? aliases = null) + public ConfigurationEntry(string key, string? product, string? documentation, string? constName, string[]? scope, string[]? aliases = null, bool sensitive = false) { Key = key; Product = product; @@ -379,6 +396,7 @@ public ConfigurationEntry(string key, string? product, string? documentation, st ConstName = constName; Scope = scope is null ? default : new EquatableArray(scope); Aliases = aliases is null ? default : new EquatableArray(aliases); + Sensitive = sensitive; } public string Key { get; } @@ -393,17 +411,20 @@ public ConfigurationEntry(string key, string? product, string? documentation, st public EquatableArray Aliases { get; } + public bool Sensitive { get; } + public bool Equals(ConfigurationEntry other) => Key == other.Key && Product == other.Product && Documentation == other.Documentation && ConstName == other.ConstName && Scope == other.Scope - && Aliases == other.Aliases; + && Aliases == other.Aliases + && Sensitive == other.Sensitive; public override bool Equals(object? obj) => obj is ConfigurationEntry other && Equals(other); - public override int GetHashCode() => HashCode.Combine(Key, Product, Documentation, ConstName, Scope, Aliases); + public override int GetHashCode() => HashCode.Combine(Key, Product, Documentation, ConstName, Scope, Aliases, Sensitive); } /// diff --git a/tracer/src/Datadog.Trace.Tools.Analyzers/ConfigurationAnalyzers/ConfigurationBuilderWithKeysAnalyzer.cs b/tracer/src/Datadog.Trace.Tools.Analyzers/ConfigurationAnalyzers/ConfigurationBuilderWithKeysAnalyzer.cs index a4e5cb81412e..b7336b22590c 100644 --- a/tracer/src/Datadog.Trace.Tools.Analyzers/ConfigurationAnalyzers/ConfigurationBuilderWithKeysAnalyzer.cs +++ b/tracer/src/Datadog.Trace.Tools.Analyzers/ConfigurationAnalyzers/ConfigurationBuilderWithKeysAnalyzer.cs @@ -4,12 +4,18 @@ // #nullable enable +using System; using System.Collections.Immutable; +using System.IO; +using System.Linq; +using System.Threading; +using Datadog.Trace.SourceGenerators.Helpers; using Datadog.Trace.Tools.Analyzers.Helpers; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; namespace Datadog.Trace.Tools.Analyzers.ConfigurationAnalyzers { @@ -20,6 +26,8 @@ namespace Datadog.Trace.Tools.Analyzers.ConfigurationAnalyzers [DiagnosticAnalyzer(LanguageNames.CSharp)] public class ConfigurationBuilderWithKeysAnalyzer : DiagnosticAnalyzer { + private const string SupportedConfigurationsFileName = "supported-configurations.yaml"; + /// /// Diagnostic descriptor for when WithKeys is called with a hardcoded string instead of a constant from PlatformKeys or ConfigurationKeys. /// @@ -44,11 +52,30 @@ public class ConfigurationBuilderWithKeysAnalyzer : DiagnosticAnalyzer isEnabledByDefault: true, description: "ConfigurationBuilder.WithKeys method calls should only accept string constants from PlatformKeys or ConfigurationKeys classes, not variables or computed values."); + private static readonly DiagnosticDescriptor RedactSensitiveConfigurationRule = new( + id: "DD0015", + title: "Redact sensitive configuration values", + messageFormat: "Sensitive configuration key '{0}' must be read with AsRedactedString, AsRedactedStringResult, AsRedactedDictionaryResult, or AsStringResult with compile-time recordValue: false", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "Sensitive configuration values must not be recorded in configuration telemetry."); + + private static readonly DiagnosticDescriptor SensitiveConfigurationMetadataRule = new( + id: "DD0016", + title: "Load sensitive configuration metadata", + messageFormat: "The analyzer requires exactly one readable and valid supported-configurations.yaml additional file", + category: "Usage", + defaultSeverity: DiagnosticSeverity.Error, + isEnabledByDefault: true, + description: "Sensitive configuration metadata must be available so configuration telemetry redaction can be enforced.", + customTags: WellKnownDiagnosticTags.CompilationEnd); + /// /// Gets the supported diagnostics /// public override ImmutableArray SupportedDiagnostics => - [UseConfigurationConstantsRule, UseConfigurationConstantsNotVariablesRule, Diagnostics.MissingRequiredType]; + [UseConfigurationConstantsRule, UseConfigurationConstantsNotVariablesRule, RedactSensitiveConfigurationRule, SensitiveConfigurationMetadataRule, Diagnostics.MissingRequiredType]; /// /// Initialize the analyzer @@ -80,15 +107,22 @@ public override void Initialize(AnalysisContext context) return; } + if (!TryGetSensitiveKeys(compilationContext.Options, compilationContext.CancellationToken, out var sensitiveKeys)) + { + compilationContext.RegisterCompilationEndAction( + c => c.ReportDiagnostic(Diagnostic.Create(SensitiveConfigurationMetadataRule, Location.None))); + return; + } + var targetTypes = new TargetTypeSymbols(configurationBuilder, configurationKeys, platformKeys); compilationContext.RegisterSyntaxNodeAction( - c => AnalyzeInvocationExpression(c, in targetTypes), + c => AnalyzeInvocationExpression(c, in targetTypes, sensitiveKeys), SyntaxKind.InvocationExpression); }); } - private static void AnalyzeInvocationExpression(SyntaxNodeAnalysisContext context, in TargetTypeSymbols targetTypes) + private static void AnalyzeInvocationExpression(SyntaxNodeAnalysisContext context, in TargetTypeSymbols targetTypes, ImmutableHashSet sensitiveKeys) { var invocation = (InvocationExpressionSyntax)context.Node; @@ -118,11 +152,17 @@ private static void AnalyzeInvocationExpression(SyntaxNodeAnalysisContext contex if (argumentList?.Arguments.Count > 0) { var argument = argumentList.Arguments[0]; - AnalyzeConfigurationArgument(context, argument, WellKnownTypeNames.WithKeysMethodName, targetTypes); + AnalyzeConfigurationArgument(context, invocation, argument, WellKnownTypeNames.WithKeysMethodName, targetTypes, sensitiveKeys); } } - private static void AnalyzeConfigurationArgument(SyntaxNodeAnalysisContext context, ArgumentSyntax argument, string methodName, TargetTypeSymbols targetTypes) + private static void AnalyzeConfigurationArgument( + SyntaxNodeAnalysisContext context, + InvocationExpressionSyntax invocation, + ArgumentSyntax argument, + string methodName, + TargetTypeSymbols targetTypes, + ImmutableHashSet sensitiveKeys) { var expression = argument.Expression; @@ -141,7 +181,7 @@ private static void AnalyzeConfigurationArgument(SyntaxNodeAnalysisContext conte case MemberAccessExpressionSyntax memberAccess: // Check if this is accessing a constant from PlatformKeys or ConfigurationKeys - if (!IsValidConfigurationConstant(memberAccess, context.SemanticModel, targetTypes)) + if (!TryGetValidConfigurationConstant(memberAccess, context.SemanticModel, targetTypes, out var field)) { // This is accessing something else - report diagnostic var memberName = memberAccess.ToString(); @@ -152,6 +192,12 @@ private static void AnalyzeConfigurationArgument(SyntaxNodeAnalysisContext conte memberName); context.ReportDiagnostic(memberDiagnostic); } + else if (field?.ConstantValue is string key + && sensitiveKeys.Contains(key) + && !IsRedactedRead(invocation, context.SemanticModel, context.CancellationToken)) + { + context.ReportDiagnostic(Diagnostic.Create(RedactSensitiveConfigurationRule, memberAccess.GetLocation(), key)); + } break; @@ -179,26 +225,113 @@ private static void AnalyzeConfigurationArgument(SyntaxNodeAnalysisContext conte } } - private static bool IsValidConfigurationConstant(MemberAccessExpressionSyntax memberAccess, SemanticModel semanticModel, TargetTypeSymbols targetTypes) + private static bool TryGetValidConfigurationConstant( + MemberAccessExpressionSyntax memberAccess, + SemanticModel semanticModel, + TargetTypeSymbols targetTypes, + out IFieldSymbol? field) { var symbolInfo = semanticModel.GetSymbolInfo(memberAccess); - if (symbolInfo.Symbol is IFieldSymbol field) + if (symbolInfo.Symbol is IFieldSymbol fieldSymbol) { // Check if this is a const string field - if (field.IsConst && field.Type?.SpecialType == SpecialType.System_String) + if (fieldSymbol.IsConst && fieldSymbol.Type?.SpecialType == SpecialType.System_String) { - var containingType = field.ContainingType; + var containingType = fieldSymbol.ContainingType; if (containingType != null) { // Check if the containing type is PlatformKeys or ConfigurationKeys (or their nested classes) - return IsValidConfigurationClass(containingType, targetTypes); + if (IsValidConfigurationClass(containingType, targetTypes)) + { + field = fieldSymbol; + return true; + } } } } + field = null; return false; } + private static bool IsRedactedRead(InvocationExpressionSyntax withKeysInvocation, SemanticModel semanticModel, CancellationToken cancellationToken) + { + if (semanticModel.GetOperation(withKeysInvocation, cancellationToken) is not IInvocationOperation withKeysOperation) + { + return false; + } + + IOperation current = withKeysOperation; + while (current.Parent is IParenthesizedOperation or IConversionOperation) + { + current = current.Parent; + } + + if (current.Parent is not IInvocationOperation accessorOperation + || accessorOperation.TargetMethod.IsStatic + || !SymbolEqualityComparer.Default.Equals(accessorOperation.TargetMethod.ContainingType, withKeysOperation.TargetMethod.ReturnType)) + { + return false; + } + + if (accessorOperation.TargetMethod.Name is "AsRedactedString" or "AsRedactedStringResult" or "AsRedactedDictionaryResult") + { + return true; + } + + if (accessorOperation.TargetMethod.Name != "AsStringResult") + { + return false; + } + + var recordValueArgument = accessorOperation.Arguments.FirstOrDefault(x => x.Parameter?.Name == "recordValue"); + return recordValueArgument?.Value.ConstantValue is { HasValue: true, Value: false }; + } + + private static bool TryGetSensitiveKeys(AnalyzerOptions options, CancellationToken cancellationToken, out ImmutableHashSet sensitiveKeys) + { + var files = options.AdditionalFiles + .Where(x => Path.GetFileName(x.Path).Equals(SupportedConfigurationsFileName, StringComparison.OrdinalIgnoreCase)) + .Take(2) + .ToArray(); + if (files.Length != 1) + { + sensitiveKeys = ImmutableHashSet.Empty; + return false; + } + + try + { + var content = files[0].GetText(cancellationToken)?.ToString(); + if (string.IsNullOrEmpty(content)) + { + sensitiveKeys = ImmutableHashSet.Empty; + return false; + } + + var configurations = YamlReader.ParseSupportedConfigurations(content!).Configurations; + if (configurations.Count == 0) + { + sensitiveKeys = ImmutableHashSet.Empty; + return false; + } + + sensitiveKeys = configurations.Where(x => x.Value.Sensitive) + .Select(x => x.Key) + .ToImmutableHashSet(StringComparer.Ordinal); + return true; + } + catch (OperationCanceledException) + { + throw; + } + catch + { + sensitiveKeys = ImmutableHashSet.Empty; + return false; + } + } + private static bool IsValidConfigurationClass(INamedTypeSymbol typeSymbol, TargetTypeSymbols targetTypes) { // Check if this is PlatformKeys or ConfigurationKeys class or their nested classes diff --git a/tracer/src/Datadog.Trace.Tools.Analyzers/Datadog.Trace.Tools.Analyzers.csproj b/tracer/src/Datadog.Trace.Tools.Analyzers/Datadog.Trace.Tools.Analyzers.csproj index cb5299e8c72e..f57631cc0b5b 100644 --- a/tracer/src/Datadog.Trace.Tools.Analyzers/Datadog.Trace.Tools.Analyzers.csproj +++ b/tracer/src/Datadog.Trace.Tools.Analyzers/Datadog.Trace.Tools.Analyzers.csproj @@ -13,6 +13,9 @@ + + + Helpers\System.Diagnostics.CodeAnalysis.Attributes.cs diff --git a/tracer/src/Datadog.Trace/Configuration/ConfigurationSources/Telemetry/ConfigurationBuilder.cs b/tracer/src/Datadog.Trace/Configuration/ConfigurationSources/Telemetry/ConfigurationBuilder.cs index 2e7045866d53..819012eaa816 100644 --- a/tracer/src/Datadog.Trace/Configuration/ConfigurationSources/Telemetry/ConfigurationBuilder.cs +++ b/tracer/src/Datadog.Trace/Configuration/ConfigurationSources/Telemetry/ConfigurationBuilder.cs @@ -10,6 +10,7 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using Datadog.Trace.Configuration.ConfigurationSources.Telemetry; +using Datadog.Trace.Telemetry; namespace Datadog.Trace.Configuration.Telemetry; @@ -468,6 +469,9 @@ public ClassConfigurationResultWithKey> AsDictionary public ClassConfigurationResultWithKey> AsDictionaryResult(bool allowOptionalMappings, char separator) => new(Telemetry, Key, recordValue: true, configurationResult: GetDictionaryResult(allowOptionalMappings, separator)); + public ClassConfigurationResultWithKey> AsRedactedDictionaryResult(char separator) + => new(Telemetry, Key, recordValue: false, configurationResult: GetDictionaryResult(allowOptionalMappings: false, separator, recordValue: false)); + public ClassConfigurationResultWithKey> AsDictionaryResult(Func> parser) => new(Telemetry, Key, recordValue: true, configurationResult: GetDictionaryResult(parser)); @@ -514,10 +518,10 @@ private ConfigurationResult GetAs(Func? validator, Func source.GetAs(key, telemetry, converter, validator, recordValue: true)); } - private ConfigurationResult> GetDictionaryResult(bool allowOptionalMappings, char separator) + private ConfigurationResult> GetDictionaryResult(bool allowOptionalMappings, char separator, bool recordValue = true) { var source = Source; - var telemetry = Telemetry; + IConfigurationTelemetry telemetry = recordValue ? Telemetry : new RedactedConfigurationTelemetry(Telemetry); return GetResultWithFallback(key => source.GetDictionary(key, telemetry, validator: null, allowOptionalMappings, separator)); } @@ -555,6 +559,38 @@ private ConfigurationResult GetResultWithFallback(Func _telemetry.Record(key, value, recordValue: false, origin, error); + + public void Record(string key, bool value, ConfigurationOrigins origin, TelemetryErrorCode? error = null) + => _telemetry.Record(key, value, origin, error); + + public void Record(string key, double value, ConfigurationOrigins origin, TelemetryErrorCode? error = null) + => _telemetry.Record(key, value, origin, error); + + public void Record(string key, int value, ConfigurationOrigins origin, TelemetryErrorCode? error = null) + => _telemetry.Record(key, value, origin, error); + + public void Record(string key, double? value, ConfigurationOrigins origin, TelemetryErrorCode? error = null) + => _telemetry.Record(key, value, origin, error); + + public void Record(string key, int? value, ConfigurationOrigins origin, TelemetryErrorCode? error = null) + => _telemetry.Record(key, value, origin, error); + + public ICollection? GetIncrementalData() + => _telemetry.GetIncrementalData(); + + public void CopyTo(IConfigurationTelemetry destination) + => _telemetry.CopyTo(destination); + + public ICollection? GetFullData() + => _telemetry.GetFullData(); + } } internal readonly struct StructConfigurationResultWithKey diff --git a/tracer/src/Datadog.Trace/Configuration/ExporterSettings.cs b/tracer/src/Datadog.Trace/Configuration/ExporterSettings.cs index 87187e6941d7..b04abba2c944 100644 --- a/tracer/src/Datadog.Trace/Configuration/ExporterSettings.cs +++ b/tracer/src/Datadog.Trace/Configuration/ExporterSettings.cs @@ -592,7 +592,7 @@ public Raw(IConfigurationSource source, IConfigurationTelemetry telemetry) .WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpTimeoutMs) .AsInt32(10_000, value => value > 0) .Value; - OtlpHeaders = config.WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpHeaders).AsString()?.Trim(); + OtlpHeaders = config.WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpHeaders).AsRedactedString()?.Trim(); OtlpMetricsProtocol = config.WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpMetricsProtocol).AsString()?.Trim(); OtlpMetricsEndpoint = config.WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpMetricsEndpoint).AsString()?.Trim(); @@ -600,7 +600,7 @@ public Raw(IConfigurationSource source, IConfigurationTelemetry telemetry) .WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpMetricsTimeoutMs) .AsInt32(OtlpTimeoutMs, value => value > 0) .Value; - OtlpMetricsHeaders = config.WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpMetricsHeaders).AsString()?.Trim(); + OtlpMetricsHeaders = config.WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpMetricsHeaders).AsRedactedString()?.Trim(); OtlpTracesProtocol = config.WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpTracesProtocol).AsString()?.Trim(); OtlpTracesEndpoint = config.WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpTracesEndpoint).AsString()?.Trim(); @@ -608,7 +608,7 @@ public Raw(IConfigurationSource source, IConfigurationTelemetry telemetry) .WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpTracesTimeoutMs) .AsInt32(OtlpTimeoutMs, value => value > 0) .Value; - OtlpTracesHeaders = config.WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpTracesHeaders).AsString()?.Trim(); + OtlpTracesHeaders = config.WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpTracesHeaders).AsRedactedString()?.Trim(); } /// diff --git a/tracer/src/Datadog.Trace/Configuration/TracerSettings.cs b/tracer/src/Datadog.Trace/Configuration/TracerSettings.cs index 81cea58b0535..22ad610548de 100644 --- a/tracer/src/Datadog.Trace/Configuration/TracerSettings.cs +++ b/tracer/src/Datadog.Trace/Configuration/TracerSettings.cs @@ -282,13 +282,6 @@ not null when string.Equals(x, "http/json", StringComparison.OrdinalIgnoreCase) validator: null, converter: uriString => new Uri(uriString)); - OtlpMetricsHeaders = config - .WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpMetricsHeaders) - .AsDictionaryResult(separator: '=') - .WithDefault(new DefaultResult>(new Dictionary(), "[]")) - .Where(kvp => !string.IsNullOrWhiteSpace(kvp.Key)) - .ToDictionary(kvp => kvp.Key.Trim(), kvp => kvp.Value?.Trim() ?? string.Empty); - OtlpMetricsTimeoutMs = config .WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpMetricsTimeoutMs) .AsInt32(defaultValue: 10_000); @@ -334,7 +327,7 @@ not null when string.Equals(x, "http/protobuf", StringComparison.OrdinalIgnoreCa OtlpLogsHeaders = config .WithKeys(ConfigurationKeys.OpenTelemetry.ExporterOtlpLogsHeaders) - .AsDictionaryResult(separator: '=') + .AsRedactedDictionaryResult(separator: '=') .WithDefault(new DefaultResult>(new Dictionary(), "[]")) .Where(kvp => !string.IsNullOrWhiteSpace(kvp.Key)) .ToDictionary(kvp => kvp.Key.Trim(), kvp => kvp.Value?.Trim() ?? string.Empty); @@ -960,14 +953,6 @@ not null when string.Equals(value, "otlp", StringComparison.OrdinalIgnoreCase) = /// internal Uri OtlpEndpoint { get; } - /// - /// Gets the OTLP headers for metrics export with fallback behavior. - /// Parsed from comma-separated key-value pairs (api-key=key,other=value). - /// - /// - /// - internal IReadOnlyDictionary OtlpMetricsHeaders { get; } - /// /// Gets the OpenTelemetry metric export interval (in milliseconds) between export attempts. /// Default is 10000ms (10s) for Datadog - deviates from OTel spec default of 60000ms (60s). diff --git a/tracer/src/Datadog.Trace/Configuration/supported-configurations.yaml b/tracer/src/Datadog.Trace/Configuration/supported-configurations.yaml index b2e4199ed3ec..079de8b63721 100644 --- a/tracer/src/Datadog.Trace/Configuration/supported-configurations.yaml +++ b/tracer/src/Datadog.Trace/Configuration/supported-configurations.yaml @@ -54,6 +54,7 @@ supportedConfigurations: - implementation: A scope: managed, native type: string + sensitive: true default: null const_name: ApiKey documentation: Configuration key for setting the API key, used by the Agent. @@ -3663,6 +3664,7 @@ supportedConfigurations: - implementation: A scope: managed type: map + sensitive: true default: '' product: OpenTelemetry const_name: ExporterOtlpHeaders @@ -3723,6 +3725,7 @@ supportedConfigurations: - implementation: C scope: managed type: map + sensitive: true default: null product: OpenTelemetry aliases: @@ -3736,6 +3739,7 @@ supportedConfigurations: - implementation: B scope: managed type: map + sensitive: true default: null product: OpenTelemetry aliases: @@ -3822,6 +3826,7 @@ supportedConfigurations: - implementation: B scope: managed type: map + sensitive: true default: null product: OpenTelemetry aliases: diff --git a/tracer/src/Datadog.Trace/Datadog.Trace.csproj b/tracer/src/Datadog.Trace/Datadog.Trace.csproj index 04d5b700bb8a..251ebc82c3b1 100644 --- a/tracer/src/Datadog.Trace/Datadog.Trace.csproj +++ b/tracer/src/Datadog.Trace/Datadog.Trace.csproj @@ -113,9 +113,6 @@ - - Never - diff --git a/tracer/src/Directory.Build.props b/tracer/src/Directory.Build.props index 16ebdf0016d8..edd958efaf40 100644 --- a/tracer/src/Directory.Build.props +++ b/tracer/src/Directory.Build.props @@ -45,6 +45,9 @@ + + Never + diff --git a/tracer/test/Datadog.Trace.SourceGenerators.Tests/ConfigurationKeysGeneratorTests.cs b/tracer/test/Datadog.Trace.SourceGenerators.Tests/ConfigurationKeysGeneratorTests.cs index e35ad7576204..925beb2c1617 100644 --- a/tracer/test/Datadog.Trace.SourceGenerators.Tests/ConfigurationKeysGeneratorTests.cs +++ b/tracer/test/Datadog.Trace.SourceGenerators.Tests/ConfigurationKeysGeneratorTests.cs @@ -310,6 +310,29 @@ public void HandlesInvalidYaml() (diagnostics.Any() || !outputs.Any()).Should().BeTrue(); } + [Fact] + public void ReportsParseErrorForInvalidSensitiveValue() + { + const string invalidYaml = """ + version: '2' + supportedConfigurations: + DD_API_KEY: + - implementation: A + scope: managed + sensitive: definitely + documentation: API key used to authenticate with Datadog. + """; + + var (diagnostics, outputs) = TestHelpers.GetGeneratedTrees( + [], + [], + [("supported-configurations.yaml", invalidYaml)], + assertOutput: false); + + diagnostics.Should().ContainSingle(diagnostic => diagnostic.Id == "DDSG0007"); + outputs.Should().BeEmpty(); + } + [Fact] public void SortsEntriesAlphabeticallyByEnvironmentVariable() { diff --git a/tracer/test/Datadog.Trace.SourceGenerators.Tests/YamlReaderTests.cs b/tracer/test/Datadog.Trace.SourceGenerators.Tests/YamlReaderTests.cs new file mode 100644 index 000000000000..564c6addbe2d --- /dev/null +++ b/tracer/test/Datadog.Trace.SourceGenerators.Tests/YamlReaderTests.cs @@ -0,0 +1,62 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +#nullable enable + +using System; +using Datadog.Trace.SourceGenerators.Helpers; +using FluentAssertions; +using Xunit; + +namespace Datadog.Trace.SourceGenerators.Tests; + +public class YamlReaderTests +{ + private const string YamlWithSensitiveAndOrdinaryEntries = """ + version: '2' + supportedConfigurations: + DD_API_KEY: + - implementation: A + sensitive: TrUe + documentation: API key used to authenticate with Datadog. + DD_TRACE_ENABLED: + - implementation: A + sensitive: FaLsE + documentation: Enables the tracer. + DD_SERVICE: + - implementation: A + documentation: The service name. + """; + + [Fact] + public void ParsesSensitiveMetadata() + { + var parsed = YamlReader.ParseSupportedConfigurations(YamlWithSensitiveAndOrdinaryEntries); + + parsed.Configurations["DD_API_KEY"].Sensitive.Should().BeTrue(); + parsed.Configurations["DD_TRACE_ENABLED"].Sensitive.Should().BeFalse(); + parsed.Configurations["DD_SERVICE"].Sensitive.Should().BeFalse(); + } + + [Theory] + [InlineData("yes")] + [InlineData("1")] + [InlineData("")] + public void RejectsInvalidSensitiveMetadata(string value) + { + var yaml = $$""" + version: '2' + supportedConfigurations: + DD_API_KEY: + - implementation: A + sensitive: {{value}} + documentation: API key used to authenticate with Datadog. + """; + + var action = () => YamlReader.ParseSupportedConfigurations(yaml); + + action.Should().Throw(); + } +} diff --git a/tracer/test/Datadog.Trace.Tests/Configuration/ExporterSettingsTests.cs b/tracer/test/Datadog.Trace.Tests/Configuration/ExporterSettingsTests.cs index 8aa583679cb8..96270f84b5b1 100644 --- a/tracer/test/Datadog.Trace.Tests/Configuration/ExporterSettingsTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Configuration/ExporterSettingsTests.cs @@ -4,6 +4,7 @@ // using System; +using System.Collections.Generic; using System.Linq; using Datadog.Trace.Agent; using Datadog.Trace.Configuration; @@ -342,6 +343,49 @@ public void TraceAgentUriBase_WhenNamedPipes_ShowsPipeName() settings.TraceAgentUriBase.Should().Be(@"\\.\pipe\" + pipeName); } + [Theory] + [InlineData(ConfigurationKeys.OpenTelemetry.ExporterOtlpHeaders, "general-header-secret", 3)] + [InlineData(ConfigurationKeys.OpenTelemetry.ExporterOtlpMetricsHeaders, "metrics-header-secret", 1)] + [InlineData(ConfigurationKeys.OpenTelemetry.ExporterOtlpTracesHeaders, "traces-header-secret", 1)] + public void OtlpHeaderIsParsedAndRedactedInTelemetry(string headerKey, string sentinel, int expectedTelemetryCount) + { + var source = BuildSource($"{headerKey}:dd-api-key={sentinel}"); + var telemetry = new ConfigurationTelemetry(); + + var settings = new ExporterSettings(source, NoFile(), telemetry); + var entries = telemetry.GetQueueForTesting(); + + if (headerKey != ConfigurationKeys.OpenTelemetry.ExporterOtlpTracesHeaders) + { + settings.OtlpMetricsHeaders.Should().Contain(new KeyValuePair("dd-api-key", sentinel)); + } + + if (headerKey != ConfigurationKeys.OpenTelemetry.ExporterOtlpMetricsHeaders) + { + settings.OtlpTracesHeaders.Should().Contain(new KeyValuePair("dd-api-key", sentinel)); + } + + entries.Where(x => x.Key == headerKey) + .Should() + .HaveCount(expectedTelemetryCount) + .And.OnlyContain(x => x.Type == ConfigurationTelemetry.ConfigurationTelemetryEntryType.Redacted && x.StringValue == null); + } + + [Fact] + public void OtlpEndpointIsRecordedAsStringTelemetry() + { + const string endpoint = "http://example.com:4318/"; + var source = BuildSource($"{ConfigurationKeys.OpenTelemetry.ExporterOtlpEndpoint}:{endpoint}"); + var telemetry = new ConfigurationTelemetry(); + + _ = new ExporterSettings(source, NoFile(), telemetry); + + telemetry.GetQueueForTesting() + .Where(x => x.Key == ConfigurationKeys.OpenTelemetry.ExporterOtlpEndpoint) + .Should() + .ContainSingle(x => x.Type == ConfigurationTelemetry.ConfigurationTelemetryEntryType.String && x.StringValue == endpoint); + } + private static ExporterSettings Setup(IConfigurationSource source, Func fileExists) { return new ExporterSettings(source, fileExists, NullConfigurationTelemetry.Instance); diff --git a/tracer/test/Datadog.Trace.Tests/Configuration/IConfigurationSourceCompatibilityTests.cs b/tracer/test/Datadog.Trace.Tests/Configuration/IConfigurationSourceCompatibilityTests.cs new file mode 100644 index 000000000000..8ffc7c6ff9b7 --- /dev/null +++ b/tracer/test/Datadog.Trace.Tests/Configuration/IConfigurationSourceCompatibilityTests.cs @@ -0,0 +1,57 @@ +// +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License. +// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. +// + +#nullable enable + +using System; +using System.Collections.Generic; +using Datadog.Trace.Configuration; +using Datadog.Trace.Configuration.ConfigurationSources.Telemetry; +using Datadog.Trace.Configuration.Telemetry; +using Xunit; + +namespace Datadog.Trace.Tests.Configuration; + +public class IConfigurationSourceCompatibilityTests +{ + [Fact] + public void ExistingContractRemainsImplementable() + { + IConfigurationSource source = new PreExistingConfigurationSource(); + + var result = source.GetString("missing", NullConfigurationTelemetry.Instance, validator: null, recordValue: true); + + Assert.False(result.IsPresent); + } + + private sealed class PreExistingConfigurationSource : IConfigurationSource + { + public ConfigurationOrigins Origin => ConfigurationOrigins.Code; + + public ConfigurationResult GetString(string key, IConfigurationTelemetry telemetry, Func? validator, bool recordValue) + => ConfigurationResult.NotFound(); + + public ConfigurationResult GetInt32(string key, IConfigurationTelemetry telemetry, Func? validator) + => ConfigurationResult.NotFound(); + + public ConfigurationResult GetDouble(string key, IConfigurationTelemetry telemetry, Func? validator) + => ConfigurationResult.NotFound(); + + public ConfigurationResult GetBool(string key, IConfigurationTelemetry telemetry, Func? validator) + => ConfigurationResult.NotFound(); + + public ConfigurationResult> GetDictionary(string key, IConfigurationTelemetry telemetry, Func, bool>? validator) + => ConfigurationResult>.NotFound(); + + public ConfigurationResult> GetDictionary(string key, IConfigurationTelemetry telemetry, Func, bool>? validator, bool allowOptionalMappings, char separator) + => ConfigurationResult>.NotFound(); + + public ConfigurationResult> GetDictionary(string key, IConfigurationTelemetry telemetry, Func, bool>? validator, Func> parser) + => ConfigurationResult>.NotFound(); + + public ConfigurationResult GetAs(string key, IConfigurationTelemetry telemetry, Func> converter, Func? validator, bool recordValue) + => ConfigurationResult.NotFound(); + } +} diff --git a/tracer/test/Datadog.Trace.Tests/Configuration/TracerSettingsTests.cs b/tracer/test/Datadog.Trace.Tests/Configuration/TracerSettingsTests.cs index 080a0e9aabc1..76f55adaba89 100644 --- a/tracer/test/Datadog.Trace.Tests/Configuration/TracerSettingsTests.cs +++ b/tracer/test/Datadog.Trace.Tests/Configuration/TracerSettingsTests.cs @@ -1179,20 +1179,6 @@ public void OtlpMetricsTemporalityPreference(string value, object expected) settings.OtlpMetricsTemporalityPreference.Should().Be((OtlpTemporalityPreference)expected); } - [Theory] - [InlineData("api-key=secret,auth=token", null, new[] { "api-key=secret", "auth=token" })] - [InlineData(null, "key1 = value1 , key2 = value2 ", new[] { "key1=value1", "key2=value2" })] - [InlineData("valid=value,invalid-no-equals,another=valid", "fallback-key=fallback-value", new[] { "valid=value", "another=valid" })] - public void OtlpHeadersParsing(string primaryValue, string fallbackValue, string[] expected) - { - var source = CreateConfigurationSource( - (ConfigurationKeys.OpenTelemetry.ExporterOtlpMetricsHeaders, primaryValue), - (ConfigurationKeys.OpenTelemetry.ExporterOtlpHeaders, fallbackValue)); - var settings = new TracerSettings(source); - - settings.OtlpMetricsHeaders.Should().BeEquivalentTo(expected.ToDictionary(v => v.Split('=').First(), v => v.Split('=').Last())); - } - [Theory] [MemberData(nameof(BooleanTestCases), false)] public void PartialFlushEnabled(string value, bool expected) @@ -1276,6 +1262,88 @@ public void OtlpLogsHeadersParsing(string primaryValue, string fallbackValue, st settings.OtlpLogsHeaders.Should().BeEquivalentTo(expected.ToDictionary(v => v.Split('=').First(), v => v.Split('=').Last())); } + [Fact] + public void OtlpLogsHeadersAreParsedAndRedactedInTelemetry() + { + const string logsSentinel = "logs-header-secret"; + var source = CreateConfigurationSource((ConfigurationKeys.OpenTelemetry.ExporterOtlpLogsHeaders, $"dd-api-key={logsSentinel}")); + var telemetry = new ConfigurationTelemetry(); + + var settings = new TracerSettings(source, telemetry, new()); + var entries = telemetry.GetQueueForTesting(); + + settings.OtlpLogsHeaders.Should().Contain(new KeyValuePair("dd-api-key", logsSentinel)); + entries.Where(x => x.Key == ConfigurationKeys.OpenTelemetry.ExporterOtlpLogsHeaders) + .Should() + .ContainSingle(x => x.Type == ConfigurationTelemetry.ConfigurationTelemetryEntryType.Redacted && x.StringValue == null); + } + + [Fact] + public void OtlpLogsHeadersFromJsonObjectAreParsedAndRedactedInTelemetry() + { + const string sentinel = "json-object-secret"; + var source = new JsonConfigurationSource( + $@"{{""{ConfigurationKeys.OpenTelemetry.ExporterOtlpLogsHeaders}"":{{"" api-key "":"" {sentinel} "",""auth"":"" token ""}}}}", + ConfigurationOrigins.Code); + var telemetry = new ConfigurationTelemetry(); + + var settings = new TracerSettings(source, telemetry, new()); + + settings.OtlpLogsHeaders.Should().BeEquivalentTo( + new Dictionary + { + ["api-key"] = sentinel, + ["auth"] = "token", + }); + telemetry.GetQueueForTesting() + .Where(x => x.Key == ConfigurationKeys.OpenTelemetry.ExporterOtlpLogsHeaders) + .Should() + .ContainSingle(x => x.Type == ConfigurationTelemetry.ConfigurationTelemetryEntryType.Redacted && x.StringValue == null); + } + + [Theory] + [InlineData(ConfigurationKeys.OpenTelemetry.ExporterOtlpLogsHeaders, 1)] + [InlineData(ConfigurationKeys.OpenTelemetry.ExporterOtlpHeaders, 4)] + public void OtlpLogsHeadersFromJsonStringAreParsedAndRedactedInTelemetry(string configuredKey, int expectedTelemetryCount) + { + const string sentinel = "json-string-secret"; + var source = new JsonConfigurationSource( + $@"{{""{configuredKey}"":"" api-key = {sentinel} , auth = token ""}}", + ConfigurationOrigins.Code); + var telemetry = new ConfigurationTelemetry(); + + var settings = new TracerSettings(source, telemetry, new()); + + settings.OtlpLogsHeaders.Should().BeEquivalentTo( + new Dictionary + { + ["api-key"] = sentinel, + ["auth"] = "token", + }); + telemetry.GetQueueForTesting() + .Where(x => x.Key == configuredKey) + .Should() + .HaveCount(expectedTelemetryCount) + .And.OnlyContain(x => x.Type == ConfigurationTelemetry.ConfigurationTelemetryEntryType.Redacted && x.StringValue == null); + } + + [Fact] + public void OtlpLogsHeadersWithUnexpectedObjectTypeDoNotLeakTelemetry() + { + var source = new DictionaryObjectConfigurationSource( + new Dictionary { [ConfigurationKeys.OpenTelemetry.ExporterOtlpLogsHeaders] = 42 }); + var telemetry = new ConfigurationTelemetry(); + + var settings = new TracerSettings(source, telemetry, new()); + + settings.OtlpLogsHeaders.Should().BeEmpty(); + telemetry.GetQueueForTesting() + .Where(x => x.Key == ConfigurationKeys.OpenTelemetry.ExporterOtlpLogsHeaders) + .Should() + .HaveCount(2) + .And.OnlyContain(x => x.Type == ConfigurationTelemetry.ConfigurationTelemetryEntryType.Redacted && x.StringValue == null); + } + [Theory] [InlineData(null, 10000)] [InlineData("5000", 5000)] // User custom value diff --git a/tracer/test/Datadog.Trace.Tools.Analyzers.Tests/ConfigurationAnalyzers/AnalyzerTestHelper.cs b/tracer/test/Datadog.Trace.Tools.Analyzers.Tests/ConfigurationAnalyzers/AnalyzerTestHelper.cs index 88daa61d5d53..be0f140bc408 100644 --- a/tracer/test/Datadog.Trace.Tools.Analyzers.Tests/ConfigurationAnalyzers/AnalyzerTestHelper.cs +++ b/tracer/test/Datadog.Trace.Tools.Analyzers.Tests/ConfigurationAnalyzers/AnalyzerTestHelper.cs @@ -3,6 +3,8 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. // +#nullable enable + using System.Threading.Tasks; using Datadog.Trace.Tools.Analyzers.ConfigurationAnalyzers; using Microsoft.CodeAnalysis; @@ -14,6 +16,17 @@ namespace Datadog.Trace.Tools.Analyzers.Tests.ConfigurationAnalyzers; internal static class AnalyzerTestHelper { + public const string SupportedConfigurationsYaml = """ + version: '2' + supportedConfigurations: + DD_API_KEY: + - implementation: A + sensitive: true + DD_SERVICE: + - implementation: A + sensitive: false + """; + /// /// Minimal required type definitions to prevent DD0009 errors in tests. /// Does not include ConfigurationBuilder/HasKeys for tests that define these themselves. @@ -52,6 +65,26 @@ public struct HasKeys { } /// public static async Task VerifyDatadogAnalyzerAsync(string source, params DiagnosticResult[] expected) where TAnalyzer : DiagnosticAnalyzer, new() + => await VerifyDatadogAnalyzerAsync(source, SupportedConfigurationsYaml, expected); + + public static async Task VerifyDatadogAnalyzerWithoutSupportedConfigurationsAsync(string source, params DiagnosticResult[] expected) + where TAnalyzer : DiagnosticAnalyzer, new() + => await VerifyDatadogAnalyzerAsync(source, supportedConfigurationsYaml: null, expected); + + public static async Task VerifyDatadogAnalyzerWithSupportedConfigurationsAsync(string source, string supportedConfigurationsYaml, params DiagnosticResult[] expected) + where TAnalyzer : DiagnosticAnalyzer, new() + => await VerifyDatadogAnalyzerAsync(source, supportedConfigurationsYaml, expected); + + public static async Task VerifyAnalyzerInAssemblyWithSupportedConfigurationsAsync(string source, string assemblyName, params DiagnosticResult[] expected) + where TAnalyzer : DiagnosticAnalyzer, new() + => await VerifyAnalyzerAsync(source, SupportedConfigurationsYaml, assemblyName, expected); + + private static async Task VerifyDatadogAnalyzerAsync(string source, string? supportedConfigurationsYaml, params DiagnosticResult[] expected) + where TAnalyzer : DiagnosticAnalyzer, new() + => await VerifyAnalyzerAsync(source, supportedConfigurationsYaml, "Datadog.Trace", expected); + + private static async Task VerifyAnalyzerAsync(string source, string? supportedConfigurationsYaml, string assemblyName, params DiagnosticResult[] expected) + where TAnalyzer : DiagnosticAnalyzer, new() { var test = new CSharpAnalyzerTest { @@ -61,8 +94,13 @@ public static async Task VerifyDatadogAnalyzerAsync(string source, pa } }; + if (supportedConfigurationsYaml is not null) + { + test.TestState.AdditionalFiles.Add(("supported-configurations.yaml", supportedConfigurationsYaml)); + } + test.TestState.ExpectedDiagnostics.AddRange(expected); - test.SolutionTransforms.Add((solution, projectId) => solution.WithProjectAssemblyName(projectId, "Datadog.Trace")); + test.SolutionTransforms.Add((solution, projectId) => solution.WithProjectAssemblyName(projectId, assemblyName)); await test.RunAsync(); } } diff --git a/tracer/test/Datadog.Trace.Tools.Analyzers.Tests/ConfigurationAnalyzers/ConfigurationBuilderWithKeysAnalyzerTests.cs b/tracer/test/Datadog.Trace.Tools.Analyzers.Tests/ConfigurationAnalyzers/ConfigurationBuilderWithKeysAnalyzerTests.cs index 510c519cf73b..b4de81a539b1 100644 --- a/tracer/test/Datadog.Trace.Tools.Analyzers.Tests/ConfigurationAnalyzers/ConfigurationBuilderWithKeysAnalyzerTests.cs +++ b/tracer/test/Datadog.Trace.Tools.Analyzers.Tests/ConfigurationAnalyzers/ConfigurationBuilderWithKeysAnalyzerTests.cs @@ -3,10 +3,17 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc. // +using System; +using System.Collections.Immutable; +using System.Reflection; +using System.Threading; using System.Threading.Tasks; using Datadog.Trace.Tools.Analyzers.ConfigurationAnalyzers; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing; +using Microsoft.CodeAnalysis.Text; using Xunit; using Verifier = Microsoft.CodeAnalysis.CSharp.Testing.CSharpAnalyzerVerifier< Datadog.Trace.Tools.Analyzers.ConfigurationAnalyzers.ConfigurationBuilderWithKeysAnalyzer, @@ -18,6 +25,367 @@ public class ConfigurationBuilderWithKeysAnalyzerTests { private const string Dd0007 = "DD0007"; // Hardcoded string literal private const string Dd0008 = "DD0008"; // Variable or expression + private const string Dd0015 = "DD0015"; + private const string Dd0016 = "DD0016"; + private const string SensitiveConfigurationTypes = AnalyzerTestHelper.MinimalRequiredTypes + """ + namespace Datadog.Trace.Configuration + { + public static partial class ConfigurationKeys + { + public const string ApiKey = "DD_API_KEY"; + public const string ServiceName = "DD_SERVICE"; + } + } + namespace Datadog.Trace.Configuration.Telemetry + { + public struct ConfigurationBuilder + { + public HasKeys WithKeys(string key) => default; + + public struct HasKeys + { + public string AsString() => null; + public object AsDictionaryResult() => null; + public object AsStringResult(object validator, object converter, bool recordValue) => null; + public string AsRedactedString() => null; + public object AsRedactedStringResult() => null; + } + } + } + """; + + [Fact] + public async Task SensitiveKeyWithAsString_ShouldReportDD0015ButNonSensitiveDoesNot() + { + var code = SensitiveConfigurationTypes + """ + namespace Datadog.Trace.Configuration + { + public class TestClass + { + public void TestMethod() + { + var builder = new Telemetry.ConfigurationBuilder(); + builder.WithKeys({|#0:ConfigurationKeys.ApiKey|}).AsString(); + builder.WithKeys(ConfigurationKeys.ServiceName).AsString(); + } + } + } + """; + + var expected = new DiagnosticResult(Dd0015, DiagnosticSeverity.Error) + .WithLocation(0) + .WithMessage("Sensitive configuration key 'DD_API_KEY' must be read with AsRedactedString, AsRedactedStringResult, AsRedactedDictionaryResult, or AsStringResult with compile-time recordValue: false"); + + await AnalyzerTestHelper.VerifyDatadogAnalyzerAsync(code, expected); + } + + [Fact] + public async Task SensitiveKeyInConsumerAssembly_ShouldReportDD0015() + { + var code = SensitiveConfigurationTypes + """ + namespace Datadog.Trace.Configuration + { + public class TestClass + { + public void TestMethod() + { + var builder = new Telemetry.ConfigurationBuilder(); + builder.WithKeys({|#0:ConfigurationKeys.ApiKey|}).AsString(); + } + } + } + """; + + var expected = new DiagnosticResult(Dd0015, DiagnosticSeverity.Error).WithLocation(0); + + await AnalyzerTestHelper.VerifyAnalyzerInAssemblyWithSupportedConfigurationsAsync(code, "Datadog.Trace.Tools.Runner", expected); + } + + [Fact] + public async Task SensitiveKeyWithAsDictionaryResult_ShouldReportDD0015() + { + var code = SensitiveConfigurationTypes + """ + namespace Datadog.Trace.Configuration + { + public class TestClass + { + public void TestMethod() + { + var builder = new Telemetry.ConfigurationBuilder(); + builder.WithKeys({|#0:ConfigurationKeys.ApiKey|}).AsDictionaryResult(); + } + } + } + """; + + var expected = new DiagnosticResult(Dd0015, DiagnosticSeverity.Error).WithLocation(0); + + await AnalyzerTestHelper.VerifyDatadogAnalyzerAsync(code, expected); + } + + [Fact] + public async Task SensitiveKeyWithRecordingAsStringResult_ShouldReportDD0015() + { + var code = SensitiveConfigurationTypes + """ + namespace Datadog.Trace.Configuration + { + public class TestClass + { + public void TestMethod() + { + var builder = new Telemetry.ConfigurationBuilder(); + builder.WithKeys({|#0:ConfigurationKeys.ApiKey|}).AsStringResult(null, null, recordValue: true); + } + } + } + """; + + var expected = new DiagnosticResult(Dd0015, DiagnosticSeverity.Error).WithLocation(0); + + await AnalyzerTestHelper.VerifyDatadogAnalyzerAsync(code, expected); + } + + [Fact] + public async Task SensitiveKeyWithNonConstantRecordValue_ShouldReportDD0015() + { + var code = SensitiveConfigurationTypes + """ + namespace Datadog.Trace.Configuration + { + public class TestClass + { + public void TestMethod() + { + var builder = new Telemetry.ConfigurationBuilder(); + bool recordValue = false; + builder.WithKeys({|#0:ConfigurationKeys.ApiKey|}).AsStringResult(null, null, recordValue); + } + } + } + """; + + var expected = new DiagnosticResult(Dd0015, DiagnosticSeverity.Error).WithLocation(0); + + await AnalyzerTestHelper.VerifyDatadogAnalyzerAsync(code, expected); + } + + [Fact] + public async Task SensitiveKeyStoredWithoutAccessor_ShouldReportDD0015() + { + var code = SensitiveConfigurationTypes + """ + namespace Datadog.Trace.Configuration + { + public class TestClass + { + public void TestMethod() + { + var builder = new Telemetry.ConfigurationBuilder(); + var sensitive = builder.WithKeys({|#0:ConfigurationKeys.ApiKey|}); + } + } + } + """; + + var expected = new DiagnosticResult(Dd0015, DiagnosticSeverity.Error).WithLocation(0); + + await AnalyzerTestHelper.VerifyDatadogAnalyzerAsync(code, expected); + } + + [Fact] + public async Task SensitiveKeyWithAsRedactedString_ShouldHaveNoDiagnostics() + { + var code = SensitiveConfigurationTypes + """ + namespace Datadog.Trace.Configuration + { + public class TestClass + { + public void TestMethod() + { + var builder = new Telemetry.ConfigurationBuilder(); + builder.WithKeys(ConfigurationKeys.ApiKey).AsRedactedString(); + } + } + } + """; + + await AnalyzerTestHelper.VerifyDatadogAnalyzerAsync(code); + } + + [Fact] + public async Task SensitiveKeyWithAsRedactedStringResult_ShouldHaveNoDiagnostics() + { + var code = SensitiveConfigurationTypes + """ + namespace Datadog.Trace.Configuration + { + public class TestClass + { + public void TestMethod() + { + var builder = new Telemetry.ConfigurationBuilder(); + builder.WithKeys(ConfigurationKeys.ApiKey).AsRedactedStringResult(); + } + } + } + """; + + await AnalyzerTestHelper.VerifyDatadogAnalyzerAsync(code); + } + + [Fact] + public async Task SensitiveKeyWithNonRecordingAsStringResult_ShouldHaveNoDiagnostics() + { + var code = SensitiveConfigurationTypes + """ + namespace Datadog.Trace.Configuration + { + public class TestClass + { + public void TestMethod() + { + var builder = new Telemetry.ConfigurationBuilder(); + builder.WithKeys(ConfigurationKeys.ApiKey).AsStringResult(null, null, recordValue: false); + } + } + } + """; + + await AnalyzerTestHelper.VerifyDatadogAnalyzerAsync(code); + } + + [Fact] + public async Task SensitiveKeyWithParenthesesAndConversionAroundWithKeys_ShouldHaveNoDiagnostics() + { + var code = SensitiveConfigurationTypes + """ + namespace Datadog.Trace.Configuration + { + public class TestClass + { + public void TestMethod() + { + var builder = new Telemetry.ConfigurationBuilder(); + (builder.WithKeys(ConfigurationKeys.ApiKey)).AsRedactedString(); + ((Telemetry.ConfigurationBuilder.HasKeys)builder.WithKeys(ConfigurationKeys.ApiKey)).AsRedactedString(); + } + } + } + """; + + await AnalyzerTestHelper.VerifyDatadogAnalyzerAsync(code); + } + + [Fact] + public async Task SensitiveKeyWithSameNamedExtensionAccessor_ShouldReportDD0015() + { + var code = SensitiveConfigurationTypes + """ + namespace Datadog.Trace.Configuration + { + public static class MaliciousExtensions + { + public static string AsRedactedString(this Telemetry.ConfigurationBuilder.HasKeys keys, string ignored) => null; + } + + public class TestClass + { + public void TestMethod() + { + var builder = new Telemetry.ConfigurationBuilder(); + builder.WithKeys({|#0:ConfigurationKeys.ApiKey|}).AsRedactedString("record-value"); + } + } + } + """; + + var expected = new DiagnosticResult(Dd0015, DiagnosticSeverity.Error).WithLocation(0); + + await AnalyzerTestHelper.VerifyDatadogAnalyzerAsync(code, expected); + } + + [Fact] + public async Task SensitiveKeyWithMalformedSupportedConfigurations_ShouldReportDD0016() + { + var code = SensitiveConfigurationTypes + """ + namespace Datadog.Trace.Configuration + { + public class TestClass + { + public void TestMethod() + { + var builder = new Telemetry.ConfigurationBuilder(); + builder.WithKeys(ConfigurationKeys.ApiKey).AsString(); + } + } + } + """; + const string malformedYaml = """ + supportedConfigurations: + DD_API_KEY: + malformed property + """; + + var expected = new DiagnosticResult(Dd0016, DiagnosticSeverity.Error).WithNoLocation(); + + await AnalyzerTestHelper.VerifyDatadogAnalyzerWithSupportedConfigurationsAsync(code, malformedYaml, expected); + } + + [Theory] + [InlineData("version: '2'")] + [InlineData("supportedConfigurations:")] + public async Task EmptySupportedConfigurations_ShouldReportDD0016(string yaml) + { + var expected = new DiagnosticResult(Dd0016, DiagnosticSeverity.Error).WithNoLocation(); + + await AnalyzerTestHelper.VerifyDatadogAnalyzerWithSupportedConfigurationsAsync(SensitiveConfigurationTypes, yaml, expected); + } + + [Fact] + public async Task SensitiveKeyWithMissingSupportedConfigurations_ShouldReportDD0016() + { + var code = SensitiveConfigurationTypes + """ + namespace Datadog.Trace.Configuration + { + public class TestClass + { + public void TestMethod() + { + var builder = new Telemetry.ConfigurationBuilder(); + builder.WithKeys(ConfigurationKeys.ApiKey).AsString(); + } + } + } + """; + + var expected = new DiagnosticResult(Dd0016, DiagnosticSeverity.Error).WithNoLocation(); + + await AnalyzerTestHelper.VerifyDatadogAnalyzerWithoutSupportedConfigurationsAsync(code, expected); + } + + [Fact] + public void CanceledSupportedConfigurationsRead_ShouldPropagateCancellation() + { + var options = new AnalyzerOptions(ImmutableArray.Create(new CanceledAdditionalText())); + var method = typeof(ConfigurationBuilderWithKeysAnalyzer).GetMethod("TryGetSensitiveKeys", BindingFlags.NonPublic | BindingFlags.Static); + Assert.NotNull(method); + + var exception = Assert.Throws(() => method.Invoke(null, [options, CancellationToken.None, null])); + Assert.IsType(exception.InnerException); + } + + [Fact] + public async Task AmbiguousSupportedConfigurations_ShouldReportDD0016() + { + var diagnostics = await CreateAnalyzer( + new LiteralAdditionalText("/first/supported-configurations.yaml", AnalyzerTestHelper.SupportedConfigurationsYaml), + new LiteralAdditionalText("/second/supported-configurations.yaml", AnalyzerTestHelper.SupportedConfigurationsYaml)) + .GetAnalyzerDiagnosticsAsync(); + + Assert.Contains(diagnostics, x => x.Id == Dd0016); + } + + [Fact] + public async Task UnreadableSupportedConfigurations_ShouldReportDD0016() + { + var diagnostics = await CreateAnalyzer(new UnreadableAdditionalText()).GetAnalyzerDiagnosticsAsync(); + + Assert.Contains(diagnostics, x => x.Id == Dd0016); + } [Fact] public async Task ValidWithKeysUsingConfigurationKeys_ShouldHaveNoDiagnostics() @@ -70,6 +438,7 @@ public void TestMethod() } }; + test.TestState.AdditionalFiles.Add(("supported-configurations.yaml", AnalyzerTestHelper.SupportedConfigurationsYaml)); test.SolutionTransforms.Add((solution, projectId) => solution.WithProjectAssemblyName(projectId, "Datadog.Trace")); await test.RunAsync(); @@ -126,6 +495,7 @@ public void TestMethod() } }; + test.TestState.AdditionalFiles.Add(("supported-configurations.yaml", AnalyzerTestHelper.SupportedConfigurationsYaml)); test.SolutionTransforms.Add((solution, projectId) => solution.WithProjectAssemblyName(projectId, "Datadog.Trace")); await test.RunAsync(); @@ -183,6 +553,7 @@ public void TestMethod() } }; + test.TestState.AdditionalFiles.Add(("supported-configurations.yaml", AnalyzerTestHelper.SupportedConfigurationsYaml)); test.SolutionTransforms.Add((solution, projectId) => solution.WithProjectAssemblyName(projectId, "Datadog.Trace")); await test.RunAsync(); @@ -355,6 +726,7 @@ public void TestMethod() } }; + test.TestState.AdditionalFiles.Add(("supported-configurations.yaml", AnalyzerTestHelper.SupportedConfigurationsYaml)); test.SolutionTransforms.Add((solution, projectId) => solution.WithProjectAssemblyName(projectId, "Datadog.Trace")); await test.RunAsync(); @@ -422,6 +794,7 @@ public void TestMethod() } }; + test.TestState.AdditionalFiles.Add(("supported-configurations.yaml", AnalyzerTestHelper.SupportedConfigurationsYaml)); test.SolutionTransforms.Add((solution, projectId) => solution.WithProjectAssemblyName(projectId, "Datadog.Trace")); await test.RunAsync(); @@ -486,4 +859,40 @@ public struct HasKeys await AnalyzerTestHelper.VerifyDatadogAnalyzerAsync(code, expected); } + + private static CompilationWithAnalyzers CreateAnalyzer(params AdditionalText[] additionalFiles) + { + var compilation = CSharpCompilation.Create( + "Datadog.Trace", + [CSharpSyntaxTree.ParseText(SensitiveConfigurationTypes)], + [MetadataReference.CreateFromFile(typeof(object).Assembly.Location)], + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + var options = new AnalyzerOptions(additionalFiles.ToImmutableArray()); + var analyzers = ImmutableArray.Create(new ConfigurationBuilderWithKeysAnalyzer()); + return compilation.WithAnalyzers(analyzers, options); + } + + private sealed class CanceledAdditionalText : AdditionalText + { + public override string Path => "supported-configurations.yaml"; + + public override SourceText GetText(CancellationToken cancellationToken = default) + => throw new OperationCanceledException(cancellationToken); + } + + private sealed class UnreadableAdditionalText : AdditionalText + { + public override string Path => "supported-configurations.yaml"; + + public override SourceText GetText(CancellationToken cancellationToken = default) => null; + } + + private sealed class LiteralAdditionalText(string path, string text) : AdditionalText + { + private readonly SourceText _text = SourceText.From(text); + + public override string Path { get; } = path; + + public override SourceText GetText(CancellationToken cancellationToken = default) => _text; + } }