Skip to content

Commit 9fe6ecd

Browse files
steipetesteipete
committed
feat(gateway): let write-scope operators manage chat session organization
sessions.patch becomes a params-aware dynamic-scope method: operator.write now authorizes patches that touch only user-level chat-organization fields (label, category, pinned, archived, unread); every other field, mixed patches, and unknown keys keep requiring operator.admin (fail closed). This unblocks the session controls shipped in #100814 for the Android app, whose bounded operator session intentionally never requests operator.admin. Also: session list ordering gains a deterministic key tiebreaker for equal pinnedAt/updatedAt (stable offset paging and prompt-cache friendliness), a user-assigned label now beats stored channel-derived display names in the row projection so renames survive refreshes, and subagent spawn pins its gateway calls to admin via the params-aware least-privilege resolver instead of the static admin-only method check (#59428 contract preserved). Refs #100712
1 parent 7cdbfc9 commit 9fe6ecd

8 files changed

Lines changed: 259 additions & 14 deletions

‎src/agents/subagent-spawn.runtime.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@ export {
2222
dispatchGatewayMethodInProcess,
2323
hasInProcessGatewayContext,
2424
} from "../gateway/server-plugins.js";
25-
export { ADMIN_SCOPE, isAdminOnlyMethod } from "../gateway/method-scopes.js";
25+
export {
26+
ADMIN_SCOPE,
27+
resolveLeastPrivilegeOperatorScopesForMethod,
28+
} from "../gateway/method-scopes.js";
2629
export { getSessionBindingService } from "../infra/outbound/session-binding-service.js";
2730
export {
2831
pruneLegacyStoreKeys,

‎src/agents/subagent-spawn.test-helpers.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import os from "node:os";
44
import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce";
55
import { expect, vi } from "vitest";
6+
import { resolveLeastPrivilegeOperatorScopesForMethod } from "../gateway/method-scopes.js";
67
import type { SubagentLifecycleHookRunner } from "../plugins/hooks.js";
78

89
type MockFn = (...args: unknown[]) => unknown;
@@ -303,8 +304,9 @@ export async function loadSubagentSpawnModuleForTest(params: {
303304
await mutator(store);
304305
return store;
305306
}),
306-
isAdminOnlyMethod: (method: string) =>
307-
method === "sessions.patch" || method === "sessions.delete",
307+
// Real scope resolver: spawn's admin-tier pinning depends on params-aware
308+
// sessions.patch policy, so a stub here would hide policy regressions.
309+
resolveLeastPrivilegeOperatorScopesForMethod,
308310
pruneLegacyStoreKeys: (...args: unknown[]) => params.pruneLegacyStoreKeysMock?.(...args),
309311
getSessionBindingService:
310312
params.getSessionBindingService ??

‎src/agents/subagent-spawn.ts‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ import {
106106
resolveMainSessionAlias,
107107
resolveSandboxRuntimeStatus,
108108
updateSessionStore,
109-
isAdminOnlyMethod,
109+
resolveLeastPrivilegeOperatorScopesForMethod,
110110
} from "./subagent-spawn.runtime.js";
111111
import type {
112112
SpawnSubagentContextMode,
@@ -242,9 +242,15 @@ async function callSubagentGateway(
242242
// scope-upgrade handshake that headless gateway-client connections cannot
243243
// complete interactively, causing close(1008) "pairing required" (#59428).
244244
//
245-
// Only admin-only methods are pinned to ADMIN_SCOPE; other methods (e.g.
246-
// "agent" -> write) keep their least-privilege scope.
247-
const scopes = params.scopes ?? (isAdminOnlyMethod(params.method) ? [ADMIN_SCOPE] : undefined);
245+
// Only admin-requiring calls are pinned to ADMIN_SCOPE; other methods (e.g.
246+
// "agent" -> write) keep their least-privilege scope. The params-aware
247+
// resolver keeps spawn-metadata sessions.patch calls on the admin tier.
248+
const leastPrivilegeScopes = resolveLeastPrivilegeOperatorScopesForMethod(
249+
params.method,
250+
params.params,
251+
);
252+
const scopes =
253+
params.scopes ?? (leastPrivilegeScopes.includes(ADMIN_SCOPE) ? [ADMIN_SCOPE] : undefined);
248254
const request = {
249255
...params,
250256
...(scopes != null ? { scopes } : {}),

‎src/gateway/method-scopes.test.ts‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,68 @@ describe("method scope resolution", () => {
181181
).toEqual({ allowed: false, missingScope: "operator.approvals" });
182182
});
183183

184+
it("resolves sessions.patch to write scope for chat-organization fields only", () => {
185+
expect(
186+
resolveLeastPrivilegeOperatorScopesForMethod("sessions.patch", {
187+
key: "agent:main:ios-1",
188+
label: "Trip planning",
189+
pinned: true,
190+
archived: false,
191+
}),
192+
).toEqual(["operator.write"]);
193+
expect(
194+
resolveLeastPrivilegeOperatorScopesForMethod("sessions.patch", {
195+
key: "agent:main:ios-1",
196+
agentId: "main",
197+
category: "Travel",
198+
unread: true,
199+
}),
200+
).toEqual(["operator.write"]);
201+
expect(isGatewayMethodClassified("sessions.patch")).toBe(true);
202+
});
203+
204+
it.each([
205+
["model", { key: "agent:main:ios-1", model: "anthropic/claude-sonnet-5" }],
206+
["sendPolicy", { key: "agent:main:ios-1", sendPolicy: "deny" }],
207+
["inheritedToolAllow", { key: "agent:main:ios-1", inheritedToolAllow: ["exec"] }],
208+
["spawnedBy", { key: "agent:main:ios-1", spawnedBy: "agent:main:main" }],
209+
["mixed with safe fields", { key: "agent:main:ios-1", label: "x", execHost: "node-1" }],
210+
["unknown fields", { key: "agent:main:ios-1", futureField: true }],
211+
])("keeps sessions.patch admin-only when params include %s", (_name, params) => {
212+
expect(resolveLeastPrivilegeOperatorScopesForMethod("sessions.patch", params)).toEqual([
213+
"operator.admin",
214+
]);
215+
expect(authorizeOperatorScopesForMethod("sessions.patch", ["operator.write"], params)).toEqual({
216+
allowed: false,
217+
missingScope: "operator.admin",
218+
});
219+
expect(authorizeOperatorScopesForMethod("sessions.patch", ["operator.admin"], params)).toEqual({
220+
allowed: true,
221+
});
222+
});
223+
224+
it("authorizes write-scoped sessions.patch for chat-organization fields and denies read scope", () => {
225+
const params = { key: "agent:main:ios-1", label: "Trip planning", pinned: true };
226+
expect(authorizeOperatorScopesForMethod("sessions.patch", ["operator.write"], params)).toEqual({
227+
allowed: true,
228+
});
229+
expect(authorizeOperatorScopesForMethod("sessions.patch", ["operator.read"], params)).toEqual({
230+
allowed: false,
231+
missingScope: "operator.write",
232+
});
233+
});
234+
235+
it("lets malformed sessions.patch params through to handler validation at write scope", () => {
236+
// Malformed params cannot mutate anything; the handler rejects them with a
237+
// precise validation error instead of a misleading missing-scope error.
238+
expect(authorizeOperatorScopesForMethod("sessions.patch", ["operator.write"])).toEqual({
239+
allowed: true,
240+
});
241+
expect(resolveLeastPrivilegeOperatorScopesForMethod("sessions.patch")).toEqual([
242+
"operator.write",
243+
]);
244+
});
245+
184246
it("falls back to broad operator scopes when a dynamic session action is not locally registered", () => {
185247
expect(
186248
resolveLeastPrivilegeOperatorScopesForMethod("plugins.sessionAction", {

‎src/gateway/method-scopes.ts‎

Lines changed: 45 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,31 @@ export function resolveRequiredOperatorScopeForMethod(method: string): OperatorS
7878
return resolveScopedMethod(method);
7979
}
8080

81+
/**
82+
* sessions.patch fields a write-scoped operator may mutate: user-level chat
83+
* organization only. Any other field (model, sendPolicy, tool inheritance,
84+
* exec routing, ...) keeps requiring operator.admin — fail closed on unknowns.
85+
*/
86+
const SESSIONS_PATCH_WRITE_SCOPE_FIELDS: ReadonlySet<string> = new Set([
87+
"key",
88+
"agentId",
89+
"label",
90+
"category",
91+
"pinned",
92+
"archived",
93+
"unread",
94+
]);
95+
96+
function resolveSessionsPatchRequiredScopes(params: unknown): OperatorScope[] {
97+
if (!params || typeof params !== "object" || Array.isArray(params)) {
98+
// Malformed params cannot mutate anything; let the handler return the
99+
// precise validation error instead of a misleading missing-scope error.
100+
return [WRITE_SCOPE];
101+
}
102+
const safeOnly = Object.keys(params).every((key) => SESSIONS_PATCH_WRITE_SCOPE_FIELDS.has(key));
103+
return safeOnly ? [WRITE_SCOPE] : [ADMIN_SCOPE];
104+
}
105+
81106
function resolveSessionActionRegisteredScopes(params: unknown): OperatorScope[] | undefined {
82107
if (!params || typeof params !== "object" || Array.isArray(params)) {
83108
return undefined;
@@ -127,9 +152,21 @@ function resolveDynamicLeastPrivilegeOperatorScopesForMethod(
127152
if (method === "plugins.sessionAction") {
128153
return resolveSessionActionLeastPrivilegeScopes(params);
129154
}
155+
if (method === "sessions.patch") {
156+
return resolveSessionsPatchRequiredScopes(params);
157+
}
130158
return [WRITE_SCOPE];
131159
}
132160

161+
function findMissingOperatorScope(
162+
requiredScopes: readonly OperatorScope[],
163+
scopes: readonly string[],
164+
): OperatorScope | undefined {
165+
return requiredScopes.find((scope) => {
166+
return !scopes.includes(scope) && !(scope === READ_SCOPE && scopes.includes(WRITE_SCOPE));
167+
});
168+
}
169+
133170
/** Returns the narrowest known operator scopes needed to call a gateway method. */
134171
export function resolveLeastPrivilegeOperatorScopesForMethod(
135172
method: string,
@@ -156,6 +193,13 @@ export function authorizeOperatorScopesForMethod(
156193
return { allowed: true };
157194
}
158195
if (isDynamicOperatorGatewayMethod(method)) {
196+
if (method === "sessions.patch") {
197+
const missingScope = findMissingOperatorScope(
198+
resolveSessionsPatchRequiredScopes(params),
199+
scopes,
200+
);
201+
return missingScope ? { allowed: false, missingScope } : { allowed: true };
202+
}
159203
const registeredScopes = resolveSessionActionRegisteredScopes(params);
160204
if (!registeredScopes && params && typeof params === "object" && !Array.isArray(params)) {
161205
const pluginId = normalizeSessionActionParam((params as { pluginId?: unknown }).pluginId);
@@ -168,10 +212,7 @@ export function authorizeOperatorScopesForMethod(
168212
: { allowed: false, missingScope: WRITE_SCOPE };
169213
}
170214
}
171-
const requiredScopes = registeredScopes ?? [WRITE_SCOPE];
172-
const missingScope = requiredScopes.find((scope) => {
173-
return !scopes.includes(scope) && !(scope === READ_SCOPE && scopes.includes(WRITE_SCOPE));
174-
});
215+
const missingScope = findMissingOperatorScope(registeredScopes ?? [WRITE_SCOPE], scopes);
175216
return missingScope ? { allowed: false, missingScope } : { allowed: true };
176217
}
177218
const requiredScope = resolveRequiredOperatorScopeForMethod(method) ?? ADMIN_SCOPE;

‎src/gateway/methods/core-descriptors.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,10 @@ export const CORE_GATEWAY_METHOD_SPECS: readonly CoreGatewayMethodSpec[] = [
162162
{ name: "sessions.create", scope: "operator.write", startup: true },
163163
{ name: "sessions.send", scope: "operator.write", startup: true },
164164
{ name: "sessions.abort", scope: "operator.write", startup: true },
165-
{ name: "sessions.patch", scope: "operator.admin" },
165+
// Params-aware: write scope may mutate chat-organization fields
166+
// (label/category/pinned/archived/unread); every other patch field stays
167+
// admin-only. Policy lives in method-scopes.ts.
168+
{ name: "sessions.patch", scope: "dynamic" },
166169
{ name: "sessions.pluginPatch", scope: "operator.admin" },
167170
{ name: "sessions.cleanup", scope: "operator.admin" },
168171
{ name: "sessions.reset", scope: "operator.admin" },

‎src/gateway/server.sessions.store-rpc.test.ts‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -673,3 +673,123 @@ test("sessions.list hides phantom agent store placeholder rows", async () => {
673673
expect(listed.ok).toBe(true);
674674
expect(listed.payload?.sessions.map((session) => session.key)).toEqual(["agent:main:main"]);
675675
});
676+
677+
test("write-scoped operators manage chat organization but not admin session settings", async () => {
678+
await createSessionStoreDir();
679+
const now = Date.now();
680+
await writeSessionStore({
681+
entries: {
682+
main: { sessionId: "sess-main", updatedAt: now },
683+
"topic-a": {
684+
sessionId: "sess-topic-a",
685+
updatedAt: now - 60_000,
686+
// Stored channel-derived name; a user rename (label) must beat it.
687+
displayName: "channel topic",
688+
},
689+
"topic-b": { sessionId: "sess-topic-b", updatedAt: now - 30_000 },
690+
},
691+
});
692+
693+
const { ws } = await openClient({ scopes: ["operator.read", "operator.write"] });
694+
try {
695+
const renamed = await rpcReq<{ ok: true; entry: { label?: string } }>(ws, "sessions.patch", {
696+
key: "agent:main:topic-a",
697+
label: "Trip planning",
698+
});
699+
expect(renamed.ok).toBe(true);
700+
expect(renamed.payload?.entry.label).toBe("Trip planning");
701+
702+
const pinned = await rpcReq<{ ok: true; entry: { pinnedAt?: number } }>(ws, "sessions.patch", {
703+
key: "agent:main:topic-a",
704+
pinned: true,
705+
});
706+
expect(pinned.ok).toBe(true);
707+
expect(pinned.payload?.entry.pinnedAt).toEqual(expect.any(Number));
708+
709+
const organized = await rpcReq<{
710+
ok: true;
711+
entry: { category?: string; markedUnreadAt?: number };
712+
}>(ws, "sessions.patch", {
713+
key: "agent:main:topic-a",
714+
category: "Travel",
715+
unread: true,
716+
});
717+
expect(organized.ok).toBe(true);
718+
expect(organized.payload?.entry.category).toBe("Travel");
719+
720+
const archived = await rpcReq<{ ok: true; entry: { archivedAt?: number } }>(
721+
ws,
722+
"sessions.patch",
723+
{ key: "agent:main:topic-b", archived: true },
724+
);
725+
expect(archived.ok).toBe(true);
726+
expect(archived.payload?.entry.archivedAt).toEqual(expect.any(Number));
727+
728+
const searched = await rpcReq<{
729+
sessions: Array<{ key: string; pinned?: boolean; displayName?: string }>;
730+
}>(ws, "sessions.list", { search: "trip plan" });
731+
expect(searched.ok).toBe(true);
732+
expect(searched.payload?.sessions.map((session) => session.key)).toEqual([
733+
"agent:main:topic-a",
734+
]);
735+
expect(searched.payload?.sessions[0]?.displayName).toBe("Trip planning");
736+
737+
const archivedList = await rpcReq<{ sessions: Array<{ key: string }> }>(ws, "sessions.list", {
738+
archived: true,
739+
});
740+
expect(archivedList.ok).toBe(true);
741+
expect(archivedList.payload?.sessions.map((session) => session.key)).toEqual([
742+
"agent:main:topic-b",
743+
]);
744+
745+
const adminFieldDenied = await rpcReq(ws, "sessions.patch", {
746+
key: "agent:main:topic-a",
747+
sendPolicy: "deny",
748+
});
749+
expect(adminFieldDenied.ok).toBe(false);
750+
expect(adminFieldDenied.error?.message).toContain("missing scope: operator.admin");
751+
752+
const mixedFieldsDenied = await rpcReq(ws, "sessions.patch", {
753+
key: "agent:main:topic-a",
754+
label: "Sneaky",
755+
model: "anthropic/claude-sonnet-5",
756+
});
757+
expect(mixedFieldsDenied.ok).toBe(false);
758+
expect(mixedFieldsDenied.error?.message).toContain("missing scope: operator.admin");
759+
} finally {
760+
ws.close();
761+
}
762+
});
763+
764+
test("sessions.list breaks timestamp ties by key for stable paging", async () => {
765+
await createSessionStoreDir();
766+
const updatedAt = Date.now() - 5_000;
767+
await writeSessionStore({
768+
entries: {
769+
main: { sessionId: "sess-main", updatedAt },
770+
"tie-c": { sessionId: "sess-tie-c", updatedAt },
771+
"tie-a": { sessionId: "sess-tie-a", updatedAt },
772+
"tie-b": { sessionId: "sess-tie-b", updatedAt },
773+
},
774+
});
775+
776+
const expectedOrder = [
777+
"agent:main:main",
778+
"agent:main:tie-a",
779+
"agent:main:tie-b",
780+
"agent:main:tie-c",
781+
];
782+
const listed = await directSessionHandlerReq<{ sessions: Array<{ key: string }> }>(
783+
"sessions.list",
784+
{ includeGlobal: false, includeUnknown: false },
785+
);
786+
expect(listed.ok).toBe(true);
787+
expect(listed.payload?.sessions.map((session) => session.key)).toEqual(expectedOrder);
788+
789+
const paged = await directSessionHandlerReq<{ sessions: Array<{ key: string }> }>(
790+
"sessions.list",
791+
{ includeGlobal: false, includeUnknown: false, limit: 2, offset: 2 },
792+
);
793+
expect(paged.ok).toBe(true);
794+
expect(paged.payload?.sessions.map((session) => session.key)).toEqual(expectedOrder.slice(2));
795+
});

‎src/gateway/session-utils.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1898,7 +1898,10 @@ export function buildGatewaySessionRow(params: {
18981898
const origin = entry?.origin;
18991899
const originLabel = origin?.label;
19001900
const isGroupSession = isGroupOrChannelDisplaySession(entry, parsed);
1901+
// A user-assigned label is an explicit rename; it must win over stored
1902+
// channel-derived display names or renames silently vanish on refresh.
19011903
const displayName =
1904+
entry?.label ??
19021905
entry?.displayName ??
19031906
(isGroupSession && channel
19041907
? buildGroupDisplayName({
@@ -1910,7 +1913,6 @@ export function buildGatewaySessionRow(params: {
19101913
key,
19111914
})
19121915
: undefined) ??
1913-
entry?.label ??
19141916
originLabel;
19151917
const deliveryFields = normalizeSessionDeliveryFields(entry);
19161918
const parsedAgent = parseAgentSessionKey(key);
@@ -2458,7 +2460,13 @@ function compareSessionEntryPairs(a: SessionEntryPair, b: SessionEntryPair): num
24582460
if (aPinnedAt !== bPinnedAt) {
24592461
return bPinnedAt - aPinnedAt;
24602462
}
2461-
return (b[1]?.updatedAt ?? 0) - (a[1]?.updatedAt ?? 0);
2463+
const byUpdatedAt = (b[1]?.updatedAt ?? 0) - (a[1]?.updatedAt ?? 0);
2464+
if (byUpdatedAt !== 0) {
2465+
return byUpdatedAt;
2466+
}
2467+
// Timestamp ties fall back to the key so list order (and offset paging) stays
2468+
// deterministic across calls; locale-independent code-unit comparison.
2469+
return a[0] < b[0] ? -1 : a[0] > b[0] ? 1 : 0;
24622470
}
24632471

24642472
function resolveSessionsListLimit(

0 commit comments

Comments
 (0)