From a0a321b2916d642d727a66687a3a676fe7d79604 Mon Sep 17 00:00:00 2001 From: Maxwell Young Date: Sat, 5 Sep 2026 16:27:21 +0800 Subject: [PATCH 1/2] fix(server): preserve inline provider secrets on redacted saves --- apps/server/src/serverSettings.test.ts | 114 +++++++++++++++++++++++++ apps/server/src/serverSettings.ts | 17 +++- 2 files changed, 128 insertions(+), 3 deletions(-) diff --git a/apps/server/src/serverSettings.test.ts b/apps/server/src/serverSettings.test.ts index 4526e2c988a4..688d9da786e7 100644 --- a/apps/server/src/serverSettings.test.ts +++ b/apps/server/src/serverSettings.test.ts @@ -1039,6 +1039,120 @@ it.layer(NodeServices.layer)("server settings", (it) => { }).pipe(Effect.provide(makeServerSettingsLayer())), ); + it.effect("keeps the inline value on disk when secret migration fails", () => { + const cause = new ServerSecretStore.SecretStorePersistError({ + resource: "provider environment secret", + cause: new Error("Secret storage unavailable"), + }); + const secretLayer = Layer.effect( + ServerSecretStore.ServerSecretStore, + Effect.map(ServerSecretStore.ServerSecretStore, (store) => ({ + ...store, + set: () => Effect.fail(cause), + })), + ).pipe(Layer.provide(ServerSecretStore.layer)); + const settingsLayer = ServerSettingsModule.layer.pipe( + Layer.provide(secretLayer), + Layer.provideMerge(Layer.fresh(SqlitePersistenceMemory)), + Layer.provideMerge( + Layer.fresh( + ServerConfig.layerTest(process.cwd(), { + prefix: "t3code-inline-secret-failure-test-", + }), + ), + ), + ); + return Effect.gen(function* () { + const instanceId = ProviderInstanceId.make("codex_personal"); + const service = yield* ServerSettingsModule.ServerSettingsService; + const config = yield* ServerConfig.ServerConfig; + const fs = yield* FileSystem.FileSystem; + const original = + '{"providerInstances":{"codex_personal":{"driver":"codex","environment":[{"name":"API_TOKEN","value":"inline-test-token","sensitive":true}],"config":{}}}}'; + yield* fs.writeFileString(config.settingsPath, original); + const error = yield* Effect.flip( + service.updateSettings({ + providerInstances: { + [instanceId]: { + driver: ProviderDriverKind.make("codex"), + environment: [{ name: "API_TOKEN", value: "", sensitive: true, valueRedacted: true }], + config: {}, + }, + }, + }), + ); + assert.equal(error.operation, "write-secret"); + assert.strictEqual(error.cause, cause); + assert.equal(yield* fs.readFileString(config.settingsPath), original); + const settings = yield* service.getSettings; + assert.equal( + settings.providerInstances[instanceId]?.environment?.[0]?.value, + "inline-test-token", + ); + }).pipe(Effect.provide(settingsLayer)); + }); + + for (const { label, variable, expected } of [ + { + label: "preserves an inline secret on a redacted settings save", + variable: { name: "API_TOKEN", value: "", sensitive: true, valueRedacted: true }, + expected: "inline-test-token", + }, + { + label: "replaces an inline secret with an explicit value", + variable: { name: "API_TOKEN", value: "replacement-test-token", sensitive: true }, + expected: "replacement-test-token", + }, + { + label: "clears an inline secret with an explicit empty value", + variable: { name: "API_TOKEN", value: "", sensitive: true }, + expected: "", + }, + ]) { + it.effect(label, () => + Effect.gen(function* () { + const instanceId = ProviderInstanceId.make("codex_personal"); + const serverSettings = yield* ServerSettingsModule.ServerSettingsService; + const serverConfig = yield* ServerConfig.ServerConfig; + const fileSystem = yield* FileSystem.FileSystem; + yield* fileSystem.writeFileString( + serverConfig.settingsPath, + '{"providerInstances":{"codex_personal":{"driver":"codex","environment":[{"name":"API_TOKEN","value":"inline-test-token","sensitive":true}],"config":{}}}}', + ); + const initial = yield* serverSettings.getSettings; + assert.equal( + initial.providerInstances[instanceId]?.environment?.[0]?.value, + "inline-test-token", + ); + + const next = yield* serverSettings.updateSettings({ + providerInstances: { + [instanceId]: { + driver: ProviderDriverKind.make("codex"), + displayName: "Renamed provider", + environment: [variable], + config: {}, + }, + }, + }); + assert.equal(next.providerInstances[instanceId]?.environment?.[0]?.value, expected); + const raw = yield* fileSystem.readFileString(serverConfig.settingsPath); + assert.notInclude(raw, "inline-test-token"); + assert.notInclude(raw, "replacement-test-token"); + + const reloaded = yield* Effect.gen(function* () { + const fresh = yield* ServerSettingsModule.ServerSettingsService; + return yield* fresh.getSettings; + }).pipe( + Effect.provide( + Layer.fresh(ServerSettingsModule.layer).pipe(Layer.provide(ServerSecretStore.layer)), + ), + ); + assert.equal(reloaded.providerInstances[instanceId]?.environment?.[0]?.value, expected); + }).pipe(Effect.provide(makeServerSettingsLayer())), + ); + } + it.effect("stores sensitive provider instance environment values outside settings.json", () => Effect.gen(function* () { const serverSettings = yield* ServerSettingsModule.ServerSettingsService; diff --git a/apps/server/src/serverSettings.ts b/apps/server/src/serverSettings.ts index 5f2550534883..c41407f1af46 100644 --- a/apps/server/src/serverSettings.ts +++ b/apps/server/src/serverSettings.ts @@ -609,9 +609,20 @@ const make = Effect.gen(function* () { } nextSecretKeys.add(secretName); - if (!variable.valueRedacted) { - if (variable.value.length > 0) { - yield* secretStore.set(secretName, textEncoder.encode(variable.value)).pipe( + // A redacted save must migrate an inline value before replacing it with a placeholder. + const inlineValue = variable.valueRedacted + ? current.providerInstances[ProviderInstanceId.make(instanceId)]?.environment?.find( + (previous) => + previous.name === variable.name && + previous.sensitive && + !previous.valueRedacted && + previous.value.length > 0, + )?.value + : undefined; + const value = inlineValue ?? variable.value; + if (!variable.valueRedacted || inlineValue !== undefined) { + if (value.length > 0) { + yield* secretStore.set(secretName, textEncoder.encode(value)).pipe( Effect.mapError( (cause) => new ServerSettingsError({ From 5b1c0f17f82550304247a4e902488d9c720f8f2f Mon Sep 17 00:00:00 2001 From: Maxwell Young Date: Sat, 5 Sep 2026 16:34:29 +0800 Subject: [PATCH 2/2] fix(server): preserve last inline value for duplicate env names --- apps/server/src/serverSettings.test.ts | 14 +++++++++++--- apps/server/src/serverSettings.ts | 18 +++++++++--------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/apps/server/src/serverSettings.test.ts b/apps/server/src/serverSettings.test.ts index 688d9da786e7..12959ea6f5d2 100644 --- a/apps/server/src/serverSettings.test.ts +++ b/apps/server/src/serverSettings.test.ts @@ -1092,12 +1092,18 @@ it.layer(NodeServices.layer)("server settings", (it) => { }).pipe(Effect.provide(settingsLayer)); }); - for (const { label, variable, expected } of [ + for (const { label, variable, expected, duplicate } of [ { label: "preserves an inline secret on a redacted settings save", variable: { name: "API_TOKEN", value: "", sensitive: true, valueRedacted: true }, expected: "inline-test-token", }, + { + label: "preserves the effective last inline secret when names are duplicated", + variable: { name: "API_TOKEN", value: "", sensitive: true, valueRedacted: true }, + expected: "last-inline-test-token", + duplicate: true, + }, { label: "replaces an inline secret with an explicit value", variable: { name: "API_TOKEN", value: "replacement-test-token", sensitive: true }, @@ -1117,7 +1123,9 @@ it.layer(NodeServices.layer)("server settings", (it) => { const fileSystem = yield* FileSystem.FileSystem; yield* fileSystem.writeFileString( serverConfig.settingsPath, - '{"providerInstances":{"codex_personal":{"driver":"codex","environment":[{"name":"API_TOKEN","value":"inline-test-token","sensitive":true}],"config":{}}}}', + duplicate + ? '{"providerInstances":{"codex_personal":{"driver":"codex","environment":[{"name":"API_TOKEN","value":"inline-test-token","sensitive":true},{"name":"API_TOKEN","value":"last-inline-test-token","sensitive":true}],"config":{}}}}' + : '{"providerInstances":{"codex_personal":{"driver":"codex","environment":[{"name":"API_TOKEN","value":"inline-test-token","sensitive":true}],"config":{}}}}', ); const initial = yield* serverSettings.getSettings; assert.equal( @@ -1130,7 +1138,7 @@ it.layer(NodeServices.layer)("server settings", (it) => { [instanceId]: { driver: ProviderDriverKind.make("codex"), displayName: "Renamed provider", - environment: [variable], + environment: duplicate ? [variable, variable] : [variable], config: {}, }, }, diff --git a/apps/server/src/serverSettings.ts b/apps/server/src/serverSettings.ts index c41407f1af46..ee4dedc2a9c0 100644 --- a/apps/server/src/serverSettings.ts +++ b/apps/server/src/serverSettings.ts @@ -609,16 +609,16 @@ const make = Effect.gen(function* () { } nextSecretKeys.add(secretName); - // A redacted save must migrate an inline value before replacing it with a placeholder. - const inlineValue = variable.valueRedacted - ? current.providerInstances[ProviderInstanceId.make(instanceId)]?.environment?.find( - (previous) => - previous.name === variable.name && - previous.sensitive && - !previous.valueRedacted && - previous.value.length > 0, - )?.value + // Match the provider environment's last-value-wins behavior for duplicate names. + const previous = variable.valueRedacted + ? current.providerInstances[ProviderInstanceId.make(instanceId)]?.environment?.findLast( + (entry) => entry.name === variable.name, + ) : undefined; + const inlineValue = + previous?.sensitive && !previous.valueRedacted && previous.value.length > 0 + ? previous.value + : undefined; const value = inlineValue ?? variable.value; if (!variable.valueRedacted || inlineValue !== undefined) { if (value.length > 0) {