feat: replace JSON config with .env format and auto-generate API token#22
Conversation
The API token is now generated automatically using the OS RNG on first startup and written back into the config file, so it persists across restarts without any manual step. Users can no longer set the token manually: the -t CLI flag and LRTMP2_API_TOKEN env override are removed. Config files change from JSON to a flat KEY=VALUE (.env) format. config.example.json is replaced by config.example.env; the default file the server looks for is config.env instead of config.json. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxGDerSY7BbB6SEKVaVCK5
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughSwitches config loading from JSON to ChangesConfig migration and API token lifecycle
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxGDerSY7BbB6SEKVaVCK5
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxGDerSY7BbB6SEKVaVCK5
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/config.rs`:
- Around line 74-104: The config parsing in the `load_env`/match block silently
ignores malformed values for known keys like `TLS_ENABLED`,
`REQUIRE_STREAM_KEY`, and `LOG_LEVEL`. Update this parsing logic to explicitly
warn or return an error when a recognized key has an invalid value instead of
falling back to defaults. Use the existing `config` assignment branches and the
`val.parse::<i32>()` / boolean match arms to detect bad inputs and surface them
clearly.
- Line 169: The config file write path currently uses std::fs::write without
tightening permissions, so newly created token-bearing files may be
world-readable. Update the config-writing logic in the config save/write flow to
create files with owner-only permissions on Unix (read/write for the owner only)
and add the corresponding Windows ACL handling or documented fallback in the
same config persistence path. Use the existing config write site around the
write-and-map_err call so the restrictive permissions are applied whenever
config.env is created or overwritten.
- Line 142: The config loading in the existing file read path is masking read
failures by falling back to an empty string, which can lead to accidental
overwrites later in the config flow. In `src/config.rs`, update the logic around
`std::fs::read_to_string` so unreadable configs are handled as an error instead
of being treated as empty content, and make sure the code that writes back the
config only runs when the file was successfully read. Use the surrounding config
load/update path to preserve all existing entries and avoid rewriting from
partial state.
- Around line 60-63: Guard the quote-stripping logic in the config parsing path
so one-character values like KEY=" or KEY=' do not reach the slice in the
quote-handling branch. Update the logic around the val normalization in
src/config.rs to check that the string is at least 2 characters long before
removing surrounding quotes, and keep the existing behavior for properly wrapped
values while leaving malformed inputs to fail cleanly instead of panicking.
In `@src/main.rs`:
- Around line 72-77: The token generation path in main currently writes the raw
bearer token to stderr, which can end up in captured process logs. Update the
token-printing logic in main to avoid emitting the token value directly; instead
only show the config path and brief usage instructions, or gate any raw token
reveal behind an explicit one-time interactive action.
- Around line 67-78: The token bootstrap in config_load/main currently only
regenerates when config.api_token is empty, so placeholder or stale values still
get trusted. Update the startup validation around config.api_token to reject
non-empty tokens that are clearly invalid (or trigger the same auto-generation
flow), and make the error message tell users to remove the API_TOKEN line from
the config if they want automatic generation. Use the existing config_load and
config_write_token flow, and keep the check in the branch that currently handles
empty tokens.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ea91af36-4161-4ceb-8fa9-659394f786ce
📒 Files selected for processing (5)
config.example.envconfig.example.jsondocker-compose.ymlsrc/config.rssrc/main.rs
💤 Files with no reviewable changes (2)
- docker-compose.yml
- config.example.json
| pub fn config_write_token(path: &str, token: &str) -> Result<(), String> { | ||
| let line = format!("API_TOKEN={token}"); | ||
|
|
||
| let existing = std::fs::read_to_string(path).unwrap_or_default(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Don’t treat unreadable configs as empty files.
unwrap_or_default() masks every read failure; a transient read error can cause Line 169 to rewrite the file with only API_TOKEN, dropping the rest of the config.
Proposed fix
- let existing = std::fs::read_to_string(path).unwrap_or_default();
+ let existing = match std::fs::read_to_string(path) {
+ Ok(existing) => existing,
+ Err(e) if e.kind() == std::io::ErrorKind::NotFound => String::new(),
+ Err(e) => return Err(format!("Cannot read config file {path}: {e}")),
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let existing = std::fs::read_to_string(path).unwrap_or_default(); | |
| let existing = match std::fs::read_to_string(path) { | |
| Ok(existing) => existing, | |
| Err(e) if e.kind() == std::io::ErrorKind::NotFound => String::new(), | |
| Err(e) => return Err(format!("Cannot read config file {path}: {e}")), | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/config.rs` at line 142, The config loading in the existing file read path
is masking read failures by falling back to an empty string, which can lead to
accidental overwrites later in the config flow. In `src/config.rs`, update the
logic around `std::fs::read_to_string` so unreadable configs are handled as an
error instead of being treated as empty content, and make sure the code that
writes back the config only runs when the file was successfully read. Use the
surrounding config load/update path to preserve all existing entries and avoid
rewriting from partial state.
| out.push('\n'); | ||
| } | ||
|
|
||
| std::fs::write(path, out).map_err(|e| format!("Cannot write config file {path}: {e}")) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Create token-bearing config files with restrictive permissions.
This path can create config.env on first startup and now writes a bearer token. Ensure newly created files are owner-readable/writable only on Unix, and document/handle equivalent Windows ACL behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/config.rs` at line 169, The config file write path currently uses
std::fs::write without tightening permissions, so newly created token-bearing
files may be world-readable. Update the config-writing logic in the config
save/write flow to create files with owner-only permissions on Unix (read/write
for the owner only) and add the corresponding Windows ACL handling or documented
fallback in the same config persistence path. Use the existing config write site
around the write-and-map_err call so the restrictive permissions are applied
whenever config.env is created or overwritten.
| if config.api_token.is_empty() { | ||
| let token = keygen::keygen_secret("tk_")?; | ||
| let cfg_path = &config.config_file; | ||
| config_write_token(cfg_path, &token) | ||
| .map_err(|e| format!("Could not persist API token: {e}"))?; | ||
| eprintln!( | ||
| "============================================================\n\ | ||
| Generated API token (saved to {cfg_path}):\n\ | ||
| {token}\n\ | ||
| ============================================================" | ||
| ); | ||
| config.api_token = token; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Keep validating loaded tokens before trusting them.
config_load still accepts any API_TOKEN, and this branch only regenerates when it is empty. A stale short/placeholder token will now start successfully. Restore a usability check for non-empty configured tokens, or reject them with guidance to remove the line for auto-generation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main.rs` around lines 67 - 78, The token bootstrap in config_load/main
currently only regenerates when config.api_token is empty, so placeholder or
stale values still get trusted. Update the startup validation around
config.api_token to reject non-empty tokens that are clearly invalid (or trigger
the same auto-generation flow), and make the error message tell users to remove
the API_TOKEN line from the config if they want automatic generation. Use the
existing config_load and config_write_token flow, and keep the check in the
branch that currently handles empty tokens.
The token can no longer be set via config file or environment variable. On first startup it is generated (128-bit OS entropy) and persisted in a new `settings` table in the SQLite database; on subsequent starts it is read back from there. This removes config_write_token and all API_TOKEN handling from config.rs and main.rs. Also fixes CodeRabbit findings: - Quote-stripping guard: val.len() >= 2 check prevents panic on KEY=" - Warn on invalid values for TLS_ENABLED, LOG_LEVEL, etc. instead of silently falling back to defaults Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxGDerSY7BbB6SEKVaVCK5
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxGDerSY7BbB6SEKVaVCK5
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/db.rs`:
- Around line 188-198: token_get currently hides SQLite read failures by
converting them into None, which causes ServerApp::create to treat DB errors as
missing tokens and regenerate one. Update token_get in the db module to return a
Result<Option<String>> instead of swallowing errors, and adjust
ServerApp::create to regenerate/persist a bearer token only when token_get
returns Ok(None), while propagating any Err from the read path.
- Around line 200-207: Avoid overwriting an already bootstrapped API token in
token_set; the current INSERT ... ON CONFLICT DO UPDATE path can replace the
winning token and leave the loser using an unpersisted value. Update the token
persistence flow in src/db.rs so it behaves like get-or-create: check for an
existing api_token first, only insert when missing, and if another process wins
the race, read back the stored token and use that instead of keeping the locally
generated one.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 217e9067-5619-415a-98a1-94b4d503f966
📒 Files selected for processing (4)
src/config.rssrc/db.rssrc/main.rssrc/server.rs
- token_get now returns Result<Option<String>> so DB read errors are propagated instead of silently becoming "no token found" - token_set uses INSERT OR IGNORE instead of ON CONFLICT DO UPDATE to avoid overwriting a concurrently bootstrapped token; callers read back the winner's token when their insert was ignored - Add docstrings to token_get and token_set for coverage threshold Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxGDerSY7BbB6SEKVaVCK5
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxGDerSY7BbB6SEKVaVCK5
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/db.rs (1)
88-91: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAllow bootstrapping to repair an empty
api_tokenrow.
token_get()treatsval=''as missing, butINSERT OR IGNORErefuses to write when theapi_tokenrow already exists. A DB containingsettings(key='api_token', val='')will fail startup permanently instead of generating a token.🐛 Proposed fix
- let rows = conn - .execute( - "INSERT OR IGNORE INTO settings(key,val) VALUES('api_token',?)", - rusqlite::params![token], - ) + let rows = conn + .execute( + "INSERT INTO settings(key,val) VALUES('api_token',?) + ON CONFLICT(key) DO UPDATE SET val=excluded.val + WHERE settings.val=''", + rusqlite::params![token], + ) .map_err(|e| format!("DB error persisting API token: {e}"))?;Also applies to: 200-215
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/db.rs` around lines 88 - 91, The settings bootstrap path currently treats api_token with an empty val as missing, but the existing INSERT OR IGNORE path never updates that row, so startup can get stuck forever. Update the bootstrapping logic around token_get() and the settings table handling in src/db.rs so that an existing api_token row with val='' is repaired by writing a newly generated token instead of being ignored. Ensure the fix targets the api_token key specifically and replaces the empty value on startup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/db.rs`:
- Around line 88-91: The settings bootstrap path currently treats api_token with
an empty val as missing, but the existing INSERT OR IGNORE path never updates
that row, so startup can get stuck forever. Update the bootstrapping logic
around token_get() and the settings table handling in src/db.rs so that an
existing api_token row with val='' is repaired by writing a newly generated
token instead of being ignored. Ensure the fix targets the api_token key
specifically and replaces the empty value on startup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b6ea7536-fe53-44e3-83a0-6d4a287072a8
📒 Files selected for processing (2)
src/db.rssrc/server.rs
INSERT OR IGNORE would permanently skip generation if the settings table already had api_token='' (e.g. after a corrupted write). Switch to ON CONFLICT DO UPDATE WHERE val='' so an empty row is overwritten with a freshly generated token while a real token is still never overwritten. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxGDerSY7BbB6SEKVaVCK5
…onfig These three settings had no business being user-configurable: - WEB_ROOT: there is no configurable web UI path - REQUIRE_STREAM_KEY: stream keys are always required - RTMP_CHUNK_SIZE: protocol internal, no reason for operators to tune this Removed from ServerConfig, Default, apply_kv, and config.example.env. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxGDerSY7BbB6SEKVaVCK5
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxGDerSY7BbB6SEKVaVCK5
Summary
.env:config.example.jsonis replaced byconfig.example.env; the server now looks forconfig.envby default (instead ofconfig.json). The new format uses flatKEY=VALUEpairs, comments (#), and optional quotes.keygen_secret), prints it to stderr, and writes it back asAPI_TOKEN=...into the config file so it persists across restarts. The-tCLI flag andLRTMP2_API_TOKENenvironment override are removed — users can no longer set the token manually.docker-compose.ymlupdated: TheLRTMP2_API_TOKENenvironment variable requirement is removed since the token is now managed automatically.Changes
src/config.rsserde_jsonJSON parser with a simple.envline parser; addconfig_write_token()to persist the generated token in-placesrc/main.rs-tflag; auto-generate token on startup ifapi_tokenis emptyconfig.example.env.envformat (noAPI_TOKENentry — generated automatically)config.example.jsondocker-compose.ymlLRTMP2_API_TOKENrequirementTest plan
cargo test— all 22 tests pass (6 new tests cover.envparsing andconfig_write_token)config.env: server generates and prints token, writesAPI_TOKEN=...to the fileconfig.envcontainingAPI_TOKEN: server reuses stored token without regeneratingHTTP_BIND) are preserved when the token is written backGenerated by Claude Code
Summary by CodeRabbit
.env-style configuration by default (config.env), with environment and CLI overrides still supported.KEY=VALUE, supports quoted values (including=), and skips blank lines/comments.