Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The settings UI promises sensitive diagnostics are off by default in all three locales (zh-CN/zh-TW/en-US: "默认关闭" / "Off by default"), and the capability docs say "Enable only when explicitly needed and after reviewing the privacy risk". The backend defaults say the opposite:
AppLoggingConfig::include_sensitive_diagnosticsused#[serde(default = "default_true")], so a config file without the key deserialized totrueDefaultimpl setinclude_sensitive_diagnostics: trueai-adaptersruntimeAtomicBoolstarted attrue(fail-open before the config layer applied the saved preference)FrontendLogLevelSyncfallbacks (3 sites) and thelogger.tsinitial value usedtrue, while the settings page fallback used?? false, so even the frontend disagreed with itselfNet effect for a fresh install: prompts, model request/response payloads, tool arguments and file paths get written to local logs even though the UI says they don't, until the user explicitly turns the switch off.
Fixes #3213
Changes
config-contracts/types.rs: serde default andDefaultimpl now producefalse; the pinned defaults test flips with them (app_logging_defaults_to_sensitive_diagnostics_disabled)ai-adapters/diagnostics.rs:AtomicBoolstartsfalse(fail closed until the config layer applies the saved preference)FrontendLogLevelSync.ts: the three no-saved-preference fallbacks returnfalse, matching the settings page and the backendlogger.ts: initial module valuefalsewith a commentExplicit user values (either polarity) are untouched everywhere: the config manager still applies the saved preference on load and on change, and the toggle still works the same. Only the no-preference default flips, to the value every user-facing surface already documents.
Testing
Red-green: renamed and flipped the Rust defaults test first (
assert!(config.include_sensitive_diagnostics)fails on base), then applied the fix; the two new vitest cases inFrontendLogLevelSync.test.tspin the off-by-default fallbacks (both fail on base withexpected false, received true).cargo test -p openbitfun-config-contracts --lib: 92 passedcargo test -p openbitfun-ai-adapters --lib: 287 passed (includes the diagnostics toggle test and the request-body logging tests, which set the flag explicitly)cargo test -p openbitfun-core --lib service::config: 58 passed (includes the export round-trip fixture that sets the preference explicitly)vitest run src/infrastructure/config: 532 passed across 65 filescargo fmt --checkclean on both touched crates; eslint exit 0 on the touched TS files; the prettier warnings on these files are pre-existing (base versions report identically)AI-assisted with human review; testing level: fully tested (all suites above run locally against this branch).