Skip to content

Add config store runtime config spec#647

Open
ChristianPavilonis wants to merge 1 commit intomainfrom
spec/ts-config-in-config-store
Open

Add config store runtime config spec#647
ChristianPavilonis wants to merge 1 commit intomainfrom
spec/ts-config-in-config-store

Conversation

@ChristianPavilonis
Copy link
Copy Markdown
Collaborator

@ChristianPavilonis ChristianPavilonis commented Apr 20, 2026

Summary

  • add a concrete spec for moving application config from build-time embedding to runtime config store loading
  • define production vs development config behavior, including Fastly/Viceroy local-dev projection into a simulated config store
  • scope the doc to config-store architecture only, with hashing included and signatures/tooling UX deferred

Closes #665

Testing

  • not run (docs-only change)

Copy link
Copy Markdown
Collaborator

@aram356 aram356 left a comment

Choose a reason for hiding this comment

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

Summary

Spec for moving trusted-server.toml from build-time embedded config to runtime config-store loading. Scope and fail-closed posture are good, but several load-bearing details are unspecified or contradict the existing pipeline. Requesting changes on the items below before merge.

Blocking

🔧 wrench

  • Hash algorithm not specified despite explicit "future attestation" goal (lines 328–332): The doc commits to a config hash over canonical TOML bytes but never names SHA-256 vs BLAKE3 vs SHA-3. This locks in (or breaks) the attestation contract in 2026-01-15-attestation-design.md. Pin the algorithm + encoding (suggest SHA-256, hex) or explicitly defer to the attestation spec with a forward reference.
  • Canonical TOML form is underspecified for hash stability (lines 310–318): "Stable ordering" leaves these undefined: key ordering rule (lexicographic vs schema-declared vs insertion), string flavor (basic "..." vs literal '...'), numeric formatting (1 vs 1.0), table vs inline-table, array-of-tables vs inline-array, Unicode normalization. Two toml-rs versions can disagree. Either enumerate the rules or pin a canonicalizer crate + version.
  • "Explicitly declared settings only" contradicts the existing Serialize round-trip (lines 313–316): Today crates/trusted-server-core/build.rs:44-45 does toml::to_string_pretty(&settings), including defaults. The new contract requires a "provided vs defaulted" distinction that #[serde(default)] erases. Implementers will need custom Serialize or a default-elision pass over a baseline Settings. Pick an approach or call this out as a known cost.
  • Bootstrap "store reference" is platform-undefined (lines 222–226): Fastly Compute references config stores by name at the WASM guest. The bootstrap section should at least state: on Fastly the store-name is a fixed constant compiled into the adapter, and fastly.toml must declare a ts-config [local_server.config_stores] for dev. As written the "bootstrap contract" is not actionable for the only currently supported platform.
  • deny_unknown_fields is a behavioral break with no migration path (lines 278–286): Settings does not currently use #[serde(deny_unknown_fields)]. The new contract fail-closes any operator TOML with stale or typo'd keys. Spec should explicitly state (a) this is a breaking change vs. existing operator configs and (b) the integrations carve-out applies only to per-integration payload internals — the dynamic top-level [integrations.<id>] keyspace itself is not denied.
  • Per-request load cost forecloses straightforward optimization (lines 198–213): "Correctness does not depend on cross-request in-memory persistence" plus Fastly's per-request guest model means a worst-case implementation does TOML parse + per-handler regex compile (prepare_runtime()) every request. The spec allows in-memory caching as a perf optimization but forbids it for correctness. Worth flagging as a hard implementation constraint so readers don't underestimate cost.

❓ question

  • Source-selection signal is unspecified (lines 249–258): Step 1 says "Select the source by environment" but never describes the runtime mechanism — cargo feature, env var, Fastly service-ID check, bootstrap flag? This is the central pipeline decision and is undefined. Either pin the mechanism or explicitly defer with a marker.
  • Secrets migration is missing: trusted-server.toml carries edge_cookie.secret_key and publisher.proxy_secret (crates/trusted-server-core/src/settings_data.rs:43-57), overridden today via TRUSTED_SERVER__EDGE_COOKIE__SECRET_KEY and TRUSTED_SERVER__PUBLISHER__PROXY_SECRET at build time. Removing build-time env merging implies either (a) plaintext secrets in the (unencrypted) Fastly Config Store, or (b) splitting secrets into PlatformSecretStore and referencing them by name from Settings. The spec is silent — which is the intended model?

Non-blocking

🤔 thinking

  • "Production vs development" runtime distinction is largely cosmetic (lines 187–190): Both paths read ts-config from a (real or simulated) config store. The actual difference is who populates the store, not how the runtime reads it. Collapsing to one runtime path plus a tooling pre-flight step would simplify the doc.
  • Fastly Config Store value-size limit is not addressed: Fastly caps each value (~8 KB historically). Current canonical TOML is ~3.4 KB; source is ~6.8 KB. Future growth could exceed the limit. One line in "Implementation notes" surfaces this at design time.
  • Multi-tenant constraint of fixed ts-config key (lines 235–243): Hard-coding the key globally precludes per-tenant config via key multiplexing in a shared binary. Likely intentional — call it out so future cross-platform abstractions don't assume key namespacing.

♻️ refactor

  • Cross-link to existing attestation spec: Spec mentions "future attestation" four times. docs/superpowers/specs/2026-01-15-attestation-design.md already exists. Add a "Related work" section linking it.
  • Name the existing platform abstraction: crates/trusted-server-core/src/platform/traits.rs:12 defines PlatformConfigStore. Spec says "existing generic config-store abstractions" but never names it. Naming makes implementation actionable.
  • Specify how the dev ts-config payload reaches Viceroy (lines 418–422): Viceroy reads config-store contents from [local_server.config_stores.<name>.contents] in fastly.toml (fastly.toml:42-49). Either dev tooling regenerates that section per fastly compute serve, or an alternate loading mechanism. Worth one paragraph.

🌱 seedling

  • Note /health carve-out from fail-closed: crates/trusted-server-adapter-fastly/src/main.rs:55-61 short-circuits /health before settings load. Worth a one-liner so a future reader doesn't "fix" this as inconsistent with fail-closed semantics.

📌 out of scope

  • Test fixtures + CI depend on tracked trusted-server.toml: Removing it touches crate_test_settings_str() consumers and 10+ tests in crates/trusted-server-core/src/settings.rs. Open a follow-up issue tracking the test/CI migration before implementation lands.

⛏ nitpick

  • Add Author line: Peers use > **Author:**. This spec omits it.
  • Inconsistent terminology: "platform config store" / "platform config-store" / "platform-store" used interchangeably. Pick one.
  • Bullet phrasing at line 314: do **not** expand… mid-sentence reads oddly inside the numbered list.

CI Status

  • cargo fmt: PASS
  • cargo test: PASS
  • vitest: PASS
  • format-typescript: PASS
  • format-docs: PASS
  • Analyze (rust / actions / js-ts): PASS
  • browser integration tests: PASS
  • integration tests: PASS
  • CodeQL: PASS

This section defines the semantic pipeline for both production and development.

### Step 1: Select the source by environment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

question — Source-selection signal is unspecified. Step 1 says "Select the source by environment" but never describes the runtime mechanism. Cargo feature? Env var read at module init? Fastly service-ID check? Bootstrap config flag? This is the central pipeline decision and is undefined. Either pin the mechanism here or defer explicitly (e.g., "to be defined in the tooling spec, but must be a compile-time signal").

The config hash is computed over the canonical TOML bytes.

This provides a stable hashable representation of application config suitable
for observability and future attestation work.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔧 wrench — Hash algorithm not specified despite explicit "future attestation" goal. The doc commits to a config hash over canonical TOML bytes but never names SHA-256 vs BLAKE3 vs SHA-3, nor the encoding (hex / base64 / raw). Choosing here locks in the attestation contract in 2026-01-15-attestation-design.md. Pin the algorithm + encoding (suggest SHA-256, hex) or explicitly defer to the attestation spec with a forward reference.

- do **not** expand the config into a full dump of all effective defaulted runtime values
- define stable ordering for map-like structures so identical semantic config
produces identical canonical bytes

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔧 wrench — Canonical TOML form is underspecified for hash stability. "Define stable ordering for map-like structures" leaves the following undefined and stability-critical:

  • key ordering rule (lexicographic vs schema-declared vs insertion)
  • string flavor (basic "..." vs literal '...')
  • numeric formatting (1 vs 1.0)
  • table vs inline-table normalization
  • array-of-tables vs inline-array
  • Unicode normalization of string values

Two correct implementations (or two toml-rs versions) can produce different bytes from the same Settings and break hash stability. Either enumerate the rules explicitly or pin a canonicalizer crate + version.

- serialize it in a deterministic TOML form
- include explicitly declared settings only
- do **not** expand the config into a full dump of all effective defaulted runtime values
- define stable ordering for map-like structures so identical semantic config
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔧 wrench — "Explicitly declared settings only" contradicts the existing Serialize round-trip. Today crates/trusted-server-core/build.rs:44-45 does toml::to_string_pretty(&settings), which includes defaults. The new contract requires a "provided vs defaulted" distinction that #[serde(default)] erases. Implementers will need either custom Serialize impls per type or a default-elision pass that compares against a baseline Settings. Pick an approach or call this out as a known cost.

Production bootstrap must provide:

- the platform config store reference needed to open/read the store

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔧 wrench — Bootstrap "store reference" is platform-undefined. Fastly Compute references config stores by name at the WASM guest level — the names live in fastly.toml, not in environment. The spec should at least state:

  • On Fastly, the bootstrap form is a fixed store-name constant compiled into the adapter.
  • fastly.toml must declare a ts-config [local_server.config_stores] entry for dev.

As written, the bootstrap contract is not actionable for the only currently supported platform.


This spec does not define a special "healthy but unusable" mode for config
failure.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🌱 seedling — Note /health carve-out from fail-closed. crates/trusted-server-adapter-fastly/src/main.rs:55-61 short-circuits /health before settings load, which is a deliberate carve-out from the fail-closed model defined here. A one-liner acknowledging this would prevent a future reader from "fixing" the early-return as a bug.

- tracked template file
- kept in sync with currently supported configuration features
- intended to help operators create a real `trusted-server.toml`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

📌 out of scope — Test fixtures + CI depend on tracked trusted-server.toml. Removing it touches crate_test_settings_str() consumers and 10+ tests in crates/trusted-server-core/src/settings.rs, plus CI workflows that rely on the file existing at the repo root. Worth a follow-up issue tracking the test/CI migration before this becomes implementable.


> **Status:** Proposal
> **Scope:** Config store architecture only
> **Date:** April 2026
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nitpick — Peer specs (e.g., 2026-01-15-attestation-design.md) include an Author: line. Adding one keeps consistency.

- **Development:** local file at repo root by default

The authoritative production payload is stored in the platform config store under
key `ts-config`.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nitpick — Inconsistent terminology. "platform config store" / "platform config-store" / "platform-store" / "config-store payload" / "platform store payload" appear interchangeably. Pick one form for the doc.

- parse valid config through the typed config model
- serialize it in a deterministic TOML form
- include explicitly declared settings only
- do **not** expand the config into a full dump of all effective defaulted runtime values
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nitpickdo **not** expand the config into a full dump of all effective defaulted runtime values reads oddly mid-sentence inside this numbered list. Consider promoting it to its own bullet.

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.

Create trusted server config store spec

2 participants