Repository navigation
fix(attachments): enforce the 3-file limit on every attach path - #1048
Conversation
The per-message limit of 3 attachments was only enforced by the drag-and-drop handler. The file picker capped each selection at 3 but appended to files already in the composer, and the library picker had no count check, so a message could carry any number of files. The chat API did not check either. Apply the same remaining-slot check to the file picker and library attach, and reject submissions with more than 3 file parts in the chat API. Co-authored-by: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3c6306449
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const latestMessage = | ||
| message ?? (Array.isArray(messages) ? messages.at(-1) : undefined) | ||
| if (countFileParts(latestMessage) > MAX_ATTACHMENTS_PER_MESSAGE) { |
There was a problem hiding this comment.
When guest chat is enabled, a caller can submit a harmless message with at most three files while placing an oversized latest user message in messages. This check always prefers message, but the guest branch later ignores it and sends messages to createEphemeralChatStreamResponse, so an unauthenticated client can bypass the server-side attachment limit and still send an arbitrarily large set of file parts to the model. Validate the payload consumed by the selected branch, or require the two representations to agree.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch. Fixed in 853a225: the check now runs on every message in messages for guests, since that is what the ephemeral path sends to the model. For authenticated users it checks message on submit-message only, so regenerating a message saved before this limit was enforced is not rejected. Covered by the new exceedsAttachmentLimit tests.
Written by an agent (Claude Code, claude-opus-5-5).
Guests send their full history in `messages` and the ephemeral path feeds all of it to the model, so checking only `message` could be bypassed. Check every guest message, and for authenticated users only the submitted message so regenerating an older one keeps working. Co-authored-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 853a225405
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| : trigger === 'submit-message' | ||
| ? [message] | ||
| : [] |
There was a problem hiding this comment.
An authenticated caller can bypass the new limit by sending trigger: "regenerate-message" with an over-limit message whose ID matches messageId. This branch checks nothing, but prepareMessages treats that payload as a user-message edit, persists it with upsertMessage, and returns it to the model. Fresh evidence beyond the previously fixed guest-history path is the regeneration logic in lib/streaming/helpers/prepare-messages.ts, which uses the supplied message when the IDs match; validate that message here as well while still allowing regeneration requests that do not replace the stored message.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in d30c07c. The user-message edit path in prepareMessages now throws DeterministicPreparationError before upsertMessage when the supplied message has more than 3 file parts and more than the stored message. Plain regeneration, which resends the stored message unchanged, still works for messages saved before the limit was enforced. Covered by two new prepare-messages tests.
Written by an agent (Claude Code, claude-opus-5-5).
A regenerate request whose message id matches the stored user message is treated as an edit, persisted, and sent to the model, so it bypassed the submit-only check. Reject such an edit when it exceeds the limit and carries more files than the stored message, so messages saved before the limit was enforced can still be regenerated. Co-authored-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d30c07c284
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| : trigger === 'submit-message' | ||
| ? [message] | ||
| : [] |
There was a problem hiding this comment.
When an authenticated client—or a deployment using ENABLE_AUTH=false—omits trigger or supplies an unrecognized value, this selects an empty list even if message contains more than three files. The route does not reject unknown triggers, and prepareMessages treats every non-regeneration trigger as a normal submission, so the oversized message is persisted and sent to the model. Reject unknown trigger values in the request schema, or validate message for every non-regeneration path.
AGENTS.md reference: AGENTS.md:L130-L130
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6155b87. For authenticated requests the check now applies to message on every trigger except regenerate-message, matching how prepareMessages treats every other trigger as a submission. Regenerate stays covered by the stored-message comparison in prepareMessages. Added a test for a missing and an unknown trigger.
Written by an agent (Claude Code, claude-opus-5-5).
prepareMessages treats any trigger other than regenerate-message as a submission, so a missing or unknown trigger skipped the check while the message was still persisted and sent to the model. Co-authored-by: Claude <noreply@anthropic.com>
This branch was successfully deployed
1 participant
Summary
The composer is supposed to allow at most 3 attachments per message, but the limit was not actually enforced. Only one of the three attach paths checked it, and the server did not check it at all, so a single message could carry far more than 3 files.
/api/chatbad_requestwhen a submitted message has more than 3 file parts. For guests every message inmessagesis checked, since that is the history the model receives. A regenerate request that edits a stored message is rejected when it adds files beyond the limit, while regenerating a message saved before this fix still worksBecause every file part is sent to the model in full, the missing limit also let one message push a very large context into a single request.
Changes
lib/utils/attachment-limits.ts:MAX_ATTACHMENTS_PER_MESSAGE = 3,countFileParts,remainingAttachmentSlotscomponents/chat-panel.tsx: remaining-slot check for the file picker and library attachhooks/use-file-dropzone.ts: uses the shared constant and the same error messageapp/api/chat/route.ts: server-side rejectionlib/streaming/helpers/prepare-messages.ts: rejects over-limit edits sent via regenerateTest plan
chat-paneltests for the file picker (partial fill and full composer) and library attach. All 3 fail againstmainand pass with this changeattachment-limitsunit tests, including the guestmessagespath and regeneratebun run test,bun lint,bun typecheck,bun format:check,bun run build