Skip to content

Commit f76e8cb

Browse files
committed
Harden PerfTrace privacy fence, snapshot immutability, open-span cap
Accept parentId only as opaque span ids (known open/ring or OPAQUE_ID_RE); return shallow snapshot copies; cap open spans with oldest-first eviction; mark() takes StartOptions; cover double-end, clear, and overflow paths.
1 parent 73158b2 commit f76e8cb

3 files changed

Lines changed: 212 additions & 14 deletions

File tree

src/perf/index.test.ts

Lines changed: 134 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { afterEach, describe, expect, test } from "bun:test";
22
import {
3+
OPEN_SPAN_CAPACITY,
34
RING_CAPACITY,
45
clear,
56
end,
@@ -43,7 +44,7 @@ describe("start / end / mark", () => {
4344
});
4445

4546
test("mark is a completed point-in-time span", () => {
46-
const id = mark("adapter.transport", { transport: "http_sse" });
47+
const id = mark("adapter.transport", { tags: { transport: "http_sse" } });
4748
expect(id.length).toBeGreaterThan(0);
4849

4950
const spans = snapshot();
@@ -52,6 +53,17 @@ describe("start / end / mark", () => {
5253
expect(spans[0]!.tags).toEqual({ transport: "http_sse" });
5354
});
5455

56+
test("mark accepts optional parentId like start", () => {
57+
const turnId = start("turn");
58+
mark("adapter.transport", { parentId: turnId, tags: { transport: "ws" } });
59+
end(turnId);
60+
61+
const spans = snapshot();
62+
const marked = spans.find((s) => s.name === "adapter.transport")!;
63+
expect(marked.parentId).toBe(turnId);
64+
expect(marked.tags).toEqual({ transport: "ws" });
65+
});
66+
5567
test("open spans appear in snapshot without endNs", () => {
5668
const id = start("session");
5769
const spans = snapshot();
@@ -72,6 +84,20 @@ describe("start / end / mark", () => {
7284
expect(snapshot()).toHaveLength(0);
7385
});
7486

87+
test("double-end is a no-op", () => {
88+
const id = start("tool");
89+
end(id);
90+
end(id);
91+
expect(snapshot()).toHaveLength(1);
92+
});
93+
94+
test("end after clear is a no-op", () => {
95+
const id = start("tool");
96+
clear();
97+
end(id, { count: 1 });
98+
expect(snapshot()).toHaveLength(0);
99+
});
100+
75101
test("end merges sanitized tags onto the span", () => {
76102
const id = start("tool", { tags: { tool_id: "t1" } });
77103
end(id, { count: 3, prompt: "secret" });
@@ -80,11 +106,93 @@ describe("start / end / mark", () => {
80106
});
81107
});
82108

109+
describe("parentId privacy fence", () => {
110+
test("strips path-like parentId", () => {
111+
const id = start("inference", { parentId: "/Users/me/secret/repo/src/main.ts" });
112+
end(id);
113+
expect(snapshot()[0]!.parentId).toBeUndefined();
114+
});
115+
116+
test("strips free-text and path-separator parentIds", () => {
117+
const a = start("tool", { parentId: "has spaces and free text" });
118+
end(a);
119+
const b = start("tool", { parentId: "../../etc/passwd" });
120+
end(b);
121+
const c = start("tool", { parentId: "C:\\Windows\\System32" });
122+
end(c);
123+
124+
for (const span of snapshot()) {
125+
expect(span.parentId).toBeUndefined();
126+
}
127+
});
128+
129+
test("accepts known open span ids and opaque ids", () => {
130+
const parent = start("turn");
131+
const child = start("inference", { parentId: parent });
132+
end(child);
133+
end(parent);
134+
135+
// Opaque id that matches OPAQUE_ID_RE but is not currently open/ring-known
136+
// after clear of only that id — still accepted when pattern matches.
137+
const orphan = start("tool", { parentId: "parent1" });
138+
end(orphan);
139+
140+
const spans = snapshot();
141+
expect(spans.find((s) => s.name === "inference")!.parentId).toBe(parent);
142+
expect(spans.find((s) => s.name === "tool")!.parentId).toBe("parent1");
143+
});
144+
145+
test("accepts parentId of a completed (ring) span", () => {
146+
const parent = start("turn");
147+
end(parent);
148+
const child = start("inference", { parentId: parent });
149+
end(child);
150+
expect(snapshot().find((s) => s.name === "inference")!.parentId).toBe(parent);
151+
});
152+
});
153+
154+
describe("snapshot immutability", () => {
155+
test("mutating a snapshot does not poison the next snapshot", () => {
156+
const id = start("tool", { tags: { tool_id: "t1", count: 1 } });
157+
end(id);
158+
159+
const first = snapshot();
160+
expect(first).toHaveLength(1);
161+
first[0]!.name = "session";
162+
first[0]!.tags!.tool_id = "POISON";
163+
first[0]!.tags!.count = 999;
164+
first[0]!.parentId = "injected";
165+
first.push({
166+
id: "fake",
167+
name: "session",
168+
startNs: 0n,
169+
endNs: 0n,
170+
});
171+
172+
const second = snapshot();
173+
expect(second).toHaveLength(1);
174+
expect(second[0]!.name).toBe("tool");
175+
expect(second[0]!.tags).toEqual({ tool_id: "t1", count: 1 });
176+
expect(second[0]!.parentId).toBeUndefined();
177+
});
178+
179+
test("mutating an open-span snapshot does not poison open state", () => {
180+
start("session", { tags: { provider_id: "openai" } });
181+
const first = snapshot();
182+
first[0]!.tags!.provider_id = "POISON";
183+
first[0]!.endNs = 1n;
184+
185+
const second = snapshot();
186+
expect(second[0]!.tags).toEqual({ provider_id: "openai" });
187+
expect(second[0]!.endNs).toBeUndefined();
188+
});
189+
});
190+
83191
describe("ring overflow", () => {
84192
test("drops oldest completed spans when capacity is exceeded", () => {
85193
// Fill past capacity with marks (cheap completed spans).
86194
for (let i = 0; i < RING_CAPACITY + 10; i += 1) {
87-
mark("tool", { count: i });
195+
mark("tool", { tags: { count: i } });
88196
}
89197

90198
const spans = snapshot();
@@ -105,6 +213,30 @@ describe("ring overflow", () => {
105213
});
106214
});
107215

216+
describe("open span capacity", () => {
217+
test("evicts oldest open span when over OPEN_SPAN_CAPACITY", () => {
218+
const ids: string[] = [];
219+
for (let i = 0; i < OPEN_SPAN_CAPACITY + 5; i += 1) {
220+
ids.push(start("tool", { tags: { count: i } }));
221+
}
222+
223+
const open = snapshot().filter((s) => s.endNs === undefined);
224+
expect(open).toHaveLength(OPEN_SPAN_CAPACITY);
225+
226+
// Oldest five (count 0..4) were evicted; survivors start at count 5.
227+
const counts = open.map((s) => s.tags?.count).sort((a, b) => (a ?? 0) - (b ?? 0));
228+
expect(counts[0]).toBe(5);
229+
expect(counts[counts.length - 1]).toBe(OPEN_SPAN_CAPACITY + 4);
230+
231+
// Ending an evicted id is a no-op; ending a survivor still works.
232+
end(ids[0]!);
233+
end(ids[ids.length - 1]!);
234+
const completed = snapshot().filter((s) => s.endNs !== undefined);
235+
expect(completed).toHaveLength(1);
236+
expect(completed[0]!.tags?.count).toBe(OPEN_SPAN_CAPACITY + 4);
237+
});
238+
});
239+
108240
describe("sanitizeTags", () => {
109241
test("keeps allowlisted enums, numbers, and opaque ids", () => {
110242
const tags = sanitizeTags({

src/perf/index.ts

Lines changed: 75 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,19 @@
33
*
44
* Fixed-size ring buffer, monotonic high-res clocks, privacy-sanitized tags.
55
* No network, no PostHog, no export side effects.
6+
*
7+
* Memory policy (fixed, not settings):
8+
* - RING_CAPACITY completed spans (oldest completed dropped on overflow)
9+
* - OPEN_SPAN_CAPACITY concurrent open spans (oldest open dropped on overflow)
10+
* - snapshot() returns shallow copies so consumers cannot poison internal state
611
*/
712

8-
import { sanitizeTags, type PerfTags, type TransportKind } from "./sanitize.js";
13+
import { isOpaqueId, sanitizeTags, type PerfTags } from "./sanitize.js";
914

1015
export {
1116
sanitizeTags,
17+
isOpaqueId,
18+
OPAQUE_ID_RE,
1219
ALLOWED_TAG_KEYS,
1320
type AllowedTagKey,
1421
type PerfTags,
@@ -48,9 +55,15 @@ export type StartOptions = {
4855
tags?: Record<string, unknown>;
4956
};
5057

51-
/** Fixed ring capacity — constant, not a settings UI. */
58+
/** Fixed ring capacity for completed spans — constant, not a settings UI. */
5259
export const RING_CAPACITY = 4096;
5360

61+
/**
62+
* Max concurrent open (un-ended) spans. Fixed memory: when exceeded, the
63+
* oldest open span is dropped so a leaked start() cannot grow without bound.
64+
*/
65+
export const OPEN_SPAN_CAPACITY = 1024;
66+
5467
// Module state: one process-wide ring. Tests call clear() between cases.
5568
let nextId = 0;
5669
const openSpans = new Map<string, PerfSpan>();
@@ -80,33 +93,77 @@ function pushRing(span: PerfSpan): void {
8093
}
8194
}
8295

96+
function ringHasId(id: string): boolean {
97+
if (ringCount === 0) return false;
98+
const startIdx = ringCount < RING_CAPACITY ? 0 : ringWrite;
99+
for (let i = 0; i < ringCount; i += 1) {
100+
const span = ring[(startIdx + i) % RING_CAPACITY];
101+
if (span !== undefined && span.id === id) return true;
102+
}
103+
return false;
104+
}
105+
106+
/**
107+
* Privacy fence for parentId: only opaque span ids (known open/ring ids, or
108+
* OPAQUE_ID_RE). Free text, paths, and long strings are stripped.
109+
*/
110+
function sanitizeParentId(parentId: string | undefined): string | undefined {
111+
if (parentId === undefined || parentId.length === 0) return undefined;
112+
if (openSpans.has(parentId) || ringHasId(parentId)) return parentId;
113+
if (isOpaqueId(parentId)) return parentId;
114+
return undefined;
115+
}
116+
117+
/** Shallow copy so snapshot consumers cannot mutate the ring / open map. */
118+
function cloneSpan(span: PerfSpan): PerfSpan {
119+
const copy: PerfSpan = {
120+
id: span.id,
121+
name: span.name,
122+
startNs: span.startNs,
123+
};
124+
if (span.parentId !== undefined) copy.parentId = span.parentId;
125+
if (span.endNs !== undefined) copy.endNs = span.endNs;
126+
if (span.tags !== undefined) copy.tags = { ...span.tags };
127+
return copy;
128+
}
129+
130+
/** Drop the oldest open span when at capacity (Map iteration is insertion order). */
131+
function evictOldestOpenIfFull(): void {
132+
if (openSpans.size < OPEN_SPAN_CAPACITY) return;
133+
const oldest = openSpans.keys().next().value;
134+
if (oldest !== undefined) openSpans.delete(oldest);
135+
}
136+
83137
/**
84138
* Open a timed span. Returns an opaque id for `end`.
85139
* Unknown span names are ignored (returns empty string; end is a no-op).
140+
* When open capacity is full, the oldest open span is dropped first.
86141
*/
87142
export function start(name: SpanName | string, opts?: StartOptions): string {
88143
if (!isSpanName(name)) return "";
89144

90145
const id = allocId();
91146
const tags = sanitizeTags(opts?.tags);
147+
const parentId = sanitizeParentId(opts?.parentId);
92148
const span: PerfSpan = {
93149
id,
94150
name,
95151
startNs: nowNs(),
96152
};
97-
if (opts?.parentId !== undefined && opts.parentId.length > 0) {
98-
span.parentId = opts.parentId;
153+
if (parentId !== undefined) {
154+
span.parentId = parentId;
99155
}
100156
if (tags !== undefined) {
101157
span.tags = tags;
102158
}
159+
evictOldestOpenIfFull();
103160
openSpans.set(id, span);
104161
return id;
105162
}
106163

107164
/**
108165
* Close a span opened by `start`. Merges optional end tags (sanitized).
109-
* Unknown or already-ended ids are ignored.
166+
* Unknown or already-ended ids are ignored (double-end is a no-op).
110167
*/
111168
export function end(id: string, tags?: Record<string, unknown>): void {
112169
if (id.length === 0) return;
@@ -126,20 +183,25 @@ export function end(id: string, tags?: Record<string, unknown>): void {
126183

127184
/**
128185
* Point-in-time event: recorded as a completed span with startNs === endNs.
129-
* Unknown span names are ignored.
186+
* Unknown span names are ignored. Accepts the same options shape as `start`
187+
* (optional parentId + tags).
130188
*/
131-
export function mark(name: SpanName | string, tags?: Record<string, unknown>): string {
189+
export function mark(name: SpanName | string, opts?: StartOptions): string {
132190
if (!isSpanName(name)) return "";
133191

134192
const id = allocId();
135193
const ns = nowNs();
136-
const sanitized = sanitizeTags(tags);
194+
const sanitized = sanitizeTags(opts?.tags);
195+
const parentId = sanitizeParentId(opts?.parentId);
137196
const span: PerfSpan = {
138197
id,
139198
name,
140199
startNs: ns,
141200
endNs: ns,
142201
};
202+
if (parentId !== undefined) {
203+
span.parentId = parentId;
204+
}
143205
if (sanitized !== undefined) {
144206
span.tags = sanitized;
145207
}
@@ -150,20 +212,23 @@ export function mark(name: SpanName | string, tags?: Record<string, unknown>): s
150212
/**
151213
* Snapshot of completed spans in chronological order (oldest first),
152214
* plus any still-open spans (endNs unset) appended after completed ones.
215+
*
216+
* Returns shallow copies of each span (and of tags) so callers cannot
217+
* mutate the ring buffer or open-span map through the returned objects.
153218
*/
154219
export function snapshot(): PerfSpan[] {
155220
const completed: PerfSpan[] = [];
156221
if (ringCount > 0) {
157222
const startIdx = ringCount < RING_CAPACITY ? 0 : ringWrite;
158223
for (let i = 0; i < ringCount; i += 1) {
159224
const span = ring[(startIdx + i) % RING_CAPACITY];
160-
if (span !== undefined) completed.push(span);
225+
if (span !== undefined) completed.push(cloneSpan(span));
161226
}
162227
}
163228

164229
if (openSpans.size === 0) return completed;
165230

166-
const open = [...openSpans.values()];
231+
const open = [...openSpans.values()].map(cloneSpan);
167232
// Stable order by start time so tests and dumps are deterministic.
168233
open.sort((a, b) => (a.startNs < b.startNs ? -1 : a.startNs > b.startNs ? 1 : 0));
169234
return completed.concat(open);

src/perf/sanitize.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,14 @@ const MAX_ID_LENGTH = 64;
7272

7373
// Opaque ids / model ids: alphanumerics, dots, underscores, hyphens, colons, @.
7474
// No spaces, slashes, backslashes, or control characters.
75-
const OPAQUE_ID_RE = /^[A-Za-z0-9._:@-]{1,64}$/;
75+
export const OPAQUE_ID_RE = /^[A-Za-z0-9._:@-]{1,64}$/;
7676

7777
function isFiniteNumber(value: unknown): value is number {
7878
return typeof value === "number" && Number.isFinite(value);
7979
}
8080

81-
function isOpaqueId(value: unknown): value is string {
81+
/** True for short opaque id strings (no paths, whitespace, or free text). */
82+
export function isOpaqueId(value: unknown): value is string {
8283
return typeof value === "string" && value.length <= MAX_ID_LENGTH && OPAQUE_ID_RE.test(value);
8384
}
8485

0 commit comments

Comments
 (0)