harden: an oversized phishing delta erases the previously stored one, dropping to vendored-only coverage for up to 24h #236
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
When a fetched phishing delta exceeds the 256 KiB storage cap, the persisted record is reduced to
{lastAttemptTime}only — the previously stored, valid delta is erased along with the oversized one.In-memory coverage survives until the worker is terminated. After that, the next wake has vendored-list-only coverage until the next successful fetch, up to 24 hours later. So a single oversized upstream response silently downgrades phishing protection, and nothing tells the user.
This is a deliberate consequence of the "no freshness lie" choice — keeping the old delta while stamping a fresh timestamp would claim currency for data no longer held, which is worse. The
lastAttemptTimefloor added in #158 correctly stops this causing a download storm. Pre-existing behaviour from the 256 KiB cap; surfaced by the independent review of #208, which flagged it rather than filing.The gap is that "cannot store the new list" and "must therefore discard the old one" are being treated as the same decision. They are not.
Options
Recommendation
(a), plus the visibility from (c). The old delta was valid when stored and does not become wrong because a newer one was too big; discarding it trades real coverage for no gain. Pairing it with an explicit stale indication means the user is never silently less protected than they think.
Secondary, noted in the same review
blacklistis used throughoutsrc/shared/phishingDomains.js, including the new storage record key. It mirrors the upstream config field name, so the wire-format reference cannot change, but the internal identifiers and the storage key can. Worth settling whether repo terminology policy applies to internal names that shadow an upstream field.Definition of done
TODO.mdupdated in the same commit.make checkpasses.