getClientIP (the rate-limit and audit key) took the LEFTMOST x-forwarded-for
hop and read x-real-ip unconditionally. In our topology — client → Cloudflare
→ same-host origin proxy → app — both proxies APPEND, so the leftmost hop is
whatever the sender typed: a rotating X-Forwarded-For minted a fresh rate-limit
bucket per request, and unlimited bug-report quota via the same pattern in
routes/api/the-nook/report.ts. x-real-ip is client-settable too; nothing in
the chain overwrites it. It is no longer read at all.
New trust order (src/server/trusted-client-ip.ts, resolveTrustedClientIp):
1. a non-loopback socket peer IS the client — its headers are ignored;
2. cf-connecting-ip (Cloudflare overwrites it at the edge; parsed as an
address or rejected);
3. x-forwarded-for counted from the RIGHT, discounting one hop for the
Cloudflare edge address the origin proxy appended (the topology has
exactly one trusted proxy below Cloudflare);
4. "unknown" — callers key stricter shared limits on it, so resolution
fails closed toward a tighter bucket, never a looser one.
Addresses are never truncated: a /24-truncated key would pool attackers with
the innocents behind them.
Share-vs-copy, stated deliberately: this is a port of the parsing primitives
FRE-366 reviewed in request-client.ts, NOT an import. resolveClientIp carries
a standing rule (1d9287a) that it never feeds a security decision; lifting
that rule for these callers would couple an access-control change to an
analytics branch under review, and the analytics helper only exposes
truncated prefixes while rate limiting needs the full address. If the two
drift, trusted-client-ip.ts is the one security paths follow. The deploy-side
precondition (origin reachable only via Cloudflare + the same-host proxy) is
documented in the file header; if the app is ever directly internet-reachable
no in-app read can recover the true client.
Tests updated deliberately, not to go green: rate-limit.test.ts now pins the
rightmost-minus-one order, cf-connecting-ip precedence, peer-over-headers,
x-real-ip rejection, and the spoof case (a rotating leftmost hop does not
change the resolved client); report.test.ts pins the quota key stability
under a rotating header. New trusted-client-ip.test.ts covers the helper's
own edges: APIEvent wrapper, loopback forms, unparseable hops failing closed.
The security review of 39d874a (FRE-369) found the address fix recorded
the wrong thing twice. Both are fixed here; the branch is the same one
under review, this commit does not rewrite it.
Q1 — the first x-forwarded-for hop is attacker-chosen text. In
client → Cloudflare → same-host proxy → app, both proxies *append*, so
the header arrives as `<client spoof…>, <true client>, <CF edge>` and
`split(",")[0]` took the sender's own invention — the loopback-peer gate
was sound, the hop selection defeated it. Preference is now
`cf-connecting-ip` (Cloudflare overwrites it, so a client value never
survives the edge), then x-forwarded-for counted from the right with one
trusted hop discounted, then the direct peer. Every candidate must parse
as an address. h3's `getRequestIP(…, { xForwardedFor: true })` has the
same leftmost flaw and is documented as not a fix.
Q4 — the table held full client addresses indefinitely. Every analytics
entry now passes through `truncateIpAddress` in `enrichAnalyticsEntry`
(IPv4 /24, IPv6 /56, unparseable dropped), so no writer can persist a
full address by accident. Retention got the schedule it never had:
`/api/analytics/retention` on a daily Vercel cron, Bearer-authenticated
and fail-closed, with the window parameterised (the old
`cleanupOldAnalytics` spliced the number into the SQL). Default 365 days
rather than the review's 90 so enabling the cron is not itself a
destructive event — the tighter window is an operational decision, named
on the task.
Also from the review: the module doc now forbids feeding this helper into
any security decision; `isLoopbackAddress` parses instead of
string-matching (`::ffff:127.9.9.9`, `0:0:0:0:0:0:0:1`); and `host` is
normalised (lowercased, port stripped) so `nook.freno.me:443` cannot
silently miss the `host = ?` scope. The weekly loopback caveat matches
the truncated shapes writers produce now, and says plainly that "unique
visitors" counts prefixes.
Gap 1 — click and served share one label: trackDownload now takes a
stage ("click" | "served") carried on a new VisitorAnalytics.stage
column; the Nook door and the tRPC proxy record clicks, the DMG file
route records served. summariseDownloads() and the /analytics downloads
block are untouched — the stage is a separate column, so the path
grouping cannot change.
Gap 2 — checkout start records nothing: the checkout route records a
created Stripe session through the shared trackRequest writer (same
referrer, campaign, country dimensions as every door) under its own
path, and STAGES.checkout reads it instead of the /checkout page proxy.
Gap 3 — loopback addresses, no host dimension: every writer now
resolves the client address through resolveClientIp, which trusts
x-forwarded-for only when the direct peer is the loopback production
proxy (trust boundary stated in request-client.ts), and records the
host on each row through the same site-context extractor routing uses.
stage and host reach the table through VISITOR_ANALYTICS_ADDED_COLUMNS;
the weekly job applies the same bootstrap before reading so it renders
seven stages against a database no deploy has migrated yet.
The weekly read splits downloadClick/dmgServed, scopes visits to
nook.freno.me, and computes its caveats from the window: pre-instrumentation
rows are stated and excluded, and the loopback warning now fires only
when rows recorded since the instrumentation still carry it.
The analytics page could not answer "how many downloads, of which
product". Three of the four download doors recorded nothing: the tRPC
click proxy only left the generic request row, whose path is the raw URL
with the asset buried in `?batch=1&input={"asset_name":...}`, and the S3
file route that serves Sparkle updates and the Nook app logged nothing
at all. The one door that did log (the Nook redirect) kept a private copy
of the logVisit/enrich/flush sequence, under its own path.
All doors now go through trackDownload, which writes one row per event
under /api/downloads/<asset> and flushes it inside the request — a click
is low-volume and serverless is not guaranteed to run the 30s batch
timer after the response. The file route maps its prefix allowlist to
asset keys, so a version bump stays one asset rather than one per
release. getAnalyticsSummary groups those rows into a downloads block and
the freno.me panel renders it under the overview cards.
History is recovered to the degree it exists: the Nook door's rows and
the tRPC clicks still carry enough to derive from (asset name in the URL).
scripts/backfill-downloads.ts inserts those derived rows with a
`backfill-<source id>` primary key and INSERT OR IGNORE, leaving the
originals untouched, so re-running is a no-op. Run against the live table
it added 66 rows: gaze 42, thenook 19, inputhalo 4, lineage 1.
Tests: new backfill suite (derivation, visitor detail carried over,
idempotency, empty history) and summariseDownloads cases; the Nook route
test now pins the canonical path, and the NOOK-337 allowlist audit reads
the map keys instead of quoted strings. Red-checked by stubbing the
downloads block back to zeros — the summary case fails, then passes.
Full suite matches master.
Co-Authored-By: nw-flash <noreply@nw-flash.local>
MISC-48
Settings > Changelog and /changelog showed literal ' wherever a
bullet had an apostrophe: embed_release_notes.py escapes appcast
descriptions with Python's html.escape, which prefers the hex form for
apostrophes, and decodeEntities only knew the decimal ' — so the
entity survived into releases.json and was rendered as-is. Add '
(and ') to the replacement table.
Tests: new sectionsFromHtml case pinning html.escape-style output;
fails without the fix, full suite otherwise matches master.
Co-Authored-By: glm-5.3-flash <noreply@z.ai>
NOOK-565
The landing page sold eight features from around 0.6 and summed up the
rest in one line, while 0.6–0.10 shipped most of the island's modules. It
also said twelve agents (Muse made thirteen) and marked phone approvals
"coming soon" although the iPhone companion is in TestFlight beta.
Adds live demos for terminal replies, the volume mixer, Capture with its
Wings, and floating actions; a bento of module tiles behind a two-way
feature marquee; and an iPhone/Watch beta section whose request form
posts the same email-only report the app's Settings > Phone card sends.
The approval flow, remote sessions and setup, timer, temperatures and
Capture mocks are rebuilt from the app's own views, so the site shows
what the island actually draws.
Tests: report.test.ts pins the landing page's beta request against the
/report schema (red when the schema demands a machine snapshot); nook and
subdomain suites pass.
NOOK-334
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0.8.0 and 0.7.0 rendered as a version header over an empty row on
/changelog and in releases.json, the feed the macOS app's Changelog pane
renders. Older still, 0.6.0/0.6.2/0.6.3 were gone from the page outright.
Both come from one gap. The appcast is the only live source and it is not a
store: it holds just the DMGs the release machine keeps, and
generate_appcast drops the description of any rebuilt item whose DMG ships
no notes file of its own — so a release it still lists can arrive with
nothing to show, and one it has aged out of vanishes. The checked-in
archive was meant to cover that, but it stopped at 0.5.0, and mergeReleases
skipped an archived version whenever the appcast mentioned it, so it could
never heal a row that existed and was blank.
The archive now carries every release that shipped notes, 0.9.0 down to
0.2.0, and withArchivedNotes fills an item only where the feed has nothing,
leaving a published description alone. /changelog and releases.json run the
same rule rather than two copies, so the page and the feed cannot disagree
about what a release said. 0.6.1 and 0.3.1 stay absent by design — they
shipped with no notes written.
Tests: bun test src/routes/api/the-nook/ (51 pass; the 4 new cases fail on
this branch's parent); bun test unchanged at 49 failures, all in
src/server/security, which needs an .env a worktree does not carry.
Proved end to end against the live appcast as published by the 0.9.0
release: 11 releases, all with sections.
NOOK-305
Co-Authored-By: qwen3.8-flash <noreply@freno.me>
The lifecycle job marked rows with an unconditional UPDATE before
sending, so two overlapping invocations (cron tick + manual hit, or
retry) both passed the SELECT, both ran the UPDATE — re-writing the
same value — and both sent the same email. Real duplicates went out to
licensees. Rows are now claimed with a conditional UPDATE (... AND
asked_at IS NULL), and the send happens only when the claim reports
rowsAffected 1; a racing invocation claims nothing and sends nothing.
Tests: lifecycle suite gains a lost-claim case; 36 nook API tests pass.
Issue: FRE-11
Trials were anonymous — a 14-day trial you cannot talk to. trials gains
an email column (idempotent ALTER), POST /api/the-nook/trial-email
attaches the optional first-launch prompt's answer, and the address
lands in a Brevo list when NOOK_TRIAL_LIST_ID is set. The lifecycle
emails follow: a D+11 trial expiry reminder with a buy link, a purchase
thank-you from the Stripe webhook, and scheduled testimonial (D+3) and
review (D+14) asks via the daily /api/the-nook/lifecycle cron. Rows are
marked before sending so retries never double-send; trial addresses
that already bought get no buy reminder; the endpoint requires a
Bearer secret (CRON_SECRET / NOOK_LIFECYCLE_SECRET) and fails closed
when unconfigured — an open trigger would email real users. All sends
are best-effort and logged, never thrown.
Tests: trial-email (3) and lifecycle (6) suites; full run 539 pass.
Issues: FRE-10, FRE-11
Three organic surfaces, each answering an objection at the moment it
bites. /agents lists all twelve agents with each hook's exact event
surface (mirrors AgentSpec.all + hook/setup.go + the plugins) and what
a card can do — allow/deny or observe. /changelog renders the appcast
HTML the Sparkle updater already publishes, so release notes have one
source of truth. /faq answers privacy (scoped honestly: no prompt or
code content leaves the device), the notch question, battery,
permissions, the macOS 14 floor and offline use — no refund question,
per decision: the trial is the answer. Nav gains the three links.
Tests: appcast parser suite (5 tests) covers CDATA, escaped HTML,
Version-prefix titles and garbage input.
Issues: FRE-12, FRE-13, FRE-14
