From 9796491fcb2624d79920e1fa53913c1ecce51743 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 23 Oct 2026 06:43:29 -0700 Subject: [PATCH] refactor(auth): remove redundant identity rereads Co-authored-by: rivet Agent-Model: codex/gpt-6-sol/medium --- .../unit/controllers/podController.test.js | 84 +++++++++++-------- .../unit/routes/agentProfile.avatar.test.js | 20 +---- ...arketplace-api.componentValidation.test.js | 23 ++++- .../registry.publisherNameJwtPath.test.js | 45 ++++------ backend/controllers/podController.ts | 14 +--- backend/routes/agentProfile.ts | 5 -- backend/routes/marketplace-api.ts | 13 +-- backend/routes/registry/helpers.ts | 22 ++--- backend/routes/registry/install.ts | 2 +- backend/routes/registry/publish.ts | 2 +- 10 files changed, 101 insertions(+), 129 deletions(-) diff --git a/backend/__tests__/unit/controllers/podController.test.js b/backend/__tests__/unit/controllers/podController.test.js index 328536903c..6e1e3b6ea0 100644 --- a/backend/__tests__/unit/controllers/podController.test.js +++ b/backend/__tests__/unit/controllers/podController.test.js @@ -217,7 +217,7 @@ describe('podController', () => { it('deletePod denies delete if user is not creator', async () => { Pod.findById.mockResolvedValue({ createdBy: 'creator' }); - const req = { params: { id: 'p1' }, userId: 'other' }; + const req = { params: { id: 'p1' }, userId: 'other', user: { id: 'other', role: 'user' } }; const res = { status: jest.fn().mockReturnThis(), json: jest.fn(), @@ -225,15 +225,11 @@ describe('podController', () => { }; await podController.deletePod(req, res); expect(res.status).toHaveBeenCalledWith(401); + expect(User.findById).not.toHaveBeenCalled(); }); it('deletePod allows global admin to delete pod they did not create', async () => { Pod.findById.mockResolvedValue({ createdBy: 'creator' }); - User.findById.mockReturnValue({ - select: jest.fn().mockReturnValue({ - lean: jest.fn().mockResolvedValue({ role: 'admin' }), - }), - }); Pod.deleteOne.mockResolvedValue({ deletedCount: 1 }); Message.deleteMany.mockResolvedValue({ deletedCount: 2 }); Post.deleteMany.mockResolvedValue({ deletedCount: 0 }); @@ -243,7 +239,7 @@ describe('podController', () => { AgentInstallation.deleteMany.mockResolvedValue({ deletedCount: 0 }); AgentProfile.deleteMany.mockResolvedValue({ deletedCount: 0 }); - const req = { params: { id: 'p1' }, userId: 'global-admin' }; + const req = { params: { id: 'p1' }, userId: 'global-admin', user: { id: 'global-admin', role: 'admin' } }; const res = { status: jest.fn().mockReturnThis(), json: jest.fn(), @@ -255,6 +251,7 @@ describe('podController', () => { expect(Message.deleteMany).toHaveBeenCalledWith({ podId: 'p1' }); expect(Pod.deleteOne).toHaveBeenCalledWith({ _id: 'p1' }); expect(res.json).toHaveBeenCalledWith({ msg: 'Pod deleted' }); + expect(User.findById).not.toHaveBeenCalled(); }); it('removeMember denies removal when user is not creator', async () => { @@ -414,6 +411,40 @@ describe('podController', () => { expect(res.status).not.toHaveBeenCalledWith(403); }); + it('joinPod admits an admin into an invite-only chat pod using req.user.role', async () => { + const pod = { _id: 'private-1', type: 'chat', members: ['creator'], joinPolicy: 'invite-only' }; + Pod.findById.mockResolvedValueOnce(pod).mockReturnValueOnce({ + populate: jest.fn().mockReturnValue({ + populate: jest.fn().mockResolvedValue({ ...pod, members: ['creator', 'admin'] }), + }), + }); + const req = { params: { id: 'private-1' }, userId: 'admin', user: { id: 'admin', role: 'admin' } }; + const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; + + await podController.joinPod(req, res); + + expect(Pod.updateOne).toHaveBeenCalledWith( + { _id: 'private-1' }, + expect.objectContaining({ $addToSet: { members: 'admin' } }), + ); + expect(res.status).not.toHaveBeenCalledWith(403); + }); + + it('joinPod refuses a non-admin for an invite-only chat pod', async () => { + Pod.findById.mockResolvedValue({ + _id: 'private-1', type: 'chat', members: ['creator'], joinPolicy: 'invite-only', + }); + const req = { params: { id: 'private-1' }, userId: 'member', user: { id: 'member', role: 'user' } }; + const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; + + await podController.joinPod(req, res); + + expect(res.status).toHaveBeenCalledWith(403); + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ code: 'join_refused' })); + expect(User.findById).not.toHaveBeenCalled(); + expect(Pod.updateOne).not.toHaveBeenCalled(); + }); + it('joinPod refuses a bot before membership when its installation cannot enter a protected pod', async () => { const pod = { _id: 'chat-1', type: 'chat', members: ['human-id'], @@ -445,11 +476,7 @@ describe('podController', () => { const populateSecond = jest.fn(() => ({ sort })); const populateFirst = jest.fn(() => ({ populate: populateSecond, sort })); Pod.find.mockReturnValue({ populate: populateFirst }); - User.findById.mockReturnValue({ - select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ role: 'user' }) }), - }); - - const req = { params: { type: 'agent-room' }, userId: 'me', user: {} }; + const req = { params: { type: 'agent-room' }, userId: 'me', user: { id: 'me', role: 'user' } }; const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; await podController.getPodsByType(req, res); expect(res.json).toHaveBeenCalledWith([myPod]); @@ -467,11 +494,7 @@ describe('podController', () => { const populateSecond = jest.fn(() => ({ sort })); const populateFirst = jest.fn(() => ({ populate: populateSecond, sort })); Pod.find.mockReturnValue({ populate: populateFirst }); - User.findById.mockReturnValue({ - select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ role: 'admin' }) }), - }); - - const req = { params: { type: 'agent-room' }, userId: 'admin-id', user: {} }; + const req = { params: { type: 'agent-room' }, userId: 'admin-id', user: { id: 'admin-id', role: 'admin' } }; const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; await podController.getPodsByType(req, res); expect(res.json).toHaveBeenCalledWith([myPod]); @@ -485,11 +508,7 @@ describe('podController', () => { const populateSecond = jest.fn(() => ({ populate: populateThird, sort })); const populateFirst = jest.fn(() => ({ populate: populateSecond, sort })); Pod.find.mockReturnValue({ populate: populateFirst }); - User.findById.mockReturnValue({ - select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ role: 'admin' }) }), - }); - - const req = { query: {}, userId: 'admin-id', user: {} }; + const req = { query: {}, userId: 'admin-id', user: { id: 'admin-id', role: 'admin' } }; const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; await podController.getAllPods(req, res); // 'admin-id' is not a castable ObjectId, so this exercises the JS fallback. @@ -579,17 +598,14 @@ describe('podController', () => { const populateSecond = jest.fn(() => ({ populate: populateThird, sort })); const populateFirst = jest.fn(() => ({ populate: populateSecond, sort })); Pod.find.mockReturnValue({ populate: populateFirst }); - User.findById.mockReturnValue({ - select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ role: 'admin' }) }), - }); - const me = new mongoose.Types.ObjectId(); - const req = { query: { scope: 'all' }, userId: String(me), user: {} }; + const req = { query: { scope: 'all' }, userId: String(me), user: { id: String(me), role: 'admin' } }; const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; await podController.getAllPods(req, res); expect(Pod.find).toHaveBeenCalledWith({ type: { $nin: [...DEFAULT_LISTING_HIDDEN_POD_TYPES] } }); expect(res.json).toHaveBeenCalledWith([otherPod]); + expect(User.findById).not.toHaveBeenCalled(); }); it('getAllPods survives a member whose User row is gone (populate leaves null)', async () => { @@ -666,14 +682,11 @@ describe('podController', () => { const populateSecond = jest.fn(() => ({ populate: populateThird, sort })); const populateFirst = jest.fn(() => ({ populate: populateSecond, sort })); Pod.find.mockReturnValue({ populate: populateFirst }); - User.findById.mockReturnValue({ - select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ role: 'admin' }) }), - }); - - const req = { query: { scope: 'all' }, userId: 'admin-id', user: {} }; + const req = { query: { scope: 'all' }, userId: 'admin-id', user: { id: 'admin-id', role: 'admin' } }; const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; await podController.getAllPods(req, res); expect(res.json).toHaveBeenCalledWith([otherPod, myPod]); + expect(User.findById).not.toHaveBeenCalled(); }); it('getAllPods scope=all is silently downgraded to scope=mine for non-admins', async () => { @@ -684,14 +697,11 @@ describe('podController', () => { const populateSecond = jest.fn(() => ({ populate: populateThird, sort })); const populateFirst = jest.fn(() => ({ populate: populateSecond, sort })); Pod.find.mockReturnValue({ populate: populateFirst }); - User.findById.mockReturnValue({ - select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue({ role: 'user' }) }), - }); - - const req = { query: { scope: 'all' }, userId: 'me', user: {} }; + const req = { query: { scope: 'all' }, userId: 'me', user: { id: 'me', role: 'user' } }; const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; await podController.getAllPods(req, res); expect(res.json).toHaveBeenCalledWith([myPod]); + expect(User.findById).not.toHaveBeenCalled(); }); it('getPodById returns 404 for personal pod types when caller is not a member', async () => { diff --git a/backend/__tests__/unit/routes/agentProfile.avatar.test.js b/backend/__tests__/unit/routes/agentProfile.avatar.test.js index 65cbb0366d..293de128c4 100644 --- a/backend/__tests__/unit/routes/agentProfile.avatar.test.js +++ b/backend/__tests__/unit/routes/agentProfile.avatar.test.js @@ -10,7 +10,6 @@ jest.mock('../../../models/User', () => ({ findOneAndUpdate: jest.fn(), - findById: jest.fn(), })); jest.mock('../../../models/Pod', () => ({})); jest.mock('../../../models/AgentMemory', () => ({})); @@ -54,12 +53,6 @@ const installationLookup = (result) => { }); }; -const callerRoleLookup = (role) => { - User.findById.mockReturnValue({ - select: jest.fn().mockReturnValue({ lean: jest.fn().mockResolvedValue(role ? { role } : null) }), - }); -}; - const putAvatar = getHandler('put', '/:agentName/:instanceId?/avatar'); const canEdit = getHandler('get', '/:agentName/:instanceId?/avatar/can-edit'); @@ -67,7 +60,6 @@ describe('agent avatar editing', () => { beforeEach(() => { jest.clearAllMocks(); User.findOneAndUpdate.mockResolvedValue({ _id: 'bot-1', profilePicture: 'bottts:theo:v3' }); - callerRoleLookup(null); AgentRegistry.updateOne.mockResolvedValue({}); AgentIdentityService.syncUserToPostgreSQL.mockResolvedValue(undefined); }); @@ -113,7 +105,7 @@ describe('agent avatar editing', () => { expect(AgentRegistry.updateOne).not.toHaveBeenCalled(); }); - it('lets an admin edit without owning an installation (API-token shape: role on req.user)', async () => { + it('lets an admin edit without owning an installation using req.user.role', async () => { const res = response(); await putAvatar(reqFor({ id: 'admin-1', role: 'admin' }), res); @@ -123,18 +115,14 @@ describe('agent avatar editing', () => { expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ ok: true })); }); - it('lets an admin edit on a JWT session, where req.user carries NO role', async () => { - // middleware/auth.ts sets req.user = { id } for JWT logins — the browser - // shape. The gate must load the role from DB or the admin path is dead for - // every human session (the bug found live on fc-verify, 2026-08-21). - callerRoleLookup('admin'); + it('can-edit admits an admin without an installation', async () => { installationLookup(null); const res = response(); - await putAvatar(reqFor({ id: 'admin-1' }), res); + await canEdit(reqFor({ id: 'admin-1', role: 'admin' }), res); expect(AgentInstallation.findOne).not.toHaveBeenCalled(); - expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ ok: true })); + expect(res.json).toHaveBeenCalledWith({ canEdit: true }); }); it('400s anything that is not a bottts preset or an upload reference', async () => { diff --git a/backend/__tests__/unit/routes/marketplace-api.componentValidation.test.js b/backend/__tests__/unit/routes/marketplace-api.componentValidation.test.js index 07559668c0..5b9c9ab517 100644 --- a/backend/__tests__/unit/routes/marketplace-api.componentValidation.test.js +++ b/backend/__tests__/unit/routes/marketplace-api.componentValidation.test.js @@ -45,11 +45,11 @@ const getRouteHandler = (path, method) => { return layer.route.stack[layer.route.stack.length - 1].handle; }; -const publish = async (components) => { +const publish = async (components, user = { id: 'user-1', username: 'nova' }) => { const handler = getRouteHandler('/publish', 'post'); const req = { userId: 'user-1', - user: { id: 'user-1', username: 'nova' }, + user, body: { installableId: '@nova/my-agent', name: 'My Agent', @@ -96,6 +96,25 @@ describe('marketplace publish validates inside the components array', () => { expect(Installable.create).toHaveBeenCalledTimes(1); }); + it('publishes from an admin session using req.user.username', async () => { + const res = await publish([], { id: 'user-1', username: 'nova', role: 'admin' }); + expect(res.status).toHaveBeenCalledWith(201); + expect(Installable.create).toHaveBeenCalledTimes(1); + }); + + it('rejects a caller without a username instead of re-reading the user row', async () => { + const res = await publish([], { id: 'user-1', role: 'user' }); + expect(res.status).toHaveBeenCalledWith(401); + expect(Installable.create).not.toHaveBeenCalled(); + }); + + it('refuses a non-admin publishing into another username namespace', async () => { + const res = await publish([], { id: 'user-1', username: 'mallory', role: 'user' }); + expect(res.status).toHaveBeenCalledWith(400); + expect(errorOf(res)).toMatch(/namespace/i); + expect(Installable.create).not.toHaveBeenCalled(); + }); + it('accepts the ADR-001 McpServer component and bounds its variables field', async () => { const res = await publish([{ name: 'Calendar', diff --git a/backend/__tests__/unit/routes/registry.publisherNameJwtPath.test.js b/backend/__tests__/unit/routes/registry.publisherNameJwtPath.test.js index 1712013f18..ff5ced416c 100644 --- a/backend/__tests__/unit/routes/registry.publisherNameJwtPath.test.js +++ b/backend/__tests__/unit/routes/registry.publisherNameJwtPath.test.js @@ -1,19 +1,10 @@ -// `middleware/auth.ts` dispatches on the token and the two branches leave -// DIFFERENT shapes on `req.user`: the `cm_` API-token path assigns -// `{ id, username, email, role }` (`:51`); the browser-JWT path assigns -// `{ id }` (`:81`). Neither errors. So the registry writers, which read -// `req.user.username` directly, persisted `publisher.name: undefined` for -// every browser-session publish — a silent data defect with nothing to go red. -// -// Per AX audit entry 42, the auth middleware is mocked to `next()` here on -// purpose and the shape is CONSTRUCTED on the request instead: the subject is -// the consumer's handling of each shape, not the dispatcher's choice between -// them, and a test that let the route pick would only ever exercise one. +// The auth middleware now supplies username on both browser JWT and API-token +// requests. These tests pin publisher.name on both shapes; the model mock has +// no findById, so a redundant user lookup would fail the suite. jest.mock('../../../middleware/auth', () => (req, res, next) => next()); -const mockFindById = jest.fn(); -jest.mock('../../../models/User', () => ({ findById: (...a) => mockFindById(...a) })); +jest.mock('../../../models/User', () => ({})); const mockCreate = jest.fn(); jest.mock('../../../models/AgentRegistry', () => ({ @@ -34,11 +25,12 @@ const handler = (() => { const manifest = { name: 'demo-agent', version: '1.0.0', description: 'd' }; -const run = async (user) => { +const run = async (user, agentUser) => { mockCreate.mockClear(); mockCreate.mockResolvedValue({ agentName: 'demo-agent' }); const req = { user, + agentUser, body: { manifest, displayName: 'Demo', readme: null, categories: [], tags: [], }, @@ -50,31 +42,24 @@ const run = async (user) => { }; describe('registry publish — publisher.name across both auth shapes', () => { - beforeEach(() => { - mockFindById.mockReset(); - mockFindById.mockReturnValue({ select: () => ({ lean: async () => ({ username: 'lily' }) }) }); - }); - - // The regression case. Pre-fix this yielded `name: undefined`. - it('resolves the username on the browser-JWT shape, which carries only { id }', async () => { - const publisher = await run({ id: 'u1' }); + it('uses the browser-JWT username without a DB read', async () => { + const publisher = await run({ id: 'u1', username: 'lily', email: 'lily@example.com', role: 'admin' }); expect(publisher.name).toBe('lily'); - expect(mockFindById).toHaveBeenCalledWith('u1'); }); - it('uses the token-supplied username on the cm_ shape without a DB read', async () => { + it('uses the API-token username without a DB read', async () => { const publisher = await run({ id: 'u1', username: 'sam', email: 'e', role: 'user', }); expect(publisher.name).toBe('sam'); - expect(mockFindById).not.toHaveBeenCalled(); }); - // A publisher row whose name is `undefined` is the defect itself, so no - // shape may produce one: an unresolvable user must persist null, not a - // missing key that reads as "not yet written". - it('persists null rather than undefined when the user cannot be resolved', async () => { - mockFindById.mockReturnValue({ select: () => ({ lean: async () => null }) }); + it('keeps the explicit agentUser username fallback', async () => { + const publisher = await run({ id: 'u1' }, { username: 'agent-one' }); + expect(publisher.name).toBe('agent-one'); + }); + + it('persists null rather than undefined when no username is supplied', async () => { const publisher = await run({ id: 'u1' }); expect(publisher.name).toBeNull(); expect('name' in publisher).toBe(true); diff --git a/backend/controllers/podController.ts b/backend/controllers/podController.ts index 30382ef59e..53414ba9e0 100644 --- a/backend/controllers/podController.ts +++ b/backend/controllers/podController.ts @@ -37,13 +37,7 @@ const buildDefaultAgentProfileId = (agentName: any, instanceId = 'default') => ( `${agentName.toLowerCase()}:${instanceId || 'default'}` ); -const isGlobalAdminRequest = async (req: any) => { - if (req.user?.role === 'admin') return true; - const userId = req.userId || req.user?.id || req.user?._id; - if (!userId) return false; - const user = await User.findById(userId).select('role').lean(); - return Boolean(user && user.role === 'admin'); -}; +const isGlobalAdminRequest = (req: any) => req.user?.role === 'admin'; const ensureDefaultAgentRegistryEntry = async (agentName: any) => { const normalized = String(agentName || '').trim().toLowerCase(); @@ -232,7 +226,7 @@ exports.getAllPods = async (req: any, res: any) => { let memberId: any = null; if (req.userId && !isCommunityScope && !isDiscoverScope) { const wantsAll = scope === 'all'; - const isAdmin = wantsAll ? await isGlobalAdminRequest(req) : false; + const isAdmin = wantsAll ? isGlobalAdminRequest(req) : false; membershipFilterToMine = isPersonal || !wantsAll || !isAdmin; if (membershipFilterToMine) { const uid = String(req.userId); @@ -543,7 +537,7 @@ exports.joinPod = async (req: any, res: any) => { // Repeated self-join requests are successful and side-effect free. This // keeps optimistic clients and retried requests idempotent. if (!isMember) { - const isAdmin = await isGlobalAdminRequest(req); + const isAdmin = isGlobalAdminRequest(req); const isJoinable = isDirectlyJoinable(pod); if (!isAdmin && !isJoinable) { return res.status(403).json({ @@ -722,7 +716,7 @@ exports.deletePod = async (req: any, res: any) => { const requesterId = req.userId || req.user?.id || req.user?._id; const isCreator = pod.createdBy.toString() === requesterId?.toString(); - const isGlobalAdmin = await isGlobalAdminRequest(req); + const isGlobalAdmin = isGlobalAdminRequest(req); // Check if user is the creator or global admin if (!isCreator && !isGlobalAdmin) { diff --git a/backend/routes/agentProfile.ts b/backend/routes/agentProfile.ts index 837becabba..21a92d0ad9 100644 --- a/backend/routes/agentProfile.ts +++ b/backend/routes/agentProfile.ts @@ -290,12 +290,7 @@ router.get('/:agentName/:instanceId?', async (req: Req, res: Res) => { const canEditAgentAvatar = async (req: any, agentName: string, instanceId: string): Promise => { const callerId = req.userId || req.user?._id || req.user?.id; if (!callerId) return false; - // JWT auth populates req.user = { id } WITHOUT role (middleware/auth.ts:81) - // — only the cm_ API-token branch carries role. Trusting req.user.role here - // silently disabled the admin path for every browser session, so load it. if (req.user?.role === 'admin') return true; - const caller = await User.findById(callerId).select('role').lean(); - if (caller?.role === 'admin') return true; const owned = await AgentInstallation.findOne({ agentName, instanceId, diff --git a/backend/routes/marketplace-api.ts b/backend/routes/marketplace-api.ts index 76732959a7..fbae1bbf1c 100644 --- a/backend/routes/marketplace-api.ts +++ b/backend/routes/marketplace-api.ts @@ -1,6 +1,5 @@ const express = require('express'); const auth = require('../middleware/auth'); -const User = require('../models/User'); const Installable = require('../models/Installable'); const { AgentRegistry, AgentInstallation } = require('../models/AgentRegistry'); const { validateMcpComponent } = require('../utils/pluginManifestParser'); @@ -9,13 +8,7 @@ const router = express.Router(); const NAMESPACE_RE = /^@[a-z0-9-]+\/[a-z0-9-]+$/; -const resolveUsername = async (req: any): Promise => { - if (req.user?.username) return req.user.username; - const userId = req.userId || req.user?.id; - if (!userId) return null; - const user = await User.findById(userId).select('username').lean(); - return user?.username || null; -}; +const resolveUsername = (req: any): string | null => req.user?.username || null; const validateNamespace = (installableId: string, username: string) => { if (!NAMESPACE_RE.test(installableId)) { @@ -233,7 +226,7 @@ const syncToAgentRegistry = async (installable: any, action: 'publish' | 'unpubl router.post('/publish', auth, async (req: any, res: any) => { try { const userId = req.userId || req.user?.id; - const username = await resolveUsername(req); + const username = resolveUsername(req); if (!userId || !username) { return res.status(401).json({ error: 'Unauthorized' }); } @@ -522,7 +515,7 @@ router.delete('/manifests/:installableId(*)', auth, async (req: any, res: any) = router.post('/fork', auth, async (req: any, res: any) => { try { const userId = req.userId || req.user?.id; - const username = await resolveUsername(req); + const username = resolveUsername(req); if (!userId || !username) { return res.status(401).json({ error: 'Unauthorized' }); } diff --git a/backend/routes/registry/helpers.ts b/backend/routes/registry/helpers.ts index 96b0e755bc..67a4f5a1fe 100644 --- a/backend/routes/registry/helpers.ts +++ b/backend/routes/registry/helpers.ts @@ -56,23 +56,11 @@ const escapeRegExp = (value: any) => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&' const getUserId = (req: any) => req.userId || req.user?.id || req.user?._id; -// `middleware/auth.ts` dispatches on the token and the two branches leave -// DIFFERENT shapes on `req.user`: the `cm_` API-token path assigns -// `{ id, username, email, role }` (`:51`), the browser-JWT path assigns -// `{ id }` (`:81`). Neither errors, so `req.user.username` is simply -// `undefined` for every browser session — and `publisher.name: undefined` -// persists into the registry with nothing going red. Load it when the token -// did not carry it. `routes/marketplace-api.ts` already resolves the same way; -// this is that resolution moved next to `getUserId`, which both registry -// writers already import. See AX audit entry 42. -const resolveUsername = async (req: any): Promise => { - if (req.user?.username) return req.user.username; - if (req.agentUser?.username) return req.agentUser.username; - const userId = getUserId(req); - if (!userId) return null; - const user = await User.findById(userId).select('username').lean(); - return user?.username || null; -}; +// Both browser JWT and API-token auth supply req.user.username. Preserve the +// agentUser shape for registry callers that pass one explicitly. +const resolveUsername = (req: any): string | null => ( + req.user?.username || req.agentUser?.username || null +); const normalizeConfigMap = (config: any) => { if (!config) return null; diff --git a/backend/routes/registry/install.ts b/backend/routes/registry/install.ts index 3d6551596a..02840996c5 100644 --- a/backend/routes/registry/install.ts +++ b/backend/routes/registry/install.ts @@ -354,7 +354,7 @@ installRouter.post('/install', installRateLimit, auth, async (req: any, res: any latestVersion: synthManifest.version, versions: [{ version: synthManifest.version, manifest: synthManifest, publishedAt: new Date() }], registry: 'private', - publisher: { userId, name: await resolveUsername(req) }, + publisher: { userId, name: resolveUsername(req) }, ephemeral: true, }); console.log('[cap self-serve-install]', { diff --git a/backend/routes/registry/publish.ts b/backend/routes/registry/publish.ts index d6f4673ef3..c6b5891617 100644 --- a/backend/routes/registry/publish.ts +++ b/backend/routes/registry/publish.ts @@ -69,7 +69,7 @@ publishRouter.post('/publish', auth, async (req: any, res: any) => { registry: 'commonly-community', publisher: { userId, - name: await resolveUsername(req), + name: resolveUsername(req), }, categories, tags,