From ceefb5000c21ba4d626c1c69a0f600a341fd7196 Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Mon, 29 Jun 2026 17:14:36 +0200 Subject: [PATCH] feat(plugins): add local-only secret storage API for plugins (#8633) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(plugins): add local-only secret storage API for plugins Add setSecret/getSecret/deleteSecret to the plugin API, backed by a dedicated 'sup-plugin-secrets' IndexedDB that is never part of Super Productivity's sync, exports, or backups (mirrors the existing OAuth token store). Secrets are namespaced per plugin and purged on uninstall. Unblocks credential-using plugins (e.g. IMAP mailbox -> task) that must not put passwords in persistDataSynced or synced issue-provider config. Also purge plugin OAuth tokens on uninstall (best-effort, alongside the secret purge) — they previously leaked past uninstall. Refs #7511 * fix(plugins): purge plugin secrets and OAuth tokens on cache clear clearUploadedPluginsFromMemory (the 'Clear plugin cache' action) wiped plugin code and persisted nodeExecution consent (#8512 Phase 2) but left secrets and OAuth tokens in their dedicated stores. A same-id re-upload after a cache clear has no existingState, so the re-upload purge never fires and the new plugin could inherit the previous plugin's credentials — the same id-reuse gap #8512 closed for consent. Purge both here too, best-effort and idempotent, mirroring the per-plugin uninstall purge. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> --- docs/plugin-development.md | 68 ++++++++++++++ packages/plugin-api/src/types.ts | 16 ++++ src/app/plugins/plugin-api.ts | 13 +++ src/app/plugins/plugin-bridge.service.ts | 13 +++ src/app/plugins/plugin.service.spec.ts | 74 +++++++++++++++ src/app/plugins/plugin.service.ts | 54 ++++++++--- src/app/plugins/secret/plugin-secret-store.ts | 93 +++++++++++++++++++ .../secret/plugin-secret.service.spec.ts | 91 ++++++++++++++++++ .../plugins/secret/plugin-secret.service.ts | 85 +++++++++++++++++ 9 files changed, 496 insertions(+), 11 deletions(-) create mode 100644 src/app/plugins/secret/plugin-secret-store.ts create mode 100644 src/app/plugins/secret/plugin-secret.service.spec.ts create mode 100644 src/app/plugins/secret/plugin-secret.service.ts diff --git a/docs/plugin-development.md b/docs/plugin-development.md index c92900612d..730fe90b20 100644 --- a/docs/plugin-development.md +++ b/docs/plugin-development.md @@ -528,6 +528,74 @@ const data = await PluginAPI.loadSyncedData(); console.log(data); // '{ count: 42 }' ``` +### Secret Storage + +For credentials — IMAP/SMTP passwords, API tokens, app passwords — use +`setSecret` / `getSecret` / `deleteSecret`. Secrets are stored **local-only**: +they are never synced, exported, or included in backups, and each plugin can +only read its own keys. + +```javascript +// Store a credential (key must be a non-empty string) +await PluginAPI.setSecret('imapPassword', 'app-password-123'); + +// Read it back when you need to connect +const pw = await PluginAPI.getSecret('imapPassword'); // string | null + +// Remove it (e.g. when the user disconnects) +await PluginAPI.deleteSecret('imapPassword'); +``` + +Rules of thumb: + +- **Never** put a credential in `persistDataSynced` or in issue-provider + config — those sync to the server and land in exports/backups. Keep only + non-secret connection details there (host, port, username, filters) and put + the password/token in secret storage. +- Secrets are **per-device**: a value set on desktop is not available on mobile, + so prompt the user to enter the credential on each device. (This matches how + IMAP app-passwords are typically used anyway.) +- Secrets are stored unencrypted at rest today (the same as plugin OAuth + tokens); the guarantee is "stays on this device, never synced," not + hardware-level encryption. Don't store anything you wouldn't accept living in + the app's local profile. +- All secrets for a plugin are purged automatically when the plugin is + uninstalled. + +#### Secrets in issue-provider plugins + +Issue-provider plugins get the same secret API (an issue provider is a normal +plugin that also calls `registerIssueProvider`). Your definition callbacks +(`getHeaders`, `getById`, `searchIssues`, …) run in your plugin's context, so +they can read secrets directly: + +```javascript +PluginAPI.registerIssueProvider({ + // Declare only NON-secret fields here — their values are stored in the + // synced issue-provider config: + configFields: [ + { key: 'host', type: 'text', label: 'Host' }, + { key: 'username', type: 'text', label: 'Username' }, + ], + // getHeaders may return a Promise, so read the credential from secret + // storage instead of from `config`: + async getHeaders(config) { + const token = await PluginAPI.getSecret('apiToken'); + return token ? { Authorization: `Bearer ${token}` } : {}; + }, + async getById(issueId, config, http) { + /* ... http call uses the headers above ... */ + }, + // ... +}); +``` + +The host passes only the synced `config` into these callbacks — there is no +secret parameter, and the declarative `configFields` form always writes to the +synced config. So collect the secret through your own UI (a config dialog +registered via `registerConfigHandler`, or a side panel) and store it with +`setSecret` there; do **not** add the credential as a `configFields` entry. + ## Best Practices ### 1. Performance diff --git a/packages/plugin-api/src/types.ts b/packages/plugin-api/src/types.ts index d1444c332a..2bfa9bdadc 100644 --- a/packages/plugin-api/src/types.ts +++ b/packages/plugin-api/src/types.ts @@ -675,6 +675,22 @@ export interface PluginAPI { clearOAuthToken(): Promise; + // secret storage + // + // Local-only, per-plugin credential storage (IMAP passwords, API tokens, …). + // Stored in a dedicated store that is never part of Super Productivity's + // sync, exports, or backups, so secrets are per-device — the user re-enters + // them on each device. (This is not protection against OS-level device + // backups; values are stored unencrypted at rest, like plugin OAuth tokens.) + // Use this instead of `persistDataSynced` / issue-provider config for + // anything sensitive; those land in synced state, exports, and backups. + // `key` must be a non-empty string. + setSecret(key: string, value: string): Promise; + + getSecret(key: string): Promise; + + deleteSecret(key: string): Promise; + // download file downloadFile(filename: string, data: string): Promise; diff --git a/src/app/plugins/plugin-api.ts b/src/app/plugins/plugin-api.ts index d765b21fef..d02b48407a 100644 --- a/src/app/plugins/plugin-api.ts +++ b/src/app/plugins/plugin-api.ts @@ -610,6 +610,19 @@ export class PluginAPI implements PluginAPIInterface { return this.#boundMethods.clearOAuthToken(); } + async setSecret(key: string, value: string): Promise { + return this.#boundMethods.setSecret(key, value); + } + + async getSecret(key: string): Promise { + return this.#boundMethods.getSecret(key); + } + + async deleteSecret(key: string): Promise { + PluginLog.log(`Plugin ${this.#pluginId} requested secret delete`); + return this.#boundMethods.deleteSecret(key); + } + /** * Clean up all resources associated with this plugin API instance * Called when the plugin is being unloaded diff --git a/src/app/plugins/plugin-bridge.service.ts b/src/app/plugins/plugin-bridge.service.ts index 2df06a2eb7..af7aeaa01d 100644 --- a/src/app/plugins/plugin-bridge.service.ts +++ b/src/app/plugins/plugin-bridge.service.ts @@ -84,6 +84,7 @@ import { IssueSyncAdapterRegistryService } from '../features/issue/two-way-sync/ import { PluginHttpService } from './issue-provider/plugin-http.service'; import { createPluginSyncAdapter } from './issue-provider/plugin-sync-adapter.service'; import { PluginOAuthBridgeService } from './oauth/plugin-oauth-bridge.service'; +import { PluginSecretService } from './secret/plugin-secret.service'; import { ISSUE_PROVIDER_TYPES } from '../features/issue/issue.const'; import { PluginService } from './plugin.service'; import { PluginI18nService } from './plugin-i18n.service'; @@ -155,6 +156,7 @@ export class PluginBridgeService implements OnDestroy { private _syncAdapterRegistry = inject(IssueSyncAdapterRegistryService); private _pluginHttpService = inject(PluginHttpService); private _pluginOAuthBridge = inject(PluginOAuthBridgeService); + private _pluginSecretService = inject(PluginSecretService); private _dataInitService = inject(DataInitService); private _globalConfigService = inject(GlobalConfigService); readonly #nodeExecutionGrantTokens = new Map(); @@ -270,6 +272,9 @@ export class PluginBridgeService implements OnDestroy { startOAuthFlow: (config: OAuthFlowConfig) => Promise; getOAuthToken: () => Promise; clearOAuthToken: () => Promise; + setSecret: (key: string, value: string) => Promise; + getSecret: (key: string) => Promise; + deleteSecret: (key: string) => Promise; translate: (key: string, params?: Record) => string; formatDate: (date: Date | string | number, format: PluginDateFormat) => string; getCurrentLanguage: () => string; @@ -364,6 +369,14 @@ export class PluginBridgeService implements OnDestroy { clearOAuthToken: (): Promise => this._pluginOAuthBridge.clearOAuthTokens(pluginId), + // Secret storage (local-only, per-plugin, never synced) + setSecret: (key: string, value: string): Promise => + this._pluginSecretService.setSecret(pluginId, key, value), + getSecret: (key: string): Promise => + this._pluginSecretService.getSecret(pluginId, key), + deleteSecret: (key: string): Promise => + this._pluginSecretService.deleteSecret(pluginId, key), + // i18n translate: (key: string, params?: Record): string => this._injector.get(PluginI18nService).translate(pluginId, key, params), diff --git a/src/app/plugins/plugin.service.spec.ts b/src/app/plugins/plugin.service.spec.ts index 38fd01c2b4..992c41a5b6 100644 --- a/src/app/plugins/plugin.service.spec.ts +++ b/src/app/plugins/plugin.service.spec.ts @@ -10,6 +10,7 @@ import { GlobalThemeService } from '../core/theme/global-theme.service'; import { IssueSyncAdapterRegistryService } from '../features/issue/two-way-sync/issue-sync-adapter-registry.service'; import { PluginIssueProviderRegistryService } from './issue-provider/plugin-issue-provider-registry.service'; import { PluginCacheService } from './plugin-cache.service'; +import { PluginSecretService } from './secret/plugin-secret.service'; import { PluginCleanupService } from './plugin-cleanup.service'; import { PluginHooksService } from './plugin-hooks'; import { PluginI18nService } from './plugin-i18n.service'; @@ -72,10 +73,12 @@ describe('PluginService', () => { 'setNodeExecutionGrantToken', 'revokeNodeExecutionGrantToken', 'revokeNodeExecutionGrant', + 'clearOAuthTokens', 'clearNodeExecutionConsent', ]); pluginBridge.hasNodeExecutionGrantToken.and.returnValue(false); pluginBridge.requestNodeExecutionGrant.and.resolveTo(null); + pluginBridge.clearOAuthTokens.and.resolveTo(undefined); pluginBridge.clearNodeExecutionConsent.and.resolveTo(undefined); pluginRunner = jasmine.createSpyObj('PluginRunner', [ 'loadPlugin', @@ -384,6 +387,44 @@ describe('PluginService', () => { expect(pluginBridge.clearNodeExecutionConsent).not.toHaveBeenCalledWith('builtin-1'); }); + it('clearUploadedPluginsFromMemory purges local-only credentials for uploaded plugins', async () => { + // Same id-reuse gap as the consent clear above: the cache wipe leaves secrets/OAuth + // tokens in their dedicated stores, so a same-id re-upload could read the previous + // plugin's credentials unless they are purged here too. + const secretService = TestBed.inject(PluginSecretService); + const removeSecretsSpy = spyOn( + secretService, + 'removeSecretsForPlugin', + ).and.resolveTo(); + const setState = ( + service as unknown as { + _setPluginState: (pluginId: string, state: PluginState) => void; + } + )._setPluginState.bind(service); + setState('uploaded-1', { + manifest: { ...mockManifest, id: 'uploaded-1' }, + status: 'not-loaded', + path: 'uploaded://uploaded-1', + type: 'uploaded', + isEnabled: true, + }); + setState('builtin-1', { + manifest: { ...mockManifest, id: 'builtin-1' }, + status: 'not-loaded', + path: 'assets/bundled-plugins/builtin-1', + type: 'built-in', + isEnabled: true, + }); + + await service.clearUploadedPluginsFromMemory(); + + expect(removeSecretsSpy).toHaveBeenCalledWith('uploaded-1'); + expect(pluginBridge.clearOAuthTokens).toHaveBeenCalledWith('uploaded-1'); + // Built-in plugins are not wiped by a cache clear, so their credentials are left alone. + expect(removeSecretsSpy).not.toHaveBeenCalledWith('builtin-1'); + expect(pluginBridge.clearOAuthTokens).not.toHaveBeenCalledWith('builtin-1'); + }); + it('disablePlugin persists isEnabled=false and revokes nodeExecution consent (Phase 2)', async () => { const pluginId = 'uploaded-node-plugin'; ( @@ -559,6 +600,39 @@ describe('PluginService', () => { expect(service.getLoadedPlugins()).toEqual([]); }); + describe('removeUploadedPlugin credential cleanup', () => { + it('purges both secrets and OAuth tokens on uninstall', async () => { + const secretService = TestBed.inject(PluginSecretService); + const removeSecretsSpy = spyOn( + secretService, + 'removeSecretsForPlugin', + ).and.resolveTo(); + + await service.removeUploadedPlugin('ghost-plugin'); + + expect(removeSecretsSpy).toHaveBeenCalledWith('ghost-plugin'); + expect(pluginBridge.clearOAuthTokens).toHaveBeenCalledWith('ghost-plugin'); + }); + + it('purges credentials even when a later cleanup step fails', async () => { + const secretService = TestBed.inject(PluginSecretService); + const removeSecretsSpy = spyOn( + secretService, + 'removeSecretsForPlugin', + ).and.resolveTo(); + const cache = TestBed.inject( + PluginCacheService, + ) as jasmine.SpyObj; + cache.removePlugin.and.rejectWith(new Error('cache failure')); + + // The credential purges run before the failing step, so they still fire. + await expectAsync(service.removeUploadedPlugin('ghost-plugin')).toBeRejected(); + + expect(removeSecretsSpy).toHaveBeenCalledWith('ghost-plugin'); + expect(pluginBridge.clearOAuthTokens).toHaveBeenCalledWith('ghost-plugin'); + }); + }); + // chokepoint #1: the actual #8385 path (startup re-activation / reload both flow through // `activatePlugin`'s catch). Driven by making `_loadPluginLazy` reject with the denial // sentinel, since the real `_fireOnReady` grant block is gated on the un-mockable diff --git a/src/app/plugins/plugin.service.ts b/src/app/plugins/plugin.service.ts index 6825e8f46d..fcf4e18144 100644 --- a/src/app/plugins/plugin.service.ts +++ b/src/app/plugins/plugin.service.ts @@ -22,6 +22,7 @@ import { import { take } from 'rxjs/operators'; import { firstValueFrom } from 'rxjs'; import { PluginCleanupService } from './plugin-cleanup.service'; +import { PluginSecretService } from './secret/plugin-secret.service'; import { PluginLoaderService } from './plugin-loader.service'; import { validatePluginManifest } from './util/validate-manifest.util'; import { TranslateService } from '@ngx-translate/core'; @@ -113,6 +114,7 @@ export class PluginService implements OnDestroy { private readonly _pluginMetaPersistenceService = inject(PluginMetaPersistenceService); private readonly _pluginUserPersistenceService = inject(PluginUserPersistenceService); private readonly _pluginCacheService = inject(PluginCacheService); + private readonly _pluginSecretService = inject(PluginSecretService); private readonly _cleanupService = inject(PluginCleanupService); private readonly _pluginLoader = inject(PluginLoaderService); private readonly _pluginBridge = inject(PluginBridgeService); @@ -1635,6 +1637,21 @@ export class PluginService implements OnDestroy { this.unloadPlugin(pluginId); } + // Purge local-only credentials (secrets + OAuth tokens) FIRST so they + // never outlive their plugin — even if a later cleanup step throws. + // Best-effort: a purge failure is logged but must not abort the uninstall + // (on IndexedDB failure the credentials orphan locally until a later purge). + try { + await this._pluginSecretService.removeSecretsForPlugin(pluginId); + } catch (error) { + PluginLog.err(`Failed to purge secrets for plugin ${pluginId}:`, error); + } + try { + await this._pluginBridge.clearOAuthTokens(pluginId); + } catch (error) { + PluginLog.err(`Failed to purge OAuth tokens for plugin ${pluginId}:`, error); + } + // Remove from cache await this._pluginCacheService.removePlugin(pluginId); @@ -1673,13 +1690,15 @@ export class PluginService implements OnDestroy { * Clear all uploaded plugins from memory. Called when the IndexedDB cache is cleared * so that in-memory state matches the empty cache. * - * Also clears each uploaded plugin's main-owned PERSISTED nodeExecution consent - * (issue #8512 Phase 2). The cache wipe removes the plugin code, but the consent lives - * in the main process, so without this a later re-upload of the same id — potentially - * *different* code — would be silently granted node execution with no prompt: the - * post-clear upload has no `existingState`, so the re-upload clear in `loadPluginFromZip` - * never fires. Mirrors `removeUploadedPlugin`, keeping "replacing code under an id always - * re-asks" true on every removal path. + * Also purges each uploaded plugin's local-only credentials (secrets + OAuth + * tokens) and main-owned PERSISTED nodeExecution consent (issue #8512 Phase 2). + * The cache wipe removes the plugin code, but credentials and consent live in + * dedicated stores / the main process, so without this a later re-upload of the + * same id — potentially *different* code — would silently inherit the previous + * plugin's secrets, tokens, and node-execution grant with no prompt: the + * post-clear upload has no `existingState`, so the re-upload clear in + * `loadPluginFromZip` never fires. Mirrors `removeUploadedPlugin`, keeping + * "replacing code under an id always re-asks" true on every removal path. */ async clearUploadedPluginsFromMemory(): Promise { const states = this._pluginStates(); @@ -1704,11 +1723,24 @@ export class PluginService implements OnDestroy { return updated; }); this._pluginIconsSignal.set(new Map(this._pluginIcons)); - // Drop persisted consent after teardown has released the live grants. Each clear is - // fail-safe (worst case: a re-prompt) and idempotent, so a single failure can't leave - // a different id's consent behind. + // Purge local-only credentials + persisted consent after teardown has released the + // live grants. Each purge is best-effort and idempotent, so a single failure can't + // skip the rest or leave a different id's credentials/consent behind for a same-id + // re-upload to inherit. await Promise.all( - uploadedIds.map((pluginId) => this.clearNodeExecutionConsent(pluginId)), + uploadedIds.map(async (pluginId) => { + try { + await this._pluginSecretService.removeSecretsForPlugin(pluginId); + } catch (error) { + PluginLog.err(`Failed to purge secrets for plugin ${pluginId}:`, error); + } + try { + await this._pluginBridge.clearOAuthTokens(pluginId); + } catch (error) { + PluginLog.err(`Failed to purge OAuth tokens for plugin ${pluginId}:`, error); + } + await this.clearNodeExecutionConsent(pluginId); + }), ); } diff --git a/src/app/plugins/secret/plugin-secret-store.ts b/src/app/plugins/secret/plugin-secret-store.ts new file mode 100644 index 0000000000..267bed699f --- /dev/null +++ b/src/app/plugins/secret/plugin-secret-store.ts @@ -0,0 +1,93 @@ +import { DBSchema, IDBPDatabase, openDB } from 'idb'; +import { PluginLog } from '../../core/log'; + +/** + * Local-only IndexedDB store for plugin secrets (passwords, API tokens, …). + * + * Mirrors the plugin OAuth token store: secrets live in a dedicated database + * ('sup-plugin-secrets') that is NOT part of the op-log sync system, exports, + * or backups. Each device stores its own secrets independently. + * + * Values are stored verbatim (plaintext at rest), exactly like OAuth tokens + * today. OS-keychain encryption is a future, optional upgrade behind the same + * PluginAPI surface and does not change this store's contract. + */ + +const DB_NAME = 'sup-plugin-secrets'; +const DB_STORE_NAME = 'secrets'; +const DB_VERSION = 1; + +interface PluginSecretDb extends DBSchema { + [DB_STORE_NAME]: { + key: string; + value: string; + }; +} + +let db: IDBPDatabase | undefined; +let initPromise: Promise> | undefined; + +const ensureDb = async (): Promise> => { + if (db) { + return db; + } + if (!initPromise) { + initPromise = openDB(DB_NAME, DB_VERSION, { + upgrade: (database) => { + if (!database.objectStoreNames.contains(DB_STORE_NAME)) { + database.createObjectStore(DB_STORE_NAME); + } + }, + }).then((opened) => { + db = opened; + return opened; + }); + // Don't cache a rejected open forever — clear it so a later call retries + // instead of leaving the store bricked for the whole session. + initPromise.catch(() => { + initPromise = undefined; + }); + } + return initPromise; +}; + +export const saveSecret = async (entityId: string, value: string): Promise => { + try { + const store = await ensureDb(); + await store.put(DB_STORE_NAME, value, entityId); + } catch (error) { + PluginLog.err('PluginSecretStore: Failed to save secret:', error); + throw error; + } +}; + +export const loadSecret = async (entityId: string): Promise => { + try { + const store = await ensureDb(); + const result = await store.get(DB_STORE_NAME, entityId); + return result ?? null; + } catch (error) { + PluginLog.err('PluginSecretStore: Failed to load secret:', error); + throw error; + } +}; + +export const deleteSecret = async (entityId: string): Promise => { + try { + const store = await ensureDb(); + await store.delete(DB_STORE_NAME, entityId); + } catch (error) { + PluginLog.err('PluginSecretStore: Failed to delete secret:', error); + throw error; + } +}; + +export const getAllSecretKeys = async (): Promise => { + try { + const store = await ensureDb(); + return await store.getAllKeys(DB_STORE_NAME); + } catch (error) { + PluginLog.err('PluginSecretStore: Failed to list secret keys:', error); + throw error; + } +}; diff --git a/src/app/plugins/secret/plugin-secret.service.spec.ts b/src/app/plugins/secret/plugin-secret.service.spec.ts new file mode 100644 index 0000000000..370f357ec4 --- /dev/null +++ b/src/app/plugins/secret/plugin-secret.service.spec.ts @@ -0,0 +1,91 @@ +import { TestBed } from '@angular/core/testing'; +import { MAX_PLUGIN_SECRET_LENGTH, PluginSecretService } from './plugin-secret.service'; +import { deleteSecret, getAllSecretKeys } from './plugin-secret-store'; + +/** + * Runs against the real `sup-plugin-secrets` IndexedDB (Karma uses real + * Chrome). afterEach wipes the store so specs don't leak into each other. + */ +describe('PluginSecretService', () => { + let service: PluginSecretService; + + beforeEach(() => { + TestBed.configureTestingModule({}); + service = TestBed.inject(PluginSecretService); + }); + + afterEach(async () => { + const keys = await getAllSecretKeys(); + for (const key of keys) { + await deleteSecret(key); + } + }); + + it('round-trips a secret', async () => { + await service.setSecret('plugin-a', 'password', 's3cret'); + expect(await service.getSecret('plugin-a', 'password')).toBe('s3cret'); + }); + + it('returns null for a missing secret', async () => { + expect(await service.getSecret('plugin-a', 'nope')).toBeNull(); + }); + + it('namespaces secrets per plugin', async () => { + await service.setSecret('plugin-a', 'password', 'a-secret'); + await service.setSecret('plugin-b', 'password', 'b-secret'); + expect(await service.getSecret('plugin-a', 'password')).toBe('a-secret'); + expect(await service.getSecret('plugin-b', 'password')).toBe('b-secret'); + }); + + it('deletes a single secret without touching others', async () => { + await service.setSecret('plugin-a', 'password', 'pw'); + await service.setSecret('plugin-a', 'token', 'tk'); + await service.deleteSecret('plugin-a', 'password'); + expect(await service.getSecret('plugin-a', 'password')).toBeNull(); + expect(await service.getSecret('plugin-a', 'token')).toBe('tk'); + }); + + it('removeSecretsForPlugin purges only the owner plugin', async () => { + await service.setSecret('plugin-a', 'password', 'pw'); + await service.setSecret('plugin-a', 'token', 'tk'); + await service.setSecret('plugin-b', 'password', 'keep'); + await service.removeSecretsForPlugin('plugin-a'); + expect(await service.getSecret('plugin-a', 'password')).toBeNull(); + expect(await service.getSecret('plugin-a', 'token')).toBeNull(); + expect(await service.getSecret('plugin-b', 'password')).toBe('keep'); + }); + + it('removeSecretsForPlugin does not purge a prefix-colliding plugin', async () => { + // 'plugin-a' must not match 'plugin-ab' during cleanup. + await service.setSecret('plugin-a', 'k', 'a'); + await service.setSecret('plugin-ab', 'k', 'ab'); + await service.removeSecretsForPlugin('plugin-a'); + expect(await service.getSecret('plugin-a', 'k')).toBeNull(); + expect(await service.getSecret('plugin-ab', 'k')).toBe('ab'); + }); + + it('rejects an empty key', async () => { + await expectAsync(service.setSecret('plugin-a', '', 'x')).toBeRejectedWithError( + /non-empty string/, + ); + }); + + it('rejects a non-string value', async () => { + await expectAsync( + service.setSecret('plugin-a', 'k', 123 as unknown as string), + ).toBeRejectedWithError(/must be a string/); + }); + + it('rejects an oversized value', async () => { + const huge = 'x'.repeat(MAX_PLUGIN_SECRET_LENGTH + 1); + await expectAsync(service.setSecret('plugin-a', 'k', huge)).toBeRejectedWithError( + /maximum length/, + ); + }); + + it('rejects a pluginId containing the reserved delimiter', async () => { + await expectAsync(service.setSecret('plugin:evil', 'k', 'x')).toBeRejectedWithError( + /must not contain/, + ); + }); +}); diff --git a/src/app/plugins/secret/plugin-secret.service.ts b/src/app/plugins/secret/plugin-secret.service.ts new file mode 100644 index 0000000000..f7ad4ad7d9 --- /dev/null +++ b/src/app/plugins/secret/plugin-secret.service.ts @@ -0,0 +1,85 @@ +import { Injectable } from '@angular/core'; +import { PluginLog } from '../../core/log'; +import { + assertPluginPersistenceKey, + composeId, + isPluginIdMatch, +} from '../util/plugin-persistence-key.util'; +import { + deleteSecret, + getAllSecretKeys, + loadSecret, + saveSecret, +} from './plugin-secret-store'; + +/** + * Per-plugin local-only secret storage. + * + * Secrets are namespaced by `pluginId` (the host injects it — a plugin can + * never address another plugin's keys) and persisted to a dedicated IndexedDB + * that is never synced, exported, or backed up. Use for credentials that must + * not leave the device (IMAP passwords, API tokens), NOT `persistDataSynced`. + */ + +/** + * Upper bound on a single secret value. Credentials are small; this only + * exists to stop a compromised iframe from writing megabytes into the store. + * Measured in UTF-16 code units (string length) — exact bytes don't matter + * for an abuse guard. + */ +export const MAX_PLUGIN_SECRET_LENGTH = 16 * 1024; + +@Injectable({ providedIn: 'root' }) +export class PluginSecretService { + async setSecret(pluginId: string, key: string, value: string): Promise { + const entityId = this._entityId(pluginId, key); + if (typeof value !== 'string') { + throw new Error('Plugin secret value must be a string'); + } + if (value.length > MAX_PLUGIN_SECRET_LENGTH) { + throw new Error( + `Plugin secret exceeds maximum length of ${MAX_PLUGIN_SECRET_LENGTH} characters`, + ); + } + await saveSecret(entityId, value); + } + + async getSecret(pluginId: string, key: string): Promise { + return loadSecret(this._entityId(pluginId, key)); + } + + async deleteSecret(pluginId: string, key: string): Promise { + await deleteSecret(this._entityId(pluginId, key)); + } + + /** + * Purge every secret owned by a plugin. Called on uninstall so credentials + * never outlive the plugin that owned them. + */ + async removeSecretsForPlugin(pluginId: string): Promise { + const allKeys = await getAllSecretKeys(); + const owned = allKeys.filter((entityId) => isPluginIdMatch(entityId, pluginId)); + for (const entityId of owned) { + await deleteSecret(entityId); + } + if (owned.length > 0) { + PluginLog.log('PluginSecretService: Removed secrets on cleanup', { + pluginId, + count: owned.length, + }); + } + } + + /** + * Compose + validate the storage id. `key` is required and non-empty for + * secrets (unlike the optional persistence key), and `composeId` throws if + * the pluginId itself contains the reserved ':' delimiter. + */ + private _entityId(pluginId: string, key: string): string { + if (typeof key !== 'string' || key === '') { + throw new Error('Plugin secret key must be a non-empty string'); + } + assertPluginPersistenceKey(key); + return composeId(pluginId, key); + } +}