release: country codes that name a country, mail in the customer's language, and no addresses in the log - #203
Merged
Merged
Conversation
…etters (#201) `billingIdentitySchema` pinned `countryCode` to `/^[A-Z]{2}$/`, and the live control plane held the proof that a shape is not a country: the one reseller identity on it was stored as `SW`, which ISO assigns to nothing (Switzerland is CH, Sweden is SE), and every surface read it as a country for nine days. It matters because that field decides the whole tax treatment. An unassigned code is not "in the EU" by any test, so `determineTaxTreatment` fell through to OUTSIDE_SCOPE and answered 0% with a confident reason naming a country that does not exist. For the Swiss buyer it was stored on, the verdict was right and the evidence was wrong — a correct answer hiding wrong data. Mistype an EU country into an unassigned code and the same path issues an IMMUTABLE 0% invoice to a customer who owed 21% or a reverse charge, with no gate anywhere saying so. `lib/country-code.ts` holds the 249 officially assigned ISO 3166-1 alpha-2 codes and two predicates: a forgiving one for input (normalises case) and a canonical one for stored rows, which `isInvoiceable` uses — a lowercase value in the database means the row did not come through the schema that uppercases. `EL` stays out of the country list and stays accepted by the tax rule: it is Greece's VAT prefix rather than a country code, and callers have always been allowed to spell Greece either way there. Putting it in the list would have made it a country; special-casing it in the readability check keeps the contract exactly where it was. The live row was corrected to CH before this landed, since the new rule would otherwise block the next invoice on an ACTIVE subscription. The two invoices already issued keep `SW` in their immutable buyer snapshots — a correction is a credit note (ADR-013), and their OUTSIDE_SCOPE verdict and amounts are right.
…the contact intake (G15, G17) (#200) * fix(email): tag the recipient in logs instead of printing it (G15) Both of sendEmail's refusal paths wrote `to=<address>` to the container log, so every send made without a key or without a verified sender put a customer's address where CLAUDE.md §5.8 says none may go. Resend's own error body quotes the recipient back at us as well (the sandbox-sender 403 names it outright), and that was logged verbatim. `lib/log-recipient.ts` replaces both with a salted digest tag, stable within a process so two lines about one recipient still read as one recipient. `LOG_HASH_SALT` pins it across containers when an operator wants that; unset, a per-process random salt makes the tags non-reversible, which is the safer default for the common case of reading one container's log. It is pseudonymisation, not anonymisation, and the module says so. The file-length checker's PII heuristic could not have caught this: it looks for an email-SHAPED literal on a console line, and an interpolated variable has no shape until runtime. It now also flags a console line that interpolates a value NAMED like a person, with the redaction helpers stripped first so the fixed code does not warn about itself. Verified against the whole tree (clean) and against a deliberately leaky file (caught). * fix(api): rate-limit the contact intake, which is unauthenticated and sends mail (G17) `/api/waitlist` never called `guardIntake`, so the one public endpoint that mails the founder on every accepted call had no limit at all — a way to burn the Resend quota that the invite and reset mails share, and to flood the only inbox that reads it. The honeypot stops a naive bot, not a loop. It now runs the same guard the partner-apply and signup intakes use (5 POSTs per IP per 15 min). Its own `company` honeypot is kept beside the shared `company_website` one: this form has sent that field since the waitlist days and every deployed client still does. The e2e spec found a trap worth keeping: `page.request` does NOT inherit the fixture's per-test `x-forwarded-for`, so all three tests landed in the single "unknown" bucket and the last two were refused by the first test's own calls. `apiClientHeaders()` makes the identity explicit for API calls, and the fixture now documents the limit of the browser-header approach. * fix(email): make the redaction scan linear, and split the checker's regex Three Sonar findings on the first push, all worth taking rather than accepting. S8786 was the real one. `redactAddresses` ran an address-SHAPED regex over text a third party controls — a provider's error body — and any such pattern backtracks quadratically on a long run with no `@`: the leading class eats the run, then gives back one character at a time, from every offset. Measured at ~1.9s for a 100 KB token, and nothing bounds the size of a body we did not write. The scan is now over whitespace-separated tokens (`\S+` cannot backtrack, nothing follows it) with the address judgement in code: exactly one `@`, content on both sides, a dotted TLD. Single-digit ms on the same input, pinned by a timing test that states both numbers. Two smaller ones: the digest is its own statement rather than a nested template literal (S4624), and the checker's PII rule is two small patterns — an interpolation, and a name inside it — rather than one 23-complexity regex (S5843). The split also fixed a real hole: it now tests EVERY interpolation on the line, so `${slug}` followed by `${user.email}` is caught, which the single combined pattern would have decided on the first match alone. * fix(email): peel punctuation with an index walk, not an anchored regex Sonar's second pass found the same class of defect one layer down. `[…]+$` is super-linear for exactly the reason the pattern it replaced was: unanchored at the start, it retries from every offset, so a token that is 50 KB of quotes and commas is quadratic — and it is reached from a provider's error body, which we neither write nor bound. Two character sets and two while-loops instead, which need no argument about backtracking, plus a timing test on the punctuation shape specifically. The duplicate-`@` guard also moves to `lastIndexOf`, which says what it means (S7765). * test(email): use a fixture address in the Resend 403 sample The sample body quoted the owner real mailbox. The sentence shape is what is under test, so a fixture address proves exactly the same thing without putting a real address in the repo.
… calling everyone a partner (G10) (#202) * feat(email): write to a customer in their own language, from the account (G9, G10) Five customer-facing mails were hardcoded English in a product that ships six languages and sells in Geneva: the invite, the partner approval, the invite re-send, the password reset and both invoice mails. Only the trial warning was localized (sofra #167), and it got there by looking the payer's address up in `PartnerApplication` at send time — a join by lower-cased email that silently downgrades a francophone partner to English the moment an admin typed the address in a different case. The missing piece was never the pattern; it was that the control plane held a locale for every INTAKE and none for a PERSON. `User.locale` is that column. Seeded from the intake that created the account, backfilled from the same intakes for the rows that exist, and refreshed when they set their password — the one moment a customer is certainly present with a language chosen. The backfill matters today: the one live reseller applied in FRENCH, so without it his account would have defaulted to English and he would have received one mail in French and every other one in English. G10 rides along, because translating a wrong persona would have baked it into six languages: the reset mail no longer calls every recipient a "partner". A restaurant owner locked out of their own dashboard was being told about a partner password they have never had, which is what a phishing mail reads like. The founder's own notices stay English on purpose — one reader, and an operational mail that is grepped rather than read. * refactor(email): resolve copy before the template, not inside it Sonar S4624 on the reset mail, and the same shape in the invite: a translator call nested inside a template literal. Hoisting the two lines of copy above the HTML removes the nesting and reads better — the body becomes two placeholders rather than two expressions. * refactor(email): hoist the last two nested translator calls Same S4624 shape in the partner-approval and billing-details templates. Sonar flagged one of the four; leaving the other three would mean the next reader copies whichever they happen to open.
|
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
1 participant
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.
Release
develop→main. Three fixes, one of them fiscal, one of them a PII leak, and one of them the thing a francophone customer has been quietly missing.What ships
SW— right shape, assigned to nothing — sodetermineTaxTreatmentfailed every EU test and answered OUTSIDE_SCOPE at 0% with a confident reason. Right for a Swiss buyer, which is why nobody found it;SWforSEis one keystroke and the same path issues an immutable 0% invoice to someone who owed 21%. Membership of ISO 3166-1 is now checked at the write schema, inisInvoiceable, and in the tax rule, which stops instead of guessingUser.locale+ five customer mails in six languages; the reset mail stopped calling every recipient a "partner"#202 adds
User.locale. Run theghcr.io/piwas-21/sofra:migrateone-off on the staging box beforecompose pull sofra && up -d sofra. The column is additive with a default, so the currently-running image keeps working against the migrated schema in the window between the two — the reverse order would run the new code against a table without the column.The migration also backfills from
PartnerApplication/SignupRequest, role-matched, most-recent-intake-per-address. On this database that is expected to set exactly one row tofr(the reseller behind O'Bresse, who applied in French on 2026-08-14) and leave every other account aten.Data already changed, before this merge
BillingIdentity cmsrn9w3h001x01nu7yq9lv03(the reseller):countryCodeSW→CH, one row, verified before and after. Done first on purpose — #201's stricterisInvoiceablewould otherwise have blocked the next settled charge on that ACTIVE €85/month plan from becoming an invoice. The two invoices already issued keepSWin their immutable snapshots; a correction is a credit note (ADR-013), and their treatment and amounts are correct for a Swiss buyer.Verification before this PR
subject=Bienvenue chez SofraPiwas — définissez votre mot de passefor the customer and an English founder notice in the same request.After merge
sync/image build green.GET /api/healthreports the new sha;/en/legaland/fr/signupstill render;User.localeisfrfor the reseller andenelsewhere.