packages/core/src/v3/isomorphic/traceContext.ts (1)

8-25: Harden traceparent parsing and update manager to reuse it

Short: I verified parseTraceparent only enforced 4 parts and found a direct split in the trace context manager that also needs the same validation. Apply the hardened parser and have the manager use it.

Files to change:

• packages/core/src/v3/isomorphic/traceContext.ts — harden parser (accept 3/4 parts, validate hex lengths, non-zero IDs, normalize lowercase, drop invalid flags).
• packages/core/src/v3/traceContext/manager.ts — stop manual split; call the hardened parseTraceparent so external tracecontext is validated consistently (split is at ~line 68).

Proposed diffs:

packages/core/src/v3/isomorphic/traceContext.ts

 export function parseTraceparent(
   traceparent?: string
 ): { traceId: string; spanId: string; traceFlags?: string } | undefined {
   if (!traceparent) {
     return undefined;
   }
-
-  const parts = traceparent.split("-");
-
-  if (parts.length !== 4) {
-    return undefined;
-  }
-
-  const [version, traceId, spanId, traceFlags] = parts;
-
-  if (version !== "00") {
-    return undefined;
-  }
-
-  if (!traceId || !spanId) {
-    return undefined;
-  }
-
-  return { traceId, spanId, traceFlags };
+  const parts = traceparent.split("-");
+
+  // Accept either 3- or 4-part headers (defensive), otherwise reject
+  if (parts.length !== 3 && parts.length !== 4) {
+    return undefined;
+  }
+
+  const [version, traceIdRaw, spanIdRaw, traceFlagsRaw] = parts;
+
+  if (version !== "00") {
+    return undefined;
+  }
+
+  if (!traceIdRaw || !spanIdRaw) {
+    return undefined;
+  }
+
+  const isHex = /^[0-9a-f]+$/i;
+  const traceId = traceIdRaw.toLowerCase();
+  const spanId = spanIdRaw.toLowerCase();
+  const traceFlagsCandidate =
+    typeof traceFlagsRaw === "string" ? traceFlagsRaw.toLowerCase() : undefined;
+
+  // Validate lengths, hex, and not-all-zero constraints
+  if (
+    traceId.length !== 32 ||
+    !isHex.test(traceId) ||
+    traceId === "00000000000000000000000000000000"
+  ) {
+    return undefined;
+  }
+  if (spanId.length !== 16 || !isHex.test(spanId) || spanId === "0000000000000000") {
+    return undefined;
+  }
+
+  // Validate flags if provided; drop invalid flags
+  const traceFlags =
+    traceFlagsCandidate && /^[0-9a-f]{2}$/.test(traceFlagsCandidate)
+      ? traceFlagsCandidate
+      : undefined;
+
+  return { traceId, spanId, traceFlags };
 }

packages/core/src/v3/traceContext/manager.ts

 import { Context, context, propagation, trace, TraceFlags } from "@opentelemetry/api";
 import { TraceContextManager } from "./types.js";
+import { parseTraceparent } from "../isomorphic/traceContext.js";
 
 export class StandardTraceContextManager implements TraceContextManager {
   public traceContext: Record<string, unknown> = {};
@@
   if ("traceparent" in traceContext && typeof traceContext.traceparent === "string") {
-    const [version, traceId, spanId, traceFlags] = traceContext.traceparent.split("-");
-
-    if (!traceId || !spanId) {
-      return undefined;
-    }
-
-    return {
-      traceId,
-      spanId,
-      traceFlags,
-      tracestate: tracestate,
-    };
+    const parsed = parseTraceparent(traceContext.traceparent);
+    if (!parsed) {
+      return undefined;
+    }
+
+    return {
+      traceId: parsed.traceId,
+      spanId: parsed.spanId,
+      traceFlags: parsed.traceFlags,
+      tracestate: tracestate,
+    };
   }

I ran a repo search — the only direct traceparent.split occurrences are:

• packages/core/src/v3/isomorphic/traceContext.ts
• packages/core/src/v3/traceContext/manager.ts (split at ~line 68)

Please apply both changes so all external traceparent parsing is consistent and validated. If you want strict W3C-only behavior instead, keep the 4-part check and reject 3-part headers.

packages/core/src/v3/traceContext/manager.ts (2)

1-3: Import and reuse shared parser to avoid duplication and inconsistencies

extractExternalTraceContext reimplements traceparent parsing. Prefer reusing the shared parseTraceparent to keep validation and normalization centralized.

Apply this diff to import the parser:

-import { Context, context, propagation, trace, TraceFlags } from "@opentelemetry/api";
+import { Context, context, propagation, trace, TraceFlags } from "@opentelemetry/api";
+import { parseTraceparent } from "../isomorphic/traceContext.js";

---

67-79: De-duplicate parsing and enforce validation via parseTraceparent()

Current code splits the string and doesn’t validate version/length/hex. Reuse parseTraceparent for correctness and consistency with isomorphic utilities.

Apply this diff:

-  if ("traceparent" in traceContext && typeof traceContext.traceparent === "string") {
-    const [version, traceId, spanId, traceFlags] = traceContext.traceparent.split("-");
-
-    if (!traceId || !spanId) {
-      return undefined;
-    }
-
-    return {
-      traceId,
-      spanId,
-      traceFlags,
-      tracestate: tracestate,
-    };
-  }
+  if ("traceparent" in traceContext && typeof traceContext.traceparent === "string") {
+    const parsed = parseTraceparent(traceContext.traceparent);
+    if (!parsed) {
+      return undefined;
+    }
+    return {
+      traceId: parsed.traceId,
+      spanId: parsed.spanId,
+      traceFlags: parsed.traceFlags,
+      tracestate,
+    };
+  }

packages/core/src/v3/otel/tracingSDK.ts (1)

413-415: Fix logs/spans correlation when there is no external trace context

Currently, logs skip traceId override if externalTraceContext is absent, but spans still use the generated fallback externalTraceId. This breaks correlation between spans and logs. Allow logs to use the fallback externalTraceId as well.

-    if (!logRecord.spanContext || !this.externalTraceId || !this.externalTraceContext) {
+    if (!logRecord.spanContext || !this.externalTraceId) {
       return logRecord;
     }

packages/core/src/v3/traceContext/types.ts (1)

11-13: Document expected format of traceFlags (2-char hex, lowercase) and consider validation at boundaries

Adding traceFlags?: string is correct. To avoid propagation of malformed flags, please document the expected format and, where feasible, normalize/validate at parse/serialize boundaries (see comments in isomorphic/traceContext.ts).

Would you like me to add a lightweight validation note in the JSDoc here and ensure callers (parse/serialize) normalize to two lowercase hex chars?

apps/webapp/app/runEngine/services/triggerTask.server.ts (1)

380-385: Optional: tolerate older/lenient traceparent formats to avoid skipping propagation

parseTraceparent currently returns undefined for non-4-part headers. If you expect any upstream sources to send 3-part headers, consider making parseTraceparent tolerant (see suggested refactor in isomorphic/traceContext.ts). Otherwise, the code rightly falls back to the original traceContext unchanged.

Do we have any external integrations known to send non-compliant traceparent values? If yes, I can adjust parsing to keep propagation working while still validating IDs.

packages/core/src/v3/otel/tracingSDK.ts (4)

306-313: Dropping internal-only spans can create dangling parents; consider reparenting children

Filtering internal-only spans is correct, but children will still point to a parentSpanId that was removed, producing broken chains in external backends.

Optional refactor: in export(), precompute the set of spans you will drop (internal-only/unsampled), build a map spanId -> parentSpanContext, and reparent exported spans to the nearest exported ancestor (or undefined if none). This preserves a consistent tree after filtering.

---

356-356: Tighten types for exporter API

Match the SpanExporter signature to improve type safety.

-  export(spans: any[], resultCallback: (result: any) => void): void {
+  export(spans: ReadableSpan[], resultCallback: (result: any) => void): void {

---

364-364: Use OTEL diag logger for exporter errors

Prefer diag.error for consistency with OTEL diagnostics and central log control.

-      console.error(e);
+      diag.error("ExternalSpanExporterWrapper.export error", e as Error);

---

395-395: Tighten types for log exporter API

Align with LogRecordExporter for stronger type safety.

-  export(logs: any[], resultCallback: (result: any) => void): void {
+  export(logs: ReadableLogRecord[], resultCallback: (result: any) => void): void {

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-07-18T17:50:25.014Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : ALWAYS generate Trigger.dev tasks using the `task` function from `trigger.dev/sdk/v3` and export them as shown in the correct pattern.

packages/core/src/v3/otel/tracingSDK.ts (2)

341-347: Bug: mapping external traceFlags using string equality drops sampling and ignores numeric flags

externalTraceContext.traceFlags is typed as a number, but the code checks typeof === "string" and compares to "01". This will always fall back to parentSpanContext?.traceFlags, losing the upstream sampling decision. Respect the numeric bitmask instead.

Apply this diff:

-        traceFlags:
-          typeof this.externalTraceContext.traceFlags === "string"
-            ? this.externalTraceContext.traceFlags === "01"
-              ? 1
-              : 0
-            : parentSpanContext?.traceFlags ?? 0,
+        traceFlags:
+          typeof this.externalTraceContext.traceFlags === "number"
+            ? (this.externalTraceContext.traceFlags & TraceFlags.SAMPLED)
+            : parentSpanContext?.traceFlags ?? TraceFlags.NONE,

---

444-476: Harden internal URL detection: avoid substring host checks; use strict hostname match

Using includes() on url.hostname can misclassify hosts (e.g., attacker.example.com/api.trigger.dev). Compare exact hostnames via a Set and check path equality for the ingest endpoint.

Apply this diff:

-  const internalHosts = [
-    "api.trigger.dev",
-    "billing.trigger.dev",
-    "cloud.trigger.dev",
-    "engine.trigger.dev",
-    "platform.trigger.dev",
-  ];
-
-  return (
-    internalHosts.some((host) => url.hostname.includes(host)) ||
-    url.pathname.includes("/api/v1/usage/ingest")
-  );
+  return INTERNAL_HOSTNAMES.has(url.hostname) || url.pathname === "/api/v1/usage/ingest";

Add this near the top of the file (outside the function) to replace the removed inline array:

const INTERNAL_HOSTNAMES = new Set([
  "api.trigger.dev",
  "billing.trigger.dev",
  "cloud.trigger.dev",
  "engine.trigger.dev",
  "platform.trigger.dev",
]);

packages/core/src/v3/traceContext/manager.ts (2)

35-37: Nit: error message references the wrong method name

The message says “withExternalSpan” but the method is withExternalTrace.

Apply this diff:

-        "No active span found. withExternalSpan must be called within an active span context."
+        "No active span found. withExternalTrace must be called within an active span context."

---

63-75: Robust extraction with graceful failure on invalid traceparent

Parsing traceparent and returning undefined when invalid prevents broken traces from malformed inputs. The returned numeric traceFlags is ideal for downstream bitmask checks.

If you want to tighten types here, consider narrowing traceContext to Record<string, unknown> before property access to appease TS without any:

-function extractExternalTraceContext(traceContext: unknown) {
+function extractExternalTraceContext(traceContext: unknown) {
   if (typeof traceContext !== "object" || traceContext === null) {
     return undefined;
   }

-  const tracestate =
-    "tracestate" in traceContext && typeof traceContext.tracestate === "string"
+  const ctx = traceContext as Record<string, unknown>;
+  const tracestate =
+    typeof ctx.tracestate === "string"
       ? traceContext.tracestate
       : undefined;

-  if ("traceparent" in traceContext && typeof traceContext.traceparent === "string") {
-    const externalSpanContext = parseTraceParent(traceContext.traceparent);
+  if (typeof ctx.traceparent === "string") {
+    const externalSpanContext = parseTraceParent(ctx.traceparent);

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

packages/core/src/v3/otel/tracingSDK.ts (1)

439-471: Fix URL host/path checks: use exact hostname match and strict path comparison (CodeQL: incomplete URL substring sanitization)

Using includes() on url.hostname and url.pathname is vulnerable to false positives (e.g., evilapi.trigger.dev matches "api.trigger.dev"; "/api/v1/usage/ingestX" matches the ingest path). Parse and compare strictly.

Apply this diff within this function:

@@
-  const internalHosts = [
-    "api.trigger.dev",
-    "billing.trigger.dev",
-    "cloud.trigger.dev",
-    "engine.trigger.dev",
-    "platform.trigger.dev",
-  ];
-
-  return (
-    internalHosts.some((host) => url.hostname.includes(host)) ||
-    url.pathname.includes("/api/v1/usage/ingest")
-  );
+  return (
+    INTERNAL_HOSTNAMES.has(url.hostname) ||
+    url.pathname === "/api/v1/usage/ingest"
+  );

Add this module-level constant (outside the function) to avoid per-call allocations:

const INTERNAL_HOSTNAMES = new Set<string>([
  "api.trigger.dev",
  "billing.trigger.dev",
  "cloud.trigger.dev",
  "engine.trigger.dev",
  "platform.trigger.dev",
]);

This resolves the CodeQL “Incomplete URL substring sanitization” findings and prevents accidental matches.

packages/core/src/v3/otel/tracingSDK.ts (2)

333-345: Propagate external traceFlags to parent and spanContext for consistency

Parent sampling is respected for gating, but exported spans will still carry their pre-existing traceFlags unless overridden. For consistency with the parent sampling decision and downstream processors, propagate the external traceFlags to both parentSpanContext (when rebuilding) and the span’s spanContext.

Apply this diff:

@@
     if (parentSpanContext) {
       parentSpanContext = {
         ...parentSpanContext,
-        traceId: externalTraceId,
+        traceId: externalTraceId,
+        traceFlags:
+          typeof this.externalTraceContext?.traceFlags === "number"
+            ? this.externalTraceContext.traceFlags
+            : parentSpanContext.traceFlags,
       };
     }
@@
     return {
       ...span,
-      spanContext: () => ({ ...spanContext, traceId: externalTraceId }),
+      spanContext: () => ({
+        ...spanContext,
+        traceId: externalTraceId,
+        traceFlags:
+          typeof this.externalTraceContext?.traceFlags === "number"
+            ? this.externalTraceContext.traceFlags
+            : spanContext.traceFlags,
+      }),
       parentSpanContext,
     };

Also applies to: 347-351

---

378-388: Type the exporter wrapper methods to OTEL types instead of any

The wrapper’s export signatures use any[], which loses type-safety and IDE help. Prefer ReadableLogRecord[]/ReadableSpan[] and the proper callback result type.

Apply these diffs:

@@
-class ExternalLogRecordExporterWrapper {
+class ExternalLogRecordExporterWrapper {
@@
-  export(logs: any[], resultCallback: (result: any) => void): void {
+  export(logs: ReadableLogRecord[], resultCallback: (result: unknown) => void): void {
@@
-    const modifiedLogs = logs.map(this.transformLogRecord.bind(this));
+    const modifiedLogs = logs.map(this.transformLogRecord.bind(this));
     this.underlyingExporter.export(modifiedLogs, resultCallback);
   }

And above, for the span wrapper:

@@
-  export(spans: any[], resultCallback: (result: any) => void): void {
+  export(spans: ReadableSpan[], resultCallback: (result: unknown) => void): void {

Note: If you want to be exact, you can import and use the concrete ExportResult type from OTEL instead of unknown, but the key improvement is avoiding any here.

Also applies to: 390-396

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.