diff --git a/README.md b/README.md index bf61ee64..010f7ee6 100644 --- a/README.md +++ b/README.md @@ -320,7 +320,7 @@ The server looks for `server-config.yaml` in the **working directory** by defaul ```bash AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_CONFIG_FILE=/etc/ci-server/config.yaml \ - uvicorn context_intelligence_server.main:app + uvicorn context_intelligence_server.main:asgi_app ``` #### Option B — Environment variables @@ -333,7 +333,7 @@ AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_NEO4J_BROWSER_URL=http://localhost:7474 \ AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_NEO4J_PASSWORD="" \ AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_BLOB_PATH=/tmp/ci-blobs \ AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_LOG_PATH=/tmp/ci-logs/server.jsonl \ - uvicorn context_intelligence_server.main:app --reload + uvicorn context_intelligence_server.main:asgi_app --reload ``` #### Option C — Mix both @@ -350,17 +350,18 @@ log_path: /data/ci-logs/server.jsonl ```bash # Override only the password at runtime (e.g. from a secrets manager) AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_NEO4J_PASSWORD=hunter2 \ - uvicorn context_intelligence_server.main:app + uvicorn context_intelligence_server.main:asgi_app ``` ### 4. Start the server ```bash # With auto-reload (development) -uvicorn context_intelligence_server.main:app --reload +uvicorn context_intelligence_server.main:asgi_app --reload -# Production — bind explicitly -uvicorn context_intelligence_server.main:app \ +# Production — bind explicitly (MUST serve asgi_app: the auth-wrapped ASGI app; +# main:app is the bare app with NO auth middleware) +uvicorn context_intelligence_server.main:asgi_app \ --host 0.0.0.0 \ --port 8000 \ --workers 1 diff --git a/context_intelligence_server/authz.py b/context_intelligence_server/authz.py index dcafef53..cb2b321b 100644 --- a/context_intelligence_server/authz.py +++ b/context_intelligence_server/authz.py @@ -21,21 +21,34 @@ def _is_write_capable(request: Request) -> bool: """True for any human/static principal; for a service iff it holds Contributor. - When ``is_service`` is absent from scope state the principal defaults to - False (human-like), making it write-capable. This default is ONLY - reachable in two safe situations: - - 1. ``allow_unauthenticated=True`` (dev/test mode, no credential required). - 2. Auth-exempt paths (/status, /version, /skills/*) — none of which carry - a capability gate, so this function is never called for them. + Fail-closed on unpopulated scope state (Step 3, doc 16 W1): when + ``is_service`` is absent from scope state entirely, the request never + passed through BearerTokenMiddleware's identity-setting path. That is + only a safe situation when the server is explicitly in the + ``allow_unauthenticated=True`` dev/test opt-out (read from + ``app.state.allow_unauthenticated``) — otherwise this denies. In auth-enabled production mode BearerTokenMiddleware ALWAYS sets - ``is_service`` on scope state before any route handler or dependency runs, - so the default is never exercised in that path. + ``is_service`` on scope state before any route handler or dependency runs + (auth.py), and the boot guard (``_assert_capability_routes_not_exempt``, + main.py) keeps every capability-gated route non-exempt, so the + absent-``is_service`` branch is unreachable in production. + + Once ``is_service`` IS present, ``False`` (human / static / easyauth) + remains always write-capable — unchanged. """ state: dict = request.scope.get("state", {}) - if not state.get("is_service", False): - return True # human / static — always write-capable, unchanged + if "is_service" not in state: + # Unpopulated scope state → fail CLOSED (Step 3, doc 16 W1). + # In auth-enabled mode this branch is unreachable: middleware always + # sets is_service (auth.py) and the boot guard keeps capability + # routes non-exempt (main.py). It is reachable ONLY in the explicit + # allow_unauthenticated dev/test opt-out, where the middleware + # short-circuits without populating state. Honour that opt-out; + # otherwise deny. + return bool(getattr(request.app.state, "allow_unauthenticated", False)) + if not state["is_service"]: + return True # human / static / easyauth — write-capable, unchanged roles: list[str] = state.get("roles", []) role: str = getattr(request.app.state, "service_data_role", "") return bool(role) and role in roles diff --git a/context_intelligence_server/config.py b/context_intelligence_server/config.py index 0afb9f6b..158fdd0c 100644 --- a/context_intelligence_server/config.py +++ b/context_intelligence_server/config.py @@ -805,6 +805,20 @@ def _normalize_service_role_fields(cls, v: object) -> str: neo4j_password: str = "password" neo4j_browser_url: str = "http://localhost:7474" + # Development mode. True reveals local-only surfaces (e.g. the Neo4j browser + # URL on /status); default False (production) hides them. Scoped bool, NOT a + # general "environment" string: this is the only consumer today, and a bool + # cannot sprout `if environment == "..."` sprawl or a case-sensitivity foot-gun + # (council W4). Widen to an environment enum later IF a real second consumer + # appears. Env: AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_IS_DEVELOPMENT=true + is_development: bool = False + + # Explicit opt-in to expose the Neo4j Browser (7474 HTTP) URL on /status even + # outside development. Default False -> hidden (fail-safe). In Azure this stays + # False: the browser URL points into a private VNet and must not be surfaced. + # Env: AMPLIFIER_CONTEXT_INTELLIGENCE_SERVER_SHOW_NEO4J_BROWSER_URL=true + show_neo4j_browser_url: bool = False + # Structured two-client config (doc 11). OPTIONAL for backward-compat: when # absent, BOTH clients fall back to the legacy flat neo4j_* fields above. # The real amplifier-online.yaml MUST set this explicitly (see @@ -841,6 +855,13 @@ def resolve_neo4j_query(self) -> Neo4jClientConfig: access_mode="READ", ) + def neo4j_browser_url_visible(self) -> bool: + """True iff the Neo4j Browser (7474 HTTP) URL may be surfaced on /status. + + Fail-safe hidden: only development OR the explicit opt-in reveals it. + """ + return self.is_development or self.show_neo4j_browser_url + # ------------------------------------------------------------------------- # Storage paths # ------------------------------------------------------------------------- diff --git a/context_intelligence_server/main.py b/context_intelligence_server/main.py index cba29c0a..21e4147e 100644 --- a/context_intelligence_server/main.py +++ b/context_intelligence_server/main.py @@ -42,7 +42,10 @@ from context_intelligence_server.identity_store import IdentityStore from context_intelligence_server.dashboard import build_status_response from context_intelligence_server.routers.admin import router as admin_router -from context_intelligence_server.routers.queues import router as queues_router +from context_intelligence_server.routers.queues import ( + dead_letter_admin_router, + router as queues_router, +) from context_intelligence_server.routers.skills import SkillRegistry from context_intelligence_server.routers.skills import router as skills_router from context_intelligence_server.routers.version import router as version_router @@ -219,6 +222,10 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: await app.state.neo4j_query_driver.close() +# NOTE (doc 16 §10.2): `app` is the BARE FastAPI app — NO auth middleware and +# NO fail-closed startup gate. It exists only for internal wiring and tests. +# The ONLY deployable entrypoint is `asgi_app` (defined below): serving `main:app` +# directly ships an unauthenticated server. Deploy `main:asgi_app`, never `main:app`. app = FastAPI( title="Context Intelligence Server", version=__version__, @@ -233,6 +240,9 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: app.include_router(skills_router) app.include_router(version_router) app.include_router(queues_router) +# doc 16 W2: dead-letter purge/replay live under /admin so the static admin-key +# fast-path (auth._is_admin_route) can reach them; list stays on queues_router. +app.include_router(dead_letter_admin_router) _start_time = time.time() registry = SessionRegistry() # Expose the registry singleton on app.state so routers can read it via @@ -352,8 +362,13 @@ def _assert_capability_routes_not_exempt(settings: Settings) -> None: require_read, require_write, ) + from context_intelligence_server.routers.admin import require_admin # noqa: PLC0415 - _capability_deps = {require_read: "require_read", require_write: "require_write"} + _capability_deps = { + require_read: "require_read", + require_write: "require_write", + require_admin: "require_admin", + } def _collect_calls(dependant: Any) -> list[Any]: """Recursively collect every ``.call`` in a FastAPI dependant tree.""" @@ -487,6 +502,10 @@ def create_asgi_app( # M2: service capability role names for require_write / require_read deps. app.state.service_data_role = s.service_data_role app.state.reader_role = s.reader_role + # Step 3 (doc 16 W1): expose the dev/test opt-out to the capability + # predicate. _is_write_capable() fails closed on unpopulated scope state + # UNLESS this flag is set — the only sanctioned empty-state write path. + app.state.allow_unauthenticated = s.allow_unauthenticated # Compute the admin-key digest for the middleware (static mode only). # The middleware checks the bearer token's sha256 against this digest BEFORE @@ -685,7 +704,13 @@ async def get_status(request: Request) -> dict[str, Any]: request.app, "neo4j_query_driver" ) response["neo4j_url"] = _settings.resolve_neo4j_admin().url - response["neo4j_browser_url"] = _settings.neo4j_browser_url + # doc 16 W4: hide the 7474 HTTP browser URL unless dev / explicit opt-in. + # Key is ALWAYS present (stable /status shape) but null when hidden so the + # dashboard treats absence-of-value as "not available". Bolt url (neo4j_url) + # and health stay visible. + response["neo4j_browser_url"] = ( + _settings.neo4j_browser_url if _settings.neo4j_browser_url_visible() else None + ) # Additive, aggregate-only conservation metrics (D3). /status is # unauthenticated, so this block must NOT carry the per-key table or the # dead-letter listing — both are authenticated-only. diff --git a/context_intelligence_server/routers/admin.py b/context_intelligence_server/routers/admin.py index 24f4d9f3..353f30fa 100644 --- a/context_intelligence_server/routers/admin.py +++ b/context_intelligence_server/routers/admin.py @@ -348,6 +348,14 @@ def _require_key_store(request: Request) -> IdentityStore: # Router # --------------------------------------------------------------------------- +# NOTE (doc 16 W2, council crusty F-W2b): this is NOT the only ``/admin``-prefixed +# router. A SECOND one — ``dead_letter_admin_router`` in ``routers/queues.py`` — +# also mounts under ``/admin`` for the dead-letter drain routes +# (``POST /admin/queues/dead-letter/{worker_key}/{purge,replay}``). The path +# spaces are disjoint (``/admin/identities`` + ``/admin/keys`` here vs +# ``/admin/queues/dead-letter/*`` there), and both share this same +# ``require_admin`` dependency object so the admin gate + test override identity +# apply uniformly. Both must be ``include_router``'d in ``main.py``. router = APIRouter(prefix="/admin", dependencies=[Depends(require_admin)]) diff --git a/context_intelligence_server/routers/queues.py b/context_intelligence_server/routers/queues.py index 184319fb..4e2d886d 100644 --- a/context_intelligence_server/routers/queues.py +++ b/context_intelligence_server/routers/queues.py @@ -15,12 +15,25 @@ from fastapi import APIRouter, Depends, HTTPException # noqa: F401 (HTTPException per spec) from fastapi.requests import Request -from context_intelligence_server.authz import require_read, require_write +from context_intelligence_server.authz import require_read +from context_intelligence_server.routers.admin import require_admin logger = logging.getLogger(__name__) router = APIRouter() +# Second router: dead-letter admin mutations (purge/replay) live under /admin so +# the static admin-key fast-path (auth._is_admin_route, /admin/* ONLY) can reach +# them (council TB-1 fix, doc 16 W2). require_admin in static mode allows only +# when scope state has is_admin=True, and that flag is set ONLY on /admin/* paths; +# a require_admin route OFF /admin/* would be unreachable by the static admin key. +# require_admin is applied router-wide here (mirrors admin.py) — do NOT add a +# per-route Depends(require_admin). +dead_letter_admin_router = APIRouter( + prefix="/admin", + dependencies=[Depends(require_admin)], +) + def _decode_payload(record: dict[str, Any]) -> bytes: """Return the original payload bytes from a dead-letter record. @@ -63,10 +76,7 @@ async def list_dead_letters(request: Request) -> dict[str, Any]: return {"dead_letters": entries} -@router.post( - "/queues/dead-letter/{worker_key:path}/purge", - dependencies=[Depends(require_write)], -) +@dead_letter_admin_router.post("/queues/dead-letter/{worker_key:path}/purge") async def purge_dead_letters(worker_key: str, request: Request) -> dict[str, Any]: """Purge all dead-letter records for ``worker_key``. @@ -83,10 +93,7 @@ async def purge_dead_letters(worker_key: str, request: Request) -> dict[str, Any return {"worker_key": worker_key, "purged": purged} -@router.post( - "/queues/dead-letter/{worker_key:path}/replay", - dependencies=[Depends(require_write)], -) +@dead_letter_admin_router.post("/queues/dead-letter/{worker_key:path}/replay") async def replay_dead_letters(worker_key: str, request: Request) -> dict[str, Any]: """Re-enqueue every dead-letter record for ``worker_key`` then purge them. diff --git a/context_intelligence_server/web/dashboard.html b/context_intelligence_server/web/dashboard.html index a1e3a350..46940c8f 100644 --- a/context_intelligence_server/web/dashboard.html +++ b/context_intelligence_server/web/dashboard.html @@ -147,7 +147,7 @@

Context Intelligence

- +
WorkerCountLast errorActionsWorkerCountLast error
diff --git a/context_intelligence_server/web/static/js/dashboard.js b/context_intelligence_server/web/static/js/dashboard.js index f9cd9b5f..66ddeaf5 100644 --- a/context_intelligence_server/web/static/js/dashboard.js +++ b/context_intelligence_server/web/static/js/dashboard.js @@ -1,5 +1,5 @@ import { fetchStatus, postCypher } from './api.js'; -import { renderQueues, fetchDeadLetters, renderDeadLetters, renderDeadLetterError, wireDeadLetterActions } from './queues-panel.js'; +import { renderQueues, fetchDeadLetters, renderDeadLetters, renderDeadLetterError } from './queues-panel.js'; function timeAgo(ts) { if (!ts) return '-'; @@ -101,7 +101,9 @@ function setTab(name) { document.getElementById('tab-overview')?.addEventListener('click', () => setTab('overview')); document.getElementById('tab-queues')?.addEventListener('click', () => setTab('queues')); document.getElementById('hint-go-queues')?.addEventListener('click', () => setTab('queues')); -wireDeadLetterActions({ onAuthLost }); +// Dead-letter drain (replay/purge) is admin-only and lives on the admin surface +// (doc 04 §3); the general dashboard renders the dead-letter list read-only, so +// there are no action handlers to wire here. async function refresh() { try { diff --git a/context_intelligence_server/web/static/js/dashboard.js.test.mjs b/context_intelligence_server/web/static/js/dashboard.js.test.mjs index d130367d..9c0a5526 100644 --- a/context_intelligence_server/web/static/js/dashboard.js.test.mjs +++ b/context_intelligence_server/web/static/js/dashboard.js.test.mjs @@ -272,10 +272,12 @@ describe('dashboard.js Queues tab wiring (C2 re-arch)', () => { ); }); - test('wires dead-letter actions and the hint-go-queues shortcut', () => { + test('does NOT wire dead-letter drain actions (admin-only, doc 04 §3) and keeps the hint-go-queues shortcut', () => { + // Dead-letter drain (replay/purge) is admin-only and lives on the admin + // surface (doc 04 §3). The general dashboard must NOT wire drain controls. assert.ok( - jsSource.includes('wireDeadLetterActions('), - 'dashboard.js should call wireDeadLetterActions(...)' + !jsSource.includes('wireDeadLetterActions('), + 'dashboard.js must NOT call wireDeadLetterActions(...) — drain is admin-only (doc 04 §3)' ); assert.ok( jsSource.includes('hint-go-queues'), diff --git a/context_intelligence_server/web/static/js/queues-panel.js b/context_intelligence_server/web/static/js/queues-panel.js index 6f1012b1..4f2f70b0 100644 --- a/context_intelligence_server/web/static/js/queues-panel.js +++ b/context_intelligence_server/web/static/js/queues-panel.js @@ -104,11 +104,10 @@ function fmtTs(ts) { } } -// ── Authenticated fetch wrappers ──────────────────────────────────────────── -// Each wrapper attaches the HTTP status to thrown errors (err.status) so -// action handlers can branch on 401/400 honestly. worker_key is -// encodeURIComponent'd defensively (it is a file stem: a session UUID or a -// _no_session__); an unsafe key yields HTTP 400 from the server. +// ── Authenticated fetch (read-only) ───────────────────────────────────────── +// The wrapper attaches the HTTP status to thrown errors (err.status) so the +// caller can branch on 401 (auth lost) honestly. Only the read-only list fetch +// remains; drain (replay/purge) moved to the admin surface (doc 04 §3). function _httpError(label, res) { const err = new Error(`${label} failed: ${res.status}`); @@ -122,26 +121,17 @@ export async function fetchDeadLetters() { return res.json(); } -export async function replayWorker(workerKey) { - const url = `/queues/dead-letter/${encodeURIComponent(workerKey)}/replay`; - const res = await fetch(url, { method: 'POST', headers: authHeaders() }); - if (!res.ok) throw _httpError('replay', res); - return res.json(); -} - -export async function purgeWorker(workerKey) { - const url = `/queues/dead-letter/${encodeURIComponent(workerKey)}/purge`; - const res = await fetch(url, { method: 'POST', headers: authHeaders() }); - if (!res.ok) throw _httpError('purge', res); - return res.json(); -} +// NOTE (doc 04 §3): dead-letter DRAIN (replay/purge) is an ADMIN operation and +// lives ONLY on the admin surface. The general dashboard is READ-ONLY for dead +// letters — it fetches and renders the list (above), but never mutates. The +// replay/purge fetch wrappers, action buttons, and confirm flow were removed +// from this panel accordingly; the backend routes now live under /admin/* and +// require admin authority. // ── DOM render functions (invoked by dashboard.js / tests) ─────────────────── -// Module-scoped state for the last rendered entries, used to re-render the -// table after an inline Purge confirm is cancelled. No DOM is touched at module -// load — only inside the functions below. - -let lastEntries = []; +// No DOM is touched at module load — only inside the functions below. These are +// all READ-ONLY: they render the invariant/totals cards and the dead-letter +// LIST. There are no mutation controls here (see doc 04 §3 note above). // renderQueues(status) — the load-bearing entry point. Receives the WHOLE // /status object and extracts status.metrics ITSELF (do not pass .metrics in). @@ -178,31 +168,19 @@ function renderTotals(metrics) { .join(''); } -// actionsCellHtml(d) — Replay + Purge buttons for one dead-letter row. -function actionsCellHtml(d) { - const key = escapeAttr(d.workerKey); - return `` - + `` - + `` - + ``; -} - -// renderDeadLetters(entries) — render the dead-letter table body. Poll guard: -// if an inline Purge confirm is open, bail so the 3s poll cannot wipe an -// irreversible-action confirmation out from under the user. +// renderDeadLetters(entries) — render the READ-ONLY dead-letter table body +// (worker key, item count, last error, last timestamp). No action controls: +// drain (replay/purge) is admin-only and lives on the admin surface (doc 04 §3). export function renderDeadLetters(entries) { const body = document.getElementById('dead-letter-body'); if (!body) return; - if (body.querySelector('.actions[data-confirming]')) return; // poll guard - lastEntries = entries || []; - if (lastEntries.length === 0) { - body.innerHTML = `` + const list = entries || []; + if (list.length === 0) { + body.innerHTML = `` + `● No dead letters — all clear`; return; } - body.innerHTML = lastEntries.map(entry => { + body.innerHTML = list.map(entry => { const d = deadLetterRowData(entry); const key = escapeAttr(d.workerKey); const err = escapeAttr(d.lastError); @@ -211,94 +189,15 @@ export function renderDeadLetters(entries) { + `${escapeAttr(d.itemCount)}` + `${err}` + `${escapeAttr(fmtTs(d.lastTs))}` - + actionsCellHtml(d) + ``; }).join(''); } // renderDeadLetterError() — a failed dead-letter LOAD renders a distinct -// "couldn't load" row, NOT an all-clear row. Respects the poll guard. +// "couldn't load" row, NOT an all-clear row. export function renderDeadLetterError() { const body = document.getElementById('dead-letter-body'); if (!body) return; - if (body.querySelector('.actions[data-confirming]')) return; // poll guard - body.innerHTML = `` + body.innerHTML = `` + `Couldn't load dead-letter queues — retrying…`; } - -// showRowBadge(workerKey, text, cls) — replace a row's actions cell with a -// single status badge (honest feedback after an action completes). -export function showRowBadge(workerKey, text, cls) { - const body = document.getElementById('dead-letter-body'); - if (!body) return; - const cell = body.querySelector(`.actions[data-key="${escapeAttr(workerKey)}"]`); - if (cell) cell.innerHTML = `${escapeAttr(text)}`; -} - -// handleActionError(err, workerKey, onAuthLost) — error honesty. -// 401 → auth lost; 400 → distinct 'Invalid'; else → 'Failed — retry'. -export function handleActionError(err, workerKey, onAuthLost) { - if (err && err.status === 401) { - if (typeof onAuthLost === 'function') onAuthLost(); - } else if (err && err.status === 400) { - showRowBadge(workerKey, 'Invalid', 'badge-error'); - } else { - showRowBadge(workerKey, 'Failed — retry', 'badge-error'); - } -} - -// beginPurgeConfirm(cell, workerKey) — replace the action buttons with an -// inline confirm (Purge is irreversible). Marks the cell data-confirming so -// the poll guard leaves it alone, and moves focus to Cancel (Focus-to-Cancel). -export function beginPurgeConfirm(cell, workerKey) { - if (!cell) return; - const key = escapeAttr(workerKey); - cell.setAttribute('data-confirming', '1'); - cell.innerHTML = `Purge ${key}?` - + `` - + ``; - const cancel = cell.querySelector('#purge-cancel'); - if (cancel) cancel.focus(); -} - -// wireDeadLetterActions({onAuthLost}) — attach delegated click/keydown handlers -// to #dead-letter-body for replay / purge / purge-confirm / purge-cancel. -// Escape cancels an open confirm. -export function wireDeadLetterActions({ onAuthLost } = {}) { - const body = document.getElementById('dead-letter-body'); - if (!body) return; - - body.addEventListener('click', async (ev) => { - const btn = ev.target.closest('button[data-action]'); - if (!btn) return; - const action = btn.getAttribute('data-action'); - const key = btn.getAttribute('data-key'); - const cell = btn.closest('.actions'); - - if (action === 'replay') { - try { - const r = await replayWorker(key); - showRowBadge(key, `Re-enqueued ${r.replayed ?? 0}`, 'badge-primary'); - } catch (err) { - handleActionError(err, key, onAuthLost); - } - } else if (action === 'purge') { - beginPurgeConfirm(cell, key); - } else if (action === 'purge-confirm') { - try { - const r = await purgeWorker(key); - showRowBadge(key, `Purged ${r.purged ?? 0}`, 'badge-primary'); - } catch (err) { - handleActionError(err, key, onAuthLost); - } - } else if (action === 'purge-cancel') { - renderDeadLetters(lastEntries); - } - }); - - body.addEventListener('keydown', (ev) => { - if (ev.key === 'Escape' && body.querySelector('.actions[data-confirming]')) { - renderDeadLetters(lastEntries); - } - }); -} diff --git a/context_intelligence_server/web/static/js/queues-panel.test.mjs b/context_intelligence_server/web/static/js/queues-panel.test.mjs index 47868dea..9a5f7191 100644 --- a/context_intelligence_server/web/static/js/queues-panel.test.mjs +++ b/context_intelligence_server/web/static/js/queues-panel.test.mjs @@ -90,8 +90,6 @@ const { renderDeadLetters, renderDeadLetterError, fetchDeadLetters, - replayWorker, - purgeWorker, } = mod; function resetFetch() { fetchCalls = []; nextFetchResponse = null; } @@ -224,27 +222,16 @@ describe('fetch wrappers', () => { assert.deepEqual(out.dead_letters[0], { worker_key: 'w1', item_count: 1 }); }); - test('replayWorker() POSTs to encoded .../replay with auth header', async () => { - nextFetchResponse = { ok: true, status: 200, json: async () => ({ worker_key: 'a/b c', replayed: 4 }) }; - const out = await replayWorker('a/b c'); - assert.equal(fetchCalls[0].url, '/queues/dead-letter/a%2Fb%20c/replay'); - assert.equal(fetchCalls[0].opts.method, 'POST'); - assert.equal(fetchCalls[0].opts.headers['Authorization'], 'Bearer tok-123'); - assert.equal(out.replayed, 4); - }); - - test('purgeWorker() POSTs to encoded .../purge', async () => { - nextFetchResponse = { ok: true, status: 200, json: async () => ({ worker_key: 'w1', purged: 2 }) }; - const out = await purgeWorker('w1'); - assert.equal(fetchCalls[0].url, '/queues/dead-letter/w1/purge'); - assert.equal(fetchCalls[0].opts.method, 'POST'); - assert.equal(out.purged, 2); - }); + // NOTE (doc 04 §3): replayWorker/purgeWorker were REMOVED from this panel — + // dead-letter drain (replay/purge) is admin-only and lives on the admin + // surface. The general dashboard fetches the list read-only (above) and never + // mutates, so there are no POST /queues/dead-letter/.../{replay,purge} calls + // here to test. test('non-ok response throws with err.status attached', async () => { nextFetchResponse = { ok: false, status: 400, json: async () => ({}) }; await assert.rejects( - () => replayWorker('w1'), + () => fetchDeadLetters(), (err) => { assert.equal(err.status, 400); return true; } ); }); @@ -262,13 +249,16 @@ describe('fetch wrappers', () => { // poll-vs-confirm guard + dead-letter rendering // ───────────────────────────────────────────────────────────────────────────── -describe('renderDeadLetters() poll-vs-confirm guard', () => { +describe('renderDeadLetters() — read-only list (drain controls removed, doc 04 §3)', () => { beforeEach(() => { setupDom(); }); - test('empty list → all-clear row', () => { + test('empty list → all-clear row spanning the 3 read-only columns', () => { renderDeadLetters([]); - assert.ok(els['dead-letter-body'].innerHTML.includes('all clear'), - els['dead-letter-body'].innerHTML); + const html = els['dead-letter-body'].innerHTML; + assert.ok(html.includes('all clear'), html); + // colspan matches the header column count after the Actions column was + // removed (Worker, Count, Last error = 3). + assert.ok(html.includes('colspan="3"'), html); }); test('entries → rows include worker key + item count', () => { @@ -278,25 +268,18 @@ describe('renderDeadLetters() poll-vs-confirm guard', () => { assert.ok(html.includes('3'), html); }); - test('does NOT wipe an open Purge confirm (poll guard)', () => { - const body = els['dead-letter-body']; - body.innerHTML = 'CONFIRM-OPEN'; - body._confirmOpen = makeElement('cell'); // .actions[data-confirming] present - renderDeadLetters([{ worker_key: 'w1', item_count: 9 }]); - assert.equal(body.innerHTML, 'CONFIRM-OPEN', 'poll must not overwrite open confirm'); - }); - - test('renderDeadLetterError also respects the confirm guard', () => { - const body = els['dead-letter-body']; - body.innerHTML = 'CONFIRM-OPEN'; - body._confirmOpen = makeElement('cell'); - renderDeadLetterError(); - assert.equal(body.innerHTML, 'CONFIRM-OPEN'); + test('rows carry NO drain controls (replay/purge are admin-only, doc 04 §3)', () => { + renderDeadLetters([{ worker_key: 'w1', item_count: 9, last_error: 'boom', last_ts: null }]); + const html = els['dead-letter-body'].innerHTML; + assert.ok(!/data-action/.test(html), `unexpected action control in row: ${html}`); + assert.ok(!/Replay|Purge/.test(html), `unexpected drain button in row: ${html}`); + assert.ok(!/class="actions"/.test(html), `unexpected actions cell in row: ${html}`); }); - test('renderDeadLetterError renders the retry message when no confirm open', () => { + test('renderDeadLetterError renders the retry message', () => { renderDeadLetterError(); - assert.ok(els['dead-letter-body'].innerHTML.includes("Couldn't load dead-letter queues"), - els['dead-letter-body'].innerHTML); + const html = els['dead-letter-body'].innerHTML; + assert.ok(html.includes("Couldn't load dead-letter queues"), html); + assert.ok(html.includes('colspan="3"'), html); }); }); diff --git a/docs/service-setup.md b/docs/service-setup.md index f3e07c10..17c5bd5a 100644 --- a/docs/service-setup.md +++ b/docs/service-setup.md @@ -356,7 +356,7 @@ stays fresh without a restart: [identity-management.md](identity-management.md). | `server_host` | `0.0.0.0` | Bind address. `0.0.0.0` = all interfaces; `127.0.0.1` = localhost only | | `server_port` | `8000` | Listen port | | `log_level` | `INFO` | Verbosity (`DEBUG` / `INFO` / `WARNING` / `ERROR`) | -| `api_key` | *(your secret)* | Legacy single bearer token (folds to contributor id `owner`). All endpoints except `/status` and static routes require `Authorization: Bearer `. The server verifies it as `sha256(token)`. | +| `api_key` | *(your secret)* | Legacy single bearer token (folds to contributor id `owner`). All endpoints except `/status`, `/version` and static routes require `Authorization: Bearer `. The server verifies it as `sha256(token)`. | | `api_keys` | *(map)* | Per-contributor keystore: `sha256_hex(token) -> {id: }`. The file holds digests; clients send raw tokens. `api_keys: {}` is a hard startup error (omit/`null` to disable auth). See [managing-api-keys.md](managing-api-keys.md). | ### Neo4j settings @@ -586,6 +586,9 @@ overrides: curl http://localhost:8000/status # → {"status":"ok","neo4j_connected":true,"neo4j_query_connected":true,"neo4j_url":"bolt://localhost:37687","neo4j_browser_url":"http://localhost:37474",...} # +# Liveness check (always unauthenticated) +curl http://localhost:8000/version +# # Both neo4j_url and neo4j_browser_url are read verbatim from server-config.yaml. # If Neo4j is on a remote host the response will show those remote addresses. diff --git a/tests/conftest.py b/tests/conftest.py index ecfd94d9..7675e72b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,7 +17,7 @@ import pytest # noqa: E402 -from context_intelligence_server.main import app, registry # noqa: E402 +from context_intelligence_server.main import app, asgi_app, registry # noqa: E402 from context_intelligence_server.services import HookStateService # noqa: E402 @@ -180,8 +180,32 @@ def reset_registry() -> Generator[None, None, None]: @pytest.fixture async def client() -> AsyncGenerator[httpx.AsyncClient, None]: + # Commit 3 (doc 16 §9): route the default fixture through `asgi_app` — the + # auth-wrapped ASGI app — NOT the bare `app`. Under the suite's + # ALLOW_UNAUTHENTICATED=true opt-out the middleware short-circuits (no scope + # state populated) and W1's _is_write_capable honours the flag, so these tests + # stay green — but they now traverse the REAL middleware stack instead of + # passing gated routes via "no middleware ran". + # + # TB-N1 guard: create_asgi_app(settings=...) MUTATES the shared module-level + # app.state (there is a single FastAPI `app`; every call reconfigures it). A + # sibling test building an auth-enabled app (allow_unauthenticated=False) leaves + # app.state.allow_unauthenticated=False behind; W1's _is_write_capable reads that + # flag LIVE, so it would fail-close and 403 this fixture's gated-route requests. + # Reset it to the suite's dev opt-out (conftest sets ALLOW_UNAUTHENTICATED=true at + # import) so gated-route `client` tests are order-independent w.r.t. THIS flag. + # + # SCOPE (do not over-read): this resets ONLY allow_unauthenticated — the one field + # proven to poison client tests (adversarial order: 28 failures without this line). + # create_asgi_app also leaks auth_mode / reader_role / service_data_role / the + # stores onto the same singleton; those are NOT reset here because no current + # `client` test observes them (the client /status callers assert status_code + + # neo4j fields only, never response["auth"]). The real fix — create_asgi_app + # mutates a shared singleton with no teardown — is tracked (SCRATCH: Commit-3 + # follow-ups), not patched field-by-field here. + app.state.allow_unauthenticated = True async with httpx.AsyncClient( - transport=httpx.ASGITransport(app=app), + transport=httpx.ASGITransport(app=asgi_app), base_url="http://test", ) as c: yield c @@ -195,7 +219,6 @@ async def auth_client( import hashlib # noqa: PLC0415 from context_intelligence_server.auth import StaticKeyResolver # noqa: PLC0415 - from context_intelligence_server.main import asgi_app # noqa: PLC0415 # Build a StaticKeyResolver that maps sha256("test-secret") → "owner" so existing # integration tests that send `Authorization: Bearer test-secret` continue to work. diff --git a/tests/routers/test_queues.py b/tests/routers/test_queues.py index c8909e57..74c64b17 100644 --- a/tests/routers/test_queues.py +++ b/tests/routers/test_queues.py @@ -1,15 +1,60 @@ -"""Tests for the GET /queues/dead-letter endpoint.""" +"""Tests for the dead-letter endpoints (list / purge / replay). + +Step 3 (doc 16 W2): list is gated by require_read at GET /queues/dead-letter; +purge/replay are gated by require_admin and live UNDER /admin at +POST /admin/queues/dead-letter/{worker}/{purge,replay}. This file exercises the +dead-letter *business logic* (aggregation, purge, replay mechanics) with the +require_admin dependency bypassed via the standard FastAPI override mechanism — +the same pattern used by the /admin router's own route tests. Authorization +itself (tier boundary + the TB-1 positive-admin proof) is covered separately by +tests/test_dead_letter_requires_admin.py, which deliberately does NOT override +require_admin. +""" from __future__ import annotations import asyncio +from collections.abc import Generator from pathlib import Path import httpx import pytest -from context_intelligence_server.main import registry +from context_intelligence_server.authz import require_read +from context_intelligence_server.main import app, registry from context_intelligence_server.queue_manager import QueueManager +from context_intelligence_server.routers.admin import require_admin + + +@pytest.fixture(autouse=True) +def _bypass_dead_letter_auth() -> Generator[None, None, None]: + """!!! AUTHORIZATION IS DISABLED FOR EVERY TEST IN THIS FILE !!! + + This autouse fixture overrides BOTH dead-letter gates to no-ops — + require_read (the list gate) and require_admin (the purge/replay gate) — so + the tests below exercise dead-letter BUSINESS LOGIC ONLY (aggregation / + purge / replay mechanics). It deliberately provides ZERO authorization + coverage. (Overriding require_read also keeps these tests independent of the + shared module-level app.state.allow_unauthenticated flag, which a sibling + test constructing an auth-enabled app via create_asgi_app can flip.) + + REAL authorization — the tier boundary (list open to any authenticated + principal; purge/replay admin-only) AND the council TB-1 positive-admin + proof (static admin key + entra admin role reaching the handler through the + real gate) — is proven in tests/test_dead_letter_requires_admin.py, which + routes through the real asgi_app and does NOT override either gate. + + ⚠️ If you add a NEW route test HERE, it inherits these overrides and gets NO + auth coverage. Either add the auth assertion to test_dead_letter_requires_admin.py + or scope/remove these overrides for your test. (Council: cranky-old-sam + tester-breaker.) + """ + app.dependency_overrides[require_admin] = lambda: None + app.dependency_overrides[require_read] = lambda: None + try: + yield + finally: + app.dependency_overrides.pop(require_admin, None) + app.dependency_overrides.pop(require_read, None) def _point_registry_at(tmp_path: Path) -> QueueManager: @@ -61,13 +106,18 @@ async def test_dead_letter_list_empty( async def test_dead_letter_list_requires_auth( self, auth_client: httpx.AsyncClient, tmp_path: Path ) -> None: + """Middleware-level authentication is still enforced (401 without any + token) even though this file bypasses the require_admin authorization + gate. Real non-admin-principal authorization (403) is covered by + tests/test_dead_letter_requires_admin.py.""" _point_registry_at(tmp_path) - # No token -> 401 + # No token -> 401 (BearerTokenMiddleware, unaffected by the + # require_admin override above). response = await auth_client.get("/queues/dead-letter") assert response.status_code == 401 - # Valid token -> 200 + # Valid token -> 200 (require_admin bypassed by the module fixture). response = await auth_client.get( "/queues/dead-letter", headers={"Authorization": "Bearer test-secret"}, @@ -86,7 +136,7 @@ async def test_purge_removes_dead_letters( await qm.dead_letter("k1", b'{"a": 1}\n', "boom-1") await qm.dead_letter("k1", b'{"a": 2}\n', "boom-2") - response = await client.post("/queues/dead-letter/k1/purge") + response = await client.post("/admin/queues/dead-letter/k1/purge") assert response.status_code == 200 assert response.json() == {"worker_key": "k1", "purged": 2} assert await qm.read_dead_letters("k1") == [] @@ -97,7 +147,7 @@ async def test_purge_missing_is_zero( ) -> None: _point_registry_at(tmp_path) - response = await client.post("/queues/dead-letter/nope/purge") + response = await client.post("/admin/queues/dead-letter/nope/purge") assert response.status_code == 200 assert response.json() == {"worker_key": "nope", "purged": 0} @@ -107,7 +157,7 @@ async def test_purge_rejects_unsafe_key( ) -> None: _point_registry_at(tmp_path) - response = await client.post("/queues/dead-letter/a%2Fb/purge") + response = await client.post("/admin/queues/dead-letter/a%2Fb/purge") assert response.status_code == 400 @@ -135,7 +185,7 @@ async def test_replay_reenqueues_and_purges( before = registry.pipeline_counters() - response = await client.post("/queues/dead-letter/k1/replay") + response = await client.post("/admin/queues/dead-letter/k1/replay") assert response.status_code == 200 assert response.json() == {"worker_key": "k1", "replayed": 2} @@ -170,7 +220,7 @@ async def test_replay_empty_is_zero( before = registry.pipeline_counters() - response = await client.post("/queues/dead-letter/nope/replay") + response = await client.post("/admin/queues/dead-letter/nope/replay") assert response.status_code == 200 assert response.json() == {"worker_key": "nope", "replayed": 0} diff --git a/tests/test_auth.py b/tests/test_auth.py index fe2874bb..c7eb411d 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -179,8 +179,9 @@ async def test_valid_token_injects_contributor_id_into_scope(self) -> None: assert scope.get("state", {}).get("contributor_id") == "alice" - async def test_status_exempt_without_token(self) -> None: - """/status is accessible without any token.""" + async def test_status_stays_exempt(self) -> None: + """/status stays exempt from auth -- it is the Azure Container Apps + liveness/health probe and must never require a token.""" app = AsyncMock() middleware = BearerTokenMiddleware(app, keystore=_keystore("secret-token")) @@ -406,7 +407,8 @@ async def test_events_with_correct_token_passes( async def test_status_without_token_returns_200( self, auth_client: httpx.AsyncClient ) -> None: - """GET /status is always exempt — returns 200 without any token.""" + """GET /status stays exempt from auth -- it is the Azure Container Apps + liveness/health probe and must never require a token.""" response = await auth_client.get("/status") assert response.status_code == 200 diff --git a/tests/test_authz_empty_state.py b/tests/test_authz_empty_state.py index 7662897a..cd75d3b7 100644 --- a/tests/test_authz_empty_state.py +++ b/tests/test_authz_empty_state.py @@ -172,35 +172,15 @@ def test_require_admin_denies_on_absent_state_key() -> None: assert exc_info.value.status_code == 403 -@pytest.mark.xfail( - reason=( - "TB-7: require_write fails OPEN on unpopulated scope state " - "(_is_write_capable defaults is_service->False->write-capable); " - "fail-closed tightening deferred to Step 3 (see docs/14). When Step 3 " - "tightens _is_write_capable this flips XFAIL->XPASS and FAILs the suite, " - "forcing removal of this marker." - ), - strict=True, -) def test_require_write_should_deny_on_empty_scope_state() -> None: - """DESIRED (fails today -> XFAIL): require_write should DENY on empty state.""" + """require_write DENIES on empty state (Step 3, doc 16 W1: fail-closed).""" req = _fake_request({}, _ENTRA_APP_STATE) with pytest.raises(HTTPException): require_write(req) -@pytest.mark.xfail( - reason=( - "TB-7: require_read fails OPEN on unpopulated scope state " - "(_is_write_capable defaults is_service->False->write-capable); " - "fail-closed tightening deferred to Step 3 (see docs/14). When Step 3 " - "tightens _is_write_capable this flips XFAIL->XPASS and FAILs the suite, " - "forcing removal of this marker." - ), - strict=True, -) def test_require_read_should_deny_on_empty_scope_state() -> None: - """DESIRED (fails today -> XFAIL): require_read should DENY on empty state.""" + """require_read DENIES on empty state (Step 3, doc 16 W1: fail-closed).""" req = _fake_request({}, _ENTRA_APP_STATE) with pytest.raises(HTTPException): require_read(req) diff --git a/tests/test_dead_letter_requires_admin.py b/tests/test_dead_letter_requires_admin.py new file mode 100644 index 00000000..bb3b5c16 --- /dev/null +++ b/tests/test_dead_letter_requires_admin.py @@ -0,0 +1,360 @@ +"""W2 (doc 16 §4.4) — dead-letter tier boundary + the TB-1 positive-admin proof. + +Human decision (authoritative): dead-letter LIST is fine for any authenticated +principal; PURGE/REPLAY are destructive → admin-only. Because ``require_admin`` +in static mode only allows when the middleware set ``is_admin=True``, and that +flag is set ONLY by the ``/admin/*`` admin-key fast-path (auth._is_admin_route), +purge/replay were RELOCATED under ``/admin`` so the static admin key can reach +them (council TB-1). This file PROVES the boundary through the REAL ``asgi_app`` +gate — it deliberately does NOT apply the ``require_admin`` override. + +Route map after W2: + - GET /queues/dead-letter → require_read (any principal) + - POST /admin/queues/dead-letter/{worker}/purge → require_admin + - POST /admin/queues/dead-letter/{worker}/replay → require_admin +""" + +from __future__ import annotations + +import asyncio +import hashlib +import time +from collections.abc import AsyncGenerator +from pathlib import Path +from typing import Any + +import httpx +import pytest + +# --------------------------------------------------------------------------- +# Test constants — never real credentials +# --------------------------------------------------------------------------- + +_DATA_TOKEN = "dead-letter-w2-non-admin-data-key" # noqa: S105 (test fixture) +_DATA_DIGEST = hashlib.sha256(_DATA_TOKEN.encode()).hexdigest() +_ADMIN_TOKEN = "dead-letter-w2-admin-key-do-not-use" # noqa: S105 (test fixture) + +# Entra fakes (mirror tests/test_m2_service_auth.py) +_FAKE_CLIENT_ID = "aaaabbbb-1111-2222-3333-ccccddddeeee" +_FAKE_TENANT_ID = "ffffeeee-dddd-cccc-bbbb-aaaa99998888" +_FAKE_OID_SERVICE = "aaaabbbb-9999-9999-9999-ccccddddffff" +_FAKE_APPID = "bbbbcccc-2222-3333-4444-eeeeffff0000" +_FAKE_ISSUER = f"https://login.microsoftonline.com/{_FAKE_TENANT_ID}/v2.0" +_ENTRA_ADMIN_ROLE = "IdentityAdmin" + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _point_registry_at(tmp_path: Path) -> None: + """Point the shared registry's durable infra at a tmp_path queues dir so + the purge/replay/list handlers return cleanly (mirrors + tests/routers/test_queues.py::_point_registry_at).""" + from context_intelligence_server.main import registry # noqa: PLC0415 + from context_intelligence_server.queue_manager import QueueManager # noqa: PLC0415 + + registry._queue_manager = QueueManager(queues_dir=tmp_path / "queues") + registry._write_semaphore = asyncio.Semaphore(2) + registry._max_delivery_attempts = 5 + + +@pytest.fixture(scope="module") +def _rsa_keypair() -> tuple[Any, Any]: + """Generate a 2048-bit RSA keypair once per module (entra token signing).""" + from cryptography.hazmat.primitives.asymmetric import rsa # noqa: PLC0415 + + private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048) + return private_key, private_key.public_key() + + +class _StubSigningKey: + def __init__(self, key: Any) -> None: + self.key = key + + +class _StubJWKSClient: + """Minimal stub JWKS client — no network (mirrors the M2 tests).""" + + def __init__(self, key: Any) -> None: + self._key = _StubSigningKey(key) + + def fetch_data(self) -> None: + pass + + def get_signing_key_from_jwt(self, token: str) -> _StubSigningKey: + return self._key + + def get_jwk_set(self) -> Any: + _k = self._key + + class _FakeJWKSet: + keys = [_k] + + return _FakeJWKSet() + + +def _sign_jwt(private_key: Any, claims: dict[str, Any]) -> str: + import jwt as pyjwt # noqa: PLC0415 + + return pyjwt.encode(claims, private_key, algorithm="RS256") + + +def _service_admin_claims() -> dict[str, Any]: + """Service/app token whose roles claim carries the admin App Role (no scp → + service branch, is_service=True, roles=[IdentityAdmin]).""" + now = int(time.time()) + return { + "oid": _FAKE_OID_SERVICE, + "tid": _FAKE_TENANT_ID, + "aud": _FAKE_CLIENT_ID, + "iss": _FAKE_ISSUER, + "exp": now + 3600, + "iat": now - 10, + "appid": _FAKE_APPID, + "roles": [_ENTRA_ADMIN_ROLE], + } + + +# --------------------------------------------------------------------------- +# Fixtures — real asgi_app, NO require_admin override +# --------------------------------------------------------------------------- + + +@pytest.fixture +async def non_admin_client(tmp_path: Path) -> AsyncGenerator[httpx.AsyncClient, None]: + """Static-mode client authenticated as a non-admin data principal + (is_admin=False), routed through the real asgi_app.""" + from context_intelligence_server.config import Settings # noqa: PLC0415 + from context_intelligence_server.main import create_asgi_app # noqa: PLC0415 + + settings = Settings( + auth_mode="static", + allow_unauthenticated=False, + api_keys={_DATA_DIGEST: {"id": "alice"}}, + admin_api_key=_ADMIN_TOKEN, + api_keys_store_path=str(tmp_path / "api-keys.json"), + entra_identities_store_path=str(tmp_path / "entra-identities.json"), + ) + wrapped = create_asgi_app(settings=settings) + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=wrapped), + base_url="http://test", + headers={"Authorization": f"Bearer {_DATA_TOKEN}"}, + ) as c: + yield c + + +@pytest.fixture +async def static_admin_client( + tmp_path: Path, +) -> AsyncGenerator[httpx.AsyncClient, None]: + """Static-mode client presenting the admin_api_key as bearer, routed through + the real asgi_app (the /admin/* fast-path sets is_admin=True).""" + from context_intelligence_server.config import Settings # noqa: PLC0415 + from context_intelligence_server.main import create_asgi_app # noqa: PLC0415 + + settings = Settings( + auth_mode="static", + allow_unauthenticated=False, + api_keys={_DATA_DIGEST: {"id": "alice"}}, + admin_api_key=_ADMIN_TOKEN, + api_keys_store_path=str(tmp_path / "api-keys.json"), + entra_identities_store_path=str(tmp_path / "entra-identities.json"), + ) + wrapped = create_asgi_app(settings=settings) + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=wrapped), + base_url="http://test", + headers={"Authorization": f"Bearer {_ADMIN_TOKEN}"}, + ) as c: + yield c + + +@pytest.fixture +async def entra_admin_client( + tmp_path: Path, _rsa_keypair: tuple[Any, Any] +) -> AsyncGenerator[httpx.AsyncClient, None]: + """Entra-mode client presenting a token whose roles claim carries the + configured entra_admin_role, routed through the real asgi_app.""" + from context_intelligence_server.config import Settings # noqa: PLC0415 + from context_intelligence_server.main import app, create_asgi_app # noqa: PLC0415 + from context_intelligence_server.routers.skills import SkillRegistry # noqa: PLC0415 + + private_key, public_key = _rsa_keypair + settings = Settings( + auth_mode="entra", + allow_unauthenticated=False, + azure_client_id=_FAKE_CLIENT_ID, + azure_tenant_id=_FAKE_TENANT_ID, + entra_identities={_FAKE_OID_SERVICE: {"id": "svc"}}, + entra_admin_role=_ENTRA_ADMIN_ROLE, + entra_identities_store_path=str(tmp_path / "entra-identities.json"), + api_keys_store_path=str(tmp_path / "api-keys.json"), + ) + if not hasattr(app.state, "skill_registry"): + app.state.skill_registry = SkillRegistry() + wrapped = create_asgi_app( + settings=settings, _jwks_client=_StubJWKSClient(public_key) + ) + token = _sign_jwt(private_key, _service_admin_claims()) + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=wrapped), + base_url="http://test", + headers={"Authorization": f"Bearer {token}"}, + ) as c: + yield c + + +# --------------------------------------------------------------------------- +# Tier boundary (the human decision) +# --------------------------------------------------------------------------- + + +class TestDeadLetterTierBoundary: + """List is open to any authenticated principal; purge/replay are admin-only.""" + + @pytest.mark.anyio + async def test_list_dead_letters_allows_non_admin( + self, non_admin_client: httpx.AsyncClient, tmp_path: Path + ) -> None: + """GET /queues/dead-letter → 200 for a non-admin data key (require_read).""" + _point_registry_at(tmp_path) + resp = await non_admin_client.get("/queues/dead-letter") + assert resp.status_code == 200 + + @pytest.mark.anyio + async def test_purge_dead_letters_denies_non_admin( + self, non_admin_client: httpx.AsyncClient + ) -> None: + """POST /admin/queues/dead-letter/w/purge → 403 for a non-admin key.""" + resp = await non_admin_client.post("/admin/queues/dead-letter/w/purge") + assert resp.status_code == 403 + + @pytest.mark.anyio + async def test_replay_dead_letters_denies_non_admin( + self, non_admin_client: httpx.AsyncClient + ) -> None: + """POST /admin/queues/dead-letter/w/replay → 403 for a non-admin key.""" + resp = await non_admin_client.post("/admin/queues/dead-letter/w/replay") + assert resp.status_code == 403 + + @pytest.mark.anyio + async def test_list_dead_letters_denies_unqualified_service_token( + self, entra_admin_client: httpx.AsyncClient + ) -> None: + """GET /queues/dead-letter → 403 for a service principal that authenticates + but holds NEITHER reader_role NOR service_data_role (require_read DENY + branch — council TB-N2). + + The newly-reopened list route uses ``require_read``; every other test only + proves the ALLOW path (200). This proves DENY through the REAL asgi_app, + no override. + + Principal construction (deliberate): a truly role-EMPTY service token + (``roles=[]``) is rejected earlier, at the Entra resolver (M2 dual-path: + "no qualifying App Role" → 403 in the middleware), so it never reaches + ``require_read``. To exercise ``require_read``'s deny branch through the + real stack we need a principal that AUTHENTICATES as a service + (``is_service=True``) yet lacks both data roles. The ``entra_admin_client`` + token is exactly that: ``roles=[IdentityAdmin]`` — a service token that + the resolver accepts (CAP-SADM-a), carrying the ADMIN role but NEITHER + ``Reader`` (reader_role) NOR ``Contributor`` (service_data_role). So it + clears the resolver, reaches ``require_read``, and is denied 403 — + proving admin authority does NOT confer data-read capability on the list. + """ + resp = await entra_admin_client.get("/queues/dead-letter") + assert resp.status_code == 403 + + +# --------------------------------------------------------------------------- +# Route-existence regression (council crusty F-W2a) +# --------------------------------------------------------------------------- + + +class TestDeadLetterAdminRouteExistence: + """Guard the silent dropped-router failure mode. + + The purge/replay routes live on ``dead_letter_admin_router``, which + ``main.py`` must ``include_router()``. If that include is ever dropped, the + routes vanish and requests 404 — and NOTHING else catches it (the boot guard + only inspects registered routes; a dropped router registers nothing). + + Deviation from the literal spec (verified empirically, and by reading + auth.py): the spec suggested an UNAUTHENTICATED request asserting 401/403 != + 404. But ``BearerTokenMiddleware`` returns 401 for any non-exempt path with + no bearer token BEFORE routing runs — so a no-auth request 401s whether or + not the route exists, and CANNOT detect a drop. We therefore send an + AUTHENTICATED non-admin principal: it clears the middleware so routing + actually happens, giving 403 when the route EXISTS (require_admin denies) and + 404 when it has been dropped. Asserting ``!= 404`` (route present) and + ``in (401, 403)`` (gated, not open) fulfills the guard's real purpose. + """ + + @pytest.mark.anyio + async def test_purge_route_exists_and_is_admin_gated( + self, non_admin_client: httpx.AsyncClient + ) -> None: + """POST /admin/queues/dead-letter/w/purge → gated (not 404). + + Proves the route EXISTS and is auth/admin-gated; guards against a dropped + include_router(dead_letter_admin_router) silently 404-ing (council + crusty F-W2a). A 404 here means the route was dropped. + """ + resp = await non_admin_client.post("/admin/queues/dead-letter/w/purge") + assert resp.status_code != 404, ( + "route dropped — 404 means the router include was lost" + ) + assert resp.status_code in (401, 403) + + @pytest.mark.anyio + async def test_replay_route_exists_and_is_admin_gated( + self, non_admin_client: httpx.AsyncClient + ) -> None: + """POST /admin/queues/dead-letter/w/replay → gated (not 404). + + Proves the route EXISTS and is auth/admin-gated; guards against a dropped + include_router(dead_letter_admin_router) silently 404-ing (council + crusty F-W2a). A 404 here means the route was dropped. + """ + resp = await non_admin_client.post("/admin/queues/dead-letter/w/replay") + assert resp.status_code != 404, ( + "route dropped — 404 means the router include was lost" + ) + assert resp.status_code in (401, 403) + + +# --------------------------------------------------------------------------- +# Positive admin proof (the TB-1 fix — relocation makes admin reachable) +# --------------------------------------------------------------------------- + + +class TestDeadLetterAdminReachable: + """PROVE purge is reachable by admin through the REAL gate, in BOTH modes.""" + + @pytest.mark.anyio + async def test_purge_dead_letters_allows_static_admin_key( + self, static_admin_client: httpx.AsyncClient, tmp_path: Path + ) -> None: + """STATIC mode: the admin_api_key as bearer → POST purge → not 403 (200). + + LOAD-BEARING TB-1 REGRESSION PIN. Before the relocation this was + impossible: require_admin off /admin/* was unreachable by the static + admin key, so purge/replay were bricked in static mode. The /admin/* + fast-path sets is_admin=True → require_admin passes → handler runs. + """ + _point_registry_at(tmp_path) + resp = await static_admin_client.post("/admin/queues/dead-letter/w/purge") + assert resp.status_code != 403 + assert resp.status_code == 200 + + @pytest.mark.anyio + async def test_purge_dead_letters_allows_entra_admin_role( + self, entra_admin_client: httpx.AsyncClient, tmp_path: Path + ) -> None: + """ENTRA mode: a token carrying entra_admin_role → POST purge → not 403 (200).""" + _point_registry_at(tmp_path) + resp = await entra_admin_client.post("/admin/queues/dead-letter/w/purge") + assert resp.status_code != 403 + assert resp.status_code == 200 diff --git a/tests/test_docker_infrastructure.py b/tests/test_docker_infrastructure.py index 26fd2922..56439cdb 100644 --- a/tests/test_docker_infrastructure.py +++ b/tests/test_docker_infrastructure.py @@ -165,8 +165,11 @@ def test_compose_server_has_healthcheck(compose: dict) -> None: hc = server["healthcheck"] test_cmd = hc.get("test", "") test_str = str(test_cmd) + # /status is the unauthenticated Azure Container Apps liveness/health probe + # and must always stay public — the healthcheck probes it directly. assert "curl" in test_str and "localhost:8000/status" in test_str, ( - "healthcheck must use curl to check http://localhost:8000/status" + "healthcheck must use curl to check http://localhost:8000/status " + "(/status is always unauthenticated — the ACA liveness probe)" ) diff --git a/tests/test_entra_integration.py b/tests/test_entra_integration.py index 9a3c1eb3..dd3cd313 100644 --- a/tests/test_entra_integration.py +++ b/tests/test_entra_integration.py @@ -343,7 +343,11 @@ async def test_status_endpoint_exempt_under_entra_mode( self, entra_auth_client: httpx.AsyncClient, ) -> None: - """GET /status → 200 without any token — exempt path, entra mode active.""" + """GET /status → 200 without any token — exempt path, entra mode active. + + /status is the Azure Container Apps liveness/health probe and must + never require auth, in any auth mode. + """ response = await entra_auth_client.get("/status") assert response.status_code == 200, ( f"Expected 200 for /status (always exempt), got " diff --git a/tests/test_m2_service_auth.py b/tests/test_m2_service_auth.py index 5cfaebcb..cb4be9ca 100644 --- a/tests/test_m2_service_auth.py +++ b/tests/test_m2_service_auth.py @@ -704,11 +704,17 @@ async def test_status_includes_reader_role_and_service_data_role( Existing fields (mode, admin_api_enabled, entra_admin_role) must remain untouched — this is an additive-only change. + + /status is exempt from auth (Azure Container Apps liveness/health probe), + but this request still carries a valid human bearer token to prove /status + does not REJECT an authenticated principal either (any authenticated + principal passes; /status has no capability dependency). """ - _, asgi = service_asgi + private_key, asgi = service_asgi + token = _sign_jwt(private_key, _human_claims()) async with _make_client(asgi) as c: - resp = await c.get("/status") + resp = await c.get("/status", headers={"Authorization": f"Bearer {token}"}) assert resp.status_code == 200 data = resp.json() @@ -942,11 +948,14 @@ def test_all_mutating_routes_have_capability_or_admin_dep(self) -> None: ) def test_data_read_routes_have_read_gate(self) -> None: - """Data-exposing GET routes in _REQUIRED_READ_GATED carry require_read/write. - - Best-effort assertion: confirms the listed data-read routes are - capability-gated so Reader-only service tokens are subject to the same - gating as write routes. + """Data-exposing GET routes in _REQUIRED_READ_GATED carry a read gate. + + Step 3 (doc 16 W2): dead-letter LIST is open to any authenticated + principal, so it is gated by require_read (NOT require_admin — the + destructive purge/replay mutations are the admin-tier routes, and they + relocated to POST /admin/queues/dead-letter/* where the mutating-route + guard below covers them). This asserts the TRUE gate on the list route: + require_read/require_write, deliberately NOT accepting require_admin. """ from fastapi.routing import APIRoute # noqa: PLC0415 @@ -974,7 +983,7 @@ def test_data_read_routes_have_read_gate(self) -> None: ) assert not unguarded, ( - "Data-exposing read route(s) have no capability gate:\n" + "Data-exposing read route(s) have no read-capability gate:\n" + "\n".join(sorted(unguarded)) ) diff --git a/tests/test_main.py b/tests/test_main.py index 1c03dbc7..abffd984 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -828,8 +828,9 @@ async def _auth_client( class TestAuthMiddleware: """Bearer token middleware integration tests against the real app.""" - async def test_status_accessible_without_token(self) -> None: - """/status is always accessible through middleware, even when api_key is set.""" + async def test_status_stays_exempt_when_api_key_set(self) -> None: + """/status stays exempt from auth even when api_key is set -- it is the + Azure Container Apps liveness/health probe and must never require a token.""" async with _auth_client() as c: response = await c.get("/status") assert response.status_code == 200 diff --git a/tests/test_neo4j_browser_url_hidden.py b/tests/test_neo4j_browser_url_hidden.py new file mode 100644 index 00000000..55658e2e --- /dev/null +++ b/tests/test_neo4j_browser_url_hidden.py @@ -0,0 +1,95 @@ +"""W4 (doc 16 §6) — Neo4j Browser (7474 HTTP) URL hide flag. + +Fail-safe hidden: the browser URL is surfaced on /status ONLY in development or +via the explicit opt-in. In Azure it points into a private VNet and must not be +exposed. The Bolt URL (neo4j_url) and health stay visible regardless. + +Isolation-only: predicate unit tests (no app) + /status composition tests routed +through the allow_unauthenticated dev ``client`` fixture (W3's auth gate is inert +there). ``get_status`` reads the module-level ``main._settings``, so the +composition tests monkeypatch that. +""" + +from __future__ import annotations + +import httpx +import pytest + +import context_intelligence_server.main as main_module +from context_intelligence_server.config import Settings + +_BROWSER_URL = "http://localhost:7474" + + +# --------------------------------------------------------------------------- +# Predicate unit tests (fast, no app) +# --------------------------------------------------------------------------- + + +def test_browser_url_hidden_by_default() -> None: + """Default (production, no opt-in) → hidden.""" + assert Settings().neo4j_browser_url_visible() is False + + +def test_browser_url_visible_in_development() -> None: + """is_development=True → visible.""" + assert Settings(is_development=True).neo4j_browser_url_visible() is True + + +def test_browser_url_visible_when_opt_in() -> None: + """show_neo4j_browser_url=True → visible even outside development.""" + assert Settings(show_neo4j_browser_url=True).neo4j_browser_url_visible() is True + + +# --------------------------------------------------------------------------- +# /status composition tests (dev client — auth gate inert) +# --------------------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_status_hides_browser_url_by_default( + client: httpx.AsyncClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """Default settings → neo4j_browser_url key present but null; Bolt url stays.""" + monkeypatch.setattr( + main_module, + "_settings", + main_module._settings.model_copy( + update={ + "is_development": False, + "show_neo4j_browser_url": False, + "neo4j_browser_url": _BROWSER_URL, + } + ), + ) + + response = await client.get("/status") + assert response.status_code == 200 + data = response.json() + # Key ALWAYS present (stable /status shape) but null when hidden. + assert "neo4j_browser_url" in data + assert data["neo4j_browser_url"] is None + # Bolt url (neo4j_url) stays visible and non-null. + assert data.get("neo4j_url") + + +@pytest.mark.anyio +async def test_status_shows_browser_url_in_development( + client: httpx.AsyncClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """is_development=True → neo4j_browser_url equals the configured URL.""" + monkeypatch.setattr( + main_module, + "_settings", + main_module._settings.model_copy( + update={ + "is_development": True, + "neo4j_browser_url": _BROWSER_URL, + } + ), + ) + + response = await client.get("/status") + assert response.status_code == 200 + data = response.json() + assert data["neo4j_browser_url"] == _BROWSER_URL diff --git a/tests/test_status_stays_public.py b/tests/test_status_stays_public.py new file mode 100644 index 00000000..ae63b5aa --- /dev/null +++ b/tests/test_status_stays_public.py @@ -0,0 +1,120 @@ +"""/status MUST remain a public, unauthenticated route -- it is the Azure +Container Apps liveness/health probe. Gating it breaks the deployment. + +This test is a tripwire: it fails loudly if anyone ever removes /status from +the exempt sets (_EXEMPT_PATHS / _EXEMPT_PATHS_API_ONLY). + +Exercises BOTH auth-exempt sets (full-web _EXEMPT_PATHS and API-only +_EXEMPT_PATHS_API_ONLY) so a future change can't silently re-gate /status +in one set while leaving the other correct. +""" + +from __future__ import annotations + +import hashlib +from pathlib import Path + +import httpx +import pytest + +from context_intelligence_server.auth import _EXEMPT_PATHS, _EXEMPT_PATHS_API_ONLY + +_DATA_TOKEN = "status-public-guard-test-token" # noqa: S105 (test fixture, not a real secret) +_DATA_DIGEST = hashlib.sha256(_DATA_TOKEN.encode()).hexdigest() + + +def test_status_in_exempt_paths() -> None: + """/status must be present in the full-web exempt set.""" + assert "/status" in _EXEMPT_PATHS + + +def test_status_in_exempt_paths_api_only() -> None: + """/status must be present in the API-only exempt set (the Azure/ACA config).""" + assert "/status" in _EXEMPT_PATHS_API_ONLY + + +def _make_settings(tmp_path: Path, *, web_ui_enabled: bool): + from context_intelligence_server.config import Settings # noqa: PLC0415 + + return Settings( + auth_mode="static", + allow_unauthenticated=False, + api_keys={_DATA_DIGEST: {"id": "alice"}}, + web_ui_enabled=web_ui_enabled, + api_keys_store_path=str(tmp_path / "api-keys.json"), + entra_identities_store_path=str(tmp_path / "entra-identities.json"), + ) + + +@pytest.mark.anyio +async def test_status_stays_public_when_web_ui_enabled(tmp_path: Path) -> None: + """GET /status with no Authorization header -> 200 (web_ui_enabled=True, + exercises _EXEMPT_PATHS).""" + from context_intelligence_server.main import create_asgi_app # noqa: PLC0415 + + settings = _make_settings(tmp_path, web_ui_enabled=True) + wrapped = create_asgi_app(settings=settings) + + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=wrapped), base_url="http://test" + ) as c: + resp = await c.get("/status") + assert resp.status_code == 200 + + +@pytest.mark.anyio +async def test_status_stays_public_when_api_only(tmp_path: Path) -> None: + """GET /status with no Authorization header -> 200 (web_ui_enabled=False, + exercises _EXEMPT_PATHS_API_ONLY -- the Azure/API-only config used by ACA + liveness/health probes).""" + from context_intelligence_server.main import create_asgi_app # noqa: PLC0415 + + settings = _make_settings(tmp_path, web_ui_enabled=False) + wrapped = create_asgi_app(settings=settings) + + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=wrapped), base_url="http://test" + ) as c: + resp = await c.get("/status") + assert resp.status_code == 200 + + +@pytest.mark.anyio +async def test_status_authenticated_also_returns_200(tmp_path: Path) -> None: + """/status WITH a valid bearer token -> also 200 (auth-enabled app). + + /status is exempt, so a request never NEEDS a token -- but a request that + happens to carry a valid one must not be rejected either. + """ + from context_intelligence_server.main import create_asgi_app # noqa: PLC0415 + + settings = _make_settings(tmp_path, web_ui_enabled=True) + wrapped = create_asgi_app(settings=settings) + + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=wrapped), base_url="http://test" + ) as c: + resp = await c.get( + "/status", headers={"Authorization": f"Bearer {_DATA_TOKEN}"} + ) + assert resp.status_code == 200 + + +@pytest.mark.anyio +async def test_version_still_exempt(tmp_path: Path) -> None: + """GET /version with no Authorization header -> 200 for both exempt sets -- + pins the liveness carve-out alongside /status.""" + from context_intelligence_server.main import create_asgi_app # noqa: PLC0415 + + for web_ui_enabled in (True, False): + wrapped = create_asgi_app( + settings=_make_settings(tmp_path, web_ui_enabled=web_ui_enabled) + ) + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=wrapped), base_url="http://test" + ) as c: + resp = await c.get("/version") + assert resp.status_code == 200, ( + f"/version must stay exempt (web_ui_enabled={web_ui_enabled}), " + f"got {resp.status_code}" + ) diff --git a/tests/test_web_ui_switch.py b/tests/test_web_ui_switch.py index e3f58843..45decb37 100644 --- a/tests/test_web_ui_switch.py +++ b/tests/test_web_ui_switch.py @@ -154,8 +154,12 @@ def test_web_ui_disabled_logs_stream_not_exempt(self) -> None: "it is an unauthenticated log drain if exempt" ) - def test_web_ui_disabled_status_still_exempt(self) -> None: - """/status is always exempt, even in api-only mode.""" + def test_web_ui_disabled_status_stays_exempt(self) -> None: + """/status IS exempt in api-only mode -- it is the Azure Container Apps + liveness/health probe and must NEVER require auth. + + /status and /version are both unauthenticated liveness carve-outs. + """ from context_intelligence_server.config import Settings # noqa: PLC0415 from context_intelligence_server.main import create_asgi_app # noqa: PLC0415 @@ -165,7 +169,10 @@ def test_web_ui_disabled_status_still_exempt(self) -> None: wrapped = create_asgi_app(settings=settings) assert "/status" in wrapped._exempt_paths, ( - "/status must be exempt even in api-only mode (health check)" + "/status must stay exempt -- it is the unauthenticated health/liveness probe" + ) + assert "/version" in wrapped._exempt_paths, ( + "/version must remain exempt even in api-only mode (health check)" ) def test_web_ui_enabled_uses_full_exempt_paths(self) -> None: @@ -305,14 +312,18 @@ async def test_logs_stream_requires_auth_in_api_only_mode( # Paths that MUST still be reachable # ------------------------------------------------------------------ - async def test_status_exempt_in_api_only_mode( + async def test_status_stays_public_in_api_only_mode( self, api_only_client: httpx.AsyncClient ) -> None: - """GET /status → 200 without token — always exempt (health check).""" + """GET /status without token → 200 in api-only mode. + + /status IS in _EXEMPT_PATHS_API_ONLY -- it is the Azure Container Apps + liveness/health probe and must never require auth, alongside /version. + """ response = await api_only_client.get("/status") assert response.status_code == 200, ( - f"GET /status must return 200 in api-only mode (always exempt), " - f"got {response.status_code}" + f"GET /status without token must return 200 in api-only mode " + f"(exempt -- health/liveness probe), got {response.status_code}" ) async def test_skills_prefix_not_auth_blocked_in_api_only_mode(