From 705c9499f2fa56a7874d40f7618ad5ac7a877b1b Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Sat, 3 Oct 2026 19:09:16 -0400 Subject: [PATCH 01/12] feat(server): 1Password provider secrets name the account they live in Signed-off-by: Yordis Prieto --- .../orchestration-v2/ProviderAdapterDriver.ts | 9 +- .../ProviderAdapterRegistry.test.ts | 12 +- .../ProviderAdapterRegistry.ts | 3 +- .../server/src/project/AgentSessionScanner.ts | 6 +- .../src/provider/Drivers/CodexDriver.test.ts | 4 +- .../src/provider/Drivers/GrokDriver.test.ts | 2 +- .../Layers/ProviderInstanceRegistryLive.ts | 2 +- .../provider/Layers/ProviderRegistry.test.ts | 31 +- .../Layers/ProviderSecretResolverLive.test.ts | 144 ++++++++- .../Layers/ProviderSecretResolverLive.ts | 151 ++++++--- apps/server/src/provider/ProviderDriver.ts | 4 +- .../ProviderInstanceEnvironment.test.ts | 42 ++- .../provider/ProviderInstanceEnvironment.ts | 38 ++- .../provider/ProviderSecretReference.test.ts | 65 +++- .../src/provider/ProviderSecretReference.ts | 87 +++-- .../Services/ProviderSecretResolver.ts | 43 ++- .../acp/AcpRegistryAuthenticationState.ts | 9 +- .../src/provider/providerInstallation.ts | 9 +- apps/server/src/serverSettings.test.ts | 42 +++ apps/server/src/serverSettings.ts | 24 +- apps/server/src/terminal/Manager.ts | 10 +- apps/server/src/usage/UsageService.ts | 10 +- .../settings/ProviderInstanceCard.tsx | 303 +++++++++++++----- docs/internals/providers.md | 18 +- docs/user/provider-secrets.md | 37 ++- .../contracts/src/providerInstance.test.ts | 72 +++++ packages/contracts/src/providerInstance.ts | 97 +++++- 27 files changed, 1018 insertions(+), 256 deletions(-) diff --git a/apps/server/src/orchestration-v2/ProviderAdapterDriver.ts b/apps/server/src/orchestration-v2/ProviderAdapterDriver.ts index 765abd21af0f..9af3843cd62e 100644 --- a/apps/server/src/orchestration-v2/ProviderAdapterDriver.ts +++ b/apps/server/src/orchestration-v2/ProviderAdapterDriver.ts @@ -1,12 +1,9 @@ -import { - ProviderDriverKind, - ProviderInstanceId, - type ProviderInstanceEnvironment, -} from "@t3tools/contracts"; +import { ProviderDriverKind, ProviderInstanceId } from "@t3tools/contracts"; import * as Schema from "effect/Schema"; import type * as Effect from "effect/Effect"; import type * as Scope from "effect/Scope"; +import type { ResolvedProviderEnvironment } from "../provider/ProviderInstanceEnvironment.ts"; import type { ProviderAdapterV2Shape } from "./ProviderAdapter.ts"; export class ProviderAdapterDriverCreateError extends Schema.TaggedError()( @@ -27,7 +24,7 @@ export interface ProviderAdapterDriverCreateInput { readonly instanceId: ProviderInstanceId; readonly displayName: string | undefined; readonly accentColor?: string | undefined; - readonly environment: ProviderInstanceEnvironment; + readonly environment: ResolvedProviderEnvironment; readonly enabled: boolean; readonly config: Config; } diff --git a/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts b/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts index c2cfdbebca7b..ee42e713cc26 100644 --- a/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts +++ b/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts @@ -347,13 +347,13 @@ it.effect("opens v2 sessions with resolved secrets and rebuilds them when a secr Effect.gen(function* () { const variables = []; const unresolved = []; - for (const variable of environment ?? []) { - if (variable.value === apiKeyReference) { - variables.push({ ...variable, value: yield* Ref.get(apiKey) }); - } else if (variable.value.startsWith("op://")) { - unresolved.push(variable.name); + for (const { name, value } of environment ?? []) { + if (value === apiKeyReference) { + variables.push({ name, value: yield* Ref.get(apiKey) }); + } else if (typeof value !== "string" || value.startsWith("op://")) { + unresolved.push(name); } else { - variables.push(variable); + variables.push({ name, value }); } } return { variables, unresolved }; diff --git a/apps/server/src/orchestration-v2/ProviderAdapterRegistry.ts b/apps/server/src/orchestration-v2/ProviderAdapterRegistry.ts index 5279100e6206..79649f1e3e4d 100644 --- a/apps/server/src/orchestration-v2/ProviderAdapterRegistry.ts +++ b/apps/server/src/orchestration-v2/ProviderAdapterRegistry.ts @@ -13,6 +13,7 @@ import * as Layer from "effect/Layer"; import * as Schema from "effect/Schema"; import * as Scope from "effect/Scope"; +import { literalProviderInstanceEnvironment } from "../provider/ProviderInstanceEnvironment.ts"; import * as ProviderInstanceRegistry from "../provider/Services/ProviderInstanceRegistry.ts"; import { ProviderAdapterDriverCreateError, @@ -271,7 +272,7 @@ const createAdapterEntryFromConfigEntry = Effect.fn( instanceId: input.instanceId, displayName: input.entry.displayName, accentColor: input.entry.accentColor, - environment: input.entry.environment ?? [], + environment: literalProviderInstanceEnvironment(input.entry.environment), enabled: input.entry.enabled ?? decodedConfigEnabled(typedConfig) ?? true, config: typedConfig, }) diff --git a/apps/server/src/project/AgentSessionScanner.ts b/apps/server/src/project/AgentSessionScanner.ts index bc30691784db..e494be5fc8a4 100644 --- a/apps/server/src/project/AgentSessionScanner.ts +++ b/apps/server/src/project/AgentSessionScanner.ts @@ -50,6 +50,7 @@ import { normalizeProjectPathForComparison } from "@t3tools/shared/path"; import * as ServerConfig from "../config.ts"; import * as ProjectStore from "../orchestration-v2/ProjectStore.ts"; import { resolveCodexHomeLayout } from "../provider/Drivers/CodexHomeLayout.ts"; +import { literalProviderInstanceEnvironment } from "../provider/ProviderInstanceEnvironment.ts"; import { expandHomePath } from "../pathExpansion.ts"; import * as ServerSettings from "../serverSettings.ts"; import { @@ -1127,8 +1128,9 @@ export const make = Effect.gen(function* () { for (const { instanceId, config: instance } of instances) { const homeVariable = source === "claudeAgent" ? "CLAUDE_CONFIG_DIR" : "CODEX_HOME"; const environmentHome = - instance.environment?.findLast((variable) => variable.name === homeVariable)?.value ?? - hostEnvironment[homeVariable]; + literalProviderInstanceEnvironment(instance.environment).findLast( + (variable) => variable.name === homeVariable, + )?.value ?? hostEnvironment[homeVariable]; let homePath: string; if (source === "claudeAgent") { diff --git a/apps/server/src/provider/Drivers/CodexDriver.test.ts b/apps/server/src/provider/Drivers/CodexDriver.test.ts index 4d1a4ce3519f..fcdacfe1a1ab 100644 --- a/apps/server/src/provider/Drivers/CodexDriver.test.ts +++ b/apps/server/src/provider/Drivers/CodexDriver.test.ts @@ -152,7 +152,7 @@ it.layer(testLayer)("CodexDriver", (it) => { instanceId, displayName: "Restored account", enabled: true, - environment: [{ name: "OPENAI_API_KEY", value: "ambient-key", sensitive: true }], + environment: [{ name: "OPENAI_API_KEY", value: "ambient-key" }], config: { ...CodexDriver.defaultConfig(), setupMode: "managed", homePath: sharedHome }, }).pipe( Effect.provideService( @@ -546,7 +546,7 @@ it.layer(testLayer)("CodexDriver", (it) => { instanceId: ProviderInstanceId.make("codex-mise-shim"), displayName: "Codex shim test", enabled: false, - environment: [{ name: "PATH", value: lookupPath, sensitive: false }], + environment: [{ name: "PATH", value: lookupPath }], config: { ...CodexDriver.defaultConfig(), binaryPath: fixture.commandName, diff --git a/apps/server/src/provider/Drivers/GrokDriver.test.ts b/apps/server/src/provider/Drivers/GrokDriver.test.ts index a9d6791a3e50..fc08006b4c31 100644 --- a/apps/server/src/provider/Drivers/GrokDriver.test.ts +++ b/apps/server/src/provider/Drivers/GrokDriver.test.ts @@ -65,7 +65,7 @@ it.layer(testLayer)("GrokDriver", (it) => { instanceId: ProviderInstanceId.make("grok-update"), displayName: "Grok test", enabled: false, - environment: [{ name: "GROK_HOME", value: grokHome, sensitive: false }], + environment: [{ name: "GROK_HOME", value: grokHome }], config: { ...GrokDriver.defaultConfig(), binaryPath }, }); diff --git a/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts b/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts index 0d860e383068..594fe83e5239 100644 --- a/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts +++ b/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts @@ -240,7 +240,7 @@ const buildEntry = (input: { instanceId, displayName: entry.displayName, accentColor: entry.accentColor, - environment: resolvedEnvironment.variables ?? [], + environment: resolvedEnvironment.variables, enabled: resolveEntryEnabled(entry, typedConfig), config: typedConfig, }) diff --git a/apps/server/src/provider/Layers/ProviderRegistry.test.ts b/apps/server/src/provider/Layers/ProviderRegistry.test.ts index d01966f489bb..260b6ef1931b 100644 --- a/apps/server/src/provider/Layers/ProviderRegistry.test.ts +++ b/apps/server/src/provider/Layers/ProviderRegistry.test.ts @@ -65,6 +65,7 @@ import { } from "../providerStatusCache.ts"; import { COMPACT_SLASH_COMMAND } from "../providerSnapshot.ts"; import type { ProviderInstance } from "../ProviderDriver.ts"; +import { literalProviderInstanceEnvironment } from "../ProviderInstanceEnvironment.ts"; import * as ProviderInstanceRegistry from "../Services/ProviderInstanceRegistry.ts"; import * as ProviderRegistry from "../Services/ProviderRegistry.ts"; import { makeManualOnlyProviderMaintenanceCapabilities } from "../providerMaintenance.ts"; @@ -1609,7 +1610,11 @@ it.layer(Layer.mergeAll(TestNodeServices, ServerSettingsModule.layerTest(), Test const invalidations = yield* Ref.make(0); const rebuiltIds = yield* Ref.make>([]); const secretResolverLayer = Layer.succeed(ProviderSecretResolver, { - resolve: (environment) => Effect.succeed({ variables: environment, unresolved: [] }), + resolve: (environment) => + Effect.succeed({ + variables: literalProviderInstanceEnvironment(environment), + unresolved: [], + }), prime: () => Effect.void, invalidate: Ref.update(invalidations, (count) => count + 1), }); @@ -1687,7 +1692,11 @@ it.layer(Layer.mergeAll(TestNodeServices, ServerSettingsModule.layerTest(), Test const rebuiltIds = yield* Ref.make>([]); const secretResolverLayer = Layer.succeed(ProviderSecretResolver, { - resolve: (environment) => Effect.succeed({ variables: environment, unresolved: [] }), + resolve: (environment) => + Effect.succeed({ + variables: literalProviderInstanceEnvironment(environment), + unresolved: [], + }), prime: () => Effect.void, invalidate: Effect.void, }); @@ -1764,9 +1773,16 @@ it.layer(Layer.mergeAll(TestNodeServices, ServerSettingsModule.layerTest(), Test const primed = yield* Ref.make>>([]); const rebuiltIds = yield* Ref.make>([]); const secretResolverLayer = Layer.succeed(ProviderSecretResolver, { - resolve: (environment) => Effect.succeed({ variables: environment, unresolved: [] }), + resolve: (environment) => + Effect.succeed({ + variables: literalProviderInstanceEnvironment(environment), + unresolved: [], + }), prime: (references) => - Ref.update(primed, (previous) => [...previous, references]).pipe(Effect.asVoid), + Ref.update(primed, (previous) => [ + ...previous, + references.map((secret) => secret.reference), + ]).pipe(Effect.asVoid), invalidate: Effect.void, }); const environmentFor = (reference: string) => [ @@ -2944,12 +2960,15 @@ it.layer(Layer.mergeAll(TestNodeServices, ServerSettingsModule.layerTest(), Test const recordingSecretResolverLayer = Layer.succeed(ProviderSecretResolver, { resolve: (environment) => Ref.update(calls, (previous) => [...previous, "resolve"]).pipe( - Effect.as({ variables: environment, unresolved: [] }), + Effect.as({ + variables: literalProviderInstanceEnvironment(environment), + unresolved: [], + }), ), prime: (references) => Ref.update(calls, (previous) => [ ...previous, - `prime:${Array.from(references).join(",")}`, + `prime:${references.map((secret) => secret.reference).join(",")}`, ]).pipe(Effect.asVoid), invalidate: Effect.void, }); diff --git a/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts b/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts index a7d8cba5ff49..7084be855f20 100644 --- a/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts +++ b/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts @@ -10,6 +10,7 @@ import { ChildProcessSpawner } from "effect/unstable/process"; import * as NodeFileSystem from "@effect/platform-node/NodeFileSystem"; import * as NodeFS from "node:fs"; +import { OnePasswordSecretReference } from "../ProviderSecretReference.ts"; import { ProviderSecretResolverLive } from "./ProviderSecretResolverLive.ts"; import { ProviderSecretResolver } from "../Services/ProviderSecretResolver.ts"; @@ -18,6 +19,10 @@ const decodeEnvironment = Schema.decodeSync(ProviderInstanceEnvironment); const TOKEN_REFERENCE = "op://Private/claude-code/credential"; +/** A plain-string `op://` value, read from the default `op` account. */ +const legacy = (reference: string) => + new OnePasswordSecretReference({ reference, account: undefined }); + /** * Spawner that answers every `op read` with `result` and records the argv it * was handed, so tests can assert both the substituted value and how many @@ -61,7 +66,10 @@ describe("ProviderSecretResolverLive", () => { const resolved = yield* resolver.resolve(environment); - assert.deepStrictEqual(resolved, { variables: environment, unresolved: [] }); + assert.deepStrictEqual(resolved, { + variables: [{ name: "CLAUDE_SECURESTORAGE_CONFIG_DIR", value: "/home/u/.claude/work" }], + unresolved: [], + }); assert.strictEqual(spawner.invocations.length, 0); }).pipe( Effect.provide( @@ -264,7 +272,7 @@ describe("ProviderSecretResolverLive.prime", () => { return Effect.gen(function* () { const resolver = yield* ProviderSecretResolver; - yield* resolver.prime([TOKEN_REFERENCE, SECOND_REFERENCE]); + yield* resolver.prime([legacy(TOKEN_REFERENCE), legacy(SECOND_REFERENCE)]); assert.strictEqual(spawner.invocations.length, 1); assert.deepStrictEqual(Array.from(spawner.invocations[0] ?? []).slice(0, 2), [ @@ -307,7 +315,7 @@ describe("ProviderSecretResolverLive.prime", () => { return Effect.gen(function* () { const resolver = yield* ProviderSecretResolver; - yield* resolver.prime([TOKEN_REFERENCE, SECOND_REFERENCE]); + yield* resolver.prime([legacy(TOKEN_REFERENCE), legacy(SECOND_REFERENCE)]); // The batch is still attempted; it is the recovery that is per reference. assert.strictEqual(spawner.invocations[0]?.[0], "inject"); @@ -352,7 +360,7 @@ describe("ProviderSecretResolverLive.prime", () => { return Effect.gen(function* () { const resolver = yield* ProviderSecretResolver; - yield* resolver.prime([TOKEN_REFERENCE, SECOND_REFERENCE]); + yield* resolver.prime([legacy(TOKEN_REFERENCE), legacy(SECOND_REFERENCE)]); // `op` only reads piped input from a named pipe, and Node hands a child // a socket pair, so a template offered on stdin is never seen and the @@ -391,7 +399,7 @@ describe("ProviderSecretResolverLive.prime", () => { return Effect.gen(function* () { const resolver = yield* ProviderSecretResolver; - yield* resolver.prime([TOKEN_REFERENCE, SECOND_REFERENCE]); + yield* resolver.prime([legacy(TOKEN_REFERENCE), legacy(SECOND_REFERENCE)]); assert.isTrue(templatePath.length > 0); assert.isFalse(NodeFS.existsSync(templatePath)); @@ -409,7 +417,7 @@ describe("ProviderSecretResolverLive.prime", () => { return Effect.gen(function* () { const resolver = yield* ProviderSecretResolver; - yield* resolver.prime([TOKEN_REFERENCE]); + yield* resolver.prime([legacy(TOKEN_REFERENCE)]); // One reference is one prompt either way, and `op read` names the // reference it could not resolve. @@ -423,3 +431,127 @@ describe("ProviderSecretResolverLive.prime", () => { ); }); }); + +const HOME_ACCOUNT = "my.1password.com"; +const WORK_ACCOUNT = "acme.1password.com"; + +const onePasswordVariable = (name: string, reference: string, account: string) => ({ + name, + value: { kind: "1password" as const, reference, account }, +}); + +const onePassword = (reference: string, account: string) => { + const [variable] = decodeEnvironment([onePasswordVariable("TOKEN", reference, account)]); + const source = variable?.value; + if (source === undefined || typeof source === "string") { + throw new Error("expected a 1Password source"); + } + return new OnePasswordSecretReference({ reference: source.reference, account: source.account }); +}; + +describe("ProviderSecretResolverLive with 1Password accounts", () => { + it.effect("reads a 1Password source from the account it names", () => { + const spawner = recordingOpSpawner({ stdout: "sk-work-token", stderr: "", code: 0 }); + return Effect.gen(function* () { + const resolver = yield* ProviderSecretResolver; + + const resolved = yield* resolver.resolve( + decodeEnvironment([onePasswordVariable("CODEX_TOKEN", TOKEN_REFERENCE, WORK_ACCOUNT)]), + ); + + assert.deepStrictEqual(resolved, { + variables: [{ name: "CODEX_TOKEN", value: "sk-work-token" }], + unresolved: [], + }); + assert.deepStrictEqual(spawner.invocations, [ + ["read", "--account", WORK_ACCOUNT, "--no-newline", TOKEN_REFERENCE], + ]); + }).pipe( + Effect.provide( + ProviderSecretResolverLive.pipe( + Layer.provide(Layer.merge(spawner.layer, NodeFileSystem.layer)), + ), + ), + ); + }); + + it.effect("keeps the same reference in two accounts apart", () => { + const spawner = scriptedOpSpawner((args) => ({ + stdout: args.includes(WORK_ACCOUNT) ? "sk-work-token" : "sk-home-token", + stderr: "", + code: 0, + })); + return Effect.gen(function* () { + const resolver = yield* ProviderSecretResolver; + + const resolved = yield* resolver.resolve( + decodeEnvironment([ + onePasswordVariable("HOME_TOKEN", TOKEN_REFERENCE, HOME_ACCOUNT), + onePasswordVariable("WORK_TOKEN", TOKEN_REFERENCE, WORK_ACCOUNT), + ]), + ); + + assert.deepStrictEqual(resolved.variables, [ + { name: "HOME_TOKEN", value: "sk-home-token" }, + { name: "WORK_TOKEN", value: "sk-work-token" }, + ]); + assert.strictEqual(spawner.invocations.length, 2); + }).pipe( + Effect.provide( + ProviderSecretResolverLive.pipe( + Layer.provide(Layer.merge(spawner.layer, NodeFileSystem.layer)), + ), + ), + ); + }); + + it.effect("primes one batch per account", () => { + const spawner = scriptedOpSpawner((args, template) => { + if (!args.includes("inject")) { + return { stdout: "should-not-be-read-one-at-a-time", stderr: "", code: 0 }; + } + const prefix = args.includes(WORK_ACCOUNT) ? "work" : "home"; + return { + stdout: [`${prefix}-claude`, `${prefix}-codex`].join(separatorOf(template)), + stderr: "", + code: 0, + }; + }); + return Effect.gen(function* () { + const resolver = yield* ProviderSecretResolver; + + yield* resolver.prime([ + onePassword(TOKEN_REFERENCE, HOME_ACCOUNT), + onePassword(TOKEN_REFERENCE, WORK_ACCOUNT), + onePassword(SECOND_REFERENCE, HOME_ACCOUNT), + onePassword(SECOND_REFERENCE, WORK_ACCOUNT), + ]); + + assert.deepStrictEqual( + spawner.invocations.map((args) => Array.from(args).slice(0, 4)), + [ + ["inject", "--account", HOME_ACCOUNT, "-i"], + ["inject", "--account", WORK_ACCOUNT, "-i"], + ], + ); + + const resolved = yield* resolver.resolve( + decodeEnvironment([ + onePasswordVariable("HOME_CODEX", SECOND_REFERENCE, HOME_ACCOUNT), + onePasswordVariable("WORK_CLAUDE", TOKEN_REFERENCE, WORK_ACCOUNT), + ]), + ); + assert.deepStrictEqual(resolved.variables, [ + { name: "HOME_CODEX", value: "home-codex" }, + { name: "WORK_CLAUDE", value: "work-claude" }, + ]); + assert.strictEqual(spawner.invocations.length, 2); + }).pipe( + Effect.provide( + ProviderSecretResolverLive.pipe( + Layer.provide(Layer.merge(spawner.layer, NodeFileSystem.layer)), + ), + ), + ); + }); +}); diff --git a/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts b/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts index a5398bc9c51c..0224ff40c40b 100644 --- a/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts +++ b/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts @@ -2,16 +2,18 @@ * ProviderSecretResolverLive: 1Password-backed implementation of * `ProviderSecretResolver`. * - * Resolution is `op read `, which is the same command the user - * would run by hand and inherits their existing `op` session, so there is no - * second place to configure credentials. Reads run one at a time: two + * Resolution is `op read --account `, which is the same + * command the user would run by hand and inherits their existing `op` session, + * so there is no second place to configure credentials. The account is what + * keeps a user signed into several 1Password accounts from reading against + * whichever one `op` picks by default. Reads run one at a time: two * concurrent reads against a locked vault stack up two biometric prompts. * * The prompt is charged per `op` invocation rather than per secret, so reading * one reference at a time makes the whole fleet cost one authorization each. * `prime` exists for that: `op inject` substitutes any number of references in * a single process, so the caller that is about to build every instance pays - * one prompt for all of them. + * one prompt per account for all of them. * * Failures are cached alongside successes. If the vault is locked when the * first thread starts, every later thread in that session would otherwise @@ -21,10 +23,7 @@ * * @module provider/Layers/ProviderSecretResolverLive */ -import type { - ProviderInstanceEnvironment, - ProviderInstanceEnvironmentVariable, -} from "@t3tools/contracts"; +import type { OnePasswordAccount } from "@t3tools/contracts"; import * as Cache from "effect/Cache"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; @@ -35,7 +34,12 @@ import * as NodeCrypto from "node:crypto"; import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; import { resolveSpawnCommand } from "@t3tools/shared/shell"; -import { hasProviderSecretReference, providerSecretReference } from "../ProviderSecretReference.ts"; +import type { ResolvedProviderEnvironmentVariable } from "../ProviderInstanceEnvironment.ts"; +import { + providerSecretReference, + type OnePasswordSecretReference, + type ProviderSecretReference, +} from "../ProviderSecretReference.ts"; import { spawnAndCollect } from "../providerSnapshot.ts"; import { ProviderSecretResolver, @@ -58,8 +62,12 @@ const SECRET_READ_TIMEOUT = Duration.seconds(45); */ const SECRET_CACHE_CAPACITY = 64; +/** `--account` arguments for `op`; legacy references use its default account. */ +const accountArgs = (account: OnePasswordAccount | undefined): ReadonlyArray => + account === undefined ? [] : ["--account", account]; + /** - * Read many references in one `op inject`. + * Read many references from one account in one `op inject`. * * `op inject` substitutes references inside a template, so the template is the * references themselves joined by a separator, and the output is the secrets @@ -81,6 +89,7 @@ const SECRET_CACHE_CAPACITY = 64; * text; the secrets themselves come back on stdout and never touch disk. */ const readSecretsTogether = Effect.fn("readSecretsTogether")(function* ( + account: OnePasswordAccount | undefined, references: ReadonlyArray, ) { const fileSystem = yield* FileSystem.FileSystem; @@ -90,6 +99,7 @@ const readSecretsTogether = Effect.fn("readSecretsTogether")(function* ( yield* fileSystem.writeFileString(templatePath, template); const spawnCommand = yield* resolveSpawnCommand(ONE_PASSWORD_BINARY, [ "inject", + ...accountArgs(account), "-i", templatePath, ]); @@ -103,6 +113,7 @@ const readSecretsTogether = Effect.fn("readSecretsTogether")(function* ( // `op` names the reference it could not resolve on stderr and never echoes // a secret, so this is safe to log verbatim. yield* Effect.logWarning("Could not batch-read provider secrets from 1Password", { + account, references: references.length, exitCode: result.code, detail: result.stderr.trim(), @@ -123,9 +134,13 @@ const readSecretsTogether = Effect.fn("readSecretsTogether")(function* ( }); }); -const readSecret = Effect.fn("readSecret")(function* (reference: string) { +const readOnePasswordSecret = Effect.fn("readOnePasswordSecret")(function* ({ + reference, + account, +}: OnePasswordSecretReference) { const spawnCommand = yield* resolveSpawnCommand(ONE_PASSWORD_BINARY, [ "read", + ...accountArgs(account), "--no-newline", reference, ]); @@ -138,6 +153,7 @@ const readSecret = Effect.fn("readSecret")(function* (reference: string) { // and never echoes the secret itself, so this is safe to log verbatim. yield* Effect.logWarning("Could not read provider secret from 1Password", { reference, + account, exitCode: result.code, detail: result.stderr.trim(), }); @@ -147,6 +163,13 @@ const readSecret = Effect.fn("readSecret")(function* (reference: string) { return secret.length > 0 ? secret : undefined; }); +const readSecret = (secret: ProviderSecretReference) => { + switch (secret._tag) { + case "1password": + return readOnePasswordSecret(secret); + } +}; + export const ProviderSecretResolverLive = Layer.effect( ProviderSecretResolver, Effect.gen(function* () { @@ -162,13 +185,14 @@ export const ProviderSecretResolverLive = Layer.effect( // No time to live: a resolved secret is held until the user asks for a // provider refresh. Expiring on a timer would reintroduce the surprise // biometric prompt mid-session that the cache exists to remove. - lookup: (reference: string) => + lookup: (reference: ProviderSecretReference) => readSecret(reference).pipe( Effect.timeoutOption(SECRET_READ_TIMEOUT), Effect.map(Option.getOrUndefined), Effect.catch((error) => Effect.logWarning("Could not run 1Password to read a provider secret", { - reference, + reference: reference.reference, + account: reference.account, detail: String(error), }).pipe(Effect.as(undefined)), ), @@ -177,60 +201,85 @@ export const ProviderSecretResolverLive = Layer.effect( const resolve: ProviderSecretResolverShape["resolve"] = (environment) => Effect.gen(function* () { - if (!hasProviderSecretReference(environment)) { - return { variables: environment, unresolved: [] }; - } - const resolved: Array = []; + const resolved: Array = []; const unresolved: Array = []; - for (const variable of environment ?? []) { - const reference = providerSecretReference(variable.value); + for (const { name, value } of environment ?? []) { + const reference = providerSecretReference(value); if (reference === undefined) { - resolved.push(variable); + if (typeof value === "string") { + resolved.push({ name, value }); + } continue; } const secret = yield* Cache.get(cache, reference); if (secret === undefined) { - unresolved.push(variable.name); + unresolved.push(name); continue; } - resolved.push({ ...variable, value: secret }); + resolved.push({ name, value: secret }); } - return { variables: resolved as ProviderInstanceEnvironment, unresolved }; + return { variables: resolved, unresolved }; }); + const primeOnePassword = Effect.fn("primeOnePassword")(function* ( + account: OnePasswordAccount | undefined, + wanted: ReadonlyArray, + ) { + // One reference costs one prompt whichever command reads it, so there + // is nothing to save and `op read` gives the better error. + if (wanted.length < 2) { + return; + } + const values = yield* readSecretsTogether( + account, + wanted.map((secret) => secret.reference), + ).pipe( + Effect.scoped, + Effect.timeoutOption(SECRET_READ_TIMEOUT), + Effect.map(Option.getOrUndefined), + Effect.catch((error) => + Effect.logWarning("Could not run 1Password to batch-read provider secrets", { + account, + references: wanted.length, + detail: String(error), + }).pipe(Effect.as(undefined)), + ), + ); + if (values === undefined) { + return; + } + yield* Effect.forEach(wanted, (secret, index) => Cache.set(cache, secret, values[index]), { + discard: true, + }); + }); + const prime: ProviderSecretResolverShape["prime"] = (references) => Effect.gen(function* () { - const wanted: Array = []; - for (const reference of new Set(references)) { - if (!(yield* Cache.has(cache, reference))) { - wanted.push(reference); + // `op inject` reads every reference in its template from one account, + // so a batch is one call per account. + const onePasswordByAccount = new Map< + OnePasswordAccount | undefined, + Array + >(); + for (const reference of references) { + if (yield* Cache.has(cache, reference)) { + continue; + } + switch (reference._tag) { + case "1password": { + const group = onePasswordByAccount.get(reference.account) ?? []; + if (!group.some((seen) => seen.reference === reference.reference)) { + group.push(reference); + } + onePasswordByAccount.set(reference.account, group); + break; + } } - } - // One reference costs one prompt whichever command reads it, so there - // is nothing to save and `op read` gives the better error. - if (wanted.length < 2) { - return; - } - const values = yield* readSecretsTogether(wanted).pipe( - Effect.scoped, - Effect.timeoutOption(SECRET_READ_TIMEOUT), - Effect.map(Option.getOrUndefined), - Effect.catch((error) => - Effect.logWarning("Could not run 1Password to batch-read provider secrets", { - references: wanted.length, - detail: String(error), - }).pipe(Effect.as(undefined)), - ), - ); - if (values === undefined) { - return; } yield* Effect.forEach( - wanted, - (reference, index) => Cache.set(cache, reference, values[index]), - { - discard: true, - }, + onePasswordByAccount, + ([account, wanted]) => primeOnePassword(account, wanted), + { discard: true }, ); }).pipe(Effect.provideContext(primeContext)); diff --git a/apps/server/src/provider/ProviderDriver.ts b/apps/server/src/provider/ProviderDriver.ts index 4f92df965c8a..432efd5d31f8 100644 --- a/apps/server/src/provider/ProviderDriver.ts +++ b/apps/server/src/provider/ProviderDriver.ts @@ -28,7 +28,6 @@ import type { AcpRegistryOperationError, AcpRegistrySetProviderInput, ProviderDriverKind, - ProviderInstanceEnvironment, ProviderInstanceId, ServerProvider, } from "@t3tools/contracts"; @@ -37,6 +36,7 @@ import type * as Schema from "effect/Schema"; import type * as Scope from "effect/Scope"; import type { TextGeneration } from "../textGeneration/TextGeneration.ts"; +import type { ResolvedProviderEnvironment } from "./ProviderInstanceEnvironment.ts"; import type { ProviderAdapterV2Shape } from "../orchestration-v2/ProviderAdapter.ts"; import type { ProviderDriverError } from "./Errors.ts"; import type { ProviderAuthController } from "./Services/ProviderAuthService.ts"; @@ -142,7 +142,7 @@ export interface ProviderDriverCreateInput { readonly instanceId: ProviderInstanceId; readonly displayName: string | undefined; readonly accentColor?: string | undefined; - readonly environment: ProviderInstanceEnvironment; + readonly environment: ResolvedProviderEnvironment; readonly enabled: boolean; readonly config: Config; } diff --git a/apps/server/src/provider/ProviderInstanceEnvironment.test.ts b/apps/server/src/provider/ProviderInstanceEnvironment.test.ts index 7d6bbe61a2aa..a2281bb14d0f 100644 --- a/apps/server/src/provider/ProviderInstanceEnvironment.test.ts +++ b/apps/server/src/provider/ProviderInstanceEnvironment.test.ts @@ -5,7 +5,13 @@ import { describe, expect, it } from "@effect/vitest"; import * as Effect from "effect/Effect"; import * as Path from "effect/Path"; -import { mergeProviderInstanceEnvironment } from "./ProviderInstanceEnvironment.ts"; +import { ProviderInstanceEnvironment } from "@t3tools/contracts"; +import * as Schema from "effect/Schema"; + +import { + literalProviderInstanceEnvironment, + mergeProviderInstanceEnvironment, +} from "./ProviderInstanceEnvironment.ts"; describe("mergeProviderInstanceEnvironment", () => { it.effect.each([ @@ -20,9 +26,9 @@ describe("mergeProviderInstanceEnvironment", () => { }; const environment = mergeProviderInstanceEnvironment( [ - { name: "CODEX_HOME", value, sensitive: false }, - { name: "CLAUDE_CONFIG_DIR", value, sensitive: false }, - { name: "CUSTOM_VALUE", value, sensitive: false }, + { name: "CODEX_HOME", value }, + { name: "CLAUDE_CONFIG_DIR", value }, + { name: "CUSTOM_VALUE", value }, ], baseEnv, ); @@ -43,10 +49,7 @@ describe("mergeProviderInstanceEnvironment", () => { const baseEnv = { CODEX_HOME: "~/.codex", CLAUDE_CONFIG_DIR: "~\\.claude" }; expect( - mergeProviderInstanceEnvironment( - [{ name: "CUSTOM_VALUE", value: "~/.custom", sensitive: false }], - baseEnv, - ), + mergeProviderInstanceEnvironment([{ name: "CUSTOM_VALUE", value: "~/.custom" }], baseEnv), ).toEqual({ ...baseEnv, CUSTOM_VALUE: "~/.custom" }); }); @@ -54,8 +57,8 @@ describe("mergeProviderInstanceEnvironment", () => { expect( mergeProviderInstanceEnvironment( [ - { name: "OPENROUTER_API_KEY", value: "sk-or-test", sensitive: true }, - { name: "ANTHROPIC_API_KEY", value: "", sensitive: false }, + { name: "OPENROUTER_API_KEY", value: "sk-or-test" }, + { name: "ANTHROPIC_API_KEY", value: "" }, ], { ANTHROPIC_API_KEY: "inherited", PATH: "/bin" }, ), @@ -66,3 +69,22 @@ describe("mergeProviderInstanceEnvironment", () => { }); }); }); + +const decodeProviderInstanceEnvironment = Schema.decodeSync(ProviderInstanceEnvironment); + +describe("literalProviderInstanceEnvironment", () => { + it("keeps literals and leaves out every value that names a secret", () => { + const environment = decodeProviderInstanceEnvironment([ + { name: "CODEX_HOME", value: "~/.codex-work" }, + { name: "LEGACY_TOKEN", value: "op://Private/item/field" }, + { + name: "TOKEN", + value: { kind: "1password", reference: "op://Private/item/field", account: "my" }, + }, + ]); + + expect(literalProviderInstanceEnvironment(environment)).toEqual([ + { name: "CODEX_HOME", value: "~/.codex-work" }, + ]); + }); +}); diff --git a/apps/server/src/provider/ProviderInstanceEnvironment.ts b/apps/server/src/provider/ProviderInstanceEnvironment.ts index 77c0c6c2dc88..d4c8d55074ac 100644 --- a/apps/server/src/provider/ProviderInstanceEnvironment.ts +++ b/apps/server/src/provider/ProviderInstanceEnvironment.ts @@ -1,9 +1,43 @@ -import type { ProviderInstanceEnvironment } from "@t3tools/contracts"; +import type { + ProviderInstanceEnvironment, + ProviderInstanceEnvironmentVariableName, +} from "@t3tools/contracts"; import { expandHomePath } from "../pathExpansion.ts"; +import { providerSecretReference } from "./ProviderSecretReference.ts"; -export function mergeProviderInstanceEnvironment( +/** + * A provider environment variable whose value is ready for a child process. + * Configured variables may name a secret source instead of a value; only + * `ProviderSecretResolver` turns those into this shape, so a source cannot + * reach a process by accident. + */ +export interface ResolvedProviderEnvironmentVariable { + readonly name: ProviderInstanceEnvironmentVariableName; + readonly value: string; +} + +export type ResolvedProviderEnvironment = ReadonlyArray; + +/** + * The configured variables that hold literal values, for callers that build a + * process environment without resolving secrets. Variables that read from a + * secret store are left out rather than passed through as references. + */ +export function literalProviderInstanceEnvironment( environment: ProviderInstanceEnvironment | undefined, +): ResolvedProviderEnvironment { + const literals: Array = []; + for (const { name, value } of environment ?? []) { + if (typeof value === "string" && providerSecretReference(value) === undefined) { + literals.push({ name, value }); + } + } + return literals; +} + +export function mergeProviderInstanceEnvironment( + environment: ResolvedProviderEnvironment | undefined, baseEnv: NodeJS.ProcessEnv = process.env, ): NodeJS.ProcessEnv { if (!environment || environment.length === 0) { diff --git a/apps/server/src/provider/ProviderSecretReference.test.ts b/apps/server/src/provider/ProviderSecretReference.test.ts index a288451d1ae0..a70c5a231c08 100644 --- a/apps/server/src/provider/ProviderSecretReference.test.ts +++ b/apps/server/src/provider/ProviderSecretReference.test.ts @@ -5,21 +5,42 @@ import * as Schema from "effect/Schema"; import { collectProviderSecretReferences, hasProviderSecretReference, + OnePasswordSecretReference, providerSecretReference, } from "./ProviderSecretReference.ts"; const decodeEnvironment = Schema.decodeSync(ProviderInstanceEnvironment); +const legacy = (reference: string) => + new OnePasswordSecretReference({ reference, account: undefined }); + +const onePasswordVariable = (name: string, reference: string, account: string) => ({ + name, + value: { kind: "1password" as const, reference, account }, +}); + describe("providerSecretReference", () => { - it("reads a 1Password reference", () => { - expect(providerSecretReference("op://Private/claude-code/credential")).toBe( - "op://Private/claude-code/credential", + it("reads a 1Password source with its account", () => { + const [variable] = decodeEnvironment([ + onePasswordVariable("TOKEN", "op://Private/claude-code/credential", "my.1password.com"), + ]); + expect(providerSecretReference(variable!.value)).toEqual( + new OnePasswordSecretReference({ + reference: "op://Private/claude-code/credential", + account: "my.1password.com" as OnePasswordSecretReference["account"], + }), + ); + }); + + it("reads a legacy op:// string from the default account", () => { + expect(providerSecretReference("op://Private/claude-code/credential")).toEqual( + legacy("op://Private/claude-code/credential"), ); }); it("trims a reference pasted with surrounding whitespace", () => { - expect(providerSecretReference(" op://Private/claude-code/credential\n")).toBe( - "op://Private/claude-code/credential", + expect(providerSecretReference(" op://Private/claude-code/credential\n")).toEqual( + legacy("op://Private/claude-code/credential"), ); }); @@ -61,6 +82,18 @@ describe("hasProviderSecretReference", () => { }); }); +describe("hasProviderSecretReference with secret sources", () => { + it("is true for a 1Password source", () => { + expect( + hasProviderSecretReference( + decodeEnvironment([ + onePasswordVariable("TOKEN", "op://Private/item/field", "my.1password.com"), + ]), + ), + ).toBe(true); + }); +}); + describe("collectProviderSecretReferences", () => { it("returns each distinct reference once, in first-seen order", () => { const shared = "op://Private/shared/credential"; @@ -78,8 +111,26 @@ describe("collectProviderSecretReferences", () => { ]; expect(Array.from(collectProviderSecretReferences(environments))).toEqual([ - shared, - "op://Private/codex/credential", + legacy(shared), + legacy("op://Private/codex/credential"), + ]); + }); + + it("keeps one reference read from different accounts apart", () => { + const shared = "op://Private/shared/credential"; + const references = collectProviderSecretReferences([ + decodeEnvironment([ + onePasswordVariable("HOME_TOKEN", shared, "my.1password.com"), + onePasswordVariable("WORK_TOKEN", shared, "acme.1password.com"), + onePasswordVariable("HOME_AGAIN", shared, "my.1password.com"), + { name: "LEGACY_TOKEN", value: shared }, + ]), + ]); + + expect(references.map(({ reference, account }) => [reference, account])).toEqual([ + [shared, "my.1password.com"], + [shared, "acme.1password.com"], + [shared, undefined], ]); }); diff --git a/apps/server/src/provider/ProviderSecretReference.ts b/apps/server/src/provider/ProviderSecretReference.ts index f9d69301a858..1adefece898d 100644 --- a/apps/server/src/provider/ProviderSecretReference.ts +++ b/apps/server/src/provider/ProviderSecretReference.ts @@ -1,35 +1,70 @@ /** * Provider environment values that name a secret instead of carrying one. * - * A user who keeps a provider credential in 1Password can paste the item's - * secret reference (`op://Vault/Item/field`) as an environment variable's - * value instead of the secret itself. `ProviderSecretResolver` swaps the - * reference for the real value on the way into the provider process, so the - * credential never lands in `settings.json` or the on-disk secret store, and - * rotating it in 1Password rotates it here. + * A user who keeps a provider credential in 1Password configures the variable + * with a 1Password secret source (`{ kind: "1password", reference, account }`) + * instead of the secret itself. `ProviderSecretResolver` swaps the source for + * the real value on the way into the provider process, so the credential never + * lands in `settings.json` or the on-disk secret store, and rotating it in + * 1Password rotates it here. * * These helpers are pure so the instance registry can ask "does this * environment read from a secret store?" without depending on the resolver. * * @module provider/ProviderSecretReference */ -import type { ProviderInstanceEnvironment } from "@t3tools/contracts"; +import type { + OnePasswordAccount, + ProviderInstanceEnvironment, + ProviderInstanceEnvironmentVariable, +} from "@t3tools/contracts"; +import * as Data from "effect/Data"; +import * as Equal from "effect/Equal"; /** URI scheme 1Password uses for secret references; `op read` consumes these. */ -const PROVIDER_SECRET_REFERENCE_PREFIX = "op://"; +const ONE_PASSWORD_SECRET_REFERENCE_PREFIX = "op://"; /** - * The secret reference an environment value names, or `undefined` when the - * value is a literal. The value is trimmed first: a reference copied out of a - * password manager routinely arrives with surrounding whitespace, which `op` - * rejects. + * One secret to read from 1Password. Structurally comparable, so it doubles as + * the resolver's cache key: the same reference in two accounts is two secrets. + * `account` is absent only for legacy plain-string references, which `op` + * reads from its default account. */ -export function providerSecretReference(value: string): string | undefined { +export class OnePasswordSecretReference extends Data.TaggedClass("1password")<{ + readonly reference: string; + readonly account: OnePasswordAccount | undefined; +}> {} + +/** Every secret a provider environment value can name. */ +export type ProviderSecretReference = OnePasswordSecretReference; + +/** + * The secret an environment value names, or `undefined` when the value is a + * literal. + */ +export function providerSecretReference( + value: ProviderInstanceEnvironmentVariable["value"], +): ProviderSecretReference | undefined { + if (typeof value !== "string") { + switch (value.kind) { + case "1password": + return new OnePasswordSecretReference({ + reference: value.reference, + account: value.account, + }); + } + } + // Legacy: a plain string beginning with `op://` predates secret sources and + // is read from the default `op` account. Trimmed because a reference copied + // out of a password manager routinely arrives with surrounding whitespace. const trimmed = value.trim(); - if (!trimmed.startsWith(PROVIDER_SECRET_REFERENCE_PREFIX)) { + if ( + !trimmed.startsWith(ONE_PASSWORD_SECRET_REFERENCE_PREFIX) || + trimmed.length === ONE_PASSWORD_SECRET_REFERENCE_PREFIX.length + ) { return undefined; } - return trimmed.length > PROVIDER_SECRET_REFERENCE_PREFIX.length ? trimmed : undefined; + return new OnePasswordSecretReference({ reference: trimmed, account: undefined }); } /** @@ -46,25 +81,25 @@ export function hasProviderSecretReference( } /** - * Every distinct secret reference across a set of environments, in the order - * they were first seen. + * Every distinct secret across a set of environments, in the order they were + * first seen. * - * Resolution is per reference but unlocking is per `op` invocation, so the - * caller that is about to build many instances wants the whole list up front: - * one call covering every reference costs one authorization, where one call - * per instance costs one each. + * Resolution is per secret but unlocking is per `op` invocation, so the caller + * that is about to build many instances wants the whole list up front: one + * call covering every secret costs one authorization, where one call per + * instance costs one each. */ export function collectProviderSecretReferences( environments: Iterable, -): ReadonlyArray { - const references = new Set(); +): ReadonlyArray { + const references: Array = []; for (const environment of environments) { for (const variable of environment ?? []) { const reference = providerSecretReference(variable.value); - if (reference !== undefined) { - references.add(reference); + if (reference !== undefined && !references.some((seen) => Equal.equals(seen, reference))) { + references.push(reference); } } } - return Array.from(references); + return references; } diff --git a/apps/server/src/provider/Services/ProviderSecretResolver.ts b/apps/server/src/provider/Services/ProviderSecretResolver.ts index 4ff448c08938..1eafe2857b9e 100644 --- a/apps/server/src/provider/Services/ProviderSecretResolver.ts +++ b/apps/server/src/provider/Services/ProviderSecretResolver.ts @@ -1,12 +1,13 @@ /** - * ProviderSecretResolver: turns `op://` environment values into the secrets - * they name, once, and holds them in memory. + * ProviderSecretResolver: turns environment values that name a secret (a + * 1Password secret source, or a legacy `op://` string) into the secrets they + * name, once, and holds them in memory. * * Every provider instance resolves its environment when the driver builds it, * and a single instance can rebuild several times per session. Shelling out * to `op` on each of those is slow (seconds) and, worse, can put a biometric * prompt in front of a user who only started a thread. The resolver therefore - * caches by reference for the lifetime of the process; `invalidate` is wired + * caches by secret (reference plus account) for the lifetime of the process; `invalidate` is wired * to the Settings refresh button, which is the user's way of saying "go read * it again" after rotating a credential. * @@ -16,6 +17,9 @@ import type { ProviderInstanceEnvironment } from "@t3tools/contracts"; import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; +import type { ResolvedProviderEnvironment } from "../ProviderInstanceEnvironment.ts"; +import type { ProviderSecretReference } from "../ProviderSecretReference.ts"; + /** * An instance environment after its secret references have been read. * @@ -27,15 +31,14 @@ import * as Effect from "effect/Effect"; * one, and it hides as "authenticated". */ export interface ResolvedProviderInstanceEnvironment { - readonly variables: ProviderInstanceEnvironment | undefined; + readonly variables: ResolvedProviderEnvironment; readonly unresolved: ReadonlyArray; } export interface ProviderSecretResolverShape { /** - * Replace every secret reference in the environment with its value. - * Literal values pass through untouched, and an environment with no - * references is returned as-is. + * Replace every secret the environment names with its value. Literal values + * pass through untouched. * * Never fails. A reference that cannot be read (1Password locked, `op` not * installed, item deleted) is reported as unresolved rather than @@ -62,7 +65,7 @@ export interface ProviderSecretResolverShape { * found it, so `resolve` falls back to reading one reference at a time with * the same per-variable failure isolation it has always had. */ - readonly prime: (references: ReadonlyArray) => Effect.Effect; + readonly prime: (references: ReadonlyArray) => Effect.Effect; /** * Drop every cached secret. The next `resolve` re-reads from the store. * Callers that need the new value to reach a running provider must also @@ -73,16 +76,32 @@ export interface ProviderSecretResolverShape { } /** - * Defaults to handing every environment back untouched, which is what a build + * Defaults to handing every literal back untouched, which is what a build * without secret-store integration behaves like, and what tests want unless - * they are testing resolution itself: an `op://` value stays an `op://` - * value, and the provider reports whatever the CLI makes of it. + * they are testing resolution itself: a legacy `op://` string stays an + * `op://` string, and the provider reports whatever the CLI makes of it. A + * secret source has no literal form, so it is reported unresolved. */ +const resolveWithoutSecretStore = ( + environment: ProviderInstanceEnvironment | undefined, +): ResolvedProviderInstanceEnvironment => { + const variables: Array = []; + const unresolved: Array = []; + for (const { name, value } of environment ?? []) { + if (typeof value === "string") { + variables.push({ name, value }); + } else { + unresolved.push(name); + } + } + return { variables, unresolved }; +}; + export class ProviderSecretResolver extends Context.Reference( "t3/provider/Services/ProviderSecretResolver", { defaultValue: () => ({ - resolve: (environment) => Effect.succeed({ variables: environment, unresolved: [] }), + resolve: (environment) => Effect.sync(() => resolveWithoutSecretStore(environment)), prime: () => Effect.void, invalidate: Effect.void, }), diff --git a/apps/server/src/provider/acp/AcpRegistryAuthenticationState.ts b/apps/server/src/provider/acp/AcpRegistryAuthenticationState.ts index 1f4799e65c24..17ef8971a87b 100644 --- a/apps/server/src/provider/acp/AcpRegistryAuthenticationState.ts +++ b/apps/server/src/provider/acp/AcpRegistryAuthenticationState.ts @@ -1,9 +1,5 @@ import * as NodeCrypto from "node:crypto"; -import type { - AcpRegistrySettings, - ProviderInstanceEnvironment, - ProviderInstanceId, -} from "@t3tools/contracts"; +import type { AcpRegistrySettings, ProviderInstanceId } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as FileSystem from "effect/FileSystem"; import * as Path from "effect/Path"; @@ -12,6 +8,7 @@ import * as Schema from "effect/Schema"; import * as Semaphore from "effect/Semaphore"; import { writeFileStringAtomically } from "../../atomicWrite.ts"; +import type { ResolvedProviderEnvironment } from "../ProviderInstanceEnvironment.ts"; const decodeState = Schema.decodeUnknownEffect( Schema.fromJsonString(Schema.Struct({ binding: Schema.String, authenticated: Schema.Boolean })), @@ -25,7 +22,7 @@ export const makeAcpRegistryAuthenticationState = Effect.fn("makeAcpRegistryAuth readonly cacheDir: string; readonly instanceId: ProviderInstanceId; readonly settings: AcpRegistrySettings; - readonly environment: ProviderInstanceEnvironment; + readonly environment: ResolvedProviderEnvironment; readonly processEnvironment: NodeJS.ProcessEnv; }) { const fs = yield* FileSystem.FileSystem; diff --git a/apps/server/src/provider/providerInstallation.ts b/apps/server/src/provider/providerInstallation.ts index 62931e2e0418..858b288ae46f 100644 --- a/apps/server/src/provider/providerInstallation.ts +++ b/apps/server/src/provider/providerInstallation.ts @@ -18,7 +18,10 @@ import * as AntigravityInstallation from "./AntigravityInstallation.ts"; import { deriveProviderInstanceConfigMap } from "./Layers/ProviderInstanceRegistryHydration.ts"; import * as ProviderInstanceRegistry from "./Services/ProviderInstanceRegistry.ts"; import * as ProviderRegistry from "./Services/ProviderRegistry.ts"; -import { mergeProviderInstanceEnvironment } from "./ProviderInstanceEnvironment.ts"; +import { + literalProviderInstanceEnvironment, + mergeProviderInstanceEnvironment, +} from "./ProviderInstanceEnvironment.ts"; const ANTIGRAVITY = ProviderDriverKind.make("antigravity"); const hasBinaryPath = Schema.is(Schema.Struct({ binaryPath: Schema.String })); @@ -140,7 +143,9 @@ export const makeProviderInstallation = Effect.fn("makeProviderInstallation")(fu } const binaryPath = entry.config.binaryPath.trim(); return resolveCommandPath(binaryPath, { - env: mergeProviderInstanceEnvironment(entry.environment), + env: mergeProviderInstanceEnvironment( + literalProviderInstanceEnvironment(entry.environment), + ), }).pipe( Effect.map((resolved) => [binaryPath, resolved]), Effect.orElseSucceed(() => [binaryPath]), diff --git a/apps/server/src/serverSettings.test.ts b/apps/server/src/serverSettings.test.ts index 51d81dc9f1ea..bc0c35574bfe 100644 --- a/apps/server/src/serverSettings.test.ts +++ b/apps/server/src/serverSettings.test.ts @@ -5,6 +5,7 @@ import { ProjectId, ProjectScript, ProviderDriverKind, + ProviderInstanceEnvironment, ProviderInstanceId, resolveProviderInstanceEnabled, ServerSettings, @@ -1417,6 +1418,47 @@ it.layer(NodeServices.layer)("server settings", (it) => { ); } + it.effect("stores 1Password secret sources as written and never as sensitive", () => + Effect.gen(function* () { + const serverSettings = yield* ServerSettingsModule.ServerSettingsService; + const serverConfig = yield* ServerConfig.ServerConfig; + const fileSystem = yield* FileSystem.FileSystem; + const instanceId = ProviderInstanceId.make("codex_personal"); + const source = { + kind: "1password", + reference: "op://Home Lab/Codex/api key", + account: "my.1password.com", + } as const; + const decodeEnvironment = Schema.decodeEffect(ProviderInstanceEnvironment); + const environment = yield* decodeEnvironment([ + { name: "OPENAI_API_KEY", value: source, sensitive: true, valueRedacted: true }, + ]); + const expected = yield* decodeEnvironment([ + { name: "OPENAI_API_KEY", value: source, sensitive: false }, + ]); + + const next = yield* serverSettings.updateSettings({ + providerInstances: { + [instanceId]: { + driver: ProviderDriverKind.make("codex"), + environment, + config: {}, + }, + }, + }); + + assert.deepEqual(next.providerInstances[instanceId]?.environment, expected); + assert.deepEqual( + ServerSettingsModule.redactServerSettingsForClient(next).providerInstances[instanceId] + ?.environment, + expected, + ); + const raw = yield* fileSystem.readFileString(serverConfig.settingsPath); + // @effect-diagnostics-next-line preferSchemaOverJson:off + assert.deepEqual(JSON.parse(raw).providerInstances.codex_personal.environment, 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 55754dcde0be..c71570ae1657 100644 --- a/apps/server/src/serverSettings.ts +++ b/apps/server/src/serverSettings.ts @@ -166,6 +166,12 @@ const redactSecret = (value: string) => (value.length > 0 ? SECRET_REDACTED : "" function redactProviderEnvironmentVariable( variable: ProviderInstanceEnvironmentVariable, ): ProviderInstanceEnvironmentVariable { + const { value } = variable; + // A secret source names a secret without carrying it, so it is never + // sensitive and is stored as written. + if (typeof value !== "string") { + return { name: variable.name, value, sensitive: false }; + } if (!variable.sensitive) { const { valueRedacted: _omit, ...rest } = variable; return rest; @@ -173,7 +179,7 @@ function redactProviderEnvironmentVariable( return { ...variable, value: "", - ...(variable.value.length > 0 || variable.valueRedacted ? { valueRedacted: true } : {}), + ...(value.length > 0 || variable.valueRedacted ? { valueRedacted: true } : {}), }; } @@ -820,7 +826,11 @@ const make = Effect.gen(function* () { if (!instance.environment) continue; const environment: ProviderInstanceEnvironmentVariable[] = []; for (const variable of instance.environment) { - if (!variable.sensitive || !variable.valueRedacted) { + if ( + !variable.sensitive || + !variable.valueRedacted || + typeof variable.value !== "string" + ) { environment.push(variable); continue; } @@ -925,7 +935,8 @@ const make = Effect.gen(function* () { const environment: ProviderInstanceEnvironmentVariable[] = []; for (const variable of instance.environment) { const secretName = providerEnvironmentSecretName({ instanceId, name: variable.name }); - if (!variable.sensitive) { + const { value: configuredValue } = variable; + if (!variable.sensitive || typeof configuredValue !== "string") { changes.push({ kind: "remove", secretName, @@ -945,10 +956,13 @@ const make = Effect.gen(function* () { ) : undefined; const inlineValue = - previous?.sensitive && !previous.valueRedacted && previous.value.length > 0 + previous?.sensitive && + !previous.valueRedacted && + typeof previous.value === "string" && + previous.value.length > 0 ? previous.value : undefined; - const value = inlineValue ?? variable.value; + const value = inlineValue ?? configuredValue; if (!variable.valueRedacted || inlineValue !== undefined) { if (value.length > 0) { changes.push({ diff --git a/apps/server/src/terminal/Manager.ts b/apps/server/src/terminal/Manager.ts index 6122c8606f0b..053af154f385 100644 --- a/apps/server/src/terminal/Manager.ts +++ b/apps/server/src/terminal/Manager.ts @@ -62,7 +62,10 @@ import * as Semaphore from "effect/Semaphore"; import * as SynchronizedRef from "effect/SynchronizedRef"; import * as ServerConfig from "../config.ts"; -import { mergeProviderInstanceEnvironment } from "../provider/ProviderInstanceEnvironment.ts"; +import { + literalProviderInstanceEnvironment, + mergeProviderInstanceEnvironment, +} from "../provider/ProviderInstanceEnvironment.ts"; import { resolveCodexHomeLayout } from "../provider/Drivers/CodexHomeLayout.ts"; import { makeClaudeEnvironment } from "../provider/Drivers/ClaudeHome.ts"; import { deriveProviderInstanceConfigMap } from "../provider/Layers/ProviderInstanceRegistryHydration.ts"; @@ -1401,7 +1404,10 @@ export const resolveProviderInstanceTerminalEnvironment = Effect.fn( return yield* new TerminalProviderInstanceNotFoundError({ providerInstanceId }); } - let resolved = mergeProviderInstanceEnvironment(instance.environment, input.env ?? {}); + let resolved = mergeProviderInstanceEnvironment( + literalProviderInstanceEnvironment(instance.environment), + input.env ?? {}, + ); if (instance.driver === "codex") { const config = decodeCodexSettings(instance.config ?? {}); if (Option.isSome(config)) { diff --git a/apps/server/src/usage/UsageService.ts b/apps/server/src/usage/UsageService.ts index a3b07d64d71b..f0e8d9b826fa 100644 --- a/apps/server/src/usage/UsageService.ts +++ b/apps/server/src/usage/UsageService.ts @@ -49,7 +49,10 @@ import { expandHomePath } from "../pathExpansion.ts"; import * as ServerSettings from "../serverSettings.ts"; import { resolveCodexHomeLayout } from "../provider/Drivers/CodexHomeLayout.ts"; import { resolveAntigravityInstanceDirectories } from "../provider/antigravityAuthSupport.ts"; -import { mergeProviderInstanceEnvironment } from "../provider/ProviderInstanceEnvironment.ts"; +import { + literalProviderInstanceEnvironment, + mergeProviderInstanceEnvironment, +} from "../provider/ProviderInstanceEnvironment.ts"; import { readOpenCodeUsage } from "./opencodeUsageReader.ts"; import { readAntigravityUsage } from "./antigravityUsageReader.ts"; import { readCursorAccountUsage } from "./cursorUsageReader.ts"; @@ -283,7 +286,10 @@ export const make = Effect.gen(function* () { }); } for (const instance of instances) { - const environment = mergeProviderInstanceEnvironment(instance.environment, hostEnvironment); + const environment = mergeProviderInstanceEnvironment( + literalProviderInstanceEnvironment(instance.environment), + hostEnvironment, + ); const provider = driver === "claudeAgent" ? "claude" : driver; let home: string; if (driver === "codex") { diff --git a/apps/web/src/components/settings/ProviderInstanceCard.tsx b/apps/web/src/components/settings/ProviderInstanceCard.tsx index b0a727f9ee5a..70d8da333f44 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.tsx +++ b/apps/web/src/components/settings/ProviderInstanceCard.tsx @@ -10,16 +10,21 @@ import { LockIcon, LockOpenIcon, ExternalLinkIcon, + KeyRoundIcon, PlusIcon, Trash2Icon, XIcon, } from "lucide-react"; import * as Arr from "effect/Array"; import * as Result from "effect/Result"; +import * as Schema from "effect/Schema"; import { useEffect, useRef, useState, type ReactElement, type ReactNode } from "react"; import { isProviderDriverKind, + OnePasswordAccount, + OnePasswordSecretReference, resolveProviderInstanceEnabled, + type OnePasswordSecretSource, type ProviderInstanceConfig, type ProviderInstanceEnvironmentVariable, type ProviderInstanceId, @@ -85,10 +90,15 @@ function ProviderStatusDiagnostic({ let environmentVariableDraftId = 0; const nextEnvironmentVariableDraftId = () => `provider-env-${environmentVariableDraftId++}`; +type EnvironmentDraftSource = "plain" | "1password"; + type EnvironmentDraftRow = { readonly id: string; readonly name: string; + readonly source: EnvironmentDraftSource; readonly value: string; + readonly reference: string; + readonly account: string; readonly sensitive: boolean; readonly valueRedacted?: boolean; }; @@ -97,15 +107,61 @@ function makeEnvironmentDraftRow( variable: ProviderInstanceEnvironmentVariable, index: number, ): EnvironmentDraftRow { + const id = `${index}:${variable.name}`; + if (typeof variable.value !== "string") { + return { + id, + name: variable.name, + source: "1password", + value: "", + reference: variable.value.reference, + account: variable.value.account, + sensitive: false, + }; + } return { - id: `${index}:${variable.name}`, + id, name: variable.name, + source: "plain", value: variable.value, + reference: "", + account: "", sensitive: variable.sensitive, ...(variable.valueRedacted !== undefined ? { valueRedacted: variable.valueRedacted } : {}), }; } +const decodeOnePasswordSecretReference = Schema.decodeUnknownResult(OnePasswordSecretReference); +const decodeOnePasswordAccount = Schema.decodeUnknownResult(OnePasswordAccount); + +/** + * The 1Password source a draft row describes, or the message explaining why + * it is not one yet. + */ +function onePasswordSourceFromDraft( + row: EnvironmentDraftRow, +): Result.Result { + const reference = decodeOnePasswordSecretReference(row.reference); + if (Result.isFailure(reference)) return Result.fail(reference.failure.message); + const account = decodeOnePasswordAccount(row.account); + if (Result.isFailure(account)) return Result.fail(account.failure.message); + return Result.succeed({ + kind: "1password", + reference: reference.success, + account: account.success, + }); +} + +function environmentValuesEqual( + left: ProviderInstanceEnvironmentVariable["value"], + right: ProviderInstanceEnvironmentVariable["value"], +): boolean { + if (typeof left === "string" || typeof right === "string") return left === right; + return ( + left.kind === right.kind && left.reference === right.reference && left.account === right.account + ); +} + function providerEnvironmentsEqual( left: ReadonlyArray, right: ReadonlyArray, @@ -117,7 +173,7 @@ function providerEnvironmentsEqual( return ( other !== undefined && variable.name === other.name && - variable.value === other.value && + environmentValuesEqual(variable.value, other.value) && variable.sensitive === other.sensitive && variable.valueRedacted === other.valueRedacted ); @@ -257,7 +313,11 @@ function ProviderEnvironmentFieldRow(props: { readonly onRemove: (field: ProviderEnvironmentFieldDefinition) => void; }) { const inputId = `${props.idPrefix}-environment-${props.field.name}`; - const value = props.variable?.valueRedacted ? "" : (props.variable?.value ?? ""); + const configuredValue = props.variable?.value; + const value = + props.variable?.valueRedacted || typeof configuredValue !== "string" + ? "" + : (configuredValue ?? ""); const placeholder = props.variable?.valueRedacted ? "Stored secret - enter a new value to replace" : props.field.placeholder; @@ -331,6 +391,7 @@ function ProviderEnvironmentSection(props: { if (!ENVIRONMENT_VARIABLE_NAME_PATTERN.test(name)) { if ( name.length > 0 || + row.source !== "plain" || row.value.length > 0 || row.sensitive !== true || row.valueRedacted !== undefined @@ -339,8 +400,18 @@ function ProviderEnvironmentSection(props: { } continue; } - const { id: _id, ...rest } = row; - published.push({ ...rest, name }); + if (row.source === "1password") { + const source = onePasswordSourceFromDraft(row); + if (Result.isFailure(source)) return; + published.push({ name, value: source.success, sensitive: false }); + continue; + } + published.push({ + name, + value: row.value, + sensitive: row.sensitive, + ...(row.valueRedacted !== undefined ? { valueRedacted: row.valueRedacted } : {}), + }); } lastPublishedEnvironmentRef.current = published; props.onChange(published); @@ -372,7 +443,10 @@ function ProviderEnvironmentSection(props: { { id: nextEnvironmentVariableDraftId(), name: "", + source: "plain", value: "", + reference: "", + account: "", sensitive: true, }, ]); @@ -390,79 +464,154 @@ function ProviderEnvironmentSection(props: { > {rows.length > 0 ? (
- {rows.map((variable, index) => ( -
- updateVariable(variable.id, { name: name.trim() })} - placeholder="VARIABLE_NAME" - spellCheck={false} - aria-label={`Environment variable name ${index + 1}`} - /> - - = - - updateVariable(variable.id, { value })} - type={variable.sensitive ? "password" : undefined} - autoComplete="off" - placeholder={ - variable.valueRedacted ? "Stored secret, enter a new value to replace" : "value" - } - spellCheck={false} - aria-label={`Environment variable value ${index + 1}`} - /> - - { - const sensitive = !variable.sensitive; - updateVariable(variable.id, { - sensitive, - ...(sensitive && variable.valueRedacted === undefined - ? {} - : { valueRedacted: sensitive ? variable.valueRedacted : false }), - }); - }} - aria-pressed={variable.sensitive} - aria-label={`Mark environment variable ${variable.name || index + 1} as sensitive`} - > - {variable.sensitive ? ( - - ) : ( - - )} - - } - /> - - {variable.sensitive ? "Sensitive, stored separately" : "Plain text"} - - - -
- ))} + {rows.map((variable, index) => { + const isOnePassword = variable.source === "1password"; + const sourceIssue = + isOnePassword && (variable.reference.length > 0 || variable.account.length > 0) + ? Result.match(onePasswordSourceFromDraft(variable), { + onFailure: (message) => message, + onSuccess: () => undefined, + }) + : undefined; + return ( +
+
+ updateVariable(variable.id, { name: name.trim() })} + placeholder="VARIABLE_NAME" + spellCheck={false} + aria-label={`Environment variable name ${index + 1}`} + /> + + = + + {isOnePassword ? ( + <> + updateVariable(variable.id, { reference })} + placeholder="op://vault/item/field" + spellCheck={false} + aria-invalid={sourceIssue !== undefined || undefined} + aria-label={`Environment variable 1Password reference ${index + 1}`} + /> + updateVariable(variable.id, { account })} + placeholder="my.1password.com" + spellCheck={false} + aria-invalid={sourceIssue !== undefined || undefined} + aria-label={`Environment variable 1Password account ${index + 1}`} + /> + + ) : ( + <> + updateVariable(variable.id, { value })} + type={variable.sensitive ? "password" : undefined} + autoComplete="off" + placeholder={ + variable.valueRedacted + ? "Stored secret, enter a new value to replace" + : "value" + } + spellCheck={false} + aria-label={`Environment variable value ${index + 1}`} + /> + + { + const sensitive = !variable.sensitive; + updateVariable(variable.id, { + sensitive, + ...(sensitive && variable.valueRedacted === undefined + ? {} + : { + valueRedacted: sensitive ? variable.valueRedacted : false, + }), + }); + }} + aria-pressed={variable.sensitive} + aria-label={`Mark environment variable ${variable.name || index + 1} as sensitive`} + > + {variable.sensitive ? ( + + ) : ( + + )} + + } + /> + + {variable.sensitive ? "Sensitive, stored separately" : "Plain text"} + + + + )} + + + updateVariable( + variable.id, + isOnePassword + ? { source: "plain", value: "", sensitive: true } + : { source: "1password", value: "", sensitive: false }, + ) + } + aria-pressed={isOnePassword} + aria-label={`Read environment variable ${variable.name || index + 1} from 1Password`} + > + + + } + /> + + {isOnePassword ? "Read from 1Password" : "Plain value"} + + + +
+ {sourceIssue !== undefined ? ( +

{sourceIssue}

+ ) : null} +
+ ); + })}

- Sensitive values are stored separately and never returned to the app. + Sensitive values are stored separately and never returned to the app. 1Password + references are read with the 1Password CLI each time the provider starts.

) : null} diff --git a/docs/internals/providers.md b/docs/internals/providers.md index e03e3254266a..60ddea13a000 100644 --- a/docs/internals/providers.md +++ b/docs/internals/providers.md @@ -128,21 +128,25 @@ current client support. ## Secret references in provider environments -A provider instance's `environment` can hold values that start with `op://`. Those are secret -references, and +A provider instance's `environment` can hold secret references: a `ProviderSecretSource` value such +as `{ kind: "1password", reference, account }`, or a legacy plain string starting with `op://` that +reads from the CLI's default account. [`ProviderSecretResolver`](../../apps/server/src/provider/Services/ProviderSecretResolver.ts) swaps each one for the value the 1Password CLI returns. This happens once per instance in [`ProviderInstanceRegistryLive`](../../apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts), before `driver.create`, so drivers and the orchestration v2 adapters built from them only ever see -resolved values. The registry keeps the raw `op://` config, which is what makes a later rebuild -possible. +resolved values; `ResolvedProviderEnvironment` is the type that enforces it. The registry keeps the +raw config, which is what makes a later rebuild possible. Paths that build a process environment +without the resolver (terminals, usage, installation, session scanning) use +`literalProviderInstanceEnvironment`, which leaves references out rather than passing them through. The parsing half lives in [`ProviderSecretReference.ts`](../../apps/server/src/provider/ProviderSecretReference.ts) and knows nothing about how a secret is fetched, so the registry can ask "does this instance read from a secret store?" without depending on the resolver. [`ProviderSecretResolverLive`](../../apps/server/src/provider/Layers/ProviderSecretResolverLive.ts) -is the half that shells out to `op read --no-newline`. +is the half that shells out to `op read --account --no-newline`. The account is part of the +cache key, because the same reference in two accounts is two different secrets. Three decisions are load-bearing: @@ -155,8 +159,8 @@ Three decisions are load-bearing: `HostProcessEnvironment` with every unresolved name removed. - **Reads are batched across instances, and sequential within one.** The store charges an unlock per `op` invocation, not per secret, and every instance resolves its own environment as it is built, - so a fleet would otherwise cost one prompt per provider. `prime` reads the whole set in a single - `op inject` before the builds start, called from the settings watcher (which covers boot) and from + so a fleet would otherwise cost one prompt per provider. `prime` reads the whole set with one + `op inject` per account before the builds start, called from the settings watcher (which covers boot) and from `reloadSecretBackedInstances` (which covers the refresh button). Whatever `prime` misses, `resolve` still walks with a plain loop rather than `Effect.forEach` with concurrency, so it produces one prompt rather than several simultaneous ones. diff --git a/docs/user/provider-secrets.md b/docs/user/provider-secrets.md index 7156e8f3b028..141ce0cf9805 100644 --- a/docs/user/provider-secrets.md +++ b/docs/user/provider-secrets.md @@ -7,22 +7,31 @@ For provider setup itself, see [Codex](./providers-codex.md) and [Claude](./prov ## I Do Not Want To Paste My Token Into T3 Code -Paste the 1Password reference instead of the value. +Point the variable at 1Password instead of pasting the value. -In the provider's Environment variables section in Settings, use the `op://` secret reference as the -value: +In the provider's Environment variables section in Settings, add the variable, switch it to read from +1Password with the key button, and fill in the secret reference and the account it lives in: ```text -Name: CLAUDE_CODE_OAUTH_TOKEN -Value: op://Private/claude-code/credential +Name: CLAUDE_CODE_OAUTH_TOKEN +Reference: op://Private/claude-code/credential +Account: my.1password.com ``` +The account is anything `op --account` accepts: the sign-in address, the account shorthand, or the +account ID. Run `op account list` to see yours. Naming it means a reference keeps resolving against +the right account when you are signed in to more than one. + T3 Code reads the value with the 1Password CLI right before it starts the agent, and hands the resolved value to the agent process only. The reference is what T3 Code stores; the secret itself never lands in your settings file or in T3 Code's secret store. -Any value beginning with `op://` is treated this way. Everything else is used exactly as typed, so -mixing literal variables and references on the same provider is fine. +Plain values are used exactly as typed, so mixing literal variables and references on the same +provider is fine. + +A plain value beginning with `op://` is also read from 1Password, from the CLI's default account. +That keeps older settings working; switch those variables to the 1Password source to pin them to an +account. To copy a reference in 1Password, open the item, use the field's overflow menu, and choose **Copy Secret Reference**. @@ -35,7 +44,7 @@ running the T3 Code server. Confirm it works from a normal shell first: ```bash -op read --no-newline "op://Private/claude-code/credential" +op read --account my.1password.com --no-newline "op://Private/claude-code/credential" ``` If that command prints your secret, T3 Code can read it too. If it asks you to sign in, sign in @@ -46,9 +55,10 @@ The vault has to be reachable from wherever `npx t3` or the desktop app is actua ## Do I Still Mark It Sensitive -You do not need to. A reference is not a secret, so there is nothing to protect by storing it as one. +No. A reference is not a secret, so there is nothing to protect by storing it as one, and variables +that read from 1Password are always stored as written. -Marking it sensitive still works if you prefer the redacted field in the UI, and the reference is +A plain `op://` value can still be marked sensitive if you prefer the redacted field, and it is resolved the same way either way. ## How Often Does It Ask Me To Unlock @@ -60,8 +70,9 @@ sending a message, and the background provider status check all reuse the value read, so a locked vault prompts you once rather than every few minutes. One unlock covers every reference T3 Code needs, across every provider. Starting the server and -refreshing provider status both read the whole set in a single request to 1Password, so five -providers backed by references cost the same one approval that one provider does. +refreshing provider status both read the whole set in a single request per 1Password account, so +five providers backed by references in one account cost the same one approval that one provider +does. ## I Rotated The Secret, How Do I Pick Up The New One @@ -91,4 +102,4 @@ The server log records which reference failed and what the 1Password CLI said ab ## Can I Use A Different Password Manager -Not yet. `op://` references are the only form T3 Code resolves today. +Not yet. 1Password is the only secret store T3 Code reads from today. diff --git a/packages/contracts/src/providerInstance.test.ts b/packages/contracts/src/providerInstance.test.ts index 1645eae48259..c8db40fc6808 100644 --- a/packages/contracts/src/providerInstance.test.ts +++ b/packages/contracts/src/providerInstance.test.ts @@ -6,6 +6,7 @@ import { ProviderInstanceConfig, ProviderInstanceConfigMap, ProviderInstanceId, + ProviderInstanceEnvironmentVariable, ProviderInstanceRef, } from "./providerInstance.ts"; @@ -14,6 +15,7 @@ const decodeProviderInstanceId = Schema.decodeUnknownSync(ProviderInstanceId); const decodeProviderInstanceRef = Schema.decodeUnknownSync(ProviderInstanceRef); const decodeProviderInstanceConfig = Schema.decodeUnknownSync(ProviderInstanceConfig); const decodeProviderInstanceConfigMap = Schema.decodeUnknownSync(ProviderInstanceConfigMap); +const decodeEnvironmentVariable = Schema.decodeUnknownSync(ProviderInstanceEnvironmentVariable); describe("provider slug validation (shared by driver + instance ids)", () => { const cases = [ @@ -206,3 +208,73 @@ describe("ProviderInstanceConfigMap", () => { ).toThrow(); }); }); + +describe("ProviderInstanceEnvironmentVariable secret sources", () => { + const onePassword = (reference: string, account = "my.1password.com") => ({ + name: "API_KEY", + value: { kind: "1password", reference, account }, + }); + + it.each([ + "op://Private/claude-code/credential", + "op://Home Lab/Claude Code/API Key", + "op://Private/claude-code/login section/password", + "op://Private/github/one-time password?attribute=otp", + ])("accepts the 1Password reference %s", (reference) => { + expect(decodeEnvironmentVariable(onePassword(reference)).value).toEqual({ + kind: "1password", + reference, + account: "my.1password.com", + }); + }); + + it.each([ + ["leading whitespace", " op://Private/item/field"], + ["trailing newline", "op://Private/item/field\n"], + ["control character", "op://Private/it\u0007em/field"], + ["missing scheme", "Private/item/field"], + ["missing field", "op://Private/item"], + ["too many segments", "op://a/b/c/d/e"], + ["empty segment", "op://Private//field"], + ["template delimiter", "op://Private/item}}/field"], + ["unsafe query", "op://Private/item/field?x="], + ])("rejects a reference with %s", (_label, reference) => { + expect(() => decodeEnvironmentVariable(onePassword(reference))).toThrow(); + }); + + it.each(["my", "my.1password.com", "team-acme.1password.eu", "ABCDEFGHIJKLMNOPQRSTUVWXYZ"])( + "accepts the account %s", + (account) => { + expect( + decodeEnvironmentVariable(onePassword("op://Private/item/field", account)).value, + ).toMatchObject({ account }); + }, + ); + + it.each([ + ["empty", ""], + ["a space", "my account"], + ["a leading dash", "--help"], + ["surrounding whitespace", " my "], + ])("rejects an account with %s", (_label, account) => { + expect(() => + decodeEnvironmentVariable(onePassword("op://Private/item/field", account)), + ).toThrow(); + }); + + it("rejects a 1Password source without an account", () => { + expect(() => + decodeEnvironmentVariable({ + name: "API_KEY", + value: { kind: "1password", reference: "op://Private/item/field" }, + }), + ).toThrow(); + }); + + it("keeps decoding literal values, including legacy op:// strings", () => { + expect(decodeEnvironmentVariable({ name: "API_KEY" }).value).toBe(""); + expect( + decodeEnvironmentVariable({ name: "API_KEY", value: "op://Private/item/field" }).value, + ).toBe("op://Private/item/field"); + }); +}); diff --git a/packages/contracts/src/providerInstance.ts b/packages/contracts/src/providerInstance.ts index c47bf992a4f4..73b63854457f 100644 --- a/packages/contracts/src/providerInstance.ts +++ b/packages/contracts/src/providerInstance.ts @@ -101,9 +101,104 @@ export const ProviderInstanceEnvironmentVariableName = TrimmedNonEmptyString.che export type ProviderInstanceEnvironmentVariableName = typeof ProviderInstanceEnvironmentVariableName.Type; +const ONE_PASSWORD_SECRET_REFERENCE_PREFIX = "op://"; +const ONE_PASSWORD_SECRET_REFERENCE_MAX_CHARS = 1024; +const ONE_PASSWORD_SECRET_REFERENCE_QUERY_PATTERN = /^[A-Za-z0-9._=&-]+$/; +const ONE_PASSWORD_ACCOUNT_MAX_CHARS = 253; +const ONE_PASSWORD_ACCOUNT_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._-]*$/; + +function hasControlCharacter(value: string): boolean { + for (let index = 0; index < value.length; index++) { + const code = value.charCodeAt(index); + if (code < 0x20 || code === 0x7f) return true; + } + return false; +} + +function onePasswordSecretReferenceIssue(value: string): string | undefined { + if (value !== value.trim()) { + return "1Password secret reference must not start or end with whitespace."; + } + if (hasControlCharacter(value)) { + return "1Password secret reference must not contain control characters."; + } + if (!value.startsWith(ONE_PASSWORD_SECRET_REFERENCE_PREFIX)) { + return "1Password secret reference must start with op://."; + } + // The resolver embeds references in an `op inject` template, where `{{` and + // `}}` delimit a reference. + if (value.includes("{{") || value.includes("}}")) { + return "1Password secret reference must not contain {{ or }}."; + } + const [path = "", query, ...extraQueries] = value + .slice(ONE_PASSWORD_SECRET_REFERENCE_PREFIX.length) + .split("?"); + if ( + extraQueries.length > 0 || + (query !== undefined && !ONE_PASSWORD_SECRET_REFERENCE_QUERY_PATTERN.test(query)) + ) { + return "1Password secret reference has an invalid query."; + } + const segments = path.split("/"); + if ( + segments.length < 3 || + segments.length > 4 || + segments.some((segment) => segment.trim().length === 0) + ) { + return "1Password secret reference must look like op://vault/item/field or op://vault/item/section/field."; + } + return undefined; +} + +/** + * A 1Password secret reference, `op://vault/item/[section/]field`, optionally + * followed by a `?query` such as `?attribute=otp`. Names may contain spaces. + */ +export const OnePasswordSecretReference = Schema.String.check( + Schema.isMaxLength(ONE_PASSWORD_SECRET_REFERENCE_MAX_CHARS), + Schema.makeFilter((value: string) => onePasswordSecretReferenceIssue(value) ?? true), +).pipe(Schema.brand("OnePasswordSecretReference")); +export type OnePasswordSecretReference = typeof OnePasswordSecretReference.Type; + +/** + * The 1Password account a reference is read from, in any form `op --account` + * accepts: account shorthand, sign-in address (`my.1password.com`), account ID, + * or user ID. It is passed to the CLI as an argument, so it can never start + * with `-`. + */ +export const OnePasswordAccount = Schema.String.check( + Schema.isMaxLength(ONE_PASSWORD_ACCOUNT_MAX_CHARS), + Schema.makeFilter((value: string) => + ONE_PASSWORD_ACCOUNT_PATTERN.test(value) + ? true + : "1Password account must be a shorthand, sign-in address, or ID with no spaces.", + ), +).pipe(Schema.brand("OnePasswordAccount")); +export type OnePasswordAccount = typeof OnePasswordAccount.Type; + +export const OnePasswordSecretSource = Schema.Struct({ + kind: Schema.Literal("1password"), + reference: OnePasswordSecretReference, + account: OnePasswordAccount, +}); +export type OnePasswordSecretSource = typeof OnePasswordSecretSource.Type; + +/** + * An environment value read from an external secret store at the moment the + * provider process starts, instead of a literal. Discriminated on `kind`. + */ +export const ProviderSecretSource = Schema.Union([OnePasswordSecretSource]); +export type ProviderSecretSource = typeof ProviderSecretSource.Type; + +/** + * `sensitive` and `valueRedacted` only describe literal string values; a + * secret source is not itself a secret and is stored as written. + */ export const ProviderInstanceEnvironmentVariable = Schema.Struct({ name: ProviderInstanceEnvironmentVariableName, - value: Schema.String.pipe(Schema.withDecodingDefault(Effect.succeed(""))), + value: Schema.Union([Schema.String, ProviderSecretSource]).pipe( + Schema.withDecodingDefault(Effect.succeed("")), + ), sensitive: Schema.Boolean.pipe(Schema.withDecodingDefault(Effect.succeed(false))), valueRedacted: Schema.optionalKey(Schema.Boolean), }); From 6289ddd56cd7472799dfe0d5ad4cefe202265adf Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Sat, 3 Oct 2026 19:12:54 -0400 Subject: [PATCH 02/12] refactor(contracts): leave 1Password reference shape to op Signed-off-by: Yordis Prieto --- .../contracts/src/providerInstance.test.ts | 5 +--- packages/contracts/src/providerInstance.ts | 24 ++++--------------- 2 files changed, 6 insertions(+), 23 deletions(-) diff --git a/packages/contracts/src/providerInstance.test.ts b/packages/contracts/src/providerInstance.test.ts index c8db40fc6808..e0267cbbbc49 100644 --- a/packages/contracts/src/providerInstance.test.ts +++ b/packages/contracts/src/providerInstance.test.ts @@ -233,11 +233,8 @@ describe("ProviderInstanceEnvironmentVariable secret sources", () => { ["trailing newline", "op://Private/item/field\n"], ["control character", "op://Private/it\u0007em/field"], ["missing scheme", "Private/item/field"], - ["missing field", "op://Private/item"], - ["too many segments", "op://a/b/c/d/e"], - ["empty segment", "op://Private//field"], + ["nothing after the scheme", "op://"], ["template delimiter", "op://Private/item}}/field"], - ["unsafe query", "op://Private/item/field?x="], ])("rejects a reference with %s", (_label, reference) => { expect(() => decodeEnvironmentVariable(onePassword(reference))).toThrow(); }); diff --git a/packages/contracts/src/providerInstance.ts b/packages/contracts/src/providerInstance.ts index 73b63854457f..7c005e830d0a 100644 --- a/packages/contracts/src/providerInstance.ts +++ b/packages/contracts/src/providerInstance.ts @@ -103,7 +103,6 @@ export type ProviderInstanceEnvironmentVariableName = const ONE_PASSWORD_SECRET_REFERENCE_PREFIX = "op://"; const ONE_PASSWORD_SECRET_REFERENCE_MAX_CHARS = 1024; -const ONE_PASSWORD_SECRET_REFERENCE_QUERY_PATTERN = /^[A-Za-z0-9._=&-]+$/; const ONE_PASSWORD_ACCOUNT_MAX_CHARS = 253; const ONE_PASSWORD_ACCOUNT_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._-]*$/; @@ -130,29 +129,16 @@ function onePasswordSecretReferenceIssue(value: string): string | undefined { if (value.includes("{{") || value.includes("}}")) { return "1Password secret reference must not contain {{ or }}."; } - const [path = "", query, ...extraQueries] = value - .slice(ONE_PASSWORD_SECRET_REFERENCE_PREFIX.length) - .split("?"); - if ( - extraQueries.length > 0 || - (query !== undefined && !ONE_PASSWORD_SECRET_REFERENCE_QUERY_PATTERN.test(query)) - ) { - return "1Password secret reference has an invalid query."; - } - const segments = path.split("/"); - if ( - segments.length < 3 || - segments.length > 4 || - segments.some((segment) => segment.trim().length === 0) - ) { - return "1Password secret reference must look like op://vault/item/field or op://vault/item/section/field."; + if (value.length === ONE_PASSWORD_SECRET_REFERENCE_PREFIX.length) { + return "1Password secret reference must name a vault, item, and field after op://."; } return undefined; } /** - * A 1Password secret reference, `op://vault/item/[section/]field`, optionally - * followed by a `?query` such as `?attribute=otp`. Names may contain spaces. + * A 1Password secret reference such as `op://vault/item/field`. Only what + * T3 Code itself depends on is checked here; `op` validates the rest and + * reports which part it could not resolve. */ export const OnePasswordSecretReference = Schema.String.check( Schema.isMaxLength(ONE_PASSWORD_SECRET_REFERENCE_MAX_CHARS), From bf4f1dc5dddf126425cf057c421f5a2a11e60848 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Sat, 3 Oct 2026 19:16:20 -0400 Subject: [PATCH 03/12] refactor(contracts): keep 1Password schemas in their own module Signed-off-by: Yordis Prieto --- packages/contracts/src/index.ts | 1 + packages/contracts/src/onePassword.test.ts | 58 ++++++++++++++++ packages/contracts/src/onePassword.ts | 69 +++++++++++++++++++ .../contracts/src/providerInstance.test.ts | 59 +--------------- packages/contracts/src/providerInstance.ts | 69 +------------------ 5 files changed, 132 insertions(+), 124 deletions(-) create mode 100644 packages/contracts/src/onePassword.test.ts create mode 100644 packages/contracts/src/onePassword.ts diff --git a/packages/contracts/src/index.ts b/packages/contracts/src/index.ts index 8690bb1b2390..25369301ef89 100644 --- a/packages/contracts/src/index.ts +++ b/packages/contracts/src/index.ts @@ -14,6 +14,7 @@ export * from "./remoteAccess.ts"; export * from "./ipc.ts"; export * from "./terminal.ts"; export * from "./provider.ts"; +export * from "./onePassword.ts"; export * from "./providerInstance.ts"; export * from "./providerSetup.ts"; export * from "./providerRuntime.ts"; diff --git a/packages/contracts/src/onePassword.test.ts b/packages/contracts/src/onePassword.test.ts new file mode 100644 index 000000000000..f1f8cff9e550 --- /dev/null +++ b/packages/contracts/src/onePassword.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "vite-plus/test"; +import * as Schema from "effect/Schema"; + +import { OnePasswordSecretSource } from "./onePassword.ts"; + +const decodeSource = Schema.decodeUnknownSync(OnePasswordSecretSource); + +describe("OnePasswordSecretSource", () => { + const onePassword = (reference: string, account = "my.1password.com") => ({ + kind: "1password", + reference, + account, + }); + + it.each([ + "op://Private/claude-code/credential", + "op://Home Lab/Claude Code/API Key", + "op://Private/claude-code/login section/password", + "op://Private/github/one-time password?attribute=otp", + ])("accepts the reference %s", (reference) => { + expect(decodeSource(onePassword(reference))).toEqual(onePassword(reference)); + }); + + it.each([ + ["leading whitespace", " op://Private/item/field"], + ["trailing newline", "op://Private/item/field\n"], + ["control character", "op://Private/it\u0007em/field"], + ["missing scheme", "Private/item/field"], + ["nothing after the scheme", "op://"], + ["template delimiter", "op://Private/item}}/field"], + ])("rejects a reference with %s", (_label, reference) => { + expect(() => decodeSource(onePassword(reference))).toThrow(); + }); + + it.each(["my", "my.1password.com", "team-acme.1password.eu", "ABCDEFGHIJKLMNOPQRSTUVWXYZ"])( + "accepts the account %s", + (account) => { + expect(decodeSource(onePassword("op://Private/item/field", account))).toMatchObject({ + account, + }); + }, + ); + + it.each([ + ["empty", ""], + ["a space", "my account"], + ["a leading dash", "--help"], + ["surrounding whitespace", " my "], + ])("rejects an account with %s", (_label, account) => { + expect(() => decodeSource(onePassword("op://Private/item/field", account))).toThrow(); + }); + + it("rejects a source without an account", () => { + expect(() => + decodeSource({ kind: "1password", reference: "op://Private/item/field" }), + ).toThrow(); + }); +}); diff --git a/packages/contracts/src/onePassword.ts b/packages/contracts/src/onePassword.ts new file mode 100644 index 000000000000..630aaff73ba8 --- /dev/null +++ b/packages/contracts/src/onePassword.ts @@ -0,0 +1,69 @@ +import * as Schema from "effect/Schema"; + +const ONE_PASSWORD_SECRET_REFERENCE_PREFIX = "op://"; +const ONE_PASSWORD_SECRET_REFERENCE_MAX_CHARS = 1024; +const ONE_PASSWORD_ACCOUNT_MAX_CHARS = 253; +const ONE_PASSWORD_ACCOUNT_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._-]*$/; + +function hasControlCharacter(value: string): boolean { + for (let index = 0; index < value.length; index++) { + const code = value.charCodeAt(index); + if (code < 0x20 || code === 0x7f) return true; + } + return false; +} + +function onePasswordSecretReferenceIssue(value: string): string | undefined { + if (value !== value.trim()) { + return "1Password secret reference must not start or end with whitespace."; + } + if (hasControlCharacter(value)) { + return "1Password secret reference must not contain control characters."; + } + if (!value.startsWith(ONE_PASSWORD_SECRET_REFERENCE_PREFIX)) { + return "1Password secret reference must start with op://."; + } + // The resolver embeds references in an `op inject` template, where `{{` and + // `}}` delimit a reference. + if (value.includes("{{") || value.includes("}}")) { + return "1Password secret reference must not contain {{ or }}."; + } + if (value.length === ONE_PASSWORD_SECRET_REFERENCE_PREFIX.length) { + return "1Password secret reference must name a vault, item, and field after op://."; + } + return undefined; +} + +/** + * A 1Password secret reference such as `op://vault/item/field`. Only what + * T3 Code itself depends on is checked here; `op` validates the rest and + * reports which part it could not resolve. + */ +export const OnePasswordSecretReference = Schema.String.check( + Schema.isMaxLength(ONE_PASSWORD_SECRET_REFERENCE_MAX_CHARS), + Schema.makeFilter((value: string) => onePasswordSecretReferenceIssue(value) ?? true), +).pipe(Schema.brand("OnePasswordSecretReference")); +export type OnePasswordSecretReference = typeof OnePasswordSecretReference.Type; + +/** + * The 1Password account a reference is read from, in any form `op --account` + * accepts: account shorthand, sign-in address (`my.1password.com`), account ID, + * or user ID. It is passed to the CLI as an argument, so it can never start + * with `-`. + */ +export const OnePasswordAccount = Schema.String.check( + Schema.isMaxLength(ONE_PASSWORD_ACCOUNT_MAX_CHARS), + Schema.makeFilter((value: string) => + ONE_PASSWORD_ACCOUNT_PATTERN.test(value) + ? true + : "1Password account must be a shorthand, sign-in address, or ID with no spaces.", + ), +).pipe(Schema.brand("OnePasswordAccount")); +export type OnePasswordAccount = typeof OnePasswordAccount.Type; + +export const OnePasswordSecretSource = Schema.Struct({ + kind: Schema.Literal("1password"), + reference: OnePasswordSecretReference, + account: OnePasswordAccount, +}); +export type OnePasswordSecretSource = typeof OnePasswordSecretSource.Type; diff --git a/packages/contracts/src/providerInstance.test.ts b/packages/contracts/src/providerInstance.test.ts index e0267cbbbc49..b60d2dc66757 100644 --- a/packages/contracts/src/providerInstance.test.ts +++ b/packages/contracts/src/providerInstance.test.ts @@ -210,62 +210,9 @@ describe("ProviderInstanceConfigMap", () => { }); describe("ProviderInstanceEnvironmentVariable secret sources", () => { - const onePassword = (reference: string, account = "my.1password.com") => ({ - name: "API_KEY", - value: { kind: "1password", reference, account }, - }); - - it.each([ - "op://Private/claude-code/credential", - "op://Home Lab/Claude Code/API Key", - "op://Private/claude-code/login section/password", - "op://Private/github/one-time password?attribute=otp", - ])("accepts the 1Password reference %s", (reference) => { - expect(decodeEnvironmentVariable(onePassword(reference)).value).toEqual({ - kind: "1password", - reference, - account: "my.1password.com", - }); - }); - - it.each([ - ["leading whitespace", " op://Private/item/field"], - ["trailing newline", "op://Private/item/field\n"], - ["control character", "op://Private/it\u0007em/field"], - ["missing scheme", "Private/item/field"], - ["nothing after the scheme", "op://"], - ["template delimiter", "op://Private/item}}/field"], - ])("rejects a reference with %s", (_label, reference) => { - expect(() => decodeEnvironmentVariable(onePassword(reference))).toThrow(); - }); - - it.each(["my", "my.1password.com", "team-acme.1password.eu", "ABCDEFGHIJKLMNOPQRSTUVWXYZ"])( - "accepts the account %s", - (account) => { - expect( - decodeEnvironmentVariable(onePassword("op://Private/item/field", account)).value, - ).toMatchObject({ account }); - }, - ); - - it.each([ - ["empty", ""], - ["a space", "my account"], - ["a leading dash", "--help"], - ["surrounding whitespace", " my "], - ])("rejects an account with %s", (_label, account) => { - expect(() => - decodeEnvironmentVariable(onePassword("op://Private/item/field", account)), - ).toThrow(); - }); - - it("rejects a 1Password source without an account", () => { - expect(() => - decodeEnvironmentVariable({ - name: "API_KEY", - value: { kind: "1password", reference: "op://Private/item/field" }, - }), - ).toThrow(); + it("decodes a 1Password source", () => { + const value = { kind: "1password", reference: "op://Private/item/field", account: "my" }; + expect(decodeEnvironmentVariable({ name: "API_KEY", value }).value).toEqual(value); }); it("keeps decoding literal values, including legacy op:// strings", () => { diff --git a/packages/contracts/src/providerInstance.ts b/packages/contracts/src/providerInstance.ts index 7c005e830d0a..49a416510f7d 100644 --- a/packages/contracts/src/providerInstance.ts +++ b/packages/contracts/src/providerInstance.ts @@ -36,6 +36,7 @@ import * as Effect from "effect/Effect"; import * as Schema from "effect/Schema"; import { TrimmedNonEmptyString } from "./baseSchemas.ts"; +import { OnePasswordSecretSource } from "./onePassword.ts"; const PROVIDER_SLUG_MAX_CHARS = 64; /** @@ -101,74 +102,6 @@ export const ProviderInstanceEnvironmentVariableName = TrimmedNonEmptyString.che export type ProviderInstanceEnvironmentVariableName = typeof ProviderInstanceEnvironmentVariableName.Type; -const ONE_PASSWORD_SECRET_REFERENCE_PREFIX = "op://"; -const ONE_PASSWORD_SECRET_REFERENCE_MAX_CHARS = 1024; -const ONE_PASSWORD_ACCOUNT_MAX_CHARS = 253; -const ONE_PASSWORD_ACCOUNT_PATTERN = /^[A-Za-z0-9][A-Za-z0-9._-]*$/; - -function hasControlCharacter(value: string): boolean { - for (let index = 0; index < value.length; index++) { - const code = value.charCodeAt(index); - if (code < 0x20 || code === 0x7f) return true; - } - return false; -} - -function onePasswordSecretReferenceIssue(value: string): string | undefined { - if (value !== value.trim()) { - return "1Password secret reference must not start or end with whitespace."; - } - if (hasControlCharacter(value)) { - return "1Password secret reference must not contain control characters."; - } - if (!value.startsWith(ONE_PASSWORD_SECRET_REFERENCE_PREFIX)) { - return "1Password secret reference must start with op://."; - } - // The resolver embeds references in an `op inject` template, where `{{` and - // `}}` delimit a reference. - if (value.includes("{{") || value.includes("}}")) { - return "1Password secret reference must not contain {{ or }}."; - } - if (value.length === ONE_PASSWORD_SECRET_REFERENCE_PREFIX.length) { - return "1Password secret reference must name a vault, item, and field after op://."; - } - return undefined; -} - -/** - * A 1Password secret reference such as `op://vault/item/field`. Only what - * T3 Code itself depends on is checked here; `op` validates the rest and - * reports which part it could not resolve. - */ -export const OnePasswordSecretReference = Schema.String.check( - Schema.isMaxLength(ONE_PASSWORD_SECRET_REFERENCE_MAX_CHARS), - Schema.makeFilter((value: string) => onePasswordSecretReferenceIssue(value) ?? true), -).pipe(Schema.brand("OnePasswordSecretReference")); -export type OnePasswordSecretReference = typeof OnePasswordSecretReference.Type; - -/** - * The 1Password account a reference is read from, in any form `op --account` - * accepts: account shorthand, sign-in address (`my.1password.com`), account ID, - * or user ID. It is passed to the CLI as an argument, so it can never start - * with `-`. - */ -export const OnePasswordAccount = Schema.String.check( - Schema.isMaxLength(ONE_PASSWORD_ACCOUNT_MAX_CHARS), - Schema.makeFilter((value: string) => - ONE_PASSWORD_ACCOUNT_PATTERN.test(value) - ? true - : "1Password account must be a shorthand, sign-in address, or ID with no spaces.", - ), -).pipe(Schema.brand("OnePasswordAccount")); -export type OnePasswordAccount = typeof OnePasswordAccount.Type; - -export const OnePasswordSecretSource = Schema.Struct({ - kind: Schema.Literal("1password"), - reference: OnePasswordSecretReference, - account: OnePasswordAccount, -}); -export type OnePasswordSecretSource = typeof OnePasswordSecretSource.Type; - /** * An environment value read from an external secret store at the moment the * provider process starts, instead of a literal. Discriminated on `kind`. From 4d448cce69b73e17e0b85560481b52989e076b9a Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Sat, 3 Oct 2026 19:18:19 -0400 Subject: [PATCH 04/12] fix(server): resolved provider environments keep which values are sensitive Signed-off-by: Yordis Prieto --- .../ProviderAdapterRegistry.test.ts | 6 +++--- .../src/provider/Drivers/CodexDriver.test.ts | 4 ++-- .../src/provider/Drivers/GrokDriver.test.ts | 2 +- .../Layers/ProviderSecretResolverLive.test.ts | 18 ++++++++++++------ .../Layers/ProviderSecretResolverLive.ts | 6 +++--- .../ProviderInstanceEnvironment.test.ts | 17 ++++++++++------- .../provider/ProviderInstanceEnvironment.ts | 8 +++++--- .../Services/ProviderSecretResolver.ts | 4 ++-- 8 files changed, 38 insertions(+), 27 deletions(-) diff --git a/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts b/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts index ee42e713cc26..76a28c6c82e8 100644 --- a/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts +++ b/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts @@ -347,13 +347,13 @@ it.effect("opens v2 sessions with resolved secrets and rebuilds them when a secr Effect.gen(function* () { const variables = []; const unresolved = []; - for (const { name, value } of environment ?? []) { + for (const { name, value, sensitive } of environment ?? []) { if (value === apiKeyReference) { - variables.push({ name, value: yield* Ref.get(apiKey) }); + variables.push({ name, value: yield* Ref.get(apiKey), sensitive: true }); } else if (typeof value !== "string" || value.startsWith("op://")) { unresolved.push(name); } else { - variables.push({ name, value }); + variables.push({ name, value, sensitive }); } } return { variables, unresolved }; diff --git a/apps/server/src/provider/Drivers/CodexDriver.test.ts b/apps/server/src/provider/Drivers/CodexDriver.test.ts index fcdacfe1a1ab..4d1a4ce3519f 100644 --- a/apps/server/src/provider/Drivers/CodexDriver.test.ts +++ b/apps/server/src/provider/Drivers/CodexDriver.test.ts @@ -152,7 +152,7 @@ it.layer(testLayer)("CodexDriver", (it) => { instanceId, displayName: "Restored account", enabled: true, - environment: [{ name: "OPENAI_API_KEY", value: "ambient-key" }], + environment: [{ name: "OPENAI_API_KEY", value: "ambient-key", sensitive: true }], config: { ...CodexDriver.defaultConfig(), setupMode: "managed", homePath: sharedHome }, }).pipe( Effect.provideService( @@ -546,7 +546,7 @@ it.layer(testLayer)("CodexDriver", (it) => { instanceId: ProviderInstanceId.make("codex-mise-shim"), displayName: "Codex shim test", enabled: false, - environment: [{ name: "PATH", value: lookupPath }], + environment: [{ name: "PATH", value: lookupPath, sensitive: false }], config: { ...CodexDriver.defaultConfig(), binaryPath: fixture.commandName, diff --git a/apps/server/src/provider/Drivers/GrokDriver.test.ts b/apps/server/src/provider/Drivers/GrokDriver.test.ts index fc08006b4c31..a9d6791a3e50 100644 --- a/apps/server/src/provider/Drivers/GrokDriver.test.ts +++ b/apps/server/src/provider/Drivers/GrokDriver.test.ts @@ -65,7 +65,7 @@ it.layer(testLayer)("GrokDriver", (it) => { instanceId: ProviderInstanceId.make("grok-update"), displayName: "Grok test", enabled: false, - environment: [{ name: "GROK_HOME", value: grokHome }], + environment: [{ name: "GROK_HOME", value: grokHome, sensitive: false }], config: { ...GrokDriver.defaultConfig(), binaryPath }, }); diff --git a/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts b/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts index 7084be855f20..2548e2d8b091 100644 --- a/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts +++ b/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts @@ -67,7 +67,13 @@ describe("ProviderSecretResolverLive", () => { const resolved = yield* resolver.resolve(environment); assert.deepStrictEqual(resolved, { - variables: [{ name: "CLAUDE_SECURESTORAGE_CONFIG_DIR", value: "/home/u/.claude/work" }], + variables: [ + { + name: "CLAUDE_SECURESTORAGE_CONFIG_DIR", + value: "/home/u/.claude/work", + sensitive: false, + }, + ], unresolved: [], }); assert.strictEqual(spawner.invocations.length, 0); @@ -460,7 +466,7 @@ describe("ProviderSecretResolverLive with 1Password accounts", () => { ); assert.deepStrictEqual(resolved, { - variables: [{ name: "CODEX_TOKEN", value: "sk-work-token" }], + variables: [{ name: "CODEX_TOKEN", value: "sk-work-token", sensitive: true }], unresolved: [], }); assert.deepStrictEqual(spawner.invocations, [ @@ -492,8 +498,8 @@ describe("ProviderSecretResolverLive with 1Password accounts", () => { ); assert.deepStrictEqual(resolved.variables, [ - { name: "HOME_TOKEN", value: "sk-home-token" }, - { name: "WORK_TOKEN", value: "sk-work-token" }, + { name: "HOME_TOKEN", value: "sk-home-token", sensitive: true }, + { name: "WORK_TOKEN", value: "sk-work-token", sensitive: true }, ]); assert.strictEqual(spawner.invocations.length, 2); }).pipe( @@ -542,8 +548,8 @@ describe("ProviderSecretResolverLive with 1Password accounts", () => { ]), ); assert.deepStrictEqual(resolved.variables, [ - { name: "HOME_CODEX", value: "home-codex" }, - { name: "WORK_CLAUDE", value: "work-claude" }, + { name: "HOME_CODEX", value: "home-codex", sensitive: true }, + { name: "WORK_CLAUDE", value: "work-claude", sensitive: true }, ]); assert.strictEqual(spawner.invocations.length, 2); }).pipe( diff --git a/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts b/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts index 0224ff40c40b..7922b9e9be5c 100644 --- a/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts +++ b/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts @@ -203,11 +203,11 @@ export const ProviderSecretResolverLive = Layer.effect( Effect.gen(function* () { const resolved: Array = []; const unresolved: Array = []; - for (const { name, value } of environment ?? []) { + for (const { name, value, sensitive } of environment ?? []) { const reference = providerSecretReference(value); if (reference === undefined) { if (typeof value === "string") { - resolved.push({ name, value }); + resolved.push({ name, value, sensitive }); } continue; } @@ -216,7 +216,7 @@ export const ProviderSecretResolverLive = Layer.effect( unresolved.push(name); continue; } - resolved.push({ name, value: secret }); + resolved.push({ name, value: secret, sensitive: true }); } return { variables: resolved, unresolved }; }); diff --git a/apps/server/src/provider/ProviderInstanceEnvironment.test.ts b/apps/server/src/provider/ProviderInstanceEnvironment.test.ts index a2281bb14d0f..5eae7d1cc8cb 100644 --- a/apps/server/src/provider/ProviderInstanceEnvironment.test.ts +++ b/apps/server/src/provider/ProviderInstanceEnvironment.test.ts @@ -26,9 +26,9 @@ describe("mergeProviderInstanceEnvironment", () => { }; const environment = mergeProviderInstanceEnvironment( [ - { name: "CODEX_HOME", value }, - { name: "CLAUDE_CONFIG_DIR", value }, - { name: "CUSTOM_VALUE", value }, + { name: "CODEX_HOME", value, sensitive: false }, + { name: "CLAUDE_CONFIG_DIR", value, sensitive: false }, + { name: "CUSTOM_VALUE", value, sensitive: false }, ], baseEnv, ); @@ -49,7 +49,10 @@ describe("mergeProviderInstanceEnvironment", () => { const baseEnv = { CODEX_HOME: "~/.codex", CLAUDE_CONFIG_DIR: "~\\.claude" }; expect( - mergeProviderInstanceEnvironment([{ name: "CUSTOM_VALUE", value: "~/.custom" }], baseEnv), + mergeProviderInstanceEnvironment( + [{ name: "CUSTOM_VALUE", value: "~/.custom", sensitive: false }], + baseEnv, + ), ).toEqual({ ...baseEnv, CUSTOM_VALUE: "~/.custom" }); }); @@ -57,8 +60,8 @@ describe("mergeProviderInstanceEnvironment", () => { expect( mergeProviderInstanceEnvironment( [ - { name: "OPENROUTER_API_KEY", value: "sk-or-test" }, - { name: "ANTHROPIC_API_KEY", value: "" }, + { name: "OPENROUTER_API_KEY", value: "sk-or-test", sensitive: true }, + { name: "ANTHROPIC_API_KEY", value: "", sensitive: false }, ], { ANTHROPIC_API_KEY: "inherited", PATH: "/bin" }, ), @@ -84,7 +87,7 @@ describe("literalProviderInstanceEnvironment", () => { ]); expect(literalProviderInstanceEnvironment(environment)).toEqual([ - { name: "CODEX_HOME", value: "~/.codex-work" }, + { name: "CODEX_HOME", value: "~/.codex-work", sensitive: false }, ]); }); }); diff --git a/apps/server/src/provider/ProviderInstanceEnvironment.ts b/apps/server/src/provider/ProviderInstanceEnvironment.ts index d4c8d55074ac..02c015feeb22 100644 --- a/apps/server/src/provider/ProviderInstanceEnvironment.ts +++ b/apps/server/src/provider/ProviderInstanceEnvironment.ts @@ -10,11 +10,13 @@ import { providerSecretReference } from "./ProviderSecretReference.ts"; * A provider environment variable whose value is ready for a child process. * Configured variables may name a secret source instead of a value; only * `ProviderSecretResolver` turns those into this shape, so a source cannot - * reach a process by accident. + * reach a process by accident. A value read from a secret store is always + * sensitive. */ export interface ResolvedProviderEnvironmentVariable { readonly name: ProviderInstanceEnvironmentVariableName; readonly value: string; + readonly sensitive: boolean; } export type ResolvedProviderEnvironment = ReadonlyArray; @@ -28,9 +30,9 @@ export function literalProviderInstanceEnvironment( environment: ProviderInstanceEnvironment | undefined, ): ResolvedProviderEnvironment { const literals: Array = []; - for (const { name, value } of environment ?? []) { + for (const { name, value, sensitive } of environment ?? []) { if (typeof value === "string" && providerSecretReference(value) === undefined) { - literals.push({ name, value }); + literals.push({ name, value, sensitive }); } } return literals; diff --git a/apps/server/src/provider/Services/ProviderSecretResolver.ts b/apps/server/src/provider/Services/ProviderSecretResolver.ts index 1eafe2857b9e..ab0aec3f9d0e 100644 --- a/apps/server/src/provider/Services/ProviderSecretResolver.ts +++ b/apps/server/src/provider/Services/ProviderSecretResolver.ts @@ -87,9 +87,9 @@ const resolveWithoutSecretStore = ( ): ResolvedProviderInstanceEnvironment => { const variables: Array = []; const unresolved: Array = []; - for (const { name, value } of environment ?? []) { + for (const { name, value, sensitive } of environment ?? []) { if (typeof value === "string") { - variables.push({ name, value }); + variables.push({ name, value, sensitive }); } else { unresolved.push(name); } From 9da3a99e7e9261c41bbe4aa8ceb6794532a08e44 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Sat, 3 Oct 2026 19:32:13 -0400 Subject: [PATCH 05/12] refactor(server): drop plain op:// values as 1Password references Signed-off-by: Yordis Prieto --- .../ProviderAdapterRegistry.test.ts | 20 +++-- .../src/provider/Drivers/ClaudeCredential.ts | 4 +- .../ProviderInstanceRegistryLive.test.ts | 15 +++- .../provider/Layers/ProviderRegistry.test.ts | 45 +++++++--- .../Layers/ProviderSecretResolverLive.test.ts | 86 +++++++++++-------- .../Layers/ProviderSecretResolverLive.ts | 16 ++-- .../ProviderInstanceEnvironment.test.ts | 5 +- .../provider/ProviderInstanceEnvironment.ts | 5 +- .../provider/ProviderSecretReference.test.ts | 44 ++++------ .../src/provider/ProviderSecretReference.ts | 33 ++----- .../Services/ProviderSecretResolver.ts | 11 ++- apps/server/src/server.ts | 2 +- .../settings/ProviderInstanceCard.tsx | 14 +++ ...0016-provider-secrets-live-in-1password.md | 8 +- docs/internals/providers.md | 4 +- docs/user/provider-secrets.md | 9 +- .../contracts/src/providerInstance.test.ts | 2 +- 17 files changed, 180 insertions(+), 143 deletions(-) diff --git a/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts b/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts index 76a28c6c82e8..49b4b6ebcc6c 100644 --- a/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts +++ b/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts @@ -2,6 +2,7 @@ import * as NodeServices from "@effect/platform-node/NodeServices"; import { assert, it } from "@effect/vitest"; import { ProviderDriverKind, + ProviderInstanceEnvironment, ProviderInstanceId, ProviderSessionId, ThreadId, @@ -38,6 +39,13 @@ const driver = ProviderDriverKind.make("codex"); const personalId = ProviderInstanceId.make("codex_personal"); const workId = ProviderInstanceId.make("codex_work"); +const HOME_ACCOUNT = "my.1password.com"; +const decodeEnvironment = Schema.decodeSync(ProviderInstanceEnvironment); +const onePasswordVariable = (name: string, reference: string) => ({ + name, + value: { kind: "1password" as const, reference, account: HOME_ACCOUNT }, +}); + const makeAdapter = (instanceId: ProviderInstanceId): ProviderAdapterV2Shape => ({ instanceId, @@ -348,9 +356,9 @@ it.effect("opens v2 sessions with resolved secrets and rebuilds them when a secr const variables = []; const unresolved = []; for (const { name, value, sensitive } of environment ?? []) { - if (value === apiKeyReference) { + if (typeof value !== "string" && value.reference === apiKeyReference) { variables.push({ name, value: yield* Ref.get(apiKey), sensitive: true }); - } else if (typeof value !== "string" || value.startsWith("op://")) { + } else if (typeof value !== "string") { unresolved.push(name); } else { variables.push({ name, value, sensitive }); @@ -408,11 +416,11 @@ it.effect("opens v2 sessions with resolved secrets and rebuilds them when a secr configMap: { [secretInstanceId]: { driver, - environment: [ - { name: "OPENAI_API_KEY", value: apiKeyReference, sensitive: true }, - { name: "ANTHROPIC_API_KEY", value: "op://Vault/Locked/api-key", sensitive: true }, + environment: decodeEnvironment([ + onePasswordVariable("OPENAI_API_KEY", apiKeyReference), + onePasswordVariable("ANTHROPIC_API_KEY", "op://Vault/Locked/api-key"), { name: "CODEX_PROFILE", value: "work", sensitive: false }, - ], + ]), config: {}, }, }, diff --git a/apps/server/src/provider/Drivers/ClaudeCredential.ts b/apps/server/src/provider/Drivers/ClaudeCredential.ts index 295add87aff5..8e82895f7a75 100644 --- a/apps/server/src/provider/Drivers/ClaudeCredential.ts +++ b/apps/server/src/provider/Drivers/ClaudeCredential.ts @@ -49,8 +49,8 @@ const VERIFY_TIMEOUT = Duration.seconds(10); * The OAuth token a Claude instance was configured with, if any. * * Reads the same variable the CLI itself reads, so an instance whose - * environment carries an `op://` reference is checked with whatever that - * reference resolved to. + * environment reads the token from a secret store is checked with whatever + * that secret resolved to. */ export function claudeOAuthTokenFromEnvironment( environment: NodeJS.ProcessEnv, diff --git a/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts b/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts index 85fa7df25b6d..5d90e83a0fff 100644 --- a/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts +++ b/apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts @@ -37,6 +37,7 @@ import { ProviderDriverKind, type ProviderInstanceConfig, type ProviderInstanceConfigMap, + ProviderInstanceEnvironment, ProviderInstanceId, } from "@t3tools/contracts"; import { HostProcessPlatform, isHostWindows } from "@t3tools/shared/hostProcess"; @@ -48,6 +49,7 @@ import * as Fiber from "effect/Fiber"; import * as Layer from "effect/Layer"; import * as Path from "effect/Path"; import * as Ref from "effect/Ref"; +import * as Schema from "effect/Schema"; import * as Stream from "effect/Stream"; import { HttpClient, HttpClientResponse } from "effect/unstable/http"; @@ -871,6 +873,8 @@ describe("ProviderInstanceRegistryLive: rebuildInstanceWhen", () => { ), ); + const decodeEnvironment = Schema.decodeSync(ProviderInstanceEnvironment); + const codexDriverKind = ProviderDriverKind.make("codex"); const firstId = ProviderInstanceId.make("codex_first"); const secondId = ProviderInstanceId.make("codex_second"); @@ -878,7 +882,16 @@ describe("ProviderInstanceRegistryLive: rebuildInstanceWhen", () => { driver: codexDriverKind, displayName: "Codex (first)", enabled: false, - environment: [{ name: "OP_TOKEN", value: "op://Vault/Item/token", sensitive: true }], + environment: decodeEnvironment([ + { + name: "OP_TOKEN", + value: { + kind: "1password", + reference: "op://Vault/Item/token", + account: "my.1password.com", + }, + }, + ]), config: makeCodexConfig({ homePath: "/home/julius/.codex_first" }), }; const secondEntry: ProviderInstanceConfig = { diff --git a/apps/server/src/provider/Layers/ProviderRegistry.test.ts b/apps/server/src/provider/Layers/ProviderRegistry.test.ts index 260b6ef1931b..b56c43821616 100644 --- a/apps/server/src/provider/Layers/ProviderRegistry.test.ts +++ b/apps/server/src/provider/Layers/ProviderRegistry.test.ts @@ -24,6 +24,7 @@ import { CodexSettings, DEFAULT_SERVER_SETTINGS, ProviderDriverKind, + ProviderInstanceEnvironment, ProviderInstanceId, ServerSettings, type ServerProvider, @@ -72,6 +73,13 @@ import { makeManualOnlyProviderMaintenanceCapabilities } from "../providerMainte const decodeServerSettings = Schema.decodeSync(ServerSettings); const encodeServerSettings = Schema.encodeSync(ServerSettings); const encodedDefaultServerSettings = encodeServerSettings(DEFAULT_SERVER_SETTINGS); +const decodeEnvironment = Schema.decodeSync(ProviderInstanceEnvironment); +const HOME_ACCOUNT = "my.1password.com"; +const onePasswordVariable = (name: string, reference: string, sensitive = true) => ({ + name, + value: { kind: "1password" as const, reference, account: HOME_ACCOUNT }, + sensitive, +}); const defaultClaudeSettings: ClaudeSettings = Schema.decodeSync(ClaudeSettings)({}); const defaultCodexSettings: CodexSettings = Schema.decodeSync(CodexSettings)({}); @@ -1628,9 +1636,9 @@ it.layer(Layer.mergeAll(TestNodeServices, ServerSettingsModule.layerTest(), Test rebuildInstanceWhen: (instanceId, shouldRebuild) => shouldRebuild({ driver: codexDriver, - environment: [ - { name: "CODEX_TOKEN", value: "op://Vault/Item/token", sensitive: true }, - ], + environment: decodeEnvironment([ + onePasswordVariable("CODEX_TOKEN", "op://Vault/Item/token"), + ]), }) ? Ref.update(rebuiltIds, (previous) => [...previous, instanceId]).pipe( Effect.as(true), @@ -1709,9 +1717,9 @@ it.layer(Layer.mergeAll(TestNodeServices, ServerSettingsModule.layerTest(), Test rebuildInstanceWhen: (instanceId, shouldRebuild) => shouldRebuild({ driver: codexDriver, - environment: [ - { name: "CODEX_TOKEN", value: "op://Vault/Item/token", sensitive: true }, - ], + environment: decodeEnvironment([ + onePasswordVariable("CODEX_TOKEN", "op://Vault/Item/token"), + ]), }) ? Ref.update(rebuiltIds, (previous) => [...previous, instanceId]).pipe( Effect.as(true), @@ -1785,9 +1793,8 @@ it.layer(Layer.mergeAll(TestNodeServices, ServerSettingsModule.layerTest(), Test ]).pipe(Effect.asVoid), invalidate: Effect.void, }); - const environmentFor = (reference: string) => [ - { name: "TOKEN", value: reference, sensitive: true }, - ]; + const environmentFor = (reference: string) => + decodeEnvironment([onePasswordVariable("TOKEN", reference)]); const instanceRegistryLayer = Layer.succeed( ProviderInstanceRegistry.ProviderInstanceRegistry, { @@ -2942,14 +2949,30 @@ it.layer(Layer.mergeAll(TestNodeServices, ServerSettingsModule.layerTest(), Test displayName: "Claude Secret", enabled: false, environment: [ - { name: "CLAUDE_CODE_OAUTH_TOKEN", value: claudeReference, sensitive: true }, + { + name: "CLAUDE_CODE_OAUTH_TOKEN", + value: { + kind: "1password", + reference: claudeReference, + account: HOME_ACCOUNT, + }, + }, ], }, codex_secret: { driver: "codex", displayName: "Codex Secret", enabled: false, - environment: [{ name: "TOKEN", value: codexReference, sensitive: true }], + environment: [ + { + name: "TOKEN", + value: { + kind: "1password", + reference: codexReference, + account: HOME_ACCOUNT, + }, + }, + ], }, } as unknown as ContractServerSettings["providerInstances"], }), diff --git a/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts b/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts index 2548e2d8b091..a3afdd361e75 100644 --- a/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts +++ b/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts @@ -19,9 +19,22 @@ const decodeEnvironment = Schema.decodeSync(ProviderInstanceEnvironment); const TOKEN_REFERENCE = "op://Private/claude-code/credential"; -/** A plain-string `op://` value, read from the default `op` account. */ -const legacy = (reference: string) => - new OnePasswordSecretReference({ reference, account: undefined }); +const HOME_ACCOUNT = "my.1password.com"; +const WORK_ACCOUNT = "acme.1password.com"; + +const onePasswordVariable = (name: string, reference: string, account: string) => ({ + name, + value: { kind: "1password" as const, reference, account }, +}); + +const onePassword = (reference: string, account: string) => { + const [variable] = decodeEnvironment([onePasswordVariable("TOKEN", reference, account)]); + const source = variable?.value; + if (source === undefined || typeof source === "string") { + throw new Error("expected a 1Password source"); + } + return new OnePasswordSecretReference({ reference: source.reference, account: source.account }); +}; /** * Spawner that answers every `op read` with `result` and records the argv it @@ -94,7 +107,7 @@ describe("ProviderSecretResolverLive", () => { const resolved = yield* resolver.resolve( decodeEnvironment([ { name: "CLAUDE_SECURESTORAGE_CONFIG_DIR", value: "/home/u/.claude/work" }, - { name: "CLAUDE_CODE_OAUTH_TOKEN", value: TOKEN_REFERENCE, sensitive: true }, + onePasswordVariable("CLAUDE_CODE_OAUTH_TOKEN", TOKEN_REFERENCE, HOME_ACCOUNT), ]), ); @@ -105,7 +118,9 @@ describe("ProviderSecretResolverLive", () => { ["CLAUDE_CODE_OAUTH_TOKEN", "sk-live-token"], ], ); - assert.deepStrictEqual(spawner.invocations, [["read", "--no-newline", TOKEN_REFERENCE]]); + assert.deepStrictEqual(spawner.invocations, [ + ["read", "--account", HOME_ACCOUNT, "--no-newline", TOKEN_REFERENCE], + ]); }).pipe( Effect.provide( ProviderSecretResolverLive.pipe( @@ -120,7 +135,7 @@ describe("ProviderSecretResolverLive", () => { return Effect.gen(function* () { const resolver = yield* ProviderSecretResolver; const environment = decodeEnvironment([ - { name: "CLAUDE_CODE_OAUTH_TOKEN", value: TOKEN_REFERENCE, sensitive: true }, + onePasswordVariable("CLAUDE_CODE_OAUTH_TOKEN", TOKEN_REFERENCE, HOME_ACCOUNT), ]); // Every thread start and every instance rebuild resolves again; none of @@ -156,7 +171,7 @@ describe("ProviderSecretResolverLive", () => { const resolved = yield* resolver.resolve( decodeEnvironment([ { name: "CLAUDE_SECURESTORAGE_CONFIG_DIR", value: "/home/u/.claude/work" }, - { name: "CLAUDE_CODE_OAUTH_TOKEN", value: TOKEN_REFERENCE, sensitive: true }, + onePasswordVariable("CLAUDE_CODE_OAUTH_TOKEN", TOKEN_REFERENCE, HOME_ACCOUNT), ]), ); @@ -182,7 +197,7 @@ describe("ProviderSecretResolverLive", () => { return Effect.gen(function* () { const resolver = yield* ProviderSecretResolver; const environment = decodeEnvironment([ - { name: "CLAUDE_CODE_OAUTH_TOKEN", value: TOKEN_REFERENCE, sensitive: true }, + onePasswordVariable("CLAUDE_CODE_OAUTH_TOKEN", TOKEN_REFERENCE, HOME_ACCOUNT), ]); yield* resolver.resolve(environment); @@ -278,11 +293,16 @@ describe("ProviderSecretResolverLive.prime", () => { return Effect.gen(function* () { const resolver = yield* ProviderSecretResolver; - yield* resolver.prime([legacy(TOKEN_REFERENCE), legacy(SECOND_REFERENCE)]); + yield* resolver.prime([ + onePassword(TOKEN_REFERENCE, HOME_ACCOUNT), + onePassword(SECOND_REFERENCE, HOME_ACCOUNT), + ]); assert.strictEqual(spawner.invocations.length, 1); - assert.deepStrictEqual(Array.from(spawner.invocations[0] ?? []).slice(0, 2), [ + assert.deepStrictEqual(Array.from(spawner.invocations[0] ?? []).slice(0, 4), [ "inject", + "--account", + HOME_ACCOUNT, "-i", ]); @@ -290,11 +310,11 @@ describe("ProviderSecretResolverLive.prime", () => { // one authorization the batch already paid for. const claude = yield* resolver.resolve( decodeEnvironment([ - { name: "CLAUDE_CODE_OAUTH_TOKEN", value: TOKEN_REFERENCE, sensitive: true }, + onePasswordVariable("CLAUDE_CODE_OAUTH_TOKEN", TOKEN_REFERENCE, HOME_ACCOUNT), ]), ); const codex = yield* resolver.resolve( - decodeEnvironment([{ name: "CODEX_TOKEN", value: SECOND_REFERENCE, sensitive: true }]), + decodeEnvironment([onePasswordVariable("CODEX_TOKEN", SECOND_REFERENCE, HOME_ACCOUNT)]), ); assert.strictEqual(claude.variables?.[0]?.value, "sk-claude-token"); @@ -321,7 +341,10 @@ describe("ProviderSecretResolverLive.prime", () => { return Effect.gen(function* () { const resolver = yield* ProviderSecretResolver; - yield* resolver.prime([legacy(TOKEN_REFERENCE), legacy(SECOND_REFERENCE)]); + yield* resolver.prime([ + onePassword(TOKEN_REFERENCE, HOME_ACCOUNT), + onePassword(SECOND_REFERENCE, HOME_ACCOUNT), + ]); // The batch is still attempted; it is the recovery that is per reference. assert.strictEqual(spawner.invocations[0]?.[0], "inject"); @@ -331,11 +354,11 @@ describe("ProviderSecretResolverLive.prime", () => { // still resolves and only the bad one is reported unresolved. const claude = yield* resolver.resolve( decodeEnvironment([ - { name: "CLAUDE_CODE_OAUTH_TOKEN", value: TOKEN_REFERENCE, sensitive: true }, + onePasswordVariable("CLAUDE_CODE_OAUTH_TOKEN", TOKEN_REFERENCE, HOME_ACCOUNT), ]), ); const codex = yield* resolver.resolve( - decodeEnvironment([{ name: "CODEX_TOKEN", value: SECOND_REFERENCE, sensitive: true }]), + decodeEnvironment([onePasswordVariable("CODEX_TOKEN", SECOND_REFERENCE, HOME_ACCOUNT)]), ); assert.strictEqual(claude.variables?.[0]?.value, "sk-claude-token"); @@ -366,14 +389,17 @@ describe("ProviderSecretResolverLive.prime", () => { return Effect.gen(function* () { const resolver = yield* ProviderSecretResolver; - yield* resolver.prime([legacy(TOKEN_REFERENCE), legacy(SECOND_REFERENCE)]); + yield* resolver.prime([ + onePassword(TOKEN_REFERENCE, HOME_ACCOUNT), + onePassword(SECOND_REFERENCE, HOME_ACCOUNT), + ]); // `op` only reads piped input from a named pipe, and Node hands a child // a socket pair, so a template offered on stdin is never seen and the // batch fails every time. The `-i` path is the delivery that works. const args = Array.from(spawner.invocations[0] ?? []); - assert.deepStrictEqual(args.slice(0, 2), ["inject", "-i"]); - assert.isTrue((args[2] ?? "").length > 0); + assert.deepStrictEqual(args.slice(0, 4), ["inject", "--account", HOME_ACCOUNT, "-i"]); + assert.isTrue((args[4] ?? "").length > 0); assert.deepStrictEqual(spawner.stdinUses, [false]); // The file `op` was pointed at held both references and nothing else, @@ -405,7 +431,10 @@ describe("ProviderSecretResolverLive.prime", () => { return Effect.gen(function* () { const resolver = yield* ProviderSecretResolver; - yield* resolver.prime([legacy(TOKEN_REFERENCE), legacy(SECOND_REFERENCE)]); + yield* resolver.prime([ + onePassword(TOKEN_REFERENCE, HOME_ACCOUNT), + onePassword(SECOND_REFERENCE, HOME_ACCOUNT), + ]); assert.isTrue(templatePath.length > 0); assert.isFalse(NodeFS.existsSync(templatePath)); @@ -423,7 +452,7 @@ describe("ProviderSecretResolverLive.prime", () => { return Effect.gen(function* () { const resolver = yield* ProviderSecretResolver; - yield* resolver.prime([legacy(TOKEN_REFERENCE)]); + yield* resolver.prime([onePassword(TOKEN_REFERENCE, HOME_ACCOUNT)]); // One reference is one prompt either way, and `op read` names the // reference it could not resolve. @@ -438,23 +467,6 @@ describe("ProviderSecretResolverLive.prime", () => { }); }); -const HOME_ACCOUNT = "my.1password.com"; -const WORK_ACCOUNT = "acme.1password.com"; - -const onePasswordVariable = (name: string, reference: string, account: string) => ({ - name, - value: { kind: "1password" as const, reference, account }, -}); - -const onePassword = (reference: string, account: string) => { - const [variable] = decodeEnvironment([onePasswordVariable("TOKEN", reference, account)]); - const source = variable?.value; - if (source === undefined || typeof source === "string") { - throw new Error("expected a 1Password source"); - } - return new OnePasswordSecretReference({ reference: source.reference, account: source.account }); -}; - describe("ProviderSecretResolverLive with 1Password accounts", () => { it.effect("reads a 1Password source from the account it names", () => { const spawner = recordingOpSpawner({ stdout: "sk-work-token", stderr: "", code: 0 }); diff --git a/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts b/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts index 7922b9e9be5c..375088a1403e 100644 --- a/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts +++ b/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts @@ -62,10 +62,6 @@ const SECRET_READ_TIMEOUT = Duration.seconds(45); */ const SECRET_CACHE_CAPACITY = 64; -/** `--account` arguments for `op`; legacy references use its default account. */ -const accountArgs = (account: OnePasswordAccount | undefined): ReadonlyArray => - account === undefined ? [] : ["--account", account]; - /** * Read many references from one account in one `op inject`. * @@ -89,7 +85,7 @@ const accountArgs = (account: OnePasswordAccount | undefined): ReadonlyArray, ) { const fileSystem = yield* FileSystem.FileSystem; @@ -99,7 +95,8 @@ const readSecretsTogether = Effect.fn("readSecretsTogether")(function* ( yield* fileSystem.writeFileString(templatePath, template); const spawnCommand = yield* resolveSpawnCommand(ONE_PASSWORD_BINARY, [ "inject", - ...accountArgs(account), + "--account", + account, "-i", templatePath, ]); @@ -140,7 +137,8 @@ const readOnePasswordSecret = Effect.fn("readOnePasswordSecret")(function* ({ }: OnePasswordSecretReference) { const spawnCommand = yield* resolveSpawnCommand(ONE_PASSWORD_BINARY, [ "read", - ...accountArgs(account), + "--account", + account, "--no-newline", reference, ]); @@ -222,7 +220,7 @@ export const ProviderSecretResolverLive = Layer.effect( }); const primeOnePassword = Effect.fn("primeOnePassword")(function* ( - account: OnePasswordAccount | undefined, + account: OnePasswordAccount, wanted: ReadonlyArray, ) { // One reference costs one prompt whichever command reads it, so there @@ -258,7 +256,7 @@ export const ProviderSecretResolverLive = Layer.effect( // `op inject` reads every reference in its template from one account, // so a batch is one call per account. const onePasswordByAccount = new Map< - OnePasswordAccount | undefined, + OnePasswordAccount, Array >(); for (const reference of references) { diff --git a/apps/server/src/provider/ProviderInstanceEnvironment.test.ts b/apps/server/src/provider/ProviderInstanceEnvironment.test.ts index 5eae7d1cc8cb..ba105b492cbf 100644 --- a/apps/server/src/provider/ProviderInstanceEnvironment.test.ts +++ b/apps/server/src/provider/ProviderInstanceEnvironment.test.ts @@ -76,10 +76,10 @@ describe("mergeProviderInstanceEnvironment", () => { const decodeProviderInstanceEnvironment = Schema.decodeSync(ProviderInstanceEnvironment); describe("literalProviderInstanceEnvironment", () => { - it("keeps literals and leaves out every value that names a secret", () => { + it("keeps every literal and leaves out secret sources", () => { const environment = decodeProviderInstanceEnvironment([ { name: "CODEX_HOME", value: "~/.codex-work" }, - { name: "LEGACY_TOKEN", value: "op://Private/item/field" }, + { name: "REFERENCE_LOOKALIKE", value: "op://Private/item/field" }, { name: "TOKEN", value: { kind: "1password", reference: "op://Private/item/field", account: "my" }, @@ -88,6 +88,7 @@ describe("literalProviderInstanceEnvironment", () => { expect(literalProviderInstanceEnvironment(environment)).toEqual([ { name: "CODEX_HOME", value: "~/.codex-work", sensitive: false }, + { name: "REFERENCE_LOOKALIKE", value: "op://Private/item/field", sensitive: false }, ]); }); }); diff --git a/apps/server/src/provider/ProviderInstanceEnvironment.ts b/apps/server/src/provider/ProviderInstanceEnvironment.ts index 02c015feeb22..ac76da1dcaaa 100644 --- a/apps/server/src/provider/ProviderInstanceEnvironment.ts +++ b/apps/server/src/provider/ProviderInstanceEnvironment.ts @@ -4,7 +4,6 @@ import type { } from "@t3tools/contracts"; import { expandHomePath } from "../pathExpansion.ts"; -import { providerSecretReference } from "./ProviderSecretReference.ts"; /** * A provider environment variable whose value is ready for a child process. @@ -24,14 +23,14 @@ export type ResolvedProviderEnvironment = ReadonlyArray = []; for (const { name, value, sensitive } of environment ?? []) { - if (typeof value === "string" && providerSecretReference(value) === undefined) { + if (typeof value === "string") { literals.push({ name, value, sensitive }); } } diff --git a/apps/server/src/provider/ProviderSecretReference.test.ts b/apps/server/src/provider/ProviderSecretReference.test.ts index a70c5a231c08..631f2239a2ba 100644 --- a/apps/server/src/provider/ProviderSecretReference.test.ts +++ b/apps/server/src/provider/ProviderSecretReference.test.ts @@ -11,14 +11,19 @@ import { const decodeEnvironment = Schema.decodeSync(ProviderInstanceEnvironment); -const legacy = (reference: string) => - new OnePasswordSecretReference({ reference, account: undefined }); +const ACCOUNT = "my.1password.com"; -const onePasswordVariable = (name: string, reference: string, account: string) => ({ +const onePasswordVariable = (name: string, reference: string, account = ACCOUNT) => ({ name, value: { kind: "1password" as const, reference, account }, }); +const secret = (reference: string) => + new OnePasswordSecretReference({ + reference, + account: ACCOUNT as OnePasswordSecretReference["account"], + }); + describe("providerSecretReference", () => { it("reads a 1Password source with its account", () => { const [variable] = decodeEnvironment([ @@ -32,25 +37,10 @@ describe("providerSecretReference", () => { ); }); - it("reads a legacy op:// string from the default account", () => { - expect(providerSecretReference("op://Private/claude-code/credential")).toEqual( - legacy("op://Private/claude-code/credential"), - ); - }); - - it("trims a reference pasted with surrounding whitespace", () => { - expect(providerSecretReference(" op://Private/claude-code/credential\n")).toEqual( - legacy("op://Private/claude-code/credential"), - ); - }); - - it("treats a literal value as a literal", () => { + it("treats every string as a literal, including one that looks like a reference", () => { expect(providerSecretReference("sk-live-token")).toBeUndefined(); expect(providerSecretReference("/home/u/.claude/work")).toBeUndefined(); - }); - - it("ignores a bare scheme with nothing behind it", () => { - expect(providerSecretReference("op://")).toBeUndefined(); + expect(providerSecretReference("op://Private/claude-code/credential")).toBeUndefined(); }); }); @@ -65,7 +55,7 @@ describe("hasProviderSecretReference", () => { hasProviderSecretReference( decodeEnvironment([ { name: "CLAUDE_SECURESTORAGE_CONFIG_DIR", value: "/home/u/.claude/work" }, - { name: "CLAUDE_CODE_OAUTH_TOKEN", value: "op://Private/claude-code/credential" }, + onePasswordVariable("CLAUDE_CODE_OAUTH_TOKEN", "op://Private/claude-code/credential"), ]), ), ).toBe(true); @@ -99,20 +89,20 @@ describe("collectProviderSecretReferences", () => { const shared = "op://Private/shared/credential"; const environments = [ decodeEnvironment([ - { name: "CLAUDE_CODE_OAUTH_TOKEN", value: shared, sensitive: true }, + onePasswordVariable("CLAUDE_CODE_OAUTH_TOKEN", shared), { name: "HOME", value: "/home/u" }, ]), undefined, decodeEnvironment([ - { name: "CODEX_TOKEN", value: "op://Private/codex/credential", sensitive: true }, + onePasswordVariable("CODEX_TOKEN", "op://Private/codex/credential"), // The same item behind two providers is one read, not two. - { name: "OTHER_TOKEN", value: shared, sensitive: true }, + onePasswordVariable("OTHER_TOKEN", shared), ]), ]; expect(Array.from(collectProviderSecretReferences(environments))).toEqual([ - legacy(shared), - legacy("op://Private/codex/credential"), + secret(shared), + secret("op://Private/codex/credential"), ]); }); @@ -123,14 +113,12 @@ describe("collectProviderSecretReferences", () => { onePasswordVariable("HOME_TOKEN", shared, "my.1password.com"), onePasswordVariable("WORK_TOKEN", shared, "acme.1password.com"), onePasswordVariable("HOME_AGAIN", shared, "my.1password.com"), - { name: "LEGACY_TOKEN", value: shared }, ]), ]); expect(references.map(({ reference, account }) => [reference, account])).toEqual([ [shared, "my.1password.com"], [shared, "acme.1password.com"], - [shared, undefined], ]); }); diff --git a/apps/server/src/provider/ProviderSecretReference.ts b/apps/server/src/provider/ProviderSecretReference.ts index 1adefece898d..e2e4e8a70ae7 100644 --- a/apps/server/src/provider/ProviderSecretReference.ts +++ b/apps/server/src/provider/ProviderSecretReference.ts @@ -21,18 +21,13 @@ import type { import * as Data from "effect/Data"; import * as Equal from "effect/Equal"; -/** URI scheme 1Password uses for secret references; `op read` consumes these. */ -const ONE_PASSWORD_SECRET_REFERENCE_PREFIX = "op://"; - /** * One secret to read from 1Password. Structurally comparable, so it doubles as * the resolver's cache key: the same reference in two accounts is two secrets. - * `account` is absent only for legacy plain-string references, which `op` - * reads from its default account. */ export class OnePasswordSecretReference extends Data.TaggedClass("1password")<{ readonly reference: string; - readonly account: OnePasswordAccount | undefined; + readonly account: OnePasswordAccount; }> {} /** Every secret a provider environment value can name. */ @@ -45,26 +40,16 @@ export type ProviderSecretReference = OnePasswordSecretReference; export function providerSecretReference( value: ProviderInstanceEnvironmentVariable["value"], ): ProviderSecretReference | undefined { - if (typeof value !== "string") { - switch (value.kind) { - case "1password": - return new OnePasswordSecretReference({ - reference: value.reference, - account: value.account, - }); - } - } - // Legacy: a plain string beginning with `op://` predates secret sources and - // is read from the default `op` account. Trimmed because a reference copied - // out of a password manager routinely arrives with surrounding whitespace. - const trimmed = value.trim(); - if ( - !trimmed.startsWith(ONE_PASSWORD_SECRET_REFERENCE_PREFIX) || - trimmed.length === ONE_PASSWORD_SECRET_REFERENCE_PREFIX.length - ) { + if (typeof value === "string") { return undefined; } - return new OnePasswordSecretReference({ reference: trimmed, account: undefined }); + switch (value.kind) { + case "1password": + return new OnePasswordSecretReference({ + reference: value.reference, + account: value.account, + }); + } } /** diff --git a/apps/server/src/provider/Services/ProviderSecretResolver.ts b/apps/server/src/provider/Services/ProviderSecretResolver.ts index ab0aec3f9d0e..58316264ce6a 100644 --- a/apps/server/src/provider/Services/ProviderSecretResolver.ts +++ b/apps/server/src/provider/Services/ProviderSecretResolver.ts @@ -1,7 +1,7 @@ /** - * ProviderSecretResolver: turns environment values that name a secret (a - * 1Password secret source, or a legacy `op://` string) into the secrets they - * name, once, and holds them in memory. + * ProviderSecretResolver: turns environment values that name a secret (such as + * a 1Password secret source) into the secrets they name, once, and holds them + * in memory. * * Every provider instance resolves its environment when the driver builds it, * and a single instance can rebuild several times per session. Shelling out @@ -78,9 +78,8 @@ export interface ProviderSecretResolverShape { /** * Defaults to handing every literal back untouched, which is what a build * without secret-store integration behaves like, and what tests want unless - * they are testing resolution itself: a legacy `op://` string stays an - * `op://` string, and the provider reports whatever the CLI makes of it. A - * secret source has no literal form, so it is reported unresolved. + * they are testing resolution itself. A secret source has no literal form, so + * it is reported unresolved. */ const resolveWithoutSecretStore = ( environment: ProviderInstanceEnvironment | undefined, diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 512e3dc09dbb..989202f285fe 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -551,7 +551,7 @@ const RuntimeCoreDependenciesBaseLive = Layer.mergeAll( // `providerInstances` hydration merges `settings.providers.` // with explicit `providerInstances` entries on boot. Layer.provideMerge(ProviderInstanceRegistryHydrationLive), - // Resolves `op://` environment values for both halves above: the instance + // Resolves secret-source environment values for both halves above: the instance // registry reads secrets while building an instance, and // `ProviderRegistryLive` drops the cached values when the user refreshes. Layer.provideMerge(ProviderSecretResolverLive), diff --git a/apps/web/src/components/settings/ProviderInstanceCard.tsx b/apps/web/src/components/settings/ProviderInstanceCard.tsx index 70d8da333f44..d42999931d0f 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.tsx +++ b/apps/web/src/components/settings/ProviderInstanceCard.tsx @@ -119,6 +119,20 @@ function makeEnvironmentDraftRow( sensitive: false, }; } + // Plain `op://` values were once read from 1Password. They are literals now, + // so they open as a 1Password source that still needs its account. + const reference = variable.value.trim(); + if (reference.startsWith("op://") && variable.valueRedacted !== true) { + return { + id, + name: variable.name, + source: "1password", + value: "", + reference, + account: "", + sensitive: false, + }; + } return { id, name: variable.name, diff --git a/docs/fork/0016-provider-secrets-live-in-1password.md b/docs/fork/0016-provider-secrets-live-in-1password.md index 00d2cf991f93..34836b6393aa 100644 --- a/docs/fork/0016-provider-secrets-live-in-1password.md +++ b/docs/fork/0016-provider-secrets-live-in-1password.md @@ -5,10 +5,10 @@ ## What you can do now -- Give a provider its credential without giving T3 Code the credential. Paste a - 1Password `op://` secret reference as an environment variable value on any - provider instance, and T3 Code reads the value from the 1Password CLI when it - starts the agent. What gets saved is the reference; the secret itself is +- Give a provider its credential without giving T3 Code the credential. Set any + provider instance's environment variable to a 1Password secret reference + plus the account it lives in, and T3 Code reads the value from the 1Password + CLI when it starts the agent. What gets saved is the reference; the secret itself is never written to the settings file or to T3 Code's secret store. - Unlock your vault once instead of all day. Each reference is read a single time and held in memory for the life of the server, so starting a thread, diff --git a/docs/internals/providers.md b/docs/internals/providers.md index 60ddea13a000..f3fa3543c83d 100644 --- a/docs/internals/providers.md +++ b/docs/internals/providers.md @@ -129,8 +129,8 @@ current client support. ## Secret references in provider environments A provider instance's `environment` can hold secret references: a `ProviderSecretSource` value such -as `{ kind: "1password", reference, account }`, or a legacy plain string starting with `op://` that -reads from the CLI's default account. +as `{ kind: "1password", reference, account }`. A plain string is always a literal, including one +that starts with `op://`. [`ProviderSecretResolver`](../../apps/server/src/provider/Services/ProviderSecretResolver.ts) swaps each one for the value the 1Password CLI returns. This happens once per instance in [`ProviderInstanceRegistryLive`](../../apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts), diff --git a/docs/user/provider-secrets.md b/docs/user/provider-secrets.md index 141ce0cf9805..2a4a2d28c51b 100644 --- a/docs/user/provider-secrets.md +++ b/docs/user/provider-secrets.md @@ -29,9 +29,9 @@ never lands in your settings file or in T3 Code's secret store. Plain values are used exactly as typed, so mixing literal variables and references on the same provider is fine. -A plain value beginning with `op://` is also read from 1Password, from the CLI's default account. -That keeps older settings working; switch those variables to the 1Password source to pin them to an -account. +A plain value is never read from 1Password, even one that starts with `op://`. Settings saved with a +plain `op://` value open as a 1Password source that still needs its account; fill it in and the +variable resolves again. To copy a reference in 1Password, open the item, use the field's overflow menu, and choose **Copy Secret Reference**. @@ -58,9 +58,6 @@ The vault has to be reachable from wherever `npx t3` or the desktop app is actua No. A reference is not a secret, so there is nothing to protect by storing it as one, and variables that read from 1Password are always stored as written. -A plain `op://` value can still be marked sensitive if you prefer the redacted field, and it is -resolved the same way either way. - ## How Often Does It Ask Me To Unlock Once, and then not again until you ask for it. diff --git a/packages/contracts/src/providerInstance.test.ts b/packages/contracts/src/providerInstance.test.ts index b60d2dc66757..441bc902d23f 100644 --- a/packages/contracts/src/providerInstance.test.ts +++ b/packages/contracts/src/providerInstance.test.ts @@ -215,7 +215,7 @@ describe("ProviderInstanceEnvironmentVariable secret sources", () => { expect(decodeEnvironmentVariable({ name: "API_KEY", value }).value).toEqual(value); }); - it("keeps decoding literal values, including legacy op:// strings", () => { + it("decodes a string value as a literal, even one that looks like a reference", () => { expect(decodeEnvironmentVariable({ name: "API_KEY" }).value).toBe(""); expect( decodeEnvironmentVariable({ name: "API_KEY", value: "op://Private/item/field" }).value, From 233034e764a2ab51aec9fe57278de6dff83e5818 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Sat, 3 Oct 2026 20:08:39 -0400 Subject: [PATCH 06/12] fix(web): unfinished 1Password rows and sources on dedicated fields no longer lose settings Signed-off-by: Yordis Prieto --- .../settings/ProviderInstanceCard.test.ts | 20 ++++++ .../settings/ProviderInstanceCard.tsx | 71 +++++++++++++------ 2 files changed, 68 insertions(+), 23 deletions(-) diff --git a/apps/web/src/components/settings/ProviderInstanceCard.test.ts b/apps/web/src/components/settings/ProviderInstanceCard.test.ts index e4cc01013286..b51dd55db43c 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.test.ts +++ b/apps/web/src/components/settings/ProviderInstanceCard.test.ts @@ -4,6 +4,8 @@ import { describe, expect, it } from "vite-plus/test"; import { ProviderDriverKind, ProviderInstanceId, + type OnePasswordAccount, + type OnePasswordSecretReference, type ServerProvider, type ServerProviderModel, } from "@t3tools/contracts"; @@ -14,6 +16,7 @@ import { providerEnvironmentWithoutNames, ProviderInstanceCard, readProviderEnvironmentVariable, + splitDedicatedProviderEnvironment, } from "./ProviderInstanceCard"; describe("deriveProviderModelsForDisplay", () => { @@ -231,4 +234,21 @@ describe("provider environment helpers", () => { { name: "EXTRA_FLAG", value: "1", sensitive: false }, ]); }); + + it("keeps a dedicated variable read from 1Password in the generic editor", () => { + const source = { + kind: "1password" as const, + reference: "op://Private/cursor/credential" as OnePasswordSecretReference, + account: "my" as OnePasswordAccount, + }; + const environment = [ + { name: "CURSOR_API_KEY", value: source, sensitive: false }, + { name: "EXTRA_FLAG", value: "1", sensitive: false }, + ]; + + expect(splitDedicatedProviderEnvironment(environment, new Set(["CURSOR_API_KEY"]))).toEqual({ + dedicated: [], + generic: environment, + }); + }); }); diff --git a/apps/web/src/components/settings/ProviderInstanceCard.tsx b/apps/web/src/components/settings/ProviderInstanceCard.tsx index d42999931d0f..a9b0685d0e8d 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.tsx +++ b/apps/web/src/components/settings/ProviderInstanceCard.tsx @@ -284,6 +284,26 @@ export function providerEnvironmentWithoutNames( return (environment ?? []).filter((variable) => !names.has(variable.name)); } +/** + * Dedicated fields only edit plain values, so a variable read from a secret + * store stays in the generic editor, which can show its source. + */ +export function splitDedicatedProviderEnvironment( + environment: ReadonlyArray | undefined, + names: ReadonlySet, +): { + readonly dedicated: ReadonlyArray; + readonly generic: ReadonlyArray; +} { + const dedicated: ProviderInstanceEnvironmentVariable[] = []; + const generic: ProviderInstanceEnvironmentVariable[] = []; + for (const variable of environment ?? []) { + if (names.has(variable.name) && typeof variable.value === "string") dedicated.push(variable); + else generic.push(variable); + } + return { dedicated, generic }; +} + export function nextProviderEnvironmentWithFieldValue( environment: ReadonlyArray | undefined, field: ProviderEnvironmentFieldDefinition, @@ -328,13 +348,14 @@ function ProviderEnvironmentFieldRow(props: { }) { const inputId = `${props.idPrefix}-environment-${props.field.name}`; const configuredValue = props.variable?.value; + const readFromSecretStore = configuredValue !== undefined && typeof configuredValue !== "string"; const value = - props.variable?.valueRedacted || typeof configuredValue !== "string" - ? "" - : (configuredValue ?? ""); - const placeholder = props.variable?.valueRedacted - ? "Stored secret - enter a new value to replace" - : props.field.placeholder; + props.variable?.valueRedacted || typeof configuredValue !== "string" ? "" : configuredValue; + const placeholder = readFromSecretStore + ? "Read from 1Password - edit it under Environment" + : props.variable?.valueRedacted + ? "Stored secret - enter a new value to replace" + : props.field.placeholder; return ( {rows.map((variable, index) => { const isOnePassword = variable.source === "1password"; - const sourceIssue = - isOnePassword && (variable.reference.length > 0 || variable.account.length > 0) - ? Result.match(onePasswordSourceFromDraft(variable), { - onFailure: (message) => message, - onSuccess: () => undefined, - }) - : undefined; + const sourceIssue = isOnePassword + ? Result.match(onePasswordSourceFromDraft(variable), { + onFailure: (message) => `Not saved yet. ${message}`, + onSuccess: () => undefined, + }) + : undefined; return (
@@ -853,17 +879,16 @@ export function ProviderInstanceCard({ // the generic editor only shows the remaining variables. const environmentFields = driverOption?.environmentFields ?? []; const environmentFieldNames = new Set(environmentFields.map((field) => field.name)); - const genericEnvironment = providerEnvironmentWithoutNames( - instance.environment, - environmentFieldNames, - ); + const { dedicated: dedicatedEnvironment, generic: genericEnvironment } = + splitDedicatedProviderEnvironment(instance.environment, environmentFieldNames); const updateGenericEnvironment = ( environment: ReadonlyArray, ) => { - const dedicatedEnvironment = (instance.environment ?? []).filter((variable) => - environmentFieldNames.has(variable.name), - ); - updateEnvironment([...dedicatedEnvironment, ...environment]); + const genericNames = new Set(environment.map((variable) => variable.name)); + updateEnvironment([ + ...providerEnvironmentWithoutNames(dedicatedEnvironment, genericNames), + ...environment, + ]); }; const updateEnvironmentField = (field: ProviderEnvironmentFieldDefinition, value: string) => { updateEnvironment(nextProviderEnvironmentWithFieldValue(instance.environment, field, value)); From f1adeb8c0e96ccd1779766cffd8a9a42697be5e7 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Sat, 3 Oct 2026 21:19:52 -0400 Subject: [PATCH 07/12] feat(web): pick the 1Password account from the accounts signed in on the server Signed-off-by: Yordis Prieto --- apps/server/src/auth/RpcAuthorization.ts | 1 + .../ProviderAdapterRegistry.test.ts | 1 + .../provider/Layers/ProviderRegistry.test.ts | 4 + .../Layers/ProviderSecretResolverLive.test.ts | 47 +++ .../Layers/ProviderSecretResolverLive.ts | 48 ++- .../Services/ProviderSecretResolver.ts | 9 +- apps/server/src/ws.ts | 10 + .../settings/ProviderInstanceCard.tsx | 336 +++++++++++------- docs/user/provider-secrets.md | 15 +- packages/client-runtime/src/state/server.ts | 4 + packages/contracts/src/onePassword.ts | 7 + packages/contracts/src/rpc.ts | 9 + 12 files changed, 358 insertions(+), 133 deletions(-) diff --git a/apps/server/src/auth/RpcAuthorization.ts b/apps/server/src/auth/RpcAuthorization.ts index 046bf8ab2186..b482a2e9c549 100644 --- a/apps/server/src/auth/RpcAuthorization.ts +++ b/apps/server/src/auth/RpcAuthorization.ts @@ -72,6 +72,7 @@ export const RPC_REQUIRED_SCOPES = { [WS_METHODS.serverDisableAcpRegistryProvider]: AuthOrchestrationOperateScope, [WS_METHODS.serverLogoutAcpRegistry]: AuthOrchestrationOperateScope, [WS_METHODS.serverDiscoverSourceControl]: AuthOrchestrationReadScope, + [WS_METHODS.serverListOnePasswordAccounts]: AuthOrchestrationReadScope, [WS_METHODS.serverGetTraceDiagnostics]: AuthOrchestrationReadScope, [WS_METHODS.serverGetProcessDiagnostics]: AuthOrchestrationReadScope, [WS_METHODS.serverGetHostResources]: AuthOrchestrationReadScope, diff --git a/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts b/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts index 49b4b6ebcc6c..299564b19023 100644 --- a/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts +++ b/apps/server/src/orchestration-v2/ProviderAdapterRegistry.test.ts @@ -368,6 +368,7 @@ it.effect("opens v2 sessions with resolved secrets and rebuilds them when a secr }), prime: () => Effect.void, invalidate: Effect.void, + listOnePasswordAccounts: Effect.succeed([]), }; const secretDriver: ProviderDriver> = { driverKind: driver, diff --git a/apps/server/src/provider/Layers/ProviderRegistry.test.ts b/apps/server/src/provider/Layers/ProviderRegistry.test.ts index b56c43821616..4a392c33f2c0 100644 --- a/apps/server/src/provider/Layers/ProviderRegistry.test.ts +++ b/apps/server/src/provider/Layers/ProviderRegistry.test.ts @@ -1625,6 +1625,7 @@ it.layer(Layer.mergeAll(TestNodeServices, ServerSettingsModule.layerTest(), Test }), prime: () => Effect.void, invalidate: Ref.update(invalidations, (count) => count + 1), + listOnePasswordAccounts: Effect.succeed([]), }); const instanceRegistryLayer = Layer.succeed( ProviderInstanceRegistry.ProviderInstanceRegistry, @@ -1707,6 +1708,7 @@ it.layer(Layer.mergeAll(TestNodeServices, ServerSettingsModule.layerTest(), Test }), prime: () => Effect.void, invalidate: Effect.void, + listOnePasswordAccounts: Effect.succeed([]), }); const instanceRegistryLayer = Layer.succeed( ProviderInstanceRegistry.ProviderInstanceRegistry, @@ -1792,6 +1794,7 @@ it.layer(Layer.mergeAll(TestNodeServices, ServerSettingsModule.layerTest(), Test references.map((secret) => secret.reference), ]).pipe(Effect.asVoid), invalidate: Effect.void, + listOnePasswordAccounts: Effect.succeed([]), }); const environmentFor = (reference: string) => decodeEnvironment([onePasswordVariable("TOKEN", reference)]); @@ -2994,6 +2997,7 @@ it.layer(Layer.mergeAll(TestNodeServices, ServerSettingsModule.layerTest(), Test `prime:${references.map((secret) => secret.reference).join(",")}`, ]).pipe(Effect.asVoid), invalidate: Effect.void, + listOnePasswordAccounts: Effect.succeed([]), }); const scope = yield* Scope.make(); diff --git a/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts b/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts index a3afdd361e75..a2c9cffc3013 100644 --- a/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts +++ b/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts @@ -573,3 +573,50 @@ describe("ProviderSecretResolverLive with 1Password accounts", () => { ); }); }); + +describe("ProviderSecretResolverLive.listOnePasswordAccounts", () => { + const provideResolver = (spawner: ReturnType) => + Effect.provide( + ProviderSecretResolverLive.pipe( + Layer.provide(Layer.merge(spawner.layer, NodeFileSystem.layer)), + ), + ); + + it.effect("offers each signed-in account by the address `op --account` accepts", () => { + const spawner = recordingOpSpawner({ + stdout: JSON.stringify([ + { url: HOME_ACCOUNT, email: "me@example.com", user_uuid: "U1", account_uuid: "A1" }, + { url: WORK_ACCOUNT, email: "me@acme.example", user_uuid: "U2", account_uuid: "A2" }, + ]), + stderr: "", + code: 0, + }); + return Effect.gen(function* () { + const resolver = yield* ProviderSecretResolver; + + const accounts = yield* resolver.listOnePasswordAccounts; + + assert.deepStrictEqual(accounts, [ + { account: HOME_ACCOUNT, email: "me@example.com" }, + { account: WORK_ACCOUNT, email: "me@acme.example" }, + ]); + assert.deepStrictEqual(spawner.invocations, [["account", "list", "--format", "json"]]); + }).pipe(provideResolver(spawner)); + }); + + it.effect("offers no accounts when `op` fails", () => { + const spawner = recordingOpSpawner({ stdout: "", stderr: "not configured", code: 1 }); + return Effect.gen(function* () { + const resolver = yield* ProviderSecretResolver; + assert.deepStrictEqual(yield* resolver.listOnePasswordAccounts, []); + }).pipe(provideResolver(spawner)); + }); + + it.effect("offers no accounts when `op` prints something unexpected", () => { + const spawner = recordingOpSpawner({ stdout: "not json", stderr: "", code: 0 }); + return Effect.gen(function* () { + const resolver = yield* ProviderSecretResolver; + assert.deepStrictEqual(yield* resolver.listOnePasswordAccounts, []); + }).pipe(provideResolver(spawner)); + }); +}); diff --git a/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts b/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts index 375088a1403e..406b09d6a4a2 100644 --- a/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts +++ b/apps/server/src/provider/Layers/ProviderSecretResolverLive.ts @@ -23,13 +23,14 @@ * * @module provider/Layers/ProviderSecretResolverLive */ -import type { OnePasswordAccount } from "@t3tools/contracts"; +import { OnePasswordAccount, OnePasswordAccountSummary } from "@t3tools/contracts"; import * as Cache from "effect/Cache"; import * as Duration from "effect/Duration"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as FileSystem from "effect/FileSystem"; import * as Option from "effect/Option"; +import * as Schema from "effect/Schema"; import * as NodeCrypto from "node:crypto"; import { ChildProcess, ChildProcessSpawner } from "effect/unstable/process"; import { resolveSpawnCommand } from "@t3tools/shared/shell"; @@ -161,6 +162,32 @@ const readOnePasswordSecret = Effect.fn("readOnePasswordSecret")(function* ({ return secret.length > 0 ? secret : undefined; }); +const OnePasswordAccountListJson = Schema.fromJsonString( + Schema.Array(Schema.Struct({ url: OnePasswordAccount, email: Schema.String })), +); + +const readOnePasswordAccounts = Effect.fn("readOnePasswordAccounts")(function* () { + const spawnCommand = yield* resolveSpawnCommand(ONE_PASSWORD_BINARY, [ + "account", + "list", + "--format", + "json", + ]); + const result = yield* spawnAndCollect( + ONE_PASSWORD_BINARY, + ChildProcess.make(spawnCommand.command, spawnCommand.args, { shell: spawnCommand.shell }), + ); + if (result.code !== 0) { + yield* Effect.logWarning("Could not list 1Password accounts", { + exitCode: result.code, + detail: result.stderr.trim(), + }); + return []; + } + const accounts = yield* Schema.decodeUnknownEffect(OnePasswordAccountListJson)(result.stdout); + return accounts.map(({ url, email }): OnePasswordAccountSummary => ({ account: url, email })); +}); + const readSecret = (secret: ProviderSecretReference) => { switch (secret._tag) { case "1password": @@ -281,6 +308,23 @@ export const ProviderSecretResolverLive = Layer.effect( ); }).pipe(Effect.provideContext(primeContext)); - return { resolve, prime, invalidate: Cache.invalidateAll(cache) }; + const listOnePasswordAccounts: ProviderSecretResolverShape["listOnePasswordAccounts"] = + readOnePasswordAccounts().pipe( + Effect.timeoutOption(SECRET_READ_TIMEOUT), + Effect.map(Option.getOrElse(() => [])), + Effect.catch((error) => + Effect.logWarning("Could not run 1Password to list accounts", { + detail: String(error), + }).pipe(Effect.as([])), + ), + Effect.provideContext(primeContext), + ); + + return { + resolve, + prime, + invalidate: Cache.invalidateAll(cache), + listOnePasswordAccounts, + }; }), ); diff --git a/apps/server/src/provider/Services/ProviderSecretResolver.ts b/apps/server/src/provider/Services/ProviderSecretResolver.ts index 58316264ce6a..679f9ff43269 100644 --- a/apps/server/src/provider/Services/ProviderSecretResolver.ts +++ b/apps/server/src/provider/Services/ProviderSecretResolver.ts @@ -13,7 +13,7 @@ * * @module provider/Services/ProviderSecretResolver */ -import type { ProviderInstanceEnvironment } from "@t3tools/contracts"; +import type { OnePasswordAccountSummary, ProviderInstanceEnvironment } from "@t3tools/contracts"; import * as Context from "effect/Context"; import * as Effect from "effect/Effect"; @@ -73,6 +73,12 @@ export interface ProviderSecretResolverShape { * spawned with. */ readonly invalidate: Effect.Effect; + /** + * The 1Password accounts signed in on this machine, so settings can offer + * them instead of asking the user to type one. Reads local `op` config only, + * so it never prompts. Empty when `op` is missing or fails. + */ + readonly listOnePasswordAccounts: Effect.Effect>; } /** @@ -103,6 +109,7 @@ export class ProviderSecretResolver extends Context.Reference Effect.sync(() => resolveWithoutSecretStore(environment)), prime: () => Effect.void, invalidate: Effect.void, + listOnePasswordAccounts: Effect.succeed([]), }), }, ) {} diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index 2a0006ddb24b..b8c78c5e28d9 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -161,6 +161,7 @@ import { } from "./observability/RpcInstrumentation.ts"; import * as ProviderRegistry from "./provider/Services/ProviderRegistry.ts"; import * as ProviderInstanceRegistry from "./provider/Services/ProviderInstanceRegistry.ts"; +import { ProviderSecretResolver } from "./provider/Services/ProviderSecretResolver.ts"; import * as AcpRegistrySupport from "./provider/acp/AcpRegistrySupport.ts"; import * as AcpRegistryRuntimeCoordinator from "./provider/acp/AcpRegistryRuntimeCoordinator.ts"; import * as ModelManifest from "./provider/ModelManifest.ts"; @@ -1199,6 +1200,7 @@ const makeWsRpcLayer = ( ); const serverAuth = yield* EnvironmentAuth.EnvironmentAuth; const sourceControlDiscovery = yield* SourceControlDiscovery.SourceControlDiscovery; + const providerSecretResolver = yield* ProviderSecretResolver; const automaticGitFetchInterval = serverSettings.getSettings.pipe( Effect.map( (settings) => resolveServerBackgroundActivitySettings(settings).automaticGitFetchInterval, @@ -2526,6 +2528,14 @@ const makeWsRpcLayer = ( "rpc.aggregate": "server", }, ), + [WS_METHODS.serverListOnePasswordAccounts]: (_input) => + observeRpcEffect( + WS_METHODS.serverListOnePasswordAccounts, + providerSecretResolver.listOnePasswordAccounts, + { + "rpc.aggregate": "server", + }, + ), [WS_METHODS.serverGetTraceDiagnostics]: (_input) => observeRpcEffect( WS_METHODS.serverGetTraceDiagnostics, diff --git a/apps/web/src/components/settings/ProviderInstanceCard.tsx b/apps/web/src/components/settings/ProviderInstanceCard.tsx index a9b0685d0e8d..ff9f4ff37685 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.tsx +++ b/apps/web/src/components/settings/ProviderInstanceCard.tsx @@ -13,6 +13,7 @@ import { KeyRoundIcon, PlusIcon, Trash2Icon, + TypeIcon, XIcon, } from "lucide-react"; import * as Arr from "effect/Array"; @@ -24,6 +25,7 @@ import { OnePasswordAccount, OnePasswordSecretReference, resolveProviderInstanceEnabled, + type OnePasswordAccountSummary, type OnePasswordSecretSource, type ProviderInstanceConfig, type ProviderInstanceEnvironmentVariable, @@ -42,12 +44,15 @@ import { toCustomModelSetting, } from "@t3tools/shared/model"; import { cn } from "../../lib/utils"; +import { useEnvironmentQuery } from "../../state/query"; +import { serverEnvironment } from "../../state/server"; import { useCopyToClipboard } from "../../hooks/useCopyToClipboard"; import { normalizeProviderAccentColor } from "../../providerInstances"; import { Badge } from "../ui/badge"; import { Button } from "../ui/button"; import { DraftInput } from "../ui/draft-input"; import { Popover, PopoverPopup, PopoverTrigger } from "../ui/popover"; +import { Select, SelectItem, SelectPopup, SelectTrigger, SelectValue } from "../ui/select"; import { Switch } from "../ui/switch"; import { stackedThreadToast, toastManager } from "../ui/toast"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; @@ -92,6 +97,35 @@ const nextEnvironmentVariableDraftId = () => `provider-env-${environmentVariable type EnvironmentDraftSource = "plain" | "1password"; +const PLAIN_SOURCE_OPTION = "plain"; +const EMPTY_ONE_PASSWORD_ACCOUNTS: ReadonlyArray = []; +const ONE_PASSWORD_SOURCE_OPTION_PREFIX = "1password:"; + +/** The source select's option for a row: a plain value, or the 1Password account it reads from. */ +function environmentDraftSourceOption(row: EnvironmentDraftRow): string { + return row.source === "1password" + ? `${ONE_PASSWORD_SOURCE_OPTION_PREFIX}${row.account}` + : PLAIN_SOURCE_OPTION; +} + +function EnvironmentDraftSourceLabel(props: { readonly option: string }) { + if (!props.option.startsWith(ONE_PASSWORD_SOURCE_OPTION_PREFIX)) { + return ( + + + Value + + ); + } + const account = props.option.slice(ONE_PASSWORD_SOURCE_OPTION_PREFIX.length); + return ( + + + {account.length > 0 ? account : "1Password"} + + ); +} + type EnvironmentDraftRow = { readonly id: string; readonly name: string; @@ -393,6 +427,7 @@ function ProviderEnvironmentFieldRow(props: { function ProviderEnvironmentSection(props: { readonly environment: ReadonlyArray; + readonly onePasswordAccounts: ReadonlyArray; readonly onChange: (environment: ReadonlyArray) => void; }) { const [rows, setRows] = useState>(() => @@ -478,6 +513,28 @@ function ProviderEnvironmentSection(props: { publishRows(nextRows); }; + // Accounts already saved stay pickable even when this server's `op` no + // longer reports them, so opening settings never rewrites a row. + const onePasswordAccounts = Arr.dedupe([ + ...props.onePasswordAccounts.map(({ account }) => account as string), + ...Arr.filterMap(rows, (row) => + row.source === "1password" && row.account.length > 0 + ? Result.succeed(row.account) + : Result.failVoid, + ), + ]); + const sourceOptions = [ + PLAIN_SOURCE_OPTION, + ...(onePasswordAccounts.length > 0 ? onePasswordAccounts : [""]).map( + (account) => `${ONE_PASSWORD_SOURCE_OPTION_PREFIX}${account}`, + ), + ]; + const emailByAccount = new Map( + props.onePasswordAccounts.map(({ account, email }) => [account as string, email]), + ); + // Without any known account there is nothing to pick, so the account is typed instead. + const typesAccount = onePasswordAccounts.length === 0; + const addVariable = () => setRows([ ...rows, @@ -504,48 +561,110 @@ function ProviderEnvironmentSection(props: { } > {rows.length > 0 ? ( -
+
{rows.map((variable, index) => { const isOnePassword = variable.source === "1password"; - const sourceIssue = isOnePassword - ? Result.match(onePasswordSourceFromDraft(variable), { - onFailure: (message) => `Not saved yet. ${message}`, - onSuccess: () => undefined, - }) - : undefined; + const sourceIssue = + isOnePassword && + (variable.name.length > 0 || + variable.reference.length > 0 || + variable.account.length > 0) + ? Result.match(onePasswordSourceFromDraft(variable), { + onFailure: (message) => `Not saved yet. ${message}`, + onSuccess: () => undefined, + }) + : undefined; return ( -
-
- + updateVariable(variable.id, { name: name.trim() })} + placeholder="VARIABLE_NAME" + spellCheck={false} + aria-label={`Environment variable name ${index + 1}`} + /> + + + {isOnePassword ? ( + <> + updateVariable(variable.id, { reference })} + placeholder="op://vault/item/field" + spellCheck={false} + aria-invalid={sourceIssue !== undefined || undefined} + aria-label={`Environment variable 1Password reference ${index + 1}`} + /> + {typesAccount ? ( updateVariable(variable.id, { account })} placeholder="my.1password.com" @@ -553,98 +672,63 @@ function ProviderEnvironmentSection(props: { aria-invalid={sourceIssue !== undefined || undefined} aria-label={`Environment variable 1Password account ${index + 1}`} /> - - ) : ( - <> - updateVariable(variable.id, { value })} - type={variable.sensitive ? "password" : undefined} - autoComplete="off" - placeholder={ - variable.valueRedacted - ? "Stored secret, enter a new value to replace" - : "value" - } - spellCheck={false} - aria-label={`Environment variable value ${index + 1}`} - /> - - { - const sensitive = !variable.sensitive; - updateVariable(variable.id, { - sensitive, - ...(sensitive && variable.valueRedacted === undefined - ? {} - : { - valueRedacted: sensitive ? variable.valueRedacted : false, - }), - }); - }} - aria-pressed={variable.sensitive} - aria-label={`Mark environment variable ${variable.name || index + 1} as sensitive`} - > - {variable.sensitive ? ( - - ) : ( - - )} - - } - /> - - {variable.sensitive ? "Sensitive, stored separately" : "Plain text"} - - - - )} - - - updateVariable( - variable.id, - isOnePassword - ? { source: "plain", value: "", sensitive: true } - : { source: "1password", value: "", sensitive: false }, - ) - } - aria-pressed={isOnePassword} - aria-label={`Read environment variable ${variable.name || index + 1} from 1Password`} - > - - + ) : null} + + ) : ( + <> + updateVariable(variable.id, { value })} + type={variable.sensitive ? "password" : undefined} + autoComplete="off" + placeholder={ + variable.valueRedacted + ? "Stored secret, enter a new value to replace" + : "value" } + spellCheck={false} + aria-label={`Environment variable value ${index + 1}`} /> - - {isOnePassword ? "Read from 1Password" : "Plain value"} - - - -
+ + { + const sensitive = !variable.sensitive; + updateVariable(variable.id, { + sensitive, + ...(sensitive && variable.valueRedacted === undefined + ? {} + : { + valueRedacted: sensitive ? variable.valueRedacted : false, + }), + }); + }} + aria-pressed={variable.sensitive} + aria-label={`Mark environment variable ${variable.name || index + 1} as sensitive`} + > + {variable.sensitive ? ( + + ) : ( + + )} + + } + /> + + {variable.sensitive ? "Sensitive, stored separately" : "Plain text"} + + + + )} {sourceIssue !== undefined ? ( -

{sourceIssue}

+

{sourceIssue}

) : null}
); @@ -879,6 +963,11 @@ export function ProviderInstanceCard({ // the generic editor only shows the remaining variables. const environmentFields = driverOption?.environmentFields ?? []; const environmentFieldNames = new Set(environmentFields.map((field) => field.name)); + const onePasswordAccounts = useEnvironmentQuery( + environmentId === undefined + ? null + : serverEnvironment.onePasswordAccounts({ environmentId, input: {} }), + ); const { dedicated: dedicatedEnvironment, generic: genericEnvironment } = splitDedicatedProviderEnvironment(instance.environment, environmentFieldNames); const updateGenericEnvironment = ( @@ -1331,6 +1420,7 @@ export function ProviderInstanceCard({ > {environmentId !== undefined && liveProvider?.driver === "acpRegistry" ? ( diff --git a/docs/user/provider-secrets.md b/docs/user/provider-secrets.md index 2a4a2d28c51b..584ef2c34389 100644 --- a/docs/user/provider-secrets.md +++ b/docs/user/provider-secrets.md @@ -9,18 +9,19 @@ For provider setup itself, see [Codex](./providers-codex.md) and [Claude](./prov Point the variable at 1Password instead of pasting the value. -In the provider's Environment variables section in Settings, add the variable, switch it to read from -1Password with the key button, and fill in the secret reference and the account it lives in: +In the provider's Environment variables section in Settings, add the variable, pick the 1Password +account it lives in as its source, and fill in the secret reference: ```text Name: CLAUDE_CODE_OAUTH_TOKEN +Source: my.1password.com Reference: op://Private/claude-code/credential -Account: my.1password.com ``` -The account is anything `op --account` accepts: the sign-in address, the account shorthand, or the -account ID. Run `op account list` to see yours. Naming it means a reference keeps resolving against -the right account when you are signed in to more than one. +The source list offers every account the 1Password CLI is signed in to on the machine running the +T3 Code server. Naming the account means a reference keeps resolving against the right one when you +are signed in to more than one. If the server cannot list any accounts, type the account instead: +anything `op --account` accepts works. T3 Code reads the value with the 1Password CLI right before it starts the agent, and hands the resolved value to the agent process only. The reference is what T3 Code stores; the secret itself @@ -30,7 +31,7 @@ Plain values are used exactly as typed, so mixing literal variables and referenc provider is fine. A plain value is never read from 1Password, even one that starts with `op://`. Settings saved with a -plain `op://` value open as a 1Password source that still needs its account; fill it in and the +plain `op://` value open as a 1Password source that still needs its account; pick it and the variable resolves again. To copy a reference in 1Password, open the item, use the field's overflow menu, and choose diff --git a/packages/client-runtime/src/state/server.ts b/packages/client-runtime/src/state/server.ts index a5077aa74f39..ee6ae1fe8afd 100644 --- a/packages/client-runtime/src/state/server.ts +++ b/packages/client-runtime/src/state/server.ts @@ -1055,6 +1055,10 @@ export function createServerEnvironmentAtoms( label: "environment-data:server:trace-diagnostics", tag: WS_METHODS.serverGetTraceDiagnostics, }), + onePasswordAccounts: createEnvironmentRpcQueryAtomFamily(runtime, { + label: "environment-data:server:one-password-accounts", + tag: WS_METHODS.serverListOnePasswordAccounts, + }), processDiagnostics: createEnvironmentRpcQueryAtomFamily(runtime, { label: "environment-data:server:process-diagnostics", tag: WS_METHODS.serverGetProcessDiagnostics, diff --git a/packages/contracts/src/onePassword.ts b/packages/contracts/src/onePassword.ts index 630aaff73ba8..c3e957b93fdc 100644 --- a/packages/contracts/src/onePassword.ts +++ b/packages/contracts/src/onePassword.ts @@ -67,3 +67,10 @@ export const OnePasswordSecretSource = Schema.Struct({ account: OnePasswordAccount, }); export type OnePasswordSecretSource = typeof OnePasswordSecretSource.Type; + +/** A 1Password account signed in on the server, as `op account list` reports it. */ +export const OnePasswordAccountSummary = Schema.Struct({ + account: OnePasswordAccount, + email: Schema.String, +}); +export type OnePasswordAccountSummary = typeof OnePasswordAccountSummary.Type; diff --git a/packages/contracts/src/rpc.ts b/packages/contracts/src/rpc.ts index ed6df13a6947..253e3130f1c3 100644 --- a/packages/contracts/src/rpc.ts +++ b/packages/contracts/src/rpc.ts @@ -10,6 +10,7 @@ import * as Schema from "effect/Schema"; import * as Rpc from "effect/unstable/rpc/Rpc"; import * as RpcGroup from "effect/unstable/rpc/RpcGroup"; import { NonNegativeInt, TrimmedNonEmptyString } from "./baseSchemas.ts"; +import { OnePasswordAccountSummary } from "./onePassword.ts"; import { CodexAuthCallbackInput, CodexAuthCallbackState, @@ -442,6 +443,7 @@ export const WS_METHODS = { serverGetSettings: "server.getSettings", serverUpdateSettings: "server.updateSettings", serverDiscoverSourceControl: "server.discoverSourceControl", + serverListOnePasswordAccounts: "server.listOnePasswordAccounts", serverSearchAcpRegistry: "server.searchAcpRegistry", serverPrepareAcpRegistryAgent: "server.prepareAcpRegistryAgent", serverUninstallAcpRegistryManagedBinary: "server.uninstallAcpRegistryManagedBinary", @@ -718,6 +720,12 @@ const WsServerDiscoverSourceControlRpc = Rpc.make(WS_METHODS.serverDiscoverSourc error: EnvironmentAuthorizationError, }); +const WsServerListOnePasswordAccountsRpc = Rpc.make(WS_METHODS.serverListOnePasswordAccounts, { + payload: Schema.Struct({}), + success: Schema.Array(OnePasswordAccountSummary), + error: EnvironmentAuthorizationError, +}); + const WsServerSearchAcpRegistryRpc = Rpc.make(WS_METHODS.serverSearchAcpRegistry, { payload: AcpRegistrySearchInput, success: AcpRegistrySearchResult, @@ -1710,6 +1718,7 @@ export const WsRpcGroup = RpcGroup.make( WsServerGetSettingsRpc, WsServerUpdateSettingsRpc, WsServerDiscoverSourceControlRpc, + WsServerListOnePasswordAccountsRpc, WsServerSearchAcpRegistryRpc, WsServerPrepareAcpRegistryAgentRpc, WsServerUninstallAcpRegistryManagedBinaryRpc, From 57c7ace0e40652c1af7633e192d1d8c6a8a1f456 Mon Sep 17 00:00:00 2001 From: Yordis Prieto Date: Sat, 3 Oct 2026 21:28:22 -0400 Subject: [PATCH 08/12] fix(web): dedicated fields can no longer clobber a 1Password source Signed-off-by: Yordis Prieto --- .../src/provider/Layers/ProviderSecretResolverLive.test.ts | 6 +++--- apps/web/src/components/settings/ProviderInstanceCard.tsx | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts b/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts index a2c9cffc3013..efe28b82baa9 100644 --- a/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts +++ b/apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts @@ -1,6 +1,6 @@ // @effect-diagnostics nodeBuiltinImport:off - asserts against the fake op CLI script this test writes to disk. import { describe, it, assert } from "@effect/vitest"; -import { ProviderInstanceEnvironment } from "@t3tools/contracts"; +import { OnePasswordAccount, ProviderInstanceEnvironment } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as Layer from "effect/Layer"; import * as Schema from "effect/Schema"; @@ -597,8 +597,8 @@ describe("ProviderSecretResolverLive.listOnePasswordAccounts", () => { const accounts = yield* resolver.listOnePasswordAccounts; assert.deepStrictEqual(accounts, [ - { account: HOME_ACCOUNT, email: "me@example.com" }, - { account: WORK_ACCOUNT, email: "me@acme.example" }, + { account: OnePasswordAccount.make(HOME_ACCOUNT), email: "me@example.com" }, + { account: OnePasswordAccount.make(WORK_ACCOUNT), email: "me@acme.example" }, ]); assert.deepStrictEqual(spawner.invocations, [["account", "list", "--format", "json"]]); }).pipe(provideResolver(spawner)); diff --git a/apps/web/src/components/settings/ProviderInstanceCard.tsx b/apps/web/src/components/settings/ProviderInstanceCard.tsx index ff9f4ff37685..8d3abfb65eaa 100644 --- a/apps/web/src/components/settings/ProviderInstanceCard.tsx +++ b/apps/web/src/components/settings/ProviderInstanceCard.tsx @@ -406,9 +406,10 @@ function ProviderEnvironmentFieldRow(props: { value={value} onCommit={(next) => props.onCommit(props.field, next)} placeholder={placeholder} + disabled={readFromSecretStore} spellCheck={false} /> - {props.variable ? ( + {props.variable && !readFromSecretStore ? (