rss email digests over ssh because you're a cool kid
herald.dunkirk.sh
go
rss
rss-reader
ssh
charm
feat: add rate limits and various other safety measures
+514
-40
+356
fix.md
+356
fix.md
···
1
+
# Code Review: Herald RSS-to-Email Project
2
+
3
+
---
4
+
5
+
# Herald - Issue Groups & Priorities
6
+
7
+
## **P0 - Critical (Security & Data Integrity)** ✅ COMPLETE
8
+
9
+
- ✅ #14: No Rate Limiting (SSH, SCP, web, email)
10
+
- ✅ #15: Token Generation (verify crypto/rand usage)
11
+
12
+
## **P1 - High (Performance & Reliability)**
13
+
14
+
- #8: N+1 Query Problem (batch operations)
15
+
- #26: No Cleanup of Old seen_items (6 month cleanup job)
16
+
- #23: Missing Graceful Shutdown for Scheduler (panic recovery)
17
+
18
+
## **P2 - Medium (Code Quality & UX)**
19
+
20
+
### Group A: Input Validation
21
+
22
+
- #16: No SMTP Auth Validation
23
+
- #27: No Feed Validation on Upload
24
+
- #37: No Cron Validation at Upload
25
+
- ✅ #36: No Max File Size on SCP Upload
26
+
27
+
### Group B: Performance Tuning
28
+
29
+
- #9: No Prepared Statements
30
+
- #10: Inefficient Sorting in Handlers
31
+
- #11: No HTTP Caching Headers
32
+
33
+
## **P3 - Low (Nice to Have)**
34
+
35
+
### Group C: Observability
36
+
37
+
- #24: No Metrics/Observability
38
+
- #35: HTTP Server Doesn't Log Requests
39
+
- #22: Inconsistent Logging Levels
40
+
41
+
### Group D: Architecture & Scalability
42
+
43
+
- #30: Scheduler Interval is Fixed
44
+
- #31: No Pagination on Feed Endpoints
45
+
46
+
### Group E: Code Hygiene
47
+
48
+
- #3: Context Timeout Duplication
49
+
- #19: Magic Numbers
50
+
- #18: Error Wrapping Inconsistency
51
+
- #21: Unused Context Parameter
52
+
- #33-34: Minor Code Cleanup
53
+
54
+
### Group F: Documentation
55
+
56
+
- #28: Inconsistent Command Help
57
+
- #29: Config Example Doesn't Match Defaults
58
+
59
+
### Group G: Testing
60
+
61
+
- #20: No Tests
62
+
63
+
---
64
+
65
+
## **Critical Issues - COMPLETED** ✅
66
+
67
+
### 1. **Database Connection Pool Not Configured** ✅
68
+
69
+
**Fixed:** Added WAL mode, busy timeout, and connection pool limits in `store/db.go:16-18`
70
+
71
+
### 2. **Code Duplication in Scheduler** ✅
72
+
73
+
**Fixed:** Refactored into shared `collectNewItems` and `sendDigestAndMarkSeen` helper methods
74
+
75
+
### 3. **No Context Timeout on HTTP Requests** 🔄
76
+
77
+
**Location:** `scheduler/fetch.go:41-43`
78
+
79
+
While you set a 30s timeout on the context, the HTTP client also has a separate 30s timeout. If the context times out first, the HTTP client won't respect it immediately.
80
+
81
+
**Fix:** Use context-aware client without separate timeout.
82
+
83
+
### 4. **Missing Index on Configs Active Status** ✅
84
+
85
+
**Fixed:** Added partial index in `store/db.go:93`
86
+
87
+
### 5. **Race Condition in seen_items** ✅
88
+
89
+
**Fixed:** Using transactions to mark items seen before email send
90
+
91
+
### 6. **Unbounded Memory Growth in Feed Fetching** ✅
92
+
93
+
**Fixed:** Added semaphore limiting to 10 concurrent fetches in `scheduler/fetch.go`
94
+
95
+
### 7. **Silent Failure on Email Send** ✅
96
+
97
+
**Fixed:** Items only marked seen after successful email via transaction commit
98
+
99
+
### 14. **No Rate Limiting** ✅
100
+
101
+
**Fixed:** Added comprehensive rate limiting using `golang.org/x/time/rate`:
102
+
- Created reusable `ratelimit.Limiter` with token bucket algorithm (`ratelimit/limiter.go`)
103
+
- Web handler middleware: 10 req/sec, burst of 20 per IP (`web/server.go:65-77`)
104
+
- SSH authentication: 5 req/sec, burst of 10 per fingerprint (`ssh/server.go:96-101`)
105
+
- SCP uploads: 5 req/sec, burst of 10 per user (`ssh/scp.go:107-110`)
106
+
- Email sending: 1 per minute per user (`scheduler/scheduler.go:207-210`)
107
+
- Added 1MB max file size limit for SCP uploads (`ssh/scp.go:112-115`)
108
+
- Rate limiter automatically cleans up inactive entries every 5 minutes
109
+
110
+
### 15. **Token Generation Not Cryptographically Secure** ✅
111
+
112
+
**Already secure:** Confirmed using `crypto/rand` in `store/unsubscribe.go:14`
113
+
114
+
---
115
+
116
+
## **Performance Issues**
117
+
118
+
### 8. **N+1 Query Problem** 🐌
119
+
120
+
**Location:** Multiple locations
121
+
122
+
- `web/handlers.go:99-103` - Gets feeds for each config in a loop
123
+
- `scheduler/scheduler.go` - Checks each item individually for seen status
124
+
125
+
**Fix:** Batch operations:
126
+
127
+
```go
128
+
// Instead of checking each item individually
129
+
seenGuids, err := s.store.GetSeenGUIDs(ctx, feedID, itemGUIDs)
130
+
```
131
+
132
+
### 9. **No Prepared Statements** 📝
133
+
134
+
**Location:** All store methods
135
+
136
+
Every query uses `QueryContext`/`ExecContext` with raw SQL strings. These are reparsed on every call.
137
+
138
+
**Fix:** Use prepared statements for frequently called queries (IsItemSeen, MarkItemSeen, etc.).
139
+
140
+
### 10. **Inefficient Sorting in Handlers** 🔢
141
+
142
+
**Location:** `web/handlers.go:231-235` and `326-330`
143
+
144
+
You sort items by parsing time strings in a comparison function. This parses the same timestamps multiple times.
145
+
146
+
**Fix:** Parse once, sort by parsed time, or use database ORDER BY.
147
+
148
+
### 11. **No HTTP Caching Headers** 🌐
149
+
150
+
**Location:** `web/handlers.go` - all feed handlers
151
+
152
+
RSS/JSON feeds don't set `Cache-Control`, `ETag`, or `Last-Modified` headers. Every request fetches from DB.
153
+
154
+
**Fix:** Add caching headers:
155
+
156
+
```go
157
+
w.Header().Set("Cache-Control", "public, max-age=300")
158
+
w.Header().Set("ETag", fmt.Sprintf(`"%s-%d"`, fingerprint, cfg.LastRun.Time.Unix()))
159
+
```
160
+
161
+
### 12. **Database Migration Runs on Every Connection** 🔄
162
+
163
+
**Location:** `store/db.go:26-28`
164
+
165
+
Migration runs inside `Open()`, which happens once at startup. However, `Migrate()` is also exposed and called separately in `main.go:160`. The schema execution uses `CREATE TABLE IF NOT EXISTS` which is fine, but it's still unnecessary work.
166
+
167
+
---
168
+
169
+
## **Security Issues**
170
+
171
+
### 13. **Missing Input Validation on Email Addresses** ✅
172
+
173
+
**Already implemented:** Using `net/mail.ParseAddress()` in `config/validate.go:24-26`
174
+
175
+
### 16. **No SMTP Auth Validation** 🔒
176
+
177
+
**Location:** `email/send.go:102-105`
178
+
179
+
SMTP auth is optional (`if m.cfg.User != "" && m.cfg.Pass != ""`). Many SMTP servers require auth, and this silently continues without it.
180
+
181
+
**Fix:** Validate SMTP config at startup.
182
+
183
+
### 17. **SQL Injection Potential in UPSERT** 💉
184
+
185
+
**Location:** `store/items.go:29-33`
186
+
187
+
While using parameterized queries (good!), the `ON CONFLICT` clause should explicitly name the conflict target for clarity and safety:
188
+
189
+
```sql
190
+
ON CONFLICT(feed_id, guid) DO UPDATE SET ...
191
+
```
192
+
193
+
(Actually you already do this correctly, but worth noting for other queries)
194
+
195
+
---
196
+
197
+
## **Code Quality Issues**
198
+
199
+
### 18. **Error Wrapping Inconsistency** 🎁
200
+
201
+
Some functions use `fmt.Errorf("verb: %w", err)`, others use `fmt.Errorf("verb %w", err)` (no colon). Inconsistent style makes logs harder to parse.
202
+
203
+
### 19. **Magic Numbers** 🎩
204
+
205
+
**Location:** Multiple
206
+
207
+
- `scheduler/scheduler.go:84` - hardcoded 3 months
208
+
- `scheduler/scheduler.go:148-150` - hardcoded 5 items threshold
209
+
- `web/handlers.go:238` and `332` - hardcoded 100 items limit
210
+
- `scheduler/fetch.go:41` - hardcoded 30s timeout
211
+
212
+
**Fix:** Extract to constants or config.
213
+
214
+
### 20. **No Tests** 🧪
215
+
216
+
**Location:** Entire codebase
217
+
218
+
Zero test coverage. Critical business logic (cron parsing, config parsing, email rendering) is untested.
219
+
220
+
### 21. **Unused Context Parameter** 🗑️
221
+
222
+
**Location:** `store/db.go:109-111`
223
+
224
+
```go
225
+
func (db *DB) Migrate(ctx context.Context) error {
226
+
return db.migrate() // ctx is ignored
227
+
}
228
+
```
229
+
230
+
Either remove the context parameter or pass it to a context-aware migrate function.
231
+
232
+
### 22. **Inconsistent Logging Levels** 📝
233
+
234
+
Some errors are `logger.Error`, some are `logger.Warn`. For example, feed fetch errors are `Warn` (line 89 of scheduler.go) but other errors are `Error`. Establish consistent criteria.
235
+
236
+
### 23. **Missing Graceful Shutdown for Scheduler** 🛑
237
+
238
+
**Location:** `main.go:194-197`
239
+
240
+
The scheduler runs in a goroutine with errgroup, but `Start()` only returns on context cancellation. If scheduler panics, errgroup won't capture it.
241
+
242
+
**Fix:** Add defer recover in scheduler or use errgroup.Go properly.
243
+
244
+
---
245
+
246
+
## **Missing Features**
247
+
248
+
### 24. **No Metrics/Observability** 📈
249
+
250
+
No Prometheus metrics, no health check endpoint, no structured logging for monitoring. For a long-running service, this is critical.
251
+
252
+
### 25. **No Email Validation on Successful Send** ✅
253
+
254
+
You log `"email sent"` but don't verify SMTP actually accepted it (some SMTP servers queue and fail later).
255
+
256
+
### 26. **No Cleanup of Old seen_items** 🧹
257
+
258
+
The `seen_items` table will grow indefinitely. With the 3-month filter, items older than 3 months can be safely deleted.
259
+
260
+
**Fix:** Add periodic cleanup job:
261
+
262
+
```go
263
+
DELETE FROM seen_items WHERE seen_at < datetime('now', '-6 months')
264
+
```
265
+
266
+
### 27. **No Feed Validation on Upload** 🔍
267
+
268
+
When a user uploads a config with feed URLs, you don't validate the URLs are actually RSS/Atom feeds. First run will fail.
269
+
270
+
**Fix:** Optionally fetch and validate feeds on upload (with short timeout).
271
+
272
+
---
273
+
274
+
## **Documentation Issues**
275
+
276
+
### 28. **Inconsistent Command Help** 📖
277
+
278
+
`ssh/server.go:160-165` shows welcome message with command list, but the actual commands are in `ssh/commands.go` (not reviewed in detail). These could drift out of sync.
279
+
280
+
### 29. **Config Example Doesn't Match Defaults** ⚙️
281
+
282
+
`main.go:88` shows `inline: false` as default in comment, but `config/parse.go:27` sets default to `false`, and `README.md:89` says default is `true`.
283
+
284
+
**Fix:** Align all documentation with actual code defaults.
285
+
286
+
---
287
+
288
+
## **Architectural Concerns**
289
+
290
+
### 30. **Scheduler Interval is Fixed** ⏲️
291
+
292
+
**Location:** `main.go:172`
293
+
294
+
60-second interval is hardcoded. This doesn't scale well—if you have thousands of users, checking every 60 seconds is wasteful. Consider event-driven scheduling with a priority queue.
295
+
296
+
### 31. **No Pagination on Feed Endpoints** 📄
297
+
298
+
**Location:** `web/handlers.go:238` and `332`
299
+
300
+
Hardcoded limit of 100 items. Users can't access older items.
301
+
302
+
### 32. **No Transaction for Config Update** ✅
303
+
304
+
**Fixed:** Config upload now uses transactions in `ssh/scp.go:134-161`
305
+
306
+
---
307
+
308
+
## **Minor Issues**
309
+
310
+
33. **Unused `getCommitHash()` function** - `main.go:127-140` - function defined but only used in one place, could be inlined
311
+
34. **Inconsistent fingerprint shortening** - Sometimes 12 chars, sometimes 7 chars
312
+
35. **HTTP server doesn't log requests** - No request logging middleware
313
+
36. ✅ **No max file size on SCP upload** - Fixed: 1MB limit in `ssh/scp.go:112-115`
314
+
37. **No validation on cron expressions at upload time** - Invalid cron is only caught on first run
315
+
316
+
---
317
+
318
+
## **Positive Notes** ✅
319
+
320
+
- Good use of Context for cancellation
321
+
- Proper use of foreign keys and CASCADE
322
+
- Clean separation of concerns (store/scheduler/ssh/web)
323
+
- Good use of Charm libraries
324
+
- ETag/Last-Modified support for feed fetching
325
+
- Unsubscribe functionality implemented
326
+
- SQL injection protection with parameterized queries
327
+
- Config file validation before accepting uploads
328
+
329
+
---
330
+
331
+
## **Priority Fixes**
332
+
333
+
### **High Priority (Fix ASAP):**
334
+
335
+
1. ✅ Database connection pool configuration (#1)
336
+
2. ✅ Race condition in seen_items (#5)
337
+
3. ✅ Silent failure on email send (#7)
338
+
4. ✅ No rate limiting (#14)
339
+
5. ✅ No transaction for config updates (#32)
340
+
6. ✅ Token generation security (#15)
341
+
7. ✅ Max file size on SCP upload (#36)
342
+
343
+
### **Medium Priority:**
344
+
345
+
6. ✅ Code duplication in scheduler (#2)
346
+
7. N+1 query problems (#8)
347
+
8. ✅ Unbounded feed fetching concurrency (#6)
348
+
9. ✅ Missing input validation (#13)
349
+
10. No cleanup of old data (#26)
350
+
351
+
### **Low Priority (Technical Debt):**
352
+
353
+
11. Add tests (#20)
354
+
12. Add metrics (#24)
355
+
13. Extract magic numbers (#19)
356
+
14. Add HTTP caching (#11)
+1
go.mod
+1
go.mod
+2
go.sum
+2
go.sum
···
201
201
golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ=
202
202
golang.org/x/text v0.33.0 h1:B3njUFyqtHDUI5jMn1YIr5B0IE2U0qck04r6d4KPAxE=
203
203
golang.org/x/text v0.33.0/go.mod h1:LuMebE6+rBincTi9+xWTY8TztLzKHc/9C1uBCG27+q8=
204
+
golang.org/x/time v0.11.0 h1:/bpjEDfN9tkoN/ryeYHnv5hcMlc8ncjMcM4XBk5NWV0=
205
+
golang.org/x/time v0.11.0/go.mod h1:CDIdPxbZBQxdj6cxyCIdrNogrJKMJ7pr37NYpMcMDSg=
204
206
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
205
207
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
206
208
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
+64
ratelimit/limiter.go
+64
ratelimit/limiter.go
···
1
+
package ratelimit
2
+
3
+
import (
4
+
"sync"
5
+
"time"
6
+
7
+
"golang.org/x/time/rate"
8
+
)
9
+
10
+
// Limiter provides per-key rate limiting using token bucket algorithm
11
+
type Limiter struct {
12
+
mu sync.RWMutex
13
+
limiters map[string]*rate.Limiter
14
+
rate rate.Limit
15
+
burst int
16
+
cleanup time.Duration
17
+
lastSeen map[string]time.Time
18
+
}
19
+
20
+
// New creates a new Limiter with the given rate (requests per second) and burst size
21
+
func New(rps float64, burst int) *Limiter {
22
+
l := &Limiter{
23
+
limiters: make(map[string]*rate.Limiter),
24
+
rate: rate.Limit(rps),
25
+
burst: burst,
26
+
cleanup: 5 * time.Minute,
27
+
lastSeen: make(map[string]time.Time),
28
+
}
29
+
go l.cleanupLoop()
30
+
return l
31
+
}
32
+
33
+
// Allow checks if the request for the given key is allowed
34
+
func (l *Limiter) Allow(key string) bool {
35
+
l.mu.Lock()
36
+
defer l.mu.Unlock()
37
+
38
+
limiter, exists := l.limiters[key]
39
+
if !exists {
40
+
limiter = rate.NewLimiter(l.rate, l.burst)
41
+
l.limiters[key] = limiter
42
+
}
43
+
44
+
l.lastSeen[key] = time.Now()
45
+
return limiter.Allow()
46
+
}
47
+
48
+
// cleanupLoop removes limiters that haven't been used recently
49
+
func (l *Limiter) cleanupLoop() {
50
+
ticker := time.NewTicker(l.cleanup)
51
+
defer ticker.Stop()
52
+
53
+
for range ticker.C {
54
+
l.mu.Lock()
55
+
cutoff := time.Now().Add(-l.cleanup * 2)
56
+
for key, lastSeen := range l.lastSeen {
57
+
if lastSeen.Before(cutoff) {
58
+
delete(l.limiters, key)
59
+
delete(l.lastSeen, key)
60
+
}
61
+
}
62
+
l.mu.Unlock()
63
+
}
64
+
}
+18
-10
scheduler/scheduler.go
+18
-10
scheduler/scheduler.go
···
8
8
"github.com/adhocore/gronx"
9
9
"github.com/charmbracelet/log"
10
10
"github.com/kierank/herald/email"
11
+
"github.com/kierank/herald/ratelimit"
11
12
"github.com/kierank/herald/store"
12
13
)
13
14
14
15
type Scheduler struct {
15
-
store *store.DB
16
-
mailer *email.Mailer
17
-
logger *log.Logger
18
-
interval time.Duration
19
-
originURL string
16
+
store *store.DB
17
+
mailer *email.Mailer
18
+
logger *log.Logger
19
+
interval time.Duration
20
+
originURL string
21
+
rateLimiter *ratelimit.Limiter
20
22
}
21
23
22
24
func NewScheduler(st *store.DB, mailer *email.Mailer, logger *log.Logger, interval time.Duration, originURL string) *Scheduler {
23
25
return &Scheduler{
24
-
store: st,
25
-
mailer: mailer,
26
-
logger: logger,
27
-
interval: interval,
28
-
originURL: originURL,
26
+
store: st,
27
+
mailer: mailer,
28
+
logger: logger,
29
+
interval: interval,
30
+
originURL: originURL,
31
+
rateLimiter: ratelimit.New(1.0/60.0, 1), // 1 email per minute per user
29
32
}
30
33
}
31
34
···
200
203
dashboardURL = s.originURL + "/" + user.PubkeyFP
201
204
} else {
202
205
s.logger.Warn("failed to get user for dashboard URL", "err", err)
206
+
}
207
+
208
+
// Rate limit email sending per user
209
+
if !s.rateLimiter.Allow(fmt.Sprintf("email:%d", cfg.UserID)) {
210
+
return fmt.Errorf("rate limit exceeded for email sending")
203
211
}
204
212
205
213
// Begin transaction to mark items seen
+16
-4
ssh/scp.go
+16
-4
ssh/scp.go
···
14
14
"github.com/charmbracelet/ssh"
15
15
"github.com/charmbracelet/wish/scp"
16
16
"github.com/kierank/herald/config"
17
+
"github.com/kierank/herald/ratelimit"
17
18
"github.com/kierank/herald/scheduler"
18
19
"github.com/kierank/herald/store"
19
20
)
20
21
21
22
type scpHandler struct {
22
-
store *store.DB
23
-
scheduler *scheduler.Scheduler
24
-
logger *log.Logger
23
+
store *store.DB
24
+
scheduler *scheduler.Scheduler
25
+
logger *log.Logger
26
+
rateLimiter *ratelimit.Limiter
25
27
}
26
28
27
29
func (h *scpHandler) Glob(s ssh.Session, pattern string) ([]string, error) {
···
106
108
return 0, fmt.Errorf("no user in context")
107
109
}
108
110
111
+
// Rate limit SCP uploads (per user)
112
+
if !h.rateLimiter.Allow(fmt.Sprintf("scp:%d", user.ID)) {
113
+
return 0, fmt.Errorf("rate limit exceeded, please try again later")
114
+
}
115
+
116
+
// Max file size: 1MB
117
+
if entry.Size > 1024*1024 {
118
+
return 0, fmt.Errorf("file too large (max 1MB)")
119
+
}
120
+
109
121
name := entry.Name
110
122
if !strings.HasSuffix(name, ".txt") {
111
123
return 0, fmt.Errorf("only .txt files are supported")
112
124
}
113
125
114
-
content, err := io.ReadAll(entry.Reader)
126
+
content, err := io.ReadAll(io.LimitReader(entry.Reader, 1024*1024))
115
127
if err != nil {
116
128
return 0, fmt.Errorf("failed to read file: %w", err)
117
129
}
+21
-11
ssh/server.go
+21
-11
ssh/server.go
···
14
14
"github.com/charmbracelet/ssh"
15
15
"github.com/charmbracelet/wish"
16
16
"github.com/charmbracelet/wish/scp"
17
+
"github.com/kierank/herald/ratelimit"
17
18
"github.com/kierank/herald/scheduler"
18
19
"github.com/kierank/herald/store"
19
20
gossh "golang.org/x/crypto/ssh"
···
28
29
}
29
30
30
31
type Server struct {
31
-
cfg Config
32
-
store *store.DB
33
-
scheduler *scheduler.Scheduler
34
-
logger *log.Logger
32
+
cfg Config
33
+
store *store.DB
34
+
scheduler *scheduler.Scheduler
35
+
logger *log.Logger
36
+
rateLimiter *ratelimit.Limiter
35
37
}
36
38
37
39
func NewServer(cfg Config, st *store.DB, sched *scheduler.Scheduler, logger *log.Logger) *Server {
38
40
return &Server{
39
-
cfg: cfg,
40
-
store: st,
41
-
scheduler: sched,
42
-
logger: logger,
41
+
cfg: cfg,
42
+
store: st,
43
+
scheduler: sched,
44
+
logger: logger,
45
+
rateLimiter: ratelimit.New(5, 10), // 5 req/sec, burst of 10 for SSH/SCP
43
46
}
44
47
}
45
48
···
49
52
}
50
53
51
54
handler := &scpHandler{
52
-
store: s.store,
53
-
scheduler: s.scheduler,
54
-
logger: s.logger,
55
+
store: s.store,
56
+
scheduler: s.scheduler,
57
+
logger: s.logger,
58
+
rateLimiter: s.rateLimiter,
55
59
}
56
60
57
61
srv, err := wish.NewServer(
···
95
99
func (s *Server) publicKeyHandler(ctx ssh.Context, key ssh.PublicKey) bool {
96
100
fp := gossh.FingerprintSHA256(key)
97
101
pubkeyStr := string(gossh.MarshalAuthorizedKey(key))
102
+
103
+
// Rate limit authentication attempts by fingerprint
104
+
if !s.rateLimiter.Allow(fmt.Sprintf("auth:%s", fp)) {
105
+
s.logger.Warn("rate limit exceeded for auth", "fingerprint", fp)
106
+
return false
107
+
}
98
108
99
109
if !s.cfg.AllowAllKeys {
100
110
allowed := false
+36
-15
web/server.go
+36
-15
web/server.go
···
4
4
"context"
5
5
"embed"
6
6
"html/template"
7
+
"net"
7
8
"net/http"
8
9
"strings"
9
10
10
11
"github.com/charmbracelet/log"
12
+
"github.com/kierank/herald/ratelimit"
11
13
"github.com/kierank/herald/store"
12
14
)
13
15
···
15
17
var templatesFS embed.FS
16
18
17
19
type Server struct {
18
-
store *store.DB
19
-
addr string
20
-
origin string
21
-
sshPort int
22
-
logger *log.Logger
23
-
tmpl *template.Template
24
-
commitHash string
20
+
store *store.DB
21
+
addr string
22
+
origin string
23
+
sshPort int
24
+
logger *log.Logger
25
+
tmpl *template.Template
26
+
commitHash string
27
+
rateLimiter *ratelimit.Limiter
25
28
}
26
29
27
30
func NewServer(st *store.DB, addr string, origin string, sshPort int, logger *log.Logger, commitHash string) *Server {
28
31
tmpl := template.Must(template.ParseFS(templatesFS, "templates/*.html"))
29
32
return &Server{
30
-
store: st,
31
-
addr: addr,
32
-
origin: origin,
33
-
sshPort: sshPort,
34
-
logger: logger,
35
-
tmpl: tmpl,
36
-
commitHash: commitHash,
33
+
store: st,
34
+
addr: addr,
35
+
origin: origin,
36
+
sshPort: sshPort,
37
+
logger: logger,
38
+
tmpl: tmpl,
39
+
commitHash: commitHash,
40
+
rateLimiter: ratelimit.New(10, 20), // 10 req/sec, burst of 20
37
41
}
38
42
}
39
43
···
45
49
46
50
srv := &http.Server{
47
51
Addr: s.addr,
48
-
Handler: mux,
52
+
Handler: s.rateLimitMiddleware(mux),
49
53
}
50
54
51
55
go func() {
···
59
63
return nil
60
64
}
61
65
return err
66
+
}
67
+
68
+
func (s *Server) rateLimitMiddleware(next http.Handler) http.Handler {
69
+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
70
+
ip, _, err := net.SplitHostPort(r.RemoteAddr)
71
+
if err != nil {
72
+
ip = r.RemoteAddr
73
+
}
74
+
75
+
if !s.rateLimiter.Allow(ip) {
76
+
s.logger.Warn("rate limit exceeded", "ip", ip, "path", r.URL.Path)
77
+
http.Error(w, "Too Many Requests", http.StatusTooManyRequests)
78
+
return
79
+
}
80
+
81
+
next.ServeHTTP(w, r)
82
+
})
62
83
}
63
84
64
85
func (s *Server) routeHandler(w http.ResponseWriter, r *http.Request) {