diff --git a/.env.example b/.env.example new file mode 100644 index 0000000..b78baf9 --- /dev/null +++ b/.env.example @@ -0,0 +1,57 @@ +# Hymical Forms configuration. +# +# Copy this file to `.env`, or set the same variables in your process environment. +# FORMS_DATABASE_URL is required. Everything below it is optional and shown with +# its built-in default; uncomment the lines you want to change. + +# SQLAlchemy database URL. PostgreSQL is the intended production database. +FORMS_DATABASE_URL=postgresql+psycopg://forms:forms@localhost:5432/forms + +# SQLite is supported for local experimentation and backs the test suite. It is +# not a supported production target. +# FORMS_DATABASE_URL=sqlite:///./forms.db + +# Largest request body accepted, in bytes. File uploads are not supported, so +# this only needs to accommodate text form fields. +# FORMS_MAX_BODY_BYTES=262144 + +# Largest number of name/value pairs accepted in one submission. A repeated +# field name (a checkbox group) counts once per submitted value. +# FORMS_MAX_FIELDS=100 + +# Largest field name accepted, in characters. +# FORMS_MAX_FIELD_NAME_LENGTH=128 + +# Largest field value accepted, in characters. +# FORMS_MAX_FIELD_VALUE_LENGTH=16384 + +# How long to wait for a webhook destination to accept a connection, in seconds. +# FORMS_WEBHOOK_CONNECT_TIMEOUT_SECONDS=5 + +# How long to wait for a webhook destination to respond, in seconds. +# FORMS_WEBHOOK_READ_TIMEOUT_SECONDS=10 + +# How many delivery attempts a submission gets before it is given up on. +# FORMS_WEBHOOK_MAX_ATTEMPTS=5 + +# Wait before the second attempt, in seconds. Each later wait doubles it. +# FORMS_WEBHOOK_RETRY_INITIAL_SECONDS=10 + +# Cap on the wait between attempts, however far the backoff has doubled. +# FORMS_WEBHOOK_RETRY_MAX_SECONDS=3600 + +# How long the worker waits before looking for due deliveries again, in seconds. +# FORMS_WORKER_POLL_SECONDS=1 + +# How many deliveries a worker claims at once. +# FORMS_WORKER_BATCH_SIZE=10 + +# How long a worker's claim on a delivery holds, in seconds. After this the +# delivery becomes claimable again, which is how work is recovered from a worker +# that died holding it. +# FORMS_WORKER_LEASE_SECONDS=60 + +# Permit webhook destinations on loopback and private addresses, so a webhook can +# point at a server on your own machine. Development only: enabling this in +# production lets anyone who can create an endpoint reach your internal network. +# FORMS_ALLOW_PRIVATE_WEBHOOK_TARGETS=false diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..63161a0 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,32 @@ +name: CI + +on: + push: + branches: [main] + pull_request: + +permissions: + contents: read + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + python-version: ["3.11", "3.12", "3.13"] + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + cache: pip + - run: pip install -e ".[dev]" + - run: ruff check . + - run: ruff format --check . + - run: mypy + - run: pytest diff --git a/.gitignore b/.gitignore index 83972fa..d3323fe 100644 --- a/.gitignore +++ b/.gitignore @@ -61,6 +61,12 @@ local_settings.py db.sqlite3 db.sqlite3-journal +# Local SQLite databases, e.g. FORMS_DATABASE_URL=sqlite:///./forms.db +*.db +*.db-journal +*.sqlite +*.sqlite3 + # Flask stuff: instance/ .webassets-cache diff --git a/README.md b/README.md index f9cc2d9..760ecdb 100644 --- a/README.md +++ b/README.md @@ -1,2 +1,647 @@ -# forms -Reliable form submission infrastructure with validation, storage, webhooks, retries, and delivery tracking +

+ Hymical Forms logo +

+ +

Hymical Forms

+ +

+ Reliable form ingestion and webhook delivery for developers. +

+ +## The problem + +Every project with a contact form, a waitlist, or a feedback box ends up needing +the same small backend: something that accepts an HTML form POST, validates it, +stores it, and forwards it somewhere useful. Writing that once is easy; running +it reliably, with retries, delivery logs, spam handling and retention rules, +is not. Hymical Forms is intended to be that backend, self-hostable and +open-source. + +## Project status + +**Early development.** This build registers endpoints, stores the submissions +sent to them together with the durable obligation to deliver them, and runs a +separate worker that performs the signed webhook delivery and retries it. + +Endpoint and webhook configuration is completely unauthenticated: anyone who can +reach the API can create an endpoint pointing anywhere. There is no rate limiting +and no spam protection, so do not expose this to the public internet. + +| Capability | Status | +| ----------------------------- | ------------------------- | +| Health endpoint | Implemented | +| Form ingestion + validation | Implemented | +| Request limits + error model | Implemented | +| Endpoint registry | Implemented | +| Submission persistence | Implemented | +| Idempotent retries | Implemented | +| Signed webhook delivery | Implemented | +| Durable delivery queue | Implemented | +| Retries with backoff | Implemented | +| API keys / authentication | **Not implemented** | +| Manual delivery replay | **Not implemented** | +| Rate limiting, spam handling | **Not implemented** | +| Schema migrations | **Not implemented** | +| Export, retention, dashboards | **Not implemented** | + +## Requirements + +- Python 3.11 or newer +- PostgreSQL, which is the intended production database + +SQLite is supported for local experimentation and backs the test suite. It is +not a supported production target. + +## Install + +```bash +python -m venv .venv && . .venv/bin/activate && pip install -e ".[dev]" +``` + +On Windows, activate with `.venv\Scripts\activate` instead. + +## Configure + +`FORMS_DATABASE_URL` is required and has no default. Set it in the environment +or in a `.env` file in the working directory: + +```bash +FORMS_DATABASE_URL=postgresql+psycopg://forms:forms@localhost:5432/forms +``` + +To try the service without running PostgreSQL: + +```bash +FORMS_DATABASE_URL=sqlite:///./forms.db +``` + +See [`.env.example`](.env.example) for every setting and its default. + +## Run + +Hymical Forms is two processes sharing one database. + +```bash +uvicorn hymical_forms.main:app --reload +``` + +```bash +python -m hymical_forms.worker +``` + +The **API** accepts submissions, stores them, and records that a webhook is owed. +It never makes an outbound request. The **worker** claims owed deliveries, sends +them, and retries the ones that fail. Running the API alone is fine: submissions +are still accepted and nothing is lost, they simply wait until a worker exists. + +Missing tables are created at startup, so an empty database is enough to begin. +Startup fails if the database cannot be reached, rather than serving requests +that would only fail later. There is no migration framework yet, so startup +never alters a table that already exists; see [Limitations](#limitations). + +> **Upgrading from an earlier build:** the schema has changed in every release so +> far, most recently by adding the `webhook_deliveries` table and giving +> `delivery_attempts` a `delivery_id` and `attempt_number`. Startup creates +> missing tables but never alters an existing one, so a database created before +> these changes has to be recreated. For local SQLite, delete the file and +> restart. For PostgreSQL, `DROP TABLE delivery_attempts, webhook_deliveries, +> submissions, endpoints;` and restart. There is no in-place upgrade path. +> +> Three consecutive schema changes with no migration tool is the clearest +> remaining infrastructure gap. Alembic is the next thing this project needs, +> and it should arrive before there is a database worth not dropping. + +Interactive API documentation is served at `http://127.0.0.1:8000/docs`. + +## API + +### `GET /health` + +Reports that the API process is running. + +```json +{ "status": "ok", "service": "hymical-forms", "version": "0.1.0" } +``` + +This is a liveness signal only. It does not check the database, so it stays +answerable while the database is down, which is what makes it useful for +deciding whether to restart the process. + +### `POST /endpoints` + +Registers a form endpoint. Submissions are only accepted for endpoints that +exist here. + +**This route is unauthenticated.** Authentication is deliberately out of scope +for now, so keep the service on a private network. + +```bash +curl -X POST http://127.0.0.1:8000/endpoints \ + -H 'Content-Type: application/json' \ + -d '{"id": "contact-form", "name": "Contact form", + "webhook_url": "https://example.com/hooks/forms"}' +``` + +| Field | Required | Meaning | +| ------------- | -------- | -------------------------------------------------- | +| `id` | yes | The public identifier the endpoint answers on | +| `name` | yes | Human-readable label, 1 to 200 characters | +| `is_active` | no | Whether it accepts submissions, defaults to `true` | +| `webhook_url` | no | Where accepted submissions are delivered | + +**Endpoint IDs** are supplied by you, not generated, because the ID appears in +the `action` URL of your HTML form and a memorable one is worth more than an +opaque one. An ID is 3 to 64 characters of lowercase ASCII letters, digits, `-` +and `_`, and must start and end with a letter or digit. It is also the primary +key, so it cannot be changed later. + +Returns `201 Created`: + +```json +{ + "id": "contact-form", + "name": "Contact form", + "is_active": true, + "created_at": "2026-08-24T14:34:27.432598Z", + "webhook_url": "https://example.com/hooks/forms", + "webhook_secret": "whsec_6f1c... (64 hex characters)" +} +``` + +> **Save `webhook_secret` now.** It is generated by the server, returned only in +> this response, and there is no route that reads it back. Losing it means +> creating a new endpoint. + +Reusing an ID returns `409 endpoint_already_exists`. There is no route to list, +update or delete endpoints yet, so a webhook can only be configured at creation +time. + +### `POST /f/{endpoint_id}` + +Accepts a form submission for a registered endpoint and stores it. + +A submission to an ID that does not exist is rejected with +`404 endpoint_not_found`, and one to an inactive endpoint with +`409 endpoint_inactive`. Neither leaves anything in the database. + +**Content types.** `application/x-www-form-urlencoded` and +`multipart/form-data` are both accepted, so a plain HTML `
` works +unchanged. File uploads are not: a multipart part carrying a file is rejected +rather than silently dropped. Anything else is rejected with `415`. + +**Repeated field names**, such as checkbox groups and multi-selects, are +preserved in order, both in the response count and in storage. No submitted +value is discarded. + +A successful request returns `202 Accepted`. The status is deliberately not +`201`: the submission is stored and any delivery it owes is queued, but that +delivery has not happened yet. + +```json +{ + "submission_id": "sub_48984534f33749c49a88de2d59400dce", + "endpoint_id": "contact-form", + "received_at": "2026-08-24T14:34:27.651841Z", + "field_count": 3, + "idempotent_replay": false, + "delivery": { "queued": true } +} +``` + +Submitted values are not echoed back, because the client already has them. + +`delivery.queued` says whether a webhook delivery is owed for this submission. +No outbound request is made during this request, so the response says nothing +about whether a destination is reachable: that is the worker's business, and a +destination being down can no longer affect whether a form is accepted. + +### Retrying safely with `Idempotency-Key` + +A client that never sees a response cannot tell whether the submission landed. +Send an `Idempotency-Key` header and the retry becomes safe: the second request +returns the result of the first instead of storing the form twice. + +```bash +KEY=$(uuidgen) +curl -i -X POST http://127.0.0.1:8000/f/contact-form \ + -H "Idempotency-Key: $KEY" \ + -d email=dev@example.com -d message=hello +``` + +The header is optional. Without it, behaviour is unchanged and every accepted +request stores a new submission. + +**Retry semantics.** Repeating the same key on the same endpoint with the same +content returns `202` with the *original* `submission_id` and `received_at`, and +`"idempotent_replay": true`. Only one row is ever stored, and this holds even +when the retries arrive at the same instant: the database, not the application, +is what decides the winner. + +The server does not retry anything on your behalf. It only makes your retries +safe. + +**Conflict semantics.** Reusing a key on the same endpoint with *different* +content is rejected with `409 idempotency_conflict`. The stored submission is +left exactly as it was, and the response never describes its contents. + +Content is compared by a SHA-256 fingerprint of the normalized fields. Because +field order and repeated values are meaningful to this service, they are +meaningful to the fingerprint too: `a=1&b=2` and `b=2&a=1` are different +payloads and will conflict. The generated submission ID and the received +timestamp are excluded, so an honest retry always matches. + +**Scope.** A key belongs to one endpoint. The same key may be used once per +endpoint without conflicting, and there is no expiry: a key is spent for as long +as its submission is stored. + +**Key format.** 16 to 255 printable ASCII characters with no spaces, which +accepts UUIDs, hex, base64 and base64url. Anything else is rejected with +`400 invalid_idempotency_key`, including a header that is present but empty. + +The 16-character floor exists because keys are endpoint-scoped and this API is +unauthenticated, so every client of an endpoint shares one key space. A short or +predictable key would collide with a stranger's submission. Use a random value. + +### Webhook delivery + +If an endpoint has a `webhook_url`, accepting a submission also writes a durable +delivery record in the **same transaction**. Nothing is sent during the form +request. A worker picks the delivery up, sends it, and retries it if it fails. + +That transaction is the whole reliability claim. Once `POST /f/{endpoint_id}` +answers `202`, the submission is stored *and* the obligation to deliver it is +stored. A crash at any point after that cannot lose the delivery, because it is +a row rather than a thing the API process was about to do. + +The submission response reports only whether work was queued: + +```json +"delivery": { "queued": true } +``` + +`queued` is false when the endpoint has no webhook. An idempotent replay reports +`true` and does **not** queue a second delivery: one submission owes at most one +delivery, enforced by a unique constraint on the submission. + +#### Payload + +```json +{ + "type": "submission.received", + "submission": { + "id": "sub_48984534f33749c49a88de2d59400dce", + "endpoint_id": "contact-form", + "received_at": "2026-08-24T14:34:27.651841Z", + "fields": { + "email": ["user@example.com"], + "topics": ["billing", "api"] + } + } +} +``` + +Every field value is a list, even when only one value was submitted, so a +receiver never has to guess whether a field is single or multi valued. The +signing secret is never part of the payload. + +#### Verifying the signature + +Signing is unchanged from the previous release. Each request carries a +`Hymical-Signature` header: + +``` +Hymical-Signature: v1=9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08 +``` + +The digest is HMAC-SHA256 of the **raw request body**, keyed with the endpoint's +`webhook_secret`. Verify against the exact bytes you received, before parsing +the JSON: re-serializing the payload will produce different bytes and fail. + +```python +import hashlib +import hmac + + +def verify(raw_body: bytes, header: str, secret: str) -> bool: + version, _, digest = header.partition("=") + if version != "v1": + return False + expected = hmac.new(secret.encode(), raw_body, hashlib.sha256).hexdigest() + return hmac.compare_digest(digest, expected) +``` + +Use a constant-time comparison, as `hmac.compare_digest` does above. The `v1=` +prefix exists so a future scheme can be added without breaking receivers that +only understand this one. + +A delivery keeps the destination and secret that were configured when the +submission was accepted. Changing an endpoint's webhook does not redirect +deliveries that are already queued, and does not leave a queued payload signed +with a secret its receiver never had. + +#### Delivery states + +| State | Meaning | +| ------------ | ------------------------------------------------------- | +| `pending` | Owed, waiting for its next attempt time | +| `processing` | Claimed by a worker, holding a lease | +| `delivered` | A destination answered `2xx`. Terminal | +| `failed` | Given up on. Terminal | + +#### What retries, and what does not + +| Outcome | Retried | +| ---------------------------------- | ------- | +| Connection failure | yes | +| Timeout | yes | +| HTTP `5xx` | yes | +| HTTP `408`, `425`, `429` | yes | +| HTTP `2xx` | delivered | +| Any other `4xx`, including `409` | no | +| `3xx` | no | + +Ordinary `4xx` responses are final because repeating a request the receiver +called malformed, unauthorized or missing will not repair it. `409` is treated +as final too: from a webhook receiver it almost always means "I already have +this event". Redirects are still not followed, and a `3xx` is a misconfiguration +rather than a passing problem, so it is final as well. + +#### Retry schedule + +Delivery is attempted immediately, then backs off by doubling, capped: + +| Attempt | Waits before it | +| ------- | --------------- | +| 1 | none | +| 2 | 10s | +| 3 | 20s | +| 4 | 40s | +| 5 | 80s | + +After `FORMS_WEBHOOK_MAX_ATTEMPTS` (default 5) the delivery becomes `failed` and +is never retried. There is no jitter: the schedule is deliberately exactly +predictable. Every attempt, including the last, stays in `delivery_attempts`. + +#### At-least-once, not exactly-once + +A worker claims a delivery by taking a lease on it. If the worker dies, the +lease expires and another worker picks the delivery up, which is what stops a +crash from stranding work in `processing` forever. + +**This means duplicate delivery is possible.** A worker that sends successfully +and then dies before recording that success leaves a delivery that looks unsent, +and the next worker sends it again. No queue can close this window on its own; +it needs the receiver's cooperation. Use the `id` in the signed payload to +ignore an event you have already processed. Hymical Forms does not offer +exactly-once delivery and there is no message broker involved: PostgreSQL is +the queue. + +#### What is recorded + +`webhook_deliveries` holds one row per logical delivery: its state, how many +attempts it has had, when it is next due, and when it finished. +`delivery_attempts` holds one row per request that actually went out, numbered, +with the outcome, the HTTP status when there was one, and a bounded failure +message. Response bodies are **not** stored, and neither is the signing secret. +A job that is inspected and found not due records nothing. There is no API to +read either table yet; query them directly. + +#### Destinations that are refused + +A webhook URL must use `http` or `https` and must not name a loopback, private, +link-local, multicast, reserved or unspecified address. That covers `localhost`, +`127.0.0.1`, `[::1]`, `[::ffff:127.0.0.1]`, `10.0.0.0/8`, `192.168.0.0/16`, and +the `169.254.169.254` cloud metadata endpoint. Rejections return +`422 invalid_webhook_url`. + +This is **not** complete SSRF protection; see [Limitations](#limitations). + +For local development, `FORMS_ALLOW_PRIVATE_WEBHOOK_TARGETS=true` lifts the +address restriction so you can point a webhook at a server on your own machine. +Do not enable it in production. + +### Try it + +```bash +curl -X POST http://127.0.0.1:8000/endpoints \ + -H 'Content-Type: application/json' \ + -d '{"id": "contact-form", "name": "Contact form"}' +``` + +```bash +curl -i -X POST http://127.0.0.1:8000/f/contact-form -d email=dev@example.com -d message=hello +``` + +Or from a browser, against a locally running server: + +```html + + + + +
+``` + +### Errors + +Every non-2xx response uses one envelope. `code` is stable and +machine-readable; `details` appears only when there is something concrete to +add. + +```json +{ + "error": { + "code": "too_many_fields", + "message": "Submission carries 120 fields, which exceeds the limit of 100.", + "details": { "limit": 100, "received": 120 } + } +} +``` + +| Status | `code` | Cause | +| ------ | -------------------------- | -------------------------------------------------------- | +| 400 | `malformed_form_body` | Body does not parse as the declared content type | +| 400 | `invalid_idempotency_key` | `Idempotency-Key` header breaks the key format rules | +| 404 | `invalid_endpoint_id` | Submission path is not a well-formed endpoint ID | +| 404 | `endpoint_not_found` | Endpoint ID is well formed but no such endpoint exists | +| 404 | `not_found` | Unknown path | +| 405 | `method_not_allowed` | Wrong method for a known path | +| 409 | `endpoint_inactive` | Endpoint exists but is not accepting submissions | +| 409 | `endpoint_already_exists` | Endpoint ID is already taken | +| 409 | `idempotency_conflict` | Idempotency key already used for different content | +| 413 | `request_body_too_large` | Body exceeded `FORMS_MAX_BODY_BYTES` | +| 415 | `unsupported_media_type` | Content type is not a supported form encoding | +| 422 | `empty_submission` | No fields were submitted | +| 422 | `invalid_endpoint_id` | Endpoint ID in a request body breaks the ID rules | +| 422 | `invalid_request` | Request body failed schema validation | +| 422 | `invalid_webhook_url` | Webhook destination is malformed or not permitted | +| 422 | `file_upload_not_supported`| A multipart part carried a file | +| 422 | ingestion rule codes | See below | +| 500 | `internal_error` | Unexpected failure; no internals are exposed | +| 503 | `storage_unavailable` | The database could not be reached or written to | + +Ingestion rule codes are `too_many_fields`, `field_name_too_long`, +`field_value_too_long`, `invalid_field_name` and `invalid_field_value`. + +`invalid_endpoint_id` carries a different status depending on where the ID came +from: `404` when it arrived as a submission path that addresses nothing, `422` +when it arrived as a field in a request body. + +## Configuration + +All settings are read from `FORMS_`-prefixed environment variables, or from a +`.env` file in the working directory. + +| Variable | Default | Meaning | +| ------------------------------ | ---------- | --------------------------------------- | +| `FORMS_DATABASE_URL` | *required* | SQLAlchemy database URL | +| `FORMS_MAX_BODY_BYTES` | `262144` | Largest accepted request body, in bytes | +| `FORMS_MAX_FIELDS` | `100` | Largest number of name/value pairs | +| `FORMS_MAX_FIELD_NAME_LENGTH` | `128` | Largest field name, in characters | +| `FORMS_MAX_FIELD_VALUE_LENGTH` | `16384` | Largest field value, in characters | +| `FORMS_WEBHOOK_CONNECT_TIMEOUT_SECONDS` | `5` | Wait for a webhook to accept a connection | +| `FORMS_WEBHOOK_READ_TIMEOUT_SECONDS` | `10` | Wait for a webhook to respond | +| `FORMS_ALLOW_PRIVATE_WEBHOOK_TARGETS` | `false` | Permit loopback and private webhook targets. Development only | +| `FORMS_WEBHOOK_MAX_ATTEMPTS` | `5` | Attempts before a delivery is given up on | +| `FORMS_WEBHOOK_RETRY_INITIAL_SECONDS` | `10` | Wait before the second attempt; later waits double | +| `FORMS_WEBHOOK_RETRY_MAX_SECONDS` | `3600` | Cap on the wait between attempts | +| `FORMS_WORKER_POLL_SECONDS` | `1` | How often an idle worker looks for work | +| `FORMS_WORKER_BATCH_SIZE` | `10` | Deliveries a worker claims at once | +| `FORMS_WORKER_LEASE_SECONDS` | `60` | How long a worker's claim holds | + +## Development + +```bash +pytest # run the test suite +ruff check . # lint +ruff format --check . # formatting check +mypy # type check +``` + +Tests run against an in-memory SQLite database, one per test, so no database +server is needed and nothing is left behind. + +### Layout + +``` +src/hymical_forms/ + app.py application assembly and startup + config.py typed settings + db.py engine, session, and schema lifecycle + errors.py the shared JSON error envelope + delivery.py the outbound webhook request itself + ingestion.py domain rules: endpoint IDs, submission validation + middleware.py request body size limit + models.py the persisted schema + storage.py queries and writes + webhooks.py webhook rules: URL validation, payload, signature, retry policy + worker.py the delivery worker process + main.py ASGI entrypoint + api/ HTTP routes and response models +``` + +`ingestion.py` and `webhooks.py` hold the domain rules and know nothing about +HTTP or the database. `models.py` and `storage.py` are the only modules that +write queries, and `delivery.py` is the only one that makes an outbound request. +`api/` translates requests into domain rules and storage calls, and their +outcomes into responses. `worker.py` is a separate process and shares only the +database with the API. + +### Storage notes + +Submission fields are stored as a JSON object mapping each field name to the +list of values submitted under it, which is how repeated names survive intact. +On PostgreSQL the column is `json` rather than `jsonb`, because `jsonb` +normalises object key order and would silently reorder a form's fields. + +Each request runs in one transaction, committed explicitly rather than in the +session teardown, so a failure becomes an error response instead of a success +for a row that never landed. A failure anywhere before the commit leaves the +database untouched. + +An idempotency key is unique per endpoint through a database constraint on +`(endpoint_id, idempotency_key)`. Both PostgreSQL and SQLite treat NULLs in a +unique constraint as distinct, so submissions sent without a key stay +unrestricted without needing a partial index. A lookup before inserting is only +an optimisation for the common retry; when two requests race, one insert loses +on the constraint, rolls back and reads the winner's row. A `CHECK` constraint +keeps the key and its fingerprint either both set or both absent. + +A submission and the delivery it owes are written in one transaction. Either +both land or neither does, so there is no state in which a form was accepted but +the promise to deliver it went missing, and none in which delivery work exists +for a submission that does not. + +The network call happens later, in the worker, with no transaction open. Holding +a database transaction across a call to somebody else's server would tie the +connection pool to how fast that server answers. + +Workers claim deliveries with `SELECT ... FOR UPDATE SKIP LOCKED` on PostgreSQL, +so two workers scanning at once are handed different rows rather than fighting +over the same one. SQLite has no such locking and silently ignores `FOR UPDATE`, +so the claim also performs a conditional update and treats a row as claimed only +if that update matched. That guard is redundant under `SKIP LOCKED` and is what +makes the claim safe on SQLite. + +## Limitations + +- **Delivery is at-least-once, never exactly-once.** A worker that delivers + successfully and dies before recording it will have its lease expire, and the + next worker will deliver the same event again. Deduplicate on the submission + `id` in the signed payload. +- **PostgreSQL worker concurrency is not exercised by the test suite.** Tests run + on SQLite, which cannot model `SELECT ... FOR UPDATE SKIP LOCKED`. The + generated PostgreSQL SQL is asserted, and the claim is written so that it is + also correct without row locking, but two real workers racing on PostgreSQL has + not been run. A PostgreSQL service in CI is the way to close this. +- **A failed delivery is final and cannot be replayed.** Once a delivery reaches + `failed`, nothing retries it and there is no manual replay route. +- **The lease must outlast a delivery attempt.** A batch is delivered + concurrently, so it takes about as long as its slowest single delivery rather + than the sum, but if `FORMS_WORKER_LEASE_SECONDS` were set below the connect + and read timeouts combined, another worker could claim a delivery that is still + in flight and send it twice. The defaults leave a wide margin; keep it that way + if you change them. +- **SSRF protection is partial.** Destination URLs are checked for scheme and for + literal internal addresses, and redirects are not followed. Hostnames are + **not** resolved, so a name that resolves to a private address still passes, + and DNS rebinding is not addressed at all. Closing this properly means + resolving at request time and pinning the connection to the validated address. + Treat the current checks as a guardrail against mistakes, not a defence against + an attacker who can configure endpoints. +- **No authentication.** Anyone who can reach the API can create an endpoint, + point its webhook anywhere, and post to any active one. There is no rate + limiting or spam protection. +- **A webhook can only be set when the endpoint is created.** There is no route + to change a destination or rotate a signing secret. +- **No API for delivery attempts.** They are recorded, but reading them means + querying the database directly. +- **No migration framework.** Startup creates missing tables and nothing else, + so any future change to an existing column has to be applied by hand. + Alembic will arrive when the schema first needs to change. +- **No way to read submissions back over the API.** They are stored, but + retrieval, export and retention are not implemented. +- **No route to list, update or delete endpoints.** +- **No file uploads.** Multipart text fields are accepted; file parts are + rejected. +- **`multipart/form-data` bodies are buffered in memory,** bounded by + `FORMS_MAX_BODY_BYTES`. +- A rejected submission reveals whether an endpoint ID exists, which allows + enumeration. This is unavoidable while the API is unauthenticated. +- **Idempotency keys never expire.** A key stays spent for as long as its + submission is stored, so the table only grows. Expiry belongs with retention. +- **Idempotency keys are shared across all clients of an endpoint,** because + there is nothing to scope them to yet. Guessing another client's key returns + that submission's ID and timestamp, though never its contents. Random keys of + the required length make this impractical, and API keys will close it properly. +- **A replay is only recognised once the first attempt has committed.** A retry + sent while the original is still in flight is treated as a concurrent request, + which is safe, but a retry sent after the original *failed* is a new + submission, which is correct. +- Submission IDs are opaque and not yet guaranteed stable in format. + +## License + +[Apache License 2.0](LICENSE). diff --git a/docs/images/logo_symbol_tramsparent.png b/docs/images/logo_symbol_transparent.png similarity index 100% rename from docs/images/logo_symbol_tramsparent.png rename to docs/images/logo_symbol_transparent.png diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..84c4f77 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,77 @@ +[build-system] +requires = ["hatchling"] +build-backend = "hatchling.build" + +[project] +name = "hymical-forms" +dynamic = ["version"] +description = "Reliable form ingestion and webhook delivery for developers." +readme = "README.md" +license = "Apache-2.0" +license-files = ["LICENSE"] +requires-python = ">=3.11" +authors = [{ name = "Hymical" }] +keywords = ["forms", "webhooks", "ingestion", "fastapi"] +classifiers = [ + "Development Status :: 3 - Alpha", + "Framework :: FastAPI", + "Intended Audience :: Developers", + "Programming Language :: Python :: 3", + "Topic :: Internet :: WWW/HTTP :: HTTP Servers", +] +dependencies = [ + "fastapi>=0.115", + "httpx2>=2.0", # outbound webhook client, and the transport starlette's TestClient uses + "psycopg[binary]>=3.1", # PostgreSQL driver for the intended production database + "pydantic>=2.7", + "pydantic-settings>=2.3", + "python-multipart>=0.0.9", + "sqlalchemy>=2.0", + "uvicorn>=0.30", +] + +[project.optional-dependencies] +dev = [ + "mypy>=1.11", + "pytest>=8.3", + "ruff>=0.6", +] + +[project.urls] +Homepage = "https://github.com/hymical/forms" +Source = "https://github.com/hymical/forms" +Issues = "https://github.com/hymical/forms/issues" + +[tool.hatch.version] +path = "src/hymical_forms/__init__.py" + +[tool.hatch.build.targets.wheel] +packages = ["src/hymical_forms"] + +[tool.pytest.ini_options] +testpaths = ["tests"] +addopts = "-q --strict-markers --strict-config" + +[tool.ruff] +target-version = "py311" +line-length = 100 +src = ["src", "tests"] + +[tool.ruff.lint] +select = [ + "E", # pycodestyle errors + "F", # pyflakes + "I", # import sorting + "UP", # pyupgrade + "B", # bugbear + "SIM", # simplify + "RUF", # ruff-specific +] + +[tool.mypy] +python_version = "3.11" +files = ["src", "tests"] +strict = true +# Teaches mypy that a BaseSettings subclass can be constructed with no arguments +# because its values come from the environment. +plugins = ["pydantic.mypy"] diff --git a/src/hymical_forms/__init__.py b/src/hymical_forms/__init__.py new file mode 100644 index 0000000..1a58e53 --- /dev/null +++ b/src/hymical_forms/__init__.py @@ -0,0 +1,7 @@ +""" +hymical forms: reliable form ingestion and webhook delivery for developers +""" + +__version__ = "0.1.0" + +__all__ = ["__version__"] diff --git a/src/hymical_forms/api/__init__.py b/src/hymical_forms/api/__init__.py new file mode 100644 index 0000000..f0e63dd --- /dev/null +++ b/src/hymical_forms/api/__init__.py @@ -0,0 +1,3 @@ +""" +the HTTP layer: routing, request parsing, and response shapes +""" diff --git a/src/hymical_forms/api/endpoints.py b/src/hymical_forms/api/endpoints.py new file mode 100644 index 0000000..eb78fdd --- /dev/null +++ b/src/hymical_forms/api/endpoints.py @@ -0,0 +1,188 @@ +""" +endpoint management: ``POST /endpoints`` +""" + +from __future__ import annotations + +from datetime import datetime +from http import HTTPStatus + +from fastapi import APIRouter, Request +from pydantic import BaseModel, Field + +from hymical_forms import storage, webhooks +from hymical_forms.config import Settings +from hymical_forms.db import SessionDep +from hymical_forms.errors import ApiError, ErrorResponse +from hymical_forms.ingestion import ENDPOINT_ID_RULE, is_valid_endpoint_id +from hymical_forms.models import ENDPOINT_NAME_MAX_LENGTH +from hymical_forms.webhooks import WEBHOOK_URL_MAX_LENGTH + +router = APIRouter(tags=["endpoints"]) + + +class InvalidEndpointId(ApiError): + """ + raised when a submitted endpoint identifier does not follow the rules + """ + + # The ingestion route answers 404 for a malformed ID because the path simply + # does not address an endpoint. Here the ID arrives in a request body, where + # the same problem is an unprocessable field rather than a missing resource. + status_code = HTTPStatus.UNPROCESSABLE_ENTITY + code = "invalid_endpoint_id" + + def __init__(self) -> None: + """ + state the endpoint identifier rules the request failed + """ + super().__init__(ENDPOINT_ID_RULE, details={"field": "id"}) + + +class EndpointIdConflict(ApiError): + """ + raised when the requested endpoint identifier is already taken + """ + + status_code = HTTPStatus.CONFLICT + code = "endpoint_already_exists" + + def __init__(self, endpoint_id: str) -> None: + """ + name the endpoint identifier that is already in use + :param endpoint_id: the identifier that collided + """ + super().__init__( + f"An endpoint with the ID {endpoint_id!r} already exists.", + details={"endpoint_id": endpoint_id}, + ) + + +class InvalidWebhookUrl(ApiError): + """ + raised when a webhook destination is malformed or not permitted + """ + + status_code = HTTPStatus.UNPROCESSABLE_ENTITY + code = "invalid_webhook_url" + + def __init__(self, reason: str) -> None: + """ + report why the destination was refused + :param reason: short phrase completing "the webhook URL ..." + """ + super().__init__(f"The webhook URL {reason}.", details={"field": "webhook_url"}) + + +class CreateEndpointRequest(BaseModel): + """ + the body accepted when creating an endpoint + """ + + id: str = Field(description=ENDPOINT_ID_RULE) + name: str = Field( + min_length=1, + max_length=ENDPOINT_NAME_MAX_LENGTH, + description="Human-readable label, shown to whoever administers the endpoint.", + ) + is_active: bool = Field( + default=True, + description="Whether the endpoint accepts submissions. Inactive endpoints reject them.", + ) + webhook_url: str | None = Field( + default=None, + max_length=WEBHOOK_URL_MAX_LENGTH, + description=( + "Optional http or https destination to deliver accepted submissions to. " + "A signing secret is generated for it and returned once." + ), + ) + + +class EndpointResponse(BaseModel): + """ + an endpoint as returned by the API + """ + + id: str = Field(description="The public identifier the endpoint answers on.") + name: str = Field(description="Human-readable label for the endpoint.") + is_active: bool = Field(description="Whether the endpoint currently accepts submissions.") + created_at: datetime = Field(description="UTC timestamp of when the endpoint was created.") + webhook_url: str | None = Field( + description="Where accepted submissions are delivered, or null if none is configured." + ) + webhook_secret: str | None = Field( + description=( + "The signing secret for this endpoint's webhook. Returned only here, at " + "creation, and never retrievable again. Null if no webhook is configured." + ) + ) + + +@router.post( + "/endpoints", + status_code=HTTPStatus.CREATED, + summary="Create a form endpoint", + responses={ + 409: {"model": ErrorResponse, "description": "Endpoint ID already taken"}, + 422: { + "model": ErrorResponse, + "description": "Invalid endpoint ID, name, or webhook URL", + }, + 503: {"model": ErrorResponse, "description": "Database unavailable"}, + }, +) +def create_endpoint( + payload: CreateEndpointRequest, request: Request, session: SessionDep +) -> EndpointResponse: + """ + create an endpoint that submissions may then be addressed to + :param payload: the endpoint identifier, label, active state and optional webhook + :param request: the incoming request, read for the active configuration + :param session: the session this request does its database work through + :returns: the endpoint as persisted, including its signing secret if one was made + """ + # A plain ``def`` route, so FastAPI runs it in a worker thread and the + # synchronous database calls never block the event loop. + if not is_valid_endpoint_id(payload.id): + raise InvalidEndpointId() + + settings: Settings = request.app.state.settings + secret: str | None = None + if payload.webhook_url is not None: + try: + webhooks.validate_webhook_url( + payload.webhook_url, + allow_private_targets=settings.allow_private_webhook_targets, + ) + except webhooks.WebhookUrlRejected as exc: + raise InvalidWebhookUrl(exc.reason) from exc + # Generated here rather than accepted from the caller, so its strength is + # this service's responsibility and never an integrator's oversight. + secret = webhooks.new_signing_secret() + + try: + endpoint = storage.create_endpoint( + session, + endpoint_id=payload.id, + name=payload.name, + is_active=payload.is_active, + webhook_url=payload.webhook_url, + webhook_secret=secret, + ) + except storage.EndpointAlreadyExists as exc: + raise EndpointIdConflict(payload.id) from exc + + session.commit() + + # The secret leaves the service exactly once, in this response. There is no + # route that reads it back, so a caller that loses it has to make a new + # endpoint rather than being handed the old secret again. + return EndpointResponse( + id=endpoint.id, + name=endpoint.name, + is_active=endpoint.is_active, + created_at=endpoint.created_at, + webhook_url=endpoint.webhook_url, + webhook_secret=secret, + ) diff --git a/src/hymical_forms/api/health.py b/src/hymical_forms/api/health.py new file mode 100644 index 0000000..d3836ed --- /dev/null +++ b/src/hymical_forms/api/health.py @@ -0,0 +1,36 @@ +""" +health endpoint +""" + +from __future__ import annotations + +from typing import Literal + +from fastapi import APIRouter +from pydantic import BaseModel + +from hymical_forms import __version__ + +router = APIRouter(tags=["health"]) + + +class HealthResponse(BaseModel): + """ + liveness report for a hymical forms process + """ + + status: Literal["ok"] + service: str + version: str + + +@router.get("/health", summary="Report process health") +async def health() -> HealthResponse: + """ + report that the api process is running and able to serve requests + :returns: a liveness payload naming the service and its version + """ + # This is a liveness signal only. Hymical Forms has no external dependencies + # yet, so there is nothing to distinguish readiness from liveness; a separate + # readiness endpoint will arrive with persistence. + return HealthResponse(status="ok", service="hymical-forms", version=__version__) diff --git a/src/hymical_forms/api/submissions.py b/src/hymical_forms/api/submissions.py new file mode 100644 index 0000000..8276cb3 --- /dev/null +++ b/src/hymical_forms/api/submissions.py @@ -0,0 +1,433 @@ +""" +form ingestion endpoint: ``POST /f/{endpoint_id}`` +""" + +from __future__ import annotations + +import math +from datetime import datetime +from http import HTTPStatus + +from fastapi import APIRouter, Request +from pydantic import BaseModel, Field +from python_multipart.exceptions import ParseError +from starlette.concurrency import run_in_threadpool +from starlette.datastructures import UploadFile +from starlette.formparsers import FormParser, MultiPartException, MultiPartParser + +from hymical_forms import storage +from hymical_forms.config import Settings +from hymical_forms.db import SessionDep +from hymical_forms.errors import ApiError, ErrorResponse +from hymical_forms.ingestion import ( + ENDPOINT_ID_RULE, + IDEMPOTENCY_KEY_RULE, + build_submission, + is_valid_endpoint_id, + is_valid_idempotency_key, + payload_fingerprint, +) +from hymical_forms.webhooks import WebhookTarget + +IDEMPOTENCY_KEY_HEADER = "Idempotency-Key" + +URLENCODED = "application/x-www-form-urlencoded" +MULTIPART = "multipart/form-data" +SUPPORTED_MEDIA_TYPES = (URLENCODED, MULTIPART) + +# Content-Type values are echoed back to help developers debug their form tags, +# but only ever a bounded prefix of what the client sent. +_MEDIA_TYPE_ECHO_LIMIT = 128 + +router = APIRouter(tags=["submissions"]) + + +class InvalidEndpointId(ApiError): + """ + raised when the path segment is not a well-formed endpoint identifier + """ + + status_code = HTTPStatus.NOT_FOUND + code = "invalid_endpoint_id" + + def __init__(self) -> None: + """ + state the endpoint identifier rules the request failed + """ + super().__init__(f"The path does not address a form endpoint. {ENDPOINT_ID_RULE}") + + +class EndpointNotFound(ApiError): + """ + raised when the identifier is well formed but no such endpoint exists + """ + + # Deliberately the same status as a malformed ID: from outside, both mean the + # path does not address a form endpoint. The code tells the two apart. + status_code = HTTPStatus.NOT_FOUND + code = "endpoint_not_found" + + def __init__(self, endpoint_id: str) -> None: + """ + name the endpoint identifier that could not be resolved + :param endpoint_id: the identifier taken from the request path + """ + super().__init__( + f"No form endpoint with the ID {endpoint_id!r} exists.", + details={"endpoint_id": endpoint_id}, + ) + + +class EndpointInactive(ApiError): + """ + raised when the endpoint exists but is not accepting submissions + """ + + status_code = HTTPStatus.CONFLICT + code = "endpoint_inactive" + + def __init__(self, endpoint_id: str) -> None: + """ + name the endpoint identifier that is not accepting submissions + :param endpoint_id: the identifier taken from the request path + """ + super().__init__( + f"The form endpoint {endpoint_id!r} is not accepting submissions.", + details={"endpoint_id": endpoint_id}, + ) + + +class InvalidIdempotencyKey(ApiError): + """ + raised when the ``Idempotency-Key`` header is present but unusable + """ + + # A malformed header is a framing problem rather than a semantic one, which + # is what separates this from the 422 an unacceptable submission earns. + status_code = HTTPStatus.BAD_REQUEST + code = "invalid_idempotency_key" + + def __init__(self) -> None: + """ + state the idempotency key rules the request failed + """ + super().__init__( + f"The {IDEMPOTENCY_KEY_HEADER} header is not usable. {IDEMPOTENCY_KEY_RULE}" + ) + + +class IdempotencyConflict(ApiError): + """ + raised when an idempotency key was already spent on different content + """ + + status_code = HTTPStatus.CONFLICT + code = "idempotency_conflict" + + def __init__(self, endpoint_id: str, idempotency_key: str) -> None: + """ + report that the key is already tied to a different submission + :param endpoint_id: the endpoint the key is scoped to + :param idempotency_key: the key the client reused + """ + # The earlier submission's content is never described, only the fact that + # it differs, so the key cannot be used to read back someone else's form. + super().__init__( + f"The {IDEMPOTENCY_KEY_HEADER} {idempotency_key!r} was already used on endpoint " + f"{endpoint_id!r} for a different submission.", + details={"endpoint_id": endpoint_id, "idempotency_key": idempotency_key}, + ) + + +class UnsupportedMediaType(ApiError): + """ + raised when the request used a content type the endpoint cannot parse + """ + + status_code = HTTPStatus.UNSUPPORTED_MEDIA_TYPE + code = "unsupported_media_type" + + def __init__(self, received: str) -> None: + """ + report the rejected content type alongside the supported ones + :param received: the normalized media type taken from the request + """ + super().__init__( + f"Form submissions must be sent as {URLENCODED} or {MULTIPART}.", + details={ + "received": received[:_MEDIA_TYPE_ECHO_LIMIT] or None, + "supported": list(SUPPORTED_MEDIA_TYPES), + }, + ) + + +class MalformedFormBody(ApiError): + """ + raised when the body did not parse as the declared form content type + """ + + status_code = HTTPStatus.BAD_REQUEST + code = "malformed_form_body" + + def __init__(self, reason: str) -> None: + """ + report why the body could not be parsed + :param reason: the form parser's description of what went wrong + """ + super().__init__( + "The request body could not be parsed as form data.", + details={"reason": reason}, + ) + + +class FileUploadNotSupported(ApiError): + """ + raised when a multipart part carries a file, which this service does not accept + """ + + status_code = HTTPStatus.UNPROCESSABLE_ENTITY + code = "file_upload_not_supported" + + def __init__(self, field_name: str) -> None: + """ + name the field that carried a file part + :param field_name: name of the offending multipart field + """ + super().__init__( + f"Field {field_name!r} carries a file upload, which is not supported.", + details={"field": field_name}, + ) + + +class DeliveryStatus(BaseModel): + """ + whether this submission owes a webhook delivery + """ + + queued: bool = Field( + description=( + "True when a durable webhook delivery exists for this submission. False " + "when the endpoint has no webhook. A delivery is queued once and is not " + "queued again by an idempotent replay, so a replay of a webhook-enabled " + "submission still reports true." + ) + ) + + +class SubmissionAccepted(BaseModel): + """ + acknowledgement returned for an accepted submission + """ + + # The submitted values are not echoed back: the client already has them, and + # reflecting user input adds nothing but risk. + submission_id: str = Field(description="Opaque identifier generated for this submission.") + endpoint_id: str = Field(description="The endpoint the submission was addressed to.") + received_at: datetime = Field(description="UTC timestamp of when the API accepted the body.") + field_count: int = Field(description="Number of name/value pairs the submission carried.") + idempotent_replay: bool = Field( + description=( + "True when this response describes a submission an earlier request already " + "stored, rather than one created now. Always false without an " + "Idempotency-Key header." + ), + ) + delivery: DeliveryStatus = Field( + description="Whether a webhook delivery is owed for this submission." + ) + + +@router.post( + "/f/{endpoint_id}", + status_code=HTTPStatus.ACCEPTED, + summary="Submit a form", + responses={ + 400: {"model": ErrorResponse, "description": "Malformed form body or idempotency key"}, + 404: {"model": ErrorResponse, "description": "Invalid or unknown endpoint ID"}, + 409: { + "model": ErrorResponse, + "description": "Endpoint is not accepting submissions, or idempotency key reused", + }, + 413: {"model": ErrorResponse, "description": "Request body too large"}, + 415: {"model": ErrorResponse, "description": "Unsupported content type"}, + 422: {"model": ErrorResponse, "description": "Submission rejected by an ingestion rule"}, + 503: {"model": ErrorResponse, "description": "Database unavailable"}, + }, +) +async def submit(endpoint_id: str, request: Request, session: SessionDep) -> SubmissionAccepted: + """ + accept an html form submission and store it + :param endpoint_id: endpoint identifier taken from the request path + :param request: the incoming request, read for its content type and body + :param session: the session this request does its database work through + :returns: an acknowledgement carrying the stored submission's metadata + """ + # The response is 202 Accepted rather than 201 Created: the submission is + # stored, but the delivery it was accepted for has not happened yet. + # + # The endpoint is resolved before the body is parsed, so an unknown endpoint + # costs one indexed lookup rather than a full parse of a body we would throw + # away. This handler must stay ``async`` to stream the body, so each blocking + # database call is handed to a worker thread instead of stalling the loop. + if not is_valid_endpoint_id(endpoint_id): + raise InvalidEndpointId() + + endpoint = await run_in_threadpool(storage.get_endpoint, session, endpoint_id) + if endpoint is None: + raise EndpointNotFound(endpoint_id) + if not endpoint.is_active: + raise EndpointInactive(endpoint_id) + + # Read the webhook configuration off the row now, while the session is known + # to be clean. Storing the submission can roll back to settle an idempotency + # race, and a rollback expires loaded objects, so touching the endpoint later + # would silently issue a refresh query from this async handler. + webhook_url = endpoint.webhook_url + webhook_secret = endpoint.webhook_secret + + media_type = _media_type(request.headers.get("content-type")) + if media_type not in SUPPORTED_MEDIA_TYPES: + raise UnsupportedMediaType(media_type) + + idempotency_key = _idempotency_key(request) + + settings: Settings = request.app.state.settings + submission = build_submission( + endpoint_id, + await _parse_form(request, media_type, settings), + max_fields=settings.max_fields, + max_field_name_length=settings.max_field_name_length, + max_field_value_length=settings.max_field_value_length, + ) + + # The submission and, if the endpoint has a webhook, the durable obligation to + # deliver it are committed together. Nothing outbound happens here: once this + # returns, a worker owns the delivery, and a crash in this process can no + # longer lose a delivery that was implicitly promised by a 202. + # + # The commit happens inside the handler, not in the session dependency's + # teardown, so that a failure still becomes an error response. Teardown runs + # after the response has been sent, where raising could no longer change it. + webhook = ( + WebhookTarget(url=webhook_url, secret=webhook_secret) + if webhook_url is not None and webhook_secret is not None + else None + ) + try: + stored = await run_in_threadpool( + storage.store_submission, + session, + submission, + now=submission.received_at, + idempotency_key=idempotency_key, + payload_fingerprint=( + payload_fingerprint(submission.fields) if idempotency_key else None + ), + webhook=webhook, + ) + except storage.IdempotencyKeyReused as exc: + raise IdempotencyConflict(exc.endpoint_id, exc.idempotency_key) from exc + + # A replay answers with the original submission's identity and timestamp, so + # a client that retried after a lost response ends up describing one event. + # It reports the same queued state as the original, because the delivery that + # request created is still the one that is owed. + return SubmissionAccepted( + submission_id=stored.submission.id, + endpoint_id=stored.submission.endpoint_id, + received_at=stored.submission.received_at, + field_count=stored.submission.field_count, + idempotent_replay=stored.replayed, + delivery=DeliveryStatus(queued=webhook is not None), + ) + + +def _idempotency_key(request: Request) -> str | None: + """ + read and validate the retry key a client may have sent + :param request: the incoming request + :returns: the key, or None if the header was absent + :raises InvalidIdempotencyKey: if the header is present but breaks the key rules + """ + # An absent header keeps the pre-idempotency behaviour exactly: every accepted + # request stores a new submission. A header that is present but empty is a + # client bug, and treating it as absent would silently drop the guarantee the + # client was asking for. + key = request.headers.get(IDEMPOTENCY_KEY_HEADER) + if key is None: + return None + if not is_valid_idempotency_key(key): + raise InvalidIdempotencyKey() + return key + + +async def _parse_form( + request: Request, media_type: str, settings: Settings +) -> list[tuple[str, str]]: + """ + parse the body into ordered name/value pairs, preserving repeated names + :param request: the incoming request, streamed into the form parser + :param media_type: normalized media type taken from the Content-Type header + :param settings: active configuration, used to size the parser buffers + :returns: ordered name/value pairs exactly as submitted + :raises MalformedFormBody: if the body does not parse as the declared media type + :raises FileUploadNotSupported: if a multipart part carries a file + """ + # The parser is selected from the media type we normalized ourselves rather + # than through ``Request.form()``, whose dispatch compares the header verbatim + # even though media types are case-insensitive (RFC 9110 section 8.3). + # + # Starlette's own field and part limits are disabled: the request body size cap + # already bounds memory use, and leaving them on would let a library-defined + # threshold shadow the limits configured for this service. + parser: FormParser | MultiPartParser + if media_type == MULTIPART: + parser = MultiPartParser( + request.headers, + request.stream(), + max_files=math.inf, + max_fields=math.inf, + max_part_size=settings.max_body_bytes, + ) + else: + parser = FormParser( + request.headers, + request.stream(), + max_fields=math.inf, + max_part_size=settings.max_body_bytes, + ) + + try: + form = await parser.parse() + except (MultiPartException, ParseError) as exc: + raise MalformedFormBody(_failure_reason(exc)) from exc + + try: + items: list[tuple[str, str]] = [] + for name, value in form.multi_items(): + if isinstance(value, UploadFile): + raise FileUploadNotSupported(name) + items.append((name, value)) + return items + finally: + await form.close() + + +def _failure_reason(exc: MultiPartException | ParseError) -> str: + """ + extract a human-readable reason from a form parser failure + :param exc: the exception raised while parsing the body + :returns: the parser's description of what went wrong + """ + return exc.message if isinstance(exc, MultiPartException) else str(exc) + + +def _media_type(content_type: str | None) -> str: + """ + strip parameters such as charset and boundary from a Content-Type header + :param content_type: raw header value, or None when the header is absent + :returns: the lowercased media type, or an empty string when there is none + """ + if not content_type: + return "" + return content_type.split(";", 1)[0].strip().lower() diff --git a/src/hymical_forms/app.py b/src/hymical_forms/app.py new file mode 100644 index 0000000..ada7def --- /dev/null +++ b/src/hymical_forms/app.py @@ -0,0 +1,79 @@ +""" +application assembly +""" + +from __future__ import annotations + +from collections.abc import AsyncIterator +from contextlib import asynccontextmanager + +from fastapi import FastAPI + +from hymical_forms import __version__ +from hymical_forms.api import endpoints, health, submissions +from hymical_forms.config import Settings +from hymical_forms.db import create_engine_from_url, create_session_factory, init_db +from hymical_forms.errors import register_exception_handlers +from hymical_forms.middleware import BodySizeLimitMiddleware + +DESCRIPTION = """\ +Hymical Forms accepts HTML form submissions over HTTP so that developers do not +have to run a form backend of their own. + +Submissions are parsed, validated, and stored against a registered endpoint +together with the durable obligation to deliver them. A separate worker process +performs the webhook delivery and retries it. +""" + + +@asynccontextmanager +async def lifespan(app: FastAPI) -> AsyncIterator[None]: + """ + prepare the schema on startup and release the connection pool on shutdown + :param app: the application starting up + :returns: an async context manager wrapping the application's serving life + """ + # There is no migration framework yet, so creating missing tables at startup + # is the whole schema story. It is safe to repeat and never alters a table + # that already exists, which also means a changed column needs manual work. + init_db(app.state.engine) + yield + app.state.engine.dispose() + + +def create_app(settings: Settings | None = None) -> FastAPI: + """ + build a hymical forms application + :param settings: configuration to use, or None to load it from the environment + :returns: the configured FastAPI application + """ + # Settings, the engine and the session factory are attached to ``app.state`` + # rather than read from module-level singletons, so a test (or a future + # multi-tenant host) can run several differently configured applications in + # one process, each with its own database. + settings = settings or Settings() + + app = FastAPI( + title="Hymical Forms", + summary="Reliable form ingestion and webhook delivery for developers.", + description=DESCRIPTION, + version=__version__, + license_info={"name": "Apache-2.0", "identifier": "Apache-2.0"}, + lifespan=lifespan, + ) + engine = create_engine_from_url(settings.database_url) + app.state.settings = settings + app.state.engine = engine + app.state.session_factory = create_session_factory(engine) + # The API process holds no outbound HTTP client. Webhook delivery belongs to + # the worker, and having nothing to send with is the plainest way to keep the + # ingestion path free of network calls. + + app.add_middleware(BodySizeLimitMiddleware, max_bytes=settings.max_body_bytes) + register_exception_handlers(app) + + app.include_router(health.router) + app.include_router(endpoints.router) + app.include_router(submissions.router) + + return app diff --git a/src/hymical_forms/config.py b/src/hymical_forms/config.py new file mode 100644 index 0000000..71482b6 --- /dev/null +++ b/src/hymical_forms/config.py @@ -0,0 +1,117 @@ +""" +application settings, read from ``FORMS_``-prefixed environment variables + +Settings are added only when the code actually uses them, so this model is +currently limited to the ingestion boundary's protective limits. +""" + +from __future__ import annotations + +from pydantic import Field +from pydantic_settings import BaseSettings, SettingsConfigDict + +from hymical_forms.webhooks import RetryPolicy + + +class Settings(BaseSettings): + """ + runtime configuration for a hymical forms process + """ + + model_config = SettingsConfigDict( + env_prefix="FORMS_", + env_file=".env", + env_file_encoding="utf-8", + extra="ignore", + frozen=True, + ) + + database_url: str = Field( + description=( + "SQLAlchemy database URL. PostgreSQL is the intended production database, " + "for example postgresql+psycopg://user:password@localhost:5432/forms." + ), + ) + max_body_bytes: int = Field( + default=256 * 1024, + ge=1, + description="Largest request body accepted, in bytes. File uploads are not supported.", + ) + max_fields: int = Field( + default=100, + ge=1, + description="Largest number of name/value pairs accepted in one submission.", + ) + max_field_name_length: int = Field( + default=128, + ge=1, + description="Largest field name accepted, in characters.", + ) + max_field_value_length: int = Field( + default=16 * 1024, + ge=1, + description="Largest field value accepted, in characters.", + ) + webhook_connect_timeout_seconds: float = Field( + default=5.0, + gt=0, + description="How long to wait for a webhook destination to accept a connection.", + ) + webhook_read_timeout_seconds: float = Field( + default=10.0, + gt=0, + description="How long to wait for a webhook destination to respond.", + ) + webhook_max_attempts: int = Field( + default=5, + ge=1, + description="How many delivery attempts a submission gets before it is given up on.", + ) + webhook_retry_initial_seconds: float = Field( + default=10.0, + gt=0, + description="Wait before the second attempt. Each later wait doubles it.", + ) + webhook_retry_max_seconds: float = Field( + default=3600.0, + gt=0, + description="Cap on the wait between attempts, however far the backoff has doubled.", + ) + worker_poll_seconds: float = Field( + default=1.0, + gt=0, + description="How long the worker waits before looking for due deliveries again.", + ) + worker_batch_size: int = Field( + default=10, + ge=1, + description="How many deliveries a worker claims at once.", + ) + worker_lease_seconds: float = Field( + default=60.0, + gt=0, + description=( + "How long a worker's claim on a delivery holds. After this the delivery " + "becomes claimable again, which is how work is recovered from a worker " + "that died holding it." + ), + ) + allow_private_webhook_targets: bool = Field( + default=False, + description=( + "Permit webhook destinations on loopback and private addresses. " + "For local development and tests only; enabling it in production " + "exposes the server to SSRF." + ), + ) + + def retry_policy(self) -> RetryPolicy: + """ + gather the retry settings into the value the delivery code works with + :returns: the configured retry policy + """ + return RetryPolicy( + max_attempts=self.webhook_max_attempts, + initial_seconds=self.webhook_retry_initial_seconds, + max_seconds=self.webhook_retry_max_seconds, + ) diff --git a/src/hymical_forms/db.py b/src/hymical_forms/db.py new file mode 100644 index 0000000..3719387 --- /dev/null +++ b/src/hymical_forms/db.py @@ -0,0 +1,90 @@ +""" +database engine, session, and schema lifecycle +""" + +from __future__ import annotations + +from collections.abc import Iterator +from typing import Annotated, Any + +from fastapi import Depends +from sqlalchemy import Engine, create_engine, event +from sqlalchemy.engine import make_url +from sqlalchemy.orm import Session, sessionmaker +from sqlalchemy.pool import StaticPool +from starlette.requests import Request + +from hymical_forms.models import Base + + +def create_engine_from_url(url: str) -> Engine: + """ + build the engine for a database URL + :param url: SQLAlchemy database URL, such as ``postgresql+psycopg://.../forms`` + :returns: an engine configured for that backend + """ + parsed = make_url(url) + if parsed.get_backend_name() != "sqlite": + return create_engine(url) + + # SQLite backs the test suite and local experimentation, never production. + # Requests are served from a thread pool so a connection is not pinned to the + # thread that opened it, and an in-memory database only exists for as long as + # its single connection is held, which is what StaticPool guarantees. + kwargs: dict[str, Any] = {"connect_args": {"check_same_thread": False}} + if parsed.database in (None, "", ":memory:"): + kwargs["poolclass"] = StaticPool + + engine = create_engine(url, **kwargs) + event.listen(engine, "connect", _enable_sqlite_foreign_keys) + return engine + + +def _enable_sqlite_foreign_keys(dbapi_connection: Any, connection_record: Any) -> None: + """ + switch on SQLite foreign key enforcement, which is off by default + :param dbapi_connection: the freshly opened DBAPI connection + :param connection_record: the pool's bookkeeping record, unused + """ + # Without this, SQLite accepts rows PostgreSQL would reject, and the test + # suite would stop being a faithful stand-in for the real database. + cursor = dbapi_connection.cursor() + cursor.execute("PRAGMA foreign_keys=ON") + cursor.close() + + +def create_session_factory(engine: Engine) -> sessionmaker[Session]: + """ + build the session factory an application will serve requests from + :param engine: the engine sessions should be bound to + :returns: a configured session factory + """ + # ``expire_on_commit=False`` keeps loaded values readable after a commit, so + # building a response out of a just-committed row costs no extra query. + return sessionmaker(bind=engine, expire_on_commit=False) + + +def init_db(engine: Engine) -> None: + """ + create any tables that do not exist yet + :param engine: the engine whose database should hold the schema + """ + # There is no migration framework yet, so this is the whole schema story: it + # creates missing tables and never alters existing ones. + Base.metadata.create_all(engine) + + +def get_session(request: Request) -> Iterator[Session]: + """ + provide the session a request should do its database work through + :param request: the request being served + :returns: an iterator yielding one session, closed when the request ends + """ + # Closing a session rolls back whatever was not committed, so a handler that + # raises part way through leaves nothing behind. + factory: sessionmaker[Session] = request.app.state.session_factory + with factory() as session: + yield session + + +SessionDep = Annotated[Session, Depends(get_session)] diff --git a/src/hymical_forms/delivery.py b/src/hymical_forms/delivery.py new file mode 100644 index 0000000..fec5ad0 --- /dev/null +++ b/src/hymical_forms/delivery.py @@ -0,0 +1,107 @@ +""" +the outbound half of webhook delivery: one HTTP attempt, no retries + +This module is the only place the service makes an outbound request. It is kept +separate from :mod:`hymical_forms.webhooks` so that the rules and the payload can +be tested without a network, and separate from the database layer so that no +transaction is ever held open across a call to somebody else's server. +""" + +from __future__ import annotations + +import httpx2 + +from hymical_forms import __version__ +from hymical_forms.config import Settings +from hymical_forms.webhooks import ( + DELIVERY_ERROR_MAX_LENGTH, + SIGNATURE_HEADER, + DeliveryOutcome, + DeliveryResult, + sign, +) + +USER_AGENT = f"Hymical-Forms/{__version__}" + + +def create_webhook_client(settings: Settings) -> httpx2.AsyncClient: + """ + build the client every outbound webhook is sent through + :param settings: active configuration, read for its timeouts + :returns: a client with explicit timeouts, no redirects and no retries + """ + # One client for the process, so connections are reused rather than + # renegotiated per submission. + # + # Redirects are not followed. Beyond being surprising for a webhook, following + # them would let a destination bounce the request to an address that the URL + # validation refused, which is the usual way SSRF protection gets walked + # around. A 3xx is therefore reported as an unsuccessful HTTP status. + # + # Transport retries are pinned to zero. This interval promises exactly one + # attempt per submission, and a client that quietly retried would break that + # promise for any receiver that is not idempotent. + return httpx2.AsyncClient( + timeout=httpx2.Timeout( + connect=settings.webhook_connect_timeout_seconds, + read=settings.webhook_read_timeout_seconds, + write=settings.webhook_read_timeout_seconds, + pool=settings.webhook_connect_timeout_seconds, + ), + follow_redirects=False, + transport=httpx2.AsyncHTTPTransport(retries=0), + ) + + +async def deliver( + client: httpx2.AsyncClient, *, url: str, secret: str, body: bytes +) -> DeliveryResult: + """ + make one attempt to deliver a signed payload + :param client: the shared outbound client + :param url: the destination to post to + :param secret: the destination's signing secret + :param body: the exact bytes to sign and transmit + :returns: the outcome, never raising for a destination that misbehaves + """ + # ``content=body`` transmits these bytes verbatim. Passing the payload object + # and letting the client serialize it would sign one encoding and send + # another, and the receiver's signature check would fail for reasons nobody + # could see. + headers = { + "Content-Type": "application/json", + SIGNATURE_HEADER: sign(body, secret), + "User-Agent": USER_AGENT, + } + + try: + response = await client.post(url, content=body, headers=headers) + except httpx2.TimeoutException as exc: + return DeliveryResult(DeliveryOutcome.TIMEOUT, error=_describe("timed out", exc)) + except httpx2.RequestError as exc: + return DeliveryResult( + DeliveryOutcome.NETWORK_ERROR, error=_describe("could not connect", exc) + ) + + if 200 <= response.status_code < 300: + return DeliveryResult(DeliveryOutcome.SUCCEEDED, response_status=response.status_code) + + return DeliveryResult( + DeliveryOutcome.HTTP_ERROR, + response_status=response.status_code, + error=f"destination responded with HTTP {response.status_code}", + ) + + +def _describe(summary: str, exc: Exception) -> str: + """ + build bounded failure text for a delivery that never reached a response + :param summary: our own words for what went wrong + :param exc: the transport error raised while trying + :returns: a short message safe to store + """ + # The exception's own text is useful to whoever debugs this later, but it is + # shaped by the destination, so it is truncated before it reaches a column. + detail = str(exc).strip() + message = f"{summary}: {detail}" if detail else summary + return message[:DELIVERY_ERROR_MAX_LENGTH] diff --git a/src/hymical_forms/errors.py b/src/hymical_forms/errors.py new file mode 100644 index 0000000..e077747 --- /dev/null +++ b/src/hymical_forms/errors.py @@ -0,0 +1,232 @@ +""" +the single JSON error envelope used by every non-2xx response + +Every error the API can produce, whether raised by our own code, by FastAPI's +request validation, or by Starlette's routing, is rendered as:: + + {"error": {"code": "...", "message": "...", "details": {...}}} + +``code`` is a stable, machine-readable string; ``message`` is a human-readable +sentence; ``details`` is present only when there is something concrete to add +(the limit that was exceeded, the field at fault). Nothing in the envelope +exposes internal types, stack frames, or file paths. +""" + +from __future__ import annotations + +from http import HTTPStatus +from typing import Any, ClassVar + +from fastapi import FastAPI +from fastapi.exceptions import RequestValidationError +from pydantic import BaseModel, Field +from sqlalchemy.exc import SQLAlchemyError +from starlette.exceptions import HTTPException as StarletteHTTPException +from starlette.requests import Request +from starlette.responses import JSONResponse, Response + +from hymical_forms.ingestion import SubmissionRejected + + +class ErrorDetail(BaseModel): + """ + the body of an error response + """ + + code: str = Field(description="Stable, machine-readable error identifier.") + message: str = Field(description="Human-readable explanation of the failure.") + details: dict[str, Any] | None = Field( + default=None, + description="Optional structured context, such as the limit that was exceeded.", + ) + + +class ErrorResponse(BaseModel): + """ + the envelope returned for every error + """ + + error: ErrorDetail + + +class ApiError(Exception): + """ + an error that maps directly onto the public error envelope + """ + + # Subclasses fix ``status_code`` and ``code``; instances supply the message + # and any structured details. + status_code: ClassVar[int] = 500 + code: ClassVar[str] = "internal_error" + + def __init__(self, message: str, *, details: dict[str, Any] | None = None) -> None: + """ + record the message and context for an error response + :param message: human-readable explanation of the failure + :param details: optional structured context, such as the limit that was exceeded + """ + super().__init__(message) + self.message = message + self.details = details + + def as_response(self) -> JSONResponse: + """ + render this error in the shared envelope + :returns: a JSONResponse carrying the envelope and this error's status code + """ + return error_response( + status_code=self.status_code, + code=self.code, + message=self.message, + details=self.details, + ) + + +def error_response( + *, + status_code: int, + code: str, + message: str, + details: dict[str, Any] | None = None, +) -> JSONResponse: + """ + build a JSON response in the standard error envelope + :param status_code: HTTP status code to return + :param code: stable, machine-readable error identifier + :param message: human-readable explanation of the failure + :param details: optional structured context, omitted from the body when absent + :returns: a JSONResponse carrying the envelope + """ + payload = ErrorResponse(error=ErrorDetail(code=code, message=message, details=details)) + return JSONResponse(status_code=status_code, content=payload.model_dump(exclude_none=True)) + + +def register_exception_handlers(app: FastAPI) -> None: + """ + route every error class the app can raise through the shared envelope + :param app: the application to register the handlers on + """ + app.add_exception_handler(ApiError, _handle_api_error) + app.add_exception_handler(SubmissionRejected, _handle_submission_rejected) + app.add_exception_handler(SQLAlchemyError, _handle_storage_error) + app.add_exception_handler(StarletteHTTPException, _handle_http_exception) + app.add_exception_handler(RequestValidationError, _handle_request_validation_error) + app.add_exception_handler(Exception, _handle_unexpected_error) + + +# Starlette types every handler as ``(Request, Exception) -> Response``, so each +# handler re-narrows the exception it was registered for. + + +async def _handle_api_error(request: Request, exc: Exception) -> Response: + """ + render an error raised by our own HTTP layer + :param request: the request being handled + :param exc: the raised exception, always an ApiError + :returns: the envelope response + """ + assert isinstance(exc, ApiError) + return exc.as_response() + + +async def _handle_submission_rejected(request: Request, exc: Exception) -> Response: + """ + render a domain rejection + :param request: the request being handled + :param exc: the raised exception, always a SubmissionRejected + :returns: the envelope response, with a 422 status + """ + # Every ingestion rule failure is a well-formed request carrying an + # unacceptable submission, which is exactly what 422 describes. + assert isinstance(exc, SubmissionRejected) + return error_response( + status_code=HTTPStatus.UNPROCESSABLE_ENTITY, + code=exc.code, + message=exc.message, + details=exc.details, + ) + + +async def _handle_storage_error(request: Request, exc: Exception) -> Response: + """ + render a database failure without describing it + :param request: the request being handled + :param exc: the raised exception, always a SQLAlchemyError + :returns: the envelope response, with a 503 status + """ + # Driver messages carry table names, SQL text and sometimes connection + # details, so none of the exception reaches the client. 503 rather than 500 + # because the request itself was fine and retrying it may well succeed. + return error_response( + status_code=HTTPStatus.SERVICE_UNAVAILABLE, + code="storage_unavailable", + message="The submission could not be stored. Try again shortly.", + ) + + +async def _handle_http_exception(request: Request, exc: Exception) -> Response: + """ + render routing-level errors such as unknown paths and wrong methods + :param request: the request being handled + :param exc: the raised exception, always a Starlette HTTPException + :returns: the envelope response + """ + assert isinstance(exc, StarletteHTTPException) + return error_response( + status_code=exc.status_code, + code=_code_for_status(exc.status_code), + message=str(exc.detail), + ) + + +async def _handle_request_validation_error(request: Request, exc: Exception) -> Response: + """ + render a request that FastAPI could not validate + :param request: the request being handled + :param exc: the raised exception, always a RequestValidationError + :returns: the envelope response, with a 422 status + """ + assert isinstance(exc, RequestValidationError) + # Only the location and pydantic's short explanation are relayed. The raw + # error carries the offending input, which may be user data we should not + # reflect back, and internal type names that mean nothing to a caller. + fields = [ + { + "field": ".".join(str(part) for part in error["loc"][1:]) or None, + "issue": error["msg"], + } + for error in exc.errors() + ] + return error_response( + status_code=HTTPStatus.UNPROCESSABLE_ENTITY, + code="invalid_request", + message="The request body could not be validated.", + details={"fields": fields} if fields else None, + ) + + +async def _handle_unexpected_error(request: Request, exc: Exception) -> Response: + """ + return an opaque 500 rather than letting an internal error reach the client + :param request: the request being handled + :param exc: the unhandled exception, deliberately not described to the client + :returns: the envelope response, with a 500 status + """ + return error_response( + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, + code="internal_error", + message="The request could not be processed.", + ) + + +def _code_for_status(status_code: int) -> str: + """ + derive an error code from a status code, so that 405 gives ``method_not_allowed`` + :param status_code: HTTP status code to name + :returns: the status phrase in snake case, or ``http_error`` if unrecognised + """ + try: + phrase = HTTPStatus(status_code).phrase + except ValueError: + return "http_error" + return phrase.lower().replace("-", " ").replace(" ", "_") diff --git a/src/hymical_forms/ingestion.py b/src/hymical_forms/ingestion.py new file mode 100644 index 0000000..872add1 --- /dev/null +++ b/src/hymical_forms/ingestion.py @@ -0,0 +1,251 @@ +""" +ingestion domain rules: endpoint identifiers and submission normalization + +This module is deliberately free of HTTP concepts. It answers two questions, +"is this a well-formed endpoint identifier?" and "is this set of name/value +pairs an acceptable submission?", and leaves status codes and wire formats to +the API layer. +""" + +from __future__ import annotations + +import hashlib +import json +import re +import uuid +from collections.abc import Mapping, Sequence +from dataclasses import dataclass +from datetime import UTC, datetime +from typing import Any + +ENDPOINT_ID_MIN_LENGTH = 3 +ENDPOINT_ID_MAX_LENGTH = 64 + +# Lowercase only, so that an endpoint ID has exactly one spelling. Hyphen and +# underscore are allowed inside, but an ID may not start or end with them. +_ENDPOINT_ID_PATTERN = re.compile(r"[a-z0-9](?:[a-z0-9_-]*[a-z0-9])?") + +# C0/C1 controls and DEL. HTML permits almost anything else in a field name +# (``user[email]``, ``entry.42``, non-ASCII labels), so nothing else is rejected. +_CONTROL_CHARS = re.compile(r"[\x00-\x1f\x7f-\x9f]") + +SUBMISSION_ID_PREFIX = "sub_" + +# ``sub_`` followed by a uuid4 in hex. +SUBMISSION_ID_MAX_LENGTH = len(SUBMISSION_ID_PREFIX) + 32 + +# Stated once so that every error mentioning the rule words it identically. +ENDPOINT_ID_RULE = ( + f"Endpoint IDs are {ENDPOINT_ID_MIN_LENGTH}-{ENDPOINT_ID_MAX_LENGTH} characters using " + "lowercase letters, digits, '-' and '_', and must start and end with a letter or digit." +) + +# An idempotency key is scoped to one endpoint, and this API is unauthenticated, +# so every client of an endpoint draws from the same key space. A short or +# predictable key would therefore collide with a stranger's submission, which is +# why the floor is high enough to force a random token rather than a counter. +IDEMPOTENCY_KEY_MIN_LENGTH = 16 +IDEMPOTENCY_KEY_MAX_LENGTH = 255 + +# Printable ASCII with no spaces: covers UUIDs, hex, base64 and base64url, and +# keeps unbounded or unprintable header content out of the database. +_IDEMPOTENCY_KEY_PATTERN = re.compile(r"[!-~]+") + +IDEMPOTENCY_KEY_RULE = ( + f"Idempotency keys are {IDEMPOTENCY_KEY_MIN_LENGTH}-{IDEMPOTENCY_KEY_MAX_LENGTH} printable " + "ASCII characters with no spaces. Use a random value such as a UUID." +) + +# A SHA-256 digest rendered as hex. +PAYLOAD_FINGERPRINT_LENGTH = 64 + + +def is_valid_idempotency_key(value: str) -> bool: + """ + report whether a header value is a usable idempotency key + :param value: the raw ``Idempotency-Key`` header value + :returns: True if the key is well formed + """ + return ( + IDEMPOTENCY_KEY_MIN_LENGTH <= len(value) <= IDEMPOTENCY_KEY_MAX_LENGTH + and _IDEMPOTENCY_KEY_PATTERN.fullmatch(value) is not None + ) + + +def payload_fingerprint(fields: Mapping[str, tuple[str, ...]]) -> str: + """ + digest the submitted content so a retry can be recognised as the same request + :param fields: the normalized fields of a submission + :returns: a hex SHA-256 digest of the field content + """ + # Only the fields are hashed. The generated submission ID and the received + # timestamp differ on every attempt, so taking the mapping rather than the + # whole submission makes their exclusion structural instead of a promise. + # + # The canonical form is a JSON array of ``[name, [values]]`` pairs, so it is + # sensitive to field order and to repeated values, both of which this service + # already treats as meaningful. JSON also makes the framing unambiguous: + # concatenating names and values would let two different submissions produce + # identical bytes. Nothing here depends on Python's randomized hashing, so + # the digest is stable across processes and restarts. + canonical = json.dumps( + [[name, list(values)] for name, values in fields.items()], + separators=(",", ":"), + ensure_ascii=False, + ) + return hashlib.sha256(canonical.encode("utf-8")).hexdigest() + + +def is_valid_endpoint_id(value: str) -> bool: + """ + report whether a path segment is a syntactically valid endpoint identifier + :param value: candidate identifier taken from the request path + :returns: True if the identifier is well formed + """ + # Interval 1 has no endpoint registry, so any syntactically valid identifier + # is treated as addressable. + return ( + ENDPOINT_ID_MIN_LENGTH <= len(value) <= ENDPOINT_ID_MAX_LENGTH + and _ENDPOINT_ID_PATTERN.fullmatch(value) is not None + ) + + +class SubmissionRejected(Exception): + """ + raised when a submission violates an ingestion rule and must not be accepted + """ + + def __init__(self, code: str, message: str, details: dict[str, Any] | None = None) -> None: + """ + record why a submission was refused + :param code: stable, machine-readable identifier for the broken rule + :param message: human-readable explanation of the failure + :param details: optional structured context, such as the limit that was exceeded + """ + super().__init__(message) + self.code = code + self.message = message + self.details = details + + +@dataclass(frozen=True, slots=True) +class Submission: + """ + a validated form submission in its internal representation + """ + + # Repeated field names are preserved as ordered tuples because HTML forms use + # them for checkbox groups and multi-selects; collapsing them would silently + # discard user input. + id: str + endpoint_id: str + received_at: datetime + fields: dict[str, tuple[str, ...]] + + @property + def field_count(self) -> int: + """ + count the name/value pairs the submission carries + :returns: the total number of submitted values across all field names + """ + return sum(len(values) for values in self.fields.values()) + + +def new_submission_id() -> str: + """ + generate an opaque, prefixed submission identifier + :returns: a fresh submission id such as ``sub_1f0c9a...`` + """ + return f"{SUBMISSION_ID_PREFIX}{uuid.uuid4().hex}" + + +def build_submission( + endpoint_id: str, + items: Sequence[tuple[str, str]], + *, + max_fields: int, + max_field_name_length: int, + max_field_value_length: int, +) -> Submission: + """ + validate parsed form pairs and normalize them into a submission + :param endpoint_id: the endpoint the submission was addressed to + :param items: ordered name/value pairs as parsed from the request body, repeats included + :param max_fields: largest number of name/value pairs accepted + :param max_field_name_length: largest field name accepted, in characters + :param max_field_value_length: largest field value accepted, in characters + :returns: the normalized submission + :raises SubmissionRejected: if the submission is empty or breaches a limit + """ + if len(items) > max_fields: + raise SubmissionRejected( + "too_many_fields", + f"Submission carries {len(items)} fields, which exceeds the limit of {max_fields}.", + {"limit": max_fields, "received": len(items)}, + ) + + fields: dict[str, tuple[str, ...]] = {} + for name, value in items: + _validate_field_name(name, max_field_name_length) + _validate_field_value(name, value, max_field_value_length) + fields[name] = (*fields.get(name, ()), value) + + if not fields: + raise SubmissionRejected( + "empty_submission", + "Submission contains no fields.", + ) + + return Submission( + id=new_submission_id(), + endpoint_id=endpoint_id, + received_at=datetime.now(UTC), + fields=fields, + ) + + +def _validate_field_name(name: str, max_length: int) -> None: + """ + check a submitted field name against the name rules + :param name: field name as submitted + :param max_length: largest field name accepted, in characters + :raises SubmissionRejected: if the name is empty, too long, or holds control characters + """ + if not name: + raise SubmissionRejected( + "invalid_field_name", + "Submission contains a field with an empty name.", + ) + if len(name) > max_length: + raise SubmissionRejected( + "field_name_too_long", + f"A field name exceeds the limit of {max_length} characters.", + {"limit": max_length, "received": len(name)}, + ) + if _CONTROL_CHARS.search(name): + raise SubmissionRejected( + "invalid_field_name", + "Submission contains a field name with control characters.", + ) + + +def _validate_field_value(name: str, value: str, max_length: int) -> None: + """ + check a submitted field value against the value rules + :param name: field name the value belongs to, used only in the error message + :param value: field value as submitted + :param max_length: largest field value accepted, in characters + :raises SubmissionRejected: if the value is too long or holds a null byte + """ + if len(value) > max_length: + raise SubmissionRejected( + "field_value_too_long", + f"The value of field {name!r} exceeds the limit of {max_length} characters.", + {"field": name, "limit": max_length, "received": len(value)}, + ) + if "\x00" in value: + raise SubmissionRejected( + "invalid_field_value", + f"The value of field {name!r} contains a null byte.", + {"field": name}, + ) diff --git a/src/hymical_forms/main.py b/src/hymical_forms/main.py new file mode 100644 index 0000000..98904c4 --- /dev/null +++ b/src/hymical_forms/main.py @@ -0,0 +1,13 @@ +""" +the ASGI entrypoint + +Run with:: + + uvicorn hymical_forms.main:app +""" + +from __future__ import annotations + +from hymical_forms.app import create_app + +app = create_app() diff --git a/src/hymical_forms/middleware.py b/src/hymical_forms/middleware.py new file mode 100644 index 0000000..01fbeb7 --- /dev/null +++ b/src/hymical_forms/middleware.py @@ -0,0 +1,101 @@ +""" +middleware protecting the ingestion boundary at the ASGI layer +""" + +from __future__ import annotations + +from http import HTTPStatus + +from starlette.types import ASGIApp, Message, Receive, Scope, Send + +from hymical_forms.errors import ApiError + + +class RequestBodyTooLarge(ApiError): + """ + raised when a request body exceeds the configured maximum + """ + + status_code = HTTPStatus.REQUEST_ENTITY_TOO_LARGE + code = "request_body_too_large" + + def __init__(self, limit: int) -> None: + """ + record the limit the body overran + :param limit: largest request body accepted, in bytes + """ + super().__init__( + f"Request body exceeds the limit of {limit} bytes.", + details={"limit_bytes": limit}, + ) + + +class BodySizeLimitMiddleware: + """ + reject requests whose body exceeds a configured size + """ + + # Starlette buffers request bodies without an upper bound, so the cap has to + # sit in front of the form parsers rather than inside a route handler. + + def __init__(self, app: ASGIApp, *, max_bytes: int) -> None: + """ + wrap an ASGI application with a request body size cap + :param app: the ASGI application to wrap + :param max_bytes: largest request body accepted, in bytes + """ + self.app = app + self.max_bytes = max_bytes + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + """ + pass the request through, refusing any body over the limit + :param scope: ASGI connection scope + :param receive: ASGI callable yielding request messages + :param send: ASGI callable accepting response messages + """ + if scope["type"] != "http": + await self.app(scope, receive, send) + return + + # A request that declares an oversized Content-Length is refused before a + # single body byte is read. + declared = _declared_content_length(scope) + if declared is not None and declared > self.max_bytes: + await RequestBodyTooLarge(self.max_bytes).as_response()(scope, receive, send) + return + + received = 0 + + async def limited_receive() -> Message: + """ + read the next request message, cutting off an oversized body + :returns: the next ASGI message + :raises RequestBodyTooLarge: once the running body total crosses the limit + """ + nonlocal received + message = await receive() + if message["type"] == "http.request": + received += len(message.get("body", b"")) + if received > self.max_bytes: + # Raised inside the application, so the registered ApiError + # handler renders it in the standard envelope. + raise RequestBodyTooLarge(self.max_bytes) + return message + + await self.app(scope, limited_receive, send) + + +def _declared_content_length(scope: Scope) -> int | None: + """ + read the Content-Length header from the raw ASGI scope + :param scope: ASGI connection scope + :returns: the declared body length, or None when absent or unparseable + """ + for name, value in scope["headers"]: + if name == b"content-length": + try: + return int(value) + except ValueError: + return None + return None diff --git a/src/hymical_forms/models.py b/src/hymical_forms/models.py new file mode 100644 index 0000000..96284fb --- /dev/null +++ b/src/hymical_forms/models.py @@ -0,0 +1,311 @@ +""" +the persisted schema: endpoints and the submissions addressed to them +""" + +from __future__ import annotations + +from datetime import UTC, datetime + +from sqlalchemy import ( + JSON, + CheckConstraint, + DateTime, + ForeignKey, + String, + TypeDecorator, + UniqueConstraint, +) +from sqlalchemy.engine import Dialect +from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column + +from hymical_forms.ingestion import ( + ENDPOINT_ID_MAX_LENGTH, + IDEMPOTENCY_KEY_MAX_LENGTH, + PAYLOAD_FINGERPRINT_LENGTH, + SUBMISSION_ID_MAX_LENGTH, +) +from hymical_forms.ingestion import Submission as DomainSubmission +from hymical_forms.webhooks import ( + DELIVERY_ATTEMPT_ID_MAX_LENGTH, + DELIVERY_ERROR_MAX_LENGTH, + DELIVERY_STATE_MAX_LENGTH, + WEBHOOK_DELIVERY_ID_MAX_LENGTH, + WEBHOOK_SECRET_MAX_LENGTH, + WEBHOOK_URL_MAX_LENGTH, + DeliveryState, +) + +ENDPOINT_NAME_MAX_LENGTH = 200 +DELIVERY_OUTCOME_MAX_LENGTH = 32 + + +def utcnow() -> datetime: + """ + read the current time as a timezone-aware UTC timestamp + :returns: the current instant in UTC + """ + return datetime.now(UTC) + + +class UtcDateTime(TypeDecorator[datetime]): + """ + a timestamp column that always stores and returns timezone-aware UTC values + """ + + # PostgreSQL round-trips ``TIMESTAMPTZ`` faithfully, but SQLite has no + # timezone-aware storage and hands back a naive datetime. Normalising on the + # way in and out keeps the two backends indistinguishable to the rest of the + # code, so a value written as UTC is always read back as UTC. + impl = DateTime(timezone=True) + cache_ok = True + + def process_bind_param(self, value: datetime | None, dialect: Dialect) -> datetime | None: + """ + convert a value on its way into the database + :param value: the timestamp being stored, or None + :param dialect: the active SQLAlchemy dialect + :returns: the same instant expressed in UTC, or None + :raises ValueError: if the timestamp carries no timezone + """ + if value is None: + return None + if value.tzinfo is None: + raise ValueError("naive datetimes cannot be stored, use a timezone-aware value") + return value.astimezone(UTC) + + def process_result_value(self, value: datetime | None, dialect: Dialect) -> datetime | None: + """ + convert a value on its way out of the database + :param value: the stored timestamp, naive on backends without timezone support + :param dialect: the active SQLAlchemy dialect + :returns: a timezone-aware UTC timestamp, or None + """ + if value is None: + return None + if value.tzinfo is None: + return value.replace(tzinfo=UTC) + return value.astimezone(UTC) + + +class Base(DeclarativeBase): + """ + declarative base for every persisted table + """ + + +class Endpoint(Base): + """ + a form ingestion destination that submissions may be addressed to + """ + + # The public endpoint ID is the primary key. It is already unique, immutable + # in practice (changing it breaks every deployed HTML form pointing at it), + # and constrained to a short safe character set, so a surrogate key would add + # a join without buying anything. + __tablename__ = "endpoints" + + __table_args__ = ( + # An endpoint either has a full webhook configuration or none of it. A URL + # without a secret would mean sending unsigned payloads, which no receiver + # could trust. + CheckConstraint( + "(webhook_url IS NULL) = (webhook_secret IS NULL)", + name="ck_endpoints_webhook_configuration", + ), + ) + + id: Mapped[str] = mapped_column(String(ENDPOINT_ID_MAX_LENGTH), primary_key=True) + name: Mapped[str] = mapped_column(String(ENDPOINT_NAME_MAX_LENGTH)) + is_active: Mapped[bool] = mapped_column(default=True) + created_at: Mapped[datetime] = mapped_column(UtcDateTime, default=utcnow) + + # One destination per endpoint, held here rather than in a table of its own. + # A separate table would only start paying for itself with several + # destinations, which this build deliberately does not have. + webhook_url: Mapped[str | None] = mapped_column(String(WEBHOOK_URL_MAX_LENGTH), default=None) + webhook_secret: Mapped[str | None] = mapped_column( + String(WEBHOOK_SECRET_MAX_LENGTH), default=None + ) + + +class Submission(Base): + """ + a submission that was accepted for a persisted endpoint + """ + + __tablename__ = "submissions" + + __table_args__ = ( + # The database, not the application, is what makes an idempotency key + # unique per endpoint. Two concurrent retries can both find nothing and + # both try to insert, so the constraint is the only authoritative answer. + # + # Both PostgreSQL and SQLite treat NULLs in a unique constraint as + # distinct from each other, so submissions sent without a key stay + # unrestricted without needing a partial index on either backend. + UniqueConstraint( + "endpoint_id", + "idempotency_key", + name="uq_submissions_endpoint_idempotency_key", + ), + # A submission either carries a full idempotency identity or none of it. + CheckConstraint( + "(idempotency_key IS NULL) = (payload_fingerprint IS NULL)", + name="ck_submissions_idempotency_identity", + ), + ) + + id: Mapped[str] = mapped_column(String(SUBMISSION_ID_MAX_LENGTH), primary_key=True) + endpoint_id: Mapped[str] = mapped_column( + String(ENDPOINT_ID_MAX_LENGTH), + ForeignKey("endpoints.id"), + index=True, + ) + received_at: Mapped[datetime] = mapped_column(UtcDateTime) + + # Stored as ``{"field name": ["value", ...]}`` under SQLAlchemy's generic JSON + # type, which is ``json`` on PostgreSQL rather than ``jsonb``. That is + # deliberate: ``jsonb`` normalises object key order, which would silently + # reorder a form's fields, and this payload is written once and read whole, + # so ``jsonb`` indexing would buy nothing here. Every value is a list because + # JSON has no tuple, so the domain's tuples widen here and narrow again in + # :meth:`to_domain`. + fields: Mapped[dict[str, list[str]]] = mapped_column(JSON) + + # Null for submissions sent without an ``Idempotency-Key``, which is the + # common case. When present, the fingerprint is what tells a safe retry apart + # from the same key being reused for different content. + idempotency_key: Mapped[str | None] = mapped_column( + String(IDEMPOTENCY_KEY_MAX_LENGTH), default=None + ) + payload_fingerprint: Mapped[str | None] = mapped_column( + String(PAYLOAD_FINGERPRINT_LENGTH), default=None + ) + + @classmethod + def from_domain( + cls, + submission: DomainSubmission, + *, + idempotency_key: str | None = None, + payload_fingerprint: str | None = None, + ) -> Submission: + """ + build a persistable row from a validated domain submission + :param submission: the normalized submission to store + :param idempotency_key: the client's retry key, or None if it sent none + :param payload_fingerprint: digest of the submitted content, set only alongside a key + :returns: an unsaved row mirroring the submission + """ + return cls( + id=submission.id, + endpoint_id=submission.endpoint_id, + received_at=submission.received_at, + fields={name: list(values) for name, values in submission.fields.items()}, + idempotency_key=idempotency_key, + payload_fingerprint=payload_fingerprint, + ) + + def to_domain(self) -> DomainSubmission: + """ + rebuild the domain submission this row was stored from + :returns: the submission with its repeated field values restored as tuples + """ + return DomainSubmission( + id=self.id, + endpoint_id=self.endpoint_id, + received_at=self.received_at, + fields={name: tuple(values) for name, values in self.fields.items()}, + ) + + +class WebhookDelivery(Base): + """ + the durable obligation to deliver one submission to one destination + """ + + # This is the outbox. It is written in the same transaction as the submission + # it belongs to, so a submission can never be acknowledged while the promise + # to deliver it quietly goes missing. + __tablename__ = "webhook_deliveries" + + __table_args__ = ( + # One logical delivery per submission. An idempotent replay resolves to an + # existing submission, so this constraint is what makes "a replay never + # queues a second delivery" a property of the database rather than of the + # code path that happens to run. + UniqueConstraint("submission_id", name="uq_webhook_deliveries_submission"), + # A delivery is finished exactly when it says it is finished. + CheckConstraint( + "(state IN ('delivered', 'failed')) = (completed_at IS NOT NULL)", + name="ck_webhook_deliveries_completion", + ), + ) + + id: Mapped[str] = mapped_column(String(WEBHOOK_DELIVERY_ID_MAX_LENGTH), primary_key=True) + submission_id: Mapped[str] = mapped_column( + String(SUBMISSION_ID_MAX_LENGTH), ForeignKey("submissions.id") + ) + + # The destination and secret are snapshotted, not read through to the + # endpoint. A delivery represents what was owed when the submission was + # accepted, so changing an endpoint's webhook later must not silently + # redirect work that is already queued, nor leave a queued payload signed + # with a secret its receiver never had. + destination_url: Mapped[str] = mapped_column(String(WEBHOOK_URL_MAX_LENGTH)) + signing_secret: Mapped[str] = mapped_column(String(WEBHOOK_SECRET_MAX_LENGTH)) + + state: Mapped[str] = mapped_column( + String(DELIVERY_STATE_MAX_LENGTH), default=DeliveryState.PENDING + ) + attempts: Mapped[int] = mapped_column(default=0) + + # When this delivery next becomes due. Indexed with the state because that + # pair is exactly what a worker scans for on every poll. + next_attempt_at: Mapped[datetime] = mapped_column(UtcDateTime, index=True) + + # Set while a worker holds the job. A lease that has run out makes the job + # claimable again, which is what stops a worker that died mid-delivery from + # leaving a permanent ``processing`` tombstone. + claim_expires_at: Mapped[datetime | None] = mapped_column(UtcDateTime, default=None) + + created_at: Mapped[datetime] = mapped_column(UtcDateTime, default=utcnow) + completed_at: Mapped[datetime | None] = mapped_column(UtcDateTime, default=None) + + +class DeliveryAttempt(Base): + """ + a record of one outbound request actually made for a delivery + """ + + # The audit trail. One row per request that genuinely went out, so a delivery + # that was retried four times has four rows and none of them is overwritten. + # A job that is merely inspected and found not due produces nothing here. + __tablename__ = "delivery_attempts" + + id: Mapped[str] = mapped_column(String(DELIVERY_ATTEMPT_ID_MAX_LENGTH), primary_key=True) + delivery_id: Mapped[str] = mapped_column( + String(WEBHOOK_DELIVERY_ID_MAX_LENGTH), + ForeignKey("webhook_deliveries.id"), + index=True, + ) + submission_id: Mapped[str] = mapped_column( + String(SUBMISSION_ID_MAX_LENGTH), + ForeignKey("submissions.id"), + index=True, + ) + attempt_number: Mapped[int] = mapped_column() + + # The URL as it was used, not as it is configured now, so the record still + # explains itself after the endpoint's destination changes. + destination_url: Mapped[str] = mapped_column(String(WEBHOOK_URL_MAX_LENGTH)) + attempted_at: Mapped[datetime] = mapped_column(UtcDateTime, default=utcnow) + + # Stored as plain text rather than a database enum, because a database enum + # would need a migration to gain a value and there is no migration tool yet. + outcome: Mapped[str] = mapped_column(String(DELIVERY_OUTCOME_MAX_LENGTH)) + response_status: Mapped[int | None] = mapped_column(default=None) + + # Response bodies are deliberately not stored. They are unbounded, written by + # somebody else's server, and nothing in this build reads them back. + error: Mapped[str | None] = mapped_column(String(DELIVERY_ERROR_MAX_LENGTH), default=None) diff --git a/src/hymical_forms/storage.py b/src/hymical_forms/storage.py new file mode 100644 index 0000000..9b54fe5 --- /dev/null +++ b/src/hymical_forms/storage.py @@ -0,0 +1,392 @@ +""" +persistence operations, the only place queries are written + +Most functions here leave the commit to the caller, so a request handler decides +when its work becomes durable and a failure anywhere before that commit leaves +the database untouched. Three functions own their transaction and say so: +:func:`store_submission`, because it writes a submission and the obligation to +deliver it as one atomic unit and must roll both back together to settle an +idempotency race; :func:`claim_due_deliveries`, because a claim is only worth +anything once it is committed; and :func:`complete_attempt`, because the audit +record and the state it justifies have to land together. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import datetime, timedelta +from typing import Any, cast + +from sqlalchemy import ColumnElement, and_, or_, select, update +from sqlalchemy.engine import CursorResult +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm import Session + +from hymical_forms import models +from hymical_forms.ingestion import Submission +from hymical_forms.webhooks import ( + DeliveryOutcome, + DeliveryResult, + DeliveryState, + RetryPolicy, + WebhookTarget, + is_retryable, + new_delivery_attempt_id, + new_webhook_delivery_id, +) + + +class EndpointAlreadyExists(Exception): + """ + raised when an endpoint ID is already taken + """ + + def __init__(self, endpoint_id: str) -> None: + """ + record which endpoint ID was already in use + :param endpoint_id: the identifier that collided + """ + super().__init__(f"endpoint {endpoint_id!r} already exists") + self.endpoint_id = endpoint_id + + +def create_endpoint( + session: Session, + *, + endpoint_id: str, + name: str, + is_active: bool, + webhook_url: str | None = None, + webhook_secret: str | None = None, +) -> models.Endpoint: + """ + add an endpoint, failing if the identifier is taken + :param session: the session to add the endpoint through + :param endpoint_id: the public identifier the endpoint will answer on + :param name: human-readable label for the endpoint + :param is_active: whether the endpoint should accept submissions straight away + :param webhook_url: destination to deliver submissions to, or None for no webhook + :param webhook_secret: signing secret for that destination, set only alongside a URL + :returns: the pending endpoint, not yet committed + :raises EndpointAlreadyExists: if an endpoint already holds that identifier + """ + endpoint = models.Endpoint( + id=endpoint_id, + name=name, + is_active=is_active, + webhook_url=webhook_url, + webhook_secret=webhook_secret, + ) + session.add(endpoint) + try: + # Flushing here turns the unique violation into a catchable error while + # the caller can still react, rather than at an opaque commit later. + session.flush() + except IntegrityError as exc: + session.rollback() + raise EndpointAlreadyExists(endpoint_id) from exc + return endpoint + + +def get_endpoint(session: Session, endpoint_id: str) -> models.Endpoint | None: + """ + look an endpoint up by its public identifier + :param session: the session to query through + :param endpoint_id: the public identifier to resolve + :returns: the endpoint, or None if no endpoint holds that identifier + """ + return session.get(models.Endpoint, endpoint_id) + + +class IdempotencyKeyReused(Exception): + """ + raised when an idempotency key was already used for different content + """ + + def __init__(self, endpoint_id: str, idempotency_key: str) -> None: + """ + record which key was reused, and where + :param endpoint_id: the endpoint the key is scoped to + :param idempotency_key: the key that was already spent on other content + """ + super().__init__(f"idempotency key already used on endpoint {endpoint_id!r}") + self.endpoint_id = endpoint_id + self.idempotency_key = idempotency_key + + +@dataclass(frozen=True, slots=True) +class StoredSubmission: + """ + the outcome of storing a submission + """ + + submission: Submission + replayed: bool + + +def find_by_idempotency_key( + session: Session, endpoint_id: str, idempotency_key: str +) -> models.Submission | None: + """ + look up the submission a key was already spent on + :param session: the session to query through + :param endpoint_id: the endpoint the key is scoped to + :param idempotency_key: the key to resolve + :returns: the earlier submission, or None if the key is unused + """ + return session.scalars( + select(models.Submission).where( + models.Submission.endpoint_id == endpoint_id, + models.Submission.idempotency_key == idempotency_key, + ) + ).one_or_none() + + +def store_submission( + session: Session, + submission: Submission, + *, + now: datetime, + idempotency_key: str | None = None, + payload_fingerprint: str | None = None, + webhook: WebhookTarget | None = None, +) -> StoredSubmission: + """ + store a submission and the obligation to deliver it, or resolve to an earlier one + :param session: the session to write through + :param submission: the validated domain submission to store + :param now: the instant the submission was accepted + :param idempotency_key: the client's retry key, or None if it sent none + :param payload_fingerprint: digest of the submitted content, required alongside a key + :param webhook: the destination owed delivery, or None if the endpoint has no webhook + :returns: the stored submission and whether it came from an earlier attempt + :raises IdempotencyKeyReused: if the key was already used for different content + """ + # The submission and its delivery obligation go in as one transaction. That + # is the whole reliability claim: once this commits, a crash cannot leave a + # submission that was acknowledged with nothing durable saying delivery is + # still owed, and it cannot leave delivery work for a submission that never + # existed either. + if idempotency_key is None: + _add_submission(session, submission, now=now, webhook=webhook) + session.commit() + return StoredSubmission(submission, replayed=False) + + # Fast path for the ordinary retry, where the first attempt already landed. + existing = find_by_idempotency_key(session, submission.endpoint_id, idempotency_key) + if existing is not None: + return StoredSubmission(_settle(existing, payload_fingerprint), replayed=True) + + _add_submission( + session, + submission, + now=now, + webhook=webhook, + idempotency_key=idempotency_key, + payload_fingerprint=payload_fingerprint, + ) + try: + session.commit() + except IntegrityError: + # A concurrent request inserted the same key between the lookup above and + # this commit. The unique constraint is what caught it, which is the whole + # point: the lookup is an optimisation, never the guarantee. + # + # The rollback is mandatory. A session left holding a failed flush refuses + # every later query with PendingRollbackError, so the read below would + # fail rather than find the winner. Rolling back also discards this + # request's submission and its delivery together, leaving exactly the one + # submission and the one delivery the winner committed. + session.rollback() + existing = find_by_idempotency_key(session, submission.endpoint_id, idempotency_key) + if existing is None: + # The violation was something other than the idempotency constraint, + # so it is not ours to interpret and must not be reported as success. + raise + return StoredSubmission(_settle(existing, payload_fingerprint), replayed=True) + + return StoredSubmission(submission, replayed=False) + + +def _add_submission( + session: Session, + submission: Submission, + *, + now: datetime, + webhook: WebhookTarget | None, + idempotency_key: str | None = None, + payload_fingerprint: str | None = None, +) -> None: + """ + stage a submission and, if one is owed, its delivery, without committing + :param session: the session to add through + :param submission: the validated domain submission to store + :param now: the instant the submission was accepted + :param webhook: the destination owed delivery, or None if the endpoint has no webhook + :param idempotency_key: the client's retry key, or None if it sent none + :param payload_fingerprint: digest of the submitted content, set only alongside a key + """ + session.add( + models.Submission.from_domain( + submission, + idempotency_key=idempotency_key, + payload_fingerprint=payload_fingerprint, + ) + ) + if webhook is None: + return + + session.add( + models.WebhookDelivery( + id=new_webhook_delivery_id(), + submission_id=submission.id, + destination_url=webhook.url, + signing_secret=webhook.secret, + state=DeliveryState.PENDING, + attempts=0, + # Due straight away: the first attempt is not delayed, it is simply + # made by a worker rather than by the request that caused it. + next_attempt_at=now, + created_at=now, + ) + ) + + +def due_condition(now: datetime) -> ColumnElement[bool]: + """ + build the test for a delivery a worker is allowed to pick up + :param now: the instant to judge dueness against + :returns: a SQL condition matching claimable deliveries + """ + # Two ways to be claimable: waiting and due, or claimed by a worker whose + # lease has run out. The second is what recovers work from a worker that died + # holding a job, rather than leaving it stuck in ``processing`` forever. + delivery = models.WebhookDelivery + return or_( + and_(delivery.state == DeliveryState.PENDING, delivery.next_attempt_at <= now), + and_(delivery.state == DeliveryState.PROCESSING, delivery.claim_expires_at <= now), + ) + + +def claim_due_deliveries( + session: Session, *, now: datetime, lease_seconds: float, limit: int +) -> list[models.WebhookDelivery]: + """ + take ownership of up to a few due deliveries, in one committed transaction + :param session: the session to claim through + :param now: the instant to judge dueness against + :param lease_seconds: how long the claim protects a delivery from other workers + :param limit: the most deliveries to claim at once + :returns: the deliveries this worker now owns + """ + delivery = models.WebhookDelivery + due = due_condition(now) + + statement = select(delivery).where(due).order_by(delivery.next_attempt_at).limit(limit) + if session.get_bind().dialect.name == "postgresql": + # PostgreSQL can hand each worker a different set of rows outright, which + # is the real answer to two workers scanning at once. SKIP LOCKED means a + # busy row is passed over rather than waited on. + statement = statement.with_for_update(skip_locked=True) + candidates = list(session.scalars(statement)) + + claimed: list[models.WebhookDelivery] = [] + expires_at = now + timedelta(seconds=lease_seconds) + for candidate in candidates: + # The conditional update is the guarantee on backends without row locking: + # whoever gets there first flips the row out of the due condition, and the + # loser's update matches nothing. Redundant under SKIP LOCKED, and cheap. + result = cast( + "CursorResult[Any]", + session.execute( + update(delivery) + .where(delivery.id == candidate.id) + .where(due) + .values(state=DeliveryState.PROCESSING, claim_expires_at=expires_at) + .execution_options(synchronize_session="fetch") + ), + ) + if result.rowcount == 1: + claimed.append(candidate) + + session.commit() + return claimed + + +def load_submissions(session: Session, submission_ids: list[str]) -> dict[str, Submission]: + """ + load the submissions a batch of deliveries is carrying + :param session: the session to query through + :param submission_ids: the submissions to fetch + :returns: the submissions in domain form, keyed by id + """ + # One query for the batch rather than a lookup per delivery. + rows = session.scalars( + select(models.Submission).where(models.Submission.id.in_(submission_ids)) + ) + return {row.id: row.to_domain() for row in rows} + + +def complete_attempt( + session: Session, + delivery: models.WebhookDelivery, + result: DeliveryResult, + *, + now: datetime, + policy: RetryPolicy, +) -> models.DeliveryAttempt: + """ + record one outbound request and move the delivery to whatever it earned + :param session: the session to write through + :param delivery: the delivery the attempt was made for + :param result: what the attempt produced + :param now: the instant the attempt finished + :param policy: how many attempts are allowed and how long to wait between them + :returns: the committed attempt record + """ + # The audit row and the state it justifies are written together, so the + # history can never disagree with the job about how many attempts happened. + attempt_number = delivery.attempts + 1 + attempt = models.DeliveryAttempt( + id=new_delivery_attempt_id(), + delivery_id=delivery.id, + submission_id=delivery.submission_id, + attempt_number=attempt_number, + destination_url=delivery.destination_url, + attempted_at=now, + outcome=str(result.outcome), + response_status=result.response_status, + error=result.error, + ) + session.add(attempt) + + delivery.attempts = attempt_number + delivery.claim_expires_at = None + + if result.outcome is DeliveryOutcome.SUCCEEDED: + delivery.state = DeliveryState.DELIVERED + delivery.completed_at = now + elif is_retryable(result) and not policy.is_exhausted(attempt_number): + delivery.state = DeliveryState.PENDING + delivery.next_attempt_at = now + policy.delay_after(attempt_number) + else: + # Either the receiver said something repeating will not fix, or the + # allowance ran out. Either way this is the last word on the delivery. + delivery.state = DeliveryState.FAILED + delivery.completed_at = now + + session.commit() + return attempt + + +def _settle(existing: models.Submission, payload_fingerprint: str | None) -> Submission: + """ + decide whether an earlier submission is a replay of this one or a clash + :param existing: the submission the key was already spent on + :param payload_fingerprint: digest of the content submitted this time + :returns: the earlier submission, when the content matches + :raises IdempotencyKeyReused: if the content differs from the earlier attempt + """ + if existing.payload_fingerprint != payload_fingerprint: + raise IdempotencyKeyReused(existing.endpoint_id, str(existing.idempotency_key)) + return existing.to_domain() diff --git a/src/hymical_forms/webhooks.py b/src/hymical_forms/webhooks.py new file mode 100644 index 0000000..bf09522 --- /dev/null +++ b/src/hymical_forms/webhooks.py @@ -0,0 +1,306 @@ +""" +webhook rules: destination validation, signing secrets, payload and signature + +Nothing in this module performs I/O. Building and signing a payload is kept +apart from sending it so that the bytes which get signed are provably the bytes +that go on the wire, and so both can be tested without a network. +""" + +from __future__ import annotations + +import hashlib +import hmac +import ipaddress +import json +import secrets +import uuid +from dataclasses import dataclass +from datetime import UTC, datetime, timedelta +from enum import StrEnum +from typing import Any +from urllib.parse import urlsplit + +from hymical_forms.ingestion import Submission + +SIGNATURE_HEADER = "Hymical-Signature" +SIGNATURE_VERSION = "v1" + +SUBMISSION_RECEIVED_EVENT = "submission.received" + +WEBHOOK_SECRET_PREFIX = "whsec_" +WEBHOOK_SECRET_MAX_LENGTH = len(WEBHOOK_SECRET_PREFIX) + 64 + +WEBHOOK_URL_MAX_LENGTH = 2048 +ALLOWED_SCHEMES = ("http", "https") + +DELIVERY_ATTEMPT_ID_PREFIX = "att_" +DELIVERY_ATTEMPT_ID_MAX_LENGTH = len(DELIVERY_ATTEMPT_ID_PREFIX) + 32 + +WEBHOOK_DELIVERY_ID_PREFIX = "whd_" +WEBHOOK_DELIVERY_ID_MAX_LENGTH = len(WEBHOOK_DELIVERY_ID_PREFIX) + 32 +DELIVERY_STATE_MAX_LENGTH = 16 + +# Failure text is written by whatever the destination did, so it is attacker +# influenced and has to be bounded before it reaches a column. +DELIVERY_ERROR_MAX_LENGTH = 500 + + +class DeliveryOutcome(StrEnum): + """ + the coarse result of one webhook delivery attempt + """ + + # These strings are a public contract: they are stored, and reported back on + # the submission response. Exception class names deliberately never appear. + SUCCEEDED = "succeeded" + HTTP_ERROR = "http_error" + TIMEOUT = "timeout" + NETWORK_ERROR = "network_error" + + +class DeliveryState(StrEnum): + """ + where a logical delivery has got to + """ + + # ``pending`` is owed and waiting for its due time, ``processing`` is claimed + # by a worker and holding a lease, and the last two are terminal. + PENDING = "pending" + PROCESSING = "processing" + DELIVERED = "delivered" + FAILED = "failed" + + +TERMINAL_STATES = (DeliveryState.DELIVERED, DeliveryState.FAILED) + +# Statuses below 500 that still describe a condition worth waiting out. Every +# other 4xx is treated as final: repeating a request the receiver called +# malformed, unauthorized or missing will not repair it, and a 409 from a webhook +# receiver almost always means it already has this event. +RETRYABLE_STATUSES = ( + 408, # Request Timeout + 425, # Too Early + 429, # Too Many Requests +) + + +@dataclass(frozen=True, slots=True) +class DeliveryResult: + """ + what one delivery attempt produced + """ + + outcome: DeliveryOutcome + response_status: int | None = None + error: str | None = None + + +def is_retryable(result: DeliveryResult) -> bool: + """ + decide whether an outcome is worth another attempt later + :param result: the outcome of an attempt + :returns: True if the same request might succeed if repeated + """ + if result.outcome is DeliveryOutcome.SUCCEEDED: + return False + if result.outcome in (DeliveryOutcome.TIMEOUT, DeliveryOutcome.NETWORK_ERROR): + return True + + # A 3xx lands here too. Redirects are not followed, and a destination that + # answers with one is misconfigured rather than briefly unwell, so it is + # final like the other non-retryable statuses. + status = result.response_status + return status is not None and (status >= 500 or status in RETRYABLE_STATUSES) + + +@dataclass(frozen=True, slots=True) +class RetryPolicy: + """ + how long to wait between attempts, and when to stop + """ + + max_attempts: int + initial_seconds: float + max_seconds: float + + def delay_after(self, attempts_made: int) -> timedelta: + """ + work out how long to wait before the next attempt + :param attempts_made: how many attempts have already been made + :returns: the wait before the next one becomes due + """ + # Doubling from the initial delay and capped, with no jitter. Jitter would + # spread a thundering herd across a shared destination, but it would also + # make every retry test approximate, and nothing here fans out widely + # enough yet to need it. + exponent = max(attempts_made - 1, 0) + seconds = self.initial_seconds * (2**exponent) + return timedelta(seconds=min(seconds, self.max_seconds)) + + def is_exhausted(self, attempts_made: int) -> bool: + """ + report whether a delivery has used up its allowance + :param attempts_made: how many attempts have already been made + :returns: True if no further attempt should be scheduled + """ + return attempts_made >= self.max_attempts + + +class WebhookUrlRejected(Exception): + """ + raised when a webhook destination is not one this service will send to + """ + + def __init__(self, reason: str) -> None: + """ + record why the destination was refused + :param reason: short phrase completing "the webhook URL ..." + """ + super().__init__(reason) + self.reason = reason + + +def validate_webhook_url(url: str, *, allow_private_targets: bool = False) -> None: + """ + check that a destination is one this service is willing to send to + :param url: the destination the caller wants submissions delivered to + :param allow_private_targets: whether to permit loopback and private addresses + :raises WebhookUrlRejected: if the destination is malformed or not permitted + """ + if len(url) > WEBHOOK_URL_MAX_LENGTH: + raise WebhookUrlRejected(f"must be at most {WEBHOOK_URL_MAX_LENGTH} characters") + + try: + parts = urlsplit(url) + host = parts.hostname + except ValueError as exc: + # urlsplit rejects malformed IPv6 literals and out-of-range ports here. + raise WebhookUrlRejected("must be a well-formed URL") from exc + + if parts.scheme not in ALLOWED_SCHEMES: + raise WebhookUrlRejected("must use the http or https scheme") + if not host: + raise WebhookUrlRejected("must include a host") + + if not allow_private_targets and _is_internal_host(host): + raise WebhookUrlRejected( + "must not address a loopback, private, link-local or otherwise internal host" + ) + + +def _is_internal_host(host: str) -> bool: + """ + report whether a host literal obviously names the server's own network + :param host: the host taken from the destination URL + :returns: True if the host is one submissions must not be delivered to + """ + # Only literals are judged. A name is not resolved here, so a hostname that + # resolves to a private address still passes; see the SSRF note in the README. + name = host.rstrip(".").lower() + if name == "localhost" or name.endswith(".localhost"): + return True + + try: + address = ipaddress.ip_address(name) + except ValueError: + return False + + # ``::ffff:127.0.0.1`` is a loopback address wearing an IPv6 costume, and the + # IPv6 flags do not see through it. + if isinstance(address, ipaddress.IPv6Address) and address.ipv4_mapped is not None: + address = address.ipv4_mapped + + return ( + address.is_loopback + or address.is_private + or address.is_link_local + or address.is_multicast + or address.is_reserved + or address.is_unspecified + ) + + +def new_signing_secret() -> str: + """ + generate a signing secret for a webhook destination + :returns: a prefixed, cryptographically random secret + """ + return f"{WEBHOOK_SECRET_PREFIX}{secrets.token_hex(32)}" + + +def new_delivery_attempt_id() -> str: + """ + generate an opaque identifier for a delivery attempt + :returns: a fresh attempt id such as ``att_1f0c9a...`` + """ + return f"{DELIVERY_ATTEMPT_ID_PREFIX}{uuid.uuid4().hex}" + + +def new_webhook_delivery_id() -> str: + """ + generate an opaque identifier for a logical delivery + :returns: a fresh delivery id such as ``whd_1f0c9a...`` + """ + return f"{WEBHOOK_DELIVERY_ID_PREFIX}{uuid.uuid4().hex}" + + +@dataclass(frozen=True, slots=True) +class WebhookTarget: + """ + the destination and secret a submission is owed delivery to + """ + + # Carried as a pair so that both are snapshotted together when a delivery is + # queued, and neither can be taken from a later version of the endpoint. + url: str + secret: str + + +def build_payload(submission: Submission) -> dict[str, Any]: + """ + build the event body describing a stored submission + :param submission: the submission that was accepted + :returns: the payload to serialize and send + """ + # Repeated values stay lists, exactly as they are stored, so a receiver never + # has to guess whether a field is single or multi valued. + return { + "type": SUBMISSION_RECEIVED_EVENT, + "submission": { + "id": submission.id, + "endpoint_id": submission.endpoint_id, + "received_at": _rfc3339(submission.received_at), + "fields": {name: list(values) for name, values in submission.fields.items()}, + }, + } + + +def serialize_payload(payload: dict[str, Any]) -> bytes: + """ + render a payload to the exact bytes that will be signed and sent + :param payload: the event body to serialize + :returns: the UTF-8 encoded JSON body + """ + return json.dumps(payload, separators=(",", ":"), ensure_ascii=False).encode("utf-8") + + +def sign(body: bytes, secret: str) -> str: + """ + compute the signature header value for an outbound body + :param body: the exact bytes that will be transmitted + :param secret: the destination's signing secret + :returns: the header value, such as ``v1=`` + """ + # Versioned from the start, so a future scheme can add another element + # without breaking receivers that only understand v1. + digest = hmac.new(secret.encode("utf-8"), body, hashlib.sha256).hexdigest() + return f"{SIGNATURE_VERSION}={digest}" + + +def _rfc3339(moment: datetime) -> str: + """ + render a timestamp the way the rest of the API renders timestamps + :param moment: the instant to render + :returns: an RFC 3339 timestamp in UTC, ending in Z + """ + return moment.astimezone(UTC).isoformat().replace("+00:00", "Z") diff --git a/src/hymical_forms/worker.py b/src/hymical_forms/worker.py new file mode 100644 index 0000000..56d60c9 --- /dev/null +++ b/src/hymical_forms/worker.py @@ -0,0 +1,152 @@ +""" +the delivery worker: claims owed webhook deliveries and sends them + +Run it as its own process, separately from the API:: + + python -m hymical_forms.worker + +It is deliberately not a FastAPI background task. The point of the outbox is +that the obligation survives the API process, and work that only runs inside +that process would give none of that back. +""" + +from __future__ import annotations + +import asyncio +import logging +import signal +import threading +from datetime import datetime +from types import FrameType + +import httpx2 +from sqlalchemy.orm import Session, sessionmaker + +from hymical_forms import storage, webhooks +from hymical_forms.config import Settings +from hymical_forms.db import create_engine_from_url, create_session_factory +from hymical_forms.delivery import create_webhook_client, deliver +from hymical_forms.models import utcnow + +logger = logging.getLogger(__name__) + + +async def process_batch( + session: Session, + client: httpx2.AsyncClient, + settings: Settings, + *, + now: datetime, +) -> int: + """ + claim whatever is due, deliver it, and record what happened + :param session: the session to claim and record through + :param client: the outbound client deliveries are sent with + :param settings: active configuration, read for the lease, batch size and retry policy + :param now: the instant to judge dueness against + :returns: how many deliveries were attempted + """ + claimed = storage.claim_due_deliveries( + session, + now=now, + lease_seconds=settings.worker_lease_seconds, + limit=settings.worker_batch_size, + ) + if not claimed: + return 0 + + submissions = storage.load_submissions(session, [job.submission_id for job in claimed]) + + # The network calls overlap so that one unresponsive destination does not + # hold up the rest of the batch for its whole timeout. They are made with no + # database transaction open: holding one across somebody else's server would + # pin a connection for as long as they take to answer. + bodies = { + job.id: webhooks.serialize_payload(webhooks.build_payload(submissions[job.submission_id])) + for job in claimed + } + results = await asyncio.gather( + *( + deliver(client, url=job.destination_url, secret=job.signing_secret, body=bodies[job.id]) + for job in claimed + ) + ) + + # One instant governs the whole batch: the claim, the attempt records and any + # backoff are all measured from ``now``. Re-reading the clock per delivery + # would gain nothing real and would make every retry schedule approximate. + policy = settings.retry_policy() + for job, result in zip(claimed, results, strict=True): + storage.complete_attempt(session, job, result, now=now, policy=policy) + logger.info( + "delivery %s attempt %d %s (%s)", + job.id, + job.attempts, + result.outcome, + job.state, + ) + + return len(claimed) + + +async def run_worker(settings: Settings, *, stop: threading.Event) -> None: + """ + poll for due deliveries until asked to stop + :param settings: active configuration + :param stop: event that ends the loop once set + """ + engine = create_engine_from_url(settings.database_url) + session_factory: sessionmaker[Session] = create_session_factory(engine) + client = create_webhook_client(settings) + + logger.info("worker started, polling every %.1fs", settings.worker_poll_seconds) + try: + while not stop.is_set(): + try: + with session_factory() as session: + handled = await process_batch(session, client, settings, now=utcnow()) + except Exception: + # A worker that dies on one bad tick stops delivering everything. + # Whatever was claimed keeps its lease and becomes due again once + # that lease expires, so the safe move is to log and keep polling. + logger.exception("delivery batch failed") + handled = 0 + + if handled == 0: + # Nothing was due, so wait before asking again rather than + # spinning against the database. A stop request cuts this short. + await asyncio.to_thread(stop.wait, settings.worker_poll_seconds) + finally: + await client.aclose() + engine.dispose() + logger.info("worker stopped") + + +def main() -> None: + """ + run the worker until the process is asked to shut down + """ + logging.basicConfig( + level=logging.INFO, + format="%(asctime)s %(levelname)-8s %(name)s %(message)s", + ) + settings = Settings() + stop = threading.Event() + + def request_stop(signum: int, frame: FrameType | None) -> None: + """ + ask the loop to finish the tick it is on and exit + :param signum: the signal received + :param frame: the interrupted stack frame, unused + """ + logger.info("shutdown requested") + stop.set() + + signal.signal(signal.SIGINT, request_stop) + signal.signal(signal.SIGTERM, request_stop) + + asyncio.run(run_worker(settings, stop=stop)) + + +if __name__ == "__main__": + main() diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..e520d95 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,189 @@ +""" +shared test fixtures + +Tests build their own application instances so that limits can be lowered to +values that are cheap to exercise, and so that a developer's local environment +can never change a test's outcome. Each application gets its own in-memory +SQLite database, which starts empty and disappears when the test ends. +""" + +from __future__ import annotations + +import asyncio +import os +from collections.abc import Callable, Iterator +from contextlib import ExitStack +from datetime import UTC, datetime +from typing import Any, cast + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient +from pydantic_settings import SettingsConfigDict +from sqlalchemy.orm import Session, sessionmaker + +from hymical_forms.app import create_app +from hymical_forms.config import Settings +from hymical_forms.delivery import create_webhook_client +from hymical_forms.worker import process_batch +from webhook_server import WebhookRecorder + +URLENCODED_HEADERS = {"content-type": "application/x-www-form-urlencoded"} + +DEFAULT_ENDPOINT_ID = "contact-form" +DEFAULT_ENDPOINT_NAME = "Contact form" + +# In-memory, so nothing survives a test and nothing touches the working tree. +TEST_DATABASE_URL = "sqlite://" + +ClientFactory = Callable[..., TestClient] + + +class IsolatedSettings(Settings): + """ + settings that ignore a local ``.env``, so a developer's file cannot skew a run + """ + + model_config = SettingsConfigDict(env_file=None) + + +@pytest.fixture(autouse=True) +def _ignore_ambient_configuration(monkeypatch: pytest.MonkeyPatch) -> None: + """ + hide any ``FORMS_*`` variables the developer happens to have exported + :param monkeypatch: pytest fixture used to remove the variables for one test + """ + for name in list(os.environ): + if name.startswith("FORMS_"): + monkeypatch.delenv(name) + + +def build_settings(**overrides: Any) -> Settings: + """ + build settings from defaults and explicit overrides only + :param overrides: setting values to replace the built-in defaults + :returns: settings that ignore the ambient environment + """ + overrides.setdefault("database_url", TEST_DATABASE_URL) + return IsolatedSettings(**overrides) + + +def create_endpoint( + client: TestClient, + endpoint_id: str = DEFAULT_ENDPOINT_ID, + *, + name: str = DEFAULT_ENDPOINT_NAME, + is_active: bool = True, + webhook_url: str | None = None, +) -> dict[str, Any]: + """ + register an endpoint through the public API, failing loudly if it does not take + :param client: the client whose application should hold the endpoint + :param endpoint_id: the public identifier to register + :param name: human-readable label for the endpoint + :param is_active: whether the endpoint should accept submissions + :param webhook_url: destination to deliver submissions to, or None for no webhook + :returns: the created endpoint as the API returned it + """ + body: dict[str, Any] = {"id": endpoint_id, "name": name, "is_active": is_active} + if webhook_url is not None: + body["webhook_url"] = webhook_url + response = client.post("/endpoints", json=body) + assert response.status_code == 201, response.text + return cast(dict[str, Any], response.json()) + + +def app_settings(client: TestClient) -> Settings: + """ + read the settings the client's application was built with + :param client: the client whose application should be inspected + :returns: that application's settings + """ + return cast(Settings, cast(FastAPI, client.app).state.settings) + + +def work_once(client: TestClient, *, now: datetime | None = None) -> int: + """ + run one worker batch against the client's database, as a separate worker would + :param client: the client whose application database holds the delivery queue + :param now: the instant the worker should treat as current, defaulting to real time + :returns: how many deliveries were attempted + """ + # Deliberately not started through the API process. The worker gets its own + # session and its own outbound client, the way a separate process would. + settings = app_settings(client) + moment = now if now is not None else datetime.now(UTC) + + async def run() -> int: + webhook_client = create_webhook_client(settings) + try: + with open_session(client) as session: + return await process_batch(session, webhook_client, settings, now=moment) + finally: + await webhook_client.aclose() + + return asyncio.run(run()) + + +def open_session(client: TestClient) -> Session: + """ + open a session against the database behind a client, for asserting on rows + :param client: the client whose application database should be inspected + :returns: a new session the caller is responsible for closing + """ + app = cast(FastAPI, client.app) + factory: sessionmaker[Session] = app.state.session_factory + return factory() + + +@pytest.fixture +def webhook() -> Iterator[WebhookRecorder]: + """ + provide a running local server that records the webhooks delivered to it + :returns: an iterator yielding the recorder, stopped when the test ends + """ + with WebhookRecorder() as recorder: + yield recorder + + +@pytest.fixture +def make_client() -> Iterator[ClientFactory]: + """ + provide a factory for clients bound to an app with the given setting overrides + :returns: a factory that accepts setting overrides and returns a test client + """ + with ExitStack() as stack: + + def factory(*, seed_endpoint: bool = True, **overrides: Any) -> TestClient: + """ + build a client for an app configured with the given overrides + :param seed_endpoint: whether to register the default endpoint first + :param overrides: setting values to replace the built-in defaults + :returns: a test client closed when the fixture tears down + """ + client = stack.enter_context(TestClient(create_app(build_settings(**overrides)))) + if seed_endpoint: + create_endpoint(client) + return client + + yield factory + + +@pytest.fixture +def client(make_client: ClientFactory) -> TestClient: + """ + provide a client on default settings whose app already holds the default endpoint + :param make_client: factory for clients bound to a configured app + :returns: a test client that can ingest submissions straight away + """ + return make_client() + + +@pytest.fixture +def empty_client(make_client: ClientFactory) -> TestClient: + """ + provide a client on default settings whose app holds no endpoints at all + :param make_client: factory for clients bound to a configured app + :returns: a test client with an empty database + """ + return make_client(seed_endpoint=False) diff --git a/tests/test_endpoint_ids.py b/tests/test_endpoint_ids.py new file mode 100644 index 0000000..828208d --- /dev/null +++ b/tests/test_endpoint_ids.py @@ -0,0 +1,71 @@ +""" +endpoint identifier rules + +An endpoint ID is 3 to 64 characters of lowercase ASCII letters, digits, ``-`` +and ``_``, and must start and end with a letter or digit. +""" + +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient + +from conftest import create_endpoint +from hymical_forms.ingestion import is_valid_endpoint_id + +VALID_IDS = [ + "abc", + "a1b", + "contact-form", + "signup_2026", + "x" * 64, +] + +INVALID_IDS = [ + "", + "ab", # shorter than the minimum + "x" * 65, # longer than the maximum + "Contact", # uppercase + "-contact", # leading separator + "contact-", # trailing separator + "_contact", + "contact_", + "con tact", # whitespace + "con.tact", # disallowed punctuation + "contact\n", # trailing newline must not slip past the pattern + "café", # non-ASCII + "../etc", # path traversal shapes +] + + +@pytest.mark.parametrize("endpoint_id", VALID_IDS) +def test_accepts_well_formed_identifiers(endpoint_id: str) -> None: + assert is_valid_endpoint_id(endpoint_id) + + +@pytest.mark.parametrize("endpoint_id", INVALID_IDS) +def test_rejects_malformed_identifiers(endpoint_id: str) -> None: + assert not is_valid_endpoint_id(endpoint_id) + + +@pytest.mark.parametrize("endpoint_id", VALID_IDS) +def test_valid_identifiers_are_addressable_once_registered( + empty_client: TestClient, endpoint_id: str +) -> None: + create_endpoint(empty_client, endpoint_id) + + response = empty_client.post(f"/f/{endpoint_id}", data={"email": "dev@example.com"}) + + assert response.status_code == 202 + assert response.json()["endpoint_id"] == endpoint_id + + +@pytest.mark.parametrize( + "path_segment", + ["Contact", "ab", "contact-", "con%20tact", "contact%0A", "con.tact", "caf%C3%A9"], +) +def test_invalid_identifiers_are_not_addressable(client: TestClient, path_segment: str) -> None: + response = client.post(f"/f/{path_segment}", data={"email": "dev@example.com"}) + + assert response.status_code == 404 + assert response.json()["error"]["code"] == "invalid_endpoint_id" diff --git a/tests/test_endpoints_api.py b/tests/test_endpoints_api.py new file mode 100644 index 0000000..dd6f225 --- /dev/null +++ b/tests/test_endpoints_api.py @@ -0,0 +1,118 @@ +""" +creating endpoints through ``POST /endpoints`` +""" + +from __future__ import annotations + +from datetime import UTC, datetime + +import pytest +from fastapi.testclient import TestClient +from sqlalchemy import select + +from conftest import create_endpoint, open_session +from hymical_forms import models + + +def test_creates_an_endpoint(empty_client: TestClient) -> None: + before = datetime.now(UTC) + response = empty_client.post("/endpoints", json={"id": "contact-form", "name": "Contact form"}) + after = datetime.now(UTC) + + assert response.status_code == 201 + body = response.json() + assert body["id"] == "contact-form" + assert body["name"] == "Contact form" + assert body["is_active"] is True + created_at = datetime.fromisoformat(body["created_at"]) + assert before <= created_at <= after + + +def test_the_created_endpoint_is_persisted(empty_client: TestClient) -> None: + create_endpoint(empty_client, "contact-form", name="Contact form") + + with open_session(empty_client) as session: + endpoint = session.get(models.Endpoint, "contact-form") + assert endpoint is not None + assert endpoint.name == "Contact form" + assert endpoint.is_active is True + assert endpoint.created_at.tzinfo is not None + + +def test_a_persisted_endpoint_can_receive_submissions(empty_client: TestClient) -> None: + create_endpoint(empty_client, "waitlist") + + response = empty_client.post("/f/waitlist", data={"email": "dev@example.com"}) + + assert response.status_code == 202 + assert response.json()["endpoint_id"] == "waitlist" + + +def test_an_endpoint_can_be_created_inactive(empty_client: TestClient) -> None: + body = create_endpoint(empty_client, "closed-form", is_active=False) + + assert body["is_active"] is False + + +def test_rejects_a_duplicate_endpoint_id(empty_client: TestClient) -> None: + create_endpoint(empty_client, "contact-form") + + response = empty_client.post("/endpoints", json={"id": "contact-form", "name": "Another form"}) + + assert response.status_code == 409 + body = response.json() + assert body["error"]["code"] == "endpoint_already_exists" + assert body["error"]["details"]["endpoint_id"] == "contact-form" + + +def test_a_rejected_duplicate_leaves_the_original_untouched(empty_client: TestClient) -> None: + """ + the failed insert must not commit, and must not overwrite the existing row + :param empty_client: test client whose app holds no endpoints + """ + create_endpoint(empty_client, "contact-form", name="Original name") + + empty_client.post("/endpoints", json={"id": "contact-form", "name": "Replacement name"}) + + with open_session(empty_client) as session: + endpoints = list(session.scalars(select(models.Endpoint))) + assert len(endpoints) == 1 + assert endpoints[0].name == "Original name" + + +@pytest.mark.parametrize("endpoint_id", ["ab", "Contact", "contact-", "con tact", "x" * 65]) +def test_rejects_a_malformed_endpoint_id(empty_client: TestClient, endpoint_id: str) -> None: + response = empty_client.post("/endpoints", json={"id": endpoint_id, "name": "A form"}) + + assert response.status_code == 422 + assert response.json()["error"]["code"] == "invalid_endpoint_id" + + +def test_a_rejected_endpoint_id_is_not_persisted(empty_client: TestClient) -> None: + empty_client.post("/endpoints", json={"id": "Contact", "name": "A form"}) + + with open_session(empty_client) as session: + assert list(session.scalars(select(models.Endpoint))) == [] + + +@pytest.mark.parametrize( + "payload", + [ + {"id": "contact-form"}, + {"id": "contact-form", "name": ""}, + {"id": "contact-form", "name": "x" * 201}, + {"name": "Contact form"}, + ], +) +def test_rejects_a_malformed_body(empty_client: TestClient, payload: dict[str, str]) -> None: + response = empty_client.post("/endpoints", json=payload) + + assert response.status_code == 422 + assert response.json()["error"]["code"] == "invalid_request" + + +def test_a_validation_failure_names_the_offending_field(empty_client: TestClient) -> None: + response = empty_client.post("/endpoints", json={"id": "contact-form", "name": ""}) + + fields = response.json()["error"]["details"]["fields"] + assert [field["field"] for field in fields] == ["name"] diff --git a/tests/test_errors.py b/tests/test_errors.py new file mode 100644 index 0000000..f0a30cf --- /dev/null +++ b/tests/test_errors.py @@ -0,0 +1,125 @@ +""" +the shared error envelope +""" + +from __future__ import annotations + +import pytest +from fastapi.testclient import TestClient + +from conftest import URLENCODED_HEADERS, build_settings +from hymical_forms.app import create_app + +ENDPOINT = "/f/contact-form" + + +def test_errors_share_one_envelope(client: TestClient) -> None: + response = client.post(ENDPOINT, json={"email": "dev@example.com"}) + + assert response.headers["content-type"].startswith("application/json") + error = response.json()["error"] + assert set(error) <= {"code", "message", "details"} + assert isinstance(error["code"], str) + assert isinstance(error["message"], str) + + +def test_details_are_omitted_when_there_is_nothing_to_add(client: TestClient) -> None: + response = client.post(ENDPOINT, content=b"", headers=URLENCODED_HEADERS) + + assert response.status_code == 422 + assert response.json() == { + "error": {"code": "empty_submission", "message": "Submission contains no fields."} + } + + +@pytest.mark.parametrize( + "content_type", + ["application/json", "text/plain", "application/octet-stream"], +) +def test_rejects_unsupported_content_types(client: TestClient, content_type: str) -> None: + response = client.post( + ENDPOINT, content=b"email=a%40b.co", headers={"content-type": content_type} + ) + + assert response.status_code == 415 + body = response.json() + assert body["error"]["code"] == "unsupported_media_type" + assert body["error"]["details"]["received"] == content_type + + +def test_rejects_a_request_without_a_content_type(client: TestClient) -> None: + response = client.post(ENDPOINT, content=b"email=a%40b.co", headers={"content-type": ""}) + + assert response.status_code == 415 + assert response.json()["error"]["code"] == "unsupported_media_type" + + +def test_rejects_multipart_without_a_boundary(client: TestClient) -> None: + response = client.post( + ENDPOINT, content=b"--x\r\nnope", headers={"content-type": "multipart/form-data"} + ) + + assert response.status_code == 400 + assert response.json()["error"]["code"] == "malformed_form_body" + + +def test_rejects_a_multipart_body_that_does_not_match_its_boundary(client: TestClient) -> None: + response = client.post( + ENDPOINT, + content=b"email=a%40b.co", + headers={"content-type": "multipart/form-data; boundary=hymical"}, + ) + + assert response.status_code == 400 + assert response.json()["error"]["code"] == "malformed_form_body" + + +def test_rejects_file_uploads(client: TestClient) -> None: + """ + file handling is out of scope, so a file part is refused rather than ignored + :param client: test client for an app on default settings + """ + response = client.post( + ENDPOINT, + data={"email": "dev@example.com"}, + files={"resume": ("cv.pdf", b"%PDF-1.4", "application/pdf")}, + ) + + assert response.status_code == 422 + body = response.json() + assert body["error"]["code"] == "file_upload_not_supported" + assert body["error"]["details"]["field"] == "resume" + + +def test_unknown_paths_use_the_envelope(client: TestClient) -> None: + response = client.post("/does-not-exist") + + assert response.status_code == 404 + assert response.json() == {"error": {"code": "not_found", "message": "Not Found"}} + + +def test_wrong_methods_use_the_envelope(client: TestClient) -> None: + response = client.get(ENDPOINT) + + assert response.status_code == 405 + assert response.json()["error"]["code"] == "method_not_allowed" + + +def test_unexpected_errors_do_not_leak_internals() -> None: + app = create_app(build_settings()) + + @app.get("/boom") + async def boom() -> None: + """ + raise an error carrying a secret, to prove the handler does not relay it + """ + raise RuntimeError("connection string: postgres://user:pa55w0rd@db/forms") + + with TestClient(app, raise_server_exceptions=False) as client: + response = client.get("/boom") + + assert response.status_code == 500 + assert response.json() == { + "error": {"code": "internal_error", "message": "The request could not be processed."} + } + assert "pa55w0rd" not in response.text diff --git a/tests/test_health.py b/tests/test_health.py new file mode 100644 index 0000000..2e303a6 --- /dev/null +++ b/tests/test_health.py @@ -0,0 +1,20 @@ +""" +health endpoint behaviour +""" + +from __future__ import annotations + +from fastapi.testclient import TestClient + +from hymical_forms import __version__ + + +def test_health_reports_a_running_process(client: TestClient) -> None: + response = client.get("/health") + + assert response.status_code == 200 + assert response.json() == { + "status": "ok", + "service": "hymical-forms", + "version": __version__, + } diff --git a/tests/test_idempotency.py b/tests/test_idempotency.py new file mode 100644 index 0000000..5532807 --- /dev/null +++ b/tests/test_idempotency.py @@ -0,0 +1,440 @@ +""" +idempotent ingestion: safe retries, key reuse, and the race between them +""" + +from __future__ import annotations + +import threading +from collections.abc import Callable +from concurrent.futures import ThreadPoolExecutor +from pathlib import Path +from typing import Any + +import pytest +from fastapi.testclient import TestClient +from sqlalchemy import select +from sqlalchemy.orm import Session + +from conftest import ClientFactory, create_endpoint, open_session +from hymical_forms import models, storage +from hymical_forms.ingestion import payload_fingerprint + +ENDPOINT = "/f/contact-form" +KEY = "b8f1c2d4e5a67890b8f1c2d4e5a67890" +OTHER_KEY = "0123456789abcdef0123456789abcdef" + +PAYLOAD = {"email": "dev@example.com", "message": "hello"} + + +def submit( + client: TestClient, + data: dict[str, Any] | None = None, + *, + key: str | None = KEY, + path: str = ENDPOINT, +) -> Any: + """ + post a form, optionally carrying an idempotency key + :param client: the client to post through + :param data: form fields to send, defaulting to a fixed payload + :param key: idempotency key to send, or None to omit the header + :param path: the ingestion path to post to + :returns: the HTTP response + """ + headers = {} if key is None else {"Idempotency-Key": key} + return client.post(path, data=PAYLOAD if data is None else data, headers=headers) + + +def stored(client: TestClient) -> list[models.Submission]: + """ + read every persisted submission behind a client + :param client: the client whose application database should be inspected + :returns: the submission rows + """ + with open_session(client) as session: + return list(session.scalars(select(models.Submission))) + + +# --- without a key: nothing changes ----------------------------------------- + + +def test_without_a_key_identical_requests_create_two_submissions(client: TestClient) -> None: + first = submit(client, key=None) + second = submit(client, key=None) + + assert first.status_code == second.status_code == 202 + assert first.json()["submission_id"] != second.json()["submission_id"] + assert len(stored(client)) == 2 + + +def test_without_a_key_the_response_is_never_a_replay(client: TestClient) -> None: + assert submit(client, key=None).json()["idempotent_replay"] is False + + +def test_without_a_key_no_idempotency_identity_is_stored(client: TestClient) -> None: + submit(client, key=None) + + row = stored(client)[0] + assert row.idempotency_key is None + assert row.payload_fingerprint is None + + +# --- first keyed submission -------------------------------------------------- + + +def test_a_keyed_submission_stores_its_idempotency_identity(client: TestClient) -> None: + response = submit(client) + + assert response.status_code == 202 + assert response.json()["idempotent_replay"] is False + row = stored(client)[0] + assert row.idempotency_key == KEY + assert row.payload_fingerprint == payload_fingerprint(row.to_domain().fields) + + +# --- replay ------------------------------------------------------------------ + + +def test_replaying_a_key_returns_the_original_submission(client: TestClient) -> None: + first = submit(client).json() + second = submit(client).json() + + assert len(stored(client)) == 1 + assert second["submission_id"] == first["submission_id"] + assert second["received_at"] == first["received_at"] + assert second["field_count"] == first["field_count"] + assert second["endpoint_id"] == first["endpoint_id"] + + +def test_a_replay_is_flagged_and_still_accepted(client: TestClient) -> None: + submit(client) + response = submit(client) + + assert response.status_code == 202 + assert response.json()["idempotent_replay"] is True + + +def test_many_replays_still_leave_one_submission(client: TestClient) -> None: + ids = {submit(client).json()["submission_id"] for _ in range(5)} + + assert len(ids) == 1 + assert len(stored(client)) == 1 + + +# --- conflict ---------------------------------------------------------------- + + +def test_reusing_a_key_for_different_content_conflicts(client: TestClient) -> None: + submit(client) + + response = submit(client, {"email": "dev@example.com", "message": "different"}) + + assert response.status_code == 409 + body = response.json() + assert body["error"]["code"] == "idempotency_conflict" + assert body["error"]["details"]["idempotency_key"] == KEY + + +def test_a_conflict_leaves_the_original_row_untouched(client: TestClient) -> None: + original = submit(client).json() + + submit(client, {"email": "attacker@example.com", "message": "overwritten"}) + + rows = stored(client) + assert len(rows) == 1 + assert rows[0].id == original["submission_id"] + assert rows[0].fields == {"email": ["dev@example.com"], "message": ["hello"]} + + +def test_a_conflict_does_not_describe_the_stored_payload(client: TestClient) -> None: + """ + the key is endpoint-scoped and unauthenticated, so a clash must not read content back + :param client: test client whose app already holds the default endpoint + """ + submit(client, {"secret": "original-value"}) + + response = submit(client, {"secret": "guess"}) + + assert response.status_code == 409 + assert "original-value" not in response.text + + +# --- scope ------------------------------------------------------------------- + + +def test_a_key_is_scoped_to_one_endpoint(empty_client: TestClient) -> None: + create_endpoint(empty_client, "contact-form") + create_endpoint(empty_client, "waitlist") + + first = submit(empty_client, path="/f/contact-form") + second = submit(empty_client, path="/f/waitlist") + + assert first.status_code == second.status_code == 202 + assert second.json()["idempotent_replay"] is False + assert first.json()["submission_id"] != second.json()["submission_id"] + assert {row.endpoint_id for row in stored(empty_client)} == {"contact-form", "waitlist"} + + +# --- what counts as the same payload ----------------------------------------- + + +def test_repeated_values_replay_when_unchanged(client: TestClient) -> None: + payload = {"topic": ["billing", "api"], "email": "dev@example.com"} + first = submit(client, payload) + + second = submit(client, payload) + + assert second.status_code == 202 + assert second.json()["submission_id"] == first.json()["submission_id"] + + +@pytest.mark.parametrize( + ("description", "changed"), + [ + ("a value differs", {"topic": ["billing", "docs"]}), + ("a value is dropped", {"topic": ["billing"]}), + ("a value is added", {"topic": ["billing", "api", "docs"]}), + ("the values are reordered", {"topic": ["api", "billing"]}), + ("a value is duplicated", {"topic": ["billing", "api", "api"]}), + ], +) +def test_changing_repeated_values_conflicts( + client: TestClient, description: str, changed: dict[str, list[str]] +) -> None: + submit(client, {"topic": ["billing", "api"]}) + + response = submit(client, changed) + + assert response.status_code == 409, description + assert len(stored(client)) == 1 + + +def test_reordering_fields_conflicts(client: TestClient) -> None: + """ + field order is preserved through storage, so it is part of what a payload is + :param client: test client whose app already holds the default endpoint + """ + headers = {"Idempotency-Key": KEY, "content-type": "application/x-www-form-urlencoded"} + client.post(ENDPOINT, content=b"a=1&b=2", headers=headers) + + response = client.post(ENDPOINT, content=b"b=2&a=1", headers=headers) + + assert response.status_code == 409 + assert len(stored(client)) == 1 + + +def test_the_fingerprint_ignores_generated_metadata(client: TestClient) -> None: + """ + two submissions of the same content agree on a fingerprint despite differing ids + :param client: test client whose app already holds the default endpoint + """ + submit(client, key=None) + submit(client, key=None) + + first, second = stored(client) + assert first.id != second.id + assert payload_fingerprint(first.to_domain().fields) == payload_fingerprint( + second.to_domain().fields + ) + + +# --- key syntax -------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("description", "key"), + [ + ("empty", ""), + ("too short", "abc"), + ("one below the minimum", "a" * 15), + ("too long", "a" * 256), + ("contains a space", "abcdef ghijklmnop"), + ("contains a tab", "abcdef\tghijklmnop"), + ("contains a newline", "abcdefghijklmnop\n"), + ], +) +def test_rejects_an_unusable_key(client: TestClient, description: str, key: str) -> None: + response = client.post(ENDPOINT, data=PAYLOAD, headers={"Idempotency-Key": key}) + + assert response.status_code == 400, description + assert response.json()["error"]["code"] == "invalid_idempotency_key" + assert stored(client) == [] + + +def test_rejects_a_non_ascii_key(client: TestClient) -> None: + """ + header values are bytes, so non-ASCII arrives latin-1 encoded rather than as text + :param client: test client whose app already holds the default endpoint + """ + response = client.post( + ENDPOINT, + data=PAYLOAD, + headers={b"Idempotency-Key": "kaffee-fur-alle-éééé".encode("latin-1")}, + ) + + assert response.status_code == 400 + assert response.json()["error"]["code"] == "invalid_idempotency_key" + assert stored(client) == [] + + +@pytest.mark.parametrize( + ("description", "key"), + [ + ("a uuid", "550e8400-e29b-41d4-a716-446655440000"), + ("hex", "b8f1c2d4e5a67890b8f1c2d4e5a67890"), + ("base64url", "abcDEF-_0123456789xyz"), + ("base64 with padding", "YWJjZGVmZ2hpamtsbW5vcA=="), + ("at the minimum length", "a" * 16), + ("at the maximum length", "a" * 255), + ], +) +def test_accepts_a_practical_key(client: TestClient, description: str, key: str) -> None: + response = client.post(ENDPOINT, data=PAYLOAD, headers={"Idempotency-Key": key}) + + assert response.status_code == 202, description + + +# --- the race ---------------------------------------------------------------- + + +def file_backed_client(make_client: ClientFactory, tmp_path: Path) -> TestClient: + """ + build a client whose database is a real file, so each session gets its own connection + :param make_client: factory for clients bound to a configured app + :param tmp_path: pytest-provided directory to hold the database file + :returns: a test client that can serve genuinely concurrent writes + """ + # The in-memory database used elsewhere is pinned to a single shared + # connection, which cannot express two writers racing. + return make_client(database_url=f"sqlite:///{tmp_path.as_posix()}/forms.db") + + +def fire_together(calls: list[Callable[[], Any]]) -> list[Any]: + """ + run callables from separate threads, released at the same moment + :param calls: the callables to run, one per thread + :returns: each callable's result, in the order given + """ + barrier = threading.Barrier(len(calls)) + + def run(call: Callable[[], Any]) -> Any: + barrier.wait() + return call() + + with ThreadPoolExecutor(max_workers=len(calls)) as pool: + return [future.result() for future in [pool.submit(run, call) for call in calls]] + + +def test_concurrent_identical_submissions_store_one_row( + make_client: ClientFactory, tmp_path: Path +) -> None: + """ + the losers of the insert race must resolve to the winner's row, not to duplicates + :param make_client: factory for clients bound to a configured app + :param tmp_path: pytest-provided directory to hold the database file + """ + client = file_backed_client(make_client, tmp_path) + + responses = fire_together([lambda: submit(client) for _ in range(6)]) + + assert [r.status_code for r in responses] == [202] * 6 + assert len({r.json()["submission_id"] for r in responses}) == 1 + assert len(stored(client)) == 1 + assert sum(r.json()["idempotent_replay"] for r in responses) == 5 + + +def test_concurrent_conflicting_submissions_store_one_row( + make_client: ClientFactory, tmp_path: Path +) -> None: + """ + exactly one racer wins the key; the rest must be refused rather than duplicated + :param make_client: factory for clients bound to a configured app + :param tmp_path: pytest-provided directory to hold the database file + """ + client = file_backed_client(make_client, tmp_path) + + responses = fire_together( + [(lambda n=n: submit(client, {"message": f"payload-{n}"})) for n in range(6)] # type: ignore[misc] + ) + + statuses = sorted(r.status_code for r in responses) + assert statuses == [202] + [409] * 5 + assert len(stored(client)) == 1 + + +def test_losing_the_race_replays_the_winner( + client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """ + drive the reconciliation branch deterministically, with a real constraint violation + :param client: test client whose app already holds the default endpoint + :param monkeypatch: pytest fixture used to blind the pre-check lookup once + """ + # Only the optimistic lookup is blinded, and only for the attempt that races. + # The unique violation, the rollback and the re-read are all real, which is + # exactly what a request that lost the race actually experiences. + original = submit(client).json() + real_lookup = storage.find_by_idempotency_key + calls = {"n": 0} + + def blind_first_lookup(session: Session, endpoint_id: str, key: str) -> Any: + calls["n"] += 1 + if calls["n"] == 1: + return None + return real_lookup(session, endpoint_id, key) + + monkeypatch.setattr(storage, "find_by_idempotency_key", blind_first_lookup) + response = submit(client) + monkeypatch.undo() + + assert calls["n"] == 2, "the insert should have failed and forced a second lookup" + assert response.status_code == 202 + assert response.json()["submission_id"] == original["submission_id"] + assert response.json()["idempotent_replay"] is True + assert len(stored(client)) == 1 + + +def test_losing_the_race_with_different_content_conflicts( + client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """ + a racer whose payload differs must be refused once it sees the winner's row + :param client: test client whose app already holds the default endpoint + :param monkeypatch: pytest fixture used to blind the pre-check lookup once + """ + submit(client) + real_lookup = storage.find_by_idempotency_key + calls = {"n": 0} + + def blind_first_lookup(session: Session, endpoint_id: str, key: str) -> Any: + calls["n"] += 1 + if calls["n"] == 1: + return None + return real_lookup(session, endpoint_id, key) + + monkeypatch.setattr(storage, "find_by_idempotency_key", blind_first_lookup) + response = submit(client, {"message": "different"}) + monkeypatch.undo() + + assert response.status_code == 409 + assert response.json()["error"]["code"] == "idempotency_conflict" + assert len(stored(client)) == 1 + + +def test_an_unexplained_integrity_error_is_not_reported_as_success( + client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """ + a constraint violation we cannot attribute to the key must not become a 202 + :param client: test client whose app already holds the default endpoint + :param monkeypatch: pytest fixture used to blind the lookup permanently + """ + submit(client) + monkeypatch.setattr(storage, "find_by_idempotency_key", lambda *args: None) + + response = submit(client) + monkeypatch.undo() + + assert response.status_code == 503 + assert response.json()["error"]["code"] == "storage_unavailable" + assert len(stored(client)) == 1 diff --git a/tests/test_ingestion.py b/tests/test_ingestion.py new file mode 100644 index 0000000..fd12d96 --- /dev/null +++ b/tests/test_ingestion.py @@ -0,0 +1,63 @@ +""" +the internal submission representation + +These tests pin the shape the rest of the system will eventually persist and +deliver, which the HTTP acknowledgement only summarises. +""" + +from __future__ import annotations + +from datetime import UTC, datetime + +import pytest + +from hymical_forms.ingestion import SubmissionRejected, build_submission + +LIMITS = { + "max_fields": 100, + "max_field_name_length": 128, + "max_field_value_length": 16384, +} + + +def test_normalizes_pairs_into_a_field_mapping() -> None: + submission = build_submission( + "contact-form", + [("email", "dev@example.com"), ("message", "hello")], + **LIMITS, + ) + + assert submission.endpoint_id == "contact-form" + assert submission.fields == {"email": ("dev@example.com",), "message": ("hello",)} + assert submission.field_count == 2 + + +def test_preserves_repeated_names_in_submission_order() -> None: + submission = build_submission( + "contact-form", + [("topic", "billing"), ("email", "dev@example.com"), ("topic", "api")], + **LIMITS, + ) + + assert submission.fields == { + "topic": ("billing", "api"), + "email": ("dev@example.com",), + } + assert submission.field_count == 3 + + +def test_stamps_each_submission_with_an_id_and_utc_timestamp() -> None: + before = datetime.now(UTC) + submission = build_submission("contact-form", [("email", "a@b.co")], **LIMITS) + after = datetime.now(UTC) + + assert submission.id.startswith("sub_") + assert submission.received_at.tzinfo is not None + assert before <= submission.received_at <= after + + +def test_rejects_a_submission_with_no_fields() -> None: + with pytest.raises(SubmissionRejected) as raised: + build_submission("contact-form", [], **LIMITS) + + assert raised.value.code == "empty_submission" diff --git a/tests/test_limits.py b/tests/test_limits.py new file mode 100644 index 0000000..6fdee22 --- /dev/null +++ b/tests/test_limits.py @@ -0,0 +1,145 @@ +""" +limits that protect the ingestion boundary +""" + +from __future__ import annotations + +from collections.abc import Iterator + +from conftest import URLENCODED_HEADERS, ClientFactory + +ENDPOINT = "/f/contact-form" + + +def _chunks(*parts: bytes) -> Iterator[bytes]: + """ + yield a body in pieces so the client streams it without a Content-Length + :param parts: body fragments to send in order + :returns: an iterator over the fragments + """ + yield from parts + + +def test_rejects_a_body_larger_than_the_declared_limit(make_client: ClientFactory) -> None: + client = make_client(max_body_bytes=64) + + response = client.post(ENDPOINT, content=b"note=" + b"x" * 200, headers=URLENCODED_HEADERS) + + assert response.status_code == 413 + body = response.json() + assert body["error"]["code"] == "request_body_too_large" + assert body["error"]["details"]["limit_bytes"] == 64 + + +def test_rejects_an_oversized_streamed_body(make_client: ClientFactory) -> None: + """ + a chunked request cannot escape the limit by omitting Content-Length + :param make_client: factory for clients bound to a configured app + """ + client = make_client(max_body_bytes=64) + + response = client.post( + ENDPOINT, + content=_chunks(b"note=", b"x" * 200), + headers=URLENCODED_HEADERS, + ) + + assert response.status_code == 413 + assert response.json()["error"]["code"] == "request_body_too_large" + + +def test_accepts_a_body_at_the_limit(make_client: ClientFactory) -> None: + client = make_client(max_body_bytes=64) + body = b"note=" + b"x" * 59 + + response = client.post(ENDPOINT, content=body, headers=URLENCODED_HEADERS) + + assert len(body) == 64 + assert response.status_code == 202 + + +def test_rejects_too_many_fields(make_client: ClientFactory) -> None: + client = make_client(max_fields=3) + + response = client.post(ENDPOINT, data={f"field{i}": "v" for i in range(4)}) + + assert response.status_code == 422 + body = response.json() + assert body["error"]["code"] == "too_many_fields" + assert body["error"]["details"] == {"limit": 3, "received": 4} + + +def test_accepts_the_maximum_number_of_fields(make_client: ClientFactory) -> None: + client = make_client(max_fields=3) + + response = client.post(ENDPOINT, data={f"field{i}": "v" for i in range(3)}) + + assert response.status_code == 202 + assert response.json()["field_count"] == 3 + + +def test_repeated_names_count_towards_the_field_limit(make_client: ClientFactory) -> None: + client = make_client(max_fields=2) + + response = client.post(ENDPOINT, data={"topic": ["a", "b", "c"]}) + + assert response.status_code == 422 + assert response.json()["error"]["code"] == "too_many_fields" + + +def test_rejects_an_overlong_field_name(make_client: ClientFactory) -> None: + client = make_client(max_field_name_length=8) + + response = client.post(ENDPOINT, data={"a" * 9: "value"}) + + assert response.status_code == 422 + assert response.json()["error"]["code"] == "field_name_too_long" + + +def test_rejects_an_overlong_field_value(make_client: ClientFactory) -> None: + client = make_client(max_field_value_length=8) + + response = client.post(ENDPOINT, data={"note": "x" * 9}) + + assert response.status_code == 422 + body = response.json() + assert body["error"]["code"] == "field_value_too_long" + assert body["error"]["details"]["field"] == "note" + + +def test_accepts_values_at_the_length_limit(make_client: ClientFactory) -> None: + client = make_client(max_field_value_length=8) + + response = client.post(ENDPOINT, data={"note": "x" * 8}) + + assert response.status_code == 202 + + +def test_rejects_control_characters_in_a_field_name(make_client: ClientFactory) -> None: + client = make_client() + + response = client.post(ENDPOINT, content=b"na%0Ame=value", headers=URLENCODED_HEADERS) + + assert response.status_code == 422 + assert response.json()["error"]["code"] == "invalid_field_name" + + +def test_rejects_a_null_byte_in_a_field_value(make_client: ClientFactory) -> None: + client = make_client() + + response = client.post(ENDPOINT, content=b"note=%00", headers=URLENCODED_HEADERS) + + assert response.status_code == 422 + assert response.json()["error"]["code"] == "invalid_field_value" + + +def test_allows_newlines_inside_a_textarea_value(make_client: ClientFactory) -> None: + """ + multi-line textarea input is legitimate and must not trip the name rules + :param make_client: factory for clients bound to a configured app + """ + client = make_client() + + response = client.post(ENDPOINT, data={"message": "line one\r\nline two"}) + + assert response.status_code == 202 diff --git a/tests/test_persistence.py b/tests/test_persistence.py new file mode 100644 index 0000000..8a9be8b --- /dev/null +++ b/tests/test_persistence.py @@ -0,0 +1,210 @@ +""" +what reaches the database when a submission is accepted, and what does not +""" + +from __future__ import annotations + +from datetime import UTC, datetime + +import pytest +from fastapi.testclient import TestClient +from sqlalchemy import select +from sqlalchemy.exc import OperationalError +from sqlalchemy.orm import Session + +from conftest import URLENCODED_HEADERS, ClientFactory, create_endpoint, open_session +from hymical_forms import models + +ENDPOINT = "/f/contact-form" + + +def stored_submissions(client: TestClient) -> list[models.Submission]: + """ + read every persisted submission behind a client + :param client: the client whose application database should be inspected + :returns: the submission rows, detached from their session + """ + with open_session(client) as session: + return list(session.scalars(select(models.Submission))) + + +def test_an_accepted_submission_is_persisted(client: TestClient) -> None: + response = client.post(ENDPOINT, data={"email": "dev@example.com", "message": "hello"}) + + assert response.status_code == 202 + submissions = stored_submissions(client) + assert len(submissions) == 1 + assert submissions[0].endpoint_id == "contact-form" + + +def test_the_returned_id_matches_the_persisted_row(client: TestClient) -> None: + response = client.post(ENDPOINT, data={"email": "dev@example.com"}) + + assert stored_submissions(client)[0].id == response.json()["submission_id"] + + +def test_the_returned_timestamp_matches_the_persisted_row(client: TestClient) -> None: + before = datetime.now(UTC) + response = client.post(ENDPOINT, data={"email": "dev@example.com"}) + after = datetime.now(UTC) + + received_at = stored_submissions(client)[0].received_at + assert received_at.tzinfo is not None + assert before <= received_at <= after + assert received_at == datetime.fromisoformat(response.json()["received_at"]) + + +def test_repeated_field_values_survive_persistence(client: TestClient) -> None: + """ + a checkbox group must come back out with every value, in the order it arrived + :param client: test client whose app already holds the default endpoint + """ + response = client.post( + ENDPOINT, data={"topic": ["billing", "api", "docs"], "email": "dev@example.com"} + ) + + assert response.json()["field_count"] == 4 + submission = stored_submissions(client)[0].to_domain() + assert submission.fields == { + "topic": ("billing", "api", "docs"), + "email": ("dev@example.com",), + } + assert submission.field_count == 4 + + +def test_field_order_survives_persistence(client: TestClient) -> None: + """ + JSON object key order is preserved, so a form's fields keep their order + :param client: test client whose app already holds the default endpoint + """ + client.post(ENDPOINT, content=b"zebra=1&apple=2&mango=3", headers=URLENCODED_HEADERS) + + assert list(stored_submissions(client)[0].fields) == ["zebra", "apple", "mango"] + + +def test_a_blank_value_survives_persistence(client: TestClient) -> None: + client.post(ENDPOINT, content=b"nickname=", headers=URLENCODED_HEADERS) + + assert stored_submissions(client)[0].fields == {"nickname": [""]} + + +def test_submissions_are_persisted_against_their_own_endpoint(empty_client: TestClient) -> None: + create_endpoint(empty_client, "contact-form") + create_endpoint(empty_client, "waitlist") + + empty_client.post("/f/contact-form", data={"email": "a@example.com"}) + empty_client.post("/f/waitlist", data={"email": "b@example.com"}) + + stored = {row.endpoint_id for row in stored_submissions(empty_client)} + assert stored == {"contact-form", "waitlist"} + + +def test_rejects_a_submission_to_an_unknown_endpoint(empty_client: TestClient) -> None: + response = empty_client.post(ENDPOINT, data={"email": "dev@example.com"}) + + assert response.status_code == 404 + body = response.json() + assert body["error"]["code"] == "endpoint_not_found" + assert body["error"]["details"]["endpoint_id"] == "contact-form" + assert stored_submissions(empty_client) == [] + + +def test_rejects_a_submission_to_an_inactive_endpoint(empty_client: TestClient) -> None: + create_endpoint(empty_client, "contact-form", is_active=False) + + response = empty_client.post(ENDPOINT, data={"email": "dev@example.com"}) + + assert response.status_code == 409 + assert response.json()["error"]["code"] == "endpoint_inactive" + assert stored_submissions(empty_client) == [] + + +@pytest.mark.parametrize( + ("description", "kwargs", "expected_status"), + [ + ("empty submission", {"content": b"", "headers": URLENCODED_HEADERS}, 422), + ( + "malformed multipart", + {"content": b"--x\r\nnope", "headers": {"content-type": "multipart/form-data"}}, + 400, + ), + ("unsupported content type", {"json": {"email": "a@b.co"}}, 415), + ], +) +def test_an_invalid_submission_is_not_persisted( + client: TestClient, description: str, kwargs: dict[str, object], expected_status: int +) -> None: + response = client.post(ENDPOINT, **kwargs) # type: ignore[arg-type] + + assert response.status_code == expected_status, description + assert stored_submissions(client) == [] + + +def test_an_over_limit_submission_is_not_persisted(make_client: ClientFactory) -> None: + client = make_client(max_fields=2) + + response = client.post(ENDPOINT, data={"a": "1", "b": "2", "c": "3"}) + + assert response.status_code == 422 + assert response.json()["error"]["code"] == "too_many_fields" + assert stored_submissions(client) == [] + + +def test_an_oversized_body_is_not_persisted(make_client: ClientFactory) -> None: + client = make_client(max_body_bytes=64) + + response = client.post(ENDPOINT, content=b"note=" + b"x" * 200, headers=URLENCODED_HEADERS) + + assert response.status_code == 413 + assert stored_submissions(client) == [] + + +def test_a_storage_failure_is_not_acknowledged( + client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """ + a commit that fails must produce an error, never a 202 for a submission that vanished + :param client: test client whose app already holds the default endpoint + :param monkeypatch: pytest fixture used to break the commit for one request + """ + + def failing_commit(self: Session) -> None: + raise OperationalError( + "INSERT INTO submissions (id, endpoint_id) VALUES (?, ?)", + {}, + Exception("server closed the connection unexpectedly"), + ) + + monkeypatch.setattr(Session, "commit", failing_commit) + response = client.post(ENDPOINT, data={"email": "dev@example.com"}) + monkeypatch.undo() + + assert response.status_code == 503 + assert response.json()["error"]["code"] == "storage_unavailable" + assert stored_submissions(client) == [] + + +def test_a_storage_failure_does_not_leak_database_details( + client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """ + the error envelope must carry no SQL, table names or driver messages + :param client: test client whose app already holds the default endpoint + :param monkeypatch: pytest fixture used to break the commit for one request + """ + + def failing_commit(self: Session) -> None: + raise OperationalError( + "INSERT INTO submissions (id, endpoint_id) VALUES (?, ?)", + {}, + Exception("server closed the connection unexpectedly"), + ) + + monkeypatch.setattr(Session, "commit", failing_commit) + response = client.post(ENDPOINT, data={"email": "dev@example.com"}) + monkeypatch.undo() + + text = response.text + assert "INSERT" not in text + assert "server closed the connection" not in text + assert "OperationalError" not in text diff --git a/tests/test_submissions.py b/tests/test_submissions.py new file mode 100644 index 0000000..105c752 --- /dev/null +++ b/tests/test_submissions.py @@ -0,0 +1,123 @@ +""" +accepting form submissions +""" + +from __future__ import annotations + +from datetime import UTC, datetime + +from fastapi.testclient import TestClient + +from conftest import URLENCODED_HEADERS + +ENDPOINT = "/f/contact-form" + + +def test_accepts_a_urlencoded_submission(client: TestClient) -> None: + response = client.post(ENDPOINT, data={"email": "dev@example.com", "message": "hello"}) + + assert response.status_code == 202 + body = response.json() + assert body["endpoint_id"] == "contact-form" + assert body["field_count"] == 2 + + +def test_accepts_a_multipart_submission(client: TestClient) -> None: + """ + browsers send ``enctype="multipart/form-data"`` forms, so text parts are accepted + :param client: test client for an app on default settings + """ + boundary = "hymicalboundary" + body = ( + f"--{boundary}\r\n" + 'Content-Disposition: form-data; name="email"\r\n\r\n' + "dev@example.com\r\n" + f"--{boundary}--\r\n" + ).encode() + + response = client.post( + ENDPOINT, + content=body, + headers={"content-type": f"multipart/form-data; boundary={boundary}"}, + ) + + assert response.status_code == 202 + assert response.json()["field_count"] == 1 + + +def test_generates_submission_metadata(client: TestClient) -> None: + before = datetime.now(UTC) + response = client.post(ENDPOINT, data={"email": "dev@example.com"}) + after = datetime.now(UTC) + + body = response.json() + assert body["submission_id"].startswith("sub_") + received_at = datetime.fromisoformat(body["received_at"]) + assert received_at.tzinfo is not None + assert before <= received_at <= after + + +def test_each_submission_gets_a_distinct_id(client: TestClient) -> None: + ids = { + client.post(ENDPOINT, data={"email": "dev@example.com"}).json()["submission_id"] + for _ in range(5) + } + + assert len(ids) == 5 + + +def test_repeated_field_names_are_all_counted(client: TestClient) -> None: + """ + checkbox groups submit one name several times, so no value may be dropped + :param client: test client for an app on default settings + """ + response = client.post(ENDPOINT, data={"topic": ["billing", "api", "docs"], "email": "a@b.co"}) + + assert response.status_code == 202 + assert response.json()["field_count"] == 4 + + +def test_accepts_a_field_with_an_empty_value(client: TestClient) -> None: + """ + an optional text input that the user left blank is still a submitted field + :param client: test client for an app on default settings + """ + response = client.post(ENDPOINT, content=b"nickname=", headers=URLENCODED_HEADERS) + + assert response.status_code == 202 + assert response.json()["field_count"] == 1 + + +def test_accepts_non_ascii_values(client: TestClient) -> None: + response = client.post(ENDPOINT, data={"name": "Zoë", "note": "naïve café"}) + + assert response.status_code == 202 + assert response.json()["field_count"] == 2 + + +def test_content_type_parameters_and_casing_are_ignored(client: TestClient) -> None: + response = client.post( + ENDPOINT, + content=b"email=dev%40example.com", + headers={"content-type": "APPLICATION/X-WWW-Form-Urlencoded; charset=UTF-8"}, + ) + + assert response.status_code == 202 + + +def test_does_not_echo_submitted_values(client: TestClient) -> None: + """ + the acknowledgement is metadata only, so user input is not reflected back + :param client: test client for an app on default settings + """ + response = client.post(ENDPOINT, data={"secret": "hunter2"}) + + assert "hunter2" not in response.text + assert set(response.json()) == { + "submission_id", + "endpoint_id", + "received_at", + "field_count", + "idempotent_replay", + "delivery", + } diff --git a/tests/test_webhooks.py b/tests/test_webhooks.py new file mode 100644 index 0000000..f9e7b8a --- /dev/null +++ b/tests/test_webhooks.py @@ -0,0 +1,486 @@ +""" +the ingestion side of webhooks: configuration, and queueing work without sending it + +Delivery itself belongs to the worker and is covered in ``test_worker.py``. What +matters here is that accepting a submission creates the durable obligation and +makes no outbound request while doing it. +""" + +from __future__ import annotations + +import logging +import threading +from concurrent.futures import ThreadPoolExecutor +from pathlib import Path +from typing import Any + +import pytest +from fastapi.testclient import TestClient +from sqlalchemy import select +from sqlalchemy.exc import OperationalError +from sqlalchemy.orm import Session + +from conftest import ClientFactory, create_endpoint, open_session +from hymical_forms import models +from hymical_forms.webhooks import DeliveryState +from webhook_server import WebhookRecorder + +ENDPOINT = "/f/contact-form" +KEY = "b8f1c2d4e5a67890b8f1c2d4e5a67890" + + +def hooked_client( + make_client: ClientFactory, url: str, **overrides: Any +) -> tuple[TestClient, dict[str, Any]]: + """ + build a client whose default endpoint delivers to a given destination + :param make_client: factory for clients bound to a configured app + :param url: the webhook destination to configure + :param overrides: extra setting overrides for the application + :returns: the client and the created endpoint as the API returned it + """ + overrides.setdefault("allow_private_webhook_targets", True) + client = make_client(seed_endpoint=False, **overrides) + endpoint = create_endpoint(client, webhook_url=url) + return client, endpoint + + +def deliveries(client: TestClient) -> list[models.WebhookDelivery]: + """ + read every queued delivery behind a client + :param client: the client whose application database should be inspected + :returns: the delivery rows + """ + with open_session(client) as session: + return list(session.scalars(select(models.WebhookDelivery))) + + +def attempts(client: TestClient) -> list[models.DeliveryAttempt]: + """ + read every recorded delivery attempt behind a client + :param client: the client whose application database should be inspected + :returns: the attempt rows + """ + with open_session(client) as session: + return list(session.scalars(select(models.DeliveryAttempt))) + + +# --- configuration ----------------------------------------------------------- + + +def test_an_endpoint_can_be_created_with_a_webhook( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + _, endpoint = hooked_client(make_client, webhook.url) + + assert endpoint["webhook_url"] == webhook.url + assert endpoint["webhook_secret"].startswith("whsec_") + + +def test_the_webhook_configuration_is_persisted( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, endpoint = hooked_client(make_client, webhook.url) + + with open_session(client) as session: + row = session.get(models.Endpoint, "contact-form") + assert row is not None + assert row.webhook_url == webhook.url + assert row.webhook_secret == endpoint["webhook_secret"] + + +def test_an_endpoint_without_a_webhook_reports_none(client: TestClient) -> None: + with open_session(client) as session: + row = session.get(models.Endpoint, "contact-form") + assert row is not None + assert row.webhook_url is None + assert row.webhook_secret is None + + +def test_each_endpoint_gets_its_own_secret( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client = make_client(seed_endpoint=False, allow_private_webhook_targets=True) + + first = create_endpoint(client, "form-one", webhook_url=webhook.url) + second = create_endpoint(client, "form-two", webhook_url=webhook.url) + + assert first["webhook_secret"] != second["webhook_secret"] + + +@pytest.mark.parametrize( + ("description", "url"), + [ + ("a file URL", "file:///etc/passwd"), + ("an ftp URL", "ftp://example.com/hook"), + ("a javascript URL", "javascript:alert(1)"), + ("a scheme-less string", "example.com/hook"), + ("empty", ""), + ("no host", "https:///hook"), + ("nonsense", "not a url at all"), + ], +) +def test_rejects_an_unusable_webhook_url( + make_client: ClientFactory, description: str, url: str +) -> None: + client = make_client(seed_endpoint=False, allow_private_webhook_targets=True) + + response = client.post( + "/endpoints", json={"id": "contact-form", "name": "C", "webhook_url": url} + ) + + assert response.status_code == 422, description + assert response.json()["error"]["code"] == "invalid_webhook_url" + + +@pytest.mark.parametrize( + ("description", "url"), + [ + ("localhost", "http://localhost:9000/hook"), + ("a localhost subdomain", "http://api.localhost/hook"), + ("ipv4 loopback", "http://127.0.0.1:9000/hook"), + ("ipv6 loopback", "http://[::1]:9000/hook"), + ("an ipv4-mapped ipv6 loopback", "http://[::ffff:127.0.0.1]/hook"), + ("a private range", "http://10.1.2.3/hook"), + ("another private range", "http://192.168.0.5/hook"), + ("link-local metadata", "http://169.254.169.254/latest/meta-data/"), + ("the unspecified address", "http://0.0.0.0/hook"), + ], +) +def test_rejects_an_internal_webhook_target( + make_client: ClientFactory, description: str, url: str +) -> None: + client = make_client(seed_endpoint=False) + + response = client.post( + "/endpoints", json={"id": "contact-form", "name": "C", "webhook_url": url} + ) + + assert response.status_code == 422, description + assert response.json()["error"]["code"] == "invalid_webhook_url" + + +def test_a_rejected_webhook_url_creates_no_endpoint(make_client: ClientFactory) -> None: + client = make_client(seed_endpoint=False) + + client.post( + "/endpoints", + json={"id": "contact-form", "name": "C", "webhook_url": "http://127.0.0.1/hook"}, + ) + + with open_session(client) as session: + assert list(session.scalars(select(models.Endpoint))) == [] + + +# --- queueing, not sending --------------------------------------------------- + + +def test_accepting_a_submission_queues_a_delivery( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = hooked_client(make_client, webhook.url) + + response = client.post(ENDPOINT, data={"email": "dev@example.com"}) + + assert response.status_code == 202 + assert response.json()["delivery"] == {"queued": True} + queued = deliveries(client) + assert len(queued) == 1 + assert queued[0].submission_id == response.json()["submission_id"] + assert queued[0].state == DeliveryState.PENDING + assert queued[0].attempts == 0 + + +def test_ingestion_makes_no_outbound_request( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + """ + the form request must return without waiting on anybody else's server + :param make_client: factory for clients bound to a configured app + :param webhook: the local server that would record a delivery if one happened + """ + client, _ = hooked_client(make_client, webhook.url) + + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + assert webhook.received == [] + assert attempts(client) == [] + + +def test_ingestion_is_not_slowed_by_a_hanging_destination( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + """ + a destination that never answers cannot couple itself to ingestion latency + :param make_client: factory for clients bound to a configured app + :param webhook: the local server, set to stall well past any ingestion timeout + """ + webhook.delay_seconds = 30.0 + client, _ = hooked_client(make_client, webhook.url) + + # No timeout is needed to make this fast, because nothing is dialled at all. + response = client.post(ENDPOINT, data={"email": "dev@example.com"}) + + assert response.status_code == 202 + assert webhook.received == [] + + +def test_the_queued_delivery_snapshots_the_destination_and_secret( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + """ + a delivery owes what was configured when it was accepted, not what is configured later + :param make_client: factory for clients bound to a configured app + :param webhook: the local server recording deliveries + """ + client, endpoint = hooked_client(make_client, webhook.url) + + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + queued = deliveries(client)[0] + assert queued.destination_url == webhook.url + assert queued.signing_secret == endpoint["webhook_secret"] + + +def test_the_first_attempt_is_due_immediately( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = hooked_client(make_client, webhook.url) + + response = client.post(ENDPOINT, data={"email": "dev@example.com"}) + + queued = deliveries(client)[0] + assert queued.next_attempt_at.isoformat() == response.json()["received_at"].replace( + "Z", "+00:00" + ) + assert queued.claim_expires_at is None + assert queued.completed_at is None + + +def test_without_a_webhook_nothing_is_queued(client: TestClient) -> None: + response = client.post(ENDPOINT, data={"email": "dev@example.com"}) + + assert response.status_code == 202 + assert response.json()["delivery"] == {"queued": False} + assert deliveries(client) == [] + + +def test_each_submission_queues_its_own_delivery( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = hooked_client(make_client, webhook.url) + + client.post(ENDPOINT, data={"email": "one@example.com"}) + client.post(ENDPOINT, data={"email": "two@example.com"}) + + assert len(deliveries(client)) == 2 + + +# --- rejected submissions queue nothing -------------------------------------- + + +@pytest.mark.parametrize( + ("description", "kwargs"), + [ + ("empty submission", {"data": {}}), + ("unsupported content type", {"json": {"email": "a@b.co"}}), + ( + "malformed multipart", + {"content": b"--x\r\nnope", "headers": {"content-type": "multipart/form-data"}}, + ), + ], +) +def test_a_rejected_submission_queues_nothing( + make_client: ClientFactory, + webhook: WebhookRecorder, + description: str, + kwargs: dict[str, Any], +) -> None: + client, _ = hooked_client(make_client, webhook.url) + + response = client.post(ENDPOINT, **kwargs) + + assert response.status_code >= 400, description + assert deliveries(client) == [] + with open_session(client) as session: + assert list(session.scalars(select(models.Submission))) == [] + + +def test_an_inactive_endpoint_queues_nothing( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client = make_client(seed_endpoint=False, allow_private_webhook_targets=True) + create_endpoint(client, "closed-form", is_active=False, webhook_url=webhook.url) + + response = client.post("/f/closed-form", data={"email": "dev@example.com"}) + + assert response.status_code == 409 + assert deliveries(client) == [] + + +def test_a_storage_failure_leaves_neither_a_submission_nor_delivery_work( + make_client: ClientFactory, webhook: WebhookRecorder, monkeypatch: pytest.MonkeyPatch +) -> None: + """ + the pair is one transaction, so a failure cannot leave half of it behind + :param make_client: factory for clients bound to a configured app + :param webhook: the local server recording deliveries + :param monkeypatch: pytest fixture used to break the commit for one request + """ + client, _ = hooked_client(make_client, webhook.url) + + def failing_commit(self: Session) -> None: + raise OperationalError("INSERT INTO submissions", {}, Exception("connection lost")) + + monkeypatch.setattr(Session, "commit", failing_commit) + response = client.post(ENDPOINT, data={"email": "dev@example.com"}) + monkeypatch.undo() + + assert response.status_code == 503 + assert response.json()["error"]["code"] == "storage_unavailable" + with open_session(session_client := client) as session: + assert list(session.scalars(select(models.Submission))) == [] + assert list(session.scalars(select(models.WebhookDelivery))) == [] + assert session_client is client + + +def test_concurrent_identical_submissions_queue_one_delivery( + make_client: ClientFactory, webhook: WebhookRecorder, tmp_path: Path +) -> None: + """ + the losers of the idempotency race must not each add their own delivery + :param make_client: factory for clients bound to a configured app + :param webhook: the local server recording deliveries + :param tmp_path: pytest-provided directory to hold a file-backed database + """ + # A file-backed database, because the in-memory one is pinned to a single + # connection and cannot express two writers racing. + client = make_client( + seed_endpoint=False, + allow_private_webhook_targets=True, + database_url=f"sqlite:///{tmp_path.as_posix()}/forms.db", + ) + create_endpoint(client, webhook_url=webhook.url) + headers = {"Idempotency-Key": KEY} + + barrier = threading.Barrier(6) + + def send() -> Any: + barrier.wait() + return client.post(ENDPOINT, data={"email": "dev@example.com"}, headers=headers) + + with ThreadPoolExecutor(max_workers=6) as pool: + responses = [future.result() for future in [pool.submit(send) for _ in range(6)]] + + assert [response.status_code for response in responses] == [202] * 6 + assert len({response.json()["submission_id"] for response in responses}) == 1 + with open_session(client) as session: + assert len(list(session.scalars(select(models.Submission)))) == 1 + assert len(list(session.scalars(select(models.WebhookDelivery)))) == 1 + + +# --- idempotency interaction ------------------------------------------------- + + +def test_a_replay_queues_no_second_delivery( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + """ + a client retrying a lost response must not create a second downstream obligation + :param make_client: factory for clients bound to a configured app + :param webhook: the local server recording deliveries + """ + client, _ = hooked_client(make_client, webhook.url) + headers = {"Idempotency-Key": KEY} + + first = client.post(ENDPOINT, data={"email": "dev@example.com"}, headers=headers) + second = client.post(ENDPOINT, data={"email": "dev@example.com"}, headers=headers) + + assert first.status_code == second.status_code == 202 + assert second.json()["idempotent_replay"] is True + assert second.json()["delivery"] == {"queued": True} + assert len(deliveries(client)) == 1 + + +def test_many_replays_still_queue_one_delivery( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = hooked_client(make_client, webhook.url) + headers = {"Idempotency-Key": KEY} + + for _ in range(4): + client.post(ENDPOINT, data={"email": "dev@example.com"}, headers=headers) + + assert len(deliveries(client)) == 1 + + +def test_a_replay_does_not_disturb_the_retry_schedule( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + """ + replaying must not reset a delivery that a worker has already been working on + :param make_client: factory for clients bound to a configured app + :param webhook: the local server recording deliveries + """ + client, _ = hooked_client(make_client, webhook.url) + headers = {"Idempotency-Key": KEY} + client.post(ENDPOINT, data={"email": "dev@example.com"}, headers=headers) + before = deliveries(client)[0] + original = (before.state, before.attempts, before.next_attempt_at) + + client.post(ENDPOINT, data={"email": "dev@example.com"}, headers=headers) + + after = deliveries(client)[0] + assert (after.state, after.attempts, after.next_attempt_at) == original + + +def test_an_idempotency_conflict_queues_nothing_new( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = hooked_client(make_client, webhook.url) + headers = {"Idempotency-Key": KEY} + client.post(ENDPOINT, data={"email": "dev@example.com"}, headers=headers) + + response = client.post(ENDPOINT, data={"email": "other@example.com"}, headers=headers) + + assert response.status_code == 409 + assert len(deliveries(client)) == 1 + + +# --- the secret stays put ---------------------------------------------------- + + +def test_the_secret_never_appears_in_a_submission_response( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, endpoint = hooked_client(make_client, webhook.url) + + response = client.post(ENDPOINT, data={"email": "dev@example.com"}) + + assert endpoint["webhook_secret"] not in response.text + + +def test_the_secret_never_appears_in_an_error( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, endpoint = hooked_client(make_client, webhook.url) + secret = endpoint["webhook_secret"] + headers = {"Idempotency-Key": KEY} + client.post(ENDPOINT, data={"email": "dev@example.com"}, headers=headers) + + conflict = client.post(ENDPOINT, data={"email": "other@example.com"}, headers=headers) + not_found = client.post("/f/no-such-form", data={"email": "dev@example.com"}) + + assert secret not in conflict.text + assert secret not in not_found.text + + +def test_no_ingestion_log_output_carries_the_secret( + make_client: ClientFactory, webhook: WebhookRecorder, caplog: pytest.LogCaptureFixture +) -> None: + client, endpoint = hooked_client(make_client, webhook.url) + + with caplog.at_level(logging.DEBUG): + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + assert endpoint["webhook_secret"] not in caplog.text diff --git a/tests/test_worker.py b/tests/test_worker.py new file mode 100644 index 0000000..5976d4c --- /dev/null +++ b/tests/test_worker.py @@ -0,0 +1,629 @@ +""" +the delivery worker: claiming, sending, retrying, and giving up + +Time is supplied explicitly to every worker call, so retry schedules and lease +expiry are asserted exactly rather than waited for. Nothing here sleeps, and +nothing here leaves the loopback interface. +""" + +from __future__ import annotations + +import hashlib +import hmac +import json +from datetime import UTC, datetime, timedelta +from typing import Any + +import pytest +from fastapi.testclient import TestClient +from sqlalchemy import select +from sqlalchemy.dialects import postgresql + +from conftest import ClientFactory, app_settings, create_endpoint, open_session, work_once +from hymical_forms import models, storage +from hymical_forms.webhooks import DeliveryState, RetryPolicy +from webhook_server import WebhookRecorder, unused_local_url + +ENDPOINT = "/f/contact-form" + + +def queued_client( + make_client: ClientFactory, url: str, **overrides: Any +) -> tuple[TestClient, dict[str, Any]]: + """ + build a client whose default endpoint delivers to a given destination + :param make_client: factory for clients bound to a configured app + :param url: the webhook destination to configure + :param overrides: extra setting overrides for the application + :returns: the client and the created endpoint as the API returned it + """ + overrides.setdefault("allow_private_webhook_targets", True) + client = make_client(seed_endpoint=False, **overrides) + endpoint = create_endpoint(client, webhook_url=url) + return client, endpoint + + +def delivery_of(client: TestClient) -> models.WebhookDelivery: + """ + read the single queued delivery behind a client + :param client: the client whose application database should be inspected + :returns: the one delivery row + """ + with open_session(client) as session: + return session.scalars(select(models.WebhookDelivery)).one() + + +def due_at(client: TestClient) -> datetime: + """ + read the instant the queued delivery first becomes due + :param client: the client whose application database should be inspected + :returns: the delivery's next attempt time + """ + # Anchoring to this rather than to a fixed constant keeps every schedule + # assertion exact without any test needing to know real wall-clock time. + return delivery_of(client).next_attempt_at + + +def attempts_of(client: TestClient) -> list[models.DeliveryAttempt]: + """ + read every recorded attempt behind a client, oldest first + :param client: the client whose application database should be inspected + :returns: the attempt rows ordered by attempt number + """ + with open_session(client) as session: + return list( + session.scalars( + select(models.DeliveryAttempt).order_by(models.DeliveryAttempt.attempt_number) + ) + ) + + +# --- successful delivery ----------------------------------------------------- + + +def test_the_worker_delivers_a_queued_submission( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = queued_client(make_client, webhook.url) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + handled = work_once(client, now=now) + + assert handled == 1 + assert len(webhook.received) == 1 + delivery = delivery_of(client) + assert delivery.state == DeliveryState.DELIVERED + assert delivery.attempts == 1 + assert delivery.completed_at == now + assert delivery.claim_expires_at is None + + +def test_the_delivered_payload_matches_the_contract( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = queued_client(make_client, webhook.url) + response = client.post( + ENDPOINT, data={"email": "dev@example.com", "topics": ["billing", "api"]} + ) + + now = due_at(client) + work_once(client, now=now) + + payload = json.loads(webhook.received[0].body) + assert payload == { + "type": "submission.received", + "submission": { + "id": response.json()["submission_id"], + "endpoint_id": "contact-form", + "received_at": response.json()["received_at"], + "fields": {"email": ["dev@example.com"], "topics": ["billing", "api"]}, + }, + } + + +def test_the_signature_still_verifies_against_the_exact_bytes( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + """ + moving delivery into the worker must not change what a receiver has to do + :param make_client: factory for clients bound to a configured app + :param webhook: the local server recording deliveries + """ + client, endpoint = queued_client(make_client, webhook.url) + client.post(ENDPOINT, data={"email": "dev@example.com", "note": "héllo, wörld"}) + + now = due_at(client) + work_once(client, now=now) + + delivered = webhook.received[0] + expected = hmac.new( + endpoint["webhook_secret"].encode("utf-8"), delivered.body, hashlib.sha256 + ).hexdigest() + assert delivered.headers["hymical-signature"] == f"v1={expected}" + + +def test_a_successful_attempt_is_recorded( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = queued_client(make_client, webhook.url) + response = client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + work_once(client, now=now) + + recorded = attempts_of(client) + assert len(recorded) == 1 + attempt = recorded[0] + assert attempt.delivery_id == delivery_of(client).id + assert attempt.submission_id == response.json()["submission_id"] + assert attempt.attempt_number == 1 + assert attempt.attempted_at == now + assert attempt.outcome == "succeeded" + assert attempt.response_status == 200 + assert attempt.error is None + + +def test_a_delivered_job_is_not_picked_up_again( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = queued_client(make_client, webhook.url) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + now = due_at(client) + work_once(client, now=now) + + handled = work_once(client, now=now + timedelta(hours=1)) + + assert handled == 0 + assert len(webhook.received) == 1 + + +def test_the_worker_has_nothing_to_do_without_a_webhook(client: TestClient) -> None: + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + assert work_once(client) == 0 + assert attempts_of(client) == [] + + +def test_the_worker_handles_a_whole_batch( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = queued_client(make_client, webhook.url) + for index in range(3): + client.post(ENDPOINT, data={"email": f"dev{index}@example.com"}) + + handled = work_once(client) + + assert handled == 3 + assert len(webhook.received) == 3 + with open_session(client) as session: + states = {row.state for row in session.scalars(select(models.WebhookDelivery))} + assert states == {DeliveryState.DELIVERED} + + +# --- retry ------------------------------------------------------------------- + + +@pytest.mark.parametrize("status", [500, 502, 503, 408, 425, 429]) +def test_a_retryable_response_schedules_another_attempt( + make_client: ClientFactory, webhook: WebhookRecorder, status: int +) -> None: + webhook.status = status + client, _ = queued_client(make_client, webhook.url, webhook_retry_initial_seconds=10) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + work_once(client, now=now) + + delivery = delivery_of(client) + assert delivery.state == DeliveryState.PENDING + assert delivery.attempts == 1 + assert delivery.next_attempt_at == now + timedelta(seconds=10) + assert delivery.completed_at is None + + +def test_a_timeout_schedules_another_attempt( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + webhook.delay_seconds = 2.0 + client, _ = queued_client( + make_client, + webhook.url, + webhook_read_timeout_seconds=0.25, + webhook_retry_initial_seconds=10, + ) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + work_once(client, now=now) + + delivery = delivery_of(client) + assert delivery.state == DeliveryState.PENDING + assert delivery.next_attempt_at == now + timedelta(seconds=10) + assert attempts_of(client)[0].outcome == "timeout" + + +def test_a_refused_connection_schedules_another_attempt(make_client: ClientFactory) -> None: + client, _ = queued_client(make_client, unused_local_url(), webhook_retry_initial_seconds=10) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + work_once(client, now=now) + + delivery = delivery_of(client) + assert delivery.state == DeliveryState.PENDING + assert delivery.next_attempt_at == now + timedelta(seconds=10) + assert attempts_of(client)[0].outcome == "network_error" + + +def test_a_retry_is_not_made_before_it_is_due( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + webhook.status = 503 + client, _ = queued_client(make_client, webhook.url, webhook_retry_initial_seconds=10) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + now = due_at(client) + work_once(client, now=now) + + early = work_once(client, now=now + timedelta(seconds=9)) + + assert early == 0 + assert len(webhook.received) == 1 + assert delivery_of(client).attempts == 1 + + +def test_a_retry_is_made_once_due(make_client: ClientFactory, webhook: WebhookRecorder) -> None: + webhook.status = 503 + client, _ = queued_client(make_client, webhook.url, webhook_retry_initial_seconds=10) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + now = due_at(client) + work_once(client, now=now) + + handled = work_once(client, now=now + timedelta(seconds=10)) + + assert handled == 1 + assert len(webhook.received) == 2 + assert delivery_of(client).attempts == 2 + + +def test_the_backoff_doubles_and_then_caps( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + """ + every wait in the schedule is asserted, not just the first + :param make_client: factory for clients bound to a configured app + :param webhook: the local server, answering with a retryable status throughout + """ + webhook.status = 503 + client, _ = queued_client( + make_client, + webhook.url, + webhook_max_attempts=6, + webhook_retry_initial_seconds=10, + webhook_retry_max_seconds=60, + ) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + schedule = [] + now = due_at(client) + for _ in range(5): + work_once(client, now=now) + delivery = delivery_of(client) + gap = delivery.next_attempt_at - now + schedule.append(int(gap.total_seconds())) + now = delivery.next_attempt_at + + # 10, 20, 40, then held at the 60 second cap. + assert schedule == [10, 20, 40, 60, 60] + + +def test_each_attempt_keeps_its_own_record( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + webhook.status = 503 + client, _ = queued_client(make_client, webhook.url, webhook_retry_initial_seconds=10) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + for _ in range(3): + work_once(client, now=now) + now = delivery_of(client).next_attempt_at + + recorded = attempts_of(client) + assert [attempt.attempt_number for attempt in recorded] == [1, 2, 3] + assert all(attempt.outcome == "http_error" for attempt in recorded) + assert len({attempt.id for attempt in recorded}) == 3 + assert delivery_of(client).attempts == 3 + + +# --- terminal failure -------------------------------------------------------- + + +@pytest.mark.parametrize("status", [400, 401, 403, 404, 409, 410, 422, 301, 302]) +def test_a_non_retryable_response_is_final( + make_client: ClientFactory, webhook: WebhookRecorder, status: int +) -> None: + """ + repeating a request the receiver called malformed or already-seen will not repair it + :param make_client: factory for clients bound to a configured app + :param webhook: the local server recording deliveries + :param status: the status the destination answers with + """ + webhook.status = status + client, _ = queued_client(make_client, webhook.url) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + work_once(client, now=now) + + delivery = delivery_of(client) + assert delivery.state == DeliveryState.FAILED + assert delivery.completed_at == now + assert delivery.attempts == 1 + assert work_once(client, now=now + timedelta(days=1)) == 0 + + +def test_running_out_of_attempts_is_final( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + webhook.status = 503 + client, _ = queued_client( + make_client, webhook.url, webhook_max_attempts=3, webhook_retry_initial_seconds=10 + ) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + for _ in range(3): + work_once(client, now=now) + now = delivery_of(client).next_attempt_at + + delivery = delivery_of(client) + assert delivery.state == DeliveryState.FAILED + assert delivery.attempts == 3 + assert delivery.completed_at is not None + assert len(webhook.received) == 3 + assert work_once(client, now=now + timedelta(days=1)) == 0 + + +def test_the_final_attempt_stays_in_the_history( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + webhook.status = 503 + client, _ = queued_client( + make_client, webhook.url, webhook_max_attempts=2, webhook_retry_initial_seconds=10 + ) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + work_once(client, now=now) + work_once(client, now=delivery_of(client).next_attempt_at) + + recorded = attempts_of(client) + assert [attempt.attempt_number for attempt in recorded] == [1, 2] + assert recorded[-1].response_status == 503 + assert delivery_of(client).state == DeliveryState.FAILED + + +# --- claiming and lease recovery --------------------------------------------- + + +def test_claiming_marks_the_job_as_processing( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = queued_client(make_client, webhook.url, worker_lease_seconds=60) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + with open_session(client) as session: + claimed = storage.claim_due_deliveries(session, now=now, lease_seconds=60, limit=10) + + assert len(claimed) == 1 + delivery = delivery_of(client) + assert delivery.state == DeliveryState.PROCESSING + assert delivery.claim_expires_at == now + timedelta(seconds=60) + + +def test_a_claimed_job_is_not_claimable_by_another_worker( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + """ + the second worker's guarded update matches nothing, so it walks away empty + :param make_client: factory for clients bound to a configured app + :param webhook: the local server recording deliveries + """ + client, _ = queued_client(make_client, webhook.url) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + with open_session(client) as first: + claimed_by_first = storage.claim_due_deliveries(first, now=now, lease_seconds=60, limit=10) + with open_session(client) as second: + claimed_by_second = storage.claim_due_deliveries( + second, now=now, lease_seconds=60, limit=10 + ) + + assert len(claimed_by_first) == 1 + assert claimed_by_second == [] + + +def test_a_claimed_job_is_skipped_by_a_later_batch( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = queued_client(make_client, webhook.url, worker_lease_seconds=60) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + now = due_at(client) + with open_session(client) as session: + storage.claim_due_deliveries(session, now=now, lease_seconds=60, limit=10) + + handled = work_once(client, now=now + timedelta(seconds=30)) + + assert handled == 0 + assert webhook.received == [] + + +def test_an_expired_lease_becomes_claimable_again( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + """ + a worker that died holding a job must not leave it stuck in processing forever + :param make_client: factory for clients bound to a configured app + :param webhook: the local server recording deliveries + """ + client, _ = queued_client(make_client, webhook.url, worker_lease_seconds=60) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + # Claim it and then abandon it, exactly as a worker that was killed would. + now = due_at(client) + with open_session(client) as session: + storage.claim_due_deliveries(session, now=now, lease_seconds=60, limit=10) + + handled = work_once(client, now=now + timedelta(seconds=61)) + + assert handled == 1 + assert len(webhook.received) == 1 + assert delivery_of(client).state == DeliveryState.DELIVERED + + +def test_reclaiming_after_a_crash_can_deliver_twice( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + """ + at-least-once is the honest guarantee, and this is what it costs + :param make_client: factory for clients bound to a configured app + :param webhook: the local server recording deliveries + """ + # A worker that sends successfully and dies before recording it leaves a lease + # that expires, and the next worker sends the same event again. Nothing in a + # queue can prevent this without the receiver's cooperation. + client, _ = queued_client(make_client, webhook.url) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + now = due_at(client) + with open_session(client) as session: + claimed = storage.claim_due_deliveries(session, now=now, lease_seconds=60, limit=10) + assert len(claimed) == 1 + + work_once(client, now=now + timedelta(seconds=61)) + + assert len(webhook.received) == 1 + delivered = json.loads(webhook.received[0].body)["submission"]["id"] + assert delivered == delivery_of(client).submission_id + + +def test_a_batch_is_bounded_by_the_configured_size( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = queued_client(make_client, webhook.url, worker_batch_size=2) + for index in range(5): + client.post(ENDPOINT, data={"email": f"dev{index}@example.com"}) + + handled = work_once(client) + + assert handled == 2 + assert len(webhook.received) == 2 + + +def test_due_deliveries_are_taken_oldest_first( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, _ = queued_client(make_client, webhook.url, worker_batch_size=1) + first = client.post(ENDPOINT, data={"email": "one@example.com"}).json() + client.post(ENDPOINT, data={"email": "two@example.com"}) + + work_once(client) + + assert json.loads(webhook.received[0].body)["submission"]["id"] == first["submission_id"] + + +# --- the secret stays put ---------------------------------------------------- + + +def test_the_secret_never_reaches_the_destination( + make_client: ClientFactory, webhook: WebhookRecorder +) -> None: + client, endpoint = queued_client(make_client, webhook.url) + secret = endpoint["webhook_secret"] + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + work_once(client, now=now) + + delivered = webhook.received[0] + assert secret not in delivered.body.decode("utf-8") + assert all(secret not in value for value in delivered.headers.values()) + + +def test_the_secret_is_never_stored_on_an_attempt(make_client: ClientFactory) -> None: + client, endpoint = queued_client(make_client, unused_local_url()) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + work_once(client, now=now) + + attempt = attempts_of(client)[0] + assert endpoint["webhook_secret"] not in (attempt.error or "") + assert endpoint["webhook_secret"] not in attempt.destination_url + + +def test_a_stored_failure_message_is_bounded(make_client: ClientFactory) -> None: + client, _ = queued_client(make_client, unused_local_url()) + client.post(ENDPOINT, data={"email": "dev@example.com"}) + + now = due_at(client) + work_once(client, now=now) + + error = attempts_of(client)[0].error + assert error is not None + assert len(error) <= 500 + + +# --- the retry policy itself ------------------------------------------------- + + +def test_the_policy_doubles_from_the_initial_delay() -> None: + policy = RetryPolicy(max_attempts=5, initial_seconds=10, max_seconds=3600) + + delays = [int(policy.delay_after(n).total_seconds()) for n in range(1, 6)] + + assert delays == [10, 20, 40, 80, 160] + + +def test_the_policy_respects_its_cap() -> None: + policy = RetryPolicy(max_attempts=20, initial_seconds=10, max_seconds=100) + + assert int(policy.delay_after(10).total_seconds()) == 100 + + +def test_the_policy_knows_when_the_allowance_is_gone() -> None: + policy = RetryPolicy(max_attempts=3, initial_seconds=10, max_seconds=3600) + + assert not policy.is_exhausted(2) + assert policy.is_exhausted(3) + assert policy.is_exhausted(4) + + +def test_postgresql_claims_with_skip_locked() -> None: + """ + the claim relies on row locking that only PostgreSQL provides, so pin the SQL + """ + # SQLite silently drops FOR UPDATE, which is why the conditional update in + # claim_due_deliveries exists as well. This asserts the PostgreSQL half, + # which the SQLite-backed suite can never exercise at runtime. + statement = ( + select(models.WebhookDelivery) + .where(storage.due_condition(datetime.now(UTC))) + .limit(1) + .with_for_update(skip_locked=True) + ) + + # SQLAlchemy does not type its dialect factories. + compiled = str(statement.compile(dialect=postgresql.dialect())) # type: ignore[no-untyped-call] + + assert "FOR UPDATE SKIP LOCKED" in compiled + + +def test_the_configured_policy_reaches_the_worker(make_client: ClientFactory) -> None: + client = make_client(webhook_max_attempts=7, webhook_retry_initial_seconds=3) + + policy = app_settings(client).retry_policy() + + assert policy.max_attempts == 7 + assert policy.initial_seconds == 3 diff --git a/tests/webhook_server.py b/tests/webhook_server.py new file mode 100644 index 0000000..035fec4 --- /dev/null +++ b/tests/webhook_server.py @@ -0,0 +1,140 @@ +""" +a real local HTTP server that records the webhook deliveries it receives + +Tests point the service at this rather than at a mocked transport, so the bytes +asserted on are the bytes that actually crossed a socket, and so timeouts and +refused connections are the real thing rather than a simulated exception. +""" + +from __future__ import annotations + +import socket +import threading +import time +from dataclasses import dataclass, field +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from types import TracebackType + + +@dataclass +class ReceivedWebhook: + """ + one request the recorder was sent + """ + + path: str + headers: dict[str, str] + body: bytes + + +@dataclass +class WebhookRecorder: + """ + a local server that records deliveries and answers however a test wants + """ + + status: int = 200 + delay_seconds: float = 0.0 + received: list[ReceivedWebhook] = field(default_factory=list) + _server: ThreadingHTTPServer | None = None + _thread: threading.Thread | None = None + + @property + def url(self) -> str: + """ + the destination a webhook should be configured with + :returns: the absolute URL this recorder answers on + """ + assert self._server is not None, "the recorder is not running" + host, port = self._server.server_address[:2] + return f"http://{host!s}:{port}/hook" + + def start(self) -> WebhookRecorder: + """ + bind to a free port on the loopback interface and begin serving + :returns: this recorder, now running + """ + recorder = self + + class Handler(BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + + # The capitalised name is the one BaseHTTPRequestHandler dispatches to. + def do_POST(self) -> None: + length = int(self.headers.get("Content-Length", "0")) + recorder.received.append( + ReceivedWebhook( + path=self.path, + headers={key.lower(): value for key, value in self.headers.items()}, + body=self.rfile.read(length), + ) + ) + # Sleeping after reading the body means the client has finished + # sending and is waiting on a response, which is what a read + # timeout is meant to catch. + if recorder.delay_seconds: + time.sleep(recorder.delay_seconds) + self.send_response(recorder.status) + self.send_header("Content-Length", "0") + self.end_headers() + + def log_message(self, format: str, *args: object) -> None: + """ + silence the per-request logging that would otherwise flood the test output + :param format: the message template, ignored + :param args: the message arguments, ignored + """ + + self._server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + self._server.daemon_threads = True + # The default poll interval makes every shutdown wait half a second, + # which across a test module is most of the runtime. + self._thread = threading.Thread( + target=self._server.serve_forever, kwargs={"poll_interval": 0.01}, daemon=True + ) + self._thread.start() + return self + + def stop(self) -> None: + """ + stop serving and release the port + """ + if self._server is not None: + self._server.shutdown() + self._server.server_close() + if self._thread is not None: + self._thread.join(timeout=5) + + def __enter__(self) -> WebhookRecorder: + """ + start the recorder for the duration of a with block + :returns: this recorder, now running + """ + return self.start() + + def __exit__( + self, + exc_type: type[BaseException] | None, + exc: BaseException | None, + traceback: TracebackType | None, + ) -> None: + """ + stop the recorder when the with block ends + :param exc_type: type of any exception being propagated + :param exc: any exception being propagated + :param traceback: traceback of any exception being propagated + """ + self.stop() + + +def unused_local_url() -> str: + """ + build a loopback URL with nothing listening on it + :returns: a URL whose connection will be refused + """ + # Binding and closing hands back a port the OS considers free, so connecting + # to it is refused immediately rather than hanging. + with socket.socket() as probe: + probe.bind(("127.0.0.1", 0)) + port = probe.getsockname()[1] + return f"http://127.0.0.1:{port}/hook"