fix: reject control characters in write tool content params

[aliasunder]

Summary

• Adds a assertNoControlCharacters guard utility that rejects C0 controls (except tab/LF/CR), DEL, and C1 controls in content written to vault notes
• Wires the guard into all 4 data-layer write functions: writeNote (body), patchNote (content), replaceInNote (newText), updateMemory (entry + section)
• Updates MCP tool descriptions with the new error in their Errors: section
• 14 unit tests for the guard + 7 integration tests across the 3 affected test suites

Motivation: A NUL byte written via JSON \uXXXX escape decoding was faithfully persisted to disk, where it silently broke exact-match old_text edits — invisible in vault_read_note output, unmatchable in tool params. Defense-in-depth: a control byte in a markdown note is never intended, and rejecting it with a clear error beats a stuck unmatchable byte on disk.

Test plan

• npm test -- --run src/utils/__tests__/assert-no-control-characters.test.ts — 14 unit tests
• npm test -- --run src/vault-mcp/vault-operations/__tests__/vault-filesystem.test.ts — writeNote rejection
• npm test -- --run src/vault-mcp/vault-operations/__tests__/vault-patcher.test.ts — patchNote + replaceInNote rejection
• npm test -- --run src/vault-mcp/vault-operations/__tests__/memory-store.test.ts — updateMemory entry + section rejection
• npm run lint — 0 errors

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Vault memory updates and note editing operations now reject non-printable control characters in writable text.
  • Validation identifies the invalid character, its location, and the affected field.
  • Tabs, line feeds, carriage returns, empty content, and normal Markdown remain supported.

• Documentation

  • Tool descriptions now explain the control-character validation and related write failures.

• Tests

  • Added coverage for invalid control characters across memory and note-writing workflows.

[sourcery-ai]

Sorry @aliasunder, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[umm-actually]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Document intentional absence of global regex flag The regex uses a character class without the g flag, so exec() returns only the first match — which is correct for reporting the first occurrence. However, the comment in the test "reports first occurrence only" confirms this is intentional. No change needed here, but consider adding a comment above the regex explaining that the absence of g is deliberate to stop at the first control character. src/utils/assert-no-control-characters.ts [3] +// No `g` flag — stop at the first control character to report its position. const CONTROL_CHARACTER_PATTERN = /[\x00-\x08\x0B\x0C\x0E-\x1F\x7F-\x9F]/ Suggestion importance[1-10]: 4 __ Why: Adding a comment explaining the absence of the g flag is a minor documentation improvement; it does not fix a bug or address a security flaw, so its impact is low.	Low
Security	Validate date parameter for control characters The assertNoControlCharacters call for params.entry and params.section is placed after the single-line validation but before the file lock. If either assertion fails, the error is thrown outside the lock, which is correct. However, the params.date field is not validated for control characters. While a date string is unlikely to contain control bytes, an attacker could inject them through the MCP transport. Add assertNoControlCharacters(params.date, "date") to close this gap. src/vault-mcp/vault-operations/memory-store.ts [508-509] assertNoControlCharacters(params.entry, "entry") assertNoControlCharacters(params.section, "section") +assertNoControlCharacters(params.date, "date") Suggestion importance[1-10]: 3 __ Why: While technically a valid defensive addition, the params.date field is tightly constrained to ISO calendar date format and is extremely unlikely to contain control characters; the improvement is marginal and does not address a realistic vulnerability.	Low
Possible issue	Throw instead of silently returning on undefined code point When charCode is undefined (which can't happen for a non-empty string matched by the regex), the function silently returns without throwing — swallowing the error. This violates the project's rule that every catch (or guard failure) must log or re-throw. Since codePointAt(0) on a matched character is guaranteed to return a number, throw an explicit error instead of silently returning. src/utils/assert-no-control-characters.ts [20-22] const charCode = match[0].codePointAt(0) -if (charCode === undefined) return +if (charCode === undefined) { + throw new Error( + `${paramName} contains a control character at position ${match.index} — unable to determine code point`, + ) +} const codePoint = charCode.toString(16).toUpperCase().padStart(4, "0") Suggestion importance[1-10]: 2 __ Why: The codePointAt(0) return on a character matched by the regex is already guaranteed to be a number. The suggestion adds an unnecessary defensive check that has minimal impact on functionality or security.	Low

[aliasunder]

@CodeRabbit review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fce8d5bb-21e7-4bf2-9762-ec655b513253

📥 Commits

Reviewing files that changed from the base of the PR and between b18d960 and e641e6f.

📒 Files selected for processing (12)

• .trivyignore
• AGENTS.md
• src/utils/__tests__/assert-no-control-characters.test.ts
• src/utils/assert-no-control-characters.ts
• src/vault-mcp/mcp-core/tools/memory-tools.ts
• src/vault-mcp/mcp-core/tools/vault-crud-tools.ts
• src/vault-mcp/vault-operations/__tests__/memory-store.test.ts
• src/vault-mcp/vault-operations/__tests__/vault-filesystem.test.ts
• src/vault-mcp/vault-operations/__tests__/vault-patcher.test.ts
• src/vault-mcp/vault-operations/memory-store.ts
• src/vault-mcp/vault-operations/vault-filesystem.ts
• src/vault-mcp/vault-operations/vault-patcher.ts

---

📝 Walkthrough

Walkthrough

Changes

Control-character validation

Layer / File(s)	Summary
Validation guard and coverage src/utils/assert-no-control-characters.ts, src/utils/__tests__/assert-no-control-characters.test.ts, AGENTS.md	Adds a reusable guard rejecting disallowed C0, DEL, and C1 control characters while allowing tab, LF, and CR, with detailed errors and tests.
Write-path enforcement src/vault-mcp/vault-operations/memory-store.ts, src/vault-mcp/vault-operations/vault-filesystem.ts, src/vault-mcp/vault-operations/vault-patcher.ts, src/vault-mcp/vault-operations/__tests__/*	Validates memory entries, sections, note bodies, patch content, and replacement text before persistence operations, with rejection tests.
Tool error documentation src/vault-mcp/mcp-core/tools/memory-tools.ts, src/vault-mcp/mcp-core/tools/vault-crud-tools.ts	Documents control-character failures for memory and vault write tools.

Trivy suppression

Layer / File(s)	Summary
Vulnerability suppression .trivyignore	Adds the GHSA-frvp-7c67-39w9 ignore entry with deployment-context comments.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• aliasunder/vault-cortex#128: Also changes vault note write and edit paths.
• aliasunder/vault-cortex#132: Also changes updateMemory section handling.
• aliasunder/vault-cortex#245: Also tightens validation in the updateMemory write path.

Suggested labels: Review effort 2/5

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: rejecting control characters in writable tool content parameters.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch worktree-control-char-sanitization

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}