Merge pull request #2960 from dhshah13/feat/2779-inbound-traceparent

fix(#2779): respect inbound TRACEPARENT in trace chain

Author: rh-hemartin

internal/cli/run.go

@@ -586,21 +586,21 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep
 		}
 	}
 
-	// Trace identity (ADR 0050 Level 1 + security correlation). Generated here,
+	// Trace identity (ADR 0050 Level 1 + security correlation). Resolved here,
 	// before the pre-script, so TRACEPARENT can propagate to child processes.
-	// The same id is reused as the security finding/audit trace id (dashed UUID)
-	// and, dash-stripped, as the W3C telemetry trace id — one id across both.
-	securityTraceID := security.GenerateTraceID()
-	wTraceID := telemetry.TraceIDFromUUID(securityTraceID)
-	rootSpanID := telemetry.NewSpanID()
+	// An inbound TRACEPARENT (nested/instrumented invocation, issue #2779) is
+	// adopted so the run continues the parent trace; otherwise a fresh id is
+	// generated. Either way one id serves as both the security finding/audit
+	// trace id (dashed UUID) and, dash-stripped, the W3C telemetry trace id.
+	securityTraceID, traceCtx := resolveTraceIdentity(os.Getenv("TRACEPARENT"))
 	workItemID := resolveWorkItemID()
 
 	// 2c. Run pre-script on the host (if configured).
 	if h.PreScript != "" {
 		preStart := time.Now()
 		printer.StepStart("Running pre-script: " + h.PreScript)
 		preCmd := exec.Command(h.PreScript)
-		preCmd.Env = childScriptEnv(h.RunnerEnv, telemetry.TraceParent(wTraceID, rootSpanID))
+		preCmd.Env = childScriptEnv(h.RunnerEnv, telemetry.TraceParentWithFlags(traceCtx.TraceID, traceCtx.RootSpanID, traceCtx.Flags))
 		preCmd.Stdout = os.Stdout
 		preCmd.Stderr = os.Stderr
 		if err := preCmd.Run(); err != nil {
@@ -626,7 +626,7 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep
 	// order — it runs last and the summary captures the whole run.
 	var lastExitCode int
 	var transcriptErrorOverride bool
-	rec := telemetry.New(runDir, wTraceID, rootSpanID, agentName, workItemID, runStart)
+	rec := telemetry.New(runDir, traceCtx, agentName, workItemID, runStart)
 	defer func() { rec.Finalize(telemetryExitCode(lastExitCode, runErr)) }()
 
 	createStart := time.Now()
@@ -675,7 +675,7 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep
 			printer.StepStart("Running post-script: " + h.PostScript)
 			postCmd := exec.Command(h.PostScript)
 			postCmd.Dir = runDir
-			postCmd.Env = childScriptEnv(h.RunnerEnv, telemetry.TraceParent(wTraceID, rootSpanID))
+			postCmd.Env = childScriptEnv(h.RunnerEnv, telemetry.TraceParentWithFlags(traceCtx.TraceID, traceCtx.RootSpanID, traceCtx.Flags))
 			postCmd.Stdout = os.Stdout
 			postCmd.Stderr = os.Stderr
 			if err := postCmd.Run(); err != nil {
@@ -1681,12 +1681,46 @@ func telemetryExitCode(lastExitCode int, runErr error) int {
 	return lastExitCode
 }
 
+// resolveTraceIdentity resolves the run's trace identity (issue #2779). A
+// valid inbound W3C TRACEPARENT is adopted: its trace-id becomes both the
+// security trace id (re-dashed UUID) and the W3C telemetry trace-id, its
+// span-id becomes the root span's remote parent, and its trace-flags carry
+// the upstream sampling decision forward. Without a valid inbound value a
+// fresh identity is generated, exactly as Level 1 always did. TRACESTATE is
+// intentionally untouched — it passes through os.Environ to child scripts.
+func resolveTraceIdentity(inbound string) (securityTraceID string, tc telemetry.TraceContext) {
+	if traceID, parentSpanID, flags, ok := telemetry.ParseTraceParent(inbound); ok {
+		return telemetry.UUIDFromTraceID(traceID), telemetry.TraceContext{
+			TraceID:      traceID,
+			RootSpanID:   telemetry.NewSpanID(),
+			ParentSpanID: parentSpanID,
+			Flags:        flags,
+		}
+	}
+	securityTraceID = security.GenerateTraceID()
+	return securityTraceID, telemetry.TraceContext{
+		TraceID:    telemetry.TraceIDFromUUID(securityTraceID),
+		RootSpanID: telemetry.NewSpanID(),
+		Flags:      "01",
+	}
+}
+
 // childScriptEnv builds the environment for a host-side child script (pre- or
 // post-script): the harness RunnerEnv layered over the process environment,
-// plus the W3C TRACEPARENT for trace propagation (ADR 0050 Level 1). An empty
-// traceparent (telemetry disabled) is omitted rather than emitted blank.
+// plus the W3C TRACEPARENT for trace propagation (ADR 0050 Level 1). Any
+// TRACEPARENT already present — inherited from the process environment or
+// set in runner_env — is filtered out first: env lookups resolve the first
+// match, so a stale value would shadow fullsend's own, and fullsend's trace
+// identity never derives from runner_env (issue #2779). An empty traceparent
+// (telemetry disabled) is omitted rather than emitted blank.
 func childScriptEnv(runnerEnv map[string]string, traceparent string) []string {
-	env := append(os.Environ(), envToList(runnerEnv)...)
+	merged := append(os.Environ(), envToList(runnerEnv)...)
+	env := make([]string, 0, len(merged)+1)
+	for _, e := range merged {
+		if !strings.HasPrefix(e, "TRACEPARENT=") {
+			env = append(env, e)
+		}
+	}
 	if traceparent != "" {
 		env = append(env, "TRACEPARENT="+traceparent)
 	}
@@ -1857,9 +1891,11 @@ func refreshOIDCToken(ctx context.Context, sandboxName, oidcURL, oidcAuth string
 // inside the sandbox. It finds known context files (including SKILL.md in
 // skill directories) in the repo directory and passes them as arguments.
 func buildScanContextCommand(repoDir, traceID string) string {
-	// Defense-in-depth: validate traceID before shell interpolation even though
-	// GenerateTraceID() only produces safe hex characters.
-	if !security.IsValidTraceID(traceID) {
+	// Defense-in-depth: validate traceID before shell interpolation. Uses
+	// IsShellSafeTraceID (not IsValidTraceID) because the id may have been
+	// adopted from an inbound W3C traceparent (issue #2779), so it is not
+	// necessarily UUID v4.
+	if !security.IsShellSafeTraceID(traceID) {
 		// Should never happen with internal generation, but fail safely.
 		traceID = "invalid-trace-id"
 	}
@@ -2196,10 +2232,10 @@ func scanOutputFiles(outputDir, traceID string, printer *ui.Printer) error {
 
 // injectTraceID appends the FULLSEND_TRACE_ID to the sandbox .env file.
 func injectTraceID(sandboxName, traceID string) error {
-	if !security.IsValidTraceID(traceID) {
+	if !security.IsShellSafeTraceID(traceID) {
 		return fmt.Errorf("invalid trace ID format: %q", traceID)
 	}
-	// Safe: IsValidTraceID() above ensures traceID matches UUID v4 format only.
+	// Safe: IsShellSafeTraceID() above ensures traceID is only hex and dashes.
 	cmd := fmt.Sprintf("echo 'export FULLSEND_TRACE_ID=%s' >> %s/.env", traceID, sandbox.SandboxWorkspace)
 	_, _, _, err := sandbox.Exec(sandboxName, cmd, 10*time.Second)
 	return err

internal/cli/run_test.go

@@ -414,6 +414,16 @@ func TestBuildScanContextCommand_SourcesEnv(t *testing.T) {
 	assert.Contains(t, cmd, "-exec fullsend scan context")
 }
 
+func TestBuildScanContextCommand_AcceptsAdoptedTraceID(t *testing.T) {
+	// A trace id adopted from an inbound W3C traceparent (issue #2779) is
+	// dashed hex but not UUID v4; it must survive validation, not be replaced
+	// with the "invalid-trace-id" sentinel.
+	traceID := "4f3a9c1b-2d8e-0a7c-1f0b-1e2d3c4a5b6d"
+	cmd := buildScanContextCommand("/sandbox/workspace/repo", traceID)
+	assert.Contains(t, cmd, "FULLSEND_TRACE_ID='"+traceID+"'")
+	assert.NotContains(t, cmd, "invalid-trace-id")
+}
+
 func TestCopyFile(t *testing.T) {
 	t.Run("copies content and preserves permissions", func(t *testing.T) {
 		src := filepath.Join(t.TempDir(), "source")

internal/cli/telemetry_run_test.go

@@ -121,6 +121,137 @@ func TestChildScriptEnv_EmptyTraceparentOmitted(t *testing.T) {
 	}
 }
 
+func TestChildScriptEnv_FiltersPreExistingTraceparent(t *testing.T) {
+	// A parent process may already export TRACEPARENT (issue #2779). Most
+	// runtimes resolve the FIRST match in the environment, so the stale value
+	// must be filtered out — exactly one TRACEPARENT entry, fullsend's own.
+	t.Setenv("TRACEPARENT", "00-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1-bbbbbbbbbbbbbbbb-01")
+	const fullsendTP = "00-4f3a9c1b2d8e4a7c9f0b1e2d3c4a5b6d-a1b2c3d4e5f60718-01"
+
+	env := childScriptEnv(map[string]string{}, fullsendTP)
+
+	traceparents := 0
+	for _, e := range env {
+		if strings.HasPrefix(e, "TRACEPARENT=") {
+			traceparents++
+			assert.Equal(t, "TRACEPARENT="+fullsendTP, e, "must be fullsend's value, not the parent's")
+		}
+	}
+	assert.Equal(t, 1, traceparents, "exactly one TRACEPARENT entry after filtering")
+}
+
+func TestChildScriptEnv_EmptyTraceparentFiltersExisting(t *testing.T) {
+	// Even with telemetry disabled (empty traceparent), a stale inherited
+	// TRACEPARENT must not leak through to child scripts.
+	t.Setenv("TRACEPARENT", "00-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1-bbbbbbbbbbbbbbbb-01")
+
+	env := childScriptEnv(map[string]string{}, "")
+	for _, e := range env {
+		assert.False(t, strings.HasPrefix(e, "TRACEPARENT="), "stale TRACEPARENT must be filtered even when disabled")
+	}
+}
+
+func TestChildScriptEnv_FiltersRunnerEnvTraceparent(t *testing.T) {
+	// A harness-provided runner_env.TRACEPARENT would land before fullsend's
+	// appended value and win first-match resolution — same shadowing bug as
+	// the inherited process env, so it gets the same filter. fullsend's
+	// trace identity never derives from runner_env; honoring it would only
+	// desync child scripts from the recorded trace.
+	const fullsendTP = "00-4f3a9c1b2d8e4a7c9f0b1e2d3c4a5b6d-a1b2c3d4e5f60718-01"
+	runnerEnv := map[string]string{
+		"TRACEPARENT": "00-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1-bbbbbbbbbbbbbbbb-01",
+		"FOO":         "bar",
+	}
+
+	env := childScriptEnv(runnerEnv, fullsendTP)
+
+	traceparents := 0
+	hasFoo := false
+	for _, e := range env {
+		if strings.HasPrefix(e, "TRACEPARENT=") {
+			traceparents++
+			assert.Equal(t, "TRACEPARENT="+fullsendTP, e, "must be fullsend's value, not runner_env's")
+		}
+		if e == "FOO=bar" {
+			hasFoo = true
+		}
+	}
+	assert.Equal(t, 1, traceparents, "exactly one TRACEPARENT entry")
+	assert.True(t, hasFoo, "other runner_env entries preserved")
+}
+
+func TestChildScriptEnv_EmptyTraceparentFiltersRunnerEnv(t *testing.T) {
+	// Telemetry disabled: a runner_env TRACEPARENT must not leak either.
+	env := childScriptEnv(map[string]string{"TRACEPARENT": "00-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa1-bbbbbbbbbbbbbbbb-01"}, "")
+	for _, e := range env {
+		assert.False(t, strings.HasPrefix(e, "TRACEPARENT="), "runner_env TRACEPARENT must be filtered when disabled")
+	}
+}
+
+func TestChildScriptEnv_PreservesTracestate(t *testing.T) {
+	// W3C tracestate carries vendor context alongside traceparent and must
+	// pass through to child scripts untouched.
+	t.Setenv("TRACESTATE", "vendor=abc123,other=def456")
+	const tp = "00-4f3a9c1b2d8e4a7c9f0b1e2d3c4a5b6d-a1b2c3d4e5f60718-01"
+
+	env := childScriptEnv(map[string]string{}, tp)
+
+	found := false
+	for _, e := range env {
+		if e == "TRACESTATE=vendor=abc123,other=def456" {
+			found = true
+		}
+	}
+	assert.True(t, found, "TRACESTATE must pass through to child scripts")
+}
+
+func TestResolveTraceIdentity(t *testing.T) {
+	const (
+		tid = "4f3a9c1b2d8e4a7c9f0b1e2d3c4a5b6d"
+		sid = "a1b2c3d4e5f60718"
+	)
+	reSpanID := regexp.MustCompile(`^[0-9a-f]{16}$`)
+
+	t.Run("adopts valid inbound sampled traceparent", func(t *testing.T) {
+		securityID, tc := resolveTraceIdentity("00-" + tid + "-" + sid + "-01")
+
+		assert.Equal(t, tid, tc.TraceID, "inbound trace-id adopted")
+		assert.Equal(t, sid, tc.ParentSpanID, "inbound span-id becomes the root span's remote parent")
+		assert.Equal(t, "01", tc.Flags)
+		assert.Equal(t, "4f3a9c1b-2d8e-4a7c-9f0b-1e2d3c4a5b6d", securityID, "security id derived from inbound trace-id")
+		assert.Equal(t, tid, telemetry.TraceIDFromUUID(securityID), "security id must round-trip to the W3C id")
+		assert.True(t, security.IsShellSafeTraceID(securityID))
+
+		assert.Regexp(t, reSpanID, tc.RootSpanID, "fresh root span id")
+		assert.NotEqual(t, sid, tc.RootSpanID, "root span is a child, not the inbound span itself")
+	})
+
+	t.Run("preserves unsampled flag", func(t *testing.T) {
+		_, tc := resolveTraceIdentity("00-" + tid + "-" + sid + "-00")
+		assert.Equal(t, "00", tc.Flags, "upstream unsampled decision preserved")
+	})
+
+	t.Run("adopted non-v4 trace-id is shell-safe", func(t *testing.T) {
+		// version nibble 0, variant nibble 1 — valid W3C, not UUID v4.
+		securityID, _ := resolveTraceIdentity("00-4f3a9c1b2d8e0a7c1f0b1e2d3c4a5b6d-" + sid + "-01")
+		assert.Equal(t, "4f3a9c1b-2d8e-0a7c-1f0b-1e2d3c4a5b6d", securityID)
+		assert.True(t, security.IsShellSafeTraceID(securityID))
+		assert.False(t, security.IsValidTraceID(securityID), "adopted id is not v4 — needs the shell-safe validator")
+	})
+
+	for _, inbound := range []string{"", "not-a-traceparent", "00-" + tid + "-" + sid, "ff-" + tid + "-" + sid + "-01"} {
+		t.Run("falls back to fresh identity for "+fmt.Sprintf("%q", inbound), func(t *testing.T) {
+			securityID, tc := resolveTraceIdentity(inbound)
+
+			assert.True(t, security.IsValidTraceID(securityID), "fresh id is a v4 UUID")
+			assert.Equal(t, telemetry.TraceIDFromUUID(securityID), tc.TraceID, "unified trace id (Level 1 invariant)")
+			assert.Empty(t, tc.ParentSpanID, "local trace root has no remote parent")
+			assert.Equal(t, "01", tc.Flags, "fresh traces are sampled")
+			assert.Regexp(t, reSpanID, tc.RootSpanID)
+		})
+	}
+}
+
 func TestAgentSpanEndAttrs(t *testing.T) {
 	var m agentruntime.RunMetrics
 	m.Model = "claude-opus-4-6"

internal/security/trace.go

@@ -31,6 +31,20 @@ func IsValidTraceID(id string) bool {
 	return reTraceID.MatchString(id)
 }
 
+// reShellSafeTraceID matches any dashed lowercase-hex string in UUID shape
+// (8-4-4-4-12), without the UUID v4 version/variant requirement of reTraceID.
+var reShellSafeTraceID = regexp.MustCompile(`^[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}$`)
+
+// IsShellSafeTraceID returns true if the trace ID consists only of lowercase
+// hex and dashes in UUID shape — the property that makes it safe for shell
+// interpolation. Unlike IsValidTraceID it accepts any version/variant, because
+// a trace id adopted from an inbound W3C traceparent (issue #2779) is not
+// necessarily UUID v4. The charset restriction is the entire safety argument;
+// version bits carry no interpolation risk.
+func IsShellSafeTraceID(id string) bool {
+	return reShellSafeTraceID.MatchString(id)
+}
+
 // seedHash is the well-known genesis hash for the first entry in a chain.
 const seedHash = "0000000000000000000000000000000000000000000000000000000000000000"
 

internal/security/trace_test.go

@@ -14,6 +14,37 @@ func TestGenerateTraceID(t *testing.T) {
 	}
 }
 
+func TestIsShellSafeTraceID(t *testing.T) {
+	// A generated UUID v4 passes both the strict and the shell-safe validator.
+	id := GenerateTraceID()
+	if !IsShellSafeTraceID(id) {
+		t.Errorf("generated trace ID %q should be shell-safe", id)
+	}
+
+	// A trace id adopted from an inbound W3C traceparent is dashed hex but
+	// generally not UUID v4: shell-safe must accept it, strict must reject it.
+	adopted := "4f3a9c1b-2d8e-0a7c-1f0b-1e2d3c4a5b6d" // version 0, variant 0
+	if !IsShellSafeTraceID(adopted) {
+		t.Error("adopted non-v4 trace ID should be shell-safe")
+	}
+	if IsValidTraceID(adopted) {
+		t.Error("adopted non-v4 trace ID should NOT pass strict v4 validation")
+	}
+
+	for _, bad := range []string{
+		"zzzzzzzz-zzzz-zzzz-zzzz-zzzzzzzzzzzz", // non-hex
+		"4F3A9C1B-2D8E-4A7C-9F0B-1E2D3C4A5B6D", // uppercase
+		"4f3a9c1b-2d8e",                        // wrong length
+		"4f3a9c1b2d8e4a7c9f0b1e2d3c4a5b6d",     // undashed
+		"",                                     // empty
+		"4f3a9c1b-2d8e-4a7c-9f0b-1e2d3c4a5b6d; rm -rf /", // injection attempt
+	} {
+		if IsShellSafeTraceID(bad) {
+			t.Errorf("IsShellSafeTraceID(%q) must be false", bad)
+		}
+	}
+}
+
 func TestAppendFindingHashChain(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "findings.jsonl")