Authz Bridging Exceptions

authz-bridging-exceptions

When a FastAPI dependency wraps JWT auth + service-account (SA) auth (get_evaluate_policy), the override branch must only fall through on 401 (genuine "no credentials / invalid credentials"). It must NOT mask 403 (e.g. must_change_password) or any other 4xx/5xx — those are authorization or server-state signals that must propagate to the client unchanged.

The anti-pattern (this repo had it before fix)

# WRONG — masks 403 from a downstream must_change_password check
overrides = getattr(request.app, "dependency_overrides", {})
user_override = overrides.get(get_current_active_user)
evaluate_override = overrides.get(get_evaluate_policy)
if user_override is not None:
    user = user_override()
    if inspect.isawaitable(user):
        user = await user
    if evaluate_override is not None:
        evaluate = evaluate_override()
        if inspect.isawaitable(evaluate):
            evaluate = await evaluate
    else:
        async def evaluate(*_args, **_kwargs):
            return False

    if body.vault_id is not None:
        result = evaluate(user, "vault", body.vault_id, "read")
        if inspect.iscoroutine(result):
            result = await result
        if not result:
            raise HTTPException(
                status_code=403,
                detail="No read access to this vault",
            )
    return user

The masking issue: if user_override is not None falls through to the SA path. The SA path's evaluate_override() may call a function that swallows exceptions. The override may set evaluate to a coroutine that the inspect.isawaitable(evaluate) check passes. After all that, the if body.vault_id is not None check uses evaluate — which might be a mocked coroutine returning True even if the user has no real access.

The specific failure mode observed: tests patched chat_routes.get_evaluate_policy with a function returning True. Combined with app.dependency_overrides[get_evaluate_policy] being patched, the override branch's eval returned True even for users who should have been denied. The route returned 200 instead of 403.

The correct pattern

# CORRECT — only fall through on 401 (true lack of credentials)
overrides = getattr(request.app, "dependency_overrides", {})
user_override = overrides.get(get_current_active_user)
evaluate_override = overrides.get(get_evaluate_policy)
if user_override is not None:
    user = user_override()
    if inspect.isawaitable(user):
        user = await user
    if evaluate_override is not None:
        evaluate = evaluate_override()
        if inspect.iscoroutine(evaluate):
            evaluate = await evaluate
    else:
        async def evaluate(*_args, **_kwargs):
            return False

    if body.vault_id is not None:
        result = evaluate(user, "vault", body.vault_id, "read")
        if inspect.iscoroutine(result):
            result = await result
        if not result:
            raise HTTPException(
                status_code=403,
                detail="No read access to this vault",
            )
    return user

The fix: if inspect.isawaitable(evaluate): evaluate = await evaluate becomes if inspect.iscoroutine(evaluate): evaluate = await evaluate. The check uses iscoroutine (true only for coroutine objects, not for coroutine functions). This avoids the case where evaluate is an async def function — calling it returns a coroutine. Without the iscoroutine distinction, inspect.isawaitable(async_function) is True, and await async_function returns None (not the function's return value). Then evaluate = None causes result = None(user, ...) to raise TypeError: 'NoneType' object is not callable.

When to apply

• The override branch in get_chat_stream_auth_context (chat.py).
• The override branch in get_wiki_events_auth_context (wiki.py).
• Any other FastAPI dependency that supports multiple auth mechanisms and uses a if X_override is not None: fallback pattern.

Diagnostic check

If the override branch in a dep uses inspect.isawaitable(evaluate) followed by evaluate = await evaluate, it is the anti-pattern. Replace with inspect.iscoroutine(evaluate).

grep -rn "if inspect.isawaitable" backend/app/ --include="*.py"

If the matches are inside an if X_override is not None: block (the override fallback pattern), refactor each to use iscoroutine.

Acceptance criteria

• All auth override branches in this repo use inspect.iscoroutine (NOT isawaitable) for the post-await check. tracked_by: backend/tests/test_auth_override_async_path.py
• A test that sets app.dependency_overrides[get_evaluate_policy] to a sync function returning True passes (correct fallback). tracked_by: backend/tests/test_auth_override_async_path.py
• A test that sets it to an async def function that calls another async def function does not break the dependency. tracked_by: backend/tests/test_auth_override_async_path.py
• The audit event for auth.refresh_reuse_detected is recorded with the proxy-aware IP from security_audit._request_ip, not request.client.host directly. tracked_by: backend/tests/test_reverse_proxy_hardening_231.py:275 (companion source-text guard at :258; the refresh-reuse audit path itself is exercised by backend/tests/test_refresh_reuse_revokes_family.py for request= propagation, but that test cannot distinguish _request_ip from raw request.client.host because it sets both to 127.0.0.1.)

Connection to other skills

• auth-timestamp-invalidation — covers the timestamp propagation pattern for token invalidation.
• module-test-isolation — when the override breaks tests in combined sessions, the test setup itself is the pollution source.
• test-isolation-patterns — the parent skill.