feat(secrets): check a secret reference before saving it

apps/server/src/environment/ServerEnvironment.ts

@@ -235,6 +235,7 @@ export const make = Effect.gen(function* () {
       storageCleanupRun: true,
       databaseStorage: true,
       otlpEndpointCheck: true,
+      secretSourceCheck: true,
       projectWorktreeCleanup: true,
       settledWorktreeCleanup: true,
       worktreesDirectory: true,

apps/server/src/observability/RpcInstrumentation.ts

@@ -77,6 +77,7 @@ const RPC_AGGREGATES = {
   [WS_METHODS.serverDisableAcpRegistryProvider]: "server",
   [WS_METHODS.serverLogoutAcpRegistry]: "server",
   [WS_METHODS.serverDiscoverSourceControl]: "server",
+  [WS_METHODS.serverCheckSecretSource]: "server",
   [WS_METHODS.serverGetTraceDiagnostics]: "server",
   [WS_METHODS.serverGetProcessDiagnostics]: "server",
   [WS_METHODS.serverGetHostResources]: "server",

apps/server/src/provider/ProviderInstanceRegistry.ts

@@ -92,18 +92,36 @@ const withoutSecretReferences = (
     typeof variable.value === "string" ? [{ ...variable, value: variable.value }] : [],
   );
 
+interface CheckedSecret {
+  readonly reference: SecretReference;
+  readonly value: string;
+}
+
+const sameReference = (a: SecretReference, b: SecretReference) =>
+  a.provider === b.provider && a.reference === b.reference;
+
+// `checked` supplies its reference's value instead of the resolver, which a concurrent refresh
+// may have re-read since the check.
 const readInstanceSecrets = Effect.fn("ProviderInstanceRegistry.readInstanceSecrets")(function* <R>(
   driver: AnyProviderDriver<R> | undefined,
   entry: ProviderInstanceConfig,
+  checked?: CheckedSecret,
 ) {
   const variables = entry.environment ?? [];
   const configFields = configSecretReferences(driver, entry);
   const references = [
     ...secretReferencesIn(variables),
     ...configFields.map(({ reference }) => reference),
   ];
-  const values =
-    references.length === 0 ? [] : yield* (yield* SecretResolver.SecretResolver).read(references);
+  const isChecked = (reference: SecretReference) =>
+    checked !== undefined && sameReference(reference, checked.reference);
+  const toRead = references.filter((reference) => !isChecked(reference));
+  const read =
+    toRead.length === 0 ? [] : yield* (yield* SecretResolver.SecretResolver).read(toRead);
+  let readIndex = 0;
+  const values = references.map((reference) =>
+    isChecked(reference) ? checked?.value : read[readIndex++],
+  );
   const unreadable: Array<string> = [];
   let index = 0;
   // An empty secret reads as unreadable, so a driver cannot fall back to an inherited credential.
@@ -130,6 +148,36 @@ const readInstanceSecrets = Effect.fn("ProviderInstanceRegistry.readInstanceSecr
   return { secrets, unreadable };
 });
 
+// Replaces only the checked reference's value in the secrets a live instance already holds, so
+// another reference whose read failed earlier is not read again and does not park the instance.
+const carryCheckedSecret = <R>(
+  driver: AnyProviderDriver<R> | undefined,
+  entry: ProviderInstanceConfig,
+  current: InstanceSecrets,
+  { reference: checked, value }: CheckedSecret,
+): InstanceSecrets => {
+  const matches = (reference: SecretReference) => sameReference(reference, checked);
+  // A live instance resolved every variable in order, so slots line up with the entry's variables
+  // by position; names may repeat.
+  const variables = entry.environment ?? [];
+  const replacedSlots = configSecretReferences(driver, entry).filter(({ reference }) =>
+    matches(reference),
+  );
+  return {
+    environment: current.environment.map((variable, index) => {
+      const source = variables[index]?.value;
+      return source !== undefined && typeof source !== "string" && matches(source)
+        ? { ...variable, value }
+        : variable;
+    }),
+    config: current.config.map((slot) =>
+      replacedSlots.some(({ field, key }) => field === slot.field && key === slot.key)
+        ? { ...slot, value }
+        : slot,
+    ),
+  };
+};
+
 /** Conflicting saved flags must preserve any explicit disable. */
 const resolveEntryEnabled = (entry: ProviderInstanceConfig, typedConfig: unknown): boolean => {
   const rawConfigEnabled = providerInstanceConfigEnabledFlag(entry.config);
@@ -161,6 +209,7 @@ const buildEntry = Effect.fn("ProviderInstanceRegistry.buildEntry")(function* <R
   readonly rawInstanceId: string;
   readonly entry: ProviderInstanceConfig;
   readonly deferSecrets: boolean;
+  readonly carriedSecrets?: InstanceSecrets | undefined;
 }): Effect.fn.Return<
   | { readonly kind: "live"; readonly live: LiveEntry }
   | {
@@ -171,7 +220,15 @@ const buildEntry = Effect.fn("ProviderInstanceRegistry.buildEntry")(function* <R
   never,
   R | SecretResolver.SecretResolver
 > {
-  const { driversById, parentScope, instanceId, rawInstanceId, entry, deferSecrets } = input;
+  const {
+    driversById,
+    parentScope,
+    instanceId,
+    rawInstanceId,
+    entry,
+    deferSecrets,
+    carriedSecrets,
+  } = input;
   const blocked = input.blockedReason(entry.driver);
   if (blocked !== undefined) {
     return {
@@ -235,12 +292,14 @@ const buildEntry = Effect.fn("ProviderInstanceRegistry.buildEntry")(function* <R
       }),
     };
   }
-  const { secrets, unreadable } = enabled
-    ? yield* readInstanceSecrets(driver, entry)
-    : {
-        secrets: { environment: withoutSecretReferences(entry.environment ?? []), config: [] },
-        unreadable: [],
-      };
+  const { secrets, unreadable } = carriedSecrets
+    ? { secrets: carriedSecrets, unreadable: [] }
+    : enabled
+      ? yield* readInstanceSecrets(driver, entry)
+      : {
+          secrets: { environment: withoutSecretReferences(entry.environment ?? []), config: [] },
+          unreadable: [],
+        };
   // Starting with a blank credential would fail later with a less useful error, or fall back to
   // whatever the server inherited.
   if (unreadable.length > 0) {
@@ -311,9 +370,10 @@ const makeReconcile = <R>(input: {
   configMap: ProviderInstanceConfigMap,
   rebuild?: ReadonlySet<ProviderInstanceId>,
   deferSecrets?: boolean,
+  carried?: ReadonlyMap<ProviderInstanceId, InstanceSecrets>,
 ) => Effect.Effect<void, never, R | SecretResolver.SecretResolver>) => {
   const { state, driversById, blockedReason, parentScope } = input;
-  return (configMap, rebuild = new Set(), deferSecrets = false) =>
+  return (configMap, rebuild = new Set(), deferSecrets = false, carried = new Map()) =>
     Effect.gen(function* () {
       const previousEntries = yield* Ref.get(state.entries);
       const previousUnavailable = yield* Ref.get(state.unavailable);
@@ -365,6 +425,7 @@ const makeReconcile = <R>(input: {
           const instanceId = ProviderInstanceId.make(rawInstanceId);
           const kept = previousEntries.has(instanceId) && !replacedIds.has(instanceId);
           return kept ||
+            carried.has(instanceId) ||
             parkedSnapshot(instanceId, entry) !== undefined ||
             !(yield* isEntryEnabled(driversById, entry))
             ? []
@@ -406,6 +467,7 @@ const makeReconcile = <R>(input: {
           rawInstanceId,
           entry,
           deferSecrets,
+          carriedSecrets: carried.get(instanceId),
         });
         if (result.kind === "live") {
           builtEntries.set(instanceId, result.live);
@@ -503,44 +565,87 @@ export const makeProviderInstanceRegistry = Effect.fn(
       );
 
   // Rebuilds an instance only when it was unavailable or its secrets changed, so a refresh does
-  // not restart providers whose credentials are the same.
-  const reloadSecrets: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape["reloadSecrets"] =
-    Effect.fn("ProviderInstanceRegistry.reloadSecrets")(
-      function* (instanceId) {
-        const configMap = yield* Ref.get(configMapRef);
-        const targets = yield* Effect.filter(Object.entries(configMap), ([rawInstanceId, entry]) =>
-          (instanceId === undefined || rawInstanceId === instanceId) &&
-          secretReferencesOf(driversById, entry).length > 0
-            ? isEntryEnabled(driversById, entry)
-            : Effect.succeed(false),
-        );
-        if (targets.length === 0) return;
+  // not restart providers whose credentials are the same. Without `checked` the cached values are
+  // dropped first; with it, only that reference takes the checked value and the rest carry over.
+  const reloadWhere = Effect.fn("ProviderInstanceRegistry.reloadWhere")(
+    function* (
+      select: (rawInstanceId: string, entry: ProviderInstanceConfig) => boolean,
+      checked?: CheckedSecret,
+    ) {
+      const configMap = yield* Ref.get(configMapRef);
+      const targets = yield* Effect.filter(Object.entries(configMap), ([rawInstanceId, entry]) =>
+        select(rawInstanceId, entry) && secretReferencesOf(driversById, entry).length > 0
+          ? isEntryEnabled(driversById, entry)
+          : Effect.succeed(false),
+      );
+      if (targets.length === 0) return [];
+      if (checked === undefined) {
         const resolver = yield* SecretResolver.SecretResolver;
         const references = targets.flatMap(([, entry]) => secretReferencesOf(driversById, entry));
         yield* resolver.invalidate(references);
         yield* resolver.read(references);
-        const live = yield* Ref.get(entries);
-        const rebuild = new Set<ProviderInstanceId>();
-        for (const [rawInstanceId, entry] of targets) {
-          const current = live.get(ProviderInstanceId.make(rawInstanceId));
-          if (current === undefined) {
-            rebuild.add(ProviderInstanceId.make(rawInstanceId));
-            continue;
+      }
+      const live = yield* Ref.get(entries);
+      const rebuild = new Set<ProviderInstanceId>();
+      const carried = new Map<ProviderInstanceId, InstanceSecrets>();
+      for (const [rawInstanceId, entry] of targets) {
+        const instanceId = ProviderInstanceId.make(rawInstanceId);
+        const current = live.get(instanceId);
+        if (current === undefined) {
+          rebuild.add(instanceId);
+          if (checked !== undefined) {
+            const { secrets, unreadable } = yield* readInstanceSecrets(
+              driversById.get(entry.driver),
+              entry,
+              checked,
+            );
+            if (unreadable.length === 0) carried.set(instanceId, secrets);
           }
-          const { secrets, unreadable } = yield* readInstanceSecrets(
+          continue;
+        }
+        if (checked !== undefined) {
+          const secrets = carryCheckedSecret(
             driversById.get(entry.driver),
-            entry,
+            current.entry,
+            current.secrets,
+            checked,
           );
-          // A failed re-read keeps the working instance and the value it already holds.
-          if (unreadable.length === 0 && !Equal.equals(secrets, current.secrets)) {
+          if (!Equal.equals(secrets, current.secrets)) {
             rebuild.add(current.instance.instanceId);
+            carried.set(current.instance.instanceId, secrets);
           }
+          continue;
         }
-        yield* reconcileWithR(configMap, rebuild);
-      },
-      (effect) => semaphore.withPermits(1)(effect),
-      Effect.provideContext(driverContext),
-    );
+        const { secrets, unreadable } = yield* readInstanceSecrets(
+          driversById.get(entry.driver),
+          entry,
+        );
+        // A failed re-read keeps the working instance and the value it already holds.
+        if (unreadable.length === 0 && !Equal.equals(secrets, current.secrets)) {
+          rebuild.add(current.instance.instanceId);
+        }
+      }
+      yield* reconcileWithR(configMap, rebuild, false, carried);
+      return targets.map(([rawInstanceId]) => ProviderInstanceId.make(rawInstanceId));
+    },
+    (effect) => semaphore.withPermits(1)(effect),
+    Effect.provideContext(driverContext),
+  );
+  const reloadSecrets: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape["reloadSecrets"] =
+    (instanceId) =>
+      reloadWhere(
+        (rawInstanceId) => instanceId === undefined || rawInstanceId === instanceId,
+        undefined,
+      ).pipe(Effect.asVoid);
+  const applySecret: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape["applySecret"] =
+    (reference, value) =>
+      reloadWhere(
+        (_, entry) =>
+          secretReferencesOf(driversById, entry).some((candidate) =>
+            sameReference(candidate, reference),
+          ),
+        { reference, value },
+      );
 
   // Reading secrets can wait on a vault unlock, so the server does not wait for it to start.
   yield* semaphore.withPermits(1)(
@@ -592,6 +697,7 @@ export const makeProviderInstanceRegistry = Effect.fn(
   const mutator: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape = {
     reconcile,
     reloadSecrets,
+    applySecret,
   };
 
   return { registry, mutator };

apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts

@@ -165,6 +165,7 @@ describe("layerSettingsWatcher", () => {
       const mutator = Layer.succeed(ProviderInstanceRegistryMutator, {
         reconcile: () => Queue.offer(reconciled, undefined),
         reloadSecrets: () => Effect.sync(() => void reloads++),
+        applySecret: () => Effect.succeed([]),
       });
       yield* Layer.build(
         layerSettingsWatcher(Stream.fromQueue(changes), initial).pipe(Layer.provide(mutator)),

apps/server/src/provider/ProviderInstanceRegistryMutator.ts

@@ -1,12 +1,24 @@
 /** Reconcile closes removed/replaced scopes before creating replacements: at most one live instance per id. */
-import type { ProviderInstanceConfigMap, ProviderInstanceId } from "@t3tools/contracts";
+import type {
+  ProviderInstanceConfigMap,
+  ProviderInstanceId,
+  SecretReference,
+} from "@t3tools/contracts";
 import * as Context from "effect/Context";
 import type * as Effect from "effect/Effect";
 
 export interface ProviderInstanceRegistryMutatorShape {
   readonly reconcile: (configMap: ProviderInstanceConfigMap) => Effect.Effect<void>;
   /** Re-reads the secrets of every instance, or one, and rebuilds those whose secrets changed. */
   readonly reloadSecrets: (instanceId?: ProviderInstanceId) => Effect.Effect<void>;
+  /**
+   * Rebuilds the instances that read `reference` with `value`, keeping the other secrets a live
+   * instance holds, and returns them. A parked instance reads its other secrets from the resolver.
+   */
+  readonly applySecret: (
+    reference: SecretReference,
+    value: string,
+  ) => Effect.Effect<ReadonlyArray<ProviderInstanceId>>;
 }
 
 export class ProviderInstanceRegistryMutator extends Context.Service<

apps/server/src/provider/ProviderRegistry.ts

@@ -3,6 +3,7 @@ import {
   isProviderWorkspaceSnapshotCurrent,
   ProviderDriverKind,
   type ProviderInstanceId,
+  type SecretReference,
   type ServerProvider,
   type ServerProviderUpdateState,
 } from "@t3tools/contracts";
@@ -69,6 +70,12 @@ export class ProviderRegistry extends Context.Service<
       instanceId?: ProviderInstanceId,
     ) => Effect.Effect<ReadonlyArray<ServerProvider>>;
 
+    /** Rebuilds and refreshes only the instances that read a secret, with the value the caller just read. */
+    readonly applySecretAndRefresh: (
+      reference: SecretReference,
+      value: string,
+    ) => Effect.Effect<ReadonlyArray<ServerProvider>>;
+
     /** Only refreshed instances can verify an update; the published list still includes every row. */
     readonly refreshAfterUpdate: (
       provider: ProviderDriverKind,
@@ -692,6 +699,15 @@ export const layer = Layer.effect(
       return yield* instanceId === undefined ? refresh() : refreshInstance(instanceId);
     });
 
+    const applySecretAndRefresh = Effect.fn("applySecretAndRefresh")(function* (
+      reference: SecretReference,
+      value: string,
+    ) {
+      const instanceIds = yield* instanceRegistryMutator.applySecret(reference, value);
+      yield* Effect.forEach(instanceIds, refreshInstance, { discard: true });
+      return yield* Ref.get(providersRef);
+    });
+
     const refreshAfterUpdate = Effect.fn("refreshAfterUpdate")(function* (
       provider: ProviderDriverKind,
       selectedInstanceId: ProviderInstanceId,
@@ -1077,6 +1093,8 @@ export const layer = Layer.effect(
         refreshInstance(instanceId).pipe(Effect.catchCause(recoverRefreshFailure)),
       reloadSecretsAndRefresh: (instanceId?: ProviderInstanceId) =>
         reloadSecretsAndRefresh(instanceId).pipe(Effect.catchCause(recoverRefreshFailure)),
+      applySecretAndRefresh: (reference: SecretReference, value: string) =>
+        applySecretAndRefresh(reference, value).pipe(Effect.catchCause(recoverRefreshFailure)),
       refreshWorkspaceSnapshot: (input) =>
         refreshWorkspaceSnapshot(input).pipe(Effect.catchCause(recoverRefreshFailure)),
       refreshAfterUpdate,

apps/server/src/provider/providerMaintenanceRunner.test.ts

@@ -198,6 +198,7 @@ function makeRegistry(
       refresh: () => Ref.get(providersRef),
       refreshInstance: () => Ref.get(providersRef),
       reloadSecretsAndRefresh: () => Ref.get(providersRef),
+      applySecretAndRefresh: () => Ref.get(providersRef),
       refreshAfterUpdate: () => {
         const version = options?.installedVersionAfterUpdate;
         if (version === undefined) {

apps/server/src/provider/testUtils/providerRegistryMock.ts

@@ -12,6 +12,7 @@ const makeProviderRegistryMock = (
   refresh: () => Effect.succeed(providers),
   refreshInstance: () => Effect.succeed(providers),
   reloadSecretsAndRefresh: () => Effect.succeed(providers),
+  applySecretAndRefresh: () => Effect.succeed(providers),
   refreshAfterUpdate: () => Effect.succeed(providers),
   refreshWorkspaceSnapshot: () => Effect.succeed(providers),
   invalidateModelCaches: () => Effect.void,