Skip to content

fix: cache lacks input validation, serialization guard, and encapsulation #367

Description

@avoidwork

Summary

The LLM cache module in ./src/cache has several quality issues: missing input validation on cache creation parameters, no serialization guard for cached values, internal state exposure through the _lru property, and potential performance overhead when hashing large messages for cache keys.

Environment

  • OS: Linux 7.0.2-7-pve
  • Node.js: v25.8.1
  • madz version: 1.12.0
  • LLM provider: Unknown — user to confirm

Reproduction

  1. Import createLlmCache from ./src/cache/llm_cache.js
  2. Call createLlmCache(0, 0) or createLlmCache(-1, -1) — no validation error
  3. Call createLlmCache(100, 60000) and set a value containing functions or circular references — silently dropped
  4. Access the _lru property on the cache instance — raw tiny-lru exposed

Expected Behavior

  • createLlmCache should validate size and ttl parameters, throwing or defaulting on invalid input
  • Cache writes should either serialize safely or warn when values cannot be serialized
  • Internal implementation details (tiny-lru instance) should not be exposed on the public API
  • Large message hashing should be optimized or documented as a limitation

Actual Behavior

  • createLlmCache accepts any numeric values without validation, creating non-functional caches silently
  • Non-serializable values are silently dropped by tiny-lru with no warning (masked by try/catch)
  • The _lru property exposes the raw tiny-lru instance, breaking encapsulation
  • getCacheKey hashes the entire message content on every call, adding CPU overhead for large messages

Additional Context

Audit performed on ./src/cache/llm_cache.js (1 file, 47 lines). The cache is a thin wrapper around tiny-lru with a SHA256-based key generation function. The fail-open pattern (try/catch on all cache operations) is appropriate for resilience but masks serialization failures.


Audit Findings

# File Type Severity Summary
1 llm_cache.js Bug Medium No input validation on createLlmCache parameters — size and ttl accepted without bounds checking
2 llm_cache.js Bug Medium No serialization guard on cache values — non-serializable values silently dropped or corrupted
3 llm_cache.js Code Smell Low _lru exposes internal tiny-lru instance, breaking encapsulation
4 llm_cache.js Performance Low getCacheKey hashes entire message content — potential CPU overhead for large messages

Finding 1: No input validation on createLlmCache parameters

Severity: Medium
File: llm_cache.js:21

The createLlmCache(size, ttl) function accepts size and ttl without any validation. Edge cases:

  • size: 0 or negative → cache accepts entries but never stores them (silent failure)
  • ttl: 0 → TTL disabled, entries never expire
  • size: undefined → tiny-lru default (130 entries), may not match intent

Recommendation: Add validation with sensible defaults or throw on invalid input.

Finding 2: No serialization guard on cache values

Severity: Medium
File: llm_cache.js:31

The set method wraps cache writes in try/catch, but tiny-lru serializes values internally. If value contains:

  • Functions
  • Symbols
  • undefined
  • Circular references

The cache will silently drop or corrupt the entry. The try/catch masks this behavior, making debugging difficult.

Recommendation: Add a serialization check before writing, or log a warning when serialization fails.

Finding 3: _lru exposes internal state

Severity: Low
File: llm_cache.js:45

The raw tiny-lru instance is exposed via _lru. This is likely intentional for testing/debugging, but:

  • Breaks encapsulation
  • Internal API changes in tiny-lru could break consumers
  • No documentation of this contract

Recommendation: Document the contract or provide a test-only accessor.

Finding 4: getCacheKey hashes entire message

Severity: Low
File: llm_cache.js:10

SHA256 hashing of the entire message content on every cache lookup adds CPU overhead for large messages. For typical LLM messages (a few KB), this is negligible. For very large payloads (100KB+), it becomes noticeable.

Recommendation: Consider a size threshold or truncated hash for large messages.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions