fix(digest): DIGEST_ONLY_USER self-expiring (mandatory until= suffix, 48h cap)#3271
Conversation
… 48h cap) Review finding on PR #3255: DIGEST_ONLY_USER was a sticky production footgun. If an operator forgot to unset after a one-off validation, the cron silently filtered every other user out indefinitely while still completing normally (exit 0) — prolonged partial outage with "green" runs. Fix: mandatory `|until=<ISO8601>` suffix within 48h of now. Otherwise the flag is IGNORED with a loud warn, fan-out proceeds normally. Active filter emits a structured console.warn at run start listing expiry + remaining minutes. Valid: DIGEST_ONLY_USER=user_xxx|until=2026-04-22T18:00Z Rejected (→ loud warn, normal fan-out): - Legacy bare `user_xxx` (missing required suffix) - Unparseable ISO - Expiry > 48h (forever-test mistake) - Expiry in past (auto-disable) Parser extracted to `scripts/lib/digest-only-user.mjs` (testable without importing seed-digest-notifications.mjs which has no isMain guard). Tests: 17 cases covering unset / reject / active branches, ISO variants, boundaries, and the 48h cap. 6066 total pass. typecheck × 2 clean. Breaking change on the flag's format, but it shipped 2h before this finding with no prod usage — tightening now is cheaper than after an incident.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR replaces the bare Confidence Score: 5/5Safe to merge — all remaining findings are P2 style suggestions with no correctness or reliability impact. The core logic (parse → filter → warn → fan-out) is correct and well-tested. The 48 h hard cap ensures both P2 findings (lenient Date.parse, misleading multi-pipe message) cannot cause a silent partial outage or incorrect filtering. No P0/P1 issues found. scripts/lib/digest-only-user.mjs — minor Date.parse leniency and multi-pipe error message (both P2) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[cron tick starts\nnowMs = Date.now] --> B[fetch all digest rules]
B --> C[parseDigestOnlyUser\nenv var, nowMs]
C --> D{kind?}
D -- unset --> E[normal fan-out\nno log]
D -- reject --> F[console.warn IGNORED\n+ reason + format hint]
F --> E
D -- active --> G[console.warn ACTIVE\nuserId + expiry + remainingMin]
G --> H[filter rules to\nonlyUser.userId]
H --> I{rules.length === 0?}
I -- yes --> J[warn no rules matched\nreturn / exit 0]
I -- no --> K[filtered fan-out]
E --> L[composeBriefsForRun\nall rules]
K --> M[composeBriefsForRun\nfiltered rules]
L --> N[digest send loop\nall users]
M --> O[digest send loop\ntest user only]
Reviews (1): Last reviewed commit: "fix(digest): DIGEST_ONLY_USER self-expir..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const untilMs = Date.parse(untilRaw); | ||
| if (!Number.isFinite(untilMs)) { | ||
| return { | ||
| kind: 'reject', | ||
| reason: `expiry "${untilRaw}" is not a parseable ISO8601 timestamp`, | ||
| }; |
There was a problem hiding this comment.
Date.parse accepts far more than ISO 8601
Date.parse is intentionally permissive in V8 — it accepts RFC 2822 strings, locale-formatted dates, and other engine-specific formats (e.g. "April 22, 2026 18:00", "22 Apr 2026 18:00:00 GMT"). The error message says "not a parseable ISO8601 timestamp" but the check doesn't actually enforce ISO 8601 — it only checks for NaN. The 48 h hard cap prevents a non-ISO future timestamp from surviving, so this is not a reliability bug today, but the validation is looser than the documented contract.
Consider using a light regex guard before Date.parse to reject obvious non-ISO inputs:
// Reject anything that doesn't start with a 4-digit year + '-' (rough ISO guard)
if (!/^\d{4}-/.test(untilRaw)) {
return { kind: 'reject', reason: `expiry "${untilRaw}" is not a parseable ISO8601 timestamp` };
}
const untilMs = Date.parse(untilRaw);| if (parts.length !== 2) { | ||
| return { | ||
| kind: 'reject', | ||
| reason: 'missing mandatory "|until=<ISO8601>" suffix', | ||
| }; | ||
| } |
There was a problem hiding this comment.
When parts.length > 2 (e.g. user_xxx|until=2026-04-22T18:00Z|extra), the returned reason is 'missing mandatory "|until=<ISO8601>" suffix' — which points the operator toward adding a suffix that is already present. A confused operator might try user_xxx|until=...|until=....
| if (parts.length !== 2) { | |
| return { | |
| kind: 'reject', | |
| reason: 'missing mandatory "|until=<ISO8601>" suffix', | |
| }; | |
| } | |
| if (parts.length !== 2) { | |
| return { | |
| kind: 'reject', | |
| reason: parts.length === 1 | |
| ? 'missing mandatory "|until=<ISO8601>" suffix' | |
| : `expected exactly one "|" separator, got ${parts.length - 1}`, | |
| }; | |
| } |
Two style fixes flagged by Greptile on PR #3271: 1. Misleading multi-pipe error message. `user_xxx|until=<iso>|extra` returned "missing mandatory suffix", which points the operator toward adding a suffix that is already present (confused operator might try `user_xxx|until=...|until=...`). Now distinguishes parts.length===1 ("missing suffix") from >2 ("expected exactly one '|' separator, got N"). 2. Date.parse is lenient — accepts RFC 2822, locale strings, "April 22". The documented contract is strict ISO 8601; the 48h cap catches accidental-valid dates but the documentation lied. Added a regex guard up-front that enforces the ISO 8601 shape (YYYY-MM-DD optionally followed by time + TZ). Rejects the 6 Date-parseable-but-not-ISO fixtures before Date.parse runs. Both regressions pinned in tests/digest-only-user.test.mjs (18 pass, was 17). typecheck × 2 clean.
|
Addressed both Greptile P2s in
Tests: +1 new (covers 6 non-ISO shapes that V8 Date.parse would have accepted). 18 pass, was 17. |
1 participant
Summary
Addresses a P2 review finding on #3255: `DIGEST_ONLY_USER` as shipped was a sticky production footgun. If an operator set it for a one-off validation and forgot to unset it, the cron silently filtered out every other user indefinitely while still completing normally and exiting 0 — a prolonged partial outage with "green" runs.
Fix
Mandatory expiry. The env var MUST now be in the form:
```
DIGEST_ONLY_USER=user_xxx|until=2026-04-22T18:00Z
```
Rules:
This means a test flag cannot sit in Railway env forever — it expires on its own, and even a misconfigured value fails safe (fan out, don't silently exclude).
Breaking change to the flag format, but the flag shipped ~2h before this finding with no prod usage — cheaper to tighten now than after an incident.
Changes
Testing
6066 tests pass (was 6049 + 17 new). typecheck + typecheck:api clean.
Coverage:
Post-Deploy Monitoring & Validation
```bash
railway variables --set DIGEST_ONLY_USER=user_3BovQ1tYlaz2YIGYAdDPXGFBgKy\|until=$(date -u -v+2H +%Y-%m-%dT%H:%MZ)
```
(2h from now). Next cron tick runs for you only; 2h later auto-disables.
🤖 Generated with Claude Opus 4.7 (1M context, extended thinking) via Claude Code