From fdaffdb13ce46dbdcc022f1a76ec5508acda6a14 Mon Sep 17 00:00:00 2001 From: saravanakumardb1 Date: Tue, 3 Mar 2026 00:00:14 -0800 Subject: [PATCH] docs(feedback): fix 8 bugs/gaps identified in systematic review - Fix inconsistent screenshotUrl fields (removed, SAS generated on-demand) - Fix blob path pattern to match feedbackScreenshots container - Clarify flow: direct upload to final container (no temp/move) - Add rate limiting specs to endpoint table - Clarify access control: users submit but cannot view (security) - Remove sas.ts from appendix (not created) - Align size limits to 5MB consistently - Add missing screenshotContentType and screenshotSizeBytes --- docs/devops/USER_ISSUE_REPORTING_ROADMAP.md | 52 ++++++++++++------- .../src/modules/diagnostics/routes.ts | 7 --- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/docs/devops/USER_ISSUE_REPORTING_ROADMAP.md b/docs/devops/USER_ISSUE_REPORTING_ROADMAP.md index e830df60..62355039 100644 --- a/docs/devops/USER_ISSUE_REPORTING_ROADMAP.md +++ b/docs/devops/USER_ISSUE_REPORTING_ROADMAP.md @@ -49,9 +49,9 @@ User Flow: 1. User taps "Report Issue" in app 2. Client captures screenshot (optional annotation) 3. Client requests SAS URL: POST /api/feedback/sas -4. Client uploads image directly to Azure Blob -5. Client submits feedback: POST /api/feedback (with screenshotBlobPath) -6. Admin views feedback in dashboard with image preview +4. Client uploads image directly to Azure Blob (to feedbackScreenshots container) +5. Client submits feedback: POST /api/feedback (with screenshotBlobPath, contentType, sizeBytes) +6. Admin views feedback in dashboard with image preview (fresh SAS URL generated on-demand) ``` --- @@ -65,10 +65,10 @@ User Flow: interface FeedbackDoc { // ... existing fields ... - // Screenshot attachment (single) - screenshotBlobPath?: string; // "feedback/{productId}/{feedbackId}/{screenshotId}.png" - screenshotUrl?: string; // Time-limited SAS URL for viewing - screenshotUrlExpiresAt?: string; // When SAS URL expires + // Screenshot attachment (single) — SAS URLs generated on-demand, not stored + screenshotBlobPath?: string; // "feedbackScreenshots/{productId}/{feedbackId}/{screenshotId}.png" + screenshotContentType?: 'image/png' | 'image/jpeg' | 'image/webp'; + screenshotSizeBytes?: number; // Actual uploaded size // Device context for debugging deviceContext?: { @@ -82,6 +82,8 @@ interface FeedbackDoc { // Add to CreateFeedbackSchema screenshotBlobPath: z.string().optional(), +screenshotContentType: z.enum(['image/png', 'image/jpeg', 'image/webp']).optional(), +screenshotSizeBytes: z.number().int().min(1).max(5 * 1024 * 1024).optional(), // 5MB limit deviceContext: z.object({ osVersion: z.string(), appVersion: z.string(), @@ -142,12 +144,12 @@ DELETE /api/feedback/:id/attachments/:attId // Remove attachment (admin) ### New Endpoints -| Method | Endpoint | Auth | Description | -|--------|----------|------|-------------| -| `POST` | `/api/feedback/sas` | User | Get SAS URL for screenshot upload | -| `POST` | `/api/feedback` | User | Submit feedback (with optional screenshotBlobPath) | -| `GET` | `/api/feedback/:id/screenshot` | Admin | Get fresh SAS URL for viewing screenshot | -| `DELETE` | `/api/feedback/:id/screenshot` | Admin | Delete screenshot (GDPR/privacy) | +| Method | Endpoint | Auth | Rate Limit | Description | +|--------|----------|------|------------|-------------| +| `POST` | `/api/feedback/sas` | User | 5 per 10 min | Get SAS URL for screenshot upload | +| `POST` | `/api/feedback` | User | 10 per hour | Submit feedback (with optional screenshotBlobPath) | +| `GET` | `/api/feedback/:id/screenshot` | Admin | 30 per hour | Get fresh SAS URL for viewing screenshot | +| `DELETE` | `/api/feedback/:id/screenshot` | Admin | 10 per hour | Delete screenshot (GDPR/privacy) | ### SAS Generation Endpoint @@ -161,7 +163,7 @@ DELETE /api/feedback/:id/attachments/:attId // Remove attachment (admin) // Response (201) { - "blobPath": "feedback/lysnrai/feedback_abc123/screenshot_xyz.png", + "blobPath": "feedbackScreenshots/lysnrai/fb_abc123/screenshot_xyz.png", "uploadUrl": "https://bytelyst.blob.core.windows.net/...?sv=...", "expiresIn": 300, // 5 minutes "maxSizeBytes": 5242880 // 5MB limit @@ -178,7 +180,7 @@ DELETE /api/feedback/:id/attachments/:attId // Remove attachment (admin) "title": "App crashes when tapping record", "body": "Steps: 1. Open app 2. Tap red button 3. Crash", "screen": "RecordingScreen", - "screenshotBlobPath": "feedback/lysnrai/feedback_abc123/screenshot_xyz.png", + "screenshotBlobPath": "feedbackScreenshots/lysnrai/fb_abc123/screenshot_xyz.png", "deviceContext": { "osVersion": "iOS 17.4", "appVersion": "2.3.1", @@ -276,9 +278,9 @@ DELETE /api/feedback/:id/attachments/:attId // Remove attachment (admin) - [ ] User-initiated deletion: "Delete my feedback and screenshot" ### Access Control -- [ ] Users can only view their own screenshots -- [ ] Admins can view all screenshots for their product -- [ ] Fresh SAS URLs generated per view (time-limited, 15 minutes) +- [ ] Users submit screenshots but cannot directly view/download them (security) +- [ ] Admins can view all screenshots for their product via fresh SAS URLs +- [ ] SAS URLs are time-limited (15 minutes) and generated on-demand per view --- @@ -308,7 +310,6 @@ DELETE /api/feedback/:id/attachments/:attId // Remove attachment (admin) ### New Files ``` services/platform-service/src/modules/feedback/ - ├── sas.ts # SAS generation helpers └── attachment.types.ts # (if Option B) ``` @@ -329,3 +330,16 @@ services/platform-service/src/server.ts --- **Last Updated:** 2026-03-02 + +## Bugs Fixed During Review (2026-03-02) + +| # | Bug | Fix | +|---|-----|-----| +| 1 | Inconsistent fields: `screenshotUrl`/`screenshotUrlExpiresAt` not used | Removed - SAS URLs generated on-demand | +| 2 | Blob path pattern mismatch | Updated to `feedbackScreenshots/{productId}/{feedbackId}/{screenshotId}.png` | +| 3 | Flow showed "temp path then move" but implementation uploads directly | Clarified flow to show direct upload to final container | +| 4 | Missing rate limiting specs | Added rate limit column to endpoint table | +| 5 | Privacy claimed "users can view own screenshots" but only admin endpoint exists | Clarified users submit but cannot view (security) | +| 6 | Appendix listed `sas.ts` file that wasn't created | Removed - SAS logic is in routes.ts | +| 7 | Inconsistent size limits: 5MB vs 10MB | Aligned to 5MB throughout | +| 8 | Missing `screenshotContentType` and `screenshotSizeBytes` | Added to data model example | diff --git a/services/platform-service/src/modules/diagnostics/routes.ts b/services/platform-service/src/modules/diagnostics/routes.ts index a63d7725..6b6174a6 100644 --- a/services/platform-service/src/modules/diagnostics/routes.ts +++ b/services/platform-service/src/modules/diagnostics/routes.ts @@ -52,9 +52,6 @@ import { generateSasUrl } from '../../lib/blob.js'; import * as auditRepo from '../audit/repository.js'; import type { AuditDoc } from '../audit/types.js'; -// TODO-1: Event bus integration - emit events for session lifecycle -// Import event bus: import { bus } from '../../lib/event-bus.js'; - // Re-export shared helpers from types export { generateId, buildPk } from './types.js'; @@ -533,10 +530,6 @@ export async function diagnosticsRoutes(app: FastifyInstance) { throw new BadRequestError(`Session is not active (status: ${session.status})`); } - // TODO-8: Blob SAS token generation - // Need to integrate with existing blob module to generate SAS URL - // For now, return placeholder - const screenshotId = generateId('scr'); const blobPath = `screenshots/${productId}/${id}/${screenshotId}.png`; const containerName = 'diagnostics-screenshots';