lee/puter
1
0
Fork 0
mirror of https://github.com/HeyPuter/puter.git synced 2026-08-24 23:17:23 +00:00
Code Issues Actions 10 Packages Projects Releases Wiki Activity

fix: refuse feedback when the deployment cannot deliver it

Browse Source 
With no email transport configured (the common self-hosted default),
submissions were stored in app_feedback — a table with no read path
beyond the abuse-cap COUNTs — the owner email was silently skipped, and
the sender was still shown 'Feedback sent. Thank you!'. The developer
never learns the feedback exists while the user believes it was
delivered.

Gate acceptsFeedback on clients.email.isConfigured so the pre-flight
reports enabled:false (the dialog shows its 'not accepting feedback'
pane) and submit returns 403 instead of swallowing messages. Owner-level
store-without-email cases (unconfirmed owner email, per-app email cap
overflow) keep their existing deliberate semantics.
This commit is contained in:
Nariman Jelveh
2026-08-11 18:16:14 -07:00
parent 4f96d371f4
commit 1460751b41
2 changed files with 62 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/backend/controllers/feedback/AppFeedbackController.test.ts
+51 -3
View File
@@ -99,6 +99,14 @@ const makeApp = async (
);
};
// Feedback is only offered when the deployment can deliver it (email
// transport configured); most tests want that baseline without asserting
// anything about the mail itself.
const mockEmailConfigured = () =>
vi.spyOn(server.clients.email, 'isConfigured', 'get').mockReturnValue(
true,
);
const confirmOwnerEmail = async (userId: number) => {
await server.clients.db.write(
'UPDATE `user` SET `email_confirmed` = ? WHERE `id` = ?',
@@ -222,6 +230,7 @@ describe('AppFeedbackController GET /target', () => {
});
it('reports enabled:false for an app that has not opted in', async () => {
mockEmailConfigured();
const { userId: ownerId } = await makeUser();
const app = await makeApp(ownerId);
const { actor } = await makeUser();
@@ -236,6 +245,7 @@ describe('AppFeedbackController GET /target', () => {
});
it('reports enabled:true with canonical title/name for an opted-in app', async () => {
mockEmailConfigured();
const { userId: ownerId } = await makeUser();
const app = await makeApp(ownerId, { feedbackEnabled: true });
const { actor } = await makeUser();
@@ -253,6 +263,7 @@ describe('AppFeedbackController GET /target', () => {
});
it('resolves an opted-in app whose name starts with "app-"', async () => {
mockEmailConfigured();
const { userId: ownerId } = await makeUser();
const name = `app-fdbk-${Math.random().toString(36).slice(2, 10)}`;
const app = await makeApp(ownerId, { feedbackEnabled: true, name });
@@ -271,6 +282,7 @@ describe('AppFeedbackController GET /target', () => {
});
it('resolves an origin to the app whose index_url it matches', async () => {
mockEmailConfigured();
const { userId: ownerId } = await makeUser();
const app = await makeApp(ownerId, { feedbackEnabled: true });
const origin = new URL(app.index_url).origin;
@@ -288,6 +300,22 @@ describe('AppFeedbackController GET /target', () => {
});
});
it('reports enabled:false when the email transport is unconfigured', async () => {
// No mockEmailConfigured(): this is the self-hosted no-SMTP default.
// Feedback that can never be delivered must not be solicited.
const { userId: ownerId } = await makeUser();
const app = await makeApp(ownerId, { feedbackEnabled: true });
const { actor } = await makeUser();
const { res, captured } = makeRes();
await callRoute(
'get',
'/target',
makeReq({ query: { app: app.name }, actor }),
res,
);
expect(captured.body).toMatchObject({ enabled: false });
});
it('reports enabled:false for an origin with no registered app', async () => {
const { actor } = await makeUser();
const { res, captured } = makeRes();
@@ -330,6 +358,7 @@ describe('AppFeedbackController POST /', () => {
});
it('throws 403 feedback_not_enabled when the app has not opted in', async () => {
mockEmailConfigured();
const { userId: ownerId } = await makeUser();
const app = await makeApp(ownerId);
const { actor } = await makeUser();
@@ -354,7 +383,23 @@ describe('AppFeedbackController POST /', () => {
).rejects.toMatchObject({ statusCode: 403 });
});
it('throws 403 feedback_not_enabled when the email transport is unconfigured', async () => {
// No mockEmailConfigured(): the opted-in app must still refuse — a
// stored row nothing can read, sold to the sender as delivered, is
// worse than an honest refusal.
const { userId: ownerId } = await makeUser();
const app = await makeApp(ownerId, { feedbackEnabled: true });
const { actor } = await makeUser();
await expect(
submit(actor, { app: app.name, message: 'into the void' }),
).rejects.toMatchObject({
statusCode: 403,
legacyCode: 'feedback_not_enabled',
});
});
it('throws 400 when the message exceeds the length limit', async () => {
mockEmailConfigured();
const { userId: ownerId } = await makeUser();
const app = await makeApp(ownerId, { feedbackEnabled: true });
const { actor } = await makeUser();
@@ -369,6 +414,7 @@ describe('AppFeedbackController POST /', () => {
});
it('stores a normalized row and responds with an empty object', async () => {
mockEmailConfigured();
const { userId: ownerId } = await makeUser();
const app = await makeApp(ownerId, { feedbackEnabled: true });
const { actor, userId } = await makeUser();
@@ -399,6 +445,7 @@ describe('AppFeedbackController POST /', () => {
});
it('records the attested origin on web submissions', async () => {
mockEmailConfigured();
const { userId: ownerId } = await makeUser();
const app = await makeApp(ownerId, { feedbackEnabled: true });
const origin = new URL(app.index_url).origin;
@@ -417,6 +464,7 @@ describe('AppFeedbackController POST /', () => {
});
it('enforces the per-user-per-app daily cap with 429', async () => {
mockEmailConfigured();
const { userId: ownerId } = await makeUser();
const app = await makeApp(ownerId, { feedbackEnabled: true });
const { actor, userId } = await makeUser();
@@ -445,6 +493,7 @@ describe('AppFeedbackController POST /', () => {
});
it('rolls back the stored row when a concurrent burst breaches the cap', async () => {
mockEmailConfigured();
const { userId: ownerId } = await makeUser();
const app = await makeApp(ownerId, { feedbackEnabled: true });
const { actor, userId } = await makeUser();
@@ -481,6 +530,7 @@ describe('AppFeedbackController POST /', () => {
});
it('enforces the per-user daily cap across apps with 429', async () => {
mockEmailConfigured();
const { userId: ownerId } = await makeUser();
const target = await makeApp(ownerId, { feedbackEnabled: true });
const other = await makeApp(ownerId, { feedbackEnabled: true });
@@ -503,9 +553,7 @@ describe('AppFeedbackController POST /', () => {
describe('AppFeedbackService owner email', () => {
const mockEmailReady = () => {
vi.spyOn(server.clients.email, 'isConfigured', 'get').mockReturnValue(
true,
);
mockEmailConfigured();
return vi
.spyOn(server.clients.email, 'send')
.mockResolvedValue(undefined);
src/backend/services/feedback/AppFeedbackService.ts
+11 -2
View File
@@ -114,10 +114,19 @@ export class AppFeedbackService extends PuterService {
/**
* Whether `app` (a row from AppStore) currently accepts user feedback: the
* developer opted in and the app has an owner to deliver to.
* developer opted in, the app has an owner to deliver to, and this
* deployment can deliver at all (email transport configured). Without a
* transport every submission would be stored-and-lost — the rows have no
* other read path — while the sender is told it was sent, so the feature
* reports itself unavailable instead.
*/
acceptsFeedback(app: Record<string, unknown> | null): boolean {
return Boolean(app && app.feedback_enabled && app.owner_user_id);
return Boolean(
this.clients.email.isConfigured &&
app &&
app.feedback_enabled &&
app.owner_user_id,
);
}
/**