Skip to content

Commit 52e9a6e

Browse files
cevhericlaude
andcommitted
fix(playground): address Copilot review — guard reserved-key writes + worker teardown race (#19)
- put/delete now refuse reserved-namespace keys, matching import and Studio's CLI safety rule (the raw kv lens is unguarded, so a U+0000-prefixed key could otherwise overwrite/remove catalog entries and corrupt the sandbox) - worker teardown: on 'close' the worker now db.close()s and self.close()s; the client no longer calls worker.terminate() on pagehide (it raced and usually won before the worker ran db.close(), so the OPFS lock wasn't released cleanly) - pagehide skips bfcache (event.persisted) so a back/forward restore keeps a live worker - tests: put/delete reserved-key refusal (58 total) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 269d452 commit 52e9a6e

4 files changed

Lines changed: 21 additions & 4 deletions

File tree

‎src/scripts/playground/client.ts‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,13 @@ function init(root: HTMLElement): void {
135135
}),
136136
);
137137

138-
// Free the exclusive OPFS lock so a reload reacquires cleanly.
139-
window.addEventListener('pagehide', () => {
138+
// Free the exclusive OPFS lock so a reload reacquires cleanly. The worker
139+
// closes the db and self-terminates on 'close'; we must NOT terminate() here,
140+
// or it would race (and usually win) before the worker runs db.close().
141+
// Skip on bfcache (persisted) so a back/forward restore keeps a live worker.
142+
window.addEventListener('pagehide', (e) => {
143+
if (e.persisted) return;
140144
void call({ op: 'close' });
141-
worker.terminate();
142145
});
143146

144147
// First view: scan the seeded users namespace.

‎src/scripts/playground/db.worker.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,14 @@ self.onmessage = async (event: MessageEvent<WorkerRequest>) => {
8686
return;
8787
case 'close':
8888
try {
89-
db.close();
89+
db.close(); // flush WAL + release the exclusive OPFS sync-access handle
9090
} catch {
9191
/* already closed */
9292
}
9393
reply({ id: msg.id, kind: 'closed' });
94+
// Self-terminate so the handle is released without the client racing us
95+
// via worker.terminate() before this handler runs.
96+
self.close();
9497
return;
9598
}
9699
} catch (e) {

‎src/scripts/playground/engine.test.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,13 @@ test('import refuses reserved-namespace keys', () => {
122122
expect(execute(db, { op: 'import', entries: { [reservedKey]: 'x' } }).kind).toBe('error');
123123
});
124124

125+
test('put and delete also refuse reserved-namespace keys (parity with import)', () => {
126+
const db = seeded();
127+
const reservedKey = String.fromCharCode(0) + 'libredb:catalog:theme';
128+
expect(execute(db, { op: 'put', key: reservedKey, value: 'x' }).kind).toBe('error');
129+
expect(execute(db, { op: 'delete', key: reservedKey }).kind).toBe('error');
130+
});
131+
125132
test('stats reports a byte size when the worker supplies one', () => {
126133
const db = seeded();
127134
const r = execute(db, { op: 'stats' }, 412);

‎src/scripts/playground/engine.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,10 +122,14 @@ export function execute(db: Database, cmd: Command, fileSize?: number): RunResul
122122
: { kind: 'rows', columns: ['key', 'value'], rows: [{ key: cmd.key, value: renderValue(v) }] };
123123
}
124124
case 'put': {
125+
if (isReservedKey(cmd.key))
126+
return { kind: 'error', error: `refused: "${cmd.key}" is in the reserved namespace` };
125127
const r = kv(db).set(cmd.key, cmd.value);
126128
return { kind: 'message', message: `OK · changed ${r.changed}` };
127129
}
128130
case 'delete': {
131+
if (isReservedKey(cmd.key))
132+
return { kind: 'error', error: `refused: "${cmd.key}" is in the reserved namespace` };
129133
const r = kv(db).delete(cmd.key);
130134
return { kind: 'message', message: `OK · changed ${r.changed}` };
131135
}

0 commit comments

Comments
 (0)