#19510: fix(config): preserve configured values on invalid config validation fallback
size: S
Cluster:
Config Management Improvements
## Summary
Fixes one critical path from #5052 where invalid config currently collapses to `{}` and downstream access control falls back to insecure defaults (e.g. `dmPolicy ?? "pairing"`).
## What changed
- `src/config/io.ts`
- On `validateConfigObjectWithPlugins` failure, `loadConfig()` now uses a best-effort fallback from the resolved config instead of returning `{}`.
- Keeps configured values like `channels.whatsapp.dmPolicy` and `allowFrom` available at runtime.
- Adds a one-time warning that fallback mode is active for the invalid config path.
- `src/config/io.invalid-config-fallback.test.ts`
- Adds regressions for:
- strict schema validation failure (unknown top-level key)
- plugin validation failure (missing plugin path)
- Both assert that `dmPolicy` remains `allowlist` and does not fall through to implicit `pairing`.
## Why
Current behavior can silently drop configured security/access controls when config validation fails, which is the core issue reported in #5052.
## Relation to #9040
This PR takes a **preserve-configured-values** approach for invalid configs.
Open PR #9040 proposes a **fail-closed/throw** approach.
## Testing
- `pnpm test src/config/io.invalid-config-fallback.test.ts`
- `pnpm test src/config/io.compat.test.ts src/config/io.write-config.test.ts`
- `pnpm test src/config/config.plugin-validation.test.ts`
- `pnpm check`
## AI-assisted disclosure
- [x] AI-assisted: yes
- [x] Degree of testing: fully tested for touched paths
- [ ] Session logs/prompts: available on request
- [x] Confirmed understanding of the code changes before submission
Closes #5052
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
This PR addresses a security regression where an invalid config was silently collapsing to `{}` (via a throw/catch cycle with `INVALID_CONFIG`), causing downstream access-control settings like `channels.whatsapp.dmPolicy` to fall back to insecure defaults. The new approach returns a best-effort fallback built directly from the resolved (but invalid) config object rather than an empty object, preserving user-configured values at runtime.
Key changes:
- `loadConfig()` in `src/config/io.ts` now calls `buildBestEffortInvalidConfigFallback(resolvedConfig)` on validation failure instead of throwing a coded error that was immediately caught and discarded.
- `buildBestEffortInvalidConfigFallback` applies `coerceConfig` → `normalizeConfigPaths` → `applyConfigEnvVars` → shell-env fallback → `applyConfigOverrides` to the raw resolved config.
- A module-level `loggedInvalidConfigFallbacks` Set provides one-time warning deduplication, consistent with the existing `loggedInvalidConfigs` Set.
- The old `INVALID_CONFIG`-coded error and its catch-handler are removed; the `INVALID_CONFIG` code in `src/infra/unhandled-rejections.ts` and `src/infra/errors.ts` is now unreachable from `loadConfig` but those handlers remain valid for other potential error sources.
**Main concern:** `buildBestEffortInvalidConfigFallback` skips the entire `apply*Defaults` chain (`applyModelDefaults`, `applyAgentDefaults`, `applyLoggingDefaults`, `applyMessageDefaults`, `applyContextPruningDefaults`, `applyCompactionDefaults`) that the valid-config path applies. Users in fallback mode will silently miss operational defaults (e.g. `agents.defaults.maxConcurrent`, `messages.ackReactionScope`, `logging.redactSensitive`), causing the fallback config to behave differently from a valid config beyond just the invalid keys.
<h3>Confidence Score: 3/5</h3>
- Safe to merge with the understanding that fallback configs will lack runtime defaults; the security regression it fixes is real and the approach is sound in principle.
- The PR correctly fixes the core issue (invalid config collapsing to `{}` and dropping security settings). However, the fallback function omits the `apply*Defaults` chain present in the valid-config path, meaning fallback configs will silently miss operational defaults like agent concurrency limits, logging redaction mode, and model aliases. This is a behavior difference that could surface as subtle runtime failures for users in fallback mode. Tests cover the primary security regression (dmPolicy preservation) but not the defaults gap. The approach is architecturally reasonable; the defaults omission is a clear functional gap to address before merge.
- src/config/io.ts — specifically the `buildBestEffortInvalidConfigFallback` function at line 530 which omits the runtime-defaults chain.
<sub>Last reviewed commit: 5186af4</sub>
<!-- greptile_other_comments_section -->
<!-- /greptile_comment -->
Most Similar PRs
#16991: fix(config): add missing defaults to config snapshot path
by AI-Reviewer-QS · 2026-02-15
77.4%
#5823: fix(config): exit cleanly on invalid config instead of high CPU loop
by gavinbmoore · 2026-02-01
77.3%
#21931: feat(config): auto-rollback to last known-good backup on invalid co...
by Protocol-zero-0 · 2026-02-20
77.1%
#21994: Config: load valid backup when primary config is invalid
by islavutin · 2026-02-20
76.9%
#13960: fix(ui): preserve structured config validation error details
by constansino · 2026-02-11
76.9%
#19670: fix(config): guard config.apply against catastrophic key loss
by nabbilkhan · 2026-02-18
76.9%
#15613: fix(config): align default pipelines across loadConfig and readConf...
by AI-Reviewer-QS · 2026-02-13
76.7%
#18801: fix(routing): use fresh config in resolveAgentRoute to prevent stal...
by mcaxtr · 2026-02-17
76.2%
#22648: fix(plugin-sdk): re-throw non-ENOENT errors in readJsonFileWithFall...
by adhitShet · 2026-02-21
76.0%
#18966: fix(config): downgrade unknown bundled plugin references to warnings
by moxunjinmu · 2026-02-17
75.6%