From 25acf14c8b652f8090bc319b60e508385987c2ec Mon Sep 17 00:00:00 2001 From: jelveh Date: Tue, 11 Aug 2026 22:10:04 -0700 Subject: [PATCH] test: cover the feedback service and store layers directly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The service owns every feedback business rule — target resolution, eligibility, message normalization, the durable caps, and the owner-email preconditions — but was only reachable through the controller's tests. Give it and the store their own suites so a regression names the layer it broke. Service coverage adds the branches the route tests could not reach: a blocked origin resolving to null rather than surfacing a 403, owners who are suspended or unsubscribed, length measured after normalization, the 24h cap window boundary, subject-header injection via the app title, and the email links being rooted at config.origin. The two describes already labelled `AppFeedbackService ...` move out of the controller test, which keeps only the caller-facing promise that a failed send still returns success. --- .../feedback/AppFeedbackController.test.ts | 142 +--- .../feedback/AppFeedbackService.test.ts | 681 ++++++++++++++++++ .../appFeedback/AppFeedbackStore.test.ts | 330 +++++++++ 3 files changed, 1016 insertions(+), 137 deletions(-) create mode 100644 src/backend/services/feedback/AppFeedbackService.test.ts create mode 100644 src/backend/stores/appFeedback/AppFeedbackStore.test.ts diff --git a/src/backend/controllers/feedback/AppFeedbackController.test.ts b/src/backend/controllers/feedback/AppFeedbackController.test.ts index b92edb3c2..122c49eda 100644 --- a/src/backend/controllers/feedback/AppFeedbackController.test.ts +++ b/src/backend/controllers/feedback/AppFeedbackController.test.ts @@ -565,122 +565,12 @@ describe('AppFeedbackController POST /', () => { // ── Owner email delivery ──────────────────────────────────────────── -describe('AppFeedbackService owner email', () => { - const mockEmailReady = () => { - mockEmailConfigured(); - return vi - .spyOn(server.clients.email, 'send') - .mockResolvedValue(undefined); - }; - - it('emails the confirmed owner with the verified sender email + reply-to', async () => { - const send = mockEmailReady(); - const { userId: ownerId } = await makeUser(); - await confirmOwnerEmail(ownerId); - const app = await makeApp(ownerId, { feedbackEnabled: true }); - const { actor, userId } = await makeUser(); - // A verified sender email is what gets shared and used as reply-to. - await confirmOwnerEmail(userId); - const sender = (await server.stores.user.getById(userId))!; - - await submit(actor, { app: app.name, message: 'hello dev' }); - - const owner = (await server.stores.user.getById(ownerId))!; - expect(send).toHaveBeenCalledTimes(1); - expect(send).toHaveBeenCalledWith( - owner.email, - 'app-user-feedback', - expect.objectContaining({ - owner_username: owner.username, - sender_username: sender.username, - sender_email: sender.email, - app_name: app.name, - message: 'hello dev', - }), - expect.objectContaining({ replyTo: sender.email }), - ); - - const rows = (await server.clients.db.read( - 'SELECT `email_sent` FROM `app_feedback` WHERE `user_id` = ?', - [userId], - )) as Array<{ email_sent: unknown }>; - expect(Boolean(rows[0]?.email_sent)).toBe(true); - }); - - it('does not share an unverified sender email (no reply-to)', async () => { - const send = mockEmailReady(); - const { userId: ownerId } = await makeUser(); - await confirmOwnerEmail(ownerId); - const app = await makeApp(ownerId, { feedbackEnabled: true }); - // Sender's email is left unverified (makeUser does not confirm it). - const { actor } = await makeUser(); - - await submit(actor, { app: app.name, message: 'hello dev' }); - - expect(send).toHaveBeenCalledTimes(1); - const [, , values, options] = send.mock.calls[0]; - expect((values as Record).sender_email).toBeNull(); - expect((options as { replyTo?: string } | undefined)?.replyTo).toBeUndefined(); - }); - - it('stores but does not email when the owner email is unconfirmed', async () => { - const send = mockEmailReady(); - const { userId: ownerId } = await makeUser(); - const app = await makeApp(ownerId, { feedbackEnabled: true }); - const { actor, userId } = await makeUser(); - - await submit(actor, { app: app.name, message: 'hello dev' }); - - expect(send).not.toHaveBeenCalled(); - const rows = (await server.clients.db.read( - 'SELECT `email_sent` FROM `app_feedback` WHERE `user_id` = ?', - [userId], - )) as Array<{ email_sent: unknown }>; - expect(Boolean(rows[0]?.email_sent)).toBe(false); - }); - - it('suppresses email past the per-app daily cap but still stores', async () => { - const send = mockEmailReady(); - const { userId: ownerId } = await makeUser(); - await confirmOwnerEmail(ownerId); - const app = await makeApp(ownerId, { feedbackEnabled: true }); - - // Seed the cap with already-emailed rows from other users. - for ( - let i = 0; - i < AppFeedbackService.PER_APP_DAILY_EMAIL_LIMIT; - i++ - ) { - const { userId: seedUserId } = await makeUser(); - const row = await server.stores.appFeedback.create({ - appId: app.id, - appUid: app.uid, - userId: seedUserId, - message: `seed ${i}`, - }); - await server.stores.appFeedback.markEmailSent(row.id); - } - - const { actor, userId } = await makeUser(); - const captured = await submit(actor, { - app: app.name, - message: 'past the cap', - }); - expect(captured.body).toEqual({}); - expect(send).not.toHaveBeenCalled(); - - const rows = (await server.clients.db.read( - 'SELECT `email_sent` FROM `app_feedback` WHERE `user_id` = ?', - [userId], - )) as Array<{ email_sent: unknown }>; - expect(rows).toHaveLength(1); - expect(Boolean(rows[0]?.email_sent)).toBe(false); - }); - +// Which submissions get emailed, and what the mail contains, is service +// logic covered in AppFeedbackService.test.ts. What the controller owes the +// caller is that mail trouble never becomes the sender's problem. +describe('AppFeedbackController owner email', () => { it('a failing email send never fails the request', async () => { - vi.spyOn(server.clients.email, 'isConfigured', 'get').mockReturnValue( - true, - ); + mockEmailConfigured(); vi.spyOn(server.clients.email, 'send').mockRejectedValue( new Error('smtp down'), ); @@ -702,25 +592,3 @@ describe('AppFeedbackService owner email', () => { expect(Boolean(rows[0]?.email_sent)).toBe(false); }); }); - -// ── Message normalization ─────────────────────────────────────────── - -describe('AppFeedbackService.normalizeMessage', () => { - it('unifies newlines, strips control chars, and trims', () => { - const service = server.services.appFeedback; - expect(service.normalizeMessage(' a\r\nb\rc ')).toBe( - 'a\nb\nc', - ); - expect(service.normalizeMessage('keep\ttabs\nand\nnewlines')).toBe( - 'keep\ttabs\nand\nnewlines', - ); - expect(service.normalizeMessage('a\u0000b\u0007c\u007F')).toBe('abc'); - }); - - it('returns null for non-strings and whitespace-only input', () => { - const service = server.services.appFeedback; - expect(service.normalizeMessage(42)).toBeNull(); - expect(service.normalizeMessage(' \n\t ')).toBeNull(); - expect(service.normalizeMessage(null)).toBeNull(); - }); -}); diff --git a/src/backend/services/feedback/AppFeedbackService.test.ts b/src/backend/services/feedback/AppFeedbackService.test.ts new file mode 100644 index 000000000..9a7c517e2 --- /dev/null +++ b/src/backend/services/feedback/AppFeedbackService.test.ts @@ -0,0 +1,681 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { + afterAll, + afterEach, + beforeAll, + describe, + expect, + it, + vi, +} from 'vitest'; +import { v4 as uuidv4 } from 'uuid'; +import { PuterServer } from '../../server.js'; +import { setupTestServer } from '../../testUtil.js'; +import { AppFeedbackService } from './AppFeedbackService.js'; + +// Drives the real wired service against the real stores and in-memory +// database; only the email transport (a genuine external boundary) is +// stubbed. The controller's own tests cover request parsing and route gates — +// everything here is the business logic the controller delegates to. + +// The email links the service builds are rooted at `config.origin`, which the +// default test config leaves unset. +const TEST_ORIGIN = 'https://puter.test'; + +let server: PuterServer; +let service: AppFeedbackService; + +beforeAll(async () => { + server = await setupTestServer({ origin: TEST_ORIGIN } as never); + service = server.services.appFeedback; +}); + +afterAll(async () => { + await server?.shutdown(); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +const makeUser = async (): Promise => { + const username = `fdbk-svc-${Math.random().toString(36).slice(2, 10)}`; + const user = await server.stores.user.create({ + username, + uuid: uuidv4(), + password: null, + email: `${username}@test.local`, + }); + return user.id; +}; + +const makeApp = async ( + ownerUserId: number, + opts: { feedbackEnabled?: boolean; name?: string; title?: string } = {}, +) => { + const name = + opts.name ?? `fdbk-svc-app-${Math.random().toString(36).slice(2, 10)}`; + return await server.stores.app.create( + { + name, + title: opts.title ?? `Feedback Service Test ${name}`, + index_url: `https://${name}.example.com`, + ...(opts.feedbackEnabled ? { feedback_enabled: 1 } : {}), + }, + { ownerUserId }, + ); +}; + +// Feedback is only offered when the deployment can deliver it; most tests +// want that baseline without asserting anything about the mail itself. +const mockEmailConfigured = () => + vi.spyOn(server.clients.email, 'isConfigured', 'get').mockReturnValue(true); + +const mockEmailReady = () => { + mockEmailConfigured(); + return vi.spyOn(server.clients.email, 'send').mockResolvedValue(undefined); +}; + +// Columns the user store has no setter for; written directly the way the +// admin tooling does, then the cached row is dropped. +const setUserFlags = async ( + userId: number, + flags: Partial<{ + email_confirmed: boolean; + suspended: boolean; + unsubscribed: boolean; + }>, +) => { + for (const [column, value] of Object.entries(flags)) { + await server.clients.db.write( + `UPDATE \`user\` SET \`${column}\` = ? WHERE \`id\` = ?`, + [server.clients.db.booleanValue(Boolean(value)), userId], + ); + } + await server.stores.user.invalidateById(userId); +}; + +// An owner who can actually receive mail: the default for delivery tests. +const makeDeliverableOwner = async (): Promise => { + const ownerId = await makeUser(); + await setUserFlags(ownerId, { email_confirmed: true }); + return ownerId; +}; + +const feedbackRows = async (userId: number) => + (await server.clients.db.read( + 'SELECT * FROM `app_feedback` WHERE `user_id` = ? ORDER BY `id`', + [userId], + )) as Array>; + +// -- Message normalization --------------------------------------------- + +describe('AppFeedbackService.normalizeMessage', () => { + it('unifies newlines, strips control chars, and trims', () => { + expect(service.normalizeMessage(' a\r\nb\rc ')).toBe('a\nb\nc'); + expect(service.normalizeMessage('keep\ttabs\nand\nnewlines')).toBe( + 'keep\ttabs\nand\nnewlines', + ); + expect(service.normalizeMessage('a\u0000b\u0007c\u007F')).toBe('abc'); + }); + + it('returns null for non-strings and whitespace-only input', () => { + expect(service.normalizeMessage(42)).toBeNull(); + expect(service.normalizeMessage(' \n\t ')).toBeNull(); + expect(service.normalizeMessage(null)).toBeNull(); + expect(service.normalizeMessage(undefined)).toBeNull(); + // Nothing but control characters normalizes away to nothing. + expect(service.normalizeMessage('\u0000\u0007')).toBeNull(); + }); +}); + +// -- Target resolution -------------------------------------------------- + +describe('AppFeedbackService.resolveTargetApp', () => { + it('resolves by uid and by name, including names starting with "app-"', async () => { + const ownerId = await makeUser(); + const app = await makeApp(ownerId); + const prefixed = await makeApp(ownerId, { + name: `app-fdbk-${Math.random().toString(36).slice(2, 10)}`, + }); + + expect(await service.resolveTargetApp({ app: app.uid })).toMatchObject({ + id: app.id, + }); + expect(await service.resolveTargetApp({ app: app.name })).toMatchObject( + { id: app.id }, + ); + // A "app-"-prefixed *name* must not be mistaken for a uid and lost. + expect( + await service.resolveTargetApp({ app: prefixed.name }), + ).toMatchObject({ id: prefixed.id }); + }); + + it('returns null when neither app nor origin is given, and for unknown apps', async () => { + expect(await service.resolveTargetApp({})).toBeNull(); + expect( + await service.resolveTargetApp({ app: 'no-such-app-xyz' }), + ).toBeNull(); + }); + + it('resolves an origin to the app whose index_url it matches', async () => { + const ownerId = await makeUser(); + const app = await makeApp(ownerId); + const origin = new URL(app.index_url).origin; + expect(await service.resolveTargetApp({ origin })).toMatchObject({ + id: app.id, + }); + }); + + it('treats a blocked origin as unknown rather than an error', async () => { + // To a feedback caller "blocked" and "unknown" mean the same thing: + // nobody is accepting feedback there. Surfacing the 403 would tell + // any page whether its origin is on the blocklist. + const ownerId = await makeUser(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + const origin = new URL(app.index_url).origin; + await server.clients.db.write( + 'INSERT INTO `blocked_app_origins` (`domain`, `include_subdomains`, `reason`) VALUES (?, ?, ?)', + [new URL(origin).host, 0, 'test'], + ); + server.services.appOriginBlocklist.invalidate(); + try { + expect(await service.resolveTargetApp({ origin })).toBeNull(); + } finally { + await server.clients.db.write( + 'DELETE FROM `blocked_app_origins` WHERE `domain` = ?', + [new URL(origin).host], + ); + server.services.appOriginBlocklist.invalidate(); + } + }); + + it('throws 400 on an unparseable origin', async () => { + await expect( + service.resolveTargetApp({ origin: 'not a url' }), + ).rejects.toMatchObject({ statusCode: 400 }); + }); +}); + +// -- Eligibility -------------------------------------------------------- + +describe('AppFeedbackService.acceptsFeedback', () => { + const app = { feedback_enabled: 1, owner_user_id: 7 }; + + it('requires opt-in, an owner, and a configured email transport', () => { + mockEmailConfigured(); + expect(service.acceptsFeedback(app)).toBe(true); + expect(service.acceptsFeedback(null)).toBe(false); + expect(service.acceptsFeedback({ ...app, feedback_enabled: 0 })).toBe( + false, + ); + expect(service.acceptsFeedback({ ...app, owner_user_id: null })).toBe( + false, + ); + }); + + it('is false without an email transport, even for an opted-in app', () => { + // The self-hosted no-SMTP default. Feedback rows have no other read + // path, so soliciting them here would store-and-lose every message + // while telling the sender it was delivered. + expect(server.clients.email.isConfigured).toBe(false); + expect(service.acceptsFeedback(app)).toBe(false); + }); +}); + +describe('AppFeedbackService.getTarget', () => { + it('returns the dialog fields for an opted-in app', async () => { + mockEmailConfigured(); + const ownerId = await makeUser(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + expect(await service.getTarget({ app: app.uid })).toEqual({ + enabled: true, + app: { name: app.name, title: app.title }, + }); + }); + + it('reports a resolved-but-ineligible app as disabled, still naming it', async () => { + // The dialog needs the name/title to say *which* app declined. + mockEmailConfigured(); + const ownerId = await makeUser(); + const app = await makeApp(ownerId); + expect(await service.getTarget({ app: app.name })).toEqual({ + enabled: false, + app: { name: app.name, title: app.title }, + }); + }); + + it('reports an unknown target as disabled with no app', async () => { + mockEmailConfigured(); + expect(await service.getTarget({ app: 'no-such-app-xyz' })).toEqual({ + enabled: false, + app: null, + }); + }); +}); + +// -- Submission --------------------------------------------------------- + +describe('AppFeedbackService.submit', () => { + it('stores the normalized message and returns the row uid', async () => { + mockEmailConfigured(); + const ownerId = await makeUser(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + const userId = await makeUser(); + + const result = await service.submit({ + userId, + app: app.uid, + message: ' Great\r\napp! ', + sourceEnv: 'app', + }); + + const rows = await feedbackRows(userId); + expect(rows).toHaveLength(1); + expect(rows[0]).toMatchObject({ + uid: result.uid, + app_uid: app.uid, + message: 'Great\napp!', + source_env: 'app', + source_origin: null, + }); + expect(Number(rows[0].app_id)).toBe(app.id); + }); + + it('stores the attested origin for web submissions', async () => { + mockEmailConfigured(); + const ownerId = await makeUser(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + const origin = new URL(app.index_url).origin; + const userId = await makeUser(); + + await service.submit({ + userId, + origin, + message: 'from the web', + sourceEnv: 'web', + sourceOrigin: origin, + }); + + expect((await feedbackRows(userId))[0]).toMatchObject({ + source_env: 'web', + source_origin: origin, + }); + }); + + it('throws 403 feedback_not_enabled for opted-out, unknown, and undeliverable targets', async () => { + const ownerId = await makeUser(); + const userId = await makeUser(); + + mockEmailConfigured(); + const optedOut = await makeApp(ownerId); + await expect( + service.submit({ userId, app: optedOut.name, message: 'hi' }), + ).rejects.toMatchObject({ + statusCode: 403, + legacyCode: 'feedback_not_enabled', + }); + await expect( + service.submit({ userId, app: 'no-such-app-xyz', message: 'hi' }), + ).rejects.toMatchObject({ statusCode: 403 }); + + // Same refusal when the deployment has no email transport at all. + vi.restoreAllMocks(); + const enabled = await makeApp(ownerId, { feedbackEnabled: true }); + await expect( + service.submit({ userId, app: enabled.name, message: 'hi' }), + ).rejects.toMatchObject({ + statusCode: 403, + legacyCode: 'feedback_not_enabled', + }); + expect(await feedbackRows(userId)).toHaveLength(0); + }); + + it('throws 400 for a message that is empty or too long after normalization', async () => { + mockEmailConfigured(); + const ownerId = await makeUser(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + const userId = await makeUser(); + + for (const message of [' ', '\u0000\u0007', '\r\n \t ']) { + await expect( + service.submit({ userId, app: app.name, message }), + ).rejects.toMatchObject({ + statusCode: 400, + legacyCode: 'bad_request', + }); + } + await expect( + service.submit({ + userId, + app: app.name, + message: 'x'.repeat(AppFeedbackService.MESSAGE_MAX_LENGTH + 1), + }), + ).rejects.toMatchObject({ statusCode: 400 }); + + // A message at exactly the limit is fine — the cap is inclusive. + await expect( + service.submit({ + userId, + app: app.name, + message: 'x'.repeat(AppFeedbackService.MESSAGE_MAX_LENGTH), + }), + ).resolves.toMatchObject({ uid: expect.any(String) }); + expect(await feedbackRows(userId)).toHaveLength(1); + }); + + it('measures length after normalization, not before', async () => { + // Padding and \r\n line endings must not push an otherwise-legal + // message over the limit. + mockEmailConfigured(); + const ownerId = await makeUser(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + const userId = await makeUser(); + const body = 'a\r\n'.repeat(AppFeedbackService.MESSAGE_MAX_LENGTH / 2); + + await expect( + service.submit({ userId, app: app.name, message: ` ${body} ` }), + ).resolves.toMatchObject({ uid: expect.any(String) }); + }); + + it('enforces the per-user-per-app daily cap with 429', async () => { + mockEmailConfigured(); + const ownerId = await makeUser(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + const userId = await makeUser(); + + for (let i = 0; i < AppFeedbackService.PER_USER_APP_DAILY_LIMIT; i++) { + await expect( + service.submit({ + userId, + app: app.name, + message: `message ${i}`, + }), + ).resolves.toMatchObject({ uid: expect.any(String) }); + } + await expect( + service.submit({ userId, app: app.name, message: 'one too many' }), + ).rejects.toMatchObject({ + statusCode: 429, + legacyCode: 'too_many_requests', + }); + expect(await feedbackRows(userId)).toHaveLength( + AppFeedbackService.PER_USER_APP_DAILY_LIMIT, + ); + }); + + it('enforces the per-user daily cap across apps with 429', async () => { + mockEmailConfigured(); + const ownerId = await makeUser(); + const target = await makeApp(ownerId, { feedbackEnabled: true }); + const userId = await makeUser(); + const other = await makeApp(ownerId, { feedbackEnabled: true }); + + for (let i = 0; i < AppFeedbackService.PER_USER_DAILY_LIMIT; i++) { + await server.stores.appFeedback.create({ + appId: other.id, + appUid: other.uid, + userId, + message: `seed ${i}`, + }); + } + // Under the per-app cap for `target`, over the all-apps cap. + await expect( + service.submit({ + userId, + app: target.name, + message: 'over the limit', + }), + ).rejects.toMatchObject({ statusCode: 429 }); + }); + + it('counts only rows inside the 24h window toward the caps', async () => { + mockEmailConfigured(); + const ownerId = await makeUser(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + const userId = await makeUser(); + const yesterday = Math.floor(Date.now() / 1000) - 25 * 60 * 60; + + for (let i = 0; i < AppFeedbackService.PER_USER_APP_DAILY_LIMIT; i++) { + const row = await server.stores.appFeedback.create({ + appId: app.id, + appUid: app.uid, + userId, + message: `stale ${i}`, + }); + await server.clients.db.write( + 'UPDATE `app_feedback` SET `created_at` = ? WHERE `id` = ?', + [yesterday, row.id], + ); + } + + await expect( + service.submit({ userId, app: app.name, message: 'new day' }), + ).resolves.toMatchObject({ uid: expect.any(String) }); + }); + + it('rolls the stored row back when a concurrent burst breaches the cap', async () => { + mockEmailConfigured(); + const ownerId = await makeUser(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + const userId = await makeUser(); + for (let i = 0; i < AppFeedbackService.PER_USER_APP_DAILY_LIMIT; i++) { + await server.stores.appFeedback.create({ + appId: app.id, + appUid: app.uid, + userId, + message: `seed ${i}`, + }); + } + // Simulate the losing side of the check-then-insert race: the + // pre-insert check reads a stale under-cap count; the post-insert + // recount sees the truth and must undo the insert. + vi.spyOn( + server.stores.appFeedback, + 'countByUserAndAppSince', + ).mockResolvedValueOnce(0); + + await expect( + service.submit({ + userId, + app: app.name, + message: 'raced past the cap', + }), + ).rejects.toMatchObject({ + statusCode: 429, + legacyCode: 'too_many_requests', + }); + expect(await feedbackRows(userId)).toHaveLength( + AppFeedbackService.PER_USER_APP_DAILY_LIMIT, + ); + }); +}); + +// -- Owner email delivery ----------------------------------------------- + +describe('AppFeedbackService owner email', () => { + it('emails the owner and shares a verified sender address as reply-to', async () => { + const send = mockEmailReady(); + const ownerId = await makeDeliverableOwner(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + const userId = await makeUser(); + // Only a verified sender email is shared and used as reply-to. + await setUserFlags(userId, { email_confirmed: true }); + const sender = (await server.stores.user.getById(userId))!; + const owner = (await server.stores.user.getById(ownerId))!; + + await service.submit({ userId, app: app.name, message: 'hello dev' }); + + expect(send).toHaveBeenCalledTimes(1); + expect(send).toHaveBeenCalledWith( + owner.email, + 'app-user-feedback', + expect.objectContaining({ + owner_username: owner.username, + sender_username: sender.username, + sender_email: sender.email, + app_name: app.name, + app_title: app.title, + message: 'hello dev', + }), + expect.objectContaining({ replyTo: sender.email }), + ); + expect(Boolean((await feedbackRows(userId))[0].email_sent)).toBe(true); + }); + + it('builds app and Dev Center links from the deployment origin', async () => { + // Both links must follow config.origin so they resolve on + // self-hosted deployments, not just puter.com. + const send = mockEmailReady(); + const ownerId = await makeDeliverableOwner(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + const userId = await makeUser(); + + await service.submit({ userId, app: app.name, message: 'links' }); + + const [, , values] = send.mock.calls[0]; + expect(values).toMatchObject({ + app_link: `${TEST_ORIGIN}/app/${encodeURIComponent(app.name)}`, + dev_center_link: `${TEST_ORIGIN}/app/dev-center`, + }); + }); + + it('collapses whitespace in the app title so it cannot forge header lines', async () => { + // app_title lands in the subject; a newline there would let a + // developer-controlled title inject its own headers or body lines. + const send = mockEmailReady(); + const ownerId = await makeDeliverableOwner(); + const app = await makeApp(ownerId, { + feedbackEnabled: true, + title: 'Evil\r\nBcc: victim@example.com\tApp', + }); + const userId = await makeUser(); + + await service.submit({ userId, app: app.name, message: 'hi' }); + + const [, , values] = send.mock.calls[0]; + expect((values as Record).app_title).toBe( + 'Evil Bcc: victim@example.com App', + ); + }); + + it('does not share an unverified sender email and sets no reply-to', async () => { + const send = mockEmailReady(); + const ownerId = await makeDeliverableOwner(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + // Sender's email is left unverified — it could be anyone's, so + // pointing the developer's reply at it is not safe. + const userId = await makeUser(); + + await service.submit({ userId, app: app.name, message: 'hello dev' }); + + expect(send).toHaveBeenCalledTimes(1); + const [, , values, options] = send.mock.calls[0]; + expect((values as Record).sender_email).toBeNull(); + expect( + (options as { replyTo?: string } | undefined)?.replyTo, + ).toBeUndefined(); + // Delivery still happens; only the reply path is withheld. + expect(Boolean((await feedbackRows(userId))[0].email_sent)).toBe(true); + }); + + it('stores without emailing when the owner cannot or will not receive mail', async () => { + const cases: Array<[string, Parameters[1]]> = [ + ['unconfirmed email', { email_confirmed: false }], + ['suspended', { email_confirmed: true, suspended: true }], + ['unsubscribed', { email_confirmed: true, unsubscribed: true }], + ]; + for (const [label, flags] of cases) { + const send = mockEmailReady(); + const ownerId = await makeUser(); + await setUserFlags(ownerId, flags); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + const userId = await makeUser(); + + await service.submit({ + userId, + app: app.name, + message: `owner is ${label}`, + }); + + expect(send, label).not.toHaveBeenCalled(); + const rows = await feedbackRows(userId); + expect(rows, label).toHaveLength(1); + expect(Boolean(rows[0].email_sent), label).toBe(false); + vi.restoreAllMocks(); + } + }); + + it('stores but does not email past the per-app daily email cap', async () => { + const send = mockEmailReady(); + const ownerId = await makeDeliverableOwner(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + + // Seed the cap with already-emailed rows from other users — the cap + // bounds mail per app, not per sender. + for (let i = 0; i < AppFeedbackService.PER_APP_DAILY_EMAIL_LIMIT; i++) { + const row = await server.stores.appFeedback.create({ + appId: app.id, + appUid: app.uid, + userId: await makeUser(), + message: `seed ${i}`, + }); + await server.stores.appFeedback.markEmailSent(row.id); + } + + const userId = await makeUser(); + await expect( + service.submit({ userId, app: app.name, message: 'past the cap' }), + ).resolves.toMatchObject({ uid: expect.any(String) }); + + expect(send).not.toHaveBeenCalled(); + const rows = await feedbackRows(userId); + expect(rows).toHaveLength(1); + // The claimed slot was released, so the cap count stays exact. + expect(Boolean(rows[0].email_sent)).toBe(false); + expect( + await server.stores.appFeedback.countEmailedByAppSince( + app.id, + Math.floor(Date.now() / 1000) - 24 * 60 * 60, + ), + ).toBe(AppFeedbackService.PER_APP_DAILY_EMAIL_LIMIT); + }); + + it('keeps the feedback and releases the email slot when the send fails', async () => { + mockEmailConfigured(); + vi.spyOn(server.clients.email, 'send').mockRejectedValue( + new Error('smtp down'), + ); + const ownerId = await makeDeliverableOwner(); + const app = await makeApp(ownerId, { feedbackEnabled: true }); + const userId = await makeUser(); + + await expect( + service.submit({ userId, app: app.name, message: 'still stored' }), + ).resolves.toMatchObject({ uid: expect.any(String) }); + + const rows = await feedbackRows(userId); + expect(rows).toHaveLength(1); + // Slot released — a failed send must not consume the app's daily + // email budget. + expect(Boolean(rows[0].email_sent)).toBe(false); + }); +}); diff --git a/src/backend/stores/appFeedback/AppFeedbackStore.test.ts b/src/backend/stores/appFeedback/AppFeedbackStore.test.ts new file mode 100644 index 000000000..53ef979e4 --- /dev/null +++ b/src/backend/stores/appFeedback/AppFeedbackStore.test.ts @@ -0,0 +1,330 @@ +/* + * Copyright (C) 2024-present Puter Technologies Inc. + * + * This file is part of Puter. + * + * Puter is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { v4 as uuidv4 } from 'uuid'; +import { PuterServer } from '../../server.js'; +import { setupTestServer } from '../../testUtil.js'; +import type { AppFeedbackStore } from './AppFeedbackStore.js'; + +// Runs against the real in-memory database, so the SQL (and the engine's +// boolean/bigint representations) is exercised rather than described. + +let server: PuterServer; +let store: AppFeedbackStore; + +beforeAll(async () => { + server = await setupTestServer(); + store = server.stores.appFeedback; +}); + +afterAll(async () => { + await server?.shutdown(); +}); + +const makeUser = async (): Promise => { + const username = `fdbk-store-${Math.random().toString(36).slice(2, 10)}`; + const user = await server.stores.user.create({ + username, + uuid: uuidv4(), + password: null, + email: `${username}@test.local`, + }); + return user.id; +}; + +const makeApp = async (ownerUserId: number) => { + const name = `fdbk-store-app-${Math.random().toString(36).slice(2, 10)}`; + return await server.stores.app.create( + { + name, + title: `Feedback Store Test ${name}`, + index_url: `https://${name}.example.com`, + }, + { ownerUserId }, + ); +}; + +// Each test gets its own user and app so the counts can't see other tests' +// rows — the table is shared across the whole file. +const makeScope = async () => { + const ownerId = await makeUser(); + const [app, userId] = await Promise.all([makeApp(ownerId), makeUser()]); + return { app, userId }; +}; + +const readRow = async (id: number) => { + const rows = (await server.clients.db.read( + 'SELECT * FROM `app_feedback` WHERE `id` = ?', + [id], + )) as Array>; + return rows[0] ?? null; +}; + +// Backdate a row so window-scoped counts can be tested without waiting. +const setCreatedAt = async (id: number, createdAt: number) => { + await server.clients.db.write( + 'UPDATE `app_feedback` SET `created_at` = ? WHERE `id` = ?', + [createdAt, id], + ); +}; + +const HOUR = 60 * 60; +const now = () => Math.floor(Date.now() / 1000); + +describe('AppFeedbackStore.create', () => { + it('inserts a row with a fresh uid, email_sent false, and a unix timestamp', async () => { + const { app, userId } = await makeScope(); + const before = now(); + + const created = await store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: 'hello dev', + }); + + expect(created.id).toBeGreaterThan(0); + expect(created.uid).toMatch( + /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/, + ); + + const row = (await readRow(created.id))!; + expect(row).toMatchObject({ + uid: created.uid, + app_uid: app.uid, + message: 'hello dev', + }); + // Engine-agnostic reads: pg returns BIGINT as string and BOOLEAN as + // boolean, sqlite returns numbers for both. + expect(Number(row.app_id)).toBe(app.id); + expect(Number(row.user_id)).toBe(userId); + expect(Boolean(row.email_sent)).toBe(false); + expect(Number(row.created_at)).toBeGreaterThanOrEqual(before); + expect(Number(row.created_at)).toBeLessThanOrEqual(now()); + }); + + it('persists source_env / source_origin and defaults them to null', async () => { + const { app, userId } = await makeScope(); + + const withSource = await store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: 'from the web', + sourceEnv: 'web', + sourceOrigin: 'https://example.com', + }); + expect(await readRow(withSource.id)).toMatchObject({ + source_env: 'web', + source_origin: 'https://example.com', + }); + + const withoutSource = await store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: 'from the desktop', + }); + expect(await readRow(withoutSource.id)).toMatchObject({ + source_env: null, + source_origin: null, + }); + }); + + it('gives every row a distinct uid', async () => { + const { app, userId } = await makeScope(); + const rows = await Promise.all( + [1, 2, 3].map((n) => + store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: `msg ${n}`, + }), + ), + ); + expect(new Set(rows.map((r) => r.uid)).size).toBe(3); + }); +}); + +describe('AppFeedbackStore counts', () => { + it('countByUserSince counts only this user and only inside the window', async () => { + const { app, userId } = await makeScope(); + const otherUserId = await makeUser(); + const since = now() - 24 * HOUR; + + const recent = await store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: 'recent', + }); + const old = await store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: 'old', + }); + await setCreatedAt(old.id, since - HOUR); + await store.create({ + appId: app.id, + appUid: app.uid, + userId: otherUserId, + message: 'someone else', + }); + + expect(await store.countByUserSince(userId, since)).toBe(1); + // The boundary is inclusive — a row exactly at `since` still counts. + const row = (await readRow(recent.id))!; + expect( + await store.countByUserSince(userId, Number(row.created_at)), + ).toBe(1); + expect(await store.countByUserSince(otherUserId, since)).toBe(1); + }); + + it('countByUserSince spans apps, countByUserAndAppSince does not', async () => { + const { app, userId } = await makeScope(); + const otherApp = await makeApp(await makeUser()); + const since = now() - 24 * HOUR; + + for (const target of [app, app, otherApp]) { + await store.create({ + appId: target.id, + appUid: target.uid, + userId, + message: 'hi', + }); + } + + expect(await store.countByUserSince(userId, since)).toBe(3); + expect(await store.countByUserAndAppSince(userId, app.id, since)).toBe( + 2, + ); + expect( + await store.countByUserAndAppSince(userId, otherApp.id, since), + ).toBe(1); + }); + + it('countEmailedByAppSince counts only emailed rows in the window', async () => { + const { app, userId } = await makeScope(); + const since = now() - 24 * HOUR; + + const emailed = await store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: 'emailed', + }); + await store.markEmailSent(emailed.id); + // Not emailed. + await store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: 'stored only', + }); + // Emailed, but yesterday. + const stale = await store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: 'emailed long ago', + }); + await store.markEmailSent(stale.id); + await setCreatedAt(stale.id, since - HOUR); + + expect(await store.countEmailedByAppSince(app.id, since)).toBe(1); + }); + + it('returns 0 rather than null when an app has no feedback', async () => { + const { app, userId } = await makeScope(); + const since = now() - 24 * HOUR; + expect(await store.countEmailedByAppSince(app.id, since)).toBe(0); + expect(await store.countByUserSince(userId, since)).toBe(0); + expect(await store.countByUserAndAppSince(userId, app.id, since)).toBe( + 0, + ); + }); +}); + +describe('AppFeedbackStore email-sent claim', () => { + it('marks and unmarks a single row, and the count follows', async () => { + const { app, userId } = await makeScope(); + const since = now() - 24 * HOUR; + const row = await store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: 'claimed', + }); + const sibling = await store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: 'untouched', + }); + + await store.markEmailSent(row.id); + expect(Boolean((await readRow(row.id))!.email_sent)).toBe(true); + expect(Boolean((await readRow(sibling.id))!.email_sent)).toBe(false); + expect(await store.countEmailedByAppSince(app.id, since)).toBe(1); + + await store.unmarkEmailSent(row.id); + expect(Boolean((await readRow(row.id))!.email_sent)).toBe(false); + expect(await store.countEmailedByAppSince(app.id, since)).toBe(0); + }); +}); + +describe('AppFeedbackStore.deleteById', () => { + it('removes the row and leaves the rest of the app alone', async () => { + const { app, userId } = await makeScope(); + const since = now() - 24 * HOUR; + const doomed = await store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: 'rolled back', + }); + const kept = await store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: 'kept', + }); + + await store.deleteById(doomed.id); + + expect(await readRow(doomed.id)).toBeNull(); + expect(await readRow(kept.id)).not.toBeNull(); + expect(await store.countByUserSince(userId, since)).toBe(1); + }); + + it('is a no-op for an id that no longer exists', async () => { + const { app, userId } = await makeScope(); + const row = await store.create({ + appId: app.id, + appUid: app.uid, + userId, + message: 'gone', + }); + await store.deleteById(row.id); + await expect(store.deleteById(row.id)).resolves.toBeUndefined(); + }); +});