Compare commits
4 Commits
fad97445ca
...
c7bf648526
| Author | SHA1 | Date | |
|---|---|---|---|
| c7bf648526 | |||
| 41ff16a817 | |||
| 5888d14438 | |||
| 7702f38168 |
@@ -13,39 +13,19 @@ jobs:
|
|||||||
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 2024-10-23
|
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 2024-10-23
|
||||||
with:
|
with:
|
||||||
# The fingerprint step below needs history to find the last commit
|
# The fingerprint step below needs history to find the last commit
|
||||||
# that touched the Docker build context.
|
# that touched the Docker build context, and the superseded-status
|
||||||
|
# step needs it to walk ancestors (it aborts on a shallow clone).
|
||||||
fetch-depth: 0
|
fetch-depth: 0
|
||||||
|
|
||||||
- name: Neutralize superseded run statuses
|
- name: Mark superseded run statuses
|
||||||
# Gitea cancels the in-flight run when another commit is pushed to the
|
# Gitea cancels the in-flight run when another commit is pushed to the
|
||||||
# same branch and records the cancellation as `failure`, so a commit
|
# same branch and records the cancellation as `failure`, so a commit
|
||||||
# that was never tested reads red. The cancellation is unconditional
|
# that was never tested reads as a test result. The script rewrites
|
||||||
# server-side for push events and cannot be disabled from a workflow
|
# those statuses to say what happened. See its header for why the
|
||||||
# file, so the superseding run rewrites those statuses to `skipped`.
|
# state stays `failure` and not `skipped`.
|
||||||
# Only the exact cancellation status is touched; a real failure is
|
|
||||||
# left alone.
|
|
||||||
env:
|
env:
|
||||||
GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
|
GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
|
||||||
run: |
|
run: script/ci-mark-superseded
|
||||||
set -eu
|
|
||||||
api="${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}"
|
|
||||||
ctx='check / check (push)'
|
|
||||||
for sha in $(git rev-list --max-count=20 "${GITHUB_SHA}^" || true); do
|
|
||||||
latest="$(curl -sf "${api}/commits/${sha}/status" | jq -r \
|
|
||||||
--arg c "$ctx" \
|
|
||||||
'[.statuses[] | select(.context == $c)][0] // empty
|
|
||||||
| "\(.status)|\(.description)"')" || continue
|
|
||||||
[ "$latest" = 'failure|Has been cancelled' ] || continue
|
|
||||||
curl -sf -X POST "${api}/statuses/${sha}" \
|
|
||||||
-H "Authorization: token ${GITEA_TOKEN}" \
|
|
||||||
-H 'Content-Type: application/json' \
|
|
||||||
-d "$(jq -nc --arg c "$ctx" '{
|
|
||||||
context: $c,
|
|
||||||
state: "skipped",
|
|
||||||
description: "Superseded by a newer commit; never tested"
|
|
||||||
}')" >/dev/null
|
|
||||||
echo "neutralized superseded status on ${sha}"
|
|
||||||
done
|
|
||||||
|
|
||||||
- name: Fingerprint the build context
|
- name: Fingerprint the build context
|
||||||
# `.dockerignore` keeps docs out of the build context, so a docs-only
|
# `.dockerignore` keeps docs out of the build context, so a docs-only
|
||||||
|
|||||||
@@ -32,7 +32,9 @@ FROM golang:1.26.1-bookworm@sha256:4465644228bc2857a954b092167e12aa59c006a349228
|
|||||||
# Depend on lint stage passing
|
# Depend on lint stage passing
|
||||||
COPY --from=lint /src/go.sum /dev/null
|
COPY --from=lint /src/go.sum /dev/null
|
||||||
|
|
||||||
RUN apt-get update && apt-get install -y --no-install-recommends make curl ca-certificates && rm -rf /var/lib/apt/lists/*
|
# jq is a runtime dependency of script/ci-mark-superseded, which the test
|
||||||
|
# suite executes.
|
||||||
|
RUN apt-get update && apt-get install -y --no-install-recommends make curl ca-certificates jq && rm -rf /var/lib/apt/lists/*
|
||||||
|
|
||||||
WORKDIR /build
|
WORKDIR /build
|
||||||
|
|
||||||
|
|||||||
228
README.md
228
README.md
@@ -284,6 +284,8 @@ are inline commands with no script behind them. We provide:
|
|||||||
- `script/docker` — build the Docker image tagged via `script/projectname`
|
- `script/docker` — build the Docker image tagged via `script/projectname`
|
||||||
- `script/cibuild` — CI entrypoint: `docker build .` (the Dockerfile
|
- `script/cibuild` — CI entrypoint: `docker build .` (the Dockerfile
|
||||||
runs the checks, so a green build implies a green repo)
|
runs the checks, so a green build implies a green repo)
|
||||||
|
- `script/ci-mark-superseded` — CI helper: mark the commits whose run a
|
||||||
|
newer push cancelled (see [CI gate honesty](#ci-gate-honesty))
|
||||||
- `script/precommit` — pre-commit checks (`go mod tidy` guard, then
|
- `script/precommit` — pre-commit checks (`go mod tidy` guard, then
|
||||||
`script/check`)
|
`script/check`)
|
||||||
- `script/install-precommit` — install the git pre-commit hook that
|
- `script/install-precommit` — install the git pre-commit hook that
|
||||||
@@ -997,8 +999,70 @@ requests and has the rest of its aggregate budget rejected there, so
|
|||||||
the aggregate limit is what bounds those `WARN` lines — to under ten
|
the aggregate limit is what bounds those `WARN` lines — to under ten
|
||||||
times `RECEIVER_RATE_LIMIT` per minute per client IP, 1080 at the
|
times `RECEIVER_RATE_LIMIT` per minute per client IP, 1080 at the
|
||||||
defaults, where before it there was no bound at all. The access log is
|
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` with
|
bounded by neither limit: every request is recorded once at `INFO`,
|
||||||
its full URL, served or rejected alike.
|
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 — `/webhook/{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.
|
||||||
|
|
||||||
|
The query string is never logged; it is replaced by the fixed marker
|
||||||
|
`?(redacted)`. It is client-chosen on every route, and
|
||||||
|
`/.well-known/healthcheck` and `/s/*` answer 200 to anyone with no rate
|
||||||
|
limiter in front of them, so a query on a fixed 200 URL would otherwise
|
||||||
|
buy the same amplification as an invented path. Nothing debuggable is
|
||||||
|
lost: `page`, on the authenticated pagination links, is the only query
|
||||||
|
parameter this service reads.
|
||||||
|
|
||||||
|
The remaining client-supplied fields are truncated rather than dropped,
|
||||||
|
each to a fixed budget: 512 bytes for `url`, `useragent` and `referer`,
|
||||||
|
128 for `request_id` (chi passes an inbound `X-Request-Id` header
|
||||||
|
through), and 32 for `method`. A truncated `User-Agent` is still worth
|
||||||
|
reading; an absent one is not. A cut value ends in `[truncated]`, which
|
||||||
|
is charged on top of the budget rather than inside it.
|
||||||
|
|
||||||
|
Each budget is spent in _encoded_ bytes, not in the bytes the client
|
||||||
|
sent. Every rune is charged what the wider of the two log handlers
|
||||||
|
emits for it: two bytes for a quotation mark, a backslash or a tab; six
|
||||||
|
for a non-printable rune below U+10000; ten for one at or above it,
|
||||||
|
which the text handler spells `\UXXXXXXXX`. Go's header parser accepts
|
||||||
|
all of them in a header value, so a budget counted raw would buy a
|
||||||
|
field several times its nominal size — and the line, not the header, is
|
||||||
|
what an operator has to store. Plain ASCII encodes one byte for one, so
|
||||||
|
a real browser's `User-Agent` still fits whole; a value built out of
|
||||||
|
escapes keeps a proportionally shorter prefix, which is the right
|
||||||
|
trade.
|
||||||
|
|
||||||
|
Net: **one `INFO` line per request, of at most 2,560 bytes.** That
|
||||||
|
ceiling is arithmetic, not an observation: 3 × (512 + 11) for `url`,
|
||||||
|
`useragent` and `referer`, plus 128 + 11 for `request_id`, plus 32 + 11
|
||||||
|
for `method`, plus a 336-byte fixed portion (the field names, the
|
||||||
|
punctuation, both timestamps at their longest, an IPv6 `remoteIP` with
|
||||||
|
a zone, the status and the latency) — 2,087 bytes, stated at 2,560 so
|
||||||
|
the figure has headroom. `internal/middleware/accesslog_test.go`
|
||||||
|
asserts it against 8 KB of client-chosen text in the path, in the
|
||||||
|
query, and in each of `User-Agent`, `Referer` and `X-Request-Id`,
|
||||||
|
including cases built from the characters the handlers escape, and
|
||||||
|
against the widest line the service can be made to write: a 5xx that
|
||||||
|
keeps its concrete path while all three header fields are also at their
|
||||||
|
budget. Every case runs through both handlers `internal/logger` can
|
||||||
|
select — the JSON one and the text one it installs on a tty — since the
|
||||||
|
two do not escape alike and the ceiling is quoted unqualified. Measured
|
||||||
|
over a real connection, the widest line is 1,972 bytes.
|
||||||
|
|
||||||
|
Multiply that ceiling by the request rate to size log storage. Note
|
||||||
|
that the rate is not bounded by the limits above on every route:
|
||||||
|
`/.well-known/healthcheck` and `/s/*` sit behind no limiter, so there
|
||||||
|
the multiplier is whatever the deployment will serve.
|
||||||
|
|
||||||
Every limiter here — receiver, login, and password change — identifies
|
Every limiter here — receiver, login, and password change — identifies
|
||||||
the client the same way, through one shared key function: the
|
the client the same way, through one shared key function: the
|
||||||
@@ -1051,33 +1115,79 @@ second administrative path. So the handler inverts the order:
|
|||||||
reachable.
|
reachable.
|
||||||
2. **Failures are counted per (client bucket, submitted username)**,
|
2. **Failures are counted per (client bucket, submitted username)**,
|
||||||
five per minute, after which further _failures_ from that pair are
|
five per minute, after which further _failures_ from that pair are
|
||||||
answered `429` with a `Retry-After`. A successful login clears the
|
answered `429` with a `Retry-After`. That `429` is a label on the
|
||||||
counter, so mistyping a few times and then getting it right leaves
|
response, not a gate in front of the work: the credential check has
|
||||||
you unthrottled. Because the submitted username is
|
already run by the time the counter is consulted, so a throttled
|
||||||
attacker-controlled, at most 1024 username counters and 1024
|
client's guess is still evaluated. See the guessing rate below. A
|
||||||
fallback address counters are tracked; past the first cap failures
|
successful login clears the counter, so mistyping a few times and
|
||||||
fall back to the address counter, and past both they are answered
|
then getting it right leaves you unthrottled. Because the submitted
|
||||||
as throttled without being recorded. Total tracked state is under
|
username is attacker-controlled, at most 1024 username counters and
|
||||||
half a megabyte and does not grow with the number of usernames an
|
1024 fallback address counters are tracked; past the first cap
|
||||||
attacker invents.
|
failures fall back to the address counter, and past both they are
|
||||||
3. **Concurrent password verifications are capped at two.** Verifying
|
answered as throttled without being recorded. Total tracked state
|
||||||
before counting means every login request costs an Argon2id hash,
|
is under half a megabyte and does not grow with the number of
|
||||||
and Argon2id here is 64 MB per hash — two slots is a 128 MB ceiling
|
usernames an attacker invents.
|
||||||
on password hashing. Every endpoint that hashes a password takes a
|
3. **Concurrent password verifications are capped at two, and the
|
||||||
slot, including the password-change endpoint, which holds one
|
queue for them at 64.** Verifying before counting means every login
|
||||||
across both the verification and the new hash. A request that waits
|
request costs an Argon2id hash, and Argon2id here is 64 MB per
|
||||||
five seconds without getting a slot is answered `503 Service
|
hash — two slots is a 128 MB ceiling on password hashing. Every
|
||||||
Unavailable` and no hash is computed for it.
|
endpoint that hashes a password takes a slot, including the
|
||||||
|
password-change endpoint, which holds one across both the
|
||||||
|
verification and the new hash. A request that waits five seconds
|
||||||
|
without getting a slot is answered `503 Service Unavailable` and no
|
||||||
|
hash is computed for it. The wait alone does not bound memory, only
|
||||||
|
how long one request holds some: a waiter reaches the guard with
|
||||||
|
its form already parsed, so it holds up to the 1 MB body cap for as
|
||||||
|
long as it waits, and at flood rates an unbounded queue would be
|
||||||
|
worth gigabytes against a 128 MB hashing budget. So the number of
|
||||||
|
waiters is capped as well, at 64 — 1 MB each against 64 MB of
|
||||||
|
committed queue memory, and shallow enough that two slots can drain
|
||||||
|
a full queue inside the five-second deadline. A request arriving
|
||||||
|
past the cap is shed with `503` immediately instead of joining the
|
||||||
|
queue. Peak commitment for the endpoint is therefore 128 MB of
|
||||||
|
hashing plus about 66 MB of parsed request bodies.
|
||||||
|
|
||||||
An unknown username is verified against a dummy hash rather than
|
An unknown username is verified against a dummy hash rather than
|
||||||
rejected early, so a nonexistent account costs the same time as a real
|
rejected early, so a nonexistent account costs the same time as a real
|
||||||
one and the response cannot be used to enumerate usernames.
|
one and the response cannot be used to enumerate usernames.
|
||||||
|
|
||||||
The residual exposure is bounded and self-clearing: a flood can keep
|
**This raises online guessing throughput by about 300x, and that is
|
||||||
both verification slots busy, so logins queue and some are shed with
|
the trade.** Because the credential check always precedes the counter,
|
||||||
`503` until it stops. That is degraded latency for everyone rather
|
what bounds online brute force is the semaphore, not the failure
|
||||||
than a permanent lockout of the operator, and an operator under one
|
counter. Two slots at the cost of one Argon2id verification is on the
|
||||||
can block the source at the reverse proxy or restart the service.
|
order of **27 guesses per second, about 2.3 million per day**, against
|
||||||
|
5 per minute under the pre-emptive limiter this replaced. Treat that
|
||||||
|
figure as a lower bound rather than a ceiling: it was measured with
|
||||||
|
Go's race detector enabled, so real hardware verifies faster and
|
||||||
|
guesses faster. Choose the admin password to survive millions of
|
||||||
|
online guesses per day — a long random passphrase, not a memorable
|
||||||
|
one. Rate-limiting `POST /pages/login` at the reverse proxy, where the
|
||||||
|
real client address is visible, is the way to put a cheaper bound back
|
||||||
|
on top.
|
||||||
|
|
||||||
|
The residual exposure is a bounded, self-clearing loss of login
|
||||||
|
**availability** — not merely of latency. A flood can keep both
|
||||||
|
verification slots busy, and a request that neither gets a slot within
|
||||||
|
five seconds nor finds room in the queue is answered `503`. Above
|
||||||
|
roughly 27 requests per second the operator is not served slowly, it
|
||||||
|
is shed: its chance per attempt is about the ratio of service rate to
|
||||||
|
flood rate, so at 400 requests per second it is roughly one attempt in
|
||||||
|
fourteen. A sufficiently determined flood still denies login for as
|
||||||
|
long as it runs.
|
||||||
|
|
||||||
|
What changed is the price and the aftermath. Denying login used to
|
||||||
|
cost an attacker 0.08 requests per second from anywhere; it now costs
|
||||||
|
30 or more sustained, about 400 times as much. Nothing accumulates
|
||||||
|
while the flood runs, nothing needs resetting when it stops, and the
|
||||||
|
operator's correct password succeeds on the first attempt afterwards.
|
||||||
|
Restarting the service is **not** a remedy: a restart clears the
|
||||||
|
failure counters, which are not what is saturated, and the flood
|
||||||
|
re-fills both verification slots on its first two requests. The
|
||||||
|
remedies are to block the source at the reverse proxy, or to
|
||||||
|
rate-limit `POST /pages/login` there — the one place a limit can be
|
||||||
|
applied without reintroducing the lockout, because the proxy sees the
|
||||||
|
real client address. Setting `TRUSTED_PROXIES` does not stop the
|
||||||
|
saturation, but it makes the source visible in the failure logs.
|
||||||
|
|
||||||
Finer-grained per-webhook rate limits (configured in the web UI and
|
Finer-grained per-webhook rate limits (configured in the web UI and
|
||||||
enforced in the webhook handler) can layer on top of this env-level
|
enforced in the webhook handler) can layer on top of this env-level
|
||||||
@@ -1099,7 +1209,7 @@ abuse limit later; they are tracked as future work.
|
|||||||
| Method | Path | Description |
|
| Method | Path | Description |
|
||||||
| ------ | --------------- | ----------- |
|
| ------ | --------------- | ----------- |
|
||||||
| `GET` | `/pages/login` | Login page (not rate limited) |
|
| `GET` | `/pages/login` | Login page (not rate limited) |
|
||||||
| `POST` | `/pages/login` | Login form submission. Credentials are verified before any limit is consulted, so a correct password is never throttled; 5 FAILED attempts per minute per bucket per submitted username, then `429`. `503` if no verification slot frees up within 5s (see [Rate Limiting](#rate-limiting)) |
|
| `POST` | `/pages/login` | Login form submission. Credentials are verified before any limit is consulted, so a correct password is never throttled; 5 FAILED attempts per minute per bucket per submitted username, then `429`. `503` if no verification slot frees up within 5s, or immediately if 64 requests are already queued for one (see [Rate Limiting](#rate-limiting)) |
|
||||||
| `POST` | `/pages/logout` | Logout (destroys session) |
|
| `POST` | `/pages/logout` | Logout (destroys session) |
|
||||||
|
|
||||||
#### Authenticated Endpoints
|
#### Authenticated Endpoints
|
||||||
@@ -1107,7 +1217,7 @@ abuse limit later; they are tracked as future work.
|
|||||||
| Method | Path | Description |
|
| Method | Path | Description |
|
||||||
| ------ | ------------------------ | ----------- |
|
| ------ | ------------------------ | ----------- |
|
||||||
| `GET` | `/user/{username}` | User profile page |
|
| `GET` | `/user/{username}` | User profile page |
|
||||||
| `POST` | `/user/{username}/password` | Change the user's password (5 per minute per bucket, then `429`; `503` if no verification slot frees up within 5s) |
|
| `POST` | `/user/{username}/password` | Change the user's password (5 per minute per bucket, then `429`; `503` if no verification slot frees up within 5s, or immediately if 64 requests are already queued for one) |
|
||||||
| `GET` | `/sources` | List user's webhooks |
|
| `GET` | `/sources` | List user's webhooks |
|
||||||
| `GET` | `/sources/new` | Create webhook form |
|
| `GET` | `/sources/new` | Create webhook form |
|
||||||
| `POST` | `/sources/new` | Create webhook submission |
|
| `POST` | `/sources/new` | Create webhook submission |
|
||||||
@@ -1306,9 +1416,11 @@ one that lies about its length, is hard-capped by
|
|||||||
Those same four route groups then apply **CSRF** and **NoCache**
|
Those same four route groups then apply **CSRF** and **NoCache**
|
||||||
(`Cache-Control: no-store`, `Pragma: no-cache`), and every group except
|
(`Cache-Control: no-store`, `Pragma: no-cache`), and every group except
|
||||||
`/pages` applies **RequireAuth**. The rate limiters are per-route
|
`/pages` applies **RequireAuth**. The rate limiters are per-route
|
||||||
rather than global: **LoginRateLimit** on `/pages/login`,
|
rather than global: **PasswordChangeRateLimit** on
|
||||||
**PasswordChangeRateLimit** on `/user/{username}/password`, and
|
`/user/{username}/password` and **ReceiverRateLimit** on
|
||||||
**ReceiverRateLimit** on `/webhook/{uuid}`.
|
`/webhook/{uuid}`. There is deliberately none on `/pages/login` — that
|
||||||
|
endpoint counts failures inside the handler, after the credential
|
||||||
|
check, see [The login endpoint](#the-login-endpoint).
|
||||||
|
|
||||||
### Authentication
|
### Authentication
|
||||||
|
|
||||||
@@ -1346,14 +1458,27 @@ rather than global: **LoginRateLimit** on `/pages/login`,
|
|||||||
both at target creation time (URL validation) and at delivery time
|
both at target creation time (URL validation) and at delivery time
|
||||||
(custom HTTP transport with SSRF-safe dialer that validates resolved
|
(custom HTTP transport with SSRF-safe dialer that validates resolved
|
||||||
IPs before connecting, preventing DNS rebinding attacks)
|
IPs before connecting, preventing DNS rebinding attacks)
|
||||||
- **Login rate limiting** via [go-chi/httprate](https://github.com/go-chi/httprate):
|
- **Login limiting is inverted, deliberately.** The login `POST` has
|
||||||
sliding-window rate limiter on the login endpoint, 5 POST attempts
|
no pre-emptive rate limiter in front of it. Credentials are
|
||||||
per minute per bucket, to slow brute-force attacks. GET requests to
|
verified first and only a _failed_ attempt spends budget, so a
|
||||||
the login page are not limited. The password-change endpoint carries
|
correct password is never throttled and no flood of wrong ones can
|
||||||
the same 5-per-minute limit. The bucket is per client IP only when
|
deny the operator the only administrative path. Failures are
|
||||||
|
counted per (bucket, submitted username), five per minute, after
|
||||||
|
which further failures are answered `429` with a `Retry-After`.
|
||||||
|
What bounds brute force is not that counter but the cap of two
|
||||||
|
concurrent Argon2id verifications: a throttled client's guess is
|
||||||
|
still evaluated, so roughly 27 guesses a second get through and the
|
||||||
|
admin password has to carry that load (see
|
||||||
|
[The login endpoint](#the-login-endpoint)). `GET` requests to the
|
||||||
|
login page are not limited
|
||||||
|
- **Password-change rate limiting** via [go-chi/httprate](https://github.com/go-chi/httprate):
|
||||||
|
sliding-window rate limiter, 5 POST attempts per minute per bucket.
|
||||||
|
It runs behind session auth, so only a client already holding a
|
||||||
|
valid session reaches it, and an operator throttled out of changing
|
||||||
|
a password can still log in. The bucket is per client IP only when
|
||||||
`TRUSTED_PROXIES` names the reverse proxy; unset, every client
|
`TRUSTED_PROXIES` names the reverse proxy; unset, every client
|
||||||
shares one bucket and the login becomes remotely deniable (see
|
shares one bucket, which costs precision rather than availability
|
||||||
[Rate Limiting](#rate-limiting)). webhooker warns at startup
|
(see [Rate Limiting](#rate-limiting)). webhooker warns at startup
|
||||||
whenever `TRUSTED_PROXIES` is empty
|
whenever `TRUSTED_PROXIES` is empty
|
||||||
- Prometheus metrics behind basic auth
|
- Prometheus metrics behind basic auth
|
||||||
- Static assets embedded in binary (no filesystem access needed at
|
- Static assets embedded in binary (no filesystem access needed at
|
||||||
@@ -1491,11 +1616,32 @@ way.
|
|||||||
A separate workflow step, run before the fingerprint is written, covers
|
A separate workflow step, run before the fingerprint is written, covers
|
||||||
a second way the gate lied: Gitea cancels an in-flight run when a newer
|
a second way the gate lied: Gitea cancels an in-flight run when a newer
|
||||||
commit lands on the same branch and records that cancellation as a
|
commit lands on the same branch and records that cancellation as a
|
||||||
`failure` status, marking a commit red that was never tested.
|
`failure` status, so a commit nothing ever tested reads as a test
|
||||||
Cancellation is unconditional server-side for
|
result. Cancellation is unconditional server-side for push events, so
|
||||||
push events, so the superseding run rewrites the exact
|
the superseding run calls `script/ci-mark-superseded`, which rewrites
|
||||||
`Has been cancelled` status to `skipped`. Genuine failures are never
|
that exact status to `failure` /
|
||||||
touched.
|
`Superseded by a newer commit; never tested`.
|
||||||
|
|
||||||
|
The state stays `failure` on purpose: Gitea's combined status folds
|
||||||
|
`skipped` into `success`, so marking a never-tested commit `skipped`
|
||||||
|
made the status API report green for it, indistinguishable from a commit
|
||||||
|
that passed. Reading a commit's status on this repo therefore goes:
|
||||||
|
|
||||||
|
- `success` / `Successful in ...` — the checks ran and passed.
|
||||||
|
- `failure` / `Failing after ...` — the checks ran and failed.
|
||||||
|
- `failure` / `Superseded by a newer commit; never tested` — the run was
|
||||||
|
cancelled, by a newer push or by hand, and nothing was verified about
|
||||||
|
this commit. Test the commit itself before concluding anything about
|
||||||
|
it.
|
||||||
|
|
||||||
|
Genuine failures and successes are never touched, and no status is left
|
||||||
|
`pending`, which would block the commit indefinitely. The step derives
|
||||||
|
its context string from the workflow name, the job **id** and the event.
|
||||||
|
That is deliberately not byte-identical to Gitea's own rule, which uses
|
||||||
|
the job's display `name:` where the runner exports the id, so giving the
|
||||||
|
job a `name:` — or renaming the workflow — makes the derived context
|
||||||
|
stop matching. The step fails loudly when no status on the commit
|
||||||
|
carries that context, so no rename can silently disable the rewrite.
|
||||||
|
|
||||||
## TODO
|
## TODO
|
||||||
|
|
||||||
|
|||||||
2
go.mod
2
go.mod
@@ -17,6 +17,7 @@ require (
|
|||||||
github.com/stretchr/testify v1.8.4
|
github.com/stretchr/testify v1.8.4
|
||||||
go.uber.org/fx v1.20.1
|
go.uber.org/fx v1.20.1
|
||||||
golang.org/x/crypto v0.38.0
|
golang.org/x/crypto v0.38.0
|
||||||
|
gopkg.in/yaml.v3 v3.0.1
|
||||||
gorm.io/driver/sqlite v1.5.4
|
gorm.io/driver/sqlite v1.5.4
|
||||||
gorm.io/gorm v1.25.5
|
gorm.io/gorm v1.25.5
|
||||||
modernc.org/sqlite v1.28.0
|
modernc.org/sqlite v1.28.0
|
||||||
@@ -52,7 +53,6 @@ require (
|
|||||||
golang.org/x/text v0.25.0 // indirect
|
golang.org/x/text v0.25.0 // indirect
|
||||||
golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
|
golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
|
||||||
google.golang.org/protobuf v1.31.0 // indirect
|
google.golang.org/protobuf v1.31.0 // indirect
|
||||||
gopkg.in/yaml.v3 v3.0.1 // indirect
|
|
||||||
lukechampine.com/uint128 v1.2.0 // indirect
|
lukechampine.com/uint128 v1.2.0 // indirect
|
||||||
modernc.org/cc/v3 v3.40.0 // indirect
|
modernc.org/cc/v3 v3.40.0 // indirect
|
||||||
modernc.org/ccgo/v3 v3.16.13 // indirect
|
modernc.org/ccgo/v3 v3.16.13 // indirect
|
||||||
|
|||||||
387
internal/ciscript/ci_mark_superseded_test.go
Normal file
387
internal/ciscript/ci_mark_superseded_test.go
Normal file
@@ -0,0 +1,387 @@
|
|||||||
|
package ciscript_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"maps"
|
||||||
|
"os"
|
||||||
|
"os/exec"
|
||||||
|
"path/filepath"
|
||||||
|
"slices"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"gopkg.in/yaml.v3"
|
||||||
|
)
|
||||||
|
|
||||||
|
const (
|
||||||
|
// supersededDesc is the description script/ci-mark-superseded
|
||||||
|
// writes, and the one an earlier revision of it wrote alongside a
|
||||||
|
// `skipped` state.
|
||||||
|
supersededDesc = "Superseded by a newer commit; never tested"
|
||||||
|
|
||||||
|
// liveContext is the commit-status context Gitea uses for this
|
||||||
|
// repository's runs, as seen in its API. The script derives it from
|
||||||
|
// the workflow and job names rather than hardcoding it; the
|
||||||
|
// derivation is checked against this value below.
|
||||||
|
liveContext = "check / check (push)"
|
||||||
|
|
||||||
|
scriptPath = "../../script/ci-mark-superseded"
|
||||||
|
workflow = "../../.gitea/workflows/check.yml"
|
||||||
|
|
||||||
|
// failure is the only state that neither folds into a combined
|
||||||
|
// `success` (as `skipped` does) nor blocks the commit forever (as
|
||||||
|
// `pending` does).
|
||||||
|
failure = "failure"
|
||||||
|
)
|
||||||
|
|
||||||
|
// repo is a throwaway git history: parent is the commit a run would be
|
||||||
|
// cancelled on, head the commit that superseded it.
|
||||||
|
type repo struct {
|
||||||
|
dir string
|
||||||
|
head string
|
||||||
|
parent string
|
||||||
|
}
|
||||||
|
|
||||||
|
// scriptEnv is the run identity the Gitea runner exports and the script
|
||||||
|
// builds its context string from.
|
||||||
|
type scriptEnv struct {
|
||||||
|
workflow string
|
||||||
|
job string
|
||||||
|
event string
|
||||||
|
}
|
||||||
|
|
||||||
|
func defaultEnv() scriptEnv {
|
||||||
|
return scriptEnv{workflow: "check", job: "check", event: "push"}
|
||||||
|
}
|
||||||
|
|
||||||
|
func cancelled() commitStatus {
|
||||||
|
return commitStatus{
|
||||||
|
Context: liveContext,
|
||||||
|
Status: failure,
|
||||||
|
Description: "Has been cancelled",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func running() commitStatus {
|
||||||
|
return commitStatus{
|
||||||
|
Context: liveContext,
|
||||||
|
Status: "pending",
|
||||||
|
Description: "Has started running",
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestMarkSuperseded(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
cases := map[string]struct {
|
||||||
|
parent commitStatus
|
||||||
|
wantMark bool
|
||||||
|
}{
|
||||||
|
"a cancelled run is marked": {
|
||||||
|
parent: cancelled(),
|
||||||
|
wantMark: true,
|
||||||
|
},
|
||||||
|
"a laundered skipped status is marked": {
|
||||||
|
parent: commitStatus{
|
||||||
|
Context: liveContext,
|
||||||
|
Status: "skipped",
|
||||||
|
Description: supersededDesc,
|
||||||
|
},
|
||||||
|
wantMark: true,
|
||||||
|
},
|
||||||
|
"a genuine failure is left alone": {
|
||||||
|
parent: commitStatus{
|
||||||
|
Context: liveContext,
|
||||||
|
Status: failure,
|
||||||
|
Description: "Failing after 3m1s",
|
||||||
|
},
|
||||||
|
wantMark: false,
|
||||||
|
},
|
||||||
|
"a passing run is left alone": {
|
||||||
|
parent: commitStatus{
|
||||||
|
Context: liveContext,
|
||||||
|
Status: "success",
|
||||||
|
Description: "Successful in 2m52s",
|
||||||
|
},
|
||||||
|
wantMark: false,
|
||||||
|
},
|
||||||
|
"another context is left alone": {
|
||||||
|
parent: commitStatus{
|
||||||
|
Context: "other / other (push)",
|
||||||
|
Status: failure,
|
||||||
|
Description: "Has been cancelled",
|
||||||
|
},
|
||||||
|
wantMark: false,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for name, tc := range cases {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
history := newRepo(t)
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, tc.parent)
|
||||||
|
|
||||||
|
out, err := runScript(t, history, api, defaultEnv())
|
||||||
|
require.NoError(t, err, out)
|
||||||
|
|
||||||
|
posted := fake.postedFor(history.parent)
|
||||||
|
if !tc.wantMark {
|
||||||
|
require.Empty(t, posted)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
require.Equal(t, []postedStatus{{
|
||||||
|
Context: liveContext,
|
||||||
|
// Not `skipped`: Gitea's combined status folds
|
||||||
|
// that into `success`, which is what made a
|
||||||
|
// never-tested commit read green.
|
||||||
|
State: failure,
|
||||||
|
Description: supersededDesc,
|
||||||
|
}}, posted)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A second run must not rewrite what the first one wrote, or every
|
||||||
|
// later push would post a duplicate status.
|
||||||
|
func TestMarkSupersededIsIdempotent(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
history := newRepo(t)
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, cancelled())
|
||||||
|
|
||||||
|
for range 2 {
|
||||||
|
out, err := runScript(t, history, api, defaultEnv())
|
||||||
|
require.NoError(t, err, out)
|
||||||
|
}
|
||||||
|
|
||||||
|
require.Len(t, fake.postedFor(history.parent), 1)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Renaming the workflow or the job changes the context string Gitea
|
||||||
|
// uses. The script must say so instead of quietly matching nothing.
|
||||||
|
func TestMarkSupersededRejectsAnUnknownContext(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
history := newRepo(t)
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, cancelled())
|
||||||
|
|
||||||
|
env := defaultEnv()
|
||||||
|
env.job = "renamed"
|
||||||
|
|
||||||
|
out, err := runScript(t, history, api, env)
|
||||||
|
require.Error(t, err)
|
||||||
|
require.Contains(t, out, "renamed")
|
||||||
|
require.Contains(t, out, liveContext)
|
||||||
|
require.Empty(t, fake.postedFor(history.parent))
|
||||||
|
}
|
||||||
|
|
||||||
|
// ANCESTOR_LIMIT is a documented knob. A value that is set but unusable
|
||||||
|
// must abort: handing it to git and discarding the exit status left the
|
||||||
|
// walk empty and the step green, marking nothing.
|
||||||
|
func TestMarkSupersededRejectsAnUnparseableAncestorLimit(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
history := newRepo(t)
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, cancelled())
|
||||||
|
|
||||||
|
out, err := runScript(
|
||||||
|
t, history, api, defaultEnv(), "ANCESTOR_LIMIT=twenty",
|
||||||
|
)
|
||||||
|
require.Error(t, err)
|
||||||
|
require.Contains(t, out, "ANCESTOR_LIMIT")
|
||||||
|
require.Contains(t, out, "twenty")
|
||||||
|
require.Empty(t, fake.postedFor(history.parent))
|
||||||
|
}
|
||||||
|
|
||||||
|
// A status read that fails is not the same as a commit with nothing to
|
||||||
|
// do. Losing curl's exit status through a pipe made the two identical
|
||||||
|
// and left a laundered commit laundered with no signal.
|
||||||
|
func TestMarkSupersededFailsOnAnUnreadableAncestorStatus(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
history := newRepo(t)
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, cancelled())
|
||||||
|
fake.failStatusRead(history.parent)
|
||||||
|
|
||||||
|
out, err := runScript(t, history, api, defaultEnv())
|
||||||
|
require.Error(t, err)
|
||||||
|
require.Contains(t, out, history.parent)
|
||||||
|
require.Contains(t, out, "cannot read commit statuses")
|
||||||
|
require.Empty(t, fake.postedFor(history.parent))
|
||||||
|
}
|
||||||
|
|
||||||
|
// A shallow clone cannot resolve the parent, so it is indistinguishable
|
||||||
|
// from a root commit to rev-parse and the walk would exit 0 having
|
||||||
|
// marked nothing. It must abort instead: dropping `fetch-depth: 0` from
|
||||||
|
// the checkout step is one edit, and a silent no-op there restores the
|
||||||
|
// false-green bug this script exists to prevent.
|
||||||
|
func TestMarkSupersededRejectsAShallowRepository(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
history := shallowClone(t, newRepo(t))
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, cancelled())
|
||||||
|
|
||||||
|
out, err := runScript(t, history, api, defaultEnv())
|
||||||
|
require.Error(t, err)
|
||||||
|
require.Contains(t, out, "shallow repository")
|
||||||
|
require.Empty(t, fake.postedFor(history.parent))
|
||||||
|
require.Empty(t, fake.postedFor(history.head))
|
||||||
|
}
|
||||||
|
|
||||||
|
// shallowClone returns the same history as a depth-1 clone. The `file://`
|
||||||
|
// URL is required: git ignores --depth for a plain local path.
|
||||||
|
func shallowClone(t *testing.T, history repo) repo {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
|
||||||
|
//nolint:gosec // fixed argv, arguments are test-local paths
|
||||||
|
cmd := exec.CommandContext(t.Context(), "git", "clone", "-q",
|
||||||
|
"--depth=1", "file://"+history.dir, dir)
|
||||||
|
|
||||||
|
out, err := cmd.CombinedOutput()
|
||||||
|
require.NoError(t, err, string(out))
|
||||||
|
|
||||||
|
return repo{dir: dir, head: history.head, parent: history.parent}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The derived context must equal the one Gitea actually uses, which is
|
||||||
|
// built from the same workflow and job names.
|
||||||
|
func TestDerivedContextMatchesGitea(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
requireTools(t)
|
||||||
|
|
||||||
|
name, job := workflowIdentity(t)
|
||||||
|
|
||||||
|
history := newRepo(t)
|
||||||
|
fake, api := newFakeGitea(t)
|
||||||
|
fake.setStatus(history.head, running())
|
||||||
|
fake.setStatus(history.parent, cancelled())
|
||||||
|
|
||||||
|
out, err := runScript(t, history, api, scriptEnv{
|
||||||
|
workflow: name,
|
||||||
|
job: job,
|
||||||
|
event: "push",
|
||||||
|
})
|
||||||
|
require.NoError(t, err, out)
|
||||||
|
|
||||||
|
posted := fake.postedFor(history.parent)
|
||||||
|
require.Len(t, posted, 1)
|
||||||
|
require.Equal(t, liveContext, posted[0].Context)
|
||||||
|
}
|
||||||
|
|
||||||
|
// workflowIdentity reads the workflow name and its single job id out of
|
||||||
|
// the checked-in workflow file.
|
||||||
|
func workflowIdentity(t *testing.T) (string, string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
raw, err := os.ReadFile(workflow)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
var parsed struct {
|
||||||
|
Name string `yaml:"name"`
|
||||||
|
Jobs map[string]any `yaml:"jobs"`
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NoError(t, yaml.Unmarshal(raw, &parsed))
|
||||||
|
|
||||||
|
jobs := slices.Collect(maps.Keys(parsed.Jobs))
|
||||||
|
require.Len(t, jobs, 1)
|
||||||
|
|
||||||
|
return parsed.Name, jobs[0]
|
||||||
|
}
|
||||||
|
|
||||||
|
func runScript(
|
||||||
|
t *testing.T, history repo, api string, env scriptEnv,
|
||||||
|
extra ...string,
|
||||||
|
) (string, error) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
script, err := filepath.Abs(scriptPath)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
//nolint:gosec // fixed argv, repo-local script under test
|
||||||
|
cmd := exec.CommandContext(t.Context(), "sh", script)
|
||||||
|
cmd.Dir = history.dir
|
||||||
|
cmd.Env = append(os.Environ(),
|
||||||
|
"GITHUB_API_URL="+api,
|
||||||
|
"GITHUB_REPOSITORY=sneak/webhooker",
|
||||||
|
"GITHUB_SHA="+history.head,
|
||||||
|
"GITHUB_WORKFLOW="+env.workflow,
|
||||||
|
"GITHUB_JOB="+env.job,
|
||||||
|
"GITHUB_EVENT_NAME="+env.event,
|
||||||
|
"GITEA_TOKEN=test-token",
|
||||||
|
)
|
||||||
|
cmd.Env = append(cmd.Env, extra...)
|
||||||
|
|
||||||
|
out, err := cmd.CombinedOutput()
|
||||||
|
|
||||||
|
return string(out), err
|
||||||
|
}
|
||||||
|
|
||||||
|
func newRepo(t *testing.T) repo {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
|
||||||
|
git := func(args ...string) string {
|
||||||
|
//nolint:gosec // fixed argv, arguments are test constants
|
||||||
|
cmd := exec.CommandContext(t.Context(), "git", args...)
|
||||||
|
cmd.Dir = dir
|
||||||
|
|
||||||
|
out, err := cmd.CombinedOutput()
|
||||||
|
require.NoError(t, err, string(out))
|
||||||
|
|
||||||
|
return strings.TrimSpace(string(out))
|
||||||
|
}
|
||||||
|
|
||||||
|
commit := func(message string) string {
|
||||||
|
git(
|
||||||
|
"-c", "user.email=ci@example.invalid",
|
||||||
|
"-c", "user.name=ci",
|
||||||
|
"-c", "commit.gpgsign=false",
|
||||||
|
"commit", "-q", "--allow-empty", "-m", message,
|
||||||
|
)
|
||||||
|
|
||||||
|
return git("rev-parse", "HEAD")
|
||||||
|
}
|
||||||
|
|
||||||
|
git("init", "-q", "-b", "main")
|
||||||
|
|
||||||
|
parent := commit("parent")
|
||||||
|
head := commit("head")
|
||||||
|
|
||||||
|
return repo{dir: dir, head: head, parent: parent}
|
||||||
|
}
|
||||||
|
|
||||||
|
func requireTools(t *testing.T) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
for _, tool := range []string{"sh", "git", "curl", "jq"} {
|
||||||
|
_, err := exec.LookPath(tool)
|
||||||
|
if err != nil {
|
||||||
|
t.Skipf("%s is not installed: %v", tool, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
10
internal/ciscript/doc.go
Normal file
10
internal/ciscript/doc.go
Normal file
@@ -0,0 +1,10 @@
|
|||||||
|
// Package ciscript holds the tests for the repository's CI shell
|
||||||
|
// scripts in script/. It carries no runtime code: the scripts run on
|
||||||
|
// the CI runner, not inside the binary, but their behaviour still has
|
||||||
|
// to be verified by the test suite.
|
||||||
|
//
|
||||||
|
// The scripts under test are outside the Go build graph, so `go test`'s
|
||||||
|
// result cache serves a stale PASS when only a script changed: run the
|
||||||
|
// container build, or GOFLAGS=-count=1, to trust a result here after
|
||||||
|
// editing script/.
|
||||||
|
package ciscript
|
||||||
162
internal/ciscript/fakegitea_test.go
Normal file
162
internal/ciscript/fakegitea_test.go
Normal file
@@ -0,0 +1,162 @@
|
|||||||
|
package ciscript_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"sync"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
// commitStatus is the part of an entry in Gitea's combined-status
|
||||||
|
// response that script/ci-mark-superseded reads.
|
||||||
|
type commitStatus struct {
|
||||||
|
Context string `json:"context"`
|
||||||
|
Status string `json:"status"`
|
||||||
|
Description string `json:"description"`
|
||||||
|
}
|
||||||
|
|
||||||
|
// postedStatus is the part of a create-status request body the script
|
||||||
|
// writes.
|
||||||
|
type postedStatus struct {
|
||||||
|
Context string `json:"context"`
|
||||||
|
State string `json:"state"`
|
||||||
|
Description string `json:"description"`
|
||||||
|
}
|
||||||
|
|
||||||
|
// fakeGitea serves the two endpoints the script talks to. Like Gitea,
|
||||||
|
// the newest status for a context replaces the previous one, so a
|
||||||
|
// second run of the script sees what the first one wrote.
|
||||||
|
type fakeGitea struct {
|
||||||
|
mu sync.Mutex
|
||||||
|
statuses map[string][]commitStatus
|
||||||
|
posted map[string][]postedStatus
|
||||||
|
// failRead is a commit whose combined-status read answers HTTP
|
||||||
|
// 500, standing in for a status API that is down.
|
||||||
|
failRead string
|
||||||
|
}
|
||||||
|
|
||||||
|
// newFakeGitea returns the fake and the base URL to hand the script as
|
||||||
|
// GITHUB_API_URL.
|
||||||
|
func newFakeGitea(t *testing.T) (*fakeGitea, string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
fake := &fakeGitea{
|
||||||
|
mu: sync.Mutex{},
|
||||||
|
statuses: map[string][]commitStatus{},
|
||||||
|
posted: map[string][]postedStatus{},
|
||||||
|
failRead: "",
|
||||||
|
}
|
||||||
|
|
||||||
|
srv := httptest.NewServer(fake.routes())
|
||||||
|
t.Cleanup(srv.Close)
|
||||||
|
|
||||||
|
return fake, srv.URL
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *fakeGitea) routes() http.Handler {
|
||||||
|
mux := http.NewServeMux()
|
||||||
|
|
||||||
|
mux.HandleFunc(
|
||||||
|
"GET /repos/{owner}/{repo}/commits/{sha}/status",
|
||||||
|
f.handleCombined,
|
||||||
|
)
|
||||||
|
mux.HandleFunc(
|
||||||
|
"POST /repos/{owner}/{repo}/statuses/{sha}",
|
||||||
|
f.handleCreate,
|
||||||
|
)
|
||||||
|
|
||||||
|
return mux
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *fakeGitea) handleCombined(
|
||||||
|
w http.ResponseWriter, r *http.Request,
|
||||||
|
) {
|
||||||
|
f.mu.Lock()
|
||||||
|
defer f.mu.Unlock()
|
||||||
|
|
||||||
|
sha := r.PathValue("sha")
|
||||||
|
if f.failRead != "" && f.failRead == sha {
|
||||||
|
http.Error(w, "boom", http.StatusInternalServerError)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
body := struct {
|
||||||
|
Statuses []commitStatus `json:"statuses"`
|
||||||
|
}{Statuses: f.statuses[sha]}
|
||||||
|
|
||||||
|
payload, err := json.Marshal(body)
|
||||||
|
if err != nil {
|
||||||
|
http.Error(w, err.Error(), http.StatusInternalServerError)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
w.Header().Set("Content-Type", "application/json")
|
||||||
|
|
||||||
|
_, _ = w.Write(payload)
|
||||||
|
}
|
||||||
|
|
||||||
|
func (f *fakeGitea) handleCreate(w http.ResponseWriter, r *http.Request) {
|
||||||
|
var got postedStatus
|
||||||
|
|
||||||
|
err := json.NewDecoder(r.Body).Decode(&got)
|
||||||
|
if err != nil {
|
||||||
|
http.Error(w, err.Error(), http.StatusBadRequest)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
sha := r.PathValue("sha")
|
||||||
|
|
||||||
|
f.mu.Lock()
|
||||||
|
defer f.mu.Unlock()
|
||||||
|
|
||||||
|
f.posted[sha] = append(f.posted[sha], got)
|
||||||
|
f.replaceLocked(sha, commitStatus{
|
||||||
|
Context: got.Context,
|
||||||
|
Status: got.State,
|
||||||
|
Description: got.Description,
|
||||||
|
})
|
||||||
|
|
||||||
|
w.WriteHeader(http.StatusCreated)
|
||||||
|
}
|
||||||
|
|
||||||
|
// failStatusRead makes the combined-status read for one commit answer
|
||||||
|
// HTTP 500.
|
||||||
|
func (f *fakeGitea) failStatusRead(sha string) {
|
||||||
|
f.mu.Lock()
|
||||||
|
defer f.mu.Unlock()
|
||||||
|
|
||||||
|
f.failRead = sha
|
||||||
|
}
|
||||||
|
|
||||||
|
// setStatus gives a commit its latest status for a context.
|
||||||
|
func (f *fakeGitea) setStatus(sha string, status commitStatus) {
|
||||||
|
f.mu.Lock()
|
||||||
|
defer f.mu.Unlock()
|
||||||
|
|
||||||
|
f.replaceLocked(sha, status)
|
||||||
|
}
|
||||||
|
|
||||||
|
// postedFor returns the statuses the script created for a commit.
|
||||||
|
func (f *fakeGitea) postedFor(sha string) []postedStatus {
|
||||||
|
f.mu.Lock()
|
||||||
|
defer f.mu.Unlock()
|
||||||
|
|
||||||
|
return append([]postedStatus(nil), f.posted[sha]...)
|
||||||
|
}
|
||||||
|
|
||||||
|
// replaceLocked requires f.mu.
|
||||||
|
func (f *fakeGitea) replaceLocked(sha string, status commitStatus) {
|
||||||
|
for i, existing := range f.statuses[sha] {
|
||||||
|
if existing.Context == status.Context {
|
||||||
|
f.statuses[sha][i] = status
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
f.statuses[sha] = append(f.statuses[sha], status)
|
||||||
|
}
|
||||||
199
internal/handlers/event_body.go
Normal file
199
internal/handlers/event_body.go
Normal file
@@ -0,0 +1,199 @@
|
|||||||
|
package handlers
|
||||||
|
|
||||||
|
import (
|
||||||
|
"database/sql"
|
||||||
|
"errors"
|
||||||
|
"net/http"
|
||||||
|
"strconv"
|
||||||
|
|
||||||
|
"github.com/go-chi/chi"
|
||||||
|
"github.com/google/uuid"
|
||||||
|
"gorm.io/gorm"
|
||||||
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
|
)
|
||||||
|
|
||||||
|
// eventBodyQuery reads one event's stored body as bytes. The cast
|
||||||
|
// to blob is what makes the driver hand back the stored bytes
|
||||||
|
// rather than a string conversion, so Content-Length taken from
|
||||||
|
// the result matches what goes on the wire. The soft-delete
|
||||||
|
// predicate is spelled out because Raw bypasses GORM's default
|
||||||
|
// scope, and it is what stops a reaped event still being
|
||||||
|
// downloadable.
|
||||||
|
const eventBodyQuery = "SELECT cast(body as blob) " +
|
||||||
|
"FROM events WHERE id = ? AND webhook_id = ? AND deleted_at IS NULL"
|
||||||
|
|
||||||
|
// HandleEventBodyDownload serves one event's stored body in
|
||||||
|
// full, which the event log page cannot: it caps each rendered
|
||||||
|
// body at maxRenderedBodyBytes.
|
||||||
|
//
|
||||||
|
// The bytes are attacker-supplied — anyone who can reach the
|
||||||
|
// public receiver chooses them — and this route hands them back
|
||||||
|
// inside the operator's own authenticated origin, so the
|
||||||
|
// response is deliberately not renderable. Content-Disposition
|
||||||
|
// makes the browser download rather than display it, and the
|
||||||
|
// octet-stream type plus nosniff stop it being interpreted as
|
||||||
|
// HTML or script. Without those a stored payload would execute
|
||||||
|
// as the logged-in operator. The application's CSP does not
|
||||||
|
// help here: script-src allows 'unsafe-inline' from 'self', so
|
||||||
|
// a document served from this origin could run its own inline
|
||||||
|
// script.
|
||||||
|
func (h *Handlers) HandleEventBodyDownload() http.HandlerFunc {
|
||||||
|
return func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
webhook, ok := h.ownedWebhook(w, r)
|
||||||
|
if !ok {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// Parsing the id before use serves two purposes: a
|
||||||
|
// malformed id can never reach the SQL or the response
|
||||||
|
// header, and the canonical form below is drawn from
|
||||||
|
// uuid's own fixed alphabet rather than from the
|
||||||
|
// request, so the Content-Disposition value cannot be
|
||||||
|
// steered by a client.
|
||||||
|
eventID, err := uuid.Parse(chi.URLParam(r, "eventID"))
|
||||||
|
if err != nil {
|
||||||
|
http.NotFound(w, r)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
h.serveEventBody(w, r, webhook, eventID.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// serveEventBody writes the named event's stored body to w.
|
||||||
|
//
|
||||||
|
// The event must belong to webhook, which is what keeps this
|
||||||
|
// route from reading any event in the system by id alone. Two
|
||||||
|
// things enforce that and they are not equally strong. The
|
||||||
|
// operative one is that events live in a per-webhook SQLite
|
||||||
|
// file, so a sibling webhook's event is not in the database
|
||||||
|
// being queried at all. The webhook_id predicate on the query
|
||||||
|
// below is the second guard, and it is currently redundant
|
||||||
|
// against that isolation; it is there so the scoping survives
|
||||||
|
// any future change that puts more than one webhook's events in
|
||||||
|
// one file.
|
||||||
|
//
|
||||||
|
// The body is read in one query and held whole in memory while
|
||||||
|
// it is written. That costs roughly two body-sized allocations
|
||||||
|
// per concurrent download, not one: the driver's column buffer
|
||||||
|
// and the copy database/sql makes in convertAssign when a
|
||||||
|
// []byte column is scanned into a *[]byte are live at the same
|
||||||
|
// time. Measured allocation is ~2x the body plus ~45 KB, so at
|
||||||
|
// the 1 MB ingest cap a download costs ~2 MB of Go heap. On
|
||||||
|
// top of that, SQLite's own materialisation of the column
|
||||||
|
// value sits in the driver's allocator outside the Go heap, so
|
||||||
|
// process peak is higher again: 2x is a floor, not a ceiling.
|
||||||
|
// There is no cheaper bound available — database/sql exposes
|
||||||
|
// no incremental handle on a SQLite BLOB, and reading byte
|
||||||
|
// ranges with substr does not avoid the cost either, because
|
||||||
|
// SQLite materialises the whole column value to evaluate each
|
||||||
|
// substr call. Range reads only pay for that materialisation
|
||||||
|
// once per range.
|
||||||
|
//
|
||||||
|
// One consequence is worth keeping in view: the read finishes
|
||||||
|
// before the client is written to, so no read lock is held for
|
||||||
|
// the length of a slow download. These per-webhook databases
|
||||||
|
// run in SQLite's default journal mode rather than WAL, so a
|
||||||
|
// lock held that long would block the receiver from recording
|
||||||
|
// new events.
|
||||||
|
func (h *Handlers) serveEventBody(
|
||||||
|
w http.ResponseWriter,
|
||||||
|
r *http.Request,
|
||||||
|
webhook database.Webhook,
|
||||||
|
eventID string,
|
||||||
|
) {
|
||||||
|
if !h.dbMgr.DBExists(webhook.ID) {
|
||||||
|
http.NotFound(w, r)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
webhookDB, err := h.dbMgr.GetDB(webhook.ID)
|
||||||
|
if err != nil {
|
||||||
|
h.serverError(w, "failed to get webhook database", err)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
body, found, err := eventBody(webhookDB, webhook.ID, eventID)
|
||||||
|
if err != nil {
|
||||||
|
h.serverError(w, "failed to read event body", err)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
// A miss is a 404 whether the event belongs to another
|
||||||
|
// webhook or does not exist at all, so the response does
|
||||||
|
// not report which. Reading the body before any header is
|
||||||
|
// written is also what keeps an event reaped mid-request
|
||||||
|
// from producing a torn response: either the read finds the
|
||||||
|
// row and the whole body is served, or it does not and the
|
||||||
|
// response is a clean 404.
|
||||||
|
if !found {
|
||||||
|
http.NotFound(w, r)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
setEventBodyHeaders(w, eventID, int64(len(body)))
|
||||||
|
|
||||||
|
_, err = w.Write(body)
|
||||||
|
if err != nil {
|
||||||
|
// The status and Content-Length are already committed,
|
||||||
|
// so the client sees a short download. There is no way
|
||||||
|
// to report a 500 from here; the log is the record.
|
||||||
|
h.log.Error(
|
||||||
|
"failed to write event body",
|
||||||
|
"webhook_id", webhook.ID,
|
||||||
|
"event_id", eventID,
|
||||||
|
"error", err,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// eventBody returns an event's stored body and whether the event
|
||||||
|
// exists within the webhook.
|
||||||
|
func eventBody(
|
||||||
|
webhookDB *gorm.DB,
|
||||||
|
webhookID, eventID string,
|
||||||
|
) ([]byte, bool, error) {
|
||||||
|
var body []byte
|
||||||
|
|
||||||
|
err := webhookDB.Raw(
|
||||||
|
eventBodyQuery, eventID, webhookID,
|
||||||
|
).Row().Scan(&body)
|
||||||
|
|
||||||
|
if errors.Is(err, sql.ErrNoRows) {
|
||||||
|
return nil, false, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
if err != nil {
|
||||||
|
return nil, false, err
|
||||||
|
}
|
||||||
|
|
||||||
|
return body, true, nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// setEventBodyHeaders applies the response headers that make
|
||||||
|
// this route safe to hand attacker-supplied bytes through. See
|
||||||
|
// HandleEventBodyDownload for why they are a security control
|
||||||
|
// and not a formatting choice.
|
||||||
|
//
|
||||||
|
// nosniff is also set by the global SecurityHeaders middleware.
|
||||||
|
// It is repeated here so the guarantee belongs to the route
|
||||||
|
// that needs it rather than to a middleware someone could
|
||||||
|
// reorder or scope away.
|
||||||
|
func setEventBodyHeaders(
|
||||||
|
w http.ResponseWriter,
|
||||||
|
eventID string,
|
||||||
|
size int64,
|
||||||
|
) {
|
||||||
|
w.Header().Set("Content-Type", "application/octet-stream")
|
||||||
|
w.Header().Set("X-Content-Type-Options", "nosniff")
|
||||||
|
w.Header().Set(
|
||||||
|
"Content-Disposition",
|
||||||
|
`attachment; filename="webhooker-event-`+eventID+`.bin"`,
|
||||||
|
)
|
||||||
|
w.Header().Set("Content-Length", strconv.FormatInt(size, 10))
|
||||||
|
}
|
||||||
506
internal/handlers/event_body_test.go
Normal file
506
internal/handlers/event_body_test.go
Normal file
@@ -0,0 +1,506 @@
|
|||||||
|
package handlers_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"net/url"
|
||||||
|
"strconv"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/go-chi/chi"
|
||||||
|
"github.com/google/uuid"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"gorm.io/gorm/clause"
|
||||||
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
|
"sneak.berlin/go/webhooker/internal/handlers"
|
||||||
|
"sneak.berlin/go/webhooker/internal/session"
|
||||||
|
)
|
||||||
|
|
||||||
|
// paramEventID is the chi URL parameter the body download
|
||||||
|
// handler reads.
|
||||||
|
const paramEventID = "eventID"
|
||||||
|
|
||||||
|
// otherTestUserID owns webhooks the session user must not be
|
||||||
|
// able to read.
|
||||||
|
const otherTestUserID = "other-user-id"
|
||||||
|
|
||||||
|
// seedWebhookFor inserts a webhook owned by the given user.
|
||||||
|
func seedWebhookFor(
|
||||||
|
t *testing.T,
|
||||||
|
db *database.Database,
|
||||||
|
userID string,
|
||||||
|
) *database.Webhook {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
wh := &database.Webhook{
|
||||||
|
UserID: userID,
|
||||||
|
Name: "wh-" + userID,
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NoError(
|
||||||
|
t,
|
||||||
|
db.DB().Omit(clause.Associations).Create(wh).Error,
|
||||||
|
)
|
||||||
|
|
||||||
|
return wh
|
||||||
|
}
|
||||||
|
|
||||||
|
// fetchEventBody runs the real download handler as the test user
|
||||||
|
// for the given source and event ids.
|
||||||
|
func fetchEventBody(
|
||||||
|
t *testing.T,
|
||||||
|
h *handlers.Handlers,
|
||||||
|
sess *session.Session,
|
||||||
|
sourceID, eventID string,
|
||||||
|
) *httptest.ResponseRecorder {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
// The path is escaped and the raw id goes in the route
|
||||||
|
// context, which is what chi hands a handler: the param is
|
||||||
|
// already percent-decoded by the time it is read.
|
||||||
|
req := httptest.NewRequestWithContext(
|
||||||
|
context.Background(),
|
||||||
|
http.MethodGet,
|
||||||
|
"/source/"+url.PathEscape(sourceID)+
|
||||||
|
"/logs/"+url.PathEscape(eventID)+"/body",
|
||||||
|
nil,
|
||||||
|
)
|
||||||
|
|
||||||
|
for _, c := range authenticatedCookies(
|
||||||
|
t, sess, deleteTestUserID, deleteTestUsername,
|
||||||
|
) {
|
||||||
|
req.AddCookie(c)
|
||||||
|
}
|
||||||
|
|
||||||
|
rctx := chi.NewRouteContext()
|
||||||
|
rctx.URLParams.Add(paramSourceID, sourceID)
|
||||||
|
rctx.URLParams.Add(paramEventID, eventID)
|
||||||
|
|
||||||
|
req = req.WithContext(
|
||||||
|
context.WithValue(
|
||||||
|
req.Context(), chi.RouteCtxKey, rctx,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
h.HandleEventBodyDownload().ServeHTTP(w, req)
|
||||||
|
|
||||||
|
return w
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleEventBodyDownload_ServesOversizeBodyInFull is the
|
||||||
|
// capability the render cap took away: a body far above what the
|
||||||
|
// event log page will show comes back whole and byte-identical,
|
||||||
|
// with the headers that keep it from being rendered.
|
||||||
|
func TestHandleEventBodyDownload_ServesOversizeBodyInFull(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var (
|
||||||
|
h *handlers.Handlers
|
||||||
|
sess *session.Session
|
||||||
|
db *database.Database
|
||||||
|
dbMgr *database.WebhookDBManager
|
||||||
|
)
|
||||||
|
|
||||||
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
||||||
|
app.RequireStart()
|
||||||
|
|
||||||
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
|
// Far above the render cap, with multibyte runes and a
|
||||||
|
// distinctive tail, so a body that the log page can only
|
||||||
|
// show a slice of comes back whole and in order.
|
||||||
|
const sentinel = "TAIL-SENTINEL-1f4a9c"
|
||||||
|
|
||||||
|
stored := strings.Repeat("A", 200*1024) +
|
||||||
|
strings.Repeat(snowman, 1000) + sentinel
|
||||||
|
|
||||||
|
wh := seedWebhook(t, db)
|
||||||
|
evt := seedEventWithBody(t, dbMgr, wh.ID, stored)
|
||||||
|
|
||||||
|
w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
|
||||||
|
|
||||||
|
require.Equal(t, http.StatusOK, w.Code)
|
||||||
|
assert.Greater(t, len(stored), bodyCap)
|
||||||
|
assert.Equal(t, stored, w.Body.String())
|
||||||
|
assert.Equal(
|
||||||
|
t, strconv.Itoa(len(stored)),
|
||||||
|
w.Header().Get("Content-Length"),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleEventBodyDownload_BodiesRoundTripByteIdentical
|
||||||
|
// covers the sizes and byte values a stored body can actually
|
||||||
|
// take: empty, one byte, either side of the render cap, and
|
||||||
|
// bytes that are not text at all. Content-Length has to equal
|
||||||
|
// the bytes written in every case, since it is derived from the
|
||||||
|
// same read that produces them.
|
||||||
|
func TestHandleEventBodyDownload_BodiesRoundTripByteIdentical(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// A NUL, invalid UTF-8 and a multibyte rune, so nothing on
|
||||||
|
// the path can be treating the body as text.
|
||||||
|
binary := "\x00\x01\xff\xfe" + snowman + "\x00tail"
|
||||||
|
|
||||||
|
cases := map[string]string{
|
||||||
|
"empty": "",
|
||||||
|
"single byte": "x",
|
||||||
|
"one below cap": strings.Repeat("b", bodyCap-1),
|
||||||
|
"exactly cap": strings.Repeat("c", bodyCap),
|
||||||
|
"one above cap": strings.Repeat("d", bodyCap+1),
|
||||||
|
"binary": binary,
|
||||||
|
}
|
||||||
|
|
||||||
|
for name, stored := range cases {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var (
|
||||||
|
h *handlers.Handlers
|
||||||
|
sess *session.Session
|
||||||
|
db *database.Database
|
||||||
|
dbMgr *database.WebhookDBManager
|
||||||
|
)
|
||||||
|
|
||||||
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
||||||
|
app.RequireStart()
|
||||||
|
|
||||||
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
|
wh := seedWebhook(t, db)
|
||||||
|
evt := seedEventWithBody(t, dbMgr, wh.ID, stored)
|
||||||
|
|
||||||
|
w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
|
||||||
|
|
||||||
|
require.Equal(t, http.StatusOK, w.Code)
|
||||||
|
assert.Equal(t, stored, w.Body.String())
|
||||||
|
assert.Equal(
|
||||||
|
t, strconv.Itoa(len(stored)),
|
||||||
|
w.Header().Get("Content-Length"),
|
||||||
|
)
|
||||||
|
assert.Equal(
|
||||||
|
t, len(stored), w.Body.Len(),
|
||||||
|
"Content-Length must equal bytes written",
|
||||||
|
)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleEventBodyDownload_HeadersAreNotRenderable pins the
|
||||||
|
// response headers that stop attacker-supplied bytes executing
|
||||||
|
// in the operator's own origin. They are a security control, not
|
||||||
|
// presentation.
|
||||||
|
func TestHandleEventBodyDownload_HeadersAreNotRenderable(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var (
|
||||||
|
h *handlers.Handlers
|
||||||
|
sess *session.Session
|
||||||
|
db *database.Database
|
||||||
|
dbMgr *database.WebhookDBManager
|
||||||
|
)
|
||||||
|
|
||||||
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
||||||
|
app.RequireStart()
|
||||||
|
|
||||||
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
|
wh := seedWebhook(t, db)
|
||||||
|
evt := seedEventWithBody(t, dbMgr, wh.ID, `{"small":true}`)
|
||||||
|
|
||||||
|
w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
|
||||||
|
|
||||||
|
require.Equal(t, http.StatusOK, w.Code)
|
||||||
|
assert.Equal(
|
||||||
|
t, "application/octet-stream",
|
||||||
|
w.Header().Get("Content-Type"),
|
||||||
|
)
|
||||||
|
assert.Equal(
|
||||||
|
t, "nosniff",
|
||||||
|
w.Header().Get("X-Content-Type-Options"),
|
||||||
|
)
|
||||||
|
|
||||||
|
disposition := w.Header().Get("Content-Disposition")
|
||||||
|
assert.Equal(
|
||||||
|
t,
|
||||||
|
`attachment; filename="webhooker-event-`+evt.ID+`.bin"`,
|
||||||
|
disposition,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleEventBodyDownload_ScriptBodyStaysInert proves a
|
||||||
|
// stored HTML payload is handed back as an attachment of opaque
|
||||||
|
// bytes rather than as anything a browser will execute. The
|
||||||
|
// bytes themselves are unaltered: this route reports what was
|
||||||
|
// delivered.
|
||||||
|
func TestHandleEventBodyDownload_ScriptBodyStaysInert(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var (
|
||||||
|
h *handlers.Handlers
|
||||||
|
sess *session.Session
|
||||||
|
db *database.Database
|
||||||
|
dbMgr *database.WebhookDBManager
|
||||||
|
)
|
||||||
|
|
||||||
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
||||||
|
app.RequireStart()
|
||||||
|
|
||||||
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
|
const payload = `<html><script>alert(document.cookie)` +
|
||||||
|
`</script></html>`
|
||||||
|
|
||||||
|
wh := seedWebhook(t, db)
|
||||||
|
evt := seedEventWithBody(t, dbMgr, wh.ID, payload)
|
||||||
|
|
||||||
|
w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
|
||||||
|
|
||||||
|
require.Equal(t, http.StatusOK, w.Code)
|
||||||
|
assert.Equal(t, payload, w.Body.String())
|
||||||
|
|
||||||
|
contentType := w.Header().Get("Content-Type")
|
||||||
|
assert.Equal(t, "application/octet-stream", contentType)
|
||||||
|
assert.NotContains(t, contentType, "html")
|
||||||
|
assert.NotContains(t, contentType, "xml")
|
||||||
|
assert.NotContains(t, contentType, "javascript")
|
||||||
|
assert.Contains(
|
||||||
|
t, w.Header().Get("Content-Disposition"), "attachment",
|
||||||
|
)
|
||||||
|
assert.Equal(
|
||||||
|
t, "nosniff",
|
||||||
|
w.Header().Get("X-Content-Type-Options"),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleEventBodyDownload_OtherUsersEvent404s is the
|
||||||
|
// authorization test the definition of done asks for: an event
|
||||||
|
// stored under a webhook the session user does not own is not
|
||||||
|
// readable, and the miss does not distinguish itself from a
|
||||||
|
// nonexistent one.
|
||||||
|
func TestHandleEventBodyDownload_OtherUsersEvent404s(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var (
|
||||||
|
h *handlers.Handlers
|
||||||
|
sess *session.Session
|
||||||
|
db *database.Database
|
||||||
|
dbMgr *database.WebhookDBManager
|
||||||
|
)
|
||||||
|
|
||||||
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
||||||
|
app.RequireStart()
|
||||||
|
|
||||||
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
|
const theirPayload = "OTHER-USERS-PAYLOAD-8b1d"
|
||||||
|
|
||||||
|
theirs := seedWebhookFor(t, db, otherTestUserID)
|
||||||
|
evt := seedEventWithBody(t, dbMgr, theirs.ID, theirPayload)
|
||||||
|
|
||||||
|
w := fetchEventBody(t, h, sess, theirs.ID, evt.ID)
|
||||||
|
|
||||||
|
assert.Equal(t, http.StatusNotFound, w.Code)
|
||||||
|
assert.NotContains(t, w.Body.String(), theirPayload)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleEventBodyDownload_EventOfAnotherWebhook404s pins
|
||||||
|
// that holding a valid event id is not enough: the event has to
|
||||||
|
// belong to the webhook in the path. Both webhooks here are the
|
||||||
|
// session user's and both have event databases, so the
|
||||||
|
// ownership check cannot be what produces the 404.
|
||||||
|
//
|
||||||
|
// What does produce it is the per-webhook database file rather
|
||||||
|
// than the webhook_id predicate on the query — removing that
|
||||||
|
// predicate leaves this test green, because the sibling's event
|
||||||
|
// is in a different file. The test is kept as the behavioural
|
||||||
|
// guard the route owes; see serveEventBody for which mechanism
|
||||||
|
// is load-bearing.
|
||||||
|
func TestHandleEventBodyDownload_EventOfAnotherWebhook404s(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var (
|
||||||
|
h *handlers.Handlers
|
||||||
|
sess *session.Session
|
||||||
|
db *database.Database
|
||||||
|
dbMgr *database.WebhookDBManager
|
||||||
|
)
|
||||||
|
|
||||||
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
||||||
|
app.RequireStart()
|
||||||
|
|
||||||
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
|
const other = "BELONGS-TO-THE-OTHER-WEBHOOK-3c7e"
|
||||||
|
|
||||||
|
mine := seedWebhook(t, db)
|
||||||
|
seedEventWithBody(t, dbMgr, mine.ID, `{"mine":true}`)
|
||||||
|
|
||||||
|
sibling := seedWebhook(t, db)
|
||||||
|
evt := seedEventWithBody(t, dbMgr, sibling.ID, other)
|
||||||
|
|
||||||
|
w := fetchEventBody(t, h, sess, mine.ID, evt.ID)
|
||||||
|
|
||||||
|
assert.Equal(t, http.StatusNotFound, w.Code)
|
||||||
|
assert.NotContains(t, w.Body.String(), other)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleEventBodyDownload_UnknownEvent404s covers the plain
|
||||||
|
// miss, including an id that is not a uuid at all and so never
|
||||||
|
// reaches the query or the response header.
|
||||||
|
func TestHandleEventBodyDownload_UnknownEvent404s(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var (
|
||||||
|
h *handlers.Handlers
|
||||||
|
sess *session.Session
|
||||||
|
db *database.Database
|
||||||
|
dbMgr *database.WebhookDBManager
|
||||||
|
)
|
||||||
|
|
||||||
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
||||||
|
app.RequireStart()
|
||||||
|
|
||||||
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
|
wh := seedWebhook(t, db)
|
||||||
|
seedEventWithBody(t, dbMgr, wh.ID, `{"mine":true}`)
|
||||||
|
|
||||||
|
for _, id := range []string{
|
||||||
|
uuid.New().String(),
|
||||||
|
`../../etc/passwd`,
|
||||||
|
"not-a-uuid",
|
||||||
|
`x"; rm -rf /`,
|
||||||
|
} {
|
||||||
|
w := fetchEventBody(t, h, sess, wh.ID, id)
|
||||||
|
|
||||||
|
assert.Equal(
|
||||||
|
t, http.StatusNotFound, w.Code,
|
||||||
|
"event id %q", id,
|
||||||
|
)
|
||||||
|
assert.Empty(
|
||||||
|
t, w.Header().Get("Content-Disposition"),
|
||||||
|
"event id %q must not reach a header", id,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleEventBodyDownload_ReapedEvent404s pins what happens
|
||||||
|
// when the retention reaper takes an event out from under this
|
||||||
|
// route. The body is read in one query before any header is
|
||||||
|
// written, so a reaped event cannot produce a partial download:
|
||||||
|
// it is a clean 404 with no Content-Length and no
|
||||||
|
// Content-Disposition. Both removals the codebase performs are
|
||||||
|
// covered — the reaper hard-deletes, and a soft-deleted row is
|
||||||
|
// excluded by the query's own deleted_at predicate rather than
|
||||||
|
// by GORM's default scope, which Raw bypasses.
|
||||||
|
func TestHandleEventBodyDownload_ReapedEvent404s(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
for name, hard := range map[string]bool{
|
||||||
|
"soft deleted": false,
|
||||||
|
"hard deleted": true,
|
||||||
|
} {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var (
|
||||||
|
h *handlers.Handlers
|
||||||
|
sess *session.Session
|
||||||
|
db *database.Database
|
||||||
|
dbMgr *database.WebhookDBManager
|
||||||
|
)
|
||||||
|
|
||||||
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
||||||
|
app.RequireStart()
|
||||||
|
|
||||||
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
|
const payload = "REAPED-PAYLOAD-4d2a"
|
||||||
|
|
||||||
|
wh := seedWebhook(t, db)
|
||||||
|
evt := seedEventWithBody(t, dbMgr, wh.ID, payload)
|
||||||
|
|
||||||
|
webhookDB, err := dbMgr.GetDB(wh.ID)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
del := webhookDB
|
||||||
|
if hard {
|
||||||
|
del = del.Unscoped()
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NoError(
|
||||||
|
t,
|
||||||
|
del.Delete(&database.Event{}, "id = ?", evt.ID).
|
||||||
|
Error,
|
||||||
|
)
|
||||||
|
|
||||||
|
w := fetchEventBody(t, h, sess, wh.ID, evt.ID)
|
||||||
|
|
||||||
|
assert.Equal(t, http.StatusNotFound, w.Code)
|
||||||
|
assert.NotContains(t, w.Body.String(), payload)
|
||||||
|
assert.Empty(t, w.Header().Get("Content-Length"))
|
||||||
|
assert.Empty(
|
||||||
|
t, w.Header().Get("Content-Disposition"),
|
||||||
|
)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandleSourceLogs_TruncationMarkerLinksToDownload proves
|
||||||
|
// the page tells the reader where the rest of the body is, and
|
||||||
|
// only when there is a rest to fetch.
|
||||||
|
func TestHandleSourceLogs_TruncationMarkerLinksToDownload(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
var (
|
||||||
|
h *handlers.Handlers
|
||||||
|
sess *session.Session
|
||||||
|
db *database.Database
|
||||||
|
dbMgr *database.WebhookDBManager
|
||||||
|
)
|
||||||
|
|
||||||
|
app := newTestApp(t, &h, &sess, &db, &dbMgr)
|
||||||
|
app.RequireStart()
|
||||||
|
|
||||||
|
t.Cleanup(app.RequireStop)
|
||||||
|
|
||||||
|
big := seedWebhook(t, db)
|
||||||
|
bigEvt := seedEventWithBody(
|
||||||
|
t, dbMgr, big.ID, strings.Repeat("A", 4*bodyCap),
|
||||||
|
)
|
||||||
|
|
||||||
|
page := renderSourceLogsPage(t, h, sess, big.ID)
|
||||||
|
assert.Contains(
|
||||||
|
t, page,
|
||||||
|
"/source/"+big.ID+"/logs/"+bigEvt.ID+"/body",
|
||||||
|
)
|
||||||
|
|
||||||
|
small := seedWebhook(t, db)
|
||||||
|
smallEvt := seedEventWithBody(
|
||||||
|
t, dbMgr, small.ID, `{"kept":"whole"}`,
|
||||||
|
)
|
||||||
|
|
||||||
|
page = renderSourceLogsPage(t, h, sess, small.ID)
|
||||||
|
assert.NotContains(
|
||||||
|
t, page,
|
||||||
|
"/source/"+small.ID+"/logs/"+smallEvt.ID+"/body",
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -25,13 +25,14 @@ const bodyCap = handlers.MaxRenderedBodyBytesForTest
|
|||||||
const snowman = "☃"
|
const snowman = "☃"
|
||||||
|
|
||||||
// seedEventWithBody records one event with the given body in the
|
// seedEventWithBody records one event with the given body in the
|
||||||
// webhook's own database.
|
// webhook's own database and returns it, so a caller that needs
|
||||||
|
// the generated event id can have it.
|
||||||
func seedEventWithBody(
|
func seedEventWithBody(
|
||||||
t *testing.T,
|
t *testing.T,
|
||||||
dbMgr *database.WebhookDBManager,
|
dbMgr *database.WebhookDBManager,
|
||||||
webhookID string,
|
webhookID string,
|
||||||
body string,
|
body string,
|
||||||
) {
|
) *database.Event {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
|
|
||||||
webhookDB, err := dbMgr.GetDB(webhookID)
|
webhookDB, err := dbMgr.GetDB(webhookID)
|
||||||
@@ -47,6 +48,8 @@ func seedEventWithBody(
|
|||||||
require.NoError(t, webhookDB.Omit(
|
require.NoError(t, webhookDB.Omit(
|
||||||
clause.Associations,
|
clause.Associations,
|
||||||
).Create(event).Error)
|
).Create(event).Error)
|
||||||
|
|
||||||
|
return event
|
||||||
}
|
}
|
||||||
|
|
||||||
// seedAndProject stores one body and returns the projection the
|
// seedAndProject stores one body and returns the projection the
|
||||||
|
|||||||
@@ -713,29 +713,55 @@ func (h *Handlers) evictArchiveWriterIfUnused(webhookID string) {
|
|||||||
h.evictArchiveWriter(webhookID)
|
h.evictArchiveWriter(webhookID)
|
||||||
}
|
}
|
||||||
|
|
||||||
// HandleSourceLogs shows the request/response logs for a
|
// ownedWebhook resolves the request's sourceID parameter to a
|
||||||
// webhook.
|
// webhook the session's user owns.
|
||||||
func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
|
//
|
||||||
return func(w http.ResponseWriter, r *http.Request) {
|
// Ownership and existence are decided by one query, so a
|
||||||
|
// webhook belonging to another user is indistinguishable from
|
||||||
|
// one that does not exist: both are a 404, and neither confirms
|
||||||
|
// the id. Callers that reach further into a webhook's data —
|
||||||
|
// the event log page and the event body download — share this
|
||||||
|
// one check rather than restating it, so the download cannot
|
||||||
|
// come to authorize differently from the page that links to it.
|
||||||
|
//
|
||||||
|
// It reports false once it has written the response, which is a
|
||||||
|
// redirect to the login page for an unauthenticated request and
|
||||||
|
// a 404 otherwise. The caller returns without writing more.
|
||||||
|
func (h *Handlers) ownedWebhook(
|
||||||
|
w http.ResponseWriter,
|
||||||
|
r *http.Request,
|
||||||
|
) (database.Webhook, bool) {
|
||||||
|
var webhook database.Webhook
|
||||||
|
|
||||||
userID, ok := h.getUserID(r)
|
userID, ok := h.getUserID(r)
|
||||||
if !ok {
|
if !ok {
|
||||||
http.Redirect(
|
http.Redirect(
|
||||||
w, r, "/pages/login", http.StatusSeeOther,
|
w, r, "/pages/login", http.StatusSeeOther,
|
||||||
)
|
)
|
||||||
|
|
||||||
return
|
return database.Webhook{}, false
|
||||||
}
|
}
|
||||||
|
|
||||||
sourceID := chi.URLParam(r, "sourceID")
|
sourceID := chi.URLParam(r, "sourceID")
|
||||||
|
|
||||||
var webhook database.Webhook
|
|
||||||
|
|
||||||
err := h.db.DB().Where(
|
err := h.db.DB().Where(
|
||||||
"id = ? AND user_id = ?", sourceID, userID,
|
"id = ? AND user_id = ?", sourceID, userID,
|
||||||
).First(&webhook).Error
|
).First(&webhook).Error
|
||||||
if err != nil {
|
if err != nil {
|
||||||
http.NotFound(w, r)
|
http.NotFound(w, r)
|
||||||
|
|
||||||
|
return database.Webhook{}, false
|
||||||
|
}
|
||||||
|
|
||||||
|
return webhook, true
|
||||||
|
}
|
||||||
|
|
||||||
|
// HandleSourceLogs shows the request/response logs for a
|
||||||
|
// webhook.
|
||||||
|
func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
|
||||||
|
return func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
webhook, ok := h.ownedWebhook(w, r)
|
||||||
|
if !ok {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
658
internal/middleware/accesslog_test.go
Normal file
658
internal/middleware/accesslog_test.go
Normal file
@@ -0,0 +1,658 @@
|
|||||||
|
package middleware_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"context"
|
||||||
|
"encoding/json"
|
||||||
|
"log/slog"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/go-chi/chi"
|
||||||
|
chimw "github.com/go-chi/chi/middleware"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"sneak.berlin/go/webhooker/internal/config"
|
||||||
|
"sneak.berlin/go/webhooker/internal/middleware"
|
||||||
|
)
|
||||||
|
|
||||||
|
// floodRequests is the number of distinct invented paths each flood
|
||||||
|
// test drives through the access log.
|
||||||
|
const floodRequests = 64
|
||||||
|
|
||||||
|
// attackerMarker is embedded in every invented path. No access log
|
||||||
|
// line for a redirected or rejected request may contain it.
|
||||||
|
const attackerMarker = "QQATTACKERTEXTQQ"
|
||||||
|
|
||||||
|
// maxLineBytes bounds a single access log line whose client-supplied
|
||||||
|
// fields are of ordinary size. Well above what the fixed fields need,
|
||||||
|
// well below the length of the oversized input the amplification tests
|
||||||
|
// send.
|
||||||
|
const maxLineBytes = 1024
|
||||||
|
|
||||||
|
// maxCappedLineBytes bounds a single access log line when every
|
||||||
|
// client-supplied field arrives oversized and is truncated to its
|
||||||
|
// budget. This is the number the README quotes as the per-line cost an
|
||||||
|
// operator sizes log storage against, and it is a bound on the
|
||||||
|
// ENCODED line, which is what the operator's disk holds.
|
||||||
|
const maxCappedLineBytes = 2560
|
||||||
|
|
||||||
|
// oversizedSegmentBytes is the length of the single attacker-chosen
|
||||||
|
// path segment, query string or header used to show line size does not
|
||||||
|
// track input size.
|
||||||
|
const oversizedSegmentBytes = 8192
|
||||||
|
|
||||||
|
// tailMarker is placed at the END of an oversized header value, so its
|
||||||
|
// absence from the log proves the value was truncated rather than
|
||||||
|
// merely being short.
|
||||||
|
const tailMarker = "QQTRUNCATEDTAILQQ"
|
||||||
|
|
||||||
|
// These mirror the middleware's own budgets, which are unexported.
|
||||||
|
// They are duplicated rather than exported so that widening a budget
|
||||||
|
// in the middleware has to be restated here deliberately.
|
||||||
|
const (
|
||||||
|
maxFieldBytes = 512
|
||||||
|
maxRequestIDBytes = 128
|
||||||
|
maxMethodBytes = 32
|
||||||
|
truncationSuffix = "[truncated]"
|
||||||
|
unmatchedRouteLiteral = "(unmatched)"
|
||||||
|
)
|
||||||
|
|
||||||
|
// capturingMiddleware returns a Middleware whose logger writes JSON
|
||||||
|
// lines into the returned buffer, so the access log can be asserted
|
||||||
|
// on directly.
|
||||||
|
func capturingMiddleware(t *testing.T) (*middleware.Middleware, *bytes.Buffer) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
buf := new(bytes.Buffer)
|
||||||
|
log := slog.New(slog.NewJSONHandler(
|
||||||
|
buf,
|
||||||
|
&slog.HandlerOptions{Level: slog.LevelInfo},
|
||||||
|
))
|
||||||
|
|
||||||
|
cfg := &config.Config{Environment: config.EnvironmentDev}
|
||||||
|
|
||||||
|
return middleware.NewForTest(log, cfg, nil), buf
|
||||||
|
}
|
||||||
|
|
||||||
|
// capturingTextMiddleware is capturingMiddleware for the other handler
|
||||||
|
// internal/logger can select: slog's text handler, which
|
||||||
|
// internal/logger/logger.go installs when stderr is a tty. It escapes
|
||||||
|
// differently from the JSON one, so the line bound has to be asserted
|
||||||
|
// against both.
|
||||||
|
func capturingTextMiddleware(
|
||||||
|
t *testing.T,
|
||||||
|
) (*middleware.Middleware, *bytes.Buffer) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
buf := new(bytes.Buffer)
|
||||||
|
log := slog.New(slog.NewTextHandler(
|
||||||
|
buf,
|
||||||
|
&slog.HandlerOptions{Level: slog.LevelInfo},
|
||||||
|
))
|
||||||
|
|
||||||
|
cfg := &config.Config{Environment: config.EnvironmentDev}
|
||||||
|
|
||||||
|
return middleware.NewForTest(log, cfg, nil), buf
|
||||||
|
}
|
||||||
|
|
||||||
|
// accessLogRouter mirrors the production route shapes that an
|
||||||
|
// unauthenticated client can reach: the public receiver, the
|
||||||
|
// authenticated profile route (which redirects to login rather than
|
||||||
|
// rejecting outright), the health check (which answers 200 to anyone,
|
||||||
|
// behind no rate limiter at all), and a plain static route.
|
||||||
|
func accessLogRouter(m *middleware.Middleware) *chi.Mux {
|
||||||
|
router := chi.NewRouter()
|
||||||
|
// Production registers RequestID ahead of Logging, and chi's
|
||||||
|
// RequestID passes an inbound X-Request-Id header straight
|
||||||
|
// through, so the request_id field is client-supplied too.
|
||||||
|
router.Use(chimw.RequestID)
|
||||||
|
router.Use(m.Logging())
|
||||||
|
|
||||||
|
router.Get(
|
||||||
|
"/.well-known/healthcheck",
|
||||||
|
func(w http.ResponseWriter, _ *http.Request) {
|
||||||
|
w.WriteHeader(http.StatusOK)
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
router.HandleFunc(
|
||||||
|
"/webhook/{uuid}",
|
||||||
|
func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
// Stands in for the real handler: an unknown entrypoint
|
||||||
|
// UUID 404s, a known one succeeds.
|
||||||
|
if chi.URLParam(r, "uuid") != "known" {
|
||||||
|
http.Error(w, "not found", http.StatusNotFound)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
w.WriteHeader(http.StatusOK)
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
router.Route("/user/{username}", func(r chi.Router) {
|
||||||
|
r.Get("/", func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
http.Redirect(
|
||||||
|
w, r, "/pages/login", http.StatusSeeOther,
|
||||||
|
)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
boom := func(w http.ResponseWriter, _ *http.Request) {
|
||||||
|
http.Error(w, "boom", http.StatusInternalServerError)
|
||||||
|
}
|
||||||
|
|
||||||
|
router.Get("/boom", boom)
|
||||||
|
// The 5xx branch keeps the concrete path, so it needs a route that
|
||||||
|
// answers 500 to a path of the client's choosing: that is where the
|
||||||
|
// url field and the header fields are both at their budget on the
|
||||||
|
// same line.
|
||||||
|
router.Get("/boom/*", boom)
|
||||||
|
|
||||||
|
return router
|
||||||
|
}
|
||||||
|
|
||||||
|
// accessLogEntries decodes the captured buffer into one map per
|
||||||
|
// logged line, holding every line to maxLineBytes.
|
||||||
|
func accessLogEntries(
|
||||||
|
t *testing.T,
|
||||||
|
buf *bytes.Buffer,
|
||||||
|
) []map[string]any {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
return accessLogEntriesWithin(t, buf, maxLineBytes)
|
||||||
|
}
|
||||||
|
|
||||||
|
// accessLogEntriesWithin decodes the captured buffer into one map per
|
||||||
|
// logged line, holding every line to bound bytes.
|
||||||
|
func accessLogEntriesWithin(
|
||||||
|
t *testing.T,
|
||||||
|
buf *bytes.Buffer,
|
||||||
|
bound int,
|
||||||
|
) []map[string]any {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
var entries []map[string]any
|
||||||
|
|
||||||
|
for line := range strings.SplitSeq(
|
||||||
|
strings.TrimSpace(buf.String()), "\n",
|
||||||
|
) {
|
||||||
|
if line == "" {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
require.LessOrEqual(
|
||||||
|
t, len(line), bound,
|
||||||
|
"access log line exceeded its bound",
|
||||||
|
)
|
||||||
|
|
||||||
|
var entry map[string]any
|
||||||
|
|
||||||
|
require.NoError(t, json.Unmarshal([]byte(line), &entry))
|
||||||
|
|
||||||
|
entries = append(entries, entry)
|
||||||
|
}
|
||||||
|
|
||||||
|
return entries
|
||||||
|
}
|
||||||
|
|
||||||
|
// get drives one GET through the router.
|
||||||
|
func get(t *testing.T, router *chi.Mux, target string) int {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
return getWithHeaders(t, router, target, nil)
|
||||||
|
}
|
||||||
|
|
||||||
|
// getWithHeaders drives one GET through the router with the supplied
|
||||||
|
// request headers set.
|
||||||
|
func getWithHeaders(
|
||||||
|
t *testing.T,
|
||||||
|
router *chi.Mux,
|
||||||
|
target string,
|
||||||
|
headers map[string]string,
|
||||||
|
) int {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
req := httptest.NewRequestWithContext(
|
||||||
|
context.Background(), http.MethodGet, target, nil,
|
||||||
|
)
|
||||||
|
|
||||||
|
for name, value := range headers {
|
||||||
|
req.Header.Set(name, value)
|
||||||
|
}
|
||||||
|
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
router.ServeHTTP(w, req)
|
||||||
|
|
||||||
|
return w.Code
|
||||||
|
}
|
||||||
|
|
||||||
|
// assertFloodIsBounded drives floodRequests distinct invented paths
|
||||||
|
// built by pathFor and asserts every logged line names wantURL, that
|
||||||
|
// none carries the invented text, and that the line count is exactly
|
||||||
|
// one per request.
|
||||||
|
func assertFloodIsBounded(
|
||||||
|
t *testing.T,
|
||||||
|
pathFor func(i int) string,
|
||||||
|
wantStatus int,
|
||||||
|
wantURL string,
|
||||||
|
) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
m, buf := capturingMiddleware(t)
|
||||||
|
router := accessLogRouter(m)
|
||||||
|
|
||||||
|
for i := range floodRequests {
|
||||||
|
assert.Equal(t, wantStatus, get(t, router, pathFor(i)))
|
||||||
|
}
|
||||||
|
|
||||||
|
assert.NotContains(
|
||||||
|
t, buf.String(), attackerMarker,
|
||||||
|
"access log carried attacker-chosen path text",
|
||||||
|
)
|
||||||
|
|
||||||
|
entries := accessLogEntries(t, buf)
|
||||||
|
require.Len(t, entries, floodRequests)
|
||||||
|
|
||||||
|
for _, entry := range entries {
|
||||||
|
assert.Equal(t, wantURL, entry["url"])
|
||||||
|
assert.InDelta(
|
||||||
|
t, float64(wantStatus), entry["status"], 0,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestAccessLog_InventedReceiverPathsLogRoutePattern(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
assertFloodIsBounded(
|
||||||
|
t,
|
||||||
|
func(i int) string {
|
||||||
|
return "/webhook/" + attackerMarker +
|
||||||
|
strings.Repeat("x", i) + "?q=" + attackerMarker
|
||||||
|
},
|
||||||
|
http.StatusNotFound,
|
||||||
|
"/webhook/{uuid}",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestAccessLog_InventedProfilePathsLogRoutePattern(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// The login redirect is a 3xx, not a 4xx, but it is just as free
|
||||||
|
// for an unauthenticated client to drive with invented input.
|
||||||
|
// The doubled slash is what chi's RoutePattern yields for a
|
||||||
|
// mounted subrouter's index route.
|
||||||
|
assertFloodIsBounded(
|
||||||
|
t,
|
||||||
|
func(i int) string {
|
||||||
|
return "/user/" + attackerMarker +
|
||||||
|
strings.Repeat("x", i) + "/"
|
||||||
|
},
|
||||||
|
http.StatusSeeOther,
|
||||||
|
"/user/{username}//",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestAccessLog_UnroutablePathsLogFixedLiteral(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
assertFloodIsBounded(
|
||||||
|
t,
|
||||||
|
func(i int) string {
|
||||||
|
return "/" + attackerMarker + strings.Repeat("x", i)
|
||||||
|
},
|
||||||
|
http.StatusNotFound,
|
||||||
|
"(unmatched)",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// oversizedValue builds an 8 KB header value out of repetitions of ch,
|
||||||
|
// with the tail marker at its end.
|
||||||
|
//
|
||||||
|
// The leading 'x' is load-bearing for tab: net/textproto strips leading
|
||||||
|
// and trailing whitespace from a header value, so a value that were
|
||||||
|
// nothing but tabs would arrive empty over a real connection and the
|
||||||
|
// case would prove nothing.
|
||||||
|
func oversizedValue(ch string) string {
|
||||||
|
return "x" + strings.Repeat(ch, oversizedSegmentBytes) + tailMarker
|
||||||
|
}
|
||||||
|
|
||||||
|
// oversizedHeaders fills every client-supplied header the access log
|
||||||
|
// reads with the same value.
|
||||||
|
func oversizedHeaders(value string) map[string]string {
|
||||||
|
return map[string]string{
|
||||||
|
"User-Agent": value,
|
||||||
|
"Referer": value,
|
||||||
|
"X-Request-Id": value,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// sizeCase is one way of pointing 8 KB of client-chosen text at the
|
||||||
|
// access log.
|
||||||
|
type sizeCase struct {
|
||||||
|
target string
|
||||||
|
headers map[string]string
|
||||||
|
wantStatus int
|
||||||
|
wantURL string
|
||||||
|
bound int
|
||||||
|
}
|
||||||
|
|
||||||
|
// lineSizeCases enumerates every part of a request that reaches the
|
||||||
|
// access log, at 8 KB apiece.
|
||||||
|
func lineSizeCases() map[string]sizeCase {
|
||||||
|
cases := map[string]sizeCase{
|
||||||
|
"oversized path segment": {
|
||||||
|
target: "/webhook/" + attackerMarker +
|
||||||
|
strings.Repeat("x", oversizedSegmentBytes),
|
||||||
|
wantStatus: http.StatusNotFound,
|
||||||
|
wantURL: "/webhook/{uuid}",
|
||||||
|
bound: maxLineBytes,
|
||||||
|
},
|
||||||
|
// /.well-known/healthcheck answers 200 to anyone and has no
|
||||||
|
// rate limiter in front of it, so an oversized query appended
|
||||||
|
// to it would otherwise buy the same amplification as an
|
||||||
|
// invented 404 path, unauthenticated and unthrottled.
|
||||||
|
"oversized query on an unauthenticated 200": {
|
||||||
|
target: "/.well-known/healthcheck?q=" + attackerMarker +
|
||||||
|
strings.Repeat("x", oversizedSegmentBytes),
|
||||||
|
wantStatus: http.StatusOK,
|
||||||
|
wantURL: "/.well-known/healthcheck?(redacted)",
|
||||||
|
bound: maxLineBytes,
|
||||||
|
},
|
||||||
|
// These reach the line on every request, including one whose
|
||||||
|
// url field is correctly redacted.
|
||||||
|
"oversized headers": {
|
||||||
|
target: "/" + attackerMarker,
|
||||||
|
headers: oversizedHeaders(oversizedValue("h")),
|
||||||
|
wantStatus: http.StatusNotFound,
|
||||||
|
wantURL: unmatchedRouteLiteral,
|
||||||
|
bound: maxCappedLineBytes,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
// The url field on a 5xx keeps the concrete path, so it reaches its
|
||||||
|
// own budget on the same line as the three header fields. That is
|
||||||
|
// the widest line the service can be made to write.
|
||||||
|
longPath := "/boom/" + strings.Repeat("x", oversizedSegmentBytes)
|
||||||
|
wantLongURL := longPath[:maxFieldBytes] + truncationSuffix
|
||||||
|
|
||||||
|
// escapeChars are the runes Go's header parser accepts in a header
|
||||||
|
// value and the log handler then escapes, coming out wider than
|
||||||
|
// they went in. A budget counted in raw bytes lets any of them buy
|
||||||
|
// a field several times its nominal size, so every one of them
|
||||||
|
// gets a case.
|
||||||
|
//
|
||||||
|
// The astral one is the case the JSON handler alone does not
|
||||||
|
// reach: U+1000C is unassigned, so it is non-printable, and
|
||||||
|
// strconv.Quote spells a non-printable rune at or above U+10000
|
||||||
|
// as a ten-byte \UXXXXXXXX. The JSON handler passes it through as
|
||||||
|
// its four UTF-8 bytes, so only the text-handler shape of this
|
||||||
|
// test holds the ten-byte charge honest.
|
||||||
|
escapeChars := map[string]string{
|
||||||
|
"quote": `"`,
|
||||||
|
"backslash": `\`,
|
||||||
|
"tab": "\t",
|
||||||
|
"astral": "\U0001000C",
|
||||||
|
}
|
||||||
|
|
||||||
|
for kind, char := range escapeChars {
|
||||||
|
fill := oversizedValue(char)
|
||||||
|
|
||||||
|
cases["oversized "+kind+" headers"] = sizeCase{
|
||||||
|
target: "/" + attackerMarker,
|
||||||
|
headers: oversizedHeaders(fill),
|
||||||
|
wantStatus: http.StatusNotFound,
|
||||||
|
wantURL: unmatchedRouteLiteral,
|
||||||
|
bound: maxCappedLineBytes,
|
||||||
|
}
|
||||||
|
|
||||||
|
cases["oversized "+kind+" headers with a 5xx concrete url"] =
|
||||||
|
sizeCase{
|
||||||
|
target: longPath,
|
||||||
|
headers: oversizedHeaders(fill),
|
||||||
|
wantStatus: http.StatusInternalServerError,
|
||||||
|
wantURL: wantLongURL,
|
||||||
|
bound: maxCappedLineBytes,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return cases
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestAccessLog_LineSizeDoesNotTrackInputSize drives 8 KB of
|
||||||
|
// client-chosen text at the access log through each part of the
|
||||||
|
// request that reaches it, and holds the resulting line to a fixed
|
||||||
|
// bound in every case.
|
||||||
|
//
|
||||||
|
// The bound is on the ENCODED line, so the cases built out of
|
||||||
|
// characters the handler escapes are the ones that matter: a budget
|
||||||
|
// spent in raw bytes passes every plain-ASCII case here and still
|
||||||
|
// writes a line half again as long as the stated ceiling.
|
||||||
|
func TestAccessLog_LineSizeDoesNotTrackInputSize(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
require.Equal(
|
||||||
|
t, middleware.MaxAccessLogLineBytes, maxCappedLineBytes,
|
||||||
|
"the README quotes this ceiling and the middleware derives "+
|
||||||
|
"it; they have to agree",
|
||||||
|
)
|
||||||
|
|
||||||
|
for name, tc := range lineSizeCases() {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
m, buf := capturingMiddleware(t)
|
||||||
|
router := accessLogRouter(m)
|
||||||
|
|
||||||
|
assert.Equal(
|
||||||
|
t,
|
||||||
|
tc.wantStatus,
|
||||||
|
getWithHeaders(t, router, tc.target, tc.headers),
|
||||||
|
)
|
||||||
|
|
||||||
|
// accessLogEntriesWithin enforces the bound, which is
|
||||||
|
// orders of magnitude smaller than the input just sent.
|
||||||
|
entries := accessLogEntriesWithin(t, buf, tc.bound)
|
||||||
|
require.Len(t, entries, 1)
|
||||||
|
assert.Equal(t, tc.wantURL, entries[0]["url"])
|
||||||
|
|
||||||
|
// The markers sit at the far end of the client-chosen
|
||||||
|
// text, so their absence is what proves the redaction and
|
||||||
|
// the truncation actually ran.
|
||||||
|
assert.NotContains(
|
||||||
|
t, buf.String(), attackerMarker,
|
||||||
|
"access log carried attacker-chosen text",
|
||||||
|
)
|
||||||
|
assert.NotContains(
|
||||||
|
t, buf.String(), tailMarker,
|
||||||
|
"access log carried an untruncated client field",
|
||||||
|
)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestAccessLog_LineSizeDoesNotTrackInputSizeOnTheTextHandler runs the
|
||||||
|
// same cases through slog's text handler, which internal/logger
|
||||||
|
// selects on a tty.
|
||||||
|
//
|
||||||
|
// MaxAccessLogLineBytes is quoted to operators unqualified, so it has
|
||||||
|
// to hold for whichever handler is installed — and the two do not
|
||||||
|
// escape alike. The astral case is the one that separates them: the
|
||||||
|
// JSON handler emits U+1000C as its four UTF-8 bytes, while
|
||||||
|
// strconv.Quote spells it \U0001000C at ten. Charging six for it, as
|
||||||
|
// this code did, put a real 2,676-byte line on the wire here while
|
||||||
|
// every JSON case stayed comfortably inside the bound.
|
||||||
|
//
|
||||||
|
// Only the size bound is asserted; the url field's contents are the
|
||||||
|
// JSON shape's business above.
|
||||||
|
func TestAccessLog_LineSizeDoesNotTrackInputSizeOnTheTextHandler(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
for name, tc := range lineSizeCases() {
|
||||||
|
t.Run(name, func(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
m, buf := capturingTextMiddleware(t)
|
||||||
|
router := accessLogRouter(m)
|
||||||
|
|
||||||
|
assert.Equal(
|
||||||
|
t,
|
||||||
|
tc.wantStatus,
|
||||||
|
getWithHeaders(t, router, tc.target, tc.headers),
|
||||||
|
)
|
||||||
|
|
||||||
|
line := strings.TrimSpace(buf.String())
|
||||||
|
|
||||||
|
require.NotEmpty(t, line)
|
||||||
|
assert.NotContains(
|
||||||
|
t, line, "\n", "expected exactly one log line",
|
||||||
|
)
|
||||||
|
require.LessOrEqual(
|
||||||
|
t, len(line), tc.bound,
|
||||||
|
"access log line exceeded its bound",
|
||||||
|
)
|
||||||
|
assert.Contains(t, line, "url=")
|
||||||
|
assert.NotContains(
|
||||||
|
t, line, attackerMarker,
|
||||||
|
"access log carried attacker-chosen text",
|
||||||
|
)
|
||||||
|
assert.NotContains(
|
||||||
|
t, line, tailMarker,
|
||||||
|
"access log carried an untruncated client field",
|
||||||
|
)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestAccessLog_OversizedMethodIsTruncated covers the last term in the
|
||||||
|
// MaxAccessLogLineBytes arithmetic that the size cases above cannot
|
||||||
|
// reach: Go accepts any RFC 7230 token as a method, and getWithHeaders
|
||||||
|
// only ever sends GET.
|
||||||
|
func TestAccessLog_OversizedMethodIsTruncated(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
m, buf := capturingMiddleware(t)
|
||||||
|
router := accessLogRouter(m)
|
||||||
|
|
||||||
|
method := strings.Repeat("M", oversizedSegmentBytes) + attackerMarker
|
||||||
|
req := httptest.NewRequestWithContext(
|
||||||
|
context.Background(), method, "/"+attackerMarker, nil,
|
||||||
|
)
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
router.ServeHTTP(rec, req)
|
||||||
|
|
||||||
|
entries := accessLogEntriesWithin(t, buf, maxLineBytes)
|
||||||
|
require.Len(t, entries, 1)
|
||||||
|
assert.Equal(
|
||||||
|
t,
|
||||||
|
strings.Repeat("M", maxMethodBytes)+truncationSuffix,
|
||||||
|
entries[0]["method"],
|
||||||
|
)
|
||||||
|
assert.NotContains(
|
||||||
|
t, buf.String(), attackerMarker,
|
||||||
|
"access log carried attacker-chosen text",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestAccessLog_OversizedHeadersKeepATruncatedPrefix checks the other
|
||||||
|
// half of the header cap: the fields are cut, not dropped, so a
|
||||||
|
// truncated User-Agent is still worth reading.
|
||||||
|
func TestAccessLog_OversizedHeadersKeepATruncatedPrefix(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
m, buf := capturingMiddleware(t)
|
||||||
|
router := accessLogRouter(m)
|
||||||
|
|
||||||
|
assert.Equal(
|
||||||
|
t,
|
||||||
|
http.StatusNotFound,
|
||||||
|
getWithHeaders(
|
||||||
|
t, router, "/nope",
|
||||||
|
oversizedHeaders(oversizedValue("h")),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
|
||||||
|
entries := accessLogEntriesWithin(t, buf, maxCappedLineBytes)
|
||||||
|
require.Len(t, entries, 1)
|
||||||
|
|
||||||
|
for key, budget := range map[string]int{
|
||||||
|
"useragent": maxFieldBytes,
|
||||||
|
"referer": maxFieldBytes,
|
||||||
|
"request_id": maxRequestIDBytes,
|
||||||
|
} {
|
||||||
|
value, ok := entries[0][key].(string)
|
||||||
|
require.True(t, ok, key)
|
||||||
|
assert.LessOrEqual(
|
||||||
|
t, len(value), budget+len(truncationSuffix), key,
|
||||||
|
)
|
||||||
|
assert.Contains(t, value, truncationSuffix, key)
|
||||||
|
assert.Contains(t, value, "hhhh", key)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestAccessLog_SuccessKeepsConcretePathAndRedactsQuery(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
m, buf := capturingMiddleware(t)
|
||||||
|
router := accessLogRouter(m)
|
||||||
|
|
||||||
|
assert.Equal(
|
||||||
|
t, http.StatusOK, get(t, router, "/webhook/known?src=ci"),
|
||||||
|
)
|
||||||
|
|
||||||
|
// The path resolved against a stored entrypoint, so it stays. The
|
||||||
|
// query never does: see TestAccessLog_UnauthenticatedSuccess...
|
||||||
|
entries := accessLogEntries(t, buf)
|
||||||
|
require.Len(t, entries, 1)
|
||||||
|
assert.Equal(t, "/webhook/known?(redacted)", entries[0]["url"])
|
||||||
|
assert.NotContains(t, buf.String(), "src=ci")
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestAccessLog_ServerErrorKeepsConcreteURL(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
m, buf := capturingMiddleware(t)
|
||||||
|
router := accessLogRouter(m)
|
||||||
|
|
||||||
|
assert.Equal(
|
||||||
|
t, http.StatusInternalServerError, get(t, router, "/boom"),
|
||||||
|
)
|
||||||
|
|
||||||
|
entries := accessLogEntries(t, buf)
|
||||||
|
require.Len(t, entries, 1)
|
||||||
|
assert.Equal(t, "/boom", entries[0]["url"])
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestAccessLog_RetainsEveryOtherField(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
m, buf := capturingMiddleware(t)
|
||||||
|
router := accessLogRouter(m)
|
||||||
|
|
||||||
|
assert.Equal(
|
||||||
|
t,
|
||||||
|
http.StatusNotFound,
|
||||||
|
get(t, router, "/webhook/"+attackerMarker),
|
||||||
|
)
|
||||||
|
|
||||||
|
entries := accessLogEntries(t, buf)
|
||||||
|
require.Len(t, entries, 1)
|
||||||
|
|
||||||
|
for _, key := range []string{
|
||||||
|
"request_start", "method", "url", "useragent", "request_id",
|
||||||
|
"referer", "proto", "remoteIP", "status", "latency_ms",
|
||||||
|
} {
|
||||||
|
assert.Contains(t, entries[0], key)
|
||||||
|
}
|
||||||
|
|
||||||
|
assert.Equal(t, http.MethodGet, entries[0]["method"])
|
||||||
|
assert.Equal(t, "HTTP/1.1", entries[0]["proto"])
|
||||||
|
}
|
||||||
@@ -50,6 +50,10 @@ const LoginFailureMaxKeysConst = loginFailureMaxKeys
|
|||||||
// Argon2id verifications.
|
// Argon2id verifications.
|
||||||
const PasswordVerifyConcurrencyConst = passwordVerifyConcurrency
|
const PasswordVerifyConcurrencyConst = passwordVerifyConcurrency
|
||||||
|
|
||||||
|
// PasswordVerifyMaxWaitersConst exposes the bound on how many
|
||||||
|
// requests may queue for a verification slot.
|
||||||
|
const PasswordVerifyMaxWaitersConst = passwordVerifyMaxWaiters
|
||||||
|
|
||||||
// LoginGuard is the login failure counter and verification
|
// LoginGuard is the login failure counter and verification
|
||||||
// semaphore, exposed for direct testing.
|
// semaphore, exposed for direct testing.
|
||||||
type LoginGuard = loginGuard
|
type LoginGuard = loginGuard
|
||||||
@@ -58,14 +62,20 @@ type LoginGuard = loginGuard
|
|||||||
func NewLoginGuardForTest(
|
func NewLoginGuardForTest(
|
||||||
limit int,
|
limit int,
|
||||||
interval time.Duration,
|
interval time.Duration,
|
||||||
maxKeys, concurrency int,
|
maxKeys, concurrency, maxWaiters int,
|
||||||
wait time.Duration,
|
wait time.Duration,
|
||||||
) *LoginGuard {
|
) *LoginGuard {
|
||||||
return newLoginGuard(
|
return newLoginGuard(
|
||||||
limit, interval, maxKeys, concurrency, wait,
|
limit, interval, maxKeys, concurrency, maxWaiters, wait,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// QueuedWaitersForTest reports how many requests are currently
|
||||||
|
// queued for a verification slot.
|
||||||
|
func (g *LoginGuard) QueuedWaitersForTest() int {
|
||||||
|
return len(g.queue)
|
||||||
|
}
|
||||||
|
|
||||||
// SetNowForTest replaces the guard's clock.
|
// SetNowForTest replaces the guard's clock.
|
||||||
func (g *LoginGuard) SetNowForTest(now func() time.Time) {
|
func (g *LoginGuard) SetNowForTest(now func() time.Time) {
|
||||||
g.mu.Lock()
|
g.mu.Lock()
|
||||||
|
|||||||
@@ -44,6 +44,32 @@ const (
|
|||||||
// whole. The wait is well inside the 60s request timeout.
|
// whole. The wait is well inside the 60s request timeout.
|
||||||
passwordVerifyWait = 5 * time.Second
|
passwordVerifyWait = 5 * time.Second
|
||||||
|
|
||||||
|
// passwordVerifyMaxWaiters bounds how many requests may be
|
||||||
|
// queued for a slot at once. Past it, acquire sheds immediately
|
||||||
|
// with 503 instead of joining the queue.
|
||||||
|
//
|
||||||
|
// The wait bounds how long one request occupies memory; this
|
||||||
|
// bounds how many do so at the same time, and without it the
|
||||||
|
// 128 MB hashing budget above is the smaller half of the real
|
||||||
|
// footprint. A waiter is not free: by the time it reaches the
|
||||||
|
// guard its form is parsed, so it holds up to maxFormBodySize —
|
||||||
|
// 1 MB — for as long as it waits. At the 400 req/s a saturation
|
||||||
|
// attack can offer, an unbounded queue would hold ~2000 of those
|
||||||
|
// for the full five seconds, which is gigabytes.
|
||||||
|
//
|
||||||
|
// Arithmetic: 1 MB a waiter, and the memory committed to the
|
||||||
|
// queue is 64 MB, so 64 waiters. Cross-check against the
|
||||||
|
// deadline: two slots at the ~27 verifications/s measured on a
|
||||||
|
// review host (with the race detector on, so the real rate is
|
||||||
|
// higher) drain a full 64-deep queue in about 2.4 s, inside
|
||||||
|
// passwordVerifyWait. Queueing deeper would buy memory rather
|
||||||
|
// than throughput, because the extra waiters could not be served
|
||||||
|
// before their deadline anyway.
|
||||||
|
//
|
||||||
|
// Peak commitment is therefore 128 MB of Argon2id plus at most
|
||||||
|
// 66 MB of parsed forms — 64 queued and the 2 being hashed.
|
||||||
|
passwordVerifyMaxWaiters = 64
|
||||||
|
|
||||||
// failureKeyHashBytes is how much of the username digest goes
|
// failureKeyHashBytes is how much of the username digest goes
|
||||||
// into a failure key. 64 bits over at most loginFailureMaxKeys
|
// into a failure key. 64 bits over at most loginFailureMaxKeys
|
||||||
// live keys makes a collision negligible, and a collision would
|
// live keys makes a collision negligible, and a collision would
|
||||||
@@ -91,6 +117,12 @@ type loginGuard struct {
|
|||||||
|
|
||||||
slots chan struct{}
|
slots chan struct{}
|
||||||
|
|
||||||
|
// queue holds one token per request waiting for a slot. A token
|
||||||
|
// is taken non-blockingly, so a request that finds it full is
|
||||||
|
// shed rather than queued, and is given up as soon as the wait
|
||||||
|
// ends however it ends.
|
||||||
|
queue chan struct{}
|
||||||
|
|
||||||
limit int
|
limit int
|
||||||
interval time.Duration
|
interval time.Duration
|
||||||
maxKeys int
|
maxKeys int
|
||||||
@@ -101,17 +133,19 @@ type loginGuard struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// newLoginGuard builds a guard with the given failure limit per
|
// newLoginGuard builds a guard with the given failure limit per
|
||||||
// interval, key-set cap, verification concurrency and slot wait.
|
// interval, key-set cap, verification concurrency, queue depth and
|
||||||
|
// slot wait.
|
||||||
func newLoginGuard(
|
func newLoginGuard(
|
||||||
limit int,
|
limit int,
|
||||||
interval time.Duration,
|
interval time.Duration,
|
||||||
maxKeys, concurrency int,
|
maxKeys, concurrency, maxWaiters int,
|
||||||
wait time.Duration,
|
wait time.Duration,
|
||||||
) *loginGuard {
|
) *loginGuard {
|
||||||
return &loginGuard{
|
return &loginGuard{
|
||||||
byUser: make(map[string]*failureWindow),
|
byUser: make(map[string]*failureWindow),
|
||||||
byAddr: make(map[string]*failureWindow),
|
byAddr: make(map[string]*failureWindow),
|
||||||
slots: make(chan struct{}, concurrency),
|
slots: make(chan struct{}, concurrency),
|
||||||
|
queue: make(chan struct{}, maxWaiters),
|
||||||
limit: limit,
|
limit: limit,
|
||||||
interval: interval,
|
interval: interval,
|
||||||
maxKeys: maxKeys,
|
maxKeys: maxKeys,
|
||||||
@@ -121,14 +155,33 @@ func newLoginGuard(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// acquire reserves a verification slot, waiting up to the guard's
|
// acquire reserves a verification slot, waiting up to the guard's
|
||||||
// wait for one. It reports false when none became available or the
|
// wait for one. It reports false when the queue of waiters is
|
||||||
|
// already full, when no slot became available in time, or when the
|
||||||
// request was cancelled first; the caller must then answer 503
|
// request was cancelled first; the caller must then answer 503
|
||||||
// without verifying anything. The returned function releases the
|
// without verifying anything. The returned function releases the
|
||||||
// slot and must be called exactly once.
|
// slot and must be called exactly once.
|
||||||
func (g *loginGuard) acquire(ctx context.Context) (func(), bool) {
|
func (g *loginGuard) acquire(ctx context.Context) (func(), bool) {
|
||||||
|
// Shedding past the queue depth is what keeps waiting memory
|
||||||
|
// bounded; the wait alone only bounds how long one waiter holds
|
||||||
|
// its parsed form, not how many hold one at once.
|
||||||
|
select {
|
||||||
|
case g.queue <- struct{}{}:
|
||||||
|
default:
|
||||||
|
return nil, false
|
||||||
|
}
|
||||||
|
|
||||||
|
// Held only for the wait. A request that gets a slot gives its
|
||||||
|
// queue token back before it starts hashing, so the depth is a
|
||||||
|
// bound on waiters rather than on requests in the handler.
|
||||||
|
defer func() { <-g.queue }()
|
||||||
|
|
||||||
timer := time.NewTimer(g.wait)
|
timer := time.NewTimer(g.wait)
|
||||||
defer timer.Stop()
|
defer timer.Stop()
|
||||||
|
|
||||||
|
// The blocking send is deliberate: a receive on a full buffered
|
||||||
|
// channel hands the slot straight to the head of the send queue,
|
||||||
|
// so slots go out in arrival order and a later arrival cannot
|
||||||
|
// barge past a request already waiting.
|
||||||
select {
|
select {
|
||||||
case g.slots <- struct{}{}:
|
case g.slots <- struct{}{}:
|
||||||
return func() { <-g.slots }, true
|
return func() { <-g.slots }, true
|
||||||
@@ -242,6 +295,7 @@ func (m *Middleware) guard() *loginGuard {
|
|||||||
loginRateInterval,
|
loginRateInterval,
|
||||||
loginFailureMaxKeys,
|
loginFailureMaxKeys,
|
||||||
passwordVerifyConcurrency,
|
passwordVerifyConcurrency,
|
||||||
|
passwordVerifyMaxWaiters,
|
||||||
passwordVerifyWait,
|
passwordVerifyWait,
|
||||||
)
|
)
|
||||||
})
|
})
|
||||||
@@ -250,10 +304,11 @@ func (m *Middleware) guard() *loginGuard {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// BeginPasswordVerification reserves one of the bounded Argon2id
|
// BeginPasswordVerification reserves one of the bounded Argon2id
|
||||||
// verification slots. It reports false when none became free within
|
// verification slots. It reports false when the queue of waiting
|
||||||
// passwordVerifyWait, in which case the caller must answer 503 and
|
// requests is already at passwordVerifyMaxWaiters, or when no slot
|
||||||
// must not verify a password. The returned function releases the
|
// became free within passwordVerifyWait; in either case the caller
|
||||||
// slot and must be called exactly once.
|
// must answer 503 and must not verify a password. The returned
|
||||||
|
// function releases the slot and must be called exactly once.
|
||||||
//
|
//
|
||||||
// Every endpoint that hashes a password on request must go through
|
// Every endpoint that hashes a password on request must go through
|
||||||
// this, or the bound has a hole: the memory is committed per hash,
|
// this, or the bound has a hole: the memory is committed per hash,
|
||||||
|
|||||||
@@ -10,9 +10,13 @@ import (
|
|||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
"sneak.berlin/go/webhooker/internal/middleware"
|
"sneak.berlin/go/webhooker/internal/middleware"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// mib converts the Argon2id memory parameter, which is in KiB, to MB.
|
||||||
|
const mib = 1024
|
||||||
|
|
||||||
const (
|
const (
|
||||||
// guardInterval is the failure window these tests use. It is
|
// guardInterval is the failure window these tests use. It is
|
||||||
// long enough that nothing lapses mid-test on its own; tests
|
// long enough that nothing lapses mid-test on its own; tests
|
||||||
@@ -35,6 +39,7 @@ func newGuard(maxKeys, concurrency int) *middleware.LoginGuard {
|
|||||||
guardInterval,
|
guardInterval,
|
||||||
maxKeys,
|
maxKeys,
|
||||||
concurrency,
|
concurrency,
|
||||||
|
middleware.PasswordVerifyMaxWaitersConst,
|
||||||
guardWait,
|
guardWait,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -283,6 +288,7 @@ func TestLoginGuard_SaturatedSemaphoreRefusesRatherThanQueueing(
|
|||||||
guardInterval,
|
guardInterval,
|
||||||
middleware.LoginFailureMaxKeysConst,
|
middleware.LoginFailureMaxKeysConst,
|
||||||
1,
|
1,
|
||||||
|
middleware.PasswordVerifyMaxWaitersConst,
|
||||||
10*time.Millisecond,
|
10*time.Millisecond,
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -329,24 +335,175 @@ func TestLoginGuard_AcquireHonoursCancellation(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// TestPasswordVerifyConcurrency_MatchesMemoryBudget pins the
|
// TestPasswordVerifyConcurrency_MatchesMemoryBudget pins the
|
||||||
// concurrency constant to the arithmetic behind it: Argon2id here is
|
// concurrency constant to the arithmetic behind it: the number of
|
||||||
// 64 MB per hash, so the number of slots is the number of 64 MB
|
// slots is the hashing budget divided by what one Argon2id hash
|
||||||
// allocations the process is willing to commit to password hashing.
|
// actually costs.
|
||||||
// Raising it raises peak resident memory by 64 MB a slot.
|
//
|
||||||
|
// The per-hash figure is read out of the shipped password
|
||||||
|
// parameters rather than copied here. A guard that asserts a literal
|
||||||
|
// against a literal cannot see the thing it guards: raising
|
||||||
|
// argon2Memory would leave it green while the real ceiling doubled.
|
||||||
func TestPasswordVerifyConcurrency_MatchesMemoryBudget(t *testing.T) {
|
func TestPasswordVerifyConcurrency_MatchesMemoryBudget(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
const (
|
// Memory is the real argon2Memory, in KiB.
|
||||||
argon2MemoryMB = 64
|
perHashMB := int(database.DefaultPasswordConfig().Memory) / mib
|
||||||
budgetMB = 128
|
|
||||||
|
require.Positive(
|
||||||
|
t, perHashMB,
|
||||||
|
"the Argon2id memory parameter must be readable in MB",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// The memory this service commits to password hashing.
|
||||||
|
const budgetMB = 128
|
||||||
|
|
||||||
assert.Equal(
|
assert.Equal(
|
||||||
t,
|
t,
|
||||||
budgetMB/argon2MemoryMB,
|
|
||||||
middleware.PasswordVerifyConcurrencyConst,
|
middleware.PasswordVerifyConcurrencyConst,
|
||||||
"the verification concurrency is %d MB of Argon2id memory "+
|
budgetMB/perHashMB,
|
||||||
"divided by %d MB per hash",
|
"the verification concurrency must be the %d MB hashing "+
|
||||||
budgetMB, argon2MemoryMB,
|
"budget divided by the %d MB one Argon2id hash costs; "+
|
||||||
|
"if the Argon2id parameters changed, the slot count "+
|
||||||
|
"must change with them",
|
||||||
|
budgetMB, perHashMB,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestLoginGuard_ShedsPastTheQueueCap pins the memory bound on
|
||||||
|
// waiting, as distinct from the bound on hashing. A waiter arrives
|
||||||
|
// with its form already parsed, so an unbounded queue would hold up
|
||||||
|
// to maxFormBodySize per waiting request for the whole wait; past
|
||||||
|
// the cap the guard must refuse instantly rather than grow.
|
||||||
|
func TestLoginGuard_ShedsPastTheQueueCap(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
const (
|
||||||
|
maxWaiters = 2
|
||||||
|
|
||||||
|
// Long enough that a queued waiter never times out on its
|
||||||
|
// own, so anything the test observes leaving the queue left
|
||||||
|
// because it was shed.
|
||||||
|
neverElapses = time.Minute
|
||||||
|
|
||||||
|
// The probe carries its own deadline, so a guard that queues
|
||||||
|
// the probe instead of shedding it fails on the elapsed time
|
||||||
|
// rather than hanging until the package test timeout.
|
||||||
|
probeWait = 200 * time.Millisecond
|
||||||
|
|
||||||
|
// Shedding takes no measurable time; queueing takes the whole
|
||||||
|
// probeWait. Anything under half of it is unambiguous.
|
||||||
|
shedFast = probeWait / 2
|
||||||
|
)
|
||||||
|
|
||||||
|
g := middleware.NewLoginGuardForTest(
|
||||||
|
middleware.LoginRateLimitConst,
|
||||||
|
guardInterval,
|
||||||
|
middleware.LoginFailureMaxKeysConst,
|
||||||
|
1,
|
||||||
|
maxWaiters,
|
||||||
|
neverElapses,
|
||||||
|
)
|
||||||
|
|
||||||
|
// Occupy the only slot, so everything after this queues.
|
||||||
|
release, ok := g.AcquireForTest(context.Background())
|
||||||
|
require.True(t, ok)
|
||||||
|
|
||||||
|
defer release()
|
||||||
|
defer fillQueue(t, g, maxWaiters)()
|
||||||
|
|
||||||
|
got := probeQueueCap(g, probeWait)
|
||||||
|
|
||||||
|
require.NotNil(
|
||||||
|
t, got,
|
||||||
|
"a request arriving past the queue cap is still waiting to "+
|
||||||
|
"be queued; it must have been shed",
|
||||||
|
)
|
||||||
|
assert.False(
|
||||||
|
t, got.ok,
|
||||||
|
"a request arriving past the queue cap must be shed",
|
||||||
|
)
|
||||||
|
assert.Less(
|
||||||
|
t, got.elapsed, shedFast,
|
||||||
|
"shedding must be immediate; waiting for a place in the "+
|
||||||
|
"queue is the memory growth this bounds",
|
||||||
|
)
|
||||||
|
assert.Equal(
|
||||||
|
t, maxWaiters, g.QueuedWaitersForTest(),
|
||||||
|
"a shed request must not have grown the queue",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// fillQueue starts n waiters on g and returns once all of them are
|
||||||
|
// queued for a slot. The returned function releases them and waits
|
||||||
|
// for them to exit.
|
||||||
|
func fillQueue(
|
||||||
|
t *testing.T,
|
||||||
|
g *middleware.LoginGuard,
|
||||||
|
n int,
|
||||||
|
) func() {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
|
||||||
|
var wg sync.WaitGroup
|
||||||
|
|
||||||
|
for range n {
|
||||||
|
wg.Go(func() {
|
||||||
|
done, got := g.AcquireForTest(ctx)
|
||||||
|
if got {
|
||||||
|
done()
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
require.Eventually(
|
||||||
|
t,
|
||||||
|
func() bool { return g.QueuedWaitersForTest() == n },
|
||||||
|
time.Second, time.Millisecond,
|
||||||
|
"the waiters must reach the queue before the cap is tested",
|
||||||
|
)
|
||||||
|
|
||||||
|
return func() {
|
||||||
|
cancel()
|
||||||
|
wg.Wait()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// probeResult is what the queue-cap probe reports: whether it got a
|
||||||
|
// slot, and how long it took to find out.
|
||||||
|
type probeResult struct {
|
||||||
|
ok bool
|
||||||
|
elapsed time.Duration
|
||||||
|
}
|
||||||
|
|
||||||
|
// probeQueueCap acquires from another goroutine and reports the
|
||||||
|
// result, or nil if the call was still blocked after wait.
|
||||||
|
//
|
||||||
|
// It runs off the test goroutine deliberately. Joining a full queue
|
||||||
|
// is not cancellable by context — refusing to join is the property
|
||||||
|
// under test — so a guard that fails this would otherwise hang the
|
||||||
|
// package until the test timeout instead of failing here.
|
||||||
|
func probeQueueCap(
|
||||||
|
g *middleware.LoginGuard,
|
||||||
|
wait time.Duration,
|
||||||
|
) *probeResult {
|
||||||
|
probed := make(chan probeResult, 1)
|
||||||
|
|
||||||
|
go func() {
|
||||||
|
start := time.Now()
|
||||||
|
|
||||||
|
release, ok := g.AcquireForTest(context.Background())
|
||||||
|
if ok {
|
||||||
|
release()
|
||||||
|
}
|
||||||
|
|
||||||
|
probed <- probeResult{ok: ok, elapsed: time.Since(start)}
|
||||||
|
}()
|
||||||
|
|
||||||
|
select {
|
||||||
|
case result := <-probed:
|
||||||
|
return &result
|
||||||
|
case <-time.After(wait):
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -6,10 +6,14 @@ import (
|
|||||||
"log/slog"
|
"log/slog"
|
||||||
"net"
|
"net"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
"time"
|
"time"
|
||||||
|
"unicode"
|
||||||
|
"unicode/utf8"
|
||||||
|
|
||||||
basicauth "github.com/99designs/basicauth-go"
|
basicauth "github.com/99designs/basicauth-go"
|
||||||
|
"github.com/go-chi/chi"
|
||||||
"github.com/go-chi/chi/middleware"
|
"github.com/go-chi/chi/middleware"
|
||||||
"github.com/go-chi/cors"
|
"github.com/go-chi/cors"
|
||||||
metrics "github.com/slok/go-http-metrics/metrics/prometheus"
|
metrics "github.com/slok/go-http-metrics/metrics/prometheus"
|
||||||
@@ -26,6 +30,75 @@ const (
|
|||||||
// corsMaxAge is the maximum time (in seconds) that a
|
// corsMaxAge is the maximum time (in seconds) that a
|
||||||
// preflight response can be cached.
|
// preflight response can be cached.
|
||||||
corsMaxAge = 300
|
corsMaxAge = 300
|
||||||
|
|
||||||
|
// unmatchedRoute is logged in the access log's url field when a
|
||||||
|
// redirected or rejected request matched no route pattern at
|
||||||
|
// all. Every byte of such a path is client-chosen, so none of it
|
||||||
|
// is logged.
|
||||||
|
unmatchedRoute = "(unmatched)"
|
||||||
|
|
||||||
|
// redactedQuery stands in for the query string on the access log
|
||||||
|
// branches that keep the concrete URL. The query is client-chosen
|
||||||
|
// on every route, including the ones that answer an
|
||||||
|
// unauthenticated 200, so logging it verbatim would let a client
|
||||||
|
// pick the size of the line it writes.
|
||||||
|
redactedQuery = "?(redacted)"
|
||||||
|
|
||||||
|
// maxLogFieldBytes bounds each access log field whose value the
|
||||||
|
// client supplies outright: the URL, the User-Agent and the
|
||||||
|
// Referer. The budget is spent in ENCODED bytes (see
|
||||||
|
// truncateLogField), so 512 still holds a real browser's User-Agent
|
||||||
|
// whole — those are plain ASCII, which encodes one byte for one —
|
||||||
|
// while a value built from characters the encoder escapes keeps a
|
||||||
|
// shorter prefix. That is the intended trade: 500 quotation marks
|
||||||
|
// are not a debugging asset.
|
||||||
|
maxLogFieldBytes = 512
|
||||||
|
|
||||||
|
// maxLogRequestIDBytes bounds the request id, which is also
|
||||||
|
// client-supplied: chi's RequestID middleware passes an inbound
|
||||||
|
// X-Request-Id header through verbatim. Its generated form is an
|
||||||
|
// order of magnitude shorter than this.
|
||||||
|
maxLogRequestIDBytes = 128
|
||||||
|
|
||||||
|
// maxLogMethodBytes bounds the method. Go accepts any RFC 7230
|
||||||
|
// token there, bounded only by the header size limit, so it is
|
||||||
|
// client-chosen text like the rest. The longest registered method
|
||||||
|
// is half this.
|
||||||
|
maxLogMethodBytes = 32
|
||||||
|
|
||||||
|
// truncationMarker is appended to any field the access log cut, so
|
||||||
|
// a short value and a truncated one cannot be confused. It is
|
||||||
|
// charged on top of the budget, not inside it.
|
||||||
|
truncationMarker = "[truncated]"
|
||||||
|
|
||||||
|
// MaxAccessLogLineBytes is the ceiling on one JSON access log line,
|
||||||
|
// and the number an operator multiplies by the request rate to size
|
||||||
|
// log storage. It is not an observation of a sample: it is the sum
|
||||||
|
// of the budgets above, each of which truncateLogField enforces in
|
||||||
|
// ENCODED bytes, plus the part of the line no client can influence.
|
||||||
|
//
|
||||||
|
// url, useragent, referer 3*(512+11) = 1569
|
||||||
|
// request_id 128+11 = 139
|
||||||
|
// method 32+11 = 43
|
||||||
|
// fixed portion = 336
|
||||||
|
// ----
|
||||||
|
// 2087
|
||||||
|
//
|
||||||
|
// The fixed portion is the JSON punctuation, the field names, the
|
||||||
|
// level and the message, both timestamps at their longest, an IPv6
|
||||||
|
// remoteIP with a zone, a three-digit status and a full-width int64
|
||||||
|
// latency. Stated at 2560 so the figure carries headroom rather
|
||||||
|
// than sitting on the arithmetic.
|
||||||
|
//
|
||||||
|
// The tty text handler in internal/logger is covered by the same
|
||||||
|
// figure. encodedLogFieldBytes charges every rune at least what
|
||||||
|
// the wider of the two handlers emits for it — including the ten
|
||||||
|
// bytes strconv.Quote spends on a non-printable rune at or above
|
||||||
|
// U+10000, which is four more than the JSON handler ever spends —
|
||||||
|
// so each budget bounds the encoded field under either handler.
|
||||||
|
// The text handler's fixed portion is 286, the smaller of the two,
|
||||||
|
// which puts its worst case at 2037.
|
||||||
|
MaxAccessLogLineBytes = 2560
|
||||||
)
|
)
|
||||||
|
|
||||||
//nolint:revive // MiddlewareParams is a standard fx naming convention.
|
//nolint:revive // MiddlewareParams is a standard fx naming convention.
|
||||||
@@ -101,6 +174,178 @@ func (lrw *loggingResponseWriter) WriteHeader(code int) {
|
|||||||
lrw.ResponseWriter.WriteHeader(code)
|
lrw.ResponseWriter.WriteHeader(code)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// encodedLogFieldBytes is what r costs on the line once the log
|
||||||
|
// handler has escaped it, taking the worse of the two handlers
|
||||||
|
// internal/logger configures.
|
||||||
|
//
|
||||||
|
// slog's JSON handler escapes quote, backslash, newline, carriage
|
||||||
|
// return and tab to two bytes each, and every other C0 control plus
|
||||||
|
// LINE SEPARATOR and PARAGRAPH SEPARATOR to a six-byte \u escape; it
|
||||||
|
// passes every other rune through as its own UTF-8. Its text handler
|
||||||
|
// quotes with strconv.Quote, which spells a non-printable rune below
|
||||||
|
// U+10000 as \uXXXX but one at or above U+10000 as \UXXXXXXXX — ten
|
||||||
|
// bytes, not six. The text handler is therefore the worse of the two
|
||||||
|
// for every non-printable rune, and by four bytes apiece for the
|
||||||
|
// 955,086 unassigned, private-use and format code points on planes 1
|
||||||
|
// to 16.
|
||||||
|
//
|
||||||
|
// Charging ten there is what makes MaxAccessLogLineBytes hold for the
|
||||||
|
// tty handler as well: U+1000C encodes as F0 90 80 8C, every byte
|
||||||
|
// >= 0x80, which httpguts.ValidHeaderFieldValue accepts and
|
||||||
|
// net/textproto does not strip, so a header can be filled with them.
|
||||||
|
//
|
||||||
|
// Both handlers pass printable runes through as their own UTF-8, so
|
||||||
|
// unicode.IsPrint separates the escaped cases from the plain ones for
|
||||||
|
// either handler.
|
||||||
|
func encodedLogFieldBytes(r rune) int {
|
||||||
|
const (
|
||||||
|
// A backslash and the character itself.
|
||||||
|
shortEscapeBytes = 2
|
||||||
|
// \uXXXX, which is also the width of \u00XX.
|
||||||
|
escapedRuneBytes = 6
|
||||||
|
// \UXXXXXXXX, strconv.Quote's spelling of a non-printable
|
||||||
|
// rune outside the basic multilingual plane.
|
||||||
|
escapedAstralRuneBytes = 10
|
||||||
|
// The first code point strconv.Quote spells with \U.
|
||||||
|
firstAstralRune = 0x10000
|
||||||
|
)
|
||||||
|
|
||||||
|
switch {
|
||||||
|
case r == '"' || r == '\\' || r == '\n' || r == '\r' || r == '\t':
|
||||||
|
return shortEscapeBytes
|
||||||
|
case !unicode.IsPrint(r) && r >= firstAstralRune:
|
||||||
|
return escapedAstralRuneBytes
|
||||||
|
case !unicode.IsPrint(r):
|
||||||
|
return escapedRuneBytes
|
||||||
|
default:
|
||||||
|
return utf8.RuneLen(r)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// truncateLogField caps s at maxBytes of ENCODED output, marking the
|
||||||
|
// value when it cuts.
|
||||||
|
//
|
||||||
|
// Budgeting raw bytes would not bound the line. Escaping only ever
|
||||||
|
// grows a value, so a raw budget spent on characters the encoder
|
||||||
|
// escapes buys a field several times its nominal size — and the line
|
||||||
|
// is the thing an operator is told to multiply by their request rate.
|
||||||
|
// Charging each rune what it will actually cost is what makes
|
||||||
|
// MaxAccessLogLineBytes true rather than merely larger. The visible
|
||||||
|
// consequence is that an escape-heavy value keeps a shorter prefix
|
||||||
|
// than a plain one, which is the correct trade.
|
||||||
|
//
|
||||||
|
// The result is always valid UTF-8. A cut on a byte boundary can split
|
||||||
|
// a multi-byte rune, and a header can carry bytes that were never
|
||||||
|
// valid UTF-8 to begin with; both are dropped rather than kept, since
|
||||||
|
// an encoder would otherwise spend six bytes replacing each one.
|
||||||
|
func truncateLogField(s string, maxBytes int) string {
|
||||||
|
// No rune encodes to fewer bytes than it occupies, so nothing past
|
||||||
|
// maxBytes raw can fit the budget. Slicing first bounds the scan
|
||||||
|
// below to the budget rather than to the size of the header the
|
||||||
|
// client sent.
|
||||||
|
window, cut := s, false
|
||||||
|
if len(window) > maxBytes {
|
||||||
|
window, cut = window[:maxBytes], true
|
||||||
|
}
|
||||||
|
|
||||||
|
var (
|
||||||
|
kept strings.Builder
|
||||||
|
spent int
|
||||||
|
)
|
||||||
|
|
||||||
|
for i := 0; i < len(window); {
|
||||||
|
r, size := utf8.DecodeRuneInString(window[i:])
|
||||||
|
if r == utf8.RuneError && size == 1 {
|
||||||
|
i += size
|
||||||
|
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
cost := encodedLogFieldBytes(r)
|
||||||
|
if spent+cost > maxBytes {
|
||||||
|
cut = true
|
||||||
|
|
||||||
|
break
|
||||||
|
}
|
||||||
|
|
||||||
|
spent += cost
|
||||||
|
|
||||||
|
kept.WriteString(window[i : i+size])
|
||||||
|
|
||||||
|
i += size
|
||||||
|
}
|
||||||
|
|
||||||
|
if !cut {
|
||||||
|
return kept.String()
|
||||||
|
}
|
||||||
|
|
||||||
|
return kept.String() + truncationMarker
|
||||||
|
}
|
||||||
|
|
||||||
|
// concreteLogURL renders the request's own URL for the access log
|
||||||
|
// branches that keep it, with the query string replaced by a fixed
|
||||||
|
// marker.
|
||||||
|
//
|
||||||
|
// The path on those branches is bounded by the service's routes or by
|
||||||
|
// the operator's data — a 2xx on the receiver means the UUID named a
|
||||||
|
// stored entrypoint, a 2xx under /s means the file is in the embedded
|
||||||
|
// tree. The query is not bounded by anything: /.well-known/healthcheck
|
||||||
|
// and /s/* take no authentication and sit behind no rate limiter, and
|
||||||
|
// /pages/login behind only the login limiter, so any of them will
|
||||||
|
// answer 200 to a URL carrying an arbitrary number of arbitrary bytes
|
||||||
|
// after the '?'. Keeping the path and dropping the query is what makes
|
||||||
|
// this branch as bounded as the pattern branches below.
|
||||||
|
//
|
||||||
|
// Nothing debuggable is lost. One route in the service reads a query
|
||||||
|
// parameter at all — `page`, on the authenticated pagination links in
|
||||||
|
// internal/handlers/source_management.go — and the alternatives that
|
||||||
|
// would preserve more (a key count, a key allowlist) all require
|
||||||
|
// parsing an attacker-sized query on every request, which is work an
|
||||||
|
// unauthenticated client would then be choosing for us.
|
||||||
|
func concreteLogURL(r *http.Request) string {
|
||||||
|
path := r.URL.EscapedPath()
|
||||||
|
|
||||||
|
if r.URL.RawQuery == "" && !r.URL.ForceQuery {
|
||||||
|
return path
|
||||||
|
}
|
||||||
|
|
||||||
|
return path + redactedQuery
|
||||||
|
}
|
||||||
|
|
||||||
|
// accessLogURL returns the value for the access log's url field.
|
||||||
|
//
|
||||||
|
// 2xx and 5xx responses get the concrete path (see concreteLogURL). A
|
||||||
|
// success resolved against a static route or against the operator's
|
||||||
|
// own data — on the receiver, a 2xx means the UUID named a stored
|
||||||
|
// entrypoint — and a server error is our own bug, where the exact URL
|
||||||
|
// is the primary evidence and which no client can provoke at will.
|
||||||
|
//
|
||||||
|
// 3xx and 4xx responses get the chi route pattern instead. Those are
|
||||||
|
// the outcomes an unauthenticated client drives for free: 404 or 429
|
||||||
|
// on any invented /webhook/ path, 303 to the login page on any
|
||||||
|
// invented /user/ path. Logging the concrete URL there lets a flood
|
||||||
|
// write attacker-chosen text, of attacker-chosen length, into the
|
||||||
|
// operator's log at one line per request. The pattern comes from the
|
||||||
|
// router's own table, so it is bounded by the service's routes while
|
||||||
|
// still naming which class of request was rejected.
|
||||||
|
//
|
||||||
|
// The pattern is only populated once routing has run, so this must be
|
||||||
|
// called after the handler returns, not before.
|
||||||
|
func accessLogURL(r *http.Request, status int) string {
|
||||||
|
if status < http.StatusMultipleChoices ||
|
||||||
|
status >= http.StatusInternalServerError {
|
||||||
|
return concreteLogURL(r)
|
||||||
|
}
|
||||||
|
|
||||||
|
if rc := chi.RouteContext(r.Context()); rc != nil {
|
||||||
|
if pattern := rc.RoutePattern(); pattern != "" {
|
||||||
|
return pattern
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return unmatchedRoute
|
||||||
|
}
|
||||||
|
|
||||||
// Logging returns middleware that logs each HTTP request with
|
// Logging returns middleware that logs each HTTP request with
|
||||||
// timing and metadata.
|
// timing and metadata.
|
||||||
func (s *Middleware) Logging() func(http.Handler) http.Handler {
|
func (s *Middleware) Logging() func(http.Handler) http.Handler {
|
||||||
@@ -125,13 +370,27 @@ func (s *Middleware) Logging() func(http.Handler) http.Handler {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Every field below that a client can influence is
|
||||||
|
// truncated to a fixed budget, so the size of this
|
||||||
|
// line does not track the size of the request.
|
||||||
s.log.Info("http request",
|
s.log.Info("http request",
|
||||||
"request_start", start,
|
"request_start", start,
|
||||||
"method", r.Method,
|
"method", truncateLogField(
|
||||||
"url", r.URL.String(),
|
r.Method, maxLogMethodBytes,
|
||||||
"useragent", r.UserAgent(),
|
),
|
||||||
"request_id", requestID,
|
"url", truncateLogField(
|
||||||
"referer", r.Referer(),
|
accessLogURL(r, lrw.statusCode),
|
||||||
|
maxLogFieldBytes,
|
||||||
|
),
|
||||||
|
"useragent", truncateLogField(
|
||||||
|
r.UserAgent(), maxLogFieldBytes,
|
||||||
|
),
|
||||||
|
"request_id", truncateLogField(
|
||||||
|
requestID, maxLogRequestIDBytes,
|
||||||
|
),
|
||||||
|
"referer", truncateLogField(
|
||||||
|
r.Referer(), maxLogFieldBytes,
|
||||||
|
),
|
||||||
"proto", r.Proto,
|
"proto", r.Proto,
|
||||||
"remoteIP", ipFromHostPort(r.RemoteAddr),
|
"remoteIP", ipFromHostPort(r.RemoteAddr),
|
||||||
"status", lrw.statusCode,
|
"status", lrw.statusCode,
|
||||||
|
|||||||
@@ -149,6 +149,15 @@ func (s *Server) setupSourceRoutes() {
|
|||||||
r.Post("/edit", s.h.HandleSourceEditSubmit())
|
r.Post("/edit", s.h.HandleSourceEditSubmit())
|
||||||
r.Post("/delete", s.h.HandleSourceDelete())
|
r.Post("/delete", s.h.HandleSourceDelete())
|
||||||
r.Get("/logs", s.h.HandleSourceLogs())
|
r.Get("/logs", s.h.HandleSourceLogs())
|
||||||
|
// The log page renders each body only up to its cap, so
|
||||||
|
// this is the only route that serves a whole one. It
|
||||||
|
// belongs to this group for its RequireAuth and
|
||||||
|
// NoCache; see HandleEventBodyDownload for the headers
|
||||||
|
// that keep the bytes it returns inert.
|
||||||
|
r.Get(
|
||||||
|
"/logs/{eventID}/body",
|
||||||
|
s.h.HandleEventBodyDownload(),
|
||||||
|
)
|
||||||
r.Post(
|
r.Post(
|
||||||
"/entrypoints",
|
"/entrypoints",
|
||||||
s.h.HandleEntrypointCreate(),
|
s.h.HandleEntrypointCreate(),
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import (
|
|||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"net/url"
|
"net/url"
|
||||||
"regexp"
|
"regexp"
|
||||||
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
@@ -14,6 +15,7 @@ import (
|
|||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
"go.uber.org/fx"
|
"go.uber.org/fx"
|
||||||
"go.uber.org/fx/fxtest"
|
"go.uber.org/fx/fxtest"
|
||||||
|
"gorm.io/gorm/clause"
|
||||||
"sneak.berlin/go/webhooker/internal/config"
|
"sneak.berlin/go/webhooker/internal/config"
|
||||||
"sneak.berlin/go/webhooker/internal/database"
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
"sneak.berlin/go/webhooker/internal/delivery"
|
"sneak.berlin/go/webhooker/internal/delivery"
|
||||||
@@ -49,6 +51,7 @@ type testEnv struct {
|
|||||||
router http.Handler
|
router http.Handler
|
||||||
sess *session.Session
|
sess *session.Session
|
||||||
db *database.Database
|
db *database.Database
|
||||||
|
dbMgr *database.WebhookDBManager
|
||||||
}
|
}
|
||||||
|
|
||||||
// newTestEnv wires the dependency graph with fx and builds the
|
// newTestEnv wires the dependency graph with fx and builds the
|
||||||
@@ -64,6 +67,7 @@ func newTestEnv(t *testing.T) *testEnv {
|
|||||||
hnd *handlers.Handlers
|
hnd *handlers.Handlers
|
||||||
sess *session.Session
|
sess *session.Session
|
||||||
db *database.Database
|
db *database.Database
|
||||||
|
dbMgr *database.WebhookDBManager
|
||||||
)
|
)
|
||||||
|
|
||||||
app := fxtest.New(
|
app := fxtest.New(
|
||||||
@@ -86,7 +90,7 @@ func newTestEnv(t *testing.T) *testEnv {
|
|||||||
middleware.New,
|
middleware.New,
|
||||||
handlers.New,
|
handlers.New,
|
||||||
),
|
),
|
||||||
fx.Populate(&log, &cfg, &mw, &hnd, &sess, &db),
|
fx.Populate(&log, &cfg, &mw, &hnd, &sess, &db, &dbMgr),
|
||||||
)
|
)
|
||||||
app.RequireStart()
|
app.RequireStart()
|
||||||
t.Cleanup(app.RequireStop)
|
t.Cleanup(app.RequireStop)
|
||||||
@@ -95,6 +99,7 @@ func newTestEnv(t *testing.T) *testEnv {
|
|||||||
router: server.NewRouterForTest(log.Get(), cfg, mw, hnd),
|
router: server.NewRouterForTest(log.Get(), cfg, mw, hnd),
|
||||||
sess: sess,
|
sess: sess,
|
||||||
db: db,
|
db: db,
|
||||||
|
dbMgr: dbMgr,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -233,6 +238,49 @@ func (e *testEnv) seedUser(
|
|||||||
return user.ID, hash
|
return user.ID, hash
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// seedWebhook creates a webhook owned by the given user.
|
||||||
|
func (e *testEnv) seedWebhook(
|
||||||
|
t *testing.T,
|
||||||
|
userID string,
|
||||||
|
) *database.Webhook {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
wh := &database.Webhook{UserID: userID, Name: "routed"}
|
||||||
|
|
||||||
|
require.NoError(
|
||||||
|
t,
|
||||||
|
e.db.DB().Omit(clause.Associations).Create(wh).Error,
|
||||||
|
)
|
||||||
|
|
||||||
|
return wh
|
||||||
|
}
|
||||||
|
|
||||||
|
// seedEvent records one event with the given body in a webhook's
|
||||||
|
// own database.
|
||||||
|
func (e *testEnv) seedEvent(
|
||||||
|
t *testing.T,
|
||||||
|
webhookID, body string,
|
||||||
|
) *database.Event {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
webhookDB, err := e.dbMgr.GetDB(webhookID)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
event := &database.Event{
|
||||||
|
WebhookID: webhookID,
|
||||||
|
Method: http.MethodPost,
|
||||||
|
Body: body,
|
||||||
|
ContentType: "application/octet-stream",
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NoError(
|
||||||
|
t,
|
||||||
|
webhookDB.Omit(clause.Associations).Create(event).Error,
|
||||||
|
)
|
||||||
|
|
||||||
|
return event
|
||||||
|
}
|
||||||
|
|
||||||
// storedHash reads the current password hash for a username.
|
// storedHash reads the current password hash for a username.
|
||||||
func (e *testEnv) storedHash(t *testing.T, username string) string {
|
func (e *testEnv) storedHash(t *testing.T, username string) string {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
@@ -372,6 +420,78 @@ func TestPagesLogin_UnderLimit_ValidToken_ReachesHandler(
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestPagesLogin_CorrectPasswordSurvivesASpentBudget pins the
|
||||||
|
// routing half of the fix, which every other login test misses by
|
||||||
|
// driving the handler directly: no pre-emptive limiter sits in front
|
||||||
|
// of POST /pages/login on the real route tree.
|
||||||
|
//
|
||||||
|
// A limiter registered there would answer the last request 429
|
||||||
|
// however correct its password is, because the wrong passwords
|
||||||
|
// before it have already spent the bucket — which is the lockout
|
||||||
|
// this endpoint exists to not have. CSRF and the body cap still run,
|
||||||
|
// since every request here carries a harvested token.
|
||||||
|
func TestPagesLogin_CorrectPasswordSurvivesASpentBudget(
|
||||||
|
t *testing.T,
|
||||||
|
) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
const (
|
||||||
|
username = "operator"
|
||||||
|
password = "correct-horse-battery-staple"
|
||||||
|
)
|
||||||
|
|
||||||
|
env := newTestEnv(t)
|
||||||
|
env.seedUser(t, username, password)
|
||||||
|
|
||||||
|
submit := func(t *testing.T, pw string) *httptest.ResponseRecorder {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
token, cookies := env.csrfFrom(t, "/pages/login", nil)
|
||||||
|
|
||||||
|
form := url.Values{}
|
||||||
|
form.Set("csrf_token", token)
|
||||||
|
form.Set("username", username)
|
||||||
|
form.Set("password", pw)
|
||||||
|
|
||||||
|
return env.post("/pages/login", form, cookies)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Spend the failure budget against this username. The exact
|
||||||
|
// limit belongs to the middleware; this waits for the throttle
|
||||||
|
// to appear rather than restating it, under a ceiling well
|
||||||
|
// above it so a broken limiter fails the test instead of
|
||||||
|
// looping.
|
||||||
|
const maxAttempts = 20
|
||||||
|
|
||||||
|
spent := false
|
||||||
|
|
||||||
|
for range maxAttempts {
|
||||||
|
code := submit(t, "wrong").Code
|
||||||
|
if code == http.StatusTooManyRequests {
|
||||||
|
spent = true
|
||||||
|
|
||||||
|
break
|
||||||
|
}
|
||||||
|
|
||||||
|
require.Equal(
|
||||||
|
t, http.StatusUnauthorized, code,
|
||||||
|
"a wrong password must be rejected, not accepted",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
require.True(
|
||||||
|
t, spent,
|
||||||
|
"repeated wrong passwords must eventually be throttled",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert.Equal(
|
||||||
|
t, http.StatusSeeOther, submit(t, password).Code,
|
||||||
|
"a correct password must be accepted on the routed "+
|
||||||
|
"endpoint even with the failure budget spent: the "+
|
||||||
|
"operator has no second administrative path",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
// --- /user/{username} group ---
|
// --- /user/{username} group ---
|
||||||
|
|
||||||
// TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
|
// TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
|
||||||
@@ -432,3 +552,95 @@ func TestPasswordChange_UnderLimit_Succeeds(t *testing.T) {
|
|||||||
"an under-limit password change should still apply",
|
"an under-limit password change should still apply",
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// --- /source/{sourceID} group ---
|
||||||
|
|
||||||
|
// TestSourceLogs_TruncationLinkDownloadsTheBody walks the whole
|
||||||
|
// feature the way a user does: render the event log page through
|
||||||
|
// the production router, take the download URL out of the markup
|
||||||
|
// the template emitted, and fetch that URL through the router
|
||||||
|
// again. Nothing here is hand-written, so a typo in either the
|
||||||
|
// route pattern or the template href fails this test — the
|
||||||
|
// handler-level tests cannot catch that, because they forge
|
||||||
|
// their own route context and assert a URL string they wrote
|
||||||
|
// themselves.
|
||||||
|
func TestSourceLogs_TruncationLinkDownloadsTheBody(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
env := newTestEnv(t)
|
||||||
|
|
||||||
|
userID, _ := env.seedUser(t, "loguser", "somepassword")
|
||||||
|
cookies := env.authCookies(t, userID, "loguser")
|
||||||
|
|
||||||
|
// Comfortably over the event log page's render cap, so the
|
||||||
|
// page truncates the body and renders the download link at
|
||||||
|
// all. The exact cap is the handlers package's business and
|
||||||
|
// is pinned by its own tests; this only needs to exceed it.
|
||||||
|
stored := strings.Repeat("Z", 64*1024)
|
||||||
|
|
||||||
|
wh := env.seedWebhook(t, userID)
|
||||||
|
env.seedEvent(t, wh.ID, stored)
|
||||||
|
|
||||||
|
page := env.get("/source/"+wh.ID+"/logs", cookies)
|
||||||
|
require.Equal(t, http.StatusOK, page.Code)
|
||||||
|
|
||||||
|
link := regexp.MustCompile(
|
||||||
|
`href="(/source/[^"]+/body)"`,
|
||||||
|
).FindStringSubmatch(page.Body.String())
|
||||||
|
require.Len(
|
||||||
|
t, link, 2,
|
||||||
|
"truncated body should render a download link",
|
||||||
|
)
|
||||||
|
|
||||||
|
w := env.get(html.UnescapeString(link[1]), cookies)
|
||||||
|
|
||||||
|
require.Equal(
|
||||||
|
t, http.StatusOK, w.Code,
|
||||||
|
"the link the page emits must be a live route",
|
||||||
|
)
|
||||||
|
assert.Equal(t, stored, w.Body.String())
|
||||||
|
assert.Equal(
|
||||||
|
t, strconv.Itoa(len(stored)),
|
||||||
|
w.Header().Get("Content-Length"),
|
||||||
|
)
|
||||||
|
assert.Equal(
|
||||||
|
t, "application/octet-stream",
|
||||||
|
w.Header().Get("Content-Type"),
|
||||||
|
)
|
||||||
|
assert.Contains(
|
||||||
|
t, w.Header().Get("Content-Disposition"), "attachment",
|
||||||
|
)
|
||||||
|
assert.Equal(
|
||||||
|
t, "nosniff", w.Header().Get("X-Content-Type-Options"),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSourceLogsBody_OtherUser404s pins that the download route
|
||||||
|
// as registered is behind the auth the group provides and the
|
||||||
|
// ownership check the handler applies: another logged-in user
|
||||||
|
// asking the real router for the same URL gets a 404, and an
|
||||||
|
// unauthenticated request never reaches the handler at all.
|
||||||
|
func TestSourceLogsBody_OtherUser404s(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
env := newTestEnv(t)
|
||||||
|
|
||||||
|
ownerID, _ := env.seedUser(t, "owner", "somepassword")
|
||||||
|
wh := env.seedWebhook(t, ownerID)
|
||||||
|
|
||||||
|
const payload = "OWNERS-PAYLOAD-77c1"
|
||||||
|
|
||||||
|
evt := env.seedEvent(t, wh.ID, payload)
|
||||||
|
path := "/source/" + wh.ID + "/logs/" + evt.ID + "/body"
|
||||||
|
|
||||||
|
intruderID, _ := env.seedUser(t, "intruder", "somepassword")
|
||||||
|
intruder := env.authCookies(t, intruderID, "intruder")
|
||||||
|
|
||||||
|
w := env.get(path, intruder)
|
||||||
|
assert.Equal(t, http.StatusNotFound, w.Code)
|
||||||
|
assert.NotContains(t, w.Body.String(), payload)
|
||||||
|
|
||||||
|
anon := env.get(path, nil)
|
||||||
|
assert.Equal(t, http.StatusSeeOther, anon.Code)
|
||||||
|
assert.Equal(t, "/pages/login", anon.Header().Get("Location"))
|
||||||
|
}
|
||||||
|
|||||||
152
script/ci-mark-superseded
Executable file
152
script/ci-mark-superseded
Executable file
@@ -0,0 +1,152 @@
|
|||||||
|
#!/bin/sh
|
||||||
|
# script/ci-mark-superseded: record an honest status on commits whose CI
|
||||||
|
# run Gitea cancelled because a newer commit landed on the same branch.
|
||||||
|
# Gitea writes `failure` / "Has been cancelled" for such a run, which
|
||||||
|
# reads as a test result on a commit nothing ever tested. Cancellation is
|
||||||
|
# unconditional server-side for push events, so the superseding run
|
||||||
|
# rewrites those statuses to `failure` with a description that says the
|
||||||
|
# commit was never tested. `skipped` cannot be used: Gitea's combined
|
||||||
|
# status folds `skipped` into `success`, so a never-tested commit would
|
||||||
|
# report green. Genuine failures and successes are never touched.
|
||||||
|
#
|
||||||
|
# Called by the Gitea Actions workflow, which supplies GITHUB_API_URL,
|
||||||
|
# GITHUB_REPOSITORY, GITHUB_SHA, GITHUB_WORKFLOW, GITHUB_JOB,
|
||||||
|
# GITHUB_EVENT_NAME and GITEA_TOKEN. ANCESTOR_LIMIT (default 20) caps how
|
||||||
|
# far back the walk looks; a value that is set but not a positive integer
|
||||||
|
# aborts rather than silently disabling the walk.
|
||||||
|
set -eu
|
||||||
|
|
||||||
|
SUPERSEDED_DESC='Superseded by a newer commit; never tested'
|
||||||
|
|
||||||
|
# Gitea builds the commit-status context as
|
||||||
|
# "<workflow name> / <job name> (<event>)", so derive it rather than
|
||||||
|
# hardcoding the result.
|
||||||
|
#
|
||||||
|
# The derivation is deliberately not byte-exact with Gitea's own rule and
|
||||||
|
# must not be "fixed" into a silent fallback. Gitea uses the job's `name:`
|
||||||
|
# (falling back to the job id) and the workflow's `name:` (falling back to
|
||||||
|
# the workflow filename), while the runner exports GITHUB_JOB as the job
|
||||||
|
# *id* and GITHUB_WORKFLOW as the parsed workflow `name:`. So giving the
|
||||||
|
# job a display `name:`, or dropping the workflow's `name:`, makes the
|
||||||
|
# derived context stop matching --- and require_own_context below then
|
||||||
|
# turns every push red with a message. That loud failure is the point
|
||||||
|
# (https://git.eeqj.de/sneak/webhooker/issues/147 item 2); guessing at a
|
||||||
|
# fallback would restore the silent no-op it replaced.
|
||||||
|
context() {
|
||||||
|
printf '%s / %s (%s)' \
|
||||||
|
"$GITHUB_WORKFLOW" "$GITHUB_JOB" "$GITHUB_EVENT_NAME"
|
||||||
|
}
|
||||||
|
|
||||||
|
# ANCESTOR_LIMIT is a documented knob, so a value that is set but
|
||||||
|
# unusable must fail loudly instead of defaulting
|
||||||
|
# (https://git.eeqj.de/sneak/webhooker/issues/80). Passing it straight to
|
||||||
|
# git would print `fatal: not an integer` into a discarded exit status
|
||||||
|
# and mark nothing.
|
||||||
|
ancestor_limit() {
|
||||||
|
# `-` and not `:-`: an explicitly empty value is set-but-unusable
|
||||||
|
# config, so it aborts like any other bad value rather than silently
|
||||||
|
# running at the default.
|
||||||
|
_limit="${ANCESTOR_LIMIT-20}"
|
||||||
|
case "$_limit" in
|
||||||
|
'' | *[!0-9]* | 0*)
|
||||||
|
echo "ANCESTOR_LIMIT must be a positive integer," \
|
||||||
|
"got '${_limit}'" >&2
|
||||||
|
|
||||||
|
return 1
|
||||||
|
;;
|
||||||
|
esac
|
||||||
|
printf '%s' "$_limit"
|
||||||
|
}
|
||||||
|
|
||||||
|
# The status Gitea created for this very job proves which context string
|
||||||
|
# it uses. If the derived one is missing, the workflow or the job was
|
||||||
|
# renamed and the match below would silently stop firing, restoring the
|
||||||
|
# false-red bug with no signal. Fail loudly instead.
|
||||||
|
require_own_context() {
|
||||||
|
if ! _body="$(curl -sf --retry 3 --retry-delay 2 --max-time 30 \
|
||||||
|
"${1}/commits/${GITHUB_SHA}/status")"; then
|
||||||
|
echo "cannot read commit statuses for ${GITHUB_SHA}" >&2
|
||||||
|
return 1
|
||||||
|
fi
|
||||||
|
_found="$(printf '%s' "$_body" | jq -r '(.statuses // [])[].context')"
|
||||||
|
if printf '%s\n' "$_found" | grep -qxF "$2"; then
|
||||||
|
return 0
|
||||||
|
fi
|
||||||
|
echo "no commit status with context '${2}' on ${GITHUB_SHA}:" >&2
|
||||||
|
echo "workflow or job renamed? contexts present:" >&2
|
||||||
|
printf '%s\n' "$_found" >&2
|
||||||
|
return 1
|
||||||
|
}
|
||||||
|
|
||||||
|
# Latest status for our context on a commit, as "state|description".
|
||||||
|
# The read is retried and bounded, and a read that still fails aborts the
|
||||||
|
# step: a laundered commit that cannot be read is not the same as one
|
||||||
|
# with nothing to do, and piping curl into jq would discard the
|
||||||
|
# difference.
|
||||||
|
status_of() {
|
||||||
|
if ! _sbody="$(curl -sf --retry 3 --retry-delay 2 --max-time 30 \
|
||||||
|
"${1}/commits/${2}/status")"; then
|
||||||
|
echo "cannot read commit statuses for ${2}" >&2
|
||||||
|
|
||||||
|
return 1
|
||||||
|
fi
|
||||||
|
printf '%s' "$_sbody" | jq -r --arg c "$3" \
|
||||||
|
'[(.statuses // [])[] | select(.context == $c)][0] // empty
|
||||||
|
| "\(.status)|\(.description)"'
|
||||||
|
}
|
||||||
|
|
||||||
|
mark_superseded() {
|
||||||
|
curl -sf -X POST "${1}/statuses/${2}" \
|
||||||
|
-H "Authorization: token ${GITEA_TOKEN}" \
|
||||||
|
-H 'Content-Type: application/json' \
|
||||||
|
-d "$(jq -nc --arg c "$3" --arg d "$SUPERSEDED_DESC" \
|
||||||
|
'{context: $c, state: "failure", description: $d}')" \
|
||||||
|
>/dev/null
|
||||||
|
}
|
||||||
|
|
||||||
|
main() {
|
||||||
|
_api="${GITHUB_API_URL}/repos/${GITHUB_REPOSITORY}"
|
||||||
|
_ctx="$(context)"
|
||||||
|
|
||||||
|
_limit="$(ancestor_limit)"
|
||||||
|
|
||||||
|
require_own_context "$_api" "$_ctx"
|
||||||
|
|
||||||
|
# A shallow clone cannot resolve the parent, so it looks exactly like
|
||||||
|
# a root commit to rev-parse below and would exit 0 having walked
|
||||||
|
# nothing (or, at depth > 1, only the ancestors that happen to be
|
||||||
|
# present). The workflow checks out with `fetch-depth: 0`; verify
|
||||||
|
# that here rather than depend on it silently.
|
||||||
|
if [ "$(git rev-parse --is-shallow-repository)" = 'true' ]; then
|
||||||
|
echo "shallow repository: the ancestor walk needs full history" >&2
|
||||||
|
|
||||||
|
return 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
# A root commit legitimately has no ancestors and is not an error.
|
||||||
|
# A SHA this repository does not have lands here too, since its
|
||||||
|
# parent is equally unresolvable, but require_own_context above has
|
||||||
|
# already aborted on the 404 for it. The walk itself carries no
|
||||||
|
# `|| true`, so a rev-list failure aborts.
|
||||||
|
if ! git rev-parse -q --verify "${GITHUB_SHA}^" >/dev/null; then
|
||||||
|
echo "no ancestor of ${GITHUB_SHA} to check"
|
||||||
|
|
||||||
|
return 0
|
||||||
|
fi
|
||||||
|
|
||||||
|
_walk="$(git rev-list --max-count="$_limit" "${GITHUB_SHA}^")"
|
||||||
|
|
||||||
|
for _sha in $_walk; do
|
||||||
|
_latest="$(status_of "$_api" "$_sha" "$_ctx")"
|
||||||
|
# A run that was cancelled, or one an earlier revision of this
|
||||||
|
# script laundered into `skipped`. Anything else stands.
|
||||||
|
case "$_latest" in
|
||||||
|
'failure|Has been cancelled' | "skipped|${SUPERSEDED_DESC}") ;;
|
||||||
|
*) continue ;;
|
||||||
|
esac
|
||||||
|
mark_superseded "$_api" "$_sha" "$_ctx"
|
||||||
|
echo "marked superseded: ${_sha}"
|
||||||
|
done
|
||||||
|
}
|
||||||
|
|
||||||
|
main "$@"
|
||||||
@@ -38,7 +38,7 @@
|
|||||||
<div x-show="open" x-cloak class="mt-3 p-3 bg-gray-50 rounded-md">
|
<div x-show="open" x-cloak class="mt-3 p-3 bg-gray-50 rounded-md">
|
||||||
<pre class="text-xs text-gray-700 overflow-x-auto whitespace-pre-wrap break-all">{{.Body}}</pre>
|
<pre class="text-xs text-gray-700 overflow-x-auto whitespace-pre-wrap break-all">{{.Body}}</pre>
|
||||||
{{if .BodyTruncated}}
|
{{if .BodyTruncated}}
|
||||||
<p class="mt-2 text-xs text-gray-500">Body truncated for display: showing {{.BodyShownBytes}} of {{.BodyBytes}} bytes. The stored body is unchanged.</p>
|
<p class="mt-2 text-xs text-gray-500">Body truncated for display: showing {{.BodyShownBytes}} of {{.BodyBytes}} bytes. The stored body is unchanged — <a href="/source/{{$.Webhook.ID}}/logs/{{.ID}}/body" class="text-primary-600 hover:text-primary-700 underline">download the full body</a>.</p>
|
||||||
{{end}}
|
{{end}}
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|||||||
Reference in New Issue
Block a user