Skip to content

Commit 3e2047e

Browse files
fix: address CodeRabbit review feedback on auth security and persistence
1 parent f25e711 commit 3e2047e

File tree

3 files changed

+46
-22
lines changed

3 files changed

+46
-22
lines changed

src/routes/settings.tsx

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ function SettingsPage() {
5757
useEffect(() => {
5858
if (typeof window === 'undefined') return;
5959

60+
let isMounted = true;
61+
6062
// If we already have a token, no need to generate
6163
const existing = localStorage.getItem('rein_auth_token');
6264
if (existing) return;
@@ -66,15 +68,19 @@ function SettingsPage() {
6668
const socket = new WebSocket(wsUrl);
6769

6870
socket.onopen = () => {
69-
socket.send(JSON.stringify({ type: 'generate-token' }));
71+
if (socket.readyState === WebSocket.OPEN) {
72+
socket.send(JSON.stringify({ type: 'generate-token' }));
73+
}
7074
};
7175

7276
socket.onmessage = (event) => {
7377
try {
7478
const data = JSON.parse(event.data);
7579
if (data.type === 'token-generated' && data.token) {
76-
setAuthToken(data.token);
77-
localStorage.setItem('rein_auth_token', data.token);
80+
if (isMounted) {
81+
setAuthToken(data.token);
82+
localStorage.setItem('rein_auth_token', data.token);
83+
}
7884
socket.close();
7985
}
8086
} catch (e) {
@@ -83,7 +89,10 @@ function SettingsPage() {
8389
};
8490

8591
return () => {
86-
if (socket.readyState === WebSocket.OPEN) socket.close();
92+
isMounted = false;
93+
if (socket.readyState === WebSocket.OPEN || socket.readyState === WebSocket.CONNECTING) {
94+
socket.close();
95+
}
8796
};
8897
}, []);
8998

src/server/tokenStore.ts

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,22 @@
11
import crypto from 'crypto';
22
import fs from 'fs';
33
import path from 'path';
4+
import { fileURLToPath } from 'url';
45

56
interface TokenEntry {
67
token: string;
78
createdAt: number;
89
lastUsed: number;
910
}
1011

11-
const TOKENS_FILE = path.resolve('./src/tokens.json');
12+
const __filename = fileURLToPath(import.meta.url);
13+
const __dirname = path.dirname(__filename);
14+
const TOKENS_FILE = path.resolve(__dirname, '../tokens.json');
1215
const EXPIRY_MS = 10 * 24 * 60 * 60 * 1000; // 10 days
1316

1417
let tokens: TokenEntry[] = [];
18+
let lastSaveTime = 0;
19+
const SAVE_THROTTLE_MS = 60 * 1000; // 1 minute
1520

1621
function load(): void {
1722
try {
@@ -23,9 +28,13 @@ function load(): void {
2328
}
2429
}
2530

26-
function save(): void {
31+
function save(force = false): void {
32+
const now = Date.now();
33+
if (!force && (now - lastSaveTime) < SAVE_THROTTLE_MS) return;
34+
2735
try {
2836
fs.writeFileSync(TOKENS_FILE, JSON.stringify(tokens, null, 2));
37+
lastSaveTime = now;
2938
} catch (e) {
3039
console.error('Failed to persist tokens:', e);
3140
}
@@ -35,7 +44,7 @@ function purgeExpired(): void {
3544
const now = Date.now();
3645
const before = tokens.length;
3746
tokens = tokens.filter(t => (now - t.lastUsed) < EXPIRY_MS);
38-
if (tokens.length !== before) save();
47+
if (tokens.length !== before) save(true);
3948
}
4049

4150
/** Constant-time string comparison to prevent timing attacks. */
@@ -61,7 +70,7 @@ export function storeToken(token: string): void {
6170
const now = Date.now();
6271
tokens.push({ token, createdAt: now, lastUsed: now });
6372
}
64-
save();
73+
save(true);
6574
}
6675

6776
/** Check if a token is already known/stored on the server. */
@@ -75,11 +84,19 @@ export function touchToken(token: string): void {
7584
const entry = tokens.find(t => timingSafeEqual(t.token, token));
7685
if (entry) {
7786
entry.lastUsed = Date.now();
78-
// Persist periodically — save only if >60s since last save
79-
// to avoid excessive disk I/O on every message
87+
save(); // Throttled internally
8088
}
8189
}
8290

91+
/** Returns the most recently used active token, if any. */
92+
export function getActiveToken(): string | null {
93+
purgeExpired();
94+
if (tokens.length === 0) return null;
95+
// Return the one used most recently
96+
const sorted = [...tokens].sort((a, b) => b.lastUsed - a.lastUsed);
97+
return sorted[0].token;
98+
}
99+
83100
/** Check if any tokens exist yet (first-run detection). */
84101
export function hasTokens(): boolean {
85102
purgeExpired();

src/server/websocket.ts

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { WebSocketServer, WebSocket } from 'ws';
22
import { InputHandler, InputMessage } from './InputHandler';
3-
import { storeToken, isKnownToken, touchToken, hasTokens, generateToken } from './tokenStore';
3+
import { storeToken, isKnownToken, touchToken, generateToken, getActiveToken } from './tokenStore';
44
import os from 'os';
55
import fs from 'fs';
66
import { IncomingMessage } from 'http';
@@ -53,14 +53,6 @@ export function createWsServer(server: any) {
5353
return;
5454
}
5555

56-
// First remote connection ever — accept and store the token
57-
if (!hasTokens()) {
58-
storeToken(token);
59-
wss.handleUpgrade(request, socket, head, (ws) => {
60-
wss.emit('connection', ws, request, token, false);
61-
});
62-
return;
63-
}
6456

6557
// Validate against known tokens
6658
if (!isKnownToken(token)) {
@@ -103,9 +95,15 @@ export function createWsServer(server: any) {
10395
ws.send(JSON.stringify({ type: 'auth-error', error: 'Only localhost can generate tokens' }));
10496
return;
10597
}
106-
const newToken = generateToken();
107-
storeToken(newToken);
108-
ws.send(JSON.stringify({ type: 'token-generated', token: newToken }));
98+
99+
// Idempotent: return active token if one exists
100+
let tokenToReturn = getActiveToken();
101+
if (!tokenToReturn) {
102+
tokenToReturn = generateToken();
103+
storeToken(tokenToReturn);
104+
}
105+
106+
ws.send(JSON.stringify({ type: 'token-generated', token: tokenToReturn }));
109107
return;
110108
}
111109

0 commit comments

Comments
 (0)