Skip to content

Commit a9e4c36

Browse files
authored
fix: canonicalize personal chat scope (#694)
1 parent 40f1378 commit a9e4c36

2 files changed

Lines changed: 110 additions & 14 deletions

File tree

src/app/api/chat/route.test.ts

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,11 @@ vi.mock("@/lib/agent-mode-diagnostics", () => ({
7070
import { PolicyViolation } from "@/lib/collaboration-policy";
7171
import { POST } from "./route";
7272

73-
function makeRequest(body: Record<string, unknown>) {
73+
function makeRequest(body: Record<string, unknown>, headers: Record<string, string> = {}) {
7474
return new NextRequest(new URL("/api/chat", "http://localhost:3000"), {
7575
method: "POST",
7676
body: JSON.stringify(body),
77-
headers: { "Content-Type": "application/json" },
77+
headers: { "Content-Type": "application/json", ...headers },
7878
});
7979
}
8080

@@ -163,6 +163,82 @@ describe("POST /api/chat", () => {
163163
expect(mockGetGatewayClient).not.toHaveBeenCalled();
164164
});
165165

166+
it("keeps explicit personal workspace scope when the company cookie is stale", async () => {
167+
mockResolveAccessibleWorkspace.mockResolvedValueOnce({
168+
id: "workspace-personal",
169+
companyId: null,
170+
});
171+
172+
const response = await POST(makeRequest({
173+
messages: [{ role: "user", content: "hello" }],
174+
agent: "main",
175+
companyId: null,
176+
workspaceId: "workspace-personal",
177+
}, {
178+
Cookie: "active_company=company-stale; active_workspace=workspace-personal",
179+
}));
180+
181+
expect(response.status).toBe(200);
182+
expect(mockResolveAccessibleWorkspace).toHaveBeenCalledWith(expect.objectContaining({
183+
explicitCompanyId: null,
184+
explicitWorkspaceId: "workspace-personal",
185+
requireExplicitForBearer: true,
186+
}));
187+
expect(mockAssertPrimaryRuntimeInvocationAllowedForContext).toHaveBeenCalledWith(expect.objectContaining({
188+
companyId: null,
189+
workspaceId: "workspace-personal",
190+
}));
191+
192+
await readFirstChunk(response);
193+
});
194+
195+
it("keeps explicit company scope when the workspace cookie is stale", async () => {
196+
mockResolveAccessibleWorkspace.mockResolvedValueOnce({
197+
id: "workspace-company",
198+
companyId: "company-1",
199+
});
200+
201+
const response = await POST(makeRequest({
202+
messages: [{ role: "user", content: "hello" }],
203+
agent: "main",
204+
companyId: "company-1",
205+
}, {
206+
Cookie: "active_company=company-1; active_workspace=workspace-personal-stale",
207+
}));
208+
209+
expect(response.status).toBe(200);
210+
expect(mockResolveAccessibleWorkspace).toHaveBeenCalledWith(expect.objectContaining({
211+
explicitCompanyId: "company-1",
212+
explicitWorkspaceId: null,
213+
requireExplicitForBearer: true,
214+
}));
215+
expect(mockAssertPrimaryRuntimeInvocationAllowedForContext).toHaveBeenCalledWith(expect.objectContaining({
216+
companyId: "company-1",
217+
workspaceId: "workspace-company",
218+
}));
219+
220+
await readFirstChunk(response);
221+
});
222+
223+
it("rejects conflicting explicit company and workspace scopes", async () => {
224+
mockResolveAccessibleWorkspace.mockResolvedValueOnce({
225+
id: "workspace-personal",
226+
companyId: null,
227+
});
228+
229+
const response = await POST(makeRequest({
230+
messages: [{ role: "user", content: "hello" }],
231+
agent: "main",
232+
companyId: "company-1",
233+
workspaceId: "workspace-personal",
234+
}));
235+
236+
expect(response.status).toBe(403);
237+
await expect(response.json()).resolves.toEqual({ error: "Forbidden" });
238+
expect(mockAssertPrimaryRuntimeInvocationAllowedForContext).not.toHaveBeenCalled();
239+
expect(mockGetGatewayClient).not.toHaveBeenCalled();
240+
});
241+
166242
it("rejects shared-context chat when the selected OpenClaw runtime is personal", async () => {
167243
mockAssertPrimaryRuntimeInvocationAllowedForContext.mockRejectedValueOnce(new PolicyViolation({
168244
allowed: false,

src/app/api/chat/route.ts

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,19 +1000,27 @@ export async function POST(request: NextRequest) {
10001000
.map((message: { content?: unknown }) => asString(message.content))
10011001
.filter((content: string | null): content is string => Boolean(content));
10021002

1003-
// Resolve durable chat scope from body or cookies. Company scope is used
1004-
// for org workspaces; workspace scope keeps personal workspace chat durable.
1005-
const companyId = bodyCompanyId ||
1006-
request.cookies.get("active_company")?.value ||
1007-
null;
1008-
const workspaceId = bodyWorkspaceId ||
1009-
request.cookies.get("active_workspace")?.value ||
1010-
null;
1003+
// Explicit request scope is authoritative, including null values. Cookies
1004+
// are only a fallback when the caller did not send either scope field.
1005+
const hasExplicitCompanyId = Object.prototype.hasOwnProperty.call(body, "companyId");
1006+
const hasExplicitWorkspaceId = Object.prototype.hasOwnProperty.call(body, "workspaceId");
1007+
const hasExplicitScope = hasExplicitCompanyId || hasExplicitWorkspaceId;
1008+
const explicitCompanyId = hasExplicitCompanyId ? asString(bodyCompanyId) : null;
1009+
const explicitWorkspaceId = hasExplicitWorkspaceId ? asString(bodyWorkspaceId) : null;
1010+
let companyId = hasExplicitCompanyId
1011+
? explicitCompanyId
1012+
: hasExplicitScope
1013+
? null
1014+
: asString(request.cookies.get("active_company")?.value);
1015+
let workspaceId = hasExplicitWorkspaceId
1016+
? explicitWorkspaceId
1017+
: hasExplicitScope
1018+
? null
1019+
: asString(request.cookies.get("active_workspace")?.value);
10111020
const channelId = typeof bodyChannelId === "string" && bodyChannelId.trim() ? bodyChannelId.trim() : null;
10121021
const channelInvocationMode = firstString(bodyChannelInvocationMode)?.toLowerCase() === "mention"
10131022
? "mention"
10141023
: "active";
1015-
const persistenceScope: ChatPersistenceScope = { companyId, workspaceId, channelId };
10161024
if (companyId || workspaceId) {
10171025
const accessibleWorkspace = await resolveAccessibleWorkspace({
10181026
request,
@@ -1023,17 +1031,28 @@ export async function POST(request: NextRequest) {
10231031
if (!accessibleWorkspace) {
10241032
return Response.json({ error: "Forbidden" }, { status: 403 });
10251033
}
1034+
if (
1035+
(explicitCompanyId && accessibleWorkspace.companyId !== explicitCompanyId) ||
1036+
(explicitWorkspaceId && accessibleWorkspace.id !== explicitWorkspaceId)
1037+
) {
1038+
return Response.json({ error: "Forbidden" }, { status: 403 });
1039+
}
1040+
1041+
// Downstream policy and persistence must use one canonical scope rather
1042+
// than a potentially contradictory mix of body and cookie identifiers.
1043+
companyId = accessibleWorkspace.companyId;
1044+
workspaceId = accessibleWorkspace.id;
10261045
if (channelId && !(await canAccessChatSession(request, {
1027-
companyId: accessibleWorkspace.companyId,
1028-
workspaceId: accessibleWorkspace.id,
1046+
companyId,
1047+
workspaceId,
10291048
channelId,
10301049
}))) {
10311050
return Response.json({ error: "Forbidden" }, { status: 403 });
10321051
}
10331052
if (channelId) {
10341053
const violation = await resolveChannelAgentInvocationViolation({
10351054
channelId,
1036-
companyId: accessibleWorkspace.companyId,
1055+
companyId,
10371056
agentCallsign: targetAgentCallsign || agentId,
10381057
agentMode: bodyAgentMode === true,
10391058
invocationMode: channelInvocationMode,
@@ -1044,6 +1063,7 @@ export async function POST(request: NextRequest) {
10441063
}
10451064
}
10461065
}
1066+
const persistenceScope: ChatPersistenceScope = { companyId, workspaceId, channelId };
10471067
const currentUser = await resolveCurrentUser(request);
10481068
try {
10491069
await assertPrimaryRuntimeInvocationAllowedForContext({

0 commit comments

Comments
 (0)