Skip to content

fix(logs): build logs alert links when a filter holds a regex - #127939

Merged
trunk-io[bot] merged 1 commit into
masterfrom
jimmy/logs-alert-link-encoded-backslash
Nov 3, 2026
Merged

trunk-io[bot] merged 1 commit into
masterfrom
jimmy/logs-alert-link-encoded-backslash

Conversation

@jzhu13

@jzhu13 jzhu13 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Problem

  • A logs alert whose filter holds a regex with a backslash, like ^4\d{2}$, gets a degraded notification from the alerts platform.
  • Slack, email and webhook messages lose the logs wording and the "View logs" link, and fall back to generic platform text.
  • Teams on the legacy webhook body get no webhook at all: building that body raises, so the send fails.
  • The cause: the link's query encodes the backslash as %5C, and absolute_uri refuses any URL that holds %5C.

Changes

  • Affected alerts now get their logs wording, their link, and their legacy webhook body again.
  • Both callers pass only the page path to absolute_uri and append the query after it. A query cannot change the host, so the host guard still covers the part it protects.
  • products/data_catalog and products/tasks already build links this way.

How did you test this code?

Test rationale: extended test_a_logs_link_opens_the_window_and_filters_the_check_queried, for both the message link and the legacy webhook body, with a regex filter. It asserts the filter survives in the link. Without the fix, both cases raise PotentialSecurityProblemException.

Not checked: a real delivery to Slack or a webhook.

Release status

  • No feature flag controls this change

Docs update

  • None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Claude Opus 5.5

Skills: /writing-tests, /writing-code-comments, /simplify, /code-review, /writing-pr-descriptions, CodeRabbit CLI (no findings)

More context for AI agents
  • Session: found from production delivery logs during an alerts platform status check; Claude Code with Metabase and Grafana.
  • Decisions: kept the fix at the call site. Narrowing the %5C check in has_ambiguous_authority to the authority is the deeper fix, but it guards OAuth and redirect URLs and needs a security review.
  • Follow-ups: the authority-only %5C check in posthog/security/url_validation.py; other absolute_uri callers with user data in the query, such as the AI observability eval report links.
  • Read closely: products/logs/backend/platform_alert_wording.py.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jzhu13 jzhu13 self-assigned this Nov 3, 2026
@trunk-io

trunk-io Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

😎 Merged directly without going through the merge queue, as the queue was empty and the PR was up to date with the target branch - details.

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

@jzhu13
jzhu13 marked this pull request as ready for review November 3, 2026 03:51
@parameterai

parameterai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Risk: No findings

Fixes logs alert notification links and legacy webhook bodies for regex filters by passing only the page path to absolute_uri and appending the urlencoded query afterwards, so the %5C of an encoded backslash no longer trips the ambiguous-authority guard. The change is safe: the validated host is fixed before the query is appended, and the query itself is fully percent-encoded via urlencode, so no attacker-influenced data can alter the link's authority.

Sentinel reviewed e60eb63 · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team November 3, 2026 03:51
@jzhu13 jzhu13 added the stamphog Request AI approval (no full review) label Nov 3, 2026

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

The security-sensitive change has current-head independent review, owning-team authorship, and strong familiarity. Source inspection supports the stated safety rationale, and there are no outstanding substantive concerns.

  • Author wrote 100% of the modified lines and has 25 merged PRs in these paths (familiarity STRONG).
  • PostHog Review (Standard) reviewed the current head: no findings.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 10L, 1F substantive, 17L/2F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1a-trivial (17L, 2F, single-area, fix)
stamphog 2.7.0 .stamphog/policy.yml @ e60eb63 · reviewed head e60eb63

@jzhu13

jzhu13 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

/trunk merge

@trunk-io
trunk-io Bot merged commit 5063109 into master Nov 3, 2026
366 checks passed
@trunk-io
trunk-io Bot deleted the jimmy/logs-alert-link-encoded-backslash branch November 3, 2026 04:13
@deployment-status-posthog

deployment-status-posthog Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-11-03 04:39 UTC Run
prod-us ✅ Deployed 2026-11-03 04:53 UTC Run
prod-eu ✅ Deployed 2026-11-03 05:00 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant