harden: an oversized phishing delta erases the previously stored one, dropping to vendored-only coverage for up to 24h #236

New Issue

2026-08-11T15:23:32+02:00

clawbot commented

2026-08-11 15:23:32 +02:00

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 lastAttemptTime floor 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

(a) Keep the last known-good delta with its ORIGINAL timestamp, and record the failed attempt separately. No freshness lie — the record still says how old the data is — and protection degrades gracefully rather than falling off a cliff.
(b) Store a truncated delta up to the cap, marked partial. Keeps some coverage but silently protects against an arbitrary subset, which is harder to reason about than either extreme.
(c) Keep current behaviour and make the degradation visible — surface that the blocklist is stale or unavailable rather than failing silently.

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

blacklist is used throughout src/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

An oversized fetch leaves the last known-good delta in place with its original timestamp.
The record still reports accurate age; no fetch is treated as fresher than it is.
A worker restart after an oversized fetch retains the previous coverage.
The retry floor from #158 still prevents repeated downloads.
Tests cover the oversized-fetch-after-valid-fetch sequence across a simulated restart, demonstrated failing first.
TODO.md updated in the same commit.
make check passes.

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 `lastAttemptTime` floor added in https://git.eeqj.de/sneak/AutistMask/issues/158 correctly stops this causing a download storm. Pre-existing behaviour from the 256 KiB cap; surfaced by the independent review of https://git.eeqj.de/sneak/AutistMask/pulls/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 - **(a) Keep the last known-good delta with its ORIGINAL timestamp**, and record the failed attempt separately. No freshness lie — the record still says how old the data is — and protection degrades gracefully rather than falling off a cliff. - **(b) Store a truncated delta** up to the cap, marked partial. Keeps some coverage but silently protects against an arbitrary subset, which is harder to reason about than either extreme. - **(c) Keep current behaviour** and make the degradation visible — surface that the blocklist is stale or unavailable rather than failing silently. ## 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 `blacklist` is used throughout `src/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 - [ ] An oversized fetch leaves the last known-good delta in place with its original timestamp. - [ ] The record still reports accurate age; no fetch is treated as fresher than it is. - [ ] A worker restart after an oversized fetch retains the previous coverage. - [ ] The retry floor from https://git.eeqj.de/sneak/AutistMask/issues/158 still prevents repeated downloads. - [ ] Tests cover the oversized-fetch-after-valid-fetch sequence across a simulated restart, demonstrated failing first. - [ ] `TODO.md` updated in the same commit. - [ ] `make check` passes.

clawbot referenced this issue

2026-09-21 09:19:59 +02:00

Review toward 1.0: identify and file all next steps #382

clawbot commented

2026-09-21 09:19:59 +02:00

Closing: the runtime phishing-list fetch and the 256 KiB delta store this describes were retired; the list is vendored at build time and nothing is fetched at runtime (src/shared/alarms.js, README.md). Nothing left to do.

model: claude-fable-5

Closing: the runtime phishing-list fetch and the 256 KiB delta store this describes were retired; the list is vendored at build time and nothing is fetched at runtime (`src/shared/alarms.js`, `README.md`). Nothing left to do. model: claude-fable-5

clawbot closed this issue

2026-09-21 09:19:59 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/AutistMask#236