Skip to content

fix: typed config — single source of defaults, load-time validation, key hygiene#231

Merged
erskingardner merged 2 commits into
masterfrom
fix/typed-config
Jul 5, 2026
Merged

fix: typed config — single source of defaults, load-time validation, key hygiene#231
erskingardner merged 2 commits into
masterfrom
fix/typed-config

Conversation

@erskingardner

@erskingardner erskingardner commented Jul 5, 2026

Copy link
Copy Markdown
Member

Summary

Configuration correctness was enforced ad hoc, in three scattered ways, so misconfiguration failed silently at runtime instead of loudly at load. This centralizes it, per the #215 tracker design.

Single source of defaults (#171)

Dropped the set_default(...) ladder in base_config_builder. The serde default_* fns (backed by the existing DEFAULT_* consts) are now the only place a default lives. The builder seeds each top-level section as an empty table so try_deserialize still reaches those serde defaults when a section is absent from every source (verified with an empty-TOML test). Env-override behavior (TRANSPONDER_* parsing, comma-list handling, ambient-var skipping, non-Unicode redaction) is byte-compatible.

Typed fields, validated at load

Key-file hygiene (#146)

On unix, before reading private_key_file, the file is stat'd; if its mode grants group/other read (mode & 0o077 != 0) the server refuses to start, mirroring the 0600 the generate path enforces.

Private key out of the config Value tree (#156)

server.private_key (inline TOML and TRANSPONDER_SERVER_PRIVATE_KEY) is resolved through a dedicated Zeroizing path — extracted from the parsed TOML document (and env iterator) before the rest of the config reaches the config crate — so the secret never lands in the crate's un-zeroized Value tree. Precedence (env > file config) is unchanged. Docs now recommend private_key_file.

#196 rider — not needed

The metrics.enabled && !health.enabled load-time warning was planned as a rider, but PR #230 (merged into master while this was in flight) landed the structural fix: /metrics is now served on health.bind_address whenever metrics are enabled, independent of health.enabled, and the health server emits its own targeted warning at bind time. A separate warning here would be redundant and factually wrong (metrics ARE now exposed), so it was dropped after merging master.

Breaking for invalid configs

Existing deployments with valid configs load identically. Only previously-silently-tolerated invalid configs now fail at startup:

  • server.max_dedup_cache_size = 0 — was silently swapped for the 100k default; now rejected.
  • server.max_rate_limit_cache_size = 0 — was silently swapped for 100k per limiter; now rejected.
  • server.max_tokens_per_event = 0 — was a total silent push outage (every event failed token parsing); now rejected.
  • logging.format = "off" — silently disabled ALL logging (including errors); now rejected with a message pointing to logging.level = "off".
  • logging.format = <typo> (e.g. "jsno") — silently fell back to the default formatter; now rejected.
  • health.bind_address = <non-SocketAddr> (e.g. "localhost:8080" or "127.0.0.1:99999") — was accepted and only failed to bind at runtime; now rejected at load.
  • relays.onion entry that is not a ws:///wss:// URL with a .onion host (e.g. a plaintext clearnet ws:// URL, a bare host, or a misspelled suffix) — was added blindly / silently dropped at connect; now rejected at load (only when the tor feature is enabled; without it the existing "requires tor feature" error fires first).
  • private_key_file with group/world-readable permissions (mode & 0o077 != 0) — was loaded silently; now refuses to start.

Testing

cargo fmt --check clean; cargo clippy --all-targets -- -D warnings zero (also with --features tor); cargo test --bin transponder all pass (603 default / 606 with tor); cargo check --all-targets --features tor clean. The config.rs test module was extended with a test for every new validation branch, and new tests were added in client.rs (onion validation, inbox normalization, newest-event selection) and main.rs (key-file permission check, LogFormat init).

Fixes #171
Fixes #167
Fixes #166
Fixes #149
Fixes #150
Fixes #142
Fixes #122
Fixes #124
Fixes #146
Fixes #156

Part of #215.

🤖 Generated with Claude Code


Open in Stage

erskingardner and others added 2 commits July 5, 2026 13:48
…key hygiene

Configuration correctness was enforced ad hoc in three scattered ways, so
misconfiguration failed silently at runtime instead of loudly at load. This
centralizes it:

- Single source of defaults (#171): drop the `set_default` ladder; the serde
  `default_*` fns (backed by the `DEFAULT_*` consts) are now the only place a
  default lives. The builder seeds each section as an empty table so
  `try_deserialize` reaches those defaults even when a section is absent.
- Reject-zero / NonZeroUsize for the cache and token sizes (#149, #166):
  `max_dedup_cache_size`, `max_rate_limit_cache_size`, and
  `max_tokens_per_event` reject `0` at load; the downstream configs take
  `NonZeroUsize`, deleting the silent zero-to-default fallbacks in
  `rate_limiter.rs` and `events.rs`.
- LogFormat enum (#150, #142): `logging.format` is a validated enum; unknown
  values (including the undocumented `off` blackout arm) are rejected with a
  message pointing at `logging.level = "off"`.
- health.bind_address SocketAddr validation (#167): a hostname or out-of-range
  port is rejected at load, even when the health server is disabled.
- Onion relay URL validation (#122): each `relays.onion` entry must parse as a
  ws://wss:// URL with a `.onion` host, symmetric with clearnet.
- Inbox relay change-detection (#124): normalize both sides through `RelayUrl`
  and select the newest kind-10050 event by `created_at`.
- Key-file permission check (#146): on unix, refuse to start when
  `private_key_file` grants group/other read (mode & 0o077), mirroring the
  0600 the generate path enforces.
- Private key out of the config Value tree (#156): the inline
  `server.private_key` and `TRANSPONDER_SERVER_PRIVATE_KEY` value are resolved
  through a dedicated Zeroizing path before parsing, so the secret never sits
  in the config crate's un-zeroized buffers.
- #196 rider: warn at load when metrics.enabled && !health.enabled.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Jul 5, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 17 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0d6fca41-31af-4ebd-8b25-41bf56f2ab8f

📥 Commits

Reviewing files that changed from the base of the PR and between 9ea7d39 and 5eab028.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • README.md
  • config/default.toml
  • config/production.toml.example
  • src/config.rs
  • src/main.rs
  • src/nostr/client.rs
  • src/nostr/events.rs
  • src/rate_limiter.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/typed-config

Comment @coderabbitai help to get the list of available commands.

@stage-review

stage-review Bot commented Jul 5, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Jul 5, 2026

Copy link
Copy Markdown

✅ Code Coverage Report

Metric Value
Line Coverage 96.73%
Lines Covered 15627 / 16156
Change from master +0.17%
View detailed coverage

Download the coverage-html artifact from this workflow run to view the detailed coverage report.

Or run locally:

cargo llvm-cov --html
open target/llvm-cov/html/index.html

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.

[LOW] server.max_tokens_per_event = 0 (and max_dedup_cache_size = 0) bypass ServerConfig::validate() — silent push outage / silent default swap [LOW] Server private-key file is loaded with no permission check — a world/group-readable key file is accepted silently [LOW] Invalid logging.format silently falls back to default formatter [LOW] inbox_relay_event_is_current compares raw config strings and uses events.first(), causing spurious kind-10050 republishing and possibly skipping needed updates [MEDIUM] Onion relay URLs bypass all URL validation — a plaintext ws:// entry in relays.onion defeats the wss-only TLS enforcement, and typo'd .onion URLs are silently dropped

1 participant