Skip to content

v0.1.71

Choose a tag to compare

@rromenskyi rromenskyi released this 02 Oct 09:23
· 50 commits to main since this release
7f8585a

Admin API — 3 of 62 confirmed findings (5-7 of 62)

Re-derived from current code by a fresh investigation fork (the original review's scratchpad detail is gone). Bundled into one PR since all three are small and land in the same subsystem.

Negative max_concurrency silently became "unlimited"

handleAdminPutProvider clamped max_concurrency < 0 to 0 — but 0 means unlimited (a nil semaphore lets every Acquire succeed). A fat-fingered or buggy admin request silently got the single most permissive outcome instead of a validation error. Now rejected as a 400.

?limit on the captures list had no upper bound

handleAuditListCaptures validated limit > 0 but never capped it, unlike the established clampHours pattern (capped at 168) used for this package's analogous usage-series endpoints. Now capped at 500.

Model-catalog cache had no single-flight lock

handleAdminProviderModels's TTL cache let N concurrent requests racing an empty (or just-expired) cache each independently call the upstream model-list API. Now deduped via golang.org/x/sync/singleflight, the same package this codebase already uses for Vertex token resolution.

  • Test: TestPutProviderRejectsNegativeMaxConcurrency, TestAuditListCapturesClampsLimit, TestProviderModelsConcurrentRequestsDedupe (-race clean, 5 concurrent requests against a lister blocked until released)
  • Surgical-revert proof: each of the three reverts genuinely broke its own test (nil-DB panic past validation; ?limit=999999999 passed through unclamped; 5 independent upstream calls instead of 1)

Shipped via #144, chart bump #145.

Next up: Store/config's 4 uncatalogued Minor findings (one, a migration PK-change edge case, is already noted as low-priority/deferred — the rest need a fresh investigation fork the same way).

Full Changelog: v0.1.70...v0.1.71