Merge pull request #132 from mean-weasel/feat/screenshot-security-hardening

Harden screenshot upload validation and security docs

Author: neonwatty

PRIVACY.md

@@ -23,15 +23,17 @@ When a user submits feedback through the BugDrop widget, the following data is c
 
 ## Where Data Goes
 
-All submitted feedback is sent to the **GitHub API** and created as a GitHub Issue in the repository configured by the site owner. Screenshots are stored in the repository's `.bugdrop/` directory. The BugDrop Cloudflare Worker acts only as a pass-through to authenticate with GitHub — it does not store any submitted data.
+All submitted feedback is sent to the **GitHub API** and created as a GitHub Issue in the repository configured by the site owner. Screenshots are stored in the repository's `.bugdrop/` directory on the `bugdrop-screenshots` branch. The BugDrop Cloudflare Worker acts only as a pass-through to authenticate with GitHub — it does not store any submitted data.
+
+Feedback and screenshots are unauthenticated user-generated content. The hosted service applies rate limits, size limits, and PNG screenshot validation, but it does not provide email-grade spam or malware filtering.
 
 ## Data Processing
 
 BugDrop runs on **Cloudflare Workers**. Requests are processed in-memory and are not logged or persisted by the BugDrop service. Cloudflare's standard infrastructure policies apply to network-level processing.
 
 ## Self-Hosting
 
-BugDrop is fully open source and self-hostable. If you run your own instance, you control all data processing. See [SELF_HOSTING.md](./SELF_HOSTING.md) for instructions.
+BugDrop is fully open source and self-hostable. If you run your own instance, you control all data processing and can add your own WAF, CAPTCHA, logging, retention, and content filtering controls. See [SELF_HOSTING.md](./SELF_HOSTING.md) for instructions.
 
 ## Contact
 

README.md

@@ -32,6 +32,8 @@ That's it! Users can now click the bug button to submit feedback as GitHub Issue
 
 > **Branch protection:** BugDrop works with repos that have branch protection rules (required PRs, merge queues). Screenshots are stored on a dedicated `bugdrop-screenshots` branch that is auto-created on first use — no manual setup needed.
 
+> **Security note:** BugDrop is not a spam or malware filtering service. Treat feedback and screenshots as unauthenticated user-generated content. Exclude `bugdrop-screenshots` from CI/deploy workflows, and self-host behind your own WAF/CAPTCHA/content controls for stricter environments.
+
 ## Widget Options
 
 | Attribute       | Values                                               | Default          |

TERMS.md

@@ -12,6 +12,7 @@ BugDrop is a free, open-source feedback widget that creates GitHub Issues with s
 - You may use the hosted widget on any website where you have authority to add scripts.
 - You must have the GitHub App installed on a repository you own or administer.
 - Do not use the service to submit spam, abusive content, or illegal material via GitHub Issues.
+- The hosted service is not a spam, malware, or content moderation service. Feedback submissions are unauthenticated user-generated content.
 
 ## No Warranty
 
@@ -21,6 +22,8 @@ THE SERVICE IS PROVIDED "AS IS" WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED
 
 The service enforces rate limits (10 requests per IP per 15 minutes, 50 per repository per hour) to prevent abuse and protect GitHub API quotas. Exceeding these limits will result in temporary throttling.
 
+For stricter security or compliance requirements, self-host BugDrop and apply your own WAF, CAPTCHA, logging, retention, and content filtering controls.
+
 ## GitHub
 
 BugDrop interacts with GitHub on your behalf via the GitHub App. Your use of GitHub is subject to [GitHub's Terms of Service](https://docs.github.com/en/site-policy/github-terms/github-terms-of-service). BugDrop requests only the minimum permissions needed: Issues (read/write) and Contents (read/write) on repositories where you install it.

docs/website/installation.mdx

@@ -104,6 +104,8 @@ If you have a strict CSP, make sure both the BugDrop worker domain and jsDelivr
 
 BugDrop stores screenshots in a dedicated branch called `bugdrop-screenshots` in your repository. This branch is created automatically when the first screenshot is uploaded.
 
+Treat this branch as user-generated content storage. Exclude `bugdrop-screenshots` from CI/deploy workflows and keep privileged workflows limited to `main` or other trusted branches.
+
 **If you use branch protection rules**, make sure the BugDrop GitHub App has permission to push to the `bugdrop-screenshots` branch. You can do this by either:
 
 1. **Excluding the branch from protection rules** -- Add `bugdrop-screenshots` to the exclusion list in your branch protection settings

docs/website/security.mdx

@@ -11,9 +11,10 @@ The BugDrop GitHub App requests the minimum permissions necessary to function:
 | **Issues** | Read & Write | Create bug reports, feature requests, and questions as GitHub Issues |
 | **Contents** | Read & Write | Store screenshots in the repository on the `bugdrop-screenshots` branch |
 
+GitHub App `Contents` permissions are repository-scoped, not branch-scoped. BugDrop's implementation writes screenshots only to `.bugdrop/screenshots/` on the dedicated `bugdrop-screenshots` branch, but GitHub does not provide a narrower branch-only permission for this API.
+
 BugDrop does **not** request access to:
 
-- Your source code (contents access is limited to the `bugdrop-screenshots` branch)
 - Pull requests
 - Actions or workflows
 - Secrets or environment variables
@@ -48,6 +49,8 @@ Screenshots are stored as image files in a `.bugdrop/` directory on a dedicated
 
 The screenshot branch is created automatically when the first screenshot is uploaded. No manual setup is required.
 
+Treat screenshots as unauthenticated user-generated content. The hosted service enforces rate limits, size limits, and PNG payload validation, but it is not a spam or malware filtering product.
+
 ### Screenshot Format
 
 Screenshots are captured client-side using [html2canvas](https://html2canvas.hertzen.com/), which renders the current page to a canvas element in the user's browser. The canvas is then converted to a PNG image and uploaded. This means:
@@ -56,6 +59,8 @@ Screenshots are captured client-side using [html2canvas](https://html2canvas.her
 - No server-side rendering or page access is required
 - The screenshot is generated entirely in the user's browser before being sent to the API
 
+Because clients are untrusted, the API validates screenshot uploads server-side before storing them. BugDrop currently accepts PNG data URLs only and rejects SVG, malformed base64, oversized payloads, and data that does not have a PNG file signature.
+
 ## Privacy
 
 BugDrop is built with a privacy-first approach:
@@ -132,8 +137,11 @@ If these limits are too restrictive for your use case, consider [self-hosting](/
 
 1. **Review app permissions** -- Periodically check the BugDrop GitHub App's permissions in your GitHub settings
 2. **Monitor the screenshots branch** -- Occasionally review the `bugdrop-screenshots` branch for unexpected content
-3. **Use branch protection** -- Keep your main branch protected. BugDrop only needs write access to the `bugdrop-screenshots` branch
-4. **Set up CSP** -- If you use a Content Security Policy, explicitly whitelist the required domains rather than using broad wildcards
+3. **Exclude screenshot storage from CI** -- Do not run privileged CI/deploy workflows on `bugdrop-screenshots`; treat it as user-generated content storage, not application source
+4. **Use branch protection** -- Keep your main branch protected and limit deploy/build workflows to `main` or other trusted branches
+5. **Set up CSP** -- If you use a Content Security Policy, explicitly whitelist the required domains rather than using broad wildcards
+
+The hosted service is intended for lightweight feedback collection. If your site is public, high-traffic, compliance-sensitive, or exposed to adversarial submissions, self-host BugDrop and place it behind your own WAF, CAPTCHA, logging, retention, and content filtering controls.
 
 ### For Self-Hosters
 
@@ -142,7 +150,9 @@ If you run your own instance of BugDrop:
 1. **Rotate your GitHub App credentials** regularly
 2. **Set appropriate rate limits** for your expected traffic
 3. **Monitor your Cloudflare Worker logs** for unusual activity
-4. **Keep your instance updated** with the latest version
+4. **Add edge protections** such as WAF rules, CAPTCHA, bot detection, and allowlists as needed
+5. **Define retention/cleanup** for the `bugdrop-screenshots` branch
+6. **Keep your instance updated** with the latest version
 
 ## Reporting Security Issues
 

src/routes/api.ts

@@ -101,18 +101,13 @@ api.post('/feedback', async c => {
     );
   }
 
-  // Validate screenshot size
+  // Validate screenshot payload. The browser widget emits PNG data URLs, but
+  // callers can hit the API directly, so the server must not trust the prefix.
   const maxSizeMB = parseInt(c.env.MAX_SCREENSHOT_SIZE_MB || '5', 10);
   if (payload.screenshot) {
-    const sizeBytes = (payload.screenshot.length * 3) / 4; // Base64 to bytes
-    const sizeMB = sizeBytes / (1024 * 1024);
-    if (sizeMB > maxSizeMB) {
-      return c.json(
-        {
-          error: `Screenshot too large: ${sizeMB.toFixed(1)}MB exceeds ${maxSizeMB}MB limit`,
-        },
-        400
-      );
+    const validation = validateScreenshotDataUrl(payload.screenshot, maxSizeMB);
+    if (!validation.valid) {
+      return c.json({ error: validation.error }, 400);
     }
   }
 
@@ -144,7 +139,7 @@ api.post('/feedback', async c => {
     // Upload screenshot as file and get URL
     let screenshotUrl: string | undefined;
     const imageData = payload.screenshot;
-    if (imageData && imageData.startsWith('data:image/')) {
+    if (imageData) {
       try {
         screenshotUrl = await uploadScreenshotAsAsset(token, owner, repo, imageData);
       } catch (error) {
@@ -190,6 +185,72 @@ api.post('/feedback', async c => {
   }
 });
 
+type ScreenshotValidationResult = { valid: true } | { valid: false; error: string };
+
+const PNG_SIGNATURE = [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a];
+
+function validateScreenshotDataUrl(dataUrl: string, maxSizeMB: number): ScreenshotValidationResult {
+  const match = dataUrl.match(/^data:image\/png;base64,([A-Za-z0-9+/]+={0,2})$/);
+  if (!match) {
+    return {
+      valid: false,
+      error: 'Invalid screenshot format. Expected a PNG data URL.',
+    };
+  }
+
+  const base64 = match[1];
+  if (!base64) {
+    return {
+      valid: false,
+      error: 'Invalid screenshot format. Expected a PNG data URL.',
+    };
+  }
+
+  const estimatedSizeBytes =
+    Math.floor((base64.length * 3) / 4) -
+    (base64.endsWith('==') ? 2 : base64.endsWith('=') ? 1 : 0);
+  const estimatedSizeMB = estimatedSizeBytes / (1024 * 1024);
+  if (estimatedSizeMB > maxSizeMB) {
+    return {
+      valid: false,
+      error: `Screenshot too large: ${estimatedSizeMB.toFixed(1)}MB exceeds ${maxSizeMB}MB limit`,
+    };
+  }
+
+  let bytes: Uint8Array;
+  try {
+    bytes = base64ToBytes(base64);
+  } catch {
+    return {
+      valid: false,
+      error: 'Invalid screenshot format. Expected valid base64 PNG data.',
+    };
+  }
+
+  if (!hasPngSignature(bytes)) {
+    return {
+      valid: false,
+      error: 'Invalid screenshot format. Expected PNG image data.',
+    };
+  }
+
+  return { valid: true };
+}
+
+function base64ToBytes(base64: string): Uint8Array {
+  const binary = atob(base64);
+  const bytes = new Uint8Array(binary.length);
+  for (let i = 0; i < binary.length; i++) {
+    bytes[i] = binary.charCodeAt(i);
+  }
+  return bytes;
+}
+
+function hasPngSignature(bytes: Uint8Array): boolean {
+  if (bytes.length < PNG_SIGNATURE.length) return false;
+  return PNG_SIGNATURE.every((byte, index) => bytes[index] === byte);
+}
+
 /**
  * Format the issue body with markdown
  */

test/api.test.ts

@@ -22,6 +22,9 @@ const createApiRoutes = async () => {
 };
 
 describe('API Routes', () => {
+  const validPngDataUrl =
+    'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==';
+
   let app: Hono;
   const mockEnv: Env = {
     GITHUB_APP_ID: 'test-app-id',
@@ -274,8 +277,6 @@ describe('API Routes', () => {
 
     it('should upload screenshot and include URL in issue body', async () => {
       mockGetInstallationToken.mockResolvedValue('test-token');
-      const screenshotDataUrl =
-        'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mNk+M9QDwADhgGAWjR9awAAAABJRU5ErkJggg==';
       const uploadedUrl =
         'https://raw.githubusercontent.com/testowner/testrepo/main/.feedback/screenshots/123456.png';
       mockUploadScreenshotAsAsset.mockResolvedValue(uploadedUrl);
@@ -286,7 +287,7 @@ describe('API Routes', () => {
 
       const payloadWithScreenshot = {
         ...validPayload,
-        screenshot: screenshotDataUrl,
+        screenshot: validPngDataUrl,
       };
 
       const req = new Request('http://localhost/feedback', {
@@ -301,7 +302,7 @@ describe('API Routes', () => {
         'test-token',
         'testowner',
         'testrepo',
-        screenshotDataUrl
+        validPngDataUrl
       );
       expect(mockCreateIssue).toHaveBeenCalledWith(
         'test-token',
@@ -315,7 +316,6 @@ describe('API Routes', () => {
 
     it('should use screenshot when provided (annotations handled client-side)', async () => {
       mockGetInstallationToken.mockResolvedValue('test-token');
-      const screenshotDataUrl = 'data:image/png;base64,screenshot';
       const uploadedUrl =
         'https://raw.githubusercontent.com/testowner/testrepo/main/.feedback/screenshots/789.png';
       mockUploadScreenshotAsAsset.mockResolvedValue(uploadedUrl);
@@ -326,7 +326,7 @@ describe('API Routes', () => {
 
       const payloadWithScreenshot = {
         ...validPayload,
-        screenshot: screenshotDataUrl,
+        screenshot: validPngDataUrl,
       };
 
       const req = new Request('http://localhost/feedback', {
@@ -340,7 +340,7 @@ describe('API Routes', () => {
         'test-token',
         'testowner',
         'testrepo',
-        screenshotDataUrl
+        validPngDataUrl
       );
     });
 
@@ -382,6 +382,60 @@ describe('API Routes', () => {
       expect(data.error).toContain('exceeds 5MB limit');
     });
 
+    it('should reject SVG screenshots', async () => {
+      const svgScreenshot =
+        'data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciPjwvc3ZnPg==';
+
+      const req = new Request('http://localhost/feedback', {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify({
+          ...validPayload,
+          screenshot: svgScreenshot,
+        }),
+      });
+      const res = await app.fetch(req, mockEnv);
+      const data = await res.json();
+
+      expect(res.status).toBe(400);
+      expect(data.error).toContain('Expected a PNG data URL');
+      expect(mockUploadScreenshotAsAsset).not.toHaveBeenCalled();
+    });
+
+    it('should reject invalid base64 screenshots', async () => {
+      const req = new Request('http://localhost/feedback', {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify({
+          ...validPayload,
+          screenshot: 'data:image/png;base64,not valid base64',
+        }),
+      });
+      const res = await app.fetch(req, mockEnv);
+      const data = await res.json();
+
+      expect(res.status).toBe(400);
+      expect(data.error).toContain('Expected a PNG data URL');
+      expect(mockUploadScreenshotAsAsset).not.toHaveBeenCalled();
+    });
+
+    it('should reject PNG data URLs with non-PNG bytes', async () => {
+      const req = new Request('http://localhost/feedback', {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify({
+          ...validPayload,
+          screenshot: 'data:image/png;base64,SGVsbG8=',
+        }),
+      });
+      const res = await app.fetch(req, mockEnv);
+      const data = await res.json();
+
+      expect(res.status).toBe(400);
+      expect(data.error).toContain('Expected PNG image data');
+      expect(mockUploadScreenshotAsAsset).not.toHaveBeenCalled();
+    });
+
     it('should return 500 when GitHub API fails', async () => {
       mockGetInstallationToken.mockResolvedValue('test-token');
       mockCreateIssue.mockRejectedValue(new Error('GitHub API error'));