Skip to content

fix: redact secret-bearing alert delivery errors - #83

Merged
VibeSWE merged 2 commits into
mainfrom
fix/alert-error-url-redaction
Oct 25, 2026
Merged

VibeSWE merged 2 commits into
mainfrom
fix/alert-error-url-redaction

Conversation

@VibeSWE

@VibeSWE VibeSWE commented Oct 25, 2026 •

Copy link
Copy Markdown

Description

Failed alert notifications could persist credential-bearing target URLs in runtime errors and expose them through alert responses or delivery logs. Remove reqwest URLs before formatting new errors, sanitize historic runtime diagnostics on startup and when constructing responses, and bound diagnostic strings to 1,024 characters. The outward policy-error wrapper retains only sanitized details, including for Actix's Debug response logging.

Preserve PromQL instance and delivery labels verbatim, including labels named error, so restoring an alert retains series identity and correctly supersedes queued notifications after resolution. Continue recursively sanitizing diagnostic containers, including ones containing objects or arrays named error.

Mask Alertmanager usernames alongside passwords, webhook URLs and header values. The target edit form requires both credentials to be re-entered to retain authentication. Storage serialization retains credentials required for delivery; malformed target deserialization errors no longer echo rejected values.

Startup cleanup is in memory only: existing stored alert objects are not rewritten until a normal save. Response shapes and RBAC stay unchanged. Related metrics/datasets/alerts integration should use sanitize_runtime_fields at alert-response boundaries so metric label identity remains intact.

Validation

  • Rust library suite: 1,066 passed, 3 ignored, using P_BUILD_NO_UI=1 CARGO_BUILD_JOBS=4 cargo test --lib -- --test-threads=4.
  • Regression coverage includes firing -> save -> restore -> resolution -> outbox pruning with URL-bearing labels, plus Actix's stored response-error Debug formatting.
  • A fresh independent Sol 6.1 adversarial re-review found no remaining in-scope correctness, memory/resource, or redaction findings.
  • cargo clippy --all-targets passed with existing warnings; none reference the changed files. Rust formatting and git diff --check passed.
  • Frontend: 2,434 unit tests passed, 12 target-focused browser tests passed, TypeScript checks and production build passed.
  • Optimized exact-source microbenchmarks compare clone + serialize with clone + sanitize + serialize, with warmup, alternating order, medians and output verification. The representative runtime fixture added about 1.5 microseconds (4.9%). A long URL-bearing error fixture processed 85% faster and shrank from 827 KB to 6.4 KB. These are runtime-field processing measurements, not whole-server throughput. An ignored benchmark is included for reproduction.

This PR has:

  • been tested with live log ingestion and queries.
  • added comments explaining non-obvious behavior.
  • added separate documentation for new or modified features or behaviors.

@VibeSWE
VibeSWE merged commit 04b73b5 into main Oct 25, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant