musiclink/docs/reviews/config.md

32 lines
1.4 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Code Review: Config & Runtime Wiring
## Scope
- `pkg/config/config.go`
- `config.example.toml`
- `config.toml`
## Findings
### ✅ Strengths
- Validation logic clearly separates Matterbridge vs Matrix-required fields.
- Defaults are set for missing Matterbridge username and Matrix state store path.
- Example config documents Matrix mode and health endpoint.
### ⚠️ Issues / Opportunities
1. **Mutually exclusive modes arent enforced**
- If both Matterbridge URL and Matrix enabled are set, both validate, but only Matrix is used in runtime.
- Consider warning or requiring explicit selection to avoid confusion.
2. **No validation for room ID format**
- `matrix.rooms` accepts any string; invalid IDs will only fail later at runtime.
- Consider validating room IDs or at least trimming whitespace.
3. **Token fields likely to be logged if config is dumped**
- Config loads raw tokens without masking; should avoid logging config objects to prevent leaks.
- Documentation could recommend loading tokens from env/secret stores.
4. **Config defaults for Matterbridge only applied when URL set**
- If a user sets `matrix.enabled=false` and forgets `matterbridge.url`, they get an error, but username default isnt set until URL present. Low impact but note the coupling.
## Notes
- Current `config.toml` is a local test file; no secrets should be committed.