openvibers/openvibe.reviews Commits
Threat review fixes: cross-site writes, correction queue limits, bounded audit text, editor ids
Changes
@@@ -255,6 +255,7 @@ CREATE TABLE IF NOT EXISTS review_corrections ();CREATE INDEX IF NOT EXISTS review_corrections_status ON review_corrections (status, created_at);CREATE INDEX IF NOT EXISTS review_corrections_entity ON review_corrections (entity_id, status);++CREATE INDEX IF NOT EXISTS review_corrections_by ON review_corrections (submitted_by, created_at);CREATE TABLE IF NOT EXISTS review_audit (id INTEGER PRIMARY KEY AUTOINCREMENT,@@@ -39,7 +39,7 @@*/const express = require('express');const contracts = require('openvibe-contracts');--const { actorMiddleware, guard, run } = require('./common');++const { actorMiddleware, guard, run, crossSite } = require('./common');const { checkCapability } = require('../auth/capabilities');const { http } = contracts;@@@ -49,6 +49,14 @@ function createApi({ svc, viewers, platform, sync, config, log = console }) {router.use(http.middleware());router.use(express.json({ limit: '256kb' }));router.use((err, req, res, next) => (err ? http.sendProblem(res, 400, 'request.invalid_json', { detail: 'Malformed JSON body', ctx: req.ov }) : next()));++ // The API also accepts the ov_token cookie: a write carried by it that another site started is++ // refused (SameSite=Lax is the first line; Bearer tokens are never sent by a browser on its own).++ const origin = new URL(config.baseUrl).origin;++ router.use((req, res, next) => {++ if (['GET', 'HEAD', 'OPTIONS'].includes(req.method) || req.headers.authorization || !(req.cookies && req.cookies.ov_token)) return next();++ if (!crossSite(req, origin)) return next();++ http.sendProblem(res, 403, 'request.cross_site', { detail: 'A write with the session cookie from another site is not accepted', ctx: req.ov });++ });router.use(actorMiddleware(viewers));const R = (fn, status) => run(fn, status, log);const body = (req) => (req.body && typeof req.body === 'object' ? req.body : {});@@@ -35,6 +35,19 @@ function guard(capabilityId) {};}++/**++ * A write another site started (CSRF), judged from what the browser says: Sec-Fetch-Site other than++ * same-origin (or none: typed by the person), or an Origin other than ours. `Origin: null` alone is++ * not proof (privacy settings send it for same-origin posts); a browser that sends Sec-Fetch-Site++ * says what it really was.++ */++function crossSite(req, origin) {++ const site = req.get('sec-fetch-site');++ if (site && site !== 'same-origin' && site !== 'none') return true;++ const o = req.get('origin');++ return !!(o && o !== 'null' && o !== origin);++}++/** Service errors → problem+json. */function sendError(res, req, err, log = console) {const status = err && Number.isInteger(err.status) ? err.status : 500;@@@ -56,4 +69,4 @@ function run(fn, status = 200, log = console) {};}--module.exports = { actorMiddleware, guard, sendError, run };++module.exports = { actorMiddleware, guard, sendError, run, crossSite };@@@ -25,7 +25,7 @@ const seo = require('openvibe-publishing/seo');const { renderPage } = require('../render/layout');const views = require('../render/views');const { ratingForStructuredData } = require('../reviews/aggregate');--const { actorMiddleware } = require('./common');++const { actorMiddleware, crossSite } = require('./common');const SCHEMA_TYPE = { product: 'Product', game: 'VideoGame', software: 'SoftwareApplication', service: 'Service', place: 'Place', organization: 'Organization', media: 'CreativeWork', other: 'Thing' };@@@ -90,10 +90,8 @@ function createPages({ svc, platform, sync, viewers, config, log = console }) {const origin = new URL(config.baseUrl).origin;// Cross-site form posts are refused (the session cookie is SameSite=Lax as well).router.use((req, res, next) => {-- if (req.method !== 'POST') return next();-- const o = req.get('origin');-- if (o && o !== origin && o !== 'null') return res.status(403).type('text/plain').set('Cache-Control', 'private, no-store').send('Cross-site form posts are not accepted.');-- next();++ if (req.method !== 'POST' || !crossSite(req, origin)) return next();++ res.status(403).type('text/plain').set('Cache-Control', 'private, no-store').send('Cross-site form posts are not accepted.');});const send = (req, res, status, body, o = {}) => {@@@ -155,7 +153,7 @@ function createPages({ svc, platform, sync, viewers, config, log = console }) {const e = locate(req, res, '.json');if (!e) return;const p = svc.page(e, req.actor);-- res.status(200).set('Cache-Control', req.actor.kind === 'anonymous' ? 'public, max-age=60' : 'private, no-store').set('X-Robots-Tag', p.decision.robots)++ res.status(200).set('Cache-Control', req.actor.kind === 'anonymous' ? 'public, max-age=60' : 'private, no-store').set('Vary', 'Cookie, Authorization').set('X-Robots-Tag', p.decision.robots).json({ ...p, decision: { indexable: p.decision.indexable, robots: p.decision.robots, reasons: p.decision.codes } });});@@@ -300,7 +300,7 @@ ${pending.length ? html`<ul>${pending.map((x) => html`<li><a href="${epath(x.ent<h2 id="flagged">Flagged summaries</h2>${flagged.length ? html`<ul>${flagged.map((s) => html`<li><a href="${epath(s.entity)}">${s.entity.name}</a>: ${s.flag_reason}</li>`)}</ul>` : html`<p class="rv-muted">None.</p>`}<h2 id="corrections">Open corrections</h2>--${corrections.length ? html`<ul>${corrections.map((c) => html`<li><a href="${epath(c.entity)}">${c.entity.name}</a> · ${c.target_type}${c.target_id ? ` ${c.target_id}` : ''} · ${t(c.created_at)}<blockquote>${c.body}</blockquote>${safeUrl(c.evidence_url) ? html`<a href="${safeUrl(c.evidence_url)}" rel="nofollow noopener">evidence</a>` : ''}++${corrections.length ? html`<ul>${corrections.map((c) => html`<li><a href="${epath(c.entity)}">${c.entity.name}</a> · ${c.target_type}${c.target_id ? ` ${c.target_id}` : ''} · ${t(c.created_at)}<blockquote>${c.body}</blockquote>${safeUrl(c.evidence_url) ? html`<a href="${safeUrl(c.evidence_url)}" rel="nofollow ugc noopener noreferrer">evidence</a> <span class="rv-muted">(sent by the reader; not checked)</span>` : ''}<form method="post" action="/editor/corrections/${e(c.id)}" class="rv-form">${c.summary_published ? html`<label>Correction note, published with a new summary revision (required to accept) <input type="text" name="correction_note" maxlength="2000"></label><p class="rv-muted">Accepting publishes the current summary text again as a correction revision with this note. To change the text as well, <a href="${epath(c.entity)}/edit?correction=${e(c.id)}#summary-form">correct the summary</a>.</p>` : ''}@@@ -363,7 +363,7 @@ ${correcting ? html`<div class="rv-notice rv-correction"><p>Correcting the summa<label>The overview cites ${sel('overview_signals', ov ? ov.citations.filter((c) => c.ok).map((c) => c.signal_id) : [])}</label><fieldset><legend>Pros</legend>${rows('pro', pros)}</fieldset><fieldset><legend>Cons</legend>${rows('con', cons)}</fieldset>--<label>Revision note <input type="text" name="message" maxlength="500"></label>++<label>Revision note (public: shown in the summary's history) <input type="text" name="message" maxlength="500"></label>${cur ? html`<label>Correction note (public; fill it in only to correct the published summary: the revision is published at once and the note stays in its history) <input type="text" name="correction_note" maxlength="2000"${correcting ? raw(' required minlength="10"') : ''}></label>` : ''}<input type="hidden" name="expected_revision" value="${String(p.summary ? p.summary.head_revision : 0)}"><button type="submit" name="publish" value="0">Save</button> <button type="submit" name="publish" value="1">Save and publish</button></form>@@@ -72,6 +72,17 @@ function keptTitle(item) {return item && item.kind === 'product' && item.title ? String(item.title).slice(0, 300) : null;}++/**++ * Why a stated rating cannot be counted on its own scale, or null. Below the stated worst (or below++ * zero when no worst is stated) or above the best is not an opinion the aggregate can use.++ */++function outOfScale(r) {++ if (r.best != null && (r.best <= 0 || r.value > r.best)) return `rating ${r.value} is outside the stated scale (best ${r.best})`;++ if (r.worst != null && r.value < r.worst) return `rating ${r.value} is outside the stated scale (worst ${r.worst})`;++ if (r.worst == null && r.value < 0) return `rating ${r.value} is below zero and the source states no scale minimum`;++ return null;++}++function trustOf(fields) {const out = {};for (const k of TRUST_FIELDS) if (fields[k] !== undefined && fields[k] !== null) out[k] = fields[k];@@@ -101,13 +112,13 @@ function extractSignal(item) {if (!r || r.value == null) return { signal: null, note: 'the product states no aggregate rating' };const n = r.count != null && r.count > 0 ? r.count : (r.review_count != null && r.review_count > 0 ? r.review_count : null);if (n == null) return { signal: null, note: 'the aggregate rating states no number of ratings' };-- if (r.best != null && (r.best <= 0 || r.value > r.best)) return { signal: null, note: `rating ${r.value} is outside the stated scale (best ${r.best})` };++ if (outOfScale(r)) return { signal: null, note: outOfScale(r) };return { signal: { type: 'rating_aggregate', rating_value: r.value, rating_best: r.best, rating_worst: r.worst, rating_count: n, trust }, note: null };}if (item.kind === 'review') {const r = f.rating;if (!r || r.value == null) return { signal: null, note: 'the review states no rating' };-- if (r.best != null && (r.best <= 0 || r.value > r.best)) return { signal: null, note: `rating ${r.value} is outside the stated scale (best ${r.best})` };++ if (outOfScale(r)) return { signal: null, note: outOfScale(r) };return { signal: { type: 'rating', rating_value: r.value, rating_best: r.best, rating_worst: r.worst, rating_count: 1, trust }, note: null };}return { signal: null, note: `items of kind "${item.kind}" carry no review signal` };@@@ -36,12 +36,21 @@ const MAX_POINT = 600;const MAX_OVERVIEW = 6000;const MIN_CORRECTION_NOTE = 10;const MAX_CORRECTION_NOTE = 2000;++// Correction requests one person may send per rolling day (the per-IP limit is in server/app.js).++const CORRECTIONS_PER_PERSON_DAY = 20;++const DAY_MS = 24 * 3600 * 1000;++const MAX_AUDIT_TEXT = 2000;// A summary is text with citations. Nothing in it can carry a score.const SUMMARY_KEYS = new Set(['overview', 'pros', 'cons', 'overview_signals', 'expected_revision', 'expectedRevision', 'message', 'publish', 'workflow', 'stub_provider', 'stubProvider', 'note', 'correction_note', 'correction_id']);const BODY_KEYS = ['overview', 'overview_signals', 'pros', 'cons'];const RATING_KEY_RE = /(rating|stars?|score|grade|verdict_value)/i;--// Numeric ratings in AI text ("4.5/5", "8 out of 10", "★★★★"): AI output never states a rating.--const RATING_TEXT_RE = /(\b\d+(?:[.,]\d+)?\s*(?:\/|out of)\s*(?:5|10|100)\b)|[★☆⭐]|\b\d+(?:[.,]\d+)?\s*stars?\b/i;++// Numeric ratings in AI text ("4.5/5", "8 out of 10", "★★★★", "four and a half stars", "nine out of++// ten"): AI output never states a rating.++const NUMBER_WORD = '(?:zero|one|two|three|four|five|six|seven|eight|nine|ten)(?:[\\s-]+and[\\s-]+a[\\s-]+half)?';++const RATING_TEXT_RE = new RegExp([++ '(\\b\\d+(?:[.,]\\d+)?\\s*(?:\\/|out of)\\s*(?:5|10|100)\\b)', '[★☆⭐]', '\\b\\d+(?:[.,]\\d+)?\\s*stars?\\b',++ `\\b${NUMBER_WORD}[\\s-]+stars?\\b`, `\\b${NUMBER_WORD}\\s+out\\s+of\\s+(?:five|ten|a\\s+hundred|one\\s+hundred|5|10|100)\\b`,++].join('|'), 'i');const toIso = (ms) => (ms == null ? null : new Date(ms).toISOString());@@@ -130,6 +139,8 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.nowallSummaries: db.prepare('SELECT * FROM review_summaries ORDER BY updated_at DESC LIMIT 500'),publishedSummaries: db.prepare("SELECT * FROM review_summaries WHERE state = 'published' ORDER BY revision_published_at DESC LIMIT ?"),correction: db.prepare('SELECT * FROM review_corrections WHERE id = ?'),++ correctionsSince: db.prepare('SELECT COUNT(*) AS n FROM review_corrections WHERE submitted_by = ? AND created_at > ?'),++ sameOpenCorrection: db.prepare("SELECT id FROM review_corrections WHERE submitted_by = ? AND entity_id = ? AND status = 'open' AND body = ?"),insertCorrection: db.prepare(`INSERT INTO review_corrections (id, entity_id, target_type, target_id, body, evidence_url, submitted_by, via, created_at)VALUES (@id, @entity_id, @target_type, @target_id, @body, @evidence_url, @submitted_by, @via, @now)`),resolveCorrection: db.prepare("UPDATE review_corrections SET status = ?, resolved_by = ?, resolution_note = ?, resolved_at = ? WHERE id = ? AND status = 'open'"),@@@ -162,8 +173,15 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.nowif (!access.isEditor(actor)) fail(403, 'reviews.editor_required', `${what} is for Reviews editors`);return actorId(actor);}++ // Audit rows are append-only and kept forever: no free text in one grows without bound.++ function clipText(v, depth = 0) {++ if (typeof v === 'string') return v.length > MAX_AUDIT_TEXT ? `${v.slice(0, MAX_AUDIT_TEXT)}…` : v;++ if (Array.isArray(v)) return v.map((x) => clipText(x, depth + 1));++ if (v && typeof v === 'object' && depth < 4) return Object.fromEntries(Object.entries(v).map(([k, x]) => [k, clipText(x, depth + 1)]));++ return v;++ }function audit(actor, action, entityId, target, detail = {}) {-- q.audit.run(now(), typeof actor === 'string' ? actor : actorId(actor), action, entityId || null, target || null, JSON.stringify(detail));++ q.audit.run(now(), typeof actor === 'string' ? actor : actorId(actor), action, entityId || null, target || null, JSON.stringify(clipText(detail)));}function emit(envelope) { return outbox.enqueue(envelope); }@@@ -905,7 +923,8 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now};}-- function summaryRevisionView(summary, rev, canonicalId) {++ /** `editor: false` (a reader): the person who wrote it is "an editor", as in the editorial log. */++ function summaryRevisionView(summary, rev, canonicalId, { editor = true } = {}) {if (!rev) return null;const rec = rev.meta && rev.meta.authorship;const review = reviews.latest(summary.id, rev.number);@@@ -917,7 +936,7 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.nowreview: review ? { decision: review.decision, reviewed_at: review.reviewedAt } : null,system: rev.meta && rev.meta.system ? rev.meta.system : null,correction: correctionView(rev),-- author: rev.author, message: rev.message, created_at: rev.createdAt,++ author: editor || !/^usr_/.test(String(rev.author)) ? rev.author : 'an editor', message: rev.message, created_at: rev.createdAt,};}@@@ -999,7 +1018,7 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.nowsummary: summary ? {id: summary.id, state: summary.state, head_revision: revisions.headNumber(summary.id), flagged: !!summary.flagged, flag_reason: summary.flag_reason, flagged_at: toIso(summary.flagged_at),published_at: toIso(summary.published_at), revision_published_at: toIso(summary.revision_published_at),-- published: pub ? summaryRevisionView(summary, pub, e.id) : null,++ published: pub ? summaryRevisionView(summary, pub, e.id, { editor }) : null,pending: editor ? pendingRevisions(summary).map((p) => summaryRevisionView(summary, p.rev, e.id)) : pendingRevisions(summary).length,history: publicHistory(summary),} : null,@@@ -1018,7 +1037,7 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.nowentity: entityView(e),aggregates: q.aggregates.all(e.id, 200).map(aggregateView),summary_revisions: summary ? revs.filter((r) => editor || publicRevision(summary, r))-- .map((r) => summaryRevisionView(summary, r, e.id)) : [],++ .map((r) => summaryRevisionView(summary, r, e.id, { editor })) : [],merges: q.mergeLinksOf.all(e.id, e.id).map((l) => ({id: l.id, from: entityView(q.entity.get(l.from_entity)), to: entityView(q.entity.get(l.to_entity)), note: l.note,merged_at: toIso(l.created_at), merged_by: editor ? l.created_by : null, split_at: toIso(l.ended_at), split_by: editor ? l.ended_by : null, split_note: l.end_note,@@@ -1037,9 +1056,9 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.nowif (!summary) fail(404, 'summary.not_found', 'This entity has no summary');const rev = revisions.get(summary.id, Number(n));if (!rev) fail(404, 'revision.not_found', `No revision ${n}`);-- const visible = access.isEditor(actor) || publicRevision(summary, rev);-- if (!visible) fail(404, 'revision.not_found', `No revision ${n}`);-- return summaryRevisionView(summary, rev, e.id);++ const editor = access.isEditor(actor);++ if (!editor && !publicRevision(summary, rev)) fail(404, 'revision.not_found', `No revision ${n}`);++ return summaryRevisionView(summary, rev, e.id, { editor });},// Resolve (reviews.entity.resolve) ---------------------------------------------------@@@ -1535,9 +1554,14 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.nowconst e = entityOrFail(ref);if (e.state === 'deleted') fail(410, 'entity.deleted', 'This entity was deleted');const t = now();++ const entityId = canonicalOf(e.id) || e.id;++ // One person cannot bury the editors' queue: a daily allowance, and the same open request once.++ if (q.sameOpenCorrection.get(actor.subject, entityId, text)) fail(409, 'correction.duplicate', 'You already sent this correction; it is waiting for an editor');++ if (q.correctionsSince.get(actor.subject, t - DAY_MS).n >= CORRECTIONS_PER_PERSON_DAY) fail(429, 'correction.rate_limited', `At most ${CORRECTIONS_PER_PERSON_DAY} corrections a day; the editors will read the ones you sent`);const id = `cor_${ulid(t)}`;-- q.insertCorrection.run({ id, entity_id: canonicalOf(e.id) || e.id, target_type: targetType, target_id: targetId ? String(targetId).slice(0, 100) : null, body: text, evidence_url: url, submitted_by: actor.subject, via: actor.kind === 'service' ? actor.service : null, now: t });-- audit(actor, 'correction.submitted', canonicalOf(e.id) || e.id, id, { target_type: targetType, target_id: targetId });++ const target = targetId ? String(targetId).slice(0, 100) : null;++ q.insertCorrection.run({ id, entity_id: entityId, target_type: targetType, target_id: target, body: text, evidence_url: url ? url.slice(0, 2000) : null, submitted_by: actor.subject, via: actor.kind === 'service' ? actor.service : null, now: t });++ audit(actor, 'correction.submitted', entityId, id, { target_type: targetType, target_id: target });return q.correction.get(id);});},nnew file mode 100644@@@ -0,0 +1,149 @@++'use strict';++/**++ * The fixes made in the threat review (docs/threat-review.md): cross-site writes (pages and the++ * cookie-authenticated API), the correction queue's per-person allowance and duplicates, bounded++ * audit text, Vary on the entity JSON, editor ids shown to readers only as "an editor", ratings++ * spelled out in words in AI text, out-of-scale source ratings, rel on reader-sent links, and the++ * public revision note labelled as public.++ */++const assert = require('assert');++const {++ boot, req, suite, steamItem, productItem, importItem, createEntity, cookieFor, editorToken, readerToken, userToken, subject, serviceToken,++ EDITOR,++} = require('./helpers');++++const t = suite('threat');++const ORIGIN = 'http://reviews.test';++const count = (h, name) => h.db.prepare('SELECT COUNT(*) AS n FROM review_entities WHERE name = ?').get(name).n;++++t('cross-site writes are refused on pages and on the cookie-authenticated API; same-origin and Bearer writes are not', async () => {++ const h = await boot();++ try {++ const e = await createEntity(h, { name: 'Portal 2', kind: 'game' });++ const reader = cookieFor(readerToken());++ const post = (headers, body) => req(h, 'POST', `/e/${e.slug}/correct`, { cookie: reader, headers, form: { target_type: 'entity', body } });++ // A sandboxed frame or data: URL sends Origin: null, but the browser still says cross-site.++ assert.strictEqual((await post({ Origin: 'null', 'Sec-Fetch-Site': 'cross-site' }, 'null origin from another site')).status, 403);++ assert.strictEqual((await post({ 'Sec-Fetch-Site': 'same-site' }, 'a sibling subdomain posting')).status, 403);++ assert.strictEqual((await post({ Origin: 'https://evil.example', 'Sec-Fetch-Site': 'same-origin' }, 'a foreign origin whatever else')).status, 403);++ assert.strictEqual(h.db.prepare('SELECT COUNT(*) AS n FROM review_corrections').get().n, 0);++ assert.strictEqual((await post({ Origin: ORIGIN, 'Sec-Fetch-Site': 'same-origin' }, 'a same-origin form post')).status, 201);++ assert.strictEqual((await post({ Origin: 'null' }, 'privacy settings, no fetch metadata')).status, 201);++++ const ed = cookieFor(editorToken());++ const api = (name, headers, auth) => req(h, 'POST', '/api/v1/entities', { ...auth, headers, body: { name, kind: 'other' } });++ let r = await api('Via cookie from evil', { Origin: 'https://evil.example' }, { cookie: ed });++ assert.strictEqual(r.status, 403, r.text);++ assert.strictEqual(r.json.code, 'request.cross_site');++ r = await api('Via cookie, cross-site fetch', { 'Sec-Fetch-Site': 'cross-site' }, { cookie: ed });++ assert.strictEqual(r.status, 403, r.text);++ assert.strictEqual(count(h, 'Via cookie from evil') + count(h, 'Via cookie, cross-site fetch'), 0);++ assert.strictEqual((await req(h, 'POST', `/api/v1/entities/${e.slug}/summary/unpublish`, { cookie: ed, headers: { 'Sec-Fetch-Site': 'cross-site' } })).status, 403, 'an empty-body write too');++ assert.strictEqual((await api('Via cookie, same origin', { Origin: ORIGIN, 'Sec-Fetch-Site': 'same-origin' }, { cookie: ed })).status, 201);++ assert.strictEqual((await api('Via Bearer', { Origin: 'https://tool.example' }, { token: editorToken() })).status, 201, 'a Bearer token is never ambient');++ assert.strictEqual((await req(h, 'GET', `/api/v1/entities/${e.slug}`, { cookie: ed, headers: { 'Sec-Fetch-Site': 'cross-site' } })).status, 200, 'reads are not writes');++ } finally { await h.stop(); }++});++++t('the correction queue: one open copy of a request, a daily allowance per person, bounded fields', async () => {++ const h = await boot();++ try {++ const e = await createEntity(h, { name: 'Portal 2', kind: 'game' });++ const person = subject();++ const tok = userToken({ subject: person, username: 'busy' });++ const send = (body, extra = {}) => req(h, 'POST', `/api/v1/entities/${e.slug}/corrections`, { token: tok, body: { target_type: 'entity', body, ...extra } });++ assert.strictEqual((await send('The release year shown is wrong.')).status, 201);++ const dup = await send('The release year shown is wrong.');++ assert.strictEqual(dup.status, 409);++ assert.strictEqual(dup.json.code, 'correction.duplicate');++ const huge = await send('A very long target id follows.', { target_id: 'x'.repeat(100000) });++ assert.strictEqual(huge.status, 201);++ assert.strictEqual(huge.json.correction.target_id.length, 100);++ const logged = h.db.prepare("SELECT detail FROM review_audit WHERE action = 'correction.submitted' ORDER BY id DESC").get();++ assert.ok(logged.detail.length < 300, 'the audit row holds the capped id, not the request');++ for (let i = 3; i <= 20; i++) assert.strictEqual((await send(`Correction number ${i} about this entity.`)).status, 201, `#${i}`);++ const over = await send('One more than the daily allowance.');++ assert.strictEqual(over.status, 429);++ assert.strictEqual(over.json.code, 'correction.rate_limited');++ // Someone else is not affected.++ assert.strictEqual((await req(h, 'POST', `/api/v1/entities/${e.slug}/corrections`, { token: readerToken(), body: { target_type: 'entity', body: 'The release year shown is wrong.' } })).status, 201);++ // And the page form says so without JavaScript.++ const page = await req(h, 'POST', `/e/${e.slug}/correct`, { cookie: cookieFor(tok), form: { target_type: 'entity', body: 'Sent from the form after the allowance.' } });++ assert.strictEqual(page.status, 429);++ assert.match(page.text, /At most 20 corrections a day/);++ } finally { await h.stop(); }++});++++t('audit text is bounded; readers see "an editor", never an editor\'s id; the entity JSON varies by credentials', async () => {++ const h = await boot();++ try {++ const a = await createEntity(h, { name: 'Widget', kind: 'product' });++ const b = await createEntity(h, { name: 'Widget (old listing)', kind: 'product' });++ const m = await req(h, 'POST', `/api/v1/entities/${b.slug}/merge`, { token: editorToken(), body: { into: a.slug, note: 'n'.repeat(50000) } });++ assert.strictEqual(m.status, 200, m.text);++ const row = h.db.prepare("SELECT detail FROM review_audit WHERE action = 'entity.merged'").get();++ assert.ok(row.detail.length < 3000, `audit detail is ${row.detail.length} characters`);++++ const e = await createEntity(h, { name: 'Portal 2', kind: 'game', aliases: [{ type: 'source', value: 'steam-reviews-portal-2' }] });++ for (const v of [true, false]) await importItem(h, steamItem({ votedUp: v }));++ const sigs = h.db.prepare("SELECT id FROM review_signals WHERE status = 'active'").all().map((s) => s.id);++ assert.strictEqual((await req(h, 'POST', `/api/v1/entities/${e.slug}/summary/revisions`, { token: editorToken(), body: { overview: 'Opinion in the sample is split.', overview_signals: sigs, publish: true } })).status, 201);++ for (const path of [`/e/${e.slug}.json`, `/api/v1/entities/${e.slug}`, `/api/v1/entities/${e.slug}/history`, `/api/v1/entities/${e.slug}/summary/revisions/1`, `/e/${e.slug}`, `/e/${e.slug}/history`]) {++ const r = await req(h, 'GET', path);++ assert.strictEqual(r.status, 200, path);++ assert.ok(!r.text.includes(EDITOR), `${path} names the editor`);++ }++ const ed = await req(h, 'GET', `/api/v1/entities/${e.slug}/summary/revisions/1`, { token: editorToken() });++ assert.strictEqual(ed.json.revision.author, EDITOR, 'editors see who wrote it');++ const pub = await req(h, 'GET', `/api/v1/entities/${e.slug}/summary/revisions/1`);++ assert.strictEqual(pub.json.revision.author, 'an editor');++ // The revision note is public (the history lists it): the form says so.++ assert.match((await req(h, 'GET', `/e/${e.slug}/edit`, { cookie: cookieFor(editorToken()) })).text, /Revision note \(public: shown in the summary's history\)/);++ const json = await req(h, 'GET', `/e/${e.slug}.json`);++ assert.match(json.headers.get('vary') || '', /Cookie/);++ assert.match(json.headers.get('vary') || '', /Authorization/);++ } finally { await h.stop(); }++});++++t('AI text cannot spell a rating out in words; out-of-scale source ratings are not signals', async () => {++ const h = await boot();++ try {++ const e = await createEntity(h, { name: 'Portal 2', kind: 'game', aliases: [{ type: 'source', value: 'steam-reviews-portal-2' }] });++ for (const v of [true, true]) await importItem(h, steamItem({ votedUp: v }));++ const sigs = h.db.prepare("SELECT id FROM review_signals WHERE status = 'active'").all().map((s) => s.id);++ const AI = serviceToken({ client: 'ai', cap: ['reviews.summary.propose'] });++ const wf = { id: 'reviews.summarize_entity', run_id: 'run_01J9ZZZZZZZZZZZZZZZZZZZZZZ' };++ for (const text of ['Easily four and a half stars.', 'A nine out of ten experience.', 'A five-star puzzle game.']) {++ const r = await req(h, 'POST', `/api/v1/entities/${e.slug}/summary/proposals`, { token: AI, body: { overview: text, overview_signals: sigs, workflow: wf } });++ assert.strictEqual(r.status, 422, text);++ assert.strictEqual(r.json.code, 'summary.rating_in_text', text);++ }++ const fine = await req(h, 'POST', `/api/v1/entities/${e.slug}/summary/proposals`, { token: AI, body: { overview: 'Both of the two sampled reviewers recommend it.', overview_signals: sigs, workflow: wf } });++ assert.strictEqual(fine.status, 201, fine.text);++++ await createEntity(h, { name: 'Widget 3000', kind: 'product', aliases: [{ type: 'url', value: 'https://shop.example/p/widget' }, { type: 'url', value: 'https://shop.example/p/widget-b' }] });++ const negative = productItem({ value: -4, best: 5, count: 50 });++ const low = productItem({ value: 0.5, best: 5, count: 50, url: 'https://shop.example/p/widget-b' });++ low.fields.aggregate_rating.worst = 1;++ for (const [it, re] of [[negative, /below zero/], [low, /worst 1/]]) {++ const out = await importItem(h, it);++ assert.strictEqual(out.signal, null, JSON.stringify(out));++ assert.strictEqual(out.item.resolution, 'resolved');++ assert.match(out.item.signal_note, re);++ }++ const agg = await req(h, 'GET', '/api/v1/entities/widget-3000/aggregate');++ assert.strictEqual(agg.json.aggregate, null);++ } finally { await h.stop(); }++});++++t('a reader-sent evidence link is nofollow ugc noreferrer on the editor desk', async () => {++ const h = await boot();++ try {++ const e = await createEntity(h, { name: 'Portal 2', kind: 'game' });++ await req(h, 'POST', `/api/v1/entities/${e.slug}/corrections`, { token: readerToken(), body: { target_type: 'entity', body: 'See the official page for the year.', evidence_url: 'https://example.org/proof' } });++ await req(h, 'POST', `/api/v1/entities/${e.slug}/corrections`, { token: readerToken(), body: { target_type: 'entity', body: 'A script link as evidence here.', evidence_url: 'javascript:alert(1)' } }).then((r) => assert.strictEqual(r.status, 422));++ const desk = await req(h, 'GET', '/editor', { cookie: cookieFor(editorToken()) });++ assert.match(desk.text, /<a href="https:\/\/example\.org\/proof" rel="nofollow ugc noopener noreferrer">evidence<\/a>/);++ assert.ok(!desk.text.includes('javascript:'));++ } finally { await h.stop(); }++});
@@ -255,6 +255,7 @@ CREATE TABLE IF NOT EXISTS review_corrections (
@@ -255,6 +255,7 @@ CREATE TABLE IF NOT EXISTS review_corrections (
);
);
CREATE INDEX IF NOT EXISTS review_corrections_status ON review_corrections (status, created_at);
CREATE INDEX IF NOT EXISTS review_corrections_status ON review_corrections (status, created_at);
CREATE INDEX IF NOT EXISTS review_corrections_entity ON review_corrections (entity_id, status);
CREATE INDEX IF NOT EXISTS review_corrections_entity ON review_corrections (entity_id, status);
+CREATE INDEX IF NOT EXISTS review_corrections_by ON review_corrections (submitted_by, created_at);
CREATE TABLE IF NOT EXISTS review_audit (
CREATE TABLE IF NOT EXISTS review_audit (
id INTEGER PRIMARY KEY AUTOINCREMENT,
id INTEGER PRIMARY KEY AUTOINCREMENT,
@@ -39,7 +39,7 @@
@@ -39,7 +39,7 @@
*/
*/
const express = require('express');
const express = require('express');
const contracts = require('openvibe-contracts');
const contracts = require('openvibe-contracts');
-const { actorMiddleware, guard, run } = require('./common');
+const { actorMiddleware, guard, run, crossSite } = require('./common');
const { checkCapability } = require('../auth/capabilities');
const { checkCapability } = require('../auth/capabilities');
const { http } = contracts;
const { http } = contracts;
@@ -49,6 +49,14 @@ function createApi({ svc, viewers, platform, sync, config, log = console }) {
@@ -49,6 +49,14 @@ function createApi({ svc, viewers, platform, sync, config, log = console }) {
router.use(http.middleware());
router.use(http.middleware());
router.use(express.json({ limit: '256kb' }));
router.use(express.json({ limit: '256kb' }));
router.use((err, req, res, next) => (err ? http.sendProblem(res, 400, 'request.invalid_json', { detail: 'Malformed JSON body', ctx: req.ov }) : next()));
router.use((err, req, res, next) => (err ? http.sendProblem(res, 400, 'request.invalid_json', { detail: 'Malformed JSON body', ctx: req.ov }) : next()));
+ // The API also accepts the ov_token cookie: a write carried by it that another site started is
+ // refused (SameSite=Lax is the first line; Bearer tokens are never sent by a browser on its own).
+ const origin = new URL(config.baseUrl).origin;
+ router.use((req, res, next) => {
+ if (['GET', 'HEAD', 'OPTIONS'].includes(req.method) || req.headers.authorization || !(req.cookies && req.cookies.ov_token)) return next();
+ if (!crossSite(req, origin)) return next();
+ http.sendProblem(res, 403, 'request.cross_site', { detail: 'A write with the session cookie from another site is not accepted', ctx: req.ov });
+ });
router.use(actorMiddleware(viewers));
router.use(actorMiddleware(viewers));
const R = (fn, status) => run(fn, status, log);
const R = (fn, status) => run(fn, status, log);
const body = (req) => (req.body && typeof req.body === 'object' ? req.body : {});
const body = (req) => (req.body && typeof req.body === 'object' ? req.body : {});
@@ -35,6 +35,19 @@ function guard(capabilityId) {
@@ -35,6 +35,19 @@ function guard(capabilityId) {
};
};
}
}
+/**
+ * A write another site started (CSRF), judged from what the browser says: Sec-Fetch-Site other than
+ * same-origin (or none: typed by the person), or an Origin other than ours. `Origin: null` alone is
+ * not proof (privacy settings send it for same-origin posts); a browser that sends Sec-Fetch-Site
+ * says what it really was.
+ */
+function crossSite(req, origin) {
+ const site = req.get('sec-fetch-site');
+ if (site && site !== 'same-origin' && site !== 'none') return true;
+ const o = req.get('origin');
+ return !!(o && o !== 'null' && o !== origin);
+}
+
/** Service errors → problem+json. */
/** Service errors → problem+json. */
function sendError(res, req, err, log = console) {
function sendError(res, req, err, log = console) {
const status = err && Number.isInteger(err.status) ? err.status : 500;
const status = err && Number.isInteger(err.status) ? err.status : 500;
@@ -56,4 +69,4 @@ function run(fn, status = 200, log = console) {
@@ -56,4 +69,4 @@ function run(fn, status = 200, log = console) {
};
};
}
}
-module.exports = { actorMiddleware, guard, sendError, run };
+module.exports = { actorMiddleware, guard, sendError, run, crossSite };
@@ -25,7 +25,7 @@ const seo = require('openvibe-publishing/seo');
@@ -25,7 +25,7 @@ const seo = require('openvibe-publishing/seo');
const { renderPage } = require('../render/layout');
const { renderPage } = require('../render/layout');
const views = require('../render/views');
const views = require('../render/views');
const { ratingForStructuredData } = require('../reviews/aggregate');
const { ratingForStructuredData } = require('../reviews/aggregate');
-const { actorMiddleware } = require('./common');
+const { actorMiddleware, crossSite } = require('./common');
const SCHEMA_TYPE = { product: 'Product', game: 'VideoGame', software: 'SoftwareApplication', service: 'Service', place: 'Place', organization: 'Organization', media: 'CreativeWork', other: 'Thing' };
const SCHEMA_TYPE = { product: 'Product', game: 'VideoGame', software: 'SoftwareApplication', service: 'Service', place: 'Place', organization: 'Organization', media: 'CreativeWork', other: 'Thing' };
@@ -90,10 +90,8 @@ function createPages({ svc, platform, sync, viewers, config, log = console }) {
@@ -90,10 +90,8 @@ function createPages({ svc, platform, sync, viewers, config, log = console }) {
const origin = new URL(config.baseUrl).origin;
const origin = new URL(config.baseUrl).origin;
// Cross-site form posts are refused (the session cookie is SameSite=Lax as well).
// Cross-site form posts are refused (the session cookie is SameSite=Lax as well).
router.use((req, res, next) => {
router.use((req, res, next) => {
- if (req.method !== 'POST') return next();
- const o = req.get('origin');
- if (o && o !== origin && o !== 'null') return res.status(403).type('text/plain').set('Cache-Control', 'private, no-store').send('Cross-site form posts are not accepted.');
- next();
+ if (req.method !== 'POST' || !crossSite(req, origin)) return next();
+ res.status(403).type('text/plain').set('Cache-Control', 'private, no-store').send('Cross-site form posts are not accepted.');
});
});
const send = (req, res, status, body, o = {}) => {
const send = (req, res, status, body, o = {}) => {
@@ -155,7 +153,7 @@ function createPages({ svc, platform, sync, viewers, config, log = console }) {
@@ -155,7 +153,7 @@ function createPages({ svc, platform, sync, viewers, config, log = console }) {
const e = locate(req, res, '.json');
const e = locate(req, res, '.json');
if (!e) return;
if (!e) return;
const p = svc.page(e, req.actor);
const p = svc.page(e, req.actor);
- res.status(200).set('Cache-Control', req.actor.kind === 'anonymous' ? 'public, max-age=60' : 'private, no-store').set('X-Robots-Tag', p.decision.robots)
+ res.status(200).set('Cache-Control', req.actor.kind === 'anonymous' ? 'public, max-age=60' : 'private, no-store').set('Vary', 'Cookie, Authorization').set('X-Robots-Tag', p.decision.robots)
.json({ ...p, decision: { indexable: p.decision.indexable, robots: p.decision.robots, reasons: p.decision.codes } });
.json({ ...p, decision: { indexable: p.decision.indexable, robots: p.decision.robots, reasons: p.decision.codes } });
});
});
@@ -300,7 +300,7 @@ ${pending.length ? html`<ul>${pending.map((x) => html`<li><a href="${epath(x.ent
@@ -300,7 +300,7 @@ ${pending.length ? html`<ul>${pending.map((x) => html`<li><a href="${epath(x.ent
<h2 id="flagged">Flagged summaries</h2>
<h2 id="flagged">Flagged summaries</h2>
${flagged.length ? html`<ul>${flagged.map((s) => html`<li><a href="${epath(s.entity)}">${s.entity.name}</a>: ${s.flag_reason}</li>`)}</ul>` : html`<p class="rv-muted">None.</p>`}
${flagged.length ? html`<ul>${flagged.map((s) => html`<li><a href="${epath(s.entity)}">${s.entity.name}</a>: ${s.flag_reason}</li>`)}</ul>` : html`<p class="rv-muted">None.</p>`}
<h2 id="corrections">Open corrections</h2>
<h2 id="corrections">Open corrections</h2>
-${corrections.length ? html`<ul>${corrections.map((c) => html`<li><a href="${epath(c.entity)}">${c.entity.name}</a> · ${c.target_type}${c.target_id ? ` ${c.target_id}` : ''} · ${t(c.created_at)}<blockquote>${c.body}</blockquote>${safeUrl(c.evidence_url) ? html`<a href="${safeUrl(c.evidence_url)}" rel="nofollow noopener">evidence</a>` : ''}
+${corrections.length ? html`<ul>${corrections.map((c) => html`<li><a href="${epath(c.entity)}">${c.entity.name}</a> · ${c.target_type}${c.target_id ? ` ${c.target_id}` : ''} · ${t(c.created_at)}<blockquote>${c.body}</blockquote>${safeUrl(c.evidence_url) ? html`<a href="${safeUrl(c.evidence_url)}" rel="nofollow ugc noopener noreferrer">evidence</a> <span class="rv-muted">(sent by the reader; not checked)</span>` : ''}
<form method="post" action="/editor/corrections/${e(c.id)}" class="rv-form">
<form method="post" action="/editor/corrections/${e(c.id)}" class="rv-form">
${c.summary_published ? html`<label>Correction note, published with a new summary revision (required to accept) <input type="text" name="correction_note" maxlength="2000"></label>
${c.summary_published ? html`<label>Correction note, published with a new summary revision (required to accept) <input type="text" name="correction_note" maxlength="2000"></label>
<p class="rv-muted">Accepting publishes the current summary text again as a correction revision with this note. To change the text as well, <a href="${epath(c.entity)}/edit?correction=${e(c.id)}#summary-form">correct the summary</a>.</p>` : ''}
<p class="rv-muted">Accepting publishes the current summary text again as a correction revision with this note. To change the text as well, <a href="${epath(c.entity)}/edit?correction=${e(c.id)}#summary-form">correct the summary</a>.</p>` : ''}
@@ -363,7 +363,7 @@ ${correcting ? html`<div class="rv-notice rv-correction"><p>Correcting the summa
@@ -363,7 +363,7 @@ ${correcting ? html`<div class="rv-notice rv-correction"><p>Correcting the summa
<label>The overview cites ${sel('overview_signals', ov ? ov.citations.filter((c) => c.ok).map((c) => c.signal_id) : [])}</label>
<label>The overview cites ${sel('overview_signals', ov ? ov.citations.filter((c) => c.ok).map((c) => c.signal_id) : [])}</label>
<fieldset><legend>Pros</legend>${rows('pro', pros)}</fieldset>
<fieldset><legend>Pros</legend>${rows('pro', pros)}</fieldset>
<fieldset><legend>Cons</legend>${rows('con', cons)}</fieldset>
<fieldset><legend>Cons</legend>${rows('con', cons)}</fieldset>
-<label>Revision note <input type="text" name="message" maxlength="500"></label>
+<label>Revision note (public: shown in the summary's history) <input type="text" name="message" maxlength="500"></label>
${cur ? html`<label>Correction note (public; fill it in only to correct the published summary: the revision is published at once and the note stays in its history) <input type="text" name="correction_note" maxlength="2000"${correcting ? raw(' required minlength="10"') : ''}></label>` : ''}
${cur ? html`<label>Correction note (public; fill it in only to correct the published summary: the revision is published at once and the note stays in its history) <input type="text" name="correction_note" maxlength="2000"${correcting ? raw(' required minlength="10"') : ''}></label>` : ''}
<input type="hidden" name="expected_revision" value="${String(p.summary ? p.summary.head_revision : 0)}">
<input type="hidden" name="expected_revision" value="${String(p.summary ? p.summary.head_revision : 0)}">
<button type="submit" name="publish" value="0">Save</button> <button type="submit" name="publish" value="1">Save and publish</button></form>
<button type="submit" name="publish" value="0">Save</button> <button type="submit" name="publish" value="1">Save and publish</button></form>
@@ -72,6 +72,17 @@ function keptTitle(item) {
@@ -72,6 +72,17 @@ function keptTitle(item) {
return item && item.kind === 'product' && item.title ? String(item.title).slice(0, 300) : null;
return item && item.kind === 'product' && item.title ? String(item.title).slice(0, 300) : null;
}
}
+/**
+ * Why a stated rating cannot be counted on its own scale, or null. Below the stated worst (or below
+ * zero when no worst is stated) or above the best is not an opinion the aggregate can use.
+ */
+function outOfScale(r) {
+ if (r.best != null && (r.best <= 0 || r.value > r.best)) return `rating ${r.value} is outside the stated scale (best ${r.best})`;
+ if (r.worst != null && r.value < r.worst) return `rating ${r.value} is outside the stated scale (worst ${r.worst})`;
+ if (r.worst == null && r.value < 0) return `rating ${r.value} is below zero and the source states no scale minimum`;
+ return null;
+}
+
function trustOf(fields) {
function trustOf(fields) {
const out = {};
const out = {};
for (const k of TRUST_FIELDS) if (fields[k] !== undefined && fields[k] !== null) out[k] = fields[k];
for (const k of TRUST_FIELDS) if (fields[k] !== undefined && fields[k] !== null) out[k] = fields[k];
@@ -101,13 +112,13 @@ function extractSignal(item) {
@@ -101,13 +112,13 @@ function extractSignal(item) {
if (!r || r.value == null) return { signal: null, note: 'the product states no aggregate rating' };
if (!r || r.value == null) return { signal: null, note: 'the product states no aggregate rating' };
const n = r.count != null && r.count > 0 ? r.count : (r.review_count != null && r.review_count > 0 ? r.review_count : null);
const n = r.count != null && r.count > 0 ? r.count : (r.review_count != null && r.review_count > 0 ? r.review_count : null);
if (n == null) return { signal: null, note: 'the aggregate rating states no number of ratings' };
if (n == null) return { signal: null, note: 'the aggregate rating states no number of ratings' };
- if (r.best != null && (r.best <= 0 || r.value > r.best)) return { signal: null, note: `rating ${r.value} is outside the stated scale (best ${r.best})` };
+ if (outOfScale(r)) return { signal: null, note: outOfScale(r) };
return { signal: { type: 'rating_aggregate', rating_value: r.value, rating_best: r.best, rating_worst: r.worst, rating_count: n, trust }, note: null };
return { signal: { type: 'rating_aggregate', rating_value: r.value, rating_best: r.best, rating_worst: r.worst, rating_count: n, trust }, note: null };
}
}
if (item.kind === 'review') {
if (item.kind === 'review') {
const r = f.rating;
const r = f.rating;
if (!r || r.value == null) return { signal: null, note: 'the review states no rating' };
if (!r || r.value == null) return { signal: null, note: 'the review states no rating' };
- if (r.best != null && (r.best <= 0 || r.value > r.best)) return { signal: null, note: `rating ${r.value} is outside the stated scale (best ${r.best})` };
+ if (outOfScale(r)) return { signal: null, note: outOfScale(r) };
return { signal: { type: 'rating', rating_value: r.value, rating_best: r.best, rating_worst: r.worst, rating_count: 1, trust }, note: null };
return { signal: { type: 'rating', rating_value: r.value, rating_best: r.best, rating_worst: r.worst, rating_count: 1, trust }, note: null };
}
}
return { signal: null, note: `items of kind "${item.kind}" carry no review signal` };
return { signal: null, note: `items of kind "${item.kind}" carry no review signal` };
@@ -36,12 +36,21 @@ const MAX_POINT = 600;
@@ -36,12 +36,21 @@ const MAX_POINT = 600;
const MAX_OVERVIEW = 6000;
const MAX_OVERVIEW = 6000;
const MIN_CORRECTION_NOTE = 10;
const MIN_CORRECTION_NOTE = 10;
const MAX_CORRECTION_NOTE = 2000;
const MAX_CORRECTION_NOTE = 2000;
+// Correction requests one person may send per rolling day (the per-IP limit is in server/app.js).
+const CORRECTIONS_PER_PERSON_DAY = 20;
+const DAY_MS = 24 * 3600 * 1000;
+const MAX_AUDIT_TEXT = 2000;
// A summary is text with citations. Nothing in it can carry a score.
// A summary is text with citations. Nothing in it can carry a score.
const SUMMARY_KEYS = new Set(['overview', 'pros', 'cons', 'overview_signals', 'expected_revision', 'expectedRevision', 'message', 'publish', 'workflow', 'stub_provider', 'stubProvider', 'note', 'correction_note', 'correction_id']);
const SUMMARY_KEYS = new Set(['overview', 'pros', 'cons', 'overview_signals', 'expected_revision', 'expectedRevision', 'message', 'publish', 'workflow', 'stub_provider', 'stubProvider', 'note', 'correction_note', 'correction_id']);
const BODY_KEYS = ['overview', 'overview_signals', 'pros', 'cons'];
const BODY_KEYS = ['overview', 'overview_signals', 'pros', 'cons'];
const RATING_KEY_RE = /(rating|stars?|score|grade|verdict_value)/i;
const RATING_KEY_RE = /(rating|stars?|score|grade|verdict_value)/i;
-// Numeric ratings in AI text ("4.5/5", "8 out of 10", "★★★★"): AI output never states a rating.
-const RATING_TEXT_RE = /(\b\d+(?:[.,]\d+)?\s*(?:\/|out of)\s*(?:5|10|100)\b)|[★☆⭐]|\b\d+(?:[.,]\d+)?\s*stars?\b/i;
+// Numeric ratings in AI text ("4.5/5", "8 out of 10", "★★★★", "four and a half stars", "nine out of
+// ten"): AI output never states a rating.
+const NUMBER_WORD = '(?:zero|one|two|three|four|five|six|seven|eight|nine|ten)(?:[\\s-]+and[\\s-]+a[\\s-]+half)?';
+const RATING_TEXT_RE = new RegExp([
+ '(\\b\\d+(?:[.,]\\d+)?\\s*(?:\\/|out of)\\s*(?:5|10|100)\\b)', '[★☆⭐]', '\\b\\d+(?:[.,]\\d+)?\\s*stars?\\b',
+ `\\b${NUMBER_WORD}[\\s-]+stars?\\b`, `\\b${NUMBER_WORD}\\s+out\\s+of\\s+(?:five|ten|a\\s+hundred|one\\s+hundred|5|10|100)\\b`,
+].join('|'), 'i');
const toIso = (ms) => (ms == null ? null : new Date(ms).toISOString());
const toIso = (ms) => (ms == null ? null : new Date(ms).toISOString());
@@ -130,6 +139,8 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
@@ -130,6 +139,8 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
allSummaries: db.prepare('SELECT * FROM review_summaries ORDER BY updated_at DESC LIMIT 500'),
allSummaries: db.prepare('SELECT * FROM review_summaries ORDER BY updated_at DESC LIMIT 500'),
publishedSummaries: db.prepare("SELECT * FROM review_summaries WHERE state = 'published' ORDER BY revision_published_at DESC LIMIT ?"),
publishedSummaries: db.prepare("SELECT * FROM review_summaries WHERE state = 'published' ORDER BY revision_published_at DESC LIMIT ?"),
correction: db.prepare('SELECT * FROM review_corrections WHERE id = ?'),
correction: db.prepare('SELECT * FROM review_corrections WHERE id = ?'),
+ correctionsSince: db.prepare('SELECT COUNT(*) AS n FROM review_corrections WHERE submitted_by = ? AND created_at > ?'),
+ sameOpenCorrection: db.prepare("SELECT id FROM review_corrections WHERE submitted_by = ? AND entity_id = ? AND status = 'open' AND body = ?"),
insertCorrection: db.prepare(`INSERT INTO review_corrections (id, entity_id, target_type, target_id, body, evidence_url, submitted_by, via, created_at)
insertCorrection: db.prepare(`INSERT INTO review_corrections (id, entity_id, target_type, target_id, body, evidence_url, submitted_by, via, created_at)
VALUES (@id, @entity_id, @target_type, @target_id, @body, @evidence_url, @submitted_by, @via, @now)`),
VALUES (@id, @entity_id, @target_type, @target_id, @body, @evidence_url, @submitted_by, @via, @now)`),
resolveCorrection: db.prepare("UPDATE review_corrections SET status = ?, resolved_by = ?, resolution_note = ?, resolved_at = ? WHERE id = ? AND status = 'open'"),
resolveCorrection: db.prepare("UPDATE review_corrections SET status = ?, resolved_by = ?, resolution_note = ?, resolved_at = ? WHERE id = ? AND status = 'open'"),
@@ -162,8 +173,15 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
@@ -162,8 +173,15 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
if (!access.isEditor(actor)) fail(403, 'reviews.editor_required', `${what} is for Reviews editors`);
if (!access.isEditor(actor)) fail(403, 'reviews.editor_required', `${what} is for Reviews editors`);
return actorId(actor);
return actorId(actor);
}
}
+ // Audit rows are append-only and kept forever: no free text in one grows without bound.
+ function clipText(v, depth = 0) {
+ if (typeof v === 'string') return v.length > MAX_AUDIT_TEXT ? `${v.slice(0, MAX_AUDIT_TEXT)}…` : v;
+ if (Array.isArray(v)) return v.map((x) => clipText(x, depth + 1));
+ if (v && typeof v === 'object' && depth < 4) return Object.fromEntries(Object.entries(v).map(([k, x]) => [k, clipText(x, depth + 1)]));
+ return v;
+ }
function audit(actor, action, entityId, target, detail = {}) {
function audit(actor, action, entityId, target, detail = {}) {
- q.audit.run(now(), typeof actor === 'string' ? actor : actorId(actor), action, entityId || null, target || null, JSON.stringify(detail));
+ q.audit.run(now(), typeof actor === 'string' ? actor : actorId(actor), action, entityId || null, target || null, JSON.stringify(clipText(detail)));
}
}
function emit(envelope) { return outbox.enqueue(envelope); }
function emit(envelope) { return outbox.enqueue(envelope); }
@@ -905,7 +923,8 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
@@ -905,7 +923,8 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
};
};
}
}
- function summaryRevisionView(summary, rev, canonicalId) {
+ /** `editor: false` (a reader): the person who wrote it is "an editor", as in the editorial log. */
+ function summaryRevisionView(summary, rev, canonicalId, { editor = true } = {}) {
if (!rev) return null;
if (!rev) return null;
const rec = rev.meta && rev.meta.authorship;
const rec = rev.meta && rev.meta.authorship;
const review = reviews.latest(summary.id, rev.number);
const review = reviews.latest(summary.id, rev.number);
@@ -917,7 +936,7 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
@@ -917,7 +936,7 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
review: review ? { decision: review.decision, reviewed_at: review.reviewedAt } : null,
review: review ? { decision: review.decision, reviewed_at: review.reviewedAt } : null,
system: rev.meta && rev.meta.system ? rev.meta.system : null,
system: rev.meta && rev.meta.system ? rev.meta.system : null,
correction: correctionView(rev),
correction: correctionView(rev),
- author: rev.author, message: rev.message, created_at: rev.createdAt,
+ author: editor || !/^usr_/.test(String(rev.author)) ? rev.author : 'an editor', message: rev.message, created_at: rev.createdAt,
};
};
}
}
@@ -999,7 +1018,7 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
@@ -999,7 +1018,7 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
summary: summary ? {
summary: summary ? {
id: summary.id, state: summary.state, head_revision: revisions.headNumber(summary.id), flagged: !!summary.flagged, flag_reason: summary.flag_reason, flagged_at: toIso(summary.flagged_at),
id: summary.id, state: summary.state, head_revision: revisions.headNumber(summary.id), flagged: !!summary.flagged, flag_reason: summary.flag_reason, flagged_at: toIso(summary.flagged_at),
published_at: toIso(summary.published_at), revision_published_at: toIso(summary.revision_published_at),
published_at: toIso(summary.published_at), revision_published_at: toIso(summary.revision_published_at),
- published: pub ? summaryRevisionView(summary, pub, e.id) : null,
+ published: pub ? summaryRevisionView(summary, pub, e.id, { editor }) : null,
pending: editor ? pendingRevisions(summary).map((p) => summaryRevisionView(summary, p.rev, e.id)) : pendingRevisions(summary).length,
pending: editor ? pendingRevisions(summary).map((p) => summaryRevisionView(summary, p.rev, e.id)) : pendingRevisions(summary).length,
history: publicHistory(summary),
history: publicHistory(summary),
} : null,
} : null,
@@ -1018,7 +1037,7 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
@@ -1018,7 +1037,7 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
entity: entityView(e),
entity: entityView(e),
aggregates: q.aggregates.all(e.id, 200).map(aggregateView),
aggregates: q.aggregates.all(e.id, 200).map(aggregateView),
summary_revisions: summary ? revs.filter((r) => editor || publicRevision(summary, r))
summary_revisions: summary ? revs.filter((r) => editor || publicRevision(summary, r))
- .map((r) => summaryRevisionView(summary, r, e.id)) : [],
+ .map((r) => summaryRevisionView(summary, r, e.id, { editor })) : [],
merges: q.mergeLinksOf.all(e.id, e.id).map((l) => ({
merges: q.mergeLinksOf.all(e.id, e.id).map((l) => ({
id: l.id, from: entityView(q.entity.get(l.from_entity)), to: entityView(q.entity.get(l.to_entity)), note: l.note,
id: l.id, from: entityView(q.entity.get(l.from_entity)), to: entityView(q.entity.get(l.to_entity)), note: l.note,
merged_at: toIso(l.created_at), merged_by: editor ? l.created_by : null, split_at: toIso(l.ended_at), split_by: editor ? l.ended_by : null, split_note: l.end_note,
merged_at: toIso(l.created_at), merged_by: editor ? l.created_by : null, split_at: toIso(l.ended_at), split_by: editor ? l.ended_by : null, split_note: l.end_note,
@@ -1037,9 +1056,9 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
@@ -1037,9 +1056,9 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
if (!summary) fail(404, 'summary.not_found', 'This entity has no summary');
if (!summary) fail(404, 'summary.not_found', 'This entity has no summary');
const rev = revisions.get(summary.id, Number(n));
const rev = revisions.get(summary.id, Number(n));
if (!rev) fail(404, 'revision.not_found', `No revision ${n}`);
if (!rev) fail(404, 'revision.not_found', `No revision ${n}`);
- const visible = access.isEditor(actor) || publicRevision(summary, rev);
- if (!visible) fail(404, 'revision.not_found', `No revision ${n}`);
- return summaryRevisionView(summary, rev, e.id);
+ const editor = access.isEditor(actor);
+ if (!editor && !publicRevision(summary, rev)) fail(404, 'revision.not_found', `No revision ${n}`);
+ return summaryRevisionView(summary, rev, e.id, { editor });
},
},
// Resolve (reviews.entity.resolve) ---------------------------------------------------
// Resolve (reviews.entity.resolve) ---------------------------------------------------
@@ -1535,9 +1554,14 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
@@ -1535,9 +1554,14 @@ function createReviewsService({ db, stores, outbox, config, now = () => Date.now
const e = entityOrFail(ref);
const e = entityOrFail(ref);
if (e.state === 'deleted') fail(410, 'entity.deleted', 'This entity was deleted');
if (e.state === 'deleted') fail(410, 'entity.deleted', 'This entity was deleted');
const t = now();
const t = now();
+ const entityId = canonicalOf(e.id) || e.id;
+ // One person cannot bury the editors' queue: a daily allowance, and the same open request once.
+ if (q.sameOpenCorrection.get(actor.subject, entityId, text)) fail(409, 'correction.duplicate', 'You already sent this correction; it is waiting for an editor');
+ if (q.correctionsSince.get(actor.subject, t - DAY_MS).n >= CORRECTIONS_PER_PERSON_DAY) fail(429, 'correction.rate_limited', `At most ${CORRECTIONS_PER_PERSON_DAY} corrections a day; the editors will read the ones you sent`);
const id = `cor_${ulid(t)}`;
const id = `cor_${ulid(t)}`;
- q.insertCorrection.run({ id, entity_id: canonicalOf(e.id) || e.id, target_type: targetType, target_id: targetId ? String(targetId).slice(0, 100) : null, body: text, evidence_url: url, submitted_by: actor.subject, via: actor.kind === 'service' ? actor.service : null, now: t });
- audit(actor, 'correction.submitted', canonicalOf(e.id) || e.id, id, { target_type: targetType, target_id: targetId });
+ const target = targetId ? String(targetId).slice(0, 100) : null;
+ q.insertCorrection.run({ id, entity_id: entityId, target_type: targetType, target_id: target, body: text, evidence_url: url ? url.slice(0, 2000) : null, submitted_by: actor.subject, via: actor.kind === 'service' ? actor.service : null, now: t });
+ audit(actor, 'correction.submitted', entityId, id, { target_type: targetType, target_id: target });
return q.correction.get(id);
return q.correction.get(id);
});
});
},
},
new file mode 100644
new file mode 100644
@@ -0,0 +1,149 @@
@@ -0,0 +1,149 @@
+'use strict';
+/**
+ * The fixes made in the threat review (docs/threat-review.md): cross-site writes (pages and the
+ * cookie-authenticated API), the correction queue's per-person allowance and duplicates, bounded
+ * audit text, Vary on the entity JSON, editor ids shown to readers only as "an editor", ratings
+ * spelled out in words in AI text, out-of-scale source ratings, rel on reader-sent links, and the
+ * public revision note labelled as public.
+ */
+const assert = require('assert');
+const {
+ boot, req, suite, steamItem, productItem, importItem, createEntity, cookieFor, editorToken, readerToken, userToken, subject, serviceToken,
+ EDITOR,
+} = require('./helpers');
+
+const t = suite('threat');
+const ORIGIN = 'http://reviews.test';
+const count = (h, name) => h.db.prepare('SELECT COUNT(*) AS n FROM review_entities WHERE name = ?').get(name).n;
+
+t('cross-site writes are refused on pages and on the cookie-authenticated API; same-origin and Bearer writes are not', async () => {
+ const h = await boot();
+ try {
+ const e = await createEntity(h, { name: 'Portal 2', kind: 'game' });
+ const reader = cookieFor(readerToken());
+ const post = (headers, body) => req(h, 'POST', `/e/${e.slug}/correct`, { cookie: reader, headers, form: { target_type: 'entity', body } });
+ // A sandboxed frame or data: URL sends Origin: null, but the browser still says cross-site.
+ assert.strictEqual((await post({ Origin: 'null', 'Sec-Fetch-Site': 'cross-site' }, 'null origin from another site')).status, 403);
+ assert.strictEqual((await post({ 'Sec-Fetch-Site': 'same-site' }, 'a sibling subdomain posting')).status, 403);
+ assert.strictEqual((await post({ Origin: 'https://evil.example', 'Sec-Fetch-Site': 'same-origin' }, 'a foreign origin whatever else')).status, 403);
+ assert.strictEqual(h.db.prepare('SELECT COUNT(*) AS n FROM review_corrections').get().n, 0);
+ assert.strictEqual((await post({ Origin: ORIGIN, 'Sec-Fetch-Site': 'same-origin' }, 'a same-origin form post')).status, 201);
+ assert.strictEqual((await post({ Origin: 'null' }, 'privacy settings, no fetch metadata')).status, 201);
+
+ const ed = cookieFor(editorToken());
+ const api = (name, headers, auth) => req(h, 'POST', '/api/v1/entities', { ...auth, headers, body: { name, kind: 'other' } });
+ let r = await api('Via cookie from evil', { Origin: 'https://evil.example' }, { cookie: ed });
+ assert.strictEqual(r.status, 403, r.text);
+ assert.strictEqual(r.json.code, 'request.cross_site');
+ r = await api('Via cookie, cross-site fetch', { 'Sec-Fetch-Site': 'cross-site' }, { cookie: ed });
+ assert.strictEqual(r.status, 403, r.text);
+ assert.strictEqual(count(h, 'Via cookie from evil') + count(h, 'Via cookie, cross-site fetch'), 0);
+ assert.strictEqual((await req(h, 'POST', `/api/v1/entities/${e.slug}/summary/unpublish`, { cookie: ed, headers: { 'Sec-Fetch-Site': 'cross-site' } })).status, 403, 'an empty-body write too');
+ assert.strictEqual((await api('Via cookie, same origin', { Origin: ORIGIN, 'Sec-Fetch-Site': 'same-origin' }, { cookie: ed })).status, 201);
+ assert.strictEqual((await api('Via Bearer', { Origin: 'https://tool.example' }, { token: editorToken() })).status, 201, 'a Bearer token is never ambient');
+ assert.strictEqual((await req(h, 'GET', `/api/v1/entities/${e.slug}`, { cookie: ed, headers: { 'Sec-Fetch-Site': 'cross-site' } })).status, 200, 'reads are not writes');
+ } finally { await h.stop(); }
+});
+
+t('the correction queue: one open copy of a request, a daily allowance per person, bounded fields', async () => {
+ const h = await boot();
+ try {
+ const e = await createEntity(h, { name: 'Portal 2', kind: 'game' });
+ const person = subject();
+ const tok = userToken({ subject: person, username: 'busy' });
+ const send = (body, extra = {}) => req(h, 'POST', `/api/v1/entities/${e.slug}/corrections`, { token: tok, body: { target_type: 'entity', body, ...extra } });
+ assert.strictEqual((await send('The release year shown is wrong.')).status, 201);
+ const dup = await send('The release year shown is wrong.');
+ assert.strictEqual(dup.status, 409);
+ assert.strictEqual(dup.json.code, 'correction.duplicate');
+ const huge = await send('A very long target id follows.', { target_id: 'x'.repeat(100000) });
+ assert.strictEqual(huge.status, 201);
+ assert.strictEqual(huge.json.correction.target_id.length, 100);
+ const logged = h.db.prepare("SELECT detail FROM review_audit WHERE action = 'correction.submitted' ORDER BY id DESC").get();
+ assert.ok(logged.detail.length < 300, 'the audit row holds the capped id, not the request');
+ for (let i = 3; i <= 20; i++) assert.strictEqual((await send(`Correction number ${i} about this entity.`)).status, 201, `#${i}`);
+ const over = await send('One more than the daily allowance.');
+ assert.strictEqual(over.status, 429);
+ assert.strictEqual(over.json.code, 'correction.rate_limited');
+ // Someone else is not affected.
+ assert.strictEqual((await req(h, 'POST', `/api/v1/entities/${e.slug}/corrections`, { token: readerToken(), body: { target_type: 'entity', body: 'The release year shown is wrong.' } })).status, 201);
+ // And the page form says so without JavaScript.
+ const page = await req(h, 'POST', `/e/${e.slug}/correct`, { cookie: cookieFor(tok), form: { target_type: 'entity', body: 'Sent from the form after the allowance.' } });
+ assert.strictEqual(page.status, 429);
+ assert.match(page.text, /At most 20 corrections a day/);
+ } finally { await h.stop(); }
+});
+
+t('audit text is bounded; readers see "an editor", never an editor\'s id; the entity JSON varies by credentials', async () => {
+ const h = await boot();
+ try {
+ const a = await createEntity(h, { name: 'Widget', kind: 'product' });
+ const b = await createEntity(h, { name: 'Widget (old listing)', kind: 'product' });
+ const m = await req(h, 'POST', `/api/v1/entities/${b.slug}/merge`, { token: editorToken(), body: { into: a.slug, note: 'n'.repeat(50000) } });
+ assert.strictEqual(m.status, 200, m.text);
+ const row = h.db.prepare("SELECT detail FROM review_audit WHERE action = 'entity.merged'").get();
+ assert.ok(row.detail.length < 3000, `audit detail is ${row.detail.length} characters`);
+
+ const e = await createEntity(h, { name: 'Portal 2', kind: 'game', aliases: [{ type: 'source', value: 'steam-reviews-portal-2' }] });
+ for (const v of [true, false]) await importItem(h, steamItem({ votedUp: v }));
+ const sigs = h.db.prepare("SELECT id FROM review_signals WHERE status = 'active'").all().map((s) => s.id);
+ assert.strictEqual((await req(h, 'POST', `/api/v1/entities/${e.slug}/summary/revisions`, { token: editorToken(), body: { overview: 'Opinion in the sample is split.', overview_signals: sigs, publish: true } })).status, 201);
+ for (const path of [`/e/${e.slug}.json`, `/api/v1/entities/${e.slug}`, `/api/v1/entities/${e.slug}/history`, `/api/v1/entities/${e.slug}/summary/revisions/1`, `/e/${e.slug}`, `/e/${e.slug}/history`]) {
+ const r = await req(h, 'GET', path);
+ assert.strictEqual(r.status, 200, path);
+ assert.ok(!r.text.includes(EDITOR), `${path} names the editor`);
+ }
+ const ed = await req(h, 'GET', `/api/v1/entities/${e.slug}/summary/revisions/1`, { token: editorToken() });
+ assert.strictEqual(ed.json.revision.author, EDITOR, 'editors see who wrote it');
+ const pub = await req(h, 'GET', `/api/v1/entities/${e.slug}/summary/revisions/1`);
+ assert.strictEqual(pub.json.revision.author, 'an editor');
+ // The revision note is public (the history lists it): the form says so.
+ assert.match((await req(h, 'GET', `/e/${e.slug}/edit`, { cookie: cookieFor(editorToken()) })).text, /Revision note \(public: shown in the summary's history\)/);
+ const json = await req(h, 'GET', `/e/${e.slug}.json`);
+ assert.match(json.headers.get('vary') || '', /Cookie/);
+ assert.match(json.headers.get('vary') || '', /Authorization/);
+ } finally { await h.stop(); }
+});
+
+t('AI text cannot spell a rating out in words; out-of-scale source ratings are not signals', async () => {
+ const h = await boot();
+ try {
+ const e = await createEntity(h, { name: 'Portal 2', kind: 'game', aliases: [{ type: 'source', value: 'steam-reviews-portal-2' }] });
+ for (const v of [true, true]) await importItem(h, steamItem({ votedUp: v }));
+ const sigs = h.db.prepare("SELECT id FROM review_signals WHERE status = 'active'").all().map((s) => s.id);
+ const AI = serviceToken({ client: 'ai', cap: ['reviews.summary.propose'] });
+ const wf = { id: 'reviews.summarize_entity', run_id: 'run_01J9ZZZZZZZZZZZZZZZZZZZZZZ' };
+ for (const text of ['Easily four and a half stars.', 'A nine out of ten experience.', 'A five-star puzzle game.']) {
+ const r = await req(h, 'POST', `/api/v1/entities/${e.slug}/summary/proposals`, { token: AI, body: { overview: text, overview_signals: sigs, workflow: wf } });
+ assert.strictEqual(r.status, 422, text);
+ assert.strictEqual(r.json.code, 'summary.rating_in_text', text);
+ }
+ const fine = await req(h, 'POST', `/api/v1/entities/${e.slug}/summary/proposals`, { token: AI, body: { overview: 'Both of the two sampled reviewers recommend it.', overview_signals: sigs, workflow: wf } });
+ assert.strictEqual(fine.status, 201, fine.text);
+
+ await createEntity(h, { name: 'Widget 3000', kind: 'product', aliases: [{ type: 'url', value: 'https://shop.example/p/widget' }, { type: 'url', value: 'https://shop.example/p/widget-b' }] });
+ const negative = productItem({ value: -4, best: 5, count: 50 });
+ const low = productItem({ value: 0.5, best: 5, count: 50, url: 'https://shop.example/p/widget-b' });
+ low.fields.aggregate_rating.worst = 1;
+ for (const [it, re] of [[negative, /below zero/], [low, /worst 1/]]) {
+ const out = await importItem(h, it);
+ assert.strictEqual(out.signal, null, JSON.stringify(out));
+ assert.strictEqual(out.item.resolution, 'resolved');
+ assert.match(out.item.signal_note, re);
+ }
+ const agg = await req(h, 'GET', '/api/v1/entities/widget-3000/aggregate');
+ assert.strictEqual(agg.json.aggregate, null);
+ } finally { await h.stop(); }
+});
+
+t('a reader-sent evidence link is nofollow ugc noreferrer on the editor desk', async () => {
+ const h = await boot();
+ try {
+ const e = await createEntity(h, { name: 'Portal 2', kind: 'game' });
+ await req(h, 'POST', `/api/v1/entities/${e.slug}/corrections`, { token: readerToken(), body: { target_type: 'entity', body: 'See the official page for the year.', evidence_url: 'https://example.org/proof' } });
+ await req(h, 'POST', `/api/v1/entities/${e.slug}/corrections`, { token: readerToken(), body: { target_type: 'entity', body: 'A script link as evidence here.', evidence_url: 'javascript:alert(1)' } }).then((r) => assert.strictEqual(r.status, 422));
+ const desk = await req(h, 'GET', '/editor', { cookie: cookieFor(editorToken()) });
+ assert.match(desk.text, /<a href="https:\/\/example\.org\/proof" rel="nofollow ugc noopener noreferrer">evidence<\/a>/);
+ assert.ok(!desk.text.includes('javascript:'));
+ } finally { await h.stop(); }
+});