Skip to content

Commit 7c7efa7

Browse files
ralyodioclaude
ralyodio
and
claude
authored
fix(security): close the wave-2 advisories (RLS, XSS sink, token exposure, KDFs) (#252)
Addresses the ten advisories left open after wave 1 (#244-#251). Application: - MessageItem no longer passes raw message content to dangerouslySetInnerHTML. The sink is currently unreachable because the format detection reads fields the detection library does not return, but any fix to that naming would have turned it into stored XSS against every recipient. (GHSA-9jq8-g7m8-wg4v) - /admin no longer serialises autoblog bearer tokens into the RSC payload; the page ships a non-reversible hint and the token is shown once, at creation. (GHSA-75px-j56m-6cpr) - /api/conversations/delete now lets a participant delete only rows they own, whatever the conversation type. The shared conversation is garbage-collected once the last participant leaves, so a single member of a direct conversation can no longer destroy the other party's ciphertext and files. (GHSA-xm8x-rpr6-4j7h) - The CoinPay OAuth callback requires email_verified before linking an identity to an existing account. A missing claim does not count as verified. (GHSA-j2m4-m3w7-w2cq) - getClientIp only reads forwarding headers when a trusted proxy is configured, and the count now defaults to 1 rather than 0. Previously an unset count meant X-Real-IP was honoured unconditionally, so rotating that header minted a fresh rate-limit bucket per request and defeated every limiter including the SMS ones. (GHSA-64m7-3h2w-2qr6) - PBKDF2 work factor raised from 100k to OWASP's 600k, and the backup-PIN scrypt from N=2^17. Key exports record the iteration count they were written with so existing files still import. (GHSA-29hv-86qw-3vx5) - /api/users/search runs as the service role after verifying the session, which is what lets the users policy below be narrowed, and only matches phone numbers once the query is long enough to not be an existence oracle. Database (20260816120000): - users_select_authenticated was USING (true): any authenticated account, including a throwaway anonymous sign-in, could read every user's phone_number and salt. Narrowed to the caller's own row plus users they share a conversation with. (GHSA-7w99-6w89-2926) - conversation_participants INSERT required only a non-NULL auth.uid(), so anyone could inject themselves into any conversation. Restricted to the conversation's creator or an existing participant. (GHSA-vxcr-4mm8-jfm3) - conversations carried a FOR SELECT USING (true) policy with no role clause, readable by anon on any fresh deploy. (GHSA-9jgr-3h36-9748) - conversation_participants was likewise USING (true), exposing the whole social graph. Scoped to conversations the caller is in. (NEW-02) - get_inactive_participants returns full phone numbers; EXECUTE revoked from authenticated, leaving the one server-side service-role caller. (GHSA-ffpr-xfm2-pp84) The policy helpers are SECURITY DEFINER so that policies referencing each other's tables do not re-enter one another, which is what this schema's earlier "fix recursion" migrations were fighting. 497 tests pass; the build is clean. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent ef19304 commit 7c7efa7

15 files changed

Lines changed: 489 additions & 177 deletions

File tree

‎src/app/admin/integrations-form.jsx‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ export function IntegrationsManager({ initial }) {
1313
const [kind, setKind] = useState("crawlproof");
1414
const [name, setName] = useState("Crawlproof");
1515
const [origin, setOrigin] = useState("");
16-
const [revealed, setRevealed] = useState({});
1716
const [copied, setCopied] = useState(null);
1817
const [error, setError] = useState(null);
1918
const [justCreatedToken, setJustCreatedToken] = useState(null);
@@ -114,8 +113,6 @@ export function IntegrationsManager({ initial }) {
114113
) : (
115114
<ul style={{ listStyle: "none", padding: 0, display: "flex", flexDirection: "column", gap: "0.5rem" }}>
116115
{items.map((it) => {
117-
const show = !!revealed[it.id];
118-
const masked = `${it.access_token.slice(0, 8)}…${it.access_token.slice(-4)}`;
119116
return (
120117
<li key={it.id} style={{ borderRadius: "0.375rem", border: `1px solid ${borderColor}`, padding: "0.75rem", fontSize: "0.875rem" }}>
121118
<div style={{ display: "flex", justifyContent: "space-between", gap: "0.75rem", alignItems: "flex-start" }}>
@@ -132,16 +129,11 @@ export function IntegrationsManager({ initial }) {
132129
</div>
133130
<div style={{ marginTop: "0.5rem", display: "flex", alignItems: "center", gap: "0.5rem" }}>
134131
<code style={{ flex: 1, wordBreak: "break-all", borderRadius: "0.25rem", border: `1px solid ${borderColor}`, background: "var(--color-bg-secondary)", padding: "0.25rem 0.5rem", fontSize: "0.75rem" }}>
135-
{show ? it.access_token : masked}
132+
{it.token_hint}
136133
</code>
137-
<button type="button" onClick={() => setRevealed((p) => ({ ...p, [it.id]: !p[it.id] }))}
138-
style={{ fontSize: "0.75rem", color: mutedColor, background: "none", border: "none", cursor: "pointer" }}>
139-
{show ? "Hide" : "Reveal"}
140-
</button>
141-
<button type="button" onClick={() => copy(it.id, it.access_token)}
142-
style={{ fontSize: "0.75rem", color: mutedColor, background: "none", border: "none", cursor: "pointer" }}>
143-
{copied === it.id ? "Copied" : "Copy"}
144-
</button>
134+
<span style={{ fontSize: "0.75rem", color: mutedColor }}>
135+
shown once at creation
136+
</span>
145137
</div>
146138
</div>
147139
<button type="button" onClick={() => onRevoke(it)} disabled={pending}

‎src/app/admin/page.jsx‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,17 @@ export default async function AdminPage() {
2727
const [{ data: integrations }, { data: posts }] = await Promise.all([
2828
svc.from('autoblog_integrations')
2929
.select('id, name, kind, access_token, request_count, last_used_at, created_at')
30-
.order('created_at', { ascending: false }),
30+
.order('created_at', { ascending: false })
31+
.then(({ data, error }) => ({
32+
// The raw bearer token must never reach the browser: anything handed to a client
33+
// component is serialised into the RSC payload, where masking in JSX is cosmetic.
34+
// Only a non-reversible hint travels over the wire.
35+
data: data?.map(({ access_token, ...rest }) => ({
36+
...rest,
37+
token_hint: access_token ? `${access_token.slice(0, 4)}…${access_token.slice(-4)}` : '',
38+
})),
39+
error,
40+
})),
3141
svc.from('blog_posts')
3242
.select('id, slug, title, source, published_at')
3343
.order('published_at', { ascending: false })

‎src/app/api/auth/backup-pin/route.js‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,15 @@ async function authenticateUser(request) {
101101
}
102102
}
103103

104-
// scrypt work factors. N=16384/r=8/p=1 is the Node default and costs ~16MB and
105-
// tens of milliseconds per derivation -- enough to make an offline sweep of the
106-
// 6-12 digit PIN keyspace impractical per user, and the per-user salt means
107-
// there is no shared work across users.
108-
const SCRYPT_N = 16384;
104+
// scrypt work factors. N=2^17/r=8/p=1 is OWASP's recommended minimum; the previous
105+
// N=16384 (Node's default) cost only ~16MB per derivation, which left the 6-12 digit
106+
// PIN keyspace within reach of an offline GPU sweep. The per-user salt means there is
107+
// no shared work across users on top of that.
108+
const SCRYPT_N = 131072;
109109
const SCRYPT_R = 8;
110110
const SCRYPT_P = 1;
111+
// scrypt needs roughly 128 * N * r bytes; Node's default 32MB cap would reject N=2^17.
112+
const SCRYPT_MAXMEM = 192 * 1024 * 1024;
111113
const SCRYPT_KEYLEN = 64;
112114
const SCRYPT_SALT_BYTES = 16;
113115
export const PIN_ALGORITHM = `scrypt-n${SCRYPT_N}-r${SCRYPT_R}-p${SCRYPT_P}`;
@@ -126,7 +128,12 @@ export const PIN_ALGORITHM = `scrypt-n${SCRYPT_N}-r${SCRYPT_R}-p${SCRYPT_P}`;
126128
async function hashPin(pin, saltHex) {
127129
const salt = saltHex ?? randomBytes(SCRYPT_SALT_BYTES).toString('hex');
128130
const derived = /** @type {Buffer} */ (
129-
await scrypt(pin, salt, SCRYPT_KEYLEN, { N: SCRYPT_N, r: SCRYPT_R, p: SCRYPT_P })
131+
await scrypt(pin, salt, SCRYPT_KEYLEN, {
132+
N: SCRYPT_N,
133+
r: SCRYPT_R,
134+
p: SCRYPT_P,
135+
maxmem: SCRYPT_MAXMEM
136+
})
130137
);
131138
return { hash: derived.toString('hex'), salt, algorithm: PIN_ALGORITHM };
132139
}

‎src/app/api/auth/coinpay/callback/route.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,15 @@ export async function GET(request) {
182182
return redirectError(appOrigin, 'coinpay_no_email');
183183
}
184184

185+
// The email is what binds this OIDC identity to a QryptChat account, so an unverified
186+
// one would let whoever controls the provider-side address take over that account.
187+
// Only an explicit affirmative counts — a missing claim is not a verified claim.
188+
const emailVerified = claims.email_verified === true || claims.email_verified === 'true';
189+
if (!emailVerified) {
190+
console.error('coinpay/callback: refusing to link an unverified email');
191+
return redirectError(appOrigin, 'coinpay_email_unverified');
192+
}
193+
185194
const serviceSupabase = createServiceClient();
186195

187196
// --- a) Find-or-create Supabase auth user by email ---

‎src/app/api/conversations/delete/route.js‎

Lines changed: 57 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
/**
22
* @fileoverview Delete Conversation API Endpoint
3-
* Handles removing a user's participation and their data from a conversation
4-
* For direct messages: deletes the entire conversation if user is a participant
5-
* For groups: only removes the user's participation and their messages/files
3+
* Handles removing a user's participation and their data from a conversation.
4+
*
5+
* A participant may only destroy rows they own — their own messages, their own file
6+
* attachments and their own participation — whatever the conversation type. The shared
7+
* conversation row and anything still belonging to other people is removed only once the
8+
* last active participant has left, at which point there is nobody left to lose data.
69
*/
710

811
import { NextResponse } from 'next/server';
@@ -84,42 +87,63 @@ export const POST = withAuth(async ({ request, locals }) => {
8487
return NextResponse.json({ error: 'Failed to fetch conversation details' }, { status: 500 });
8588
}
8689

87-
const isDirectMessage = conversation.type === 'direct';
90+
// Every participant — direct or group — may only remove what belongs to them.
91+
// Dependent rows (deliveries, message_recipients, message_status, encrypted_files)
92+
// are ON DELETE CASCADE from messages, so deleting the sender's own messages is enough.
93+
const { error: ownMessagesError } = await supabase
94+
.from('messages')
95+
.delete()
96+
.eq('conversation_id', conversationId)
97+
.eq('sender_id', internalUserId);
8898

89-
if (isDirectMessage) {
90-
// For direct messages, delete everything for all participants
91-
console.log(`Deleting direct message conversation ${conversationId} for all participants`);
99+
if (ownMessagesError) {
100+
console.error('Error deleting user messages:', ownMessagesError);
101+
return NextResponse.json({ error: 'Failed to delete your messages' }, { status: 500 });
102+
}
92103

93-
// Use service role client to bypass RLS policies for deletion
94-
const serviceClient = getServiceRoleClient();
95-
console.log('🔑 Using service role client to bypass RLS for deletion');
104+
const { error: ownFilesError } = await supabase
105+
.from('file_attachments')
106+
.delete()
107+
.eq('conversation_id', conversationId)
108+
.eq('uploaded_by', internalUserId);
96109

97-
// Step 1: Get all message IDs for this conversation
98-
const { data: messages, error: messagesQueryError } = await serviceClient
99-
.from('messages')
100-
.select('id')
101-
.eq('conversation_id', conversationId);
110+
if (ownFilesError) {
111+
console.error('Error deleting user file attachments:', ownFilesError);
112+
}
102113

103-
if (messagesQueryError) {
104-
console.error('Error querying messages:', messagesQueryError);
105-
return NextResponse.json({ error: 'Failed to query messages' }, { status: 500 });
106-
}
114+
const { error: leaveError } = await supabase
115+
.from('conversation_participants')
116+
.delete()
117+
.eq('conversation_id', conversationId)
118+
.eq('user_id', internalUserId);
107119

108-
const messageIds = messages?.map(m => m.id) || [];
120+
if (leaveError) {
121+
console.error('Error removing user participation:', leaveError);
122+
return NextResponse.json({ error: 'Failed to leave conversation' }, { status: 500 });
123+
}
109124

110-
// Step 2: Delete SMS notifications (references conversation_id and message_id)
111-
if (messageIds.length > 0) {
112-
const { error: smsError } = await serviceClient
113-
.from('sms_notifications')
114-
.delete()
115-
.in('message_id', messageIds);
125+
// Garbage-collect the shared conversation only once nobody is left in it.
126+
const serviceClient = getServiceRoleClient();
127+
const { data: remaining, error: remainingError } = await serviceClient
128+
.from('conversation_participants')
129+
.select('id')
130+
.eq('conversation_id', conversationId)
131+
.is('left_at', null)
132+
.limit(1);
116133

117-
if (smsError) {
118-
console.error('Error deleting SMS notifications:', smsError);
119-
}
120-
}
134+
if (remainingError) {
135+
console.error('Error counting remaining participants:', remainingError);
136+
return NextResponse.json({ error: 'Failed to finalise deletion' }, { status: 500 });
137+
}
138+
139+
const isOrphaned = (remaining?.length ?? 0) === 0;
140+
console.log(
141+
`🗑️ ${conversation.type} conversation ${conversationId}: caller removed, orphaned=${isOrphaned}`
142+
);
121143

122-
// Also delete SMS notifications by conversation_id
144+
if (isOrphaned) {
145+
// Nobody is left in this conversation, so the leftovers belong to no one. Rows that
146+
// hang off messages cascade; the rest are cleaned up explicitly.
123147
const { error: smsConvError } = await serviceClient
124148
.from('sms_notifications')
125149
.delete()
@@ -129,55 +153,6 @@ export const POST = withAuth(async ({ request, locals }) => {
129153
console.error('Error deleting SMS notifications by conversation:', smsConvError);
130154
}
131155

132-
// Step 3: Delete deliveries (references message_id)
133-
if (messageIds.length > 0) {
134-
const { error: deliveriesError } = await serviceClient
135-
.from('deliveries')
136-
.delete()
137-
.in('message_id', messageIds);
138-
139-
if (deliveriesError) {
140-
console.error('Error deleting deliveries:', deliveriesError);
141-
}
142-
}
143-
144-
// Step 4: Delete message_recipients (references message_id)
145-
if (messageIds.length > 0) {
146-
const { error: recipientsError } = await serviceClient
147-
.from('message_recipients')
148-
.delete()
149-
.in('message_id', messageIds);
150-
151-
if (recipientsError) {
152-
console.error('Error deleting message recipients:', recipientsError);
153-
}
154-
}
155-
156-
// Step 5: Delete message_status (references message_id)
157-
if (messageIds.length > 0) {
158-
const { error: statusError } = await serviceClient
159-
.from('message_status')
160-
.delete()
161-
.in('message_id', messageIds);
162-
163-
if (statusError) {
164-
console.error('Error deleting message status:', statusError);
165-
}
166-
}
167-
168-
// Step 6: Delete encrypted_files (references message_id)
169-
if (messageIds.length > 0) {
170-
const { error: encryptedFilesError } = await serviceClient
171-
.from('encrypted_files')
172-
.delete()
173-
.in('message_id', messageIds);
174-
175-
if (encryptedFilesError) {
176-
console.error('Error deleting encrypted files:', encryptedFilesError);
177-
}
178-
}
179-
180-
// Step 7: Delete all messages
181156
const { error: messagesError } = await serviceClient
182157
.from('messages')
183158
.delete()
@@ -188,7 +163,6 @@ export const POST = withAuth(async ({ request, locals }) => {
188163
return NextResponse.json({ error: 'Failed to delete messages' }, { status: 500 });
189164
}
190165

191-
// Step 8: Delete typing_indicators
192166
const { error: typingError } = await serviceClient
193167
.from('typing_indicators')
194168
.delete()
@@ -198,7 +172,6 @@ export const POST = withAuth(async ({ request, locals }) => {
198172
console.error('Error deleting typing indicators:', typingError);
199173
}
200174

201-
// Step 9: Delete all participants
202175
const { error: participantsError } = await serviceClient
203176
.from('conversation_participants')
204177
.delete()
@@ -209,7 +182,6 @@ export const POST = withAuth(async ({ request, locals }) => {
209182
return NextResponse.json({ error: 'Failed to delete participants' }, { status: 500 });
210183
}
211184

212-
// Step 10: Delete the conversation
213185
const { error: conversationError } = await serviceClient
214186
.from('conversations')
215187
.delete()
@@ -220,50 +192,10 @@ export const POST = withAuth(async ({ request, locals }) => {
220192
return NextResponse.json({ error: 'Failed to delete conversation' }, { status: 500 });
221193
}
222194

223-
console.log(`✅ Successfully deleted direct message conversation ${conversationId}`);
224-
} else {
225-
// For group conversations, only delete user's own data
226-
console.log(`Removing user ${internalUserId} from group conversation ${conversationId}`);
227-
228-
// Delete only the user's messages
229-
const { error: messagesError } = await supabase
230-
.from('messages')
231-
.delete()
232-
.eq('conversation_id', conversationId)
233-
.eq('sender_id', internalUserId);
234-
235-
if (messagesError) {
236-
console.error('Error deleting user messages:', messagesError);
237-
return NextResponse.json({ error: 'Failed to delete your messages' }, { status: 500 });
238-
}
239-
240-
// Delete only the user's file attachments
241-
const { error: filesError } = await supabase
242-
.from('file_attachments')
243-
.delete()
244-
.eq('conversation_id', conversationId)
245-
.eq('uploaded_by', internalUserId);
246-
247-
if (filesError) {
248-
console.error('Error deleting user file attachments:', filesError);
249-
}
250-
251-
// Remove user's participation
252-
const { error: participantError } = await supabase
253-
.from('conversation_participants')
254-
.delete()
255-
.eq('conversation_id', conversationId)
256-
.eq('user_id', internalUserId);
257-
258-
if (participantError) {
259-
console.error('Error removing user participation:', participantError);
260-
return NextResponse.json({ error: 'Failed to leave conversation' }, { status: 500 });
261-
}
262-
263-
console.log(`✅ Successfully removed user ${internalUserId} from group conversation ${conversationId}`);
195+
console.log(`✅ Garbage-collected orphaned conversation ${conversationId}`);
264196
}
265197

266-
return NextResponse.json({ success: true });
198+
return NextResponse.json({ success: true, conversationRemoved: isOrphaned });
267199
} catch (error) {
268200
console.error('Delete conversation error:', error);
269201
return NextResponse.json({ error: 'Internal server error' }, { status: 500 });

0 commit comments

Comments
 (0)