From 8fc6b5cf764066b140e3e5a0265142e8daa1efa1 Mon Sep 17 00:00:00 2001 From: Tek Raj Chhetri Date: Thu, 13 Aug 2026 17:44:48 +0545 Subject: [PATCH] ml_service: add an unauthenticated read surface for published reviews MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing could read a review its author marked Public. Every synth-scholar route depends on get_current_user, GET /reviews is owner-scoped, and _session_or_404 404s for non-owners — so the public web pages at /knowledge-base/synth-scholar failed for anonymous visitors and showed signed-in ones their own reviews. The is_public flag was write-only. Add /api/synth-scholar/public/reviews{,/{id},/{id}/log,/{id}/export}. Three rules hold across all four: * no get_current_user, so an anonymous browser can read them; * every lookup goes through store.list_public / get_public, which require is_public AND completed — the filter is in SQL, not the caller, so other people's drafts are never shipped to a browser to be filtered there; * an unpublished id answers 404, not 403, so the status code does not confirm it exists. list_public skips the runtime-state merge: a completed review has none, and consulting it would leak progress for a row being re-run. Export shares one _export_session helper with the authenticated route (the two were identical past the access check) and one _EXPORT_FORMAT_PATTERN constant, so a format added for signed-in users cannot silently 400 on a public review. No behaviour change to the authenticated routes. Nothing new is exposed: the detail payload already excludes openrouter_api_key from run_request, and these routes serve only what an author explicitly published. --- ml_service/core/synth_scholar/routes.py | 102 +++++++++++++++++++++++- ml_service/core/synth_scholar/store.py | 31 +++++++ 2 files changed, 129 insertions(+), 4 deletions(-) diff --git a/ml_service/core/synth_scholar/routes.py b/ml_service/core/synth_scholar/routes.py index 73c935f..3d6c57d 100644 --- a/ml_service/core/synth_scholar/routes.py +++ b/ml_service/core/synth_scholar/routes.py @@ -1247,18 +1247,29 @@ async def cancel_review( ) +_EXPORT_FORMAT_PATTERN = ( + r"^(markdown|json|bibtex|ttl|jsonld|rubric_markdown|rubric_json|charting_markdown" + r"|charting_json|appraisal_markdown|appraisal_json|narrative_summary_markdown" + r"|narrative_summary_json)$" +) + + @router.get("/synth-scholar/reviews/{review_id}/export", tags=["SynthScholar — export"]) async def export_review( review_id: str, user: Annotated[dict, Depends(get_current_user)], - format: str = Query( - default="markdown", - pattern=r"^(markdown|json|bibtex|ttl|jsonld|rubric_markdown|rubric_json|charting_markdown|charting_json|appraisal_markdown|appraisal_json|narrative_summary_markdown|narrative_summary_json)$", - ), + format: str = Query(default="markdown", pattern=_EXPORT_FORMAT_PATTERN), model: Optional[str] = Query(default=None, description="Compare-mode only: export a single model's result by model_name"), ): """Export a completed review in the requested format.""" session = await _session_or_404(review_id, user) + return await _export_session(session, format, model) + + +# Everything past the access check is identical for the authenticated route above +# and the public one further down, so it lives here once — otherwise the two +# drift, and a format added for signed-in users silently 400s on public reviews. +async def _export_session(session: ReviewSession, format: str, model: Optional[str]): if session.status != ReviewStatus.COMPLETED or not session.result: raise HTTPException( status_code=400, @@ -1440,6 +1451,89 @@ async def get_pipeline_log( } +# --------------------------------------------------------------------------- +# Public (unauthenticated) read surface +# +# Backs /knowledge-base/synth-scholar in the web UI, which is reachable without +# signing in. Those pages previously called the authenticated routes above, which +# cannot work for an anonymous visitor: the UI has no credential to present, so +# the token exchange fails before any request is sent ("ML service requires a +# signed-in session"). Nor could they work for a signed-in visitor, because +# GET /reviews is owner-scoped — the "public listing" showed the viewer their own +# reviews, and a public review's detail page 404'd for everyone but its author. +# +# So `is_public` had no reader. These routes are it. Three rules hold throughout: +# * no `Depends(get_current_user)` — that is the point; +# * every lookup goes through review_store.get_public / list_public, which +# require is_public AND completed, so an unpublished review is invisible; +# * a review that is not published answers 404, never 403 — a 403 would confirm +# the id exists. +# Nothing here is owner-scoped, because published means published to everyone. +# --------------------------------------------------------------------------- + + +async def _public_session_or_404(review_id: str) -> ReviewSession: + session = await review_store.get_public(review_id) + if not session: + raise HTTPException(status_code=404, detail=f"Review '{review_id}' not found") + return session + + +@router.get( + "/synth-scholar/public/reviews", + response_model=list[ReviewSummaryResponse], + tags=["SynthScholar — public"], +) +async def list_public_reviews(): + """List every completed review its author marked Public. No auth.""" + sessions = await review_store.list_public() + return [_to_summary_response(s) for s in sessions] + + +@router.get( + "/synth-scholar/public/reviews/{review_id}", + response_model=ReviewDetailResponse, + tags=["SynthScholar — public"], +) +async def get_public_review(review_id: str): + """Full detail for one published review. 404 if it is not published.""" + return _to_detail_response(await _public_session_or_404(review_id)) + + +@router.get("/synth-scholar/public/reviews/{review_id}/log", tags=["SynthScholar — public"]) +async def get_public_review_log(review_id: str): + """Pipeline log for a published review — the provenance timeline reads this. + + Same content the author sees: the log records which pipeline step ran when, + which is exactly the provenance a published review is meant to carry. + """ + session = await _public_session_or_404(review_id) + log_entries = list(session.pipeline_log) + return { + "review_id": review_id, + "status": session.status.value, + "step_count": session.progress_step, + "log": log_entries, + "log_events": [ + {"step": i + 1, "message": msg, "timestamp": ts} + for i, (ts, msg) in enumerate(_parse_log_entry(e) for e in log_entries) + ], + } + + +@router.get("/synth-scholar/public/reviews/{review_id}/export", tags=["SynthScholar — public"]) +async def export_public_review( + review_id: str, + format: str = Query(default="markdown", pattern=_EXPORT_FORMAT_PATTERN), + model: Optional[str] = Query( + default=None, description="Compare-mode only: export a single model's result by model_name" + ), +): + """Export a published review. Same formats as the authenticated route.""" + session = await _public_session_or_404(review_id) + return await _export_session(session, format, model) + + @router.patch( "/synth-scholar/reviews/{review_id}/visibility", response_model=ReviewSummaryResponse, diff --git a/ml_service/core/synth_scholar/store.py b/ml_service/core/synth_scholar/store.py index 2bb6769..201284c 100644 --- a/ml_service/core/synth_scholar/store.py +++ b/ml_service/core/synth_scholar/store.py @@ -723,6 +723,37 @@ async def list_for_owner(self, owner_email: Optional[str]) -> list[ReviewSession sessions.append(s) return sessions + async def list_public(self) -> list[ReviewSession]: + """List every review its author published — `is_public` and completed. + + Read by the unauthenticated public routes, so the filter lives in SQL + rather than in the caller: a client-side filter over a full listing would + mean shipping other people's unpublished reviews to the browser first. + Runtime (in-flight) state is deliberately not merged in — a completed + review has none, and consulting it would only leak progress for rows that + are being re-run. + """ + async with async_session() as db: + result = await db.execute( + select(ReviewRow) + .where(ReviewRow.is_public.is_(True)) + .where(ReviewRow.status == ReviewStatus.COMPLETED.value) + .order_by(ReviewRow.created_at.desc()) + ) + rows = result.scalars().all() + return [_row_to_session(row) for row in rows] + + async def get_public(self, review_id: str) -> Optional[ReviewSession]: + """Fetch a review only if its author published it. Returns None otherwise + — callers turn that into a 404 so an unpublished review's existence is not + disclosed by the status code.""" + session = await self.get(review_id) + if not session: + return None + if not session.is_public or session.status != ReviewStatus.COMPLETED: + return None + return session + async def delete(self, review_id: str) -> bool: self._runtime.pop(review_id, None) async with async_session() as db: