Skip to content

fix: validate traceparent before using trace context #1897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
miloquinn wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: validate traceparent before using trace context #1897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/helpers/context-header.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,20 @@ export function getTraceContext(traceContext: string | undefined) {
return { traceId: null, spanId: null, sampled: null };
}

const [_version, traceId, spanId, flags] = parts;
const [version, traceId, spanId, flags] = parts;

const isValidTraceContext =

@sagzy sagzy Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: would be worth adding an inline comment here to explain why we're doing this

e.g. "Reject malformed/spoofed traceparent so we don't emit junk trace IDs to Cloud Logging"

/^[0-9a-f]{2}$/.test(version) &&
/^[0-9a-f]{32}$/.test(traceId) &&
traceId !== '00000000000000000000000000000000' &&
/^[0-9a-f]{16}$/.test(spanId) &&
spanId !== '0000000000000000' &&

@sagzy sagzy Jun 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: the 32/16 zeros are hard to read, should we simplify with

!/^0+$/.test(traceId) &&
!/^0+$/.test(spanId)

as the preceding regex already checks for hex of the right length?

/^[0-9a-f]{2}$/.test(flags);
Comment on lines +14 to +20

@coderabbitai coderabbitai Bot Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In the W3C Trace Context traceparent specification, is version value "ff" explicitly invalid/reserved?

💡 Result:

Yes, in the W3C Trace Context traceparent specification, the version value "ff" is explicitly defined as invalid [1][2][3]. The specification states that the version field is an 8-bit unsigned integer serialized as two hexadecimal characters, and specifically reserves "ff" (decimal 255) as an invalid value [1][2][4]. This constraint is formally documented within the header format specification, which includes the ABNF note: "; this document assumes version 00. Version ff is forbidden" [1][3][5].

Citations:

Reject reserved traceparent version ff.

The version field currently accepts ff, but the W3C Trace Context specification explicitly forbids this value. Without this check, invalid trace metadata will be emitted downstream.

Suggested patch 
     const isValidTraceContext =
         /^[0-9a-f]{2}$/.test(version) &&
+        version !== 'ff' &&
         /^[0-9a-f]{32}$/.test(traceId) &&
         traceId !== '00000000000000000000000000000000' &&
         /^[0-9a-f]{16}$/.test(spanId) &&
         spanId !== '0000000000000000' &&
         /^[0-9a-f]{2}$/.test(flags);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const isValidTraceContext =
/^[0-9a-f]{2}$/.test(version) &&
/^[0-9a-f]{32}$/.test(traceId) &&
traceId !== '00000000000000000000000000000000' &&
/^[0-9a-f]{16}$/.test(spanId) &&
spanId !== '0000000000000000' &&
/^[0-9a-f]{2}$/.test(flags);
const isValidTraceContext =
/^[0-9a-f]{2}$/.test(version) &&
version !== 'ff' &&
/^[0-9a-f]{32}$/.test(traceId) &&
traceId !== '00000000000000000000000000000000' &&
/^[0-9a-f]{16}$/.test(spanId) &&
spanId !== '0000000000000000' &&
/^[0-9a-f]{2}$/.test(flags);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/helpers/context-header.ts` around lines 14 - 20, The version field
validation in the isValidTraceContext logic currently accepts the reserved value
`ff`, which violates the W3C Trace Context specification. Add an additional
condition to the isValidTraceContext validation that explicitly checks version
!== 'ff' to reject this reserved version value, ensuring invalid trace metadata
is not emitted downstream.

@sagzy sagzy Jun 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment from Rabbit looks valid / worth addressing

@coderabbitai coderabbitai Bot Jun 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@sagzy, glad we're on the same page! The fix is straightforward — just add version !== 'ff' after the regex check in src/helpers/context-header.ts:

     const isValidTraceContext =
         /^[0-9a-f]{2}$/.test(version) &&
+        version !== 'ff' &&
         /^[0-9a-f]{32}$/.test(traceId) &&
         traceId !== '00000000000000000000000000000000' &&
         /^[0-9a-f]{16}$/.test(spanId) &&
         spanId !== '0000000000000000' &&
         /^[0-9a-f]{2}$/.test(flags);

Don't forget to add a corresponding test case for version === 'ff' in src/helpers/context-header.unit.test.ts as well.


if (!isValidTraceContext) {
return { traceId: null, spanId: null, sampled: null };
}

const sampled = (Number.parseInt(flags, 16) & 0x1) === 1;
return { traceId, spanId, sampled };
}
52 changes: 52 additions & 0 deletions src/helpers/context-header.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import { describe, expect, it } from 'vitest';

import { getTraceContext } from '@/helpers/context-header';

describe('getTraceContext', () => {
const nullTraceContext = {
traceId: null,
spanId: null,
sampled: null,
};

it('returns null fields when traceparent is missing', () => {
expect(getTraceContext(undefined)).toEqual(nullTraceContext);
});

it('parses a valid sampled traceparent header', () => {
expect(
getTraceContext(
'00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01',
),
).toEqual({
traceId: '4bf92f3577b34da6a3ce929d0e0e4736',
spanId: '00f067aa0ba902b7',
sampled: true,
});
});

it('parses a valid unsampled traceparent header', () => {
expect(
getTraceContext(
'00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00',
),
).toEqual({
traceId: '4bf92f3577b34da6a3ce929d0e0e4736',
spanId: '00f067aa0ba902b7',
sampled: false,
});
});

it.each([
'00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7',
'00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01-extra',
'zz-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01',
'00-xyz-00f067aa0ba902b7-01',
'00-00000000000000000000000000000000-00f067aa0ba902b7-01',
'00-4bf92f3577b34da6a3ce929d0e0e4736-xyz-01',
'00-4bf92f3577b34da6a3ce929d0e0e4736-0000000000000000-01',
'00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-zz',
])('returns null fields for invalid traceparent %s', (traceparent) => {
Comment on lines +40 to +49

@coderabbitai coderabbitai Bot Jun 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a regression case for invalid version ff.

The invalid-input matrix is missing the reserved-version case, so the strict-validation objective isn’t fully pinned by tests.

Suggested patch 
     it.each([
         '00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7',
         '00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01-extra',
+        'ff-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01',
         'zz-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01',
         '00-xyz-00f067aa0ba902b7-01',
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/helpers/context-header.unit.test.ts` around lines 40 - 49, The test
matrix in the it.each block for invalid traceparent validation is missing a
regression case for the reserved version "ff", which means strict version
validation isn't fully tested. Add a new traceparent string to the test array
that uses "ff" as the version prefix (the first segment before the first dash)
while keeping the other segments valid, such as
"ff-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", to ensure the
reserved-version validation is properly covered by the test.

expect(getTraceContext(traceparent)).toEqual(nullTraceContext);
});
});