chnm / chnm/apiary

Bound response cache memory and enforce result-size limits

Open
#129 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

go performance reliability security
Dominant language
Go
Stars
10
Forks
1
PR merge metrics
No merged PRs in 30d

Description

Problem

Three independent defects combine into a single denial-of-service path against the response cache and the database. They are filed together because fixing any one in isolation leaves the path open.

1. The response cache is bounded by entry count, not bytes

server.go:56

memory.AdapterWithCapacity(int(math.Pow(1000, 2))), // 1000^3 = gigabyte

The comment is wrong twice over. math.Pow(1000, 2) is 1,000,000, not 1000³; and in the pinned victorspringer/http-cache version, AdapterWithCapacity sets the maximum number of cached responses, not a byte budget. The library's byte-capacity option (AdapterWithStorageCapacity) is not used, and cache.ClientWithMaxBodySize is not set, so body size is unlimited.

The effective configuration permits one million cached entries of unbounded size, each holding a full uncompressed response body for a one-hour TTL. Several endpoints serve large GeoJSON payloads. The adapter also pre-allocates two maps sized to capacity at startup, and its LRU eviction is a linear scan under a write lock, so a full cache is a latency hazard as well as a memory one.

2. No maximum limit, and /bom/causes has no default limit at all
  • internal/datasets/bom/causes.go:165
  • internal/datasets/bom/bills.go:307
  • internal/datasets/pinkertons/pinkertons.go:287
  • internal/datasets/bom/christenings.go:152

No endpoint enforces an upper bound on limit; ?limit=100000000 is accepted verbatim. Pinkertons rejects only values less than or equal to zero.

/bom/causes is worse: it emits a LIMIT clause only when the client supplies one, so a bare GET /bom/causes returns the entire bom.causes_of_death table joined across two more tables, with a COUNT(*) OVER() window over the whole result set.

Each response is then held in memory three times: the accumulated result slice, the full json.Marshal buffer, and the cache middleware's stored copy.

3. nocache is an unauthenticated cache-bypass

server.go:67

cache.ClientWithRefreshKey("nocache"),

When this key appears in the query string, the middleware releases the stored entry and re-executes the handler. Any anonymous caller can purge any URL's cache entry and force a full database round-trip, repeatedly.

Risk

Combined, these remove the cache as a protective buffer and turn cheap requests into unbounded full-table scans on demand. Cache keys include the full query string, so junk parameters mint unlimited distinct entries; with unbounded body size, resident memory is limited only by available RAM.

Acceptance criteria

  • Set memory.AdapterWithStorageCapacity to an explicit byte budget and cache.ClientWithMaxBodySize to a per-response ceiling.
  • Reduce AdapterWithCapacity to a realistic entry count and correct the misleading comment.
  • Add a shared limit helper in internal/params taking a default and a maximum, returning 400 Bad Request above the ceiling.
  • Apply the helper to the BOM bills, BOM causes, BOM christenings, and Pinkertons handlers.
  • Give /bom/causes a default limit so an unparameterized request cannot return the full table.
  • Decide the fate of ClientWithRefreshKey("nocache"): remove it, or gate it behind a shared secret checked before the cache middleware runs.
  • Add tests covering the limit ceiling and the absent-limit default.

Notes

Removing or gating the refresh key conflicts with the final acceptance criterion of #126, which asks the cache benchmarks to exercise a ?nocache variant. Decide this issue first; if the key changes, the benchmark needs another mechanism for its uncached arm.

Found during the repository audit recorded in AUDIT.md (H-1, H-2, H-3).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the cache configuration in server.go and the handlers named in internal/datasets/bom/causes.go, bills.go, christenings.go, and internal/datasets/pinkertons/pinkertons.go. Review the shared cache and parameter APIs before deciding how to handle nocache and the benchmark conflict in #126. Done means bounded cache and result sizes, safe default and maximum limits, and tests for the ceiling and absent-limit behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, backend, databases, performance, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.