diff --git a/CHANGELOG.md b/CHANGELOG.md index 4aebb5a..e38d0d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ CalVer image tags (`stable-YYYY.MM.N`, `dev-YYYY.MM.N`) are produced for every C ## [Unreleased] +### Fixed +- **Unauthenticated browser requests to `GET /api/initial-workspace.zip` now redirect to `/login?next=/api/initial-workspace.zip` instead of returning a raw JSON 401** (#315). This is the one `/api/*` endpoint that's designed to be hit directly from a browser bookmark (the analyst clean-install zip), so it intentionally opts out of the global `_API_PATH_PREFIXES` "never redirect /api/*" contract in `app/main.py`. CLI / curl / other API clients (any `Accept` without `text/html` — including the `*/*` default) keep getting the 401 they can handle. + ## [0.54.17] — 2026-05-15 ### Changed diff --git a/app/api/initial_workspace.py b/app/api/initial_workspace.py index 838763c..1858833 100644 --- a/app/api/initial_workspace.py +++ b/app/api/initial_workspace.py @@ -30,11 +30,12 @@ from datetime import datetime, timezone from typing import Any, Optional import duckdb -from fastapi import APIRouter, Depends, HTTPException, Response +from fastapi import APIRouter, Depends, HTTPException, Request, Response +from fastapi.responses import RedirectResponse from pydantic import BaseModel from app.auth.access import require_admin -from app.auth.dependencies import _get_db, get_current_user +from app.auth.dependencies import _get_db, get_current_user, get_optional_user from app.secrets import persist_overlay_token from src.initial_workspace import ( TemplateValidationError, @@ -510,7 +511,8 @@ async def analyst_status( @router.get("/api/initial-workspace.zip") async def analyst_zip( - user: dict = Depends(get_current_user), + request: Request, + user: Optional[dict] = Depends(get_optional_user), conn: duckdb.DuckDBPyConnection = Depends(_get_db), ): """Return the zip of the cloned template tree (sans ``.git/``). @@ -524,6 +526,22 @@ async def analyst_zip( this; defense in depth). 503 when configured but never synced — the CLI then surfaces a typed error pointing at "Sync now". """ + if user is None: + # Browser → redirect to /login (target preserved via ?next=). + # CLI / curl / API client → raw 401 they can handle. + # This endpoint is the one `/api/*` URL designed to be hit directly + # from a browser bookmark (analyst clean-install zip), so it + # intentionally opts out of the global `_API_PATH_PREFIXES` + # "never redirect /api/*" contract in `app/main.py`. Matching only + # `text/html` — NOT `*/*` — mirrors `_wants_html()` in `app/main.py`: + # `*/*` is curl's default and must keep getting the raw 401 so + # tooling that parses `{"detail": "..."}` doesn't silently break. + if "text/html" in request.headers.get("accept", ""): + return RedirectResponse( + url="/login?next=/api/initial-workspace.zip", status_code=302 + ) + raise HTTPException(status_code=401, detail="Missing or invalid Authorization header") + section = _read_section() if not section.get("url"): raise HTTPException(status_code=404, detail={"kind": "not_configured"}) diff --git a/tests/test_initial_workspace_api.py b/tests/test_initial_workspace_api.py index 69e627f..ebef314 100644 --- a/tests/test_initial_workspace_api.py +++ b/tests/test_initial_workspace_api.py @@ -586,6 +586,50 @@ def test_analyst_status_configured_synced(web_client, fake_remote): assert "CLAUDE.md" in body["files"] +def test_analyst_zip_browser_unauthenticated_redirects_to_login(web_client): + """Unauthenticated browser request (Accept: text/html) redirects to /login.""" + r = web_client.get( + "/api/initial-workspace.zip", + headers={"Accept": "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"}, + follow_redirects=False, + ) + assert r.status_code == 302 + assert r.headers["location"] == "/login?next=/api/initial-workspace.zip" + + +def test_analyst_zip_api_unauthenticated_returns_401(web_client): + """Unauthenticated API client (no text/html in Accept) still gets a JSON 401.""" + r = web_client.get( + "/api/initial-workspace.zip", + headers={"Accept": "application/json"}, + ) + assert r.status_code == 401 + + +def test_analyst_zip_curl_default_accept_returns_401(web_client): + """`Accept: */*` (curl's default with no `-H`) lands in the 401 branch. + + Mirrors the `_wants_html()` contract in `app/main.py`: `*/*` must NOT + silently flip a curl/tooling client to an HTML response — they expect + `{"detail": "..."}` and a real 401. + """ + r = web_client.get( + "/api/initial-workspace.zip", + headers={"Accept": "*/*"}, + ) + assert r.status_code == 401 + + +def test_analyst_zip_empty_accept_returns_401(web_client): + """Empty `Accept` header lands in the 401 branch — same shape as the `*/*` + case (no `text/html` substring means: not a browser, give the raw 401).""" + r = web_client.get( + "/api/initial-workspace.zip", + headers={"Accept": ""}, + ) + assert r.status_code == 401 + + def test_analyst_zip_404_when_not_configured(web_client): """GET /api/initial-workspace.zip returns 404 when no template.""" headers = _make_user(web_client)