docs: true up api-description.yaml with the mounted routes

[dobby-coder]

Closes #196
Part of encryption4all/postguard#247 (workstream C)

The issue asks for the oasdiff gate, with a first step: check that api-description.yaml still describes the service. It did not, so this PR fixes the spec and adds a test that keeps it honest. The gate itself is workflow YAML, which the App cannot push, so it is in a comment below for a maintainer to land.

Drift found

Every row below was checked against the handler in src/main.rs, not inferred from the spec.

Route	Spec said	Service does
GET /staging/preview/{uuid}	absent	mounted on every deployment, answers when staging_mode = true, else 404
PUT /fileupload/{uuid}	409 on a cryptifytoken mismatch	400 (check_cryptify_token -> Error::BadRequest); nothing in the service returns 409
POST /fileupload/finalize/{uuid}	409 on a cryptifytoken mismatch, no cryptifytoken parameter	requires the cryptifytoken header (FinalizeHeaders) and answers 400 on a mismatch, so a client following the spec got a 400 out of the extractor
both of the above	400 body is {"message": "…"} JSON	plain-text message; extractor-level rejections get Rocket's default error page
GET /metrics	firewall-only, no auth, no 401	Authorization: Bearer <metrics_token> is required when the deployment sets one, 401 otherwise; open when it does not
GET /filedownload/{uuid}	200 or 400 or 404	200, 206 with Content-Range, 404, 416; always advertises Accept-Ranges: bytes; the documented 400 is unreachable
POST /fileupload/init	200 only	also 400 (unparsable recipient), 422 (bad JSON body), 500 (cannot create the file)
PUT /fileupload/{uuid}	no 5xx	500 (write failed), 503 (over the default tier while pg-pkg is unreachable)
POST /fileupload/finalize/{uuid}	no 500	500 (unreadable file, not a PostGuard message, no sender attribute, or the email failed)
GET /usage	200 or 401	also 503 when pg-pkg cannot validate the key
PayloadTooLarge	four fields	plus resets_at on the rolling-window 413 from finalize

Also documented, since they were missing rather than wrong: the optional Authorization header that puts an upload on the API-key tier, the X-Cryptify-Source and X-POSTGUARD-CLIENT-VERSION metrics headers, and cryptify_uploads_by_app_total in the /metrics list.

Keeping it true

api_routes() is now the single mount list build_rocket uses, and mod api_description_tests compares it against the spec in both directions. Adding, removing, or renaming a route fails cargo test until the spec follows. On the pre-fix spec the test fails with:

GET /staging/preview/<uuid> is mounted but missing from api-description.yaml

The comparison ignores placeholder names, so the route's <filename> binding and the spec's {uuid} still match. The wire contract is the same either way, and {uuid} is the better name for a segment that is always an upload UUID. Response codes and schemas are not covered; those were checked by hand this round.

The spec is parsed by scanning indentation rather than with a YAML crate, to leave the dependency tree alone. spec_layout_assumption_holds fails loudly if the file's layout changes underneath it.

One thing left to decide

GET /filedownload/{uuid} sets no Content-Type at all (verified: res.content_type() is None). The spec claimed application/cryptify+octet-stream. I documented application/octet-stream and noted the missing header rather than changing the handler, since that is a behaviour change and not what this issue asked for. If the sealed-blob media type was intentional, the handler is the side that should move. Say so and I will open a separate PR.

Verification

cargo fmt --all -- --check, cargo clippy --all-targets -- -D warnings, cargo test --all-targets (154 tests, up from 151) all pass locally. The spec validates as OpenAPI 3.0.3 through @apidevtools/swagger-parser, with all $refs resolving and every tag declared.

The gate config in the comment below was run against oasdiff v1.26.1 locally:

• identical specs -> No changes detected, exit 0
• this PR's spec vs main -> 4 errors, exit 1 (see the comment for why, and for the landing order)
• /usage deleted from the spec -> api-path-removed-without-deprecation, exit 1
• notifyRecipients moved into required -> request-property-became-required-with-default, exit 1
• a new optional request property -> no breaking changes, exit 0

[dobby-coder]

The oasdiff gate (needs a maintainer to land)

dobby-coder[bot] has no workflows permission, so this file is not in the diff. Save it as .github/workflows/api-diff.yml on this branch or on main:

name: API diff
#
# Breaking-change gate on the OpenAPI contract (#196).
#
# Cryptify's routes are unversioned, so there is no way to keep an old shape
# running next to a new one: any breaking change to api-description.yaml
# breaks the clients pinned to it (pg-js, pg-dotnet, the add-ins). This job
# diffs the PR's spec against the branch it targets and fails on anything
# oasdiff rates ERR. Additive changes pass.
#
# To land a genuinely breaking change: add a new versioned route, leave the
# old one in place, and deprecate it once privacybydesign/postguard-ops#64
# telemetry shows nobody is calling it.
#

on:
  pull_request:

permissions:
  contents: read

jobs:

  breaking-changes:
    name: oasdiff breaking changes
    runs-on: ubuntu-latest
    steps:
      - name: Check out the pull request
        uses: actions/checkout@v6
      - name: Check out the base spec
        uses: actions/checkout@v6
        with:
          ref: ${{ github.event.pull_request.base.sha }}
          path: base
      - name: Diff the spec against the base branch
        uses: oasdiff/oasdiff-action/[email protected]
        with:
          base: base/api-description.yaml
          revision: api-description.yaml
          fail-on: ERR
          # Do not upload the spec to oasdiff.com for a side-by-side review
          # page. The default is `true`; detection and annotations work
          # without it, so nothing leaves CI.
          review: false

Notes on the choices, in case you want them different:

• review: false matters. The action defaults to review: true, which uploads both specs to oasdiff.com (encrypted client-side, but still an upload). With it off, the diff runs entirely in the job.
• No paths: filter, so the job runs on every PR and can be made a required check. A path-filtered required check never reports on PRs that miss the filter and leaves them stuck pending. The cost is one short job per PR.
• The base spec comes from pull_request.base.sha, the commit the PR actually targets, rather than the branch tip, so a merge into main mid-review does not change the verdict.
• Path parameter names are ignored by default (include-path-params is false), which is why {uuid} versus the route's <filename> binding is not a diff.
• fail-on: ERR lets WARN-level findings through, deprecation-window checks among them. Set it to WARN if you want those blocking too.
• [email protected] runs tufin/oasdiff:v1.26.1, which is the version I ran the checks below with, so the numbers should match what CI reports.

Landing order

The gate should land after this PR merges, not on this branch.

The spec on main is wrong today, so truing it up is itself a breaking change on paper. Run against main, oasdiff v1.26.1 reports 4 errors and exits 1:

error [response-media-type-name-generalized]  GET /filedownload/{uuid}
      media type `application/cryptify+octet-stream` was changed to a more
      general media type `application/octet-stream` for the response status `200`
error [new-required-request-parameter]        POST /fileupload/finalize/{uuid}
      added the new required `header` request parameter `cryptifytoken`
error [response-media-type-removed]           POST /fileupload/finalize/{uuid}
      removed the media type `application/json` for the response with the status `400`
error [response-media-type-removed]           PUT /fileupload/{uuid}
      removed the media type `application/json` for the response with the status `400`

None of these are changes to the service. The cryptifytoken header has always been required by FinalizeHeaders, the 400 bodies have always been plain text, and the handler has never set a Content-Type. The spec is catching up to code that did not move.

So: merge this PR, then add the workflow on main. From that point the base spec is accurate and the gate is quiet until something real changes. If you would rather have the gate visible on this PR, add it here and I will point err-ignore at a one-line file listing those four checks, then remove the file in a follow-up.

What it does on real changes

Checked locally with oasdiff v1.26.1 against the spec in this PR:

Change	Result
no change	No changes detected, exit 0
new optional property on the init request body	no breaking changes, exit 0
/usage removed	api-path-removed-without-deprecation, exit 1
notifyRecipients moved into required	request-property-became-required-with-default, exit 1

[dobby-coder]

VERDICT: approve

Rules check + review sign-off. Non-blocking: one suggestion left inline, everything else is either fixed on the branch or verified as-is.

Rule check

Checked the 14 rules that plausibly apply to this diff (OpenAPI spec, Rust, PR hygiene, comment-delivered workflow YAML). One finding, now fixed; the rest pass:

• oasdiff-action-uploads-specs-by-default — passes. The comment's YAML sets review: false, pins oasdiff/oasdiff-action/[email protected] (not @main), and leaves allow-external-refs at its false default. The landing-order note is also right, and matches the rule's gate-vs-truth sequencing: land the truing-up PR first, add the gate on main afterwards, or the gate's debut run fails on the very commit that made the spec honest.
• actions-default-shell-has-no-pipefail — not applicable. Reviewed the comment-delivered YAML as in-diff code (the bot cannot push .github/workflows/, so a defect there lands via a maintainer's commit and no diff review catches it). Every step is a uses:; there is no run: block and no pipe, so there is nothing for the missing pipefail to swallow. Flagging it here rather than inline because GitHub cannot anchor a comment to a path outside the diff.
• cryptify-content-range-end-exclusive — was breached, now fixed. Detail below.
• Passing: conventional-commit-pr-titles (pr-title.yml exists, check green), pr-close-issue-keywords (Closes #196, own line, keyword adjacent, targets main), cross-repo-link-format (both cross-repo refs fully qualified as owner/repo#N), github-app-permissions / workflows-permission-recurring-blocker (workflow correctly delivered in a comment), rust-run-cargo-fmt-before-push (fmt + clippy + test all green), tests-required-on-fixes (3 committed tests, and the PR shows the failure message on the pre-fix spec), no-private-emails-in-public-artifacts (only [email protected]), no-justification-paragraphs-for-simple-changes (not a small self-evident change — the drift table is the substance), default-prs-to-ready / flip-drafts-after-ci (flipping to ready now).

Fixed on the branch (2190e96)

Four review nits plus the rule finding were one-line-ish documentation corrections, so I pushed them rather than bouncing the PR. Each was verified against the handler, not inferred from the spec:

• Content-Range end byte is exclusive, and the spec has never said so. upload_chunk rejects start >= end and takes the chunk length to be end - start (src/main.rs:670, :676, :709), so bytes 200-1000/* is 800 bytes, not the 801 RFC 7233 would mean. A client written from the spec sends one byte too many and gets a 400. This is the one semantic that has already cost real time — an audit of postguard-js nearly filed storeChunk's bytes <off>-<off+len>/* as an off-by-one before checking the server. Documented on the parameter and in CLAUDE.md. Pre-existing text, not introduced here, but this PR is the spec-truing-up PR and the spec is about to become an oasdiff-gated contract.
• Accept-Ranges: bytes is not "always" advertised. It is set on the response builder at src/main.rs:1401, after both 404 early returns, so the 404 does not carry it. Reworded to "every response that reaches the file (200, 206 and 416, but not the 404)", and added the header to the 416, which does carry it but only declared Content-Range.
• GET /filedownload/{uuid} can answer 500. A failed file.seek on the range path maps to InternalServerError. Added — the PR already documented the equally hard-to-reach 500s on init, chunk and finalize, so omitting this one read as an oversight, and an undeclared status is exactly the gap an oasdiff gate never catches later.
• The init 400 has a second cause. A syntactically invalid JSON body is a 400 with Rocket's default error page (rocket 0.5.1 maps serde Category::Syntax to 400 and only Category::Data to 422), so the 422 covers well-formed-but-wrong-shape and syntax errors land on the 400. Added the same caveat the chunk and finalize 400s already carry.
• StagingPreview.confirmation was missing its dominant null case. render_confirmation_email returns Ok(None) whenever state.sender is None (src/email.rs:334), and sender is only populated at finalize (src/main.rs:1007). So previewing before finalize — the normal staging flow, since preview exists to inspect the mail before sending — yields confirmation: null with confirm: true and no rendering failure, which the old wording excluded.

cargo fmt --all -- --check, cargo clippy --all-targets -- -D warnings, cargo test --all-targets (154 passed) all green, and the spec still validates as OpenAPI 3.0.3 through @apidevtools/swagger-parser with all $refs resolving. CI is green on 2190e96.

Left for you

One inline suggestion on the Authorization header parameter, not applied because it is a judgment call rather than a factual error: OpenAPI 3.0.3 says a header parameter named Authorization SHALL be ignored, so security: is the conformant spelling — but /usage already documents it the parameter way, so switching only /fileupload/init trades one inconsistency for another. Your call which way both should go.

Also still open, from the PR body: GET /filedownload/{uuid} sets no Content-Type at all. The spec now says application/octet-stream. If the old application/cryptify+octet-stream was intentional, the handler is the side that should move — say so and that is a separate PR.

[dobby-coder]

Non-blocking, and the only review finding I did not apply — it is a consistency call rather than a factual error.

OpenAPI 3.0.3's Parameter Object says a header parameter named Accept, Content-Type or Authorization SHALL be ignored, so this block is the one place in the diff where the newly added documentation may not survive tooling. The conformant spelling is a security requirement, and apiKeyBearer is already declared in securitySchemes for exactly this credential:

        security:
        - apiKeyBearer: []
        - {}

with the tier-degradation prose (validated → API-key tier, absent or invalid → default tier, never rejected here) moved into the operation description. That is the same shape this diff already uses correctly for /metrics 20 lines up.

The reason I left it: /usage documents the same credential as a header parameters entry too, so changing only /fileupload/init swaps one inconsistency for another. Either move both to security: or leave both as-is — worth deciding before the oasdiff gate lands and makes the shape harder to change, since dropping a documented parameter is the kind of edit the gate will have an opinion about.

[rubenhensen]

CI note: the red "oasdiff breaking changes" run you may have seen was the gate flagging this PR's own spec corrections — documenting the always-required cryptifytoken header, the plain-text 400s, and the real download media type reads as breaking to a spec-to-spec diff, even though service behavior is unchanged. My mistake stacking the gate onto the correction PR.

Fixed by splitting: the workflow moved to #201, stacked on this branch (auto-retargets to main when this merges), so this PR is back to spec+test only and the gate arms against a corrected base. Merge order: this one first, then #201.