Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
70 changes: 53 additions & 17 deletions internal/cli/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Comment thread
qodo-code-review[bot] marked this conversation as resolved.
Expand Down Expand Up @@ -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"
}
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions internal/cli/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
131 changes: 131 additions & 0 deletions internal/cli/telemetry_run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
14 changes: 14 additions & 0 deletions internal/security/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
31 changes: 31 additions & 0 deletions internal/security/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading
Loading