-
Notifications
You must be signed in to change notification settings - Fork 7
fix(storage): cross-tab admission for protected writes vs. active migrations #339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7057cf8
3f70e0e
814ff04
349b962
79b482c
8145eec
366fcfe
f978606
c0707ed
a130b00
82e2e8d
05d590c
0ebbca2
9afbdec
7498444
58a3a82
cb87963
2017957
c4b64f8
997b2f6
88016dd
fa3cd98
fd7ed7c
dda48b3
fefd9ef
8b0afe5
a623213
264fb7d
d318abc
ebdcac1
d0ca4e6
759a9ec
5e80aaa
09d8c59
1a081cf
e42fd3c
da3b4d7
501a403
e204463
fd38847
8a6f895
a01e298
20c9954
756e3c0
2438f99
7cdd128
3253c41
56e2509
a8dd917
46198b2
411943a
beadfa2
3f31c1e
68050d8
dc0b526
ed46bef
667f6f3
99c2839
0353364
ad4364a
dd92628
74ce8fd
58d95d1
be11482
d536649
ebafce7
c3f00cf
0b1cc2e
bc078b8
edc3ef1
e99f354
5bd4c77
451f681
b01564e
1096861
6dc90eb
3a3ed23
652fa72
a24a987
369d064
09eae35
afbcdb8
56389e6
5568d91
43e4afc
55bc5ea
1f1ec61
e708940
1335e81
5fed880
0ab3341
92dd2b5
ed22774
8bd6947
1419352
1940fe9
179f11b
096cf96
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ import type { BinderAssetMeta, BinderAssetPayload } from '../storageBackend'; | |
| import { makeBinderAssetIdsPrefix, makeBinderAssetStorageKey } from '../storageBackend'; | ||
| import { getUserFriendlyDbError, retryDb } from './idbCore'; | ||
| import { IdbSnapshotStore } from './idbSnapshotStore'; | ||
| import { withProtectedWriteAdmission } from './protectedWriteAdmission'; | ||
| import { | ||
| assertIdbProtectedWriteAllowed, | ||
| assertNoActiveEncryptionMigration, | ||
|
|
@@ -23,19 +24,21 @@ export class IdbAssetStore extends IdbSnapshotStore { | |
| // --- Image Store Methods --- | ||
|
|
||
| async saveImage(id: string, base64: string): Promise<void> { | ||
| // QNBS-v3: Resolve the write key BEFORE opening the transaction — `await idbEncryptWithKey` | ||
| // yields the event loop, which auto-commits an already-open IDB transaction | ||
| // (TransactionInactiveError on put), and re-reading isIdbEncryptionReady() after any | ||
| // later await could race with Lock Session and silently fall back to plaintext. | ||
| const writeKey = await resolveProtectedWriteKey(); | ||
| const payload = writeKey ? await idbEncryptWithKey(writeKey, base64) : base64; | ||
| // QNBS-v3: only the migration guard is re-checked here — resolveProtectedWriteKey() already made its own lock check atomically with the key snapshot, so re-running that too would wrongly reject an already-safely-encrypted write if the session locks mid-write. | ||
| await assertNoActiveEncryptionMigration(); | ||
| const store = await this.getObjectStore(IMAGES_STORE, 'readwrite'); | ||
| return new Promise((resolve, reject) => { | ||
| const request = store.put(payload, id); | ||
| request.onsuccess = () => resolve(); | ||
| request.onerror = () => reject(request.error); | ||
| return withProtectedWriteAdmission(async () => { | ||
| // QNBS-v3: Resolve the write key BEFORE opening the transaction — `await idbEncryptWithKey` | ||
| // yields the event loop, which auto-commits an already-open IDB transaction | ||
| // (TransactionInactiveError on put), and re-reading isIdbEncryptionReady() after any | ||
| // later await could race with Lock Session and silently fall back to plaintext. | ||
| const writeKey = await resolveProtectedWriteKey(); | ||
| const payload = writeKey ? await idbEncryptWithKey(writeKey, base64) : base64; | ||
| // QNBS-v3: only the migration guard is re-checked here — resolveProtectedWriteKey() already made its own lock check atomically with the key snapshot, so re-running that too would wrongly reject an already-safely-encrypted write if the session locks mid-write. | ||
| await assertNoActiveEncryptionMigration(); | ||
| const store = await this.getObjectStore(IMAGES_STORE, 'readwrite'); | ||
| return new Promise<void>((resolve, reject) => { | ||
| const request = store.put(payload, id); | ||
| request.onsuccess = () => resolve(); | ||
| request.onerror = () => reject(request.error); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -66,13 +69,15 @@ export class IdbAssetStore extends IdbSnapshotStore { | |
| } | ||
|
|
||
| async deleteImage(id: string): Promise<void> { | ||
| // QNBS-v3: A locked session must not be able to destroy protected images it cannot read. | ||
| await assertIdbProtectedWriteAllowed(); | ||
| const store = await this.getObjectStore(IMAGES_STORE, 'readwrite'); | ||
| return new Promise((resolve, reject) => { | ||
| const request = store.delete(id); | ||
| request.onsuccess = () => resolve(); | ||
| request.onerror = () => reject(request.error); | ||
| return withProtectedWriteAdmission(async () => { | ||
| // QNBS-v3: A locked session must not be able to destroy protected images it cannot read. | ||
| await assertIdbProtectedWriteAllowed(); | ||
| const store = await this.getObjectStore(IMAGES_STORE, 'readwrite'); | ||
| return new Promise<void>((resolve, reject) => { | ||
| const request = store.delete(id); | ||
| request.onsuccess = () => resolve(); | ||
| request.onerror = () => reject(request.error); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -84,30 +89,32 @@ export class IdbAssetStore extends IdbSnapshotStore { | |
| data: ArrayBuffer, | ||
| meta: BinderAssetMeta, | ||
| ): Promise<void> { | ||
| return retryDb(async () => { | ||
| const writeKey = await resolveProtectedWriteKey(); | ||
| const key = makeBinderAssetStorageKey(projectId, assetId); | ||
| const fullMeta = { ...meta, byteSize: data.byteLength }; | ||
| // QNBS-v3: idbEncrypt serialises via JSON.stringify, which silently drops a Blob ({} → no data). | ||
| // When encrypting, persist the raw bytes; otherwise store a structured-clone-friendly Blob. | ||
| const payload = writeKey | ||
| ? await idbEncryptWithKey(writeKey, { | ||
| meta: fullMeta, | ||
| bytes: Array.from(new Uint8Array(data)), | ||
| }) | ||
| : { | ||
| meta: fullMeta, | ||
| blob: new Blob([data], { type: meta.mimeType || 'application/octet-stream' }), | ||
| }; | ||
| // QNBS-v3: only the migration guard is re-checked here — resolveProtectedWriteKey() already made its own lock check atomically with the key snapshot, so re-running that too would wrongly reject an already-safely-encrypted write if the session locks mid-write. | ||
| await assertNoActiveEncryptionMigration(); | ||
| const store = await this.getObjectStore(BINDER_ASSETS_STORE, 'readwrite'); | ||
| return new Promise<void>((resolve, reject) => { | ||
| const req = store.put(payload, key); | ||
| req.onsuccess = () => resolve(); | ||
| req.onerror = () => reject(getUserFriendlyDbError(req.error)); | ||
| }); | ||
| }); | ||
| return retryDb(() => | ||
| withProtectedWriteAdmission(async () => { | ||
| const writeKey = await resolveProtectedWriteKey(); | ||
| const key = makeBinderAssetStorageKey(projectId, assetId); | ||
| const fullMeta = { ...meta, byteSize: data.byteLength }; | ||
| // QNBS-v3: idbEncrypt serialises via JSON.stringify, which silently drops a Blob ({} → no data). | ||
| // When encrypting, persist the raw bytes; otherwise store a structured-clone-friendly Blob. | ||
| const payload = writeKey | ||
| ? await idbEncryptWithKey(writeKey, { | ||
| meta: fullMeta, | ||
| bytes: Array.from(new Uint8Array(data)), | ||
| }) | ||
|
Comment on lines
+99
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: Encrypted binder assets are converted into a JavaScript Severity Level: Major
|
||
| : { | ||
| meta: fullMeta, | ||
| blob: new Blob([data], { type: meta.mimeType || 'application/octet-stream' }), | ||
| }; | ||
| // QNBS-v3: only the migration guard is re-checked here — resolveProtectedWriteKey() already made its own lock check atomically with the key snapshot, so re-running that too would wrongly reject an already-safely-encrypted write if the session locks mid-write. | ||
| await assertNoActiveEncryptionMigration(); | ||
| const store = await this.getObjectStore(BINDER_ASSETS_STORE, 'readwrite'); | ||
| return new Promise<void>((resolve, reject) => { | ||
| const req = store.put(payload, key); | ||
| req.onsuccess = () => resolve(); | ||
| req.onerror = () => reject(getUserFriendlyDbError(req.error)); | ||
| }); | ||
| }), | ||
| ); | ||
| } | ||
|
|
||
| async getBinderAsset(projectId: string, assetId: string): Promise<BinderAssetPayload | null> { | ||
|
|
@@ -136,17 +143,19 @@ export class IdbAssetStore extends IdbSnapshotStore { | |
| } | ||
|
|
||
| async deleteBinderAsset(projectId: string, assetId: string): Promise<void> { | ||
| return retryDb(async () => { | ||
| // QNBS-v3: A locked session must not be able to destroy protected binder assets it cannot read. | ||
| await assertIdbProtectedWriteAllowed(); | ||
| const key = makeBinderAssetStorageKey(projectId, assetId); | ||
| const store = await this.getObjectStore(BINDER_ASSETS_STORE, 'readwrite'); | ||
| return new Promise<void>((resolve, reject) => { | ||
| const req = store.delete(key); | ||
| req.onsuccess = () => resolve(); | ||
| req.onerror = () => reject(getUserFriendlyDbError(req.error)); | ||
| }); | ||
| }); | ||
| return retryDb(() => | ||
| withProtectedWriteAdmission(async () => { | ||
| // QNBS-v3: A locked session must not be able to destroy protected binder assets it cannot read. | ||
| await assertIdbProtectedWriteAllowed(); | ||
| const key = makeBinderAssetStorageKey(projectId, assetId); | ||
| const store = await this.getObjectStore(BINDER_ASSETS_STORE, 'readwrite'); | ||
| return new Promise<void>((resolve, reject) => { | ||
| const req = store.delete(key); | ||
| req.onsuccess = () => resolve(); | ||
| req.onerror = () => reject(getUserFriendlyDbError(req.error)); | ||
| }); | ||
| }), | ||
| ); | ||
| } | ||
|
|
||
| async listBinderAssetIds(projectId: string): Promise<string[]> { | ||
|
|
@@ -176,6 +185,13 @@ export class IdbAssetStore extends IdbSnapshotStore { | |
| } | ||
|
|
||
| async deleteAllBinderAssetsForProject(projectId: string): Promise<void> { | ||
| return withProtectedWriteAdmission(() => | ||
| this.deleteAllBinderAssetsForProjectUnadmitted(projectId), | ||
| ); | ||
| } | ||
|
|
||
| // QNBS-v3: unwrapped core for deleteProject() to call inside its own single outer admission — nesting withProtectedWriteAdmission (same shared lock name, same call stack) can deadlock if an exclusive migration request queues between the outer and inner acquisition. | ||
| protected async deleteAllBinderAssetsForProjectUnadmitted(projectId: string): Promise<void> { | ||
| return retryDb(async () => { | ||
| await assertIdbProtectedWriteAllowed(); | ||
| const ids = await this.listBinderAssetIds(projectId); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.