From 5772b1ce9638dd760b535e34d2edb3d7b6a30d56 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Thu, 5 Nov 2026 18:06:01 -0500 Subject: [PATCH 1/7] feat(secrets): check a secret reference before saving it Settings gains a check button beside a secret reference. It reads the secret fresh from its provider and says whether it resolved, without sending the value to the client. A readable secret also rebuilds the provider instances that depend on it. Signed-off-by: Yordis Prieto --- .../src/observability/RpcInstrumentation.ts | 1 + .../src/provider/ProviderInstanceRegistry.ts | 93 ++++++---- .../ProviderInstanceRegistryHydration.test.ts | 1 + .../ProviderInstanceRegistryMutator.ts | 13 +- apps/server/src/provider/ProviderRegistry.ts | 16 ++ .../providerMaintenanceRunner.test.ts | 1 + .../testUtils/providerRegistryMock.ts | 1 + apps/server/src/secrets/SecretCheck.test.ts | 170 ++++++++++++++++++ apps/server/src/secrets/SecretCheck.ts | 40 +++++ apps/server/src/server.ts | 2 + apps/server/src/ws.test.ts | 2 + apps/server/src/ws.ts | 3 + .../settings/AddUsageLimitSourceDialog.tsx | 1 + .../settings/EasDeviceHostEditor.tsx | 1 + .../settings/ProviderInstanceCard.tsx | 6 + .../settings/ProviderSettingsForm.tsx | 1 + .../components/settings/SecretSourceInput.tsx | 162 +++++++++++++---- .../src/state/commandPermissions.test.ts | 40 +++-- packages/client-runtime/src/state/server.ts | 4 + .../contracts/src/clientRpcPermissions.ts | 1 + packages/contracts/src/rpc.ts | 10 ++ packages/contracts/src/settingsSecretValue.ts | 6 + 22 files changed, 487 insertions(+), 88 deletions(-) create mode 100644 apps/server/src/secrets/SecretCheck.test.ts create mode 100644 apps/server/src/secrets/SecretCheck.ts diff --git a/apps/server/src/observability/RpcInstrumentation.ts b/apps/server/src/observability/RpcInstrumentation.ts index 89f93a9df2c81..ba6b8a0905ca5 100644 --- a/apps/server/src/observability/RpcInstrumentation.ts +++ b/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", diff --git a/apps/server/src/provider/ProviderInstanceRegistry.ts b/apps/server/src/provider/ProviderInstanceRegistry.ts index 3f4df0eb35d18..e518a4c7330c5 100644 --- a/apps/server/src/provider/ProviderInstanceRegistry.ts +++ b/apps/server/src/provider/ProviderInstanceRegistry.ts @@ -503,44 +503,64 @@ 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), + // not restart providers whose credentials are the same. `fresh` drops the cached values first; + // without it the values a caller just read are used as they are. + const reloadWhere = Effect.fn("ProviderInstanceRegistry.reloadWhere")( + function* ( + select: (rawInstanceId: string, entry: ProviderInstanceConfig) => boolean, + fresh: boolean, + ) { + 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 []; + const resolver = yield* SecretResolver.SecretResolver; + const references = targets.flatMap(([, entry]) => secretReferencesOf(driversById, entry)); + if (fresh) yield* resolver.invalidate(references); + yield* resolver.read(references); + const live = yield* Ref.get(entries); + const rebuild = new Set(); + for (const [rawInstanceId, entry] of targets) { + const current = live.get(ProviderInstanceId.make(rawInstanceId)); + if (current === undefined) { + rebuild.add(ProviderInstanceId.make(rawInstanceId)); + continue; + } + const { secrets, unreadable } = yield* readInstanceSecrets( + driversById.get(entry.driver), + entry, ); - if (targets.length === 0) return; - 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(); - for (const [rawInstanceId, entry] of targets) { - const current = live.get(ProviderInstanceId.make(rawInstanceId)); - if (current === undefined) { - rebuild.add(ProviderInstanceId.make(rawInstanceId)); - continue; - } - 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); - } + // 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); - }, - (effect) => semaphore.withPermits(1)(effect), - Effect.provideContext(driverContext), - ); + } + yield* reconcileWithR(configMap, rebuild); + 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, + true, + ).pipe(Effect.asVoid); + const applySecret: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape["applySecret"] = + (reference) => + reloadWhere( + (_, entry) => + secretReferencesOf(driversById, entry).some( + (candidate) => + candidate.provider === reference.provider && + candidate.reference === reference.reference, + ), + false, + ); // Reading secrets can wait on a vault unlock, so the server does not wait for it to start. yield* semaphore.withPermits(1)( @@ -592,6 +612,7 @@ export const makeProviderInstanceRegistry = Effect.fn( const mutator: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape = { reconcile, reloadSecrets, + applySecret, }; return { registry, mutator }; diff --git a/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts b/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts index dc44f0324a6b0..02bc92f4b4504 100644 --- a/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts +++ b/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)), diff --git a/apps/server/src/provider/ProviderInstanceRegistryMutator.ts b/apps/server/src/provider/ProviderInstanceRegistryMutator.ts index 35eb52d3f2906..1d6324d97b98a 100644 --- a/apps/server/src/provider/ProviderInstanceRegistryMutator.ts +++ b/apps/server/src/provider/ProviderInstanceRegistryMutator.ts @@ -1,5 +1,9 @@ /** 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"; @@ -7,6 +11,13 @@ export interface ProviderInstanceRegistryMutatorShape { readonly reconcile: (configMap: ProviderInstanceConfigMap) => Effect.Effect; /** Re-reads the secrets of every instance, or one, and rebuilds those whose secrets changed. */ readonly reloadSecrets: (instanceId?: ProviderInstanceId) => Effect.Effect; + /** + * Rebuilds the instances that read `reference`, from the value its resolver already holds, and + * returns them. Nothing else is read. + */ + readonly applySecret: ( + reference: SecretReference, + ) => Effect.Effect>; } export class ProviderInstanceRegistryMutator extends Context.Service< diff --git a/apps/server/src/provider/ProviderRegistry.ts b/apps/server/src/provider/ProviderRegistry.ts index fb6aeaac26b63..6ced8c4f6f971 100644 --- a/apps/server/src/provider/ProviderRegistry.ts +++ b/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,11 @@ export class ProviderRegistry extends Context.Service< instanceId?: ProviderInstanceId, ) => Effect.Effect>; + /** Rebuilds and refreshes only the instances that read a secret the caller just read. */ + readonly applySecretAndRefresh: ( + reference: SecretReference, + ) => Effect.Effect>; + /** Only refreshed instances can verify an update; the published list still includes every row. */ readonly refreshAfterUpdate: ( provider: ProviderDriverKind, @@ -692,6 +698,14 @@ export const layer = Layer.effect( return yield* instanceId === undefined ? refresh() : refreshInstance(instanceId); }); + const applySecretAndRefresh = Effect.fn("applySecretAndRefresh")(function* ( + reference: SecretReference, + ) { + const instanceIds = yield* instanceRegistryMutator.applySecret(reference); + yield* Effect.forEach(instanceIds, refreshInstance, { discard: true }); + return yield* Ref.get(providersRef); + }); + const refreshAfterUpdate = Effect.fn("refreshAfterUpdate")(function* ( provider: ProviderDriverKind, selectedInstanceId: ProviderInstanceId, @@ -1077,6 +1091,8 @@ export const layer = Layer.effect( refreshInstance(instanceId).pipe(Effect.catchCause(recoverRefreshFailure)), reloadSecretsAndRefresh: (instanceId?: ProviderInstanceId) => reloadSecretsAndRefresh(instanceId).pipe(Effect.catchCause(recoverRefreshFailure)), + applySecretAndRefresh: (reference: SecretReference) => + applySecretAndRefresh(reference).pipe(Effect.catchCause(recoverRefreshFailure)), refreshWorkspaceSnapshot: (input) => refreshWorkspaceSnapshot(input).pipe(Effect.catchCause(recoverRefreshFailure)), refreshAfterUpdate, diff --git a/apps/server/src/provider/providerMaintenanceRunner.test.ts b/apps/server/src/provider/providerMaintenanceRunner.test.ts index 77a44c7fa29a5..1d2603e9b211b 100644 --- a/apps/server/src/provider/providerMaintenanceRunner.test.ts +++ b/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) { diff --git a/apps/server/src/provider/testUtils/providerRegistryMock.ts b/apps/server/src/provider/testUtils/providerRegistryMock.ts index f75ecd141e71a..2958ce7614353 100644 --- a/apps/server/src/provider/testUtils/providerRegistryMock.ts +++ b/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, diff --git a/apps/server/src/secrets/SecretCheck.test.ts b/apps/server/src/secrets/SecretCheck.test.ts new file mode 100644 index 0000000000000..f5aa6a847ae15 --- /dev/null +++ b/apps/server/src/secrets/SecretCheck.test.ts @@ -0,0 +1,170 @@ +import * as NodeCrypto from "@effect/platform-node/NodeCrypto"; +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { assert, it } from "@effect/vitest"; +import { + ProviderDriverKind, + ProviderInstanceId, + SecretProviderId, + SecretProviderKind, + type SecretReference, +} from "@t3tools/contracts"; +import type { AnyProviderDriver, ProviderInstance } from "@t3tools/provider-core/server/driver"; +import * as SecretResolver from "@t3tools/secret-provider-core/server/SecretResolver"; +import { defineSecretProviderDriver } from "@t3tools/secret-provider-core/server/driver"; +import { defineSecretProvider } from "@t3tools/secret-provider-core/shared/definition"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import * as PubSub from "effect/PubSub"; +import * as Schema from "effect/Schema"; + +import { makeProviderInstanceRegistry } from "../provider/ProviderInstanceRegistry.ts"; +import * as ProviderRegistry from "../provider/ProviderRegistry.ts"; +import * as SecretCheck from "./SecretCheck.ts"; + +const reference: SecretReference = { + provider: SecretProviderId.make("work"), + reference: "op://Private/OpenAI/api-key", +}; + +const run = (value: string | undefined) => + Effect.gen(function* () { + const calls: Array = []; + const layer = SecretCheck.layer.pipe( + Layer.provide( + Layer.mergeAll( + Layer.mock(SecretResolver.SecretResolver)({ + invalidate: () => Effect.sync(() => void calls.push("invalidate")), + read: () => Effect.sync(() => (calls.push("read"), [value])), + }), + Layer.mock(ProviderRegistry.ProviderRegistry)({ + applySecretAndRefresh: () => Effect.sync(() => (calls.push("apply"), [])), + }), + ), + ), + ); + const result = yield* SecretCheck.SecretCheck.use((check) => check.check(reference)).pipe( + Effect.provide(layer), + ); + return { result, calls }; + }); + +it.effect( + "a readable secret resolves without returning its value and applies it to providers", + () => + Effect.gen(function* () { + const { result, calls } = yield* run("sk-live-value"); + assert.deepStrictEqual(result, { _tag: "Resolved" }); + assert.deepStrictEqual(calls, ["invalidate", "read", "apply"]); + assert.notInclude(JSON.stringify(result), "sk-live-value"); + }), +); + +it.effect("an unreadable secret fails with the provider named and applies nothing", () => + Effect.gen(function* () { + const { result, calls } = yield* run(undefined); + assert.strictEqual(result._tag, "Failed"); + if (result._tag === "Failed") assert.include(result.detail, '"work"'); + assert.deepStrictEqual(calls, ["invalidate", "read"]); + }), +); + +const FAKE_KIND = SecretProviderKind.make("fake"); + +it.effect( + "applies a check to the instances that read the secret, whatever a second read would return", + () => + Effect.gen(function* () { + const store = new Map>(); + const reads: Array = []; + const secretDriver = defineSecretProviderDriver({ + kind: FAKE_KIND, + definition: defineSecretProvider({ + kind: FAKE_KIND, + label: "Fake", + configSchema: Schema.Struct({}), + isReference: (value) => value.startsWith("fake://"), + referencePlaceholder: "fake://item", + invalidReferenceMessage: "Use fake://item.", + }), + read: (_config, references) => + Effect.sync(() => + references.map((reference) => { + reads.push(reference); + const answers = store.get(reference) ?? []; + return answers.length > 1 ? answers.shift() : answers[0]; + }), + ), + }); + const provider = SecretProviderId.make("personal"); + const wanted: SecretReference = { provider, reference: "fake://wanted" }; + const other: SecretReference = { provider, reference: "fake://other" }; + store.set(wanted.reference, [undefined]); + store.set(other.reference, ["other-key"]); + + const driverKind = ProviderDriverKind.make("fake-provider"); + const dependent = ProviderInstanceId.make("dependent"); + const unrelated = ProviderInstanceId.make("unrelated"); + const driver: AnyProviderDriver = { + driverKind, + metadata: { displayName: "Fake" }, + configSchema: Schema.Unknown, + defaultConfig: () => ({}), + create: (input) => + Effect.succeed({ + instanceId: input.instanceId, + enabled: input.enabled, + } as ProviderInstance), + }; + const instanceConfig = (secret: SecretReference) => ({ + driver: driverKind, + config: {}, + environment: [{ name: "API_KEY", value: secret, sensitive: true }], + }); + + const resolverLayer = SecretResolver.layer({ + drivers: [secretDriver], + secretProviders: Effect.succeed({ + [provider]: { kind: FAKE_KIND, label: "Personal", config: {} }, + }), + }).pipe(Layer.provide(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer))); + + yield* Effect.gen(function* () { + const resolver = yield* SecretResolver.SecretResolver; + const { registry, mutator } = yield* makeProviderInstanceRegistry({ + drivers: [driver], + configMap: { [dependent]: instanceConfig(wanted), [unrelated]: instanceConfig(other) }, + }); + const changes = yield* registry.subscribeChanges; + yield* PubSub.take(changes); + assert.deepStrictEqual( + (yield* registry.listUnavailable).map((entry) => entry.instanceId), + [dependent], + ); + + // Unlocked for the check's read only: any later read of it would fail again. + store.set(wanted.reference, ["wanted-key", undefined]); + const readsBefore = reads.length; + const result = yield* SecretCheck.SecretCheck.use((check) => check.check(wanted)).pipe( + Effect.provide( + SecretCheck.layer.pipe( + Layer.provide( + Layer.mock(ProviderRegistry.ProviderRegistry)({ + applySecretAndRefresh: (reference) => + mutator.applySecret(reference).pipe(Effect.as([])), + }), + ), + Layer.provideMerge(Layer.succeed(SecretResolver.SecretResolver, resolver)), + ), + ), + ); + + assert.deepStrictEqual(result, { _tag: "Resolved" }); + assert.deepStrictEqual( + (yield* registry.listInstances).map((instance) => instance.instanceId).sort(), + [dependent, unrelated], + ); + assert.deepStrictEqual(yield* registry.listUnavailable, []); + assert.deepStrictEqual(reads.slice(readsBefore), [wanted.reference]); + }).pipe(Effect.provide(resolverLayer), Effect.scoped); + }), +); diff --git a/apps/server/src/secrets/SecretCheck.ts b/apps/server/src/secrets/SecretCheck.ts new file mode 100644 index 0000000000000..bce705b05afa6 --- /dev/null +++ b/apps/server/src/secrets/SecretCheck.ts @@ -0,0 +1,40 @@ +import type { SecretReference, SecretSourceCheckResult } from "@t3tools/contracts"; +import * as SecretResolver from "@t3tools/secret-provider-core/server/SecretResolver"; +import * as Context from "effect/Context"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; + +import * as ProviderRegistry from "../provider/ProviderRegistry.ts"; + +export class SecretCheck extends Context.Service< + SecretCheck, + { + /** + * Reads one secret fresh for the Settings "check" button and never returns the value. A + * readable secret also rebuilds the provider instances that read it, from that same read. + */ + readonly check: (reference: SecretReference) => Effect.Effect; + } +>()("t3/secrets/SecretCheck") {} + +const make = Effect.gen(function* () { + const resolver = yield* SecretResolver.SecretResolver; + const providerRegistry = yield* ProviderRegistry.ProviderRegistry; + + const check = Effect.fn("SecretCheck.check")(function* (reference: SecretReference) { + yield* resolver.invalidate([reference]); + const [value] = yield* resolver.read([reference]); + if (value === undefined) { + return { + _tag: "Failed", + detail: `Could not read this secret from "${reference.provider}". Check the provider's settings and that its store is unlocked.`, + } as const; + } + yield* providerRegistry.applySecretAndRefresh(reference); + return { _tag: "Resolved" } as const; + }); + + return SecretCheck.of({ check }); +}); + +export const layer = Layer.effect(SecretCheck, make); diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 5c9ab9c433ffc..fcbc4b653bf00 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -72,6 +72,7 @@ import * as SourceControlBuiltInDrivers from "./sourceControl/builtInDrivers.ts" import * as SourceControlSignIn from "./sourceControl/SourceControlSignIn.ts"; import { BUILT_IN_SOURCE_CONTROL_DRIVER_LIST } from "./sourceControl/sourceControlDefinitions.ts"; import * as TextGeneration from "./textGeneration/TextGeneration.ts"; +import * as SecretCheck from "./secrets/SecretCheck.ts"; import * as SecretResolver from "./secrets/SecretResolver.ts"; import * as ProviderInstanceRegistryHydration from "./provider/ProviderInstanceRegistryHydration.ts"; import * as ProjectSettingsOverrideSweep from "./provider/ProjectSettingsOverrideSweep.ts"; @@ -671,6 +672,7 @@ const layerRuntimeCoreDependenciesBase = Layer.mergeAll( Layer.provideMerge(layerOrchestrationApplication), Layer.provideMerge(RuntimeLayer.layerEventInfrastructure), Layer.provideMerge(Layer.merge(ProjectStore.layer, ThreadSearch.layer)), + Layer.provideMerge(SecretCheck.layer), Layer.provideMerge(SecretResolver.layer), Layer.provideMerge(layerServerSettings), Layer.provideMerge(layerSourceControlProviderRegistry), diff --git a/apps/server/src/ws.test.ts b/apps/server/src/ws.test.ts index bf2298427e411..db2a015aa1a87 100644 --- a/apps/server/src/ws.test.ts +++ b/apps/server/src/ws.test.ts @@ -114,6 +114,7 @@ import * as TraceDiagnostics from "./diagnostics/TraceDiagnostics.ts"; import * as TriageService from "./triage/TriageService.ts"; import * as WorkspacePaths from "./workspace/WorkspacePaths.ts"; import * as ServerSecretStore from "./auth/ServerSecretStore.ts"; +import * as SecretCheck from "./secrets/SecretCheck.ts"; import * as ServerConfigModule from "./config.ts"; import * as Keybindings from "./keybindings.ts"; import { USAGE_LIMITS_COMMAND } from "@t3tools/shared/usageLimits"; @@ -659,6 +660,7 @@ it.effect("shell snapshots omit idle children and older settled threads only on const layerUnusedConfigRpcServices = Layer.mergeAll( Layer.mock(DocumentText.DocumentText)({}), Layer.mock(ServerSecretStore.ServerSecretStore)({}), + Layer.mock(SecretCheck.SecretCheck)({}), Layer.mock(WorkspacePaths.WorkspacePaths)({}), Layer.mock(TraceDiagnostics.TraceDiagnostics)({}), Layer.mock(TriageService.TriageService)({}), diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index e3c7fdaa47476..93f0429526c85 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -172,6 +172,7 @@ import * as ThreadSearch from "./orchestration-v2/ThreadSearch.ts"; import * as OrchestrationEventStore from "./persistence/OrchestrationEventStore.ts"; import { userFacingDispatchErrorMessage } from "./orchestration-v2/UserFacingErrors.ts"; import * as ProviderRegistry from "./provider/ProviderRegistry.ts"; +import * as SecretCheck from "./secrets/SecretCheck.ts"; import * as ProviderInstanceRegistry from "./provider/ProviderInstanceRegistry.ts"; import * as AcpRegistrySupport from "@t3tools/provider-acp-registry/server/AcpRegistrySupport"; import * as AcpRegistryRuntimeCoordinator from "@t3tools/provider-acp-registry/server/AcpRegistryRuntimeCoordinator"; @@ -1532,6 +1533,7 @@ export const layerWsRpc = ( ); const serverAuth = yield* EnvironmentAuth.EnvironmentAuth; const sourceControlDiscovery = yield* SourceControlDiscovery.SourceControlDiscovery; + const secretCheck = yield* SecretCheck.SecretCheck; const otlpEndpointCheck = yield* OtlpEndpointCheck.OtlpEndpointCheck; const sourceControlRepositories = yield* SourceControlRepositoryService.SourceControlRepositoryService; @@ -2699,6 +2701,7 @@ export const layerWsRpc = ( return ServerSettings.redactServerSettingsForClient(settings, settingsClient); }), [WS_METHODS.serverDiscoverSourceControl]: (_input) => sourceControlDiscovery.discover, + [WS_METHODS.serverCheckSecretSource]: (input) => secretCheck.check(input.source), [WS_METHODS.serverGetTraceDiagnostics]: (_input) => TraceDiagnostics.readTraceDiagnostics({ traceFilePath: config.serverTracePath, diff --git a/apps/web/src/components/settings/AddUsageLimitSourceDialog.tsx b/apps/web/src/components/settings/AddUsageLimitSourceDialog.tsx index 780fba4a7007d..2098055289e02 100644 --- a/apps/web/src/components/settings/AddUsageLimitSourceDialog.tsx +++ b/apps/web/src/components/settings/AddUsageLimitSourceDialog.tsx @@ -107,6 +107,7 @@ export function AddUsageLimitSourceDialog({ aria-label="Management key" value={keyDraft} sources={secretSources} + environmentId={environmentId} onChange={(event) => setKeyDraft(event.target.value)} onSecretChange={(secret) => setKeyDraft(secret ?? "")} onValidityChange={setKeyValid} diff --git a/apps/web/src/components/settings/EasDeviceHostEditor.tsx b/apps/web/src/components/settings/EasDeviceHostEditor.tsx index 1223fe09b7d7f..e11d9dfa748aa 100644 --- a/apps/web/src/components/settings/EasDeviceHostEditor.tsx +++ b/apps/web/src/components/settings/EasDeviceHostEditor.tsx @@ -213,6 +213,7 @@ export function EasDeviceHostEditor(props: { : "" } sources={secretSources} + environmentId={props.environmentId} onValidityChange={(valid) => { setTokenValid(valid); resetAccount(); diff --git a/apps/web/src/components/settings/ProviderInstanceCard.tsx b/apps/web/src/components/settings/ProviderInstanceCard.tsx index b676e64399d97..4de75ab8bbe8a 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.tsx +++ b/apps/web/src/components/settings/ProviderInstanceCard.tsx @@ -282,6 +282,7 @@ function ProviderEnvironmentFieldRow(props: { readonly onRemove: (field: ProviderEnvironmentField) => void; readonly managedReason?: string | undefined; readonly secretSources: ReadonlyArray; + readonly environmentId?: EnvironmentId | undefined; }) { const inputId = `${props.idPrefix}-environment-${props.field.name}`; const value = props.variable?.valueRedacted ? "" : (props.variable?.value ?? ""); @@ -316,6 +317,7 @@ function ProviderEnvironmentFieldRow(props: { spellCheck={false} aria-label={props.field.label} sources={props.secretSources} + environmentId={props.environmentId} onSecretChange={(secret) => props.onSecretChange(props.field, secret)} /> )} @@ -342,6 +344,7 @@ export function ProviderEnvironmentSection(props: { /** Variables an administrator's policy pins: listed by name, never editable or removable. */ readonly managed?: { readonly names: ReadonlySet; readonly reason: string } | undefined; readonly secretSources?: ReadonlyArray; + readonly environmentId?: EnvironmentId | undefined; }) { const [storedDraft, setDraft] = useState(() => seedRowsDraft(props.environment, toEnvironmentDraftRows), @@ -531,6 +534,7 @@ export function ProviderEnvironmentSection(props: { spellCheck={false} aria-label={`Environment variable value ${index + 1}`} sources={props.secretSources ?? []} + environmentId={props.environmentId} onSecretChange={(secret) => setVariableSecret(variable.id, secret)} > {(parts) => layout(parts.source, parts.value)} @@ -1306,6 +1310,7 @@ export function ProviderInstanceCard({ : undefined } secretSources={secretSources} + environmentId={environmentId} /> ))} @@ -1353,6 +1358,7 @@ export function ProviderInstanceCard({ environment={genericEnvironment} onChange={updateGenericEnvironment} secretSources={secretSources} + environmentId={environmentId} managed={ instanceLocks && { names: instanceLocks.environmentNames, diff --git a/apps/web/src/components/settings/ProviderSettingsForm.tsx b/apps/web/src/components/settings/ProviderSettingsForm.tsx index 646dfd80edc20..421f7d7b16696 100644 --- a/apps/web/src/components/settings/ProviderSettingsForm.tsx +++ b/apps/web/src/components/settings/ProviderSettingsForm.tsx @@ -373,6 +373,7 @@ function SecretSettingsControl({ value={reference ?? ""} configured={reference === undefined && configured} sources={sources} + environmentId={environmentId} onCommit={(next) => onChange(nextProviderConfigWithFieldValue(value, field, next))} onSecretChange={(secret) => onChange(nextProviderConfigWithFieldValue(value, field, secret ?? "")) diff --git a/apps/web/src/components/settings/SecretSourceInput.tsx b/apps/web/src/components/settings/SecretSourceInput.tsx index 56723508c8544..7bd85809cc348 100644 --- a/apps/web/src/components/settings/SecretSourceInput.tsx +++ b/apps/web/src/components/settings/SecretSourceInput.tsx @@ -9,15 +9,19 @@ import { builtInSecretProviderDefinitions } from "@t3tools/secret-provider-defin import { useAtomValue } from "@effect/atom-react"; import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; -import { KeyRoundIcon, TypeIcon } from "lucide-react"; +import { AlertTriangleIcon, CheckIcon, KeyRoundIcon, RefreshCwIcon, TypeIcon } from "lucide-react"; import { useState, type ComponentProps, type ComponentType, type ReactNode } from "react"; import { isSecretsFeatureEnabled } from "../../env"; import { useEnvironmentSettings } from "../../hooks/useSettings"; import { OnePasswordIcon } from "../Icons"; import { serverEnvironment } from "../../state/server"; +import { useAtomCommand } from "../../state/use-atom-command"; +import { Button } from "../ui/button"; import { DraftInput } from "../ui/draft-input"; import { Select, SelectItem, SelectPopup, SelectTrigger, SelectValue } from "../ui/select"; +import { Spinner } from "../ui/spinner"; +import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { SecretInput } from "./SecretInput"; import { classifySecretProvider } from "./SecretProviderDialog.logic"; @@ -111,8 +115,91 @@ export function SecretSourceSelect(props: { ); } +type SecretCheck = + | { readonly status: "checking" } + | { readonly status: "resolved" } + | { readonly status: "failed"; readonly detail: string }; + +/** Asks the server to resolve the committed reference, so a typo shows here and not at provider start. */ +function SecretCheckButton(props: { + readonly environmentId: EnvironmentId; + readonly secret: SecretReference; + readonly label: string; +}) { + const { environmentId, secret } = props; + const canCheck = useAtomValue(serverEnvironment.checkSecretSource.permissionAtom(environmentId)); + const checkSecretSource = useAtomCommand(serverEnvironment.checkSecretSource); + const secretKey = `${secret.provider}\u0000${secret.reference}`; + const [check, setCheck] = useState<{ + readonly secretKey: string; + readonly state: SecretCheck; + } | null>(null); + const state = check?.secretKey === secretKey ? check.state : undefined; + + const run = async () => { + setCheck({ secretKey, state: { status: "checking" } }); + const response = await checkSecretSource({ environmentId, input: { source: secret } }); + const next: SecretCheck = + response._tag !== "Success" + ? { status: "failed", detail: "Could not reach the server" } + : response.value._tag === "Resolved" + ? { status: "resolved" } + : { status: "failed", detail: response.value.detail }; + setCheck((previous) => + previous?.secretKey === secretKey ? { secretKey, state: next } : previous, + ); + }; + + const description = !canCheck + ? "You cannot check secrets on this environment" + : state === undefined + ? "Check that the secret resolves" + : state.status === "checking" + ? "Checking the secret" + : state.status === "resolved" + ? "Secret resolved" + : state.detail; + return ( + + + + + } + /> + {description} + + ); +} + type SecretSourceInputProps = Omit, "value" | "children"> & { readonly value: string | SecretReference; + /** Offers a check of the committed reference on this environment. */ + readonly environmentId?: EnvironmentId | undefined; readonly sources: ReadonlyArray; readonly onSecretChange: (secret: SecretReference | undefined) => void; /** Whether the selected source and committed field can be saved or used. */ @@ -127,6 +214,7 @@ type SecretSourceInputProps = Omit, "value" | /** A secret entered here, or read from a secret provider when one is configured. */ export function SecretSourceInput({ value, + environmentId, sources, onSecretChange, onValidityChange, @@ -168,38 +256,48 @@ export function SecretSourceInput({ ) : (
- { - if (next.trim().length === 0) { +
+ { + if (next.trim().length === 0) { + setRejectedReference(null); + onValidityChange?.(pendingProvider === null && secret !== undefined); + if (pendingProvider === null && secret !== undefined) onSecretChange(undefined); + return; + } + const decoded = decodeSecretReference({ provider, reference: next }); + if ( + Option.isNone(decoded) || + definition?.isReference(decoded.value.reference) === false + ) { + setRejectedReference(next); + onValidityChange?.(false); + return; + } setRejectedReference(null); - onValidityChange?.(pendingProvider === null && secret !== undefined); - if (pendingProvider === null && secret !== undefined) onSecretChange(undefined); - return; - } - const decoded = decodeSecretReference({ provider, reference: next }); - if ( - Option.isNone(decoded) || - definition?.isReference(decoded.value.reference) === false - ) { - setRejectedReference(next); - onValidityChange?.(false); - return; - } - setRejectedReference(null); - setPendingProvider(null); - onValidityChange?.(true); - onSecretChange(decoded.value); - }} - autoComplete="off" - placeholder={definition?.referencePlaceholder} - spellCheck={false} - aria-label={`${label} reference`} - aria-invalid={rejectedReference !== null} - /> + setPendingProvider(null); + onValidityChange?.(true); + onSecretChange(decoded.value); + }} + autoComplete="off" + placeholder={definition?.referencePlaceholder} + spellCheck={false} + aria-label={`${label} reference`} + aria-invalid={rejectedReference !== null} + /> + {environmentId !== undefined && secret !== undefined && pendingProvider === null ? ( + + ) : null} +
{rejectedReference !== null ? (

{definition?.invalidReferenceMessage ?? "Enter a valid secret reference."} diff --git a/packages/client-runtime/src/state/commandPermissions.test.ts b/packages/client-runtime/src/state/commandPermissions.test.ts index 226931f38c0b9..42cf3c5a78a36 100644 --- a/packages/client-runtime/src/state/commandPermissions.test.ts +++ b/packages/client-runtime/src/state/commandPermissions.test.ts @@ -156,25 +156,27 @@ describe("command permissions", () => { ), ); - it.effect("requires the destination settings grant to check OTLP endpoints", () => - Effect.scoped( - Effect.gen(function* () { - const registry = yield* setup; - const check = createCommandPermissions(runtime, WS_METHODS.serverCheckOtlpEndpoint); - registry.set(sessions(env), AsyncResult.success(grant(true))); - expect(registry.get(check.permissionAtom(env))).toBe(false); - registry.set( - sessions(env), - AsyncResult.success({ - ...grant(false), - scopes: [AuthSettingsWriteScope], - permissions: [AuthSettingsWriteScope], - }), - ); - expect(registry.get(check.permissionAtom(env))).toBe(true); - yield* check.authorize(registry, env); - }), - ), + it.effect.each([WS_METHODS.serverCheckOtlpEndpoint, WS_METHODS.serverCheckSecretSource])( + "requires the destination settings grant for %s", + (method) => + Effect.scoped( + Effect.gen(function* () { + const registry = yield* setup; + const check = createCommandPermissions(runtime, method); + registry.set(sessions(env), AsyncResult.success(grant(true))); + expect(registry.get(check.permissionAtom(env))).toBe(false); + registry.set( + sessions(env), + AsyncResult.success({ + ...grant(false), + scopes: [AuthSettingsWriteScope], + permissions: [AuthSettingsWriteScope], + }), + ); + expect(registry.get(check.permissionAtom(env))).toBe(true); + yield* check.authorize(registry, env); + }), + ), ); it.effect.each([WS_METHODS.scheduledTasksDelete, ORCHESTRATION_V2_WS_METHODS.stopWorkflow])( diff --git a/packages/client-runtime/src/state/server.ts b/packages/client-runtime/src/state/server.ts index f5c9d8f45b3db..b57f7b1d293ac 100644 --- a/packages/client-runtime/src/state/server.ts +++ b/packages/client-runtime/src/state/server.ts @@ -1604,6 +1604,10 @@ export function createServerEnvironmentAtoms( label: "environment-data:server:check-otlp-endpoint", tag: WS_METHODS.serverCheckOtlpEndpoint, }), + checkSecretSource: createEnvironmentRpcCommand(runtime, { + label: "environment-data:server:check-secret-source", + tag: WS_METHODS.serverCheckSecretSource, + }), retryResourceTelemetry: createEnvironmentRpcCommand(runtime, { label: "environment-data:server:retry-resource-telemetry", tag: WS_METHODS.serverRetryResourceTelemetry, diff --git a/packages/contracts/src/clientRpcPermissions.ts b/packages/contracts/src/clientRpcPermissions.ts index 91db78715551e..4ca4625b3391a 100644 --- a/packages/contracts/src/clientRpcPermissions.ts +++ b/packages/contracts/src/clientRpcPermissions.ts @@ -20,6 +20,7 @@ export const CLIENT_GUARDED_RPC_SCOPES = { [WS_METHODS.previewStopLocalServer]: AuthTerminalOperateScope, [WS_METHODS.serverRunStorageCleanup]: AuthSettingsWriteScope, [WS_METHODS.serverCheckOtlpEndpoint]: AuthSettingsWriteScope, + [WS_METHODS.serverCheckSecretSource]: AuthSettingsWriteScope, [WS_METHODS.serverRefreshProviders]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsRunAction]: AuthSourceControlWriteScope, [WS_METHODS.pullRequestsUpdate]: AuthSourceControlWriteScope, diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index 3106462d35784..c0b68e542ea00 100644 --- a/packages/contracts/src/rpc.ts +++ b/packages/contracts/src/rpc.ts @@ -38,6 +38,8 @@ import * as Rpc from "effect/rpc/Rpc"; import * as RpcGroup from "effect/rpc/RpcGroup"; import * as RpcMiddleware from "effect/rpc/RpcMiddleware"; import { NonNegativeInt, TrimmedNonEmptyString } from "./baseSchemas.ts"; +import { SecretSourceCheckResult } from "./settingsSecretValue.ts"; +import { SecretReference } from "./secretReference.ts"; import { ProviderAuthCancelInput, ProviderAuthCompleteInput, @@ -514,6 +516,7 @@ export const WS_METHODS = { peerLinksAnswerRequest: "peerLinks.answerRequest", serverUpdateSettings: "server.updateSettings", serverDiscoverSourceControl: "server.discoverSourceControl", + serverCheckSecretSource: "server.checkSecretSource", serverSearchAcpRegistry: "server.searchAcpRegistry", serverPrepareAcpRegistryAgent: "server.prepareAcpRegistryAgent", serverUninstallAcpRegistryManagedBinary: "server.uninstallAcpRegistryManagedBinary", @@ -837,6 +840,12 @@ const WsServerDiscoverSourceControlRpc = Rpc.make(WS_METHODS.serverDiscoverSourc error: EnvironmentAuthorizationError, }); +const WsServerCheckSecretSourceRpc = Rpc.make(WS_METHODS.serverCheckSecretSource, { + payload: Schema.Struct({ source: SecretReference }), + success: SecretSourceCheckResult, + error: EnvironmentAuthorizationError, +}); + const WsServerSearchAcpRegistryRpc = Rpc.make(WS_METHODS.serverSearchAcpRegistry, { payload: AcpRegistrySearchInput, success: AcpRegistrySearchResult, @@ -2033,6 +2042,7 @@ export const WsRpcGroup = RpcGroup.make( WsPeerLinksUnlinkRpc, WsPeerLinksAnswerRequestRpc, WsServerDiscoverSourceControlRpc, + WsServerCheckSecretSourceRpc, WsServerSearchAcpRegistryRpc, WsServerPrepareAcpRegistryAgentRpc, WsServerUninstallAcpRegistryManagedBinaryRpc, diff --git a/packages/contracts/src/settingsSecretValue.ts b/packages/contracts/src/settingsSecretValue.ts index 669c804013230..93d720ba6555d 100644 --- a/packages/contracts/src/settingsSecretValue.ts +++ b/packages/contracts/src/settingsSecretValue.ts @@ -121,3 +121,9 @@ export function withSettingsSecrets Date: Fri, 6 Nov 2026 01:27:22 +0000 Subject: [PATCH 2/7] test(secrets): takeover tests for the secret source check These are the takeover's tests for #31723. They replace the PR's SecretCheck tests, which mocked SecretResolver and ProviderRegistry by internal method name, with two tests through SecretCheck.check over the real SecretResolver, makeProviderInstanceRegistry and ProviderRegistry: - a readable secret resolves without returning its value, and rebuilds and refreshes only the instance that reads it, from the check's one read (no second read, the unrelated secret is not re-read) - an unreadable secret fails naming its provider and leaves instances alone The commandPermissions row for server.checkSecretSource stays. --- apps/server/src/secrets/SecretCheck.test.ts | 323 ++++++++++++-------- 1 file changed, 189 insertions(+), 134 deletions(-) diff --git a/apps/server/src/secrets/SecretCheck.test.ts b/apps/server/src/secrets/SecretCheck.test.ts index f5aa6a847ae15..b73db133c6857 100644 --- a/apps/server/src/secrets/SecretCheck.test.ts +++ b/apps/server/src/secrets/SecretCheck.test.ts @@ -7,164 +7,219 @@ import { SecretProviderId, SecretProviderKind, type SecretReference, + type ServerProvider, } from "@t3tools/contracts"; import type { AnyProviderDriver, ProviderInstance } from "@t3tools/provider-core/server/driver"; +import { makeManualOnlyProviderMaintenanceCapabilities } from "@t3tools/provider-core/server/maintenanceResolver"; +import * as ProviderLatestVersions from "@t3tools/provider-core/server/ProviderLatestVersions"; import * as SecretResolver from "@t3tools/secret-provider-core/server/SecretResolver"; import { defineSecretProviderDriver } from "@t3tools/secret-provider-core/server/driver"; import { defineSecretProvider } from "@t3tools/secret-provider-core/shared/definition"; import * as Effect from "effect/Effect"; +import { HttpClient, HttpClientResponse } from "effect/http"; import * as Layer from "effect/Layer"; import * as PubSub from "effect/PubSub"; import * as Schema from "effect/Schema"; +import * as Stream from "effect/Stream"; -import { makeProviderInstanceRegistry } from "../provider/ProviderInstanceRegistry.ts"; +import * as ServerConfig from "../config.ts"; +import * as ModelManifest from "../provider/ModelManifest.ts"; +import * as ProviderInstanceRegistry from "../provider/ProviderInstanceRegistry.ts"; +import * as ProviderInstanceRegistryMutator from "../provider/ProviderInstanceRegistryMutator.ts"; import * as ProviderRegistry from "../provider/ProviderRegistry.ts"; import * as SecretCheck from "./SecretCheck.ts"; -const reference: SecretReference = { - provider: SecretProviderId.make("work"), - reference: "op://Private/OpenAI/api-key", -}; - -const run = (value: string | undefined) => - Effect.gen(function* () { - const calls: Array = []; - const layer = SecretCheck.layer.pipe( - Layer.provide( - Layer.mergeAll( - Layer.mock(SecretResolver.SecretResolver)({ - invalidate: () => Effect.sync(() => void calls.push("invalidate")), - read: () => Effect.sync(() => (calls.push("read"), [value])), - }), - Layer.mock(ProviderRegistry.ProviderRegistry)({ - applySecretAndRefresh: () => Effect.sync(() => (calls.push("apply"), [])), - }), - ), - ), - ); - const result = yield* SecretCheck.SecretCheck.use((check) => check.check(reference)).pipe( - Effect.provide(layer), - ); - return { result, calls }; - }); - -it.effect( - "a readable secret resolves without returning its value and applies it to providers", - () => - Effect.gen(function* () { - const { result, calls } = yield* run("sk-live-value"); - assert.deepStrictEqual(result, { _tag: "Resolved" }); - assert.deepStrictEqual(calls, ["invalidate", "read", "apply"]); - assert.notInclude(JSON.stringify(result), "sk-live-value"); - }), -); +const FAKE_KIND = SecretProviderKind.make("fake"); +const secretProvider = SecretProviderId.make("personal"); +const wanted: SecretReference = { provider: secretProvider, reference: "fake://wanted" }; +const other: SecretReference = { provider: secretProvider, reference: "fake://other" }; +const driverKind = ProviderDriverKind.make("fake-provider"); +const dependent = ProviderInstanceId.make("dependent"); +const unrelated = ProviderInstanceId.make("unrelated"); -it.effect("an unreadable secret fails with the provider named and applies nothing", () => - Effect.gen(function* () { - const { result, calls } = yield* run(undefined); - assert.strictEqual(result._tag, "Failed"); - if (result._tag === "Failed") assert.include(result.detail, '"work"'); - assert.deepStrictEqual(calls, ["invalidate", "read"]); - }), -); +const providerSnapshot = (instanceId: ProviderInstanceId): ServerProvider => ({ + instanceId, + driver: driverKind, + status: "ready", + enabled: true, + installed: true, + auth: { status: "authenticated" }, + checkedAt: "2026-10-03T00:00:00.000Z", + version: "1.0.0", + models: [], + slashCommands: [], + skills: [], +}); -const FAKE_KIND = SecretProviderKind.make("fake"); +/** + * A server whose `dependent` instance reads `wanted` and `unrelated` reads `other`, both through + * the real resolver, instance registry and provider registry. `vault` answers each read of a + * reference with the next queued value, repeating the last one. + */ +const setup = Effect.fn("SecretCheckTest.setup")(function* ( + vault: Map>, +) { + const reads: Array = []; + const created: Array<{ instanceId: ProviderInstanceId; secret: unknown }> = []; + const refreshed: Array = []; -it.effect( - "applies a check to the instances that read the secret, whatever a second read would return", - () => - Effect.gen(function* () { - const store = new Map>(); - const reads: Array = []; - const secretDriver = defineSecretProviderDriver({ - kind: FAKE_KIND, - definition: defineSecretProvider({ - kind: FAKE_KIND, - label: "Fake", - configSchema: Schema.Struct({}), - isReference: (value) => value.startsWith("fake://"), - referencePlaceholder: "fake://item", - invalidReferenceMessage: "Use fake://item.", + const secretDriver = defineSecretProviderDriver({ + kind: FAKE_KIND, + definition: defineSecretProvider({ + kind: FAKE_KIND, + label: "Fake", + configSchema: Schema.Struct({}), + isReference: (value) => value.startsWith("fake://"), + referencePlaceholder: "fake://item", + invalidReferenceMessage: "Use fake://item.", + }), + read: (_config, references) => + Effect.sync(() => + references.map((reference) => { + reads.push(reference); + const answers = vault.get(reference) ?? []; + return answers.length > 1 ? answers.shift() : answers[0]; }), - read: (_config, references) => - Effect.sync(() => - references.map((reference) => { - reads.push(reference); - const answers = store.get(reference) ?? []; - return answers.length > 1 ? answers.shift() : answers[0]; + ), + }); + + const driver: AnyProviderDriver = { + driverKind, + metadata: { displayName: "Fake" }, + configSchema: Schema.Unknown, + defaultConfig: () => ({}), + create: (input) => + Effect.sync(() => { + created.push({ instanceId: input.instanceId, secret: input.environment[0]?.value }); + const snapshot = providerSnapshot(input.instanceId); + return { + instanceId: input.instanceId, + driverKind, + continuationIdentity: { driverKind, continuationKey: `fake:${input.instanceId}` }, + displayName: undefined, + enabled: input.enabled, + snapshot: { + resolveMaintenance: () => + Effect.succeed( + makeManualOnlyProviderMaintenanceCapabilities({ + provider: driverKind, + packageName: null, + }), + ), + getSnapshot: Effect.succeed(snapshot), + refresh: Effect.sync(() => { + refreshed.push(input.instanceId); + return snapshot; }), - ), - }); - const provider = SecretProviderId.make("personal"); - const wanted: SecretReference = { provider, reference: "fake://wanted" }; - const other: SecretReference = { provider, reference: "fake://other" }; - store.set(wanted.reference, [undefined]); - store.set(other.reference, ["other-key"]); + streamChanges: Stream.empty, + applyUsageLimits: () => Effect.void, + }, + orchestrationAdapter: {} as ProviderInstance["orchestrationAdapter"], + textGeneration: {} as ProviderInstance["textGeneration"], + } satisfies ProviderInstance; + }), + }; + const instanceConfig = (secret: SecretReference) => ({ + driver: driverKind, + config: {}, + environment: [{ name: "API_KEY", value: secret, sensitive: true }], + }); - const driverKind = ProviderDriverKind.make("fake-provider"); - const dependent = ProviderInstanceId.make("dependent"); - const unrelated = ProviderInstanceId.make("unrelated"); - const driver: AnyProviderDriver = { - driverKind, - metadata: { displayName: "Fake" }, - configSchema: Schema.Unknown, - defaultConfig: () => ({}), - create: (input) => - Effect.succeed({ - instanceId: input.instanceId, - enabled: input.enabled, - } as ProviderInstance), - }; - const instanceConfig = (secret: SecretReference) => ({ - driver: driverKind, - config: {}, - environment: [{ name: "API_KEY", value: secret, sensitive: true }], - }); + const resolverContext = yield* Layer.build( + SecretResolver.layer({ + drivers: [secretDriver], + secretProviders: Effect.succeed({ + [secretProvider]: { kind: FAKE_KIND, label: "Personal", config: {} }, + }), + }), + ); + const { registry, mutator } = yield* ProviderInstanceRegistry.makeProviderInstanceRegistry({ + drivers: [driver], + configMap: { [dependent]: instanceConfig(wanted), [unrelated]: instanceConfig(other) }, + }).pipe(Effect.provide(resolverContext)); + // Secrets are read after startup, so wait for the change that applies them. + const changes = yield* registry.subscribeChanges; + yield* PubSub.take(changes); - const resolverLayer = SecretResolver.layer({ - drivers: [secretDriver], - secretProviders: Effect.succeed({ - [provider]: { kind: FAKE_KIND, label: "Personal", config: {} }, - }), - }).pipe(Layer.provide(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer))); + const services = yield* Layer.build( + SecretCheck.layer.pipe( + Layer.provideMerge(ProviderRegistry.layer), + Layer.provide( + Layer.mergeAll( + Layer.succeedContext(resolverContext), + Layer.succeed(ProviderInstanceRegistry.ProviderInstanceRegistry, registry), + Layer.succeed(ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutator, mutator), + ), + ), + ), + ); + const secretCheck = yield* SecretCheck.SecretCheck.pipe(Effect.provide(services)); + return { secretCheck, registry, reads, created, refreshed }; +}); - yield* Effect.gen(function* () { - const resolver = yield* SecretResolver.SecretResolver; - const { registry, mutator } = yield* makeProviderInstanceRegistry({ - drivers: [driver], - configMap: { [dependent]: instanceConfig(wanted), [unrelated]: instanceConfig(other) }, - }); - const changes = yield* registry.subscribeChanges; - yield* PubSub.take(changes); +const layerTest = Layer.mergeAll( + ServerConfig.layerTest(process.cwd(), { prefix: "t3-secret-check-" }), + ModelManifest.layerTest, + ProviderLatestVersions.layer, + Layer.succeed( + HttpClient.HttpClient, + HttpClient.make((request) => + Effect.succeed(HttpClientResponse.fromWeb(request, Response.json({ version: "0.0.0" }))), + ), + ), +).pipe(Layer.provideMerge(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer))); + +it.layer(layerTest)("SecretCheck", (it) => { + it.effect( + "a readable secret rebuilds and refreshes only the instances that read it, from that one read", + () => + Effect.gen(function* () { + const vault = new Map>([ + [wanted.reference, [undefined]], + [other.reference, ["other-key"]], + ]); + const server = yield* setup(vault); assert.deepStrictEqual( - (yield* registry.listUnavailable).map((entry) => entry.instanceId), + (yield* server.registry.listUnavailable).map((entry) => entry.instanceId), [dependent], ); + const createdBefore = server.created.length; + const readsBefore = server.reads.length; - // Unlocked for the check's read only: any later read of it would fail again. - store.set(wanted.reference, ["wanted-key", undefined]); - const readsBefore = reads.length; - const result = yield* SecretCheck.SecretCheck.use((check) => check.check(wanted)).pipe( - Effect.provide( - SecretCheck.layer.pipe( - Layer.provide( - Layer.mock(ProviderRegistry.ProviderRegistry)({ - applySecretAndRefresh: (reference) => - mutator.applySecret(reference).pipe(Effect.as([])), - }), - ), - Layer.provideMerge(Layer.succeed(SecretResolver.SecretResolver, resolver)), - ), - ), - ); + // Unlocked for the check's own read only: any second read would fail again. + vault.set(wanted.reference, ["wanted-key", undefined]); + const result = yield* server.secretCheck.check(wanted); assert.deepStrictEqual(result, { _tag: "Resolved" }); - assert.deepStrictEqual( - (yield* registry.listInstances).map((instance) => instance.instanceId).sort(), - [dependent, unrelated], - ); - assert.deepStrictEqual(yield* registry.listUnavailable, []); - assert.deepStrictEqual(reads.slice(readsBefore), [wanted.reference]); - }).pipe(Effect.provide(resolverLayer), Effect.scoped); - }), -); + assert.notInclude(JSON.stringify(result), "wanted-key"); + assert.deepStrictEqual(server.reads.slice(readsBefore), [wanted.reference]); + assert.deepStrictEqual(server.created.slice(createdBefore), [ + { instanceId: dependent, secret: "wanted-key" }, + ]); + assert.deepStrictEqual(server.refreshed, [dependent]); + assert.deepStrictEqual(yield* server.registry.listUnavailable, []); + }).pipe(Effect.scoped), + ); + + it.effect("an unreadable secret fails naming its provider and leaves instances alone", () => + Effect.gen(function* () { + const vault = new Map>([ + [wanted.reference, [undefined]], + [other.reference, ["other-key"]], + ]); + const server = yield* setup(vault); + const createdBefore = server.created.length; + + const result = yield* server.secretCheck.check(wanted); + + assert.strictEqual(result._tag, "Failed"); + if (result._tag === "Failed") assert.include(result.detail, secretProvider); + assert.deepStrictEqual(server.created.slice(createdBefore), []); + assert.deepStrictEqual(server.refreshed, []); + assert.deepStrictEqual( + (yield* server.registry.listUnavailable).map((entry) => entry.instanceId), + [dependent], + ); + }).pipe(Effect.scoped), + ); +}); From 0dbb797e0987282abd3c980da5dc04fb6d1cc35d Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 6 Nov 2026 01:33:04 +0000 Subject: [PATCH 3/7] fix(secrets): every secret reference field offers the check Source control tokens, Simlock device host tokens and a new local ACP provider's environment now pass their environment, so the check button shows there too. An empty secret reads as unresolved, as it does for settings secrets. Users learn about the check in the secrets guide. --- apps/server/src/secrets/SecretCheck.ts | 2 +- .../src/components/settings/AddProviderInstanceDialog.tsx | 1 + apps/web/src/components/settings/SecretSourceInput.tsx | 6 +++--- .../web/src/components/settings/SimlockDeviceHostEditor.tsx | 1 + .../src/components/settings/SourceControlTokenSettings.tsx | 1 + docs/user/secrets.md | 2 ++ 6 files changed, 9 insertions(+), 4 deletions(-) diff --git a/apps/server/src/secrets/SecretCheck.ts b/apps/server/src/secrets/SecretCheck.ts index bce705b05afa6..b7aee618ac40f 100644 --- a/apps/server/src/secrets/SecretCheck.ts +++ b/apps/server/src/secrets/SecretCheck.ts @@ -24,7 +24,7 @@ const make = Effect.gen(function* () { const check = Effect.fn("SecretCheck.check")(function* (reference: SecretReference) { yield* resolver.invalidate([reference]); const [value] = yield* resolver.read([reference]); - if (value === undefined) { + if (value === undefined || value.trim() === "") { return { _tag: "Failed", detail: `Could not read this secret from "${reference.provider}". Check the provider's settings and that its store is unlocked.`, diff --git a/apps/web/src/components/settings/AddProviderInstanceDialog.tsx b/apps/web/src/components/settings/AddProviderInstanceDialog.tsx index 9b1829879405f..30c0e48654eed 100644 --- a/apps/web/src/components/settings/AddProviderInstanceDialog.tsx +++ b/apps/web/src/components/settings/AddProviderInstanceDialog.tsx @@ -548,6 +548,7 @@ export function AddProviderInstanceDialog({ environment={localEnvironment} onChange={setLocalEnvironment} secretSources={secretSources} + environmentId={environmentId} /> ) : null} diff --git a/apps/web/src/components/settings/SecretSourceInput.tsx b/apps/web/src/components/settings/SecretSourceInput.tsx index 7bd85809cc348..7d2dfb51e1a11 100644 --- a/apps/web/src/components/settings/SecretSourceInput.tsx +++ b/apps/web/src/components/settings/SecretSourceInput.tsx @@ -181,11 +181,11 @@ function SecretCheckButton(props: { {state?.status === "checking" ? ( ) : state?.status === "resolved" ? ( - + ) : state?.status === "failed" ? ( - + ) : ( - + )} diff --git a/apps/web/src/components/settings/SimlockDeviceHostEditor.tsx b/apps/web/src/components/settings/SimlockDeviceHostEditor.tsx index 9037508b2790e..4f2a3f1a7f855 100644 --- a/apps/web/src/components/settings/SimlockDeviceHostEditor.tsx +++ b/apps/web/src/components/settings/SimlockDeviceHostEditor.tsx @@ -311,6 +311,7 @@ function SimlockTokenField(props: { : "" } sources={sources} + environmentId={props.environmentId} onValidityChange={props.onValidityChange} onSecretChange={(secret) => props.onChange( diff --git a/apps/web/src/components/settings/SourceControlTokenSettings.tsx b/apps/web/src/components/settings/SourceControlTokenSettings.tsx index 5a78b5f28287d..2ce1c75414b49 100644 --- a/apps/web/src/components/settings/SourceControlTokenSettings.tsx +++ b/apps/web/src/components/settings/SourceControlTokenSettings.tsx @@ -130,6 +130,7 @@ export function SourceControlTokenSettings({ configured={isSaved && !isSecretReference(savedToken)} value={draft} sources={secretSources} + environmentId={environmentId} onChange={(event) => setPending({ key: sourceKey, draft: event.target.value })} onSecretChange={(secret) => setPending({ key: sourceKey, draft: secret ?? "" })} /> diff --git a/docs/user/secrets.md b/docs/user/secrets.md index fb1256f323002..13e449d072216 100644 --- a/docs/user/secrets.md +++ b/docs/user/secrets.md @@ -25,6 +25,8 @@ Credential fields in Settings, such as provider environment variables and creden control tokens, usage limit keys, and device host tokens, show a source picker next to the field. Pick **Value** to save the credential itself, or pick a secret provider and enter a reference such as `op://vault/item/field`. Copy the reference from 1Password with **Copy Secret Reference**. +Select the check button next to a reference to read it now. If it reads, providers that use it +start using the new value. T3 Code reads the secret when it needs it, for example when a provider starts. Change the credential in 1Password and refresh to use the new value. From b7568770042cd0883231f55d78b08de37d624911 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 6 Nov 2026 01:46:58 +0000 Subject: [PATCH 4/7] fix(secrets): a failed secret check keeps the value instances already read The check invalidated the reference before reading it, so a locked vault replaced a working cached value with a failure, and a later rebuild of an instance that also reads that secret was skipped. The check now reads with `fresh`, which stores a readable answer and leaves a held value in place otherwise. The failure names the secret provider's label, the check button hides on servers without the `secretSourceCheck` capability, and it hides while the field shows a rejected reference. --- .../src/environment/ServerEnvironment.ts | 1 + apps/server/src/secrets/SecretCheck.test.ts | 54 +++++++++++++++---- apps/server/src/secrets/SecretCheck.ts | 5 +- .../components/settings/SecretSourceInput.tsx | 10 +++- packages/contracts/src/environment.ts | 2 + .../src/server/SecretResolver.ts | 25 +++++++-- 6 files changed, 78 insertions(+), 19 deletions(-) diff --git a/apps/server/src/environment/ServerEnvironment.ts b/apps/server/src/environment/ServerEnvironment.ts index 3e780298d70a6..561da20bbe56e 100644 --- a/apps/server/src/environment/ServerEnvironment.ts +++ b/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, diff --git a/apps/server/src/secrets/SecretCheck.test.ts b/apps/server/src/secrets/SecretCheck.test.ts index b73db133c6857..f71baf24d73b4 100644 --- a/apps/server/src/secrets/SecretCheck.test.ts +++ b/apps/server/src/secrets/SecretCheck.test.ts @@ -33,6 +33,7 @@ const FAKE_KIND = SecretProviderKind.make("fake"); const secretProvider = SecretProviderId.make("personal"); const wanted: SecretReference = { provider: secretProvider, reference: "fake://wanted" }; const other: SecretReference = { provider: secretProvider, reference: "fake://other" }; +const locked: SecretReference = { provider: secretProvider, reference: "fake://locked" }; const driverKind = ProviderDriverKind.make("fake-provider"); const dependent = ProviderInstanceId.make("dependent"); const unrelated = ProviderInstanceId.make("unrelated"); @@ -52,15 +53,16 @@ const providerSnapshot = (instanceId: ProviderInstanceId): ServerProvider => ({ }); /** - * A server whose `dependent` instance reads `wanted` and `unrelated` reads `other`, both through - * the real resolver, instance registry and provider registry. `vault` answers each read of a - * reference with the next queued value, repeating the last one. + * A server whose `dependent` instance reads `dependentReads` and `unrelated` reads `other`, all + * through the real resolver, instance registry and provider registry. `vault` answers each read of + * a reference with the next queued value, repeating the last one. */ const setup = Effect.fn("SecretCheckTest.setup")(function* ( vault: Map>, + dependentReads: ReadonlyArray = [wanted], ) { const reads: Array = []; - const created: Array<{ instanceId: ProviderInstanceId; secret: unknown }> = []; + const created: Array<{ instanceId: ProviderInstanceId; secrets: ReadonlyArray }> = []; const refreshed: Array = []; const secretDriver = defineSecretProviderDriver({ @@ -90,7 +92,10 @@ const setup = Effect.fn("SecretCheckTest.setup")(function* ( defaultConfig: () => ({}), create: (input) => Effect.sync(() => { - created.push({ instanceId: input.instanceId, secret: input.environment[0]?.value }); + created.push({ + instanceId: input.instanceId, + secrets: input.environment.map((variable) => variable.value), + }); const snapshot = providerSnapshot(input.instanceId); return { instanceId: input.instanceId, @@ -119,10 +124,14 @@ const setup = Effect.fn("SecretCheckTest.setup")(function* ( } satisfies ProviderInstance; }), }; - const instanceConfig = (secret: SecretReference) => ({ + const instanceConfig = (secrets: ReadonlyArray) => ({ driver: driverKind, config: {}, - environment: [{ name: "API_KEY", value: secret, sensitive: true }], + environment: secrets.map((secret, index) => ({ + name: `API_KEY_${index}`, + value: secret, + sensitive: true, + })), }); const resolverContext = yield* Layer.build( @@ -135,7 +144,10 @@ const setup = Effect.fn("SecretCheckTest.setup")(function* ( ); const { registry, mutator } = yield* ProviderInstanceRegistry.makeProviderInstanceRegistry({ drivers: [driver], - configMap: { [dependent]: instanceConfig(wanted), [unrelated]: instanceConfig(other) }, + configMap: { + [dependent]: instanceConfig(dependentReads), + [unrelated]: instanceConfig([other]), + }, }).pipe(Effect.provide(resolverContext)); // Secrets are read after startup, so wait for the change that applies them. const changes = yield* registry.subscribeChanges; @@ -194,7 +206,7 @@ it.layer(layerTest)("SecretCheck", (it) => { assert.notInclude(JSON.stringify(result), "wanted-key"); assert.deepStrictEqual(server.reads.slice(readsBefore), [wanted.reference]); assert.deepStrictEqual(server.created.slice(createdBefore), [ - { instanceId: dependent, secret: "wanted-key" }, + { instanceId: dependent, secrets: ["wanted-key"] }, ]); assert.deepStrictEqual(server.refreshed, [dependent]); assert.deepStrictEqual(yield* server.registry.listUnavailable, []); @@ -213,7 +225,7 @@ it.layer(layerTest)("SecretCheck", (it) => { const result = yield* server.secretCheck.check(wanted); assert.strictEqual(result._tag, "Failed"); - if (result._tag === "Failed") assert.include(result.detail, secretProvider); + if (result._tag === "Failed") assert.include(result.detail, "Personal"); assert.deepStrictEqual(server.created.slice(createdBefore), []); assert.deepStrictEqual(server.refreshed, []); assert.deepStrictEqual( @@ -222,4 +234,26 @@ it.layer(layerTest)("SecretCheck", (it) => { ); }).pipe(Effect.scoped), ); + + it.effect("a failed check keeps the value an instance already reads", () => + Effect.gen(function* () { + const vault = new Map>([ + [wanted.reference, ["wanted-old"]], + [locked.reference, ["locked-key", undefined]], + [other.reference, ["other-key"]], + ]); + const server = yield* setup(vault, [wanted, locked]); + const readsBefore = server.reads.length; + + assert.strictEqual((yield* server.secretCheck.check(locked))._tag, "Failed"); + vault.set(wanted.reference, ["wanted-new"]); + assert.deepStrictEqual(yield* server.secretCheck.check(wanted), { _tag: "Resolved" }); + + assert.deepStrictEqual(server.reads.slice(readsBefore), [locked.reference, wanted.reference]); + assert.deepStrictEqual(server.created.at(-1), { + instanceId: dependent, + secrets: ["wanted-new", "locked-key"], + }); + }).pipe(Effect.scoped), + ); }); diff --git a/apps/server/src/secrets/SecretCheck.ts b/apps/server/src/secrets/SecretCheck.ts index b7aee618ac40f..74802195d1284 100644 --- a/apps/server/src/secrets/SecretCheck.ts +++ b/apps/server/src/secrets/SecretCheck.ts @@ -22,12 +22,11 @@ const make = Effect.gen(function* () { const providerRegistry = yield* ProviderRegistry.ProviderRegistry; const check = Effect.fn("SecretCheck.check")(function* (reference: SecretReference) { - yield* resolver.invalidate([reference]); - const [value] = yield* resolver.read([reference]); + const [value] = yield* resolver.read([reference], { fresh: true }); if (value === undefined || value.trim() === "") { return { _tag: "Failed", - detail: `Could not read this secret from "${reference.provider}". Check the provider's settings and that its store is unlocked.`, + detail: `Could not read this secret from "${yield* resolver.label(reference.provider)}". Check the provider's settings and that its store is unlocked.`, } as const; } yield* providerRegistry.applySecretAndRefresh(reference); diff --git a/apps/web/src/components/settings/SecretSourceInput.tsx b/apps/web/src/components/settings/SecretSourceInput.tsx index 7d2dfb51e1a11..27ac4af301231 100644 --- a/apps/web/src/components/settings/SecretSourceInput.tsx +++ b/apps/web/src/components/settings/SecretSourceInput.tsx @@ -127,6 +127,10 @@ function SecretCheckButton(props: { readonly label: string; }) { const { environmentId, secret } = props; + const supported = useAtomValue( + serverEnvironment.configValueAtom(environmentId), + (config) => config?.environment.capabilities.secretSourceCheck === true, + ); const canCheck = useAtomValue(serverEnvironment.checkSecretSource.permissionAtom(environmentId)); const checkSecretSource = useAtomCommand(serverEnvironment.checkSecretSource); const secretKey = `${secret.provider}\u0000${secret.reference}`; @@ -150,6 +154,7 @@ function SecretCheckButton(props: { ); }; + if (!supported) return null; const description = !canCheck ? "You cannot check secrets on this environment" : state === undefined @@ -290,7 +295,10 @@ export function SecretSourceInput({ aria-label={`${label} reference`} aria-invalid={rejectedReference !== null} /> - {environmentId !== undefined && secret !== undefined && pendingProvider === null ? ( + {environmentId !== undefined && + secret !== undefined && + pendingProvider === null && + rejectedReference === null ? ( , + options?: { readonly fresh?: boolean }, ) => Effect.Effect>; /** Forgets the given references, or every reference, so the next read asks the provider again. */ readonly invalidate: (references?: ReadonlyArray) => Effect.Effect; @@ -90,12 +95,14 @@ export const make = Effect.fn("SecretResolver.make")(function* (options: { // Cache hits answer without the lock, so a read waiting on an unlock never stalls a cached one. const read: SecretResolver["Service"]["read"] = Effect.fn("SecretResolver.read")(function* ( references, + readOptions, ) { + const fresh = readOptions?.fresh === true; const providers = yield* options.secretProviders; const plans = references.map((reference) => planFor(providers, reference)); const isMissing = (reference: SecretReference, index: number) => { const plan = plans[index]; - return plan !== undefined && !isCached(reference, plan); + return plan !== undefined && (fresh || !isCached(reference, plan)); }; if (!references.some(isMissing)) return answer(references, plans); @@ -118,15 +125,23 @@ export const make = Effect.fn("SecretResolver.make")(function* (options: { ]); wanted.set(reference.provider, group); }); + const answered = new Map(); for (const { plan, byReference } of wanted.values()) { const values = yield* plan.driver.read(plan.config, [...byReference.keys()]); [...byReference.values()].forEach((sameReference, index) => { + const value = values[index]; + const readable = value !== undefined && value.trim() !== ""; for (const reference of sameReference) { - cache.set(cacheKey(reference), { config: plan.configKey, value: values[index] }); + answered.set(cacheKey(reference), value); + if (!fresh || readable || !isCached(reference, plan)) { + cache.set(cacheKey(reference), { config: plan.configKey, value }); + } } }); } - return answer(references, plans); + return fresh + ? references.map((reference) => answered.get(cacheKey(reference))) + : answer(references, plans); }), ); }, Effect.provideContext(context)); From 49eb332acb7e201927fcf4b4024d949c88fa1fa1 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Thu, 5 Nov 2026 18:06:01 -0500 Subject: [PATCH 5/7] feat(secrets): check a secret reference before saving it Settings gains a check button beside a secret reference. It reads the secret fresh from its provider and says whether it resolved, without sending the value to the client. A readable secret also rebuilds the provider instances that depend on it. Signed-off-by: Yordis Prieto --- .../src/observability/RpcInstrumentation.ts | 1 + .../src/provider/ProviderInstanceRegistry.ts | 165 ++++++++--- .../ProviderInstanceRegistryHydration.test.ts | 1 + .../ProviderInstanceRegistryMutator.ts | 13 +- apps/server/src/provider/ProviderRegistry.ts | 16 + .../providerMaintenanceRunner.test.ts | 1 + .../testUtils/providerRegistryMock.ts | 1 + apps/server/src/secrets/SecretCheck.test.ts | 276 ++++++++++++++++++ apps/server/src/secrets/SecretCheck.ts | 40 +++ apps/server/src/server.ts | 2 + apps/server/src/ws.test.ts | 2 + apps/server/src/ws.ts | 3 + .../settings/AddUsageLimitSourceDialog.tsx | 1 + .../settings/EasDeviceHostEditor.tsx | 1 + .../settings/ProviderInstanceCard.tsx | 6 + .../settings/ProviderSettingsForm.tsx | 1 + .../components/settings/SecretSourceInput.tsx | 162 ++++++++-- .../settings/SimlockDeviceHostEditor.tsx | 1 + .../settings/SourceControlTokenSettings.tsx | 1 + .../src/state/commandPermissions.test.ts | 40 +-- packages/client-runtime/src/state/server.ts | 4 + .../contracts/src/clientRpcPermissions.ts | 1 + packages/contracts/src/rpc.ts | 10 + packages/contracts/src/settingsSecretValue.ts | 6 + 24 files changed, 662 insertions(+), 93 deletions(-) create mode 100644 apps/server/src/secrets/SecretCheck.test.ts create mode 100644 apps/server/src/secrets/SecretCheck.ts diff --git a/apps/server/src/observability/RpcInstrumentation.ts b/apps/server/src/observability/RpcInstrumentation.ts index 89f93a9df2c81..ba6b8a0905ca5 100644 --- a/apps/server/src/observability/RpcInstrumentation.ts +++ b/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", diff --git a/apps/server/src/provider/ProviderInstanceRegistry.ts b/apps/server/src/provider/ProviderInstanceRegistry.ts index 3f4df0eb35d18..d858c176cec7f 100644 --- a/apps/server/src/provider/ProviderInstanceRegistry.ts +++ b/apps/server/src/provider/ProviderInstanceRegistry.ts @@ -130,6 +130,39 @@ 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 = ( + driver: AnyProviderDriver | undefined, + entry: ProviderInstanceConfig, + current: InstanceSecrets, + checked: SecretReference, + value: string | undefined, +): InstanceSecrets => { + if (value === undefined || value.trim() === "") return current; + const matches = (reference: SecretReference) => + reference.provider === checked.provider && reference.reference === checked.reference; + // 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 replacedFields = new Set( + configSecretReferences(driver, entry).flatMap(({ field, reference }) => + matches(reference) ? [field] : [], + ), + ); + 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) => + replacedFields.has(slot.field) ? { ...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 +194,7 @@ const buildEntry = Effect.fn("ProviderInstanceRegistry.buildEntry")(function* { - 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 +277,14 @@ const buildEntry = Effect.fn("ProviderInstanceRegistry.buildEntry")(function* 0) { @@ -311,9 +355,10 @@ const makeReconcile = (input: { configMap: ProviderInstanceConfigMap, rebuild?: ReadonlySet, deferSecrets?: boolean, + carried?: ReadonlyMap, ) => Effect.Effect) => { 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 +410,7 @@ const makeReconcile = (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 +452,7 @@ const makeReconcile = (input: { rawInstanceId, entry, deferSecrets, + carriedSecrets: carried.get(instanceId), }); if (result.kind === "live") { builtEntries.set(instanceId, result.live); @@ -503,44 +550,79 @@ 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; - 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(); - for (const [rawInstanceId, entry] of targets) { - const current = live.get(ProviderInstanceId.make(rawInstanceId)); - if (current === undefined) { - rebuild.add(ProviderInstanceId.make(rawInstanceId)); - continue; - } - const { secrets, unreadable } = yield* readInstanceSecrets( + // not restart providers whose credentials are the same. a missing `checked` reference drops + // the cached values first; with one, only that value is replaced and the rest are carried over. + const reloadWhere = Effect.fn("ProviderInstanceRegistry.reloadWhere")( + function* ( + select: (rawInstanceId: string, entry: ProviderInstanceConfig) => boolean, + checked?: SecretReference, + ) { + 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 []; + const resolver = yield* SecretResolver.SecretResolver; + const references = targets.flatMap(([, entry]) => secretReferencesOf(driversById, entry)); + if (checked === undefined) yield* resolver.invalidate(references); + const [checkedValue] = yield* resolver.read(checked === undefined ? references : [checked]); + const live = yield* Ref.get(entries); + const rebuild = new Set(); + const carried = new Map(); + for (const [rawInstanceId, entry] of targets) { + const current = live.get(ProviderInstanceId.make(rawInstanceId)); + if (current === undefined) { + rebuild.add(ProviderInstanceId.make(rawInstanceId)); + continue; + } + if (checked !== undefined) { + const secrets = carryCheckedSecret( driversById.get(entry.driver), - entry, + current.entry, + current.secrets, + checked, + checkedValue, ); - // 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) => + reloadWhere( + (_, entry) => + secretReferencesOf(driversById, entry).some( + (candidate) => + candidate.provider === reference.provider && + candidate.reference === reference.reference, + ), + reference, + ); // Reading secrets can wait on a vault unlock, so the server does not wait for it to start. yield* semaphore.withPermits(1)( @@ -592,6 +674,7 @@ export const makeProviderInstanceRegistry = Effect.fn( const mutator: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape = { reconcile, reloadSecrets, + applySecret, }; return { registry, mutator }; diff --git a/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts b/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts index dc44f0324a6b0..02bc92f4b4504 100644 --- a/apps/server/src/provider/ProviderInstanceRegistryHydration.test.ts +++ b/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)), diff --git a/apps/server/src/provider/ProviderInstanceRegistryMutator.ts b/apps/server/src/provider/ProviderInstanceRegistryMutator.ts index 35eb52d3f2906..1d6324d97b98a 100644 --- a/apps/server/src/provider/ProviderInstanceRegistryMutator.ts +++ b/apps/server/src/provider/ProviderInstanceRegistryMutator.ts @@ -1,5 +1,9 @@ /** 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"; @@ -7,6 +11,13 @@ export interface ProviderInstanceRegistryMutatorShape { readonly reconcile: (configMap: ProviderInstanceConfigMap) => Effect.Effect; /** Re-reads the secrets of every instance, or one, and rebuilds those whose secrets changed. */ readonly reloadSecrets: (instanceId?: ProviderInstanceId) => Effect.Effect; + /** + * Rebuilds the instances that read `reference`, from the value its resolver already holds, and + * returns them. Nothing else is read. + */ + readonly applySecret: ( + reference: SecretReference, + ) => Effect.Effect>; } export class ProviderInstanceRegistryMutator extends Context.Service< diff --git a/apps/server/src/provider/ProviderRegistry.ts b/apps/server/src/provider/ProviderRegistry.ts index fb6aeaac26b63..6ced8c4f6f971 100644 --- a/apps/server/src/provider/ProviderRegistry.ts +++ b/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,11 @@ export class ProviderRegistry extends Context.Service< instanceId?: ProviderInstanceId, ) => Effect.Effect>; + /** Rebuilds and refreshes only the instances that read a secret the caller just read. */ + readonly applySecretAndRefresh: ( + reference: SecretReference, + ) => Effect.Effect>; + /** Only refreshed instances can verify an update; the published list still includes every row. */ readonly refreshAfterUpdate: ( provider: ProviderDriverKind, @@ -692,6 +698,14 @@ export const layer = Layer.effect( return yield* instanceId === undefined ? refresh() : refreshInstance(instanceId); }); + const applySecretAndRefresh = Effect.fn("applySecretAndRefresh")(function* ( + reference: SecretReference, + ) { + const instanceIds = yield* instanceRegistryMutator.applySecret(reference); + yield* Effect.forEach(instanceIds, refreshInstance, { discard: true }); + return yield* Ref.get(providersRef); + }); + const refreshAfterUpdate = Effect.fn("refreshAfterUpdate")(function* ( provider: ProviderDriverKind, selectedInstanceId: ProviderInstanceId, @@ -1077,6 +1091,8 @@ export const layer = Layer.effect( refreshInstance(instanceId).pipe(Effect.catchCause(recoverRefreshFailure)), reloadSecretsAndRefresh: (instanceId?: ProviderInstanceId) => reloadSecretsAndRefresh(instanceId).pipe(Effect.catchCause(recoverRefreshFailure)), + applySecretAndRefresh: (reference: SecretReference) => + applySecretAndRefresh(reference).pipe(Effect.catchCause(recoverRefreshFailure)), refreshWorkspaceSnapshot: (input) => refreshWorkspaceSnapshot(input).pipe(Effect.catchCause(recoverRefreshFailure)), refreshAfterUpdate, diff --git a/apps/server/src/provider/providerMaintenanceRunner.test.ts b/apps/server/src/provider/providerMaintenanceRunner.test.ts index 77a44c7fa29a5..1d2603e9b211b 100644 --- a/apps/server/src/provider/providerMaintenanceRunner.test.ts +++ b/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) { diff --git a/apps/server/src/provider/testUtils/providerRegistryMock.ts b/apps/server/src/provider/testUtils/providerRegistryMock.ts index f75ecd141e71a..2958ce7614353 100644 --- a/apps/server/src/provider/testUtils/providerRegistryMock.ts +++ b/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, diff --git a/apps/server/src/secrets/SecretCheck.test.ts b/apps/server/src/secrets/SecretCheck.test.ts new file mode 100644 index 0000000000000..c5d0bf3433e27 --- /dev/null +++ b/apps/server/src/secrets/SecretCheck.test.ts @@ -0,0 +1,276 @@ +import * as NodeCrypto from "@effect/platform-node/NodeCrypto"; +import * as NodeServices from "@effect/platform-node/NodeServices"; +import { assert, it } from "@effect/vitest"; +import { + ProviderDriverKind, + ProviderInstanceId, + SecretProviderId, + SecretProviderKind, + type SecretReference, +} from "@t3tools/contracts"; +import type { AnyProviderDriver, ProviderInstance } from "@t3tools/provider-core/server/driver"; +import * as SecretResolver from "@t3tools/secret-provider-core/server/SecretResolver"; +import { defineSecretProviderDriver } from "@t3tools/secret-provider-core/server/driver"; +import { defineSecretProvider } from "@t3tools/secret-provider-core/shared/definition"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; +import * as PubSub from "effect/PubSub"; +import * as Schema from "effect/Schema"; + +import { makeProviderInstanceRegistry } from "../provider/ProviderInstanceRegistry.ts"; +import * as ProviderRegistry from "../provider/ProviderRegistry.ts"; +import * as SecretCheck from "./SecretCheck.ts"; + +const reference: SecretReference = { + provider: SecretProviderId.make("work"), + reference: "op://Private/OpenAI/api-key", +}; + +const run = (value: string | undefined) => + Effect.gen(function* () { + const calls: Array = []; + const layer = SecretCheck.layer.pipe( + Layer.provide( + Layer.mergeAll( + Layer.mock(SecretResolver.SecretResolver)({ + invalidate: () => Effect.sync(() => void calls.push("invalidate")), + read: () => Effect.sync(() => (calls.push("read"), [value])), + }), + Layer.mock(ProviderRegistry.ProviderRegistry)({ + applySecretAndRefresh: () => Effect.sync(() => (calls.push("apply"), [])), + }), + ), + ), + ); + const result = yield* SecretCheck.SecretCheck.use((check) => check.check(reference)).pipe( + Effect.provide(layer), + ); + return { result, calls }; + }); + +it.effect( + "a readable secret resolves without returning its value and applies it to providers", + () => + Effect.gen(function* () { + const { result, calls } = yield* run("sk-live-value"); + assert.deepStrictEqual(result, { _tag: "Resolved" }); + assert.deepStrictEqual(calls, ["invalidate", "read", "apply"]); + assert.notInclude(JSON.stringify(result), "sk-live-value"); + }), +); + +it.effect("an unreadable secret fails with the provider named and applies nothing", () => + Effect.gen(function* () { + const { result, calls } = yield* run(undefined); + assert.strictEqual(result._tag, "Failed"); + if (result._tag === "Failed") assert.include(result.detail, '"work"'); + assert.deepStrictEqual(calls, ["invalidate", "read"]); + }), +); + +const FAKE_KIND = SecretProviderKind.make("fake"); + +it.effect( + "applies a check to the instances that read the secret, whatever a second read would return", + () => + Effect.gen(function* () { + const store = new Map>(); + const reads: Array = []; + const secretDriver = defineSecretProviderDriver({ + kind: FAKE_KIND, + definition: defineSecretProvider({ + kind: FAKE_KIND, + label: "Fake", + configSchema: Schema.Struct({}), + isReference: (value) => value.startsWith("fake://"), + referencePlaceholder: "fake://item", + invalidReferenceMessage: "Use fake://item.", + }), + read: (_config, references) => + Effect.sync(() => + references.map((reference) => { + reads.push(reference); + const answers = store.get(reference) ?? []; + return answers.length > 1 ? answers.shift() : answers[0]; + }), + ), + }); + const provider = SecretProviderId.make("personal"); + const wanted: SecretReference = { provider, reference: "fake://wanted" }; + const other: SecretReference = { provider, reference: "fake://other" }; + store.set(wanted.reference, [undefined]); + store.set(other.reference, ["other-key"]); + + const driverKind = ProviderDriverKind.make("fake-provider"); + const dependent = ProviderInstanceId.make("dependent"); + const unrelated = ProviderInstanceId.make("unrelated"); + const driver: AnyProviderDriver = { + driverKind, + metadata: { displayName: "Fake" }, + configSchema: Schema.Unknown, + defaultConfig: () => ({}), + create: (input) => + Effect.succeed({ + instanceId: input.instanceId, + enabled: input.enabled, + } as ProviderInstance), + }; + const instanceConfig = (secret: SecretReference) => ({ + driver: driverKind, + config: {}, + environment: [{ name: "API_KEY", value: secret, sensitive: true }], + }); + + const resolverLayer = SecretResolver.layer({ + drivers: [secretDriver], + secretProviders: Effect.succeed({ + [provider]: { kind: FAKE_KIND, label: "Personal", config: {} }, + }), + }).pipe(Layer.provide(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer))); + + yield* Effect.gen(function* () { + const resolver = yield* SecretResolver.SecretResolver; + const { registry, mutator } = yield* makeProviderInstanceRegistry({ + drivers: [driver], + configMap: { [dependent]: instanceConfig(wanted), [unrelated]: instanceConfig(other) }, + }); + const changes = yield* registry.subscribeChanges; + yield* PubSub.take(changes); + assert.deepStrictEqual( + (yield* registry.listUnavailable).map((entry) => entry.instanceId), + [dependent], + ); + + // Unlocked for the check's read only: any later read of it would fail again. + store.set(wanted.reference, ["wanted-key", undefined]); + const readsBefore = reads.length; + const result = yield* SecretCheck.SecretCheck.use((check) => check.check(wanted)).pipe( + Effect.provide( + SecretCheck.layer.pipe( + Layer.provide( + Layer.mock(ProviderRegistry.ProviderRegistry)({ + applySecretAndRefresh: (reference) => + mutator.applySecret(reference).pipe(Effect.as([])), + }), + ), + Layer.provideMerge(Layer.succeed(SecretResolver.SecretResolver, resolver)), + ), + ), + ); + + assert.deepStrictEqual(result, { _tag: "Resolved" }); + assert.deepStrictEqual( + (yield* registry.listInstances).map((instance) => instance.instanceId).sort(), + [dependent, unrelated], + ); + assert.deepStrictEqual(yield* registry.listUnavailable, []); + assert.deepStrictEqual(reads.slice(readsBefore), [wanted.reference]); + }).pipe(Effect.provide(resolverLayer), Effect.scoped); + }), +); + +it.effect("applies a check to a live instance without re-reading its other references", () => + Effect.gen(function* () { + const store = new Map(); + const reads: Array = []; + const secretDriver = defineSecretProviderDriver({ + kind: FAKE_KIND, + definition: defineSecretProvider({ + kind: FAKE_KIND, + label: "Fake", + configSchema: Schema.Struct({}), + isReference: (value) => value.startsWith("fake://"), + referencePlaceholder: "fake://item", + invalidReferenceMessage: "Use fake://item.", + }), + read: (_config, references) => + Effect.sync(() => + references.map((reference) => { + reads.push(reference); + return store.get(reference); + }), + ), + }); + const provider = SecretProviderId.make("personal"); + const first: SecretReference = { provider, reference: "fake://first" }; + const second: SecretReference = { provider, reference: "fake://second" }; + store.set(first.reference, "first-old"); + store.set(second.reference, "second-old"); + + const driverKind = ProviderDriverKind.make("fake-provider"); + const pair = ProviderInstanceId.make("pair"); + const created: Array> = []; + const driver: AnyProviderDriver = { + driverKind, + metadata: { displayName: "Fake" }, + configSchema: Schema.Unknown, + defaultConfig: () => ({}), + create: (input) => + Effect.sync(() => { + created.push(input.environment.map((variable) => [variable.name, variable.value])); + return { instanceId: input.instanceId, enabled: input.enabled } as ProviderInstance; + }), + }; + + const resolverLayer = SecretResolver.layer({ + drivers: [secretDriver], + secretProviders: Effect.succeed({ + [provider]: { kind: FAKE_KIND, label: "Personal", config: {} }, + }), + }).pipe(Layer.provide(Layer.mergeAll(NodeServices.layer, NodeCrypto.layer))); + + yield* Effect.gen(function* () { + const resolver = yield* SecretResolver.SecretResolver; + const { registry, mutator } = yield* makeProviderInstanceRegistry({ + drivers: [driver], + configMap: { + [pair]: { + driver: driverKind, + config: {}, + environment: [ + { name: "TOKEN", value: first, sensitive: true }, + { name: "TOKEN", value: second, sensitive: true }, + ], + }, + }, + }); + const changes = yield* registry.subscribeChanges; + yield* PubSub.take(changes); + assert.deepStrictEqual(created, [ + [ + ["TOKEN", "first-old"], + ["TOKEN", "second-old"], + ], + ]); + + // A failed read of the second secret stays cached while the live instance keeps its value. + yield* resolver.invalidate([second]); + store.set(second.reference, undefined); + yield* resolver.read([second]); + store.set(first.reference, "first-new"); + const readsBefore = reads.length; + + const result = yield* SecretCheck.SecretCheck.use((check) => check.check(first)).pipe( + Effect.provide( + SecretCheck.layer.pipe( + Layer.provide( + Layer.mock(ProviderRegistry.ProviderRegistry)({ + applySecretAndRefresh: (reference) => + mutator.applySecret(reference).pipe(Effect.as([])), + }), + ), + Layer.provideMerge(Layer.succeed(SecretResolver.SecretResolver, resolver)), + ), + ), + ); + + assert.deepStrictEqual(result, { _tag: "Resolved" }); + assert.deepStrictEqual(created.at(-1), [ + ["TOKEN", "first-new"], + ["TOKEN", "second-old"], + ]); + assert.deepStrictEqual(yield* registry.listUnavailable, []); + assert.deepStrictEqual(reads.slice(readsBefore), [first.reference]); + }).pipe(Effect.provide(resolverLayer), Effect.scoped); + }), +); diff --git a/apps/server/src/secrets/SecretCheck.ts b/apps/server/src/secrets/SecretCheck.ts new file mode 100644 index 0000000000000..bce705b05afa6 --- /dev/null +++ b/apps/server/src/secrets/SecretCheck.ts @@ -0,0 +1,40 @@ +import type { SecretReference, SecretSourceCheckResult } from "@t3tools/contracts"; +import * as SecretResolver from "@t3tools/secret-provider-core/server/SecretResolver"; +import * as Context from "effect/Context"; +import * as Effect from "effect/Effect"; +import * as Layer from "effect/Layer"; + +import * as ProviderRegistry from "../provider/ProviderRegistry.ts"; + +export class SecretCheck extends Context.Service< + SecretCheck, + { + /** + * Reads one secret fresh for the Settings "check" button and never returns the value. A + * readable secret also rebuilds the provider instances that read it, from that same read. + */ + readonly check: (reference: SecretReference) => Effect.Effect; + } +>()("t3/secrets/SecretCheck") {} + +const make = Effect.gen(function* () { + const resolver = yield* SecretResolver.SecretResolver; + const providerRegistry = yield* ProviderRegistry.ProviderRegistry; + + const check = Effect.fn("SecretCheck.check")(function* (reference: SecretReference) { + yield* resolver.invalidate([reference]); + const [value] = yield* resolver.read([reference]); + if (value === undefined) { + return { + _tag: "Failed", + detail: `Could not read this secret from "${reference.provider}". Check the provider's settings and that its store is unlocked.`, + } as const; + } + yield* providerRegistry.applySecretAndRefresh(reference); + return { _tag: "Resolved" } as const; + }); + + return SecretCheck.of({ check }); +}); + +export const layer = Layer.effect(SecretCheck, make); diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 5c9ab9c433ffc..fcbc4b653bf00 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -72,6 +72,7 @@ import * as SourceControlBuiltInDrivers from "./sourceControl/builtInDrivers.ts" import * as SourceControlSignIn from "./sourceControl/SourceControlSignIn.ts"; import { BUILT_IN_SOURCE_CONTROL_DRIVER_LIST } from "./sourceControl/sourceControlDefinitions.ts"; import * as TextGeneration from "./textGeneration/TextGeneration.ts"; +import * as SecretCheck from "./secrets/SecretCheck.ts"; import * as SecretResolver from "./secrets/SecretResolver.ts"; import * as ProviderInstanceRegistryHydration from "./provider/ProviderInstanceRegistryHydration.ts"; import * as ProjectSettingsOverrideSweep from "./provider/ProjectSettingsOverrideSweep.ts"; @@ -671,6 +672,7 @@ const layerRuntimeCoreDependenciesBase = Layer.mergeAll( Layer.provideMerge(layerOrchestrationApplication), Layer.provideMerge(RuntimeLayer.layerEventInfrastructure), Layer.provideMerge(Layer.merge(ProjectStore.layer, ThreadSearch.layer)), + Layer.provideMerge(SecretCheck.layer), Layer.provideMerge(SecretResolver.layer), Layer.provideMerge(layerServerSettings), Layer.provideMerge(layerSourceControlProviderRegistry), diff --git a/apps/server/src/ws.test.ts b/apps/server/src/ws.test.ts index bf2298427e411..db2a015aa1a87 100644 --- a/apps/server/src/ws.test.ts +++ b/apps/server/src/ws.test.ts @@ -114,6 +114,7 @@ import * as TraceDiagnostics from "./diagnostics/TraceDiagnostics.ts"; import * as TriageService from "./triage/TriageService.ts"; import * as WorkspacePaths from "./workspace/WorkspacePaths.ts"; import * as ServerSecretStore from "./auth/ServerSecretStore.ts"; +import * as SecretCheck from "./secrets/SecretCheck.ts"; import * as ServerConfigModule from "./config.ts"; import * as Keybindings from "./keybindings.ts"; import { USAGE_LIMITS_COMMAND } from "@t3tools/shared/usageLimits"; @@ -659,6 +660,7 @@ it.effect("shell snapshots omit idle children and older settled threads only on const layerUnusedConfigRpcServices = Layer.mergeAll( Layer.mock(DocumentText.DocumentText)({}), Layer.mock(ServerSecretStore.ServerSecretStore)({}), + Layer.mock(SecretCheck.SecretCheck)({}), Layer.mock(WorkspacePaths.WorkspacePaths)({}), Layer.mock(TraceDiagnostics.TraceDiagnostics)({}), Layer.mock(TriageService.TriageService)({}), diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index e3c7fdaa47476..93f0429526c85 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -172,6 +172,7 @@ import * as ThreadSearch from "./orchestration-v2/ThreadSearch.ts"; import * as OrchestrationEventStore from "./persistence/OrchestrationEventStore.ts"; import { userFacingDispatchErrorMessage } from "./orchestration-v2/UserFacingErrors.ts"; import * as ProviderRegistry from "./provider/ProviderRegistry.ts"; +import * as SecretCheck from "./secrets/SecretCheck.ts"; import * as ProviderInstanceRegistry from "./provider/ProviderInstanceRegistry.ts"; import * as AcpRegistrySupport from "@t3tools/provider-acp-registry/server/AcpRegistrySupport"; import * as AcpRegistryRuntimeCoordinator from "@t3tools/provider-acp-registry/server/AcpRegistryRuntimeCoordinator"; @@ -1532,6 +1533,7 @@ export const layerWsRpc = ( ); const serverAuth = yield* EnvironmentAuth.EnvironmentAuth; const sourceControlDiscovery = yield* SourceControlDiscovery.SourceControlDiscovery; + const secretCheck = yield* SecretCheck.SecretCheck; const otlpEndpointCheck = yield* OtlpEndpointCheck.OtlpEndpointCheck; const sourceControlRepositories = yield* SourceControlRepositoryService.SourceControlRepositoryService; @@ -2699,6 +2701,7 @@ export const layerWsRpc = ( return ServerSettings.redactServerSettingsForClient(settings, settingsClient); }), [WS_METHODS.serverDiscoverSourceControl]: (_input) => sourceControlDiscovery.discover, + [WS_METHODS.serverCheckSecretSource]: (input) => secretCheck.check(input.source), [WS_METHODS.serverGetTraceDiagnostics]: (_input) => TraceDiagnostics.readTraceDiagnostics({ traceFilePath: config.serverTracePath, diff --git a/apps/web/src/components/settings/AddUsageLimitSourceDialog.tsx b/apps/web/src/components/settings/AddUsageLimitSourceDialog.tsx index 780fba4a7007d..2098055289e02 100644 --- a/apps/web/src/components/settings/AddUsageLimitSourceDialog.tsx +++ b/apps/web/src/components/settings/AddUsageLimitSourceDialog.tsx @@ -107,6 +107,7 @@ export function AddUsageLimitSourceDialog({ aria-label="Management key" value={keyDraft} sources={secretSources} + environmentId={environmentId} onChange={(event) => setKeyDraft(event.target.value)} onSecretChange={(secret) => setKeyDraft(secret ?? "")} onValidityChange={setKeyValid} diff --git a/apps/web/src/components/settings/EasDeviceHostEditor.tsx b/apps/web/src/components/settings/EasDeviceHostEditor.tsx index 1223fe09b7d7f..e11d9dfa748aa 100644 --- a/apps/web/src/components/settings/EasDeviceHostEditor.tsx +++ b/apps/web/src/components/settings/EasDeviceHostEditor.tsx @@ -213,6 +213,7 @@ export function EasDeviceHostEditor(props: { : "" } sources={secretSources} + environmentId={props.environmentId} onValidityChange={(valid) => { setTokenValid(valid); resetAccount(); diff --git a/apps/web/src/components/settings/ProviderInstanceCard.tsx b/apps/web/src/components/settings/ProviderInstanceCard.tsx index b676e64399d97..4de75ab8bbe8a 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.tsx +++ b/apps/web/src/components/settings/ProviderInstanceCard.tsx @@ -282,6 +282,7 @@ function ProviderEnvironmentFieldRow(props: { readonly onRemove: (field: ProviderEnvironmentField) => void; readonly managedReason?: string | undefined; readonly secretSources: ReadonlyArray; + readonly environmentId?: EnvironmentId | undefined; }) { const inputId = `${props.idPrefix}-environment-${props.field.name}`; const value = props.variable?.valueRedacted ? "" : (props.variable?.value ?? ""); @@ -316,6 +317,7 @@ function ProviderEnvironmentFieldRow(props: { spellCheck={false} aria-label={props.field.label} sources={props.secretSources} + environmentId={props.environmentId} onSecretChange={(secret) => props.onSecretChange(props.field, secret)} /> )} @@ -342,6 +344,7 @@ export function ProviderEnvironmentSection(props: { /** Variables an administrator's policy pins: listed by name, never editable or removable. */ readonly managed?: { readonly names: ReadonlySet; readonly reason: string } | undefined; readonly secretSources?: ReadonlyArray; + readonly environmentId?: EnvironmentId | undefined; }) { const [storedDraft, setDraft] = useState(() => seedRowsDraft(props.environment, toEnvironmentDraftRows), @@ -531,6 +534,7 @@ export function ProviderEnvironmentSection(props: { spellCheck={false} aria-label={`Environment variable value ${index + 1}`} sources={props.secretSources ?? []} + environmentId={props.environmentId} onSecretChange={(secret) => setVariableSecret(variable.id, secret)} > {(parts) => layout(parts.source, parts.value)} @@ -1306,6 +1310,7 @@ export function ProviderInstanceCard({ : undefined } secretSources={secretSources} + environmentId={environmentId} /> ))}

@@ -1353,6 +1358,7 @@ export function ProviderInstanceCard({ environment={genericEnvironment} onChange={updateGenericEnvironment} secretSources={secretSources} + environmentId={environmentId} managed={ instanceLocks && { names: instanceLocks.environmentNames, diff --git a/apps/web/src/components/settings/ProviderSettingsForm.tsx b/apps/web/src/components/settings/ProviderSettingsForm.tsx index 646dfd80edc20..421f7d7b16696 100644 --- a/apps/web/src/components/settings/ProviderSettingsForm.tsx +++ b/apps/web/src/components/settings/ProviderSettingsForm.tsx @@ -373,6 +373,7 @@ function SecretSettingsControl({ value={reference ?? ""} configured={reference === undefined && configured} sources={sources} + environmentId={environmentId} onCommit={(next) => onChange(nextProviderConfigWithFieldValue(value, field, next))} onSecretChange={(secret) => onChange(nextProviderConfigWithFieldValue(value, field, secret ?? "")) diff --git a/apps/web/src/components/settings/SecretSourceInput.tsx b/apps/web/src/components/settings/SecretSourceInput.tsx index 56723508c8544..7bd85809cc348 100644 --- a/apps/web/src/components/settings/SecretSourceInput.tsx +++ b/apps/web/src/components/settings/SecretSourceInput.tsx @@ -9,15 +9,19 @@ import { builtInSecretProviderDefinitions } from "@t3tools/secret-provider-defin import { useAtomValue } from "@effect/atom-react"; import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; -import { KeyRoundIcon, TypeIcon } from "lucide-react"; +import { AlertTriangleIcon, CheckIcon, KeyRoundIcon, RefreshCwIcon, TypeIcon } from "lucide-react"; import { useState, type ComponentProps, type ComponentType, type ReactNode } from "react"; import { isSecretsFeatureEnabled } from "../../env"; import { useEnvironmentSettings } from "../../hooks/useSettings"; import { OnePasswordIcon } from "../Icons"; import { serverEnvironment } from "../../state/server"; +import { useAtomCommand } from "../../state/use-atom-command"; +import { Button } from "../ui/button"; import { DraftInput } from "../ui/draft-input"; import { Select, SelectItem, SelectPopup, SelectTrigger, SelectValue } from "../ui/select"; +import { Spinner } from "../ui/spinner"; +import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { SecretInput } from "./SecretInput"; import { classifySecretProvider } from "./SecretProviderDialog.logic"; @@ -111,8 +115,91 @@ export function SecretSourceSelect(props: { ); } +type SecretCheck = + | { readonly status: "checking" } + | { readonly status: "resolved" } + | { readonly status: "failed"; readonly detail: string }; + +/** Asks the server to resolve the committed reference, so a typo shows here and not at provider start. */ +function SecretCheckButton(props: { + readonly environmentId: EnvironmentId; + readonly secret: SecretReference; + readonly label: string; +}) { + const { environmentId, secret } = props; + const canCheck = useAtomValue(serverEnvironment.checkSecretSource.permissionAtom(environmentId)); + const checkSecretSource = useAtomCommand(serverEnvironment.checkSecretSource); + const secretKey = `${secret.provider}\u0000${secret.reference}`; + const [check, setCheck] = useState<{ + readonly secretKey: string; + readonly state: SecretCheck; + } | null>(null); + const state = check?.secretKey === secretKey ? check.state : undefined; + + const run = async () => { + setCheck({ secretKey, state: { status: "checking" } }); + const response = await checkSecretSource({ environmentId, input: { source: secret } }); + const next: SecretCheck = + response._tag !== "Success" + ? { status: "failed", detail: "Could not reach the server" } + : response.value._tag === "Resolved" + ? { status: "resolved" } + : { status: "failed", detail: response.value.detail }; + setCheck((previous) => + previous?.secretKey === secretKey ? { secretKey, state: next } : previous, + ); + }; + + const description = !canCheck + ? "You cannot check secrets on this environment" + : state === undefined + ? "Check that the secret resolves" + : state.status === "checking" + ? "Checking the secret" + : state.status === "resolved" + ? "Secret resolved" + : state.detail; + return ( + + + + + } + /> + {description} + + ); +} + type SecretSourceInputProps = Omit, "value" | "children"> & { readonly value: string | SecretReference; + /** Offers a check of the committed reference on this environment. */ + readonly environmentId?: EnvironmentId | undefined; readonly sources: ReadonlyArray; readonly onSecretChange: (secret: SecretReference | undefined) => void; /** Whether the selected source and committed field can be saved or used. */ @@ -127,6 +214,7 @@ type SecretSourceInputProps = Omit, "value" | /** A secret entered here, or read from a secret provider when one is configured. */ export function SecretSourceInput({ value, + environmentId, sources, onSecretChange, onValidityChange, @@ -168,38 +256,48 @@ export function SecretSourceInput({ ) : (
- { - if (next.trim().length === 0) { +
+ { + if (next.trim().length === 0) { + setRejectedReference(null); + onValidityChange?.(pendingProvider === null && secret !== undefined); + if (pendingProvider === null && secret !== undefined) onSecretChange(undefined); + return; + } + const decoded = decodeSecretReference({ provider, reference: next }); + if ( + Option.isNone(decoded) || + definition?.isReference(decoded.value.reference) === false + ) { + setRejectedReference(next); + onValidityChange?.(false); + return; + } setRejectedReference(null); - onValidityChange?.(pendingProvider === null && secret !== undefined); - if (pendingProvider === null && secret !== undefined) onSecretChange(undefined); - return; - } - const decoded = decodeSecretReference({ provider, reference: next }); - if ( - Option.isNone(decoded) || - definition?.isReference(decoded.value.reference) === false - ) { - setRejectedReference(next); - onValidityChange?.(false); - return; - } - setRejectedReference(null); - setPendingProvider(null); - onValidityChange?.(true); - onSecretChange(decoded.value); - }} - autoComplete="off" - placeholder={definition?.referencePlaceholder} - spellCheck={false} - aria-label={`${label} reference`} - aria-invalid={rejectedReference !== null} - /> + setPendingProvider(null); + onValidityChange?.(true); + onSecretChange(decoded.value); + }} + autoComplete="off" + placeholder={definition?.referencePlaceholder} + spellCheck={false} + aria-label={`${label} reference`} + aria-invalid={rejectedReference !== null} + /> + {environmentId !== undefined && secret !== undefined && pendingProvider === null ? ( + + ) : null} +
{rejectedReference !== null ? (

{definition?.invalidReferenceMessage ?? "Enter a valid secret reference."} diff --git a/apps/web/src/components/settings/SimlockDeviceHostEditor.tsx b/apps/web/src/components/settings/SimlockDeviceHostEditor.tsx index 9037508b2790e..f0614b771ea1f 100644 --- a/apps/web/src/components/settings/SimlockDeviceHostEditor.tsx +++ b/apps/web/src/components/settings/SimlockDeviceHostEditor.tsx @@ -301,6 +301,7 @@ function SimlockTokenField(props: { key={inputKey} id={id} aria-label="Agent token" + environmentId={props.environmentId} className="min-w-0 flex-1" configured={props.token.kind === "saved" && typeof props.saved === "string"} value={ diff --git a/apps/web/src/components/settings/SourceControlTokenSettings.tsx b/apps/web/src/components/settings/SourceControlTokenSettings.tsx index 5a78b5f28287d..f83d0de6e6f72 100644 --- a/apps/web/src/components/settings/SourceControlTokenSettings.tsx +++ b/apps/web/src/components/settings/SourceControlTokenSettings.tsx @@ -127,6 +127,7 @@ export function SourceControlTokenSettings({ id={`${definition.kind}-token-${environmentId}`} size="sm" aria-label={`${definition.label} token`} + environmentId={environmentId} configured={isSaved && !isSecretReference(savedToken)} value={draft} sources={secretSources} diff --git a/packages/client-runtime/src/state/commandPermissions.test.ts b/packages/client-runtime/src/state/commandPermissions.test.ts index 226931f38c0b9..42cf3c5a78a36 100644 --- a/packages/client-runtime/src/state/commandPermissions.test.ts +++ b/packages/client-runtime/src/state/commandPermissions.test.ts @@ -156,25 +156,27 @@ describe("command permissions", () => { ), ); - it.effect("requires the destination settings grant to check OTLP endpoints", () => - Effect.scoped( - Effect.gen(function* () { - const registry = yield* setup; - const check = createCommandPermissions(runtime, WS_METHODS.serverCheckOtlpEndpoint); - registry.set(sessions(env), AsyncResult.success(grant(true))); - expect(registry.get(check.permissionAtom(env))).toBe(false); - registry.set( - sessions(env), - AsyncResult.success({ - ...grant(false), - scopes: [AuthSettingsWriteScope], - permissions: [AuthSettingsWriteScope], - }), - ); - expect(registry.get(check.permissionAtom(env))).toBe(true); - yield* check.authorize(registry, env); - }), - ), + it.effect.each([WS_METHODS.serverCheckOtlpEndpoint, WS_METHODS.serverCheckSecretSource])( + "requires the destination settings grant for %s", + (method) => + Effect.scoped( + Effect.gen(function* () { + const registry = yield* setup; + const check = createCommandPermissions(runtime, method); + registry.set(sessions(env), AsyncResult.success(grant(true))); + expect(registry.get(check.permissionAtom(env))).toBe(false); + registry.set( + sessions(env), + AsyncResult.success({ + ...grant(false), + scopes: [AuthSettingsWriteScope], + permissions: [AuthSettingsWriteScope], + }), + ); + expect(registry.get(check.permissionAtom(env))).toBe(true); + yield* check.authorize(registry, env); + }), + ), ); it.effect.each([WS_METHODS.scheduledTasksDelete, ORCHESTRATION_V2_WS_METHODS.stopWorkflow])( diff --git a/packages/client-runtime/src/state/server.ts b/packages/client-runtime/src/state/server.ts index f5c9d8f45b3db..b57f7b1d293ac 100644 --- a/packages/client-runtime/src/state/server.ts +++ b/packages/client-runtime/src/state/server.ts @@ -1604,6 +1604,10 @@ export function createServerEnvironmentAtoms( label: "environment-data:server:check-otlp-endpoint", tag: WS_METHODS.serverCheckOtlpEndpoint, }), + checkSecretSource: createEnvironmentRpcCommand(runtime, { + label: "environment-data:server:check-secret-source", + tag: WS_METHODS.serverCheckSecretSource, + }), retryResourceTelemetry: createEnvironmentRpcCommand(runtime, { label: "environment-data:server:retry-resource-telemetry", tag: WS_METHODS.serverRetryResourceTelemetry, diff --git a/packages/contracts/src/clientRpcPermissions.ts b/packages/contracts/src/clientRpcPermissions.ts index 91db78715551e..4ca4625b3391a 100644 --- a/packages/contracts/src/clientRpcPermissions.ts +++ b/packages/contracts/src/clientRpcPermissions.ts @@ -20,6 +20,7 @@ export const CLIENT_GUARDED_RPC_SCOPES = { [WS_METHODS.previewStopLocalServer]: AuthTerminalOperateScope, [WS_METHODS.serverRunStorageCleanup]: AuthSettingsWriteScope, [WS_METHODS.serverCheckOtlpEndpoint]: AuthSettingsWriteScope, + [WS_METHODS.serverCheckSecretSource]: AuthSettingsWriteScope, [WS_METHODS.serverRefreshProviders]: AuthOrchestrationReadScope, [WS_METHODS.pullRequestsRunAction]: AuthSourceControlWriteScope, [WS_METHODS.pullRequestsUpdate]: AuthSourceControlWriteScope, diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index 3106462d35784..c0b68e542ea00 100644 --- a/packages/contracts/src/rpc.ts +++ b/packages/contracts/src/rpc.ts @@ -38,6 +38,8 @@ import * as Rpc from "effect/rpc/Rpc"; import * as RpcGroup from "effect/rpc/RpcGroup"; import * as RpcMiddleware from "effect/rpc/RpcMiddleware"; import { NonNegativeInt, TrimmedNonEmptyString } from "./baseSchemas.ts"; +import { SecretSourceCheckResult } from "./settingsSecretValue.ts"; +import { SecretReference } from "./secretReference.ts"; import { ProviderAuthCancelInput, ProviderAuthCompleteInput, @@ -514,6 +516,7 @@ export const WS_METHODS = { peerLinksAnswerRequest: "peerLinks.answerRequest", serverUpdateSettings: "server.updateSettings", serverDiscoverSourceControl: "server.discoverSourceControl", + serverCheckSecretSource: "server.checkSecretSource", serverSearchAcpRegistry: "server.searchAcpRegistry", serverPrepareAcpRegistryAgent: "server.prepareAcpRegistryAgent", serverUninstallAcpRegistryManagedBinary: "server.uninstallAcpRegistryManagedBinary", @@ -837,6 +840,12 @@ const WsServerDiscoverSourceControlRpc = Rpc.make(WS_METHODS.serverDiscoverSourc error: EnvironmentAuthorizationError, }); +const WsServerCheckSecretSourceRpc = Rpc.make(WS_METHODS.serverCheckSecretSource, { + payload: Schema.Struct({ source: SecretReference }), + success: SecretSourceCheckResult, + error: EnvironmentAuthorizationError, +}); + const WsServerSearchAcpRegistryRpc = Rpc.make(WS_METHODS.serverSearchAcpRegistry, { payload: AcpRegistrySearchInput, success: AcpRegistrySearchResult, @@ -2033,6 +2042,7 @@ export const WsRpcGroup = RpcGroup.make( WsPeerLinksUnlinkRpc, WsPeerLinksAnswerRequestRpc, WsServerDiscoverSourceControlRpc, + WsServerCheckSecretSourceRpc, WsServerSearchAcpRegistryRpc, WsServerPrepareAcpRegistryAgentRpc, WsServerUninstallAcpRegistryManagedBinaryRpc, diff --git a/packages/contracts/src/settingsSecretValue.ts b/packages/contracts/src/settingsSecretValue.ts index 669c804013230..93d720ba6555d 100644 --- a/packages/contracts/src/settingsSecretValue.ts +++ b/packages/contracts/src/settingsSecretValue.ts @@ -121,3 +121,9 @@ export function withSettingsSecrets Date: Fri, 6 Nov 2026 01:56:15 +0000 Subject: [PATCH 6/7] fix(web): the secret check button follows its field's disabled state --- apps/web/src/components/settings/SecretSourceInput.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/web/src/components/settings/SecretSourceInput.tsx b/apps/web/src/components/settings/SecretSourceInput.tsx index 27ac4af301231..98d8e0f9e52e2 100644 --- a/apps/web/src/components/settings/SecretSourceInput.tsx +++ b/apps/web/src/components/settings/SecretSourceInput.tsx @@ -125,6 +125,7 @@ function SecretCheckButton(props: { readonly environmentId: EnvironmentId; readonly secret: SecretReference; readonly label: string; + readonly disabled?: boolean | undefined; }) { const { environmentId, secret } = props; const supported = useAtomValue( @@ -173,7 +174,7 @@ function SecretCheckButton(props: { type="button" size="icon-micro" variant="ghost-muted" - disabled={!canCheck || state?.status === "checking"} + disabled={props.disabled === true || !canCheck || state?.status === "checking"} onClick={() => void run()} aria-label={ state?.status === "failed" @@ -303,6 +304,7 @@ export function SecretSourceInput({ environmentId={environmentId} secret={secret} label={`Check ${label}`} + disabled={input.disabled} /> ) : null}

From 9559e3a496de915d8b65bfdd0242dd5d69736f83 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Fri, 6 Nov 2026 02:22:33 +0000 Subject: [PATCH 7/7] fix(server): a secret check applies the value it read, to its own config key The check passed only the reference to the registry, which read the value back from the resolver cache. A Settings refresh holding the registry lock could re-read that reference from a now-locked vault in between, so the check reported Resolved while the instance kept the old value or stayed parked. The checked value now travels with the reference and is used for both live and parked instances without going through the cache. A record-valued config secret matched its slots by field alone, so checking one key overwrote its siblings; slots now match on field and key. --- .../src/provider/ProviderInstanceRegistry.ts | 79 ++++++++----- .../ProviderInstanceRegistryMutator.ts | 5 +- apps/server/src/provider/ProviderRegistry.ts | 10 +- apps/server/src/secrets/SecretCheck.test.ts | 110 ++++++++++++++++-- apps/server/src/secrets/SecretCheck.ts | 2 +- 5 files changed, 159 insertions(+), 47 deletions(-) diff --git a/apps/server/src/provider/ProviderInstanceRegistry.ts b/apps/server/src/provider/ProviderInstanceRegistry.ts index d858c176cec7f..ae51e8009645c 100644 --- a/apps/server/src/provider/ProviderInstanceRegistry.ts +++ b/apps/server/src/provider/ProviderInstanceRegistry.ts @@ -92,9 +92,20 @@ 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* ( driver: AnyProviderDriver | undefined, entry: ProviderInstanceConfig, + checked?: CheckedSecret, ) { const variables = entry.environment ?? []; const configFields = configSecretReferences(driver, entry); @@ -102,8 +113,15 @@ const readInstanceSecrets = Effect.fn("ProviderInstanceRegistry.readInstanceSecr ...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 = []; let index = 0; // An empty secret reads as unreadable, so a driver cannot fall back to an inherited credential. @@ -136,19 +154,14 @@ const carryCheckedSecret = ( driver: AnyProviderDriver | undefined, entry: ProviderInstanceConfig, current: InstanceSecrets, - checked: SecretReference, - value: string | undefined, + { reference: checked, value }: CheckedSecret, ): InstanceSecrets => { - if (value === undefined || value.trim() === "") return current; - const matches = (reference: SecretReference) => - reference.provider === checked.provider && reference.reference === checked.reference; + 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 replacedFields = new Set( - configSecretReferences(driver, entry).flatMap(({ field, reference }) => - matches(reference) ? [field] : [], - ), + const replacedSlots = configSecretReferences(driver, entry).filter(({ reference }) => + matches(reference), ); return { environment: current.environment.map((variable, index) => { @@ -158,7 +171,9 @@ const carryCheckedSecret = ( : variable; }), config: current.config.map((slot) => - replacedFields.has(slot.field) ? { ...slot, value } : slot, + replacedSlots.some(({ field, key }) => field === slot.field && key === slot.key) + ? { ...slot, value } + : slot, ), }; }; @@ -550,12 +565,12 @@ 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. a missing `checked` reference drops - // the cached values first; with one, only that value is replaced and the rest are carried over. + // 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?: SecretReference, + checked?: CheckedSecret, ) { const configMap = yield* Ref.get(configMapRef); const targets = yield* Effect.filter(Object.entries(configMap), ([rawInstanceId, entry]) => @@ -564,17 +579,28 @@ export const makeProviderInstanceRegistry = Effect.fn( : Effect.succeed(false), ); if (targets.length === 0) return []; - const resolver = yield* SecretResolver.SecretResolver; - const references = targets.flatMap(([, entry]) => secretReferencesOf(driversById, entry)); - if (checked === undefined) yield* resolver.invalidate(references); - const [checkedValue] = yield* resolver.read(checked === undefined ? references : [checked]); + 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(); const carried = new Map(); for (const [rawInstanceId, entry] of targets) { - const current = live.get(ProviderInstanceId.make(rawInstanceId)); + const instanceId = ProviderInstanceId.make(rawInstanceId); + const current = live.get(instanceId); if (current === undefined) { - rebuild.add(ProviderInstanceId.make(rawInstanceId)); + 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); + } continue; } if (checked !== undefined) { @@ -583,7 +609,6 @@ export const makeProviderInstanceRegistry = Effect.fn( current.entry, current.secrets, checked, - checkedValue, ); if (!Equal.equals(secrets, current.secrets)) { rebuild.add(current.instance.instanceId); @@ -613,15 +638,13 @@ export const makeProviderInstanceRegistry = Effect.fn( undefined, ).pipe(Effect.asVoid); const applySecret: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape["applySecret"] = - (reference) => + (reference, value) => reloadWhere( (_, entry) => - secretReferencesOf(driversById, entry).some( - (candidate) => - candidate.provider === reference.provider && - candidate.reference === reference.reference, + secretReferencesOf(driversById, entry).some((candidate) => + sameReference(candidate, reference), ), - reference, + { reference, value }, ); // Reading secrets can wait on a vault unlock, so the server does not wait for it to start. diff --git a/apps/server/src/provider/ProviderInstanceRegistryMutator.ts b/apps/server/src/provider/ProviderInstanceRegistryMutator.ts index 1d6324d97b98a..911a055d79625 100644 --- a/apps/server/src/provider/ProviderInstanceRegistryMutator.ts +++ b/apps/server/src/provider/ProviderInstanceRegistryMutator.ts @@ -12,11 +12,12 @@ export interface ProviderInstanceRegistryMutatorShape { /** Re-reads the secrets of every instance, or one, and rebuilds those whose secrets changed. */ readonly reloadSecrets: (instanceId?: ProviderInstanceId) => Effect.Effect; /** - * Rebuilds the instances that read `reference`, from the value its resolver already holds, and - * returns them. Nothing else is read. + * 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>; } diff --git a/apps/server/src/provider/ProviderRegistry.ts b/apps/server/src/provider/ProviderRegistry.ts index 6ced8c4f6f971..3c534c333d75e 100644 --- a/apps/server/src/provider/ProviderRegistry.ts +++ b/apps/server/src/provider/ProviderRegistry.ts @@ -70,9 +70,10 @@ export class ProviderRegistry extends Context.Service< instanceId?: ProviderInstanceId, ) => Effect.Effect>; - /** Rebuilds and refreshes only the instances that read a secret the caller just read. */ + /** 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>; /** Only refreshed instances can verify an update; the published list still includes every row. */ @@ -700,8 +701,9 @@ export const layer = Layer.effect( const applySecretAndRefresh = Effect.fn("applySecretAndRefresh")(function* ( reference: SecretReference, + value: string, ) { - const instanceIds = yield* instanceRegistryMutator.applySecret(reference); + const instanceIds = yield* instanceRegistryMutator.applySecret(reference, value); yield* Effect.forEach(instanceIds, refreshInstance, { discard: true }); return yield* Ref.get(providersRef); }); @@ -1091,8 +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) => - applySecretAndRefresh(reference).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, diff --git a/apps/server/src/secrets/SecretCheck.test.ts b/apps/server/src/secrets/SecretCheck.test.ts index 08def438379d7..9c0bad4cd3111 100644 --- a/apps/server/src/secrets/SecretCheck.test.ts +++ b/apps/server/src/secrets/SecretCheck.test.ts @@ -7,6 +7,7 @@ import { SecretProviderId, SecretProviderKind, type SecretReference, + SettingsSecretValue, type ServerProvider, } from "@t3tools/contracts"; import type { AnyProviderDriver, ProviderInstance } from "@t3tools/provider-core/server/driver"; @@ -15,7 +16,9 @@ import * as ProviderLatestVersions from "@t3tools/provider-core/server/ProviderL import * as SecretResolver from "@t3tools/secret-provider-core/server/SecretResolver"; import { defineSecretProviderDriver } from "@t3tools/secret-provider-core/server/driver"; import { defineSecretProvider } from "@t3tools/secret-provider-core/shared/definition"; +import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; +import * as Fiber from "effect/Fiber"; import { HttpClient, HttpClientResponse } from "effect/http"; import * as Layer from "effect/Layer"; import * as PubSub from "effect/PubSub"; @@ -55,14 +58,19 @@ const providerSnapshot = (instanceId: ProviderInstanceId): ServerProvider => ({ /** * A server whose `dependent` instance reads `dependentReads` and `unrelated` reads `other`, all * through the real resolver, instance registry and provider registry. `vault` answers each read of - * a reference with the next queued value, repeating the last one. + * a reference with the next queued value, repeating the last one, after `beforeRead`. */ const setup = Effect.fn("SecretCheckTest.setup")(function* ( vault: Map>, dependentReads: ReadonlyArray = [wanted], + options: { + readonly dependentConfig?: unknown; + readonly beforeRead?: (reference: string) => Effect.Effect; + } = {}, ) { const reads: Array = []; const created: Array<{ instanceId: ProviderInstanceId; secrets: ReadonlyArray }> = []; + const createdConfigs: Array = []; const refreshed: Array = []; const secretDriver = defineSecretProviderDriver({ @@ -76,19 +84,27 @@ const setup = Effect.fn("SecretCheckTest.setup")(function* ( invalidReferenceMessage: "Use fake://item.", }), read: (_config, references) => - Effect.sync(() => - references.map((reference) => { - reads.push(reference); - const answers = vault.get(reference) ?? []; - return answers.length > 1 ? answers.shift() : answers[0]; - }), + Effect.forEach(references, (reference) => + (options.beforeRead?.(reference) ?? Effect.void).pipe( + Effect.andThen( + Effect.sync(() => { + reads.push(reference); + const answers = vault.get(reference) ?? []; + return answers.length > 1 ? answers.shift() : answers[0]; + }), + ), + ), ), }); const driver: AnyProviderDriver = { driverKind, metadata: { displayName: "Fake" }, - configSchema: Schema.Unknown, + configSchema: Schema.Struct({ + keys: Schema.Record(Schema.String, SettingsSecretValue).pipe( + Schema.withDecodingDefault(Effect.succeed({})), + ), + }), defaultConfig: () => ({}), create: (input) => Effect.sync(() => { @@ -96,6 +112,7 @@ const setup = Effect.fn("SecretCheckTest.setup")(function* ( instanceId: input.instanceId, secrets: input.environment.map((variable) => variable.value), }); + createdConfigs.push(input.config); const snapshot = providerSnapshot(input.instanceId); return { instanceId: input.instanceId, @@ -124,9 +141,9 @@ const setup = Effect.fn("SecretCheckTest.setup")(function* ( } satisfies ProviderInstance; }), }; - const instanceConfig = (secrets: ReadonlyArray) => ({ + const instanceConfig = (secrets: ReadonlyArray, config: unknown = {}) => ({ driver: driverKind, - config: {}, + config, environment: secrets.map((secret, index) => ({ name: `API_KEY_${index}`, value: secret, @@ -145,7 +162,7 @@ const setup = Effect.fn("SecretCheckTest.setup")(function* ( const { registry, mutator } = yield* ProviderInstanceRegistry.makeProviderInstanceRegistry({ drivers: [driver], configMap: { - [dependent]: instanceConfig(dependentReads), + [dependent]: instanceConfig(dependentReads, options.dependentConfig), [unrelated]: instanceConfig([other]), }, }).pipe(Effect.provide(resolverContext)); @@ -167,7 +184,7 @@ const setup = Effect.fn("SecretCheckTest.setup")(function* ( ); const secretCheck = yield* SecretCheck.SecretCheck.pipe(Effect.provide(services)); const resolver = yield* SecretResolver.SecretResolver.pipe(Effect.provide(resolverContext)); - return { secretCheck, registry, mutator, resolver, reads, created, refreshed }; + return { secretCheck, registry, mutator, resolver, reads, created, createdConfigs, refreshed }; }); const layerTest = Layer.mergeAll( @@ -274,4 +291,73 @@ it.layer(layerTest)("SecretCheck", (it) => { assert.deepStrictEqual(yield* server.registry.listUnavailable, []); }).pipe(Effect.scoped), ); + + it.effect("a check applies the value it read when a refresh re-reads that secret first", () => + Effect.forEach( + // A live instance holding the old value, and one parked on a locked vault. + ["wanted-old", undefined], + (initial) => + Effect.gen(function* () { + const vault = new Map>([ + [wanted.reference, [initial]], + [other.reference, ["other-key"]], + ]); + const readStarted = yield* Deferred.make(); + const release = yield* Deferred.make(); + let gate: Effect.Effect = Effect.void; + const server = yield* setup(vault, [wanted], { + beforeRead: () => { + const current = gate; + gate = Effect.void; + return current; + }, + }); + // The check reads the rotated value; the refresh's read after it finds the vault locked. + vault.set(wanted.reference, ["wanted-new", undefined]); + gate = Deferred.succeed(readStarted, undefined).pipe( + Effect.andThen(Deferred.await(release)), + ); + + const check = yield* server.secretCheck + .check(wanted) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(readStarted); + // The refresh takes the registry lock, then waits on the resolver for the check's read. + const refresh = yield* server.mutator + .reloadSecrets(dependent) + .pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.succeed(release, undefined); + + assert.deepStrictEqual(yield* Fiber.join(check), { _tag: "Resolved" }); + yield* Fiber.join(refresh); + assert.deepStrictEqual(server.created.at(-1), { + instanceId: dependent, + secrets: ["wanted-new"], + }); + assert.deepStrictEqual(yield* server.registry.listUnavailable, []); + }).pipe(Effect.scoped), + { discard: true }, + ), + ); + + it.effect("a check replaces only its own key of a record-valued config secret", () => + Effect.gen(function* () { + const vault = new Map>([ + [wanted.reference, ["wanted-old"]], + [locked.reference, ["locked-key"]], + [other.reference, ["other-key"]], + ]); + const server = yield* setup(vault, [], { + dependentConfig: { keys: { a: wanted, b: locked } }, + }); + vault.set(wanted.reference, ["wanted-new"]); + + assert.deepStrictEqual(yield* server.secretCheck.check(wanted), { _tag: "Resolved" }); + + assert.deepStrictEqual(server.created.at(-1), { instanceId: dependent, secrets: [] }); + assert.deepStrictEqual(server.createdConfigs.at(-1), { + keys: { a: "wanted-new", b: "locked-key" }, + }); + }).pipe(Effect.scoped), + ); }); diff --git a/apps/server/src/secrets/SecretCheck.ts b/apps/server/src/secrets/SecretCheck.ts index 74802195d1284..f2a6241eeb7a0 100644 --- a/apps/server/src/secrets/SecretCheck.ts +++ b/apps/server/src/secrets/SecretCheck.ts @@ -29,7 +29,7 @@ const make = Effect.gen(function* () { detail: `Could not read this secret from "${yield* resolver.label(reference.provider)}". Check the provider's settings and that its store is unlocked.`, } as const; } - yield* providerRegistry.applySecretAndRefresh(reference); + yield* providerRegistry.applySecretAndRefresh(reference, value); return { _tag: "Resolved" } as const; });