Compare commits
2 Commits
7d95e9b6f9
...
50a49e8d4e
| Author | SHA1 | Date | |
|---|---|---|---|
| 50a49e8d4e | |||
| c3b6623be1 |
@@ -13,39 +13,19 @@ jobs:
|
||||
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 2024-10-23
|
||||
with:
|
||||
# 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
|
||||
|
||||
- name: Neutralize superseded run statuses
|
||||
- name: Mark superseded run statuses
|
||||
# Gitea cancels the in-flight run when another commit is pushed to the
|
||||
# same branch and records the cancellation as `failure`, so a commit
|
||||
# that was never tested reads red. The cancellation is unconditional
|
||||
# server-side for push events and cannot be disabled from a workflow
|
||||
# file, so the superseding run rewrites those statuses to `skipped`.
|
||||
# Only the exact cancellation status is touched; a real failure is
|
||||
# left alone.
|
||||
# that was never tested reads as a test result. The script rewrites
|
||||
# those statuses to say what happened. See its header for why the
|
||||
# state stays `failure` and not `skipped`.
|
||||
env:
|
||||
GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
|
||||
run: |
|
||||
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
|
||||
run: script/ci-mark-superseded
|
||||
|
||||
- name: Fingerprint the build context
|
||||
# `.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
|
||||
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
|
||||
|
||||
|
||||
42
README.md
42
README.md
@@ -282,6 +282,8 @@ are inline commands with no script behind them. We provide:
|
||||
- `script/docker` — build the Docker image tagged via `script/projectname`
|
||||
- `script/cibuild` — CI entrypoint: `docker build .` (the Dockerfile
|
||||
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/check`)
|
||||
- `script/install-precommit` — install the git pre-commit hook that
|
||||
@@ -1001,7 +1003,14 @@ Every limiter here — receiver, login, and password change — identifies
|
||||
the client the same way, through one shared key function: the
|
||||
connection's own address, unless the peer is listed in
|
||||
`TRUSTED_PROXIES`, in which case the forwarded client address is used
|
||||
instead. See [Trusted proxies](#trusted-proxies). Deployed without that
|
||||
instead. That address becomes a bucket by family: IPv4 keys on the full
|
||||
address, IPv6 on its `/64` prefix. A routed `/64` is the normal
|
||||
residential and mobile IPv6 allocation, so keying IPv6 per address would
|
||||
let one subscriber rotate source addresses and mint a fresh bucket per
|
||||
request, evading these limits at the network layer without spoofing
|
||||
anything; the cost is that distinct clients inside one `/64` share a
|
||||
bucket. IPv4-mapped addresses (`::ffff:1.2.3.4`) key as the IPv4 address
|
||||
they carry. See [Trusted proxies](#trusted-proxies). Deployed without that
|
||||
variable set, a client behind a reverse proxy shares one bucket with
|
||||
every other client behind the same proxy. Set `TRUSTED_PROXIES` to the
|
||||
proxy's address to get per-client limits back. What the shared bucket
|
||||
@@ -1366,11 +1375,32 @@ way.
|
||||
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
|
||||
commit lands on the same branch and records that cancellation as a
|
||||
`failure` status, marking a commit red that was never tested.
|
||||
Cancellation is unconditional server-side for
|
||||
push events, so the superseding run rewrites the exact
|
||||
`Has been cancelled` status to `skipped`. Genuine failures are never
|
||||
touched.
|
||||
`failure` status, so a commit nothing ever tested reads as a test
|
||||
result. Cancellation is unconditional server-side for push events, so
|
||||
the superseding run calls `script/ci-mark-superseded`, which rewrites
|
||||
that exact status to `failure` /
|
||||
`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
|
||||
|
||||
|
||||
2
go.mod
2
go.mod
@@ -17,6 +17,7 @@ require (
|
||||
github.com/stretchr/testify v1.8.4
|
||||
go.uber.org/fx v1.20.1
|
||||
golang.org/x/crypto v0.38.0
|
||||
gopkg.in/yaml.v3 v3.0.1
|
||||
gorm.io/driver/sqlite v1.5.4
|
||||
gorm.io/gorm v1.25.5
|
||||
modernc.org/sqlite v1.28.0
|
||||
@@ -52,7 +53,6 @@ require (
|
||||
golang.org/x/text v0.25.0 // indirect
|
||||
golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect
|
||||
google.golang.org/protobuf v1.31.0 // indirect
|
||||
gopkg.in/yaml.v3 v3.0.1 // indirect
|
||||
lukechampine.com/uint128 v1.2.0 // indirect
|
||||
modernc.org/cc/v3 v3.40.0 // indirect
|
||||
modernc.org/ccgo/v3 v3.16.13 // indirect
|
||||
|
||||
349
internal/ciscript/ci_mark_superseded_test.go
Normal file
349
internal/ciscript/ci_mark_superseded_test.go
Normal file
@@ -0,0 +1,349 @@
|
||||
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))
|
||||
}
|
||||
|
||||
// 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)
|
||||
}
|
||||
}
|
||||
}
|
||||
5
internal/ciscript/doc.go
Normal file
5
internal/ciscript/doc.go
Normal file
@@ -0,0 +1,5 @@
|
||||
// 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.
|
||||
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)
|
||||
}
|
||||
@@ -48,6 +48,12 @@ const (
|
||||
// bound every request pays a walk proportional to whatever the
|
||||
// client sent.
|
||||
maxForwardedHops = 64
|
||||
|
||||
// ipv6BucketBits is the prefix length IPv6 clients are bucketed
|
||||
// on. A routed /64 is the normal residential and mobile
|
||||
// allocation, so it is the unit an attacker gets addresses in
|
||||
// and therefore the unit worth limiting.
|
||||
ipv6BucketBits = 64
|
||||
)
|
||||
|
||||
// normalizeAddr strips the IPv4-in-IPv6 wrapper and any zone from
|
||||
@@ -56,6 +62,40 @@ func normalizeAddr(addr netip.Addr) netip.Addr {
|
||||
return addr.Unmap().WithZone("")
|
||||
}
|
||||
|
||||
// bucketKey is the rate-limit bucket identity of a client address.
|
||||
// IPv4 keys on the full address; IPv6 keys on its /64 prefix,
|
||||
// because keying IPv6 per /128 lets one ordinary subscriber rotate
|
||||
// source addresses inside its own routed /64 and mint a fresh bucket
|
||||
// per request — evading every limiter here at the network layer,
|
||||
// with no spoofing and nothing to detect.
|
||||
//
|
||||
// An IPv4-mapped address (::ffff:1.2.3.4) is keyed as the IPv4
|
||||
// address it carries, never masked to a /64: mapped form all shares
|
||||
// the ::ffff:0:0/96 prefix, so masking would collapse every IPv4
|
||||
// client reaching a proxy that emits it into one bucket. Callers
|
||||
// pass addresses through normalizeAddr, which already unmaps; the
|
||||
// unmap here keeps the property true of the key function itself.
|
||||
//
|
||||
// The two families cannot collide: an IPv4 key is a bare dotted
|
||||
// quad, and an IPv6 key always carries a "/64" suffix.
|
||||
func bucketKey(addr netip.Addr) string {
|
||||
addr = addr.Unmap()
|
||||
|
||||
if addr.Is4() {
|
||||
return addr.String()
|
||||
}
|
||||
|
||||
// Prefix errors only on a negative bit count, on over 32 bits
|
||||
// for an IPv4 address, or on over 128 for IPv6. The count here
|
||||
// is the constant 64 and the IPv4 case returned above, so the
|
||||
// error is unreachable. (The zero Addr does not error either: it
|
||||
// yields the zero Prefix. Neither call site can produce one,
|
||||
// since both parse the address first.)
|
||||
prefix, _ := addr.Prefix(ipv6BucketBits)
|
||||
|
||||
return prefix.String()
|
||||
}
|
||||
|
||||
// isTrustedProxy reports whether addr belongs to a network the
|
||||
// operator listed in TRUSTED_PROXIES. The list is empty by default,
|
||||
// so by default nothing is trusted.
|
||||
@@ -143,6 +183,9 @@ func (m *Middleware) forwardedClientAddr(
|
||||
// another client's bucket, by picking an X-Forwarded-For value —
|
||||
// which makes every limit here decorative against a deliberate
|
||||
// attacker.
|
||||
//
|
||||
// The address that identifies the client is then reduced to a bucket
|
||||
// by bucketKey: full address for IPv4, /64 prefix for IPv6.
|
||||
func (m *Middleware) rateLimitKey(r *http.Request) (string, error) {
|
||||
return m.clientKey(r), nil
|
||||
}
|
||||
@@ -152,23 +195,25 @@ func (m *Middleware) clientKey(r *http.Request) string {
|
||||
peer, err := netip.ParseAddr(ipFromHostPort(r.RemoteAddr))
|
||||
if err != nil {
|
||||
// Not an address we can reason about; key on the raw
|
||||
// value, the most specific identity left. On a
|
||||
// Unix-socket listener every peer carries the same
|
||||
// RemoteAddr and so shares one bucket, which is the
|
||||
// fail-closed direction.
|
||||
// value, the most specific identity left. Distinct
|
||||
// RemoteAddr values stay in distinct buckets, so this
|
||||
// path cannot silently collapse unrelated clients
|
||||
// together. On a Unix-socket listener every peer
|
||||
// carries the same RemoteAddr and so shares one bucket,
|
||||
// which is the fail-closed direction.
|
||||
return r.RemoteAddr
|
||||
}
|
||||
|
||||
peer = normalizeAddr(peer)
|
||||
if !m.isTrustedProxy(peer) {
|
||||
return peer.String()
|
||||
return bucketKey(peer)
|
||||
}
|
||||
|
||||
if addr, ok := m.forwardedClientAddr(r); ok {
|
||||
return addr.String()
|
||||
return bucketKey(addr)
|
||||
}
|
||||
|
||||
return peer.String()
|
||||
return bucketKey(peer)
|
||||
}
|
||||
|
||||
// tooManyRequests returns the 429 handler used by the login,
|
||||
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"sneak.berlin/go/webhooker/internal/config"
|
||||
"sneak.berlin/go/webhooker/internal/middleware"
|
||||
)
|
||||
@@ -370,6 +371,30 @@ const (
|
||||
headerXFF = "X-Forwarded-For"
|
||||
headerReal = "X-Real-IP"
|
||||
headerTrue = "True-Client-IP"
|
||||
|
||||
// clientIPv4 is the sample IPv4 client address these tests key
|
||||
// on, both directly and in IPv4-mapped form. clientIPv4Alt is
|
||||
// its neighbour, used to show the two do not share a bucket.
|
||||
clientIPv4 = "198.51.100.7"
|
||||
clientIPv4Alt = "198.51.100.8"
|
||||
|
||||
// clientIPv6 and clientIPv6Same are two addresses inside one
|
||||
// routed /64, so both must key on clientBucketV6.
|
||||
// clientIPv6Other is a different allocation and must key on
|
||||
// clientOtherBucketV6.
|
||||
clientIPv6 = "2001:db8:1:2:3:4:5:6"
|
||||
clientIPv6Same = "2001:db8:1:2:aaaa:bbbb:cccc:dddd"
|
||||
clientIPv6Other = "2001:db8:1:3::1"
|
||||
clientBucketV6 = "2001:db8:1:2::/64"
|
||||
clientOtherBucketV6 = "2001:db8:1:3::/64"
|
||||
|
||||
// trustedProxyCIDR is the proxy network the forwarded-path
|
||||
// tests configure, and trustedPeer an address inside it. A
|
||||
// production deployment is required to run behind a reverse
|
||||
// proxy with TRUSTED_PROXIES set, so this is the shape the
|
||||
// bucketing has to hold in.
|
||||
trustedProxyCIDR = "10.0.0.0/8"
|
||||
trustedPeer = "10.0.0.1:44444"
|
||||
)
|
||||
|
||||
// assertSharedBucket drives the login limiter from peer with the
|
||||
@@ -458,8 +483,8 @@ func TestRateLimitKey_SingleValuedHeadersIgnoredFromTrustedPeer(
|
||||
t.Parallel()
|
||||
|
||||
assertSharedBucket(
|
||||
t, trustedProxies("10.0.0.0/8"),
|
||||
"10.0.0.1:44444",
|
||||
t, trustedProxies(trustedProxyCIDR),
|
||||
trustedPeer,
|
||||
func(i int) map[string]string {
|
||||
return map[string]string{
|
||||
header: fmt.Sprintf(
|
||||
@@ -495,8 +520,8 @@ func TestRateLimitKey_MalformedRightmostHopFallsBackToPeer(
|
||||
t.Parallel()
|
||||
|
||||
assertSharedBucket(
|
||||
t, trustedProxies("10.0.0.0/8"),
|
||||
"10.0.0.1:44444",
|
||||
t, trustedProxies(trustedProxyCIDR),
|
||||
trustedPeer,
|
||||
func(i int) map[string]string {
|
||||
return map[string]string{
|
||||
headerXFF: fmt.Sprintf(
|
||||
@@ -522,13 +547,13 @@ func TestRateLimitKey_ForwardedHonouredFromTrustedPeer(
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{
|
||||
TrustedProxies: trustedProxies("10.0.0.0/8"),
|
||||
TrustedProxies: trustedProxies(trustedProxyCIDR),
|
||||
})
|
||||
handler := m.LoginRateLimit()(okHandler())
|
||||
|
||||
const peer = "10.0.0.1:44444"
|
||||
const peer = trustedPeer
|
||||
|
||||
first := map[string]string{headerXFF: "198.51.100.7"}
|
||||
first := map[string]string{headerXFF: clientIPv4}
|
||||
|
||||
for range middleware.LoginRateLimitConst {
|
||||
postWithHeaders(handler, peer, loginPath, first)
|
||||
@@ -542,7 +567,7 @@ func TestRateLimitKey_ForwardedHonouredFromTrustedPeer(
|
||||
|
||||
w = postWithHeaders(
|
||||
handler, peer, loginPath,
|
||||
map[string]string{headerXFF: "198.51.100.8"},
|
||||
map[string]string{headerXFF: clientIPv4Alt},
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
@@ -559,7 +584,7 @@ func TestRateLimitKey_ChainWalkSkipsClientPrepended(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assertSharedBucket(
|
||||
t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
|
||||
t, trustedProxies(trustedProxyCIDR), trustedPeer,
|
||||
func(i int) map[string]string {
|
||||
return map[string]string{
|
||||
headerXFF: fmt.Sprintf(
|
||||
@@ -594,7 +619,7 @@ func TestRateLimitKey_LongChainCapsWalkAndFallsBackToPeer(
|
||||
start := time.Now()
|
||||
|
||||
assertSharedBucket(
|
||||
t, trustedProxies("10.0.0.0/8"), "10.0.0.1:44444",
|
||||
t, trustedProxies(trustedProxyCIDR), trustedPeer,
|
||||
func(i int) map[string]string {
|
||||
return map[string]string{
|
||||
headerXFF: fmt.Sprintf("9.9.9.%d%s", i+1, padding),
|
||||
@@ -633,13 +658,13 @@ func TestRateLimitKey_LongChainAllocationIsBounded(t *testing.T) {
|
||||
)
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{
|
||||
TrustedProxies: trustedProxies("10.0.0.0/8"),
|
||||
TrustedProxies: trustedProxies(trustedProxyCIDR),
|
||||
})
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodPost, loginPath, nil,
|
||||
)
|
||||
req.RemoteAddr = "10.0.0.1:44444"
|
||||
req.RemoteAddr = trustedPeer
|
||||
req.Header.Set(
|
||||
headerXFF, "9.9.9.9"+strings.Repeat(", 10.0.0.2", hops),
|
||||
)
|
||||
@@ -835,3 +860,369 @@ func TestReceiverRateLimit_IgnoresForwardedFromUntrustedPeer(
|
||||
"not mint a fresh receiver bucket",
|
||||
)
|
||||
}
|
||||
|
||||
// clientKeyFor returns the bucket key m computes for a request whose
|
||||
// direct peer is remoteAddr and which carries no forwarded headers.
|
||||
func clientKeyFor(
|
||||
t *testing.T, m *middleware.Middleware, remoteAddr string,
|
||||
) string {
|
||||
t.Helper()
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodPost, loginPath, nil,
|
||||
)
|
||||
req.RemoteAddr = remoteAddr
|
||||
|
||||
return middleware.ClientKeyForTest(m, req)
|
||||
}
|
||||
|
||||
// TestRateLimitKey_IPv6BucketsByPrefix pins the key function's
|
||||
// address-family behaviour. IPv6 clients must bucket by /64 — a
|
||||
// routed /64 is the normal residential and mobile allocation, so
|
||||
// per-/128 keying lets one subscriber rotate source addresses and
|
||||
// mint a fresh bucket per request — while IPv4 keeps keying on the
|
||||
// full address and IPv4-mapped form is keyed as the IPv4 address it
|
||||
// carries.
|
||||
func TestRateLimitKey_IPv6BucketsByPrefix(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{})
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
peer string
|
||||
want string
|
||||
about string
|
||||
}{{
|
||||
name: "ipv6",
|
||||
peer: "[" + clientIPv6 + "]:44444",
|
||||
want: clientBucketV6,
|
||||
about: "an IPv6 peer must key on its /64",
|
||||
}, {
|
||||
name: "ipv6-other-in-same-64",
|
||||
peer: "[" + clientIPv6Same + "]:1",
|
||||
want: clientBucketV6,
|
||||
about: "another address in the same /64 must key the same",
|
||||
}, {
|
||||
name: "ipv6-different-64",
|
||||
peer: "[" + clientIPv6Other + "]:44444",
|
||||
want: clientOtherBucketV6,
|
||||
about: "a different /64 must key differently",
|
||||
}, {
|
||||
name: "ipv4",
|
||||
peer: clientIPv4 + ":44444",
|
||||
want: clientIPv4,
|
||||
about: "IPv4 must keep keying on the full address",
|
||||
}, {
|
||||
name: "ipv4-neighbour",
|
||||
peer: clientIPv4Alt + ":44444",
|
||||
want: clientIPv4Alt,
|
||||
about: "adjacent IPv4 addresses must not share a bucket",
|
||||
}, {
|
||||
name: "ipv4-mapped",
|
||||
peer: "[::ffff:" + clientIPv4 + "]:44444",
|
||||
want: clientIPv4,
|
||||
about: "IPv4-mapped form must key as the IPv4 address, " +
|
||||
"not be masked to a /64: mapped addresses all share " +
|
||||
"::ffff:0:0/96, so masking would collapse every IPv4 " +
|
||||
"client behind a mapping proxy into one bucket",
|
||||
}} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.Equal(
|
||||
t, tc.want, clientKeyFor(t, m, tc.peer), tc.about,
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestRateLimitKey_FamiliesDoNotCollide pins the structure the
|
||||
// no-collision property rests on, rather than one sample pair: every
|
||||
// IPv4 key is a bare address and every IPv6 key is a /64 in CIDR
|
||||
// form, so the two name spaces are disjoint by shape. Dropping the
|
||||
// masking strips the suffix that guarantees it, which is why this
|
||||
// asserts the form of each key and not just that two of them differ.
|
||||
func TestRateLimitKey_FamiliesDoNotCollide(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Restated here rather than imported from the package under
|
||||
// test, so that changing the production bucket width fails this
|
||||
// test instead of silently moving with it.
|
||||
const wantBits = 64
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{})
|
||||
|
||||
v4Keys := map[string]bool{}
|
||||
|
||||
for _, peer := range []string{
|
||||
clientIPv4 + ":44444",
|
||||
clientIPv4Alt + ":44444",
|
||||
"[::ffff:" + clientIPv4 + "]:44444",
|
||||
} {
|
||||
key := clientKeyFor(t, m, peer)
|
||||
|
||||
addr, err := netip.ParseAddr(key)
|
||||
require.NoError(
|
||||
t, err, "%s: an IPv4 key must be a bare address", peer,
|
||||
)
|
||||
assert.True(
|
||||
t, addr.Is4(),
|
||||
"%s: an IPv4 key must be a dotted quad, got %q", peer, key,
|
||||
)
|
||||
|
||||
v4Keys[key] = true
|
||||
}
|
||||
|
||||
for _, peer := range []string{
|
||||
"[" + clientIPv6 + "]:44444",
|
||||
"[" + clientIPv6Same + "]:44444",
|
||||
"[" + clientIPv6Other + "]:44444",
|
||||
"[2001:db8::" + clientIPv4 + "]:44444",
|
||||
} {
|
||||
key := clientKeyFor(t, m, peer)
|
||||
|
||||
prefix, err := netip.ParsePrefix(key)
|
||||
require.NoError(
|
||||
t, err, "%s: an IPv6 key must be a CIDR prefix", peer,
|
||||
)
|
||||
assert.Equal(
|
||||
t, wantBits, prefix.Bits(),
|
||||
"%s: an IPv6 key must name a /64", peer,
|
||||
)
|
||||
assert.False(
|
||||
t, v4Keys[key],
|
||||
"%s: an IPv6 key must never equal an IPv4 key", peer,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRateLimitKey_UnparseablePeerKeepsDistinctBuckets covers the
|
||||
// fallback path. A RemoteAddr that is not an address must not panic,
|
||||
// and must not drop unrelated clients into one shared bucket by
|
||||
// accident: the raw value is the most specific identity left, so
|
||||
// distinct values stay in distinct buckets.
|
||||
func TestRateLimitKey_UnparseablePeerKeepsDistinctBuckets(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{})
|
||||
|
||||
first := clientKeyFor(t, m, "not-an-address")
|
||||
second := clientKeyFor(t, m, "also-not-an-address:1234")
|
||||
|
||||
assert.NotEmpty(t, first)
|
||||
assert.NotEqual(
|
||||
t, first, second,
|
||||
"unparseable peers must not collapse into one bucket",
|
||||
)
|
||||
}
|
||||
|
||||
// TestLoginRateLimit_IPv6SharesBucketWithinSlash64 is the behavioural
|
||||
// half, and the regression test for the bypass itself: a client that
|
||||
// rotates source addresses inside its own routed /64 must stay in one
|
||||
// bucket. Reverting the masking makes this test fail, because each
|
||||
// rotated address would mint a fresh bucket and nothing would be
|
||||
// rejected.
|
||||
func TestLoginRateLimit_IPv6SharesBucketWithinSlash64(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{})
|
||||
handler := m.LoginRateLimit()(okHandler())
|
||||
|
||||
for i := range middleware.LoginRateLimitConst {
|
||||
w := postWithHeaders(
|
||||
handler,
|
||||
fmt.Sprintf("[2001:db8:1:2::%d]:44444", i+1),
|
||||
loginPath, nil,
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code, "request %d should pass", i,
|
||||
)
|
||||
}
|
||||
|
||||
w := postWithHeaders(
|
||||
handler, "[2001:db8:1:2::ffff]:44444", loginPath, nil,
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusTooManyRequests, w.Code,
|
||||
"rotating source addresses inside one routed /64 must not "+
|
||||
"mint fresh buckets",
|
||||
)
|
||||
}
|
||||
|
||||
// TestLoginRateLimit_IPv6IndependentAcrossSlash64 is the other side
|
||||
// of the trade: bucketing by /64 must not merge separate allocations,
|
||||
// so a client in a different /64 keeps its own limit.
|
||||
func TestLoginRateLimit_IPv6IndependentAcrossSlash64(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{})
|
||||
handler := m.LoginRateLimit()(okHandler())
|
||||
|
||||
for range middleware.LoginRateLimitConst + 1 {
|
||||
postWithHeaders(
|
||||
handler, "[2001:db8:1:2::1]:44444", loginPath, nil,
|
||||
)
|
||||
}
|
||||
|
||||
w := postWithHeaders(
|
||||
handler, "[2001:db8:1:3::1]:44444", loginPath, nil,
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
"a different /64 must have its own bucket",
|
||||
)
|
||||
}
|
||||
|
||||
// TestLoginRateLimit_IPv4IndependentPerAddress guards against the
|
||||
// masking leaking into IPv4: two addresses one apart must still hold
|
||||
// separate buckets.
|
||||
func TestLoginRateLimit_IPv4IndependentPerAddress(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{})
|
||||
handler := m.LoginRateLimit()(okHandler())
|
||||
|
||||
for range middleware.LoginRateLimitConst + 1 {
|
||||
postWithHeaders(
|
||||
handler, clientIPv4+":44444", loginPath, nil,
|
||||
)
|
||||
}
|
||||
|
||||
w := postWithHeaders(
|
||||
handler, clientIPv4Alt+":44444", loginPath, nil,
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
"a second IPv4 address must have its own bucket",
|
||||
)
|
||||
}
|
||||
|
||||
// forwardedKeyFor returns the bucket key m computes for a request
|
||||
// that arrives from trustedPeer — a configured trusted proxy — and
|
||||
// names forwarded as its client in X-Forwarded-For. That is the
|
||||
// production path: a deployment is required to run behind a reverse
|
||||
// proxy with TRUSTED_PROXIES set, so the forwarded address, not the
|
||||
// peer, is what the limiters bucket on there.
|
||||
func forwardedKeyFor(
|
||||
t *testing.T, m *middleware.Middleware, forwarded string,
|
||||
) string {
|
||||
t.Helper()
|
||||
|
||||
req := httptest.NewRequestWithContext(
|
||||
context.Background(), http.MethodPost, loginPath, nil,
|
||||
)
|
||||
req.RemoteAddr = trustedPeer
|
||||
req.Header.Set(headerXFF, forwarded)
|
||||
|
||||
return middleware.ClientKeyForTest(m, req)
|
||||
}
|
||||
|
||||
// TestRateLimitKey_ForwardedIPv6BucketsByPrefix pins the /64
|
||||
// bucketing on the trusted-proxy branch. The direct-peer tests above
|
||||
// cannot reach it, so without this the masking could be reverted for
|
||||
// forwarded clients alone — the only shape a production deployment
|
||||
// runs in — and the rest of the suite would stay green.
|
||||
func TestRateLimitKey_ForwardedIPv6BucketsByPrefix(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{
|
||||
TrustedProxies: trustedProxies(trustedProxyCIDR),
|
||||
})
|
||||
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
forwarded string
|
||||
want string
|
||||
about string
|
||||
}{{
|
||||
name: "ipv6",
|
||||
forwarded: clientIPv6,
|
||||
want: clientBucketV6,
|
||||
about: "a forwarded IPv6 client must key on its /64",
|
||||
}, {
|
||||
name: "ipv6-other-in-same-64",
|
||||
forwarded: clientIPv6Same,
|
||||
want: clientBucketV6,
|
||||
about: "another forwarded address in the same /64 must " +
|
||||
"key the same",
|
||||
}, {
|
||||
name: "ipv6-different-64",
|
||||
forwarded: clientIPv6Other,
|
||||
want: clientOtherBucketV6,
|
||||
about: "a forwarded address in another /64 must differ",
|
||||
}, {
|
||||
name: "ipv4",
|
||||
forwarded: clientIPv4,
|
||||
want: clientIPv4,
|
||||
about: "a forwarded IPv4 client must key on the address",
|
||||
}, {
|
||||
name: "ipv4-mapped",
|
||||
forwarded: "::ffff:" + clientIPv4,
|
||||
want: clientIPv4,
|
||||
about: "a proxy that forwards IPv4-mapped form must key as " +
|
||||
"the IPv4 address it carries, not be masked to a /64: " +
|
||||
"mapped addresses all share ::ffff:0:0/96",
|
||||
}} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
assert.Equal(
|
||||
t, tc.want,
|
||||
forwardedKeyFor(t, m, tc.forwarded), tc.about,
|
||||
)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestLoginRateLimit_ForwardedIPv6SharesBucketWithinSlash64 is the
|
||||
// behavioural half on the production path: behind a trusted proxy, a
|
||||
// client rotating source addresses inside its own routed /64 must
|
||||
// stay in one bucket.
|
||||
func TestLoginRateLimit_ForwardedIPv6SharesBucketWithinSlash64(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
assertSharedBucket(
|
||||
t, trustedProxies(trustedProxyCIDR), trustedPeer,
|
||||
func(i int) map[string]string {
|
||||
return map[string]string{
|
||||
headerXFF: fmt.Sprintf("2001:db8:1:2::%d", i+1),
|
||||
}
|
||||
},
|
||||
"rotating forwarded source addresses inside one routed /64 "+
|
||||
"must not mint fresh buckets",
|
||||
)
|
||||
}
|
||||
|
||||
// TestLoginRateLimit_ForwardedIPv6IndependentAcrossSlash64 is the
|
||||
// other side of that trade on the same path: bucketing by /64 must
|
||||
// not merge two allocations reaching the proxy.
|
||||
func TestLoginRateLimit_ForwardedIPv6IndependentAcrossSlash64(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
m := rateLimitMiddleware(t, &config.Config{
|
||||
TrustedProxies: trustedProxies(trustedProxyCIDR),
|
||||
})
|
||||
handler := m.LoginRateLimit()(okHandler())
|
||||
|
||||
spent := map[string]string{headerXFF: clientIPv6}
|
||||
for range middleware.LoginRateLimitConst + 1 {
|
||||
postWithHeaders(handler, trustedPeer, loginPath, spent)
|
||||
}
|
||||
|
||||
w := postWithHeaders(
|
||||
handler, trustedPeer, loginPath,
|
||||
map[string]string{headerXFF: clientIPv6Other},
|
||||
)
|
||||
assert.Equal(
|
||||
t, http.StatusOK, w.Code,
|
||||
"a forwarded client in a different /64 must have its own "+
|
||||
"bucket",
|
||||
)
|
||||
}
|
||||
|
||||
150
script/ci-mark-superseded
Executable file
150
script/ci-mark-superseded
Executable file
@@ -0,0 +1,150 @@
|
||||
#!/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;
|
||||
# every other rev-list failure (an unknown SHA) must abort, so the
|
||||
# walk itself carries no `|| true`.
|
||||
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 "$@"
|
||||
Reference in New Issue
Block a user