Skip to content

Commit d1ec004

Browse files
committed
Partition encrypted-secrets rows by the owner embedded in the item id
1 parent 21fb0c1 commit d1ec004

10 files changed

Lines changed: 378 additions & 9 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"executor": patch
3+
---
4+
5+
**Fix: org OAuth connections on self-host worked only for whoever ran the consent**
6+
7+
The encrypted-secrets credential provider (the writable provider on the self-hosted and Cloudflare hosts) filed token rows under the _acting user's_ private partition instead of the credential's own owner. An org-owned OAuth connection whose consent completed in one member's browser session therefore resolved only for that member — every other principal failed with `oauth_connection_missing`, while the UI showed the connection healthy. The provider now partitions by the owner embedded in the item id (`oauth:org:…` → org-shared), matching the WorkOS Vault provider, and a boot-time data migration re-files rows already written wrong. The encrypted value itself was never affected.

apps/host-cloudflare/src/db/data-migrations.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ describe("runCloudflareDataMigrations", () => {
241241
"2026-06-20-google-openapi-ownership",
242242
"2026-07-08-provider-service-split",
243243
"2026-07-09-openapi-ndjson-output-arrays",
244+
"2026-07-27-encrypted-secrets-owner-repartition",
244245
]);
245246
expect(yield* Effect.promise(() => runCloudflareDataMigrations(d1, bucket))).toEqual([]);
246247

@@ -285,6 +286,7 @@ describe("runCloudflareDataMigrations", () => {
285286
"2026-06-20-google-openapi-ownership",
286287
"2026-07-08-provider-service-split",
287288
"2026-07-09-openapi-ndjson-output-arrays",
289+
"2026-07-27-encrypted-secrets-owner-repartition",
288290
]);
289291
expect(yield* Effect.promise(() => runCloudflareDataMigrations(d1, bucket))).toEqual([]);
290292

@@ -336,6 +338,7 @@ describe("runCloudflareDataMigrations", () => {
336338
"2026-06-20-google-openapi-ownership",
337339
"2026-07-08-provider-service-split",
338340
"2026-07-09-openapi-ndjson-output-arrays",
341+
"2026-07-27-encrypted-secrets-owner-repartition",
339342
]);
340343
expect(yield* Effect.promise(() => runCloudflareDataMigrations(d1, bucket))).toEqual([]);
341344

apps/host-cloudflare/src/db/data-migrations.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
} from "@executor-js/sdk";
1010
import { openApiNdjsonOutputDataMigration } from "@executor-js/plugin-openapi";
1111
import { googleOpenApiOwnershipDataMigration } from "@executor-js/plugin-openapi/providers/google";
12+
import { encryptedSecretsRepartitionDataMigration } from "@executor-js/plugin-encrypted-secrets";
1213

1314
import {
1415
providerServiceSplitDataMigration,
@@ -246,6 +247,9 @@ const cloudflareDataMigrations = (bucket: R2Bucket | undefined): readonly Sqlite
246247
// Stale-mark connections whose operations return NDJSON so their tool rows
247248
// rebuild with array-wrapped output schemas (mirrors cloud's drizzle 0010).
248249
openApiNdjsonOutputDataMigration,
250+
// Re-file credential rows the pre-fix provider stored under the acting
251+
// caller's partition instead of the owner embedded in the item id (#1453).
252+
encryptedSecretsRepartitionDataMigration,
249253
];
250254

251255
export const runCloudflareDataMigrations = (

apps/host-selfhost/src/db/data-migrations.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { graphqlIntrospectionBlobDataMigration } from "@executor-js/plugin-graph
1616
import { googleOpenApiOwnershipDataMigration } from "@executor-js/plugin-openapi/providers/google";
1717

1818
import { providerServiceSplitDataMigration } from "@executor-js/plugin-provider-service-split";
19+
import { encryptedSecretsRepartitionDataMigration } from "@executor-js/plugin-encrypted-secrets";
1920
import { authConfigTransforms } from "./auth-config-migration";
2021

2122
export const selfHostDataMigrations: readonly SqliteDataMigration[] = [
@@ -37,4 +38,7 @@ export const selfHostDataMigrations: readonly SqliteDataMigration[] = [
3738
// Stale-mark connections whose operations return NDJSON so their tool rows
3839
// rebuild with array-wrapped output schemas (mirrors cloud's drizzle 0010).
3940
openApiNdjsonOutputDataMigration,
41+
// Re-file credential rows the pre-fix provider stored under the acting
42+
// caller's partition instead of the owner embedded in the item id (#1453).
43+
encryptedSecretsRepartitionDataMigration,
4044
];

bun.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/plugins/encrypted-secrets/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
},
1919
"devDependencies": {
2020
"@effect/vitest": "catalog:",
21+
"@libsql/client": "catalog:",
2122
"@types/node": "catalog:",
2223
"bun-types": "catalog:",
2324
"tsup": "catalog:",

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

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,10 @@ const makeFakeStorage = () => {
5959
}),
6060
remove: (input: { collection: string; key: string; owner: Owner }) =>
6161
Effect.sync(() => {
62-
rows.delete(composite(input.collection, input.key));
62+
// Owner-filtered like the real facade: a remove aimed at the wrong
63+
// partition deletes nothing.
64+
const row = rows.get(composite(input.collection, input.key));
65+
if (row && row.owner === input.owner) rows.delete(composite(input.collection, input.key));
6366
}),
6467
};
6568
return { facade, rows };
@@ -135,7 +138,8 @@ describe("provider", () => {
135138
// the scope arg entirely. The provider keys solely by the opaque
136139
// `ProviderItemId`; the referencing connection row owns the (tenant, owner,
137140
// subject) partition, so cross-scope isolation is no longer the provider's
138-
// concern to enforce or test.
141+
// concern to enforce or test. Filing rows into the RIGHT partition still is
142+
// — see the partitioning describe below (issue #1453).
139143

140144
test("get returns null for a missing id", async () => {
141145
const { provider } = makeProvider("master");
@@ -162,3 +166,50 @@ describe("provider", () => {
162166
expect(entries.map((e) => e.id).sort()).toEqual(["alpha", "beta"]);
163167
});
164168
});
169+
170+
// The partition a write files under is the CREDENTIAL's owner (embedded in
171+
// the item id), not the acting caller's binding. Issue #1453: a user-bound
172+
// caller completing an org connection's OAuth consent filed the tokens under
173+
// their private partition, so every other org principal resolved nothing and
174+
// failed with `oauth_connection_missing`.
175+
describe("owner partitioning", () => {
176+
const storedOwner = (
177+
rows: Map<string, { owner: Owner; key: string; collection: string; data: unknown }>,
178+
key: string,
179+
) => [...rows.values()].find((row) => row.key === key)?.owner;
180+
181+
test("an org-embedded id written by a user-bound caller files org-shared", async () => {
182+
const { provider, rows } = makeProvider("master", Owner.make("user"));
183+
await Effect.runPromise(provider.set!(id("oauth:org:slack:workspace"), "access_tok"));
184+
await Effect.runPromise(provider.set!(id("oauth:org:slack:workspace:refresh"), "refresh_tok"));
185+
await Effect.runPromise(provider.set!(id("connection:org:exa:main:token"), "api_key"));
186+
await Effect.runPromise(provider.set!(id("oauth-client:org:my_app:secret"), "client_sec"));
187+
expect(storedOwner(rows, "oauth:org:slack:workspace")).toBe("org");
188+
expect(storedOwner(rows, "oauth:org:slack:workspace:refresh")).toBe("org");
189+
expect(storedOwner(rows, "connection:org:exa:main:token")).toBe("org");
190+
expect(storedOwner(rows, "oauth-client:org:my_app:secret")).toBe("org");
191+
});
192+
193+
test("a user-embedded id written by an org-bound caller files user-private", async () => {
194+
const { provider, rows } = makeProvider("master", Owner.make("org"));
195+
await Effect.runPromise(provider.set!(id("connection:user:notion:personal:token"), "private"));
196+
expect(storedOwner(rows, "connection:user:notion:personal:token")).toBe("user");
197+
});
198+
199+
test("an id with no embedded owner falls back to the caller binding", async () => {
200+
const asUser = makeProvider("master", Owner.make("user"));
201+
await Effect.runPromise(asUser.provider.set!(id("github"), "v"));
202+
expect(storedOwner(asUser.rows, "github")).toBe("user");
203+
204+
const asOrg = makeProvider("master", Owner.make("org"));
205+
await Effect.runPromise(asOrg.provider.set!(id("github"), "v"));
206+
expect(storedOwner(asOrg.rows, "github")).toBe("org");
207+
});
208+
209+
test("delete targets the embedded owner's partition", async () => {
210+
const { provider, rows } = makeProvider("master", Owner.make("user"));
211+
await Effect.runPromise(provider.set!(id("oauth:org:slack:workspace"), "tok"));
212+
await Effect.runPromise(provider.delete!(id("oauth:org:slack:workspace")));
213+
expect(rows.size).toBe(0);
214+
});
215+
});

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

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ import {
2929
// v2: the provider sees only an opaque `ProviderItemId` — there is NO scope
3030
// arg. The connection row that references the id owns the (tenant, owner,
3131
// subject) partition; the encrypted value is keyed solely by the opaque id.
32-
// Plugin storage writes still carry an `owner` (the executor's binding), which
33-
// is captured once from the ctx at provider construction.
32+
// Plugin storage writes carry an `owner`: the one embedded in the item id
33+
// (`oauth:org:…` → org-shared) when present, else the executor's binding.
3434
// ---------------------------------------------------------------------------
3535

3636
type PluginStorage = PluginCtx<unknown>["pluginStorage"];
@@ -80,14 +80,37 @@ const ENCRYPTED_PROVIDER_KEY = ProviderKey.make("encrypted");
8080

8181
/** Map the executor's (tenant, subject?) binding onto the storage `Owner`
8282
* literal: a bound subject writes the user's own partition, otherwise the
83-
* org-shared one. */
83+
* org-shared one. Fallback only — prefer `ownerForItemId`. */
8484
const ownerOf = (binding: OwnerBinding): Owner =>
8585
binding.subject == null ? Owner.make("org") : Owner.make("user");
8686

87+
// Item ids whose SECOND colon-segment is the owning partition:
88+
// connection:<owner>:<integration>:<name>:<variable>
89+
// oauth:<owner>:<integration>:<name>[:refresh]
90+
// oauth-client:<owner>:<slug>:secret
91+
const OWNER_SCOPED_PREFIXES: ReadonlySet<string> = new Set(["connection", "oauth", "oauth-client"]);
92+
93+
/** The owner a logical item id embeds, or null for ids that carry none. */
94+
const embeddedOwner = (id: string): Owner | null => {
95+
const [prefix, owner] = id.split(":");
96+
if (OWNER_SCOPED_PREFIXES.has(prefix ?? "") && (owner === "org" || owner === "user")) {
97+
return Owner.make(owner);
98+
}
99+
return null;
100+
};
101+
102+
/** The partition a credential's value belongs to: the CREDENTIAL's owner
103+
* (embedded in the item id), not the acting caller's binding — so an org
104+
* connection whose OAuth consent completes in one member's browser session
105+
* files tokens every member can resolve. Ids without an embedded owner fall
106+
* back to the caller binding. Mirrors the workos-vault provider. */
107+
const ownerForItemId = (id: string, binding: OwnerBinding): Owner =>
108+
embeddedOwner(id) ?? ownerOf(binding);
109+
87110
const makeEncryptedProvider = (
88111
key: Buffer,
89112
storage: PluginStorage,
90-
owner: Owner,
113+
binding: OwnerBinding,
91114
): CredentialProvider => ({
92115
key: ENCRYPTED_PROVIDER_KEY,
93116
writable: true,
@@ -105,12 +128,18 @@ const makeEncryptedProvider = (
105128
set: (id: ProviderItemId, value: string) =>
106129
encryptSecret(key, value).pipe(
107130
Effect.flatMap((payload) =>
108-
storage.put({ collection: COLLECTION, key: id, owner, data: payload }),
131+
storage.put({
132+
collection: COLLECTION,
133+
key: id,
134+
owner: ownerForItemId(id, binding),
135+
data: payload,
136+
}),
109137
),
110138
Effect.asVoid,
111139
),
112140

113-
delete: (id: ProviderItemId) => storage.remove({ collection: COLLECTION, key: id, owner }),
141+
delete: (id: ProviderItemId) =>
142+
storage.remove({ collection: COLLECTION, key: id, owner: ownerForItemId(id, binding) }),
114143

115144
list: () =>
116145
storage
@@ -143,10 +172,17 @@ export const encryptedSecretsPlugin = definePlugin((options?: EncryptedSecretsPl
143172
id: "encryptedSecrets" as const,
144173
storage: () => ({}),
145174
credentialProviders: (ctx: PluginCtx<unknown>): readonly CredentialProvider[] => [
146-
makeEncryptedProvider(derivedKey, ctx.pluginStorage, ownerOf(ctx.owner)),
175+
makeEncryptedProvider(derivedKey, ctx.pluginStorage, ctx.owner),
147176
],
148177
};
149178
});
150179

151180
// Exported for host-side tests / reuse.
152181
export { deriveKey, encryptSecret, decryptSecret };
182+
183+
// Boot-time repair for rows the pre-fix provider filed under the acting
184+
// caller's partition (issue #1453).
185+
export {
186+
encryptedSecretsRepartitionDataMigration,
187+
runSqliteEncryptedSecretsRepartition,
188+
} from "./repartition-migration";
Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
import { createClient, type Client } from "@libsql/client";
2+
import { afterEach, describe, expect, test } from "@effect/vitest";
3+
import { Effect } from "effect";
4+
5+
import { runSqliteEncryptedSecretsRepartition } from "./repartition-migration";
6+
7+
// A real in-memory libSQL database with the plugin_storage shape the hosts
8+
// use (unique on tenant/owner/subject/plugin_id/collection/key), so the
9+
// insert-or-ignore + delete choreography is exercised for real.
10+
const PLUGIN_STORAGE_DDL = `CREATE TABLE plugin_storage (
11+
row_id text PRIMARY KEY NOT NULL,
12+
tenant text NOT NULL,
13+
owner text NOT NULL,
14+
subject text NOT NULL,
15+
plugin_id text NOT NULL,
16+
collection text NOT NULL,
17+
key text NOT NULL,
18+
data text NOT NULL,
19+
created_at integer NOT NULL,
20+
updated_at integer NOT NULL,
21+
CONSTRAINT plugin_storage_uidx UNIQUE (tenant, owner, subject, plugin_id, collection, key)
22+
)`;
23+
24+
type Row = {
25+
readonly rowId: string;
26+
readonly owner: string;
27+
readonly subject: string;
28+
readonly key: string;
29+
readonly pluginId?: string;
30+
readonly collection?: string;
31+
};
32+
33+
let client: Client | undefined;
34+
35+
const makeDb = async (rows: readonly Row[]): Promise<Client> => {
36+
client = createClient({ url: ":memory:" });
37+
await client.execute(PLUGIN_STORAGE_DDL);
38+
for (const row of rows) {
39+
await client.execute({
40+
sql: `INSERT INTO plugin_storage
41+
(row_id, tenant, owner, subject, plugin_id, collection, key, data, created_at, updated_at)
42+
VALUES (?, 'tenant-a', ?, ?, ?, ?, ?, 'v1.iv.tag.ct', 1, 1)`,
43+
args: [
44+
row.rowId,
45+
row.owner,
46+
row.subject,
47+
row.pluginId ?? "encryptedSecrets",
48+
row.collection ?? "secrets",
49+
row.key,
50+
],
51+
});
52+
}
53+
return client;
54+
};
55+
56+
afterEach(() => {
57+
client?.close();
58+
client = undefined;
59+
});
60+
61+
const partitions = async (db: Client) => {
62+
const result = await db.execute(
63+
"SELECT owner, subject, key, data FROM plugin_storage ORDER BY key",
64+
);
65+
return result.rows.map((row) => ({
66+
owner: row.owner,
67+
subject: row.subject,
68+
key: row.key,
69+
data: row.data,
70+
}));
71+
};
72+
73+
describe("runSqliteEncryptedSecretsRepartition", () => {
74+
test("moves org-embedded rows out of a user partition, ciphertext intact", async () => {
75+
const db = await makeDb([
76+
{ rowId: "r1", owner: "user", subject: "user-123", key: "oauth:org:linear:main" },
77+
{ rowId: "r2", owner: "user", subject: "user-123", key: "oauth:org:linear:main:refresh" },
78+
]);
79+
const moved = await Effect.runPromise(runSqliteEncryptedSecretsRepartition(db));
80+
expect(moved).toBe(2);
81+
expect(await partitions(db)).toEqual([
82+
{ owner: "org", subject: "", key: "oauth:org:linear:main", data: "v1.iv.tag.ct" },
83+
{ owner: "org", subject: "", key: "oauth:org:linear:main:refresh", data: "v1.iv.tag.ct" },
84+
]);
85+
});
86+
87+
test("leaves correctly filed rows and unscoped ids alone", async () => {
88+
const db = await makeDb([
89+
{ rowId: "r1", owner: "org", subject: "", key: "oauth:org:linear:main" },
90+
{ rowId: "r2", owner: "user", subject: "user-123", key: "connection:user:notion:p:token" },
91+
{ rowId: "r3", owner: "user", subject: "user-123", key: "legacy-random-id" },
92+
]);
93+
const moved = await Effect.runPromise(runSqliteEncryptedSecretsRepartition(db));
94+
expect(moved).toBe(0);
95+
expect((await partitions(db)).map((row) => row.key)).toEqual([
96+
"connection:user:notion:p:token",
97+
"legacy-random-id",
98+
"oauth:org:linear:main",
99+
]);
100+
});
101+
102+
test("a post-fix org row wins over the mis-filed duplicate", async () => {
103+
const db = await makeDb([
104+
{ rowId: "old", owner: "user", subject: "user-123", key: "oauth:org:linear:main" },
105+
{ rowId: "new", owner: "org", subject: "", key: "oauth:org:linear:main" },
106+
]);
107+
await db.execute("UPDATE plugin_storage SET data = 'v1.new' WHERE row_id = 'new'");
108+
const moved = await Effect.runPromise(runSqliteEncryptedSecretsRepartition(db));
109+
expect(moved).toBe(1);
110+
expect(await partitions(db)).toEqual([
111+
{ owner: "org", subject: "", key: "oauth:org:linear:main", data: "v1.new" },
112+
]);
113+
});
114+
115+
test("does not touch other plugins' rows", async () => {
116+
const db = await makeDb([
117+
{
118+
rowId: "r1",
119+
owner: "user",
120+
subject: "user-123",
121+
key: "oauth:org:slack:main",
122+
pluginId: "workosVault",
123+
collection: "metadata",
124+
},
125+
]);
126+
const moved = await Effect.runPromise(runSqliteEncryptedSecretsRepartition(db));
127+
expect(moved).toBe(0);
128+
expect((await partitions(db))[0]).toMatchObject({ owner: "user", subject: "user-123" });
129+
});
130+
131+
test("is a no-op on a database without the table", async () => {
132+
client = createClient({ url: ":memory:" });
133+
const moved = await Effect.runPromise(runSqliteEncryptedSecretsRepartition(client));
134+
expect(moved).toBe(0);
135+
});
136+
137+
test("is idempotent", async () => {
138+
const db = await makeDb([
139+
{ rowId: "r1", owner: "user", subject: "user-123", key: "oauth:org:linear:main" },
140+
]);
141+
expect(await Effect.runPromise(runSqliteEncryptedSecretsRepartition(db))).toBe(1);
142+
expect(await Effect.runPromise(runSqliteEncryptedSecretsRepartition(db))).toBe(0);
143+
});
144+
});

0 commit comments

Comments
 (0)