Env-var expansion runs before TOML parsing, so `${VAR}` in a comment breaks startup #31

New issue

Closed

opened 2026-09-28 16:16:41 +00:00 by james.oates · 0 comments

james.oates commented

2026-09-28 16:16:41 +00:00

Owner

Problem

routes_config::expand_env_refs scans the raw file text before the TOML is parsed, so it has no concept of a comment. Any ${VAR} anywhere in the config demands that ER_VAR be set, or startup fails — including a placeholder inside a comment, or on a config line the operator has commented out.

The practical consequences:

The shipped edge-router.example.toml could not be loaded at all. Its TLS section documented the feature with Use ${VAR_NAME} to inject secrets… and a commented-out # cloudflare_api_token = "${CF_TOKEN}". Starting the router against the example we ship failed with:
```
Environment variable 'ER_VAR_NAME' is not set (referenced as '${VAR_NAME}' in config)
```
Commenting a line out can break a running deployment. Disable a service by commenting out its cloudflare_api_token and the router stops starting, with an error pointing at a line that is no longer active config.

Found by the shipped_example_config_parses test added in 2f6e830 — nothing had ever loaded that file, so it had drifted into being unloadable unnoticed.

Current state

Worked around in 2f6e830 by rewording the example's comments to describe the syntax in words instead of showing it, and documenting the trap in CLAUDE.md. That keeps the example loadable but makes those comments worse — their whole job was to show the ${VAR} syntax, and they now can't.

The test pins the example, so this can't silently regress.

Proposed fix

Expand after TOML parsing rather than before: parse to toml::Value, walk the string leaves substituting placeholders, then deserialize into RawRouteConfig. That is comment-safe and #-inside-a-string-safe by construction, rather than by a heuristic that tries to guess where comments start.

Deliberately not done as a comment-stripping pass over the raw text: password = "a#b" is a valid TOML value and any regex-ish approach either mangles it or needs to tokenize TOML properly anyway.

Once fixed:

Restore the ${VAR} examples in edge-router.example.toml's TLS section.
Drop the landmine note from CLAUDE.md's routes_config.rs row.
Keep shipped_example_config_parses.

Also worth checking while in there

The error message had doubled braces and rendered as a literal '${{var_name}}', naming nothing useful. Fixed in 2f6e830, with an assertion in test_expand_missing_var_returns_error so it stays fixed — no action needed, just context if you're reading that code.

## Problem `routes_config::expand_env_refs` scans the **raw file text before the TOML is parsed**, so it has no concept of a comment. Any `${VAR}` anywhere in the config demands that `ER_VAR` be set, or startup fails — including a placeholder inside a comment, or on a config line the operator has commented out. The practical consequences: - **The shipped `edge-router.example.toml` could not be loaded at all.** Its TLS section documented the feature with `Use ${VAR_NAME} to inject secrets…` and a commented-out `# cloudflare_api_token = "${CF_TOKEN}"`. Starting the router against the example we ship failed with: ``` Environment variable 'ER_VAR_NAME' is not set (referenced as '${VAR_NAME}' in config) ``` - **Commenting a line out can break a running deployment.** Disable a service by commenting out its `cloudflare_api_token` and the router stops starting, with an error pointing at a line that is no longer active config. Found by the `shipped_example_config_parses` test added in 2f6e830 — nothing had ever loaded that file, so it had drifted into being unloadable unnoticed. ## Current state Worked around in 2f6e830 by rewording the example's comments to describe the syntax in words instead of showing it, and documenting the trap in `CLAUDE.md`. That keeps the example loadable but makes those comments worse — their whole job was to show the `${VAR}` syntax, and they now can't. The test pins the example, so this can't silently regress. ## Proposed fix Expand **after** TOML parsing rather than before: parse to `toml::Value`, walk the string leaves substituting placeholders, then deserialize into `RawRouteConfig`. That is comment-safe and `#`-inside-a-string-safe by construction, rather than by a heuristic that tries to guess where comments start. Deliberately **not** done as a comment-stripping pass over the raw text: `password = "a#b"` is a valid TOML value and any regex-ish approach either mangles it or needs to tokenize TOML properly anyway. Once fixed: - Restore the `${VAR}` examples in `edge-router.example.toml`'s TLS section. - Drop the landmine note from `CLAUDE.md`'s `routes_config.rs` row. - Keep `shipped_example_config_parses`. ## Also worth checking while in there The error message had doubled braces and rendered as a literal `'${{var_name}}'`, naming nothing useful. Fixed in 2f6e830, with an assertion in `test_expand_missing_var_returns_error` so it stays fixed — no action needed, just context if you're reading that code.

james.oates referenced this issue from a commit

2026-10-05 17:48:51 +00:00

fix(config): expand env placeholders after parsing, not before

james.oates closed this issue

2026-10-05 17:48:51 +00:00

james.oates referenced this issue

2026-10-05 17:59:36 +00:00

edge-router.example.toml can't be run directly: ${...} expansion runs over comments #15

james.oates referenced this issue from a commit

2026-10-21 09:36:58 +00:00

docs: strip CLAUDE.md of anything the code already states