fix(tz): close the hole that let a display option pass for a timezone

The guard tested whether the argument list contained the substring "timeZone".
`timeZoneName: "short"` contains it. That option appends "GMT+1" to the output
and leaves the zone as the browser's, so a formatter written that way sailed
through while being exactly as wrong as a bare one — and looking more careful
than a bare one, which is worse.

Tightening it to a colon-terminated property then reported nine correct call
sites, because the shorthand `{ timeZone }` is what most of this tree writes.
The honest response to that would have been to allowlist all nine, which is
how a guard stops guarding. It accepts both forms now, and a test pins both
directions rather than leaving the regex to be trusted.

Two exclusion reasons in the backup plan aged inside their own release and are
corrected: push attempts now also hold the reminder dedup anchors this release
introduced, and the thumbnail's "rebuilds from the stored bytes" only holds on
the disaster-recovery path, since a portable export carries no bytes to rebuild
from.
This commit is contained in:
Marc-André Bombeck committed 2026-07-27 16:10:30 +02:00
1 parent 4ad498dbff
commit ea55886445
2 files changed
+31 -3

No files matched your search

@@ -93,7 +93,18 @@ function bareFormatterSites(): Array<{ file: string; line: number }> {
pattern.lastIndex = 0;
for (let m = pattern.exec(source); m !== null; m = pattern.exec(source)) {
const args = argumentList(source, m.index + m[0].length - 1);
if (args.includes("timeZone")) continue;
// The property, not the substring, and both ways of writing it.
//
// `timeZoneName: "short"` is a DISPLAY option — it appends "GMT+1" to
// the output and leaves the zone as the browser's. A plain substring
// check waves it through, and the result then looks MORE
// timezone-aware than a bare formatter while being exactly as wrong.
//
// The shorthand `{ timeZone }` is the common form in this tree (nine
// correct call sites use it), so a colon-only check would report all
// of them and the honest response would be to allowlist them — which
// is how a guard stops guarding.
if (/\btimeZone\s*(?::|,|\}|$)/m.test(args)) continue;
hits.push({
file,
line: source.slice(0, m.index).split("\n").length,
@@ -144,6 +155,23 @@ describe("date formatters name the calendar they render in", () => {
}
});
it("does not accept timeZoneName as a substitute for timeZone", () => {
// `timeZoneName: "short"` appends "GMT+1" to the output and leaves the
// zone as the browser's. The first version of this guard tested for the
// substring "timeZone", which that satisfies — so a formatter that LOOKS
// more timezone-aware than a bare one sailed through while being exactly
// as wrong. An independent read found it; this pins it.
const property = /\btimeZone\s*(?::|,|\}|$)/m;
expect(property.test('{ hour: "2-digit", timeZoneName: "short" }')).toBe(
false,
);
expect(property.test('{ hour: "2-digit", timeZone: tz }')).toBe(true);
expect(property.test("{ timeZone : tz }")).toBe(true);
// The shorthand forms, which are what most of this tree actually writes.
expect(property.test("{ timeZone, year: 'numeric' }")).toBe(true);
expect(property.test("{ ...options, timeZone }")).toBe(true);
});
it("carries no allowlist entry that no longer has a bare formatter", () => {
// A stale exception is worse than none: it documents a decision about code
// that has moved on, and it quietly covers whatever lands in that file next.
+2 -2
View File
@@ -154,7 +154,7 @@ export const DERIVED_MODELS: Readonly<Record<string, string>> = {
DocumentContentIndex:
"A search index over `InboundDocument` text; rebuilt by the indexer.",
DocumentThumbnail:
"Rendered from the stored document bytes on demand, and those bytes are carried, so the picture rebuilds itself on first view.",
"Rendered from the stored document bytes on demand. Those bytes travel in a disaster-recovery payload, so the picture rebuilds itself on first view; a portable export carries metadata only, and there the thumbnail has nothing to rebuild from — but neither does the document, so nothing is lost that the export did not already disclose.",
ArrivalReaction:
"Derived from what changed since the last visit; recomputed per visit.",
DismissedPriorityItem:
@@ -206,7 +206,7 @@ export const NOT_IN_BACKUP_MODELS: Readonly<Record<string, string>> = {
// Ledgers whose meaning is local to the deployment.
PushAttempt:
"A 90-day delivery ledger for this host's sends. It documents what happened here.",
"A 90-day delivery ledger for this host's sends, and since v1.33.1 also the anchor rows the reminder dedup writes under a sentinel channel. Both describe what this instance did; restoring either onto another host would assert sends it never made, and the dedup anchors would suppress reminders that host still owes.",
AuditLog:
"An append-only record of actions taken on this instance. Merging one instance's audit trail into another's would make the trail untrue.",
CoachUsage: