fix(acp): /acp sessions exposes every gateway session to non-owner senders (#110745)

* fix(acp): scope /acp sessions listing for non-owner senders

/acp sessions listed every ACP session on the gateway for any sender
allowlisted via commands.allowFrom, exposing other senders' session
labels, agent ids, runtime state, and thread bindings. The handler now
returns only the current bound or requester session for non-owner
senders, while owner identity and operator.admin clients keep the full
gateway-wide listing, matching the documented contract.

Fixes #103055

* test(acp): cover empty and missing-session cases for /acp sessions scoping

* fix(acp): avoid non-owner session scans

* docs(acp): remove duplicated session scope text

* test(acp): cover internal session visibility

* fix(acp): require current ACP metadata

* test(acp): reject sessions target tokens

---------

Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
Yuval Dinodia
2026-07-18 20:38:48 -04:00
committed by GitHub
parent c745e7d66c
commit 0a588fa795
3 changed files with 135 additions and 20 deletions
+7 -5
View File
@@ -790,14 +790,16 @@ Runtime controls (`spawn`, `cancel`, `steer`, `close`, `status`, `set-mode`,
`set`, `cwd`, `permissions`, `timeout`, `model`, and `reset-options`) require
owner identity from external channels and `operator.admin` from internal
Gateway clients. Authorized non-owner senders can still use `sessions`,
`doctor`, `install`, and `help`.
`doctor`, `install`, and `help`. For non-owner senders, `/acp sessions`
lists only the current bound or requester session; owner identity and
`operator.admin` clients see all recent sessions.
`/acp status` shows the effective runtime options plus runtime-level and
backend-level session identifiers. Unsupported-control errors surface
clearly when a backend lacks a capability. `/acp sessions` reads the store
for the current bound or requester session; target tokens (`session-key`,
`session-id`, or `session-label`) resolve through gateway session discovery,
including custom per-agent `session.store` roots.
clearly when a backend lacks a capability. Commands that accept target tokens
(`session-key`, `session-id`, or `session-label`) resolve them through gateway
session discovery, including custom per-agent `session.store` roots. `/acp sessions`
does not accept a target token.
### Runtime options mapping
+118 -12
View File
@@ -1910,6 +1910,105 @@ describe("/acp command", () => {
expect(result?.reply?.text).toContain(`thread:${defaultThreadId}`);
});
it("lists all stored ACP sessions for the owner", async () => {
hoisted.sessionBindingResolveByConversationMock.mockReturnValue(
createBoundThreadSession("agent:codex:acp:own"),
);
hoisted.listAcpSessionEntriesMock.mockResolvedValue([
createAcpSessionEntry({ sessionKey: "agent:codex:acp:own" }),
createAcpSessionEntry({ sessionKey: "agent:claude:acp:foreign" }),
]);
const result = await runDiscordAcpCommand("/acp sessions", baseCfg);
expect(result?.reply?.text).toContain("agent:codex:acp:own");
expect(result?.reply?.text).toContain("agent:claude:acp:foreign");
expect(hoisted.readAcpSessionEntryMock).not.toHaveBeenCalled();
});
it("lists only the current raw ACP session for an authorized non-owner sender", async () => {
const currentSessionKey = "agent:codex:acp:current";
hoisted.readAcpSessionEntryMock.mockReturnValue(
createAcpSessionEntry({ sessionKey: currentSessionKey }),
);
const params = createDiscordParams("/acp sessions");
params.command.senderIsOwner = false;
params.sessionKey = currentSessionKey;
const result = await handleAcpCommand(params, true);
expect(result?.reply?.text).toContain(currentSessionKey);
expect(hoisted.readAcpSessionEntryMock).toHaveBeenCalledWith({
cfg: baseCfg,
sessionKey: currentSessionKey,
});
expect(hoisted.listAcpSessionEntriesMock).not.toHaveBeenCalled();
});
it("prefers the bound-thread ACP session over a non-owner sender's raw session key", async () => {
const boundSessionKey = "agent:codex:acp:bound";
hoisted.sessionBindingResolveByConversationMock.mockReturnValue(
createBoundThreadSession(boundSessionKey),
);
hoisted.readAcpSessionEntryMock.mockReturnValue(
createAcpSessionEntry({ sessionKey: boundSessionKey }),
);
const params = createDiscordParams("/acp sessions");
params.command.senderIsOwner = false;
params.sessionKey = "agent:main:raw-requester";
const result = await handleAcpCommand(params, true);
expect(result?.reply?.text).toContain(boundSessionKey);
expect(result?.reply?.text).not.toContain("agent:main:raw-requester");
expect(hoisted.readAcpSessionEntryMock).toHaveBeenCalledWith({
cfg: baseCfg,
sessionKey: boundSessionKey,
});
expect(hoisted.listAcpSessionEntriesMock).not.toHaveBeenCalled();
});
it("returns an empty listing when a non-owner's raw session key is not an ACP session", async () => {
hoisted.readAcpSessionEntryMock.mockReturnValue({
...createAcpSessionEntry({ sessionKey: "agent:main:raw-requester" }),
acp: undefined,
});
const params = createDiscordParams("/acp sessions");
params.command.senderIsOwner = false;
params.sessionKey = "agent:main:raw-requester";
const result = await handleAcpCommand(params, true);
expect(result?.reply?.text).toContain("(none)");
expect(hoisted.readAcpSessionEntryMock).toHaveBeenCalledWith({
cfg: baseCfg,
sessionKey: "agent:main:raw-requester",
});
expect(hoisted.listAcpSessionEntriesMock).not.toHaveBeenCalled();
});
it("warns when no session key resolves for /acp sessions", async () => {
hoisted.listAcpSessionEntriesMock.mockResolvedValue([createAcpSessionEntry()]);
const params = createDiscordParams("/acp sessions");
params.command.senderIsOwner = false;
params.sessionKey = "";
const result = await handleAcpCommand(params, true);
expect(result?.reply?.text).toContain("Missing session key");
});
it("rejects explicit target tokens for /acp sessions", async () => {
const params = createDiscordParams("/acp sessions agent:claude:acp:foreign");
params.command.senderIsOwner = false;
const result = await handleAcpCommand(params, true);
expect(result?.reply?.text).toBe("Usage: /acp sessions");
expect(hoisted.readAcpSessionEntryMock).not.toHaveBeenCalled();
expect(hoisted.listAcpSessionEntriesMock).not.toHaveBeenCalled();
});
it("shows ACP status for the thread-bound ACP session", async () => {
mockBoundThreadSession({
identity: {
@@ -2088,17 +2187,7 @@ describe("/acp command", () => {
});
it("keeps read-only /acp actions available to internal operator.write clients", async () => {
hoisted.listAcpSessionEntriesMock.mockResolvedValue([
createAcpSessionEntry({
identity: {
state: "resolved",
source: "status",
acpxSessionId: "runtime-1",
agentSessionId: "session-1",
lastUpdatedAt: Date.now(),
},
}),
]);
hoisted.readAcpSessionEntryMock.mockReturnValue(createAcpSessionEntry());
const result = await runInternalAcpCommand({
commandBody: "/acp sessions",
@@ -2106,7 +2195,24 @@ describe("/acp command", () => {
});
expect(result?.shouldContinue).toBe(false);
expect(result?.reply?.text).toContain("ACP sessions");
expect(result?.reply?.text).toContain(defaultAcpSessionKey);
expect(hoisted.listAcpSessionEntriesMock).not.toHaveBeenCalled();
});
it("lists all ACP sessions for internal operator.admin clients", async () => {
hoisted.listAcpSessionEntriesMock.mockResolvedValue([
createAcpSessionEntry({ sessionKey: "agent:codex:acp:own" }),
createAcpSessionEntry({ sessionKey: "agent:claude:acp:foreign" }),
]);
const result = await runInternalAcpCommand({
commandBody: "/acp sessions",
scopes: ["operator.admin"],
});
expect(result?.reply?.text).toContain("agent:codex:acp:own");
expect(result?.reply?.text).toContain("agent:claude:acp:foreign");
expect(hoisted.readAcpSessionEntryMock).not.toHaveBeenCalled();
});
it("allows mutating /acp actions for internal operator.admin clients", async () => {
@@ -7,7 +7,7 @@ import {
import { getAcpSessionManager } from "../../../acp/control-plane/manager.js";
import { toAcpRuntimeError } from "../../../acp/runtime/errors.js";
import { getAcpRuntimeBackend, requireAcpRuntimeBackend } from "../../../acp/runtime/registry.js";
import { listAcpSessionEntries } from "../../../acp/runtime/session-meta.js";
import { listAcpSessionEntries, readAcpSessionEntry } from "../../../acp/runtime/session-meta.js";
import type { SessionEntry } from "../../../config/sessions/types.js";
import type { SessionAcpMeta } from "../../../config/sessions/types.js";
import { getSessionBindingService } from "../../../infra/outbound/session-binding-service.js";
@@ -190,9 +190,16 @@ export async function handleAcpSessionsAction(
const normalizedChannel = bindingContext.channel;
const normalizedAccountId = bindingContext.accountId || undefined;
const bindingService = getSessionBindingService();
const entries = await listAcpSessionEntries({ cfg: params.cfg });
const currentEntry = params.command.senderIsOwner
? null
: readAcpSessionEntry({ cfg: params.cfg, sessionKey: currentSessionKey });
const visibleEntries = params.command.senderIsOwner
? await listAcpSessionEntries({ cfg: params.cfg })
: currentEntry?.entry && currentEntry.acp
? [currentEntry]
: [];
const rows = entries
const rows = visibleEntries
.toSorted((a, b) => (b.entry?.updatedAt ?? 0) - (a.entry?.updatedAt ?? 0))
.slice(0, 20)
.map(({ storeSessionKey, entry, acp }) => {