From 037faa3e96d9f6e35c38ff30c1e91b1a55897276 Mon Sep 17 00:00:00 2001 From: myohei Date: Sat, 11 Jul 2026 01:09:11 +0900 Subject: [PATCH] fix(encrypted-secrets): store tokens at org partition for cross-subject visibility When using Service Token auth (e.g. MCP clients via Cloudflare Access), tokens saved during browser OAuth (by a human subject) were invisible to the Service Token subject due to the owner-read policy on plugin_storage. Always store encrypted secrets at owner='org' so any subject within the tenant can retrieve them. Access control remains enforced by the connection row's (tenant, owner, subject) partition. --- .../encrypted-secrets/src/index.test.ts | 206 +++++++++++++----- .../plugins/encrypted-secrets/src/index.ts | 11 +- 2 files changed, 153 insertions(+), 64 deletions(-) diff --git a/packages/plugins/encrypted-secrets/src/index.test.ts b/packages/plugins/encrypted-secrets/src/index.test.ts index 42ef88139..6caf0fa1f 100644 --- a/packages/plugins/encrypted-secrets/src/index.test.ts +++ b/packages/plugins/encrypted-secrets/src/index.test.ts @@ -6,70 +6,120 @@ import { Owner, ProviderItemId, type CredentialProvider, type PluginCtx } from " import { decryptSecret, deriveKey, encryptSecret, encryptedSecretsPlugin } from "./index"; // --------------------------------------------------------------------------- -// In-memory PluginStorageFacade fake (owner-partitioned), enough to exercise -// the provider exactly as the executor's plugin-storage table would. +// In-memory PluginStorageFacade fake that mirrors the executor's owner policy. +// +// The real plugin_storage table is owner-scoped: reads return rows matching +// tenant = current AND (owner = 'org' OR (owner = 'user' AND subject = current)) +// This fake accepts a `visibility` binding at construction time and replicates +// that predicate so tests can exercise cross-subject visibility. // // v2: the provider keys values by the opaque `ProviderItemId` (the storage -// `key`); writes carry an `owner` the host supplies. Reads via `get`/`list` -// are not owner-filtered — the connection row that references the id owns the -// partition. +// `key`); writes carry an `owner` the host supplies. The connection row that +// references the id owns the (tenant, owner, subject) partition. // --------------------------------------------------------------------------- -const makeFakeStorage = () => { - const rows = new Map(); - const composite = (collection: string, key: string) => `${collection} ${key}`; - const toEntry = (row: { owner: Owner; key: string; collection: string; data: unknown }) => ({ - id: composite(row.collection, row.key), - owner: row.owner, - pluginId: "encryptedSecrets", - collection: row.collection, - key: row.key, - data: row.data, - createdAt: new Date(0), - updatedAt: new Date(0), - }); - const facade = { - get: (input: { collection: string; key: string }) => - Effect.sync(() => { - const row = rows.get(composite(input.collection, input.key)); - return row ? (toEntry(row) as never) : null; - }), - getForOwner: (input: { collection: string; key: string; owner: Owner }) => - Effect.sync(() => { - const row = rows.get(composite(input.collection, input.key)); - return row && row.owner === input.owner ? (toEntry(row) as never) : null; - }), - list: (input: { collection: string }) => - Effect.sync( - () => - [...rows.values()] - .filter((row) => row.collection === input.collection) - .map((row) => toEntry(row)) as never, - ), - put: (input: { collection: string; key: string; owner: Owner; data: unknown }) => - Effect.sync(() => { - const row = { - owner: input.owner, - key: input.key, - collection: input.collection, - data: input.data, - }; - rows.set(composite(input.collection, input.key), row); - return toEntry(row) as never; - }), - remove: (input: { collection: string; key: string; owner: Owner }) => - Effect.sync(() => { - rows.delete(composite(input.collection, input.key)); - }), - }; - return { facade, rows }; +/** Row shape in the fake storage, including partition columns. */ +interface FakeRow { + readonly tenant: string; + readonly owner: Owner; + readonly subject: string; + readonly key: string; + readonly collection: string; + readonly data: unknown; +} + +/** The acting (tenant, subject) binding that gates reads, as in the real DB. */ +interface FakeVisibility { + readonly tenant: string; + readonly subject: string | null; +} + +/** Whether a stored row is visible under the given binding (mirrors onRead policy). */ +const isVisible = (row: FakeRow, vis: FakeVisibility): boolean => { + if (row.tenant !== vis.tenant) return false; + if (row.owner === "org") return true; + if (row.owner === "user" && vis.subject != null && row.subject === vis.subject) return true; + return false; }; -const makeProvider = (key: string, owner: Owner = Owner.make("org")) => { - const { facade, rows } = makeFakeStorage(); +/** + * A single shared row store that multiple providers (with different + * visibilities) can read/write — mirroring the real D1 table where all + * subjects in one tenant share the same physical table. + */ +class SharedStore { + readonly rows = new Map(); + private readonly composite = (collection: string, key: string) => `${collection} ${key}`; + + /** Build a PluginStorageFacade view bound to a specific (tenant, subject). */ + view(vis: FakeVisibility) { + const rows = this.rows; + const composite = this.composite; + const toEntry = (row: FakeRow) => ({ + id: composite(row.collection, row.key), + owner: row.owner, + pluginId: "encryptedSecrets", + collection: row.collection, + key: row.key, + data: row.data, + createdAt: new Date(0), + updatedAt: new Date(0), + }); + return { + get: (input: { collection: string; key: string }) => + Effect.sync(() => { + const row = rows.get(composite(input.collection, input.key)); + return row && isVisible(row, vis) ? (toEntry(row) as never) : null; + }), + getForOwner: (input: { collection: string; key: string; owner: Owner }) => + Effect.sync(() => { + const row = rows.get(composite(input.collection, input.key)); + return row && row.owner === input.owner && isVisible(row, vis) + ? (toEntry(row) as never) + : null; + }), + list: (input: { collection: string }) => + Effect.sync( + () => + [...rows.values()] + .filter((row) => row.collection === input.collection && isVisible(row, vis)) + .map((row) => toEntry(row)) as never, + ), + put: (input: { collection: string; key: string; owner: Owner; data: unknown }) => + Effect.sync(() => { + const subject = input.owner === "org" ? "" : (vis.subject ?? ""); + const row: FakeRow = { + tenant: vis.tenant, + owner: input.owner, + subject, + key: input.key, + collection: input.collection, + data: input.data, + }; + rows.set(composite(input.collection, input.key), row); + return toEntry(row) as never; + }), + remove: (input: { collection: string; key: string; owner: Owner }) => + Effect.sync(() => { + const row = rows.get(composite(input.collection, input.key)); + if (row && isVisible(row, vis)) { + rows.delete(composite(input.collection, input.key)); + } + }), + }; + } +} + +/** Build a provider bound to a specific (tenant, subject), sharing one store. */ +const makeProviderFromStore = ( + store: SharedStore, + key: string, + vis: FakeVisibility, +): { provider: CredentialProvider; store: SharedStore } => { + const facade = store.view(vis); // oxlint-disable-next-line executor/no-double-cast -- test boundary: minimal PluginCtx fake for the provider under test const ctx = { - owner: { tenant: "tenant-a", subject: owner === Owner.make("user") ? "subject-a" : null }, + owner: { tenant: vis.tenant, subject: vis.subject }, pluginStorage: facade, } as unknown as PluginCtx; const plugin = encryptedSecretsPlugin({ key }); @@ -77,7 +127,17 @@ const makeProvider = (key: string, owner: Owner = Owner.make("org")) => { ctx: PluginCtx, ) => readonly CredentialProvider[]; const provider = credentialProviders(ctx)[0]!; - return { provider, rows }; + return { provider, store }; +}; + +/** Standalone provider+store for simple (non-cross-subject) tests. */ +const makeProvider = (key: string, owner: Owner = Owner.make("org")) => { + const vis: FakeVisibility = { + tenant: "tenant-a", + subject: owner === Owner.make("user") ? "subject-a" : null, + }; + const store = new SharedStore(); + return makeProviderFromStore(store, key, vis); }; const id = (value: string) => ProviderItemId.make(value); @@ -124,9 +184,9 @@ describe("provider", () => { }); test("stores ciphertext at rest, not plaintext", async () => { - const { provider, rows } = makeProvider("master"); + const { provider, store } = makeProvider("master"); await Effect.runPromise(provider.set!(id("github"), "ghp_plaintext")); - const stored = String([...rows.values()][0]!.data); + const stored = String([...store.rows.values()][0]!.data); expect(stored).not.toContain("ghp_plaintext"); expect(stored.startsWith("v1.")).toBe(true); }); @@ -162,3 +222,31 @@ describe("provider", () => { expect(entries.map((e) => e.id).sort()).toEqual(["alpha", "beta"]); }); }); + +describe("cross-subject visibility (Service Token scenario)", () => { + // In production, a human user authenticates in the browser and the OAuth + // callback stores the access token via the encrypted provider. Later, an + // MCP client authenticates with a Service Token (different subject within + // the same tenant) and calls a tool that needs to retrieve that token. + // + // The token must be visible to both identities because the connection row + // (which gates access) is org-owned. The provider must therefore store + // tokens at the org partition, not at the individual subject's partition. + + test("a token stored by a human subject is visible to a Service Token subject", async () => { + const store = new SharedStore(); + const tenant = "tenant-a"; + + // Human user (browser OAuth) stores the token. + const human = makeProviderFromStore(store, "master", { tenant, subject: "human@example.com" }); + await Effect.runPromise(human.provider.set!(id("oauth:org:gmail:default"), "ya29.token")); + + // Service Token (different subject) retrieves it. + const svc = makeProviderFromStore(store, "master", { + tenant, + subject: "service-token-id.access", + }); + const got = await Effect.runPromise(svc.provider.get(id("oauth:org:gmail:default"))); + expect(got).toBe("ya29.token"); + }); +}); diff --git a/packages/plugins/encrypted-secrets/src/index.ts b/packages/plugins/encrypted-secrets/src/index.ts index 8447fb7f0..c06d8e793 100644 --- a/packages/plugins/encrypted-secrets/src/index.ts +++ b/packages/plugins/encrypted-secrets/src/index.ts @@ -78,11 +78,12 @@ const decryptSecret = (key: Buffer, payload: string): Effect.Effect - binding.subject == null ? Owner.make("org") : Owner.make("user"); +/** Always store encrypted secrets at the org partition so that any subject + * within the tenant — a human user who completed browser OAuth or a Service + * Token driving an MCP tool call — can retrieve them. Access control is + * enforced by the connection row's own (tenant, owner, subject) partition; + * the provider is a shared, opaque-key vault. */ +const ownerOf = (_binding: OwnerBinding): Owner => Owner.make("org"); const makeEncryptedProvider = ( key: Buffer,