diff --git a/.github/scripts/setup-moonbit.sh b/.github/scripts/setup-moonbit.sh new file mode 100755 index 0000000..be5fc45 --- /dev/null +++ b/.github/scripts/setup-moonbit.sh @@ -0,0 +1,74 @@ +#!/usr/bin/env bash + +set -euo pipefail + +readonly MOONBIT_VERSION='0.10.5+5e7afb0c0' +readonly MOONBIT_VERSION_URL='0.10.5%2B5e7afb0c0' +readonly MOONBIT_BASE_URL='https://cli.moonbitlang.com' +readonly TOOLCHAIN_SHA256='07da4e4b21d3ea203b00183906e9e6a3104d77b9756a95c4d9e648471cf2e2d8' +readonly CORE_SHA256='d92991190e30d10a1ab6fcd7d0282b8ddd0004d0309bc40fbc056310cf20bfc5' + +: "${RUNNER_TEMP:?RUNNER_TEMP must be set}" +: "${GITHUB_ENV:?GITHUB_ENV must be set}" +: "${GITHUB_PATH:?GITHUB_PATH must be set}" + +moonbit_setup_root=$(mktemp -d "${RUNNER_TEMP}/moonbit-toolchain.XXXXXX") +readonly moonbit_setup_root +readonly moon_home="${moonbit_setup_root}/home" +readonly download_dir="${moonbit_setup_root}/downloads" +readonly toolchain_archive="${download_dir}/moonbit-linux-x86_64.tar.gz" +readonly core_archive="${download_dir}/core.tar.gz" + +cleanup_downloads() { + if [[ "${download_dir}" == "${RUNNER_TEMP}"/moonbit-toolchain.*/downloads ]]; then + rm -rf -- "${download_dir}" + fi +} +trap cleanup_downloads EXIT + +mkdir -p "${download_dir}" "${moon_home}/lib" + +download() { + local source_url=$1 + local destination=$2 + curl \ + --proto '=https' \ + --tlsv1.2 \ + --fail \ + --location \ + --show-error \ + --silent \ + --output "${destination}" \ + "${source_url}" +} + +verify_archive() { + local expected_sha256=$1 + local archive=$2 + printf '%s %s\n' "${expected_sha256}" "${archive}" | sha256sum --check --strict +} + +download \ + "${MOONBIT_BASE_URL}/binaries/${MOONBIT_VERSION_URL}/moonbit-linux-x86_64.tar.gz" \ + "${toolchain_archive}" +download \ + "${MOONBIT_BASE_URL}/cores/core-${MOONBIT_VERSION_URL}.tar.gz" \ + "${core_archive}" + +verify_archive "${TOOLCHAIN_SHA256}" "${toolchain_archive}" +verify_archive "${CORE_SHA256}" "${core_archive}" + +tar -xzf "${toolchain_archive}" -C "${moon_home}" +tar -xzf "${core_archive}" -C "${moon_home}/lib" +ln -s moon "${moon_home}/bin/moonx" +chmod +x "${moon_home}"/bin/* +chmod +x "${moon_home}/bin/internal/tcc" + +PATH="${moon_home}/bin:${PATH}" \ + "${moon_home}/bin/moon" -C "${moon_home}/lib/core" bundle --warn-list -a --all +PATH="${moon_home}/bin:${PATH}" \ + "${moon_home}/bin/moon" -C "${moon_home}/lib/core" bundle --warn-list -a --target wasm-gc --quiet + +printf 'MOON_HOME=%s\n' "${moon_home}" >> "${GITHUB_ENV}" +printf '%s\n' "${moon_home}/bin" >> "${GITHUB_PATH}" +printf 'Installed MoonBit %s in %s\n' "${MOONBIT_VERSION}" "${moon_home}" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5babdc7..5d17acc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,9 @@ on: pull_request: branches: [main] +permissions: + contents: read + jobs: check: runs-on: ubuntu-latest @@ -14,9 +17,7 @@ jobs: uses: actions/checkout@v5 - name: Set up MoonBit - run: | - curl -fsSL https://cli.moonbitlang.com/install/unix.sh | bash - echo "$HOME/.moon/bin" >> $GITHUB_PATH + run: .github/scripts/setup-moonbit.sh - name: Report MoonBit toolchain run: moon version --all --json diff --git a/.github/workflows/copilot-setup-steps.yml b/.github/workflows/copilot-setup-steps.yml index 6f8811a..8b399bb 100644 --- a/.github/workflows/copilot-setup-steps.yml +++ b/.github/workflows/copilot-setup-steps.yml @@ -30,12 +30,10 @@ jobs: uses: actions/checkout@v5 - name: Set up MoonBit - run: | - curl -fsSL https://cli.moonbitlang.com/install/unix.sh | bash - echo "$HOME/.moon/bin" >> $GITHUB_PATH + run: .github/scripts/setup-moonbit.sh + + - name: Report MoonBit toolchain + run: moon version --all --json - name: Update MoonBit dependencies - run: | - moon version --all - moon update - + run: moon update diff --git a/CHANGELOG.md b/CHANGELOG.md index 93af019..14a5b90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security + +- Human-readable event messages, field keys, and trace-context diagnostics now render C0/C1 control codes as visible escapes, preventing injected line breaks and terminal control sequences while leaving structured JSON unchanged. +- W3C `traceparent` and `tracestate` parsing now rejects inputs over 512 characters before eager allocation; `tracestate` also rejects duplicate wire keys instead of applying last-write-wins behavior. +- Trace and span identifier generation now seeds ChaCha8 from 32 bytes of platform entropy when available, with the existing clock-derived seed retained only as a documented fallback. +- Newly created native file-subscriber logs and gzip rotations request owner-only `0600` permissions on Unix. Existing file modes are not changed. +- The default OTLP HTTP client now streams response bodies with a 64 KiB ceiling. Oversized responses fail as non-retryable request errors. +- GitHub workflows now install a pinned MoonBit toolchain and core from versioned archives whose SHA-256 digests are verified before extraction, replacing the mutable remote installer pipeline. + ## [0.13.0] - 2026-06-01 ### Added diff --git a/README.mbt.md b/README.mbt.md index cbc94ba..7c8b29c 100644 --- a/README.mbt.md +++ b/README.mbt.md @@ -242,6 +242,12 @@ match @moontrace.parse_tracestate("rojo=00f067aa0ba902b7,congo=t61rcWkgMzE") { `SpanContext` keeps the remote trace ID, span ID, sampled flag, tracestate, and remote/local marker. `span_from_remote_context` creates a local child span while preserving the incoming sampled flag for downstream export. +Incoming `traceparent` and `tracestate` values are limited to 512 characters. +Serialized `tracestate` values may contain at most 32 non-empty members and must +not repeat a key. `parse_span_context` preserves a valid `traceparent` while +dropping malformed, oversized, or duplicate-key `tracestate`, matching its +partial-recovery behavior. + ### Span Links Parent/child spans model ownership. Links model causal edges that should not change the parent relationship, such as retries, queued work, or fan-in. @@ -336,6 +342,12 @@ Output: 14:31:44.500 | ERROR | handler — delivery failed target="leaf-1" exit_code=2 ``` +Human-readable formatting keeps each event on one physical record. C0/C1 +control codes in event messages, field keys, and trace-context diagnostics are +rendered visibly (`\\n`, `\\r`, `\\t`, or `\\u{001b}`-style escapes). Printable +Unicode is preserved. The JSON subscriber remains governed by JSON string +escaping and is unchanged. + ### JSON Subscriber Machine-readable JSON output: @@ -400,6 +412,12 @@ pub async fn export_batch( } ``` +The default HTTP client streams response bodies and retains at most 64 KiB. +Larger responses return `Request("HTTP response body exceeds 65536 bytes")` and +are not retried. Successful and error response bodies within the limit remain +available through `HttpResponse.body`; injected clients keep the same public +interface. + ### Subscriber Composition Route events to multiple subscribers: @@ -492,6 +510,9 @@ See [docs/flame.md](docs/flame.md) for rendering with `inferno-flamegraph` or `f `brickfrog/moontrace/file` provides a native-only buffered JSONL file subscriber. The synchronous subscriber enqueues without blocking the logging call site; an async worker drains, rotates, optionally gzips rotated files, and tracks written/dropped counts. +On Unix, files newly created by the subscriber request owner-only `0600` +permissions. Existing file modes are left unchanged. + ```mbt pub async fn install_file_subscriber() -> Unit { let files = @file.file_subscriber( diff --git a/docs/file.md b/docs/file.md index 5e2d0bd..41a8a64 100644 --- a/docs/file.md +++ b/docs/file.md @@ -36,6 +36,17 @@ renamed to `path.1`, older rotated files are shifted up, and files beyond `max_files` are removed. With `gzip=true`, rotated files are written as `path.N.gz` and the temporary uncompressed rotation is removed. +On Unix, every active or gzip file newly created by the subscriber requests +mode `0600` (owner read/write only). The process umask may make that mode more +restrictive, but cannot grant group or other access. A raw rotated file is a +rename of the active file and retains its mode. + +The subscriber does not change the permissions of an existing active file. If +the configured path already exists, its mode and ownership remain the caller's +responsibility. The caller is also responsible for the ownership and +permissions of the parent directory. The underlying `moonbitlang/async/fs` +permission argument is ignored on Windows. + The file subscriber uses `moonbitlang/async/fs` and is only supported on the native target. Constructing it on other targets aborts immediately with a clear unsupported-target error instead of returning a subscriber that drops events. diff --git a/src/escape.mbt b/src/escape.mbt new file mode 100644 index 0000000..a0d2d31 --- /dev/null +++ b/src/escape.mbt @@ -0,0 +1,25 @@ +///| +/// Render control code points visibly in human-readable output while preserving +/// printable text verbatim. +fn escape_control_chars(value : String) -> String { + let buf = StringBuilder() + for ch in value.iter() { + match ch { + '\b' => buf.write_string("\\b") + '\u{000c}' => buf.write_string("\\f") + '\n' => buf.write_string("\\n") + '\r' => buf.write_string("\\r") + '\t' => buf.write_string("\\t") + _ => + if ch.is_control() { + let hex = ch.to_int().to_string(radix=16).pad_start(4, '0') + buf.write_string("\\u{") + buf.write_string(hex) + buf.write_string("}") + } else { + buf.write_char(ch) + } + } + } + buf.to_string() +} diff --git a/src/event_format.mbt b/src/event_format.mbt index 9da9cd5..518a941 100644 --- a/src/event_format.mbt +++ b/src/event_format.mbt @@ -54,7 +54,7 @@ pub fn format_event( buf.write_string(source) } buf.write_string(" — ") - buf.write_string(event.message) + buf.write_string(escape_control_chars(event.message)) if !event.fields.is_empty() { buf.write_string(" ") let mut first = true @@ -64,7 +64,7 @@ pub fn format_event( } else { buf.write_string(" ") } - buf.write_string(f.key) + buf.write_string(escape_control_chars(f.key)) buf.write_string("=") buf.write_string(f.value.stringify()) }) diff --git a/src/event_format_test.mbt b/src/event_format_test.mbt index 6a56863..d8af275 100644 --- a/src/event_format_test.mbt +++ b/src/event_format_test.mbt @@ -92,6 +92,19 @@ test "event format two spaces before fields one between" { ) } +///| +test "event format escapes control characters in messages and field keys" { + let event = sample_event( + @moontrace.Info, + "line1\nline2\r\t\u001b\u007f\u009f", + fields=@moontrace.fields([("key\n\u001b", "printable 汉字 \\n".to_json())]), + ) + assert_eq( + event.format(), + "14:31:43.903 | INFO | choir/server — line1\\nline2\\r\\t\\u{001b}\\u{007f}\\u{009f} key\\n\\u{001b}=\"printable 汉字 \\\\n\"", + ) +} + ///| test "format timestamp utc at zero" { assert_eq(@moontrace.format_timestamp_utc((0 : UInt64)), "00:00:00.000") diff --git a/src/field_show.mbt b/src/field_show.mbt index 4e60dad..8b92706 100644 --- a/src/field_show.mbt +++ b/src/field_show.mbt @@ -1,6 +1,8 @@ ///| pub impl Show for Field with fn output(self, logger) { - logger.write_string("\{self.key}=\{self.value.stringify()}") + logger.write_string(escape_control_chars(self.key)) + logger.write_string("=") + logger.write_string(self.value.stringify()) } ///| diff --git a/src/field_show_test.mbt b/src/field_show_test.mbt index ba3edd3..f7af4ad 100644 --- a/src/field_show_test.mbt +++ b/src/field_show_test.mbt @@ -11,6 +11,18 @@ test "field show via interpolation" { assert_eq(shown, "count=42") } +///| +test "field show escapes control characters in keys" { + let f = @moontrace.Field::{ + key: "na\b\u{000c}\n\r\t\u001b\u007f\u009fme", + value: "printable 汉字 \\n".to_json(), + } + assert_eq( + f.to_string(), + "na\\b\\f\\n\\r\\t\\u{001b}\\u{007f}\\u{009f}me=\"printable 汉字 \\\\n\"", + ) +} + ///| test "field to_json wraps as object" { let f = @moontrace.Field::{ key: "key", value: "val".to_json() } diff --git a/src/file/file_subscriber.mbt b/src/file/file_subscriber.mbt index 2712a75..50651ec 100644 --- a/src/file/file_subscriber.mbt +++ b/src/file/file_subscriber.mbt @@ -10,6 +10,10 @@ const DEFAULT_MAX_BATCH : Int = 128 ///| const DEFAULT_QUEUE_CAPACITY : Int = 1024 +///| +#cfg(target="native") +const FILE_PERMISSION : Int = 0o600 + ///| pub(all) struct FileSubscriberConfig { path : String @@ -351,6 +355,7 @@ async fn FileSubscriber::write_batch( self.config.path, payload, create_mode=@fs.CreateMode::OpenOrCreate, + permission=FILE_PERMISSION, append=true, ) self.current_size += payload_bytes @@ -396,6 +401,7 @@ async fn FileSubscriber::ensure_active_file(self : FileSubscriber) -> Unit { self.config.path, "", create_mode=@fs.CreateMode::OpenOrCreate, + permission=FILE_PERMISSION, append=true, ) } @@ -496,7 +502,7 @@ async fn FileSubscriber::gzip_file( destination, mode=@fs.Mode::WriteOnly, create_mode=@fs.CreateMode::CreateOrTruncate, - permission=0o600, + permission=FILE_PERMISSION, ) defer writer.close() let encoder = @gzip.Encoder(writer) diff --git a/src/id.mbt b/src/id.mbt index 41a36c7..353be1b 100644 --- a/src/id.mbt +++ b/src/id.mbt @@ -1,12 +1,14 @@ ///| -// Stretch an 8-byte wall-clock timestamp to 32 bytes using splitmix64-style -// mixing constants. Not cryptographically strong, but sufficient to prevent -// accidental trace-ID collisions between independently-launched processes. -// Two processes starting within the same nanosecond would still collide — -// for adversarial collision resistance, OS entropy would be needed, which -// moonbitlang/core/random does not currently expose. -fn make_seed() -> Bytes { - let t = @env.now() +// Prefer a 32-byte platform entropy source. If the target cannot provide one, +// stretch the millisecond wall clock into the required seed size. The fallback +// is deterministic and non-cryptographic, but keeps unsupported environments +// functional. +fn make_seed(random_bytes : (Int) -> Bytes?, now : () -> UInt64) -> Bytes { + match random_bytes(32) { + Some(seed) if seed.length() == 32 => return seed + _ => () + } + let t = now() let constants : Array[UInt64] = [ 0UL, 0x9e3779b97f4a7c15UL, 0x6c62272e07bb0142UL, 0x94d049bb133111ebUL, ] @@ -21,7 +23,9 @@ fn make_seed() -> Bytes { } ///| -let rng : @random.Rand = @random.Rand::chacha8(seed=make_seed()) +let rng : @random.Rand = @random.Rand::chacha8( + seed=make_seed(@env.rand, @env.now), +) ///| fn to_hex_16(val : UInt64) -> String { diff --git a/src/id_wbtest.mbt b/src/id_wbtest.mbt new file mode 100644 index 0000000..17f2cbc --- /dev/null +++ b/src/id_wbtest.mbt @@ -0,0 +1,31 @@ +///| +test "make seed prefers exact platform entropy" { + let expected = Bytes::makei(32, i => i.to_byte()) + let clock_read : Ref[Bool] = Ref(false) + let actual = make_seed( + size => if size == 32 { Some(expected) } else { None }, + () => { + clock_read.val = true + 0UL + }, + ) + assert_eq(actual, expected) + assert_true(!clock_read.val) +} + +///| +test "make seed falls back deterministically without entropy" { + let make = fn() { make_seed(fn(_) { None }, fn() { 42UL }) } + let first = make() + let second = make() + assert_eq(first.length(), 32) + assert_eq(first, second) + assert_true(first != Bytes::make(32, b'\x00')) +} + +///| +test "make seed rejects platform entropy with the wrong length" { + let seed = make_seed(fn(_) { Some(b"too short") }, fn() { 7UL }) + assert_eq(seed.length(), 32) + assert_true(seed != b"too short") +} diff --git a/src/otlp/transport/client.mbt b/src/otlp/transport/client.mbt index a10d141..481fcfb 100644 --- a/src/otlp/transport/client.mbt +++ b/src/otlp/transport/client.mbt @@ -4,6 +4,31 @@ pub(all) struct AsyncHttpClient { mut is_shut_down : Bool } +///| +#cfg(any(target="native", target="js")) +const MAX_RESPONSE_BODY_BYTES : Int = 64 * 1024 + +///| +#cfg(any(target="native", target="js")) +const RESPONSE_READ_CHUNK_BYTES : Int = 8 * 1024 + +///| +#cfg(any(target="native", target="js")) +async fn read_bounded_response_body( + client : @http.Client, +) -> Result[Bytes, TransportError] { + let body : Array[Byte] = [] + while client.read_some(max_len=RESPONSE_READ_CHUNK_BYTES) is Some(chunk) { + if body.length() + chunk.length() > MAX_RESPONSE_BODY_BYTES { + return Err(Request("HTTP response body exceeds 65536 bytes")) + } + for byte in chunk.iter() { + body.push(byte) + } + } + Ok(Bytes::from_array(body[:])) +} + ///| pub fn async_http_client(timeout_ms? : Int = 5000) -> AsyncHttpClient { let timeout_ms = if timeout_ms < 1 { 1 } else { timeout_ms } @@ -28,12 +53,19 @@ pub impl OtlpHttpClient for AsyncHttpClient with fn post( } @async.with_timeout(self.timeout_ms, () => { let content : &@io.Data = body - let (response, data) = @http.post(url, content, headers~) - Ok({ - status_code: response.code, - body: data.text(), - retry_after_ms: parse_retry_after_ms(response.headers), - }) + let client = @http.post_stream(url, headers~) + defer client.close() + client.write(content) + let response = client.end_request() + match read_bounded_response_body(client) { + Err(err) => Err(err) + Ok(data) => + Ok({ + status_code: response.code, + body: @utf8.decode(data), + retry_after_ms: parse_retry_after_ms(response.headers), + }) + } }) catch { @async.TimeoutError => Err(Timeout("HTTP request timed out after \{self.timeout_ms}ms")) diff --git a/src/otlp/transport/client_test.mbt b/src/otlp/transport/client_test.mbt new file mode 100644 index 0000000..62cf9be --- /dev/null +++ b/src/otlp/transport/client_test.mbt @@ -0,0 +1,132 @@ +///| +#cfg(target="native") +async fn with_response_server( + requests : Ref[Int], + f : async (String) -> Unit, +) -> Unit { + @async.with_task_group(group => { + let server = @http.Server(@socket.Addr::parse("127.0.0.1:0")) + group.add_defer(() => server.close()) + group.spawn_bg(no_wait=true, () => { + server.run_forever((request, request_body, conn) => { + requests.val += 1 + while request_body.read_some() is Some(_) { + + } + conn.send_response(200, "OK", extra_headers={ "Retry-After": "2" }) + match request.path { + "/small" => conn.write("small response") + "/exact" => conn.write("x".repeat(64 * 1024)) + "/oversized" => conn.write("x".repeat(64 * 1024 + 1)) + "/slow" => { + conn.flush() + @async.sleep(100) + conn.write("too late") + } + _ => conn.write("unknown path") + } + }) + }) + f("http://127.0.0.1:\{server.addr.port()}") + }) +} + +///| +#cfg(target="native") +fn expect_http_response( + result : Result[@transport.HttpResponse, @transport.TransportError], +) -> @transport.HttpResponse { + match result { + Ok(response) => response + Err(err) => abort("expected HTTP response, got \{err}") + } +} + +///| +#cfg(target="native") +async test "async HTTP client preserves bounded response bodies" { + @async.with_timeout(2000, () => { + let requests : Ref[Int] = Ref(0) + with_response_server(requests, endpoint => { + let client = @transport.async_http_client(timeout_ms=1000) + let small = expect_http_response( + client.post(endpoint + "/small", "{}", @transport.default_headers()), + ) + assert_eq(small.status_code, 200) + assert_eq(small.body, "small response") + assert_true(small.retry_after_ms == Some(2000)) + let exact = expect_http_response( + client.post(endpoint + "/exact", "{}", @transport.default_headers()), + ) + assert_eq(exact.body.length(), 64 * 1024) + assert_eq(requests.val, 2) + }) + }) +} + +///| +#cfg(target="native") +async test "async HTTP client rejects oversized response and remains usable" { + @async.with_timeout(2000, () => { + let requests : Ref[Int] = Ref(0) + with_response_server(requests, endpoint => { + let client = @transport.async_http_client(timeout_ms=1000) + match + client.post(endpoint + "/oversized", "{}", @transport.default_headers()) { + Err(@transport.Request(message)) => + assert_eq(message, "HTTP response body exceeds 65536 bytes") + Err(err) => abort("unexpected transport error: \{err}") + Ok(_) => abort("expected oversized response to fail") + } + let response = expect_http_response( + client.post(endpoint + "/small", "{}", @transport.default_headers()), + ) + assert_eq(response.body, "small response") + assert_eq(requests.val, 2) + }) + }) +} + +///| +#cfg(target="native") +async test "oversized OTLP response is non-retryable" { + @async.with_timeout(2000, () => { + let requests : Ref[Int] = Ref(0) + with_response_server(requests, endpoint => { + let client = @transport.async_http_client(timeout_ms=1000) + let tx = @transport.transport( + client, + endpoint, + traces_path="/oversized", + max_retries=3, + base_backoff_ms=0, + ) + match tx.export_json(endpoint + "/oversized", "{}") { + Err(@transport.Request(message)) => + assert_eq(message, "HTTP response body exceeds 65536 bytes") + Err(err) => abort("unexpected transport error: \{err}") + Ok(_) => abort("expected oversized response to fail") + } + assert_eq(requests.val, 1) + }) + }) +} + +///| +#cfg(target="native") +async test "response body read obeys HTTP client timeout" { + @async.with_timeout(2000, () => { + let requests : Ref[Int] = Ref(0) + with_response_server(requests, endpoint => { + let client = @transport.async_http_client(timeout_ms=25) + match + client.post(endpoint + "/slow", "{}", @transport.default_headers()) { + Err(@transport.Timeout(message)) => + assert_eq(message, "HTTP request timed out after 25ms") + Err(err) => abort("unexpected transport error: \{err}") + Ok(_) => abort("expected body read timeout") + } + assert_eq(requests.val, 1) + }) + }) +} diff --git a/src/otlp/transport/moon.pkg b/src/otlp/transport/moon.pkg index c86619f..022acbe 100644 --- a/src/otlp/transport/moon.pkg +++ b/src/otlp/transport/moon.pkg @@ -10,6 +10,7 @@ import { import { "moonbitlang/core/json", + "moonbitlang/async/socket", } for "test" warnings = "-unused_package" diff --git a/src/trace_context.mbt b/src/trace_context.mbt index 4c1aa9a..2fb3c18 100644 --- a/src/trace_context.mbt +++ b/src/trace_context.mbt @@ -16,13 +16,17 @@ pub impl Show for TraceContextError with fn output(self, logger) { ///| pub fn TraceContextError::to_string(self : TraceContextError) -> String { match self { - InvalidTraceId(reason) => "invalid trace id: " + reason - InvalidSpanId(reason) => "invalid span id: " + reason - InvalidTraceFlags(reason) => "invalid trace flags: " + reason - InvalidTraceParent(reason) => "invalid traceparent: " + reason + InvalidTraceId(reason) => + "invalid trace id: " + escape_control_chars(reason) + InvalidSpanId(reason) => "invalid span id: " + escape_control_chars(reason) + InvalidTraceFlags(reason) => + "invalid trace flags: " + escape_control_chars(reason) + InvalidTraceParent(reason) => + "invalid traceparent: " + escape_control_chars(reason) UnsupportedTraceParentVersion(version) => - "unsupported traceparent version: " + version - InvalidTraceState(reason) => "invalid tracestate: " + reason + "unsupported traceparent version: " + escape_control_chars(version) + InvalidTraceState(reason) => + "invalid tracestate: " + escape_control_chars(reason) } } @@ -61,6 +65,12 @@ pub struct SpanContext { is_remote : Bool } derive(Debug) +///| +const TRACEPARENT_MAX_LENGTH : Int = 512 + +///| +const TRACESTATE_MAX_LENGTH : Int = 512 + ///| fn is_lower_hex_unit(unit : Int) -> Bool { (unit >= 48 && unit <= 57) || (unit >= 97 && unit <= 102) @@ -496,14 +506,14 @@ fn copy_trace_state(state : TraceState) -> TraceState { pub fn parse_tracestate( header : String, ) -> Result[TraceState, TraceContextError] { - let mut parsed_entries : Array[TraceStateEntry] = [] + if header.length() > TRACESTATE_MAX_LENGTH { + return Result::Err(InvalidTraceState("header exceeds 512 characters")) + } + let parsed_entries : Array[TraceStateEntry] = [] + let seen : Map[String, Bool] = Map([]) let mut non_empty_count = 0 - let parts : Array[String] = header - .split(",") - .map(part => part.to_owned()) - .collect() - for i in 0.. 32 { @@ -514,8 +524,13 @@ pub fn parse_tracestate( let key = key_view.to_owned() let value = value_view.to_owned() match validate_trace_state_entry(key, value) { - Result::Ok(entry) => - parsed_entries = append_trace_state_entry(parsed_entries, entry) + Result::Ok(entry) => { + if seen.contains(key) { + return Result::Err(InvalidTraceState("duplicate key: " + key)) + } + seen.set(key, true) + parsed_entries.push(entry) + } Result::Err(err) => return Result::Err(err) } } @@ -599,32 +614,41 @@ pub fn format_traceparent(ctx : SpanContext) -> String { pub fn parse_traceparent( header : String, ) -> Result[SpanContext, TraceContextError] { - let parts : Array[String] = header - .split("-") - .map(part => part.to_owned()) - .collect() - if parts.length() < 4 { - return Result::Err( - InvalidTraceParent("expected version-trace-id-span-id-flags"), - ) + let length = header.length() + if length < 55 { + return Result::Err(InvalidTraceParent("expected at least 55 characters")) + } + if length > TRACEPARENT_MAX_LENGTH { + return Result::Err(InvalidTraceParent("header exceeds 512 characters")) + } + if header.code_unit_at(2).to_int() != 45 || + header.code_unit_at(35).to_int() != 45 || + header.code_unit_at(52).to_int() != 45 { + return Result::Err(InvalidTraceParent("invalid field separators")) } - let version = parts[0] - if version.length() != 2 || !is_lower_hex_string(version, 2) { + let version = header[0:2].to_owned() + if !is_lower_hex_string(version, 2) { return Result::Err(InvalidTraceParent("invalid version")) } if version == "ff" { return Result::Err(InvalidTraceParent("version ff is reserved")) } - if version == "00" && parts.length() != 4 { + if version == "00" && length != 55 { return Result::Err( InvalidTraceParent("version 00 cannot contain extra fields"), ) } - match TraceId::parse(parts[1]) { + if version != "00" && length > 55 && header.code_unit_at(55).to_int() != 45 { + return Result::Err(InvalidTraceParent("invalid extension separator")) + } + let trace_id = header[3:35].to_owned() + let span_id = header[36:52].to_owned() + let trace_flags = header[53:55].to_owned() + match TraceId::parse(trace_id) { Result::Ok(tid) => - match SpanId::parse(parts[2]) { + match SpanId::parse(span_id) { Result::Ok(sid) => - match TraceFlags::from_hex(parts[3]) { + match TraceFlags::from_hex(trace_flags) { Result::Ok(flags) => Result::Ok( span_context( diff --git a/src/trace_context_test.mbt b/src/trace_context_test.mbt index 13cec3b..54e4475 100644 --- a/src/trace_context_test.mbt +++ b/src/trace_context_test.mbt @@ -113,6 +113,17 @@ fn make_tracestate_header(count : Int) -> String { buf.to_string() } +///| +test "trace context errors escape control characters" { + let error = @moontrace.TraceContextError::InvalidTraceState( + "bad\nkey\r\t\u001b\u007f\u009f", + ) + assert_eq( + error.to_string(), + "invalid tracestate: bad\\nkey\\r\\t\\u{001b}\\u{007f}\\u{009f}", + ) +} + ///| test "traceparent parses and formats sampled context" { let header = "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01" @@ -169,6 +180,36 @@ test "traceparent parses higher versions without extra fields" { assert_eq(ctx.trace_flags_int(), 0) } +///| +test "traceparent bounds future-version extensions" { + let base = "01-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-00" + let max_header = base + "-" + "x".repeat(456) + assert_eq(max_header.length(), 512) + let ctx = expect_context(@moontrace.parse_traceparent(max_header)) + assert_eq(ctx.trace_id, "4bf92f3577b34da6a3ce929d0e0e4736") + assert_context_err(@moontrace.parse_traceparent(max_header + "x")) + assert_context_err(@moontrace.parse_traceparent(base + "x")) +} + +///| +test "traceparent validates fixed field separators" { + assert_context_err( + @moontrace.parse_traceparent( + "00x4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01", + ), + ) + assert_context_err( + @moontrace.parse_traceparent( + "00-4bf92f3577b34da6a3ce929d0e0e4736x00f067aa0ba902b7-01", + ), + ) + assert_context_err( + @moontrace.parse_traceparent( + "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7x01", + ), + ) +} + ///| test "invalid traceparent headers are structured errors" { assert_context_err(@moontrace.parse_traceparent("00-too-short")) @@ -224,16 +265,14 @@ test "id and flag constructors reject invalid values" { } ///| -test "tracestate parses whitespace and duplicate keys with last write winning" { +test "tracestate parses whitespace and preserves wire order" { let state = expect_state( - @moontrace.parse_tracestate( - " rojo=00f067aa0ba902b7,\tcongo=t61rcWkgMzE ,rojo=latest", - ), + @moontrace.parse_tracestate(" rojo=00f067aa0ba902b7,\tcongo=t61rcWkgMzE "), ) assert_eq(state.len(), 2) - @debug.assert_eq(state.get("rojo"), Some("latest")) + @debug.assert_eq(state.get("rojo"), Some("00f067aa0ba902b7")) @debug.assert_eq(state.get("congo"), Some("t61rcWkgMzE")) - assert_eq(state.to_header(), "congo=t61rcWkgMzE,rojo=latest") + assert_eq(state.to_header(), "rojo=00f067aa0ba902b7,congo=t61rcWkgMzE") let reparsed = expect_state(@moontrace.parse_tracestate(state.to_header())) assert_eq(reparsed.to_header(), state.to_header()) @@ -258,6 +297,7 @@ test "tracestate constructors preserve order and validate entries" { ///| test "invalid tracestate headers are structured errors" { assert_state_err(@moontrace.parse_tracestate(make_tracestate_header(33))) + assert_state_err(@moontrace.parse_tracestate("rojo=first,rojo=second")) assert_state_err(@moontrace.parse_tracestate("Rojo=value")) assert_state_err(@moontrace.parse_tracestate("ro jo=value")) assert_state_err(@moontrace.parse_tracestate("rojo=value=bad")) @@ -265,6 +305,15 @@ test "invalid tracestate headers are structured errors" { assert_entry_err(@moontrace.trace_state_entry("rojo", "")) } +///| +test "tracestate bounds input before parsing" { + let max_header = "a".repeat(255) + "=" + "v".repeat(256) + assert_eq(max_header.length(), 512) + let state = expect_state(@moontrace.parse_tracestate(max_header)) + assert_eq(state.len(), 1) + assert_state_err(@moontrace.parse_tracestate(max_header + "x")) +} + ///| test "empty and whitespace-only tracestate parse as empty" { assert_true(expect_state(@moontrace.parse_tracestate("")).is_empty()) @@ -287,6 +336,22 @@ test "span context drops malformed tracestate but keeps valid traceparent" { assert_true(ctx.trace_state.is_empty()) } +///| +test "span context drops duplicate and oversized tracestate" { + let traceparent = "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01" + let duplicate = expect_context( + @moontrace.parse_span_context( + traceparent, + tracestate="rojo=first,rojo=second", + ), + ) + assert_true(duplicate.trace_state.is_empty()) + let oversized = expect_context( + @moontrace.parse_span_context(traceparent, tracestate="a".repeat(513)), + ) + assert_true(oversized.trace_state.is_empty()) +} + ///| test "remote context creates child span and preserves propagation data" { let remote = expect_context(