fix(encrypted-secrets): store tokens at org partition for Service Token compatibility

packages/plugins/encrypted-secrets/src/index.test.ts

@@ -6,78 +6,138 @@ 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<string, { owner: Owner; key: string; collection: string; data: unknown }>();
-  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<string, FakeRow>();
+  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<unknown>;
   const plugin = encryptedSecretsPlugin({ key });
   const credentialProviders = plugin.credentialProviders as (
     ctx: PluginCtx<unknown>,
   ) => 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");
+  });
+});

packages/plugins/encrypted-secrets/src/index.ts

@@ -78,11 +78,12 @@ const decryptSecret = (key: Buffer, payload: string): Effect.Effect<string, Stor
 
 const ENCRYPTED_PROVIDER_KEY = ProviderKey.make("encrypted");
 
-/** Map the executor's (tenant, subject?) binding onto the storage `Owner`
- *  literal: a bound subject writes the user's own partition, otherwise the
- *  org-shared one. */
-const ownerOf = (binding: OwnerBinding): Owner =>
-  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,