Skip to content

Commit 0633c23

Browse files
committed
fix: add trace ID validation to trace view + UUID dash-stripping
Fix two related issues (CLI-7Z, CLI-4X) in `sentry trace view`: 1. Add missing `validateTraceId()` call in `parsePositionalArgs()` — the raw, unvalidated trace ID was going directly to the API, causing confusing "No trace found" errors for malformed input. Now matches the pattern used by `trace logs` and `log view`. 2. Add UUID dash-stripping in `validateHexId()` — when input matches UUID format (8-4-4-4-12 hex with dashes), dashes are automatically stripped and a warning is printed to stderr. This handles the common case of users copy-pasting trace IDs in UUID format (e.g., `ed29abc8-71c4-475b-9675-4655ef1a02d0`). Changes: - src/lib/hex-id.ts: Add UUID_DASH_RE regex and dash-stripping before validation in validateHexId() - src/commands/trace/view.ts: Add validateTraceId() at all 4 return paths in parsePositionalArgs(), with validateAndWarn() helper for UUID auto-correction warnings - Updated tests: hex-id (unit + property), trace-id (unit + property), trace view (unit, property, func)
1 parent ecda37f commit 0633c23

File tree

7 files changed

+387
-75
lines changed

7 files changed

+387
-75
lines changed

src/commands/trace/view.ts

Lines changed: 61 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {
3434
resolveProjectBySlug,
3535
} from "../../lib/resolve-target.js";
3636
import { buildTraceUrl } from "../../lib/sentry-urls.js";
37+
import { validateTraceId } from "../../lib/trace-id.js";
3738
import type { Writer } from "../../types/index.js";
3839

3940
type ViewFlags = {
@@ -46,18 +47,51 @@ type ViewFlags = {
4647
/** Usage hint for ContextError messages */
4748
const USAGE_HINT = "sentry trace view <org>/<project> <trace-id>";
4849

50+
/**
51+
* Validate a trace ID and detect UUID auto-correction.
52+
*
53+
* Returns the validated trace ID and an optional warning when dashes were
54+
* stripped from a UUID-format input (e.g., `ed29abc8-71c4-475b-...`).
55+
*/
56+
function validateAndWarn(raw: string): {
57+
traceId: string;
58+
uuidWarning?: string;
59+
} {
60+
const traceId = validateTraceId(raw);
61+
const trimmedRaw = raw.trim().toLowerCase();
62+
const uuidWarning =
63+
trimmedRaw.includes("-") && trimmedRaw !== traceId
64+
? `Auto-corrected trace ID: stripped dashes → ${traceId}`
65+
: undefined;
66+
return { traceId, uuidWarning };
67+
}
68+
69+
/**
70+
* Merge multiple optional warning strings into a single warning, or undefined.
71+
*/
72+
function mergeWarnings(
73+
...warnings: (string | undefined)[]
74+
): string | undefined {
75+
const filtered = warnings.filter(Boolean);
76+
return filtered.length > 0 ? filtered.join("\n") : undefined;
77+
}
78+
4979
/**
5080
* Parse positional arguments for trace view.
5181
* Handles: `<trace-id>` or `<target> <trace-id>`
5282
*
83+
* Validates the trace ID format (32-character hex) and auto-corrects
84+
* UUID-format inputs by stripping dashes.
85+
*
5386
* @param args - Positional arguments from CLI
5487
* @returns Parsed trace ID and optional target arg
5588
* @throws {ContextError} If no arguments provided
89+
* @throws {ValidationError} If the trace ID format is invalid
5690
*/
5791
export function parsePositionalArgs(args: string[]): {
5892
traceId: string;
5993
targetArg: string | undefined;
60-
/** Warning message if arguments appear to be in the wrong order */
94+
/** Warning message if arguments appear to be in the wrong order or UUID was auto-corrected */
6195
warning?: string;
6296
/** Suggestion when first arg looks like an issue short ID */
6397
suggestion?: string;
@@ -72,23 +106,38 @@ export function parsePositionalArgs(args: string[]): {
72106
}
73107

74108
if (args.length === 1) {
75-
const { id: traceId, targetArg } = parseSlashSeparatedArg(
109+
const { id, targetArg } = parseSlashSeparatedArg(
76110
first,
77111
"Trace ID",
78112
USAGE_HINT
79113
);
80-
return { traceId, targetArg };
114+
const validated = validateAndWarn(id);
115+
return {
116+
traceId: validated.traceId,
117+
targetArg,
118+
warning: validated.uuidWarning,
119+
};
81120
}
82121

83122
const second = args[1];
84123
if (second === undefined) {
85-
return { traceId: first, targetArg: undefined };
124+
const validated = validateAndWarn(first);
125+
return {
126+
traceId: validated.traceId,
127+
targetArg: undefined,
128+
warning: validated.uuidWarning,
129+
};
86130
}
87131

88132
// Detect swapped args: user put ID first and target second
89133
const swapWarning = detectSwappedViewArgs(first, second);
90134
if (swapWarning) {
91-
return { traceId: first, targetArg: second, warning: swapWarning };
135+
const validated = validateAndWarn(first);
136+
return {
137+
traceId: validated.traceId,
138+
targetArg: second,
139+
warning: mergeWarnings(swapWarning, validated.uuidWarning),
140+
};
92141
}
93142

94143
// Detect issue short ID passed as first arg (e.g., "CAM-82X some-trace-id")
@@ -97,7 +146,13 @@ export function parsePositionalArgs(args: string[]): {
97146
: undefined;
98147

99148
// Two or more args - first is target, second is trace ID
100-
return { traceId: second, targetArg: first, suggestion };
149+
const validated = validateAndWarn(second);
150+
return {
151+
traceId: validated.traceId,
152+
targetArg: first,
153+
warning: validated.uuidWarning,
154+
suggestion,
155+
};
101156
}
102157

103158
/**

src/lib/hex-id.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,25 @@ import { ValidationError } from "./errors.js";
1010
/** Regex for a valid 32-character hexadecimal ID */
1111
export const HEX_ID_RE = /^[0-9a-f]{32}$/i;
1212

13+
/**
14+
* Regex for UUID format with dashes: 8-4-4-4-12 hex groups.
15+
* Users often copy trace/log IDs from tools that display them in UUID format.
16+
* Stripping the dashes yields a valid 32-character hex ID.
17+
*/
18+
export const UUID_DASH_RE =
19+
/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
20+
1321
/** Max display length for invalid IDs in error messages before truncation */
1422
const MAX_DISPLAY_LENGTH = 40;
1523

1624
/**
1725
* Validate that a string is a 32-character hexadecimal ID.
1826
* Trims whitespace and normalizes to lowercase before validation.
1927
*
28+
* When the input matches UUID format (8-4-4-4-12 hex with dashes), the dashes
29+
* are automatically stripped. This is a common copy-paste mistake — the
30+
* underlying hex content is valid, just formatted differently.
31+
*
2032
* Normalization to lowercase ensures consistent comparison with API responses,
2133
* which return lowercase hex IDs regardless of input casing.
2234
*
@@ -29,7 +41,12 @@ const MAX_DISPLAY_LENGTH = 40;
2941
* @throws {ValidationError} If the format is invalid
3042
*/
3143
export function validateHexId(value: string, label: string): string {
32-
const trimmed = value.trim().toLowerCase();
44+
let trimmed = value.trim().toLowerCase();
45+
46+
// Auto-correct UUID format: strip dashes (8-4-4-4-12 → 32 hex chars)
47+
if (UUID_DASH_RE.test(trimmed)) {
48+
trimmed = trimmed.replace(/-/g, "");
49+
}
3350

3451
if (!HEX_ID_RE.test(trimmed)) {
3552
const display =

test/commands/trace/view.func.test.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ describe("viewCommand.func", () => {
197197
context,
198198
{ json: false, web: false, spans: 100 },
199199
"test-org/test-project",
200-
"0000000000000000"
200+
"00000000000000000000000000000000"
201201
)
202202
).rejects.toThrow(ValidationError);
203203
});
@@ -213,12 +213,14 @@ describe("viewCommand.func", () => {
213213
context,
214214
{ json: false, web: false, spans: 100 },
215215
"test-org/test-project",
216-
"deadbeef12345678"
216+
"deadbeef12345678deadbeef12345678"
217217
);
218218
expect.unreachable("Should have thrown");
219219
} catch (error) {
220220
expect(error).toBeInstanceOf(ValidationError);
221-
expect((error as ValidationError).message).toContain("deadbeef12345678");
221+
expect((error as ValidationError).message).toContain(
222+
"deadbeef12345678deadbeef12345678"
223+
);
222224
}
223225
});
224226

test/commands/trace/view.property.test.ts

Lines changed: 66 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,13 @@ import { describe, expect, test } from "bun:test";
99
import {
1010
array,
1111
assert as fcAssert,
12-
pre,
1312
property,
1413
string,
1514
stringMatching,
1615
tuple,
1716
} from "fast-check";
1817
import { parsePositionalArgs } from "../../../src/commands/trace/view.js";
19-
import { ContextError } from "../../../src/lib/errors.js";
18+
import { ContextError, ValidationError } from "../../../src/lib/errors.js";
2019
import { DEFAULT_NUM_RUNS } from "../../model-based/helpers.js";
2120

2221
/** Valid trace IDs (32-char hex) */
@@ -28,16 +27,21 @@ const slugArb = stringMatching(/^[a-z][a-z0-9-]{1,20}[a-z0-9]$/);
2827
/** Non-empty strings for general args */
2928
const nonEmptyStringArb = string({ minLength: 1, maxLength: 50 });
3029

31-
/** Non-empty strings without slashes (valid plain IDs) */
32-
const plainIdArb = nonEmptyStringArb.filter((s) => !s.includes("/"));
30+
/**
31+
* Insert dashes at UUID positions (8-4-4-4-12) into a 32-char hex string.
32+
*/
33+
function toUuidFormat(hex: string): string {
34+
return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}`;
35+
}
3336

3437
describe("parsePositionalArgs properties", () => {
35-
test("single arg without slashes: returns it as traceId with undefined targetArg", async () => {
38+
test("single valid trace ID: returns it as traceId with undefined targetArg", async () => {
3639
await fcAssert(
37-
property(plainIdArb, (input) => {
40+
property(traceIdArb, (input) => {
3841
const result = parsePositionalArgs([input]);
3942
expect(result.traceId).toBe(input);
4043
expect(result.targetArg).toBeUndefined();
44+
expect(result.warning).toBeUndefined();
4145
}),
4246
{ numRuns: DEFAULT_NUM_RUNS }
4347
);
@@ -69,18 +73,13 @@ describe("parsePositionalArgs properties", () => {
6973
);
7074
});
7175

72-
test("two args: first is always targetArg, second is always traceId", async () => {
76+
test("two args with valid trace ID: first is targetArg, second is traceId", async () => {
7377
await fcAssert(
74-
property(
75-
tuple(nonEmptyStringArb, nonEmptyStringArb),
76-
([first, second]) => {
77-
// Skip swap-detection cases (second has / but first doesn't)
78-
pre(first.includes("/") || !second.includes("/"));
79-
const result = parsePositionalArgs([first, second]);
80-
expect(result.targetArg).toBe(first);
81-
expect(result.traceId).toBe(second);
82-
}
83-
),
78+
property(tuple(nonEmptyStringArb, traceIdArb), ([first, traceId]) => {
79+
const result = parsePositionalArgs([first, traceId]);
80+
expect(result.targetArg).toBe(first);
81+
expect(result.traceId).toBe(traceId);
82+
}),
8483
{ numRuns: DEFAULT_NUM_RUNS }
8584
);
8685
});
@@ -106,17 +105,15 @@ describe("parsePositionalArgs properties", () => {
106105
property(
107106
tuple(
108107
nonEmptyStringArb,
109-
nonEmptyStringArb,
108+
traceIdArb,
110109
array(nonEmptyStringArb, { minLength: 1, maxLength: 5 })
111110
),
112-
([first, second, extras]) => {
113-
// Skip swap-detection cases (second has / but first doesn't)
114-
pre(first.includes("/") || !second.includes("/"));
115-
const args = [first, second, ...extras];
111+
([first, traceId, extras]) => {
112+
const args = [first, traceId, ...extras];
116113
const result = parsePositionalArgs(args);
117114

118115
expect(result.targetArg).toBe(first);
119-
expect(result.traceId).toBe(second);
116+
expect(result.traceId).toBe(traceId);
120117
}
121118
),
122119
{ numRuns: DEFAULT_NUM_RUNS }
@@ -125,17 +122,12 @@ describe("parsePositionalArgs properties", () => {
125122

126123
test("parsing is deterministic: same input always produces same output", async () => {
127124
await fcAssert(
128-
property(
129-
array(nonEmptyStringArb, { minLength: 1, maxLength: 3 }),
130-
(args) => {
131-
// Skip single-arg with slashes — those throw ContextError (tested separately)
132-
pre(args.length > 1 || !args[0]?.includes("/"));
133-
134-
const result1 = parsePositionalArgs(args);
135-
const result2 = parsePositionalArgs(args);
136-
expect(result1).toEqual(result2);
137-
}
138-
),
125+
property(tuple(slugArb, traceIdArb), ([target, traceId]) => {
126+
const args = [target, traceId];
127+
const result1 = parsePositionalArgs(args);
128+
const result2 = parsePositionalArgs(args);
129+
expect(result1).toEqual(result2);
130+
}),
139131
{ numRuns: DEFAULT_NUM_RUNS }
140132
);
141133
});
@@ -144,19 +136,48 @@ describe("parsePositionalArgs properties", () => {
144136
expect(() => parsePositionalArgs([])).toThrow(ContextError);
145137
});
146138

147-
test("result always has traceId property defined", async () => {
139+
test("result always has traceId property defined (valid inputs)", async () => {
148140
await fcAssert(
149-
property(
150-
array(nonEmptyStringArb, { minLength: 1, maxLength: 3 }),
151-
(args) => {
152-
// Skip single-arg with slashes — those throw ContextError (tested separately)
153-
pre(args.length > 1 || !args[0]?.includes("/"));
141+
property(traceIdArb, (traceId) => {
142+
const result = parsePositionalArgs([traceId]);
143+
expect(result.traceId).toBeDefined();
144+
expect(typeof result.traceId).toBe("string");
145+
}),
146+
{ numRuns: DEFAULT_NUM_RUNS }
147+
);
148+
});
154149

155-
const result = parsePositionalArgs(args);
156-
expect(result.traceId).toBeDefined();
157-
expect(typeof result.traceId).toBe("string");
158-
}
159-
),
150+
test("UUID-format trace IDs are accepted and produce 32-char hex", async () => {
151+
await fcAssert(
152+
property(traceIdArb, (hex) => {
153+
const uuid = toUuidFormat(hex);
154+
const result = parsePositionalArgs([uuid]);
155+
expect(result.traceId).toBe(hex);
156+
expect(result.warning).toContain("Auto-corrected");
157+
}),
158+
{ numRuns: DEFAULT_NUM_RUNS }
159+
);
160+
});
161+
162+
test("UUID-format trace IDs work in two-arg case", async () => {
163+
await fcAssert(
164+
property(tuple(slugArb, traceIdArb), ([target, hex]) => {
165+
const uuid = toUuidFormat(hex);
166+
const result = parsePositionalArgs([target, uuid]);
167+
expect(result.traceId).toBe(hex);
168+
expect(result.targetArg).toBe(target);
169+
expect(result.warning).toContain("Auto-corrected");
170+
}),
171+
{ numRuns: DEFAULT_NUM_RUNS }
172+
);
173+
});
174+
175+
test("invalid trace IDs always throw ValidationError", async () => {
176+
const invalidIdArb = stringMatching(/^[g-z]{10,20}$/);
177+
await fcAssert(
178+
property(invalidIdArb, (badId) => {
179+
expect(() => parsePositionalArgs([badId])).toThrow(ValidationError);
180+
}),
160181
{ numRuns: DEFAULT_NUM_RUNS }
161182
);
162183
});

0 commit comments

Comments
 (0)