fix: enhance attachment handling and folder deletion logic; improve error responses and rate limiting

src/services/ratelimit.ts

@@ -6,7 +6,7 @@
 // Rate limit configuration
 const CONFIG = {
   // Friendly default: short cooldown instead of long lockouts.
-  LOGIN_MAX_ATTEMPTS: 8,
+  LOGIN_MAX_ATTEMPTS: 5,
   LOGIN_LOCKOUT_MINUTES: 2,
 
   // Write operations only (POST/PUT/DELETE/PATCH) should use this budget.
@@ -113,18 +113,26 @@ export class RateLimitService {
     await this.db.prepare('DELETE FROM login_attempts_ip WHERE ip = ?').bind(key).run();
   }
 
-  async checkApiRateLimit(identifier: string): Promise<{ allowed: boolean; remaining: number; retryAfterSeconds?: number }> {
+  // Atomically consume one write budget unit for the current fixed window.
+  // Uses SQLite UPSERT-with-WHERE so requests at/over limit do not increment.
+  async consumeApiWriteBudget(identifier: string): Promise<{ allowed: boolean; remaining: number; retryAfterSeconds?: number }> {
     const nowSec = Math.floor(Date.now() / 1000);
     const windowStart = nowSec - (nowSec % CONFIG.API_WINDOW_SECONDS);
     const windowEnd = windowStart + CONFIG.API_WINDOW_SECONDS;
 
     const row = await this.db
-      .prepare('SELECT count FROM api_rate_limits WHERE identifier = ? AND window_start = ?')
-      .bind(identifier, windowStart)
+      .prepare(
+        'INSERT INTO api_rate_limits(identifier, window_start, count) VALUES(?, ?, 1) ' +
+        'ON CONFLICT(identifier, window_start) DO UPDATE SET count = count + 1 ' +
+        'WHERE api_rate_limits.count < ? ' +
+        'RETURNING count'
+      )
+      .bind(identifier, windowStart, CONFIG.API_WRITE_REQUESTS_PER_MINUTE)
       .first<{ count: number }>();
 
-    const count = row?.count || 0;
-    if (count >= CONFIG.API_WRITE_REQUESTS_PER_MINUTE) {
+    // No returned row means conflict happened and WHERE prevented the increment:
+    // current count is already at/above the configured limit.
+    if (!row) {
       return {
         allowed: false,
         remaining: 0,
@@ -132,24 +140,8 @@ export class RateLimitService {
       };
     }
 
-    return {
-      allowed: true,
-      remaining: CONFIG.API_WRITE_REQUESTS_PER_MINUTE - count,
-    };
-  }
-
-  async incrementApiCount(identifier: string): Promise<void> {
-    const nowSec = Math.floor(Date.now() / 1000);
-    const windowStart = nowSec - (nowSec % CONFIG.API_WINDOW_SECONDS);
-
-    // Atomic increment via UPSERT.
-    await this.db
-      .prepare(
-        'INSERT INTO api_rate_limits(identifier, window_start, count) VALUES(?, ?, 1) ' +
-        'ON CONFLICT(identifier, window_start) DO UPDATE SET count = count + 1'
-      )
-      .bind(identifier, windowStart)
-      .run();
+    const remaining = Math.max(0, CONFIG.API_WRITE_REQUESTS_PER_MINUTE - row.count);
+    return { allowed: true, remaining };
   }
 }
 

src/services/storage.ts

@@ -7,6 +7,8 @@ import { User, Cipher, Folder, Attachment } from '../types';
 // - Revision date is maintained per user for Bitwarden sync.
 
 export class StorageService {
+  private static attachmentTokenTableReady = false;
+
   constructor(private db: D1Database) {}
 
   /**
@@ -395,6 +397,23 @@ CREATE INDEX IF NOT EXISTS idx_api_rate_window ON api_rate_limits(window_start);
     await this.db.prepare('DELETE FROM folders WHERE id = ? AND user_id = ?').bind(id, userId).run();
   }
 
+  // Clear folder references from all ciphers owned by the user.
+  // Without this, deleting a folder leaves stale folderId values in cipher JSON.
+  async clearFolderFromCiphers(userId: string, folderId: string): Promise<void> {
+    const now = new Date().toISOString();
+    const res = await this.db
+      .prepare('SELECT data FROM ciphers WHERE user_id = ? AND folder_id = ?')
+      .bind(userId, folderId)
+      .all<{ data: string }>();
+
+    for (const row of (res.results || [])) {
+      const cipher = JSON.parse(row.data) as Cipher;
+      cipher.folderId = null;
+      cipher.updatedAt = now;
+      await this.saveCipher(cipher);
+    }
+  }
+
   async getAllFolders(userId: string): Promise<Folder[]> {
     const res = await this.db
       .prepare('SELECT id, user_id, name, created_at, updated_at FROM folders WHERE user_id = ? ORDER BY updated_at DESC')
@@ -579,4 +598,37 @@ CREATE INDEX IF NOT EXISTS idx_api_rate_window ON api_rate_limits(window_start);
       .run();
     return date;
   }
+
+  // --- One-time attachment download tokens ---
+
+  private async ensureUsedAttachmentDownloadTokenTable(): Promise<void> {
+    if (StorageService.attachmentTokenTableReady) return;
+
+    await this.db.prepare(
+      'CREATE TABLE IF NOT EXISTS used_attachment_download_tokens (' +
+      'jti TEXT PRIMARY KEY, ' +
+      'expires_at INTEGER NOT NULL' +
+      ')'
+    ).run();
+
+    StorageService.attachmentTokenTableReady = true;
+  }
+
+  // Marks an attachment download token JTI as consumed.
+  // Returns true only on first use. Reuse returns false.
+  async consumeAttachmentDownloadToken(jti: string, expUnixSeconds: number): Promise<boolean> {
+    await this.ensureUsedAttachmentDownloadTokenTable();
+
+    const nowMs = Date.now();
+    // Best-effort cleanup of expired entries.
+    await this.db.prepare('DELETE FROM used_attachment_download_tokens WHERE expires_at < ?').bind(nowMs).run();
+
+    const expiresAtMs = expUnixSeconds * 1000;
+    const result = await this.db.prepare(
+      'INSERT INTO used_attachment_download_tokens(jti, expires_at) VALUES(?, ?) ' +
+      'ON CONFLICT(jti) DO NOTHING'
+    ).bind(jti, expiresAtMs).run();
+
+    return (result.meta.changes ?? 0) > 0;
+  }
 }