From 45baba572cd58d8b9e06e75e5c81a4eb35c74eb1 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Thu, 29 Oct 2026 10:26:02 -0700 Subject: [PATCH] fix(settings): servers check secret provider writes and keep unknown configs to themselves Follow-up to #25709. The server now rejects a secret provider write whose kind it does not know or whose config fails that kind's schema, strips the config of unknown kinds before settings reach clients, and advertises secretProviderSettings so the Secrets page tells you to update an older server instead of losing writes. Co-Authored-By: Claude Opus 5.5 --- apps/server/src/serverSettings.test.ts | 18 ++++++++++++++ apps/server/src/serverSettings.ts | 17 ++++++++++++- apps/server/src/ws.ts | 1 + .../components/settings/SecretsSettings.tsx | 11 ++++++++- packages/contracts/src/server.ts | 2 ++ packages/contracts/src/settings.test.ts | 13 +++++----- packages/contracts/src/settings.ts | 24 +++++++++++++++++-- 7 files changed, 75 insertions(+), 11 deletions(-) diff --git a/apps/server/src/serverSettings.test.ts b/apps/server/src/serverSettings.test.ts index bd985cab203ac..29dea381e759a 100644 --- a/apps/server/src/serverSettings.test.ts +++ b/apps/server/src/serverSettings.test.ts @@ -235,6 +235,24 @@ it.layer(NodeServices.layer)("server settings", (it) => { }).pipe(Effect.provide(layerServerSettings())), ); + it.effect("sends clients a known secret provider's config but never an unknown kind's", () => + Effect.gen(function* () { + const settings = yield* decodeServerSettings({ + secretProviders: { + personal: { kind: "one_password", label: "Personal", config: { account: "my" } }, + vault: { kind: "openbao", label: "Vault", config: { token: "bao-token" } }, + }, + }); + assert.deepStrictEqual( + ServerSettingsModule.redactServerSettingsForClient(settings).secretProviders, + { + personal: { kind: "one_password", label: "Personal", config: { account: "my" } }, + vault: { kind: "openbao", label: "Vault" }, + }, + ); + }), + ); + it.effect("saves through a symlinked settings file without replacing the link", () => Effect.gen(function* () { const config = yield* ServerConfig.ServerConfig; diff --git a/apps/server/src/serverSettings.ts b/apps/server/src/serverSettings.ts index a1e0d34c7dd53..e386b95cf506c 100644 --- a/apps/server/src/serverSettings.ts +++ b/apps/server/src/serverSettings.ts @@ -7,6 +7,7 @@ import { type ProviderInstanceConfig, type ProviderInstanceMutation, defaultInstanceIdForDriver, + isKnownSecretProviderKind, isProviderDriverKind, ProviderDriverKind, ProviderInstanceId, @@ -128,11 +129,25 @@ const normalizeServerSettings = ( ); export function redactServerSettingsForClient(settings: ServerSettings): ServerSettings { - return mapSettingsSecrets(settings, (secret) => + const redacted = mapSettingsSecrets(settings, (secret) => secret.sensitive ? { value: "", redacted: secret.redacted || secret.value.length > 0 } : { value: secret.value, redacted: false }, ); + return { ...redacted, secretProviders: withoutUnknownSecretProviderConfigs(redacted) }; +} + +// A newer kind's config may hold credentials this build cannot recognize, so it never leaves the server. +function withoutUnknownSecretProviderConfigs( + settings: ServerSettings, +): ServerSettings["secretProviders"] { + return Object.fromEntries( + Object.entries(settings.secretProviders).map(([id, entry]) => { + if (isKnownSecretProviderKind(entry.kind)) return [id, entry]; + const { config: _config, ...envelope } = entry; + return [id, envelope]; + }), + ) as ServerSettings["secretProviders"]; } export function applyProviderInstanceMutation( diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 8c8509550bc11..86c7a4db56af4 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -1791,6 +1791,7 @@ export const layerWsRpc = ( threadSnapshotPagination: true, threadFind: true, threadFindProgressive: true, + secretProviderSettings: true, ...Option.match(scratchWorkspaceRoot, { onNone: () => ({}), onSome: (root) => ({ scratchWorkspaceRoot: root }), diff --git a/apps/web/src/components/settings/SecretsSettings.tsx b/apps/web/src/components/settings/SecretsSettings.tsx index e9efe2d85f6ed..2ff329ca6b73a 100644 --- a/apps/web/src/components/settings/SecretsSettings.tsx +++ b/apps/web/src/components/settings/SecretsSettings.tsx @@ -35,9 +35,18 @@ const SECRET_PROVIDER_KINDS: Readonly< export function SecretsSettingsPanel() { const { scope, environment } = useSettingsScope(); + const connected = environment?.connection.phase === "connected"; return ( - {environment?.connection.phase === "connected" ? ( + {connected && + environment.serverConfig !== null && + environment.serverConfig.secretProviderSettings !== true ? ( + +

+ Update T3 Code on {environment.label} to manage its secret providers. +

+ + ) : connected ? ( { @@ -155,13 +153,14 @@ describe("ServerSettings secret providers", () => { ).toThrow(); }); - it("rejects a 1Password account op could misread", () => { + it("rejects writes of an unknown kind or an account op could misread", () => { + const write = (entry: unknown) => () => + decodeServerSettingsPatch({ secretProviders: { personal: entry } }); for (const account of ["", "my account", "--help"]) { - expect(() => decodeOnePasswordConfig({ account })).toThrow(); + expect(write(onePassword(account))).toThrow(); } - expect(decodeOnePasswordConfig({ account: "my.1password.com" }).account).toBe( - "my.1password.com", - ); + expect(write({ kind: "one_password", label: "Personal" })).toThrow(); + expect(write({ kind: "openbao", label: "Vault", config: { address: "https://x" } })).toThrow(); }); }); diff --git a/packages/contracts/src/settings.ts b/packages/contracts/src/settings.ts index 1339a9322fa3a..c7d61aeb5ee21 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -859,7 +859,7 @@ export const UsageLimitSourceConfig = Schema.Struct({ }); export type UsageLimitSourceConfig = typeof UsageLimitSourceConfig.Type; -// Final once shipped: a key or field that fails here fails the whole settings decode on older builds. +// Final once shipped: older builds drop entries whose key fails here, and erase them on their next save. // Unlike provider slugs it may start with a digit, so a provider named "1Password" fits. const SecretProviderSlug = TrimmedNonEmptyString.check( Schema.isMaxLength(64), @@ -903,11 +903,31 @@ export const SecretProviderConfig = Schema.Struct({ }); export type SecretProviderConfig = typeof SecretProviderConfig.Type; +/** Kinds this build can resolve. Their `config` holds no credentials, so clients may read it. */ +const SECRET_PROVIDER_KIND_CONFIG_GUARDS: Readonly boolean>> = { + one_password: Schema.is(OnePasswordSecretProviderConfig), +}; + +export function isKnownSecretProviderKind(kind: string): boolean { + return Object.hasOwn(SECRET_PROVIDER_KIND_CONFIG_GUARDS, kind); +} + // Limits that may change later are enforced on writes only, never on the stored envelope. +// Writes must be a kind this build knows with a config it accepts, so an older server +// rejects a newer kind instead of storing what it cannot check. const SecretProviderConfigWrite = Schema.Struct({ ...SecretProviderConfig.fields, label: SecretProviderLabel, -}); +}).check( + Schema.makeFilter((entry) => { + if (!isKnownSecretProviderKind(entry.kind)) { + return `Unsupported secret provider kind: ${entry.kind}.`; + } + return SECRET_PROVIDER_KIND_CONFIG_GUARDS[entry.kind]!(entry.config) + ? true + : `Invalid ${entry.kind} secret provider config.`; + }), +); export const SourceControlHostSettings = Schema.Record(Schema.String, Schema.Unknown); export type SourceControlHostSettings = typeof SourceControlHostSettings.Type;