Skip to content

fix(routes): gate content-analytics view/feedback writes by page access (#890) - #985

Merged
laboef1900 merged 2 commits into
devfrom
feature/issue-890-analytics-page-access-gate
Jul 11, 2026
Merged

fix(routes): gate content-analytics view/feedback writes by page access (#890)#985
laboef1900 merged 2 commits into
devfrom
feature/issue-890-analytics-page-access-gate

Conversation

@laboef1900

Copy link
Copy Markdown
Contributor

Summary

  • POST /pages/:id/feedback and POST /pages/:id/view in content-analytics.ts wrote directly into article_feedback / page_views with no page lookup or access check.
  • A nonexistent page id triggered a Postgres FK violation (23503) surfaced as HTTP 500 — a 201-vs-500 existence oracle distinguishing restricted pages from missing ones — and any authenticated user could pump views/feedback into pages they cannot read, polluting admin-only analytics.
  • Both handlers now gate on userCanAccessPage before touching the tables, mirroring the Security: knowledge routes missing per-space/page authorization (IDOR) #733 hardening already applied to comments.ts.

Closes #890.

Root cause

The two POST write handlers never resolved the page or called userCanAccessPage, so the page_id NOT NULL REFERENCES pages(id) FK was the only (and wrong) line of defense — raising an unhandled 500 for missing ids and leaving restricted pages writable.

Fix

  • Import userCanAccessPage from rbac-service.
  • Add if (!(await userCanAccessPage(userId, pageId))) return reply.notFound('Page not found'); to the feedback handler (before the upsert) and the view handler (before the dedup SELECT).
  • userCanAccessPage returns false for missing, soft-deleted, and restricted pages alike, collapsing them into a uniform 404 that closes the FK-500 path and the oracle at once.

Testing

  • TDD: extended content-analytics-routes.test.ts with two boundary-mocked gating tests (rbac-service mocked); fails before the fix (feedback 500, view 201, query invoked), passes after (404, no query call).
  • cd backend && npx vitest run src/routes/knowledge/content-analytics-routes.test.ts (23 pass) - eslint (pass) - tsc --noEmit (pass)

Generated with Claude Code

laboef1900 and others added 2 commits July 11, 2026 00:17
…ss (#890)

POST /pages/:id/feedback and /pages/:id/view wrote straight into article_feedback/page_views with no access check, so a missing page id raised a Postgres FK 23503 surfaced as HTTP 500 (an existence oracle) and any user could pollute admin analytics for pages they cannot read. Both handlers now call userCanAccessPage before touching the tables (mirroring the #733 hardening in comments.ts), returning a uniform 404 for missing/restricted/deleted pages. Covered by two new boundary-mocked route tests that fail before the gate (500/201) and pass after (404, no query).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-up on #985: the aggregate feedback read was left
ungated, leaving the same existence/restricted-page oracle the PR
closed for writes.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@laboef1900
laboef1900 merged commit cf4505d into dev Jul 11, 2026
5 checks passed
@laboef1900
laboef1900 deleted the feature/issue-890-analytics-page-access-gate branch July 13, 2026 05:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

POST /pages/:id/feedback and /pages/:id/view skip page access/existence checks — private-page probing and 500s via FK violation

1 participant