-
-
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 15 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
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,227 @@ | ||
| 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' }, | ||
| memorizedManualLicenseKey: 'memorized-license-secret', | ||
| lastSelectedLicenseByUser: { 'alice@example.com': 'selected-license-secret' }, | ||
| providers: { | ||
| openai: { | ||
| apiKey: 'sk-secret', | ||
| accessKey: 'access-secret', | ||
| secretKey: 'secret-key', | ||
| sessionToken: 'session-token', | ||
| apiHost: 'https://api.example.com', | ||
| oauth: { | ||
| accessToken: 'oauth-access-secret', | ||
| refreshToken: 'oauth-refresh-secret', | ||
| expiresAt: 1893456000000, | ||
| }, | ||
| activeAuthMode: 'oauth', | ||
| }, | ||
| }, | ||
| 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', | ||
| lastSeenEndpoint: 'https://dav.example.com/files/me/\nalice', | ||
| lastSeenETag: '"seen-etag"', | ||
| }, | ||
| extension: { | ||
| ...defaults.settings().extension, | ||
| webSearch: { | ||
| ...defaults.settings().extension.webSearch, | ||
| provider: 'tavily', | ||
| tavilyApiKey: 'tavily-secret', | ||
| bochaApiKey: 'bocha-secret', | ||
| queritApiKey: 'querit-secret', | ||
| }, | ||
| documentParser: { | ||
| type: 'mineru', | ||
| mineru: { apiToken: 'mineru-secret' }, | ||
| }, | ||
| }, | ||
| mcp: { | ||
| servers: [ | ||
| { | ||
| id: 'local-fs', | ||
| name: 'Local FS', | ||
| enabled: true, | ||
| transport: { | ||
| type: 'stdio', | ||
| command: 'npx', | ||
| args: ['srv'], | ||
| env: { GITHUB_TOKEN: 'ghp-secret' }, | ||
| }, | ||
| }, | ||
| { | ||
| id: 'remote-http', | ||
| name: 'Remote HTTP', | ||
| enabled: true, | ||
| transport: { | ||
| type: 'http', | ||
| url: 'https://mcp.example.com/', | ||
| headers: { Authorization: 'Bearer secret' }, | ||
| }, | ||
| }, | ||
| ], | ||
| enabledBuiltinServers: [], | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| 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', | ||
| activeAuthMode: 'oauth', | ||
| }) | ||
| 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('removes nested OAuth credentials when key export is not selected', () => { | ||
| const settings = settingsWithSecrets() | ||
| const sanitized = sanitizeSettingsForExport(settings, false) | ||
|
|
||
| expect(sanitized.providers?.openai?.oauth).toBeUndefined() | ||
| // The original settings object must keep its credentials untouched. | ||
| expect(settings.providers?.openai?.oauth?.accessToken).toBe('oauth-access-secret') | ||
| expect(settings.providers?.openai?.oauth?.refreshToken).toBe('oauth-refresh-secret') | ||
| }) | ||
|
|
||
| it('removes remembered and per-account license keys when key export is not selected', () => { | ||
| const sanitized = sanitizeSettingsForExport(settingsWithSecrets(), false) | ||
|
|
||
| expect(sanitized.memorizedManualLicenseKey).toBeUndefined() | ||
| expect(sanitized.lastSelectedLicenseByUser).toBeUndefined() | ||
| }) | ||
|
|
||
| it('removes web search API keys when key export is not selected', () => { | ||
| const settings = settingsWithSecrets() | ||
| const sanitized = sanitizeSettingsForExport(settings, false) | ||
|
|
||
| expect(sanitized.extension.webSearch.provider).toBe('tavily') | ||
| expect(sanitized.extension.webSearch.tavilyApiKey).toBeUndefined() | ||
| expect(sanitized.extension.webSearch.bochaApiKey).toBeUndefined() | ||
| expect(sanitized.extension.webSearch.queritApiKey).toBeUndefined() | ||
| // The original settings object must keep its keys untouched. | ||
| expect(settings.extension.webSearch.tavilyApiKey).toBe('tavily-secret') | ||
| expect(settings.extension.webSearch.bochaApiKey).toBe('bocha-secret') | ||
| expect(settings.extension.webSearch.queritApiKey).toBe('querit-secret') | ||
| }) | ||
|
|
||
| it('removes device-local sync state from every export', () => { | ||
| const withSecrets = sanitizeSettingsForExport(settingsWithSecrets(), true) | ||
| const withoutSecrets = sanitizeSettingsForExport(settingsWithSecrets(), false) | ||
|
|
||
| // lastSeen is this device's sync identity, not user configuration: a | ||
| // restored device inheriting it would skip its first download merge | ||
| // against the unchanged remote snapshot. Strip it regardless of the | ||
| // includeSecrets flag. | ||
| for (const sanitized of [withSecrets, withoutSecrets]) { | ||
| expect(sanitized.sync.lastSyncedAt).toBeUndefined() | ||
| expect(sanitized.sync.lastSeenEndpoint).toBeUndefined() | ||
| expect(sanitized.sync.lastSeenETag).toBeUndefined() | ||
| } | ||
| // Credentials still follow the includeSecrets flag; only the device-local | ||
| // state is always stripped. | ||
| expect(withSecrets.sync.webdav.password).toBe('dav-secret') | ||
| expect(withoutSecrets.sync.webdav.password).toBe('') | ||
| }) | ||
|
|
||
| it('removes the MinerU API token when key export is not selected', () => { | ||
| const settings = settingsWithSecrets() | ||
| const sanitized = sanitizeSettingsForExport(settings, false) | ||
|
|
||
| expect(sanitized.extension.documentParser?.type).toBe('mineru') | ||
| expect(sanitized.extension.documentParser?.mineru?.apiToken).toBe('') | ||
| expect(sanitizeSettingsForExport(settingsWithSecrets(), true).extension.documentParser?.mineru?.apiToken).toBe( | ||
| 'mineru-secret' | ||
| ) | ||
| // The original settings object must keep its token untouched. | ||
| expect(settings.extension.documentParser?.mineru?.apiToken).toBe('mineru-secret') | ||
| }) | ||
|
|
||
| it('removes MCP transport env and headers when key export is not selected', () => { | ||
| const settings = settingsWithSecrets() | ||
| const sanitized = sanitizeSettingsForExport(settings, false) | ||
|
|
||
| const [stdioServer, httpServer] = sanitized.mcp.servers | ||
| expect(stdioServer.transport).toEqual({ type: 'stdio', command: 'npx', args: ['srv'] }) | ||
| expect(httpServer.transport).toEqual({ type: 'http', url: 'https://mcp.example.com/' }) | ||
| // Transport credentials survive when key export is selected. | ||
| const withSecrets = sanitizeSettingsForExport(settingsWithSecrets(), true) | ||
| expect(withSecrets.mcp.servers[0].transport).toMatchObject({ env: { GITHUB_TOKEN: 'ghp-secret' } }) | ||
| expect(withSecrets.mcp.servers[1].transport).toMatchObject({ headers: { Authorization: 'Bearer secret' } }) | ||
| // The original settings object must keep its credentials untouched. | ||
| expect(settings.mcp.servers[0].transport).toMatchObject({ env: { GITHUB_TOKEN: 'ghp-secret' } }) | ||
| expect(settings.mcp.servers[1].transport).toMatchObject({ headers: { Authorization: 'Bearer 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.memorizedManualLicenseKey).toBe('memorized-license-secret') | ||
| expect(sanitized.lastSelectedLicenseByUser).toEqual({ 'alice@example.com': 'selected-license-secret' }) | ||
| expect(sanitized.providers?.openai?.apiKey).toBe('sk-secret') | ||
| expect(sanitized.providers?.openai?.oauth?.accessToken).toBe('oauth-access-secret') | ||
| expect(sanitized.providers?.openai?.oauth?.refreshToken).toBe('oauth-refresh-secret') | ||
| expect(sanitized.extension.webSearch.tavilyApiKey).toBe('tavily-secret') | ||
| expect(sanitized.extension.webSearch.bochaApiKey).toBe('bocha-secret') | ||
| expect(sanitized.extension.webSearch.queritApiKey).toBe('querit-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() | ||
| }) | ||
|
|
||
| it('does not crash on settings persisted before sync and extension fields existed', () => { | ||
| // Raw storage from older app versions lacks these objects entirely; the | ||
| // export path casts without schema parsing, so sanitize must cope. | ||
| const legacy = { | ||
| ...defaults.settings(), | ||
| licenseKey: 'license-secret', | ||
| providers: { | ||
| openai: { | ||
| apiKey: 'sk-secret', | ||
| apiHost: 'https://api.example.com', | ||
| }, | ||
| }, | ||
| sync: undefined, | ||
| extension: undefined, | ||
| } as unknown as Settings | ||
|
|
||
| const sanitized = sanitizeSettingsForExport(legacy, false) | ||
|
|
||
| expect(sanitized.licenseKey).toBeUndefined() | ||
| expect(sanitized.providers?.openai).toEqual({ apiHost: 'https://api.example.com' }) | ||
| expect(sanitized.sync).toBeUndefined() | ||
| expect(sanitized.extension).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,103 @@ | ||
| 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 | ||
| // Nested OAuth credentials (accessToken / refreshToken / extra tokens) are | ||
| // just as sensitive as API keys; the whole object must not be exported. | ||
| delete cleanedProvider.oauth | ||
| return cleanedProvider | ||
| } | ||
|
|
||
| export function sanitizeSettingsForExport(settings: Settings, includeSecrets: boolean): Settings { | ||
| // The export path reads raw storage via a type cast, and settings persisted | ||
| // before the sync/extension fields existed are not re-normalized until the | ||
| // next settings save — so both objects can be absent at runtime despite | ||
| // being required in the schema. Spread and redact defensively. | ||
| const cleanedSettings: Settings = { | ||
| ...settings, | ||
| licenseDetail: undefined, | ||
| licenseInstances: undefined, | ||
| providers: settings.providers ? { ...settings.providers } : settings.providers, | ||
| extension: settings.extension | ||
| ? { | ||
| ...settings.extension, | ||
| webSearch: { | ||
| ...settings.extension.webSearch, | ||
| }, | ||
| documentParser: settings.extension.documentParser | ||
| ? { | ||
| ...settings.extension.documentParser, | ||
| mineru: settings.extension.documentParser.mineru | ||
| ? { ...settings.extension.documentParser.mineru } | ||
| : undefined, | ||
| } | ||
| : settings.extension.documentParser, | ||
| } | ||
| : settings.extension, | ||
| sync: settings.sync | ||
| ? { | ||
| ...settings.sync, | ||
|
themez marked this conversation as resolved.
|
||
| // Device-local sync state (which snapshot this device last saw, and | ||
| // when it synced) must not travel with an export: a restored device | ||
| // would inherit the old device's lastSeen identity and skip its | ||
| // first download merge against the unchanged remote snapshot. | ||
| lastSyncedAt: undefined, | ||
| lastSeenEndpoint: undefined, | ||
| lastSeenETag: undefined, | ||
| webdav: { | ||
| ...settings.sync.webdav, | ||
| }, | ||
| } | ||
| : settings.sync, | ||
| mcp: settings.mcp | ||
| ? { | ||
| ...settings.mcp, | ||
| servers: Array.isArray(settings.mcp.servers) | ||
| ? settings.mcp.servers.map((server) => ({ ...server, transport: { ...server.transport } })) | ||
| : settings.mcp.servers, | ||
| } | ||
| : settings.mcp, | ||
| } | ||
|
|
||
| if (!includeSecrets) { | ||
|
themez marked this conversation as resolved.
|
||
| delete cleanedSettings.licenseKey | ||
| // License keys remembered for the UI or selected per account are still | ||
| // credentials even though the active licenseKey lives elsewhere. | ||
| delete cleanedSettings.memorizedManualLicenseKey | ||
| delete cleanedSettings.lastSelectedLicenseByUser | ||
| if (cleanedSettings.providers) { | ||
| cleanedSettings.providers = Object.fromEntries( | ||
| Object.entries(cleanedSettings.providers).map(([id, provider]) => [id, sanitizeProviderForExport(provider)]) | ||
| ) as Settings['providers'] | ||
| } | ||
| if (cleanedSettings.extension?.webSearch) { | ||
| delete cleanedSettings.extension.webSearch.tavilyApiKey | ||
| delete cleanedSettings.extension.webSearch.bochaApiKey | ||
| delete cleanedSettings.extension.webSearch.queritApiKey | ||
| } | ||
| if (cleanedSettings.sync?.webdav) { | ||
| cleanedSettings.sync.webdav.password = '' | ||
| cleanedSettings.sync.webdav.syncPassword = '' | ||
| } | ||
| if (cleanedSettings.extension?.documentParser?.mineru) { | ||
| cleanedSettings.extension.documentParser.mineru.apiToken = '' | ||
| } | ||
| // MCP transports routinely carry credentials — stdio env vars (API tokens) | ||
| // and HTTP headers (Authorization) must not leave the device in an export. | ||
| if (cleanedSettings.mcp?.servers) { | ||
| for (const server of cleanedSettings.mcp.servers) { | ||
| if (server.transport.type === 'stdio') { | ||
| delete server.transport.env | ||
| } else { | ||
| delete server.transport.headers | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return cleanedSettings | ||
| } | ||
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.