fix(ui): send task messages from plain-http origins - #13428
capo-the-ai-bot wants to merge 2 commits into
Conversation
Browsers expose crypto.randomUUID only in secure contexts (https or localhost). On a LAN or tailnet origin the task composer's submit path threw while generating the comment clientRequestId, the composer's own catch swallowed the error and restored the text, and nothing was sent. No toast appeared, so the Send button looked dead. Move the schema-valid fallback generator that issue-execution-policy already carried into a shared randomUuid() helper. It uses the native function when present and otherwise builds a v4 UUID from getRandomValues. Route the task composer, the classic composer, the page-level fallback, and the other direct crypto.randomUUID() call sites (decision resolution, the externally connected task banner, provider credential setup, email endpoint setup) through it. Refs paperclipai#3529
![superagent-security[bot]](https://avatars.githubusercontent.com/in/3287076?s=60&v=4){kind=small}
| bytes[index] = Math.floor(Math.random() * 256); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
P3: Do not use Math.random as a fallback for security-sensitive identifiers
The shared UUID helper uses predictable Math.random bytes when Web Crypto is unavailable.
Require a CSPRNG for idempotency and credential identifiers; do not fall back to Math.random.
AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.
<file name="ui/src/lib/random-uuid.ts">
<violation number="1" location="ui/src/lib/random-uuid.ts:24">
<priority>P3</priority>
<title>Do not use Math.random as a fallback for security-sensitive identifiers</title>
<evidence>When globalThis.crypto and getRandomValues are unavailable, the new shared helper fills UUID bytes with Math.random(). This helper now supplies client request IDs, idempotency keys, and provider secret-definition keys, so those identifiers can be predictable in environments without Web Crypto.</evidence>
<recommendation>Remove the Math.random fallback for identifiers used in idempotency or credential-related flows. Prefer crypto.getRandomValues and fail with a clear error when no CSPRNG exists, or use a platform-provided secure randomness source before generating the UUID.</recommendation>
</violation>
</file>
There was a problem hiding this comment.
Fixed in 2fb6c25. The helper now throws a clear error when crypto.getRandomValues is unavailable instead of filling bytes from Math.random. Every supported browser exposes getRandomValues, including insecure contexts, so the throw is unreachable in practice. The tests cover the fallback path, distinct ids across calls, and the error.
There was a problem hiding this comment.
Fix accepted. Removing the Math.random fallback and throwing when crypto.getRandomValues is unavailable correctly eliminates predictable identifier generation for security-sensitive flows. This fail-secure approach is appropriate since modern browsers universally expose getRandomValues, and the added tests validate the error path and uniqueness guarantees.
 |
The shared helper filled UUID bytes from Math.random when neither crypto.randomUUID nor crypto.getRandomValues existed, a branch carried over from the execution-policy generator. These IDs serve as idempotency keys and secret-definition suffixes, so predictable bytes are wrong even on that unreachable path. Throw a clear error instead; every supported browser exposes getRandomValues, including insecure contexts.
|
Confirmed on 2026.916.0. On a LAN instance over plain HTTP, task comments did not send, with no error. A local build with the same fallback approach fixed it. This is probably also the main cause of #13767. The versions match, and the silent One call site is not in this PR: |
|
I hit this on a self-hosted board opened over plain HTTP from another machine on the LAN ( One gap appears after a rebase. onClick={() => change({ headers: [...s.headers, { id: crypto.randomUUID(), name: "", value: "" }] })}On a plain-HTTP origin this throws, and the button does nothing. Please replace it with |
2fb6c25 to
0d7080c
Compare
|
Hey @capo-the-ai-bot! Before this PR can be reviewed, a few things need attention: Informational:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
This branch has not been deployed
3 participants
Thinking Path
Linked Issues or Issue Description
No public issue is filed for the send failure. Related references:
getRandomValuesfallback generator for execution policy stage IDs. This PR moves that generator into a shared helper and reuses it.Bug report fields:
What happened?
On a Paperclip instance opened at
http://<lan-host>:3100or a tailnet host, typing a message on the task page and clicking Send does nothing. The text clears and comes back. No comment is posted and no toast appears. The same click works onhttp://localhost:3100.Expected behavior
The message posts on every origin the UI is served from.
Steps to reproduce
paperclipaiand open the UI from a different machine over plain http, so the URL is notlocalhostor127.0.0.1.POST /api/issues/:id/commentsis never sent.Local reproduction without a second machine: open a task on a dev server, run
delete Crypto.prototype.randomUUIDin the browser console, then send.Paperclip version or commit
Reproduced on
f2c5e54dc(master, 2026-09-14). The composer submit path has contained a directcrypto.randomUUID()call since #13038.Deployment mode
Self-hosted (
local_trustedorauthenticated) with the UI opened over plain http on a non-loopback host. Verified on a local embedded-Postgres instance with the function removed from the page.What Changed
ui/src/lib/random-uuid.ts(new):randomUuid()returnscrypto.randomUUID()when the browser exposes it, and otherwise builds an RFC 4122 v4 UUID fromcrypto.getRandomValues. The output passes the server'sz.string().uuid()validators. Without any CSPRNG it throws a clear error instead of producing predictable identifiers.ui/src/lib/issue-execution-policy.ts: the privatenewId()generator moves to the shared helper.ui/src/components/task-chat/TaskChatComposer.tsx: submission attempt IDs and local attachment IDs use the helper. This is the task page Send button.ui/src/components/IssueChatThread.tsxandui/src/pages/IssueDetail.tsx: the classic composer and the page-levelclientRequestIdfallback use the helper.ui/src/components/DecisionResolver.tsx,ui/src/components/chat/ExternallyConnectedTaskBanner.tsx,ui/src/lib/provider-credential.ts,ui/src/pages/apps/chat/EmailEndpointSetup.tsx: the remaining directcrypto.randomUUID()calls use the helper. Each one fails the same way on plain http.ui/src/lib/random-uuid.test.tscovers the native path, thegetRandomValuesfallback, distinct ids across calls, and the error raised without a CSPRNG.TaskChatComposer.test.tsxgains a send withcrypto.randomUUIDabsent. It asserts that the posted client request ID is a valid v4 UUID and that the draft is cleared.Verification
pnpm --filter @paperclipai/ui typecheck: clean.pnpm exec vitest run ui/src/lib/random-uuid.test.ts ui/src/lib/issue-execution-policy.test.ts ui/src/components/task-chat/TaskChatComposer.test.tsx ui/src/components/IssueChatThread.test.tsx ui/src/components/TaskChatThread.test.tsx ui/src/pages/IssueDetail.test.tsx ui/src/components/DecisionResolver.test.tsx ui/src/components/chat/ExternallyConnectedTaskBanner.test.tsx: 8 files, 462 tests pass.delete Crypto.prototype.randomUUIDin the console, type a message, click Send. Before the fix: no request, text restored, no toast. After the fix:POST /api/issues/:id/commentsreturns 201, the bubble renders, and the stored comment'sclientRequestIdis a valid v4 UUID.Risks
getRandomValues, a CSPRNG, and carry the v4 version and variant bits. Server validators that require a UUID accept them.RunnerGoalWidgetandcross-tab-pollkeep their own non-UUID fallbacks. Their IDs never reach a UUID validator, so they are unchanged.Math.random. The execution-policy generator it replaces did. A browser withoutgetRandomValuesnow gets a clear error instead of a predictable ID; no supported browser lacks it.Model Used
claude-fable-5-1) through Claude Code (desktop app), with extended thinking and tool use: shell, file edits, and an in-app browser driving a local Paperclip instance. It produced the diagnosis, the code, the tests, and this description.Checklist
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue template#NNN/github.com/paperclipai/paperclipURLs)docs/...,fix/...) and contains no internal Paperclip ticket id or instance-derived details