Merge branch 'feat/do-a-complete-review-of-the-extration-66f89a'

* feat/do-a-complete-review-of-the-extration-66f89a:
  fix(sync): correct error class names, JSDoc, and missed cache-clear
This commit is contained in:
Johannes Millan 2026-05-13 19:59:36 +02:00
commit ef4cbff44b
6 changed files with 56 additions and 67 deletions

View file

@ -263,9 +263,12 @@ export const encryptBatch = async (
* Operations with the same salt (e.g., encrypted in the same batch) share
* the cached key, avoiding redundant Argon2id derivations.
*
* SECURITY: Unlike single-item decrypt(), uses explicit format detection so
* Argon2 decryption errors are not silently masked as legacy fallbacks.
* Only ciphertexts too short for Argon2 attempt legacy decryption.
* Format handling mirrors single-item `decrypt()`: ciphertexts in the
* legacy-length range (28..43 bytes) take the PBKDF2 path; >= 44 bytes are
* attempted as Argon2id first, with a legacy fallback on auth failure (a
* long legacy ciphertext can be misclassified as Argon2 by the length
* heuristic); < 28 bytes throws as invalid. The fallback is part of the
* public wire-format contract; see the module-level JSDoc.
*/
export const decryptBatch = async (
dataItems: string[],

View file

@ -1,57 +0,0 @@
import { describe, expect, it, vi } from 'vitest';
import type { ConflictUiPort, SyncConfigPort, SyncConfigSnapshot } from '../src';
describe('sync-core ports', () => {
it('keeps sync config provider IDs as host-owned strings', async () => {
type ProviderId = 'super-sync' | 'file-sync';
const config: SyncConfigSnapshot<ProviderId> = {
isEnabled: true,
syncProvider: 'super-sync',
isEncryptionEnabled: true,
isCompressionEnabled: false,
isManualSyncOnly: false,
syncInterval: 5,
};
const port: SyncConfigPort<ProviderId> = {
getSyncConfig: vi.fn().mockResolvedValue(config),
};
await expect(port.getSyncConfig()).resolves.toBe(config);
expect(port.getSyncConfig).toHaveBeenCalledOnce();
});
it('keeps conflict UI reasons and resolutions host-owned strings', async () => {
type Resolution = 'USE_LOCAL' | 'USE_REMOTE' | 'CANCEL';
const notify = vi.fn();
const port: ConflictUiPort<Resolution> = {
showConflictDialog: vi.fn().mockResolvedValue('USE_LOCAL'),
notify,
};
await expect(
port.showConflictDialog({
conflictType: 'sync-import',
scenario: 'LOCAL_IMPORT_FILTERS_REMOTE',
reason: 'BACKUP_RESTORE',
counts: { filteredOpCount: 3 },
timestamps: { localImportTimestamp: 123 },
meta: { providerId: 'super-sync' },
}),
).resolves.toBe('USE_LOCAL');
port.notify?.({
severity: 'warning',
message: 'sync-conflict',
reason: 'BACKUP_RESTORE',
meta: { filteredOps: 3 },
});
expect(port.showConflictDialog).toHaveBeenCalledOnce();
expect(notify).toHaveBeenCalledWith({
severity: 'warning',
message: 'sync-conflict',
reason: 'BACKUP_RESTORE',
meta: { filteredOps: 3 },
});
});
});

View file

@ -39,11 +39,11 @@ export class AdditionalLogErrorBase<T = unknown[]> extends Error {
// --------------API ERRORS--------------
export class NoRevAPIError extends AdditionalLogErrorBase {
override name = ' NoRevAPIError';
override name = 'NoRevAPIError';
}
export class FileHashCreationAPIError extends AdditionalLogErrorBase {
override name = ' FileHashCreationAPIError';
override name = 'FileHashCreationAPIError';
}
export class TooManyRequestsAPIError extends AdditionalLogErrorBase<{
@ -51,7 +51,7 @@ export class TooManyRequestsAPIError extends AdditionalLogErrorBase<{
retryAfter?: number;
path?: string;
}> {
override name = ' TooManyRequestsAPIError';
override name = 'TooManyRequestsAPIError';
readonly status: number;
readonly retryAfter?: number;
readonly path?: string;
@ -66,19 +66,19 @@ export class TooManyRequestsAPIError extends AdditionalLogErrorBase<{
}
export class RemoteFileNotFoundAPIError extends AdditionalLogErrorBase {
override name = ' RemoteFileNotFoundAPIError';
override name = 'RemoteFileNotFoundAPIError';
}
export class MissingRefreshTokenAPIError extends Error {
override name = ' MissingRefreshTokenAPIError';
override name = 'MissingRefreshTokenAPIError';
}
export class UploadRevToMatchMismatchAPIError extends AdditionalLogErrorBase {
override name = ' UploadRevToMatchMismatchAP';
override name = 'UploadRevToMatchMismatchAPIError';
}
export class HttpNotOkAPIError extends AdditionalLogErrorBase {
override name = ' HttpNotOkAPIError';
override name = 'HttpNotOkAPIError';
/**
* Raw `Response` object retained so callers can read `.status` /
* `.statusText`.

View file

@ -9,6 +9,7 @@ import {
InvalidDataSPError,
MissingCredentialsSPError,
MissingRefreshTokenAPIError,
NetworkUnavailableSPError,
NoRevAPIError,
PotentialCorsError,
RemoteFileChangedUnexpectedly,
@ -171,6 +172,7 @@ describe('Error class identity (single definition per class)', () => {
['UploadRevToMatchMismatchAPIError', UploadRevToMatchMismatchAPIError],
['PotentialCorsError', PotentialCorsError],
['RemoteFileChangedUnexpectedly', RemoteFileChangedUnexpectedly],
['NetworkUnavailableSPError', NetworkUnavailableSPError],
];
it.each(ERROR_CLASSES)('%s is an Error subclass', (_name, ErrCtor) => {
@ -178,6 +180,33 @@ describe('Error class identity (single definition per class)', () => {
expect(Object.create(ErrCtor.prototype) instanceof Error).toBe(true);
});
// Regression guard: prior versions shipped seven classes with a leading
// space in `name` (' NoRevAPIError') and one truncated to
// ' UploadRevToMatchMismatchAP'. Equality with the class identifier keeps
// the string identity log-correct and matches Error subclass convention.
// Asserting against `ErrCtor.name` (the static class identifier, which TS
// cannot synthesize with a leading space) rather than the tuple string
// means a future "fix" that co-mutates the test fixture and the source
// cannot mask a regression.
// `name` is an instance override (not a prototype property), so we must
// instantiate. Each class has a different constructor signature; pass a
// minimal stub that exercises the field-only branches.
it.each(ERROR_CLASSES)('%s instance.name matches the class name', (_name, ErrCtor) => {
let instance: Error;
if (ErrCtor === HttpNotOkAPIError) {
instance = new HttpNotOkAPIError(
new Response(null, { status: 500, statusText: 'Internal Server Error' }),
);
} else if (ErrCtor === TooManyRequestsAPIError) {
instance = new TooManyRequestsAPIError({ status: 429 });
} else if (ErrCtor === PotentialCorsError) {
instance = new PotentialCorsError('https://example.test');
} else {
instance = new (ErrCtor as new (...a: never[]) => Error)();
}
expect(instance.name).toBe(ErrCtor.name);
});
it('EmptyRemoteBodySPError extends InvalidDataSPError', () => {
const err = new EmptyRemoteBodySPError('empty body');
expect(err).toBeInstanceOf(InvalidDataSPError);

View file

@ -96,6 +96,9 @@ describe('SuperSyncEncryptionToggleService', () => {
isEncryptionEnabled: true,
logPrefix: 'SuperSyncEncryptionToggleService',
});
// clearSessionKeyCache() is called directly on success (module-level
// function, not spyable). Matches the pattern in
// file-based-encryption.service.ts and encryption-password-change.service.ts.
});
it('should revert config on failure using pre-captured config (preserving auth credentials)', async () => {
@ -157,6 +160,9 @@ describe('SuperSyncEncryptionToggleService', () => {
isEncryptionEnabled: false,
logPrefix: 'SuperSyncEncryptionToggleService',
});
// clearSessionKeyCache() is called directly on success (module-level
// function, not spyable). Matches the pattern in
// file-based-encryption.service.ts and encryption-password-change.service.ts.
});
it('should revert config to original state (including encryption key) on failure', async () => {

View file

@ -1,5 +1,6 @@
import { inject, Injectable } from '@angular/core';
import type { SuperSyncPrivateCfg } from '@sp/sync-providers/super-sync';
import { clearSessionKeyCache } from '@sp/sync-core';
import { SyncLog } from '../../core/log';
import { SnapshotUploadService } from './snapshot-upload.service';
import { SyncProviderManager } from '../../op-log/sync-providers/provider-manager.service';
@ -66,6 +67,10 @@ export class SuperSyncEncryptionToggleService {
logPrefix: LOG_PREFIX,
});
// Drop any cached derived key from the pre-toggle password; subsequent
// sync cycles must re-derive against the new `encryptKey`.
clearSessionKeyCache();
SyncLog.normal(`${LOG_PREFIX}: Encryption enabled successfully!`);
} catch (error) {
// Revert config on failure (server data is already deleted at this point)
@ -109,6 +114,9 @@ export class SuperSyncEncryptionToggleService {
logPrefix: LOG_PREFIX,
});
// Drop any cached derived key now that encryption is off.
clearSessionKeyCache();
SyncLog.normal(`${LOG_PREFIX}: Encryption disabled successfully!`);
} catch (uploadError) {
SyncLog.err(