fix: prevent statusline script from corrupting settings.json#3091
Merged
Conversation
Some models generate shell commands with complex quoting (e.g. single-quote escaping like '\'') that break JSON syntax when written to settings.json, causing qwen-code to fail to start with a FatalConfigError. This adds four layers of defense: 1. **Agent prompt** (builtin-agents.ts): Require commands using jq/pipes/quotes to be saved as script files instead of inline in settings.json. Mark examples as script-only to prevent models from copying them inline. 2. **Write validation** (commentJson.ts): Validate JSON output before writing to disk in updateSettingsFilePreservingFormat. 3. **Startup recovery** (settings.ts): When settings.json has invalid JSON, try .orig backup first, then degrade gracefully to empty settings instead of crashing. Rename corrupted file to .corrupted for manual recovery. Show warning to user via migrationWarnings. 4. **Test update** (settings.test.ts): Update test to verify graceful degradation behavior instead of expecting FatalConfigError. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📋 Review SummaryThis PR implements a comprehensive four-layer defense against settings.json corruption caused by complex shell quoting in statusline commands. The changes are well-structured, addressing prevention, write-time validation, startup recovery, and user notification. The implementation demonstrates excellent defensive programming practices and significantly improves resilience. 🔍 General Feedback
🎯 Specific Feedback🔴 CriticalNo critical issues identified. 🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
BZ-D
reviewed
Apr 10, 2026
BZ-D
reviewed
Apr 10, 2026
1. Backup recovery now surfaces warning via migrationWarnings (reviewer: P2 correctness) 2. Corrupted file uses timestamped suffix to avoid overwriting (reviewer: P2 robustness) 3. Remove misleading underscore prefix on used catch variable (reviewer: P2 code quality) 4. updateSettingsFilePreservingFormat returns boolean (reviewer: P2 correctness) 5. Add 3 new tests: backup recovery, both-corrupted, rename-failure (reviewer: P2 testing) 6. Consistent shebang lines in agent prompt examples (reviewer: P3 nit) 7. Improve catch block error message for backup recovery (reviewer: P2 correctness) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move warningMsg construction after renameSync so the message accurately reflects the outcome: "renamed to X" on success, "fix manually" on failure. Add assertion to rename-failure test verifying the fallback message. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
tanzhenxin
approved these changes
Apr 11, 2026
tanzhenxin
left a comment
tanzhenxin
left a comment
Collaborator
There was a problem hiding this comment.
Review
Well-structured defense-in-depth fix for the statusline agent corrupting settings.json. Prompt prevention, write-time validation, and startup recovery — each layer independent and well-tested.
Verdict
APPROVE
xaelistic
pushed a commit
to xaelistic/qwen-code
that referenced
this pull request
Jun 7, 2026
…3091) * fix: prevent statusline script from corrupting settings.json Some models generate shell commands with complex quoting (e.g. single-quote escaping like '\'') that break JSON syntax when written to settings.json, causing qwen-code to fail to start with a FatalConfigError. This adds four layers of defense: 1. **Agent prompt** (builtin-agents.ts): Require commands using jq/pipes/quotes to be saved as script files instead of inline in settings.json. Mark examples as script-only to prevent models from copying them inline. 2. **Write validation** (commentJson.ts): Validate JSON output before writing to disk in updateSettingsFilePreservingFormat. 3. **Startup recovery** (settings.ts): When settings.json has invalid JSON, try .orig backup first, then degrade gracefully to empty settings instead of crashing. Rename corrupted file to .corrupted for manual recovery. Show warning to user via migrationWarnings. 4. **Test update** (settings.test.ts): Update test to verify graceful degradation behavior instead of expecting FatalConfigError. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address review comments on statusline JSON corruption fix 1. Backup recovery now surfaces warning via migrationWarnings (reviewer: P2 correctness) 2. Corrupted file uses timestamped suffix to avoid overwriting (reviewer: P2 robustness) 3. Remove misleading underscore prefix on used catch variable (reviewer: P2 code quality) 4. updateSettingsFilePreservingFormat returns boolean (reviewer: P2 correctness) 5. Add 3 new tests: backup recovery, both-corrupted, rename-failure (reviewer: P2 testing) 6. Consistent shebang lines in agent prompt examples (reviewer: P3 nit) 7. Improve catch block error message for backup recovery (reviewer: P2 correctness) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: warningMsg says "renamed" even when rename fails Move warningMsg construction after renameSync so the message accurately reflects the outcome: "renamed to X" on success, "fix manually" on failure. Add assertion to rename-failure test verifying the fallback message. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
3 participants
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.
Problem
When a user runs
/statusline 显示模型ID, thestatusline-setupagent generates a shell command and writes it to~/.qwen/settings.json. Some models produce commands with complex shell quoting (e.g.'\'') that is not valid inside a JSON string value, corruptingsettings.jsonand making qwen-code unable to start:Root cause chain
/statusline 显示模型IDstatusline-setupagent generates a command like:bash -lc 'input=$(cat); model_id=$(echo "$input" | jq -r '\''(.metrics.models | keys[0])'\'')'WriteFileToolto write this directly intosettings.json'\''sequences are invalid JSON escape characters →settings.jsonbecomes unparseableloadSettings()callsJSON.parse()which throws →FatalConfigError→ app won't startFix
Four layers of defense, from prevention to recovery:
builtin-agents.tscommentJson.tsupdateSettingsFilePreservingFormat. Note: this does NOT protect the main attack path (agent usesWriteFileTooldirectly), but guards thesaveSettings()codepath as defense-in-depthsettings.tsJSON.parsefails: try.origbackup → if unavailable or also invalid, rename corrupted file to.corruptedand continue with empty settings → show user-visible warning viamigrationWarningswith recovery instructionssettings.test.tsFatalConfigErrorWhy not validate at
WriteFileToollevel?WriteFileToolis a general-purpose file writer — adding JSON validation there would be too invasive and settings-specific. The prompt-level prevention is the primary defense; the startup recovery is the safety net.Tradeoff: graceful degradation vs crash
Previously, corrupted
settings.jsonthrewFatalConfigError(app won't start). Now it degrades to empty settings (app starts, but user config is temporarily lost). The corrupted file is preserved as.corruptedfor manual recovery, and a warning is displayed to the user:This tradeoff favors availability (app starts) over strictness (crash on bad config), which is appropriate because the corruption is caused by the tool itself, not by the user.
Test plan
cliandcorepackages~/.qwen/settings.json, verify app starts with warning/statusline 显示模型IDwith a weaker model, verify it uses script file🤖 Generated with Claude Code