Skip to content

fix: Hide potentially-sensitive alert errors#2136

Merged
kodiakhq[bot] merged 1 commit intomainfrom
drew/alert-error-security
Apr 20, 2026
Merged

fix: Hide potentially-sensitive alert errors#2136
kodiakhq[bot] merged 1 commit intomainfrom
drew/alert-error-security

Conversation

@pulpdrew
Copy link
Copy Markdown
Contributor

@pulpdrew pulpdrew commented Apr 20, 2026

Summary

This PR updates the recent alert runner error persistence + display (#2132) to hardcode webhook and unknown-type errors. The raw error messages could contain potentially sensitive information, so we won't persist them or show them in the UI.

Screenshot 2026-04-20 at 7 13 57 AM

How to test locally or on Vercel

This can be tested locally by running an alert with an invalid webhook destination.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 20, 2026

⚠️ No Changeset found

Latest commit: 72d3340

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 20, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
hyperdx-oss Ready Ready Preview, Comment Apr 20, 2026 11:14am

Request Review

@github-actions github-actions Bot added the review/tier-4 Critical — deep review + domain expert sign-off label Apr 20, 2026
@github-actions
Copy link
Copy Markdown
Contributor

🔴 Tier 4 — Critical

Touches auth, data models, config, tasks, OTel pipeline, ClickHouse, or CI/CD.

Why this tier:

  • Critical-path files (1):
    • packages/api/src/tasks/checkAlerts/index.ts

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats
  • Production files changed: 1
  • Production lines changed: 14 (+ 86 in test files, excluded from tier calculation)
  • Branch: drew/alert-error-security
  • Author: pulpdrew

To override this classification, remove the review/tier-4 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions
Copy link
Copy Markdown
Contributor

PR Review

✅ No critical issues found.

  • ⚠️ .slice(0, 10000) is applied to hardcoded strings in makeAlertError — harmless but redundant since the constants are under 200 chars. No fix required, minor cleanup opportunity.
  • ℹ️ QUERY_ERROR and INVALID_ALERT still surface raw messages (ClickHouse errors, internal validation). The inline comment explains this is intentional ("authored by us"), but worth a second look if ClickHouse errors could ever include user-supplied query content that embeds sensitive data.

Overall: clean approach, well-tested, tests explicitly assert the raw error string does not leak (!e.message.includes('group webhook failed')). Good addition of the UNKNOWN error test case.

@github-actions
Copy link
Copy Markdown
Contributor

E2E Test Results

All tests passed • 147 passed • 3 skipped • 1073s

Status Count
✅ Passed 147
❌ Failed 0
⚠️ Flaky 3
⏭️ Skipped 3

Tests ran across 4 shards in parallel.

View full report →

@pulpdrew pulpdrew added review/tier-2 Low risk — AI review + quick human skim and removed review/tier-4 Critical — deep review + domain expert sign-off labels Apr 20, 2026
@pulpdrew pulpdrew requested review from a team and wrn14897 and removed request for a team April 20, 2026 11:23
@kodiakhq kodiakhq Bot merged commit 00222da into main Apr 20, 2026
19 checks passed
@kodiakhq kodiakhq Bot deleted the drew/alert-error-security branch April 20, 2026 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge review/tier-2 Low risk — AI review + quick human skim

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants