Skip to content

fix: correct expired DeepSeek prices, enforce curated expiries, harden Host + sync lock - #91

Merged
pitimon merged 5 commits into
mainfrom
fix/87-pricing-expiry-and-local-hardening
Jul 25, 2026
Merged

fix: correct expired DeepSeek prices, enforce curated expiries, harden Host + sync lock#91
pitimon merged 5 commits into
mainfrom
fix/87-pricing-expiry-and-local-hardening

Conversation

@pitimon

@pitimon pitimon commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Summary

Two independent fixes from a Fable-model QA review of 0.39.40, kept in one PR because both are small and touch nothing in common. One commit each.

  • Pricing data (Expired DeepSeek v4-pro promo prices still active; curated expiries are not machine-checked #87)deepseek-v4-pro was still pinned to its 75%-off launch promo 55 days after the promo ended, billing every DeepSeek row at 25% of true cost. The file documented its own cutover date in prose and nothing read it. Rates corrected, and the free-text _meta.*_expiry keys are replaced with a structured _meta.expiries array that ci:local now enforces — including the Sonnet 5 2026-08-31 cutover that was queued up to repeat the same failure.
  • Loopback hardening (No Host-header validation: DNS rebinding can read all dashboard data #88) — binding to 127.0.0.1 does not make the Host header trustworthy. Under DNS rebinding a browser sends Host: attacker.example to loopback and treats the response as same-origin, so CORS never applies. Mutations were already gated on a loopback Origin, but every GET /functions/* endpoint was readable by any page the victim had open. Non-loopback hosts now get a 403 before any routing.
  • Sync lock (Sync lock: 5-min staleness with no heartbeat, plus stale-takeover TOCTOU #89) — the lock had a 5-minute staleness window and never refreshed its own mtime, while local-sync fires on an interval. Any sync longer than one tick (full-corpus rebuilds, migration reparses) had its lock stolen, letting two writers interleave appends into queue.jsonl; a torn line is silently skipped by the reader, and a skipped retraction row is a permanent overcount. Now heartbeated, with an atomic takeover.

Why these three, in this shape

A stale price is worse than a missing one: a missing price shows $0 and looks broken, a stale price looks fine forever. Both #87 and the recent claude-opus-5 $0 incident come from the same place — a pricing fact that depends on a human remembering something — so the fix is a check, not a corrected number.

Two implementation notes worth reviewing:

  1. The request handler moved out of cmdServe into createRequestHandler. That is what lets the test boot a real http.Server and assert the API handler is never reached on a rebound host. A guard that exists but is never wired is precisely the failure mode being prevented, so a predicate-only test would not have been worth much.
  2. The lock takeover uses an atomic mkdir mutex, not a rename. A rename-based claim was written first and still failed a 4-way race test with 2 winners: rename is atomic, but it does not bind the staleness check to the act, so a waiter that stat'd earlier can rename away the winner's freshly created lock. The mkdir mutex puts the re-check and the delete in the same critical section.

Test plan

  • npm run ci:local green end to end (dashboard build + dashboard suite + 794 root tests + all validators)
  • test/curated-expiry.test.js — 13 cases: not-yet-expired, expired, inclusive UTC-midnight boundary, missing/blank fields, malformed and impossible dates (2026-02-31), duplicate ids, regression to free-text *_expiry, non-array, non-object entries, plus a numeric pin on the corrected DeepSeek rates
  • test/host-header-guard.test.js — 7 cases against a real server: rebound host 403s and the API handler is never invoked, OPTIONS preflight also rejected, all four loopback spellings pass on an arbitrary port, lookalikes rejected (127.0.0.1.attacker.example, localhost:17680@attacker.example), absent Host allowed, malformed Host rejected rather than thrown
  • test/sync-lock.test.js — 9 cases: owner recorded, release removes the file, a live holder is not stolen past the old 5-minute window, heartbeat advances mtime, release stops the heartbeat, expired lock reclaimed, dead-owner lock reclaimed immediately, another host's lock left alone, corrupt lock falls back to the mtime rule, and exactly one winner in a concurrent takeover
  • Race stress outside the suite: 50 rounds × 8 concurrent racers on one stale lock — exactly one winner every round, no mutex left behind
  • npm run validate:curated-expiry passes today and fails on a backdated entry
  • Reviewer: confirm the 30-minute stale window is acceptable given the new dead-owner fast path (a killed holder is now reclaimed immediately rather than after the window, so recovery is faster than before, not slower)

Not in this PR

The systemic pricing fix — no in-process refresh, unknown models billing $0, invisible fuzzy tiers — is #90, deliberately separate because it touches the pricing path that just had a live incident and deserves an isolated diff.

Closes #87
Closes #88
Closes #89

itarun.p added 2 commits July 25, 2026 05:23
…expiries

deepseek-v4-pro was still pinned to its 75%-off launch promo 55 days after
the promo ended, so every DeepSeek row billed at 25% of true cost. The file
documented the cutover date itself, in prose, and nothing read it.

A stale price is worse than a missing one: a missing price shows $0 and looks
broken, a stale price looks fine forever.

- Correct deepseek-v4-pro to the standard rates (1.74 / 3.48 / 0.0145 / 1.74)
- Replace the free-text _meta.*_expiry keys with a structured _meta.expiries
  array (id / expires_at / what / action), carrying the Sonnet 5 2026-08-31
  cutover that was about to repeat the same failure
- Add validate:curated-expiry to ci:local so a PR opened on or after an
  expiry date fails until a human applies the action and clears the entry;
  it also rejects any regression to free-text *_expiry keys
- Stop duplicating price literals in model-breakdown's coverage test — it
  asserts the lookup path (aliases, prefixes, casing) and reads expected
  rates from the curated table, so a legitimate price fix touches one file

Closes #87
Two small hardening fixes for windows that are cheap to close now and
expensive to diagnose afterwards.

Host header (#88): binding to loopback does not make the Host header
trustworthy. Under DNS rebinding a browser sends Host: attacker.example to
127.0.0.1 and treats the response as same-origin, so CORS never applies.
Mutations were already gated on a loopback Origin, but every GET
/functions/* endpoint — full spend history, model mix, project names — was
readable by any page the victim had open. Requests whose Host is not
loopback now get a 403 before any routing. isLoopbackHostname is reused from
local-api so the Host allowlist and the Origin allowlist cannot drift.
The request handler moved out of cmdServe into createRequestHandler so the
test boots a real server and asserts the API handler is never reached — a
guard that exists but is never wired is exactly the failure being prevented.

Sync lock (#89): the lock had a 5-minute staleness window and never
refreshed its own mtime, while local-sync fires on an interval. Any sync
longer than one tick — full-corpus rebuilds and migration reparses are —
had its lock stolen, letting two writers interleave appends into
queue.jsonl. A torn line is silently skipped by the reader, and a skipped
retraction row is a permanent overcount.

- Heartbeat the lock mtime every 30s (unref'd, cleared on release) and raise
  the stale window to 30 minutes: "stale" now means the holder died
- Record pid/host/startedAt in the lock, and reclaim immediately when the
  recorded process is gone — faster recovery than the old window, not slower
- Gate the stale takeover behind an atomic mkdir mutex. The previous
  check-then-act let two waiters both delete and both acquire; a rename-based
  claim was tried first and still failed a 4-way race test, because rename is
  atomic but does not bind the check to the act
- Clean up a failed lock write instead of leaking the fd and an empty file

Closes #88
Closes #89
itarun.p added 3 commits July 25, 2026 06:12
Independent QA pass (Codex, xhigh) on the merged preview of #91 + #92.

- Refuse absolute-form request targets. Host said loopback while the target
  carried its own authority, and routing parses the absolute URL — so the
  allowlist and the router disagreed about which site the request was for.
  Not reachable from a browser (absolute-form only goes to proxies), but a
  parser differential is not something to leave open in the one guard that
  stands between a rebound page and the whole spend history.
- Allow the fully-qualified loopback spelling. WHATWG URL canonicalises a
  trailing dot away for IPv4 literals but not for names, so `localhost.`
  got a spurious 403 while `127.0.0.1.` passed.
- Refuse userinfo in a Host header. `evil.example@127.0.0.1` was accepted;
  the origin genuinely is loopback so this was never a bypass, but Host has
  no userinfo component and anything carrying one is malformed.
- Scan _meta values, not key names, for unenforced dates. Matching only
  `*_expiry` meant `promo_cutover: "2026-08-31 — update the price"` sailed
  past and would have expired in silence — the exact failure the validator
  exists to prevent. Now any YYYY-MM-DD parked anywhere in _meta (including
  nested) must live in the expiries array.
…gets

QA re-check found two gaps in the previous round's guard.

- "@localhost" and ":@localhost" parse to a falsy url.username, so checking
  the parsed fields let exactly the malformed forms through while the
  fully-spelled "user:pass@localhost" was refused. Test the raw header for
  "@" instead.
- "//evil/x" and "/\\evil/x" start with a slash and so passed the
  origin-form check, but WHATWG URL resolves both against a foreign authority
  (new URL("/\\evil/x", "http://localhost").hostname === "evil").
  Routing only reads url.pathname today, so nothing is exploitable now — but
  handing a handler a URL that points at someone else's origin is the same
  guard-vs-parser disagreement absolute-form creates.
…target

QA re-check found the last hole in the prefix checks: WHATWG URL strips tab,
LF and CR from its input BEFORE parsing, so "/<tab>//evil/x" becomes
"//evil/x" and adopts a foreign authority after passing a startsWith("//")
test. Same trick applies to the Host string.

Verified over a socket that Node's own parser returns 400 for those bytes in a
request-target before the handler ever runs, so this was not reachable through
the real server — recorded in the test so the next reader does not have to
re-derive it. Fixed anyway: a guard that holds only because a different layer
happens to be strict is exactly the guard-vs-parser disagreement this function
exists to prevent.
@pitimon
pitimon merged commit 2cf4e14 into main Jul 25, 2026
1 check passed
@pitimon
pitimon deleted the fix/87-pricing-expiry-and-local-hardening branch July 25, 2026 00:10
pitimon pushed a commit that referenced this pull request Jul 25, 2026
Ships the QA-gated fixes from #91 and #92: corrected DeepSeek rates with a
machine-checked expiry, Host-header and sync-lock hardening, and pricing that
refreshes in-process and reports how each price resolved.

prepublishOnly re-vendored the LiteLLM seed, which now carries claude-opus-5 —
so a cold start prices it correctly even before the first background refresh.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant