openvibers/openvibe.deals Commits

Deals: per-address limits on pages, sitemaps and feeds; search is bounded (#25)

OP OpenVibers committed 10 Oct 2026 · 06e9c3ae7c · 6 files, +56 −9

From an independent security review of Deals: public reads had no rate limit, and some cost thousands of queries per request.

- **nginx:**
  - pages are limited to 10 a second per address;
  - the sitemaps, feeds and `llms-full.txt` to 30 a minute;
  - `/internal/` and `/metrics` are refused case-insensitively.
- **Search:** at most 100 characters and 5 words, with results capped, under the app's read limiter.

Tests: `test/pages.test.js` and `test/actor-limits.test.js`. Suite: 24/24 files pass; the vhost passes `nginx -t` on the host.

Changes

ddiff --git a/deploy/nginx/openvibe.deals.conf b/deploy/nginx/openvibe.deals.conf
iindex 52b03e9..9a558dc 100644
---- a/deploy/nginx/openvibe.deals.conf
++++ b/deploy/nginx/openvibe.deals.conf
@@@ -16,6 +16,10 @@
  limit_req_zone $binary_remote_addr zone=ovdeals_api:10m rate=10r/s;
  limit_req_zone $binary_remote_addr zone=ovdeals_auth:10m rate=10r/m;
  limit_req_zone $binary_remote_addr zone=ovdeals_write:10m rate=5r/s;
++## Public reads: pages and the index artifacts (sitemaps, llms-full.txt, feeds). The artifacts build a
++## view per indexable offer (up to 50,000), so they get their own, far slower budget.
++limit_req_zone $binary_remote_addr zone=ovdeals_pages:10m rate=10r/s;
++limit_req_zone $binary_remote_addr zone=ovdeals_index:10m rate=30r/m;
  
  server {
      listen 80;
@@@ -86,8 +90,9 @@ server {
          proxy_pass http://127.0.0.1:4840;
      }
  
--    # ── OpenVibe.Events deliveries come from the host (127.0.0.1:4840/internal/events), never from the internet ──
--    location /internal/ { return 404; }
++    # ── OpenVibe.Events deliveries come from the host (127.0.0.1:4840/internal/events), never from the internet.
++    #    Match case-insensitively: Express routes are case-insensitive, so /Internal/events would reach the app. ──
++    location ~* ^/internal(/|$) { return 404; }
  
      # ── API ─────────────────────────────────────────────────
      location /api/ {
@@@ -95,11 +100,21 @@ server {
          proxy_pass http://127.0.0.1:4840;
      }
  
--    # ── Prometheus metrics: host-local scrapers only (http://127.0.0.1:4840/metrics) ──
--    location = /metrics { return 404; }
++    # ── Prometheus metrics: host-local scrapers only (http://127.0.0.1:4840/metrics). Case-insensitive for
++    #    the same reason as /internal/: Express would match /Metrics. ──
++    location ~* ^/metrics(/|$) { return 404; }
++
++    # ── Index artifacts: the app builds a view per indexable offer (up to 50,000), so they read slowly ──
++    #    No proxy_set_header here: the server level already sets them, and setting any one of them inside a
++    #    location would drop all the inherited ones.
++    location ~* ^/(sitemap[^/]*\.xml|sitemaps/|llms-full\.txt|feed\.) {
++        limit_req zone=ovdeals_index burst=10 nodelay;
++        proxy_pass http://127.0.0.1:4840;
++    }
  
      # ── Pages, feeds, sitemaps, robots.txt, llms.txt, static assets ──
      location / {
++        limit_req zone=ovdeals_pages burst=40 nodelay;
          proxy_pass http://127.0.0.1:4840;
          proxy_set_header X-Real-IP $remote_addr;
          proxy_set_header X-Forwarded-For $remote_addr;
ddiff --git a/server/domain/listings.js b/server/domain/listings.js
iindex 7637a63..8c3cba8 100644
---- a/server/domain/listings.js
++++ b/server/domain/listings.js
@@@ -10,6 +10,11 @@
   */
  const { tokens } = require('./values');
  
++/** Search bounds: a search runs a LIKE scan per word, so a caller cannot grow the work without limit. */
++const SEARCH_QUERY_MAX = 100;   // characters kept from the raw query
++const SEARCH_WORDS_MAX = 5;     // words matched (every word must match, so extras only narrow); extras ignored
++const SEARCH_RESULTS_MAX = 100; // rows per query, whatever the caller asks
++
  function createListings({ store }) {
      const { db } = store;
      const LATEST = 'h.id = (SELECT MAX(id) FROM deal_hotness_snapshots WHERE offer_id = o.id)';
@@@ -31,14 +36,16 @@ function createListings({ store }) {
      };
  
      async function search(query, { limit = 20, offset = 0, includeExpired = false } = {}) {
--        const words = [...new Set(tokens(query))].slice(0, 6);
++        const words = [...new Set(tokens(String(query || '').slice(0, SEARCH_QUERY_MAX)))].slice(0, SEARCH_WORDS_MAX);
          if (!words.length) return { total: 0, rows: [] };
++        const take = Math.min(Math.max(1, Math.trunc(limit) || 20), SEARCH_RESULTS_MAX);
++        const skip = Math.max(0, Math.trunc(offset) || 0);
          const where = words.map((_, i) => `(' ' || lower(o.title) || ' ' || lower(COALESCE(o.description, '')) || ' ') LIKE @w${i}`).join(' AND ');
          const params = Object.fromEntries(words.map((w, i) => [`w${i}`, `%${w.replace(/[%_\\]/g, (c) => `\\${c}`)}%`]));
          const status = includeExpired ? "o.status <> 'disabled'" : "o.status = 'active'";
          const base = `FROM deal_offers o WHERE o.merged_into IS NULL AND ${status} AND ${where.replace(/LIKE (@w\d+)/g, "LIKE $1 ESCAPE '\\'")}`;
          const total = (await db.prepare(`SELECT COUNT(*) AS n ${base}`).get(params)).n;
--        const rows = await db.prepare(`SELECT o.* ${base} ORDER BY o.created_at DESC LIMIT @limit OFFSET @offset`).all({ ...params, limit, offset });
++        const rows = await db.prepare(`SELECT o.* ${base} ORDER BY o.created_at DESC LIMIT @limit OFFSET @offset`).all({ ...params, limit: take, offset: skip });
          return { total, rows, words };
      }
  
ddiff --git a/server/http/actor-limits.js b/server/http/actor-limits.js
iindex d697b51..24231d5 100644
---- a/server/http/actor-limits.js
++++ b/server/http/actor-limits.js
@@@ -26,7 +26,8 @@
   *
   * Never limited: /api/health, /api/ready, /release.json, /metrics, sign-in, the signed Events
   * deliveries at /internal/events (Sources wake-ups: Events pushes at its own pace, and a 429 would only
-- * make it retry and fall behind), and the pages and feeds people read.
++ * make it retry and fall behind), and the pages and feeds people read — except /search, which runs a
++ * LIKE scan per word and so takes the read defaults like the API.
   */
  const { createActorLimiter, createValkeyLimitStore, defaultActor } = require('openvibe-sdk/limits');
  
ddiff --git a/server/http/pages.js b/server/http/pages.js
iindex 21aade1..f2e3795 100644
---- a/server/http/pages.js
++++ b/server/http/pages.js
@@@ -298,8 +298,8 @@ function createPages(ctx) {
  
      // ── search ──────────────────────────────────────────────
  
--    router.get('/search', wrap(async (req, res) => {
--        const q = String(req.query.q || '').slice(0, 200);
++    router.get('/search', actorLimits.reads('deals.search'), wrap(async (req, res) => {
++        const q = String(req.query.q || '').slice(0, 100);
          const page = pageNumber(req);
          const r = q ? await listings.search(q, { limit: PER_PAGE, offset: (page - 1) * PER_PAGE }) : { total: 0, rows: [] };
          const pager = q ? ssr.paginate({ page, perPage: PER_PAGE, total: r.total, href: (p) => `/search?q=${encodeURIComponent(q)}&page=${p}` }) : null;
ddiff --git a/test/actor-limits.test.js b/test/actor-limits.test.js
iindex eb76888..70ec130 100644
---- a/test/actor-limits.test.js
++++ b/test/actor-limits.test.js
@@@ -34,6 +34,14 @@ const { actor, serviceItself } = require('../server/http/actor-limits');
          assert.strictEqual((await t.get('/api/v1/offers', { as: ben })).status, 200, 'another person still passes');
      });
  
++    await check('/search takes the read defaults too: 3 a minute per person, then 429 rate_limited', async () => {
++        const cat = t.network.addUser('cat');
++        for (let i = 0; i < 3; i++) assert.strictEqual((await t.get('/search?q=drone', { as: cat })).status, 200);
++        const r = await t.get('/search?q=drone', { as: cat });
++        assert.strictEqual(r.status, 429, r.text);
++        assert.ok(r.json().detail.includes('deals.search'), r.json().detail);
++    });
++
      await check('a service naming the person counts against that person', async () => {
          const r = await t.get('/api/v1/watches', { as: svc, headers: { 'X-OV-Subject': ann.subject } });
          assert.deepStrictEqual([r.status, r.json().code], [429, 'rate_limited']);
ddiff --git a/test/pages.test.js b/test/pages.test.js
iindex c6cb59c..47f7173 100644
---- a/test/pages.test.js
++++ b/test/pages.test.js
@@@ -99,6 +99,22 @@ const { boot, check, done, robotsOf, jsonLd, HOUR } = require('./helpers/boot');
          assert.strictEqual((await t.get('/p/nope')).status, 404);
      });
  
++    await check('search caps its query: 100 characters and 5 words, extra words ignored', async () => {
++        // Five words that all match the deal (title "Mini drone with camera", description "Folding **mini** drone.").
++        const five = await t.get(`/search?q=${encodeURIComponent('mini drone with camera folding')}`);
++        assert.strictEqual(five.status, 200);
++        assert.match(five.text, /Mini drone with camera/);
++        // A sixth word that matches nothing is ignored rather than required, so the deal still matches.
++        const six = await t.get(`/search?q=${encodeURIComponent('mini drone with camera folding zzznomatch')}`);
++        assert.strictEqual(six.status, 200);
++        assert.match(six.text, /Mini drone with camera/, 'the 6th word is ignored');
++        assert.doesNotMatch(six.text, /No active deals match/);
++        // The raw query is truncated to 100 characters before it is echoed or searched: the search box
++        // holds exactly 100 characters (the quote right after the 100th proves no more survive there).
++        const capped = await t.get(`/search?q=${'a'.repeat(150)}`);
++        assert.ok(capped.text.includes(`value="${'a'.repeat(100)}"`), 'the echoed query is capped at 100 characters');
++    });
++
      await check('the submitter marks it expired: badge, noindex (expired), deals.offer.expired, out of lists and sitemaps', async () => {
          const other = await t.get(`/d/${deal.slug}/expire`, { as: bob, form: {} });
          assert.strictEqual(other.status, 403, 'only the submitter or a moderator');