319 lines
18 KiB
Markdown
319 lines
18 KiB
Markdown
# Workflow
|
|
|
|
One issue per unit of work, one branch and one PR per issue:
|
|
|
|
* ensure a tracked issue exists with a definition of done
|
|
* branch from `next` (never from `main`)
|
|
* do the work; open a PR based on `next` (never on `main`)
|
|
* pass an independent review, then the manager squash-merges into `next`
|
|
* push; nothing stays local-only
|
|
|
|
`next` is the branch for the next milestone and must stay green and
|
|
mergeable to `main` without notice. One `next` -> `main` PR accumulates
|
|
the milestone; releases are cut from `main` separately.
|
|
|
|
Issue branches do NOT touch this file — the manager maintains it on
|
|
`next`. Every branch editing `TODO.md` conflicts with every other
|
|
(#112).
|
|
|
|
# Status
|
|
|
|
pre-1.0. No git tags exist. `main` (4f5ecb1) is a working webhook proxy
|
|
with auth, CSRF/SSRF protections, login rate limiting, Slack target,
|
|
event retention (#63), the database archiving target (#43), the admin
|
|
password change flow (#65), policy compliance (#6), pinned lint tooling
|
|
(#55), and fail-loud configuration parsing (#80).
|
|
|
|
`next` holds the **complete 1.0.0 milestone**: every issue in it is
|
|
closed, and it is verified green both by CI and by cache-defeated
|
|
container runs (`docker build --no-cache-filter=lint
|
|
--no-cache-filter=builder`).
|
|
|
|
One caveat on reading a green check, narrower than it used to be. A
|
|
docs-only commit deliberately replays from the layer cache (#119), so a
|
|
green status on such a commit evidences a replay rather than an executed
|
|
run; a code commit invalidates the `COPY` layer and genuinely executes.
|
|
Superseded runs are no longer the hazard they were: before #152 they
|
|
were recorded as `skipped` and rolled up green, and before #119 a warm
|
|
layer cache let the gate report success without executing anything,
|
|
replaying the previous build's console log so the lie looked like a real
|
|
run. Both are fixed. Note: `TODO.md` was deliberately
|
|
deleted from this repo in f9a9569 (2026-03-01, #6); its content was
|
|
folded into the README TODO section, which this draft reconstructs as
|
|
of 2026-07-06.
|
|
|
|
# Next Step
|
|
|
|
Merge the milestone PR (#111) to `main` and tag 1.0.0 from it. The
|
|
milestone is empty and `next` is green; nothing else blocks the tag.
|
|
|
|
Three items belong to the owner, none of them blocking. #150 was decided
|
|
by the manager rather than left to stall the queue and is flagged on the
|
|
issue for reversal if that call was wrong. #112 (whether `Completed
|
|
Steps` should exist at all, given it once conflicted on every unit) is
|
|
unanswered; the provisional ruling in force is that issue branches do
|
|
not touch this file. #198 records that `make test` is past the org 20s
|
|
target — 46s of test execution inside a 62.8s CI layer — and turns on
|
|
which quantity the 60s hard cap governs; it is scoped as the improvement
|
|
bug the 20-60s band requires, and should be milestoned instead if the
|
|
cap is read as covering the whole invocation.
|
|
|
|
After the tag, the largest open cluster is the unmilestoned follow-up
|
|
backlog these units generated: #183, #184, #185, #190, #191, #193 and
|
|
#198.
|
|
|
|
# Completed Steps
|
|
|
|
- 2026-08-18 Raise `script/test`'s per-package timeout from 30s to 90s,
|
|
matching the org-wide backstop. `go test` applies `-timeout` per
|
|
package, and `internal/handlers` had grown past the old budget: a
|
|
cache-defeated build failed outright at `GOMAXPROCS=4`, and every run
|
|
under deliberate host load breached 30s. The measurement table lives
|
|
in the script (#194)
|
|
- 2026-08-18 Re-sync `REPO_POLICIES.md` from `prompts`. The local copy
|
|
was stale and still mandated a 20s test target with a 30s timeout,
|
|
which the org replaced with a 60s cap and a 90s backstop. A synced
|
|
copy is not a source; reading it as one nearly produced a PR against
|
|
`prompts` proposing a change already merged there (#196)
|
|
- 2026-08-18 Report handler panics through the logger and answer 500.
|
|
chi v1.5.5's `Recoverer` scans for a `panic(0x` frame the runtime no
|
|
longer emits, then indexes `pkg[-1:]`, so it panicked inside its own
|
|
stack printer before writing a byte: the recovery never ran, the
|
|
client got a dropped connection instead of a 500, and the original
|
|
panic was lost. A local middleware replaces it, bounded by
|
|
`MaxPanicLogLineBytes` (#187)
|
|
- 2026-08-18 Route GORM's logger through `slog` and bound it. Every
|
|
`gorm.Open` left `logger.Default` in place at `Warn` with
|
|
`IgnoreRecordNotFoundError` false, so **every record-not-found
|
|
printed the fully interpolated SQL to stdout** — including the
|
|
client-chosen path on `/webhook/{uuid}` and the submitted username on
|
|
the login form, at no level the operator set and outside
|
|
`internal/logger` entirely. Three call sites, not the two the issue
|
|
named (#178)
|
|
- 2026-08-18 Bound every `slog` line against client-chosen text. Eight
|
|
sites reachable unauthenticated, found by reading every `slog` call in
|
|
the tree rather than only the one reported; the budget moved to a
|
|
shared `internal/logfield` so no second truncation exists. `DEBUG`
|
|
being off by default is not a bound and is not treated as one (#176)
|
|
- 2026-08-18 Stop a slow host turning a login-guard test into a
|
|
segfault. A non-fatal `assert` on an acquire result was dereferenced
|
|
on the next line, so one timing miss killed the whole
|
|
`internal/middleware` binary and reddened CI for unrelated PRs. The
|
|
fix also removed a real production race — `acquire` could shed a
|
|
request with a slot standing free, because Go picks uniformly among
|
|
ready `select` cases (#186)
|
|
- 2026-08-18 Send the chi route pattern to Sentry rather than the
|
|
concrete path. The receiver's path carries the entrypoint capability
|
|
token, so every Sentry event from `/webhook/{uuid}` shipped a live
|
|
credential to a third party. Request `Data`, `QueryString`, `Cookies`
|
|
and `Env` are dropped and headers reduced to an allowlist (#179)
|
|
- 2026-08-18 Read form fields from the POST body only. `r.FormValue`
|
|
merges the query string, so a login could be driven by URL parameters
|
|
— putting the password somewhere that lands in access logs, proxy
|
|
logs and browser history (#160)
|
|
- 2026-08-18 Verify login credentials before spending rate-limit
|
|
budget, so a flood of wrong passwords cannot lock out the account it
|
|
is guessing at. The manager took this decision rather than stall the
|
|
queue; it is flagged on the issue for reversal (#150)
|
|
- 2026-08-18 Run all linting in Docker via `Dockerfile.lint`. Host lint
|
|
was wrong in both directions from version skew and shared caches.
|
|
`script/lint` asserts the summary line, because `--no-cache-filter`
|
|
silently ignores a stage name it does not match — the flag that makes
|
|
the gate meaningful fails open (#109)
|
|
- 2026-08-18 Serve an event's full stored body over HTTP. The list
|
|
query truncates for rendering, and that truncated value was the only
|
|
way to read a body, so the full payload was unreachable (#157)
|
|
- 2026-08-18 Bound the access log line against client-chosen text.
|
|
`internal/logfield` budgets by *encoded* bytes, not runes, so a
|
|
handler's JSON escaping cannot multiply a field past its allowance
|
|
(#146)
|
|
- 2026-08-18 Mark superseded CI commits `failure` rather than
|
|
`skipped`. A skipped run rolls up green, so a commit that was never
|
|
tested reported success (#152)
|
|
- 2026-08-18 Set `fx.StopTimeout` inside the container stop grace, so
|
|
shutdown hooks are bounded by a deadline the orchestrator will
|
|
actually honour rather than being killed mid-flush (#134)
|
|
- 2026-08-17 Bucket IPv6 rate-limit keys by `/64`. A single allocation
|
|
hands out 2^64 addresses, so per-address keying let one client mint
|
|
unlimited buckets. Manager decision, recorded on the issue (#125)
|
|
- 2026-08-17 Correct release-blocking README and startup-warning
|
|
inaccuracies, including claims about behaviour the code does not have
|
|
(#151)
|
|
- 2026-08-17 Fetch and verify Alpine.js at build time against
|
|
`static/vendor.sha256` instead of committing the minified blob, so
|
|
the dependency is pinned by hash rather than by trust (#145)
|
|
- 2026-08-17 Bound the event log's rendered bodies in the query itself,
|
|
so a large stored payload cannot be read into memory just to be
|
|
truncated for display (#135)
|
|
- 2026-08-17 Mask the `http` target's destination URL in the UI: it can
|
|
carry a bearer credential in its path or query, and was rendered
|
|
verbatim. Manager decision to mask unconditionally (#115)
|
|
- 2026-08-14 Bound shutdown hooks by their stop context, so a hook that
|
|
hangs cannot hold the process past its grace period (#102)
|
|
- 2026-08-14 Render templates via a buffer rather than the
|
|
`ResponseWriter`, so a template error part-way through cannot commit
|
|
a 200 and then fail — the response is written only once it is whole
|
|
(#123)
|
|
- 2026-08-14 Align the session codec's max-age with the 7-day absolute
|
|
cap. The codec accepted cookies the session layer considered expired,
|
|
so the cap was enforced in one place and not the other (#108)
|
|
- 2026-08-12 Warn when `TRUSTED_PROXIES` is empty in production, where
|
|
the safe default silently discards forwarded headers and every client
|
|
rate-limits as the proxy's address (#149)
|
|
- 2026-08-12 Bound the receiver rate limit per client IP across the
|
|
whole `/webhook/*` route. The existing limiter keyed on the request
|
|
path and `/webhook/{uuid}` matches any single segment, so a client
|
|
that invented a fresh path per request minted a fresh bucket per
|
|
request: the limit on the only unauthenticated endpoint bounded
|
|
nothing in aggregate, and every request still cost an entrypoint
|
|
lookup before it 404ed. An outer limiter keyed on the client address
|
|
alone now bounds that, chained in front of the unchanged
|
|
per-entrypoint limiter (#139)
|
|
- 2026-08-12 Correct release-blocking documentation inaccuracies: the
|
|
README promised manual redelivery in the present tense in three
|
|
places when nothing implements it (the same false claim also sat in
|
|
the doc comment that was its source text), the env table omitted
|
|
`RETENTION_SWEEP_INTERVAL`, and `TODO.md` itself omitted five landed
|
|
units (#141)
|
|
- 2026-08-12 Make the CI gate execute the checks it reports on. The
|
|
workflow now writes a build-context fingerprint before calling
|
|
`script/cibuild`, so a code commit invalidates the `COPY` layer of
|
|
the lint and builder stages while a docs-only commit still replays
|
|
from cache; a superseding run also rewrites the `failure` status
|
|
Gitea leaves on commits it cancelled and never tested. Verified by
|
|
pushing a deliberately broken test and watching CI go red (#119)
|
|
- 2026-08-12 Require a positive `RETENTION_SWEEP_INTERVAL`: a
|
|
non-positive value reached `time.NewTicker` in both the retention
|
|
reaper and the archive sweeper, panicking two goroutines with no
|
|
recover after startup had already reported success (#140)
|
|
- 2026-08-12 Bound the `X-Forwarded-For` scan's allocation to the hop
|
|
cap: the reverse walk cuts entries with `strings.LastIndexByte`
|
|
instead of joining and splitting, so a 1 MB header allocates 16 bytes
|
|
rather than 1.6 MB per request on the unauthenticated receiver.
|
|
Semantics proven unchanged by differential testing against the
|
|
previous implementation (#133)
|
|
- 2026-08-12 Cap the `X-Forwarded-For` hop walk at 64 entries, so an
|
|
attacker-supplied chain cannot burn unbounded CPU in the rate-limit
|
|
key function; running off the end falls back to the peer address
|
|
(#124)
|
|
- 2026-08-12 Gate forwarded-header trust behind a `TRUSTED_PROXIES` CIDR
|
|
list: all three rate limiters key on the connection's own address
|
|
unless the direct peer is a configured proxy, in which case
|
|
`X-Forwarded-For` is walked right to left for the first non-proxy hop.
|
|
Default trusts nothing, and a set-but-unparseable value aborts
|
|
startup. Before this, any client could mint a fresh bucket or drain
|
|
another's by rotating a spoofed header (#88)
|
|
- 2026-08-11 Web UI cleanup: nav terminology unified on Webhooks, the
|
|
Profile settings placeholder removed, a progressive-enhancement copy
|
|
button for the entrypoint URL, and retention form copy that states the
|
|
actual policy (deletion by the reaper, 0 retains forever) (#57)
|
|
- 2026-08-11 Mask the webhook credential in delivery errors and logs:
|
|
Go embeds the request URL in `*url.Error`, so every transport failure
|
|
persisted the full Slack webhook URL into the per-webhook event
|
|
database via `DeliveryResult.Error`, a field a future REST API would
|
|
have served. `maskURLError` drops path, query and userinfo while
|
|
preserving the wrapped cause, so `errors.Is`/`As` and `Timeout()`
|
|
still work and DNS, TLS and timeout failures still read differently
|
|
(#118)
|
|
- 2026-08-11 Rate-limit the public webhook receiver endpoint
|
|
(`RECEIVER_RATE_LIMIT`, default 120/min), keyed on client IP plus
|
|
entrypoint path so one entrypoint cannot exhaust another's budget;
|
|
over-limit requests get 429 with `Retry-After`. It was the one
|
|
unauthenticated, internet-facing endpoint with no limit at all (#64)
|
|
- 2026-08-11 Enforce the body size limit before CSRF parses the form:
|
|
`MaxBodySize` is now first in all four form-parsing route groups, so
|
|
an oversized request is rejected with 413 instead of being read in
|
|
full by the CSRF middleware before any cap applied (#90)
|
|
- 2026-08-11 Mask target config on the source detail page, which
|
|
rendered the stored blob verbatim and so exposed the Slack
|
|
incoming-webhook URL — a bearer credential that cannot be revoked
|
|
per-holder. Config reaches the template only as a `TargetView` of
|
|
labelled fields, and header values are rendered as a count (#113)
|
|
- 2026-08-11 Allow `retention_days` of 0 to mean retain forever, via a
|
|
sentinel written in `BeforeSave` so the GORM column default cannot
|
|
win the race. Also bounds the reaper's cutoff arithmetic: day counts
|
|
above 106751 overflowed `time.Duration` and wrapped the cutoff into
|
|
the future, where every row matched and the sweep deleted everything
|
|
(#79)
|
|
- 2026-08-09 Inactivity-based session timeout: sliding idle expiry
|
|
(`SESSION_IDLE_TIMEOUT`, default `24h`) refreshed on authenticated
|
|
requests, with the 7-day absolute cap kept as an independent
|
|
backstop that activity never extends (#66)
|
|
- 2026-08-09 Restart recovery and the 60s retry sweep terminally fail an
|
|
orphaned `retrying` delivery whose target type no longer supports
|
|
retries, recording a `DeliveryResult` with the reason instead of
|
|
leaving the delivery stuck forever (#82)
|
|
- 2026-08-09 Root the delivery engine's worker pool and the retention
|
|
reaper's sweep loop at `context.Background()` rather than the fx
|
|
`OnStart` hook context (#97), which carries fx's 15s start timeout and
|
|
killed both roughly fifteen seconds after boot: the proxy silently
|
|
stopped delivering webhooks entirely, and the reaper never ran a
|
|
single sweep under its default one-hour interval
|
|
- 2026-08-09 Archive writer lifecycle (#89): deleting a webhook (or its
|
|
last `database` target) evicts the cached archive writer and closes
|
|
its handle while deliberately leaving `archive-{webhookID}.db` on
|
|
disk, and a new `ArchiveSweeper` prunes idle archives on the existing
|
|
`RETENTION_SWEEP_INTERVAL` without ever creating an archive file
|
|
- 2026-08-09 Configuration parsing fails loudly on set-but-unparseable
|
|
environment values: `envInt` removed in favour of `envPositiveInt`
|
|
plus a `PORT` range check, `envBool` now parses with
|
|
`strconv.ParseBool`, and defaults apply only to unset variables (#80)
|
|
- 2026-08-07 Automatic event retention cleanup based on
|
|
`retention_days`, deleting expired events, deliveries, and delivery
|
|
results from each per-webhook event database (#63)
|
|
- 2026-08-07 Update golangci-lint to v2.12.2 (Docker image digest in
|
|
`Dockerfile`, release-archive sha256 pins in `script/bootstrap`),
|
|
adopt the canonical `.golangci.yml` (v2 `linters.settings` layout so
|
|
`lll`/`funlen`/`cyclop`/`dupl` thresholds actually apply), and fix
|
|
all newly surfaced lint findings
|
|
- 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints,
|
|
Makefile shims, README Entrypoints section
|
|
- 2026-03-25 pin golangci-lint Docker image for linting (#55)
|
|
- 2026-03-18 CSRF middleware detects TLS per-request, fixing login over
|
|
plain HTTP and behind reverse proxies (#54)
|
|
- 2026-03-17 root path redirects based on auth state (#52)
|
|
- 2026-03-17 CSRF protection, SSRF prevention for HTTP delivery targets
|
|
with DNS rebinding defense, and per-IP login rate limiting (#42)
|
|
- 2026-03-17 Slack target type for incoming webhook notifications (#47)
|
|
- 2026-03-17 Dockerfile absolute paths and static linking (#49);
|
|
absolute dev DATA_DIR default and clarified env docs (#46)
|
|
- 2026-03-05 security headers middleware, session regeneration on
|
|
login, request body size limits (#41)
|
|
- 2026-03-04 tests for delivery, middleware, and session packages
|
|
(#32); removed globals.Buildarch (#31)
|
|
- 2026-03-04 1.0 MVP merge: Webhook/Entrypoint/Target rename, core
|
|
delivery engine with bounded worker pool and circuit breaker,
|
|
parallel fan-out, per-webhook event databases, management UI (#16)
|
|
- 2026-03-01 repo brought to REPO_POLICIES standards; TODO.md folded
|
|
into README (#6)
|
|
|
|
# Future Steps
|
|
|
|
- Manual event redelivery from the web UI — the "Replay" capability the
|
|
README describes as planned. No redelivery code exists anywhere in the
|
|
tree; events are stored in full, which is all it would be built on
|
|
- Delivery status and retry management UI
|
|
- Per-webhook rate limiting in the receiver handler (per-webhook config
|
|
plus handler enforcement; global limits must not apply to receiver
|
|
endpoints)
|
|
- Webhook signature verification for GitHub and Stripe HMAC formats
|
|
- API key authentication for programmatic access (APIKey model exists;
|
|
Bearer token middleware does not)
|
|
- REST API v1
|
|
- CRUD for webhooks, entrypoints, targets
|
|
- event viewing and filtering endpoints
|
|
- event redelivery endpoint
|
|
- OpenAPI specification
|
|
- Analytics dashboard: success rates, response times, volume
|
|
- A remember-me option at login
|
|
- Password reset flow for a forgotten password. The authenticated
|
|
password *change* flow already landed on `main` (#65); reset does not
|
|
exist
|
|
- Later, nice to have
|
|
- email delivery target type
|
|
- SNS and S3 delivery targets
|
|
- data transformations (e.g. webhook to Slack message formatting)
|
|
- JSONL file delivery with periodic S3 upload
|
|
- webhook event search and filtering
|
|
- multi-user with role-based access control
|