Protect real RAG search and answer APIs with authenticated owner scope#22
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 971ebe4bed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const user = await serverAuth.requireAuthenticatedUser(request, supabase); | ||
| ownerId = user.id; |
There was a problem hiding this comment.
Adding this auth gate changes the real-mode contract, but the existing tests/private-access-routes.test.ts suite still calls /api/search, /api/answer, and /api/answer/stream without an Authorization header and asserts successful unauthenticated behavior (for example the tests around allows unauthenticated search and answer without owner scope, public rate limiting, coalescing, and streaming without auth). With this line in place those requests return 401 before the mocked RAG functions run, so the normal npm test suite will fail unless those tests are updated or converted to demo/local-no-auth/authenticated scenarios.
Useful? React with 👍 / 👎.
| supabase = createAdminClient(); | ||
| const user = await serverAuth.requireAuthenticatedUser(request, supabase); | ||
| ownerId = user.id; |
There was a problem hiding this comment.
This new real-mode auth requirement also breaks the existing npm run eval:search:api workflow outside demo/local-no-auth mode: scripts/eval-search-api.ts posts to /api/search with only Content-Type, so every eval case now receives 401 and --fail-on-threshold will fail before measuring retrieval quality. The API eval script needs a way to supply a bearer token or otherwise opt into the same authenticated/local owner flow as the other RAG eval scripts.
Useful? React with 👍 / 👎.
| supabase = createAdminClient(); | ||
| const user = await serverAuth.requireAuthenticatedUser(request, supabase); | ||
| ownerId = user.id; |
There was a problem hiding this comment.
After this auth check, the dashboard still enables the main search button in real mode whenever setup is ready because ClinicalDashboard passes realDataReady={canRunSearch} and canRunSearch does not include canUsePrivateApis. For an unsigned-in user with a configured real backend, submitting a search now just calls these endpoints without credentials, marks the session expired, and shows a 401 error instead of disabling the action or requiring sign-in first.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae29521c57
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return ( | ||
| process.env.RAG_EVAL_API_AUTH_TOKEN?.trim() || | ||
| process.env.RAG_EVAL_AUTH_TOKEN?.trim() || | ||
| process.env.SUPABASE_ACCESS_TOKEN?.trim() || |
There was a problem hiding this comment.
When a developer has the Supabase CLI configured in CI/local, SUPABASE_ACCESS_TOKEN is a personal access token for the Management API, not an Auth user JWT (Supabase docs describe it as a personal access token). This script now silently forwards that long-lived token as Authorization: Bearer to whichever --base-url is used and suppresses the no-token diagnostic, so real-mode evals still 401 while potentially exposing a management token to a preview/non-local URL. Please only auto-read the RAG-specific env vars/explicit flag, or otherwise require a real user session JWT.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| supabase = createAdminClient(); | ||
| const user = await serverAuth.requireAuthenticatedUser(request, supabase); |
There was a problem hiding this comment.
For requests that include any invalid Authorization: Bearer ... header, this call goes out to Supabase Auth and throws before the per-IP limiter at lines 581-582 is consumed, so those requests are never counted; the same auth-before-limit order was added to /api/answer and /api/answer/stream. A client can therefore send unlimited invalid-token attempts and force external auth validation on each request instead of receiving 429s from this endpoint. Move the cheap limiter before token validation, or add a separate auth-failure limiter, so authentication failures are throttled too.
Useful? React with 👍 / 👎.
1 participant
Summary
Issues addressed
/api/search/api/answerand/api/answer/streamFiles changed
src/app/api/search/route.tsownerIdinto retrieval/related-document lookup, scope coalescing by owner, and write touched telemetry with owner ID.src/app/api/answer/route.tsownerIdinto answer generation.src/app/api/answer/stream/route.tsownerIdinto streamed answer generation.tests/private-rag-access.test.tsBehaviour changes
/api/searchrequests now return401before retrieval./api/answerrequests now return401before generation./api/answer/streamrequests now return401before stream retrieval/generation starts.Tests added or updated
tests/private-rag-access.test.tscovering:Checks run
npm run lintnpm run typechecknpm run testnpm run buildManual QA
Suggested reviewer steps:
POST /api/searchwithout auth and expect401.POST /api/answerwithout auth and expect401.POST /api/answer/streamwithout auth and expect401before SSE retrieval starts.Risk and rollback
Risk:
Rollback:
Out of scope
Notes for reviewer
scripts/eval-search-api.tscurrently calls/api/searchwithout an auth header; after this PR, real non-demo usage of that script may need an auth/local-no-auth follow-up if it is still used against private data.response.status === 401handling.