Skip to content

fix: TOML-escape iris token and base URL in docker entrypoint (COD-470) - #15

Merged
shivros merged 2 commits into
mainfrom
runner/COD-467-toml-token-escaping
Sep 8, 2026
Merged

fix: TOML-escape iris token and base URL in docker entrypoint (COD-470)#15
shivros merged 2 commits into
mainfrom
runner/COD-467-toml-token-escaping

Conversation

@shivros

@shivros shivros commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a TOML escaping defect in docker-entrypoint.sh discovered while executing the deferred COD-467 Docker materialization acceptance check against merged main (a868849).

The defect: the entrypoint escaped RITE_IRIS_API_TOKEN SQL-style (''') inside a TOML literal string. TOML literal strings cannot contain single quotes and have no escape mechanism — a token like o'brien generated an unparseable rite.toml and a crashing container. base_url was also interpolated unescaped into a double-quoted basic string, breaking on " or \, and raw control characters are invalid in basic strings.

Reproduction on merged main:

RITE_IRIS_API_TOKEN=o'brien ./docker-entrypoint.sh true
# → api_token = 'o''brien'   ← invalid TOML
# tomllib.TOMLDecodeError: Expected newline or end of document (line 6)

Fix

  • toml_escape() emits TOML basic strings: \\ and \" escaped, newline → \n, tab left raw (legal), every other control byte (0x01–0x1F) plus DEL → \uXXXX — applied to both api_token and base_url.
  • GNU sed -z (NUL-delimited records; env values cannot contain NUL) so embedded newlines are matchable without a sentinel byte.
  • Runtime image is Debian bookworm-slim (GNU sed), script stays POSIX-sh.

Test coverage (new: crates/rite-server/tests/docker_entrypoint.rs)

Runs the real entrypoint script and parses its output through rite's production parser (rite_server::load_config), asserting exact round-trip:

  • plain / token-unset / blank base_url (no [sources.iris] emitted)
  • o'brien, to"ken, back\slash, mixed delimiters
  • embedded newline, CRLF, tab
  • full sweep of every control byte 0x01–0x1F except tab/newline, plus DEL, in the token
  • control bytes + quotes + backslash in base_url
  • token value never echoed to stdout/stderr

Review panel

Both seats BLOCKed the first version (correctly — findings below were fixed in this head):

  • STX (0x02) was used as a newline sentinel → collided with real STX bytes (both reviewers, MAJOR) → fixed: sed -z removes the sentinel entirely; STX regression test added.
  • Only 0x01 of the control range was escaped; 0x03–0x1F and DEL leaked raw → invalid TOML (both reviewers, MAJOR) → fixed: complete \uXXXX table; full-sweep test added.
  • Tests missed control/newline coverage in base_url (Reviewer A MINOR) → fixed: dedicated test.
  • generated_token_is_never_logged didn't assert exit success (Reviewer A MINOR) → fixed.
  • Round-trip failure messages dumped the generated TOML — potential token leak into test output (Reviewer A MINOR) → fixed: length-only messages.
  • Invalid-UTF-8 env values (Reviewer A MAJOR): not fixable at this boundary — Rust's own env access is UTF-8-only (std::env::var fails on non-UTF-8), and rite's read_to_string startup path already requires UTF-8 configs, so non-UTF-8 values fail loudly at the same place they would have before. Documented in the script comment instead.

Verification

  • cargo build --all-targets
  • cargo test --all-targets ✅ (36 tests / 6 suites, incl. 14 new)
  • cargo clippy --all-targets -- -D warnings
  • cargo fmt --all -- --check
  • cargo run -p rite-codegen -- check
  • sh -n docker-entrypoint.sh
  • Direct materialization checks against merged main (defect) and this branch (fix) with tomllib + od byte-level inspection: raw SOH/STX/newline never appear on disk, only \uXXXX/\n escapes.

Closes COD-470 (discovered during the COD-467 acceptance check).

The entrypoint escaped RITE_IRIS_API_TOKEN SQL-style inside a TOML
literal string. TOML literal strings cannot contain single quotes,
so a token like o'brien produced an unparseable rite.toml and a
crashing container. base_url was also interpolated unescaped into a
basic string, breaking on quotes/backslashes.

Emit TOML basic strings with backslash and double-quote escaped,
newline as \n, tab left raw (legal), and every other control byte
plus DEL as \uXXXX. Use GNU sed -z so embedded newlines are
matchable without a sentinel byte. Add an integration test that
runs the real entrypoint and parses its output through rite's
production load_config: quote/backslash/control/multiline round-
trips for both fields, full 0x01-0x1F/0x7F sweep, STX sentinel
regression, token absence, blank base_url, and token-never-logged.

Closes COD-470.
@shivros

shivros commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Automated Review Panel

Dual review of e7d7c22 vs main. Both seats BLOCKed the first head (7a79ffe) — findings applied and re-gated; this head is the corrected version.

Reviewer A — BLOCK → resolved

# Sev Finding Resolution
1 MAJOR tr '\n' '\002' sentinel collides with real STX bytes → emitted as \n, corrupts round-trip Fixed: GNU sed -z (NUL-delimited records; env values can't contain NUL) — no sentinel byte. STX regression test added
2 MAJOR Invalid-UTF-8 env bytes copied unchanged; read_to_string startup path can't load them Documented, not fixable here: Rust's std::env::var is UTF-8-only and rite's startup already requires UTF-8 configs — non-UTF-8 values fail loudly at the same boundary as before this change. No behavior regression
3 MINOR Coverage missed STX collision, remaining controls/DEL, invalid UTF-8, control/newline in base_url Fixed: full 0x01–0x1F (+DEL) sweep test, STX test, control_chars_in_base_url_round_trip test
4 MINOR generated_token_is_never_logged didn't assert exit success Fixed: asserts status.success() first
5 MINOR Round-trip failure messages interpolated the generated TOML (token could leak to test output) Fixed: length-only failure messages
scratch_dir PID + atomic counter: race-free No action needed

Reviewer B — BLOCK → resolved

# Sev Finding Resolution
1 MAJOR STX sentinel collision (converges with A#1) Fixed as above
2 MAJOR Only 0x01 escaped; raw 0x03–0x1F + DEL survive into basic strings → invalid TOML (verified 0x03, tab, 0x1f, 0x7f) Fixed: complete \uXXXX escape table (tab left raw — legal in basic strings); sweep test proves all of them
3 MINOR "BOTH fields" requirement not regression-protected (only token had control/newline coverage) Fixed: dedicated base_url test with newline, quote, backslash, SOH
sed \x01 pattern behavior identical under LC_ALL=C and UTF-8 locales No action needed

Panel outcome: all five blocking/major findings resolved in e7d7c22 (the UTF-8 item is a documented boundary, not a regression). Gates after fixes: build ✅, cargo test --all-targets 36/36 ✅ (14 new), clippy -D warnings ✅, fmt ✅, codegen check ✅, sh -n ✅.

Note: this panel ran on glm-5.1 children (cron pin differed from the usual gpt-5.6-terra runner seat). Both seats produced reproducible, verified findings; Reviewer A re-ran the failing materializations manually before blocking.

dash's builtin echo interprets backslash escapes in its arguments,
so the \\n / \\b / \\f / \\r sequences produced by toml_escape were
mangled when /bin/sh is dash — which it is on the production
runtime image (Debian bookworm). Emit lines with printf, whose
format string is the only escape-processed part. Verified under the
actual production shell by running the script inside
debian:bookworm-slim (dash + GNU sed 4.9).
@shivros

shivros commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Review Panel Addendum — CI-caught defect fixed in 2bb0f57

CI on e7d7c22 failed the new entrypoint suite with a pattern invisible on the authoring host: backslash/newline/CRLF/control-sweep tests failed while quote/STX/plain tests passed.

Root cause: /bin/sh on GitHub's runner — and, more importantly, on the production runtime image (Debian bookworm) — is dash, whose builtin echo interprets \n, \\, \b, \f, \r in its arguments. The escaper produced correct TOML escape sequences; echo then mangled them (\\n in the token value became a literal newline on disk). The authoring host's /bin/sh is bash-as-sh, which doesn't interpret escapes — local gates were green while production would have been broken. The original entrypoint (echo with simple values) never hit this because unescaped values contain no backslash sequences.

Fix: emit all generated lines with printf (only the format string is escape-processed). 8 lines changed, no behavior change otherwise.

Verification:

  • The fixed script was executed inside debian:bookworm-slim itself (the production base image): /bin/sh → dash, GNU sed 4.9. With RITE_IRIS_BASE_URL='https://x"a\b' and a multi-line quote/backslash token, the generated config is exactly base_url = "https://x\"a\\b" / api_token = "q'q\"b\\s\ntwo" — byte-correct.
  • Local full gates re-passed (14/14 entrypoint tests, build, clippy, fmt, codegen).
  • CI on 2bb0f57: test: SUCCESS.

This is the second environment-specific shell defect this PR has caught (after the TOML literal-string bug on merged main) — both are now regression-locked by the 14-test suite running the real script through the production parser.

@shivros

shivros commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Auto-Merge Gate — eligible (0.94)

Rationale: COD-470's focused two-file fix replaces invalid TOML literal-string handling with escaped TOML basic strings for both Iris fields. The regression suite executes the real entrypoint through Rite's production parser, including quote, backslash, CRLF/newline, control-byte, DEL, and no-token-leak cases.

Checks observed: GitHub CI test succeeded; local gate re-ran cargo test -p rite-server --test docker_entrypoint (14 passed) and cargo fmt --all -- --check.

Scope limits: No deployment/cutover, credentials, or destructive behavior is included. Linked Linear ticket has no manual-review marker and its acceptance criteria are satisfied by this PR.

@shivros
shivros merged commit b01b7aa into main Sep 8, 2026
1 check passed
@shivros
shivros deleted the runner/COD-467-toml-token-escaping branch September 8, 2026 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant