Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 43 additions & 58 deletions apps/cloud/src/secrets-isolation.e2e.node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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");
}),
);

Expand All @@ -108,36 +99,34 @@ 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)) },
}),
);
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");
}),
);

Expand All @@ -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");
}),
);

Expand Down Expand Up @@ -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");
}),
);

Expand Down Expand Up @@ -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");
}),
);
});
79 changes: 36 additions & 43 deletions apps/cloud/src/services/secrets-api.node.test.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
// 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";
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)}`;
Expand All @@ -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);
}),
);

Expand Down Expand Up @@ -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)}`;
Expand All @@ -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");
}),
);

Expand All @@ -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)}`;
Expand All @@ -146,25 +143,21 @@ 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* () {
yield* client.secrets.set({
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");
}),
);
});
16 changes: 10 additions & 6 deletions apps/cloud/src/services/tenant-isolation.node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()}`;
Expand All @@ -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);
}),
);

Expand Down
3 changes: 0 additions & 3 deletions packages/core/api/src/connections/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { Schema } from "effect";
import {
ConnectionId,
ScopeId,
SecretId,
} from "@executor/sdk";

import { InternalError } from "../observability";
Expand All @@ -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,
Expand Down
2 changes: 0 additions & 2 deletions packages/core/api/src/handlers/connections.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
12 changes: 1 addition & 11 deletions packages/core/api/src/handlers/secrets.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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;
Expand Down
Loading
Loading