feat(security): rework the Ignored items (allowlist) review surface#322
Merged
Conversation
…nizable rows
Users couldn't tell WHAT they had ignored: the modal was titled "Manage
allowlist" and each row showed a kind plus an opaque value-hash slice.
The allowlist deliberately stores only a non-crypto hash of the value
(never plaintext) for rescan matching — correct and privacy-preserving,
but unreadable to a human reviewing past decisions.
Reframe the surface for recognition (industry pattern: separate "what
you store to match" from "what you show to recognize"):
- Title → "Ignored items"; plain-language subtitle; action button
"Remove" → "Stop ignoring" with a non-destructive EyeOff icon
(it un-ignores, doesn't delete data). Click-twice in-place confirm
preserved. Buckets regrouped + sentence-cased: "Everywhere" /
"This session". All 7 locales updated, plus the Settings → Security
section copy ("Ignored items" / "Review") so the entry point matches.
- Rows now show kind label, location ("in {title}" / "in every
session"), relative ignored-time (formatScanAgo), an optional masked
preview, and an optional reason — no hash slice.
Masked preview (new @spool-lab/redact `previewValueByKind`): a lossy,
non-reversible recognition hint computed at dismiss time from the live
message text — `Stripe ••a39f`, `m••@gmail.com`, `•• 1111`. It retains
only fragments that are not themselves secret (a vendor name inferred
from a public token prefix, an email domain, at most the last 4 chars);
the random secret body is dropped, so a preview can never be expanded
back to the value. Short values collapse to kind-only. Stored via a
forward-only migration (v14: nullable `preview` + `reason` columns on
both allowlist tables); `dismissFinding` reconstructs the value from
message offsets and reduces it to the preview without persisting
plaintext. Pre-v14 rows and unreconstructable values (purged) render
gracefully on the kind-only fallback.
Reason: storage + display wired end-to-end (enum: not-secret /
test-credential / low-risk / acknowledged) and threaded through IPC.
The dismiss-time reason PICKER is deferred — the existing dismiss
control is a flat scope menu (session/everywhere); adding a reason
dimension would either nest 8 combinations or add a second step,
against the project's "single-item dropdowns are over-engineering" /
low-friction guidance. Storage + render land now; the picker is a
clean follow-up.
Tests: unit-test previewValueByKind (kind-aware, non-reversible, short
values); core repo tests for the v14 columns, dismiss populating the
preview/reason, listAllowlistEntries returning them, the upsert
preserving an existing preview, and null fallbacks; migration-v12 spec
extended for the additive v14 upgrade. Adjacent suites green: core
(353), redact (128), locale parity (17). Existing settings-security
e2e relies only on preserved testids + empty-state, so it stays valid;
populated-row recognition is covered by the core unit tests (avoids a
heavy electron-ABI rebuild + scan-dependent fixture).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… into tests launch.ts isolated SPOOL_CLAUDE_DIR/CODEX/GEMINI but not the OpenCode source added in #265. With SPOOL_OPENCODE_DIR unset, source-paths.ts falls back to the real ~/.local/share/opencode, so the developer's actual OpenCode sessions were indexed into every e2e run — shifting ordering/counts and stalling first-launch sync. That is the root cause of the recurring 30s first-launch timeout in security-findings-actions and others that reproduced locally but stayed green in clean CI. Point SPOOL_OPENCODE_DIR at an isolated empty dir (zero opencode sessions, deterministic) and expose opencodeDir to extraFixtures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…" (UI + test-ids + search qualifier + tests) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…hemaSanity The v14 migration adds preview/reason to the allowlist tables inside a version-gated block. A DB whose user_version already reached 14 via another path (shared dev DB stamped by a parallel branch, or a half-finished upgrade) skips that block forever and sits at 14 with the columns absent — so the dismiss/ignore INSERT throws "table allowlist_global has no column named preview" and the action silently no-ops (no toast, no removal). ensureSchemaSanity already exists to repair exactly this (version pragma bumped without the column); extend it to backfill the allowlist preview/reason columns (tableExists-guarded for pre-v12 DBs), matching the sessions backfill. Regression test reproduces the v14-without-columns collision and asserts the heal + a working INSERT. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rop the stored-preview/v14 machinery Abandon the stored masked-preview approach for the "Ignored items" modal. The plaintext already lives in messages.content_text (the findings view shows it, blurred), so reconstruct the live value at read time from a matching finding's message — no new persistence. Removed: - v14 schema migration (preview/reason columns); LATEST_SCHEMA_VERSION back to 13; ensureSchemaSanity allowlist backfill loop dropped. - @spool-lab/redact previewValueByKind (preview.ts/preview.test.ts deleted). - repo AllowlistMeta, DismissReason, previewForFinding, and preview/reason on addAllowlist*/dismissFinding(s); add functions back to INSERT OR IGNORE. - reason threading through DISMISS_FINDING(S) IPC/preload/renderer api. - allowlist_reason_* / reason_* i18n keys in all 7 locales. Added: - AllowlistEntryRow.value reconstructed in listAllowlistEntries via substr(messages.content_text, …) over a non-purged finding (session + global scopes, prepared statements); null when purged/orphaned/gone. - Modal renders the value with FindingsStrip's blur + hover/click reveal, driven by the canonical securityPageValuesBlurred pref; null falls back to kind label + an "original no longer available" hint (i18n, 7 locales). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reconcile the allowlist/ignored i18n keys with the overhauled modal: add allowlist_all_scopes, allowlist_all_types, allowlist_scope_everywhere, and security.ignored_label across all 7 locales; remove the now-dead allowlist_modal_sub, allowlist_bucket_global, allowlist_in_every_session, and security.ignored_count. Locale key-parity test stays green. Extract the modal's scope/type/text filter into a pure filterIgnoredEntries helper with unit tests, and add an e2e spec covering the scope dropdown, type dropdown, text filter, and Stop ignoring against a populated list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ignored items are values the user explicitly chose to stop flagging, so the management modal shows them in the clear — no blur/hover-reveal. (The value is still reconstructed read-time from the source message and never persisted.) Also clear a pending click-twice 'Stop ignoring' confirm when the filters change, so a filtered-out row can't reappear mid-confirm. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
What
Redesigns the "Ignored items" surface (formerly "Manage allowlist") — how users review and undo the findings they told Spool to stop flagging.
substrat the finding's offsets) — exactly the plaintext the findings view already displays — and shown in the clear (these are values the user deliberately ignored). Nothing new is persisted.is:ignored, withis:dismissedkept as an alias) all say "Ignore" now, replacing the old Dismiss/allowlist/ignore mix. Internal function/IPC names and the persisted'dismissed'state are intentionally unchanged (no migration).countAllowlistEntries, instead of only Settings → Security → Manage.SPOOL_OPENCODE_DIRis now isolated in the e2e launch helper — without it, a developer's real OpenCode sessions leaked into tests and caused machine-dependent first-launch timeouts. Independent infra fix; bundled because the Ignored-items e2e relies on it.Why
The old modal was unreadable ("Email · 57cfb8f5…") and buried four levels deep in Settings. Reviewing past ignore decisions is the point of the surface, so the value has to be recognizable and the entry discoverable.
How it connects
Part of the pre-GA Security Scan feature (flag-gated, off in prod). No schema change — an earlier iteration in this branch's history added a v14 migration + a stored masked-preview, both fully removed once live reconstruction landed;
LATEST_SCHEMA_VERSIONstays 13 and the allowlist tables gain no columns. (Squash-merge collapses the intermediate churn.)Test plan
countAllowlistEntries, schema asserts the allowlist tables have no preview/reason columns and version 13.filterIgnoredEntries(scope/kind/text/combined) unit suite.MessageList.tsxerror remains on the base).