diff --git a/backend/__tests__/unit/routes/agentsRuntime.noInlineAvatars.test.js b/backend/__tests__/unit/routes/agentsRuntime.noInlineAvatars.test.js new file mode 100644 index 00000000..f32df668 --- /dev/null +++ b/backend/__tests__/unit/routes/agentsRuntime.noInlineAvatars.test.js @@ -0,0 +1,126 @@ +/** + * #758 — the agent runtime router must never emit an inline base64 avatar. + * + * This mounts the REAL router in an express app (rather than pulling a handler + * out of the stack) specifically so the router-level middleware runs. The + * guarantee under test is the choke point itself: individual handlers are + * allowed to hand up whole populated user documents, and the router is what + * makes that safe. A per-handler test would pass even if the middleware were + * deleted. + */ + +// Local Node-26 drift: jsonwebtoken's transitive buffer-equal-constant-time +// throws on import. CI (Node 20) is unaffected; this keeps the suite runnable +// on a dev machine without changing what is under test. +jest.mock('jsonwebtoken', () => ({ sign: jest.fn(), verify: jest.fn(), decode: jest.fn() })); + +jest.mock('../../../middleware/agentRuntimeAuth', () => (req, res, next) => { + req.agentUser = { _id: 'bot-1' }; + req.agentInstallations = [{ podId: 'pod-1', status: 'active' }]; + req.agentAuthorizedPodIds = ['pod-1']; + next(); +}); +jest.mock('../../../middleware/auth', () => (req, res, next) => next()); +jest.mock('../../../middleware/apiTokenScopes', () => ({ + requireApiTokenScopes: () => (req, res, next) => next(), +})); + +jest.mock('../../../services/agentEventService', () => ({})); +jest.mock('../../../services/agentIdentityService', () => ({ + buildAgentUsername: jest.fn((a) => a), +})); +jest.mock('../../../services/agentMessageService', () => ({ + getRecentMessages: jest.fn(), +})); +jest.mock('../../../services/agentThreadService', () => ({})); +jest.mock('../../../services/podContextService', () => ({})); +jest.mock('../../../services/globalModelConfigService', () => ({})); +jest.mock('../../../services/socialPolicyService', () => ({})); +jest.mock('../../../integrations', () => ({ get: jest.fn() })); +jest.mock('../../../models/Activity', () => ({})); +jest.mock('../../../models/User', () => ({ findById: jest.fn() })); +jest.mock('../../../models/Post', () => ({ findById: jest.fn() })); +jest.mock('../../../models/Pod', () => ({ find: jest.fn() })); +jest.mock('../../../services/dmService', () => ({ getOrCreateAgentDM: jest.fn() })); +jest.mock('../../../models/Integration', () => ({ find: jest.fn(), findOne: jest.fn() })); +jest.mock('../../../models/AgentRegistry', () => ({ + AgentInstallation: { findOne: jest.fn(), find: jest.fn() }, +})); + +const express = require('express'); +const request = require('supertest'); + +const AgentMessageService = require('../../../services/agentMessageService'); +const router = require('../../../routes/agentsRuntime'); + +const DATA_URI = `data:image/jpeg;base64,${'A'.repeat(4000)}`; + +const app = express(); +app.use(express.json()); +app.use('/api/agents/runtime', router); + +describe('agent runtime router strips inline avatars (#758)', () => { + beforeEach(() => jest.clearAllMocks()); + + test('GET /pods/:podId/messages returns no data: URI even when the service supplies one', async () => { + AgentMessageService.getRecentMessages.mockResolvedValue([ + { + _id: 'm1', + id: 'm1', + content: 'hello', + username: 'sam', + isBot: false, + self: false, + userId: { _id: 'u1', username: 'sam', profilePicture: DATA_URI }, + profile_picture: DATA_URI, + }, + ]); + + const res = await request(app).get('/api/agents/runtime/pods/pod-1/messages'); + + expect(res.status).toBe(200); + expect(JSON.stringify(res.body)).not.toContain('data:'); + // The conversation itself must survive untouched. + expect(res.body.messages[0].content).toBe('hello'); + expect(res.body.messages[0].username).toBe('sam'); + // And so must the fields the wrapper's self-post detection depends on (#757). + expect(res.body.messages[0]).toHaveProperty('isBot', false); + expect(res.body.messages[0]).toHaveProperty('self', false); + }); + + test('a URL avatar still reaches the agent — only base64 is dropped', async () => { + AgentMessageService.getRecentMessages.mockResolvedValue([ + { + _id: 'm1', + id: 'm1', + content: 'hi', + username: 'sam', + userId: { _id: 'u1', username: 'sam', profilePicture: '/api/uploads/a.png' }, + }, + ]); + + const res = await request(app).get('/api/agents/runtime/pods/pod-1/messages'); + + expect(res.status).toBe(200); + expect(res.body.messages[0].userId.profilePicture).toBe('/api/uploads/a.png'); + }); + + test('the payload actually shrinks — this is a context-window fix, not cosmetics', async () => { + AgentMessageService.getRecentMessages.mockResolvedValue( + Array.from({ length: 20 }, (_, i) => ({ + _id: `m${i}`, + id: `m${i}`, + content: 'a short line of conversation', + username: 'sam', + userId: { _id: 'u1', username: 'sam', profilePicture: DATA_URI }, + profile_picture: DATA_URI, + })), + ); + + const res = await request(app).get('/api/agents/runtime/pods/pod-1/messages'); + const size = JSON.stringify(res.body).length; + + // 20 messages x 2 copies x ~4KB would be ~160KB unstripped. + expect(size).toBeLessThan(5000); + }); +}); diff --git a/backend/__tests__/unit/services/avatarService.stripInline.test.js b/backend/__tests__/unit/services/avatarService.stripInline.test.js new file mode 100644 index 00000000..30b9f997 --- /dev/null +++ b/backend/__tests__/unit/services/avatarService.stripInline.test.js @@ -0,0 +1,94 @@ +/** + * #758 — no inline base64 avatars in agent-bound payloads. + * + * Avatars are stored as `data:` URIs on the user row, so every message carries + * its author's full image bytes and the same avatar repeats on every message + * that author sent. Measured on a real pod, a 20-message agent read returned + * 230,170 characters of which 71% was image data — an agent asked to read a + * busy room exhausts its context on profile pictures before reaching the + * conversation. + * + * The rule is "no base64 in an agent payload", NOT "no avatars": a URL costs + * nothing and stays useful, so it survives. + */ + +const { stripInlineAvatars } = require('../../../services/avatarService'); + +const DATA_URI = 'data:image/jpeg;base64,/9j/4AAQSkZJRgABAQAAAQABAAD'; + +describe('stripInlineAvatars', () => { + test('drops a data: avatar under either spelling', () => { + const out = stripInlineAvatars({ + userId: { _id: 'u1', username: 'sam', profilePicture: DATA_URI }, + profile_picture: DATA_URI, + content: 'hello', + }); + + expect(out.userId).not.toHaveProperty('profilePicture'); + expect(out).not.toHaveProperty('profile_picture'); + expect(out.content).toBe('hello'); + expect(out.userId.username).toBe('sam'); + }); + + test('keeps a URL avatar — the rule is no base64, not no avatars', () => { + const out = stripInlineAvatars({ + userId: { profilePicture: '/api/uploads/abc.png' }, + profile_picture: 'https://example.com/a.png', + }); + + expect(out.userId.profilePicture).toBe('/api/uploads/abc.png'); + expect(out.profile_picture).toBe('https://example.com/a.png'); + }); + + test('reaches into arrays and nested members (the create-pod member list)', () => { + const out = stripInlineAvatars({ + pod: { + createdBy: { username: 'sam', profilePicture: DATA_URI }, + members: [ + { user: { username: 'a', profilePicture: DATA_URI } }, + { user: { username: 'b', profilePicture: '/api/uploads/b.png' } }, + ], + }, + }); + + expect(JSON.stringify(out)).not.toContain('data:'); + expect(out.pod.members[1].user.profilePicture).toBe('/api/uploads/b.png'); + expect(out.pod.members.map((m) => m.user.username)).toEqual(['a', 'b']); + }); + + test('does NOT mutate the input — the same object feeds the human socket broadcast', () => { + const original = { userId: { profilePicture: DATA_URI }, content: 'x' }; + stripInlineAvatars(original); + + // The Socket.io newMessage frame reuses this object and the UI renders + // that avatar; stripping in place would blank it for every human viewer. + expect(original.userId.profilePicture).toBe(DATA_URI); + }); + + test('passes non-plain objects (Date, ObjectId-like) through untouched', () => { + const when = new Date('2026-07-26T00:00:00Z'); + const oid = { toHexString: () => 'abc' }; + Object.setPrototypeOf(oid, { custom: true }); + + const out = stripInlineAvatars({ createdAt: when, _id: oid }); + + expect(out.createdAt).toBe(when); + expect(out._id).toBe(oid); + }); + + test('survives a self-referential payload without recursing forever', () => { + const node = { username: 'a', profilePicture: DATA_URI }; + node.self = node; + + const out = stripInlineAvatars({ node }); + + expect(out.node).not.toHaveProperty('profilePicture'); + expect(out.node.self).toBe(out.node); + }); + + test('leaves primitives and null alone', () => { + expect(stripInlineAvatars(null)).toBeNull(); + expect(stripInlineAvatars('x')).toBe('x'); + expect(stripInlineAvatars(3)).toBe(3); + }); +}); diff --git a/backend/routes/agentsRuntime.ts b/backend/routes/agentsRuntime.ts index bceae27f..6f8ea1b7 100644 --- a/backend/routes/agentsRuntime.ts +++ b/backend/routes/agentsRuntime.ts @@ -96,7 +96,26 @@ try { PGPod = null; } +const { stripInlineAvatars } = require('../services/avatarService'); + const router = express.Router(); + +// Every response on this router is bound for an agent's context window, so no +// inline base64 avatar may ride along. Enforcing it here rather than at each +// handler is deliberate: the leak was spread across getRecentMessages, the +// post-message echo, thread comments and the create-pod member list, there is +// no shared user serializer to fix instead, and a per-handler fix would not +// cover the next endpoint someone adds. Measured before this: a 20-message +// read returned 230,170 chars, 71% of it image data (#758). +// +// Only `data:` values are dropped — URLs are cheap and stay useful. The +// original objects are never mutated, because the same message object is +// reused for the human-facing Socket.io broadcast where avatars ARE rendered. +router.use((_req: any, res: any, next: any) => { + const originalJson = res.json.bind(res); + res.json = (body: unknown) => originalJson(stripInlineAvatars(body)); + next(); +}); const parseNonNegativeInt = (value: any, fallback: any) => { const parsed = parseInt(value, 10); if (Number.isNaN(parsed)) return fallback; diff --git a/backend/services/avatarService.ts b/backend/services/avatarService.ts index 3e6ea478..e8ea2325 100644 --- a/backend/services/avatarService.ts +++ b/backend/services/avatarService.ts @@ -62,6 +62,52 @@ export function resolveAvatarUrl(user: AvatarUser | null | undefined): string | return normalized; } +// Keys that carry an avatar reference anywhere in a serialized payload. Both +// spellings exist because the PG row is snake_case and the Mongo/populated +// shape is camelCase, and several responses carry BOTH for the same user. +const AVATAR_KEYS = new Set(['profilePicture', 'profile_picture']); + +/** + * Remove inline base64 avatars from a payload bound for an agent. + * + * Avatars are stored as `data:` URIs on the user row, so every message carries + * its author's full image bytes — and the same avatar repeats on every message + * that author has sent. Measured on a real pod, `commonly_get_messages` at the + * default limit returned 230,170 characters of which 71% was image data: an + * agent asked to read a busy room exhausts its context on profile pictures + * before it reaches the conversation (#758). + * + * Only `data:` values are dropped. A URL costs nothing and stays useful, so it + * is preserved — the rule is "no base64 in an agent payload", not "no avatars". + * + * Returns a structurally-shared copy; the input is never mutated, because the + * same message object is reused for the human-facing Socket.io broadcast where + * the avatar IS rendered. + */ +export function stripInlineAvatars(payload: T): T { + const seen = new WeakMap(); + + const walk = (value: unknown): unknown => { + if (Array.isArray(value)) return value.map(walk); + if (!value || typeof value !== 'object') return value; + // Dates, ObjectIds and other non-plain objects must pass through intact. + const proto = Object.getPrototypeOf(value); + if (proto !== Object.prototype && proto !== null) return value; + const cached = seen.get(value as object); + if (cached !== undefined) return cached; + + const out: Record = {}; + seen.set(value as object, out); + for (const [key, val] of Object.entries(value as Record)) { + if (AVATAR_KEYS.has(key) && typeof val === 'string' && /^data:/i.test(val)) continue; + out[key] = walk(val); + } + return out; + }; + + return walk(payload) as T; +} + export async function resolveAvatarUrls( userIds: string[], ): Promise> {