validate secret provider config before save

src/lib/server/db.ts

@@ -6,7 +6,7 @@
  */
 
 import type { SecretProviderConfig, SecretProviderType } from './secretproviders/shared';
-import { SECRET_CONFIG_KEYS } from './secretproviders/shared';
+import { mergeProviderConfigForWrite } from './secretproviders/shared';
 import {
 	db,
 	isPostgres,
@@ -386,13 +386,10 @@ export async function updateSecretProvider(
 		if (typeUnchanged) {
 			const existing = await getSecretProviderById(id);
 			if (existing) {
-				for (const key of SECRET_CONFIG_KEYS) {
-					const incoming = (data.config as Record<string, unknown>)[key];
-					if (incoming === undefined || incoming === '') {
-						const stored = (existing.config as Record<string, unknown>)[key];
-						if (stored !== undefined) merged[key] = stored;
-					}
-				}
+				merged = mergeProviderConfigForWrite(
+					data.config as Record<string, unknown>,
+					existing.config as Record<string, unknown>
+				);
 			}
 		}
 		const encrypted = encrypt(JSON.stringify(merged));

src/lib/server/secretproviders/connect.ts

@@ -15,7 +15,7 @@
 
 import { request } from 'undici';
 import type { ConnectConfig, SecretProvider, TestConnectionResult } from './shared';
-import { assertSafeProviderHost } from './shared';
+import { assertSafeProviderHost, parseProviderError, isJsonResponse } from './shared';
 import { UnsupportedOperationError } from './shared';
 
 /** A vault as returned by GET /v1/vaults. */
@@ -251,6 +251,9 @@ export const connectProvider: SecretProvider<ConnectConfig> = {
 			});
 			const text = await body.text();
 			if (statusCode >= 200 && statusCode < 300) {
+				if (!isJsonResponse(text)) {
+					return { ok: false, error: '1Password Connect did not return a JSON response - the host may not be a Connect server' };
+				}
 				return { ok: true };
 			}
 			if (statusCode === 401 || statusCode === 403) {

src/lib/server/secretproviders/doppler.ts

@@ -20,7 +20,7 @@
 
 import { request } from 'undici';
 import type { DopplerConfig, SecretProvider, TestConnectionResult } from './shared';
-import { assertSafeProviderHost, parseProviderError } from './shared';
+import { assertSafeProviderHost, parseProviderError, isJsonResponse } from './shared';
 import { UnsupportedOperationError } from './shared';
 
 const DEFAULT_HOST = 'https://api.doppler.com';
@@ -77,6 +77,9 @@ export const dopplerProvider: SecretProvider<DopplerConfig> = {
 			// message parsed from Doppler's own {messages:[...]} shape.
 			const rawBody = await body.text().catch(() => '');
 			if (statusCode >= 200 && statusCode < 300) {
+				if (!isJsonResponse(rawBody)) {
+					return { ok: false, error: 'Doppler did not return a JSON response - the host may not be a Doppler server' };
+				}
 				return { ok: true };
 			}
 			if (rawBody) console.warn(`[Doppler] testConnection ${statusCode}: ${rawBody}`);

src/lib/server/secretproviders/infisical.ts

@@ -18,7 +18,7 @@
 
 import { request } from 'undici';
 import type { InfisicalConfig, SecretProvider, TestConnectionResult } from './shared';
-import { assertSafeProviderHost, parseProviderError } from './shared';
+import { assertSafeProviderHost, parseProviderError, isJsonResponse } from './shared';
 import { UnsupportedOperationError } from './shared';
 
 /** Shape of a single secret in the /api/v3/secrets/raw response. */
@@ -97,6 +97,9 @@ export const infisicalProvider: SecretProvider<InfisicalConfig> = {
 			// message parsed from Infisical's own {message}/{messages} shape.
 			const rawBody = await body.text().catch(() => '');
 			if (statusCode >= 200 && statusCode < 300) {
+				if (!isJsonResponse(rawBody)) {
+					return { ok: false, error: 'Infisical did not return a JSON response - the host may not be an Infisical server' };
+				}
 				return { ok: true };
 			}
 			if (rawBody) console.warn(`[Infisical] testConnection ${statusCode}: ${rawBody}`);

src/lib/server/secretproviders/shared.ts

@@ -102,6 +102,22 @@ export function parseProviderError(rawBody: string | undefined | null): string |
 	return pick(obj.errors) ?? pick(obj.messages) ?? pick(obj.message) ?? null;
 }
 
+/**
+ * True when `body` parses as a JSON object or array. A 2xx status alone does not prove a
+ * host is the expected backend - a parked domain / captive portal / reverse proxy answers
+ * 200 with an HTML page. Every provider backend answers JSON, so testConnection requires
+ * this before reporting success. Rejects empty bodies and bare scalars (`5`, `true`).
+ */
+export function isJsonResponse(body: string | undefined | null): boolean {
+	if (!body || !body.trim()) return false;
+	try {
+		const v = JSON.parse(body);
+		return typeof v === 'object' && v !== null;
+	} catch {
+		return false;
+	}
+}
+
 /** 1Password service account: a single bearer token. */
 export interface ServiceAccountConfig {
 	token: string;
@@ -170,6 +186,28 @@ export type SecretProviderConfig =
  */
 export const SECRET_CONFIG_KEYS = new Set(['token']);
 
+/**
+ * Merges an incoming (edit-form) config OVER the stored one for a write/test: the incoming
+ * non-secret coordinates win, but any SECRET_CONFIG_KEY (the token) that is blank/absent in
+ * the incoming falls back to the STORED value - because the edit form leaves the token blank
+ * to mean "keep the stored secret". Used by BOTH the update (persist) and the edit-mode Test
+ * so a Test validates exactly what a Save would persist. Keep them on this one helper so they
+ * can never diverge.
+ */
+export function mergeProviderConfigForWrite(
+	incoming: Record<string, unknown>,
+	stored: Record<string, unknown>
+): Record<string, unknown> {
+	const merged: Record<string, unknown> = { ...incoming };
+	for (const key of SECRET_CONFIG_KEYS) {
+		const v = incoming[key];
+		if (v === undefined || v === '') {
+			if (stored[key] !== undefined) merged[key] = stored[key];
+		}
+	}
+	return merged;
+}
+
 /**
  * Returns a copy of a provider config with secret values removed, so the edit form
  * can pre-fill the non-secret coordinates without the token ever leaving the server.

src/lib/server/secretproviders/vault.ts

@@ -18,7 +18,7 @@
  */
 
 import { request } from 'undici';
-import { UnsupportedOperationError, assertSafeProviderHost, sanitizeSelectorPath, parseProviderError } from './shared';
+import { UnsupportedOperationError, assertSafeProviderHost, sanitizeSelectorPath, parseProviderError, isJsonResponse } from './shared';
 import type { SecretProvider, TestConnectionResult, VaultConfig } from './shared';
 
 /** Undici response body handle, derived so we don't import undici's types. */
@@ -93,19 +93,26 @@ export const vaultProvider: SecretProvider<VaultConfig> = {
 			return { ok: false, error: 'Token is empty' };
 		}
 		try {
-			const { statusCode, body } = await request(`${apiBase(config)}/auth/token/lookup-self`, {
+			// Read the KV v2 mount config: validates host + token + the configured mount
+			// in one authenticated call - the same properties deploy (resolveBulk) uses. A
+			// wrong mount 404s and a bad token 403s, so both fail Test instead of passing.
+			const mount = encodeURIComponent(config.mount || 'secret');
+			const { statusCode, body } = await request(`${apiBase(config)}/${mount}/config`, {
 				method: 'GET',
 				headers: authHeaders(config)
 			});
+			const rawBody = await body.text().catch(() => '');
 			if (statusCode >= 200 && statusCode < 300) {
-				await body.text().catch(() => '');
+				// A 2xx alone isn't proof: a parked domain / proxy can answer 200 with HTML.
+				if (!isJsonResponse(rawBody)) {
+					return { ok: false, error: 'Vault did not return a JSON response - the host may not be a Vault server' };
+				}
 				return { ok: true };
 			}
-			const rawBody = await body.text().catch(() => '');
 			// Log the full upstream body server-side; show the client only a message
 			// parsed from Vault's own {errors:[...]} shape (a non-Vault host probed via
 			// SSRF returns other text that won't parse, so nothing leaks).
-			if (rawBody) console.warn(`[HashiCorp Vault] token lookup ${statusCode}: ${rawBody}`);
+			if (rawBody) console.warn(`[HashiCorp Vault] mount config ${mount} ${statusCode}: ${rawBody}`);
 			const safe = parseProviderError(rawBody);
 			return {
 				ok: false,
@@ -143,7 +150,9 @@ export const vaultProvider: SecretProvider<VaultConfig> = {
 		if (statusCode < 200 || statusCode >= 300) {
 			const detail = await readErrorDetail(body);
 			if (detail) console.warn(`[HashiCorp Vault] read ${mount}/${path} ${statusCode}: ${detail}`);
-			throw new Error(`HashiCorp Vault: read failed for ${mount}/${path} (status ${statusCode})`);
+			throw new Error(
+				`HashiCorp Vault: read failed for ${mount}/${path} (status ${statusCode})${detail ? `: ${detail}` : ''}`
+			);
 		}
 
 		const payload = (await body.json()) as KvV2ReadResponse;

src/routes/api/secret-providers/[id]/test/+server.ts

@@ -3,9 +3,17 @@ import type { RequestHandler } from './$types';
 import { getSecretProviderById } from '$lib/server/db';
 import { authorize } from '$lib/server/authorize';
 import { testProviderConnection } from '$lib/server/secretproviders';
+import { mergeProviderConfigForWrite } from '$lib/server/secretproviders/shared';
+import type { SecretProviderConfig } from '$lib/server/secretproviders/shared';
 
-/** Test a stored provider using its persisted (decrypted) config. */
-export const POST: RequestHandler = async ({ params, cookies }) => {
+/**
+ * Test a stored provider. With no body it tests the persisted config. With an optional
+ * `{ config }` override (the edit form's typed non-secret fields) it tests exactly what a
+ * Save would persist: the incoming config merged over the stored one, with a blank token
+ * falling back to the stored secret. The secret is NEVER supplied by the client - it comes
+ * from storage server-side - so this stays a `secrets:view` operation.
+ */
+export const POST: RequestHandler = async ({ params, cookies, request }) => {
 	const auth = await authorize(cookies);
 	if (auth.authEnabled && !(await auth.can('secrets', 'view'))) {
 		return json({ error: 'Permission denied' }, { status: 403 });
@@ -21,7 +29,20 @@ export const POST: RequestHandler = async ({ params, cookies }) => {
 		return json({ error: 'Secret provider not found' }, { status: 404 });
 	}
 
-	const result = await testProviderConnection(provider.type, provider.config);
+	let configToTest: SecretProviderConfig = provider.config;
+	try {
+		const body = await request.json();
+		if (body && typeof body.config === 'object' && body.config !== null) {
+			configToTest = mergeProviderConfigForWrite(
+				body.config as Record<string, unknown>,
+				provider.config as unknown as Record<string, unknown>
+			) as unknown as SecretProviderConfig;
+		}
+	} catch {
+		// No/invalid body: test the stored config as-is.
+	}
+
+	const result = await testProviderConnection(provider.type, configToTest);
 	if (!result.ok) {
 		return json({ ok: false, error: result.error }, { status: 200 });
 	}

src/routes/settings/secrets/ProviderModal.svelte

@@ -212,23 +212,24 @@
 		formError = '';
 		try {
 			const config = collectConfig();
-			// On edit the token is server-side (blank field = keep stored), so if the
-			// user hasn't typed a new one, test the STORED config; the pre-filled
-			// non-secret fields aren't enough to re-test from the client.
-			const hasNewSecret = fields.some((f) => f.type === 'password' && (config[f.key] ?? '') !== '');
+			const missing = missingRequired(config, isEditing);
+			if (missing) {
+				formError = missing;
+				return;
+			}
 
 			let response: Response;
-			if (isEditing && !hasNewSecret) {
-				// Test the stored (server-side) config.
+			if (isEditing) {
+				// Test EXACTLY what a Save would persist: the typed non-secret fields, merged
+				// server-side over the stored config (a blank token keeps the stored one). This
+				// makes an edited address/mount/namespace actually get tested - not the old
+				// stored config.
 				response = await fetch(`/api/secret-providers/${provider!.id}/test`, {
 					method: 'POST',
+					headers: { 'Content-Type': 'application/json' },
+					body: JSON.stringify({ config }),
 				});
 			} else {
-				const missing = missingRequired(config, isEditing);
-				if (missing) {
-					formError = missing;
-					return;
-				}
 				response = await fetch('/api/secret-providers/test', {
 					method: 'POST',
 					headers: { 'Content-Type': 'application/json' },