Skip to content

fix(#5436): inject --header values into collector YAML config #5442

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
fullsend-ai-coder wants to merge 5 commits into
base: main
Choose a base branch
from
+87 −33
Open

fix(#5436): inject --header values into collector YAML config #5442

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0edd109
fix(#5436): inject --header values into collector YAML config
fullsend-ai-coder[bot] Jul 22, 2026
1722ffb
chore(traces): Add debug information to upload script
rh-hemartin Jul 22, 2026
ab0ed98
fix: remove dead CONFIG_TEMPLATE code and harden YAML header injection
fullsend-ai-coder[bot] Jul 22, 2026
2843acf
fix(hack): remove unused otelcol config template
fullsend-ai-coder[bot] Jul 22, 2026
afacbac
fix(hack): harden header handling in upload-traces script
rh-hemartin Jul 22, 2026
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
69 changes: 58 additions & 11 deletions hack/upload-traces.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ Arguments:
--endpoint OTLP HTTP endpoint (e.g. http://localhost:4318)
--header key=value Header to send with OTLP requests (repeatable)

Headers are passed via the OTEL_EXPORTER_OTLP_HEADERS env var. Any value
already set in the environment is preserved and appended to.
Headers are injected into the Collector's otlphttp exporter config.
Any key=value pairs already set in the OTEL_EXPORTER_OTLP_HEADERS env
var are also included and merged with --header flags.

Examples:
$(basename "$0") run/agent-run-123/run-telemetry.jsonl --endpoint http://localhost:4318
Expand Down Expand Up @@ -89,19 +90,65 @@ else
fi
echo "include: $INCLUDE"

# --- run collector ---
export REPLAY_INCLUDE="$INCLUDE"
export REPLAY_ENDPOINT="$ENDPOINT"

# Merge --header flags with any existing OTEL_EXPORTER_OTLP_HEADERS.
# --- merge headers ---
# Combine --header flags with any pre-existing OTEL_EXPORTER_OTLP_HEADERS
# into a single comma-separated string for config generation.
MERGED_HEADERS="${OTEL_EXPORTER_OTLP_HEADERS:-}"
if [[ -n "$HEADERS" ]]; then
if [[ -n "${OTEL_EXPORTER_OTLP_HEADERS:-}" ]]; then
export OTEL_EXPORTER_OTLP_HEADERS="${OTEL_EXPORTER_OTLP_HEADERS},${HEADERS}"
if [[ -n "$MERGED_HEADERS" ]]; then
MERGED_HEADERS="${MERGED_HEADERS},${HEADERS}"
else
export OTEL_EXPORTER_OTLP_HEADERS="$HEADERS"
MERGED_HEADERS="$HEADERS"
fi
fi

# --- generate runtime config ---
# The otelcol-contrib Collector reads headers from its YAML config
# (exporters.otlphttp.headers), not from OTEL_EXPORTER_OTLP_HEADERS.
# Generate a runtime config that injects any headers into the YAML.
Comment thread
rh-hemartin marked this conversation as resolved.
Outdated
export REPLAY_INCLUDE="$INCLUDE"
export REPLAY_ENDPOINT="$ENDPOINT"

RUNTIME_CONFIG=$(mktemp "${TMPDIR:-/tmp}/otelcol-config-XXXXXX.yaml")
Comment thread
rh-hemartin marked this conversation as resolved.
Outdated
cleanup() { rm -f "$RUNTIME_CONFIG"; }
trap cleanup EXIT

cat > "$RUNTIME_CONFIG" <<'YAML'
receivers:
otlpjsonfile:
include:
- "${env:REPLAY_INCLUDE}"
start_at: beginning

exporters:
otlphttp:
endpoint: "${env:REPLAY_ENDPOINT}"
YAML

if [[ -n "$MERGED_HEADERS" ]]; then
Comment thread
rh-hemartin marked this conversation as resolved.
echo " headers:" >> "$RUNTIME_CONFIG"
IFS=',' read -ra HEADER_ARRAY <<< "$MERGED_HEADERS"

@waynesun09 waynesun09 Jul 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

HIGH — Header/endpoint values containing a literal comma are silently shredded, corrupting secrets and promoting value fragments into header names

File: hack/upload-traces.sh:53, 90-97, 134-152
Finding: --header flags are joined with a literal comma (line 53: HEADERS="${HEADERS},$2") and merged with OTEL_EXPORTER_OTLP_HEADERS the same way (lines 90-97), then split back apart on that same comma (line 134: IFS=',' read -ra HEADER_ARRAY <<< "$MERGED_HEADERS") with no escaping or encoding step in between. Any header value that itself contains a comma is silently torn into multiple bogus header entries with no error or warning.

Reproduced directly: --header 'Authorization=AWS4-HMAC-SHA256 Credential=AKIAEXAMPLE/20260101/us-east-1/s3/aws4_request,SignedHeaders=host,Signature=deadbeef1234' (a realistic SigV4-style value) splits into three separate headers — Authorization (truncated to just the credential fragment), SignedHeaders: host, and Signature: deadbeef1234 — while the script still prints headers: 3 configured (values redacted) as if nothing were wrong.

Each fragment still passes through escape_confmap, so this doesn't reopen the confmap exfiltration issue. But it does silently corrupt the secret the user intended to send (breaking auth against the real endpoint in a confusing way) and it promotes a chunk of what the user considers one secret value into a header name (Signature above), which some proxies/access logs record more liberally than header values — undermining the redaction work already done elsewhere in this PR. This has been present since early in this PR's header-generation logic and hasn't been exercised by any prior review round's test cases.
Suggestion: Don't reuse the same delimiter for joining CLI flags and splitting the merged string. Build --header key/value pairs into a real bash array at parse time (append on each --header occurrence) instead of a comma-joined string, and reserve comma-splitting only for the legacy OTEL_EXPORTER_OTLP_HEADERS env var. Alternatively, validate that every comma-delimited fragment contains exactly one = and die with a clear error otherwise, rather than silently fabricating headers.

for h in "${HEADER_ARRAY[@]}"; do
key="${h%%=*}"
value="${h#*=}"
# Escape double quotes for YAML safety
value="${value//\"/\\\"}"
printf ' %s: "%s"\n' "$key" "$value" >> "$RUNTIME_CONFIG"
done
fi
Comment thread
rh-hemartin marked this conversation as resolved.

cat >> "$RUNTIME_CONFIG" <<'YAML'

service:
pipelines:
traces:
receivers: [otlpjsonfile]
exporters: [otlphttp]
YAML

echo "endpoint: $ENDPOINT"
if [[ -n "$MERGED_HEADERS" ]]; then
echo "headers: $MERGED_HEADERS"
fi
echo "starting otelcol-contrib (runs continuously, watching for new data; press Ctrl+C to stop)..."
otelcol-contrib --config "$CONFIG_TEMPLATE"
otelcol-contrib --config "$RUNTIME_CONFIG"
Loading