Skip to content

fix(api): bound message writes across REST and MCP - #445

Merged
duyetbot merged 1 commit into
mainfrom
fix/security-message-limits
Sep 16, 2026
Merged

duyetbot merged 1 commit into
mainfrom
fix/security-message-limits

Conversation

@duyetbot

@duyetbot duyetbot commented Sep 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Closes [audit] P2: message writes are unbounded — no array cap, no content length limit (write amplification) #435 by enforcing 100 messages per write, 64 KiB UTF-8 per content string, and 1 MiB combined content across REST create/append, trace ingestion, and MCP store_conversation.
  • Preserve existing validation responses: REST 400 BAD_REQUEST and MCP INVALID_PARAMS tool errors. Empty initial arrays remain allowed; append/trace arrays must be non-empty.
  • Insert valid batches in transactional D1 chunks of five rows (at most 85 bound parameters with the current 17-column schema).
  • Document the limits and add regression coverage for boundaries, Unicode, rejected-write side effects, order/counters, chunk rollback, and generated SQL bind counts.

Verification

  • API lint and TypeScript check: passed.
  • Full API suite before the final bind-count test: 497/497 passed.
  • Final targeted message-limits suite: 29/29 passed.
  • Test formatting and final API TypeScript check: passed.

Scope

  • Preserves and completes the existing WIP on fix/security-message-limits.
  • No release-please changes. Leave this PR open; do not merge.

Co-Authored-By: Duyet Le me@duyet.net
Co-Authored-By: duyetbot bot@duyet.net

🤖 Generated with Claude Code

Summary by Sourcery

Bound message writes across all supported API ingestion paths and make large valid batches transactional within D1 limits.

Bug Fixes:

  • Enforce bounded message writes across REST conversation creation and appends, trace ingestion, and the MCP conversation storage tool.
  • Prevent partial message persistence and preserve conversation counters when multi-chunk writes fail.

Enhancements:

  • Centralize message insertion into transactional D1 batches that remain within binding limits while preserving message order and trace relationships.

Documentation:

  • Document message count, per-content UTF-8 byte, combined-content, and empty-batch validation behavior for REST and MCP APIs.

Tests:

  • Add regression coverage for message-limit boundaries, Unicode byte sizing, rejected-write side effects, ordering and counters, chunk rollback, and SQL bind counts.

Cap writes at 100 messages, 64 KiB UTF-8 per content string, and 1 MiB combined content. Insert valid batches transactionally in D1-safe chunks and cover boundary rejection, counters, ordering, bind limits, and rollback.

Refs #435

Co-Authored-By: Duyet Le <me@duyet.net>
Co-Authored-By: duyetbot <bot@duyet.net>

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @duyetbot, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 5 days and 22 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 54cb7bf1-dd87-423a-9729-2588f060333b


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

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

@sourcery-ai

sourcery-ai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR enforces shared 100-message, 64 KiB-per-string, and 1 MiB-combined UTF-8 limits across REST, trace, and MCP writes, then routes all message persistence through transactional five-row D1 batches to stay under bind limits; documentation and comprehensive boundary/atomicity regression tests accompany the implementation.

Sequence diagram for transactional message insertion

sequenceDiagram
    participant API as REST, Trace, or MCP handler
    participant Service as Conversation or message service
    participant Insert as insertMessageRows
    participant D1 as D1 database

    API->>Service: Validate bounded message input
    Service->>Insert: insertMessageRows(rows)
    Insert->>D1: db.batch(5-row insert statements)
    D1-->>Insert: Commit all chunks or roll back
    Insert-->>Service: Insertion result
    Service-->>API: Success or database error
Loading

Flow diagram for bounded message writes

flowchart LR
    Write["REST create or append, trace ingestion, or MCP store_conversation"] --> Schema["Shared validation"]
    Schema --> Limits["100 messages; 64 KiB per content; 1 MiB combined UTF-8"]
    Limits -->|valid| Rows["Build message rows"]
    Limits -->|invalid| Error["REST 400 BAD_REQUEST or MCP INVALID_PARAMS"]
    Rows --> Batch["insertMessageRows"]
    Batch --> D1["Transactional D1 batches of 5 rows"]
Loading

File-Level Changes

Change Details Files
Centralize validation of message count and UTF-8 content-size limits across all write entry points.
  • Define reusable per-message and batch limits of 100 messages, 64 KiB per content string, and 1 MiB combined content.
  • Apply bounded schemas to REST create/append and trace observations.
  • Reuse the same content and batch validation in MCP conversation storage while preserving empty-create and non-empty append/trace semantics.
  • Retain REST and MCP validation error contracts and document the limits.
packages/api/src/lib/validation.ts
packages/api/src/routes/mcp/tools.ts
docs/api-reference.md
Make message persistence safe for D1 bind limits while preserving transactional writes.
  • Insert message rows in batches of five, keeping each generated statement within the 100-parameter D1 limit for the 17-column schema.
  • Route REST, MCP, trace, and append message writes through the shared chunked insertion helper.
  • Use D1 batch transaction semantics so later-chunk failures roll back earlier inserts.
packages/api/src/services/messages.ts
packages/api/src/services/conversations.ts
packages/api/src/services/mcp-conversations.ts
packages/api/src/services/traces.ts
Add regression coverage for validation boundaries, write atomicity, and chunked insertion behavior.
  • Cover count, ASCII and multibyte UTF-8 boundaries, combined-size limits, empty-input semantics, and all REST/MCP/trace paths.
  • Verify rejected requests leave conversations, messages, counters, and tokens unchanged.
  • Verify 100-message writes preserve order, parent references, counters, and token totals across chunks.
  • Assert generated SQL bind counts and rollback behavior when a later chunk fails.
packages/api/test/message-limits.test.ts

Assessment against linked issues

Issue Objective Addressed Explanation
#435 Enforce a maximum number of messages per write request and a maximum UTF-8 byte length for each message's content, including embedded messages during conversation creation and message appends.
#435 Prevent write amplification from batches whose combined message content is excessively large, and apply equivalent limits to other message-writing paths such as trace ingestion and MCP conversation storage.
#435 Document the new payload limits and ensure rejected requests preserve existing validation behavior without partially writing data.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@duyetbot
duyetbot merged commit f8f053a into main Sep 16, 2026
6 checks passed
@duyetbot
duyetbot deleted the fix/security-message-limits branch September 16, 2026 21:10
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.

[audit] P2: message writes are unbounded — no array cap, no content length limit (write amplification)

1 participant