Repository navigation
feat(config): add managed configuration mode - #6717
Conversation
LibreFang treats config.toml as application state: the dashboard writes to it, several API routes persist into it, and boot-time migration rewrites it in place. That is wrong for a deployment whose configuration comes from a ConfigMap or a config-management system, where a dashboard write is either lost on the next rollout or silently drifts the running config away from the manifest. LIBREFANG_CONFIG_PATH relocates the file and LIBREFANG_CONFIG_MODE=managed locks it. The two are independent because relocating a file is not a statement about who owns it, and inferring the lock from a custom path would hand a read-only dashboard to an operator who only wanted the file elsewhere. The mode is read from the process environment and never from the config file, so an API write cannot unlock the file it is being refused access to. A typo resolves to mutable rather than managed, so a misspelling cannot take the dashboard away from a deployment that never asked for it. Managed mode answers 423 Locked with a structured body from the handler, before it reads the existing file, so a refused write never opens or truncates anything. Enforcement is in the handlers rather than on a read-only mount: an EACCES surfaces as a 500 with an errno and says nothing about why, and it does not apply at all when the deployment leaves the file writable but still expects the manifest to win. GET /api/config/status reports mode, source, writability, a SHA-256 over the file bytes, and last-modified, so the dashboard can present managed settings as read-only from server metadata instead of attempting a save and reading the refusal back. Boot-time schema migration no longer writes the migrated config back when the file is managed. That write previously failed against a read-only mount with only a warn!, so the migration silently re-ran on every boot forever. The in-memory config is migrated either way; the warning now tells the operator their manifest is a schema version behind.
Deploying librefang-docs with
|
| Latest commit: |
3e94a87
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://33c79d1d.librefang-docs.pages.dev |
| Branch Preview URL: | https://feat-managed-config.librefang-docs.pages.dev |
The new /api/config/status doc comment was inserted above the existing /// GET /api/config/schema line instead of below it, so utoipa attributed the merged two-endpoint comment block to config_status and left config_schema's OpenAPI summary empty. Move the schema doc comment back to its own function and regenerate openapi.json.
CLAUDE.md requires breaking prose only at sentence boundaries, not at a column width. The doc comments and docs/operations/managed-config.md added by this PR hard-wrapped multi-sentence blocks at ~90-100 columns instead. Reflowed the new comments/docs in routes/mod.rs, config.rs, config/manage.rs, config_managed_mode_test.rs, and docs/operations/managed-config.md; regenerated openapi.json and its sha256 baseline since one of the affected doc comments feeds a utoipa summary/description.
houko
left a comment
There was a problem hiding this comment.
Daily automated review pass — CLAUDE.md compliance check.
Clean PR overall: changelog fragment present and correctly attributed (changelog.d/added/6695-managed-config-mode.md), the new /api/config/status route is registered in both openapi.rs and config/mod.rs, the new doc comments and docs/operations/managed-config.md are one-sentence-per-line per the prose-wrapping rule, no config.toml field was added for the mode (correctly — the design note explains why the mode must not be settable through the file itself), and the new integration test's use of server::build_router directly (rather than a start_test_server() wrapper) matches the existing pattern used by ~25 other files in crates/librefang-api/tests/, not a deviation. CI is green across the board.
One finding worth a maintainer call rather than a mechanical fix. This PR's own docs/operations/managed-config.md "Known gaps" section discloses that the skills, memory, and dashboard-credential routes write config.toml without going through guard_config_write() (or config_write_lock). Confirmed the three sites: routes/memory.rs:1622 (PATCH /api/memory/config), server.rs:1178 (dashboard password / API-key-hash rotation), and routes/skills/mod.rs (upsert_mcp_server_entry / remove_mcp_server_entry, ~L748-801). All three live in this same crate (librefang-api) that the diff is already touching. Per CLAUDE.md's "fix what you found" rule, the bar for deferring to a follow-up is "would fixing it require touching a different crate or domain" — here the answer is no, so wiring the guard into these three sites arguably belongs in this PR rather than being documented as a known gap, unless there's a reason (e.g. the pre-existing missing-lock race needs its own fix first) that makes splitting it out the right call. Flagging for your judgment rather than pushing a fix myself, since closing it touches three more handlers' write paths and I'd rather not guess at behavior changes across all of them in one pass.
Generated by Claude Code
houko
left a comment
There was a problem hiding this comment.
Daily automated review pass, continued from the prior pass on this same head commit (3e94a87).
Re-verified: openapi.json / xtask/baselines/openapi.sha256 are in sync with the current tree (sha256sum matches the baseline), no new KernelConfig struct fields were added (the mode is intentionally env-only, matching the design note), no HashMap/HashSet was introduced on any LLM-prompt-adjacent path, the changelog fragment is correctly formatted and attributed, commit messages are conventional-commit with no AI attribution, and the new/touched prose (doc comments, docs/operations/managed-config.md) is sentence-per-line per the prose-wrapping rule. Scoped cargo check -p librefang-api -p librefang-kernel --lib was run locally against this head (full --workspace --lib fails in this environment only on an unrelated, pre-existing librefang-desktop / gdk-sys system-library gap, not on anything in this diff).
One new finding not covered by the earlier review, left as a single inline comment: the managed-mode guard added to the budget and user persistence paths has no integration test coverage of its own, only config/set does.
Generated by Claude Code
| state: &Arc<AppState>, | ||
| new_budget: &librefang_types::config::BudgetConfig, | ||
| ) -> Result<(), PersistBudgetError> { | ||
| if crate::routes::guard_config_write().is_some() { |
There was a problem hiding this comment.
Separate from the already-flagged "Known gaps" coverage question: the new config_managed_mode_test.rs only exercises the 423 path through POST /api/config/set. This guard call (and the matching one in routes/users.rs:1184) is new behavior on PUT /api/budget, PUT /api/providers/{name}/budget, the per-user budget routes, and POST/PUT/DELETE /api/users / key rotation — none of which have a test asserting the 423 Locked response or that the underlying file is left untouched when managed mode is active.
Per CLAUDE.md's Integration Testing section, a changed route's behavior (mutable write vs. refused write is a real behavior change, not just an internal refactor) should get its own #[tokio::test] against TestServer/build_router, following the pattern already established for config/set in this same file. Flagging rather than authoring the tests myself since picking representative cases across ~6 guarded handlers is a judgment call on scope, not a mechanical addition.
Generated by Claude Code
houko
left a comment
There was a problem hiding this comment.
Read through the full diff, the RFC discussion referenced in the description, docs/operations/managed-config.md, and docs/operations/config-reload.md.
Ran cargo check -p librefang-kernel --lib, cargo check -p librefang-api --lib, cargo clippy -p librefang-api -p librefang-kernel --all-targets -- -D warnings (clean except one pre-existing nonminimal_bool lint in routes/config/system.rs:338 that is untouched by this diff and present on origin/main too — unrelated to this PR), cargo test -p librefang-api --test config_managed_mode_test (3 passed), and cargo test -p librefang-api --test config_routes_integration (45 passed, no regressions).
Confirmed this PR does not add any new KernelConfig struct field (the mode is intentionally env-only, matching the design note in the description), so the struct-field/Default/serde checklist and the build_reload_plan classification table don't apply here.
No HashMap introduced on anything prompt-reaching.
Changelog fragment is present, correctly formatted, and its (#6695, #6717) correctly suppresses the generated line.
SDK bindings (Go/JS/Python/Rust) and the OpenAPI spec/checksum are all updated consistently for the one new route.
Left two comments — both need a maintainer call rather than a mechanical fix, so no commits from me this pass.
Generated by Claude Code
| /// | ||
| /// `LIBREFANG_CONFIG_PATH` relocates the file and nothing more. | ||
| /// It is independent of [`config_mode`] on purpose: a Compose deployment may reasonably bind-mount the config directory somewhere outside `LIBREFANG_HOME` while still editing it from the dashboard, and inferring the lock from the path would hand that operator a read-only UI they never asked for. | ||
| pub fn default_config_path() -> PathBuf { |
There was a problem hiding this comment.
LIBREFANG_CONFIG_PATH only relocates the initial read; the write, reload, and status paths do not follow it, and I think that breaks the PR's own advertised scenario.
Traced the actual read/write targets: config_set and persist_budget/persist_users build their file path from state.kernel.home_dir().join("config.toml") (routes/config/manage.rs:1342, routes/budget.rs, routes/users.rs), and reload_config() uses self.home_dir_boot.join("config.toml") (kernel/config_reload_ops.rs:38).
None of those three call default_config_path() — only load_config(None) (here) and guard_config_write() / config_provenance() in routes/mod.rs do.
KernelConfig.home_dir is populated from KernelConfig::default().home_dir, which is librefang_home_dir() in librefang-types/src/config/types.rs — that function checks LIBREFANG_HOME only, it has no knowledge of LIBREFANG_CONFIG_PATH.
So an operator who sets only LIBREFANG_CONFIG_PATH=/etc/librefang/config.toml (exactly the Compose bind-mount example in the PR description and in docs/operations/managed-config.md) without also setting LIBREFANG_HOME=/etc/librefang, or hand-adding home_dir = "/etc/librefang" inside the relocated file itself, gets:
- the file correctly loaded from
/etc/librefang/config.tomlat boot, - but
config_set/budget/user writes andreload_config()reading/writing~/.librefang/config.toml(or whereverLIBREFANG_HOMEpoints) instead — a different file, - while
GET /api/config/statusand the423refusal body'ssourcefield (both viadefault_config_path()) keep reporting the original relocated path, so the operator has no visible signal that writes are landing somewhere else.
I don't think this is a one-line fix — it's a design choice between (a) deriving home_dir from the resolved config path's parent whenever LIBREFANG_CONFIG_PATH is set, (b) a boot-time WARN when home_dir and default_config_path().parent() diverge, or (c) documenting that LIBREFANG_CONFIG_PATH requires a matching home_dir/LIBREFANG_HOME and isn't just a drop-in path override.
Grepped the repo for LIBREFANG_CONFIG_PATH/CONFIG_PATH_ENV — it's referenced nowhere outside this function's four lines, so there's no test exercising the relocation case at all today.
Generated by Claude Code
|
|
||
| /// The default deployment is unchanged: writable, mode `mutable`, and `config/set` still works. | ||
| /// This is the compatibility guarantee the RFC asks for, so it is asserted rather than assumed. | ||
| #[tokio::test(flavor = "multi_thread")] |
There was a problem hiding this comment.
The three tests here only exercise GET /api/config/status and POST /api/config/set under managed mode.
The same guard is also wired into 10 more call sites across routes/budget.rs (update_budget, update_provider_budget, update_user_budget, delete_user_budget) and routes/users.rs (create_user, update_user, delete_user, import_users, rotate_user_key, update_user_policy), and none of them are exercised under LIBREFANG_CONFIG_MODE=managed anywhere in the suite.
The wiring is small (each site is if guard_config_write().is_some() { return Err(...::Managed) } ahead of the existing config_write_lock), and I did trace all ten by hand and they look correct, so I'm not flagging this as a bug.
But it's exactly the kind of "un-registered new response branch" that CLAUDE.md's integration-testing section asks for a #[tokio::test] on, and a single case (e.g. PUT /api/budget under managed mode refused with 423 and the daily-budget file byte-for-byte unchanged, mirroring config_set_is_locked_in_managed_mode_and_leaves_the_file_untouched) would cover the PersistBudgetError::Managed / PersistError::Managed arms at least once outside hand-reading.
Leaving this for your call on whether the config_set case is representative enough given the shared guard_config_write() helper, or whether it's worth one more case per domain.
Generated by Claude Code
|
Found one process issue while re-checking this PR: commit CLAUDE.md's git conventions section is explicit that commit messages/PR bodies/comments must carry no AI/Claude/Anthropic attribution, and the repo's own I'm not rewriting history here (this routine's hard rule is never force-push, even on a branch I opened), so flagging for a manual fix before merge: Generated by Claude Code |
houko
left a comment
There was a problem hiding this comment.
Daily automated review pass. No mechanical fixes needed — changelog fragment, docs cross-references, openapi.json/SDK regeneration, and test coverage all check out against the PR's own claims (verified by building and running the new test locally). One inline comment left on a real, already-self-flagged gap for maintainer judgment on scope.
Generated by Claude Code
| Stated plainly rather than left to be discovered: | ||
|
|
||
| - **Coverage is not yet complete.** | ||
| The skills routes (`crates/librefang-api/src/routes/skills/mod.rs`), the memory route, and the dashboard-credential change in `server.rs` write `config.toml` without going through the guard. |
There was a problem hiding this comment.
Verified this gap is real, not just theoretical: change_password in crates/librefang-api/src/server.rs (~line 1178-1233) reads config.toml, mutates dashboard_user/dashboard_pass_hash, and calls std::fs::write(&config_path, &toml_string) directly — no guard_config_write() call and no config_write_lock acquisition anywhere in the function. Same pattern confirmed in routes/skills/mod.rs (upsert_mcp_server_config / the sibling remove function around line 748-800).
This one seems worth prioritizing over the others: it's the credential-rotation path, so under managed mode an operator (or anyone who reaches this authenticated route) can still silently rewrite the file the deployment believes it owns, and the write will vanish on the next ConfigMap rollout with no warning — the opposite of the guarantee GET /api/config/status advertises. The skills/memory gaps are "another feature can still edit config" whereas this one is "the credential story managed mode is partly meant to protect can still mutate the locked file."
Given the PR description already offers to fold this in or do it as a follow-up: my read is change_password is small and self-contained enough (single guard check + the existing config_write_lock, mirroring the budget.rs/users.rs pattern already in this diff) that it could land in this PR without touching the concurrency-lock question for the other six sites, which does deserve its own PR given the "changes concurrency semantics on paths I have not otherwise touched" concern you raised. But that's a scope call for a maintainer, not something I'm changing unilaterally here.
(Automated review pass — verified via a fresh clone + cargo check -p librefang-api -p librefang-kernel --lib (clean) and cargo test -p librefang-api --test config_managed_mode_test (3 passed, matches the PR description). No mechanical issues found: changelog fragment, docs cross-reference, openapi.json/SDK regen, and auth-allowlist placement all check out.)
Generated by Claude Code
| /// | ||
| /// `LIBREFANG_CONFIG_PATH` relocates the file and nothing more. | ||
| /// It is independent of [`config_mode`] on purpose: a Compose deployment may reasonably bind-mount the config directory somewhere outside `LIBREFANG_HOME` while still editing it from the dashboard, and inferring the lock from the path would hand that operator a read-only UI they never asked for. | ||
| pub fn default_config_path() -> PathBuf { |
There was a problem hiding this comment.
LIBREFANG_CONFIG_PATH only changes where the initial boot load reads from — it does not change where the daemon writes or reloads from afterward, so relocation and reload/write silently diverge onto two different files.
default_config_path() is called from load_config(None) at boot, but every other place that touches config.toml builds the path independently from home_dir (which defaults to librefang_home() — unrelated to LIBREFANG_CONFIG_PATH — see KernelConfig::default() in crates/librefang-types/src/config/types.rs:6462):
reload_config—crates/librefang-kernel/src/kernel/config_reload_ops.rs:40:self.home_dir_boot.join("config.toml")config_set—crates/librefang-api/src/routes/config/manage.rs:1342:state.kernel.home_dir().join("config.toml")persist_budget—crates/librefang-api/src/routes/budget.rs:596: same patternpersist_users(crates/librefang-api/src/routes/users.rs) andexport_config(crates/librefang-api/src/routes/config/manage.rs:1045): same pattern
Concretely: set LIBREFANG_CONFIG_PATH=/etc/librefang/config.toml with LIBREFANG_HOME left at its default, and mutable mode. Boot reads /etc/librefang/config.toml, but POST /api/config/set (and the budget/users persist paths) will read-modify-write ~/.librefang/config.toml instead — a different file than the one the daemon booted from — and POST /api/config/reload re-reads ~/.librefang/config.toml too, not the relocated file. That contradicts the managed-config.md claim that reload "still works... it re-reads the file" (it re-reads the wrong file when relocated), and it means the doc's own headline scenario — "a Compose bind mount or a ConfigMap mounted outside LIBREFANG_HOME" — doesn't actually work end-to-end for anything past the first boot.
This is new: default_config_path() and home_dir-based path construction used to always agree (both reduced to librefang_home().join("config.toml")) before CONFIG_PATH_ENV was introduced here. Worth resolving before merge, or at minimum calling out as a known limitation in docs/operations/managed-config.md next to the other "Known gaps" — right now the doc states the relocation case as the supported scenario without the caveat.
Generated by Claude Code
| "Config was migrated in memory but NOT written back: the file is deployment-managed (LIBREFANG_CONFIG_MODE=managed). \ | ||
| This migration re-runs on every boot until the managed source is updated to the new schema." | ||
| ); | ||
| } else if migrated && file_version < CONFIG_VERSION { |
There was a problem hiding this comment.
This managed-mode migration-skip branch (and config_mode() / default_config_path()'s LIBREFANG_CONFIG_PATH handling below) has no unit test in this file — coverage is only indirect, via the API-level tests in crates/librefang-api/tests/config_managed_mode_test.rs, which don't exercise load_config's migration path at all.
A direct test here (write a v1 config under LIBREFANG_CONFIG_MODE=managed, call load_config, assert the in-memory config is migrated but the on-disk bytes are untouched and the warning fires) would be the natural analog of the existing test_load_config_migrates_v1_api_section right above it. The catch is the same one config_managed_mode_test.rs calls out in its own module doc: LIBREFANG_CONFIG_MODE is process-global, and this crate's #[cfg(test)] mod tests runs many other tests in the same binary on parallel threads, so setting the env var here risks bleeding into unrelated tests (e.g. test_load_config_v2_skips_migration, test_strict_mode_*) the same way the API test's own comment says the first version of that file broke five unrelated cases. Given that documented hazard, I'd rather flag it than land a same-file env-mutating test speculatively — worth a deliberate call on serial-test / a dedicated binary / a mutex before adding it, not a five-minute follow-up.
Generated by Claude Code
… the documented gap list (#6737) * fix(api): lock the provider config routes in managed mode `set_provider_key`, `set_provider_url` and `set_default_provider` persisted into `config.toml` without passing through `guard_config_write`, so a managed deployment could still have `[default_model]`, `[provider_urls]` and `[provider_proxy_urls]` rewritten from the dashboard. Each now returns the documented 423 with `code: "config_managed"` before reading anything. The guard sits in the handler rather than in `persist_default_model` / `upsert_provider_url` / `upsert_provider_proxy_url` because those are sync fns returning a boxed error, which the handlers map to a 500 or swallow into a `warn!` — either way losing the 423 and its structured body. `set_provider_key` is refused in full, including its `secrets.env` write. Its config write is conditional on live daemon state the caller cannot observe, so guarding only the write would accept or refuse the identical request depending on timing, having already rewritten `secrets.env` in the refusing case. `set_default_provider` is refused before the OpenRouter / EveryAPI catalog refreshes so a refused request costs no outbound HTTP, and because its persist failure is only a `warn!` — without the guard it would answer 200 and hot-switch the live default against the manifest. `PUT /api/providers/{name}/discovery` stays unguarded: its `atomic_write` targets a per-provider catalog fragment under `providers_dir`, not `config.toml`. Also corrects the Known-gaps section of docs/operations/managed-config.md, which named `routes/memory.rs`, `routes/skills/mod.rs` and `server.rs` without their write sites. All three do write `config.toml` — via `std::fs::write` rather than `crate::atomic_write` — so the section keeps them but now names each route with the exact call that performs the write, adds the sidecar-channel and quick-init routes it had missed, records that the MCP routes only write the file under the default `mcp_runtime_store = "file"`, and drops the inaccurate claim that none of the gap routes take `config_write_lock` (the sidecar routes do). Refs #6695, #6717 * docs(changelog): add fragment for the managed-mode provider route guard * fix(api): rewrap new test comments to one sentence per line CLAUDE.md's prose-wrapping rule requires new prose to break only at sentence boundaries, not at a fixed column. Four `//` comment blocks added by this PR in config_managed_mode_test.rs still hard-wrapped mid-sentence across multiple lines while the new `///` doc comments in the same PR already followed the rule. * docs(managed-config): clarify mcp_runtime_store escape does not cover extension routes The known-gaps table groups the extension install/uninstall routes with the direct MCP-server routes under the same mcp_runtime_store qualifier, but routes/skills/extensions.rs calls upsert_mcp_server_config and remove_mcp_server_config unconditionally, with no mcp_runtime_store check, so setting mcp_runtime_store = "db" does not stop install_extension/uninstall_extension from writing config.toml. --------- Co-authored-by: Claude <noreply@anthropic.com>
Implements the core of #6695. Refs #6695 rather than
Closes— the RFC's acceptance criteria are broader than this PR, and the gaps are listed at the bottom rather than left to be discovered.The four design questions in the RFC are answered in the issue thread; this is those answers built.
The contract
LIBREFANG_CONFIG_PATHconfig.tomlfrom this exact path instead of$LIBREFANG_HOME/config.toml.LIBREFANG_CONFIG_MODEmanagedlocks the file. Anything else — unset, empty, a typo — is mutable.They are independent, which is the RFC's first design question. Relocating a file is not a statement about who owns it: a Compose deployment may reasonably bind-mount the config directory outside
LIBREFANG_HOMEand still want to edit it from the dashboard. Inferring the lock from a custom path hands that operator a read-only UI they never asked for, with no way out short of moving the file back.A typo resolves to mutable. Defaulting a misspelling to the locked mode would take the dashboard away from a deployment that never asked for it; defaulting to mutable preserves existing behaviour, which is the compatibility guarantee the RFC requires.
The mode is read from the process environment and never from the config file, so an API write cannot unlock the very file it is being refused access to.
Enforcement
guard_config_write()inroutes/mod.rsis the single place that decides the status and body:{ "ok": false, "error": "configuration is managed by the deployment", "code": "config_managed", "source": "/etc/librefang/config.toml" }423 Locked, returned before the handler reads the existing file, so a refused write never opens, truncates, or rewrites anything — asserted by comparing the file byte-for-byte after a refusedconfig/set.Enforcement is in the handlers, not on the mount. A read-only mount is still worth having as defence in depth, but it cannot be the mechanism: an
EACCESsurfaces as a500with an errno, which tells an operator nothing about why, and it does not apply at all to a deployment that leaves the file writable while still expecting the manifest to win. That is the RFC's "filesystem read-only errors must never be the enforcement mechanism", and this is the concrete reason it is right.Guarded:
POST /api/config/set, the budget persistence path (PUT /api/budget,PUT /api/providers/{name}/budget, per-user budget routes), and the user persistence path (POST/PUT/DELETE /api/users, key rotation).Provenance
GET /api/config/status, authenticated like every/api/*route:{ "mode": "managed", "source": "/etc/librefang/config.toml", "writable": false, "checksum": "sha256:9f2b…", "modified_at": "2026-08-04T09:41:12+00:00" }writableis what a client branches on — equivalent tomode == "mutable", exposed separately so the dashboard uses a boolean rather than string-matching a mode name. The checksum is over the file's raw bytes and carries no value from inside it.This is the RFC's "the UI must consume server-provided capability metadata rather than infer managed mode from failed writes".
The bug the RFC did not mention
crates/librefang-kernel/src/config.rswrites the migrated config back to disk whenever it loads a file written against an older schema version. Against a read-only ConfigMap mount thatstd::fs::writefails — and because the failure was only awarn!, the migration re-ran silently on every boot, forever, with nothing but a repeating log line.Managed mode now skips the write and logs one targeted warning naming the file and both schema versions. The in-memory config is migrated either way, so nothing is degraded at runtime; what the operator learns is that their manifest is a schema version behind.
I flagged this on the issue as worth adding to the acceptance criteria. It is the case that proves enforcement cannot live in the filesystem.
Verification
cargo clippy --workspace --all-targets -- -D warnings— zero warnings.cargo test -p librefang-api --test config_managed_mode_test— 3 passed.cargo test -p librefang-api --test config_routes_integration— 45 passed.cargo test -p librefang-kernel --lib config— 113 passed.openapi_spec_test,openapi_path_coverage_test,dead_route_audit_test— green;openapi.json, the four SDKs, and theopenapi.sha256baseline regenerated.The managed-mode tests live in their own test binary.
LIBREFANG_CONFIG_MODEis process-global and Rust runs a binary's tests on parallel threads, so the first version of these tests broke five unrelatedconfig_setcases by setting the variable out from under them — a mutex only serializes the tests that take it. A separate file is a separate process, so the blast radius stops there. Keeping them inconfig_routes_integration.rswould reintroduce that, so please don't merge them back.No new dependencies:
sha2,chrono, andserdewere alreadylibrefang-kerneldeps.Deliberately not in this PR
WRITABLE_EXACT_PATHS/WRITABLE_SECTION_PREFIXESis already a field-level model and already load-bearing for security (external_auth.is excluded because of the OIDC login does not require email_verified — allowed_domains can be impersonated #3703 impersonation vector); a second orthogonal axis produces a matrix where the interesting cases are the corners.Known gaps, stated rather than buried
Coverage is not complete. The skills routes (
routes/skills/mod.rs, six sites), the memory route, and the dashboard-credential change inserver.rsalso writeconfig.tomland are not guarded here.Auditing for this turned up something worth its own attention: those same sites do not take
config_write_lockeither, so they can already race a concurrentconfig/setin mutable mode. That is a pre-existing bug this PR surfaces rather than introduces, and adding the lock to seven call sites changes concurrency semantics on paths I have not otherwise touched — which is why it is not bundled here. Happy to do it as a follow-up, or to fold it in if you'd rather it land together.Both gaps are documented in
docs/operations/managed-config.mdunder "Known gaps", so an operator does not discover them by trusting the lock and being wrong.