Agent PR 快取鍵漏洞：實測跨租戶資料洩漏

The pull request arrived at 02:14 with a green checkmark and three files changed. Its claim was modest: skip duplicate model calls, cut latency on repeated prompts. The reviewer's first instinct was to trust the test suite.

The tests only covered the happy path. That is exactly where a caching change is least likely to break.

Note: the diff below is a minimal reconstruction for teaching, not a verbatim vendor pull request.

The Diff Under Review

import hashlib

CACHE = {}

def complete(prompt, tenant_id, model="default"):
    key = hashlib.sha256(prompt.encode()).hexdigest()
    if key in CACHE:
        return CACHE[key]
    resp = call_model(model, prompt)   # network call, can raise
    CACHE[key] = resp
    return resp

Three things stand out on the first read. The tenant_id argument is accepted and never used. Nothing checks whether the response is usable before storing it. The dictionary has no size bound, no TTL, and no eviction path.

The diff passes review because it is small and readable. Small and readable is not the same as correct.

Why the Happy Path Passes

The accompanying test calls complete twice with one prompt and one tenant. It asserts that the model client was invoked once.

That assertion is true. It also stays true after every defect below is fixed, which is why the suite gives reviewers no signal at all.

Three Defects the Existing Tests Never Touch

1. The Cache Key Ignores Tenant Identity

The key is a hash of the prompt text alone. Two tenants that send an identical string share one cache entry.

For a prompt like "Summarize the incident report." the collision is not theoretical. Support tooling, templates, and system prompts repeat across accounts all day long.

2. Blank and Failed Results Get Cached

If call_model returns None or an empty string on a transient upstream failure, that value becomes the stored answer. Every later caller reads the failure until the process restarts.

Negative caching converts a one-second blip into a multi-hour outage. The diff never distinguishes a good answer from a bad one.

3. The Cache Is Process-Local and Unbounded

Four workers mean four separate dictionaries and four independent hit rates. Memory grows with the number of unique prompts and never shrinks.

The diff looks like a shared cache. It behaves like a per-worker notebook with no eraser.

What to Trust, What to Revert, What to Test

1. Trust the pure helper. hashlib.sha256 is a reasonable key primitive.
2. Keep the tenant_id parameter and thread it into the key.
3. Revert the module-level mutable dictionary.
4. Revert any write of an empty, failed, or partial response.
5. Replace the single happy-path test with isolation, failure, and concurrency tests.

A Reproduction Plan You Can Run in Ten Minutes

Step 1: Write the isolation test first

import pytest

calls = []

def fake_call_model(model, prompt, tenant_id):
    calls.append(tenant_id)
    return f"{tenant_id}-answer"

def test_cache_does_not_leak_between_tenants():
    calls.clear()
    a = complete("Summarize the incident report.", tenant_id="acme")
    b = complete("Summarize the incident report.", tenant_id="globex")
    assert calls == ["acme", "globex"]
    assert a != b

Run pytest -k tenant -x. It fails against the diff. That failure is the evidence, not an opinion.

Step 2: Patch the key and rerun

def cache_key(tenant_id, model, prompt):
    parts = [tenant_id, model, prompt]
    return hashlib.sha256("\x00".join(parts).encode()).hexdigest()

The null-byte separator prevents ambiguous concatenation. The same test should now pass unchanged.

Step 3: Prove failures are not cached

def test_failures_are_not_cached(monkeypatch):
    calls.clear()
    def boom(*args, **kwargs):
        calls.append("attempt")
        raise TimeoutError("upstream")
    monkeypatch.setattr("app.model.call_model", boom)
    for _ in range(2):
        with pytest.raises(TimeoutError):
            complete("boom", tenant_id="acme")
    assert len(calls) == 2

Two attempts, two upstream calls. A cached exception would produce one.

Step 4: Measure the cross-worker hit rate

for i in $(seq 1 40); do
  curl -s -X POST localhost:8000/complete \
    -H 'content-type: application/json' \
    -d '{"prompt":"ping","tenant_id":"acme"}' > /dev/null
done
grep -c 'model_call' /var/log/app.log

Forty requests with one unique prompt should log far fewer than forty model calls. If the count stays near forty, the cache is not shared across workers.

Decision Table

Reusing This Review Loop on a Free Server

The agent that produced the diff ran against MonkeyCode's free model access. The repro suite above ran on the project's free server option, which is enough for a few hundred test calls in one sitting.

The operator states the free tier also includes a promotional token allowance, currently advertised at 10 million tokens. Confirm the current terms in the repository before planning around any number. Disclosure: This article was prepared as part of MonkeyCode's product outreach.

The model is not the interesting part here. The loop is: generate, reproduce, revert, retest.

Limitations and Who Should Skip This

• Teams with strict data residency rules should not send prompts to any hosted endpoint, free or paid.
• Multi-tenant products need cache keys derived from tenant, model, and prompt version together.
• A process-local dictionary is acceptable for a single-worker prototype and wrong in every other deployment.
• If hit rate matters at scale, use a shared store with TTL, eviction, and a size cap.
• This review does not attempt to validate model output quality. It only checks whether one tenant can read another tenant's answer.

Closing Note

An agent PR is a draft, not a decision. Trust the pure functions, revert the shared mutable state, and test the tenant boundary before anything else.