mirror of
https://github.com/juherr/kill-the-news.git
synced 2026-06-20 22:03:48 +00:00
feat: complete Phase 2 tech debt remediation
- Extract shared RSS/Atom fetch logic into feed-fetcher utility (P1-3)
- Split email-processor into validateEmail/storeEmail functions (P1-6)
- Add stateless HMAC-SHA256 CSRF protection to admin forms (P2-8)
- Fix Hono<{ Bindings: Env }> type safety across all routes (P3-13)
- Add entries.test.ts and files.test.ts with full coverage (P1-7)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
+105
@@ -0,0 +1,105 @@
|
||||
# Tech Debt — Remediation Plan
|
||||
|
||||
Generated: 2026-05-22
|
||||
|
||||
## Scoring: Priority = (Impact + Risk) × (6 − Effort)
|
||||
|
||||
---
|
||||
|
||||
## Phase 1 — This sprint (quick wins, high ROI)
|
||||
|
||||
| # | Task | Priority | Status |
|
||||
| ----- | ------------------------------------------------------------ | -------- | ------ |
|
||||
| P0-2 | Add `inbound.test.ts` with full coverage | 40 | DONE |
|
||||
| P0-1 | Document race condition + TODO for Durable Objects migration | 36 | DONE |
|
||||
| P2-9 | Fix orphaned KV key deletion in `storage.ts` feed trim | 18 | DONE |
|
||||
| P2-12 | Add `/health` endpoint | 15 | DONE |
|
||||
|
||||
## Phase 2 — Next milestone (3–5 days)
|
||||
|
||||
| # | Task | Priority | Status |
|
||||
| ----- | ----------------------------------------------------------- | -------- | ------ |
|
||||
| P1-7 | Add `entries.test.ts`, `files.test.ts`, hub lifecycle tests | 32 | DONE |
|
||||
| P1-3 | Extract shared RSS/Atom feed-fetching logic | 21 | DONE |
|
||||
| P1-6 | Split `email-processor.ts` responsibilities | 21 | DONE |
|
||||
| P2-8 | Add CSRF token to admin forms | 21 | DONE |
|
||||
| P3-13 | Fix `Hono<{ Bindings: Env }>` type safety | 8 | DONE |
|
||||
|
||||
## Phase 3 — Ongoing / Infrastructure
|
||||
|
||||
| # | Task | Priority |
|
||||
| ----- | ---------------------------------------------------------- | -------- |
|
||||
| P1-4 | Structured logging + error aggregation | 36 |
|
||||
| P1-5 | Rate limiting (Cloudflare WAF rules) | 24 |
|
||||
| P1-1 | Migrate feed metadata to Durable Objects for atomic writes | 36 |
|
||||
| P2-10 | Extract constants module (`src/config/constants.ts`) | 12 |
|
||||
| P2-11 | Split `admin.ts` into sub-modules | 8 |
|
||||
|
||||
---
|
||||
|
||||
## Top 3 Business Risks
|
||||
|
||||
1. **Data loss** — concurrent email ingest silently drops emails (KV race condition)
|
||||
2. **Undetected regressions** — inbound route has zero tests; any change is a gamble
|
||||
3. **Silent failures** — no error tracking means production issues are invisible
|
||||
|
||||
---
|
||||
|
||||
## Detailed Findings
|
||||
|
||||
### Critical
|
||||
|
||||
**Race condition in KV metadata updates** (`src/lib/email-processor.ts`)
|
||||
Two concurrent emails to the same feed read-modify-write the metadata non-atomically.
|
||||
The second writer silently overwrites the first's changes. Accepted limitation of KV;
|
||||
long-term fix is Cloudflare Durable Objects for serialised writes.
|
||||
|
||||
**Email ingest path has zero tests** (`src/routes/inbound.ts`)
|
||||
The most critical path in the system is entirely unguarded. Required: happy path,
|
||||
invalid IPs, missing fields, KV write failure, sender validation, attachment upload.
|
||||
|
||||
### High
|
||||
|
||||
**Duplicated RSS/Atom route logic** (`src/routes/rss.ts`, `src/routes/atom.ts`)
|
||||
Param extraction, feedId validation, KV fetches, pagination, cache headers and Link
|
||||
header building are duplicated verbatim. Fix: extract `fetchFeedAndEmails()` utility.
|
||||
|
||||
**No error tracking or production alerting**
|
||||
All errors are console-only, invisible in production. Fix: structured JSON logging
|
||||
|
||||
- Cloudflare Logpush or Sentry integration.
|
||||
|
||||
**No rate limiting on `/api/inbound` or `/admin` login**
|
||||
IP validation is insufficient; admin login has no brute-force protection.
|
||||
Fix: Cloudflare WAF rate-limiting rules.
|
||||
|
||||
**`email-processor.ts` violates single responsibility**
|
||||
Feed ID extraction, sender validation, attachment upload, KV write, feed trimming,
|
||||
and WebSub notification in one function. Fix: split into `validateEmail()`, `storeEmail()`.
|
||||
|
||||
### Medium
|
||||
|
||||
**No CSRF protection on admin forms** (`src/routes/admin.ts`)
|
||||
State-changing POSTs have no CSRF token. Fix: generate token on page load, validate on POST.
|
||||
|
||||
**Orphaned KV keys in `storage.ts` feed trim**
|
||||
`updateFeedMetadata()` truncates the metadata list but never deletes the actual KV entries.
|
||||
(Note: `email-processor.ts` already handles this correctly in the live code path.)
|
||||
|
||||
**Magic numbers without constants**
|
||||
Limits scattered as inline literals: 20 items, 50 metadata, 512 KB, 24h TTL, 30-day lease.
|
||||
Fix: extract to `src/config/constants.ts`.
|
||||
|
||||
**No `/health` endpoint**
|
||||
Monitoring tools cannot detect a bad deploy. Fix: `GET /health` returning `{ status: "ok" }`.
|
||||
|
||||
### Low
|
||||
|
||||
**`as unknown as Env` type cast** (`src/index.ts` line 7)
|
||||
Bypasses TypeScript; Env mismatches become runtime errors.
|
||||
|
||||
**Inconsistent utility organisation** (`lib/` vs `utils/`)
|
||||
No clear rule for placement of new utilities.
|
||||
|
||||
**No audit log for admin actions**
|
||||
Create/delete feed, bulk-delete emails — no record of who did what.
|
||||
Reference in New Issue
Block a user