-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Harden WebDAV chat sync #3769
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
Open
NewstarDevelop
wants to merge
19
commits into
chatboxai:main
Choose a base branch
from
NewstarDevelop:webdav-chat-sync-hardening
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Harden WebDAV chat sync #3769
Changes from 6 commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
0245929
Harden WebDAV chat sync before enabling push
NewstarDevelop 0279b3b
Address WebDAV sync review feedback
NewstarDevelop d581e2b
Strip local file keys from synced attachment ids
NewstarDevelop 49ebca2
Redact WebDAV sync secrets from settings export
NewstarDevelop d2a0920
Handle metadata-only WebDAV sync updates
NewstarDevelop 700ef0a
Use conditional WebDAV snapshot uploads
NewstarDevelop 5b28d21
fix(webdav-sync): transactional import, serialized writes, and stable…
NewstarDevelop 7830ed7
fix(sync): add updatedAt to snapshots, skip stale remote merge — prev…
NewstarDevelop 1e62243
chore: revert unrelated .gitignore/ralph.sh noise
NewstarDevelop 842ada9
fix(export): sanitize nested OAuth, memorized license, and webSearch …
NewstarDevelop efdc1c9
fix(sync): replace wall-clock stale gate with endpoint-scoped ETag de…
NewstarDevelop 159bf84
fix(export): tolerate legacy settings missing sync/extension; test sy…
NewstarDevelop d90ba9c
fix(export): strip device-local sync state and sanitize mineru/MCP se…
NewstarDevelop 6274c17
fix(sync): skip recording PUT ETag when upload merged unseen remote s…
NewstarDevelop ade1859
fix(sync): reject weak ETags, locale-independent conflict IDs, valida…
NewstarDevelop a7ead88
fix(sync): provenance tracking, active-generation guard, Android WebD…
NewstarDevelop 3701b49
chore: add capacitor-webdav-http dist (force-added, excluded by globa…
NewstarDevelop e9f73c4
fix(sync): hardening — upload preview, scoped undo, selective column …
NewstarDevelop edb3c87
fix(sync): harden WebDAV import — idempotent retry, parity-aware conf…
NewstarDevelop File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| import * as defaults from '@shared/defaults' | ||
| import type { Settings } from '@shared/types' | ||
| import { describe, expect, it } from 'vitest' | ||
| import { sanitizeSettingsForExport } from './settings-export' | ||
|
|
||
| function settingsWithSecrets(): Settings { | ||
| return { | ||
| ...defaults.settings(), | ||
| licenseKey: 'license-secret', | ||
| licenseDetail: { plan: 'pro' } as unknown as Settings['licenseDetail'], | ||
| licenseInstances: { 'license-secret': 'device-1' }, | ||
| providers: { | ||
| openai: { | ||
| apiKey: 'sk-secret', | ||
| accessKey: 'access-secret', | ||
| secretKey: 'secret-key', | ||
| sessionToken: 'session-token', | ||
| apiHost: 'https://api.example.com', | ||
| }, | ||
| }, | ||
| sync: { | ||
| enabled: true, | ||
| provider: 'webdav', | ||
| webdav: { | ||
| url: 'https://dav.example.com/files/me/', | ||
| username: 'alice', | ||
| password: 'dav-secret', | ||
| syncPassword: 'sync-secret', | ||
| }, | ||
| lastSyncedAt: '2026-07-05T00:00:00.000Z', | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| describe('sanitizeSettingsForExport', () => { | ||
| it('removes WebDAV and provider secrets when key export is not selected', () => { | ||
| const settings = settingsWithSecrets() | ||
| const sanitized = sanitizeSettingsForExport(settings, false) | ||
|
|
||
| expect(sanitized.licenseKey).toBeUndefined() | ||
| expect(sanitized.licenseDetail).toBeUndefined() | ||
| expect(sanitized.licenseInstances).toBeUndefined() | ||
| expect(sanitized.providers?.openai).toEqual({ | ||
| apiHost: 'https://api.example.com', | ||
| }) | ||
| expect(sanitized.sync.webdav).toEqual({ | ||
| url: 'https://dav.example.com/files/me/', | ||
| username: 'alice', | ||
| password: '', | ||
| syncPassword: '', | ||
| }) | ||
| expect(settings.sync.webdav.password).toBe('dav-secret') | ||
| expect(settings.sync.webdav.syncPassword).toBe('sync-secret') | ||
| }) | ||
|
|
||
| it('keeps WebDAV and provider secrets when key export is selected', () => { | ||
| const sanitized = sanitizeSettingsForExport(settingsWithSecrets(), true) | ||
|
|
||
| expect(sanitized.licenseKey).toBe('license-secret') | ||
| expect(sanitized.providers?.openai?.apiKey).toBe('sk-secret') | ||
| expect(sanitized.sync.webdav.password).toBe('dav-secret') | ||
| expect(sanitized.sync.webdav.syncPassword).toBe('sync-secret') | ||
| expect(sanitized.licenseDetail).toBeUndefined() | ||
| expect(sanitized.licenseInstances).toBeUndefined() | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import type { ProviderSettings, Settings } from '@shared/types' | ||
|
|
||
| function sanitizeProviderForExport(provider: ProviderSettings): ProviderSettings { | ||
| const cleanedProvider = { ...provider } | ||
| delete cleanedProvider.apiKey | ||
| delete cleanedProvider.accessKey | ||
| delete cleanedProvider.secretKey | ||
| delete cleanedProvider.sessionToken | ||
| return cleanedProvider | ||
| } | ||
|
|
||
| export function sanitizeSettingsForExport(settings: Settings, includeSecrets: boolean): Settings { | ||
| const cleanedSettings: Settings = { | ||
| ...settings, | ||
| licenseDetail: undefined, | ||
| licenseInstances: undefined, | ||
| providers: settings.providers ? { ...settings.providers } : settings.providers, | ||
| sync: { | ||
| ...settings.sync, | ||
| webdav: { | ||
| ...settings.sync.webdav, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| if (!includeSecrets) { | ||
|
themez marked this conversation as resolved.
|
||
| delete cleanedSettings.licenseKey | ||
| if (cleanedSettings.providers) { | ||
| cleanedSettings.providers = Object.fromEntries( | ||
| Object.entries(cleanedSettings.providers).map(([id, provider]) => [id, sanitizeProviderForExport(provider)]) | ||
| ) as Settings['providers'] | ||
| } | ||
| cleanedSettings.sync.webdav.password = '' | ||
| cleanedSettings.sync.webdav.syncPassword = '' | ||
| } | ||
|
|
||
| return cleanedSettings | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import { describe, expect, it } from 'vitest' | ||
| import { decryptJsonEnvelope, encryptJsonEnvelope } from './crypto' | ||
|
|
||
| describe('sync crypto envelope', () => { | ||
| it('round trips JSON with the sync password', async () => { | ||
| const payload = { version: 1, sessions: [{ id: 's1', name: 'Hello' }] } | ||
|
|
||
| const envelope = await encryptJsonEnvelope(payload, 'correct horse battery staple') | ||
| const decrypted = await decryptJsonEnvelope(envelope, 'correct horse battery staple') | ||
|
|
||
| expect(envelope.version).toBe(1) | ||
| expect(envelope.kdf).toBe('PBKDF2-SHA256') | ||
| expect(envelope.ciphertext).not.toContain('Hello') | ||
| expect(decrypted).toEqual(payload) | ||
| }) | ||
|
|
||
| it('rejects a wrong sync password', async () => { | ||
| const envelope = await encryptJsonEnvelope({ secret: 'chat history' }, 'right-password') | ||
|
|
||
| await expect(decryptJsonEnvelope(envelope, 'wrong-password')).rejects.toThrow(/decrypt/i) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| import type { SyncCryptoEnvelope } from './types' | ||
|
|
||
| const ENVELOPE_VERSION = 1 | ||
| const PBKDF2_ITERATIONS = 250_000 | ||
| const SALT_BYTES = 16 | ||
| const IV_BYTES = 12 | ||
|
|
||
| function getCrypto(): Crypto { | ||
| const cryptoImpl = globalThis.crypto | ||
| if (!cryptoImpl?.subtle) { | ||
| throw new Error('Web Crypto is not available') | ||
| } | ||
| return cryptoImpl | ||
| } | ||
|
|
||
| function bytesToBase64(bytes: Uint8Array): string { | ||
| let binary = '' | ||
| for (const byte of bytes) { | ||
| binary += String.fromCharCode(byte) | ||
| } | ||
| return btoa(binary) | ||
| } | ||
|
|
||
| function base64ToBytes(value: string): Uint8Array { | ||
| const binary = atob(value) | ||
| const bytes = new Uint8Array(binary.length) | ||
| for (let i = 0; i < binary.length; i += 1) { | ||
| bytes[i] = binary.charCodeAt(i) | ||
| } | ||
| return bytes | ||
| } | ||
|
|
||
| function toArrayBuffer(bytes: Uint8Array): ArrayBuffer { | ||
| return bytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.byteLength) as ArrayBuffer | ||
| } | ||
|
|
||
| async function deriveAesKey(password: string, salt: Uint8Array, iterations: number): Promise<CryptoKey> { | ||
| if (!password) { | ||
| throw new Error('Sync password is required') | ||
| } | ||
| const cryptoImpl = getCrypto() | ||
| const passwordKey = await cryptoImpl.subtle.importKey('raw', new TextEncoder().encode(password), 'PBKDF2', false, [ | ||
| 'deriveKey', | ||
| ]) | ||
| return cryptoImpl.subtle.deriveKey( | ||
| { | ||
| name: 'PBKDF2', | ||
| hash: 'SHA-256', | ||
| salt: toArrayBuffer(salt), | ||
| iterations, | ||
| }, | ||
| passwordKey, | ||
| { | ||
| name: 'AES-GCM', | ||
| length: 256, | ||
| }, | ||
| false, | ||
| ['encrypt', 'decrypt'] | ||
| ) | ||
| } | ||
|
|
||
| export async function encryptJsonEnvelope<T>(payload: T, password: string): Promise<SyncCryptoEnvelope> { | ||
| const cryptoImpl = getCrypto() | ||
| const salt = cryptoImpl.getRandomValues(new Uint8Array(SALT_BYTES)) | ||
| const iv = cryptoImpl.getRandomValues(new Uint8Array(IV_BYTES)) | ||
| const key = await deriveAesKey(password, salt, PBKDF2_ITERATIONS) | ||
| const plaintext = new TextEncoder().encode(JSON.stringify(payload)) | ||
| const ciphertext = await cryptoImpl.subtle.encrypt({ name: 'AES-GCM', iv: toArrayBuffer(iv) }, key, plaintext) | ||
|
|
||
| return { | ||
| version: ENVELOPE_VERSION, | ||
| kdf: 'PBKDF2-SHA256', | ||
| cipher: 'AES-GCM', | ||
| iterations: PBKDF2_ITERATIONS, | ||
| salt: bytesToBase64(salt), | ||
| iv: bytesToBase64(iv), | ||
| ciphertext: bytesToBase64(new Uint8Array(ciphertext)), | ||
| } | ||
| } | ||
|
|
||
| export async function decryptJsonEnvelope<T = unknown>(envelope: SyncCryptoEnvelope, password: string): Promise<T> { | ||
| if (envelope.version !== ENVELOPE_VERSION || envelope.kdf !== 'PBKDF2-SHA256' || envelope.cipher !== 'AES-GCM') { | ||
| throw new Error('Unsupported sync encryption envelope') | ||
| } | ||
|
|
||
| try { | ||
| const cryptoImpl = getCrypto() | ||
| const salt = base64ToBytes(envelope.salt) | ||
| const iv = base64ToBytes(envelope.iv) | ||
| const key = await deriveAesKey(password, salt, envelope.iterations) | ||
| const decrypted = await cryptoImpl.subtle.decrypt( | ||
| { name: 'AES-GCM', iv: toArrayBuffer(iv) }, | ||
| key, | ||
| toArrayBuffer(base64ToBytes(envelope.ciphertext)) | ||
| ) | ||
| return JSON.parse(new TextDecoder().decode(decrypted)) as T | ||
| } catch (error) { | ||
| throw new Error(`Failed to decrypt sync data: ${error instanceof Error ? error.message : String(error)}`) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| export * from './crypto' | ||
| export * from './local' | ||
| export * from './service' | ||
| export * from './snapshot' | ||
| export * from './types' | ||
| export * from './webdav' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| import type { Session, SessionMetaRecord } from '@shared/types' | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest' | ||
| import { listLocalSyncMetas, listLocalSyncSessions } from './local' | ||
|
|
||
| vi.mock('@/stores/chatStore', () => ({ | ||
| listAllSessionsMeta: vi.fn(), | ||
| })) | ||
|
|
||
| vi.mock('@/storage', () => ({ | ||
| default: { | ||
| getItem: vi.fn(), | ||
| }, | ||
| })) | ||
|
|
||
| const { listAllSessionsMeta } = await import('@/stores/chatStore') | ||
| const { default: storage } = await import('@/storage') | ||
|
|
||
| function session(id: string, type?: Session['type']): Session { | ||
| return { | ||
| id, | ||
| type, | ||
| name: id, | ||
| messages: [], | ||
| } | ||
| } | ||
|
|
||
| function meta(id: string, type?: SessionMetaRecord['type']): SessionMetaRecord { | ||
| return { | ||
| id, | ||
| type, | ||
| name: id, | ||
| sortOrder: 1, | ||
| createdAt: 1, | ||
| } | ||
| } | ||
|
|
||
| describe('local sync data selection', () => { | ||
| beforeEach(() => { | ||
| vi.mocked(listAllSessionsMeta).mockReset() | ||
| vi.mocked(storage.getItem).mockReset() | ||
| }) | ||
|
|
||
| it('lists only chat and legacy chat sessions for sync', async () => { | ||
| vi.mocked(listAllSessionsMeta).mockResolvedValue([ | ||
| meta('chat-1', 'chat'), | ||
| meta('legacy-chat'), | ||
| meta('picture-1', 'picture'), | ||
| meta('guide-1', 'guide'), | ||
| ]) | ||
| vi.mocked(storage.getItem).mockImplementation((key) => { | ||
| const id = String(key).replace('session:', '') | ||
| return Promise.resolve(session(id, id === 'legacy-chat' ? undefined : 'chat')) | ||
| }) | ||
|
|
||
| const sessions = await listLocalSyncSessions() | ||
| const metas = await listLocalSyncMetas() | ||
|
|
||
| expect(sessions.map((item) => item.id)).toEqual(['chat-1', 'legacy-chat']) | ||
| expect(metas.map((item) => item.id)).toEqual(['chat-1', 'legacy-chat']) | ||
| expect(storage.getItem).toHaveBeenCalledTimes(2) | ||
| }) | ||
|
|
||
| it('migrates legacy message content before syncing local sessions', async () => { | ||
| vi.mocked(listAllSessionsMeta).mockResolvedValue([meta('legacy-chat')]) | ||
| vi.mocked(storage.getItem).mockResolvedValue({ | ||
| id: 'legacy-chat', | ||
| name: 'Legacy', | ||
| messages: [ | ||
| { | ||
| id: 'message-1', | ||
| role: 'user', | ||
| content: 'legacy text', | ||
| }, | ||
| ], | ||
| } as unknown as Session) | ||
|
|
||
| const sessions = await listLocalSyncSessions() | ||
|
|
||
| expect(sessions[0].messages[0].contentParts).toEqual([{ type: 'text', text: 'legacy text' }]) | ||
| }) | ||
| }) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.