Add tenant-opt-in secret detection - #7885
carles-grafana wants to merge 1 commit into
Conversation
ea5f818 to
c227b75
Compare
c227b75 to
6182fa1
Compare
Scan trace values using built-in and custom rules before metrics filtering, including when metric generation is skipped. Require the process-wide flag and tenant processor selection, use existing overrides for policies, and keep secret values out of finding logs.
6182fa1 to
e87e86a
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e87e86a. Configure here.
| cfg *Config | ||
|
|
||
| validForwarders map[string]struct{} | ||
| policyCompiler func() (*secrets.PolicyCompiler, error) |
There was a problem hiding this comment.
Default policies skip startup validation
Medium Severity
Startup validates secrets.enabled_rules and user-configurable secret_detection policies, but runtimeConfigValidator never compiles runtime or default secret_detection policies. An invalid default custom regex or exclusion still starts Tempo, then fail-opens to the native baseline, so intended disables and custom rules never apply.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e87e86a. Configure here.
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical findings affect runtime policy validation and policy exposure, with an additional override recovery issue.
Get a fresh assessment by requesting another Copilot review.
Warning
- Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.
Pull request overview
Adds tenant-opt-in secret detection to the metrics-generator, including built-in and tenant-defined rules, policy handling, operational support, and tests.
Changes:
- Adds trace scanning, rule compilation, matching, and catalog support.
- Integrates tenant policies with metrics-generator processing and overrides.
- Adds validation, diagnostics redaction, documentation, alerts, and tests.
File summaries
| File | Reviewed change |
|---|---|
pkg/secrets/trace_walk.go |
Traverses trace values for detection. |
pkg/secrets/trace_walk_test.go |
Tests trace traversal. |
pkg/secrets/testdata/catalog/priority_services.json |
Adds priority catalog fixtures. |
pkg/secrets/testdata/catalog/priority_cloud.json |
Adds priority catalog fixtures. |
pkg/secrets/testdata/catalog/postgres.json |
Adds catalog fixtures. |
pkg/secrets/testdata/catalog/generic.json |
Adds generic catalog fixtures. |
pkg/secrets/single_keyword_guidance_test.go |
Tests keyword guidance. |
pkg/secrets/rule_analysis_test.go |
Tests rule analysis. |
pkg/secrets/postgres_test.go |
Tests PostgreSQL rules. |
pkg/secrets/policy_provider.go |
Provides dynamic policies. |
pkg/secrets/policy_compiler.go |
Compiles secret policies. |
pkg/secrets/NOTICE.titus |
Adds attribution. |
pkg/secrets/matcher.go |
Implements matching. |
pkg/secrets/LICENSE.titus |
Adds license text. |
pkg/secrets/LICENSE.gitleaks |
Adds license text. |
pkg/secrets/LICENSE.betterleaks |
Adds license text. |
pkg/secrets/lazy_regexp.go |
Adds lazy regex compilation. |
pkg/secrets/keyword_hits.go |
Tracks keyword hits. |
pkg/secrets/evaluation_benchmark_test.go |
Benchmarks evaluation. |
pkg/secrets/curl_literal_word_test.go |
Tests literal matching. |
pkg/secrets/catalog.go |
Defines the secret catalog. |
pkg/secrets/catalog_test.go |
Tests catalog behavior. |
pkg/secrets/catalog_generic.go |
Adds generic rules. |
pkg/secrets/catalog_evidence_betterleaks_1.go |
Adds catalog evidence. |
pkg/secrets/catalog_assignment.go |
Adds assignment rules. |
operations/tempo-mixin/runbook.md |
Documents operational response. |
operations/tempo-mixin/alerts.libsonnet |
Adds policy alerts. |
operations/tempo-mixin-compiled/alerts.yaml |
Updates compiled alerts. |
modules/overrides/userconfigurable/client/limits.go |
Adds policy override limits. |
modules/overrides/userconfigurable/client/limits_test.go |
Tests override limits. |
modules/overrides/userconfigurable/client/client.go |
Validates persisted policies. |
modules/overrides/userconfigurable/client/client_test.go |
Tests policy decoding. |
modules/overrides/userconfigurable/api/limits.go |
Exposes policy limits. |
modules/overrides/userconfigurable/api/limits_test.go |
Tests API limits. |
modules/overrides/userconfigurable/api/api.go |
Validates override requests. |
modules/overrides/user_configurable_overrides.go |
Integrates user overrides. |
modules/overrides/runtime_config_overrides.go |
Loads runtime policies. |
modules/overrides/runtime_config_overrides_test.go |
Tests runtime overrides. |
modules/overrides/overrides_tenant_status_http.go |
Redacts tenant status. |
modules/overrides/interface.go |
Extends override interfaces. |
modules/overrides/config.go |
Adds override configuration. |
modules/overrides/config_test.go |
Tests override configuration. |
modules/overrides/config_legacy.go |
Supports legacy overrides. |
modules/generator/validation/fields.go |
Validates processor fields. |
modules/generator/validation/fields_test.go |
Tests field validation. |
modules/generator/processor/secretdetection/tenant_metrics.go |
Defines tenant metrics. |
modules/generator/processor/secretdetection/processor_benchmark_test.go |
Benchmarks detection. |
modules/generator/processor/secretdetection/config.go |
Defines processor configuration. |
modules/generator/processor/processor_names.go |
Registers processor names. |
modules/generator/overrides.go |
Resolves generator policies. |
modules/generator/overrides_test.go |
Tests generator overrides. |
modules/generator/instance.go |
Integrates detection lifecycle. |
modules/generator/instance_test.go |
Tests generator integration. |
modules/generator/generator.go |
Initializes generator support. |
modules/generator/generator_kafka.go |
Integrates Kafka behavior. |
modules/generator/config.go |
Adds generator configuration. |
modules/generator/config_test.go |
Tests generator configuration. |
modules/distributor/distributor_test.go |
Tests distributor integration. |
docs/sources/tempo/configuration/_index.md |
Documents secret detection configuration. |
cmd/tempo/main.go |
Wires application configuration. |
cmd/tempo/main_test.go |
Tests application startup. |
cmd/tempo/app/overrides_validation.go |
Validates secret policies. |
cmd/tempo/app/overrides_validation_test.go |
Tests policy validation. |
cmd/tempo/app/modules.go |
Wires application modules. |
cmd/tempo/app/modules_test.go |
Tests module wiring. |
cmd/tempo/app/config.go |
Adds application configuration. |
cmd/tempo/app/app.go |
Initializes secret detection. |
cmd/tempo/app/app_test.go |
Tests application behavior. |
.chloggen/native-secret-catalog.yaml |
Adds the changelog entry. |
Review details
Files excluded by content exclusion policy (26)
- vendor/golang.org/x/crypto/curve25519/curve25519.go
- vendor/golang.org/x/crypto/ssh/buffer.go
- vendor/golang.org/x/crypto/ssh/certs.go
- vendor/golang.org/x/crypto/ssh/channel.go
- vendor/golang.org/x/crypto/ssh/cipher.go
- vendor/golang.org/x/crypto/ssh/client.go
- vendor/golang.org/x/crypto/ssh/client_auth.go
- vendor/golang.org/x/crypto/ssh/common.go
- vendor/golang.org/x/crypto/ssh/connection.go
- vendor/golang.org/x/crypto/ssh/control.go
- vendor/golang.org/x/crypto/ssh/doc.go
- vendor/golang.org/x/crypto/ssh/handshake.go
- vendor/golang.org/x/crypto/ssh/internal/bcrypt_pbkdf/bcrypt_pbkdf.go
- vendor/golang.org/x/crypto/ssh/kex.go
- vendor/golang.org/x/crypto/ssh/keys.go
- vendor/golang.org/x/crypto/ssh/mac.go
- vendor/golang.org/x/crypto/ssh/messages.go
- vendor/golang.org/x/crypto/ssh/mlkem.go
- vendor/golang.org/x/crypto/ssh/mux.go
- vendor/golang.org/x/crypto/ssh/server.go
- vendor/golang.org/x/crypto/ssh/session.go
- vendor/golang.org/x/crypto/ssh/ssh_gss.go
- vendor/golang.org/x/crypto/ssh/streamlocal.go
- vendor/golang.org/x/crypto/ssh/tcpip.go
- vendor/golang.org/x/crypto/ssh/transport.go
- vendor/modules.txt
Suppressed comments (1)
modules/overrides/userconfigurable/client/client.go:118
- HIGH: Once a persisted policy has an unsupported field or malformed policy JSON,
GetreturnsErrInvalidSecretsPolicy; the PATCH path returns this error before it can apply a replacement, while GET cannot provide the ETag needed by DELETE. A forward-incompatible or manually corrupted tenant document can therefore be stranded with no recovery through the API. Could the API provide a safe replace/delete recovery path that does not first decode the old policy?
- Files reviewed: 76/175 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if _, err := compiler.CompilePolicy(*policy); err != nil { | ||
| return fmt.Errorf("invalid metrics_generator.processor.secret_detection: %w", err) |
| HostIdentifiers: strArrPtr(overrides.MetricsGeneratorProcessorHostInfoHostIdentifiers(userID)), | ||
| MetricName: strPtr(overrides.MetricsGeneratorProcessorHostInfoMetricName(userID)), | ||
| }, | ||
| SecretDetection: secretsPolicyPtr(overrides, userID), |


What this PR does
Adds secret detection to the metrics-generator using built-in rules and tenant-defined rules. Tenants opt in through
metrics_generator.processors: [secret-detection], with the process-widesecrets.detection_enabledflag enabled. Detection scans trace values before preprocessing and timestamp filtering, including when metric generation is skipped.Tenant policies use the existing overrides system. They can exclude selected built-in rules and define custom rules with
idandregex. Finding logs omit matched values and attribute names, and diagnostic output redacts private policy text. Detection keeps the existing Kafka automatic-commit behavior and is best-effort across crashes.Scope
The private redaction HTTP API is now in #7886. The two PRs target
mainindependently. This PR excludes the comparison tools, design/selection dossiers, unused metadata, and unrelated forwarder and Kafka commit-tracking changes removed during cleanup.Validation
go test -timeout=5m ./cmd/tempo ./cmd/tempo/app ./modules/generator ./modules/generator/processor/secretdetection ./modules/overrides/... ./pkg/apimake chlog-validatemake generate-manifestChecklist