From d8d02ab577b09de795689a5be3ca6f8b3c142923 Mon Sep 17 00:00:00 2001 From: "dobby-yivi-agent[bot]" <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Date: Mon, 27 Jul 2026 13:30:35 +0000 Subject: [PATCH 1/2] docs: true up api-description.yaml with the mounted routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The spec had drifted from the service in ways a client reading it would hit: - `/staging/preview/{uuid}` was mounted but undocumented. - `PUT /fileupload/{uuid}` and `POST /fileupload/finalize/{uuid}` documented a 409 for a `cryptifytoken` mismatch; both return 400, and no route in the service returns 409. - Finalize validates a `cryptifytoken` header, which the spec did not list, so a client following the spec got a 400 from the extractor. - 400 bodies are plain-text messages, not `{"message": …}` JSON. - `/metrics` requires `Authorization: Bearer ` when the deployment configures one; the spec said firewall-only and listed no 401. - `/filedownload/{uuid}` answers `Range` requests with 206 or 416 and never returns the documented 400, and it sets no `Content-Type` at all. - Missing responses: 400/422/500 on init, 500/503 on chunk PUT, 500 on finalize, 503 on `/usage`. - `PayloadTooLarge` gained `resets_at` on the rolling-window 413. `api_routes()` is now the single mount list, and `mod api_description_tests` compares it against the spec so a route can no longer be added, removed, or renamed without the spec following. Refs encryption4all/postguard#247 --- CLAUDE.md | 12 ++ api-description.yaml | 334 +++++++++++++++++++++++++++++++++++++------ src/main.rs | 153 ++++++++++++++++++-- 3 files changed, 437 insertions(+), 62 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c1e4fef..1c3012c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -98,6 +98,18 @@ Release-plz automation. rolling window). `usage_db` unset means in-memory only (old behaviour). A configured-but-unopenable DB panics at startup. +## api-description.yaml is tied to the mounted routes by a test +`api_routes()` in `src/main.rs` is the single mount list; `build_rocket` mounts it +and `mod api_description_tests` compares it against `api-description.yaml`. A new, +removed, or renamed route fails `cargo test` until the spec is updated too. The +test only checks method + path shape (placeholder *names* are ignored, so the +route's `` binding and the spec's `{uuid}` compare equal) — response +codes and schemas are still on you. + +The spec is also the external contract for pg-js, pg-dotnet, and the add-ins, so +a breaking edit to it needs a new versioned route rather than an in-place change +(cryptify's routes are unversioned, so there is no other escape hatch). + ## Token chain must be checked on every route touching a FileState The upload token chain (`SHA256(prev || chunk)`) must be validated on every route that operates on an existing `FileState`, not just `PUT`. An earlier version only diff --git a/api-description.yaml b/api-description.yaml index bc15503..b45dde1 100644 --- a/api-description.yaml +++ b/api-description.yaml @@ -19,6 +19,8 @@ tags: description: "Upload usage quotas" - name: "Email template" description: "Email template linked to an API key" +- name: "Staging" + description: "Endpoints that only respond on a staging deployment" paths: /health: get: @@ -41,13 +43,20 @@ paths: summary: "Prometheus text-format metrics" description: | Returns usage counters and gauges suitable for Prometheus scraping - by the Grafana instance on Scaleway. Intended to be reachable only - from the internal monitoring network; firewall or reverse-proxy - allow-list in front of Cryptify. + by the Grafana instance on Scaleway. + + Access depends on the deployment's `metrics_token` setting. With a + token configured, the request must carry + `Authorization: Bearer ` and anything else is + rejected with 401. With no token configured the endpoint is open, + and the deployment is expected to restrict it at the firewall or + reverse proxy. Exposed metrics: * `cryptify_uploads_total{channel}` — counter of finalized uploads. * `cryptify_upload_bytes_total{channel}` — counter of bytes. + * `cryptify_uploads_by_app_total{app}` — counter of finalized + uploads per client app. * `cryptify_storage_bytes` — gauge, current disk usage. * `cryptify_active_files` — gauge, current file count. * `cryptify_expired_files_total` — counter of uploads purged @@ -56,8 +65,12 @@ paths: The `channel` label is derived from the `X-Cryptify-Source` header, falling back to `Authorization`/`X-Api-Key` (→ `api`), then the `Origin` header (`website` / `staging-website`), then `User-Agent` - (`outlook` / `thunderbird`), then `unknown`. + (`outlook` / `thunderbird`), then `unknown`. The `app` label comes + from the `app` field of `X-POSTGUARD-CLIENT-VERSION`. operationId: "metrics" + security: + - metricsBearer: [] + - {} responses: "200": description: "Prometheus text exposition format" @@ -65,12 +78,45 @@ paths: text/plain: schema: type: "string" + "401": + description: + "A `metrics_token` is configured and the request did not present + it as `Authorization: Bearer `." /fileupload/init: post: tags: - "File upload" summary: "Initialize multipart file upload" operationId: "initFileUpload" + parameters: + - in: "header" + name: "Authorization" + description: + "Optional `Bearer PG-…` API key. When present and validated by + pg-pkg, the upload is accounted against that tenant and gets the + API-key limits (100 GB); without it the upload runs on the default + tier (5 GB). An invalid key is not rejected here, it degrades to + the default tier." + schema: + type: "string" + required: false + - in: "header" + name: "X-Cryptify-Source" + description: + "Optional traffic-source label for the `channel` metrics label. + Sanitized to `[a-z0-9_-]`, truncated to 32 characters." + schema: + type: "string" + required: false + - in: "header" + name: "X-POSTGUARD-CLIENT-VERSION" + description: + "Optional client identification, `host,host_version,app,app_version`. + The `app` field becomes the `app` label on + `cryptify_uploads_by_app_total`." + schema: + type: "string" + required: false requestBody: content: application/json: @@ -136,6 +182,20 @@ paths: `X-Recovery-Token` header to recover from a page refresh, tab crash, or navigate-away-and-back. Hex-encoded 32-byte random." + "400": + description: "The `recipient` address could not be parsed." + content: + text/plain: + schema: + type: "string" + "422": + description: "The request body is not valid `InitBody` JSON." + "500": + description: "The upload file could not be created on disk." + content: + text/plain: + schema: + type: "string" /fileupload/{uuid}: put: tags: @@ -195,24 +255,21 @@ paths: schema: description: "Identifies the new version of the upload file parts. Needs to be passed into the next file part upload request." type: "string" - "409": - description: "Server file parts cryptifytoken differs from cryptifytoken in request." - headers: - cryptifytoken: - required: true - schema: - description: - "Identifies the version of the upload file parts. Needs to be passed into the next file part upload request." - type: "string" "400": - description: "One of the input parameters is incorrect." + description: + "One of the input parameters is incorrect: a chunk larger than + the configured chunk size, a `Content-Range` start that does not + continue the upload, a body whose length does not match the + range, or a `cryptifytoken` that matches neither the current + token nor the previous one on the retry path. A token mismatch is + reported here, not as 409. A missing or unparsable + `cryptifytoken` / `Content-Range` header is rejected the same way + but by the header extractor, so that body is Rocket's default + error page rather than the plain-text message below." content: - application/json: - schema: - type: "object" - properties: - message: - type: "string" + text/plain: + schema: + type: "string" "404": description: "The upload session is not known to the server. Either the @@ -230,6 +287,22 @@ paths: application/json: schema: $ref: "#/components/schemas/PayloadTooLarge" + "500": + description: "The chunk could not be written to disk." + content: + text/plain: + schema: + type: "string" + "503": + description: + "The upload exceeds the default tier and pg-pkg was unreachable + for the whole retry budget, so the caller's entitlement to the + API-key tier could not be confirmed. Uploads that stay inside the + default tier are unaffected." + content: + text/plain: + schema: + type: "string" /fileupload/finalize/{uuid}: post: @@ -238,6 +311,15 @@ paths: summary: "Finalize multipart file upload and send mail to recipient" operationId: "finalizeFileUpload" parameters: + - in: "header" + name: "cryptifytoken" + description: + "The current token, as returned by the last chunk PUT. Finalize + validates it too — knowing the UUID is not enough to finalize + someone else's upload." + schema: + type: "string" + required: true - in: "header" name: "Content-Range" description: @@ -255,23 +337,17 @@ paths: responses: "200": description: "Successful operation" - "409": - description: "Server file parts cryptifytoken differs from cryptifytoken in request" - headers: - cryptifytoken: - required: true - schema: - description: "Identifies the version of the upload file parts. Needs to be passed into the next file part upload request." - type: "string" "400": - description: "One of the input parameters is incorrect." + description: + "The `cryptifytoken` header does not match the token the server + holds for this upload. A missing or unparsable `cryptifytoken` / + `Content-Range` header is rejected the same way but by the header + extractor, so that body is Rocket's default error page rather + than the plain-text message below." content: - application/json: - schema: - type: "object" - properties: - message: - type: "string" + text/plain: + schema: + type: "string" "404": description: "The upload session is not known to the server (see @@ -288,7 +364,18 @@ paths: schema: $ref: "#/components/schemas/PayloadTooLarge" "422": - description: "Data is missing to form complete file." + description: + "The `Content-Range` total does not match the number of bytes the + server has committed, so the file is incomplete." + "500": + description: + "The uploaded file could not be read back, is not a valid + PostGuard message, carries no sender email attribute, or the + notification email could not be sent." + content: + text/plain: + schema: + type: "string" /fileupload/{uuid}/status: get: @@ -430,6 +517,10 @@ paths: "No valid `Authorization: Bearer PG-…` API key was presented. Usage can only be queried by the authenticated tenant it is accounted for." + "503": + description: + "pg-pkg was unreachable while validating the API key, so the + tenant could not be established." /email-template: get: @@ -474,6 +565,12 @@ paths: tags: - "File download" summary: "Download a file" + description: + "Streams the PostGuard-sealed file. Byte ranges are supported so an + interrupted download can resume: `Accept-Ranges: bytes` is always + advertised, a single `Range` is answered with 206, and an + unsatisfiable one with 416. The server does not set a `Content-Type` + on the body." operationId: "downloadFile" parameters: - in: "path" @@ -483,25 +580,104 @@ paths: schema: type: "string" format: "uuid" + - in: "header" + name: "Range" + description: + "Optional single byte range, `bytes=-`, + `bytes=-` or `bytes=-`. Multiple ranges are not + supported and are answered with 416." + required: false + schema: + type: "string" responses: "200": - description: "Successful operation." + description: "The whole file." + headers: + Accept-Ranges: + required: true + schema: + type: "string" + enum: ["bytes"] + Content-Length: + required: true + schema: + type: "integer" + format: "int64" content: - application/cryptify+octet-stream: + application/octet-stream: schema: type: "string" format: "binary" - "400": - description: "One of the input parameters is incorrect." + "206": + description: "The requested byte range." + headers: + Accept-Ranges: + required: true + schema: + type: "string" + enum: ["bytes"] + Content-Range: + required: true + description: "`bytes -/`." + schema: + type: "string" + Content-Length: + required: true + schema: + type: "integer" + format: "int64" content: - application/json: - schema: - type: "object" - properties: - message: - type: "string" + application/octet-stream: + schema: + type: "string" + format: "binary" "404": - description: "Uploaded file does not exist." + description: + "Uploaded file does not exist, or the path segment is not a safe + filename (empty, longer than 128 characters, `.`, `..`, or + containing a separator or NUL)." + "416": + description: "The `Range` header could not be satisfied." + headers: + Content-Range: + required: true + description: "`bytes */`." + schema: + type: "string" + + /staging/preview/{uuid}: + get: + tags: + - "Staging" + summary: "Preview the notification emails for an upload" + description: + "Returns the notification emails cryptify would send for an upload, + rendered but not delivered, so developers on the staging website can + inspect the message without an SMTP transport. The route is mounted on + every deployment but only answers when `staging_mode = true` is + configured; everywhere else it returns 404. Recipients that fail to + render are logged and left out of the response rather than failing the + request." + operationId: "stagingPreview" + parameters: + - in: "path" + name: "uuid" + required: true + description: "The unique identifier received when initializing file upload." + schema: + type: "string" + format: "uuid" + responses: + "200": + description: "The rendered emails for this upload." + content: + application/json: + schema: + $ref: "#/components/schemas/StagingPreview" + "404": + description: + "`staging_mode` is off, or the upload session is not known to the + server." components: securitySchemes: @@ -511,6 +687,14 @@ components: description: "PostGuard API key, sent as `Authorization: Bearer PG-…`. Validated against pg-pkg's `/v2/api-key/validate` endpoint." + metricsBearer: + type: "http" + scheme: "bearer" + description: + "The deployment's `metrics_token`, sent as `Authorization: Bearer + ` and compared in constant time. Unrelated to the + PostGuard API key. Only required when the deployment configures a + token." schemas: PayloadTooLarge: type: "object" @@ -540,6 +724,15 @@ components: type: "integer" format: "int64" description: "The limit value in bytes." + resets_at: + type: "string" + format: "date-time" + description: + "RFC-3339 timestamp at which the oldest recorded upload falls out + of the rolling window, partially freeing quota. Set on the + `rolling_window` 413 from finalize; omitted on the `per_upload` + 413 from a chunk PUT, and when the sender has no recorded + uploads." UploadSessionNotFound: type: "object" required: @@ -603,3 +796,50 @@ components: "Byte offset where the most recently committed chunk started (i.e. `uploaded - chunk_len`). Omitted until at least one chunk has been committed." + RenderedEmail: + type: "object" + required: + - recipient + - subject + - from + - html + - text + properties: + recipient: + type: "string" + description: + "The address this rendering targets: the recipient for a + notification, the sender for the confirmation copy." + subject: + type: "string" + from: + type: "string" + description: "The configured `email_from`, as `Name `." + reply_to: + type: "string" + nullable: true + description: + "The sender's address on a recipient notification; null on the + sender's own confirmation copy." + html: + type: "string" + text: + type: "string" + StagingPreview: + type: "object" + required: + - recipients + - confirmation + properties: + recipients: + type: "array" + description: "One rendering per recipient of the upload." + items: + $ref: "#/components/schemas/RenderedEmail" + confirmation: + allOf: + - $ref: "#/components/schemas/RenderedEmail" + nullable: true + description: + "The sender's confirmation copy, or null when the upload was + initialized with `confirm: false` or the rendering failed." diff --git a/src/main.rs b/src/main.rs index 81fa501..3310d4f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1477,6 +1477,24 @@ fn build_cors(allowed_origins: AllowedOrigins) -> rocket_cors::Cors { .expect("unable to configure CORS") } +/// Every route the service mounts. Single source of truth so the +/// `api-description.yaml` drift test can compare the spec against the routes +/// production actually serves instead of a hand-copied list. +fn api_routes() -> Vec { + routes![ + health, + metrics_endpoint, + upload_init, + upload_chunk, + upload_finalize, + upload_status, + usage, + email_template, + download, + staging_preview + ] +} + /// Build a Rocket instance from a pre-loaded config figment and verifying key. /// /// Extracted so integration tests can inject their own figment (temp data_dir, @@ -1513,21 +1531,7 @@ pub fn build_rocket(figment: Figment, vk: Parameters) -> Rocket()) .manage(Store::with_idle_ttl( std::time::Duration::from_secs(config.session_ttl_secs()), @@ -3730,3 +3734,122 @@ mod email_template_tests { assert!(vk.is_none(), "fetch must give up once the budget is spent"); } } + +/// Guards `api-description.yaml` against the routes the service actually +/// mounts. The spec is hand-maintained, so a new or renamed route silently +/// drifts away from it; this test fails the build instead. +#[cfg(test)] +mod api_description_tests { + use super::*; + + const SPEC: &str = include_str!("../api-description.yaml"); + + /// Operations declared in the spec, as `("GET", "/health")` pairs. + /// + /// Hand-rolled rather than parsed with a YAML crate to keep the + /// dependency tree unchanged. It relies on the file's layout: path keys + /// sit at two spaces of indentation under `paths:`, HTTP methods at four. + /// `spec_layout_assumption_holds` fails loudly if that stops being true. + fn spec_operations() -> Vec<(String, String)> { + const METHODS: [&str; 5] = ["get", "post", "put", "delete", "patch"]; + let mut ops = Vec::new(); + let mut in_paths = false; + let mut path: Option = None; + + for line in SPEC.lines() { + if line.trim().is_empty() { + continue; + } + // A top-level key ends the `paths:` block. + if !line.starts_with(' ') { + in_paths = line.starts_with("paths:"); + path = None; + continue; + } + if !in_paths { + continue; + } + let indent = line.len() - line.trim_start().len(); + let key = line.trim_end().trim_start().trim_end_matches(':'); + if indent == 2 && key.starts_with('/') { + path = Some(key.to_owned()); + } else if indent == 4 && METHODS.contains(&key) { + if let Some(path) = path.as_ref() { + ops.push((key.to_uppercase(), path.clone())); + } + } + } + ops + } + + /// Normalize a path so a Rocket route and a spec path compare equal: + /// `/fileupload//status` and `/fileupload/{uuid}/status` both become + /// `/fileupload/{}/status`. Placeholder *names* are dropped on purpose — + /// they are labels with no effect on the wire contract, and the Rocket + /// binding name (``) is not always the name that documents the + /// value best (`{uuid}`). + fn normalize_path(path: &str) -> String { + let mut out = String::with_capacity(path.len()); + let mut in_placeholder = false; + for c in path.chars() { + match c { + '<' | '{' => { + in_placeholder = true; + out.push_str("{}"); + } + '>' | '}' => in_placeholder = false, + _ if in_placeholder => {} + _ => out.push(c), + } + } + out + } + + #[test] + fn spec_layout_assumption_holds() { + assert!( + !spec_operations().is_empty(), + "no operations parsed out of api-description.yaml — the indentation \ + layout the parser assumes (paths at 2 spaces, methods at 4) changed" + ); + } + + /// Mounted routes as `("GET", "/fileupload//status")` pairs. + fn mounted_operations() -> Vec<(String, String)> { + api_routes() + .iter() + .map(|route| (route.method.to_string(), route.uri.path().to_string())) + .collect() + } + + fn matches(ops: &[(String, String)], method: &str, path: &str) -> bool { + ops.iter() + .any(|(m, p)| m == method && normalize_path(p) == normalize_path(path)) + } + + #[test] + fn every_mounted_route_is_in_the_spec() { + let spec = spec_operations(); + for (method, path) in mounted_operations() { + assert!( + matches(&spec, &method, &path), + "{} {} is mounted but missing from api-description.yaml", + method, + path + ); + } + } + + #[test] + fn every_spec_operation_is_mounted() { + let mounted = mounted_operations(); + for (method, path) in spec_operations() { + assert!( + matches(&mounted, &method, &path), + "{} {} is in api-description.yaml but no route is mounted for it", + method, + path + ); + } + } +} From 2190e96715c4ae44c7088f20e0be82756bf88ba3 Mon Sep 17 00:00:00 2001 From: "dobby-yivi-agent[bot]" <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Date: Mon, 27 Jul 2026 14:00:56 +0000 Subject: [PATCH 2/2] docs: correct four spec statements and document the exclusive Content-Range end MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up on the review of this PR. All five were verified against the handlers, not inferred from the spec: - `PUT /fileupload/{uuid}`: the `Content-Range` end byte is exclusive (`upload_chunk` uses `end - start`), which no version of the spec has ever said. An external client following RFC 7233 sends one byte too many and gets a 400. Documented on the parameter, since the spec is the contract pg-js and pg-dotnet are written against. - `GET /filedownload/{uuid}`: `Accept-Ranges: bytes` is set on the response builder after both 404 early returns, so it is not "always" advertised. Reworded, and added the header to the 416, which does carry it. - `GET /filedownload/{uuid}`: added the 500 from a failed `file.seek` on the range path. - `POST /fileupload/init`: the 400 also covers a syntactically invalid JSON body, which Rocket answers with its default error page rather than the plain-text message. Same caveat the chunk and finalize 400s already carry. - `StagingPreview.confirmation`: null whenever `state.sender` is unset, i.e. when previewing before finalize — the normal staging flow, and the case the old wording excluded. cargo fmt/clippy/test all pass (154 tests); the spec still validates as OpenAPI 3.0.3 with all $refs resolving. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 7 +++++++ api-description.yaml | 34 +++++++++++++++++++++++++++------- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1c3012c..8646dd3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -110,6 +110,13 @@ The spec is also the external contract for pg-js, pg-dotnet, and the add-ins, so a breaking edit to it needs a new versioned route rather than an in-place change (cryptify's routes are unversioned, so there is no other escape hatch). +## Content-Range end byte is EXCLUSIVE on the chunk PUT +`upload_chunk` rejects `start >= end` and takes the chunk length to be +`end - start`, so `bytes 200-1000/*` is 800 bytes at offset 200, not the 801 that +RFC 7233 would mean. `postguard-js` (`src/api/cryptify.ts:storeChunk`) sends +`bytes -/*` to match — that is correct, not an off-by-one. Do not +"fix" it in either repo alone; both sides move together or neither does. + ## Token chain must be checked on every route touching a FileState The upload token chain (`SHA256(prev || chunk)`) must be validated on every route that operates on an existing `FileState`, not just `PUT`. An earlier version only diff --git a/api-description.yaml b/api-description.yaml index b45dde1..fff6004 100644 --- a/api-description.yaml +++ b/api-description.yaml @@ -183,7 +183,12 @@ paths: page refresh, tab crash, or navigate-away-and-back. Hex-encoded 32-byte random." "400": - description: "The `recipient` address could not be parsed." + description: + "The `recipient` address could not be parsed. A body that is not + syntactically valid JSON is rejected the same way but by the body + guard, so that response is Rocket's default error page rather + than the plain-text message below; a body that parses as JSON but + does not fit `InitBody` is the 422." content: text/plain: schema: @@ -229,7 +234,13 @@ paths: - in: "header" name: "Content-Range" description: - "Which offset of a file is sent, example: `bytes 200-1000/*`." + "Which offset of a file is sent, example: `bytes 200-1000/*`.\n\n + **The end byte is EXCLUSIVE**, unlike RFC 7233. The server takes the + chunk length to be `end - start`, so `bytes 200-1000/*` carries 800 + bytes at offset 200, not 801, and a chunk of length `n` at offset + `off` is sent as `bytes -/*`. Sending the RFC's inclusive + end makes the body one byte longer than the declared range and is + rejected with 400. `start == end` is rejected too." schema: type: "string" required: true @@ -567,10 +578,10 @@ paths: summary: "Download a file" description: "Streams the PostGuard-sealed file. Byte ranges are supported so an - interrupted download can resume: `Accept-Ranges: bytes` is always - advertised, a single `Range` is answered with 206, and an - unsatisfiable one with 416. The server does not set a `Content-Type` - on the body." + interrupted download can resume: `Accept-Ranges: bytes` is advertised + on every response that reaches the file (200, 206 and 416, but not the + 404), a single `Range` is answered with 206, and an unsatisfiable one + with 416. The server does not set a `Content-Type` on the body." operationId: "downloadFile" parameters: - in: "path" @@ -639,11 +650,18 @@ paths: "416": description: "The `Range` header could not be satisfied." headers: + Accept-Ranges: + required: true + schema: + type: "string" + enum: ["bytes"] Content-Range: required: true description: "`bytes */`." schema: type: "string" + "500": + description: "Seeking to the start of the requested range failed." /staging/preview/{uuid}: get: @@ -842,4 +860,6 @@ components: nullable: true description: "The sender's confirmation copy, or null when the upload was - initialized with `confirm: false` or the rendering failed." + initialized with `confirm: false`, the sender's email is not known + yet because the upload has not been finalized (the usual case when + previewing), or the rendering failed."