fix: make feedback daily caps fail closed under concurrent submissions

The per-user and per-user-per-app caps were check-then-insert and the
per-app email cap was count-then-send, so parallel requests (or multiple
nodes, or the route limiter failing open) could all read a stale
under-cap count and push past every limit — the exact scenario the
DB-backed caps exist to stop.

Now the user caps recount after the insert (own row included) and roll
the row back with 429 if a burst breached them, and the email cap claims
its slot (email_sent=1) before sending, recounts, and releases the slot
if over cap or if the send fails.

src/backend/controllers/feedback/AppFeedbackController.test.ts

@@ -444,6 +444,42 @@ describe('AppFeedbackController POST /', () => {
         });
     });
 
+    it('rolls back the stored row when a concurrent burst breaches the cap', async () => {
+        const { userId: ownerId } = await makeUser();
+        const app = await makeApp(ownerId, { feedbackEnabled: true });
+        const { actor, 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 (real implementation) sees the truth.
+        vi.spyOn(
+            server.stores.appFeedback,
+            'countByUserAndAppSince',
+        ).mockResolvedValueOnce(0);
+
+        await expect(
+            submit(actor, { app: app.name, message: 'raced past the cap' }),
+        ).rejects.toMatchObject({
+            statusCode: 429,
+            legacyCode: 'too_many_requests',
+        });
+
+        const rows = (await server.clients.db.read(
+            'SELECT COUNT(*) AS n FROM `app_feedback` WHERE `user_id` = ?',
+            [userId],
+        )) as Array<{ n: unknown }>;
+        expect(Number(rows[0]?.n)).toBe(
+            AppFeedbackService.PER_USER_APP_DAILY_LIMIT,
+        );
+    });
+
     it('enforces the per-user daily cap across apps with 429', async () => {
         const { userId: ownerId } = await makeUser();
         const target = await makeApp(ownerId, { feedbackEnabled: true });

src/backend/services/feedback/AppFeedbackService.ts

@@ -188,24 +188,35 @@ export class AppFeedbackService extends PuterService {
 
         // Durable caps. Deliberately DB-backed (see class doc); the counts
         // ride the (user_id, created_at) / (app_id, created_at) indexes.
+        // `includeOwnRow` distinguishes the pre-insert check (this
+        // submission not yet counted) from the post-insert recount (it is).
         const since = Math.floor(Date.now() / 1000) - 24 * 60 * 60;
-        const [userAppCount, userCount] = await Promise.all([
-            this.stores.appFeedback.countByUserAndAppSince(
-                userId,
-                appId,
-                since,
-            ),
-            this.stores.appFeedback.countByUserSince(userId, since),
-        ]);
-        if (
-            userAppCount >= AppFeedbackService.PER_USER_APP_DAILY_LIMIT ||
-            userCount >= AppFeedbackService.PER_USER_DAILY_LIMIT
-        ) {
-            throw new HttpError(
+        const capsBreached = async (
+            includeOwnRow: boolean,
+        ): Promise<boolean> => {
+            const slack = includeOwnRow ? 1 : 0;
+            const [userAppCount, userCount] = await Promise.all([
+                this.stores.appFeedback.countByUserAndAppSince(
+                    userId,
+                    appId,
+                    since,
+                ),
+                this.stores.appFeedback.countByUserSince(userId, since),
+            ]);
+            return (
+                userAppCount >=
+                    AppFeedbackService.PER_USER_APP_DAILY_LIMIT + slack ||
+                userCount >= AppFeedbackService.PER_USER_DAILY_LIMIT + slack
+            );
+        };
+        const tooManyError = () =>
+            new HttpError(
                 429,
                 'You have sent a lot of feedback recently — please try again later',
                 { legacyCode: 'too_many_requests' },
             );
+        if (await capsBreached(false)) {
+            throw tooManyError();
         }
 
         const row = await this.stores.appFeedback.create({
@@ -217,6 +228,16 @@ export class AppFeedbackService extends PuterService {
             sourceOrigin: sourceOrigin ?? null,
         });
 
+        // The pre-check is check-then-insert, so parallel submissions (or
+        // multiple nodes) can all pass it on the same stale count. Recount
+        // with this row included and roll it back if a concurrent burst
+        // pushed past a cap — these caps must fail closed, not just usually
+        // hold.
+        if (await capsBreached(true)) {
+            await this.stores.appFeedback.deleteById(row.id);
+            throw tooManyError();
+        }
+
         // Email delivery is best-effort: any failure past this point must
         // not fail the request — the feedback is already stored.
         try {
@@ -271,9 +292,17 @@ export class AppFeedbackService extends PuterService {
         }
         if (!(await this.clients.email.validate(owner.email))) return;
 
+        // Claim an email-cap slot *before* sending: flip email_sent, recount
+        // with the claim included, and release the slot if a concurrent
+        // burst pushed past the cap. Counting before sending would fail
+        // open — parallel submissions could each read an under-cap count and
+        // all send. The cost is that a crash mid-send burns a slot without
+        // delivering; the cap is an upper bound, not a quota owed.
+        await this.stores.appFeedback.markEmailSent(feedbackId);
         const emailedToday =
             await this.stores.appFeedback.countEmailedByAppSince(appId, since);
-        if (emailedToday >= AppFeedbackService.PER_APP_DAILY_EMAIL_LIMIT) {
+        if (emailedToday > AppFeedbackService.PER_APP_DAILY_EMAIL_LIMIT) {
+            await this.stores.appFeedback.unmarkEmailSent(feedbackId);
             return;
         }
 
@@ -287,23 +316,30 @@ export class AppFeedbackService extends PuterService {
         const senderEmail =
             sender?.email && sender.email_confirmed ? sender.email : null;
 
-        await this.clients.email.send(
-            owner.email,
-            'app-user-feedback',
-            {
-                owner_username: owner.username,
-                sender_username: sender?.username ?? 'A Puter user',
-                sender_email: senderEmail,
-                // Collapse whitespace so a crafted title can't break the
-                // subject header or spoof extra lines in the body.
-                app_title: String(app.title ?? app.name).replace(/\s+/g, ' '),
-                app_name: String(app.name),
-                app_link: `${this.config.origin}/app/${encodeURIComponent(String(app.name))}`,
-                message,
-            },
-            senderEmail ? { replyTo: senderEmail } : {},
-        );
-
-        await this.stores.appFeedback.markEmailSent(feedbackId);
+        try {
+            await this.clients.email.send(
+                owner.email,
+                'app-user-feedback',
+                {
+                    owner_username: owner.username,
+                    sender_username: sender?.username ?? 'A Puter user',
+                    sender_email: senderEmail,
+                    // Collapse whitespace so a crafted title can't break the
+                    // subject header or spoof extra lines in the body.
+                    app_title: String(app.title ?? app.name).replace(
+                        /\s+/g,
+                        ' ',
+                    ),
+                    app_name: String(app.name),
+                    app_link: `${this.config.origin}/app/${encodeURIComponent(String(app.name))}`,
+                    message,
+                },
+                senderEmail ? { replyTo: senderEmail } : {},
+            );
+        } catch (e) {
+            // Release the claimed slot — the mail never went out.
+            await this.stores.appFeedback.unmarkEmailSent(feedbackId);
+            throw e;
+        }
     }
 }

src/backend/stores/appFeedback/AppFeedbackStore.ts

@@ -89,9 +89,10 @@ export class AppFeedbackStore extends PuterStore {
     // -- Writes -------------------------------------------------------
 
     /**
-     * Insert one feedback row. `email_sent` starts false; the service flips it
-     * with {@link markEmailSent} after the owner email actually goes out, so the
-     * email cap only counts delivered mail.
+     * Insert one feedback row. `email_sent` starts false; the service claims it
+     * with {@link markEmailSent} before sending and releases it with
+     * {@link unmarkEmailSent} if the cap was breached or the send failed, so the
+     * email cap fails closed under concurrent submissions.
      */
     async create(fields: {
         appId: number;
@@ -127,11 +128,31 @@ export class AppFeedbackStore extends PuterStore {
         return { id: Number(insertId), uid };
     }
 
-    /** Record that the owner email for this row was sent. */
+    /** Record that the owner email for this row was sent (claim a cap slot). */
     async markEmailSent(id: number): Promise<void> {
+        await this.#setEmailSent(id, true);
+    }
+
+    /**
+     * Revert {@link markEmailSent} — releases a claimed email-cap slot when the
+     * cap turned out breached or the send failed.
+     */
+    async unmarkEmailSent(id: number): Promise<void> {
+        await this.#setEmailSent(id, false);
+    }
+
+    async #setEmailSent(id: number, sent: boolean): Promise<void> {
         await this.clients.db.write(
             'UPDATE `app_feedback` SET `email_sent` = ? WHERE `id` = ?',
-            [this.clients.db.booleanValue(true), id],
+            [this.clients.db.booleanValue(sent), id],
+        );
+    }
+
+    /** Delete one feedback row. Backs the service's cap-race rollback. */
+    async deleteById(id: number): Promise<void> {
+        await this.clients.db.write(
+            'DELETE FROM `app_feedback` WHERE `id` = ?',
+            [id],
         );
     }
 }