diff --git a/apps/cloud/src/secrets-isolation.e2e.node.test.ts b/apps/cloud/src/secrets-isolation.e2e.node.test.ts index 9b24951ca..80fe8dc1e 100644 --- a/apps/cloud/src/secrets-isolation.e2e.node.test.ts +++ b/apps/cloud/src/secrets-isolation.e2e.node.test.ts @@ -9,12 +9,11 @@ // // Invariants the product is staking on: // -// 1. Users in different orgs can't see each other's secrets — not even -// secrets written at the org scope of the other org. -// 2. Users in the same org can't see each other's user-scoped secrets -// (per-user OAuth tokens etc. don't leak to co-workers). -// 3. Org-scoped secrets ARE visible to every user in that org — an -// admin writing a shared API key serves the whole tenant. +// 1. Users in different orgs can't see each other's secret metadata. +// 2. Users in the same org can't see each other's user-scoped secret +// metadata (per-user OAuth tokens etc. don't leak to co-workers). +// 3. Org-scoped secret metadata IS visible to every user in that org +// — an admin writing a shared API key serves the whole tenant. // 4. The same user id in different orgs gets distinct per-user scopes — // the userOrgScope id bakes in the org id on purpose. // 5. secrets.set rejects a scope id outside the caller's executor stack. @@ -75,14 +74,6 @@ describe("cloud secret isolation (HTTP, user-org scope stack)", () => { ); expect(charlieList.map((s) => s.id)).not.toContain(id); - const charlieResolve = yield* asUser(charlie, orgB, (client) => - client.secrets - .resolve({ - path: { scopeId: ScopeId.make(orgB), secretId: SecretId.make(id) }, - }) - .pipe(Effect.either), - ); - expect(charlieResolve._tag).toBe("Left"); }), ); @@ -108,7 +99,7 @@ describe("cloud secret isolation (HTTP, user-org scope stack)", () => { ); // Bob is in the same org — his user-org scope differs. He should - // see neither the token in a list nor be able to resolve it. + // not see the token in a list. const bobList = yield* asUser(bobId, orgId, (client) => client.secrets.list({ path: { scopeId: ScopeId.make(testUserOrgScopeId(bobId, orgId)) }, @@ -116,28 +107,26 @@ describe("cloud secret isolation (HTTP, user-org scope stack)", () => { ); expect(bobList.map((s) => s.id)).not.toContain(id); - const bobResolve = yield* asUser(bobId, orgId, (client) => - client.secrets - .resolve({ - path: { - scopeId: ScopeId.make(testUserOrgScopeId(bobId, orgId)), - secretId: SecretId.make(id), - }, - }) - .pipe(Effect.either), + const bobStatus = yield* asUser(bobId, orgId, (client) => + client.secrets.status({ + path: { + scopeId: ScopeId.make(testUserOrgScopeId(bobId, orgId)), + secretId: SecretId.make(id), + }, + }), ); - expect(bobResolve._tag).toBe("Left"); + expect(bobStatus.status).toBe("missing"); - // And Alice still sees her own token. - const aliceResolve = yield* asUser(aliceId, orgId, (client) => - client.secrets.resolve({ + // And Alice still sees her own token metadata. + const aliceStatus = yield* asUser(aliceId, orgId, (client) => + client.secrets.status({ path: { scopeId: ScopeId.make(testUserOrgScopeId(aliceId, orgId)), secretId: SecretId.make(id), }, }), ); - expect(aliceResolve.value).toBe("alice-token-value"); + expect(aliceStatus.status).toBe("resolved"); }), ); @@ -161,18 +150,18 @@ describe("cloud secret isolation (HTTP, user-org scope stack)", () => { }), ); - const adminValue = yield* asUser(adminId, orgId, (client) => - client.secrets.resolve({ + const adminStatus = yield* asUser(adminId, orgId, (client) => + client.secrets.status({ path: { scopeId: ScopeId.make(orgId), secretId: SecretId.make(id) }, }), ); - const memberValue = yield* asUser(memberId, orgId, (client) => - client.secrets.resolve({ + const memberStatus = yield* asUser(memberId, orgId, (client) => + client.secrets.status({ path: { scopeId: ScopeId.make(orgId), secretId: SecretId.make(id) }, }), ); - expect(adminValue.value).toBe("shared-org-key"); - expect(memberValue.value).toBe("shared-org-key"); + expect(adminStatus.status).toBe("resolved"); + expect(memberStatus.status).toBe("resolved"); }), ); @@ -206,29 +195,27 @@ describe("cloud secret isolation (HTTP, user-org scope stack)", () => { ); expect(listInB.map((s) => s.id)).not.toContain(id); - const resolveInB = yield* asUser(userId, orgB, (client) => - client.secrets - .resolve({ - path: { - scopeId: ScopeId.make(testUserOrgScopeId(userId, orgB)), - secretId: SecretId.make(id), - }, - }) - .pipe(Effect.either), + const statusInB = yield* asUser(userId, orgB, (client) => + client.secrets.status({ + path: { + scopeId: ScopeId.make(testUserOrgScopeId(userId, orgB)), + secretId: SecretId.make(id), + }, + }), ); - expect(resolveInB._tag).toBe("Left"); + expect(statusInB.status).toBe("missing"); - // Sanity: the original write is still readable under the org-A + // Sanity: the original write is still visible under the org-A // user-org scope. - const resolveInA = yield* asUser(userId, orgA, (client) => - client.secrets.resolve({ + const statusInA = yield* asUser(userId, orgA, (client) => + client.secrets.status({ path: { scopeId: ScopeId.make(testUserOrgScopeId(userId, orgA)), secretId: SecretId.make(id), }, }), ); - expect(resolveInA.value).toBe("value-in-a"); + expect(statusInA.status).toBe("resolved"); }), ); @@ -256,16 +243,14 @@ describe("cloud secret isolation (HTTP, user-org scope stack)", () => { // at that org must not see `wrong-scope`. const foreignUser = nextUserId(); const leaked = yield* asUser(foreignUser, foreignOrg, (client) => - client.secrets - .resolve({ - path: { - scopeId: ScopeId.make(foreignOrg), - secretId: SecretId.make("wrong-scope"), - }, - }) - .pipe(Effect.either), + client.secrets.status({ + path: { + scopeId: ScopeId.make(foreignOrg), + secretId: SecretId.make("wrong-scope"), + }, + }), ); - expect(leaked._tag).toBe("Left"); + expect(leaked.status).toBe("missing"); }), ); }); diff --git a/apps/cloud/src/services/secrets-api.node.test.ts b/apps/cloud/src/services/secrets-api.node.test.ts index f829b7ade..9cd55a509 100644 --- a/apps/cloud/src/services/secrets-api.node.test.ts +++ b/apps/cloud/src/services/secrets-api.node.test.ts @@ -1,4 +1,4 @@ -// Secrets endpoints — set / list / status / resolve / remove round-trip +// Secrets endpoints — set / list / status / remove round-trip // and error fidelity within a single org. import { describe, expect, it } from "@effect/vitest"; @@ -6,10 +6,10 @@ import { Effect } from "effect"; import { ScopeId, SecretId } from "@executor/sdk"; -import { asOrg } from "./__test-harness__/api-harness"; +import { asOrg, fetchForOrg, TEST_BASE_URL } from "./__test-harness__/api-harness"; describe("secrets api (HTTP)", () => { - it.effect("set → list → resolve round-trips a new secret", () => + it.effect("set → list → status round-trips a new secret without exposing plaintext", () => Effect.gen(function* () { const org = `org_${crypto.randomUUID()}`; const id = `sec_${crypto.randomUUID().slice(0, 8)}`; @@ -28,12 +28,31 @@ describe("secrets api (HTTP)", () => { ); expect(list.find((s) => s.id === id)?.name).toBe("My API Token"); - const resolved = yield* asOrg(org, (client) => - client.secrets.resolve({ + const status = yield* asOrg(org, (client) => + client.secrets.status({ path: { scopeId: ScopeId.make(org), secretId: SecretId.make(id) }, }), ); - expect(resolved.value).toBe("sk-test-abc"); + expect(status.status).toBe("resolved"); + }), + ); + + it.effect("does not expose a plaintext resolve endpoint", () => + Effect.gen(function* () { + const org = `org_${crypto.randomUUID()}`; + const id = `sec_${crypto.randomUUID().slice(0, 8)}`; + + yield* asOrg(org, (client) => + client.secrets.set({ + path: { scopeId: ScopeId.make(org) }, + payload: { id: SecretId.make(id), name: "n", value: "v" }, + }), + ); + + const response = yield* Effect.promise(() => + fetchForOrg(org)(`${TEST_BASE_URL}/scopes/${org}/secrets/${id}/resolve`), + ); + expect(response.status).toBe(404); }), ); @@ -68,7 +87,7 @@ describe("secrets api (HTTP)", () => { }), ); - it.effect("remove deletes the secret; subsequent resolve fails and list drops it", () => + it.effect("remove deletes the secret; subsequent status is missing and list drops it", () => Effect.gen(function* () { const org = `org_${crypto.randomUUID()}`; const id = `sec_${crypto.randomUUID().slice(0, 8)}`; @@ -90,34 +109,12 @@ describe("secrets api (HTTP)", () => { ); expect(list.map((s) => s.id)).not.toContain(id); - const afterResolve = yield* asOrg(org, (client) => - client.secrets - .resolve({ path: { scopeId: ScopeId.make(org), secretId: SecretId.make(id) } }) - .pipe(Effect.either), - ); - expect(afterResolve._tag).toBe("Left"); - }), - ); - - it.effect("resolve on an unknown id fails with a typed error", () => - Effect.gen(function* () { - const org = `org_${crypto.randomUUID()}`; - const missing = `missing_${crypto.randomUUID().slice(0, 8)}`; - - const result = yield* asOrg(org, (client) => - client.secrets - .resolve({ path: { scopeId: ScopeId.make(org), secretId: SecretId.make(missing) } }) - .pipe(Effect.either), + const afterStatus = yield* asOrg(org, (client) => + client.secrets.status({ + path: { scopeId: ScopeId.make(org), secretId: SecretId.make(id) }, + }), ); - expect(result._tag).toBe("Left"); - if (result._tag === "Left") { - // The API declares SecretNotFoundError / SecretResolutionError - // as typed errors; either is acceptable for an unknown id. - const err = result.left as { _tag?: string }; - expect( - err._tag === "SecretNotFoundError" || err._tag === "SecretResolutionError", - ).toBe(true); - } + expect(afterStatus.status).toBe("missing"); }), ); @@ -135,7 +132,7 @@ describe("secrets api (HTTP)", () => { }), ); - it.effect("set with the same id twice updates the value (upsert)", () => + it.effect("set with the same id twice updates the visible metadata", () => Effect.gen(function* () { const org = `org_${crypto.randomUUID()}`; const id = `sec_${crypto.randomUUID().slice(0, 8)}`; @@ -146,12 +143,10 @@ describe("secrets api (HTTP)", () => { path: { scopeId: ScopeId.make(org) }, payload: { id: SecretId.make(id), name: "first", value: "first-value" }, }); - return yield* client.secrets.resolve({ - path: { scopeId: ScopeId.make(org), secretId: SecretId.make(id) }, - }); + return yield* client.secrets.list({ path: { scopeId: ScopeId.make(org) } }); }), ); - expect(first.value).toBe("first-value"); + expect(first.find((s) => s.id === id)?.name).toBe("first"); const second = yield* asOrg(org, (client) => Effect.gen(function* () { @@ -159,12 +154,10 @@ describe("secrets api (HTTP)", () => { path: { scopeId: ScopeId.make(org) }, payload: { id: SecretId.make(id), name: "updated", value: "second-value" }, }); - return yield* client.secrets.resolve({ - path: { scopeId: ScopeId.make(org), secretId: SecretId.make(id) }, - }); + return yield* client.secrets.list({ path: { scopeId: ScopeId.make(org) } }); }), ); - expect(second.value).toBe("second-value"); + expect(second.find((s) => s.id === id)?.name).toBe("updated"); }), ); }); diff --git a/apps/cloud/src/services/tenant-isolation.node.test.ts b/apps/cloud/src/services/tenant-isolation.node.test.ts index 49d433808..41a7f1ee6 100644 --- a/apps/cloud/src/services/tenant-isolation.node.test.ts +++ b/apps/cloud/src/services/tenant-isolation.node.test.ts @@ -133,7 +133,7 @@ describe("tenant isolation (HTTP)", () => { }), ); - it.effect("secrets.resolve cannot return another org's plaintext", () => + it.effect("secret metadata is not visible across orgs", () => Effect.gen(function* () { const orgA = `org_${crypto.randomUUID()}`; const orgB = `org_${crypto.randomUUID()}`; @@ -146,13 +146,17 @@ describe("tenant isolation (HTTP)", () => { }), ); - const result = yield* asOrg(orgB, (client) => - client.secrets - .resolve({ path: { scopeId: ScopeId.make(orgB), secretId: SecretId.make(secretIdA) } }) - .pipe(Effect.either), + const status = yield* asOrg(orgB, (client) => + client.secrets.status({ + path: { scopeId: ScopeId.make(orgB), secretId: SecretId.make(secretIdA) }, + }), + ); + const list = yield* asOrg(orgB, (client) => + client.secrets.list({ path: { scopeId: ScopeId.make(orgB) } }), ); - expect(result._tag).toBe("Left"); + expect(status.status).toBe("missing"); + expect(list.map((s) => s.id)).not.toContain(secretIdA); }), ); diff --git a/packages/core/api/src/connections/api.ts b/packages/core/api/src/connections/api.ts index ea898e668..5f809da03 100644 --- a/packages/core/api/src/connections/api.ts +++ b/packages/core/api/src/connections/api.ts @@ -4,7 +4,6 @@ import { Schema } from "effect"; import { ConnectionId, ScopeId, - SecretId, } from "@executor/sdk"; import { InternalError } from "../observability"; @@ -25,8 +24,6 @@ const ConnectionRefResponse = Schema.Struct({ scopeId: ScopeId, provider: Schema.String, identityLabel: Schema.NullOr(Schema.String), - accessTokenSecretId: SecretId, - refreshTokenSecretId: Schema.NullOr(SecretId), expiresAt: Schema.NullOr(Schema.Number), oauthScope: Schema.NullOr(Schema.String), createdAt: Schema.Number, diff --git a/packages/core/api/src/handlers/connections.ts b/packages/core/api/src/handlers/connections.ts index 69b5f5f22..b4c1826ab 100644 --- a/packages/core/api/src/handlers/connections.ts +++ b/packages/core/api/src/handlers/connections.ts @@ -12,8 +12,6 @@ const refToResponse = (ref: ConnectionRef) => ({ scopeId: ref.scopeId, provider: ref.provider, identityLabel: ref.identityLabel, - accessTokenSecretId: ref.accessTokenSecretId, - refreshTokenSecretId: ref.refreshTokenSecretId, expiresAt: ref.expiresAt, oauthScope: ref.oauthScope, createdAt: ref.createdAt.getTime(), diff --git a/packages/core/api/src/handlers/secrets.ts b/packages/core/api/src/handlers/secrets.ts index d311c49f1..e79c30283 100644 --- a/packages/core/api/src/handlers/secrets.ts +++ b/packages/core/api/src/handlers/secrets.ts @@ -1,6 +1,6 @@ import { HttpApiBuilder } from "@effect/platform"; import { Effect } from "effect"; -import { SecretNotFoundError, SetSecretInput, type SecretRef } from "@executor/sdk"; +import { SetSecretInput, type SecretRef } from "@executor/sdk"; import { ExecutorApi } from "../api"; import { ExecutorService } from "../services"; @@ -45,16 +45,6 @@ export const SecretsHandlers = HttpApiBuilder.group(ExecutorApi, "secrets", (han return refToResponse(ref); })), ) - .handle("resolve", ({ path }) => - capture(Effect.gen(function* () { - const executor = yield* ExecutorService; - const value = yield* executor.secrets.get(path.secretId); - if (value === null) { - return yield* Effect.fail(new SecretNotFoundError({ secretId: path.secretId })); - } - return { secretId: path.secretId, value }; - })), - ) .handle("remove", ({ path }) => capture(Effect.gen(function* () { const executor = yield* ExecutorService; diff --git a/packages/core/api/src/secrets/api.ts b/packages/core/api/src/secrets/api.ts index a9629be9a..940f53d5a 100644 --- a/packages/core/api/src/secrets/api.ts +++ b/packages/core/api/src/secrets/api.ts @@ -34,11 +34,6 @@ const SecretStatusResponse = Schema.Struct({ status: Schema.Literal("resolved", "missing"), }); -const SecretResolveResponse = Schema.Struct({ - secretId: SecretId, - value: Schema.String, -}); - const SetSecretPayload = Schema.Struct({ id: SecretId, name: Schema.String, @@ -79,12 +74,6 @@ export class SecretsApi extends HttpApiGroup.make("secrets") .addSuccess(SecretRefResponse) .addError(SecretResolution), ) - .add( - HttpApiEndpoint.get("resolve")`/scopes/${scopeIdParam}/secrets/${secretIdParam}/resolve` - .addSuccess(SecretResolveResponse) - .addError(SecretNotFound) - .addError(SecretResolution), - ) .add( HttpApiEndpoint.del("remove")`/scopes/${scopeIdParam}/secrets/${secretIdParam}` .addSuccess(Schema.Struct({ removed: Schema.Boolean })) diff --git a/packages/core/sdk/src/executor.test.ts b/packages/core/sdk/src/executor.test.ts index 52c9bbd12..d4159bc87 100644 --- a/packages/core/sdk/src/executor.test.ts +++ b/packages/core/sdk/src/executor.test.ts @@ -5,6 +5,7 @@ import { makeMemoryAdapter } from "@executor/storage-core/testing/memory"; import type { DBAdapter, Where } from "@executor/storage-core"; import { makeInMemoryBlobStore } from "./blob"; +import { CreateConnectionInput, TokenMaterial } from "./connections"; import { collectSchemas, createExecutor } from "./executor"; import { ElicitationResponse, @@ -15,7 +16,7 @@ import { defineSchema, definePlugin } from "./plugin"; import { SetSecretInput } from "./secrets"; import { makeTestConfig } from "./testing"; import type { SecretProvider } from "./secrets"; -import { ScopeId, SecretId } from "./ids"; +import { ConnectionId, ScopeId, SecretId } from "./ids"; import { Scope } from "./scope"; type FindManyCall = { @@ -192,6 +193,12 @@ const memorySecretsPlugin = definePlugin(() => ({ secretProviders: [memoryProvider], })); +const memoryConnectionPlugin = definePlugin(() => ({ + id: "memory-connection" as const, + storage: () => ({}), + connectionProviders: [{ key: "memory-connection" }], +})); + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- @@ -556,6 +563,58 @@ describe("createExecutor", () => { }), ); + it.effect("secrets.get rejects connection-owned token secrets", () => + Effect.gen(function* () { + const executor = yield* createExecutor( + makeTestConfig({ + plugins: [memorySecretsPlugin(), memoryConnectionPlugin()] as const, + }), + ); + + yield* executor.connections.create( + new CreateConnectionInput({ + id: ConnectionId.make("conn-owned"), + scope: ScopeId.make("test-scope"), + provider: "memory-connection", + identityLabel: "Alice", + accessToken: new TokenMaterial({ + secretId: SecretId.make("conn-owned.access_token"), + name: "Access", + value: "access-secret", + }), + refreshToken: new TokenMaterial({ + secretId: SecretId.make("conn-owned.refresh_token"), + name: "Refresh", + value: "refresh-secret", + }), + expiresAt: null, + oauthScope: "read", + providerState: null, + }), + ); + + const leaked = yield* executor.secrets + .get("conn-owned.access_token") + .pipe(Effect.either); + expect(leaked._tag).toBe("Left"); + if (leaked._tag === "Left") { + expect((leaked.left as { _tag?: string })._tag).toBe( + "SecretOwnedByConnectionError", + ); + } + + const status = yield* executor.secrets.status("conn-owned.access_token"); + expect(status).toBe("missing"); + const visibleIds = (yield* executor.secrets.list()).map( + (s) => s.id as unknown as string, + ); + expect(visibleIds).not.toContain("conn-owned.access_token"); + + const token = yield* executor.connections.accessToken("conn-owned"); + expect(token).toBe("access-secret"); + }), + ); + it.effect("invoke fails with ToolNotFoundError for unknown tool", () => Effect.gen(function* () { const executor = yield* createExecutor(makeTestConfig()); diff --git a/packages/core/sdk/src/executor.ts b/packages/core/sdk/src/executor.ts index 80212c67e..14f262c7a 100644 --- a/packages/core/sdk/src/executor.ts +++ b/packages/core/sdk/src/executor.ts @@ -171,7 +171,7 @@ export type Executor = { readonly secrets: { readonly get: ( id: string, - ) => Effect.Effect; + ) => Effect.Effect; /** Fast-path existence check — hits the core `secret` routing table * only, never calls the provider. Use this for UI state ("secret * missing, prompt to add") to avoid keychain permission prompts @@ -682,16 +682,19 @@ export const createExecutor = < return winner ?? null; }; - const secretsGet = ( + const secretRowsForId = ( + id: string, + ): Effect.Effect[], StorageFailure> => + core.findMany({ + model: "secret", + where: [{ field: "id", value: id }], + }); + + const resolveSecretValueFromRows = ( id: string, + rows: readonly Record[], ): Effect.Effect => Effect.gen(function* () { - // The scope-wrapped adapter injects `scope_id IN (scopeIds)` - // automatically, so we only filter by id here. - const rows = yield* core.findMany({ - model: "secret", - where: [{ field: "id", value: id }], - }); const ordered = [...rows].sort( (a, b) => (scopePrecedence.get(a.scope_id as string) ?? Infinity) - @@ -727,6 +730,37 @@ export const createExecutor = < return null; }); + const secretsGet = ( + id: string, + ): Effect.Effect => + Effect.gen(function* () { + // The scope-wrapped adapter injects `scope_id IN (scopeIds)` + // automatically, so we only filter by id here. Connection-owned + // token rows are internal plumbing; public secret resolution + // must not expose them even if a token secret id is leaked. + const rows = yield* secretRowsForId(id); + const owned = rows.find((row) => row.owned_by_connection_id); + if (owned) { + return yield* Effect.fail( + new SecretOwnedByConnectionError({ + secretId: SecretId.make(id), + connectionId: ConnectionId.make( + owned.owned_by_connection_id as string, + ), + }), + ); + } + return yield* resolveSecretValueFromRows(id, rows); + }); + + const connectionSecretGet = ( + id: string, + ): Effect.Effect => + Effect.gen(function* () { + const rows = yield* secretRowsForId(id); + return yield* resolveSecretValueFromRows(id, rows); + }); + const secretsSet = ( input: SetSecretInput, ): Effect.Effect => @@ -1448,7 +1482,7 @@ export const createExecutor = < } const refreshTokenValue = ref.refreshTokenSecretId - ? yield* secretsGet(ref.refreshTokenSecretId as unknown as string) + ? yield* connectionSecretGet(ref.refreshTokenSecretId as unknown as string) : null; // RFC 6749 §5.2 `invalid_grant` (and anything else the @@ -1524,7 +1558,7 @@ export const createExecutor = < ref.expiresAt - CONNECTION_REFRESH_SKEW_MS <= now; if (!needsRefresh) { - const current = yield* secretsGet( + const current = yield* connectionSecretGet( ref.accessTokenSecretId as unknown as string, ); if (current !== null) return current; @@ -2328,11 +2362,9 @@ export const createExecutor = < id: string, ): Effect.Effect<"resolved" | "missing", StorageFailure> => Effect.gen(function* () { - const row = yield* core.findOne({ - model: "secret", - where: [{ field: "id", value: id }], - }); - if (row) return "resolved"; + const rows = yield* secretRowsForId(id); + if (rows.some((row) => row.owned_by_connection_id)) return "missing"; + if (rows.length > 0) return "resolved"; for (const provider of secretProviders.values()) { if (!provider.list) continue; diff --git a/packages/core/sdk/src/plugin.ts b/packages/core/sdk/src/plugin.ts index 0d65a5735..81ea30a33 100644 --- a/packages/core/sdk/src/plugin.ts +++ b/packages/core/sdk/src/plugin.ts @@ -132,7 +132,7 @@ export interface PluginCtx { readonly secrets: { readonly get: ( id: string, - ) => Effect.Effect; + ) => Effect.Effect; /** List user-visible secrets. Connection-owned secrets (rows with * `owned_by_connection_id` set) are filtered out so they don't * clutter the UI — users see the Connection instead. */ diff --git a/packages/plugins/google-discovery/src/sdk/plugin.ts b/packages/plugins/google-discovery/src/sdk/plugin.ts index 3ece7b4be..f4b0c1f14 100644 --- a/packages/plugins/google-discovery/src/sdk/plugin.ts +++ b/packages/plugins/google-discovery/src/sdk/plugin.ts @@ -180,6 +180,27 @@ export interface GoogleDiscoveryPluginExtension { ) => Effect.Effect; } +const oauthSecretError = (message: string) => + new GoogleDiscoveryOAuthError({ message }); + +const resolveOAuthSecret = ( + ctx: PluginCtx, + id: string, + label: string, +): Effect.Effect => + ctx.secrets.get(id).pipe( + Effect.mapError((err) => + "_tag" in err && err._tag === "SecretOwnedByConnectionError" + ? oauthSecretError(`${label} secret not found: ${id}`) + : err, + ), + Effect.flatMap((value) => + value === null + ? Effect.fail(oauthSecretError(`${label} secret not found: ${id}`)) + : Effect.succeed(value), + ), + ); + // --------------------------------------------------------------------------- // URL normalization + slug helpers (unchanged) // --------------------------------------------------------------------------- @@ -422,12 +443,11 @@ export const googleDiscoveryPlugin = definePlugin(() => ({ message: "This Google Discovery document does not declare any OAuth scopes", }); } - const clientIdValue = yield* ctx.secrets.get(input.clientIdSecretId); - if (clientIdValue === null) { - return yield* new GoogleDiscoveryOAuthError({ - message: `OAuth client ID secret not found: ${input.clientIdSecretId}`, - }); - } + const clientIdValue = yield* resolveOAuthSecret( + ctx, + input.clientIdSecretId, + "OAuth client ID", + ); const sessionId = randomUUID(); const codeVerifier = createPkceCodeVerifier(); const tokenScope = input.tokenScope ?? (ctx.scopes[0]!.id as string); @@ -476,26 +496,19 @@ export const googleDiscoveryPlugin = definePlugin(() => ({ }); } - const clientIdValue = yield* ctx.secrets.get(session.clientIdSecretId); - if (clientIdValue === null) { - return yield* new GoogleDiscoveryOAuthError({ - message: `OAuth client ID secret not found: ${session.clientIdSecretId}`, - }); - } + const clientIdValue = yield* resolveOAuthSecret( + ctx, + session.clientIdSecretId, + "OAuth client ID", + ); const clientSecretValue = session.clientSecretSecretId === null ? null - : yield* ctx.secrets.get(session.clientSecretSecretId).pipe( - Effect.flatMap((v) => - v === null - ? Effect.fail( - new GoogleDiscoveryOAuthError({ - message: `OAuth client secret not found: ${session.clientSecretSecretId}`, - }), - ) - : Effect.succeed(v), - ), + : yield* resolveOAuthSecret( + ctx, + session.clientSecretSecretId, + "OAuth client secret", ); const tokenResponse = yield* exchangeAuthorizationCode({ diff --git a/packages/plugins/mcp/src/sdk/plugin.ts b/packages/plugins/mcp/src/sdk/plugin.ts index 841a563aa..c1b42c6c6 100644 --- a/packages/plugins/mcp/src/sdk/plugin.ts +++ b/packages/plugins/mcp/src/sdk/plugin.ts @@ -366,7 +366,13 @@ const resolveConnectorInput = ( const auth = sd.auth; if (auth.kind === "header") { - const val = yield* ctx.secrets.get(auth.secretId); + const val = yield* ctx.secrets.get(auth.secretId).pipe( + Effect.mapError((err) => + "_tag" in err && err._tag === "SecretOwnedByConnectionError" + ? remoteConnectionError(`Failed to resolve secret "${auth.secretId}"`) + : err, + ), + ); if (val === null) { return yield* Effect.fail( remoteConnectionError(`Failed to resolve secret "${auth.secretId}"`), diff --git a/packages/plugins/onepassword/src/sdk/plugin.ts b/packages/plugins/onepassword/src/sdk/plugin.ts index f2f0c090e..e52bb2a0f 100644 --- a/packages/plugins/onepassword/src/sdk/plugin.ts +++ b/packages/plugins/onepassword/src/sdk/plugin.ts @@ -162,6 +162,14 @@ const resolveAuth = ( }); } return ctx.secrets.get(auth.tokenSecretId).pipe( + Effect.mapError((err) => + "_tag" in err && err._tag === "SecretOwnedByConnectionError" + ? new OnePasswordError({ + operation: "auth resolution", + message: `Service account token secret "${auth.tokenSecretId}" not found`, + }) + : err, + ), Effect.flatMap((token) => { if (token === null) { return Effect.fail( diff --git a/packages/plugins/openapi/src/sdk/invoke.ts b/packages/plugins/openapi/src/sdk/invoke.ts index 68371a17b..f794e5332 100644 --- a/packages/plugins/openapi/src/sdk/invoke.ts +++ b/packages/plugins/openapi/src/sdk/invoke.ts @@ -1,7 +1,7 @@ import { Effect, Layer, Option } from "effect"; import { HttpClient, HttpClientRequest } from "@effect/platform"; -import type { StorageFailure } from "@executor/sdk"; +import type { SecretOwnedByConnectionError, StorageFailure } from "@executor/sdk"; import { OpenApiInvocationError } from "./errors"; import { @@ -97,7 +97,9 @@ const resolvePath = Effect.fn("OpenApi.resolvePath")(function* ( export const resolveHeaders = ( headers: Record, secrets: { - readonly get: (id: string) => Effect.Effect; + readonly get: ( + id: string, + ) => Effect.Effect; }, ): Effect.Effect, OpenApiInvocationError | StorageFailure> => { const entries = Object.entries(headers); @@ -115,6 +117,14 @@ export const resolveHeaders = ( typeof value === "string" ? Effect.succeed({ name, value }) : secrets.get(value.secretId).pipe( + Effect.mapError((err) => + "_tag" in err && err._tag === "SecretOwnedByConnectionError" + ? new OpenApiInvocationError({ + message: `Failed to resolve secret "${value.secretId}" for header "${name}"`, + statusCode: Option.none(), + }) + : err, + ), Effect.flatMap((secret) => secret === null ? Effect.fail( diff --git a/packages/plugins/openapi/src/sdk/plugin.ts b/packages/plugins/openapi/src/sdk/plugin.ts index 3f6571e6a..da5dbdf44 100644 --- a/packages/plugins/openapi/src/sdk/plugin.ts +++ b/packages/plugins/openapi/src/sdk/plugin.ts @@ -524,10 +524,19 @@ const resolveConfiguredHeaders = ( value.slot, ); if (binding?.value.kind === "secret") { - const secret = yield* ctx.secrets.get(binding.value.secretId as string); + const secretId = binding.value.secretId as string; + const secret = yield* ctx.secrets.get(secretId).pipe( + Effect.mapError((err) => + "_tag" in err && err._tag === "SecretOwnedByConnectionError" + ? new OpenApiOAuthError({ + message: `Missing secret "${secretId}" for header "${name}"`, + }) + : err, + ), + ); if (secret === null) { return yield* new OpenApiOAuthError({ - message: `Missing secret "${binding.value.secretId}" for header "${name}"`, + message: `Missing secret "${secretId}" for header "${name}"`, }); } resolved[name] = value.prefix ? `${value.prefix}${secret}` : secret; diff --git a/packages/react/src/api/atoms.tsx b/packages/react/src/api/atoms.tsx index f85584b07..08c0796b8 100644 --- a/packages/react/src/api/atoms.tsx +++ b/packages/react/src/api/atoms.tsx @@ -79,8 +79,6 @@ export const connectionsAtom = (scopeId: ScopeId) => export const setSecret = ExecutorApiClient.mutation("secrets", "set"); -export const resolveSecret = ExecutorApiClient.mutation("secrets", "resolve"); - export const removeSecret = ExecutorApiClient.mutation("secrets", "remove"); export const removeConnection = ExecutorApiClient.mutation( diff --git a/packages/react/src/plugins/secret-header-auth.tsx b/packages/react/src/plugins/secret-header-auth.tsx index 17debbf94..d2e74f018 100644 --- a/packages/react/src/plugins/secret-header-auth.tsx +++ b/packages/react/src/plugins/secret-header-auth.tsx @@ -1,14 +1,13 @@ import { useId, useState, type CSSProperties } from "react"; import { useAtomSet } from "@effect-atom/atom-react"; -import { setSecret, resolveSecret } from "../api/atoms"; +import { setSecret } from "../api/atoms"; import { secretWriteKeys } from "../api/reactivity-keys"; import { useScope } from "../api/scope-context"; import { SecretId, type ScopeId } from "@executor/sdk"; import { Button } from "../components/button"; import { Field, FieldError, FieldGroup, FieldLabel } from "../components/field"; import { Input } from "../components/input"; -import { Spinner } from "../components/spinner"; import { SecretPicker, type SecretPickerSecret } from "./secret-picker"; export interface HeaderAuthPreset { @@ -182,66 +181,16 @@ export function InlineCreateSecret(props: { ); } -type ResolveState = - | { status: "hidden" } - | { status: "loading" } - | { status: "revealed"; value: string } - | { status: "error" }; - function HeaderValuePreview(props: { headerName: string; secretId: string; prefix?: string }) { - const { headerName, secretId, prefix } = props; - const scopeId = useScope(); - const [state, setState] = useState({ status: "hidden" }); - const doResolve = useAtomSet(resolveSecret, { mode: "promise" }); - - const handleToggle = async () => { - if (state.status === "revealed") { - setState({ status: "hidden" }); - return; - } - setState({ status: "loading" }); - try { - const result = await doResolve({ - path: { - scopeId, - secretId: SecretId.make(secretId), - }, - }); - setState({ status: "revealed", value: result.value }); - } catch { - setState({ status: "error" }); - } - }; - - const displayValue = - state.status === "revealed" - ? state.value - : state.status === "error" - ? "failed to resolve" - : "•".repeat(12); - const isLoading = state.status === "loading"; - const isRevealed = state.status === "revealed"; + const { headerName, prefix } = props; return (
{headerName}: {prefix && {prefix}} - {displayValue} + {"•".repeat(12)} -
); }