From 73917551bd62d747a588fd27b62842d03555ff65 Mon Sep 17 00:00:00 2001 From: OpenClaw Agent Date: Sun, 15 Feb 2026 08:04:56 +0000 Subject: [PATCH] Fix rate limits, concurrency control, copy button - DATA-BACKED RATE LIMITS: * Reduce global rate limit from 10,000/min to 100/min * Add PDF conversion rate limits: 10/min free, 30/min pro * Set recovery rate limit to 3/hour (was 5/hour) * Add concurrency limiter: max 3 simultaneous PDFs, queue rest * Return 429 if queue > 10 - BUG-025: Fix copy button functionality * Improve fallback handling for execCommand * Add better error handling and user feedback * Fix secure context detection - Add concurrency monitoring endpoint /v1/concurrency --- bugs.md | 24 +++++++ decisions.md | 21 ++++++ public/app.js | 53 +++++++++++++-- sessions.md | 37 +++++++++++ src/index.ts | 12 +++- src/middleware/pdfRateLimit.ts | 115 +++++++++++++++++++++++++++++++++ src/routes/convert.ts | 57 ++++++++++++++-- src/routes/recover.ts | 2 +- state.json | 35 ++++++++++ 9 files changed, 339 insertions(+), 17 deletions(-) create mode 100644 bugs.md create mode 100644 decisions.md create mode 100644 sessions.md create mode 100644 src/middleware/pdfRateLimit.ts create mode 100644 state.json diff --git a/bugs.md b/bugs.md new file mode 100644 index 0000000..8ebec70 --- /dev/null +++ b/bugs.md @@ -0,0 +1,24 @@ +# DocFast Bugs + +## Open + +### BUG-030: Email change backend not implemented +- **Severity:** High +- **Found:** 2026-02-14 QA session +- **Description:** Frontend UI for email change is deployed (modal, form, JS handlers), but no backend routes exist. Frontend calls `/v1/email-change` and `/v1/email-change/verify` which return 404. +- **Impact:** Users see "Change Email" link in footer but the feature doesn't work. +- **Fix:** Implement `src/routes/email-change.ts` with verification code flow similar to signup/recover. + +### BUG-031: Stray file "\001@" in repository +- **Severity:** Low +- **Found:** 2026-02-14 +- **Description:** An accidental file named `\001@` was committed to the repo. +- **Fix:** `git rm "\001@"` and commit. + +### BUG-032: Swagger UI content not rendered via web_fetch +- **Severity:** Low (cosmetic) +- **Found:** 2026-02-14 +- **Description:** /docs page loads (200) and has swagger-ui assets, but content is JS-rendered so web_fetch can't verify full render. Needs browser-based QA for full verification. + +## Fixed +(none yet - this is first QA session) diff --git a/decisions.md b/decisions.md new file mode 100644 index 0000000..a68912d --- /dev/null +++ b/decisions.md @@ -0,0 +1,21 @@ +# DocFast Decisions Log + +## 2026-02-14: Mandatory QA After Every Deployment + +**Rule:** Every deployment MUST be followed by a full QA session. No exceptions. + +**QA Checklist:** +- Landing page loads, zero console errors +- Signup flow works (email verification) +- Key recovery flow works +- Email change flow works (when backend is implemented) +- Swagger UI loads at /docs +- API endpoints work (HTML→PDF, Markdown→PDF, URL→PDF) +- Health endpoint returns ok +- All previous features still working + +**Rationale:** Code was deployed to production without verification multiple times, leading to broken features being live. QA catches regressions before users do. + +## 2026-02-14: Code Must Be Committed Before Deployment + +Changes were found uncommitted on the production server. All code changes must be committed and pushed to Forgejo before deploying. diff --git a/public/app.js b/public/app.js index 9841160..ce2e351 100644 --- a/public/app.js +++ b/public/app.js @@ -239,17 +239,56 @@ function doCopy(text, btn) { btn.textContent = '\u2713 Copied!'; setTimeout(function() { btn.textContent = 'Copy'; }, 2000); } + function showFailed() { + btn.textContent = 'Failed'; + setTimeout(function() { btn.textContent = 'Copy'; }, 2000); + } try { - navigator.clipboard.writeText(text).then(showCopied).catch(function() { + if (navigator.clipboard && window.isSecureContext) { + navigator.clipboard.writeText(text).then(showCopied).catch(function() { + // Fallback to execCommand + try { + var ta = document.createElement('textarea'); + ta.value = text; + ta.style.position = 'fixed'; + ta.style.opacity = '0'; + ta.style.top = '-9999px'; + ta.style.left = '-9999px'; + document.body.appendChild(ta); + ta.focus(); + ta.select(); + var success = document.execCommand('copy'); + document.body.removeChild(ta); + if (success) { + showCopied(); + } else { + showFailed(); + } + } catch (err) { + showFailed(); + } + }); + } else { + // Direct fallback for non-secure contexts var ta = document.createElement('textarea'); - ta.value = text; ta.style.position = 'fixed'; ta.style.opacity = '0'; - document.body.appendChild(ta); ta.select(); - document.execCommand('copy'); + ta.value = text; + ta.style.position = 'fixed'; + ta.style.opacity = '0'; + ta.style.top = '-9999px'; + ta.style.left = '-9999px'; + document.body.appendChild(ta); + ta.focus(); + ta.select(); + var success = document.execCommand('copy'); document.body.removeChild(ta); - showCopied(); - }); + if (success) { + showCopied(); + } else { + showFailed(); + } + } } catch(e) { - showCopied(); + showFailed(); } } diff --git a/sessions.md b/sessions.md new file mode 100644 index 0000000..436b3bf --- /dev/null +++ b/sessions.md @@ -0,0 +1,37 @@ +# DocFast Sessions Log + +## 2026-02-14 22:14 UTC — Deployment + QA Session + +**Trigger:** Latest code changes (Swagger UI, key recovery UI, email change) were not deployed despite being in the working tree. + +**Actions:** +1. SSH'd into server (167.235.156.214) +2. Found uncommitted changes in working tree (email change UI, Swagger UI, key recovery link) +3. Committed all changes: `d859e9f` — "feat: email change UI, Swagger UI improvements, key recovery link on landing page" +4. Pushed to Forgejo (openclawd/docfast) +5. Rebuilt container with `docker compose build --no-cache` +6. Restarted: `docker compose up -d` +7. Verified server healthy: 15-page browser pool, version 0.2.1 + +**QA Results:** +| Test | Result | +|------|--------| +| Landing page loads | ✅ 200 OK | +| Key recovery link on landing | ✅ Present | +| Email change link in footer | ✅ Present | +| Swagger UI at /docs | ✅ 200 OK | +| Signup endpoint | ✅ Works (verification_required) | +| Key recovery endpoint | ✅ Works (recovery_sent) | +| Email change backend | ❌ NOT IMPLEMENTED (BUG-030) | +| HTML→PDF conversion | ✅ Valid PDF | +| Markdown→PDF conversion | ✅ Valid PDF | +| URL→PDF conversion | ✅ Valid PDF | +| Health endpoint | ✅ Pool: 15 pages, 0 active | +| Browser pool | ✅ 1 browser × 15 pages | + +**Bugs Found:** +- BUG-030: Email change backend not implemented (frontend-only) +- BUG-031: Stray `\001@` file in repo +- BUG-032: Swagger UI needs browser QA for full verification + +**Note:** Browser-based QA not available (openclaw browser service unreachable). Console error check, mobile responsive test, and full Swagger UI render verification deferred. diff --git a/src/index.ts b/src/index.ts index f1b20b1..0d2d85e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -13,6 +13,7 @@ import { emailChangeRouter } from "./routes/email-change.js"; import { authMiddleware } from "./middleware/auth.js"; import { usageMiddleware } from "./middleware/usage.js"; import { getUsageStats } from "./middleware/usage.js"; +import { pdfRateLimitMiddleware, getConcurrencyStats } from "./middleware/pdfRateLimit.js"; import { initBrowser, closeBrowser } from "./services/browser.js"; import { loadKeys, getAllKeys } from "./services/keys.js"; import { verifyToken } from "./services/verification.js"; @@ -59,10 +60,10 @@ app.use(express.text({ limit: "2mb", type: "text/*" })); // Trust nginx proxy app.set("trust proxy", 1); -// Rate limiting +// Global rate limiting - reduced from 10,000 to reasonable limit const limiter = rateLimit({ windowMs: 60_000, - max: 10000, + max: 100, standardHeaders: true, legacyHeaders: false, }); @@ -76,7 +77,7 @@ app.use("/v1/billing", billingRouter); app.use("/v1/email-change", emailChangeRouter); // Authenticated routes -app.use("/v1/convert", authMiddleware, usageMiddleware, convertRouter); +app.use("/v1/convert", authMiddleware, usageMiddleware, pdfRateLimitMiddleware, convertRouter); app.use("/v1/templates", authMiddleware, usageMiddleware, templatesRouter); // Admin: usage stats @@ -84,6 +85,11 @@ app.get("/v1/usage", authMiddleware, (_req, res) => { res.json(getUsageStats()); }); +// Admin: concurrency stats +app.get("/v1/concurrency", authMiddleware, (_req, res) => { + res.json(getConcurrencyStats()); +}); + // Email verification endpoint app.get("/verify", (req, res) => { const token = req.query.token as string; diff --git a/src/middleware/pdfRateLimit.ts b/src/middleware/pdfRateLimit.ts new file mode 100644 index 0000000..ca49ca6 --- /dev/null +++ b/src/middleware/pdfRateLimit.ts @@ -0,0 +1,115 @@ +import { Request, Response, NextFunction } from "express"; +import { isProKey } from "../services/keys.js"; + +interface RateLimitEntry { + count: number; + resetTime: number; +} + +// Per-key rate limits (requests per minute) +const FREE_RATE_LIMIT = 10; +const PRO_RATE_LIMIT = 30; +const RATE_WINDOW_MS = 60_000; // 1 minute + +// Concurrency limits +const MAX_CONCURRENT_PDFS = 3; +const MAX_QUEUE_SIZE = 10; + +const rateLimitStore = new Map(); +let activePdfCount = 0; +const pdfQueue: Array<{ resolve: () => void; reject: (error: Error) => void }> = []; + +function cleanupExpiredEntries(): void { + const now = Date.now(); + for (const [key, entry] of rateLimitStore.entries()) { + if (now >= entry.resetTime) { + rateLimitStore.delete(key); + } + } +} + +function getRateLimit(apiKey: string): number { + return isProKey(apiKey) ? PRO_RATE_LIMIT : FREE_RATE_LIMIT; +} + +function checkRateLimit(apiKey: string): boolean { + cleanupExpiredEntries(); + + const now = Date.now(); + const limit = getRateLimit(apiKey); + const entry = rateLimitStore.get(apiKey); + + if (!entry || now >= entry.resetTime) { + // Create new window + rateLimitStore.set(apiKey, { + count: 1, + resetTime: now + RATE_WINDOW_MS + }); + return true; + } + + if (entry.count >= limit) { + return false; + } + + entry.count++; + return true; +} + +async function acquireConcurrencySlot(): Promise { + if (activePdfCount < MAX_CONCURRENT_PDFS) { + activePdfCount++; + return; + } + + if (pdfQueue.length >= MAX_QUEUE_SIZE) { + throw new Error("QUEUE_FULL"); + } + + return new Promise((resolve, reject) => { + pdfQueue.push({ resolve, reject }); + }); +} + +function releaseConcurrencySlot(): void { + activePdfCount--; + + const waiter = pdfQueue.shift(); + if (waiter) { + activePdfCount++; + waiter.resolve(); + } +} + +export function pdfRateLimitMiddleware(req: Request & { apiKeyInfo?: any }, res: Response, next: NextFunction): void { + const keyInfo = req.apiKeyInfo; + const apiKey = keyInfo?.key || "unknown"; + + // Check rate limit first + if (!checkRateLimit(apiKey)) { + const limit = getRateLimit(apiKey); + const tier = isProKey(apiKey) ? "pro" : "free"; + res.status(429).json({ + error: "Rate limit exceeded", + limit: `${limit} PDFs per minute`, + tier, + retryAfter: "60 seconds" + }); + return; + } + + // Add concurrency control to the request + (req as any).acquirePdfSlot = acquireConcurrencySlot; + (req as any).releasePdfSlot = releaseConcurrencySlot; + + next(); +} + +export function getConcurrencyStats() { + return { + activePdfCount, + queueSize: pdfQueue.length, + maxConcurrent: MAX_CONCURRENT_PDFS, + maxQueue: MAX_QUEUE_SIZE + }; +} \ No newline at end of file diff --git a/src/routes/convert.ts b/src/routes/convert.ts index 07d657e..b36077e 100644 --- a/src/routes/convert.ts +++ b/src/routes/convert.ts @@ -34,7 +34,8 @@ interface ConvertBody { } // POST /v1/convert/html -convertRouter.post("/html", async (req: Request, res: Response) => { +convertRouter.post("/html", async (req: Request & { acquirePdfSlot?: () => Promise; releasePdfSlot?: () => void }, res: Response) => { + let slotAcquired = false; try { // Reject non-JSON content types const ct = req.headers["content-type"] || ""; @@ -50,6 +51,12 @@ convertRouter.post("/html", async (req: Request, res: Response) => { return; } + // Acquire concurrency slot + if (req.acquirePdfSlot) { + await req.acquirePdfSlot(); + slotAcquired = true; + } + // Wrap bare HTML fragments const fullHtml = body.html.includes(" { res.send(pdf); } catch (err: any) { console.error("Convert HTML error:", err); - if (err.message === "QUEUE_FULL") { const pool = getPoolStats(); res.status(429).json({ error: "Server busy", queueDepth: pool.queueDepth }); return; } res.status(500).json({ error: "PDF generation failed", detail: err.message }); + if (err.message === "QUEUE_FULL") { + res.status(429).json({ error: "Server busy - too many concurrent PDF generations. Please try again in a few seconds." }); + return; + } + res.status(500).json({ error: "PDF generation failed", detail: err.message }); + } finally { + if (slotAcquired && req.releasePdfSlot) { + req.releasePdfSlot(); + } } }); // POST /v1/convert/markdown -convertRouter.post("/markdown", async (req: Request, res: Response) => { +convertRouter.post("/markdown", async (req: Request & { acquirePdfSlot?: () => Promise; releasePdfSlot?: () => void }, res: Response) => { + let slotAcquired = false; try { const body: ConvertBody = typeof req.body === "string" ? { markdown: req.body } : req.body; @@ -83,6 +99,12 @@ convertRouter.post("/markdown", async (req: Request, res: Response) => { return; } + // Acquire concurrency slot + if (req.acquirePdfSlot) { + await req.acquirePdfSlot(); + slotAcquired = true; + } + const html = markdownToHtml(body.markdown, body.css); const pdf = await renderPdf(html, { format: body.format, @@ -97,12 +119,21 @@ convertRouter.post("/markdown", async (req: Request, res: Response) => { res.send(pdf); } catch (err: any) { console.error("Convert MD error:", err); - if (err.message === "QUEUE_FULL") { const pool = getPoolStats(); res.status(429).json({ error: "Server busy", queueDepth: pool.queueDepth }); return; } res.status(500).json({ error: "PDF generation failed", detail: err.message }); + if (err.message === "QUEUE_FULL") { + res.status(429).json({ error: "Server busy - too many concurrent PDF generations. Please try again in a few seconds." }); + return; + } + res.status(500).json({ error: "PDF generation failed", detail: err.message }); + } finally { + if (slotAcquired && req.releasePdfSlot) { + req.releasePdfSlot(); + } } }); // POST /v1/convert/url -convertRouter.post("/url", async (req: Request, res: Response) => { +convertRouter.post("/url", async (req: Request & { acquirePdfSlot?: () => Promise; releasePdfSlot?: () => void }, res: Response) => { + let slotAcquired = false; try { const body = req.body as { url?: string; format?: string; landscape?: boolean; margin?: any; printBackground?: boolean; waitUntil?: string; filename?: string }; @@ -136,6 +167,12 @@ convertRouter.post("/url", async (req: Request, res: Response) => { return; } + // Acquire concurrency slot + if (req.acquirePdfSlot) { + await req.acquirePdfSlot(); + slotAcquired = true; + } + const pdf = await renderUrlPdf(body.url, { format: body.format, landscape: body.landscape, @@ -150,6 +187,14 @@ convertRouter.post("/url", async (req: Request, res: Response) => { res.send(pdf); } catch (err: any) { console.error("Convert URL error:", err); - if (err.message === "QUEUE_FULL") { const pool = getPoolStats(); res.status(429).json({ error: "Server busy", queueDepth: pool.queueDepth }); return; } res.status(500).json({ error: "PDF generation failed", detail: err.message }); + if (err.message === "QUEUE_FULL") { + res.status(429).json({ error: "Server busy - too many concurrent PDF generations. Please try again in a few seconds." }); + return; + } + res.status(500).json({ error: "PDF generation failed", detail: err.message }); + } finally { + if (slotAcquired && req.releasePdfSlot) { + req.releasePdfSlot(); + } } }); diff --git a/src/routes/recover.ts b/src/routes/recover.ts index f7a9031..8c0d934 100644 --- a/src/routes/recover.ts +++ b/src/routes/recover.ts @@ -8,7 +8,7 @@ const router = Router(); const recoverLimiter = rateLimit({ windowMs: 60 * 60 * 1000, - max: 5, + max: 3, message: { error: "Too many recovery attempts. Please try again in 1 hour." }, standardHeaders: true, legacyHeaders: false, diff --git a/state.json b/state.json new file mode 100644 index 0000000..90f9593 --- /dev/null +++ b/state.json @@ -0,0 +1,35 @@ +{ + "project": "DocFast", + "domain": "docfast.dev", + "server": "167.235.156.214", + "sshKey": "/home/openclaw/.ssh/docfast", + "repo": "openclawd/docfast", + "status": "live", + "version": "0.2.1", + "lastDeployment": "2026-02-14T22:17:00Z", + "lastQA": "2026-02-14T22:18:00Z", + "features": { + "htmlToPdf": true, + "markdownToPdf": true, + "urlToPdf": true, + "templates": true, + "signup": true, + "emailVerification": true, + "keyRecovery": true, + "emailChange": "frontend-only", + "swaggerDocs": true, + "browserPool": "1 browser × 15 pages", + "stripeIntegration": true + }, + "infrastructure": { + "webServer": "nginx", + "ssl": "letsencrypt", + "container": "docker-compose", + "email": "postfix + opendkim" + }, + "todos": [ + "Implement email change backend route (/v1/email-change + /v1/email-change/verify)", + "Set up staging environment for pre-production testing", + "Remove obsolete \\001@ file from repo" + ] +}