test: pin consent, session-key injection, and the contact rate limit - #114
Draft
cursor[bot] wants to merge 5 commits into
Draft
test: pin consent, session-key injection, and the contact rate limit#114cursor[bot] wants to merge 5 commits into
cursor[bot] wants to merge 5 commits into
Conversation
…WT that must not carry an address The delivery confirmation route, the key-box callback, and questionnaire JWTs shipped without tests. Pin fail-closed auth, write-once pdf_delivered_at, GPC/DNT opt-out, and session tokens that verify as hashes not mailboxes. Read KEYBOX_RENDER_* and RENDER_CALLBACK_* on each call (same boot-order trap jwt.ts already documents) so the tests can set them, and so a missing .env at import cannot lock the consent path into "unavailable" forever. Co-authored-by: Ralphie B. <nyagrodha@users.noreply.github.com>
… nowhere These suites already existed or just landed; the check job never invoked them. Add --allow-write for keystore temp dirs. Leave tests/ out: it still carries the pre-existing type errors that fail before any test executes. Co-authored-by: Ralphie B. <nyagrodha@users.noreply.github.com>
…dentity These contracts lived under tests/ (which CI cannot type-check) or skipped whenever typst/qpdf were absent. Colocate them next to the production code and run them on the check job so a hostile session id, a forged X-Forwarded-For, or a guessed "yes" cannot regress silently. Co-authored-by: Ralphie B. <nyagrodha@users.noreply.github.com>
Keep production's lib/gate_encrypt_test.ts on the check job alongside the delivery, auth, and audience suites this branch already added. The two edits collided on the same deno test path list after #113 landed. Co-authored-by: rbauer <rbauer@colorado.edu>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Coverage for production paths that shipped without tests in CI, or whose tests lived under
tests/(which the check job cannot type-check) or skipped whenever typst/qpdf were absent.Risky behavior now covered
POST /api/responses/deliver): only the literal form valueyesmails a copy; anything else — absent, empty, mis-cased, or an array — is treated as no. The assembled bundle is canonical 0–34, fills gaps rather than shortening the document, and refuses duplicate or out-of-range indices. A declined copy never looks up a session.lib/session-keys.ts): a hostile session id (shell metacharacters, traversal, newlines) is refused before any transport runs — that id is interpolated into a remote shell command on the box that holds every respondent's private key. A transport failure isIdentityPushFailedwithambiguous === trueand carries no key material.shredRemoteIdentityis best-effort and uses the same allowlist. UnsetBREAKGLASS_AGE_RECIPIENTis fatal.fresh_gate_responsesINSERT still binds NULL for both plaintext answer columns.X-Forwarded-Fordoes not mint a new identity whenTRUST_PROXYis unset — the bug that made the 5/hour cap bypassable. PGP-marked plaintext is rejected. UnsetCONTACT_AGE_RECIPIENTis 503, and the response never echoes the message.[](gate default) and must not consult break-glass; a keyed session pairs the session key with break-glass and fails closed if that env var is missing.session_idor email in the query string is not a resume token.AGE_RECIPIENThas no baked-in fallback.RESUME_TOKEN_SECRETthrows; rotating the secret unlinks sessions.Test files added/updated
routes/api/responses/deliver_test.ts(new)lib/session-keys_test.ts(new)lib/age-encrypt_test.ts(new)lib/crypto_resume_test.ts(new)routes/api/questions/answer_test.ts(new)routes/api/questions/next_test.ts(new)routes/api/contact_test.ts(new)routes/api/newsletter/subscribe_test.ts(new)routes/api/health_test.ts(new)romania/tests/protect_guard_test.ts(new)routes/api/gate-submit_test.ts— retargeted at the current provision-first orderingroutes/api/auth/magic-link_test.ts— assertion now matches the handler (Valid email required).github/workflows/ci.yml— these suites, plus existinglib/audience_test.ts,lib/client-ip_test.ts,lib/qr-scans_test.ts,lib/email_test.ts,romania/tests/, privacy, and WillyStCo-op tests, now run on the check job (--allow-write --allow-run)No production behavior changes.
Why this reduces regression risk
The session-id allowlist is the difference between a filename and remote code execution on the key box; it was only tested in a directory CI cannot run. The contact form's rate limit was bypassable by setting
X-Forwarded-For— that exact forgery is now a test, against the handler, not a copy ofgetClientIp. Consent fail-closed is the last check before mailing someone's questionnaire. PDF newline rejection is an argument-injection guard that previously skipped green in CI. Gate-submit now provisions keys before answers leave; the old suite still assumed the reverse and would have gone red for the wrong reason, or not run at all.Hermetic: no Postgres, no SMTP (email_test aims at a closed loopback port), no live key box.
fetchis stubbed. 223 passed locally on the expanded CI command (13 romania PDF tests ignored where typst/qpdf are absent — the password-injection guards now run anyway).