diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 79211b1..2dd8287 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -18,6 +18,7 @@ import Services from "./components/Services"; import Alerts from "./components/Alerts"; import { listAlerts, listServices } from "./api"; import { useControls, rangeParams, RANGES, INTERVALS } from "./timerange"; +import { traceHref } from "./links"; const TABS: { to: string; label: string }[] = [ { to: "/traces", label: "Traces" }, @@ -57,7 +58,7 @@ function TracesRoute() { const { service } = useControls(); // Keep the focus in the URL through the drill-in so the header + tabs still show it. // A real per row (primary cell) makes rows keyboard-operable + open-in-new-tab. - return `/traces/${id}${serviceSearch(service)}`} />; + return traceHref(id, { service })} />; } function TraceRoute() { diff --git a/ui/src/components/LogView.tsx b/ui/src/components/LogView.tsx index 5cfe746..446d58e 100644 --- a/ui/src/components/LogView.tsx +++ b/ui/src/components/LogView.tsx @@ -2,6 +2,7 @@ import { Fragment, useEffect, useState } from "react"; import { Link, useSearchParams } from "react-router-dom"; import { listLogs, type LogRow } from "../api"; import { firstRunHint } from "../empty"; +import { traceHref } from "../links"; import { useControls, rangeParams } from "../timerange"; function sevClass(n: number | null): string { @@ -70,12 +71,9 @@ export default function LogView() { }); // Preserve the service focus when drilling into a trace/span from a log row. - const svc = service ? `service=${encodeURIComponent(service)}` : ""; - const traceLink = (l: LogRow) => `/traces/${l.trace_id}${svc ? `?${svc}` : ""}`; - const spanInTraceLink = (l: LogRow) => { - const parts = [l.span_id ? `span=${l.span_id}` : "", svc].filter(Boolean); - return `/traces/${l.trace_id}${parts.length ? `?${parts.join("&")}` : ""}`; - }; + const traceLink = (l: LogRow) => traceHref(l.trace_id ?? "", { service: service || undefined }); + const spanInTraceLink = (l: LogRow) => + traceHref(l.trace_id ?? "", { spanId: l.span_id, service: service || undefined }); const emptyMessage = spanFilter ? "No logs recorded for this span." @@ -172,7 +170,7 @@ export default function LogView() { {l.service ?? "—"} {l.trace_id ? ( - + {l.trace_id.slice(0, 8)} ) : ( diff --git a/ui/src/components/MetricChart.tsx b/ui/src/components/MetricChart.tsx index 3c44886..685f513 100644 --- a/ui/src/components/MetricChart.tsx +++ b/ui/src/components/MetricChart.tsx @@ -10,6 +10,7 @@ import { } from "../api"; import { formatValue } from "../format"; import { facetLabels } from "../metricLabels"; +import { traceHref } from "../links"; const RANGES: { label: string; hours: number }[] = [ { label: "1h", hours: 1 }, @@ -358,9 +359,7 @@ function FacetView({ firingFrom={firingFrom} desc={`${name} ${data.rated ? "per-second rate" : "value"}, ${rangeLabel}`} exemplars={exemplars.map((e) => ({ t: e.t, v: e.v, traceId: e.trace_id }))} - onExemplarClick={(traceId) => - navigate(`/traces/${traceId}${service ? `?service=${encodeURIComponent(service)}` : ""}`) - } + onExemplarClick={(traceId) => navigate(traceHref(traceId, { service }))} /> ); diff --git a/ui/src/components/TraceWaterfall.tsx b/ui/src/components/TraceWaterfall.tsx index a566787..ec41828 100644 --- a/ui/src/components/TraceWaterfall.tsx +++ b/ui/src/components/TraceWaterfall.tsx @@ -2,6 +2,7 @@ import { useEffect, useRef, useState } from "react"; import { Link, useSearchParams } from "react-router-dom"; import { getTrace, type SpanRow } from "../api"; import { rowKeyActivate } from "../a11y"; +import { logsHref } from "../links"; import { fmtDuration } from "./TraceList"; export interface Row { @@ -145,10 +146,7 @@ function SpanDetail({ span, onClose }: { span: SpanRow; onClose: () => void }) { {span.name} {/* Cross-signal drill: this span's logs (trace_id + span_id), or the whole service's logs (which also sets the global service focus). */} - + logs for this span ↗ {span.service && ( @@ -379,7 +377,7 @@ export default function TraceWaterfall({

{spans[0].service ?? "trace"} · {traceId.slice(0, 16)}… ·{" "} {fmtDuration(totalMs)} ·{" "} - + logs ↗ {errorSpans.length > 0 && ( diff --git a/ui/src/links.test.ts b/ui/src/links.test.ts new file mode 100644 index 0000000..942d474 --- /dev/null +++ b/ui/src/links.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, it } from "vitest"; +import { logsHref, traceHref } from "./links"; + +// JEF-534: trace_id/span_id are ingested OTLP data (attacker-shapeable) and were +// interpolated raw into these deep-links, unlike the neighboring `service` focus +// which was already `encodeURIComponent`-ed. These tests pin the well-formed-id +// shape (no behavior change vs. the pre-fix raw interpolation) and lock the +// encoding for ids with special characters. +describe("traceHref", () => { + it("links to just the trace for a well-formed hex id (no behavior change)", () => { + expect(traceHref("abcdef0123456789")).toBe("/traces/abcdef0123456789"); + }); + + it("carries the service focus", () => { + expect(traceHref("abcdef0123456789", { service: "api" })).toBe( + "/traces/abcdef0123456789?service=api", + ); + }); + + it("carries span + service, span first (matches the pre-fix param order)", () => { + expect(traceHref("abcdef0123456789", { spanId: "0123456789abcdef", service: "api" })).toBe( + "/traces/abcdef0123456789?span=0123456789abcdef&service=api", + ); + }); + + it("encodes a trace id with special characters into the path segment", () => { + expect(traceHref("dead&beef#1")).toBe("/traces/dead%26beef%231"); + }); + + it("encodes a span id with special characters into the query string", () => { + expect(traceHref("t1", { spanId: "span&1" })).toBe("/traces/t1?span=span%261"); + }); +}); + +describe("logsHref", () => { + it("carries just the trace id when no span is given", () => { + expect(logsHref("abcdef0123456789")).toBe("/logs?trace_id=abcdef0123456789"); + }); + + it("carries trace_id and span_id for a well-formed hex id (no behavior change)", () => { + expect(logsHref("abcdef0123456789", "0123456789abcdef")).toBe( + "/logs?trace_id=abcdef0123456789&span_id=0123456789abcdef", + ); + }); + + it("encodes special characters instead of injecting an extra query param", () => { + expect(logsHref("dead&beef", "span#1")).toBe( + "/logs?trace_id=dead%26beef&span_id=span%231", + ); + }); +}); diff --git a/ui/src/links.ts b/ui/src/links.ts new file mode 100644 index 0000000..773bad3 --- /dev/null +++ b/ui/src/links.ts @@ -0,0 +1,26 @@ +// Deep-link builders shared by every cross-signal drill-in (trace ↔ logs ↔ +// metrics). Trace/span ids come from ingested OTLP data — attacker-shapeable — +// so every id here is either `encodeURIComponent`-ed as a path segment or set +// via URLSearchParams: a stray `&`/`#`/`?` must stay inside its own param, +// never land on an unexpected route or inject an extra one (JEF-534). + +// `/traces/:traceId`, optionally scoped to a span and/or the global service +// focus. Used by the trace list row link, a log row's drill-in, and a metric +// chart exemplar click. +export function traceHref( + traceId: string, + opts: { spanId?: string | null; service?: string | null } = {}, +): string { + const p = new URLSearchParams(); + if (opts.spanId) p.set("span", opts.spanId); + if (opts.service) p.set("service", opts.service); + const q = p.toString(); + return `/traces/${encodeURIComponent(traceId)}${q ? `?${q}` : ""}`; +} + +// `/logs`, scoped to a trace and (optionally) a span. +export function logsHref(traceId: string, spanId?: string): string { + const p = new URLSearchParams({ trace_id: traceId }); + if (spanId) p.set("span_id", spanId); + return `/logs?${p.toString()}`; +}