feat(THU-596): user data export & import#991
Conversation
Semgrep Security ScanNo security issues found. |
|
Preview environment deployed 🚀
Stack: Auto-destroys on PR close/merge. Login via the bundled Keycloak realm — |
PR Metrics
Updated Sun, 21 Jun 2026 14:21:29 GMT · run #1978 |
ital0
left a comment
There was a problem hiding this comment.
🤖 thunder-deep-review — advisory (pending; submit or discard after review).
CONCERNS — core feature works, but import writes untrusted/verbatim rows into synced tables, bypassing DAL invariants and silently propagating cross-device (and potentially cross-account/E2EE) with no size/column guards; several blocking architecture+powersync+security items plus a wrong forward-compat doc claim should be resolved before merge.
5 blocking · 7 convention · 17 nit
All 5 blocking + 7 convention findings are attached as inline comments. The 17 nits:
src/dal/import.ts:147-152— N+1 write pattern: per-row SELECT then UPDATE/INSERT, serial inside one tx; batch-load existing PKs with a single IN(...) SELECT into a Setsrc/dal/import.ts:149— UPDATE writes set(row) including the PK column; omit pk from the SET payloadsrc/dal/export.ts:91-93— Export materializes every row of every table via Promise.all then pretty JSON.stringify (~2x); large transient memory spike, no streaming/paginationsrc/settings/preferences.tsx:376— handleConfirmImport clears importError but never importSuccess; stale success banner + new error can render togethersrc/settings/preferences.tsx:877— Export/Import UI gated on isAuthenticated (anonymous too) though component derives isFullUser; copy implies a connected account; gate vs copy vs cross-user provenancesrc/dal/import.ts:132— Recognized table whose value is a non-array is silently skipped by the empty-array branch (not counted, not ignored); rows-not-object inside a valid array throw — inconsistent contractsrc/settings/preferences.tsx:939— aria-describedby resolves to the success element id after import, so the button is permanently described by the last result instead of its instructional descriptionsrc/settings/preferences.tsx:362— Inline 7-line sourceEmail type-guard ternary breaks symmetry with its extracted siblings; double cast appears twice — extract a readSourceEmail helpersrc/dal/import.ts:39— Single-use candidates const split from the .find on the next line; collapse into one chained expressionsrc/db/tables.ts:315— Comment on agentsSecretsTable says 'Never leaves the device' but this PR's export now includes agents_secrets — invariant no longer true (NOT in diff → body)docs/architecture/export-format.md:82— Grammar: 'aImportFormatError' → 'anImportFormatError'docs/architecture/export-format.md:87— Cites multi-device-sync.md for 'no FK constraints' but that doc only says 'No composite foreign keys'; weaken wording or drop citationsrc/settings/preferences.tsx:1448— Dialog 'contains {totalRows} rows' counts every bucket (incl. excluded/unknown) while success toast 'Imported {total} rows' sums only upserted — numbers silently disagreesrc/dal/export.ts:13— Widening syncedTables/localOnlyTables const→export to feed export.ts removes a guardrail; add a comment marking them the canonical classification sourcedocs/architecture/export-format.md:44-46— New SYNCED table is auto-included in export AND import-with-upload with zero sync-classification review; note + import test to mirror the export allowlist gatesrc/dal/import.test.ts:284-292— derivePkSpec tests suppress console.warn with a hand-rolled save/restore; use spyOn(console,'warn').mockImplementation(()=>{}) per R-SUPPRESSCONSOLEsrc/settings/preferences.tsx:67-68— New reducer actions modeled as setters (SET_IS_EXPORTING) not events; R-REDUCER prefers EXPORT_STARTED/FINISHED (low priority, file is locally consistent)
Note: the src/db/tables.ts:315 nit is outside the PR diff, so it could not be anchored inline — listed above only.
Summary
INSERT ... ON CONFLICT DO UPDATEis rejected).schemaVersion: 1); v1 ships without a workspace concept so it can land before Workspaces v1. The post-workspaces refactor (THU-597) bumps toschemaVersion: 2withworkspaceIdon rows and no selector UI — spec indocs/architecture/export-format.md.includedTablesderived fromsrc/db/powersync/schema.ts; adding a new synced/local-only table flows through automatically with the export's allowlist test as the safety net.Test plan
bun test src/dal/export.test.ts src/dal/import.test.ts src/lib/export-download.test.ts src/lib/import-upload.test.tsall green (~40 tests)bun tsc --noEmit,bun lint,bun run format-checkall cleanthunderbolt-export-YYYY-MM-DD.jsonwithschemaVersion: 1and the expected table bucketstables.integrations_secrets/tables.devices/tables.agents_systemabsent;models_secretscontains the typed API key{}or wrongschemaVersion) → inline error, no DB writes; tampered file containingdevices/integrations_secretslands inignoredTableNamesNote
High Risk
Exports plaintext API keys and MCP/agent secrets; import overwrites matching IDs locally and propagates to all synced devices, with destructive restore semantics.
Overview
Adds Export My Data and Import Data under Settings → Preferences → Data for signed-in users, backed by a versioned
thunderbolt-exportJSON envelope (schemaVersion: 1).Export walks PowerSync’s
syncedTables+localOnlyTables(now exported fromschema.ts), minusdevices,integrations_secrets, andagents_system, and snapshots full rows—including soft-deleted rows and local credential tables (models_secrets,mcp_secrets,agents_secrets). Downloads usedownloadJsonwith a dated filename.Import validates the envelope, previews via
summarizeExportEnvelope(row count, source email, cross-account mismatch), then restores in one transaction using per-row SELECT + UPDATE/INSERT (avoids upserts on PowerSync views). Imported rows win on PK collision; other local rows stay put. Synced tables getuserIdre-stamped from the session; unknown table keys are ignored. File pick goes throughreadJsonFilewith a 200 MB cap.docs/architecture/export-format.mddocuments the format and import semantics (including multi-device sync overwrite). PostHog eventssettings_data_export/settings_data_importwere added.Reviewed by Cursor Bugbot for commit 2f5b997. Bugbot is set up for automated code reviews on this repo. Configure here.