diff --git a/backend/__tests__/service/uploads.signedurl.integration.test.js b/backend/__tests__/service/uploads.signedurl.integration.test.js index da93b1ef..74e03688 100644 --- a/backend/__tests__/service/uploads.signedurl.integration.test.js +++ b/backend/__tests__/service/uploads.signedurl.integration.test.js @@ -23,6 +23,7 @@ const express = require('express'); const request = require('supertest'); +const { createHash } = require('crypto'); const { setupMongoDb, closeMongoDb, clearMongoDb, generateTestToken } = require('../utils/testUtils'); @@ -143,6 +144,43 @@ describe('ADR-002 Phase 1b-a — signed-URL mint (integration)', () => { .expect(403); }); + it('authorizes a runtime-token agent who belongs to the file pod before the object lookup', async () => { + const owner = await makeUser(); + const agentToken = 'cm_agent_attachment-read-integration'; + const agent = await makeUser({ + isBot: true, + botType: 'agent', + botMetadata: { agentName: 'attachment-reader', instanceId: 'default' }, + agentRuntimeTokens: [{ + tokenHash: createHash('sha256').update(agentToken).digest('hex'), + label: 'attachment-read-integration', + createdAt: new Date(), + }], + }); + const pod = await Pod.create({ + name: 'agent-readable attachment pod', + type: 'chat', + createdBy: owner._id, + members: [owner._id, agent._id], + }); + await File.create({ + fileName: 'agent-readable.png', + originalName: 'agent-readable.png', + contentType: 'image/png', + size: 10, + uploadedBy: owner._id, + podId: pod._id, + }); + + // No object bytes are seeded, so a 404 proves authorization succeeded + // and the request reached the storage lookup. A failed agent ACL would + // stop earlier with 403. + await request(app) + .get('/api/uploads/agent-readable.png') + .set('Authorization', `Bearer ${agentToken}`) + .expect(404); + }); + it('matches a profile picture stored as an absolute URL (fix #4 — real substring regex)', async () => { const subject = await makeUser({ profilePicture: 'https://api-dev.commonly.me/api/uploads/avatar.png', diff --git a/backend/__tests__/unit/routes/uploads.test.js b/backend/__tests__/unit/routes/uploads.test.js index 26ab2f6c..1c64e3a7 100644 --- a/backend/__tests__/unit/routes/uploads.test.js +++ b/backend/__tests__/unit/routes/uploads.test.js @@ -25,6 +25,17 @@ jest.mock('jsonwebtoken', () => ({ verify: jest.fn() })); jest.mock('../../../middleware/auth', () => (req, res, next) => next()); +const mockCanReadAttachment = jest.fn(); +jest.mock('../../../services/attachmentAccess', () => ({ + DEFAULT_TOKEN_TTL_SECONDS: 300, + canReadAttachment: (...args) => mockCanReadAttachment(...args), + signAttachmentToken: jest.fn(), + verifyAttachmentToken: jest.fn(), +})); + +const mockAgentRuntimeAuth = jest.fn(); +jest.mock('../../../middleware/agentRuntimeAuth', () => (...args) => mockAgentRuntimeAuth(...args)); + const File = require('../../../models/File'); const routes = require('../../../routes/uploads'); @@ -38,6 +49,11 @@ describe('uploads GET /:fileName (ADR-002 Phase 1)', () => { mockStore.put.mockReset(); mockStore.delete.mockReset(); File.findByFileName.mockReset(); + mockCanReadAttachment.mockReset(); + mockAgentRuntimeAuth.mockReset().mockImplementation((req, _res, next) => { + req.agentUser = { _id: 'agent-user' }; + next(); + }); }); it('streams bytes from the driver when the key is present', async () => { @@ -96,4 +112,52 @@ describe('uploads GET /:fileName (ADR-002 Phase 1)', () => { mockStore.get.mockRejectedValue(new Error('boom')); await request(app).get('/api/uploads/explode').expect(500); }); + + it('allows a runtime agent to read a pod attachment when its bot user passes the ACL', async () => { + File.findByFileName.mockResolvedValue({ podId: 'private-pod' }); + mockCanReadAttachment.mockResolvedValue(true); + mockStore.get.mockResolvedValue({ + stream: Readable.from(Buffer.from('agent-readable')), + mime: 'text/plain', + size: 14, + }); + + const res = await request(app) + .get('/api/uploads/private.txt') + .set('Authorization', 'Bearer cm_agent_runtime-token') + .expect(200); + + expect(res.text).toBe('agent-readable'); + expect(mockAgentRuntimeAuth).toHaveBeenCalledTimes(1); + expect(mockCanReadAttachment).toHaveBeenCalledWith('private.txt', 'agent-user'); + }); + + it('keeps a runtime agent out of a pod attachment when the ACL denies its bot user', async () => { + File.findByFileName.mockResolvedValue({ podId: 'private-pod' }); + mockCanReadAttachment.mockResolvedValue(false); + + await request(app) + .get('/api/uploads/private.txt') + .set('Authorization', 'Bearer cm_agent_runtime-token') + .expect(403); + + expect(mockCanReadAttachment).toHaveBeenCalledWith('private.txt', 'agent-user'); + expect(mockStore.get).not.toHaveBeenCalled(); + }); + + it('uses the same runtime-agent authorization for a private PPTX preview', async () => { + File.findByFileName.mockResolvedValue({ podId: 'private-pod' }); + mockCanReadAttachment.mockResolvedValue(true); + mockStore.get.mockResolvedValue(null); + + // No PPTX bytes are needed: the 404 proves the agent cleared the shared + // authorization helper and reached storage instead of being denied 403. + await request(app) + .get('/api/uploads/private.pptx/preview-pptx-html') + .set('Authorization', 'Bearer cm_agent_runtime-token') + .expect(404); + + expect(mockAgentRuntimeAuth).toHaveBeenCalledTimes(1); + expect(mockCanReadAttachment).toHaveBeenCalledWith('private.pptx', 'agent-user'); + }); }); diff --git a/backend/routes/uploads.ts b/backend/routes/uploads.ts index 3fef94e9..71f45952 100644 --- a/backend/routes/uploads.ts +++ b/backend/routes/uploads.ts @@ -39,6 +39,8 @@ const multer = require('multer'); const File = require('../models/File'); // eslint-disable-next-line global-require const auth = require('../middleware/auth'); +// eslint-disable-next-line global-require +const agentRuntimeAuth = require('../middleware/agentRuntimeAuth'); import { getObjectStore } from '../services/objectStore'; interface AuthReq { @@ -174,6 +176,20 @@ const artifactReadRateLimit = rateLimit({ res.status(429).json({ msg: 'rate limit exceeded: 240 requests per 60s' }), }); +// Attachment reads also serve browser image requests, so they cannot require +// auth unconditionally: signed URLs and public avatars arrive without an +// Authorization header. Runtime clients do send their opaque `cm_agent_*` +// token, however. Resolve that token before the ACL check so the existing +// user-based attachment policy can evaluate the agent's bot User row. +// Human JWTs deliberately keep the existing inline verification path in +// authorizePodFile below. +const maybeAgentRuntimeAuth = (req: AuthReq, res: Res, next: () => void) => { + const bearer = req.get?.('Authorization')?.replace(/^Bearer\s+/i, '').trim(); + const alternate = req.get?.('x-commonly-agent-token')?.trim(); + if (!(bearer || alternate)?.startsWith('cm_agent_')) return next(); + return agentRuntimeAuth(req as never, res as never, next); +}; + const router: ReturnType = express.Router(); // Shared upload handler — used by both the user-auth POST / and the @@ -299,6 +315,9 @@ const authorizePodFile = async (req: AuthReq, fileName: string): Promise { +router.get('/:fileName', artifactReadRateLimit, maybeAgentRuntimeAuth, async (req: AuthReq, res: Res) => { try { const fileName = req.params?.fileName; if (!fileName) return res.status(400).json({ msg: 'fileName required' }); @@ -353,12 +372,8 @@ router.get('/:fileName', artifactReadRateLimit, async (req: AuthReq, res: Res) = // CSS, three.js loaded from a CDN inside the iframe sandbox) so we just pass // it through to the browser as text/html. // -// Auth — preview is not strictly bound to the file-fetch ACL since: -// - the file fetch (`/api/uploads/:fileName`) is publicly readable today -// (ADR-002 Phase 1, public-read flip is a follow-up); and -// - rendering is read-only, no side effects. -// Once Phase 1b lands and signed URLs become required, this route gains the -// same token check as `/:fileName`. +// Auth — preview shares the raw-file route's pod-scoped authorization below. +// A private deck must not become readable merely because it is rendered. const officecliPreview = (() => { // eslint-disable-next-line @typescript-eslint/no-require-imports const { spawn } = require('child_process'); @@ -418,7 +433,7 @@ const officecliPreview = (() => { // officecli process. Plenty for normal decks (sub-second to a few seconds). const PPTX_RENDER_TIMEOUT_MS = 30_000; -router.get('/:fileName/preview-pptx-html', artifactReadRateLimit, async (req: AuthReq, res: Res) => { +router.get('/:fileName/preview-pptx-html', artifactReadRateLimit, maybeAgentRuntimeAuth, async (req: AuthReq, res: Res) => { try { const fileName = req.params?.fileName; if (!fileName) return res.status(400).json({ msg: 'fileName required' });