92f3a016e1aefca458e2d950de11259f88bd8d7a
2 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| 92f3a016e1 |
Correct release-blocking README and startup-warning inaccuracies (closes #151)
All checks were successful
check / check (push) Successful in 2m59s
The empty-TRUSTED_PROXIES warning was gated on IsProd(), but WEBHOOKER_ENVIRONMENT defaults to dev, so an internet-exposed deployment whose operator never set it got no warning at all — the exact operator error the warning exists to catch. It now fires whenever the list is empty, in any environment, and its text is accurate both behind a reverse proxy (shared buckets, remotely deniable admin login) and with nothing in front of the process (harmless). The startup configuration summary also now logs sessionIdleTimeout, the one value where a valid setting silently disables a security control. The README documented a two-stage Docker build on golang:1.24 running "make check" (the tree has three stages: a golangci-lint lint stage running fmt-check and lint, a golang:1.26.1-bookworm builder running test and build, then the Alpine runtime), advertised the public receiver as accepting all methods (it answers 405 to everything but POST), claimed unqualified per-IP login rate limiting, and left the session-expiry prose orphaned inside the trusted-proxy subsection. The rest of the README was swept against the code rather than only the reported lines: every documented route checked method-by-method against internal/server/routes.go (adding the password-change, entrypoint and target routes that were missing), every environment variable checked against internal/config/config.go (MAINTENANCE_MODE serves no maintenance page — it only sets a healthcheck field), the fx wiring, package tree, prerequisites and dev commands brought back in line with the tree. Statements the sweep had waved through, each re-derived from the code this time: - Circuit breakers are not HTTP-only. target_slack.go embeds httpCore and hands its own MaxRetries to the same retry path, so a slack target with max_retries > 0 gets a breaker on the same defaults. - No WAL exists. Both DSNs are file:%s?cache=shared&mode=rwc and no journal_mode pragma is issued anywhere, so every database runs on SQLite's rollback journal. - Slack config is keyed webhookUrl, not webhook_url; the underscored spelling is only the error message. An operator copying the README wrote config that was silently ignored. - Setting carries no BaseModel, so neither the common-field list nor "soft deletes on all entities" held for it. - DeliveryTask names no type; it is delivery.Task. - script/lint runs golangci-lint on the host. Only script/cibuild and script/docker involve Docker; #109 tracks changing that, and until it lands the README says what the tree does. - An entrypoint's path column holds a bare UUID. The /webhook/ prefix is route only and never stored. - max_queue_size is stored and displayed but consulted by nothing; queue depth is the two fixed 10,000-entry channels. - Inline event bodies are cut at "< 16 KiB", not "≤". - retention_days 0 belongs to the retain-forever band — BeforeSave rewrites it to the sentinel — not the finite one, whose floor is 1. A negative value is a 400 in parseRetentionDays. - The receiver's per-client limit keys on the request path (httprate.KeyByEndpoint), not on the entrypoint; the paragraph eight lines below already said so and the two contradicted each other. - Shutdown goes through lifecycle.WaitForShutdown, which is bounded by fx's stop context and can return with goroutines still running, rather than a bare WaitGroup.Wait(). - Nine of the Makefile's fifteen targets shim script/; build, run, dev, deps, clean and css are inline. - Nine entities are documented and nine model files exist, not eight. - The metrics middleware is only registered when METRICS_USERNAME is set, so an over-limit request is not always counted. Two gaps closed while checking: the target-type list omitted slack entirely, and the package tree omitted event_log_view.go and the three testing.go files, which are ordinary compiled sources exporting NewTestDatabase and NewForTest rather than _test.go scaffolding. TestStaticServesEveryMethod settles what /s/* actually answers: chi's Mount registers every method and http.FileServer special-cases only HEAD, so POST, PUT and DELETE to an asset are served the file. The README claimed GET and HEAD. TODO.md drops the unsupported half of its CI claim, keeping the cache-defeated container runs, and splits the landed password change away from the unimplemented reset flow. Rebasing onto current next moved the tree under three of these sections, so they were re-derived against the rebased tree rather than the one the branch was cut from: alpine.min.js is no longer committed but fetched and hash-verified by script/fetch-assets, so the package tree entry for it was wrong and vendor.sha256 was absent; the builder stage runs that script before make test; the Makefile is now ten shims out of sixteen targets, and fmt-check was missing from the development command list. The Quick Start was also wrong in a way that costs a new contributor a red build: it said make deps, which only runs go mod download and tidy, and make check then fails two tests on the absent asset. It now says make bootstrap, which fetches it. |
|||
| d51cd0fd29 |
Enforce the body size limit before CSRF parses the form (closes #90)
Some checks failed
check / check (push) Superseded by a newer commit; never tested
CSRF ran before MaxBodySize, so the CSRF middleware parsed the form body
before any cap applied and an oversized request was read in full before
being rejected. MaxBodySize is now the first middleware in all four route
groups that parse forms, ahead of CSRF and RequireAuth.
An oversize request therefore gets 413 without the handler running and
without state changing, including the password-change route.
Note the ordering trade: an unauthenticated client now receives 413 rather
than an auth redirect on /user/{username}/password.
|
