Skip to content

Commit db269f4

Browse files
committed
Fix #270 review: use Web Crypto RNG, soften doc claim, reword stale comments
1 parent 0bf33c5 commit db269f4

3 files changed

Lines changed: 12 additions & 9 deletions

File tree

‎apps/web/src/__tests__/postmark-webhook.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1675,7 +1675,7 @@ describe('Postmark inbound webhook', () => {
16751675

16761676
// ── MailboxHash is not a capability token (issue #189) ───────────────
16771677
//
1678-
// `generateTicketId()` is 8 chars of Math.random(), and the Discord/Slack
1678+
// `generateTicketId()` is short (8 chars) and the Discord/Slack
16791679
// bots posted "Ticket TKT-XXXXXXXX created" into public threads. Display
16801680
// IDs are therefore harvestable, so the plus-address path is gated the
16811681
// same way the header path is: EMAIL-sourced ticket, participating sender.

‎apps/web/src/app/api/webhooks/postmark/route.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
* BOTH signals are attacker-supplied, so both paths resolve through the same
1818
* two gates: the ticket must be `source: 'EMAIL'`, and the sender must already
1919
* be a participant on it. `MailboxHash` is a token we mint, but minting it does
20-
* not make it a secret — `generateTicketId()` is 8 characters of `Math.random()`
21-
* and the Discord/Slack bots published "Ticket TKT-XXXXXXXX created" into public
20+
* not make it a secret — `generateTicketId()` is short (8 characters) and the
21+
* Discord/Slack bots published "Ticket TKT-XXXXXXXX created" into public
2222
* threads, so display IDs are harvestable. Without the gates, anyone holding one
2323
* could append to (and reopen) any ticket on any channel.
2424
*
@@ -66,7 +66,7 @@ const REPLY_TARGET_SELECT = {
6666
* Resolve a plus-addressed `MailboxHash` to the ticket it names.
6767
*
6868
* A display ID is not a secret. `generateTicketId()` draws 8 characters from a
69-
* 32-character alphabet with `Math.random()`, and the Discord/Slack bots posted
69+
* 32-character alphabet, and the Discord/Slack bots posted
7070
* "Ticket TKT-XXXXXXXX created" into public threads — those threads still carry
7171
* the IDs. So naming a ticket is not evidence of belonging to it, and this path
7272
* gets exactly the gates the header path has:

‎packages/outpost/shared/src/utils.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,23 @@
1-
import { randomInt } from 'node:crypto';
21
import { TICKET_ID_PREFIX, BACKOFF_BASE_MS, BACKOFF_MAX_MS } from './constants.js';
32

43
/**
54
* Generate a unique ticket ID in the format TKT-XXXXXXXX.
65
* Uses 8 random characters from a 32-char alphabet (~1.1 trillion keyspace)
76
* to make collisions negligible at scale.
87
*
9-
* Drawn from `crypto.randomInt` (CSPRNG): display IDs are pasted into public
10-
* threads and are therefore harvestable, so a non-crypto RNG would let an
11-
* attacker shrink the search space for ID enumeration.
8+
* Drawn from Web Crypto (`globalThis.crypto.getRandomValues`, a CSPRNG, so it
9+
* works in both Node and browser bundles): display IDs are short and public,
10+
* so keeping them unguessable is good hygiene. Authorization still lives at
11+
* the gates — knowing an ID alone grants nothing.
1212
*/
1313
export function generateTicketId(): string {
1414
const chars = 'ABCDEFGHJKLMNPQRSTUVWXYZ23456789'; // Omit ambiguous chars
15+
const bytes = new Uint8Array(8);
16+
globalThis.crypto.getRandomValues(bytes);
1517
let id = '';
1618
for (let i = 0; i < 8; i++) {
17-
id += chars[randomInt(chars.length)];
19+
// 256 is an exact multiple of 32, so masking is uniform.
20+
id += chars[bytes[i]! & 31];
1821
}
1922
return `${TICKET_ID_PREFIX}-${id}`;
2023
}

0 commit comments

Comments
 (0)