Skip to content

fix: namespace Valkey cache keys so clear() removes this cache's entries - #137

Open
souta-lab wants to merge 2 commits into
sahenjp:mainfrom
souta-lab:fix/a12-valkey-clear-namespace
Open

souta-lab wants to merge 2 commits into
sahenjp:mainfrom
souta-lab:fix/a12-valkey-clear-namespace

Conversation

@souta-lab

Copy link
Copy Markdown
Contributor

Summary

Give the Valkey cache backend one key namespace for store, load and delete, and make clear() walk that namespace with SCAN instead of KEYS, as prescribed by A12 in docs/AUDIT-2026-11-02.md.

Root cause

ResponseCache::key() builds keys as "{method} {url} {body}", and ValkeyCache::set() / get() used that raw key. But ValkeyCache::clear() in src/cache.rs ran KEYS searxng:* and deleted whatever matched. Stored keys never start with searxng:, so clear() removed none of this cache's own entries — and if the same Valkey/Redis database is shared with an upstream SearXNG instance, it deleted that instance's keys instead.

Changes

All in src/cache.rs (+217/−26):

  • A single namespace constant, rustxng:cache:, is applied inside ValkeyCache on set() and get() and matched by clear(), so all three operations agree on the key format. ResponseCache::key() and the in-memory backend are unchanged.
    About the prefix: the audit asks for one namespace but does not name it. rustxng: keeps these keys clear of upstream SearXNG's searxng:* keys in a shared database (the collision this bug is about), and the cache: segment keeps clear() from ever wiping keys a future RustXNG feature might store in the same database. Happy to adjust if another prefix is preferred.
  • clear() now iterates SCAN <cursor> MATCH rustxng:cache:* COUNT 100 until the cursor returns to 0, deleting each page with a single multi-key DEL. A malformed reply or a failed command stops the walk and counts an error instead of silently ignoring it.
  • To support that, the connection layer now exposes request(), which returns a reply with its structure intact (SCAN replies are [cursor, [keys…]], which the existing flattening would lose). The previous command() behaviour is preserved exactly as request() plus ValkeyReply::flatten().
  • Tests: a new regression test, clear_removes_own_entries_and_leaves_other_namespaces, runs against the existing fake-RESP-server harness (no real Valkey needed). It stores two entries through the cache, seeds the fake keyspace with foreign keys (searxng:limiter, searxng:cache, other), calls clear(), and asserts its own entries are gone while all foreign keys remain. The fake SCAN returns one key per page so the cursor loop and empty pages are exercised. The wire-level key assertions in cancelled_commands_do_not_reuse_pending_replies were updated for the prefixed keys.

One compatibility note: entries written by older versions under the raw, unprefixed keys become unreachable after this change. They are not deleted explicitly; each one simply expires through its TTL (5 minutes by default), the same as any other cache entry.

Verification

Linux x86_64, Rust 1.99.1:

  • Before the fix: the new regression test, applied on its own to unmodified main, fails — clear() leaves this cache's entries in place (and removes the foreign searxng:* keys instead), which is the reported bug.
  • cargo test --locked --lib cache: 11 passed, 0 failed (includes the new regression test).
  • cargo test --locked --all-targets: library suite 1036 passed, 5 failed, 1 ignored. The 5 failures (aggregator::orchestration_tests::raw_content_* ×3 and fetch::direct::tests::{configured_body_limit_is_enforced_and_classified, renderer_fallback_static_keeps_the_extracted_text}) fail identically on unmodified main in this sandbox and are unrelated to this change; they appear to depend on the sandbox's network/fetch environment.
  • cargo fmt --all --check: passed.
  • cargo clippy --locked --all-targets: no warnings (CI runs it with -D warnings).

Limitations

Other platforms are left to CI. No live Valkey/Redis server was available in the sandbox, so server behaviour is covered through the fake-RESP harness only.

Related

Addresses A12 in the existing audit.

AI assistance: implementation and tests.

The Valkey backend stored entries under the raw "METHOD URL BODY" key,
but clear() deleted `KEYS searxng:*`. It never matched this cache's own
entries, and on a database shared with an upstream SearXNG it deleted
that instance's keys instead (audit A12).

ValkeyCache now prefixes every key with "rustxng:cache:" for GET, SET
and clear(). clear() walks the keyspace with SCAN ... MATCH instead of a
blocking KEYS and DELs each page. Entries written by older versions use
the old key format; they are no longer reachable and expire via their
TTL.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@souta-lab
souta-lab requested a review from sahenjp as a code owner November 11, 2026 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant