Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ These are prescriptive rules not derivable from reading the code:

- **DM pods are strictly 1:1 (ADR-001 §3.10).** `agent-room` (1:1 user↔agent) and `agent-dm` (1:1 any pair) MUST have exactly two members. Single source of truth: `agentIdentityService.DM_POD_TYPES_GUARD = {'agent-room', 'agent-dm'}`. `ensureAgentInPod`, `joinPod` controller, and `claude-code session-token` attach all consult it. **`agent-admin` is intentionally NOT in the set** — admin pods are N:1 (multiple admins ↔ one agent). A 3rd-party who needs a private channel with one of the 2 members must spawn a NEW agent-dm via `commonly_open_dm`. Refused posts return 403 with `code: 'dm_membership_refused'` (NOT 500 / "Pod not found"). Sweep scripts: `scripts/migrate-agent-{dm,room}-multimember.ts`.

- **Pod-scoped reads are membership-gated; admin moderation is a separate opt-in (PRs #375 / #377 / #378, 2026-05-15).** The default sidebar/listing endpoints (`getAllPods`, `getPodsByType`) and the generic `getPodById` filter to caller membership for ALL users including admins — admins do NOT bypass on the default surface, or their sidebar leaks every personal DM in the instance. Cross-instance moderation is an explicit `?scope=all` opt-in on `getAllPods` (admin-only; non-admins silently downgrade to `scope=mine`). Personal pod types (`agent-room`, `agent-dm`, `agent-admin`) 404 non-members on direct GET. Pod-scoped read endpoints for content — `/api/posts?podId=<x>`, `/api/posts/:id`, `/api/pods/:id/external-links`, `/api/pods/:id/announcements`, `/api/pods/:id/files`, `/api/pods/:id/children`, `/api/summaries/pod/:id` — all run through `DMService.canViewPod` (members + admins + agent-dm §3.7 fan-out; everyone else 403). Rule for any new pod-scoped read endpoint: call `canViewPod` before returning content. The §3.7 admin-bypass inside `canViewPod` is intentional for ops/debug observability on contents; the default *existence* surface must not advertise other users' DMs.
- **Pod-scoped reads are membership-gated; admin moderation is a separate opt-in (PRs #375 / #377 / #378, 2026-05-15).** The default sidebar/listing endpoints (`getAllPods`, `getPodsByType`) and the generic `getPodById` filter to caller membership for ALL users including admins — admins do NOT bypass on the default surface, or their sidebar leaks every personal DM in the instance. Cross-instance moderation is an explicit `?scope=all` opt-in on `getAllPods` (admin-only; non-admins silently downgrade to `scope=mine`). Personal pod types (`agent-room`, `agent-admin`) 404 non-members on direct GET; `agent-dm` carves out the §3.7 fan-out (PR #381, 2026-05-15) so humans sharing a pod with either agent participant can navigate to the a2a DM read-only — the V2 inspector "Direct messages" list links there. Pod-scoped read endpoints for content — `/api/posts?podId=<x>`, `/api/posts/:id`, `/api/pods/:id/external-links`, `/api/pods/:id/announcements`, `/api/pods/:id/files`, `/api/pods/:id/children`, `/api/summaries/pod/:id` — all run through `DMService.canViewPod` (members + admins + agent-dm §3.7 fan-out; everyone else 403). Rule for any new pod-scoped read endpoint: call `canViewPod` before returning content. The §3.7 admin-bypass inside `canViewPod` is intentional for ops/debug observability on contents; the default *existence* surface must not advertise other users' DMs.

- **DM display labels — never use `botMetadata.agentName`.** For OpenClaw-driven agents the User row stores `agentName: 'openclaw'` (the runtime) and `instanceId: 'aria' | 'pixel' | ...` (the actual identity). Pod names + `AgentInstallation.displayName` + chat.mention DM cues all resolve via `agentIdentityService.resolveAgentDisplayLabel(user, fallback)` with the chain: `botMetadata.displayName` → `instanceId` (when not 'default') → `username` → fallback. **Never** falls back to `botMetadata.agentName` — that produces "openclaw ↔ openclaw" pod names. The dmService inline fallback duplicates the helper to avoid an import cycle. Sweep script for stale data: `scripts/rename-agent-dm-pods.ts` (also handles `agent-room`).

Expand Down
86 changes: 86 additions & 0 deletions backend/__tests__/unit/controllers/podController.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ jest.mock('../../../models/AgentProfile', () => ({
updateOne: jest.fn(),
deleteMany: jest.fn(),
}));
jest.mock('../../../services/dmService', () => ({
__esModule: true,
default: { canViewPod: jest.fn() },
}));
jest.mock('../../../services/agentIdentityService', () => ({
getAgentTypeConfig: jest.fn(),
getOrCreateAgentUser: jest.fn(),
Expand Down Expand Up @@ -374,4 +378,86 @@ describe('podController', () => {
await podController.getPodById(req, res);
expect(res.json).toHaveBeenCalledWith(pod);
});

// ── Gap 2 (PR #381): agent-dm §3.7 fan-out carve-out for direct ID lookup ─
// PR #375 made getPodById 404 non-members of all personal pod types so
// admins couldn't accidentally land in someone else's DM via the sidebar.
// But agent-dm specifically has §3.7 fan-out: humans who share a pod with
// either DM member can observe (canViewPod returns true). Without this
// carve-out, V2's "Direct messages" list in the inspector navigates to
// /v2/pods/<a2a-dm-id> and the layout 404s — user reports "clicking the
// DM row doesn't open the pod." Carve-out is agent-dm only — agent-room
// (user↔agent) and agent-admin (ops) keep strict membership.
it('getPodById allows agent-dm read when canViewPod (§3.7 fan-out) allows', async () => {
const DMService = require('../../../services/dmService').default;
DMService.canViewPod.mockResolvedValueOnce(true);

const pod = {
_id: 'a2a-dm-1',
type: 'agent-dm',
members: [{ _id: 'agent-a' }, { _id: 'agent-b' }],
};
Pod.findById.mockReturnValue({
populate: jest.fn().mockReturnValue({
populate: jest.fn().mockReturnValue({
populate: jest.fn().mockResolvedValue(pod),
}),
}),
});
const req = {
params: { id: 'a2a-dm-1' },
userId: 'observer-human',
user: {},
};
const res = { status: jest.fn().mockReturnThis(), json: jest.fn() };
await podController.getPodById(req, res);
expect(res.json).toHaveBeenCalledWith(pod);
expect(res.status).not.toHaveBeenCalledWith(404);
});

it('getPodById still 404s agent-dm when canViewPod denies (no shared pod, not admin)', async () => {
const DMService = require('../../../services/dmService').default;
DMService.canViewPod.mockResolvedValueOnce(false);

const pod = {
_id: 'a2a-dm-2',
type: 'agent-dm',
members: [{ _id: 'agent-a' }, { _id: 'agent-b' }],
};
Pod.findById.mockReturnValue({
populate: jest.fn().mockReturnValue({
populate: jest.fn().mockReturnValue({
populate: jest.fn().mockResolvedValue(pod),
}),
}),
});
const req = {
params: { id: 'a2a-dm-2' },
userId: 'stranger',
user: {},
};
const res = { status: jest.fn().mockReturnThis(), json: jest.fn() };
await podController.getPodById(req, res);
expect(res.status).toHaveBeenCalledWith(404);
});

it('getPodById keeps strict membership 404 for agent-room (no §3.7 fan-out)', async () => {
const pod = {
_id: 'agent-room-x',
type: 'agent-room',
members: [{ _id: 'agent-id' }, { _id: 'someone-else' }],
};
Pod.findById.mockReturnValue({
populate: jest.fn().mockReturnValue({
populate: jest.fn().mockReturnValue({
populate: jest.fn().mockResolvedValue(pod),
}),
}),
});
const req = { params: { id: 'agent-room-x' }, userId: 'observer', user: {} };
const res = { status: jest.fn().mockReturnThis(), json: jest.fn() };
await podController.getPodById(req, res);
// agent-room never gets the §3.7 carve-out — strict membership only.
expect(res.status).toHaveBeenCalledWith(404);
});
});
28 changes: 22 additions & 6 deletions backend/controllers/podController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -289,19 +289,35 @@ exports.getPodById = async (req: any, res: any) => {
.json({ error: 'Pod not found or is not of specified type' });
}

// Personal pod types are private 1:1/N:1 surfaces. Only members can
// read them — direct ID lookups by non-members 404 to avoid leaking
// existence and to keep admins from accidentally landing in another
// user's DM (the sidebar bug that surfaced this fix). Admins can
// still moderate via dedicated admin tooling, not this generic GET.
// Personal pod types are private 1:1/N:1 surfaces. Direct ID lookups
// by non-members 404 to avoid leaking existence (the sidebar bug that
// surfaced PR #375). Two carve-outs that match canViewPod semantics:
// - `agent-dm` (agent ↔ agent): humans who share a pod with either
// DM participant get read access via the §3.7 fan-out — they can
// observe their team's agent collaborations without being a
// formal DM member. Without this carve-out a user can't navigate
// to an a2a DM from the agent inspector even though messages
// under /api/messages/:podId already serve them (canViewPod).
// - Global admins reading agent-dm — moderation observability.
// `agent-room` and `agent-admin` keep strict membership: those are
// truly private surfaces (user↔agent rooms; ops admin pods) and we
// don't want admins resolving someone else's agent-room by ID either.
const personalTypes = new Set(['agent-room', 'agent-dm', 'agent-admin']);
if (personalTypes.has(pod.type) && req.userId) {
const uid = String(req.userId);
const isMember = (pod.members || []).some(
(m: any) => String(m?._id || m) === uid,
);
if (!isMember) {
return res.status(404).json({ error: 'Pod not found' });
if (pod.type === 'agent-dm') {
// eslint-disable-next-line @typescript-eslint/no-require-imports, global-require
const DMService = require('../services/dmService').default
|| require('../services/dmService');
const allowed = await DMService.canViewPod(uid, pod);
if (!allowed) return res.status(404).json({ error: 'Pod not found' });
} else {
return res.status(404).json({ error: 'Pod not found' });
}
}
}

Expand Down
Loading