[ToddHebebrand]

Summary

• The signature certificate's "How" line for a token-link acceptance now reads "Electronically accepted via signing link". The old wording was "via emailed link", but the link may have been copied and shared another way (the neutral wording Todd chose on [Agreements] Signature certificate says "via emailed link" even when the tech copied the link #10513).
• The certificate wording is now versioned, so signed evidence never changes after the fact. The certificate is derived on demand. Changing the string in place would have rewritten the certificate of every agreement already signed. The new column contract_documents.certificate_version is stamped at signing, and the append-only trigger then freezes it. NULL = v1: every agreement signed before this ships keeps printing "via emailed link". New signatures get v2.
• Covers both sources: request-sourced agreements (agreement_requests.public_response_jti) and quote-sourced ones (quote_acceptances.acceptance_token_jti).

Linked Issues

Closes #10513

Design

• services/agreementCertificateVersion.ts defines CertificateVersion = 1 | 2, CURRENT_CERTIFICATE_VERSION = 2, and certificateVersionFromColumn (NULL → 1, 2 → 2; anything else throws).
  • An unknown stored version fails loudly rather than printing another version's wording on signed evidence. This can only happen if a newer build stamps a row and the deploy is then rolled back.
• freezeSignedAgreement (the single production insert site) writes certificateVersion: CURRENT_CERTIFICATE_VERSION explicitly, on both sources and every path (authored, uploaded template, external upload).
• Renderer: howLines takes the version. Only the token-channel string is keyed by it (SIGNING_LINK_VIA). A test pins that v1 and v2 differ in that one line only. Portal, unknown, on-behalf, upload and unavailable certificates come out identical under both versions.
• The internal channel value is renamed from emailed_link to signing_link, because the jti records that the token link was used, not how it was delivered.
• Migration 2027-01-15-110000-contract-documents-certificate-version.sql:
  • ADD COLUMN IF NOT EXISTS certificate_version smallint with no default, so existing rows stay NULL = v1. ADD COLUMN … DEFAULT 2 would have stamped them v2.
  • A separate SET DEFAULT 2 affects future inserts only. It covers an instance still on older code that inserts during the rolling deploy.
  • contract_documents_certificate_version_chk (NULL OR >= 2) keeps NULL the only spelling of v1.
  • No backfill and no change to the append-only trigger. The migration writes no rows.
• Export policy: certificate_version is included.
• This adds a column to a per-org transactional table, not a config or policy table, so the partner-wide-first rule does not apply.

Overlap with #10654: merge order

Merge #10654 first, then this PR.

• fix(agreements): customers see the template name, never the internal label; Send dialog expiry no longer jumps (#10294) #10654 adds 2027-01-14-120000-contract-documents-template-name-snapshot.sql. This PR's migration (2027-01-15-110000-…) sorts after it and after main's newest (2027-01-15-100000-…).
• If this PR merged first, fix(agreements): customers see the template name, never the internal label; Send dialog expiry no longer jumps (#10294) #10654's migration would sort before an already-merged one, and check-migration-naming would reject it. I saw that exact violation in a local compose check.
• git merge-tree --write-tree HEAD origin/fix/10294-agreement-portal-title shows one textual conflict: the single-line contract_documents entry in tenantExportPolicyRegistry.ts. Both PRs append a column to the same included array, so the resolution is the union (…"acceptance_render_locale","template_name_snapshot","certificate_version"]).
• Everything else auto-merges: the schema, agreementCertificate.ts, contractDocumentService.ts and both test files.
• The two designs compose. fix(agreements): customers see the template name, never the internal label; Send dialog expiry no longer jumps (#10294) #10654's backfill disables the trigger only for its own UPDATE of template_name_snapshot. This PR adds a column at INSERT time and touches neither the trigger nor existing rows.
• Composed-tree check: merging fix(agreements): customers see the template name, never the internal label; Send dialog expiry no longer jumps (#10294) #10654 locally with that resolution gives 180/180 unit tests green (certificate, contractDocumentService, both export-policy pins, signed-copy email, portal signed agreements, public DTO, agreementsPublic). On a fresh DB the composed integration run passed 61/62.
  • The one failure is portalAgreements.integration.test.ts:185, which expects the portal list name to be the request label "BAA 2027". fix(agreements): customers see the template name, never the internal label; Send dialog expiry no longer jumps (#10294) #10654 deliberately changes that to the template name and does not touch that file.
  • So the failure looks like a fix(agreements): customers see the template name, never the internal label; Send dialog expiry no longer jumps (#10294) #10654 gap, not an interaction with this PR (inferred: I could not get a clean standalone run of fix(agreements): customers see the template name, never the internal label; Send dialog expiry no longer jumps (#10294) #10654's branch against the composed DB).

Merge Danger

Door: one-way, in a narrow sense. The new column is additive and nullable, and a revert of the code alone is safe because old code ignores it. But each certificate rendered for a v2 row says "via signing link" from then on. That is the intended frozen evidence. Rolling back after v2 rows exist would make old code print "via emailed link" for them until this PR is redeployed.

Blast radius: every signed agreement's certified copy (signature certificate page), both the portal/app download and the signed-copy email.

Type of Change

• Bug fix

Testing

• Added new tests
• Red before green. I wrote the tests first. Against the unchanged code, agreementCertificate.test.ts + contractDocumentService.test.ts + the new export-policy pin gave 14 failed / 86 passed. After the change: 100/100. Red tests included:
  • v1 token channel keeps "via emailed link";
  • v2 prints "via signing link";
  • loader NULL → v1 / 2 → v2 for both request- and quote-sourced rows;
  • an unknown version throws;
  • freezeSignedAgreement stamps 2 on both sources;
  • certificate_version is included in the export policy.
• New integration suite contractDocumentsCertificateVersion.integration.test.ts (real Postgres, through the real loader), 5/5:
  • request and quote sources each render "emailed link" for a NULL row and "signing link" for a v2 row;
  • an insert that omits the column defaults to 2;
  • the CHECK rejects 1 and 0 (23514);
  • breeze_app UPDATE is 42501 and owner UPDATE is 55000, and the value stays NULL.
• Mutation control: with certificateVersionFromColumn forced to return 2 for NULL, that suite goes 3 failed / 2 passed.
• Related integration suites, single-worker, 101/101:
  • portalAgreements (updated to assert v2 "signing link"), contractDocumentsAppendOnly, contractDocumentsEvidenceBackfill, contractDocumentsSourceCheck, agreementRequestAccept, agreementRequestRecordSigned;
  • tenant-export-policy, tenantExportErasureRoundtrip, tenantCascadeErasureBreadth.
• Affected unit suites: 425/425 (certificate consumers, routes, export registry, agreementRequest*). Migration and schema unit suites: 3045/3045.
• NODE_OPTIONS=--max-old-space-size=12288 npx tsc --build tsconfig.tests.json (apps/api) exit 0. pnpm db:check-drift: no drift. check-migration-naming --against-ref origin/main: OK.

Checklist

• My code follows the project's code style
• I have reviewed my own changes
• No secrets, credentials, or personal data included

🤖 Generated with Claude Code

[strix-security]

Strix Security Review

Warning

This pull request has 274 commits after the last Strix review (d7d0c33). Strix has not reviewed these changes.
Automatic review on push is off for this repository. To review the latest changes, tag @strix-security in a comment, or turn on re-review on push.

✅ No open security findings.

PR summary: Changes the signature certificate's token-link wording to "Electronically accepted via signing link" and makes the wording versioned: a new `contract_documents.certificate_version` column is stamped at signing (NULL = v1, new signatures = v2) so certificates of agreements already signed keep rendering the old text. Adds the migration, a version module, the renderer/loader plumbing, the insert-site stamp, and the export-policy entry.

Review summary

This PR changes the wording of the signature certificate's "How" line for token-link acceptances and, to avoid rewriting already-signed evidence, adds a certificate_version column to contract_documents that is stamped at signing and frozen by the existing append-only guard.

I reviewed every changed file, tracing the new column from the migration and schema through the single insert site, the derived-certificate loader/renderer, the export policy, and all certificate download and signed-copy email call sites.

No security vulnerability was confirmed. Key points verified: breeze_app holds only column-level UPDATE grants on contract_id/org_id while INSERT is table-level, so the new column is append-only without extra grants; freezeSignedAgreement is the only production insert and writes a compile-time constant, so no request body, portal or public-link input can influence the version (no mass assignment); the certificate loader's document scoping is unchanged and its system-scope read is still preceded by the caller's own org/RLS check; the emailed_link → signing_link rename leaves no consumer keying on the old literal and the channel never leaves the certificate module; and certificate_version is correctly classified in the fail-closed tenant export policy, riding the existing per-org row scope.

One candidate — the unbounded CHECK (certificate_version IS NULL OR certificate_version >= 2) leaving a value like 3 permanently unrenderable — was investigated and rejected: the value can only be written by a database owner or by a future build followed by a rollback, access that already permits disabling the append-only trigger or deleting the evidence row outright, and the fail-loud behaviour is deliberate and documented. The remaining suggestion is operational only (a deploy/rollback note for a future version bump), not a security fix.

Ruled out (1)

• Can an out-of-range `certificate_version` permanently break a signed agreement's certified copy?
  The value is written only from the compile-time constant in `freezeSignedAgreement`, so setting anything else requires database-owner SQL or a future build plus rollback — access that already allows disabling the append-only trigger or deleting the row — and the fail-loud throw is a deliberate, doc…

_{Review 1 on this PR · d7d0c33 · requested Oct 29 17:39 UTC when the PR was opened}

---

Reviewed by Strix
_{Re-run review · Configure security review settings}

[cloudflare-workers-and-pages]

Deploying breeze with    Cloudflare Pages

Latest commit:	f5bc794
Status:	✅  Deploy successful!
Preview URL:	https://614da1a7.breeze-9te.pages.dev
Branch Preview URL:	https://fix-10513-certificate-signin.breeze-9te.pages.dev

View logs

[ToddHebebrand]

Review run: one independent round, two Sonnet reviewers: pr-review-toolkit:code-reviewer (correctness, migration rules, the guarantee that old rows never change) and pr-review-toolkit:pr-test-analyzer (whether the tests discriminate).

Findings: 1 critical, raised by the test analyzer and addressed in 38d037d:

• Gap: no test proved that rows existing at migration time stay NULL. A migration written as ADD COLUMN … DEFAULT 2 would have passed every test.
• Fix: an integration test now asserts pg_attribute.atthasmissing = false for contract_documents.certificate_version.
• Positive control: the same test first adds DEFAULT 2 on a probe table and confirms the probe sees the fast default.

The code reviewer found 0 significant issues. I addressed two of its low notes in 38d037d:

• the migration header now states what a code rollback does (it re-labels v2 documents as "emailed link");
• agreementCertificateVersion.ts now states that every insert path must stamp the version explicitly.

Left as is: a captureException for the unknown-version 500. It is optional, and the throw is deliberate.

0 findings outstanding.

Tests:

• Unit: 125/125 (agreementCertificate, contractDocumentService, export-policy pin, migrationRlsScope).
• New integration suite: 6/6, plus 101/101 related integration suites, run before the review fix.
• Red before green: 14 failed against the unchanged code.
• tsc --build tsconfig.tests.json: exit 0. db:check-drift: clean. Migration naming: OK. Test stack torn down.

Merge order: #10654 first, then rebase this PR. The rebase has one one-line conflict in tenantExportPolicyRegistry.ts; resolve it with the union of both column lists.

Status: review-clean, awaiting maintainer merge.

[ToddHebebrand]

Main merged; unshipped migration moved to 2027-01-18-162000-contract-documents-certificate-version.sql so it sorts after main's newest (83272d8).

[ToddHebebrand]

Main merged; unshipped migration moved to 2027-01-18-182000-contract-documents-certificate-version.sql so it sorts after main's newest (2027-01-18-170000). No functional change.

[ToddHebebrand]

Main merged more migrations; unshipped migration moved to 2027-01-18-191000-contract-documents-certificate-version.sql so it sorts after everything queued ahead. No functional change.

[ToddHebebrand]

Merged origin/main (01a6d59) into this branch, giving new head f5bc794.

Conflict: apps/api/src/services/tenantExportPolicyRegistry.ts had one conflict, in the contract_documents entry. #10654 added template_name_snapshot (varchar(255)) and this PR adds certificate_version (smallint). Both are scalar columns, so both stay in included, in migration order (188000 snapshot, then 191000 certificate version). I kept main's comment about template_name_snapshot. excludedOpen is unchanged (pdf_data). Nothing else conflicted.

Tests: these were run locally. The vitest runs for the tenantExportPolicyRegistry tests, agreementCertificate and contractDocumentService passed: 15 files, 170 tests. The tenantExportPolicy*, tenantExport and admin tenantExport route tests passed: 19 files, 130 tests. The DB-backed integration suites (orgCascadeFkOnDelete, tenantExportErasureRoundtrip, contractDocumentsCertificateVersion, portalAgreements) were not run locally, so CI's Integration Tests will cover them. git merge-tree --write-tree HEAD origin/main is clean.