fix: only connect and proxy plugin credentials that a plugin known to the server declares

app/api/plugin-approval-status/route.ts

@@ -107,6 +107,7 @@ export async function POST(request: NextRequest) {
       permissions: manifest.permissions,
       httpOrigins: manifest.httpOrigins,
       apiPostPaths: manifest.apiPostPaths,
+      ...(manifest.credentials ? { credentials: manifest.credentials } : {}),
     };
 
     let entry: ApprovalEntry;

app/api/plugin-secrets/__tests__/route.test.ts

@@ -26,6 +26,14 @@ vi.mock('@/lib/plugin-secrets/vault', () => ({
   listUserCredentials: vi.fn(async () => []),
   deleteUserCredentials: vi.fn(async () => {}),
 }));
+// What the server knows the plugins declare (lib/plugin-secrets/known-plugins).
+const declared: Record<string, { scope: 'user'; label: string; inject: { type: string }; origin?: string }> = {
+  'nextcloud/cloud': { scope: 'user', label: 'Nextcloud', inject: { type: 'basic' } },
+  'paperless/docs': { scope: 'user', label: 'Paperless', inject: { type: 'bearer' }, origin: 'https://paperless.example.com' },
+};
+vi.mock('@/lib/plugin-secrets/known-plugins', () => ({
+  declaredUserCredential: async (pluginId: string, credentialId: string) => declared[`${pluginId}/${credentialId}`] ?? null,
+}));
 const proxied: unknown[] = [];
 vi.mock('@/lib/plugin-secrets/proxy', async (orig) => {
   const real = await orig<typeof import('@/lib/plugin-secrets/proxy')>();
@@ -90,6 +98,20 @@ describe('POST /api/plugin-secrets', () => {
     expect(store.size).toBe(0);
   });
 
+  it('refuses a credential no known plugin declares, so the proxy is no relay for made-up plugin ids', async () => {
+    expect((await save({ ...VALID, pluginId: 'made-up' })).status).toBe(404);
+    expect((await save({ ...VALID, credentialId: 'other' })).status).toBe(404);
+    expect(store.size).toBe(0);
+  });
+
+  it('keeps a credential on the server its plugin fixes and stores the inject the manifest declares', async () => {
+    const paperless = { ...VALID, pluginId: 'paperless', credentialId: 'docs', inject: { type: 'basic' } };
+    expect((await save({ ...paperless, origin: 'https://elsewhere.example.com' })).status).toBe(400);
+    expect(store.size).toBe(0);
+    expect((await save({ ...paperless, origin: 'https://paperless.example.com/api/' })).status).toBe(200);
+    expect(store.get('paperless/docs')).toMatchObject({ origin: 'https://paperless.example.com', inject: { type: 'bearer' } });
+  });
+
   it('refuses without a session, with plugins off, or from another site', async () => {
     signedIn = false;
     expect((await save(VALID)).status).toBe(401);

app/api/plugin-secrets/route.ts

@@ -4,6 +4,7 @@ import { rejectCrossOriginRequest } from '@/lib/security/same-origin';
 import { getStalwartCredentials } from '@/lib/stalwart/credentials';
 import { CREDENTIAL_ID_RE, checkInject, credentialOrigin } from '@/lib/plugin-secrets/credentials';
 import { adminCredentialOrigins, adminCredentialStatus } from '@/lib/plugin-secrets/proxy';
+import { declaredUserCredential } from '@/lib/plugin-secrets/known-plugins';
 import {
   VaultFullError,
   deleteUserCredentials,
@@ -76,9 +77,17 @@ export async function POST(request: NextRequest) {
   if (username !== undefined && (typeof username !== 'string' || username.length > 256 || /[\r\n]/.test(username))) return bad('Invalid username');
   if (secret !== undefined && (typeof secret !== 'string' || !secret || secret.length > MAX_SECRET_LENGTH || /[\r\n]/.test(secret))) return bad('Invalid secret');
 
+  // A credential belongs to a plugin this server knows declares it: without
+  // that, any signed-in user could park a secret under a made-up plugin id
+  // and use the proxy as a relay to any server. The manifest's own inject
+  // and, when fixed, its origin are what count, not what the browser sent.
+  const spec = await declaredUserCredential(pluginId, credentialId);
+  if (!spec) return NextResponse.json({ error: 'No such plugin credential' }, { status: 404 });
+  if (spec.origin && credentialOrigin(spec.origin) !== origin) return bad('This credential is only for the server its plugin names');
+
   try {
     await putUserCredential(user, pluginId, credentialId, {
-      origin, inject, label: label.trim(),
+      origin, inject: spec.inject, label: label.trim(),
       ...(typeof username === 'string' && username ? { username } : {}),
       ...(typeof secret === 'string' ? { secret } : {}),
     });

lib/__tests__/plugin-known-credentials.test.ts

@@ -0,0 +1,80 @@
+/**
+ * @vitest-environment node
+ */
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+
+// The proxy injected whatever credential the vault held for the (plugin,
+// credential) pair in the query string, and the connect route stored one for
+// any plugin id of the right shape. Neither asked whether a plugin the server
+// knows declares that credential, so a signed-in user could relay requests
+// through the server to any origin, and a plugin could reach for another's
+// secret by id. Now a user credential exists only for a plugin the server
+// can name: an installed one, or an admin-approved upload.
+
+const registry = new Map<string, { enabled: boolean; credentials?: Record<string, unknown> }>();
+const approvals: { pluginId: string; status: string; manifest: { credentials?: Record<string, unknown> } }[] = [];
+vi.mock('@/lib/admin/plugin-registry', () => ({ getPlugin: async (id: string) => registry.get(id) ?? null }));
+vi.mock('@/lib/admin/plugin-approvals', () => ({ listApprovals: async () => approvals }));
+vi.mock('@/lib/admin/plugin-config', () => ({ getPluginConfig: async () => ({}) }));
+vi.mock('@/lib/logger', () => ({ logger: { info: () => {}, warn: () => {}, error: () => {}, debug: () => {} } }));
+
+const vault = new Map<string, { origin: string; inject: { type: string }; secret?: string }>();
+vi.mock('@/lib/plugin-secrets/vault', () => ({
+  getUserCredential: async (_u: unknown, p: string, c: string) => vault.get(`${p}/${c}`) ?? null,
+  setUserCredentialSecret: async () => {},
+  userKey: () => 'user',
+}));
+
+import { declaredUserCredential } from '@/lib/plugin-secrets/known-plugins';
+import { resolveCredential } from '@/lib/plugin-secrets/proxy';
+
+const USER = { username: 'alice@example.org', serverUrl: 'https://mail.example.org' };
+const CLOUD = { scope: 'user', label: 'Nextcloud', inject: { type: 'basic' } };
+
+beforeEach(() => {
+  registry.clear();
+  approvals.length = 0;
+  vault.clear();
+});
+
+describe('declaredUserCredential', () => {
+  it('finds the credential of an installed, enabled plugin', async () => {
+    registry.set('nextcloud', { enabled: true, credentials: { cloud: CLOUD } });
+    expect(await declaredUserCredential('nextcloud', 'cloud')).toEqual(CLOUD);
+    expect(await declaredUserCredential('nextcloud', 'other')).toBeNull();
+    registry.set('nextcloud', { enabled: false, credentials: { cloud: CLOUD } });
+    expect(await declaredUserCredential('nextcloud', 'cloud')).toBeNull();
+  });
+
+  it('finds the credential of an approved upload, not of a pending or denied one', async () => {
+    approvals.push({ pluginId: 'upload', status: 'pending', manifest: { credentials: { cloud: CLOUD } } });
+    expect(await declaredUserCredential('upload', 'cloud')).toBeNull();
+    approvals.push({ pluginId: 'upload', status: 'approved', manifest: { credentials: { cloud: CLOUD } } });
+    expect(await declaredUserCredential('upload', 'cloud')).toEqual(CLOUD);
+  });
+
+  it('knows nothing about a made-up plugin or an admin-scope credential', async () => {
+    expect(await declaredUserCredential('x-y', 'c')).toBeNull();
+    registry.set('ai', { enabled: true, credentials: { key: { scope: 'admin', label: 'Key', inject: { type: 'bearer' }, secretKey: 'apiKey' } } });
+    expect(await declaredUserCredential('ai', 'key')).toBeNull();
+  });
+});
+
+describe('resolveCredential for a stored user credential', () => {
+  it('refuses one stored under a plugin the server does not know', async () => {
+    vault.set('x-y/c', { origin: 'https://any.example', inject: { type: 'none' } });
+    expect(await resolveCredential(USER, 'x-y', 'c')).toBeNull();
+  });
+
+  it('serves one a known plugin declares, on the origin the manifest fixes', async () => {
+    registry.set('nextcloud', { enabled: true, credentials: { cloud: CLOUD } });
+    vault.set('nextcloud/cloud', { origin: 'https://cloud.example.org', inject: { type: 'basic' }, secret: 's' });
+    expect(await resolveCredential(USER, 'nextcloud', 'cloud')).toMatchObject({ origin: 'https://cloud.example.org', scope: 'user' });
+
+    registry.set('paperless', { enabled: true, credentials: { docs: { ...CLOUD, origin: 'https://paperless.example.com' } } });
+    vault.set('paperless/docs', { origin: 'https://elsewhere.example.com', inject: { type: 'basic' }, secret: 's' });
+    expect(await resolveCredential(USER, 'paperless', 'docs')).toBeNull();
+    vault.set('paperless/docs', { origin: 'https://paperless.example.com', inject: { type: 'basic' }, secret: 's' });
+    expect(await resolveCredential(USER, 'paperless', 'docs')).toMatchObject({ origin: 'https://paperless.example.com' });
+  });
+});

lib/__tests__/plugin-secret-proxy.test.ts

@@ -8,6 +8,15 @@ import path from 'node:path';
 
 vi.mock('@/lib/admin/plugin-registry', () => ({ getPlugin: vi.fn(async () => null) }));
 vi.mock('@/lib/admin/plugin-config', () => ({ getPluginConfig: vi.fn(async () => ({})) }));
+// A user credential exists only for a plugin the server knows declares it
+// (lib/plugin-secrets/known-plugins.ts): the Nextcloud plugin of these tests
+// is an admin-approved upload.
+vi.mock('@/lib/admin/plugin-approvals', () => ({
+  listApprovals: async () => [{
+    pluginId: 'nextcloud', status: 'approved',
+    manifest: { credentials: { cloud: { scope: 'user', label: 'Nextcloud', inject: { type: 'basic' } } } },
+  }],
+}));
 const auditLines: Record<string, unknown>[] = [];
 vi.mock('@/lib/logger', () => ({
   logger: {

lib/admin/plugin-approvals.ts

@@ -15,6 +15,7 @@ import nodePath from 'node:path';
 import { existsSync } from 'node:fs';
 import { logger } from '@/lib/logger';
 import { ensureConfigDir, getConfigPath, assertWritable } from './paths';
+import type { CredentialSpec } from '@/lib/plugin-secrets/credentials';
 
 export type ApprovalStatus = 'pending' | 'approved' | 'denied';
 
@@ -31,6 +32,8 @@ export interface ApprovalEntry {
     permissions?: string[];
     httpOrigins?: string[];
     apiPostPaths?: string[];
+    /** The credentials it may use through the plugin proxy (lib/plugin-secrets). */
+    credentials?: Record<string, CredentialSpec>;
   };
   requestedBy: string;     // JMAP username who triggered the request
   requestedAt: string;     // ISO 8601

lib/plugin-secrets/known-plugins.ts

@@ -0,0 +1,28 @@
+import { getPlugin } from '@/lib/admin/plugin-registry';
+import { listApprovals } from '@/lib/admin/plugin-approvals';
+import type { CredentialSpec, UserCredentialSpec } from './credentials';
+
+/**
+ * The user-scope credential `credentialId` as the manifest of `pluginId`
+ * declares it, when this server knows that manifest: an installed (registry)
+ * plugin, or a user-uploaded package an admin approved.
+ *
+ * The connect dialog and the proxy used to take the plugin id at its word,
+ * so any signed-in user could store a credential under a made-up plugin id
+ * and relay requests through the server to any origin with it, and a
+ * plugin could reach for a credential declared by none. A credential is
+ * only ever for a plugin the server can name, and only on the server the
+ * manifest fixes, when it fixes one.
+ */
+export async function declaredUserCredential(pluginId: string, credentialId: string): Promise<UserCredentialSpec | null> {
+  const plugin = await getPlugin(pluginId);
+  const fromRegistry = (plugin as { credentials?: Record<string, CredentialSpec> } | null)?.credentials?.[credentialId];
+  if (plugin && plugin.enabled && fromRegistry?.scope === 'user') return fromRegistry;
+  if (plugin) return null;
+  for (const entry of await listApprovals()) {
+    if (entry.pluginId !== pluginId || entry.status !== 'approved') continue;
+    const spec = entry.manifest.credentials?.[credentialId];
+    if (spec?.scope === 'user') return spec;
+  }
+  return null;
+}

lib/plugin-secrets/proxy.ts

@@ -11,6 +11,7 @@ import { logger } from '@/lib/logger';
 import { DisallowedUrlError, fetchPublicUrl } from '@/lib/security/url-guard';
 import { authHeaderFor, credentialOrigin, privateProxyOrigins, type CredentialInject, type CredentialSpec } from './credentials';
 import { getUserCredential, setUserCredentialSecret, userKey, type VaultUser } from './vault';
+import { declaredUserCredential } from './known-plugins';
 
 /** Largest request body the proxy forwards (one upload chunk); see proxyClientMaxBodySize in next.config.ts. */
 export const MAX_PROXY_REQUEST_BYTES = 100 * 1024 * 1024;
@@ -69,6 +70,11 @@ export interface ResolvedCredential {
 export async function resolveCredential(user: VaultUser, pluginId: string, credentialId: string): Promise<ResolvedCredential | null> {
   const stored = await getUserCredential(user, pluginId, credentialId);
   if (stored) {
+    // Only for a plugin this server knows declares it, on the server the
+    // manifest fixes (see declaredUserCredential).
+    const spec = await declaredUserCredential(pluginId, credentialId);
+    if (!spec) return null;
+    if (spec.origin && credentialOrigin(spec.origin) !== stored.origin) return null;
     return {
       origin: stored.origin, inject: stored.inject, secret: stored.secret, username: stored.username,
       trusted: privateProxyOrigins().includes(stored.origin), scope: 'user',