From a92394c867dbf065fbc7c023eec3515c8219423c Mon Sep 17 00:00:00 2001 From: Jose Gleiser Date: Sun, 22 Mar 2026 16:18:17 -0300 Subject: [PATCH 1/5] security phase 0: bind invitation recipient and sanitize markdown rendering --- src/api/invitations.js | 10 ++- src/public/admin-jobs.html | 2 + src/public/index.html | 2 + src/public/js/admin-jobs.js | 6 +- src/public/js/components/artifact-viewer.js | 8 +-- src/public/js/utils/markdown-sanitizer.js | 47 ++++++++++++ src/services/workspaceService.js | 56 +++++++++++---- tests/integration/invitations.test.js | 80 +++++++++++++++++++-- tests/unit/markdown-sanitizer.test.js | 65 +++++++++++++++++ tests/unit/workspaceService.test.js | 31 +++++++- 10 files changed, 278 insertions(+), 29 deletions(-) create mode 100644 src/public/js/utils/markdown-sanitizer.js create mode 100644 tests/unit/markdown-sanitizer.test.js diff --git a/src/api/invitations.js b/src/api/invitations.js index 5f57b43..81e849b 100644 --- a/src/api/invitations.js +++ b/src/api/invitations.js @@ -70,6 +70,9 @@ router.post('/:token/accept', authenticateToken, async (req, res) => { res.json(result); } catch (error) { logger.error({ err: error }, 'Error accepting invitation'); + if (error.message === 'Invitation does not belong to authenticated user') { + return res.status(403).json({ error: error.message }); + } if (error.message === 'Invalid invitation' || error.message === 'Invitation expired') { return res.status(400).json({ error: error.message }); } @@ -85,14 +88,17 @@ router.post('/:token/decline', authenticateToken, async (req, res) => { try { const { token } = req.params; - if (!req.user) { + if (!req.user || !req.user.id) { return res.status(401).json({ error: 'Authentication required' }); } - const result = workspaceService.declineInvitation(token); + const result = workspaceService.declineInvitation(token, req.user.id); res.json(result); } catch (error) { logger.error({ err: error }, 'Error declining invitation'); + if (error.message === 'Invitation does not belong to authenticated user') { + return res.status(403).json({ error: error.message }); + } if (error.message === 'Invalid invitation') { return res.status(400).json({ error: error.message }); } diff --git a/src/public/admin-jobs.html b/src/public/admin-jobs.html index f7e2ada..38ed032 100644 --- a/src/public/admin-jobs.html +++ b/src/public/admin-jobs.html @@ -164,8 +164,10 @@

Artifact View

+ + diff --git a/src/public/index.html b/src/public/index.html index 6ddc01e..b68655a 100644 --- a/src/public/index.html +++ b/src/public/index.html @@ -155,8 +155,10 @@

Create Workspace

+ + diff --git a/src/public/js/admin-jobs.js b/src/public/js/admin-jobs.js index fa4b715..e220590 100644 --- a/src/public/js/admin-jobs.js +++ b/src/public/js/admin-jobs.js @@ -9,7 +9,7 @@ let totalPages = 1; let jobsData = []; // Store jobs for modal access const t = window.i18n ? window.i18n.t.bind(window.i18n) : (k) => k; -/* global marked, BpmnJS */ +/* global BpmnJS */ // DOM Elements const loadingState = document.getElementById('loadingState'); @@ -605,12 +605,12 @@ function renderArtifactContent(type, content, _artifactId) { artifactModalBody.innerHTML = html; } else if (type === 'doc') { setModalClasses(false); - if (typeof marked === 'undefined') { + if (typeof window.renderSafeMarkdown !== 'function') { artifactModalBody.innerHTML = '

Error: Marked library not loaded.

'; return; } artifactModalBody.innerHTML = ` -
${marked.parse(content)}
+
${window.renderSafeMarkdown(content)}
`; } else { setModalClasses(false); diff --git a/src/public/js/components/artifact-viewer.js b/src/public/js/components/artifact-viewer.js index 59c0b60..214d7b4 100644 --- a/src/public/js/components/artifact-viewer.js +++ b/src/public/js/components/artifact-viewer.js @@ -2,7 +2,7 @@ * Artifact Viewer * Handles viewing and editing artifacts (BPMN, SIPOC, RACI, Narrative) inside a modal. */ -/* global marked, BpmnJS, EasyMDE */ +/* global BpmnJS, EasyMDE */ globalThis.ArtifactViewer = (function () { const t = () => (globalThis.i18n ? globalThis.i18n.t : (k) => k); @@ -249,7 +249,7 @@ globalThis.ArtifactViewer = (function () { const cancelBtn = document.getElementById('btn-cancel-table'); if (cancelBtn) cancelBtn.addEventListener('click', cancelTableEdit); } else if (type === 'doc') { - if (typeof marked === 'undefined') { + if (typeof globalThis.renderSafeMarkdown !== 'function') { modalBody.innerHTML = `

${t()('artifacts.markedNotLoaded')}

`; return; } @@ -265,7 +265,7 @@ globalThis.ArtifactViewer = (function () { -
${marked.parse(content)}
+
${globalThis.renderSafeMarkdown(content)}
`; @@ -617,7 +617,7 @@ globalThis.ArtifactViewer = (function () { function cancelDocEdit() { destroyDocEditor(); const viewDiv = document.getElementById('markdown-content'); - viewDiv.innerHTML = marked.parse(currentArtifactContent); + viewDiv.innerHTML = globalThis.renderSafeMarkdown(currentArtifactContent); viewDiv.style.display = 'block'; document.getElementById('editDoc').style.display = 'inline-block'; diff --git a/src/public/js/utils/markdown-sanitizer.js b/src/public/js/utils/markdown-sanitizer.js new file mode 100644 index 0000000..319e51a --- /dev/null +++ b/src/public/js/utils/markdown-sanitizer.js @@ -0,0 +1,47 @@ +/* global DOMPurify, marked */ +(function attachSafeMarkdownRenderer(globalScope) { + const escapeHtml = (value) => + String(value).replaceAll('&', '&').replaceAll('<', '<').replaceAll('>', '>').replaceAll('"', '"').replaceAll("'", '''); + + const createMarkedRenderer = () => { + if (typeof marked === 'undefined' || typeof marked.Renderer !== 'function') { + return null; + } + + const renderer = new marked.Renderer(); + // Rendering raw HTML from markdown is blocked to reduce scriptable surface area before sanitization. + renderer.html = () => ''; + return renderer; + }; + + const markedRenderer = createMarkedRenderer(); + + const parseMarkdown = (markdownText) => { + if (typeof marked === 'undefined' || typeof marked.parse !== 'function') { + return escapeHtml(markdownText); + } + + return marked.parse(markdownText, { + breaks: false, + gfm: true, + renderer: markedRenderer || undefined, + }); + }; + + globalScope.renderSafeMarkdown = (markdownText) => { + const normalizedMarkdown = typeof markdownText === 'string' ? markdownText : String(markdownText || ''); + const rawHtml = parseMarkdown(normalizedMarkdown); + + if (typeof DOMPurify === 'undefined' || typeof DOMPurify.sanitize !== 'function') { + return escapeHtml(normalizedMarkdown); + } + + return DOMPurify.sanitize(rawHtml, { + FORBID_ATTR: ['style'], + FORBID_TAGS: ['embed', 'form', 'iframe', 'object', 'script', 'style'], + USE_PROFILES: { + html: true, + }, + }); + }; +})(globalThis); diff --git a/src/services/workspaceService.js b/src/services/workspaceService.js index c9263ad..f802543 100644 --- a/src/services/workspaceService.js +++ b/src/services/workspaceService.js @@ -14,6 +14,8 @@ const { isTransferredPersonalWorkspaceName, } = require('../utils/workspaces'); +const normalizeEmail = (email) => (typeof email === 'string' ? email.trim().toLowerCase() : ''); + class WorkspaceService { repairLegacyPersonalWorkspace(workspace) { if (!workspace) { @@ -378,11 +380,16 @@ class WorkspaceService { const token = uuidv4(); const now = new Date(); const expiresAt = new Date(now.getTime() + 7 * 24 * 60 * 60 * 1000).toISOString(); // 7 days + const normalizedRecipientEmail = normalizeEmail(email); + + if (!normalizedRecipientEmail) { + throw new Error('Recipient email is required'); + } // Fixed syntax error try { // 1. Check if user exists (ENFORCED) - const recipientUser = db.prepare('SELECT id, email, name FROM users WHERE email = ?').get(email); + const recipientUser = db.prepare('SELECT id, email, name FROM users WHERE lower(email) = ?').get(normalizedRecipientEmail); if (!recipientUser) { throw new Error('User must be registered to be invited'); } @@ -396,8 +403,8 @@ class WorkspaceService { // Check if invite already exists, update it if so const existingInvite = db - .prepare('SELECT id FROM workspace_invitations WHERE workspace_id = ? AND recipient_email = ?') - .get(workspaceId, email); + .prepare('SELECT id FROM workspace_invitations WHERE workspace_id = ? AND lower(recipient_email) = ?') + .get(workspaceId, normalizedRecipientEmail); if (existingInvite) { db.prepare( @@ -414,7 +421,7 @@ class WorkspaceService { INSERT INTO workspace_invitations (id, workspace_id, inviter_id, recipient_email, role, token, created_at, expires_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?) `, - ).run(id, workspaceId, inviterId, email, role, token, now.toISOString(), expiresAt); + ).run(id, workspaceId, inviterId, recipientUser.email, role, token, now.toISOString(), expiresAt); } // Create In-App Notification @@ -438,7 +445,7 @@ class WorkspaceService { }, ); - return { token, email, expiresAt, status: existingInvite ? 'updated' : 'created' }; + return { token, email: recipientUser.email, expiresAt, status: existingInvite ? 'updated' : 'created' }; } catch (error) { logger.error({ err: error }, 'Error creating invitation'); throw error; @@ -467,6 +474,11 @@ class WorkspaceService { * @param {string} email */ getUserInvitations(email) { + const normalizedEmail = normalizeEmail(email); + if (!normalizedEmail) { + return []; + } + return db .prepare( ` @@ -474,10 +486,10 @@ class WorkspaceService { FROM workspace_invitations wi JOIN workspaces w ON wi.workspace_id = w.id LEFT JOIN users u ON wi.inviter_id = u.id - WHERE wi.recipient_email = ? AND wi.status = 'pending' + WHERE lower(wi.recipient_email) = ? AND wi.status = 'pending' `, ) - .all(email); + .all(normalizedEmail); } /** @@ -519,17 +531,35 @@ class WorkspaceService { * @param {string} token * @param {string} userId */ + getInvitationActor(userId) { + return db.prepare('SELECT id, email FROM users WHERE id = ?').get(userId); + } + + ensureInvitationRecipient(invite, actor) { + if (!actor || !normalizeEmail(actor.email)) { + throw new Error('Authentication required'); + } + + if (normalizeEmail(invite.recipient_email) !== normalizeEmail(actor.email)) { + throw new Error('Invitation does not belong to authenticated user'); + } + } + acceptInvitation(token, userId) { const invite = this.getInvitation(token); if (!invite) throw new Error('Invalid invitation'); if (invite.expired) throw new Error('Invitation expired'); + const actor = this.getInvitationActor(userId); + this.ensureInvitationRecipient(invite, actor); const dbTx = db.transaction(() => { - // Add member - this.addMember(invite.workspace_id, userId, invite.role); + // Avoid duplicate member inserts while preserving existing member privileges. + if (!this.isMember(invite.workspace_id, userId)) { + this.addMember(invite.workspace_id, userId, invite.role); + } // Mark invite as accepted - db.prepare("UPDATE workspace_invitations SET status = 'accepted' WHERE id = ?").run(invite.id); + db.prepare("UPDATE workspace_invitations SET status = 'accepted' WHERE id = ? AND status = 'pending'").run(invite.id); }); dbTx(); @@ -540,11 +570,13 @@ class WorkspaceService { * Decline an invitation * @param {string} token */ - declineInvitation(token) { + declineInvitation(token, userId) { const invite = this.getInvitation(token); if (!invite) throw new Error('Invalid invitation'); + const actor = this.getInvitationActor(userId); + this.ensureInvitationRecipient(invite, actor); - db.prepare("UPDATE workspace_invitations SET status = 'declined' WHERE id = ?").run(invite.id); + db.prepare("UPDATE workspace_invitations SET status = 'declined' WHERE id = ? AND status = 'pending'").run(invite.id); return { id: invite.id, status: 'declined' }; } diff --git a/tests/integration/invitations.test.js b/tests/integration/invitations.test.js index 5c5021a..47fe8e3 100644 --- a/tests/integration/invitations.test.js +++ b/tests/integration/invitations.test.js @@ -14,8 +14,14 @@ const db = require('../../src/services/db'); describe('Invitations API integration tests', () => { let server; let adminAgent; + let inviteeAgent; + let intruderAgent; let invitationToken; + let expiredInvitationToken; let workspaceName; + let workspaceId; + let inviteeUserId; + let intruderUserId; const adminUser = { name: 'Invitation Admin', @@ -29,23 +35,53 @@ describe('Invitations API integration tests', () => { password: 'Password123!', }; + const intruderUser = { + name: 'Invitation Intruder', + email: `invitation_intruder_${Date.now()}@example.com`, + password: 'Password123!', + }; + + const expiredInviteeUser = { + name: 'Invitation Expired Invitee', + email: `invitation_expired_${Date.now()}@example.com`, + password: 'Password123!', + }; + before(async () => { server = app.listen(0); adminAgent = request.agent(server); + inviteeAgent = request.agent(server); + intruderAgent = request.agent(server); await adminAgent.post('/api/auth/register').send(adminUser).expect(201); await adminAgent.post('/api/auth/login').send({ email: adminUser.email, password: adminUser.password }).expect(200); - await request(server).post('/api/auth/register').send(inviteeUser).expect(201); + const inviteeRes = await request(server).post('/api/auth/register').send(inviteeUser).expect(201); + inviteeUserId = inviteeRes.body.user.id; + await adminAgent.post(`/api/admin/users/${inviteeUserId}/approve`).expect(200); + await inviteeAgent.post('/api/auth/login').send({ email: inviteeUser.email, password: inviteeUser.password }).expect(200); + + const intruderRes = await request(server).post('/api/auth/register').send(intruderUser).expect(201); + intruderUserId = intruderRes.body.user.id; + await adminAgent.post(`/api/admin/users/${intruderUserId}/approve`).expect(200); + await intruderAgent.post('/api/auth/login').send({ email: intruderUser.email, password: intruderUser.password }).expect(200); + + const expiredInviteeRes = await request(server).post('/api/auth/register').send(expiredInviteeUser).expect(201); + await adminAgent.post(`/api/admin/users/${expiredInviteeRes.body.user.id}/approve`).expect(200); const workspacesRes = await adminAgent.get('/api/workspaces').expect(200); + workspaceId = workspacesRes.body[0].id; workspaceName = workspacesRes.body[0].name; - const inviteRes = await adminAgent - .post(`/api/workspaces/${workspacesRes.body[0].id}/invite`) - .send({ email: inviteeUser.email, role: 'editor' }) - .expect(200); + const inviteRes = await adminAgent.post(`/api/workspaces/${workspaceId}/invite`).send({ email: inviteeUser.email, role: 'editor' }).expect(200); invitationToken = inviteRes.body.token; + + const expiredInviteRes = await adminAgent + .post(`/api/workspaces/${workspaceId}/invite`) + .send({ email: expiredInviteeUser.email, role: 'viewer' }) + .expect(200); + + expiredInvitationToken = expiredInviteRes.body.token; }); after(() => { @@ -67,9 +103,9 @@ describe('Invitations API integration tests', () => { }); it('returns a minimized expired response when the invitation is no longer valid', async () => { - db.prepare("UPDATE workspace_invitations SET expires_at = '2000-01-01T00:00:00.000Z' WHERE token = ?").run(invitationToken); + db.prepare("UPDATE workspace_invitations SET expires_at = '2000-01-01T00:00:00.000Z' WHERE token = ?").run(expiredInvitationToken); - const res = await request(server).get(`/api/invitations/${invitationToken}`).expect(410); + const res = await request(server).get(`/api/invitations/${expiredInvitationToken}`).expect(410); assert.deepStrictEqual(res.body, { workspaceName, @@ -77,4 +113,34 @@ describe('Invitations API integration tests', () => { expired: true, }); }); + + it('blocks invitation decline for authenticated users who are not the recipient', async () => { + const res = await intruderAgent.post(`/api/invitations/${invitationToken}/decline`).expect(403); + assert.strictEqual(res.body.error, 'Invitation does not belong to authenticated user'); + + const invitation = db.prepare('SELECT status FROM workspace_invitations WHERE token = ?').get(invitationToken); + assert.strictEqual(invitation.status, 'pending'); + }); + + it('blocks invitation acceptance for authenticated users who are not the recipient', async () => { + const res = await intruderAgent.post(`/api/invitations/${invitationToken}/accept`).expect(403); + assert.strictEqual(res.body.error, 'Invitation does not belong to authenticated user'); + + const invitation = db.prepare('SELECT status FROM workspace_invitations WHERE token = ?').get(invitationToken); + assert.strictEqual(invitation.status, 'pending'); + + const membership = db.prepare('SELECT 1 FROM workspace_members WHERE workspace_id = ? AND user_id = ?').get(workspaceId, intruderUserId); + assert.strictEqual(Boolean(membership), false); + }); + + it('allows the invitation recipient to accept and become a workspace member', async () => { + const acceptRes = await inviteeAgent.post(`/api/invitations/${invitationToken}/accept`).expect(200); + assert.strictEqual(acceptRes.body.workspaceId, workspaceId); + + const invitation = db.prepare('SELECT status FROM workspace_invitations WHERE token = ?').get(invitationToken); + assert.strictEqual(invitation.status, 'accepted'); + + const membership = db.prepare('SELECT 1 FROM workspace_members WHERE workspace_id = ? AND user_id = ?').get(workspaceId, inviteeUserId); + assert.strictEqual(Boolean(membership), true); + }); }); diff --git a/tests/unit/markdown-sanitizer.test.js b/tests/unit/markdown-sanitizer.test.js new file mode 100644 index 0000000..ca261b6 --- /dev/null +++ b/tests/unit/markdown-sanitizer.test.js @@ -0,0 +1,65 @@ +const { describe, it } = require('node:test'); +const assert = require('node:assert'); +const fs = require('node:fs'); +const path = require('node:path'); +const vm = require('node:vm'); +const { marked } = require('marked'); + +const sanitizerScript = fs.readFileSync(path.resolve(__dirname, '../../src/public/js/utils/markdown-sanitizer.js'), 'utf8'); + +const buildRenderer = ({ withDomPurify }) => { + const context = { + marked, + globalThis: null, + }; + + if (withDomPurify) { + context.DOMPurify = { + sanitize: (html) => + html + .replace(//gi, '') + .replace(/\s+on\w+=(?:"[^"]*"|'[^']*'|[^\s>]+)/gi, '') + .replace(/javascript:/gi, ''), + }; + } + + context.globalThis = context; + vm.createContext(context); + vm.runInContext(sanitizerScript, context); + return context.renderSafeMarkdown; +}; + +describe('markdown-sanitizer utility', () => { + it('sanitizes markdown render output using the configured sanitizer', () => { + const renderSafeMarkdown = buildRenderer({ withDomPurify: true }); + const html = renderSafeMarkdown( + `# Title + + + +[click](javascript:alert(1)) + +`, + ); + + assert.ok(!/ **text**'); + + assert.ok(/<script>alert\(1\)<\/script>/.test(html)); + assert.ok(!/