Skip to content

Conversation

@code-crusher
Copy link
Member

  • Add plan file editing capabilities with new PlanFileEditTool
  • Implement PlanMemoryManager for better memory management
  • Add PlanFileIndicator component for UI feedback
  • Update tool interfaces and type definitions
  • Enhance assistant message presentation
  • Improve storage utilities and webview message handling

Uploading Screenshot 2025-12-29 at 17.56.24.png…

- Add plan file editing capabilities with new PlanFileEditTool
- Implement PlanMemoryManager for better memory management
- Add PlanFileIndicator component for UI feedback
- Update tool interfaces and type definitions
- Enhance assistant message presentation
- Improve storage utilities and webview message handling
@matter-code-review
Copy link
Contributor

matter-code-review bot commented Dec 29, 2025

Summary By MatterAI MatterAI logo

🔄 What Changed

Implemented filename sanitization using path.basename across all PlanMemoryManager operations (writeFile, readFile, hasFile, deleteFile). Added entry.isFile() validation during directory scanning and improved error logging for file deletions.

🔍 Impact of the Change

Hardens the Planning Mode storage logic against directory traversal and ensures memory is only populated with valid file entries, increasing system reliability and security.

📁 Total Files Changed

Click to Expand
File ChangeLog
Storage Hardening src/core/kilocode/PlanMemoryManager.ts Added path.basename sanitization and isFile checks.

🧪 Test Added/Recommended

Recommended

  • Unit Tests: Verify PlanMemoryManager correctly handles malicious path strings (e.g., ../, /etc/) by ensuring they are stripped to the basename.

🔒 Security Vulnerabilities

N/A

Context

Ensuring that the virtual planning memory is strictly confined to its designated directory and only processes valid file types.

Implementation

Wrapped all incoming filename parameters in path.basename() to prevent directory traversal. Updated the readdir loop to filter for files using entry.isFile() and added a check to prevent redundant memory writes if a file is already loaded.

Screenshots

before after
N/A N/A

How to Test

  1. Invoke writeFile with a path containing traversal characters (e.g., test/../../secret.txt).
  2. Verify the file is stored as secret.txt within the plan-memory directory.
  3. Check logs for a warning if deleteFile is called on a non-existent file.

Get in Touch

Axon Code Discord: @matterai_lead_eng

Total Score: 5/5

Copy link
Contributor

@matter-code-review matter-code-review bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧪 PR Review is completed: Review of Plan Mode implementation. Identified critical security issues regarding path traversal in file operations and a race condition in memory initialization.

- Fix path traversal vulnerabilities in writeFile() and deleteFile() using path.basename()
- Fix race condition in loadExistingFiles() that could cause data loss
- Remove .md extension filter to load all file types consistently
- Add filename sanitization to readFile() and hasFile() for consistency
- Prevents arbitrary file write/delete vulnerabilities and data loss
@matter-code-review
Copy link
Contributor

✅ Reviewed the changes: The changes successfully address the previous security and logic concerns. The use of path.basename mitigates path traversal risks, and the race condition in loadExistingFiles is handled correctly by checking for existence after the async read.

@code-crusher code-crusher merged commit ed85a7e into main Dec 29, 2025
6 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants