🔥 Roast
你不是程序员。你是 AI 的复制粘贴键,还带 commit 权限。
说句实话 —— 这代码不是你写的,你往聊天框敲了句“给我搞个安全登录”,AI 吐什么你就上什么。典型 slop:随手一个 timing-safe 比对就让你飘了,压根没发现 auth 大门洞开。等着某个小孩拿脚本刷爆你接口、截图挂论坛,配文“又一个没听过限流的创业公司”,底下 900 个赞,你的名字挂在帖子里。
83 分 —— 行,可以跳一个。
哦,真棒。 timing-safe 的 token 比对、CI 里的 CodeQL、23 对 try/catch、一整套离线测试 —— 看得出你很会上锁。所以你把这么漂亮一把锁,装在一栋卸了两扇侧门的房子上,就更显本事了。
一个 Cloudflare Worker(worker/worker.js,约两千行)前面顶着 R2 存文档、KV 管会话 —— 没有框架,路由全是手搓的一个大 switch。鉴权用 GitHub Device Flow(/api/auth/device/*),作者 HTML 直接从 R2 吐出来。作为一个单人 Worker 其实相当整洁:timing-safe 的 token 比对、一整套测试、CI 里的 CodeQL。所以才更扎心 —— 前门(device-flow 端点)对「敲门频率」没上锁,响应还一个安全头都不带。
底子是好的 —— 缺的两块一个下午就能补上。玩笑到此为止,下面是逐条改法。
六项分数好看,两项把总分往下拽 —— 你就是那个忘穿裤子来毕业典礼的优等生。
| 优先级 | 范围 | 问题 | 确认 |
|---|---|---|---|
| High | Rate limiting | Device-flow auth endpoints have no per-IP / global cap | 已确认 |
| High | Deploy config | doc-serve responses ship no CSP / frame / nosniff / HSTS headers | 已确认 |
| Medium | Secrets | .env / .env* not in .gitignore | 已确认 |
| Medium | Auth | Author HTML served verbatim (self-XSS on own tenant) | 已确认 |
| Medium | Deploy config | Wildcard CORS (no Allow-Credentials — currently safe) | 已确认 |
| Medium | Auth | 4 mutating routes guarded; no default-deny test for route #5 | 已确认 |
| Medium | Observability | Errors only via console.error + wrangler tail; no tracker | 已确认 |
| Low | Observability | No request-id on log lines — hard to trace one user’s path | 已确认 |
| Low | Deploy config | wrangler.toml pins no compatibility_date lower bound | 待核实 |
两件事挡在你和上线之间。下面我不开玩笑了,直接告诉你怎么补。
| HighCongrats — you built a free brute-force playground That’s not an auth endpoint — it’s an open bar: free, unlimited, 24/7, serving every stranger who fancies grinding your GitHub Device Flow quota to dust with one while(true). | |
| Evidence | worker/worker.js:1631 — POST /api/auth/device/start worker/worker.js:1650 — POST /api/auth/device/poll proxy GitHub Device Flow with no rate limiter wired |
|---|---|
| Root cause | Unthrottled auth endpoints let an attacker brute-force device codes or burn your Device Flow quota; any public auth surface needs a per-IP + global cap. |
| The fix | const ip = request.headers.get("CF-Connecting-IP");
const n = Number(await env.RL.get(`rl:device:${ip}`)) || 0;
if (n > 10) return new Response("rate limited", { status: 429 });
await env.RL.put(`rl:device:${ip}`, String(n + 1), { expirationTtl: 60 }); |
| Verify | for i in $(seq 1 15); do curl -s -o /dev/null -w "%{http_code}\n" \
https://<worker>/api/auth/device/start; done # expect 429 after the cap |
| HighYou serve other people’s HTML on your own origin and call it a day Your doc-serve responses set Content-Type and stop there — no CSP, no X-Frame-Options, no nosniff. You built a beautiful printing press and skipped the part where the ink doesn’t set the building on fire. | |
| Evidence | worker/worker.js:1616 — doc-serve Response sets Content-Type only; no CSP / X-Frame-Options / X-Content-Type-Options / Referrer-Policy / HSTS |
| Root cause | Author HTML is served from the worker’s own origin with no framing or MIME-sniff protection — a real clickjacking / sniff risk for an HTML publisher. |
| The fix | const headers = {
’Content-Type’: ’text/html; charset=utf-8’,
+ ’X-Content-Type-Options’: ’nosniff’,
+ ’Content-Security-Policy’: "frame-ancestors ’self’",
+ ’Strict-Transport-Security’: ’max-age=31536000; includeSubDomains’,
}; |
| Verify | curl -sI https://<worker>/d/<slug>/v/1 | grep -iE \ ’x-content-type|content-security|strict-transport’ |
| MediumYour .gitignore is one line short of a public apology tour Right now a stray git add . is all that stands between your secrets and a very public commit history. | |
| Evidence | .gitignore:0 — .env / .env* not listed; a secret-bearing dotenv could be committed |
| Root cause | An untracked .env is one `git add .` from leaking every key in it — once it hits history, rotating is the only fix. |
| The fix | # append to .gitignore .env .env.* !.env.example |
| Verify | git check-ignore .env # should print .env |
| MediumAuthor HTML served verbatim — your diary on your own kitchen table It’s “fine” the way leaving your diary open on the kitchen table is fine — because it’s your kitchen, for now. | |
| Evidence | worker/worker.js:1610 — author HTML returned verbatim from R2 (reader comments/logins ARE escaped) |
| Root cause | Self-XSS on the author’s own tenant, not cross-user — acceptable on a single-owner worker where the author is the only writer. |
| The fix | if multi-author publishing is added, isolate each doc to a sandboxed per-author origin. |
| Verify | confirm no shared-origin multi-tenant publishing before scaling. |
| MediumWildcard CORS — door wide open, nothing worth stealing behind it yet Access-Control-Allow-Origin: * is fine while nothing sensitive is on the other side — the moment something is, it’s a liability. | |
| Evidence | worker/worker.js:19 — Access-Control-Allow-Origin: * WITHOUT Allow-Credentials |
| Root cause | Public non-credentialed API shape; fine while nothing sensitive is served, dangerous the instant a response carries user data. |
| The fix | if any response becomes sensitive, replace * with an explicit allowlist — never pair * with Allow-Credentials. |
| Verify | curl -sI <worker>/api/... | grep -i access-control |
| Medium4 mutating routes guarded — great until route #5 ships without it Every write route is guarded today; the trouble is the sixth one nobody remembers to guard. | |
| Evidence | worker/worker.js:1774,1859,1907,1981 — requireUploadAuth guards the 4 mutating routes found |
| Root cause | A 5th mutating route could ship without the guard and nobody notices until it’s abused. |
| The fix | add a default-deny test so any unguarded mutating route fails CI. |
| Verify | add a test asserting every POST/DELETE route calls requireUploadAuth. |
| MediumYour errors are confessing to an empty room console.error plus a terminal tail means you find out things broke from users, not from alerts. | |
| Evidence | worker/worker.js:88 — errors only via console.error + wrangler tail; no error-tracking provider |
| Root cause | You find out about breakage from users, not alerts — uncaught errors vanish the moment you stop tailing. |
| The fix | wire a Workers error tracker (Sentry / Logpush) so uncaught errors surface without tailing. |
| Verify | trigger a handled error and confirm it lands in the tracker. |
不挡上线,但顺手修了更稳 —— 每条都带确切改法。
| LowYour logs are anonymous — every error is a stranger with no name tag When two users hit the same bug you’ll be squinting at a wall of identical console lines trying to tell whose session was whose. Add a request id now; your future 2am self will send a thank-you card. | |
| Evidence | worker/worker.js:88 — log lines carry no request/trace id |
|---|---|
| Root cause | Without a per-request id you can’t correlate a user’s error to the rest of their session — every debugging session starts from zero. |
| The fix | const rid = crypto.randomUUID().slice(0, 8);
console.error(`[${rid}]`, err); // and return it in an `x-request-id` response header |
| Verify | trigger an error and confirm the same 8-char id appears in the log line and the response header. |
| Lowwrangler.toml forgot to say how old it’s allowed to be No compatibility_date floor means a future Workers runtime could quietly change behaviour under you — the config equivalent of “it works on my machine, ship it.” | |
| Evidence | wrangler.toml:0 — no compatibility_date pinned |
| Root cause | Without a pinned compatibility_date the Worker’s runtime semantics can drift as Cloudflare ships changes. |
| The fix | # wrangler.toml compatibility_date = "2026-07-01" |
| Verify | npx wrangler deploy --dry-run # should report the pinned compatibility date |