1.0/mvp #33

New Issue

Open

opened 2026-03-04 12:09:21 +01:00 by sneak · 7 comments

sneak commented 2026-03-04 12:09:21 +01:00

Owner

Copy Link

@clawbot are we ready to put this into prod and tag a 1.0.0, exposed to the whole internet? are we ready? what are we missing? does it have all appropriate prod security headers, all security pitfalls have been checked?

@clawbot are we ready to put this into prod and tag a 1.0.0, exposed to the whole internet? are we ready? what are we missing? does it have all appropriate prod security headers, all security pitfalls have been checked?

clawbot commented 2026-03-04 12:19:16 +01:00

Collaborator

Copy Link

Pre-1.0 Security Audit -- sneak/webhooker

Comprehensive security audit for internet-facing 1.0.0 readiness.

---

BLOCKERS -- Must Fix Before Internet Exposure

1. No Security Headers (middleware)

File: internal/middleware/middleware.go (entire file -- missing middleware)
File: internal/server/routes.go (middleware stack, lines 10-40)

The application sets zero production security headers. None of the following are present:

Header	Status
Strict-Transport-Security (HSTS)	Missing
X-Content-Type-Options: nosniff	Missing
X-Frame-Options: DENY	Missing
Content-Security-Policy	Missing
X-XSS-Protection: 0	Missing
Referrer-Policy: strict-origin-when-cross-origin	Missing
Permissions-Policy	Missing

Impact: Without these, the app is vulnerable to clickjacking (iframe embedding), MIME type sniffing attacks, and lacks defense-in-depth for XSS. HSTS is critical for an internet-facing app to prevent SSL stripping.

Suggested fix: Add a SecurityHeaders() middleware that sets all headers on every response. Apply it early in the middleware stack in routes.go. CSP needs unsafe-eval for Alpine.js and unsafe-inline for Tailwind/style blocks. These should be tightened with nonces if possible.

The README TODO section confirms this is known unfinished work.

---

2. No CSRF Protection on Any State-Changing Forms

Files: All form templates and their POST handlers:

• templates/login.html -- login form
• templates/sources_new.html -- create webhook
• templates/source_edit.html -- edit webhook
• templates/source_detail.html -- delete webhook, add/delete/toggle entrypoints and targets
• internal/handlers/auth.go -- login handler
• internal/handlers/source_management.go -- all CRUD handlers

None of the 12+ POST forms include a CSRF token. No CSRF middleware exists.

Impact: An attacker can craft a malicious webpage that, when visited by an authenticated user, silently submits forms to create webhooks, add HTTP targets pointing anywhere, delete webhooks, or modify configuration. This is especially dangerous combined with the SSRF issue below -- an attacker could create a target that exfiltrates data from the internal network.

Note: SameSite=Lax on the session cookie provides partial mitigation -- it blocks cross-site POST requests from some vectors, but Lax is not sufficient for security-critical state-changing operations.

Suggested fix: Implement CSRF token middleware (e.g., gorilla/csrf or justinas/nosurf). Add a hidden csrf_token field to every form. Validate on every POST handler.

The README TODO section confirms: "CSRF protection for forms" is listed as unfinished.

---

3. No SSRF Protection -- HTTP Targets Can Hit Internal/Private IPs

File: internal/handlers/source_management.go, HandleTargetCreate() (line ~497-548)
File: internal/delivery/engine.go, doHTTPRequest() (line ~459-505)

When a user creates an HTTP target, the URL is accepted without any validation beyond checking it's non-empty. The delivery engine then makes HTTP POST requests to whatever URL is configured, including:

• http://127.0.0.1:* / http://localhost:* -- local services
• http://169.254.169.254/ -- cloud metadata (AWS/GCP/Azure instance credentials)
• http://10.0.0.0/8, http://172.16.0.0/12, http://192.168.0.0/16 -- internal networks
• file://, gopher://, etc. -- other URL schemes

Impact: A user (or an attacker who gained access via CSRF) can use webhooker as a proxy to scan internal networks, access cloud metadata services, or attack internal services. This is a critical vulnerability for any internet-facing service that makes outbound HTTP requests to user-controlled URLs.

Suggested fix:

1. Validate target URLs at creation time -- must be http:// or https://, must not resolve to private/reserved IP ranges.
2. Add a DNS resolution check before making outbound requests in doHTTPRequest() -- resolve the hostname, check if the IP is in a private range, and reject if so. Use a custom net.Dialer with a Control function that blocks private IPs.
3. Block well-known metadata endpoints explicitly (169.254.169.254, fd00::, etc.)

---

4. No Rate Limiting on Login Endpoint

File: internal/handlers/auth.go, HandleLoginSubmit() (lines 28-89)
File: internal/server/routes.go, line 52

There is no rate limiting, account lockout, or delay on failed login attempts. An attacker can make unlimited login attempts at full speed.

Impact: Trivial brute-force attacks against user passwords. Even with Argon2id (which provides some computational cost per attempt), the lack of any rate limiting means an attacker can try millions of passwords.

Suggested fix:

1. Add per-IP rate limiting on the login endpoint (e.g., 5 attempts per minute per IP).
2. Add account lockout after N consecutive failed attempts (e.g., 10 failures, 15 minute lockout).
3. Consider adding a progressive delay between failed attempts.

---

5. Session Fixation Vulnerability -- No Session Regeneration on Login

File: internal/handlers/auth.go, HandleLoginSubmit() (lines 74-82)

After successful password verification, the handler calls h.session.Get(r) to retrieve the existing session, sets user info on it, and saves it. The session ID (cookie) is never regenerated.

Impact: If an attacker can set a session cookie before the victim logs in (via XSS, network sniffing on HTTP, or subdomain cookie injection), the attacker's cookie remains valid after login, giving them access to the authenticated session.

Suggested fix: After successful authentication, destroy the old session and create a new one. gorilla/sessions doesn't have a built-in "regenerate" method, so destroy the old session, save to clear the cookie, then get a new session and set user info.

---

SHOULD-FIX -- Important for Production Hardening

6. No Target URL Validation

File: internal/handlers/source_management.go, HandleTargetCreate() (line ~530)

The only check on the URL is url == "". There's no validation that it's a well-formed URL, uses an allowed scheme (http/https only), or that the hostname is not an IP literal in a private range.

7. User Profile Route Missing Auth Middleware

File: internal/server/routes.go, lines 62-64

The /user/{username} route is not wrapped with s.mw.RequireAuth(). The handler checks auth internally, but this is defense-by-implementation rather than defense-by-design.

Suggested fix: Add r.Use(s.mw.RequireAuth()) to the route group.

8. No Request Body Size Limit on Form Endpoints

File: internal/handlers/auth.go, internal/handlers/source_management.go

The webhook handler properly limits body size to 1MB, but the login form, create form, edit form, and all other POST handlers call r.ParseForm() without setting a body size limit.

Suggested fix: Wrap r.Body with http.MaxBytesReader(w, r.Body, maxFormSize) before calling ParseForm(), or add a global MaxBytesReader middleware for non-webhook routes.

9. Admin Password Logged as Structured Log Field

File: internal/database/database.go, line 133

The initial admin password is logged via d.log.Info("admin user created", "password", password, ...). In production with structured JSON logging, this password will be written to wherever logs are shipped.

Suggested fix: Print the password to stderr directly (not via slog) with a clear banner, or use a separate output channel.

10. No Inactivity-Based Session Timeout

File: internal/session/session.go, lines 79-82

Sessions have a 7-day MaxAge but no activity-based expiration. A session remains valid for the full 7 days regardless of whether the user has been active.

11. No Cache-Control on Authenticated Pages

Authenticated pages don't set Cache-Control: no-store. Browsers and proxies may cache these pages, potentially exposing sensitive webhook configuration and logs.

12. WriteTimeout (10s) Conflicts with Middleware Timeout (60s)

File: internal/server/http.go, line 13 -- WriteTimeout: 10 * time.Second
File: internal/server/routes.go, line 31 -- middleware.Timeout(60 * time.Second)

The HTTP server's WriteTimeout of 10 seconds will kill connections before the 60-second middleware timeout fires.

---

NICE-TO-HAVE -- Defense in Depth

13. No Webhook Signature Verification

The webhook receiver doesn't verify HMAC signatures from senders. Listed in the TODO. Not strictly required for a store-and-forward proxy, but important for validating webhook authenticity.

14. No Per-Webhook Rate Limiting in Receiver

Listed in the README TODO. Without this, a misbehaving sender could flood a webhook with events.

15. No Automatic Event Retention Cleanup

The retention_days field exists but automatic cleanup is not implemented. Per-webhook databases will grow unbounded.

16. Password Change Flow Not Implemented

Currently there's no way to change the admin password through the UI.

---

What's Already Good

• Templates use html/template -- automatic context-aware escaping, no XSS. No use of template.HTML or other unescaped types.
• GORM parameterized queries -- no SQL injection. All Where() calls use parameterized ? placeholders.
• Argon2id password hashing -- 64MB memory, 1 iteration, 4 threads, 32-byte key, 16-byte salt. Good parameters.
• Constant-time password comparison -- uses subtle.ConstantTimeCompare.
• Session cookie security -- HttpOnly: true, Secure: true in prod, SameSite: Lax.
• Container runs as non-root -- UID 1000, dedicated webhooker user.
• Docker base images pinned by SHA256 -- compliant with REPO_POLICIES.
• CORS is no-op in production -- no Access-Control-Allow-Origin: * in prod mode.
• Error messages don't leak internals -- generic "Internal server error" to users, details only in server logs.
• Webhook body size limited to 1MB -- prevents memory exhaustion from large payloads.
• Outbound HTTP client has 30s timeout -- prevents indefinite hangs.
• No secrets in error responses -- verified across all handlers.
• 32-byte cryptographically random session key -- auto-generated and stored in DB.
• 3,383 lines of tests -- good coverage of delivery engine, middleware, sessions, passwords.
• docker build . passes -- CI gate is green.

---

Dependency Check

Go modules are reasonably current. Key dependency versions:

• golang.org/x/crypto v0.38.0 -- current, no known CVEs
• gorilla/sessions v1.4.0 -- current
• gorilla/securecookie v1.1.2 -- current
• gorm.io/gorm v1.25.5 -- current
• go-chi/chi v1.5.5 -- current
• sentry-go v0.25.0 -- current
• prometheus/client_golang v1.18.0 -- slightly old but no security CVEs

No critical CVEs identified in the dependency tree.

---

Verdict: NO-GO

webhooker is not ready for internet-facing 1.0.0 deployment. The codebase has a solid foundation (good architecture, proper password hashing, template escaping, parameterized queries, non-root container), but it is missing five critical security features that the README itself acknowledges as TODO:

1. Security headers -- zero production headers
2. CSRF protection -- zero forms protected
3. SSRF prevention -- delivery engine will POST to any URL including internal networks
4. Login rate limiting -- zero brute-force protection
5. Session fixation -- sessions not regenerated on login

Items 1-4 are explicitly listed as unfinished in the README TODO. Item 5 is a code-level vulnerability. All five are standard expectations for any internet-facing authenticated web application.

Minimum for 1.0: Fix all five BLOCKER items. The SHOULD-FIX items (especially #6 URL validation, #8 body size limits, #9 password logging) should also be addressed but are lower priority.

---

Suggested Issues to File

1. Add production security headers middleware (HSTS, CSP, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy)
2. Implement CSRF protection for all forms
3. Add SSRF prevention for HTTP delivery targets (private IP blocking, URL validation)
4. Add rate limiting on login endpoint
5. Fix session fixation: regenerate session on login
6. Add request body size limits on form endpoints
7. Improve admin password bootstrap logging (don't use structured slog for passwords)

# Pre-1.0 Security Audit -- sneak/webhooker Comprehensive security audit for internet-facing 1.0.0 readiness. --- ## BLOCKERS -- Must Fix Before Internet Exposure ### 1. No Security Headers (middleware) **File:** `internal/middleware/middleware.go` (entire file -- missing middleware) **File:** `internal/server/routes.go` (middleware stack, lines 10-40) The application sets **zero** production security headers. None of the following are present: | Header | Status | |--------|--------| | `Strict-Transport-Security` (HSTS) | Missing | | `X-Content-Type-Options: nosniff` | Missing | | `X-Frame-Options: DENY` | Missing | | `Content-Security-Policy` | Missing | | `X-XSS-Protection: 0` | Missing | | `Referrer-Policy: strict-origin-when-cross-origin` | Missing | | `Permissions-Policy` | Missing | **Impact:** Without these, the app is vulnerable to clickjacking (iframe embedding), MIME type sniffing attacks, and lacks defense-in-depth for XSS. HSTS is critical for an internet-facing app to prevent SSL stripping. **Suggested fix:** Add a `SecurityHeaders()` middleware that sets all headers on every response. Apply it early in the middleware stack in `routes.go`. CSP needs `unsafe-eval` for Alpine.js and `unsafe-inline` for Tailwind/style blocks. These should be tightened with nonces if possible. The README TODO section confirms this is known unfinished work. --- ### 2. No CSRF Protection on Any State-Changing Forms **Files:** All form templates and their POST handlers: - `templates/login.html` -- login form - `templates/sources_new.html` -- create webhook - `templates/source_edit.html` -- edit webhook - `templates/source_detail.html` -- delete webhook, add/delete/toggle entrypoints and targets - `internal/handlers/auth.go` -- login handler - `internal/handlers/source_management.go` -- all CRUD handlers **None** of the 12+ POST forms include a CSRF token. No CSRF middleware exists. **Impact:** An attacker can craft a malicious webpage that, when visited by an authenticated user, silently submits forms to create webhooks, add HTTP targets pointing anywhere, delete webhooks, or modify configuration. This is especially dangerous combined with the SSRF issue below -- an attacker could create a target that exfiltrates data from the internal network. **Note:** `SameSite=Lax` on the session cookie provides *partial* mitigation -- it blocks cross-site POST requests from some vectors, but Lax is not sufficient for security-critical state-changing operations. **Suggested fix:** Implement CSRF token middleware (e.g., `gorilla/csrf` or `justinas/nosurf`). Add a hidden `csrf_token` field to every form. Validate on every POST handler. The README TODO section confirms: "CSRF protection for forms" is listed as unfinished. --- ### 3. No SSRF Protection -- HTTP Targets Can Hit Internal/Private IPs **File:** `internal/handlers/source_management.go`, `HandleTargetCreate()` (line ~497-548) **File:** `internal/delivery/engine.go`, `doHTTPRequest()` (line ~459-505) When a user creates an HTTP target, the URL is accepted without any validation beyond checking it's non-empty. The delivery engine then makes HTTP POST requests to whatever URL is configured, including: - `http://127.0.0.1:*` / `http://localhost:*` -- local services - `http://169.254.169.254/` -- cloud metadata (AWS/GCP/Azure instance credentials) - `http://10.0.0.0/8`, `http://172.16.0.0/12`, `http://192.168.0.0/16` -- internal networks - `file://`, `gopher://`, etc. -- other URL schemes **Impact:** A user (or an attacker who gained access via CSRF) can use webhooker as a proxy to scan internal networks, access cloud metadata services, or attack internal services. This is a **critical** vulnerability for any internet-facing service that makes outbound HTTP requests to user-controlled URLs. **Suggested fix:** 1. Validate target URLs at creation time -- must be `http://` or `https://`, must not resolve to private/reserved IP ranges. 2. Add a DNS resolution check before making outbound requests in `doHTTPRequest()` -- resolve the hostname, check if the IP is in a private range, and reject if so. Use a custom `net.Dialer` with a `Control` function that blocks private IPs. 3. Block well-known metadata endpoints explicitly (169.254.169.254, fd00::, etc.) --- ### 4. No Rate Limiting on Login Endpoint **File:** `internal/handlers/auth.go`, `HandleLoginSubmit()` (lines 28-89) **File:** `internal/server/routes.go`, line 52 There is no rate limiting, account lockout, or delay on failed login attempts. An attacker can make unlimited login attempts at full speed. **Impact:** Trivial brute-force attacks against user passwords. Even with Argon2id (which provides some computational cost per attempt), the lack of any rate limiting means an attacker can try millions of passwords. **Suggested fix:** 1. Add per-IP rate limiting on the login endpoint (e.g., 5 attempts per minute per IP). 2. Add account lockout after N consecutive failed attempts (e.g., 10 failures, 15 minute lockout). 3. Consider adding a progressive delay between failed attempts. --- ### 5. Session Fixation Vulnerability -- No Session Regeneration on Login **File:** `internal/handlers/auth.go`, `HandleLoginSubmit()` (lines 74-82) After successful password verification, the handler calls `h.session.Get(r)` to retrieve the **existing** session, sets user info on it, and saves it. The session ID (cookie) is never regenerated. **Impact:** If an attacker can set a session cookie before the victim logs in (via XSS, network sniffing on HTTP, or subdomain cookie injection), the attacker's cookie remains valid after login, giving them access to the authenticated session. **Suggested fix:** After successful authentication, destroy the old session and create a new one. gorilla/sessions doesn't have a built-in "regenerate" method, so destroy the old session, save to clear the cookie, then get a new session and set user info. --- ## SHOULD-FIX -- Important for Production Hardening ### 6. No Target URL Validation **File:** `internal/handlers/source_management.go`, `HandleTargetCreate()` (line ~530) The only check on the URL is `url == ""`. There's no validation that it's a well-formed URL, uses an allowed scheme (http/https only), or that the hostname is not an IP literal in a private range. ### 7. User Profile Route Missing Auth Middleware **File:** `internal/server/routes.go`, lines 62-64 The `/user/{username}` route is **not** wrapped with `s.mw.RequireAuth()`. The handler checks auth internally, but this is defense-by-implementation rather than defense-by-design. **Suggested fix:** Add `r.Use(s.mw.RequireAuth())` to the route group. ### 8. No Request Body Size Limit on Form Endpoints **File:** `internal/handlers/auth.go`, `internal/handlers/source_management.go` The webhook handler properly limits body size to 1MB, but the login form, create form, edit form, and all other POST handlers call `r.ParseForm()` without setting a body size limit. **Suggested fix:** Wrap `r.Body` with `http.MaxBytesReader(w, r.Body, maxFormSize)` before calling `ParseForm()`, or add a global `MaxBytesReader` middleware for non-webhook routes. ### 9. Admin Password Logged as Structured Log Field **File:** `internal/database/database.go`, line 133 The initial admin password is logged via `d.log.Info("admin user created", "password", password, ...)`. In production with structured JSON logging, this password will be written to wherever logs are shipped. **Suggested fix:** Print the password to stderr directly (not via slog) with a clear banner, or use a separate output channel. ### 10. No Inactivity-Based Session Timeout **File:** `internal/session/session.go`, lines 79-82 Sessions have a 7-day `MaxAge` but no activity-based expiration. A session remains valid for the full 7 days regardless of whether the user has been active. ### 11. No Cache-Control on Authenticated Pages Authenticated pages don't set `Cache-Control: no-store`. Browsers and proxies may cache these pages, potentially exposing sensitive webhook configuration and logs. ### 12. WriteTimeout (10s) Conflicts with Middleware Timeout (60s) **File:** `internal/server/http.go`, line 13 -- `WriteTimeout: 10 * time.Second` **File:** `internal/server/routes.go`, line 31 -- `middleware.Timeout(60 * time.Second)` The HTTP server's `WriteTimeout` of 10 seconds will kill connections before the 60-second middleware timeout fires. --- ## NICE-TO-HAVE -- Defense in Depth ### 13. No Webhook Signature Verification The webhook receiver doesn't verify HMAC signatures from senders. Listed in the TODO. Not strictly required for a store-and-forward proxy, but important for validating webhook authenticity. ### 14. No Per-Webhook Rate Limiting in Receiver Listed in the README TODO. Without this, a misbehaving sender could flood a webhook with events. ### 15. No Automatic Event Retention Cleanup The `retention_days` field exists but automatic cleanup is not implemented. Per-webhook databases will grow unbounded. ### 16. Password Change Flow Not Implemented Currently there's no way to change the admin password through the UI. --- ## What's Already Good - **Templates use `html/template`** -- automatic context-aware escaping, no XSS. No use of `template.HTML` or other unescaped types. - **GORM parameterized queries** -- no SQL injection. All `Where()` calls use parameterized `?` placeholders. - **Argon2id password hashing** -- 64MB memory, 1 iteration, 4 threads, 32-byte key, 16-byte salt. Good parameters. - **Constant-time password comparison** -- uses `subtle.ConstantTimeCompare`. - **Session cookie security** -- `HttpOnly: true`, `Secure: true` in prod, `SameSite: Lax`. - **Container runs as non-root** -- UID 1000, dedicated `webhooker` user. - **Docker base images pinned by SHA256** -- compliant with REPO_POLICIES. - **CORS is no-op in production** -- no `Access-Control-Allow-Origin: *` in prod mode. - **Error messages don't leak internals** -- generic "Internal server error" to users, details only in server logs. - **Webhook body size limited to 1MB** -- prevents memory exhaustion from large payloads. - **Outbound HTTP client has 30s timeout** -- prevents indefinite hangs. - **No secrets in error responses** -- verified across all handlers. - **32-byte cryptographically random session key** -- auto-generated and stored in DB. - **3,383 lines of tests** -- good coverage of delivery engine, middleware, sessions, passwords. - **`docker build .` passes** -- CI gate is green. --- ## Dependency Check Go modules are reasonably current. Key dependency versions: - `golang.org/x/crypto v0.38.0` -- current, no known CVEs - `gorilla/sessions v1.4.0` -- current - `gorilla/securecookie v1.1.2` -- current - `gorm.io/gorm v1.25.5` -- current - `go-chi/chi v1.5.5` -- current - `sentry-go v0.25.0` -- current - `prometheus/client_golang v1.18.0` -- slightly old but no security CVEs No critical CVEs identified in the dependency tree. --- ## Verdict: **NO-GO** webhooker is **not ready** for internet-facing 1.0.0 deployment. The codebase has a solid foundation (good architecture, proper password hashing, template escaping, parameterized queries, non-root container), but it is missing **five critical security features** that the README itself acknowledges as TODO: 1. **Security headers** -- zero production headers 2. **CSRF protection** -- zero forms protected 3. **SSRF prevention** -- delivery engine will POST to any URL including internal networks 4. **Login rate limiting** -- zero brute-force protection 5. **Session fixation** -- sessions not regenerated on login Items 1-4 are explicitly listed as unfinished in the README TODO. Item 5 is a code-level vulnerability. All five are standard expectations for any internet-facing authenticated web application. **Minimum for 1.0:** Fix all five BLOCKER items. The SHOULD-FIX items (especially #6 URL validation, #8 body size limits, #9 password logging) should also be addressed but are lower priority. --- ## Suggested Issues to File 1. **Add production security headers middleware** (HSTS, CSP, X-Frame-Options, X-Content-Type-Options, Referrer-Policy, Permissions-Policy) 2. **Implement CSRF protection for all forms** 3. **Add SSRF prevention for HTTP delivery targets** (private IP blocking, URL validation) 4. **Add rate limiting on login endpoint** 5. **Fix session fixation: regenerate session on login** 6. **Add request body size limits on form endpoints** 7. **Improve admin password bootstrap logging** (don't use structured slog for passwords) <!-- session: agent:sdlc-manager:subagent:9f8f5e11-3608-4eda-bfb3-c0720af17e60 -->

clawbot referenced this issue 2026-03-04 12:21:18 +01:00

[security] Add production security headers middleware #34

clawbot referenced this issue 2026-03-04 12:21:18 +01:00

[security] Implement CSRF protection for all forms #35

clawbot referenced this issue 2026-03-04 12:21:19 +01:00

[security] Add SSRF prevention for HTTP delivery targets #36

clawbot referenced this issue 2026-03-04 12:21:20 +01:00

[security] Add rate limiting on login endpoint #37

clawbot referenced this issue 2026-03-04 12:21:20 +01:00

[security] Fix session fixation: regenerate session on login #38

clawbot referenced this issue 2026-03-04 12:21:21 +01:00

[security] Add request body size limits on form endpoints #39

clawbot referenced this issue 2026-03-04 12:21:22 +01:00

[security] Fix admin password bootstrap logging #40

clawbot commented 2026-03-04 12:21:32 +01:00

Collaborator

Copy Link

Security Audit Complete — Issues Created

Created 7 tracking issues from the audit findings above:

Blockers (must-fix before internet exposure):

• #34 — Add production security headers middleware
• #35 — Implement CSRF protection for all forms
• #36 — Add SSRF prevention for HTTP delivery targets
• #37 — Add rate limiting on login endpoint
• #38 — Fix session fixation: regenerate session on login

Should-fix:

• #39 — Add request body size limits on form endpoints
• #40 — Fix admin password bootstrap logging

Verdict: NO-GO for internet-facing 1.0.0. 5 blockers must be resolved first. What's solid: template escaping, parameterized queries, Argon2id passwords, secure session cookies, non-root container, SHA256-pinned images, CORS disabled in prod, good test coverage.

Add bot label to issues you want automated.

## Security Audit Complete — Issues Created Created 7 tracking issues from the [audit findings above](https://git.eeqj.de/sneak/webhooker/issues/33#issuecomment-10915): **Blockers (must-fix before internet exposure):** - [#34](https://git.eeqj.de/sneak/webhooker/issues/34) — Add production security headers middleware - [#35](https://git.eeqj.de/sneak/webhooker/issues/35) — Implement CSRF protection for all forms - [#36](https://git.eeqj.de/sneak/webhooker/issues/36) — Add SSRF prevention for HTTP delivery targets - [#37](https://git.eeqj.de/sneak/webhooker/issues/37) — Add rate limiting on login endpoint - [#38](https://git.eeqj.de/sneak/webhooker/issues/38) — Fix session fixation: regenerate session on login **Should-fix:** - [#39](https://git.eeqj.de/sneak/webhooker/issues/39) — Add request body size limits on form endpoints - [#40](https://git.eeqj.de/sneak/webhooker/issues/40) — Fix admin password bootstrap logging **Verdict: NO-GO for internet-facing 1.0.0.** 5 blockers must be resolved first. What's solid: template escaping, parameterized queries, Argon2id passwords, secure session cookies, non-root container, SHA256-pinned images, CORS disabled in prod, good test coverage. Add `bot` label to issues you want automated.

sneak commented 2026-03-05 11:49:02 +01:00

Author

Owner

Copy Link

#40 is closed bc its intentional. fix all the others. add the bot label and do it.

#40 is closed bc its intentional. fix all the others. add the bot label and do it.

clawbot referenced this issue 2026-08-07 13:10:51 +02:00

Wrap the /user/{username} route in RequireAuth middleware #60

clawbot referenced this issue 2026-08-07 13:10:55 +02:00

Set Cache-Control: no-store on authenticated pages #61

clawbot referenced this issue 2026-08-07 13:10:59 +02:00

Reconcile HTTP WriteTimeout with the request middleware timeout #62

clawbot referenced this issue 2026-08-07 13:11:03 +02:00

Enforce per-webhook event retention (RetentionDays reaper) #63

clawbot referenced this issue 2026-08-07 13:11:08 +02:00

Rate-limit the public webhook receiver endpoint #64

clawbot referenced this issue 2026-08-07 13:11:13 +02:00

Add an admin password change flow in the web UI #65

clawbot referenced this issue 2026-08-07 13:11:16 +02:00

Add inactivity-based session timeout #66

clawbot referenced this issue 2026-08-07 13:11:19 +02:00

Optional inbound webhook HMAC signature verification #67

clawbot commented 2026-08-07 13:12:23 +02:00

Collaborator

Copy Link

1.0 readiness — re-audit (2026-08-07)

Re-audited origin/main against the March security audit above. The five blockers and the should-fix items you asked to fix are all done and verified in the current code, so the March NO-GO no longer stands. What remains for 1.0 is a tight, fully-tracked set, collected under the new 1.0.0 milestone.

Verified resolved since March

• Security headers — SecurityHeaders() sets HSTS, X-Content-Type-Options: nosniff, X-Frame-Options: DENY, CSP, Referrer-Policy, Permissions-Policy, applied globally (#34)
• CSRF — CSRF() middleware on every state-changing route group, with tests (#35)
• SSRF — dedicated internal/delivery/ssrf.go plus tests (#36)
• Login rate limiting — LoginRateLimit() on the login routes (#37)
• Session fixation — session.Regenerate() is called on login in auth.go (#38)
• Form body-size limits — MaxBodySize() on all form route groups (#39)
• Also confirmed: receiver is POST-only with a 1 MB body cap, non-root container, SHA-pinned base images, prod CORS is a no-op, Argon2id hashing, parameterized queries, html/template escaping, and real test coverage across the security-critical packages.

Still required for 1.0 — 1.0.0 milestone

Each is small and carries a definition of done:

• #60 — wrap /user/{username} in RequireAuth (today it is handler-only)
• #61 — Cache-Control: no-store on authenticated pages
• #62 — reconcile the 10s WriteTimeout against the 60s request-timeout middleware (the 10s deadline currently wins, so the 60s timeout is dead)
• #63 — enforce RetentionDays (a reaper; the field exists but per-webhook DBs currently grow unbounded)
• #65 — admin password-change flow (no way to rotate the bootstrap password today)

Please confirm in-or-out for 1.0 (3 items)

• #64 — rate-limit the public /webhook/{uuid} receiver. Recommend IN: it is the one unauthenticated, internet-facing surface and pairs with #63 for growth/abuse control.
• #43 — your database-target archiving feature. Specced but unbuilt. Your call whether it blocks 1.0 or ships immediately after.
• #57 — web-UI cleanup. Currently a goals list; to execute it well I need either your specific pain points or a go-ahead to draft a concrete proposal first, which I would then split into sub-issues.

Post-1.0 backlog (filed, not blocking)

• #66 — inactivity-based session timeout
• #67 — optional inbound HMAC signature verification

Housekeeping

• #56 (move schema_migrations into 000.sql) — not applicable: webhooker has no schema_migrations table at all (pure GORM AutoMigrate). Recommend closing; it has been assigned to you since July with this same finding.

Decisions for you

1. Confirm 1.0 scope — the milestone as-is, plus your in/out calls on #64, #43, and #57.
2. Go/no-go — once the milestone clears, are you happy to tag 1.0.0 and expose it, or is there anything else you want on the bar?
3. Automation — want me to drive these under the bot label as before, or hand-pick which ones to automate?

Assigning to you for the scope and go/no-go call.

## 1.0 readiness — re-audit (2026-08-07) Re-audited `origin/main` against the March security audit above. The five blockers and the should-fix items you asked to fix are all done and verified in the current code, so the March NO-GO no longer stands. What remains for 1.0 is a tight, fully-tracked set, collected under the new `1.0.0` milestone. ### Verified resolved since March - Security headers — `SecurityHeaders()` sets HSTS, `X-Content-Type-Options: nosniff`, `X-Frame-Options: DENY`, CSP, `Referrer-Policy`, `Permissions-Policy`, applied globally (#34) - CSRF — `CSRF()` middleware on every state-changing route group, with tests (#35) - SSRF — dedicated `internal/delivery/ssrf.go` plus tests (#36) - Login rate limiting — `LoginRateLimit()` on the login routes (#37) - Session fixation — `session.Regenerate()` is called on login in `auth.go` (#38) - Form body-size limits — `MaxBodySize()` on all form route groups (#39) - Also confirmed: receiver is POST-only with a 1 MB body cap, non-root container, SHA-pinned base images, prod CORS is a no-op, Argon2id hashing, parameterized queries, `html/template` escaping, and real test coverage across the security-critical packages. ### Still required for 1.0 — `1.0.0` milestone Each is small and carries a definition of done: - #60 — wrap `/user/{username}` in `RequireAuth` (today it is handler-only) - #61 — `Cache-Control: no-store` on authenticated pages - #62 — reconcile the 10s `WriteTimeout` against the 60s request-timeout middleware (the 10s deadline currently wins, so the 60s timeout is dead) - #63 — enforce `RetentionDays` (a reaper; the field exists but per-webhook DBs currently grow unbounded) - #65 — admin password-change flow (no way to rotate the bootstrap password today) ### Please confirm in-or-out for 1.0 (3 items) - #64 — rate-limit the public `/webhook/{uuid}` receiver. Recommend IN: it is the one unauthenticated, internet-facing surface and pairs with #63 for growth/abuse control. - #43 — your database-target archiving feature. Specced but unbuilt. Your call whether it blocks 1.0 or ships immediately after. - #57 — web-UI cleanup. Currently a goals list; to execute it well I need either your specific pain points or a go-ahead to draft a concrete proposal first, which I would then split into sub-issues. ### Post-1.0 backlog (filed, not blocking) - #66 — inactivity-based session timeout - #67 — optional inbound HMAC signature verification ### Housekeeping - #56 (move `schema_migrations` into `000.sql`) — not applicable: webhooker has no `schema_migrations` table at all (pure GORM `AutoMigrate`). Recommend closing; it has been assigned to you since July with this same finding. ### Decisions for you 1. Confirm 1.0 scope — the milestone as-is, plus your in/out calls on #64, #43, and #57. 2. Go/no-go — once the milestone clears, are you happy to tag `1.0.0` and expose it, or is there anything else you want on the bar? 3. Automation — want me to drive these under the `bot` label as before, or hand-pick which ones to automate? Assigning to you for the scope and go/no-go call.

sneak was assigned by clawbot 2026-08-07 13:12:24 +02:00

clawbot referenced this issue 2026-08-07 13:16:06 +02:00

Validate Slack target URLs at creation time (SSRF parity with HTTP targets) #68

clawbot referenced this issue 2026-08-07 13:16:10 +02:00

Keep the SSRF-safe Transport in clientForConfig when a per-target timeout is set #69

clawbot referenced this issue 2026-08-07 13:16:13 +02:00

Implement the log delivery target #70

clawbot referenced this issue 2026-08-07 13:16:27 +02:00

Clean up and clarify the web UI #57

clawbot commented 2026-08-07 13:16:34 +02:00

Collaborator

Copy Link

Follow-up: a deeper build-level pass finished after the comment above (assessed origin/main @ 2cc8723 in a throwaway worktree; make check and docker build . are both GREEN — the CI-pinned golangci-lint v2.11.3 stays clean, host v2.12.1 shows 9 pre-existing goconst findings only).

It surfaced three more evidence-backed items, now filed and added to the 1.0.0 milestone:

• #68 — buildSlackTargetConfig skips ValidateTargetURL, so Slack target URLs are only guarded at request time, not at creation (SSRF parity gap).
• #69 — clientForConfig() drops the SSRF-safe Transport when a per-target timeout is set; latent today (UI can't set one) but a real bypass to close.
• #70 — the database and log target types are selectable in the UI but their handlers are no-ops (deliverDatabase() just marks the delivery delivered — the design rejected in #43). 1.0 should not offer target types that do nothing.

I also concretized #57 with the specific UI problems found (terminology split, placeholder Profile settings, raw JSON config rendered in the UI, dead target options, misleading retention copy).

Net: no new hard blocker beyond the milestone, and the build is green. The scope and go/no-go decisions in my previous comment still stand — the milestone is now the complete 1.0.0 picture.

Follow-up: a deeper build-level pass finished after the comment above (assessed `origin/main` @ `2cc8723` in a throwaway worktree; `make check` and `docker build .` are both GREEN — the CI-pinned `golangci-lint` v2.11.3 stays clean, host v2.12.1 shows 9 pre-existing `goconst` findings only). It surfaced three more evidence-backed items, now filed and added to the `1.0.0` milestone: - #68 — `buildSlackTargetConfig` skips `ValidateTargetURL`, so Slack target URLs are only guarded at request time, not at creation (SSRF parity gap). - #69 — `clientForConfig()` drops the SSRF-safe Transport when a per-target timeout is set; latent today (UI can't set one) but a real bypass to close. - #70 — the `database` and `log` target types are selectable in the UI but their handlers are no-ops (`deliverDatabase()` just marks the delivery delivered — the design rejected in #43). 1.0 should not offer target types that do nothing. I also concretized #57 with the specific UI problems found (terminology split, placeholder Profile settings, raw JSON config rendered in the UI, dead target options, misleading retention copy). Net: no new hard blocker beyond the milestone, and the build is green. The scope and go/no-go decisions in my previous comment still stand — the milestone is now the complete `1.0.0` picture.

clawbot commented 2026-08-07 13:20:25 +02:00

Collaborator

Copy Link

Correction per your decision (2026-08-07): the database and log delivery targets are REQUIRED for 1.0, so nothing gets hidden. #70 is reframed from "hide the no-op options" to "implement the log target"; database archiving remains #43. The 1.0.0 milestone is otherwise unchanged.

Correction per your decision (2026-08-07): the `database` and `log` delivery targets are REQUIRED for 1.0, so nothing gets hidden. #70 is reframed from "hide the no-op options" to "implement the `log` target"; database archiving remains #43. The `1.0.0` milestone is otherwise unchanged.

clawbot referenced this issue 2026-08-07 15:05:53 +02:00

Refactor delivery targets to a Target interface #77

clawbot commented 2026-08-07 18:07:44 +02:00

Collaborator

Copy Link

Session handoff — 1.0 status (2026-08-07)

Where webhooker 1.0 stands, for the next session (which will only have the tracker, not the chat history).

Merged to main this session (all independently reviewed)

• #60 (/user RequireAuth), #62 (WriteTimeout vs middleware timeout), #68 (Slack create-time SSRF validation), #69 (SSRF-safe client on per-target timeout) — first hardening wave.
• #61 (Cache-Control: no-store on authenticated pages), #63 (per-webhook event retention reaper), #77 (delivery Target-interface refactor). #77 has a post-merge independent review confirming no regressions; the log target now logs full body+headers, and Slack gained MaxRetries-gated retries.

Open PRs

• PR #83 (#65 admin password change) — reviewed PASS, merge-ready, assigned @sneak. Just needs your merge.
• PR #84 (#43 database archiving) — in REWORK (assigned clawbot). The archive mechanics (separate archive-{webhookID}.db, close/reopen debounce, auto-recreate, optional expiry, prune-on-open) are solid and reviewed, but ONE required fix is pending and NOT yet applied (the rework was interrupted): a failed archive must record a Failed delivery rather than silently Delivered. Full instructions are in the last two comments on PR #84.

1.0.0 milestone — still open

• #43 — in-flight via PR #84 (finish the rework, re-review, then merge).
• #57 — web UI cleanup. Concretized into specific problems in a comment on the issue, but it needs your direction (or a go-ahead to draft a UI proposal first). Not started.
• #64 — rate-limit the public webhook receiver. Not started; full implementation instructions are on the issue; ready to pick up (touches routes.go + ratelimit.go + config.go).
• #65 — closes automatically when PR #83 merges.

Follow-ups filed (triage for scope)

• #79 — RetentionDays can't be set to 0 (retain forever) via the normal create path.
• #80 — config env helpers (envInt, etc.) should fail loudly on set-but-unparseable values (extends the fix already merged for the duration parser).
• #82 — recovery skips orphaned retrying deliveries whose target type was changed to a non-retry type (narrow edge case).
• #85 — design question: should targets own their recovery/sweep loop, not just backoff (you raised this on #77).
• #66 (idle session timeout), #67 (inbound HMAC verification) — post-1.0 backlog.

Housekeeping

• #56 (move schema_migrations into 000.sql) — not applicable: webhooker uses GORM AutoMigrate and has no schema_migrations table. Recommend closing.

Decisions still needing you

1. 1.0 scope + go/no-go: confirm which of #64, #43, #57 are in 1.0 (my read: #64 and #43 in; #57 quality pass).
2. Merge PR #83; let the next agent finish PR #84.
3. #57 direction, close #56, and the relative priority of #79/#80/#82/#85.

Working method for the next agent

One small unit at a time: a Gitea issue with a definition of done → a subagent (rooted at the clone, isolated git worktree from origin/main) implements it and opens a PR, validated by docker build . (host Go is 1.25 but go.mod needs 1.26, so Docker is the authoritative gate — make check won't run on the host) → an independent adversarial review → rework driven via PR comments until clean → assign @sneak to merge. Parallel units must touch non-overlapping files. CI pins golangci-lint to an older version than a current host may have, so host-only goconst warnings in untouched files are pre-existing, not blockers.

## Session handoff — 1.0 status (2026-08-07) Where webhooker 1.0 stands, for the next session (which will only have the tracker, not the chat history). ### Merged to main this session (all independently reviewed) - #60 (`/user` RequireAuth), #62 (WriteTimeout vs middleware timeout), #68 (Slack create-time SSRF validation), #69 (SSRF-safe client on per-target timeout) — first hardening wave. - #61 (Cache-Control: no-store on authenticated pages), #63 (per-webhook event retention reaper), #77 (delivery Target-interface refactor). #77 has a post-merge independent review confirming no regressions; the log target now logs full body+headers, and Slack gained `MaxRetries`-gated retries. ### Open PRs - PR #83 (#65 admin password change) — reviewed PASS, merge-ready, assigned @sneak. Just needs your merge. - PR #84 (#43 database archiving) — in REWORK (assigned `clawbot`). The archive mechanics (separate `archive-{webhookID}.db`, close/reopen debounce, auto-recreate, optional expiry, prune-on-open) are solid and reviewed, but ONE required fix is pending and NOT yet applied (the rework was interrupted): a failed archive must record a `Failed` delivery rather than silently `Delivered`. Full instructions are in the last two comments on PR #84. ### 1.0.0 milestone — still open - #43 — in-flight via PR #84 (finish the rework, re-review, then merge). - #57 — web UI cleanup. Concretized into specific problems in a comment on the issue, but it needs your direction (or a go-ahead to draft a UI proposal first). Not started. - #64 — rate-limit the public webhook receiver. Not started; full implementation instructions are on the issue; ready to pick up (touches `routes.go` + `ratelimit.go` + `config.go`). - #65 — closes automatically when PR #83 merges. ### Follow-ups filed (triage for scope) - #79 — `RetentionDays` can't be set to 0 (retain forever) via the normal create path. - #80 — config env helpers (`envInt`, etc.) should fail loudly on set-but-unparseable values (extends the fix already merged for the duration parser). - #82 — recovery skips orphaned `retrying` deliveries whose target type was changed to a non-retry type (narrow edge case). - #85 — design question: should targets own their recovery/sweep loop, not just backoff (you raised this on #77). - #66 (idle session timeout), #67 (inbound HMAC verification) — post-1.0 backlog. ### Housekeeping - #56 (move `schema_migrations` into `000.sql`) — not applicable: webhooker uses GORM AutoMigrate and has no `schema_migrations` table. Recommend closing. ### Decisions still needing you 1. 1.0 scope + go/no-go: confirm which of #64, #43, #57 are in 1.0 (my read: #64 and #43 in; #57 quality pass). 2. Merge PR #83; let the next agent finish PR #84. 3. #57 direction, close #56, and the relative priority of #79/#80/#82/#85. ### Working method for the next agent One small unit at a time: a Gitea issue with a definition of done → a subagent (rooted at the clone, isolated git worktree from `origin/main`) implements it and opens a PR, validated by `docker build .` (host Go is 1.25 but `go.mod` needs 1.26, so Docker is the authoritative gate — `make check` won't run on the host) → an independent adversarial review → rework driven via PR comments until clean → assign @sneak to merge. Parallel units must touch non-overlapping files. CI pins `golangci-lint` to an older version than a current host may have, so host-only `goconst` warnings in untouched files are pre-existing, not blockers.

clawbot referenced this issue 2026-08-09 07:46:12 +02:00

Add inactivity-based session timeout #66

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

main

next

issue-141-release-readiness

issue-139-aggregate-receiver-ratelimit

issue-119-real-ci-gate

issue-108-session-codec-maxage

issue-102-stop-hook-context

issue-123-render-buffer

issue-115-mask-http-destination-url

feat/receiver-rate-limit

issue-70-log-target

No results found.

Labels

Clear labels

merge-ready

needs-checks

needs-rebase

needs-review

needs-rework

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

clawbot sneak

No Assignees sneak

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/webhooker#33