From 3bb07fc6d00a45239f0460ae2a44de2bdcd414ee Mon Sep 17 00:00:00 2001 From: johannesjo Date: Thu, 14 May 2026 12:11:48 +0200 Subject: [PATCH] fix(sync): address package review feedback --- docs/sync-and-op-log/README.md | 2 +- docs/sync-and-op-log/package-boundaries.md | 9 +++++---- electron/tsconfig.electron.json | 3 ++- package-lock.json | 5 ++++- packages/sync-core/README.md | 11 +++++------ packages/sync-core/package.json | 3 ++- .../tests/conflict-resolution.spec.ts | 13 ++++++++----- packages/sync-core/tsconfig.spec.json | 13 +++++++++++++ packages/sync-providers/package.json | 13 ++++++++++++- packages/sync-providers/src/log.ts | 1 + .../file-based/dropbox/dropbox-api.spec.ts | 4 ++-- .../tests/log/error-meta.spec.ts | 2 +- packages/sync-providers/tsconfig.spec.json | 18 ++++++++++++++++++ packages/sync-providers/tsup.config.ts | 1 + src/tsconfig.spec.json | 3 ++- tsconfig.base.json | 3 ++- 16 files changed, 79 insertions(+), 25 deletions(-) create mode 100644 packages/sync-core/tsconfig.spec.json create mode 100644 packages/sync-providers/src/log.ts create mode 100644 packages/sync-providers/tsconfig.spec.json diff --git a/docs/sync-and-op-log/README.md b/docs/sync-and-op-log/README.md index 07fda89ea5..ef69b39346 100644 --- a/docs/sync-and-op-log/README.md +++ b/docs/sync-and-op-log/README.md @@ -134,7 +134,7 @@ packages/sync-providers/src/ ├── file-based/dropbox/ # Dropbox provider implementation ├── file-based/webdav/ # WebDAV + Nextcloud providers ├── file-based/local-file/ # LocalFile provider classes -└── provider.types.ts # Provider-neutral contracts +└── provider-types.ts # Provider-neutral contracts ``` ### App Sync Wiring diff --git a/docs/sync-and-op-log/package-boundaries.md b/docs/sync-and-op-log/package-boundaries.md index ff715dd78a..c733f79f95 100644 --- a/docs/sync-and-op-log/package-boundaries.md +++ b/docs/sync-and-op-log/package-boundaries.md @@ -111,10 +111,11 @@ the package barrel deliberately and check that it is not app-owned. The root `@sp/sync-providers` barrel has been removed; consumers MUST import from focused subpath barrels: `@sp/sync-providers/dropbox`, `/webdav`, `/super-sync`, `/local-file`, `/http`, `/errors`, `/file-based`, `/pkce`, -`/platform`, `/provider-types`, and `/credential-store`. Provider classes and -provider-owned string constants are exported there, but app enums such as -`SyncProviderId` are not. Internal helpers such as WebDAV API/adapter classes -stay unexported unless a second host needs them. +`/platform`, `/provider-types`, `/credential-store`, and `/log`. Provider +classes, provider-owned string constants, and shared privacy-boundary logging +helpers are exported there, but app enums such as `SyncProviderId` are not. +Internal helpers such as WebDAV API/adapter classes stay unexported unless a +second host needs them. `@sp/sync-core` still exports deprecated full-state op compatibility defaults and host-defined `OpType.SyncImport` / `BackupImport` / `Repair` strings for diff --git a/electron/tsconfig.electron.json b/electron/tsconfig.electron.json index 6f542a838e..0431c0a006 100644 --- a/electron/tsconfig.electron.json +++ b/electron/tsconfig.electron.json @@ -33,7 +33,8 @@ "@sp/sync-providers/platform": ["packages/sync-providers/dist/platform.d.ts"], "@sp/sync-providers/provider-types": [ "packages/sync-providers/dist/provider-types.d.ts" - ] + ], + "@sp/sync-providers/log": ["packages/sync-providers/dist/log.d.ts"] } }, "include": ["main.ts", "**/*.ts", "../src/app/core/window-ea.d.ts"], diff --git a/package-lock.json b/package-lock.json index 1d3498e4d6..2260692896 100644 --- a/package-lock.json +++ b/package-lock.json @@ -29976,14 +29976,17 @@ "name": "@sp/sync-providers", "version": "1.0.0", "dependencies": { - "@sp/sync-core": "*", "hash-wasm": "^4.12.0" }, "devDependencies": { + "@sp/sync-core": "*", "@xmldom/xmldom": "^0.8.12", "tsup": "^8.0.0", "typescript": "^5.0.0", "vitest": "^3.2.4" + }, + "peerDependencies": { + "@sp/sync-core": "*" } }, "packages/sync-providers/node_modules/@vitest/expect": { diff --git a/packages/sync-core/README.md b/packages/sync-core/README.md index 0be7ef4ec5..afb4d01206 100644 --- a/packages/sync-core/README.md +++ b/packages/sync-core/README.md @@ -32,7 +32,7 @@ All ciphertexts are base64-encoded for transport. The format is discriminated by ### Salt and IV semantics - The **IV** (12 bytes) is freshly random per call. AES-GCM security under a fixed key reduces to IV uniqueness, which this guarantees. -- The **salt** (16 bytes) is derived once per `(process session, password)` pair and reused across every `encrypt`/`encryptWithDerivedKey`/`encryptBatch` call in that session. This is intentional — it lets the session cache amortize the ~500 ms–2 s Argon2id derivation. Two encryptions of the same plaintext within a session therefore share the salt prefix and differ only in IV and ciphertext. Do not assert per-call salt uniqueness in tests. +- The **salt** (16 bytes) is derived once per `(process session, password)` pair and reused across every `encrypt`/`encryptBatch` call in that session. This is intentional — it lets the session cache amortize the ~500 ms–2 s Argon2id derivation. Two encryptions of the same plaintext within a session therefore share the salt prefix and differ only in IV and ciphertext. Do not assert per-call salt uniqueness in tests. ### Session key caching @@ -42,10 +42,9 @@ Call `clearSessionKeyCache()` whenever the user changes their password or logs o ### Legacy-KDF migration -Old data was encrypted with PBKDF2 using the password as its own salt — cryptographically weak. Two complementary mechanisms surface legacy ciphertext: +Old data was encrypted with PBKDF2 using the password as its own salt — cryptographically weak. `decrypt()` and `decryptBatch()` still read legacy ciphertexts so existing sync data remains accessible. -1. **Structural** — `decryptWithMigration(data, password)` returns a `DecryptResult` with `wasLegacyKdf` and `migratedCiphertext`. Persist `migratedCiphertext` to migrate the record off PBKDF2. -2. **Side-channel** — `setLegacyKdfWarningHandler(fn)` registers a callback fired on every successful legacy decrypt, regardless of which entry point was used. The host throttles user-facing messages (e.g. show a deprecation banner once per session). +`setLegacyKdfWarningHandler(fn)` registers a callback fired on every successful legacy decrypt, regardless of which entry point was used. The host throttles user-facing messages (e.g. show a deprecation banner once per session). ### Argon2id parameters @@ -66,9 +65,9 @@ See `src/index.ts` for the full barrel and the JSDoc on individual symbols for u ## Tests ```bash -npm test # vitest run, Node WebCrypto + @noble fallback +npm test # typecheck specs + vitest run, Node WebCrypto + @noble fallback npm run test:watch # watch mode -npm run build # tsup → ESM + CJS + .d.ts +npm run build # tsup -> ESM + CJS + .d.ts ``` Browser-context smoke coverage lives in the consuming app at `src/app/op-log/encryption/encryption.browser.spec.ts` (Karma + real Chrome). diff --git a/packages/sync-core/package.json b/packages/sync-core/package.json index 422d6156a1..aaa86b45ca 100644 --- a/packages/sync-core/package.json +++ b/packages/sync-core/package.json @@ -21,7 +21,8 @@ "scripts": { "build": "tsup", "build:tsc": "tsc", - "test": "vitest run", + "test": "npm run test:typecheck && vitest run", + "test:typecheck": "tsc --noEmit -p tsconfig.spec.json", "test:watch": "vitest" }, "dependencies": { diff --git a/packages/sync-core/tests/conflict-resolution.spec.ts b/packages/sync-core/tests/conflict-resolution.spec.ts index c65d60d1f7..016810e2a8 100644 --- a/packages/sync-core/tests/conflict-resolution.spec.ts +++ b/packages/sync-core/tests/conflict-resolution.spec.ts @@ -545,17 +545,20 @@ describe('partitionLwwResolutions', () => { [createOp({ id: 'local' })], [createOp({ id: 'remote', actionType: '[Test] Remote' })], ); - const processedRemoteOp = { + const processedRemoteOp: Operation = { ...conflict.remoteOps[0], id: 'processed-remote', entityId: 'processed-task', actionType: '[Test] Processed Remote', }; - const processRemoteWinnerOps = vi.fn(() => [processedRemoteOp]); + const processRemoteWinnerOps = vi.fn((_conflict: EntityConflict): Operation[] => [ + processedRemoteOp, + ]); - const result = partitionLwwResolutions([{ conflict, winner: 'remote' }], { - processRemoteWinnerOps, - }); + const result = partitionLwwResolutions( + [{ conflict, winner: 'remote' }], + { processRemoteWinnerOps }, + ); expect(processRemoteWinnerOps).toHaveBeenCalledWith(conflict); expect(result.remoteWinsOps).toEqual([processedRemoteOp]); diff --git a/packages/sync-core/tsconfig.spec.json b/packages/sync-core/tsconfig.spec.json new file mode 100644 index 0000000000..008be266a9 --- /dev/null +++ b/packages/sync-core/tsconfig.spec.json @@ -0,0 +1,13 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "composite": false, + "declaration": false, + "declarationMap": false, + "noEmit": true, + "rootDir": ".", + "types": ["vitest"] + }, + "include": ["src/**/*", "tests/**/*"], + "exclude": ["node_modules", "dist"] +} diff --git a/packages/sync-providers/package.json b/packages/sync-providers/package.json index 1e7b2ca048..a37b033626 100644 --- a/packages/sync-providers/package.json +++ b/packages/sync-providers/package.json @@ -113,6 +113,16 @@ "types": "./dist/provider-types.d.ts", "default": "./dist/provider-types.js" } + }, + "./log": { + "import": { + "types": "./dist/log.d.mts", + "default": "./dist/log.mjs" + }, + "require": { + "types": "./dist/log.d.ts", + "default": "./dist/log.js" + } } }, "dependencies": { @@ -124,7 +134,8 @@ "scripts": { "build": "tsup", "build:tsc": "tsc", - "test": "vitest run", + "test": "npm run test:typecheck && vitest run", + "test:typecheck": "tsc --noEmit -p tsconfig.spec.json", "test:watch": "vitest" }, "devDependencies": { diff --git a/packages/sync-providers/src/log.ts b/packages/sync-providers/src/log.ts new file mode 100644 index 0000000000..f56df87b68 --- /dev/null +++ b/packages/sync-providers/src/log.ts @@ -0,0 +1 @@ +export { errorMeta, urlPathOnly } from './log/error-meta'; diff --git a/packages/sync-providers/tests/file-based/dropbox/dropbox-api.spec.ts b/packages/sync-providers/tests/file-based/dropbox/dropbox-api.spec.ts index 006ec50ccc..32febfdf3a 100644 --- a/packages/sync-providers/tests/file-based/dropbox/dropbox-api.spec.ts +++ b/packages/sync-providers/tests/file-based/dropbox/dropbox-api.spec.ts @@ -11,7 +11,7 @@ import { type DropboxPrivateCfg, PROVIDER_ID_DROPBOX, } from '../../../src/dropbox'; -import type { NativeHttpExecutor } from '../../../src/http'; +import type { NativeHttpExecutor, NativeHttpResponse } from '../../../src/http'; import type { SyncCredentialStorePort } from '../../../src/credential-store'; import { DropboxApi } from '../../../src/file-based/dropbox/dropbox-api'; import { createMockSyncLogger } from '../../helpers/sync-logger'; @@ -758,7 +758,7 @@ describe('DropboxApi Native Platform Routing', () => { describe('error handling on native platform', () => { it('should handle 401 errors and retry on native platform', async () => { let callCount = 0; - nativeExecutor.mockImplementation(async (options) => { + nativeExecutor.mockImplementation(async (options): Promise => { callCount++; if (callCount === 1) { diff --git a/packages/sync-providers/tests/log/error-meta.spec.ts b/packages/sync-providers/tests/log/error-meta.spec.ts index c9559c116f..00a35dfda3 100644 --- a/packages/sync-providers/tests/log/error-meta.spec.ts +++ b/packages/sync-providers/tests/log/error-meta.spec.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { errorMeta, urlPathOnly } from '../../src/log/error-meta'; +import { errorMeta, urlPathOnly } from '@sp/sync-providers/log'; describe('urlPathOnly', () => { it('strips query string', () => { diff --git a/packages/sync-providers/tsconfig.spec.json b/packages/sync-providers/tsconfig.spec.json new file mode 100644 index 0000000000..d0e1fe6cfd --- /dev/null +++ b/packages/sync-providers/tsconfig.spec.json @@ -0,0 +1,18 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "baseUrl": ".", + "composite": false, + "declaration": false, + "declarationMap": false, + "noEmit": true, + "paths": { + "@sp/sync-core": ["../sync-core/src/index.ts"], + "@sp/sync-providers/log": ["src/log.ts"] + }, + "rootDir": "..", + "types": ["vitest"] + }, + "include": ["src/**/*", "tests/**/*"], + "exclude": ["node_modules", "dist"] +} diff --git a/packages/sync-providers/tsup.config.ts b/packages/sync-providers/tsup.config.ts index 36983fb5b0..7e14f0392a 100644 --- a/packages/sync-providers/tsup.config.ts +++ b/packages/sync-providers/tsup.config.ts @@ -13,6 +13,7 @@ export default defineConfig({ 'src/pkce.ts', 'src/platform.ts', 'src/provider-types.ts', + 'src/log.ts', ], format: ['esm', 'cjs'], tsconfig: 'tsconfig.build.json', diff --git a/src/tsconfig.spec.json b/src/tsconfig.spec.json index 01004b6969..c3962aa4ab 100644 --- a/src/tsconfig.spec.json +++ b/src/tsconfig.spec.json @@ -22,7 +22,8 @@ "@sp/sync-providers/platform": ["packages/sync-providers/src/platform.ts"], "@sp/sync-providers/provider-types": [ "packages/sync-providers/src/provider-types.ts" - ] + ], + "@sp/sync-providers/log": ["packages/sync-providers/src/log.ts"] } }, "files": ["test.ts", "polyfills.ts"], diff --git a/tsconfig.base.json b/tsconfig.base.json index 31d7709d8b..18ad94aab1 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -41,7 +41,8 @@ "@sp/sync-providers/platform": ["packages/sync-providers/src/platform.ts"], "@sp/sync-providers/provider-types": [ "packages/sync-providers/src/provider-types.ts" - ] + ], + "@sp/sync-providers/log": ["packages/sync-providers/src/log.ts"] }, "plugins": [ {