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/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..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. @@ -130,6 +148,36 @@ const readInstanceSecrets = Effect.fn("ProviderInstanceRegistry.readInstanceSecr return { secrets, unreadable }; }); +// Replaces only the checked reference's value in the secrets a live instance already holds, so +// another reference whose read failed earlier is not read again and does not park the instance. +const carryCheckedSecret = ( + driver: AnyProviderDriver | undefined, + entry: ProviderInstanceConfig, + current: InstanceSecrets, + { reference: checked, value }: CheckedSecret, +): InstanceSecrets => { + const matches = (reference: SecretReference) => sameReference(reference, checked); + // A live instance resolved every variable in order, so slots line up with the entry's variables + // by position; names may repeat. + const variables = entry.environment ?? []; + const replacedSlots = configSecretReferences(driver, entry).filter(({ reference }) => + matches(reference), + ); + return { + environment: current.environment.map((variable, index) => { + const source = variables[index]?.value; + return source !== undefined && typeof source !== "string" && matches(source) + ? { ...variable, value } + : variable; + }), + config: current.config.map((slot) => + replacedSlots.some(({ field, key }) => field === slot.field && key === slot.key) + ? { ...slot, value } + : slot, + ), + }; +}; + /** Conflicting saved flags must preserve any explicit disable. */ const resolveEntryEnabled = (entry: ProviderInstanceConfig, typedConfig: unknown): boolean => { const rawConfigEnabled = providerInstanceConfigEnabledFlag(entry.config); @@ -161,6 +209,7 @@ const buildEntry = Effect.fn("ProviderInstanceRegistry.buildEntry")(function* { - const { driversById, parentScope, instanceId, rawInstanceId, entry, deferSecrets } = input; + const { + driversById, + parentScope, + instanceId, + rawInstanceId, + entry, + deferSecrets, + carriedSecrets, + } = input; const blocked = input.blockedReason(entry.driver); if (blocked !== undefined) { return { @@ -235,12 +292,14 @@ const buildEntry = Effect.fn("ProviderInstanceRegistry.buildEntry")(function* 0) { @@ -311,9 +370,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 +425,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 +467,7 @@ const makeReconcile = (input: { rawInstanceId, entry, deferSecrets, + carriedSecrets: carried.get(instanceId), }); if (result.kind === "live") { builtEntries.set(instanceId, result.live); @@ -503,44 +565,87 @@ export const makeProviderInstanceRegistry = Effect.fn( ); // Rebuilds an instance only when it was unavailable or its secrets changed, so a refresh does - // not restart providers whose credentials are the same. - const reloadSecrets: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape["reloadSecrets"] = - Effect.fn("ProviderInstanceRegistry.reloadSecrets")( - function* (instanceId) { - const configMap = yield* Ref.get(configMapRef); - const targets = yield* Effect.filter(Object.entries(configMap), ([rawInstanceId, entry]) => - (instanceId === undefined || rawInstanceId === instanceId) && - secretReferencesOf(driversById, entry).length > 0 - ? isEntryEnabled(driversById, entry) - : Effect.succeed(false), - ); - if (targets.length === 0) return; + // not restart providers whose credentials are the same. Without `checked` the cached values are + // dropped first; with it, only that reference takes the checked value and the rest carry over. + const reloadWhere = Effect.fn("ProviderInstanceRegistry.reloadWhere")( + function* ( + select: (rawInstanceId: string, entry: ProviderInstanceConfig) => boolean, + checked?: CheckedSecret, + ) { + const configMap = yield* Ref.get(configMapRef); + const targets = yield* Effect.filter(Object.entries(configMap), ([rawInstanceId, entry]) => + select(rawInstanceId, entry) && secretReferencesOf(driversById, entry).length > 0 + ? isEntryEnabled(driversById, entry) + : Effect.succeed(false), + ); + if (targets.length === 0) return []; + if (checked === undefined) { const resolver = yield* SecretResolver.SecretResolver; const references = targets.flatMap(([, entry]) => secretReferencesOf(driversById, entry)); yield* resolver.invalidate(references); yield* resolver.read(references); - const live = yield* Ref.get(entries); - const rebuild = new Set(); - for (const [rawInstanceId, entry] of targets) { - const current = live.get(ProviderInstanceId.make(rawInstanceId)); - if (current === undefined) { - rebuild.add(ProviderInstanceId.make(rawInstanceId)); - continue; + } + const live = yield* Ref.get(entries); + const rebuild = new Set(); + const carried = new Map(); + for (const [rawInstanceId, entry] of targets) { + const instanceId = ProviderInstanceId.make(rawInstanceId); + const current = live.get(instanceId); + if (current === undefined) { + rebuild.add(instanceId); + if (checked !== undefined) { + const { secrets, unreadable } = yield* readInstanceSecrets( + driversById.get(entry.driver), + entry, + checked, + ); + if (unreadable.length === 0) carried.set(instanceId, secrets); } - const { secrets, unreadable } = yield* readInstanceSecrets( + continue; + } + if (checked !== undefined) { + const secrets = carryCheckedSecret( driversById.get(entry.driver), - entry, + current.entry, + current.secrets, + checked, ); - // A failed re-read keeps the working instance and the value it already holds. - if (unreadable.length === 0 && !Equal.equals(secrets, current.secrets)) { + if (!Equal.equals(secrets, current.secrets)) { rebuild.add(current.instance.instanceId); + carried.set(current.instance.instanceId, secrets); } + continue; } - yield* reconcileWithR(configMap, rebuild); - }, - (effect) => semaphore.withPermits(1)(effect), - Effect.provideContext(driverContext), - ); + const { secrets, unreadable } = yield* readInstanceSecrets( + driversById.get(entry.driver), + entry, + ); + // A failed re-read keeps the working instance and the value it already holds. + if (unreadable.length === 0 && !Equal.equals(secrets, current.secrets)) { + rebuild.add(current.instance.instanceId); + } + } + yield* reconcileWithR(configMap, rebuild, false, carried); + return targets.map(([rawInstanceId]) => ProviderInstanceId.make(rawInstanceId)); + }, + (effect) => semaphore.withPermits(1)(effect), + Effect.provideContext(driverContext), + ); + const reloadSecrets: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape["reloadSecrets"] = + (instanceId) => + reloadWhere( + (rawInstanceId) => instanceId === undefined || rawInstanceId === instanceId, + undefined, + ).pipe(Effect.asVoid); + const applySecret: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape["applySecret"] = + (reference, value) => + reloadWhere( + (_, entry) => + secretReferencesOf(driversById, entry).some((candidate) => + sameReference(candidate, reference), + ), + { reference, value }, + ); // Reading secrets can wait on a vault unlock, so the server does not wait for it to start. yield* semaphore.withPermits(1)( @@ -592,6 +697,7 @@ export const makeProviderInstanceRegistry = Effect.fn( const mutator: ProviderInstanceRegistryMutator.ProviderInstanceRegistryMutatorShape = { reconcile, reloadSecrets, + applySecret, }; return { registry, mutator }; 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..911a055d79625 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,14 @@ 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` 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>; } export class ProviderInstanceRegistryMutator extends Context.Service< diff --git a/apps/server/src/provider/ProviderRegistry.ts b/apps/server/src/provider/ProviderRegistry.ts index fb6aeaac26b63..3c534c333d75e 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,12 @@ export class ProviderRegistry extends Context.Service< instanceId?: ProviderInstanceId, ) => Effect.Effect>; + /** 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. */ readonly refreshAfterUpdate: ( provider: ProviderDriverKind, @@ -692,6 +699,15 @@ export const layer = Layer.effect( return yield* instanceId === undefined ? refresh() : refreshInstance(instanceId); }); + const applySecretAndRefresh = Effect.fn("applySecretAndRefresh")(function* ( + reference: SecretReference, + value: string, + ) { + const instanceIds = yield* instanceRegistryMutator.applySecret(reference, value); + yield* Effect.forEach(instanceIds, refreshInstance, { discard: true }); + return yield* Ref.get(providersRef); + }); + const refreshAfterUpdate = Effect.fn("refreshAfterUpdate")(function* ( provider: ProviderDriverKind, selectedInstanceId: ProviderInstanceId, @@ -1077,6 +1093,8 @@ export const layer = Layer.effect( refreshInstance(instanceId).pipe(Effect.catchCause(recoverRefreshFailure)), reloadSecretsAndRefresh: (instanceId?: ProviderInstanceId) => reloadSecretsAndRefresh(instanceId).pipe(Effect.catchCause(recoverRefreshFailure)), + applySecretAndRefresh: (reference: SecretReference, value: string) => + applySecretAndRefresh(reference, value).pipe(Effect.catchCause(recoverRefreshFailure)), refreshWorkspaceSnapshot: (input) => refreshWorkspaceSnapshot(input).pipe(Effect.catchCause(recoverRefreshFailure)), refreshAfterUpdate, 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..9c0bad4cd3111 --- /dev/null +++ b/apps/server/src/secrets/SecretCheck.test.ts @@ -0,0 +1,363 @@ +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, + SettingsSecretValue, + 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 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"; +import * as Schema from "effect/Schema"; +import * as Stream from "effect/Stream"; + +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 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"); + +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: [], +}); + +/** + * 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, 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({ + 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.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.Struct({ + keys: Schema.Record(Schema.String, SettingsSecretValue).pipe( + Schema.withDecodingDefault(Effect.succeed({})), + ), + }), + defaultConfig: () => ({}), + create: (input) => + Effect.sync(() => { + created.push({ + instanceId: input.instanceId, + secrets: input.environment.map((variable) => variable.value), + }); + createdConfigs.push(input.config); + 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; + }), + streamChanges: Stream.empty, + applyUsageLimits: () => Effect.void, + }, + orchestrationAdapter: {} as ProviderInstance["orchestrationAdapter"], + textGeneration: {} as ProviderInstance["textGeneration"], + } satisfies ProviderInstance; + }), + }; + const instanceConfig = (secrets: ReadonlyArray, config: unknown = {}) => ({ + driver: driverKind, + config, + environment: secrets.map((secret, index) => ({ + name: `API_KEY_${index}`, + 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(dependentReads, options.dependentConfig), + [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 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)); + const resolver = yield* SecretResolver.SecretResolver.pipe(Effect.provide(resolverContext)); + return { secretCheck, registry, mutator, resolver, reads, created, createdConfigs, refreshed }; +}); + +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* server.registry.listUnavailable).map((entry) => entry.instanceId), + [dependent], + ); + const createdBefore = server.created.length; + const readsBefore = server.reads.length; + + // 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.notInclude(JSON.stringify(result), "wanted-key"); + assert.deepStrictEqual(server.reads.slice(readsBefore), [wanted.reference]); + assert.deepStrictEqual(server.created.slice(createdBefore), [ + { instanceId: dependent, secrets: ["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, "Personal"); + assert.deepStrictEqual(server.created.slice(createdBefore), []); + assert.deepStrictEqual(server.refreshed, []); + assert.deepStrictEqual( + (yield* server.registry.listUnavailable).map((entry) => entry.instanceId), + [dependent], + ); + }).pipe(Effect.scoped), + ); + + it.effect("a failed check keeps the value the server already holds", () => + Effect.gen(function* () { + const vault = new Map>([ + [wanted.reference, ["wanted-key"]], + [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"); + + assert.deepStrictEqual(yield* server.resolver.read([locked]), ["locked-key"]); + assert.deepStrictEqual(server.reads.slice(readsBefore), [locked.reference]); + }).pipe(Effect.scoped), + ); + + it.effect("a check applies to an instance whose other secret failed a later refresh", () => + 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]); + // A Settings refresh while the vault is locked keeps the live instance and caches the failure. + yield* server.mutator.reloadSecrets(dependent); + vault.set(wanted.reference, ["wanted-new"]); + + assert.deepStrictEqual(yield* server.secretCheck.check(wanted), { _tag: "Resolved" }); + + assert.deepStrictEqual(server.created.at(-1), { + instanceId: dependent, + secrets: ["wanted-new", "locked-key"], + }); + 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 new file mode 100644 index 0000000000000..f2a6241eeb7a0 --- /dev/null +++ b/apps/server/src/secrets/SecretCheck.ts @@ -0,0 +1,39 @@ +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) { + const [value] = yield* resolver.read([reference], { fresh: true }); + if (value === undefined || value.trim() === "") { + return { + _tag: "Failed", + 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, value); + 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/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/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..98d8e0f9e52e2 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,97 @@ 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; + readonly disabled?: boolean | undefined; +}) { + 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}`; + 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, + ); + }; + + if (!supported) return null; + 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 +220,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 +262,52 @@ 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 && + rejectedReference === 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/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. 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/environment.ts b/packages/contracts/src/environment.ts index 627b9529a6cc5..d0f1cc2b280c1 100644 --- a/packages/contracts/src/environment.ts +++ b/packages/contracts/src/environment.ts @@ -97,6 +97,8 @@ export const ExecutionEnvironmentCapabilities = Schema.Struct({ storageCleanupRun: Schema.optionalKey(Schema.Boolean), databaseStorage: Schema.optionalKey(Schema.Boolean), otlpEndpointCheck: Schema.optionalKey(Schema.Boolean), + /** Serves `server.checkSecretSource`; hide the secret check where it is absent. */ + secretSourceCheck: Schema.optionalKey(Schema.Boolean), projectWorktreeCleanup: Schema.optionalKey(Schema.Boolean), settledWorktreeCleanup: Schema.optionalKey(Schema.Boolean), worktreesDirectory: Schema.optionalKey(Schema.Boolean), diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index 6465558d5fe03..1bb7d4d3d91ed 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, @@ -515,6 +517,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", @@ -838,6 +841,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, @@ -2034,6 +2043,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, + 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));