From ef15014084b419e91bfbbb22ccdf1c8a4442e4b4 Mon Sep 17 00:00:00 2001 From: Michael Guimaraes Date: Wed, 12 Aug 2026 20:49:09 +0300 Subject: [PATCH] fix(auth): resolve agent runtime consoles through workspace membership MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Hermes dashboard embed, the Hermes Skills routes, and the whole OpenClaw gateway surface authorized browser sessions with `WHERE id = $1 AND user_id = $2` — direct ownership only. Their sibling routes (/agents/:id/hermes-ui and its chat/cron/channel endpoints) resolve access through workspace membership, so a member who could see and manage a shared agent got a 404 from exactly these paths. The "Official Dashboard" tab was the visible symptom: the metadata call reported the dashboard ready, the iframe mounted, and the embed proxy answered "agent not found or not running" forever. Add buildAccessibleAgentQuery to middleware/ownership: one statement resolving owner-or-sharing-member and exposing the caller's highest role as effective_role. The embed and gateway lookups need it because their SSRF allowlist authorizes against a pinned column projection and so cannot route through findAccessibleAgent's SELECT *. user_id stays the owner in that projection — the remote-host grant check depends on it. Role thresholds follow the capability, not the transport: - Hermes dashboard embed: reads take viewer, mutations take editor, matching the native panels. The proxy relays DELETE/PATCH/POST/PUT into the dashboard's own API, so a viewer reaching it for writes now gets 403 instead of silently succeeding. - Hermes Skills: viewer to list, editor to install/delete. - OpenClaw gateway (embed, assets, bootstrap.js, REST router, WS relay): editor throughout, with no per-method split. bootstrap.js inlines the decrypted gateway password and rebinds the UI socket onto the relay, so even a GET grants live control. Viewers lose nothing here; the surface was owner-only before, so editor only widens access. Roles are re-resolved per request and per WS connection — the embed cookie carries none — so a demotion applies on the next request rather than at token expiry. Five existing fixtures pinned the old owner-only SQL string or omitted user_id/effective_role from mock rows; corrected without changing the assertions they guard (remote-host grant revocation, credential non-exposure). Co-Authored-By: Claude Opus 5 --- backend-api/__tests__/controlPlane.test.ts | 273 +++++++++++++++++- backend-api/__tests__/gatewayProxy.test.ts | 78 +++++ backend-api/__tests__/hermesSkills.test.ts | 126 ++++++++ .../remoteHostGatewayAllowlist.test.ts | 87 +++++- backend-api/gatewayProxy.ts | 63 +++- backend-api/middleware/ownership.ts | 49 ++++ backend-api/routes/hermesSkills.ts | 24 +- backend-api/server.ts | 88 ++++-- docs/concepts/workspaces.mdx | 9 + 9 files changed, 750 insertions(+), 47 deletions(-) diff --git a/backend-api/__tests__/controlPlane.test.ts b/backend-api/__tests__/controlPlane.test.ts index e2a9508d..56eb7d91 100644 --- a/backend-api/__tests__/controlPlane.test.ts +++ b/backend-api/__tests__/controlPlane.test.ts @@ -848,8 +848,10 @@ describe("gateway control-plane embed", () => { }; mockDb.query.mockImplementation(async (sql) => { const text = String(sql); - if (text.includes("FROM agents") && text.includes("WHERE id = $1 AND user_id = $2")) { - return { rows: [agent] }; + if (text.includes("FROM agents")) { + // The caller owns this agent, so the access lookup resolves it with the + // owner role; the grant re-check below is what must still reject it. + return { rows: [{ ...agent, effective_role: "owner" }] }; } if (text.includes("FROM remote_hosts")) return { rows: [] }; return { rows: [] }; @@ -881,8 +883,10 @@ describe("gateway control-plane embed", () => { const internalError = new Error("postgres://internal-user:secret@db/private"); mockDb.query.mockImplementation(async (sql) => { const text = String(sql); - if (text.includes("FROM agents") && text.includes("WHERE id = $1 AND user_id = $2")) { - return { rows: [agent] }; + if (text.includes("FROM agents")) { + // The caller owns this agent, so the access lookup resolves it with the + // owner role; the grant re-check below is what must still reject it. + return { rows: [{ ...agent, effective_role: "owner" }] }; } if (text.includes("FROM remote_hosts")) throw internalError; return { rows: [] }; @@ -1396,6 +1400,128 @@ describe("gateway control-plane embed", () => { expect(res.status).toBe(401); expect(global.fetch).not.toHaveBeenCalled(); }); + + // Unlike the Hermes dashboard, the OpenClaw control UI has no meaningful + // read-only mode: bootstrap.js hands the browser the decrypted gateway + // password and the relay WebSocket it drives is a live chat/terminal channel. + // The whole surface therefore takes `editor` rather than a per-method split. + describe("workspace-shared access", () => { + const OWNER_ID = "agent-owner"; + const SHARED_AGENT = { + id: "agent-shared-oc", + host: "10.0.0.50", + gateway_token: "enc(gateway-password)", + gateway_host_port: null, + status: "running", + runtime_family: "openclaw", + deploy_target: "docker", + user_id: OWNER_ID, + }; + + // Honors an owner-scoped `AND user_id = $2` faithfully, so a lookup that + // never consults workspace_members cannot pass these tests by accident. + function mockSharedAgentLookup(memberships) { + mockDb.query.mockImplementation(async (sql, params = []) => { + const text = String(sql); + if (!/FROM agents/i.test(text)) return { rows: [] }; + const [agentId, userId] = params; + if (agentId !== SHARED_AGENT.id) return { rows: [] }; + if (userId === OWNER_ID) { + return { rows: [{ ...SHARED_AGENT, effective_role: "owner" }] }; + } + if (!/workspace_members/i.test(text)) return { rows: [] }; + const role = memberships[userId]; + if (!role) return { rows: [] }; + return { rows: [{ ...SHARED_AGENT, effective_role: role }] }; + }); + } + + function memberToken(userId) { + return jwt.sign({ id: userId, role: "user" }, JWT_SECRET, { expiresIn: "1h" }); + } + + it.each(["editor", "admin", "owner"])( + "serves the gateway embed to a workspace %s who does not own the agent", + async (role) => { + mockSharedAgentLookup({ "member-1": role }); + global.fetch.mockResolvedValueOnce({ + ok: true, + status: 200, + headers: new Headers({ "content-type": "text/html; charset=utf-8" }), + text: async () => "ok", + }); + + const res = await request(app) + .get( + `/agents/${SHARED_AGENT.id}/gateway/embed?token=${encodeURIComponent(memberToken("member-1"))}`, + ) + .set("Host", "nora.test") + .set("Accept", "text/html"); + + expect(res.status).toBe(200); + expect(res.text).toContain( + ``, + ); + }, + ); + + it("gives a workspace editor the bootstrap script and its gateway credential", async () => { + mockSharedAgentLookup({ "member-1": "editor" }); + + const res = await request(app) + .get( + `/agents/${SHARED_AGENT.id}/gateway/embed/bootstrap.js?token=${encodeURIComponent(memberToken("member-1"))}`, + ) + .set("Host", "nora.test"); + + expect(res.status).toBe(200); + expect(res.text).toContain('"gateway-password"'); + }); + + // A viewer gains nothing here today (the surface is owner-only), so holding + // the line at editor is not a regression — it keeps the runtime password + // away from the one role that cannot already operate the agent. + it("keeps the gateway embed away from a read-only workspace viewer", async () => { + mockSharedAgentLookup({ "member-1": "viewer" }); + + const res = await request(app) + .get( + `/agents/${SHARED_AGENT.id}/gateway/embed?token=${encodeURIComponent(memberToken("member-1"))}`, + ) + .set("Host", "nora.test"); + + // 403, not 404: a viewer can already list this agent, so the role gap is + // the honest answer and hiding it would only be theatre. + expect(res.status).toBe(403); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it("keeps the bootstrap credential away from a read-only workspace viewer", async () => { + mockSharedAgentLookup({ "member-1": "viewer" }); + + const res = await request(app) + .get( + `/agents/${SHARED_AGENT.id}/gateway/embed/bootstrap.js?token=${encodeURIComponent(memberToken("member-1"))}`, + ) + .set("Host", "nora.test"); + + expect(res.status).toBe(403); + expect(res.text).not.toContain("gateway-password"); + }); + + it("still hides the embed from a user with no membership in any sharing workspace", async () => { + mockSharedAgentLookup({ "member-1": "editor" }); + + const res = await request(app) + .get( + `/agents/${SHARED_AGENT.id}/gateway/embed?token=${encodeURIComponent(memberToken("stranger"))}`, + ) + .set("Host", "nora.test"); + + expect(res.status).toBe(404); + expect(global.fetch).not.toHaveBeenCalled(); + }); + }); }); describe("Hermes dashboard embed", () => { @@ -1792,6 +1918,145 @@ describe("Hermes dashboard embed", () => { expect(res.status).toBe(404); expect(global.fetch).not.toHaveBeenCalled(); }); + + describe("workspace-shared access", () => { + const OWNER_ID = "agent-owner"; + const SHARED_AGENT = { + id: "agent-shared", + host: "10.0.0.44", + runtime_host: "10.0.0.44", + runtime_port: 8642, + runtime_family: "hermes", + backend_type: "docker", + deploy_target: "docker", + status: "running", + user_id: OWNER_ID, + }; + + // Stands in for the real `agents` lookup the embed proxy performs. It reads + // the SQL to decide *which* access model the query actually implements: a + // statement that never joins workspace_members can only ever authorize the + // agent's direct owner, which is exactly the bug these tests pin down. + function mockSharedAgentLookup(memberships) { + mockDb.query.mockImplementation(async (sql, params) => { + if (!/FROM agents/i.test(String(sql))) return { rows: [] }; + const [agentId, userId, acceptableRoles] = params || []; + if (agentId !== SHARED_AGENT.id) return { rows: [] }; + if (userId === OWNER_ID) { + return { rows: [{ ...SHARED_AGENT, effective_role: "owner" }] }; + } + if (!/workspace_members/i.test(String(sql))) return { rows: [] }; + const role = memberships[userId]; + if (!role) return { rows: [] }; + if (Array.isArray(acceptableRoles) && !acceptableRoles.includes(role)) { + return { rows: [] }; + } + return { rows: [{ ...SHARED_AGENT, effective_role: role }] }; + }); + } + + function memberToken(userId) { + return jwt.sign({ id: userId, role: "user" }, JWT_SECRET, { expiresIn: "1h" }); + } + + function mockDashboardHtml() { + global.fetch.mockResolvedValueOnce({ + ok: true, + status: 200, + headers: new Headers({ "content-type": "text/html; charset=utf-8" }), + text: async () => "ok", + }); + } + + it.each(["viewer", "editor", "admin", "owner"])( + "serves the dashboard embed to a workspace %s who does not own the agent", + async (role) => { + mockSharedAgentLookup({ "member-1": role }); + mockDashboardHtml(); + + const res = await request(app) + .get( + `/agents/${SHARED_AGENT.id}/hermes-ui/embed?token=${encodeURIComponent(memberToken("member-1"))}`, + ) + .set("Host", "nora.test") + .set("Accept", "text/html"); + + expect(res.status).toBe(200); + expect(res.headers["set-cookie"]).toEqual( + expect.arrayContaining([ + expect.stringContaining(`__nora_hermes_embed_${SHARED_AGENT.id}=`), + ]), + ); + }, + ); + + it("still hides the embed from a user with no access to the agent", async () => { + mockSharedAgentLookup({ "member-1": "editor" }); + + const res = await request(app) + .get( + `/agents/${SHARED_AGENT.id}/hermes-ui/embed?token=${encodeURIComponent(memberToken("stranger"))}`, + ) + .set("Host", "nora.test"); + + expect(res.status).toBe(404); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it("blocks a workspace viewer from writing through the embedded dashboard", async () => { + const agentClient = request.agent(app); + mockSharedAgentLookup({ "member-1": "viewer" }); + mockDashboardHtml(); + + const htmlRes = await agentClient + .get( + `/agents/${SHARED_AGENT.id}/hermes-ui/embed?token=${encodeURIComponent(memberToken("member-1"))}`, + ) + .set("Host", "nora.test"); + expect(htmlRes.status).toBe(200); + + global.fetch.mockClear(); + const writeRes = await agentClient + .put(`/agents/${SHARED_AGENT.id}/hermes-ui/embed/api/config`) + .set("Host", "nora.test") + .send({ config: { model: "gpt-5.5" } }); + + // A viewer may read the dashboard but must not gain write capability the + // native Hermes panels reserve for editors. + expect(writeRes.status).toBe(403); + expect(global.fetch).not.toHaveBeenCalled(); + }); + + it("lets a workspace editor write through the embedded dashboard", async () => { + const agentClient = request.agent(app); + mockSharedAgentLookup({ "member-1": "editor" }); + mockDashboardHtml(); + + const htmlRes = await agentClient + .get( + `/agents/${SHARED_AGENT.id}/hermes-ui/embed?token=${encodeURIComponent(memberToken("member-1"))}`, + ) + .set("Host", "nora.test"); + expect(htmlRes.status).toBe(200); + + global.fetch.mockClear(); + global.fetch.mockResolvedValueOnce({ + ok: true, + status: 200, + headers: new Headers({ "content-type": "application/json" }), + arrayBuffer: async () => Buffer.from('{"ok":true}'), + }); + + const writeRes = await agentClient + .put(`/agents/${SHARED_AGENT.id}/hermes-ui/embed/api/config`) + .set("Host", "nora.test") + .send({ config: { model: "gpt-5.5" } }); + + expect(writeRes.status).toBe(200); + expect(global.fetch.mock.calls[0][0]).toBe("http://10.0.0.44:9119/api/config"); + expect(global.fetch.mock.calls[0][1].method).toBe("PUT"); + }); + }); }); describe("Hermes runtime host grants", () => { diff --git a/backend-api/__tests__/gatewayProxy.test.ts b/backend-api/__tests__/gatewayProxy.test.ts index ff47158e..76a6e089 100644 --- a/backend-api/__tests__/gatewayProxy.test.ts +++ b/backend-api/__tests__/gatewayProxy.test.ts @@ -555,6 +555,84 @@ describe("gateway proxy control-plane routes", () => { expect(mockDb.query.mock.calls[1][1]).toEqual(["ws-A", "agent-1"]); }); + // Browser sessions were owner-only here, so a workspace member who could see + // and manage a shared OpenClaw agent still got a bare 404 from every gateway + // route. The whole surface now takes `editor` — it carries chat, exec, and + // restart, so there is no read-only slice to hand a viewer. + describe("workspace-shared agents", () => { + const SHARED_AGENT = { + id: "agent-1", + user_id: "agent-owner", + status: "running", + host: "10.0.0.10", + gateway_token: "gateway-token", + gateway_host_port: null, + backend_type: "docker", + runtime_family: "openclaw", + deploy_target: "docker", + }; + + // Models the real table semantics rather than a fixed call sequence, so it + // answers either access shape: findAccessibleAgent's row-then-membership + // pair, or a single statement that joins the membership in. An owner-scoped + // `AND user_id = $2` really filters non-owners out, so a lookup that never + // consults workspace_members cannot pass these tests by accident. + function mockSharedAgentLookup(memberships) { + mockDb.query.mockImplementation(async (sql, params = []) => { + const text = String(sql); + const joinsMembership = /workspace_members/i.test(text); + const role = memberships[params[1]]; + if (!/FROM agents/i.test(text)) { + // The standalone membership probe. + return { rows: joinsMembership && role ? [{ role }] : [] }; + } + const isOwner = params[1] === SHARED_AGENT.user_id; + if (/user_id\s*=\s*\$2/.test(text) && !joinsMembership) { + return { rows: isOwner ? [{ ...SHARED_AGENT, effective_role: "owner" }] : [] }; + } + if (joinsMembership) { + if (isOwner) return { rows: [{ ...SHARED_AGENT, effective_role: "owner" }] }; + return { rows: role ? [{ ...SHARED_AGENT, effective_role: role }] : [] }; + } + // Unscoped `SELECT * FROM agents WHERE id = $1`; the caller authorizes + // the row it gets back. + return { rows: [SHARED_AGENT] }; + }); + } + + it.each(["editor", "admin", "owner"])( + "serves gateway routes to a workspace %s who does not own the agent", + async (role) => { + app = buildApp({}, { user: { id: "member-1" } }); + mockSharedAgentLookup({ "member-1": role }); + + const res = await request(app).get("/agents/agent-1/gateway/status"); + + expect(res.status).toBe(200); + }, + ); + + it("refuses gateway routes for a read-only workspace viewer", async () => { + app = buildApp({}, { user: { id: "member-1" } }); + mockSharedAgentLookup({ "member-1": "viewer" }); + + const res = await request(app).get("/agents/agent-1/gateway/status"); + + expect(res.status).toBe(404); + expect(mockFakeWebSocket.instances).toHaveLength(0); + }); + + it("refuses gateway routes for a user with no membership in any sharing workspace", async () => { + app = buildApp({}, { user: { id: "stranger" } }); + mockSharedAgentLookup({ "member-1": "editor" }); + + const res = await request(app).get("/agents/agent-1/gateway/status"); + + expect(res.status).toBe(404); + expect(mockFakeWebSocket.instances).toHaveLength(0); + }); + }); + it("fails closed when a legacy adopted runtime has a weak gateway token", async () => { mockRunningAgent({ backend_type: "external", diff --git a/backend-api/__tests__/hermesSkills.test.ts b/backend-api/__tests__/hermesSkills.test.ts index f9b9db5c..edca8566 100644 --- a/backend-api/__tests__/hermesSkills.test.ts +++ b/backend-api/__tests__/hermesSkills.test.ts @@ -480,6 +480,132 @@ describe("hermes skills routes", () => { }); }); + // The Skills panel lives inside the Hermes WebUI tab, whose sibling routes + // (/agents/:id/hermes-ui and its chat/cron/channel endpoints) authorize + // through workspace membership. These pin the Skills routes to the same + // model: a member who can see and operate a shared agent must not hit a bare + // 404 here just because someone else owns the agent row. + describe("workspace-shared agents", () => { + const OWNER_ID = "agent-owner"; + const SHARED_AGENT = { + id: "agent-shared", + user_id: OWNER_ID, + status: "running", + container_id: "container-shared", + backend_type: "docker", + runtime_family: "hermes", + deploy_target: "docker", + sandbox_profile: "standard", + hermes_skills: [], + }; + + // Faithful stand-in for the `agents` lookup: an owner-scoped statement + // (`AND user_id = $2`) really does filter non-owners out, so a lookup that + // never consults workspace_members cannot pass these tests by accident. + function mockSharedAgentLookup(memberships) { + db.query.mockImplementation(async (sql, params = []) => { + const text = String(sql); + if (text.includes("workspace_members")) { + const role = memberships[params[1]]; + return { rows: role ? [{ role }] : [] }; + } + if (!text.includes("FROM agents")) return { rows: [] }; + if (params[0] !== SHARED_AGENT.id) return { rows: [] }; + if (/user_id\s*=\s*\$2/.test(text) && params[1] !== OWNER_ID) return { rows: [] }; + return { rows: [SHARED_AGENT] }; + }); + } + + function sessionReq(userId, body = {}) { + return { params: { agentId: SHARED_AGENT.id }, user: { id: userId }, body }; + } + + it("lists skills for a workspace viewer who does not own the agent", async () => { + mockSharedAgentLookup({ "member-1": "viewer" }); + runContainerCommand.mockResolvedValueOnce({ output: EMPTY_LOCK_B64 }); + hermesSkillsQueue.getJobs.mockResolvedValueOnce([]); + + const res = createMockRes(); + await getRouteHandler("/agents/:agentId/skills")(sessionReq("member-1"), res); + + expect(res.statusCode).toBe(200); + expect(res.body).toEqual({ skills: [] }); + }); + + it("installs a skill for a workspace editor who does not own the agent", async () => { + mockSharedAgentLookup({ "member-1": "editor" }); + findInFlightHermesSkillJob.mockResolvedValueOnce(null); + addHermesSkillJob.mockResolvedValueOnce({ id: "job-1" }); + + const res = createMockRes(); + await getRouteHandler("/agents/:agentId/skills/install", "post")( + sessionReq("member-1", { ref: "official/security/1password", name: "1password" }), + res, + ); + + expect(res.statusCode).toBe(202); + expect(addHermesSkillJob).toHaveBeenCalledWith( + expect.objectContaining({ agentId: SHARED_AGENT.id, name: "1password" }), + ); + }); + + // Mirrors loadHermesUiAgent: an insufficient role resolves to no agent, so + // the mutation reports agent_not_found rather than leaking the role gap. + it("refuses installs from a read-only workspace viewer", async () => { + mockSharedAgentLookup({ "member-1": "viewer" }); + + const res = createMockRes(); + await getRouteHandler("/agents/:agentId/skills/install", "post")( + sessionReq("member-1", { ref: "official/security/1password", name: "1password" }), + res, + ); + + expect(res.statusCode).toBe(404); + expect(addHermesSkillJob).not.toHaveBeenCalled(); + }); + + it("refuses deletes from a read-only workspace viewer", async () => { + mockSharedAgentLookup({ "member-1": "viewer" }); + + const res = createMockRes(); + await getRouteHandler("/agents/:agentId/skills/delete", "post")( + sessionReq("member-1", { name: "1password" }), + res, + ); + + expect(res.statusCode).toBe(404); + expect(addHermesSkillJob).not.toHaveBeenCalled(); + }); + + it("hides the agent from a user with no membership in any sharing workspace", async () => { + mockSharedAgentLookup({ "member-1": "editor" }); + + const res = createMockRes(); + await getRouteHandler("/agents/:agentId/skills")(sessionReq("stranger"), res); + + expect(res.statusCode).toBe(404); + expect(runContainerCommand).not.toHaveBeenCalled(); + }); + + it("scopes job polling to a workspace member's access", async () => { + mockSharedAgentLookup({ "member-1": "viewer" }); + getHermesSkillJobStatus.mockResolvedValueOnce({ + agentId: SHARED_AGENT.id, + status: "completed", + operation: "install", + }); + + const res = createMockRes(); + await getRouteHandler("/jobs/:jobId")( + { params: { jobId: "job-1" }, user: { id: "member-1" } }, + res, + ); + + expect(res.statusCode).toBe(200); + expect(res.body).toMatchObject({ agentId: SHARED_AGENT.id, status: "completed" }); + }); + }); + it("returns unsupported_runtime for non-hermes agents", async () => { const handler = getRouteHandler("/agents/:agentId/skills/install", "post"); db.query.mockResolvedValueOnce({ diff --git a/backend-api/__tests__/remoteHostGatewayAllowlist.test.ts b/backend-api/__tests__/remoteHostGatewayAllowlist.test.ts index 252656fe..fb47b5eb 100644 --- a/backend-api/__tests__/remoteHostGatewayAllowlist.test.ts +++ b/backend-api/__tests__/remoteHostGatewayAllowlist.test.ts @@ -419,7 +419,9 @@ describe("hosted-mode Remote Docker gateway shutdown", () => { status: "running", gateway_token: "legacy-gateway-token", }; - mockDbQuery.mockResolvedValue({ rows: [agent] }); + // The relay's access lookup resolves the caller's role alongside the row; + // these fixtures connect as the agent's own owner. + mockDbQuery.mockResolvedValue({ rows: [{ ...agent, effective_role: "owner" }] }); const server = { on: jest.fn() }; const wss = attachGatewayWS(server); const ws = { send: jest.fn(), close: jest.fn() }; @@ -451,7 +453,9 @@ describe("remote-host gateway relay grant revocation", () => { gatewayHost: PUBLIC_IP, sshHost: PUBLIC_IP, }; - mockDbQuery.mockResolvedValue({ rows: [agent] }); + // The relay's access lookup resolves the caller's role alongside the row; + // these fixtures connect as the agent's own owner. + mockDbQuery.mockResolvedValue({ rows: [{ ...agent, effective_role: "owner" }] }); mockAssertRemoteHostAgentUse.mockImplementationOnce(() => authorization); const server = { on: jest.fn() }; @@ -483,7 +487,9 @@ describe("remote-host gateway relay grant revocation", () => { sshHost: PUBLIC_IP, }; - mockDbQuery.mockResolvedValue({ rows: [agent] }); + // The relay's access lookup resolves the caller's role alongside the row; + // these fixtures connect as the agent's own owner. + mockDbQuery.mockResolvedValue({ rows: [{ ...agent, effective_role: "owner" }] }); mockGetRemoteHostByExecutionTarget.mockResolvedValue(host); mockUserCanUseRemoteHost.mockImplementation(async () => grantActive); @@ -538,7 +544,9 @@ describe("remote-host gateway relay grant revocation", () => { sshHost: PUBLIC_IP, }; - mockDbQuery.mockResolvedValue({ rows: [agent] }); + // The relay's access lookup resolves the caller's role alongside the row; + // these fixtures connect as the agent's own owner. + mockDbQuery.mockResolvedValue({ rows: [{ ...agent, effective_role: "owner" }] }); mockGetRemoteHostByExecutionTarget.mockResolvedValue(host); mockUserCanUseRemoteHost.mockImplementation(async () => grantActive); @@ -626,7 +634,9 @@ describe("remote-host gateway relay grant revocation", () => { status: "running", gateway_token: "legacy-gateway-token", }; - mockDbQuery.mockResolvedValue({ rows: [agent] }); + // The relay's access lookup resolves the caller's role alongside the row; + // these fixtures connect as the agent's own owner. + mockDbQuery.mockResolvedValue({ rows: [{ ...agent, effective_role: "owner" }] }); mockGetRemoteHostByExecutionTarget.mockResolvedValue({ id: "my-vps", ownerUserId: agent.user_id, @@ -653,6 +663,73 @@ describe("remote-host gateway relay grant revocation", () => { }); }); +// The relay carries chat and exec, so it takes the same `editor` bar as the +// rest of the OpenClaw gateway surface. The embed cookie that authenticates the +// upgrade carries no role of its own, so the role is re-resolved per connection +// and a demotion takes effect on the next connect rather than at token expiry. +describe("gateway relay workspace roles", () => { + const SHARED_AGENT = { + id: "agent-shared-ws", + user_id: "agent-owner", + status: "running", + host: "10.0.0.60", + gateway_host: "10.0.0.60", + gateway_port: 18789, + gateway_token: "gateway-token", + deploy_target: "docker", + runtime_family: "openclaw", + }; + + function mockRelayLookup(memberships) { + mockDbQuery.mockImplementation(async (sql, params = []) => { + const text = String(sql); + if (!/FROM agents/i.test(text)) return { rows: [] }; + const isOwner = params[1] === SHARED_AGENT.user_id; + if (/user_id\s*=\s*\$2/.test(text) && !/workspace_members/i.test(text)) { + return { rows: isOwner ? [{ ...SHARED_AGENT, effective_role: "owner" }] : [] }; + } + if (isOwner) return { rows: [{ ...SHARED_AGENT, effective_role: "owner" }] }; + const role = memberships[params[1]]; + return { rows: role ? [{ ...SHARED_AGENT, effective_role: role }] : [] }; + }); + } + + it("refuses the relay for a read-only workspace viewer", async () => { + mockRelayLookup({ "member-1": "viewer" }); + const wss = attachGatewayWS({ on: jest.fn() }); + const clientWs = createClientWebSocket(); + + await wss.handlers.connection(clientWs, {}, SHARED_AGENT.id, { id: "member-1" }); + + expect(clientWs.sent).toContainEqual({ type: "error", message: "Agent not found" }); + expect(mockGatewaySockets).toHaveLength(0); + }); + + it("refuses the relay for a user with no membership in any sharing workspace", async () => { + mockRelayLookup({ "member-1": "editor" }); + const wss = attachGatewayWS({ on: jest.fn() }); + const clientWs = createClientWebSocket(); + + await wss.handlers.connection(clientWs, {}, SHARED_AGENT.id, { id: "stranger" }); + + expect(clientWs.sent).toContainEqual({ type: "error", message: "Agent not found" }); + expect(mockGatewaySockets).toHaveLength(0); + }); + + it("opens the relay for a workspace editor who does not own the agent", async () => { + mockRelayLookup({ "member-1": "editor" }); + const wss = attachGatewayWS({ on: jest.fn() }); + const clientWs = createClientWebSocket(); + + await wss.handlers.connection(clientWs, {}, SHARED_AGENT.id, { id: "member-1" }); + + expect(clientWs.sent).not.toContainEqual( + expect.objectContaining({ message: "Agent not found" }), + ); + expect(mockGatewaySockets).toHaveLength(1); + }); +}); + describe("hermes dashboard embed-proxy allowlist (SSRF)", () => { it("allows a local Hermes agent's RFC1918 dashboard host", async () => { const agent = { diff --git a/backend-api/gatewayProxy.ts b/backend-api/gatewayProxy.ts index dc776c37..5eeb2054 100644 --- a/backend-api/gatewayProxy.ts +++ b/backend-api/gatewayProxy.ts @@ -13,7 +13,19 @@ const { decrypt } = require("./crypto"); const integrations = require("./integrations"); const { resolveAgentRuntimeFamily } = require("./agentRuntimeFields"); const { scopeByMethod } = require("./middleware/auth"); -const { findAgentForRequest, requireApiKeyAgentScope } = require("./middleware/ownership"); +const { + buildAccessibleAgentQuery, + findAccessibleAgentForRequest, + requireApiKeyAgentScope, + roleSatisfies, +} = require("./middleware/ownership"); + +// The OpenClaw gateway — REST routes, the control-UI embed, and the relay +// WebSocket — is an operate-the-agent capability end to end: it carries chat, +// exec, restart, and the runtime password. There is no read-only slice worth +// carving out, so the whole surface takes one bar. Workspace viewers are +// unaffected; this path was owner-only before, so `editor` only widens access. +const GATEWAY_MIN_WORKSPACE_ROLE = "editor"; const remoteHosts = require("./remoteHosts"); const { PRIVATE_IP_RE } = require("./networkSafety"); const { normalizeDeployTargetName } = require("../agent-runtime/lib/backendCatalog"); @@ -1484,23 +1496,44 @@ async function getConnection(agent) { // ─── Helpers ───────────────────────────────────────────────────── +const GATEWAY_RELAY_AGENT_COLUMNS = [ + "id", + "name", + "status", + "container_id", + "host", + "backend_type", + "gateway_token", + "gateway_host_port", + "gateway_host", + "gateway_port", + "runtime_host", + "runtime_port", + "runtime_family", + "deploy_target", + "execution_target_id", + "user_id", +]; + /** - * Load an agent for a gateway request and enforce exact owner matching. + * Load an agent for the gateway relay and enforce the minimum role. + * + * The relay is a live chat/terminal channel onto the runtime, so it takes the + * same `editor` bar as the rest of the OpenClaw gateway surface: the agent's + * owner, or a member of a sharing workspace who can operate it. Role is + * re-resolved per connection because the embed cookie carries none of its own. * * @param {string} agentId - Requested agent id. - * @param {string} userId - Authenticated owner id. - * @returns {Promise} Owner-scoped agent or `null` without existence disclosure. + * @param {string} userId - Authenticated user id. + * @returns {Promise} Authorized agent or `null` without existence disclosure. */ async function resolveAgent(agentId, userId) { - const result = await db.query( - `SELECT id, name, status, container_id, host, backend_type, gateway_token, - gateway_host_port, gateway_host, gateway_port, runtime_host, - runtime_port, runtime_family, deploy_target, execution_target_id, user_id - FROM agents WHERE id = $1`, - [agentId], - ); + const result = await db.query(buildAccessibleAgentQuery(GATEWAY_RELAY_AGENT_COLUMNS), [ + agentId, + userId, + ]); const agent = result.rows[0]; - if (!agent || agent.user_id !== userId) return null; + if (!agent || !roleSatisfies(agent.effective_role, GATEWAY_MIN_WORKSPACE_ROLE)) return null; return agent; } @@ -1643,7 +1676,11 @@ function createGatewayRouter(options = {}) { // gateway eventually starts successfully. router.use("/agents/:agentId/gateway", async (req, res, next) => { try { - const agent = await findAgentForRequest(req, req.params.agentId); + const agent = await findAccessibleAgentForRequest( + req, + req.params.agentId, + GATEWAY_MIN_WORKSPACE_ROLE, + ); if (!agent) return res.status(404).json({ error: "Agent not found" }); if (resolveAgentRuntimeFamily(agent) !== "openclaw") { return res.status(409).json({ diff --git a/backend-api/middleware/ownership.ts b/backend-api/middleware/ownership.ts index ab33b343..16703477 100644 --- a/backend-api/middleware/ownership.ts +++ b/backend-api/middleware/ownership.ts @@ -235,6 +235,54 @@ async function findAccessibleAgentForRequest(req, agentId, requiredRole = "viewe return assertApiKeyCanAccessLoadedAgent(req, agent); } +/** + * Build the `agents` SELECT used by lookups that cannot call findAccessibleAgent + * because they need a narrow, pinned column projection instead of `SELECT *` — + * the embed and gateway proxies, whose SSRF allowlist authorizes against an + * exact field list (see embedAgentColumns.ts). + * + * The statement resolves the same access model findAccessibleAgent implements — + * the agent's owner, or a member of any workspace the agent is shared into — and + * exposes the caller's highest role as `effective_role` so the caller can apply + * its own minimum-role rule. Rows reachable by neither route are filtered out, + * so a caller that ignores `effective_role` still gets owner-or-member scoping. + * + * Bind exactly `[agentId, userId]`. `user_id` in the projection stays the + * *owner*, never the caller — the remote-host grant check depends on that. + * + * @param {string[]} columns - Agent columns to select; identifiers only. + * @returns {string} Parameterized SELECT statement. + */ +function buildAccessibleAgentQuery(columns) { + for (const column of columns) { + // These lists are module constants, never request input, but interpolating + // them into SQL is only safe while that stays true. + if (typeof column !== "string" || !/^[a-z_]+$/.test(column)) { + throw new Error(`Unsafe agent column: ${String(column)}`); + } + } + return `SELECT ${columns.map((column) => `a.${column}`).join(", ")}, + CASE WHEN a.user_id = $2 THEN 'owner' ELSE shared.role END AS effective_role + FROM agents a + LEFT JOIN LATERAL ( + SELECT m.role + FROM workspace_agents wa + JOIN workspace_members m + ON m.workspace_id = wa.workspace_id AND m.user_id = $2 + WHERE wa.agent_id = a.id + ORDER BY + CASE m.role + WHEN 'owner' THEN 0 + WHEN 'admin' THEN 1 + WHEN 'editor' THEN 2 + WHEN 'viewer' THEN 3 + END + LIMIT 1 + ) shared ON TRUE + WHERE a.id = $1 + AND (a.user_id = $2 OR shared.role IS NOT NULL)`; +} + /** * Resolve an agent through direct ownership or any sharing workspace where the * user meets the requested role, attaching the effective role on success. @@ -447,6 +495,7 @@ module.exports = { requireApiKeyAgentPathScope, findOwnedAgent, findAgentForRequest, + buildAccessibleAgentQuery, findAccessibleAgent, findAccessibleAgentForRequest, findAccessibleAgentForActor, diff --git a/backend-api/routes/hermesSkills.ts b/backend-api/routes/hermesSkills.ts index b4e8facc..c0b64661 100644 --- a/backend-api/routes/hermesSkills.ts +++ b/backend-api/routes/hermesSkills.ts @@ -11,7 +11,7 @@ const { const { runContainerCommand } = require("../authSync"); const { requireScope, scopeByMethod } = require("../middleware/auth"); const { - findAgentForRequest, + findAccessibleAgentForRequest, isRemoteDockerAgent, requireApiKeyAgentScope, } = require("../middleware/ownership"); @@ -174,12 +174,22 @@ function sendSkillNameValidationError(res, name, action) { /** * Load an agent authorized for the current session or scoped API key request. * + * The Skills panel is one sub-tab of the Hermes WebUI, so it resolves access the + * same way its siblings do (routes/agents.ts → loadHermesUiAgent): the agent's + * owner, or a member of a workspace the agent is shared into who meets the + * route's minimum role. Reads take `viewer`, mutations take `editor`. An + * insufficient role resolves to no agent, which the callers surface as + * `agent_not_found` — matching the sibling routes rather than advertising the + * role gap. API-key requests keep their exact workspace binding via + * findAccessibleAgentForRequest's key branch. + * * @param {Object} req - Request carrying session or API-key authorization context. * @param {string} agentId - Agent to load. + * @param {string} [requiredRole="viewer"] - Minimum workspace role. * @returns {Promise} Request-accessible agent row, or `null`. */ -async function loadOwnedAgent(req, agentId) { - return findAgentForRequest(req, agentId); +async function loadAccessibleAgent(req, agentId, requiredRole = "viewer") { + return findAccessibleAgentForRequest(req, agentId, requiredRole); } /** @@ -255,7 +265,7 @@ router.get("/skills/detail", async (req, res) => { router.get("/agents/:agentId/skills", async (req, res) => { try { - const agent = await loadOwnedAgent(req, req.params.agentId); + const agent = await loadAccessibleAgent(req, req.params.agentId); validateHermesMutableAgent(agent); const { output } = await runContainerCommand(agent, HERMES_SKILLS_LOCK_READ_COMMAND); const decoded = Buffer.from( @@ -280,7 +290,7 @@ router.get("/agents/:agentId/skills", async (req, res) => { router.post("/agents/:agentId/skills/install", async (req, res) => { try { - const agent = await loadOwnedAgent(req, req.params.agentId); + const agent = await loadAccessibleAgent(req, req.params.agentId, "editor"); validateHermesMutableAgent(agent); const ref = typeof req.body?.ref === "string" ? req.body.ref.trim() : ""; const name = typeof req.body?.name === "string" ? req.body.name.trim() : ""; @@ -343,7 +353,7 @@ router.post("/agents/:agentId/skills/install", async (req, res) => { router.post("/agents/:agentId/skills/delete", async (req, res) => { try { - const agent = await loadOwnedAgent(req, req.params.agentId); + const agent = await loadAccessibleAgent(req, req.params.agentId, "editor"); validateHermesMutableAgent(agent); const name = typeof req.body?.name === "string" ? req.body.name.trim() : ""; if (sendSkillNameValidationError(res, name, "removed")) { @@ -404,7 +414,7 @@ router.get("/jobs/:jobId", requireScope("agents:read"), async (req, res) => { let agent; try { - agent = await loadOwnedAgent(req, status.agentId); + agent = await loadAccessibleAgent(req, status.agentId); } catch (error) { if (error?.statusCode === 403 || error?.code === "session_required") { return res.status(404).json({ error: "job_not_found" }); diff --git a/backend-api/server.ts b/backend-api/server.ts index 908d63ff..f03ed889 100644 --- a/backend-api/server.ts +++ b/backend-api/server.ts @@ -42,7 +42,11 @@ const { STARTER_TEMPLATES } = require("./starterTemplates"); const { allowsFirstAdminSignupClaim, getBootstrapAdminSeedConfig } = require("./bootstrapAdmin"); const { ensureFirstRegisteredUserIsAdmin } = require("./ensureAdminUser"); const { authenticateToken } = require("./middleware/auth"); -const { requireApiKeyAgentPathScope } = require("./middleware/ownership"); +const { + buildAccessibleAgentQuery, + requireApiKeyAgentPathScope, + roleSatisfies, +} = require("./middleware/ownership"); const { correlationId, errorHandler } = require("./middleware/errorHandler"); const { createGatewayRouter, @@ -444,12 +448,15 @@ function setProxyResponseHeaders(res, resp, { cachePolicy = "asset" } = {}) { } async function lookupEmbedAgent(agentId, userId) { - const result = await db.query( - `SELECT ${GATEWAY_EMBED_AGENT_COLUMNS.join(", ")} - FROM agents - WHERE id = $1 AND user_id = $2`, - [agentId, userId], - ); + // Owner or sharing-workspace member, same as the Hermes lookup below. The + // OpenClaw callers all demand `editor` (see GATEWAY_EMBED_MIN_WORKSPACE_ROLE) + // rather than the Hermes per-method split: bootstrap.js hands the browser the + // decrypted gateway password, and the relay socket it configures is a live + // chat/terminal channel, so there is no coherent read-only mode to grant. + const result = await db.query(buildAccessibleAgentQuery(GATEWAY_EMBED_AGENT_COLUMNS), [ + agentId, + userId, + ]); if ( !result.rows[0] || !isGatewayAvailableStatus(result.rows[0].status) || @@ -465,12 +472,21 @@ async function lookupHermesEmbedAgent(agentId, userId) { // user_id / gateway_host) the embed proxy's allowlist authorizes against — see // embedAgentColumns.ts. Omitting them would mis-route a remote-docker/k8s agent // or short-circuit the owner-scoping check. - const result = await db.query( - `SELECT ${HERMES_EMBED_AGENT_COLUMNS.join(", ")} - FROM agents - WHERE id = $1 AND user_id = $2`, - [agentId, userId], - ); + // + // Access mirrors the native Hermes WebUI routes (routes/agents.ts → + // loadHermesUiAgent → findAccessibleAgent): the agent's owner, plus any member + // of a workspace the agent is shared into. An owner-only lookup here made the + // embedded dashboard the one Hermes surface a workspace member could see in + // the tab bar but never load. `user_id` stays selected because it is the + // *owner* the remote-host grant check authorizes against — never the caller. + // + // effective_role is the caller's highest role across the sharing workspaces; + // resolveEmbedAccess gates mutating embed requests on it so a viewer cannot + // reach through the proxy for writes the native panels reserve for editors. + const result = await db.query(buildAccessibleAgentQuery(HERMES_EMBED_AGENT_COLUMNS), [ + agentId, + userId, + ]); if ( !result.rows[0] || !isGatewayAvailableStatus(result.rows[0].status) || @@ -490,12 +506,12 @@ async function fetchAgentForHermesRepair(agentId) { /** * Authenticate an embedded UI through a verified JWT or agent-scoped HttpOnly - * session, verify direct ownership/runtime availability, and mint the scoped + * session, verify agent access/runtime availability, and mint the scoped * cookie when needed. * * @param {Object} req - Express embed request. * @param {Object} res - Express response used for auth failures and cookies. - * @param {Object} [options={}] - Scope, cookie, lookup, and query-token policy. + * @param {Object} [options={}] - Scope, cookie, lookup, role, and query-token policy. * @returns {Promise} Authorized embed context, or `null` after responding. */ async function resolveEmbedAccess( @@ -506,6 +522,7 @@ async function resolveEmbedAccess( lookupAgent = lookupEmbedAgent, cookiePrefix = EMBED_SESSION_COOKIE_PREFIX, scope = "gateway-embed", + requiredRole = "viewer", } = {}, ) { const jwt = require("jsonwebtoken"); @@ -570,6 +587,15 @@ async function resolveEmbedAccess( return null; } + // Owner-only lookups return no effective_role; the row *is* the owner's, so + // it satisfies every threshold. Workspace-aware lookups carry the caller's + // highest sharing role, which must clear this request's bar. 403 (not 404) is + // correct here: the caller already knows the agent exists. + if (!roleSatisfies(agent.effective_role || "owner", requiredRole)) { + res.status(403).send("insufficient workspace permissions for this agent"); + return null; + } + try { // Embed sessions and the OpenClaw bootstrap script expose live runtime // traffic and, for bootstrap.js, the decrypted gateway password. Re-check @@ -880,6 +906,25 @@ app.get("/api-docs", (req, res) => { const gatewayUIAssetProxy = require("express").Router(); const PREAUTH_ASSET_METHODS = new Set(["GET", "HEAD"]); const EMBED_PROXY_METHODS = new Set(["DELETE", "GET", "HEAD", "OPTIONS", "PATCH", "POST", "PUT"]); +const EMBED_READ_METHODS = new Set(["GET", "HEAD", "OPTIONS"]); + +// Every OpenClaw gateway embed surface — the control UI, its assets, and the +// bootstrap script — takes this single role instead of the Hermes per-method +// split. bootstrap.js inlines the decrypted gateway password into browser JS +// and rebinds the UI's WebSocket onto the relay, so even a GET here is a grant +// of live control. A workspace viewer loses nothing: this surface was +// owner-only before, so `editor` only ever widens access. +const GATEWAY_EMBED_MIN_WORKSPACE_ROLE = "editor"; + +// The embedded dashboard is a full control UI, so proxying it verbatim would let +// any caller who can read it also write through it. Map the request method onto +// the same viewer/editor split the native Hermes WebUI routes use +// (routes/agents.ts: reads default to viewer, mutations require editor) so a +// workspace viewer gets the read-only dashboard and nothing more. Re-derived per +// request — the embed session cookie deliberately carries no role of its own. +function embedRoleForMethod(method) { + return EMBED_READ_METHODS.has(String(method || "").toUpperCase()) ? "viewer" : "editor"; +} gatewayUIAssetProxy.use("/agents/:agentId/gateway", (req, res, next) => { if (!PREAUTH_ASSET_METHODS.has(req.method)) return next(); @@ -900,7 +945,9 @@ gatewayUIAssetProxy.use("/agents/:agentId/gateway", (req, res, next) => { // cookie so the control UI can keep using its own relative paths. gatewayUIAssetProxy.get("/agents/:agentId/gateway/embed/bootstrap.js", async (req, res) => { try { - const access = await resolveEmbedAccess(req, res); + const access = await resolveEmbedAccess(req, res, { + requiredRole: GATEWAY_EMBED_MIN_WORKSPACE_ROLE, + }); if (!access) return; res.setHeader("Content-Type", "application/javascript; charset=utf-8"); @@ -938,7 +985,9 @@ gatewayUIAssetProxy.get("/agents/:agentId/gateway/embed/bootstrap.js", async (re */ async function proxyEmbeddedGateway(req, res) { try { - const access = await resolveEmbedAccess(req, res); + const access = await resolveEmbedAccess(req, res, { + requiredRole: GATEWAY_EMBED_MIN_WORKSPACE_ROLE, + }); if (!access) return; const gatewayPath = getEmbeddedGatewayPath(req); @@ -1021,6 +1070,7 @@ async function proxyEmbeddedHermes(req, res) { lookupAgent: lookupHermesEmbedAgent, cookiePrefix: HERMES_EMBED_SESSION_COOKIE_PREFIX, scope: "hermes-embed", + requiredRole: embedRoleForMethod(req.method), }); if (!access) return; @@ -1191,7 +1241,9 @@ gatewayUIAssetProxy.use("/agents/:agentId/hermes-ui", (req, res, next) => { */ async function proxyGatewayAsset(req, res) { try { - const access = await resolveEmbedAccess(req, res); + const access = await resolveEmbedAccess(req, res, { + requiredRole: GATEWAY_EMBED_MIN_WORKSPACE_ROLE, + }); if (!access) return; const gatewayPath = req.path || "/"; diff --git a/docs/concepts/workspaces.mdx b/docs/concepts/workspaces.mdx index 5c9a7e50..683506da 100644 --- a/docs/concepts/workspaces.mdx +++ b/docs/concepts/workspaces.mdx @@ -53,6 +53,15 @@ Workspace roles apply to assigned agents: | `admin` | Manage members, invitations, budgets, alert rules, API keys, and remove agent assignments. | | `owner` | Full workspace control, including deleting the workspace. | +Embedded runtime consoles follow the same rule. A Hermes agent's **Hermes WebUI → Official +Dashboard** tab loads for every member of a sharing workspace, and mutating requests made through it +require `editor` or above, exactly like the native chat, cron, and channel panels. + +An OpenClaw agent's gateway — its control UI, gateway API routes, and the chat/terminal +WebSocket — requires `editor` or above in full. That surface carries live agent control and the +runtime's own gateway credential, so it has no read-only mode: a `viewer` can see a shared OpenClaw +agent and its metrics, but not open its console. + Removing an agent from a workspace only removes the assignment. The agent continues running and remains available to its direct owner and any other workspace where it is assigned. ## Sharing a Remote Docker host