Document the config template and harden config validation

[aram356]

Closes #869. Closes #871.

Config-surface cleanup in four focused commits: document the app-config template, close deploy-validation gaps that let placeholder values through, and lean the config structs onto the validator crate's built-in validators instead of hand-rolled functions.

---

1 · Document trusted-server.example.toml and restore supported sections

Reworks the source-controlled app-config template (used by ts config init, embedded into the Cloudflare/Spin adapters via include_str!, and text-patched by ts audit).

• Required minimum stays active + documented: [[handlers]] (admin), [publisher], [ec].
• Every optional section/integration is commented out with a one-line description.
• Restores sections that still map to live config structs but had been dropped: [tester_cookie], [rewrite], [consent], [image_optimizer], [tinybird], [integrations.osano], publisher.max_buffered_body_bytes, sourcepoint.auth_cookie_name, DataDome protection fields, Prebid override rules.
• Keeps gpt/didomi/datadome/google_tag_manager as active enabled = false stubs so ts audit can flip them in place. All hosts are example.com.

2 · Reject fail-open placeholder config values at deploy validation

ts config validate/push route through TrustedServerAppConfig::validate → validate_settings_for_deploy. That path now rejects template placeholders that previously validated OK and only failed later at runtime:

• publisher.domain / cookie_domain / origin_url left at the example.com template defaults.
• request_signing.config_store_id / secret_store_id when the block is enabled (empty or the <management-...> placeholders).
• integrations.aps.pub_id when APS is enabled — non-empty via the built-in length(min = 1) validator, plus a custom validator for the reserved your-aps-publisher-id placeholder.

Why some checks use #[validate] and some don't: APS validates lazily through get_typed (enabled-gated), so an attribute fires only when APS is on. Publisher / request_signing stay in the deploy-only reject_placeholder_secrets — Publisher::validate() runs at parse time on the embedded example.com template (an attribute would reject the template itself), and request_signing rejection must be gated on the sibling enabled flag. This mirrors how the existing placeholder-secret rejection already works.

3 · Replace hand-rolled GTM/Prebid validators with the built-in regex validator

• GTM container_id and Prebid external_bundle_sha256 used custom validator functions that just ran a regex / hex-format check. Both now use #[validate(regex(...))] (validator 0.20 ships regex support + an AsRegex impl for LazyLock<Regex>); operator-facing messages preserved via message.
• Added a test per validator asserting the accepted and rejected inputs.

4 · Bound timeout_ms with the built-in range validator

testlight/datadome already constrain their timeouts via range; aps, prebid, adserver_mock, and auction did not. Added range(min = 1, max = 60000) (catches 0 and absurd values) for consistency. AuctionConfig now derives Validate and is wired via #[validate(nested)]; all existing configs use 500–2000 ms.

---

Intentionally left imperative

Tinybird, image-optimizer, proxy asset-route, creative-opportunities, and consent validators were not converted to attributes. Their logic interleaves normalize() mutation, charset checks, enabled-gating, cross-field rules, and contextual per-item error messages (e.g. "slot X must have positive width") with the one or two convertible checks. Converting them would fragment validation across #[validate] attrs and prepare_runtime/validate_runtime, cascade Validate derives up the parent chains, and downgrade contextual errors to generic ones — a regression, not a win. (consent additionally clamps rather than rejects, so a range attribute would change its behavior.) That code isn't reimplementing a built-in; it does things built-ins can't express.

Verification

• cargo test -p trusted-server-core --lib → 1631 pass, 0 fail (incl. new deploy-validation, GTM/Prebid regex, and timeout-range tests).
• cargo test --package trusted-server-cli … audit → pass (ts audit still patches the template).
• Settings::from_toml on the template → pass (deploy rejection is separate from parse).
• cargo clippy -p trusted-server-core --lib with -D warnings → clean.
• Integration fixture timeout values confirmed within range.

Not yet run: the full clippy-fastly / integration-parity CI gates — the clippy-fastly alias hits a pre-existing local env error compiling the JS build script, unrelated to these changes.

---

Upgrade note

A present [request_signing] block now requires real store IDs even when enabled = false. The key rotate/deactivate admin routes are registered unconditionally and read these IDs regardless of enabled, so deploy validation now rejects placeholder or empty store IDs whenever the block exists. Operators who declared [request_signing] with enabled = false and left placeholder IDs will get a ts config validate/push failure naming request_signing.config_store_id / secret_store_id. To keep signing off with no store IDs, leave the entire [request_signing] block commented out (the template documents this).

[ChristianPavilonis]

Summary

Reviewed the config hardening and template changes. I found one high-severity configuration compatibility regression and three medium validation/template gaps; details are inline.

[prk-Jr]

Summary

The configuration-template and validation hardening is well scoped, and all CI checks pass. One enabled APS configuration edge case still bypasses the new publisher-ID validation.

Blocking

🔧 wrench

• Reject whitespace-only APS publisher IDs: the built-in length validator accepts non-empty whitespace, while the custom validator trims only for placeholder comparison (crates/trusted-server-core/src/integrations/aps.rs:204).

CI Status

• fmt: PASS
• clippy / cargo checks: PASS
• rust tests: PASS
• js tests: PASS
• integration tests: PASS

[ChristianPavilonis]

Summary

The validator refactors and previously requested fixes are generally sound. I found one high-impact silent configuration regression and three medium-severity validation/documentation gaps; details and concrete fixes are included inline.

[ChristianPavilonis]

Documented environment overrides are silently ignored for the now-commented creative-opportunities config

This PR comments out [creative_opportunities] and auction_timeout_ms, but lines 216–219 still advertise TRUSTED_SERVER__CREATIVE_OPPORTUNITIES__AUCTION_TIMEOUT_MS and ...__SLOT overrides. The typed EdgeZero loader only overlays existing scalar leaves; it does not create absent tables/fields or arrays. The timeout override worked with the previous active template but is now silently ignored.

Operators can therefore validate and push a config believing auction timeout or slots were applied when the creative-opportunities configuration remains absent. I confirmed that an invalid override for existing publisher.domain is observed and rejected, while an invalid CREATIVE_OPPORTUNITIES__AUCTION_TIMEOUT_MS override validates successfully when the table or leaf is absent; the documented ...__SLOT override is likewise ignored.

Please either preserve the seed table/scalar leaves needed by the overlay, add typed-loader support for absent tables and arrays, or remove/qualify these instructions and direct operators to edit private TOML. A CLI test should use these exact variables and assert the resulting typed values.

[aram356]

Fixed in e4e0e8f, together with prk-Jr's overlapping comment. The header now states the overlay only replaces scalar leaves that already exist in the parsed TOML (commented/absent keys silently ignored; arrays and tables must be edited in TOML). The ...__AUCTION_TIMEOUT_MS inline note and the ...__SLOT array example are dropped/requalified to point at editing the [[creative_opportunities.slot]] blocks directly. Re your test suggestion: I added documented_integration_blocks_validate_when_uncommented for the copy-and-edit blocks; I did not add a CLI test asserting the timeout env var applies, because the correct behavior here is that it is NOT applied for a commented-out leaf - the fix is the doc caveat, not restoring an active leaf.

[ChristianPavilonis]

A semantically unchanged placeholder origin bypasses the new deploy check

is_placeholder_origin_url compares the complete raw string, so https://origin.example.com:443 passes even though it resolves to the same reserved template host as https://origin.example.com. I verified that ts config validate accepts this variant in a template-derived config. This defeats the new fail-fast protection and can publish a configuration that proxies production traffic to a domain the operator cannot own.

Please parse the URL and reject based on normalized host_str() matching origin.example.com, independent of explicit default ports or equivalent spellings, and add default-port regression coverage.

[aram356]

Fixed in e4e0e8f. is_placeholder_origin_url now parses the URL and compares host_str() against the reserved host (origin.example.com), so :443, a trailing slash, http://, and mixed case are all rejected. Regression coverage in is_placeholder_origin_url_rejects_equivalent_spellings_of_reserved_host (covers all those variants) plus an ..._accepts_non_placeholder case.

[ChristianPavilonis]

Identifier validation trims for comparison but forwards the untrimmed values

APS validation checks pub_id.trim(), but the request uses the original value at aps.rs:426. Request-signing validation has the same pattern at settings.rs:701, while request_signing/endpoints.rs:205-206 forwards the original store IDs. A config containing pub_id = " 5128 " or store IDs such as " 01GCFG " therefore passes deploy validation while retaining the whitespace sent to the APS or management API boundary. This is distinct from the previously fixed whitespace-only APS case.

Please reject surrounding whitespace (value != value.trim()) or normalize these identifiers during deserialization/finalization. Tests should ensure that accepted values are exactly what reaches APS payloads and KeyRotationManager.

[aram356]

Fixed in e4e0e8f. APS validate_aps_pub_id now rejects a value that differs from its trimmed form (aps_pub_id_whitespace), and request-signing store IDs go through a new RequestSigning::is_unusable_store_id (placeholder/empty OR surrounding whitespace) in reject_placeholder_secrets. Since deploy validation now rejects padded values, an accepted id has no surrounding whitespace and is forwarded verbatim, so accepted == what reaches the APS payload / KeyRotationManager. Tests: validate_aps_pub_id_accepts_valid_and_rejects_bad_values, is_unusable_store_id_rejects_placeholders_empty_and_padded_values, and deploy-level deploy_validation_rejects_blank_or_padded_aps_pub_id / deploy_validation_rejects_padded_request_signing_store_ids. I went with reject rather than silent normalization to stay consistent with the PR's fail-fast theme.

[ChristianPavilonis]

The restored Tinybird example omits the fields required to enable it

The documented block exposes only enabled, but enabled Tinybird requires a non-empty api_host; its store, dataset, and token-key settings are also operationally relevant. Uncommenting this section and setting enabled = true therefore immediately fails validation, so the restored template does not provide enough guidance to configure the advertised feature.

Please show api_host as required, including its host-only format, and document the store/dataset/token fields and defaults. A template-derived validation test for enabled Tinybird would prevent this from regressing.

[aram356]

Fixed in e4e0e8f. The [tinybird] block now shows api_host as required (host-only format) plus the secret_store / auction_dataset / auction_token_secret fields with their defaults. documented_tinybird_block_validates_when_uncommented uncomments the block from the actual template and asserts it parses and passes the validate_tinybird_api_host check.

[prk-Jr]

Summary

Re-review at 0c2b38cd1. My earlier blocking finding is fixed: validate_aps_pub_id now trims before the placeholder comparison and deploy_validation_rejects_blank_aps_pub_id covers both empty and whitespace-only through enabled deploy validation. The get_typed reordering, the request-signing always-on check, and the sourcepoint cdn_origin removal are all in place. CI is green across all 19 checks.

Two findings remain that concern the template as documentation: a documented env-override mechanism that now silently does nothing, and four documented blocks that fail validation if uncommented as written.

Blocking

🔧 wrench

• Documented env-var overrides are silently ignored for the leaves this PR commented out (trusted-server.example.toml:216, 218-219, 20-23) — see inline comment.
• Four documented blocks cannot validate as written (trusted-server.example.toml:113-116, 228-229, 317-318, 326) — see inline comment on the permutive block.

Non-blocking

📌 out of scope

• Gate key-management routes on request_signing.enabled: rejecting placeholder store IDs while the block is disabled is the right call for this PR, but the underlying asymmetry is that rotate/deactivate are registered unconditionally and read the IDs without consulting enabled. Worth a follow-up issue so the config-side workaround does not become the permanent answer.

👍 praise

• Removing the example.com origin overrides is a fail-open fix, not just documentation: main shipped active sdk_origin = "https://sdk.example.com" / api_origin for didomi and datadome and script_url = "https://ads.example.com/gpt.js" for gpt. Since all three fields have real vendor defaults, ts audit flipping enabled = true on those sections previously produced a config that proxied to example.com. With the overrides gone the defaults apply. The PR description does not claim this, but it is the most valuable behavioral consequence of the template rework.
• GTM-REPLACE-ME is fail-closed by construction: it cannot match ^GTM-[A-Z0-9]{4,20}$ (unlike the previous GTM-EXAMPLE, which validated fine), and audit/mod.rs:313-330 only flips enabled when extract_gtm_container_id actually returned an id — otherwise the integration goes to manual review. Nice pairing.
• The get_typed reordering is covered by a test that would actually fail without it: deploy_validation_skips_field_validation_for_integrations_with_omitted_enabled uses an adserver_mock endpoint that parses but fails the url validator, so the test cannot pass vacuously.

CI Status

• fmt: PASS
• clippy / cargo check (cloudflare native + wasm, spin native + wasm): PASS
• rust tests: PASS (fastly, axum, cloudflare, spin, ts CLI, cross-adapter parity)
• js tests (vitest) / format-typescript / format-docs: PASS
• integration + browser + Fastly EC lifecycle: PASS
• CodeQL: PASS

[prk-Jr]

🔧 wrench — This override is now silently ignored, and the array form below can never work.

Upstream edgezero-core/src/app_config.rs (apply_env_overlay) states it plainly: "The overlay only overrides keys that already exist in the parsed tree (the existing TOML value's type drives coercion of the env string)." And coerce_env_value rejects Value::Array | Value::Table with env overlay supports scalar leaves only.

So:

• auction_timeout_ms = 500 was an active leaf on main, which is why TRUSTED_SERVER__CREATIVE_OPPORTUNITIES__AUCTION_TIMEOUT_MS worked. Commented out, the env var is dropped with no diagnostic.
• The TRUSTED_SERVER__CREATIVE_OPPORTUNITIES__SLOT='[{...}]' form on line 219 cannot work in either state: absent leaf → ignored, present array leaf → EnvOverlay error.
• The same silent drop now applies to every leaf this PR commented out that an operator or the dev loop might override: auction.timeout_ms, integrations.prebid.server_url / timeout_ms, integrations.aps.pub_id, request_signing.*, debug.*. [proxy] disappearing also takes .env.dev's documented TRUSTED_SERVER__PROXY__CERTIFICATE_CHECK with it.

The header at lines 20-23 promises the mechanism without the caveat, which is what makes this land as a footgun rather than a preference: an operator can validate and push believing an override applied.

Fix — three small edits:

1. Add the caveat to the header block:

# `TRUSTED_SERVER__` env vars can override values when the `ts` CLI builds and
# validates this config for push (e.g. TRUSTED_SERVER__PUBLISHER__DOMAIN=...;
# nested keys use `__`). The overlay only replaces SCALAR leaves that already
# exist in the TOML — a commented-out key is silently ignored, and arrays or
# tables must be edited in TOML directly. The deployed runtime reads the pushed
# config blob, so these env vars do not change live behavior on their own.

2. Drop the ...__SLOT example on lines 218-219 (or requalify it as "edit the [[creative_opportunities.slot]] blocks below").
3. Keep an active seed leaf for anything the local dev loop overrides today.

This overlaps @ChristianPavilonis's open comment on the same lines; the upstream quote and the wider set of affected leaves are the additions.

[aram356]

Fixed in e4e0e8f. Added the scalar-leaves-only caveat to the header block (commented/absent keys silently ignored; arrays and tables must be edited in TOML directly) and requalified the ...__SLOT example to point at editing the [[creative_opportunities.slot]] blocks. On the seed-leaf point: .env.dev's only active overrides are PUBLISHER__ORIGIN_URL (present, works) and the synthetic stores; PROXY__CERTIFICATE_CHECK is itself commented in .env.dev and proxy.certificate_check was already commented on main, so that overlay was inert before this PR too - not a regression I introduced, and the header caveat now documents why.

[prk-Jr]

🔧 wrench — Uncommenting this block as written fails validation: PermutiveConfig marks both organization_id and workspace_id #[validate(length(min = 1))], so enabled = true with "" is rejected. Three more documented blocks share the defect:

• [integrations.lockr] (line 326): app_id = "", also length(min = 1).
• [tinybird] (lines 228-229): only enabled is shown, but prepare_runtime → validate_tinybird_api_host requires a non-empty host-only api_host when enabled, and secret_store / auction_dataset / auction_token_secret are operationally relevant.
• [request_signing] (lines 113-116): documented with enabled = false plus both <management-...> placeholders — which this PR's own always-present check now rejects. Uncommenting the block in order to explicitly disable signing fails ts config validate.

This is the same class as the sourcepoint cdn_origin finding already fixed in this PR: a block that reads as copy-and-edit but cannot be pushed as shown. In a PR whose stated purpose is making the template the operator-facing documentation, that matters more than it normally would.

Fix — use self-describing fill-me-in values (the APS block already does this well with your-aps-publisher-id) and mark required-when-enabled fields:

# Permutive DMP. `organization_id` and `workspace_id` required when enabled.
# [integrations.permutive]
# enabled = true
# organization_id = "your-permutive-organization-id"   # required
# workspace_id = "your-permutive-workspace-id"         # required

For [request_signing], either comment out the two id lines or state that a present block requires real store IDs even when disabled.

Worth a regression test that uncomments each documented block individually and asserts parse + field validation, excluding the deliberately invalid placeholders (ec.passphrase, handlers[].password, GTM container_id, APS pub_id). That test is what would keep this from drifting again.

(The tinybird half of this overlaps @ChristianPavilonis's open comment.)

[aram356]

Fixed in e4e0e8f. Permutive and lockr now use self-describing required values (your-permutive-organization-id, your-permutive-workspace-id, your-lockr-app-id) marked # required (non-empty); tinybird shows api_host + the store/dataset/token fields. For [request_signing]: the store-id fields are required (no serde default), so commenting out just those two lines would break parsing - instead the block header now documents that a present block needs REAL store IDs even with enabled = false, and to keep signing off you leave the whole block commented. Added the regression test you suggested: documented_integration_blocks_validate_when_uncommented uncomments each block from the real template and asserts parse + field validation; I verified it fails if a required value is emptied back out.

[prk-Jr]

♻️ refactor — length(min = 1) is now redundant: validate_aps_pub_id trims first and rejects the empty result, which subsumes the empty-string case the length rule was added for. Both validators run, so a blank pub_id reports two errors for one cause.

#[validate(custom(function = validate_aps_pub_id))]
pub pub_id: String,

The doc comment on validate_aps_pub_id already explains why the built-in cannot carry this check on its own, so dropping the attribute keeps the single source of truth.

[aram356]

Done in e4e0e8f - dropped length(min = 1), leaving #[validate(custom(function = validate_aps_pub_id))] as the single source of truth (it rejects blank, surrounding whitespace, and the placeholder).

[prk-Jr]

⛏ nitpick — This adds a second impl ApsConfig block for one associated const while impl IntegrationConfig for ApsConfig follows immediately below. Moving PUB_ID_PLACEHOLDERS next to the existing ApsConfig impl (or directly above validate_aps_pub_id, its only caller) keeps the type's inherent items in one place — matching how RequestSigning::STORE_ID_PLACEHOLDERS sits in its type's single impl block.

[aram356]

Done in e4e0e8f. Moved PUB_ID_PLACEHOLDERS to a module const directly above validate_aps_pub_id, its only caller, removing the one-item impl ApsConfig block.

[prk-Jr]

🏕 camp site — The four pre-existing helpers in this file each have direct unit tests: is_placeholder_passphrase_rejects_all_known_placeholders, ..._is_case_insensitive, ..._accepts_non_placeholder, and the same trio for api_token / proxy_secret / password. The three new ones are only exercised indirectly through validate_settings_for_deploy.

Adding the mirror tests is cheap and would have surfaced the normalization gap @ChristianPavilonis flagged separately — a ..._accepts_non_placeholder case naturally invites https://origin.example.com:443 and https://origin.example.com/, both of which currently pass because the comparison is on the whole raw string.

[aram356]

Done in e4e0e8f. Added mirror unit tests for all three helpers: is_placeholder_domain_*, is_placeholder_cookie_domain_*, and is_placeholder_origin_url_*. As you predicted, the origin test naturally invited https://origin.example.com:443 and the trailing-slash form - both of which prompted the normalization fix in is_placeholder_origin_url and are now asserted rejected.

[prk-Jr]

🤔 thinking — Agreed this is the right half to fix here, and the comment explaining why is exactly the sort of rationale that stops a future reader from "simplifying" it back. One consequence worth writing down outside the code: this rejects placeholder or empty store IDs in configs that are already pushed and working, where [request_signing] is present with enabled = false. Those operators get a ts config push failure on their next unrelated change, with an error naming a subsystem they deliberately turned off.

A CHANGELOG or release-note line ("a present [request_signing] block now requires real store IDs even when disabled, because the key-management routes read them regardless") turns that from a surprise into an expected upgrade step.

[aram356]

Agreed. There is no CHANGELOG file in the repo, so I captured the upgrade note two ways: (1) an "Upgrade note" section added to the PR description, and (2) inline in the template - the [request_signing] block header now spells out that a present block needs real store IDs even when disabled, because the key-management routes read them regardless of enabled. Happy to move it into a CHANGELOG if the project adds one.