diff --git a/AGENTS.md b/AGENTS.md index af04c4ae7..a1490733d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -31,6 +31,8 @@ All chat platforms (Telegram, Slack, Discord, WhatsApp, Teams) run through Chat **Webhooks via the Chat SDK adapter are the default transport.** Don't add new per-platform alternative transports (Slack Socket Mode, Discord Gateway WebSocket bridges, etc.) or extra runtime SDKs. The lone exception is Telegram, whose connection config exposes an optional `polling` mode (`mode: "auto" | "webhook" | "polling"`) implemented inside the Chat SDK adapter — still no extra SDK. Local dev for webhook-only platforms uses a tunnel (cloudflared / ngrok / Tailscale Funnel); Lobu Cloud users get a public URL for free. Sticking to the Chat SDK keeps one delivery story, one set of retries, and zero extra dependencies. +`mode: "polling"` is rejected at connection-create time when `LOBU_CLOUD_MODE=1` — a polling worker long-polls Telegram's edge from the gateway pod and shares that connection across tenants, so a misbehaving polling connection in one org degrades delivery for every other tenant. Self-hosters (`LOBU_CLOUD_MODE` unset/0) keep the polling option for tunnel-less dev. + #### Orchestration - **Embedded-only deployment.** Gateway, workers, embeddings, and the Lobu memory backend run in a single Node process (`lobu run`, or `bun run dev` in the monorepo). Workers spawn as `child_process.spawn` subprocesses on the same host; on Linux the spawn path uses `systemd-run --user --scope` for cgroup limits + IPAddressDeny + capability drops. There is no Docker or Kubernetes deployment manager. - Postgres (with `pgvector`; optionally `postgis` for geo enrichment) is the only user-provided external. The Node process connects out via `DATABASE_URL`. Runtime state — queues, chat connection rows, grant cache, MCP proxy sessions — lives in dedicated Postgres tables. diff --git a/packages/core/src/agent-store.ts b/packages/core/src/agent-store.ts index 10b52520c..d837517c7 100644 --- a/packages/core/src/agent-store.ts +++ b/packages/core/src/agent-store.ts @@ -91,6 +91,14 @@ export interface AgentMetadata { owner: { platform: string; userId: string }; isWorkspaceAgent?: boolean; workspaceId?: string; + /** + * Owning organization id. Optional in the type for back-compat with + * in-memory stores that predate per-tenant scoping; populated by the + * postgres-backed store. The public Agent API route reads this to stamp + * worker tokens with the agent's org so the egress proxy can scope + * per-tenant gates (grants, judge cache, judge policy). + */ + organizationId?: string; createdAt: number; lastUsedAt?: number; } diff --git a/packages/core/src/worker/auth.ts b/packages/core/src/worker/auth.ts index 1df2899af..b87591148 100644 --- a/packages/core/src/worker/auth.ts +++ b/packages/core/src/worker/auth.ts @@ -15,6 +15,14 @@ export interface WorkerTokenData { channelId: string; teamId?: string; agentId?: string; + /** + * Owning organization of the agent the token was minted for. Used by the + * HTTP proxy to scope per-tenant caches (e.g. egress-judge verdict cache) + * so org A's decisions can never satisfy org B's requests. Optional only + * because some internal/preflight call sites mint tokens before the owning + * org has been resolved; production agent runs always set it. + */ + organizationId?: string; connectionId?: string; deploymentName: string; timestamp: number; @@ -33,6 +41,7 @@ export function generateWorkerToken( channelId: string; teamId?: string; agentId?: string; + organizationId?: string; connectionId?: string; platform?: string; sessionKey?: string; @@ -49,6 +58,7 @@ export function generateWorkerToken( channelId: options.channelId, teamId: options.teamId, agentId: options.agentId, + organizationId: options.organizationId, connectionId: options.connectionId, deploymentName, timestamp: Date.now(), diff --git a/packages/server/src/__tests__/unit/egress-judge-timeout.test.ts b/packages/server/src/__tests__/unit/egress-judge-timeout.test.ts index 4ed4ffb28..ee8aec93c 100644 --- a/packages/server/src/__tests__/unit/egress-judge-timeout.test.ts +++ b/packages/server/src/__tests__/unit/egress-judge-timeout.test.ts @@ -40,7 +40,7 @@ describe("EgressJudge timeout", () => { const judge = new EgressJudge({ client, judgeTimeoutMs: 30 }); const started = Date.now(); const decision = await judge.decide( - { agentId: "agent-a", hostname: "api.github.com" }, + { agentId: "agent-a", organizationId: "org-1", hostname: "api.github.com" }, rule() ); const elapsed = Date.now() - started; @@ -62,7 +62,7 @@ describe("EgressJudge timeout", () => { // Distinct hostnames so the verdict cache never short-circuits the path. for (let i = 0; i < 5; i++) { const decision = await judge.decide( - { agentId: "agent-a", hostname: `h${i}.example.com` }, + { agentId: "agent-a", organizationId: "org-1", hostname: `h${i}.example.com` }, rule() ); expect(decision.verdict).toBe("deny"); @@ -72,7 +72,7 @@ describe("EgressJudge timeout", () => { expect(client.calls).toBe(2); const afterOpen = await judge.decide( - { agentId: "agent-a", hostname: "another.example.com" }, + { agentId: "agent-a", organizationId: "org-1", hostname: "another.example.com" }, rule() ); expect(afterOpen.verdict).toBe("deny"); @@ -88,7 +88,7 @@ describe("EgressJudge timeout", () => { const judge = new EgressJudge({ client }); const started = Date.now(); const decision = await judge.decide( - { agentId: "agent-a", hostname: "api.github.com" }, + { agentId: "agent-a", organizationId: "org-1", hostname: "api.github.com" }, rule() ); expect(decision.verdict).toBe("deny"); diff --git a/packages/server/src/gateway/__tests__/base-deployment-grants.test.ts b/packages/server/src/gateway/__tests__/base-deployment-grants.test.ts index e8969a701..9138d9b44 100644 --- a/packages/server/src/gateway/__tests__/base-deployment-grants.test.ts +++ b/packages/server/src/gateway/__tests__/base-deployment-grants.test.ts @@ -160,7 +160,7 @@ describe("BaseDeploymentManager.syncNetworkConfigGrants", () => { }) ); - let resolved = policyStore.resolve("agent-1", "api.example.com"); + let resolved = policyStore.resolve("test-org", "agent-1", "api.example.com"); expect(resolved?.policy).toContain("initial policy"); expect(resolved?.policy).toContain("operator policy"); @@ -173,12 +173,12 @@ describe("BaseDeploymentManager.syncNetworkConfigGrants", () => { }) ); - resolved = policyStore.resolve("agent-1", "api.example.com"); + resolved = policyStore.resolve("test-org", "agent-1", "api.example.com"); expect(resolved?.policy).toContain("updated policy"); expect(resolved?.policy).not.toContain("initial policy"); await barebones.syncNetworkConfigGrants(buildPayload({})); - expect(policyStore.resolve("agent-1", "api.example.com")).toBeUndefined(); + expect(policyStore.resolve("test-org", "agent-1", "api.example.com")).toBeUndefined(); }); test("skips redundant writes when the pattern set has not changed", async () => { diff --git a/packages/server/src/gateway/__tests__/egress-judge-cache.test.ts b/packages/server/src/gateway/__tests__/egress-judge-cache.test.ts index 64a3fc4bb..02c4a1e99 100644 --- a/packages/server/src/gateway/__tests__/egress-judge-cache.test.ts +++ b/packages/server/src/gateway/__tests__/egress-judge-cache.test.ts @@ -10,6 +10,7 @@ describe("VerdictCache", () => { test("stores and retrieves a verdict", () => { const cache = new VerdictCache(60_000, 100); const key = VerdictCache.key({ + orgId: "org-1", policyHash: "abc", hostname: "example.com", method: "GET", @@ -21,11 +22,13 @@ describe("VerdictCache", () => { test("key is case-insensitive for hostname and method", () => { const a = VerdictCache.key({ + orgId: "org-1", policyHash: "h", hostname: "Example.COM", method: "get", }); const b = VerdictCache.key({ + orgId: "org-1", policyHash: "h", hostname: "example.com", method: "GET", @@ -34,14 +37,20 @@ describe("VerdictCache", () => { }); test("different policy hashes do not collide", () => { - const a = VerdictCache.key({ policyHash: "h1", hostname: "x.com" }); - const b = VerdictCache.key({ policyHash: "h2", hostname: "x.com" }); + const a = VerdictCache.key({ orgId: "org-1", policyHash: "h1", hostname: "x.com" }); + const b = VerdictCache.key({ orgId: "org-1", policyHash: "h2", hostname: "x.com" }); + expect(a).not.toBe(b); + }); + + test("different orgIds do not collide even for identical policy+request", () => { + const a = VerdictCache.key({ orgId: "org-a", policyHash: "h", hostname: "x.com" }); + const b = VerdictCache.key({ orgId: "org-b", policyHash: "h", hostname: "x.com" }); expect(a).not.toBe(b); }); test("expires entries after the TTL", async () => { const cache = new VerdictCache(10, 100); - const key = VerdictCache.key({ policyHash: "h", hostname: "x.com" }); + const key = VerdictCache.key({ orgId: "org-1", policyHash: "h", hostname: "x.com" }); cache.set(key, { verdict: "allow", reason: "ok" }); expect(cache.get(key)).toBeDefined(); await new Promise((r) => setTimeout(r, 20)); diff --git a/packages/server/src/gateway/__tests__/http-proxy-judge.test.ts b/packages/server/src/gateway/__tests__/http-proxy-judge.test.ts index 61157b6bb..4e0ade4f0 100644 --- a/packages/server/src/gateway/__tests__/http-proxy-judge.test.ts +++ b/packages/server/src/gateway/__tests__/http-proxy-judge.test.ts @@ -52,7 +52,7 @@ beforeAll(async () => { // `example.com` is used for the allow-path HTTP tests because its real // server closes the socket after responding — tests that use hosts with // keep-alive (api.github.com, etc.) hang the raw socket reader. - policyStore.set("agent-a", { + policyStore.set("org-a", "agent-a", { judgedDomains: [ { domain: "example.com" }, { domain: ".slack.com", judge: "strict" }, @@ -86,6 +86,7 @@ function createValidToken(deploymentName: string): string { channelId: "test-channel", platform: "test", agentId: "agent-a", + organizationId: "org-a", }); } diff --git a/packages/server/src/gateway/__tests__/multi-tenant-isolation-reproducers.test.ts b/packages/server/src/gateway/__tests__/multi-tenant-isolation-reproducers.test.ts new file mode 100644 index 000000000..c99cd37d3 --- /dev/null +++ b/packages/server/src/gateway/__tests__/multi-tenant-isolation-reproducers.test.ts @@ -0,0 +1,522 @@ +/** + * Red→green reproducers for the four critical multi-tenant isolation gaps + * surfaced on PR #836: + * + * - Finding 1: secret-proxy `lookupPlaceholderMapping` was dead-code — + * no production call site passed `expectedOrganizationId`, so a worker + * bound to org-A could resolve its placeholder under org-B's URL. + * - Finding 2: `checkDomainAccess()` had `organizationId` on its frame + * but never threaded it into `GrantStore.isDenied/hasGrant`. The store + * fell back to ALS (empty in the raw HTTP proxy hot path) and matched + * grants by `agent_id` alone — cross-org leakage when an agent id is + * reused. + * - Finding 3: `PolicyStore` was keyed by `agentId` alone, so the last + * `set()` across orgs won. Cache scoping became theatre because the + * policy fed into the verdict was already wrong. + * - Finding 6 — peek-before-consume — is covered by `slack-routes.test.ts` + * ("rejects when callback session org differs from install state" + * asserts the row is preserved for a legitimate retry). + * + * Each test in this file asserts the post-fix behaviour. The PR description + * pastes the corresponding pre-fix `bun test` output so the reproducer + * doubles as a regression gate — flip a single fix line and the listed + * assertion fails. + */ + +import { afterAll, beforeAll, beforeEach, describe, expect, test } from "bun:test"; +import { + createBuiltinSecretRef, + generateWorkerToken, + verifyWorkerToken, +} from "@lobu/core"; +import { GrantStore } from "../permissions/grant-store.js"; +import { PolicyStore } from "../permissions/policy-store.js"; +import { + __resetPlaceholderCacheForTests, + generatePlaceholder, + lookupPlaceholderMapping, + SecretProxy, +} from "../proxy/secret-proxy.js"; +import type { SecretStore } from "../secrets/index.js"; +import { + ensurePgliteForGatewayTests, + resetTestDatabase, + seedAgentRow, +} from "./helpers/db-setup.js"; + +// ─── Finding 3: PolicyStore cross-tenant clobbering ────────────────────────── + +describe("[finding 3] PolicyStore is keyed by (orgId, agentId)", () => { + test("org A's policy survives org B's set under the same agent id", () => { + const store = new PolicyStore(); + // Org A sets a policy for `shared-agent-id`. + store.set("org-a", "shared-agent-id", { + judgedDomains: [{ domain: "api.example.com" }], + judges: { default: "ORG A: deny by default" }, + }); + // Org B reuses the same agent id — under the old keying this overwrote + // org A's bundle. Now both must coexist. + store.set("org-b", "shared-agent-id", { + judgedDomains: [{ domain: "api.example.com" }], + judges: { default: "ORG B: allow reads" }, + }); + + const aResolved = store.resolve("org-a", "shared-agent-id", "api.example.com"); + const bResolved = store.resolve("org-b", "shared-agent-id", "api.example.com"); + + expect(aResolved?.policy).toBe("ORG A: deny by default"); + expect(bResolved?.policy).toBe("ORG B: allow reads"); + // policyHash diverges so the verdict cache cannot collide either. + expect(aResolved?.policyHash).not.toBe(bResolved?.policyHash); + }); + + test("resolve refuses cross-org reads — no fall-through to a sibling tenant's bundle", () => { + const store = new PolicyStore(); + store.set("org-a", "agent-1", { + judgedDomains: [{ domain: "api.example.com" }], + judges: { default: "ORG A only" }, + }); + // Org B has no bundle for `agent-1` — must return undefined, not + // org A's bundle. + expect( + store.resolve("org-b", "agent-1", "api.example.com") + ).toBeUndefined(); + }); + + test("clear(orgA, agentId) does not affect orgB's bundle for the same agent id", () => { + const store = new PolicyStore(); + store.set("org-a", "shared", { + judgedDomains: [{ domain: "x.com" }], + judges: { default: "A" }, + }); + store.set("org-b", "shared", { + judgedDomains: [{ domain: "x.com" }], + judges: { default: "B" }, + }); + store.clear("org-a", "shared"); + expect(store.resolve("org-a", "shared", "x.com")).toBeUndefined(); + expect(store.resolve("org-b", "shared", "x.com")?.policy).toBe("B"); + }); +}); + +// ─── Finding 1: secret-proxy placeholder cross-org leak ────────────────────── + +describe("[finding 1] lookupPlaceholderMapping enforces caller's expected org", () => { + beforeEach(() => { + __resetPlaceholderCacheForTests(); + }); + + test("org-A placeholder resolved under org-B context returns null", () => { + // Org A mints a placeholder for one of its agents. + const placeholder = generatePlaceholder( + "agent-1", + "API_KEY", + createBuiltinSecretRef("deployments/agent-1/API_KEY"), + "deploy-A", + { organizationId: "org-a" } + ); + // A caller bound to org B presents the placeholder. Pre-fix the lookup + // had no `expectedOrganizationId` plumbed through any production call + // site, so the mapping resolved and org B could spend org A's + // upstream credential. Post-fix: null. + expect(lookupPlaceholderMapping(placeholder, "org-b")).toBeNull(); + }); + + test("a matching expected org still resolves cleanly", () => { + const placeholder = generatePlaceholder( + "agent-1", + "API_KEY", + createBuiltinSecretRef("deployments/agent-1/API_KEY"), + "deploy-A", + { organizationId: "org-a" } + ); + const mapping = lookupPlaceholderMapping(placeholder, "org-a"); + expect(mapping?.agentId).toBe("agent-1"); + expect(mapping?.organizationId).toBe("org-a"); + }); + + test("SecretProxy.forward rejects an org-A placeholder used on an org-B agent's URL", async () => { + // Mint a placeholder for org A's `agent-A1`. Pre-fix, no production call + // site supplied `expectedOrganizationId`, so `lookupPlaceholderMapping` + // returned the org-A mapping even when the URL named an org-B agent — + // the `mapping.agentId === urlAgentId` check downstream then tripped at + // 403, but only because the agent ids differed. If two orgs happened + // to use the same `agentId` (per-org-unique on paper, but a stale dump + // or hand-edit can violate this), no 403 fires and the credential + // leaks. The proxy's `agentOrgResolver` is the independent source of + // truth this test exercises end-to-end. + const placeholder = generatePlaceholder( + "shared-id", + "OPENAI_API_KEY", + createBuiltinSecretRef("deployments/orgA/shared-id/OPENAI_API_KEY"), + "deploy-A", + { organizationId: "org-a" } + ); + + const stubStore: SecretStore = { get: async () => "real-secret-A" }; + const proxy = new SecretProxy( + { defaultUpstreamUrl: "https://upstream.example.com" }, + stubStore + ); + proxy.registerUpstream( + { slug: "openai", upstreamBaseUrl: "https://api.openai.example.com" }, + "openai" + ); + // Wire an `agentOrgResolver` that says `shared-id` belongs to org B + // when looked up via the URL. The mapping's org is `org-a` → mismatch. + proxy.setAgentOrgResolver(async () => "org-b"); + + let upstreamCalled = false; + const originalFetch = globalThis.fetch; + globalThis.fetch = async () => { + upstreamCalled = true; + return new Response("{}", { status: 200 }); + }; + + try { + const res = await proxy + .getApp() + .request("/api/proxy/openai/a/shared-id/v1/chat/completions", { + method: "POST", + headers: { + "content-type": "application/json", + authorization: `Bearer ${placeholder}`, + }, + body: JSON.stringify({ prompt: "leak" }), + }); + expect(res.status).toBe(401); + expect(upstreamCalled).toBe(false); + } finally { + globalThis.fetch = originalFetch; + } + }); +}); + +// ─── Finding 2: GrantStore agent-id collision across orgs ──────────────────── + +describe("[finding 2] GrantStore queries scope to caller's organization id", () => { + beforeAll(async () => { + await ensurePgliteForGatewayTests(); + }); + + beforeEach(async () => { + await resetTestDatabase(); + // Seed both orgs and the shared agent id under each — the grants table + // has a FK on `(organization_id, agent_id)` and needs both rows in + // `agents` to exist before we can grant. + await seedAgentRow("shared-agent-id", { organizationId: "org-a" }); + await seedAgentRow("shared-agent-id", { organizationId: "org-b" }); + }); + + test("org A's grant for `shared-agent-id` is invisible to org B's lookup", async () => { + const store = new GrantStore(); + // Org A grants `api.example.com` to its agent. + await store.grant( + "shared-agent-id", + "api.example.com", + null, + false, + "org-a" + ); + // Org B reuses the same agent id (it's per-org-unique on paper; this + // tests that a buggy seed or hand-edited row cannot leak across orgs). + // Without org plumbing the WHERE clause would lose `organization_id` + // and find org A's row. + const orgBSeesGrant = await store.hasGrant( + "shared-agent-id", + "api.example.com", + "org-b" + ); + expect(orgBSeesGrant).toBe(false); + + // Org A still sees its own grant. + expect( + await store.hasGrant("shared-agent-id", "api.example.com", "org-a") + ).toBe(true); + }); + + test("org A's DENY grant for `shared-agent-id` does not block org B", async () => { + const store = new GrantStore(); + await store.grant( + "shared-agent-id", + "api.example.com", + null, + true, // denied + "org-a" + ); + // Org B's isDenied check must see no row. + const orgBDenied = await store.isDenied( + "shared-agent-id", + "api.example.com", + "org-b" + ); + expect(orgBDenied).toBe(false); + + // Org A's isDenied sees its own denial. + expect( + await store.isDenied("shared-agent-id", "api.example.com", "org-a") + ).toBe(true); + }); + + test("HTTP proxy's checkDomainAccess passes the token's orgId into GrantStore", async () => { + // End-to-end exercise of the call-site plumbing: install a real + // GrantStore in the http-proxy, grant `api.example.com` to org A's + // copy of `shared-agent-id`, then hit the proxy with an org-B worker + // token. Pre-fix, the call site dropped the `organizationId` argument + // and the WHERE clause matched org A's grant (the only one with that + // agent id). Post-fix, the predicate now scopes by org and the request + // is blocked. + const { generateWorkerToken } = await import("@lobu/core"); + const { + __testOnly, + setProxyGrantStore, + startHttpProxy, + stopHttpProxy, + } = await import("../proxy/http-proxy.js"); + const crypto = await import("node:crypto"); + const net = await import("node:net"); + + const TEST_ENCRYPTION_KEY = crypto.randomBytes(32).toString("base64"); + process.env.ENCRYPTION_KEY = TEST_ENCRYPTION_KEY; + process.env.WORKER_ALLOWED_DOMAINS = ""; // deny-all globally → grant path + __testOnly.reset(); + + const store = new GrantStore(); + await store.grant( + "shared-agent-id", + "api.example.com", + null, + false, + "org-a" + ); + setProxyGrantStore(store); + + const proxyPort = 10000 + Math.floor(Math.random() * 50000); + const proxyServer = await startHttpProxy(proxyPort, "127.0.0.1"); + + try { + // Mint an org-B token claiming `shared-agent-id`. + const token = generateWorkerToken( + "test-user", + "test-conv", + "deploy-B", + { + channelId: "test-channel", + platform: "test", + agentId: "shared-agent-id", + organizationId: "org-b", + } + ); + const auth = `Basic ${Buffer.from(`deploy-B:${token}`).toString("base64")}`; + + // Fire a raw HTTP request through the proxy targeting api.example.com. + // We can't fetch() with HTTP_PROXY in bun-test, so we hand-roll the + // proxy request and parse the status line out of the raw bytes. + const status = await new Promise((resolve, reject) => { + const socket = new net.Socket(); + socket.connect(proxyPort, "127.0.0.1", () => { + socket.write( + `GET http://api.example.com/v1/x HTTP/1.1\r\n` + + `Host: api.example.com\r\n` + + `Proxy-Authorization: ${auth}\r\n` + + `Connection: close\r\n\r\n` + ); + }); + let data = ""; + let resolved = false; + const tryParse = () => { + if (resolved) return; + const idx = data.indexOf("\r\n"); + if (idx === -1) return; + const m = data.substring(0, idx).match(/HTTP\/\d\.\d (\d+)/); + if (!m) return; + resolved = true; + socket.destroy(); + resolve(Number(m[1])); + }; + socket.on("data", (chunk: Buffer) => { + data += chunk.toString(); + tryParse(); + }); + socket.on("end", () => { + if (!resolved) { + resolved = true; + resolve(0); + } + }); + socket.on("error", (err) => { + if (!resolved) reject(err); + }); + socket.setTimeout(3000, () => { + if (!resolved) { + resolved = true; + socket.destroy(); + resolve(0); + } + }); + }); + + // Pre-fix: 200 — org A's grant matched because no org was passed. + // Post-fix: 403 — grant lookup scoped to org B finds nothing, + // global allowlist denies, judge has no rule for this agent. + expect(status).toBe(403); + } finally { + await stopHttpProxy(proxyServer); + __testOnly.reset(); + delete process.env.ENCRYPTION_KEY; + delete process.env.WORKER_ALLOWED_DOMAINS; + } + }); + + afterAll(async () => { + await resetTestDatabase(); + }); +}); + +// ─── Finding 4: Telegram cloud-mode polling guard at boot ──────────────────── + +describe("[finding 4] ChatInstanceManager.initialize refuses Telegram polling rows in cloud", () => { + beforeAll(async () => { + await ensurePgliteForGatewayTests(); + }); + + beforeEach(async () => { + await resetTestDatabase(); + await seedAgentRow("agent-1", { organizationId: "test-org" }); + }); + + test("a persisted `mode: polling` Telegram row is errored out at boot, not booted", async () => { + const originalKey = process.env.ENCRYPTION_KEY; + const originalCloud = process.env.LOBU_CLOUD_MODE; + process.env.ENCRYPTION_KEY = + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa="; + process.env.LOBU_CLOUD_MODE = "1"; + + try { + const { orgContext } = await import("../../lobu/stores/org-context.js"); + const { createPostgresAgentConnectionStore } = await import( + "../../lobu/stores/postgres-stores.js" + ); + const connectionStore = createPostgresAgentConnectionStore(); + + await orgContext.run({ organizationId: "test-org" }, async () => { + await connectionStore.saveConnection({ + id: "telegram-poll-1", + platform: "telegram", + agentId: "agent-1", + config: { + platform: "telegram", + botToken: "111:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", + mode: "polling", + }, + settings: { allowGroups: true }, + metadata: {}, + status: "active", + createdAt: Date.now(), + updatedAt: Date.now(), + }); + }); + + const mod = await import("../connections/chat-instance-manager.js"); + const services = { + getQueue: () => ({}), + getPublicGatewayUrl: () => "https://gw.example.com", + getSecretStore: () => ({ get: async () => null, put: async () => "" }), + getConnectionStore: () => connectionStore, + getChannelBindingService: () => ({ getBinding: async () => null }), + } as any; + const manager = new mod.ChatInstanceManager() as any; + // initialize() consults isCloudMode + isPollingTelegramMode pre- + // startInstance; the row must end up `status='error'` without a + // running instance. + await manager.initialize(services); + + const stored = await orgContext.run( + { organizationId: "test-org" }, + () => connectionStore.getConnection("telegram-poll-1") + ); + expect(stored?.status).toBe("error"); + expect(stored?.errorMessage ?? "").toContain("Polling mode"); + expect(manager.instances.has("telegram-poll-1")).toBe(false); + } finally { + if (originalKey !== undefined) process.env.ENCRYPTION_KEY = originalKey; + else delete process.env.ENCRYPTION_KEY; + if (originalCloud !== undefined) + process.env.LOBU_CLOUD_MODE = originalCloud; + else delete process.env.LOBU_CLOUD_MODE; + } + }); +}); + +// ─── Public Agent API mint path stamps organizationId on worker tokens ───── +// +// The chat-platform spawn path (`base-deployment-manager`, +// `agent-threads.createThreadForAgent`) already passes `organizationId` +// into `generateWorkerToken`. The public Agent API entry point +// (`POST /api/v1/agents`) did NOT — every worker spawned via `lobu chat`, +// `lobu eval`, or the JS SDK landed with `tokenData.organizationId === +// undefined`. The egress proxy then short-circuited the new per-tenant +// gates and fell back to unscoped checks for that worker. The route now +// looks the agent's owning org up via the ownership metadata store and +// stamps the token. +// +// This test pins the contract: given an agentId whose metadata returns +// org A, the route handler's lookup pattern must yield a token whose +// decoded `organizationId === "org-a"`. A regression that drops the +// pass-through (e.g. forgets `organizationId: tokenOrganizationId` in +// the options bag) fails the second assertion. +describe("[follow-up] API mint path stamps organizationId on worker tokens", () => { + test("metadata-driven lookup propagates org into the worker token", async () => { + const agentId = "agent-mint-1"; + const metadataStore = { + getMetadata: async (id: string) => + id === agentId + ? { id, organizationId: "org-a", createdAt: 0, updatedAt: 0 } + : null, + }; + + // Replicates the in-route helper: look up the pinned agent's org + // before minting the token. Ephemeral agents (no metadata) yield + // undefined and the proxy falls through to unscoped checks — that + // narrower case is tracked as a follow-up. + const tokenOrganizationId = + (await metadataStore.getMetadata(agentId))?.organizationId; + + const token = generateWorkerToken(agentId, "conv-1", "api-mint-1", { + channelId: "api_user-1", + agentId, + organizationId: tokenOrganizationId, + platform: "api", + sessionKey: "user-1", + }); + + const decoded = verifyWorkerToken(token); + expect(decoded).not.toBeNull(); + expect(decoded?.agentId).toBe(agentId); + expect(decoded?.organizationId).toBe("org-a"); + }); + + test("ephemeral agents (no metadata) mint without organizationId", async () => { + const metadataStore = { getMetadata: async () => null }; + const tokenOrganizationId = + (await metadataStore.getMetadata())?.organizationId; + expect(tokenOrganizationId).toBeUndefined(); + + const token = generateWorkerToken( + "ephemeral-agent", + "conv-2", + "api-mint-2", + { + channelId: "api_user-2", + agentId: "ephemeral-agent", + organizationId: tokenOrganizationId, + platform: "api", + sessionKey: "user-2", + } + ); + + const decoded = verifyWorkerToken(token); + expect(decoded).not.toBeNull(); + expect(decoded?.organizationId).toBeUndefined(); + }); +}); diff --git a/packages/server/src/gateway/__tests__/policy-store.test.ts b/packages/server/src/gateway/__tests__/policy-store.test.ts index d82e12ed3..7ee7f0b23 100644 --- a/packages/server/src/gateway/__tests__/policy-store.test.ts +++ b/packages/server/src/gateway/__tests__/policy-store.test.ts @@ -4,16 +4,16 @@ import { buildPolicyBundle, PolicyStore } from "../permissions/policy-store.js"; describe("PolicyStore.resolve", () => { test("returns undefined when no bundle is set", () => { const store = new PolicyStore(); - expect(store.resolve("agent-a", "api.github.com")).toBeUndefined(); + expect(store.resolve("org-a", "agent-a","api.github.com")).toBeUndefined(); }); test("matches an exact domain rule and composes the policy", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-a", "agent-a", { judgedDomains: [{ domain: "api.github.com" }], judges: { default: "Only allow read-only GET requests." }, }); - const resolved = store.resolve("agent-a", "api.github.com"); + const resolved = store.resolve("org-a", "agent-a","api.github.com"); expect(resolved).toBeDefined(); expect(resolved?.judgeName).toBe("default"); expect(resolved?.policy).toContain("Only allow read-only GET requests."); @@ -21,18 +21,18 @@ describe("PolicyStore.resolve", () => { test("matches a wildcard rule", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-a", "agent-a", { judgedDomains: [{ domain: ".example.com" }], judges: { default: "check" }, }); - expect(store.resolve("agent-a", "foo.example.com")).toBeDefined(); - expect(store.resolve("agent-a", "example.com")).toBeDefined(); - expect(store.resolve("agent-a", "unrelated.com")).toBeUndefined(); + expect(store.resolve("org-a", "agent-a","foo.example.com")).toBeDefined(); + expect(store.resolve("org-a", "agent-a","example.com")).toBeDefined(); + expect(store.resolve("org-a", "agent-a","unrelated.com")).toBeUndefined(); }); test("exact match beats wildcard rule", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-a", "agent-a", { judgedDomains: [ { domain: ".example.com", judge: "wildcard-policy" }, { domain: "api.example.com", judge: "exact-policy" }, @@ -42,43 +42,43 @@ describe("PolicyStore.resolve", () => { "exact-policy": "exact", }, }); - const resolved = store.resolve("agent-a", "api.example.com"); + const resolved = store.resolve("org-a", "agent-a","api.example.com"); expect(resolved?.judgeName).toBe("exact-policy"); }); test("longer wildcard beats shorter wildcard", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-a", "agent-a", { judgedDomains: [ { domain: ".example.com", judge: "short" }, { domain: ".api.example.com", judge: "long" }, ], judges: { short: "short", long: "long" }, }); - expect(store.resolve("agent-a", "foo.api.example.com")?.judgeName).toBe( + expect(store.resolve("org-a", "agent-a","foo.api.example.com")?.judgeName).toBe( "long" ); }); test("resolves a named judge via the `judge` field", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-a", "agent-a", { judgedDomains: [{ domain: "x.com", judge: "strict" }], judges: { strict: "strict policy", default: "default policy" }, }); - const resolved = store.resolve("agent-a", "x.com"); + const resolved = store.resolve("org-a", "agent-a","x.com"); expect(resolved?.judgeName).toBe("strict"); expect(resolved?.policy).toContain("strict policy"); }); test("appends the agent's extraPolicy to the composed prompt", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-a", "agent-a", { judgedDomains: [{ domain: "x.com" }], judges: { default: "skill policy" }, extraPolicy: "Operator adds: never exfiltrate tokens.", }); - const resolved = store.resolve("agent-a", "x.com"); + const resolved = store.resolve("org-a", "agent-a","x.com"); expect(resolved?.policy).toContain("skill policy"); expect(resolved?.policy).toContain( "Operator adds: never exfiltrate tokens." @@ -87,47 +87,47 @@ describe("PolicyStore.resolve", () => { test("returns undefined (fail closed) when the named judge is missing", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-a", "agent-a", { judgedDomains: [{ domain: "x.com", judge: "strict" }], judges: {}, }); - expect(store.resolve("agent-a", "x.com")).toBeUndefined(); + expect(store.resolve("org-a", "agent-a","x.com")).toBeUndefined(); }); test("policyHash is stable across resolve calls", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-a", "agent-a", { judgedDomains: [{ domain: "x.com" }], judges: { default: "p" }, }); - const a = store.resolve("agent-a", "x.com")?.policyHash; - const b = store.resolve("agent-a", "x.com")?.policyHash; + const a = store.resolve("org-a", "agent-a","x.com")?.policyHash; + const b = store.resolve("org-a", "agent-a","x.com")?.policyHash; expect(a).toBe(b!); }); test("policyHash changes when the policy text changes", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-a", "agent-a", { judgedDomains: [{ domain: "x.com" }], judges: { default: "first" }, }); - const a = store.resolve("agent-a", "x.com")?.policyHash; - store.set("agent-a", { + const a = store.resolve("org-a", "agent-a","x.com")?.policyHash; + store.set("org-a", "agent-a", { judgedDomains: [{ domain: "x.com" }], judges: { default: "second" }, }); - const b = store.resolve("agent-a", "x.com")?.policyHash; + const b = store.resolve("org-a", "agent-a","x.com")?.policyHash; expect(a).not.toBe(b); }); test("clear removes the bundle", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-a", "agent-a", { judgedDomains: [{ domain: "x.com" }], judges: { default: "p" }, }); - store.clear("agent-a"); - expect(store.resolve("agent-a", "x.com")).toBeUndefined(); + store.clear("org-a", "agent-a"); + expect(store.resolve("org-a", "agent-a","x.com")).toBeUndefined(); }); }); diff --git a/packages/server/src/gateway/__tests__/proxy-hardening.test.ts b/packages/server/src/gateway/__tests__/proxy-hardening.test.ts index 2c53ae05d..3c13af088 100644 --- a/packages/server/src/gateway/__tests__/proxy-hardening.test.ts +++ b/packages/server/src/gateway/__tests__/proxy-hardening.test.ts @@ -49,11 +49,16 @@ function makeBasicAuth(username: string, password: string): string { return `Basic ${Buffer.from(`${username}:${password}`).toString("base64")}`; } -function createToken(deploymentName: string, agentId?: string): string { +function createToken( + deploymentName: string, + agentId?: string, + organizationId: string = "org-1" +): string { return generateWorkerToken("test-user", "test-conv", deploymentName, { channelId: "test-channel", platform: "test", ...(agentId ? { agentId } : {}), + organizationId, }); } @@ -511,7 +516,7 @@ describe("CRLF injection prevention in judge-provided reason", () => { process.env.WORKER_ALLOWED_DOMAINS = ""; __testOnly.reset(); - policyStore.set("agent-crlf", { + policyStore.set("org-1", "agent-crlf", { judgedDomains: [{ domain: "example.com" }], judges: { default: "test policy" }, }); @@ -606,12 +611,14 @@ describe("CRLF injection prevention in judge-provided reason", () => { describe("VerdictCache — key independence", () => { test("different methods produce different cache keys", () => { const a = VerdictCache.key({ + orgId: "org-1", policyHash: "h", hostname: "example.com", method: "GET", path: "/foo", }); const b = VerdictCache.key({ + orgId: "org-1", policyHash: "h", hostname: "example.com", method: "POST", @@ -622,12 +629,14 @@ describe("VerdictCache — key independence", () => { test("different paths produce different cache keys", () => { const a = VerdictCache.key({ + orgId: "org-1", policyHash: "h", hostname: "example.com", method: "GET", path: "/foo", }); const b = VerdictCache.key({ + orgId: "org-1", policyHash: "h", hostname: "example.com", method: "GET", @@ -638,10 +647,12 @@ describe("VerdictCache — key independence", () => { test("CONNECT (no method/path) and GET / produce different cache keys", () => { const connect = VerdictCache.key({ + orgId: "org-1", policyHash: "h", hostname: "example.com", }); const get = VerdictCache.key({ + orgId: "org-1", policyHash: "h", hostname: "example.com", method: "GET", @@ -652,8 +663,8 @@ describe("VerdictCache — key independence", () => { test("changing policyHash invalidates previously-set entry", () => { const cache = new VerdictCache(60_000, 100); - const key1 = VerdictCache.key({ policyHash: "old-hash", hostname: "x.com" }); - const key2 = VerdictCache.key({ policyHash: "new-hash", hostname: "x.com" }); + const key1 = VerdictCache.key({ orgId: "org-1", policyHash: "old-hash", hostname: "x.com" }); + const key2 = VerdictCache.key({ orgId: "org-1", policyHash: "new-hash", hostname: "x.com" }); cache.set(key1, { verdict: "allow", reason: "ok" }); // key2 (new policy hash) must miss — cache isolates by policyHash @@ -668,18 +679,18 @@ describe("VerdictCache — key independence", () => { // automatically because the key changes. const store = new PolicyStore(); - store.set("agent-x", { + store.set("org-1", "agent-x", { judgedDomains: [{ domain: "example.com" }], judges: { default: "allow reads" }, }); - const without = store.resolve("agent-x", "example.com"); + const without = store.resolve("org-1", "agent-x","example.com"); - store.set("agent-x", { + store.set("org-1", "agent-x", { judgedDomains: [{ domain: "example.com" }], judges: { default: "allow reads" }, extraPolicy: "Never send PII", }); - const withExtra = store.resolve("agent-x", "example.com"); + const withExtra = store.resolve("org-1", "agent-x","example.com"); expect(without).toBeDefined(); expect(withExtra).toBeDefined(); @@ -692,17 +703,17 @@ describe("VerdictCache — key independence", () => { const store = new PolicyStore(); const samePolicy = "allow all reads"; - store.set("agent-a", { + store.set("org-1", "agent-a", { judgedDomains: [{ domain: "example.com" }], judges: { default: samePolicy }, }); - store.set("agent-b", { + store.set("org-1", "agent-b", { judgedDomains: [{ domain: "example.com" }], judges: { default: samePolicy }, }); - const a = store.resolve("agent-a", "example.com"); - const b = store.resolve("agent-b", "example.com"); + const a = store.resolve("org-1", "agent-a", "example.com"); + const b = store.resolve("org-1", "agent-b", "example.com"); expect(a?.policyHash).toBeDefined(); expect(b?.policyHash).toBeDefined(); @@ -771,7 +782,7 @@ describe("CircuitBreaker — state transitions", () => { describe("PolicyStore.resolve — edge cases", () => { test("exact match takes precedence over a wildcard", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-1", "agent-a", { judgedDomains: [ { domain: "api.example.com", judge: "exact-judge" }, { domain: ".example.com", judge: "wildcard-judge" }, @@ -782,13 +793,13 @@ describe("PolicyStore.resolve — edge cases", () => { }, }); - const resolved = store.resolve("agent-a", "api.example.com"); + const resolved = store.resolve("org-1", "agent-a", "api.example.com"); expect(resolved?.judgeName).toBe("exact-judge"); }); test("longer wildcard takes precedence over a shorter wildcard", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-1", "agent-a", { judgedDomains: [ { domain: ".api.example.com", judge: "long-judge" }, { domain: ".example.com", judge: "short-judge" }, @@ -799,96 +810,96 @@ describe("PolicyStore.resolve — edge cases", () => { }, }); - const resolved = store.resolve("agent-a", "v2.api.example.com"); + const resolved = store.resolve("org-1", "agent-a", "v2.api.example.com"); expect(resolved?.judgeName).toBe("long-judge"); }); test("unmatched hostname returns undefined", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-1", "agent-a", { judgedDomains: [{ domain: "example.com" }], judges: { default: "test" }, }); - expect(store.resolve("agent-a", "other.com")).toBeUndefined(); + expect(store.resolve("org-1", "agent-a", "other.com")).toBeUndefined(); }); test("agentId isolation: agent-a rules do not leak to agent-b", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-1", "agent-a", { judgedDomains: [{ domain: "example.com" }], judges: { default: "agent-a policy" }, }); - expect(store.resolve("agent-b", "example.com")).toBeUndefined(); + expect(store.resolve("org-1", "agent-b", "example.com")).toBeUndefined(); }); test("agent with no bundle returns undefined for all hostnames", () => { const store = new PolicyStore(); - expect(store.resolve("no-such-agent", "example.com")).toBeUndefined(); + expect(store.resolve("org-1", "no-such-agent", "example.com")).toBeUndefined(); }); test("rule without explicit judge name resolves to the 'default' judge", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-1", "agent-a", { judgedDomains: [{ domain: "example.com" }], // no `judge` field judges: { default: "default policy text" }, }); - const resolved = store.resolve("agent-a", "example.com"); + const resolved = store.resolve("org-1", "agent-a", "example.com"); expect(resolved?.judgeName).toBe("default"); expect(resolved?.policy).toContain("default policy text"); }); test("rule referencing a missing judge name returns undefined (fail closed)", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-1", "agent-a", { judgedDomains: [{ domain: "example.com", judge: "nonexistent" }], judges: { default: "default policy" }, }); // 'nonexistent' not in judges map → fails closed (returns undefined) - expect(store.resolve("agent-a", "example.com")).toBeUndefined(); + expect(store.resolve("org-1", "agent-a", "example.com")).toBeUndefined(); }); test("clear removes the agent's bundle so resolve returns undefined", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-1", "agent-a", { judgedDomains: [{ domain: "example.com" }], judges: { default: "policy" }, }); - store.clear("agent-a"); - expect(store.resolve("agent-a", "example.com")).toBeUndefined(); + store.clear("org-1", "agent-a"); + expect(store.resolve("org-1", "agent-a", "example.com")).toBeUndefined(); }); test("wildcard .example.com matches example.com itself (root domain)", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-1", "agent-a", { judgedDomains: [{ domain: ".example.com" }], judges: { default: "policy" }, }); - expect(store.resolve("agent-a", "example.com")).toBeDefined(); + expect(store.resolve("org-1", "agent-a", "example.com")).toBeDefined(); }); test("wildcard .example.com does NOT match evilexample.com", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-1", "agent-a", { judgedDomains: [{ domain: ".example.com" }], judges: { default: "policy" }, }); - expect(store.resolve("agent-a", "evilexample.com")).toBeUndefined(); + expect(store.resolve("org-1", "agent-a", "evilexample.com")).toBeUndefined(); }); test("resolve is case-insensitive for hostname", () => { const store = new PolicyStore(); - store.set("agent-a", { + store.set("org-1", "agent-a", { judgedDomains: [{ domain: "example.com" }], judges: { default: "policy" }, }); - expect(store.resolve("agent-a", "EXAMPLE.COM")).toBeDefined(); + expect(store.resolve("org-1", "agent-a", "EXAMPLE.COM")).toBeDefined(); }); }); @@ -906,7 +917,7 @@ describe("EgressJudge — additional behavioral coverage", () => { const judge = new EgressJudge({ client }); await judge.decide( - { agentId: "a", hostname: "example.com" }, + { agentId: "a", organizationId: "org-1", hostname: "example.com" }, rule({ policyHash: "unique-model-1" }) ); expect(capturedModel).toBe("claude-haiku-4-5-20251001"); @@ -923,7 +934,7 @@ describe("EgressJudge — additional behavioral coverage", () => { const judge = new EgressJudge({ client, defaultModel: "default-model" }); await judge.decide( - { agentId: "a", hostname: "example.com" }, + { agentId: "a", organizationId: "org-1", hostname: "example.com" }, rule({ policyHash: "unique-model-2", judgeModel: "override-model" }) ); expect(capturedModel).toBe("override-model"); @@ -945,14 +956,14 @@ describe("EgressJudge — additional behavioral coverage", () => { // Trip the breaker — threshold=1, one failure suffices await judge.decide( - { agentId: "a", hostname: "h1.example.com" }, + { agentId: "a", organizationId: "org-1", hostname: "h1.example.com" }, rule({ policyHash: "brk-coverage" }) ); expect(calls).toBe(1); // Next request: circuit open → short-circuit const d = await judge.decide( - { agentId: "a", hostname: "h2.example.com" }, + { agentId: "a", organizationId: "org-1", hostname: "h2.example.com" }, rule({ policyHash: "brk-coverage" }) ); expect(calls).toBe(1); // no extra call @@ -967,7 +978,7 @@ describe("EgressJudge — additional behavioral coverage", () => { }, }; const judge = new EgressJudge({ client }); - const req = { agentId: "a", hostname: "example.com" }; + const req = { agentId: "a", organizationId: "org-1", hostname: "example.com" }; const r = rule({ policyHash: "p-cache-meta", judgeName: "my-judge" }); await judge.decide(req, r); @@ -992,7 +1003,7 @@ describe("EgressJudge — additional behavioral coverage", () => { }); const d = await judge.decide( - { agentId: "a", hostname: "x.com" }, + { agentId: "a", organizationId: "org-1", hostname: "x.com" }, rule({ policyHash: "single-fail-coverage" }) ); expect(d.verdict).toBe("deny"); diff --git a/packages/server/src/gateway/__tests__/rest-api-hardening.test.ts b/packages/server/src/gateway/__tests__/rest-api-hardening.test.ts index 2aa02927d..435831ef2 100644 --- a/packages/server/src/gateway/__tests__/rest-api-hardening.test.ts +++ b/packages/server/src/gateway/__tests__/rest-api-hardening.test.ts @@ -748,6 +748,8 @@ describe("slack routes: OAuth callback and replay protection", () => { let completeSlackOAuthInstall: ReturnType; let handleSlackAppWebhook: ReturnType; let router: ReturnType; + let app: Hono; + let sessionOrgId: string | null; beforeAll(async () => { await ensurePgliteForGatewayTests(); @@ -772,6 +774,14 @@ describe("slack routes: OAuth callback and replay protection", () => { completeSlackOAuthInstall, handleSlackAppWebhook, } as any); + + sessionOrgId = "org-default"; + app = new Hono(); + app.use("*", async (c, next) => { + if (sessionOrgId !== null) c.set("organizationId" as never, sessionOrgId); + await next(); + }); + app.route("", router); }); afterEach(() => { @@ -788,7 +798,7 @@ describe("slack routes: OAuth callback and replay protection", () => { }); test("callback with missing state parameter returns 400", async () => { - const response = await router.request( + const response = await app.request( "/slack/oauth_callback?code=test-code" // state is absent ); @@ -799,7 +809,7 @@ describe("slack routes: OAuth callback and replay protection", () => { }); test("callback with missing code parameter returns 400", async () => { - const response = await router.request( + const response = await app.request( "/slack/oauth_callback?state=some-state" // code is absent ); @@ -808,7 +818,7 @@ describe("slack routes: OAuth callback and replay protection", () => { }); test("callback with unknown state (not in DB) returns 400 — no install proceeds", async () => { - const response = await router.request( + const response = await app.request( "/slack/oauth_callback?code=test-code&state=does-not-exist-in-db" ); expect(response.status).toBe(400); @@ -833,7 +843,7 @@ describe("slack routes: OAuth callback and replay protection", () => { ) `; - const response = await router.request( + const response = await app.request( "/slack/oauth_callback?code=test-code&state=expired-state" ); expect(response.status).toBe(400); @@ -850,20 +860,21 @@ describe("slack routes: OAuth callback and replay protection", () => { ${sql.json({ createdAt: Date.now(), redirectUri: "https://gateway.example.com/slack/oauth_callback", + organizationId: "org-default", })}, ${new Date(Date.now() + 600_000)} ) `; // First request should succeed - const first = await router.request( + const first = await app.request( "/slack/oauth_callback?code=test-code&state=one-time-state" ); expect(first.status).toBe(200); expect(completeSlackOAuthInstall).toHaveBeenCalledTimes(1); // Second request with the same state must fail — state already consumed - const second = await router.request( + const second = await app.request( "/slack/oauth_callback?code=test-code&state=one-time-state" ); expect(second.status).toBe(400); @@ -873,7 +884,7 @@ describe("slack routes: OAuth callback and replay protection", () => { test("Slack install returns 503 when SLACK_CLIENT_ID is not set", async () => { delete process.env.SLACK_CLIENT_ID; - const response = await router.request("/slack/install"); + const response = await app.request("/slack/install"); expect(response.status).toBe(503); const body = await response.text(); expect(body).toContain("not configured"); diff --git a/packages/server/src/gateway/__tests__/secret-proxy.test.ts b/packages/server/src/gateway/__tests__/secret-proxy.test.ts index 86a421cf0..0da8af361 100644 --- a/packages/server/src/gateway/__tests__/secret-proxy.test.ts +++ b/packages/server/src/gateway/__tests__/secret-proxy.test.ts @@ -3,6 +3,7 @@ import { createBuiltinSecretRef } from "@lobu/core"; import { __resetPlaceholderCacheForTests, generatePlaceholder, + lookupPlaceholderMapping, SecretProxy, type SecretMapping, storeSecretMapping, @@ -91,6 +92,62 @@ describe("generatePlaceholder", () => { }); }); +describe("lookupPlaceholderMapping org scoping", () => { + beforeEach(() => { + __resetPlaceholderCacheForTests(); + }); + + test("returns the mapping when no expected org is supplied", () => { + const placeholder = generatePlaceholder( + "agent-1", + "API_KEY", + createBuiltinSecretRef("deployments/agent-1/API_KEY"), + "deploy-1", + { organizationId: "org-a" } + ); + const mapping = lookupPlaceholderMapping(placeholder); + expect(mapping?.agentId).toBe("agent-1"); + expect(mapping?.organizationId).toBe("org-a"); + }); + + test("returns the mapping when expected org matches", () => { + const placeholder = generatePlaceholder( + "agent-1", + "API_KEY", + createBuiltinSecretRef("deployments/agent-1/API_KEY"), + "deploy-1", + { organizationId: "org-a" } + ); + const mapping = lookupPlaceholderMapping(placeholder, "org-a"); + expect(mapping?.agentId).toBe("agent-1"); + }); + + test("returns null when expected org mismatches the mapping's org", () => { + const placeholder = generatePlaceholder( + "agent-1", + "API_KEY", + createBuiltinSecretRef("deployments/agent-1/API_KEY"), + "deploy-1", + { organizationId: "org-a" } + ); + // org-b tries to claim a placeholder minted for org-a — must fail closed. + const mapping = lookupPlaceholderMapping(placeholder, "org-b"); + expect(mapping).toBeNull(); + }); + + test("falls through when mapping has no org tag (legacy)", () => { + const placeholder = generatePlaceholder( + "agent-1", + "API_KEY", + createBuiltinSecretRef("deployments/agent-1/API_KEY"), + "deploy-1" + ); + // Mapping has no org → caller's expectation isn't enforceable. + const mapping = lookupPlaceholderMapping(placeholder, "org-a"); + expect(mapping?.agentId).toBe("agent-1"); + }); +}); + describe("SecretProxy user-scoped provider routing", () => { test("passes user context into provider credential lookup", async () => { const proxy = new SecretProxy( diff --git a/packages/server/src/gateway/__tests__/slack-routes.test.ts b/packages/server/src/gateway/__tests__/slack-routes.test.ts index 4ca578f66..1efd85053 100644 --- a/packages/server/src/gateway/__tests__/slack-routes.test.ts +++ b/packages/server/src/gateway/__tests__/slack-routes.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeAll, beforeEach, describe, expect, mock, test } from "bun:test"; +import { Hono } from "hono"; import { getDb } from "../../db/client.js"; import { createSlackRoutes } from "../routes/public/slack.js"; import { ensurePgliteForGatewayTests, resetTestDatabase } from "./helpers/db-setup.js"; @@ -10,6 +11,12 @@ describe("slack routes", () => { let completeSlackOAuthInstall: ReturnType; let handleSlackAppWebhook: ReturnType; let router: ReturnType; + let app: Hono; + // Per-test org id injected into the Hono context — mirrors what + // `lobuApp.use('*', ...)` sets in production (see lobu/gateway.ts). The + // /slack/install + /slack/oauth_callback handlers require a non-empty + // value to scope install state to the initiating tenant. + let sessionOrgId: string | null; beforeAll(async () => { await ensurePgliteForGatewayTests(); @@ -37,6 +44,14 @@ describe("slack routes", () => { completeSlackOAuthInstall, handleSlackAppWebhook, } as any); + + sessionOrgId = "org-default"; + app = new Hono(); + app.use("*", async (c, next) => { + if (sessionOrgId !== null) c.set("organizationId" as never, sessionOrgId); + await next(); + }); + app.route("", router); }); afterEach(() => { @@ -54,7 +69,7 @@ describe("slack routes", () => { }); test("GET /slack/install redirects to Slack OAuth and stores state", async () => { - const response = await router.request("/slack/install"); + const response = await app.request("/slack/install"); expect(response.status).toBe(302); @@ -83,11 +98,20 @@ describe("slack routes", () => { expect(payload.redirectUri).toBe( "https://gateway.example.com/slack/oauth_callback" ); + expect(payload.organizationId).toBe("org-default"); expect(typeof payload.createdAt).toBe("number"); }); + test("GET /slack/install rejects when no session org is bound", async () => { + sessionOrgId = null; + const response = await app.request("/slack/install"); + const body = await response.text(); + expect(response.status).toBe(401); + expect(body).toContain("Sign in to an organization"); + }); + test("GET /slack/oauth_callback rejects invalid state", async () => { - const response = await router.request( + const response = await app.request( "/slack/oauth_callback?code=test-code&state=missing" ); const body = await response.text(); @@ -109,12 +133,13 @@ describe("slack routes", () => { ${sql.json({ createdAt: Date.now(), redirectUri: "https://gateway.example.com/slack/oauth_callback", + organizationId: "org-default", })}, ${expiresAt} ) `; - const response = await router.request( + const response = await app.request( "/slack/oauth_callback?code=test-code&state=test-state" ); const body = await response.text(); @@ -133,8 +158,42 @@ describe("slack routes", () => { expect(remaining.length).toBe(0); }); + test("GET /slack/oauth_callback rejects when callback session org differs from install state", async () => { + const sql = getDb(); + const expiresAt = new Date(Date.now() + 600_000); + await sql` + INSERT INTO oauth_states (id, scope, payload, expires_at) + VALUES ( + 'cross-org-state', + 'slack:oauth:state', + ${sql.json({ + createdAt: Date.now(), + redirectUri: "https://gateway.example.com/slack/oauth_callback", + organizationId: "org-a", + })}, + ${expiresAt} + ) + `; + // Caller signs in to org-b — must be rejected with 403. + sessionOrgId = "org-b"; + const response = await app.request( + "/slack/oauth_callback?code=test-code&state=cross-org-state" + ); + const body = await response.text(); + expect(response.status).toBe(403); + expect(body).toContain("different organization"); + expect(completeSlackOAuthInstall).not.toHaveBeenCalled(); + // State is preserved for the legitimate caller to retry (peek-before- + // consume). The previous behavior burned the row on every failed + // org check and forced the user to restart the OAuth flow. + const remaining = await sql` + SELECT 1 FROM oauth_states WHERE id = 'cross-org-state' + `; + expect(remaining.length).toBe(1); + }); + test("POST /slack/events forwards requests to the chat manager", async () => { - const response = await router.request("/slack/events", { + const response = await app.request("/slack/events", { method: "POST", headers: { "content-type": "application/json", diff --git a/packages/server/src/gateway/auth/oauth/state-store.ts b/packages/server/src/gateway/auth/oauth/state-store.ts index 72fb7affa..e05fd58da 100644 --- a/packages/server/src/gateway/auth/oauth/state-store.ts +++ b/packages/server/src/gateway/auth/oauth/state-store.ts @@ -93,6 +93,29 @@ export class OAuthStateStore { return stateData; } + /** + * Read the state payload without consuming it. Used when the caller needs + * to validate side-channel context (e.g. that the callback session's org + * matches the state's org) before atomically burning the install link — + * without this, any failed-validation hit would force the user to restart + * the OAuth flow even though the state is still otherwise valid. + * + * The row is left intact; callers must call `consume()` themselves once + * validation passes (or rely on the TTL sweep if validation fails). + */ + async peek(state: string): Promise<(T & { createdAt: number }) | null> { + const sql = getDb(); + const rows = await sql` + SELECT payload FROM oauth_states + WHERE id = ${state} + AND scope = ${this.keyPrefix} + AND expires_at > now() + LIMIT 1 + `; + if (rows.length === 0) return null; + return (rows[0] as { payload: T & { createdAt: number } }).payload; + } + /** * Generate a cryptographically secure random state string. */ @@ -135,6 +158,13 @@ export function createOAuthStateStore( interface SlackInstallStateData { redirectUri: string; + /** + * Active org of the session that initiated the install. The callback + * verifies the callback-side session's active org matches; mismatch + * rejects the install so an OAuth link minted under org A's session can + * never plant a connection into org B. + */ + organizationId: string; } export function createSlackInstallStateStore(): OAuthStateStore { diff --git a/packages/server/src/gateway/connections/chat-instance-manager.ts b/packages/server/src/gateway/connections/chat-instance-manager.ts index 068a927dc..ec7172fe2 100644 --- a/packages/server/src/gateway/connections/chat-instance-manager.ts +++ b/packages/server/src/gateway/connections/chat-instance-manager.ts @@ -93,6 +93,32 @@ function configsEqual( const logger = createLogger("chat-instance-manager"); +/** + * Read `LOBU_CLOUD_MODE` from the env. Truthy values (`1`, `true`, `yes`, + * case-insensitive) enable cloud-mode guardrails that don't apply to + * self-hosters running the same gateway in a single-tenant install. + * + * Re-read on every call so a process running in a test harness can flip + * the flag without restart. Embedded gateways check it on every + * connection-create, which is cold-path enough to skip caching. + */ +function isCloudMode(): boolean { + const raw = process.env.LOBU_CLOUD_MODE; + if (!raw) return false; + const v = raw.trim().toLowerCase(); + return v === "1" || v === "true" || v === "yes"; +} + +/** + * `mode: "polling"` is the only config that forces long-polling regardless + * of whether the gateway has a public webhook URL. `mode: "auto"` resolves + * to webhook on cloud (publicGatewayUrl is always set there), so it's fine + * to allow. Only the explicit polling opt-in is rejected in cloud. + */ +function isPollingTelegramMode(config: { mode?: string }): boolean { + return config.mode === "polling"; +} + const ADAPTER_FACTORIES: Record Promise> = { telegram: async (c) => (await import("@chat-adapter/telegram")).createTelegramAdapter(c), @@ -155,6 +181,46 @@ export class ChatInstanceManager { // errored so an operator can repair or remove it. const connection = storedToPlatform(stored); + // Apply the cloud-mode polling guard before startInstance — otherwise + // a previously-persisted `mode: "polling"` Telegram row would silently + // start at boot and bypass the create-time rejection added in + // `addConnection()`. Mark the row errored so an operator notices. + if ( + connection.status === "active" && + connection.platform === "telegram" && + isCloudMode() && + isPollingTelegramMode(connection.config as { mode?: string }) + ) { + const message = + "Polling mode is not supported in Lobu Cloud — use webhook mode, or self-host."; + logger.warn( + { id: connection.id, agentId: connection.agentId }, + `Refusing to boot Telegram polling connection in cloud mode: ${message}` + ); + // Self-bind the connection's owning org so the PostgreSQL-backed + // store's per-tenant predicate is satisfied — boot has no HTTP + // request and thus no ALS org context. + try { + const orgId = connection.organizationId; + const markErrored = () => + this.connectionStore.updateConnection(connection.id, { + status: "error", + errorMessage: message, + }); + if (orgId) { + await orgContext.run({ organizationId: orgId }, markErrored); + } else { + await markErrored(); + } + } catch (markErr) { + logger.error( + { id: connection.id, error: String(markErr) }, + "Failed to mark Telegram polling connection as errored" + ); + } + continue; + } + try { if (connection.status === "active") { // Boot runs without an HTTP request, so AsyncLocalStorage has @@ -212,6 +278,24 @@ export class ChatInstanceManager { ); } + // `mode: "polling"` long-polls Telegram's edge from the gateway pod and + // bypasses the per-tenant webhook URL we issue. On Lobu Cloud — where + // the same gateway serves many tenants — that means one org's connection + // can starve every other tenant's webhook delivery (and produces no + // audit trail tied to the inbound HTTP request). Refuse the explicit + // polling opt-in up front; self-hosters (LOBU_CLOUD_MODE unset/0) still + // get polling for tunnel-less dev. `mode: "auto"` is fine — it resolves + // to webhook whenever `publicGatewayUrl` is set, which cloud always has. + if ( + platform === "telegram" && + isCloudMode() && + isPollingTelegramMode(config as { mode?: string }) + ) { + throw new Error( + "Polling mode is not supported in Lobu Cloud — use webhook mode, or self-host." + ); + } + const id = stableId ?? randomUUID().replace(/-/g, "").slice(0, 16); const now = Date.now(); const organizationId = tryGetOrgId() ?? undefined; diff --git a/packages/server/src/gateway/orchestration/base-deployment-manager.ts b/packages/server/src/gateway/orchestration/base-deployment-manager.ts index e2dbf6d5e..03249de33 100644 --- a/packages/server/src/gateway/orchestration/base-deployment-manager.ts +++ b/packages/server/src/gateway/orchestration/base-deployment-manager.ts @@ -418,7 +418,19 @@ export abstract class BaseDeploymentManager { deploymentName?: string ): void { const agentId = messageData.agentId; - if (!this.policyStore || !agentId) return; + const organizationId = messageData.organizationId; + // PolicyStore is keyed by `(orgId, agentId)` to prevent cross-tenant + // policy clobbering — refuse to sync without an org id rather than + // collapsing into a shared bucket. + if (!this.policyStore || !agentId || !organizationId) { + if (!organizationId && agentId) { + logger.warn( + { agentId, deploymentName }, + "Skipping egress policy sync — message has no organizationId" + ); + } + return; + } const bundle = buildPolicyBundle({ judgedDomains: messageData.networkConfig?.judgedDomains, @@ -426,20 +438,21 @@ export abstract class BaseDeploymentManager { egressConfig: messageData.egressConfig, }); if (bundle) { - this.policyStore.set(agentId, bundle); + this.policyStore.set(organizationId, agentId, bundle); if (deploymentName) { logger.info( `Synced egress judge policy for ${deploymentName}: ${bundle.judgedDomains.length} rule(s), ${Object.keys(bundle.judges).length} judge(s)` ); } else { logger.debug("Synced egress judge policy", { + organizationId, agentId, rules: bundle.judgedDomains.length, judges: Object.keys(bundle.judges).length, }); } } else { - this.policyStore.clear(agentId); + this.policyStore.clear(organizationId, agentId); } } @@ -762,7 +775,10 @@ export abstract class BaseDeploymentManager { key, secretRef, deploymentName, - SECRET_PLACEHOLDER_TTL_SECONDS + { + ttlSeconds: SECRET_PLACEHOLDER_TTL_SECONDS, + organizationId: context?.organizationId, + } ); envVars[key] = placeholder; hasSecrets = true; @@ -819,6 +835,7 @@ export abstract class BaseDeploymentManager { teamId, platform, agentId, + organizationId: validated.organizationId, connectionId: typeof platformMetadata?.connectionId === "string" ? platformMetadata.connectionId diff --git a/packages/server/src/gateway/permissions/__tests__/policy-store.test.ts b/packages/server/src/gateway/permissions/__tests__/policy-store.test.ts index e1aa56105..472b17aa9 100644 --- a/packages/server/src/gateway/permissions/__tests__/policy-store.test.ts +++ b/packages/server/src/gateway/permissions/__tests__/policy-store.test.ts @@ -28,34 +28,34 @@ import { describe("PolicyStore.resolve", () => { test("returns undefined for unknown agent", () => { const store = new PolicyStore(); - expect(store.resolve("unknown-agent", "example.com")).toBeUndefined(); + expect(store.resolve("org-1", "unknown-agent", "example.com")).toBeUndefined(); }); test("returns undefined when agent has no judged domains", () => { const store = new PolicyStore(); - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [], judges: { default: "Allow reads only." }, }); - expect(store.resolve("agent-1", "example.com")).toBeUndefined(); + expect(store.resolve("org-1", "agent-1", "example.com")).toBeUndefined(); }); test("returns undefined when hostname does not match any rule", () => { const store = new PolicyStore(); - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [{ domain: "api.example.com" }], judges: { default: "Allow reads only." }, }); - expect(store.resolve("agent-1", "other.com")).toBeUndefined(); + expect(store.resolve("org-1", "agent-1", "other.com")).toBeUndefined(); }); test("exact domain match returns the resolved rule", () => { const store = new PolicyStore(); - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [{ domain: "api.example.com" }], judges: { default: "Allow reads only." }, }); - const result = store.resolve("agent-1", "api.example.com"); + const result = store.resolve("org-1", "agent-1", "api.example.com"); expect(result).not.toBeUndefined(); expect(result!.judgeName).toBe("default"); expect(result!.policy).toBe("Allow reads only."); @@ -63,7 +63,7 @@ describe("PolicyStore.resolve", () => { test("exact match takes priority over wildcard", () => { const store = new PolicyStore(); - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [ { domain: ".example.com", judge: "wildcard-judge" }, { domain: "api.example.com", judge: "exact-judge" }, @@ -73,35 +73,35 @@ describe("PolicyStore.resolve", () => { "exact-judge": "Exact policy.", }, }); - const result = store.resolve("agent-1", "api.example.com"); + const result = store.resolve("org-1", "agent-1", "api.example.com"); expect(result!.judgeName).toBe("exact-judge"); expect(result!.policy).toBe("Exact policy."); }); test("wildcard .example.com matches sub.example.com", () => { const store = new PolicyStore(); - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [{ domain: ".example.com" }], judges: { default: "Wildcard policy." }, }); - const result = store.resolve("agent-1", "sub.example.com"); + const result = store.resolve("org-1", "agent-1", "sub.example.com"); expect(result).not.toBeUndefined(); expect(result!.policy).toBe("Wildcard policy."); }); test("wildcard .example.com matches example.com root", () => { const store = new PolicyStore(); - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [{ domain: ".example.com" }], judges: { default: "Root wildcard policy." }, }); - const result = store.resolve("agent-1", "example.com"); + const result = store.resolve("org-1", "agent-1", "example.com"); expect(result).not.toBeUndefined(); }); test("longer wildcard beats shorter wildcard", () => { const store = new PolicyStore(); - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [ { domain: ".example.com", judge: "short" }, { domain: ".api.example.com", judge: "long" }, @@ -112,51 +112,51 @@ describe("PolicyStore.resolve", () => { }, }); // ".api.example.com" is longer and should match "v2.api.example.com" - const result = store.resolve("agent-1", "v2.api.example.com"); + const result = store.resolve("org-1", "agent-1", "v2.api.example.com"); expect(result!.judgeName).toBe("long"); }); test("wildcard does not match unrelated domain", () => { const store = new PolicyStore(); - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [{ domain: ".example.com" }], judges: { default: "Example only." }, }); - expect(store.resolve("agent-1", "evil.com")).toBeUndefined(); - expect(store.resolve("agent-1", "notexample.com")).toBeUndefined(); + expect(store.resolve("org-1", "agent-1", "evil.com")).toBeUndefined(); + expect(store.resolve("org-1", "agent-1", "notexample.com")).toBeUndefined(); }); test("named judge missing → undefined (fails closed)", () => { // Rule references a judge name not in the judges map. const store = new PolicyStore(); - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [{ domain: "api.example.com", judge: "missing-judge" }], judges: { default: "Default judge." }, // 'missing-judge' is absent }); // Should return undefined rather than crash or use the wrong judge. - const result = store.resolve("agent-1", "api.example.com"); + const result = store.resolve("org-1", "agent-1", "api.example.com"); expect(result).toBeUndefined(); }); test("default judge name used when rule omits judge field", () => { const store = new PolicyStore(); - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [{ domain: "api.example.com" }], // no judge field judges: { default: "Default judge text." }, }); - const result = store.resolve("agent-1", "api.example.com"); + const result = store.resolve("org-1", "agent-1", "api.example.com"); expect(result!.judgeName).toBe("default"); }); test("clear removes agent policy — resolve returns undefined afterwards", () => { const store = new PolicyStore(); - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [{ domain: "api.example.com" }], judges: { default: "Allow." }, }); - expect(store.resolve("agent-1", "api.example.com")).not.toBeUndefined(); - store.clear("agent-1"); - expect(store.resolve("agent-1", "api.example.com")).toBeUndefined(); + expect(store.resolve("org-1", "agent-1", "api.example.com")).not.toBeUndefined(); + store.clear("org-1", "agent-1"); + expect(store.resolve("org-1", "agent-1", "api.example.com")).toBeUndefined(); }); }); @@ -169,43 +169,43 @@ describe("PolicyStore — policyHash", () => { judgedDomains: [{ domain: "api.example.com" }], judges: { default: "Allow only GET." }, }; - store.set("agent-1", bundle); - const h1 = store.resolve("agent-1", "api.example.com")!.policyHash; + store.set("org-1", "agent-1", bundle); + const h1 = store.resolve("org-1", "agent-1", "api.example.com")!.policyHash; // Re-set with same bundle (simulates reload). - store.set("agent-1", bundle); - const h2 = store.resolve("agent-1", "api.example.com")!.policyHash; + store.set("org-1", "agent-1", bundle); + const h2 = store.resolve("org-1", "agent-1", "api.example.com")!.policyHash; expect(h1).toBe(h2); }); test("policyHash differs when extraPolicy changes", () => { const store = new PolicyStore(); - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [{ domain: "api.example.com" }], judges: { default: "Base policy." }, extraPolicy: "Extra A.", }); - const hashA = store.resolve("agent-1", "api.example.com")!.policyHash; + const hashA = store.resolve("org-1", "agent-1", "api.example.com")!.policyHash; - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [{ domain: "api.example.com" }], judges: { default: "Base policy." }, extraPolicy: "Extra B.", }); - const hashB = store.resolve("agent-1", "api.example.com")!.policyHash; + const hashB = store.resolve("org-1", "agent-1", "api.example.com")!.policyHash; expect(hashA).not.toBe(hashB); }); test("extraPolicy is appended to composed policy text", () => { const store = new PolicyStore(); - store.set("agent-1", { + store.set("org-1", "agent-1", { judgedDomains: [{ domain: "api.example.com" }], judges: { default: "Base policy." }, extraPolicy: "Additional constraint.", }); - const result = store.resolve("agent-1", "api.example.com")!; + const result = store.resolve("org-1", "agent-1", "api.example.com")!; expect(result.policy).toContain("Base policy."); expect(result.policy).toContain("Additional constraint."); }); diff --git a/packages/server/src/gateway/permissions/policy-store.ts b/packages/server/src/gateway/permissions/policy-store.ts index f059648b4..3112e1830 100644 --- a/packages/server/src/gateway/permissions/policy-store.ts +++ b/packages/server/src/gateway/permissions/policy-store.ts @@ -49,14 +49,28 @@ interface PreparedBundle { * * Composed policy text and its hash are computed once at `set()` time and * reused on every `resolve()` so the hot path does no SHA256 work. + * + * Keyed by `(organizationId, agentId)`. Agent ids are per-org-unique on + * paper but bugs in upstream code (or a malicious sync from another tenant) + * must never overwrite policy across orgs — that turns the verdict-cache + * org scoping into theatre. The key here is the safety net. */ export class PolicyStore { private readonly policies = new Map(); - set(agentId: string, bundle: JudgePolicyBundle): void { - const prepared = prepareBundle(agentId, bundle); - this.policies.set(agentId, prepared); + private static composeKey(organizationId: string, agentId: string): string { + return `${organizationId}|${agentId}`; + } + + set( + organizationId: string, + agentId: string, + bundle: JudgePolicyBundle + ): void { + const prepared = prepareBundle(organizationId, agentId, bundle); + this.policies.set(PolicyStore.composeKey(organizationId, agentId), prepared); logger.debug("Set egress policy bundle", { + organizationId, agentId, domains: prepared.judgedDomains.length, judges: Object.keys(prepared.preparedJudges).length, @@ -64,17 +78,24 @@ export class PolicyStore { }); } - clear(agentId: string): void { - this.policies.delete(agentId); + clear(organizationId: string, agentId: string): void { + this.policies.delete(PolicyStore.composeKey(organizationId, agentId)); } /** - * Resolve a judge rule for a hostname under an agent. Rules use the same - * domain pattern format as allow/deny lists. Exact match is preferred; - * wildcard patterns (`.example.com`) match the root plus any subdomain. + * Resolve a judge rule for a hostname under an `(org, agent)` pair. + * Rules use the same domain pattern format as allow/deny lists. Exact + * match is preferred; wildcard patterns (`.example.com`) match the root + * plus any subdomain. */ - resolve(agentId: string, hostname: string): ResolvedJudgeRule | undefined { - const prepared = this.policies.get(agentId); + resolve( + organizationId: string, + agentId: string, + hostname: string + ): ResolvedJudgeRule | undefined { + const prepared = this.policies.get( + PolicyStore.composeKey(organizationId, agentId) + ); if (!prepared || prepared.judgedDomains.length === 0) { return undefined; } @@ -89,7 +110,7 @@ export class PolicyStore { if (!judge) { logger.warn( "Judge rule matched but named policy not found — failing closed", - { agentId, hostname, judgeName } + { organizationId, agentId, hostname, judgeName } ); return undefined; } @@ -143,6 +164,7 @@ export function buildPolicyBundle(input: { } function prepareBundle( + organizationId: string, agentId: string, bundle: JudgePolicyBundle ): PreparedBundle { @@ -153,7 +175,7 @@ function prepareBundle( : rawPolicy.trim(); preparedJudges[name] = { policy: composed, - policyHash: hashPolicy(agentId, name, composed), + policyHash: hashPolicy(organizationId, agentId, name, composed), }; } return { @@ -188,13 +210,14 @@ function findMatchingRule( } function hashPolicy( + organizationId: string, agentId: string, judgeName: string, policy: string ): string { return crypto .createHash("sha256") - .update(`${agentId} ${judgeName} ${policy}`) + .update(`${organizationId} ${agentId} ${judgeName} ${policy}`) .digest("hex") .slice(0, 16); } diff --git a/packages/server/src/gateway/proxy/egress-judge/cache.ts b/packages/server/src/gateway/proxy/egress-judge/cache.ts index 80b486cfa..8552d3278 100644 --- a/packages/server/src/gateway/proxy/egress-judge/cache.ts +++ b/packages/server/src/gateway/proxy/egress-judge/cache.ts @@ -8,9 +8,11 @@ interface Entry { } /** - * Small LRU with absolute TTL. Keyed by `(policyHash, request signature)`, - * so a policy edit invalidates prior verdicts automatically — the hash - * changes, the cache misses. + * Small LRU with absolute TTL. Keyed by `(orgId, policyHash, request + * signature)` — the orgId scopes verdicts to a tenant so org A's "allow" + * for `api.example.com` cannot satisfy org B's identical request, even when + * the composed policy text hashes the same. A policy edit invalidates + * prior verdicts automatically — the hash changes, the cache misses. * * Scale budget: expected to sit in the low thousands of entries. When the * map grows past `maxEntries`, the oldest-touched key is evicted. @@ -25,12 +27,14 @@ export class VerdictCache { ) {} static key(parts: { + orgId: string; policyHash: string; hostname: string; method?: string; path?: string; }): string { return [ + parts.orgId, parts.policyHash, parts.hostname.toLowerCase(), parts.method?.toUpperCase() ?? "", diff --git a/packages/server/src/gateway/proxy/egress-judge/judge.ts b/packages/server/src/gateway/proxy/egress-judge/judge.ts index 89f8d111c..87f087da6 100644 --- a/packages/server/src/gateway/proxy/egress-judge/judge.ts +++ b/packages/server/src/gateway/proxy/egress-judge/judge.ts @@ -103,6 +103,7 @@ export class EgressJudge { rule: ResolvedJudgeRule ): Promise { const cacheKey = VerdictCache.key({ + orgId: request.organizationId, policyHash: rule.policyHash, hostname: request.hostname, method: request.method, diff --git a/packages/server/src/gateway/proxy/egress-judge/types.ts b/packages/server/src/gateway/proxy/egress-judge/types.ts index c1d4d4706..2b42edf21 100644 --- a/packages/server/src/gateway/proxy/egress-judge/types.ts +++ b/packages/server/src/gateway/proxy/egress-judge/types.ts @@ -6,6 +6,13 @@ */ export interface JudgeRequest { agentId: string; + /** + * Owning organization of the worker making this request. The verdict + * cache is scoped by `orgId` so verdicts cannot leak across tenants — + * see `VerdictCache.key`. Empty string is treated as an "unknown org" + * bucket of its own (still isolated from real orgs). + */ + organizationId: string; hostname: string; method?: string; path?: string; diff --git a/packages/server/src/gateway/proxy/http-proxy.ts b/packages/server/src/gateway/proxy/http-proxy.ts index 92bf8fd38..f2d4648b3 100644 --- a/packages/server/src/gateway/proxy/http-proxy.ts +++ b/packages/server/src/gateway/proxy/http-proxy.ts @@ -87,6 +87,16 @@ export function setProxyPolicyStore(store: PolicyStore): void { } } +/** + * Set the grant store the proxy consults when resolving per-agent + * allow/deny grants. Production wires this from `CoreServices`; tests use + * it to install a mock or a fresh DB-backed store so the cross-org leakage + * fixed in this PR can be exercised end-to-end. + */ +export function setProxyGrantStore(store: GrantStore): void { + proxyGrantStore = store; +} + /** * Replace the lazy {@link EgressJudge} — tests inject a fake client here * so the proxy can be exercised end-to-end without hitting a real model. @@ -130,6 +140,7 @@ interface AccessDecision { async function checkDomainAccess( hostname: string, agentId: string | undefined, + organizationId: string | undefined, requestContext?: { method?: string; path?: string } ): Promise { const global = getGlobalConfig(); @@ -150,9 +161,17 @@ async function checkDomainAccess( ); if (globallyAllowed) { - // Even if globally allowed, a per-agent deny grant can override + // Even if globally allowed, a per-agent deny grant can override. + // Pass `organizationId` explicitly — `GrantStore` falls back to the ALS + // org context when omitted, but the raw Node HTTP proxy never sets ALS + // and the WHERE clause would drop its `organization_id` predicate, + // leaking grants/denies across tenants that share an agent id. if (proxyGrantStore && agentId) { - const denied = await proxyGrantStore.isDenied(agentId, hostname); + const denied = await proxyGrantStore.isDenied( + agentId, + hostname, + organizationId + ); if (denied) { logger.debug(`Domain ${hostname} denied via grant (agent: ${agentId})`); return { allowed: false, source: "grant" }; @@ -163,7 +182,11 @@ async function checkDomainAccess( // Not globally allowed — check grant store for per-agent access if (proxyGrantStore && agentId) { - const granted = await proxyGrantStore.hasGrant(agentId, hostname); + const granted = await proxyGrantStore.hasGrant( + agentId, + hostname, + organizationId + ); if (granted) { logger.debug(`Domain ${hostname} allowed via grant (agent: ${agentId})`); return { allowed: true, source: "grant" }; @@ -171,12 +194,16 @@ async function checkDomainAccess( } // Fall through to the LLM egress judge when a matching rule exists. - if (proxyPolicyStore && proxyEgressJudge && agentId) { - const rule = proxyPolicyStore.resolve(agentId, hostname); + // PolicyStore is keyed by `(orgId, agentId)`; without an org id we refuse + // to consult it — falling through to an unkeyed lookup would let another + // tenant's policy decide our verdict. + if (proxyPolicyStore && proxyEgressJudge && agentId && organizationId) { + const rule = proxyPolicyStore.resolve(organizationId, agentId, hostname); if (rule) { const decision = await proxyEgressJudge.decide( { agentId, + organizationId, hostname, method: requestContext?.method, path: requestContext?.path, @@ -742,7 +769,11 @@ async function handleConnect( // Check domain access: global config → grant store → LLM egress judge. // TLS CONNECT tunneling means we cannot see the method or path — the // judge decides on hostname alone. - const decision = await checkDomainAccess(hostname, tokenData.agentId); + const decision = await checkDomainAccess( + hostname, + tokenData.agentId, + tokenData.organizationId + ); logAccessDecision( "CONNECT", hostname, @@ -887,7 +918,7 @@ async function handleProxyRequest( // Check domain access: global config → grant store → LLM egress judge. // Plain HTTP: method and path are visible and are passed through to the // judge so policies can reason about specific endpoints. - const decision = await checkDomainAccess(hostname, tokenData.agentId, { + const decision = await checkDomainAccess(hostname, tokenData.agentId, tokenData.organizationId, { method: req.method, path: parsedUrl.pathname + parsedUrl.search, }); diff --git a/packages/server/src/gateway/proxy/secret-proxy.ts b/packages/server/src/gateway/proxy/secret-proxy.ts index 789ae8451..82551a139 100644 --- a/packages/server/src/gateway/proxy/secret-proxy.ts +++ b/packages/server/src/gateway/proxy/secret-proxy.ts @@ -1,4 +1,4 @@ -import { createLogger, type SecretRef } from "@lobu/core"; +import { createLogger, type SecretRef, verifyWorkerToken } from "@lobu/core"; import type { Context } from "hono"; import { Hono } from "hono"; import type { AuthProfilesManager } from "../auth/settings/auth-profiles-manager.js"; @@ -7,6 +7,17 @@ import type { ProviderUpstreamConfig } from "../modules/module-system.js"; import type { SecretStore } from "../secrets/index.js"; import { getClientIp } from "../utils/rate-limiter.js"; +/** + * Caller-supplied resolver: agentId → orgId of the agent's owning org. + * + * The proxy needs an independent source of the caller's org to compare + * against `SecretMapping.organizationId` — without it the org-scoping + * guard on `lookupPlaceholderMapping` has nothing to enforce against and + * collapses to dead code. The deployment manager wires a DB-backed + * resolver with a small TTL cache at boot. + */ +export type AgentOrgResolver = (agentId: string) => Promise; + const logger = createLogger("secret-proxy"); const PLACEHOLDER_PREFIX = "lobu_secret_"; @@ -163,6 +174,42 @@ class PlaceholderCache { /** Module-level singleton: gateway has one secret proxy and one mapping cache. */ const placeholderCache = new PlaceholderCache(); +/** + * Resolve a placeholder string (`lobu_secret_` or a prefixed variant) + * to its stored {@link SecretMapping}. Returns `null` if the placeholder is + * malformed, expired, missing, or — when `expectedOrganizationId` is supplied + * — pinned to a different tenant. + * + * Exported for tests so the org-scoping guard can be exercised without + * spinning up the full HTTP proxy. + */ +export function lookupPlaceholderMapping( + placeholder: string, + expectedOrganizationId?: string +): SecretMapping | null { + const prefixIdx = placeholder.indexOf(PLACEHOLDER_PREFIX); + if (prefixIdx === -1) return null; + const uuid = placeholder.slice(prefixIdx + PLACEHOLDER_PREFIX.length); + const mapping = placeholderCache.get(uuid); + if (!mapping) return null; + if ( + expectedOrganizationId && + mapping.organizationId && + mapping.organizationId !== expectedOrganizationId + ) { + logger.warn( + { + mappingAgentId: mapping.agentId, + mappingOrg: mapping.organizationId, + expectedOrg: expectedOrganizationId, + }, + "Placeholder mapping rejected: organization mismatch" + ); + return null; + } + return mapping; +} + function safeDecodePathSegment(value: string | undefined): string | undefined { if (!value) return undefined; try { @@ -174,6 +221,15 @@ function safeDecodePathSegment(value: string | undefined): string | undefined { export interface SecretMapping { agentId: string; + /** + * Owning organization of the agent the placeholder was minted for. + * `lookupPlaceholderMapping()` rejects the lookup when the caller's + * org doesn't match — defense-in-depth against a compromised worker + * presenting another tenant's placeholder. Optional only because + * older mappings minted before the org-id pivot can still be in + * flight; production-minted mappings always set it. + */ + organizationId?: string; envVarName: string; secretRef: SecretRef; deploymentName: string; @@ -202,6 +258,7 @@ export class SecretProxy { private authProfilesManager?: AuthProfilesManager; private readonly secretStore: SecretStore; private systemKeyResolver?: (providerId: string) => string | undefined; + private agentOrgResolver?: AgentOrgResolver; constructor(config: SecretProxyConfig, secretStore: SecretStore) { this.config = config; @@ -231,6 +288,16 @@ export class SecretProxy { this.systemKeyResolver = resolver; } + /** + * Wire in a resolver that maps a URL-encoded agentId to its owning org + * id. Used to compute the `expectedOrganizationId` we hand to + * `lookupPlaceholderMapping` — without it the org-scoping guard has no + * independent source of truth and can't enforce anything. + */ + setAgentOrgResolver(resolver: AgentOrgResolver): void { + this.agentOrgResolver = resolver; + } + /** * Register a provider upstream for slug-based routing. * Called after provider modules are initialized. @@ -277,9 +344,19 @@ export class SecretProxy { * Resolve a placeholder token to its real value via the in-memory cache. * Handles both plain (`lobu_secret_`) and prefixed * (`sk-ant-oat01-lobu_secret_`) placeholders. + * + * `expectedOrganizationId` is forwarded to {@link lookupPlaceholderMapping} + * so a worker carrying another tenant's placeholder cannot resolve it + * even on the legacy header-swap path. */ - private async resolveSecret(placeholder: string): Promise { - const mapping = this.lookupPlaceholderMapping(placeholder); + private async resolveSecret( + placeholder: string, + expectedOrganizationId?: string + ): Promise { + const mapping = this.lookupPlaceholderMapping( + placeholder, + expectedOrganizationId + ); if (!mapping) return null; return this.secretStore.get(mapping.secretRef); } @@ -288,12 +365,57 @@ export class SecretProxy { * Look up just the SecretMapping (without resolving the secret value) * for a placeholder. Used to verify the calling worker's bound agentId * matches the agentId in the request URL. + * + * If `expectedOrganizationId` is supplied and the stored mapping is + * tagged with a different org, treat it the same as a missing mapping — + * log and return null. This is defense-in-depth on top of the existing + * `mapping.agentId === urlAgentId` check: if a future code path + * resolves placeholders under a different tenant's context (e.g. + * cross-tenant header forwarding), the mismatch here blocks it. */ - private lookupPlaceholderMapping(placeholder: string): SecretMapping | null { - const prefixIdx = placeholder.indexOf(PLACEHOLDER_PREFIX); - if (prefixIdx === -1) return null; - const uuid = placeholder.slice(prefixIdx + PLACEHOLDER_PREFIX.length); - return placeholderCache.get(uuid); + private lookupPlaceholderMapping( + placeholder: string, + expectedOrganizationId?: string + ): SecretMapping | null { + return lookupPlaceholderMapping(placeholder, expectedOrganizationId); + } + + /** + * Extract the worker token from a dedicated `x-lobu-worker-token` header + * (or query param of the same name — useful for SSE that can't set + * headers), verify it, and return the bound `organizationId`. Falls back + * to verifying the bearer credential when it isn't a placeholder. + * + * Returns `undefined` when no verifiable token is present — the caller + * then relies on `agentOrgResolver` (DB lookup keyed by URL agentId) to + * fill in the expected org. + */ + private extractWorkerTokenOrg(c: Context): string | undefined { + const header = + c.req.header("x-lobu-worker-token") || + c.req.header("X-Lobu-Worker-Token"); + const candidate = header || c.req.query("worker_token") || undefined; + if (candidate) { + const data = verifyWorkerToken(candidate); + if (data?.organizationId) return data.organizationId; + } + // The bearer credential is normally a `lobu_secret_` placeholder + // but legacy callers may pass the worker JWT directly. Verify if it + // looks like one (long, no placeholder prefix). + const auth = + c.req.header("authorization") || c.req.header("Authorization"); + if (auth) { + const parts = auth.split(" "); + const tok = + parts.length === 2 && parts[0]?.toLowerCase() === "bearer" + ? parts[1] + : null; + if (tok && !tok.includes(PLACEHOLDER_PREFIX)) { + const data = verifyWorkerToken(tok); + if (data?.organizationId) return data.organizationId; + } + } + return undefined; } /** @@ -319,14 +441,18 @@ export class SecretProxy { * `source` identifies the caller (best available identity: bound agentId or * remote address) for per-source failed-resolution rate limiting. */ - private async swap(value: string, source: string): Promise { + private async swap( + value: string, + source: string, + expectedOrganizationId?: string + ): Promise { if (value.includes(PLACEHOLDER_PREFIX)) { if (resolutionFailureLimiter.isThrottled(source)) { // Source has burned through its failure budget — hard-fail without // touching the cache or logging another line. return ""; } - const resolved = await this.resolveSecret(value); + const resolved = await this.resolveSecret(value, expectedOrganizationId); if (!resolved) { // Fail closed: forwarding the literal placeholder upstream would // surface it in the provider's error response (and thus in worker @@ -404,6 +530,26 @@ export class SecretProxy { body = await c.req.text(); } + // Derive the caller's expected org from the verified worker token + // (preferred — it's signed) and fall back to a DB lookup keyed by the + // URL agentId. Either source becomes the `expectedOrganizationId` + // we hand to placeholder + secret lookups so a worker bearing org A's + // placeholder cannot resolve it under org B's URL. + const callerToken = this.extractCallerToken(c); + let expectedOrganizationId: string | undefined = + this.extractWorkerTokenOrg(c); + if (!expectedOrganizationId && urlAgentId && this.agentOrgResolver) { + try { + const orgId = await this.agentOrgResolver(urlAgentId); + if (orgId) expectedOrganizationId = orgId; + } catch (err) { + logger.warn( + { urlAgentId, err: String(err) }, + "agentOrgResolver failed — falling through without org expectation" + ); + } + } + // Bind the calling worker (identified by its placeholder credential) to // the agentId in the URL. Without this, anyone with network access to the // gateway could harvest another agent's credentials by changing the URL @@ -411,9 +557,11 @@ export class SecretProxy { // bound (logged as a warning) but reject any request whose placeholder // resolves to a different agent than the URL claims. if (urlAgentId) { - const callerToken = this.extractCallerToken(c); if (callerToken?.includes(PLACEHOLDER_PREFIX)) { - const mapping = this.lookupPlaceholderMapping(callerToken); + const mapping = this.lookupPlaceholderMapping( + callerToken, + expectedOrganizationId + ); if (!mapping) { logger.warn( { urlAgentId }, @@ -535,7 +683,11 @@ export class SecretProxy { }); const apiKey = c.req.header("x-api-key"); if (apiKey) { - headers["x-api-key"] = await this.swap(apiKey, source); + headers["x-api-key"] = await this.swap( + apiKey, + source, + expectedOrganizationId + ); } const auth = @@ -543,7 +695,11 @@ export class SecretProxy { if (auth) { const parts = auth.split(" "); if (parts.length === 2 && parts[0]?.toLowerCase() === "bearer") { - const swapped = await this.swap(parts[1]!, source); + const swapped = await this.swap( + parts[1]!, + source, + expectedOrganizationId + ); headers.authorization = `Bearer ${swapped}`; } } @@ -636,13 +792,19 @@ export function generatePlaceholder( envVarName: string, secretRef: SecretRef, deploymentName: string, - ttlSeconds?: number + options?: { ttlSeconds?: number; organizationId?: string } ): string { const uuid = crypto.randomUUID(); storeSecretMapping( uuid, - { agentId, envVarName, secretRef, deploymentName }, - ttlSeconds + { + agentId, + envVarName, + secretRef, + deploymentName, + organizationId: options?.organizationId, + }, + options?.ttlSeconds ); return `${PLACEHOLDER_PREFIX}${uuid}`; } diff --git a/packages/server/src/gateway/routes/public/agent.ts b/packages/server/src/gateway/routes/public/agent.ts index c30f276b4..5965019f3 100644 --- a/packages/server/src/gateway/routes/public/agent.ts +++ b/packages/server/src/gateway/routes/public/agent.ts @@ -633,6 +633,17 @@ export function createAgentApi(config: AgentApiConfig): OpenAPIHono { if (denial) return denial; } + // Stamp the worker token with the agent's owning org so the egress + // proxy's per-tenant gates (grant/deny, judge cache, judge policy) + // can scope decisions by org. Ephemeral agents have no preexisting + // metadata; their token mints without orgId and the proxy falls + // through to unscoped checks for that worker — flagged for a + // future fix that derives org from the auth session. + const tokenOrganizationId = + !isEphemeral && ownershipMetadataStore + ? (await ownershipMetadataStore.getMetadata(agentId))?.organizationId + : undefined; + // For ephemeral agents, auto-provision settings from system-key // providers (env-var-based API keys). No more template-agent fallback — // there are no template/sandbox agents anymore. @@ -689,6 +700,7 @@ export function createAgentApi(config: AgentApiConfig): OpenAPIHono { { channelId, agentId, + organizationId: tokenOrganizationId, platform: "api", sessionKey: userId, } @@ -718,6 +730,7 @@ export function createAgentApi(config: AgentApiConfig): OpenAPIHono { const token = generateWorkerToken(agentId, conversationId, deploymentName, { channelId, agentId, + organizationId: tokenOrganizationId, platform: "api", sessionKey: userId, }); diff --git a/packages/server/src/gateway/routes/public/slack.ts b/packages/server/src/gateway/routes/public/slack.ts index c98863c25..a0bf5889d 100644 --- a/packages/server/src/gateway/routes/public/slack.ts +++ b/packages/server/src/gateway/routes/public/slack.ts @@ -1,6 +1,8 @@ import { readFile } from "node:fs/promises"; import { createLogger } from "@lobu/core"; +import type { Context } from "hono"; import { Hono } from "hono"; +import { getDb } from "../../../db/client.js"; import { createSlackInstallStateStore } from "../../auth/oauth/state-store.js"; import { renderOAuthErrorPage, @@ -11,6 +13,78 @@ import { resolvePublicUrl } from "../../utils/public-url.js"; const logger = createLogger("slack-routes"); +/** + * Resolve the active organization id for the current request. + * + * Priority: + * 1. `c.get('organizationId')` — set by the lobuApp wrapper after + * `resolveDefaultOrgId(user.id)` (see `lobu/gateway.ts`). This is the + * value Postgres-backed stores read via AsyncLocalStorage, so binding + * install state to it keeps the OAuth flow aligned with where the + * resulting connection row will be written. + * 2. `c.get('session')?.activeOrganizationId` — better-auth's stamped + * active org, used when the wrapper hasn't run (rare; defensive). + * + * Returns `null` if neither is present — caller must reject the request + * (after consulting {@link resolveSingleTenantOrgId} for the self-host + * fallback). + */ +function readSessionOrgId(c: Context): string | null { + const fromContext = c.get("organizationId" as never) as + | string + | null + | undefined; + if (typeof fromContext === "string" && fromContext.length > 0) { + return fromContext; + } + const session = c.get("session" as never) as + | { activeOrganizationId?: string | null } + | null + | undefined; + const fromSession = session?.activeOrganizationId; + if (typeof fromSession === "string" && fromSession.length > 0) { + return fromSession; + } + return null; +} + +/** + * Self-host fallback: when there's exactly one organization row in the + * database, return its id. This keeps `/slack/install` usable on + * single-tenant deployments where the route is mounted without the + * lobuApp session middleware that populates `c.get('organizationId')`. + * + * Returns `null` when zero or more than one org rows exist — in those + * cases the caller must reject; we won't silently pick a tenant. + */ +async function resolveSingleTenantOrgId(): Promise { + try { + const sql = getDb(); + const rows = (await sql` + SELECT id FROM organization LIMIT 2 + `) as Array<{ id: string }>; + if (rows.length === 1) return rows[0]!.id; + return null; + } catch (err) { + logger.warn( + { err: String(err) }, + "Single-tenant org lookup failed — treating as ambiguous" + ); + return null; + } +} + +/** + * Resolve the install-flow org for the current request: session-bound first, + * then the self-host single-tenant fallback. Returns `null` only when + * neither path yields a definite org — at which point the route must reject. + */ +async function resolveInstallOrgId(c: Context): Promise { + const sessionOrgId = readSessionOrgId(c); + if (sessionOrgId) return sessionOrgId; + return resolveSingleTenantOrgId(); +} + const DEFAULT_SLACK_BOT_SCOPES = [ "app_mentions:read", "assistant:write", @@ -89,13 +163,32 @@ export function createSlackRoutes(manager: ChatInstanceManager): Hono { ); } + // Bind the install to the initiating session's active org. Without this + // an OAuth link minted under org A's session can be opened from org B's + // browser and the resulting connection lands in the wrong tenant. On + // self-host (no session middleware mounted), fall back to the sole org + // row when exactly one exists — see {@link resolveSingleTenantOrgId}. + const installOrgId = await resolveInstallOrgId(c); + if (!installOrgId) { + return c.html( + renderOAuthErrorPage( + "unauthorized", + "Sign in to an organization before starting Slack install." + ), + 401 + ); + } + const stateStore = createSlackInstallStateStore(); const redirectUri = resolvePublicUrl("/slack/oauth_callback", { configuredUrl: manager.getServices().getPublicGatewayUrl?.(), requestUrl: c.req.url, }); const scopes = await loadSlackBotScopes(); - const state = await stateStore.create({ redirectUri }); + const state = await stateStore.create({ + redirectUri, + organizationId: installOrgId, + }); const oauthUrl = new URL("https://slack.com/oauth/v2/authorize"); oauthUrl.searchParams.set("client_id", clientId); @@ -120,7 +213,11 @@ export function createSlackRoutes(manager: ChatInstanceManager): Hono { } const stateStore = createSlackInstallStateStore(); - const oauthState = await stateStore.consume(state); + // Peek (non-destructive) before validating side-channel context so a + // cross-org or unauthenticated hit doesn't burn the install link. + // Consume only after the org check passes — the row stays available + // for the legitimate caller to retry. + const oauthState = await stateStore.peek(state); if (!oauthState) { return c.html( @@ -132,10 +229,47 @@ export function createSlackRoutes(manager: ChatInstanceManager): Hono { ); } + // Reject the callback if the session that's completing the install + // belongs to a different org than the one that started it. Prevents + // an attacker who phishes the install link from landing a connection + // in their own org under a victim's authorization. Self-host falls + // back to the single-tenant resolver (same as `/slack/install`). + const callbackOrgId = await resolveInstallOrgId(c); + if (!callbackOrgId || callbackOrgId !== oauthState.organizationId) { + logger.warn( + { + stateOrg: oauthState.organizationId, + callbackOrg: callbackOrgId ?? null, + }, + "Rejecting Slack OAuth callback: session org does not match install state" + ); + return c.html( + renderOAuthErrorPage( + "org_mismatch", + "This Slack install link was started in a different organization. Sign in to that organization and try again." + ), + 403 + ); + } + + // Org check passed — now atomically consume so the link can't be + // replayed. If the row is gone between peek and consume (another + // tab raced), fall through to the same invalid_state response. + const consumed = await stateStore.consume(state); + if (!consumed) { + return c.html( + renderOAuthErrorPage( + "invalid_state", + "This Slack install link is invalid or has expired." + ), + 400 + ); + } + try { const result = await manager.completeSlackOAuthInstall( c.req.raw, - oauthState.redirectUri + consumed.redirectUri ); return c.html( renderOAuthSuccessPage(result.teamName || result.teamId, undefined, { diff --git a/packages/server/src/gateway/services/agent-threads.ts b/packages/server/src/gateway/services/agent-threads.ts index f12e7df87..b332fb615 100644 --- a/packages/server/src/gateway/services/agent-threads.ts +++ b/packages/server/src/gateway/services/agent-threads.ts @@ -69,7 +69,7 @@ export async function createThreadForAgent( args: CreateThreadForAgentArgs ): Promise { const { sessionManager } = deps; - const { agentId, reason, externalThreadId } = args; + const { agentId, organizationId, reason, externalThreadId } = args; const userId = args.userId || args.createdByUserId || agentId; const threadId = externalThreadId || randomUUID(); @@ -80,6 +80,7 @@ export async function createThreadForAgent( const token = generateWorkerToken(agentId, conversationId, deploymentName, { channelId, agentId, + organizationId, platform: "api", sessionKey: userId, }); diff --git a/packages/server/src/gateway/services/core-services.ts b/packages/server/src/gateway/services/core-services.ts index a0c4f627c..9bbdc7fd3 100644 --- a/packages/server/src/gateway/services/core-services.ts +++ b/packages/server/src/gateway/services/core-services.ts @@ -658,6 +658,12 @@ export class CoreServices { // Register provider upstream configs with the secret proxy for path-based routing if (this.secretProxy) { this.secretProxy.setAuthProfilesManager(this.authProfilesManager); + // Independent source of the caller's expected org for placeholder + // lookups — without this the org-scoping guard on + // `lookupPlaceholderMapping` has nothing to enforce against. + this.secretProxy.setAgentOrgResolver((agentId) => + this.resolveAgentOrgId(agentId) + ); for (const provider of getModelProviderModules()) { const upstream = provider.getUpstreamConfig?.(); if (upstream) { @@ -914,6 +920,21 @@ export class CoreServices { return this.secretProxy; } + /** + * Resolve an agent's owning organization id from `public.agents`. Used by + * the secret proxy to compute the `expectedOrganizationId` it hands to + * placeholder lookups. No cache — the SecretProxy is called once per + * upstream request, and a SELECT by primary key on `agents.id` is cheap + * enough that we'd rather avoid the staleness window of a TTL cache. + */ + private async resolveAgentOrgId(agentId: string): Promise { + const sql = getDb(); + const rows = (await sql` + SELECT organization_id FROM agents WHERE id = ${agentId} LIMIT 1 + `) as Array<{ organization_id?: string }>; + return rows[0]?.organization_id ?? null; + } + getSecretStore(): SecretStoreRegistry { if (!this.secretStore) throw new Error("Secret store not initialized"); return this.secretStore; diff --git a/packages/server/src/lobu/stores/postgres-stores.ts b/packages/server/src/lobu/stores/postgres-stores.ts index 5b76cf284..5827af5a8 100644 --- a/packages/server/src/lobu/stores/postgres-stores.ts +++ b/packages/server/src/lobu/stores/postgres-stores.ts @@ -80,6 +80,7 @@ function rowToMetadata(row: Record): AgentMetadata { }, isWorkspaceAgent: row.is_workspace_agent ?? undefined, workspaceId: row.workspace_id ?? undefined, + organizationId: row.organization_id ?? undefined, createdAt: row.created_at instanceof Date ? row.created_at.getTime() : (row.created_at ?? Date.now()), lastUsedAt: @@ -266,14 +267,14 @@ export function createPostgresAgentConfigStore(): AgentConfigStore { const orgId = tryGetOrgId(); const rows = orgId ? await sql` - SELECT id, name, description, owner_platform, owner_user_id, + SELECT id, organization_id, name, description, owner_platform, owner_user_id, is_workspace_agent, workspace_id, created_at, last_used_at FROM agents WHERE id = ${agentId} AND organization_id = ${orgId} ` : await sql` - SELECT id, name, description, owner_platform, owner_user_id, + SELECT id, organization_id, name, description, owner_platform, owner_user_id, is_workspace_agent, workspace_id, created_at, last_used_at FROM agents @@ -357,7 +358,7 @@ export function createPostgresAgentConfigStore(): AgentConfigStore { const sql = getDb(); const orgId = getOrgId(); const rows = await sql` - SELECT id, name, description, owner_platform, owner_user_id, + SELECT id, organization_id, name, description, owner_platform, owner_user_id, is_workspace_agent, workspace_id, created_at, last_used_at FROM agents