Keep the entrypoint UUID out of webhooker's own logs (closes #531)
check / check (push) Waiting to run
check / check (push) Waiting to run
On the receiver route the access log and the per-entrypoint 429 line name the route pattern whatever the status. A request chi never routed, such as a dev-mode CORS preflight to the receiver, logs (unmatched) whatever the status. The received line names the entrypoint and webhook IDs, and the unknown-entrypoint line logs no path. The pattern is middleware.ReceiverRoute, which routes.go registers the receiver with. The README says the proxy's access log holds the UUID and must be protected like the URL, and its access-log, rate-limit, Sentry and log-size passages now match the code. Model: opus-5-5
This commit is contained in:
@@ -802,7 +802,11 @@ comes up, serves pages, and is broken in a way nothing reports.
|
||||
proxy's log names it regardless. nginx's default `combined` format already
|
||||
logs `$remote_addr`; do not replace it with one that drops the client
|
||||
address, and retain those logs as long as you would want to answer a question
|
||||
about traffic.
|
||||
about traffic. That log also records the full request path, so it holds the
|
||||
entrypoint UUID of every event a sender posts. webhooker's own log never
|
||||
does. Protect the proxy's log as you would the entrypoint URLs themselves:
|
||||
anyone who can read it can post events to every entrypoint in it (see
|
||||
[The entrypoint URL is the authentication secret](#the-entrypoint-url-is-the-authentication-secret)).
|
||||
|
||||
### nginx
|
||||
|
||||
@@ -828,7 +832,9 @@ server {
|
||||
client_max_body_size 1m;
|
||||
|
||||
# $remote_addr is the client. webhooker's own log names it, as
|
||||
# clientIP, only while TRUSTED_PROXIES covers this proxy.
|
||||
# clientIP, only while TRUSTED_PROXIES covers this proxy. The
|
||||
# request path in this log holds the entrypoint UUID, so protect
|
||||
# the file like the entrypoint URL itself.
|
||||
access_log /var/log/nginx/webhooker.access.log combined;
|
||||
|
||||
location / {
|
||||
@@ -1160,7 +1166,12 @@ What that means for an operator:
|
||||
|
||||
- **The URL is a capability, so treat it as a secret.** Keep it out of logs,
|
||||
ticket bodies, chat messages and screenshots. Anyone who reads it anywhere can
|
||||
post events as that sender.
|
||||
post events as that sender. webhooker's own log lines never carry the UUID:
|
||||
they name a request to the receiver by the route pattern `/h/{uuid}`, and the
|
||||
`webhook request received` line names the entrypoint by its `entrypoint_id`.
|
||||
The reverse proxy's access log does carry the UUID, in the request path, so it
|
||||
must be protected like the URL itself (see
|
||||
[Deployment behind a reverse proxy](#deployment-behind-a-reverse-proxy)).
|
||||
- **Rotating means minting a new entrypoint, not changing a key.** There is no
|
||||
way to rotate the UUID in place. To retire one, delete the entrypoint (or
|
||||
deactivate it, which answers `410`) and create a new one, then point the
|
||||
@@ -2422,27 +2433,32 @@ log volume it caps rather than eliminates. A path that names no entrypoint is
|
||||
recorded by the handler at `DEBUG`, and the aggregate limiter logs its own
|
||||
rejections at `DEBUG` and without the path, so neither appears at all under the
|
||||
default level. The per-entrypoint limiter is the loud one: it logs every
|
||||
rejection at `WARN` with the request path, which on this route is
|
||||
attacker-controlled text. A client hammering a single invented path is served
|
||||
rejection at `WARN`, naming the route pattern `/h/{uuid}` rather than the
|
||||
request path. A client hammering a single invented path is served
|
||||
`RECEIVER_RATE_LIMIT` requests and has the rest of its aggregate budget rejected
|
||||
there, so the aggregate limit is what bounds the _number_ of those `WARN` lines
|
||||
— to under ten times `RECEIVER_RATE_LIMIT` per minute per client IP, 1080 at the
|
||||
defaults, where before it there was no bound at all. Their _width_ is bounded by
|
||||
the field budgets below, the same ones the access log spends. The access log is
|
||||
bounded by neither limit: every request is recorded once at `INFO`, served or
|
||||
rejected alike.
|
||||
defaults, where before it there was no bound at all. The access log is bounded
|
||||
by neither limit: every request is recorded once at `INFO`, served or rejected
|
||||
alike.
|
||||
|
||||
What the access log does bound is the _content_ of those lines. A 3xx or 4xx
|
||||
response logs the chi route pattern — `/h/{uuid}`, `/user/{username}//`, or the
|
||||
literal `(unmatched)` when the request hit no route at all — in place of the
|
||||
concrete URL. Those are the outcomes an unauthenticated client can drive for
|
||||
free: 404 and 429 on any invented receiver path, a login redirect on any
|
||||
invented profile path. Logging the URL there would let a flood write text of its
|
||||
own choosing, at a length of its own choosing, into the log. 2xx and 5xx
|
||||
responses keep the concrete path — a success resolved against a static route or
|
||||
against the operator's own data (on the receiver, against a stored entrypoint
|
||||
UUID), and a 5xx is a bug in this service, where the exact path is the evidence
|
||||
and no client can provoke one at will.
|
||||
What the access log does bound is the _content_ of those lines. On the receiver
|
||||
route it logs the route pattern `/h/{uuid}` whatever the status, because the
|
||||
concrete path carries the entrypoint UUID, which is the entrypoint's credential
|
||||
(see
|
||||
[The entrypoint URL is the authentication secret](#the-entrypoint-url-is-the-authentication-secret)).
|
||||
A request chi never routed, because it matched no route or because a middleware
|
||||
answered it first, logs the literal `(unmatched)` whatever the status. In the
|
||||
`dev` environment the CORS middleware answers a preflight that way, so a
|
||||
preflight to the receiver does not log its UUID either. On every other route, a
|
||||
3xx or 4xx response logs the chi route pattern, such as `/user/{username}//`, in
|
||||
place of the concrete URL. Those are the outcomes an unauthenticated client can
|
||||
drive for free: a 404 on any invented path, a login redirect on any invented
|
||||
profile path. Logging the URL there would let a flood write text of its own
|
||||
choosing, at a length of its own choosing, into the log. 2xx and 5xx responses
|
||||
keep the concrete path — a success resolved against a static route or against
|
||||
the operator's own data, and a 5xx is a bug in this service, where the exact
|
||||
path is the evidence and no client can provoke one at will.
|
||||
|
||||
The query string is never logged; it is replaced by the fixed marker
|
||||
`?(redacted)`. It is client-chosen on every route, and
|
||||
@@ -2468,9 +2484,10 @@ The same hook rewrites the request URL. The SDK builds it as
|
||||
`scheme://host/path` from the concrete path, which on the receiver route is
|
||||
`/h/<uuid>` in full — and that UUID is a write capability, not an identifier:
|
||||
anyone holding it can post events this service accepts and its targets then
|
||||
deliver. A tracker has its own retention, access control and deletion policy, so
|
||||
the rule the access log follows above does not carry across that boundary. What
|
||||
is sent is the chi route pattern instead: `http://host/h/{uuid}`.
|
||||
deliver. What is sent is the chi route pattern instead: `http://host/h/{uuid}`.
|
||||
A tracker has its own retention, access control and deletion policy, so the
|
||||
pattern is sent on every route, including the 2xx and 5xx responses outside the
|
||||
receiver for which the access log keeps the concrete path.
|
||||
|
||||
The scheme and the host are kept, and everything else in the URL is discarded
|
||||
rather than edited, so a future SDK version that starts appending a query string
|
||||
@@ -2599,9 +2616,7 @@ than it:
|
||||
| ------------------------------------------ | ------- | ------------------- | ------------------------------------------ |
|
||||
| `request body exceeds limit` (413) | `WARN` | path, method | yes — `MaxBodySize` precedes `RequireAuth` |
|
||||
| `csrf: token validation failed` (403) | `WARN` | path, method | yes — `CSRF` precedes `RequireAuth` |
|
||||
| `... rate limit exceeded` (429) | `WARN` | path | yes, on the receiver |
|
||||
| `auth middleware: unauthenticated request` | `DEBUG` | path, method | yes, by definition |
|
||||
| `entrypoint not found` | `DEBUG` | entrypoint UUID | yes, on the receiver |
|
||||
| `user not found` / `invalid password` | `DEBUG` | username | yes, on the sign-in form |
|
||||
| `login failure limit exceeded` (429) | `WARN` | path | yes, on the sign-in form |
|
||||
| `password verification capacity exhausted` | `WARN` | path | yes, on the sign-in form |
|
||||
@@ -2629,13 +2644,12 @@ wider of the two handlers spends more on than the client spent sending them.
|
||||
Every case holds each line to the 2,560-byte ceiling. That per-line ceiling is
|
||||
what the figure above states, and every row establishes it.
|
||||
|
||||
Three of the sites go further and bound the whole flood's output — the total
|
||||
bytes a run of distinct invented values wrote, which is the shape an operator
|
||||
sizing storage cares about. They are `request body exceeds limit`
|
||||
(`TestMaxBodySize_FloodOfOversizePathsDoesNotGrowTheLog`),
|
||||
`entrypoint not found` and `user not found` (the last two through
|
||||
`assertBoundedFlood`). The other rows carry no aggregate assertion; the per-line
|
||||
ceiling is what they establish.
|
||||
Two of the sites go further and bound the whole flood's output — the total bytes
|
||||
a run of distinct invented values wrote, which is the shape an operator sizing
|
||||
storage cares about. They are `request body exceeds limit`
|
||||
(`TestMaxBodySize_FloodOfOversizePathsDoesNotGrowTheLog`) and `user not found`
|
||||
(through `assertBoundedFlood`). The other rows carry no aggregate assertion; the
|
||||
per-line ceiling is what they establish.
|
||||
|
||||
`internal/logfield/logfield_test.go` measures the per-rune charge against what
|
||||
the handlers really emit, over roughly 3,000 code points on each, so an
|
||||
@@ -2653,7 +2667,7 @@ everything else, so its lines take the level the operator set and the handler
|
||||
`internal/logger` selected, and every value it emits is spent through the same
|
||||
`internal/logfield` budget. A record-not-found is not logged as an error: it is
|
||||
the expected outcome on both of those paths, and each handler already records
|
||||
its own miss at `DEBUG` — bounded, per the table above — without the SQL. Slow
|
||||
its own miss at `DEBUG`, without the SQL and within the ceiling above. Slow
|
||||
statements are kept, at `WARN`, above the same 200 ms threshold GORM used and
|
||||
with the statement bounded, because that report is the one thing GORM's logger
|
||||
gave an operator that nothing else here does. The adapter orders its cases
|
||||
|
||||
Reference in New Issue
Block a user