feat(security): prevent write_file overwriting non-empty files#4026
Open
1105623876 wants to merge 3 commits intoagentscope-ai:mainfrom
Open
feat(security): prevent write_file overwriting non-empty files#40261105623876 wants to merge 3 commits intoagentscope-ai:mainfrom
1105623876 wants to merge 3 commits intoagentscope-ai:mainfrom
Conversation
|
Hi @1105623876, thank you for your first Pull Request! 🎉 🙌 Join Developer CommunityThanks so much for your contribution! We'd love to invite you to join the official QwenPaw developer group! You can find the Discord and DingTalk group links under the "Developer Community" section on our docs page: We truly appreciate your enthusiasm—and look forward to your future contributions! 😊 We'll review your PR soon. |
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.

Description
This PR adds a file-state-aware tool guard for
write_file.write_fileis intended for creating new files or writing empty files, but using it on an existing non-empty file cansilently overwrite content. This PR adds
WriteFileOverwriteGuardian, which blockswrite_filewhen the target pathalready exists as a non-empty regular file and guides the agent to use
edit_fileinstead.New files and existing empty files are still allowed.
The behavior is configurable through
security.file_guard.prevent_write_file_overwrite, which defaults totrueandalso respects the parent
security.file_guard.enabledswitch.Related Issue: Fixes #4020
Security Considerations: This change strengthens tool-call safety by preventing accidental destructive overwrites
through
write_file. The new guard is controlled bysecurity.file_guard.prevent_write_file_overwriteand alsorespects
security.file_guard.enabled.Type of Change
Component(s) Affected
Checklist
pre-commit run --all-fileslocally and it passespytestor as relevant) and they passFor Channel Changes (DingTalk, Feishu, QQ, Console, etc.)
./scripts/check-channels.sh(or./scripts/check-channels.sh --changed) and it passestests/contract/channels/test_<channel>_contract.py(REQUIRED)create_instance()with proper channel initializationtests/contract/channels/__init__.py)tests/unit/channels/test_<channel>.pyfor complex internal logicTesting
Tested the new guardian behavior with unit tests covering:
write_fileallows new files.write_fileallows existing empty files.write_fileblocks existing non-empty files.edit_fileare not affected.security.file_guard.prevent_write_file_overwriteswitch disables this guard when set tofalse.ToolGuardEngineregisters the new guardian.Also ran the full
tests/unit/securitysuite.Local Verification Evidence
pre-commit run --all-files was not run locally.
Warnings observed during pytest are due to the local environment not having pytest-asyncio installed, so pytest
reports unknown asyncio config options. The tested security cases passed.
Additional Notes
This PR does not extend custom_rules or security.file_guard.sensitive_files. The new guard is a filesystem-state check
rather than a regex rule or sensitive-path rule, so it is implemented as a separate guardian under the file guard
system.