diff --git a/backend/api/admin.py b/backend/api/admin.py index 03ef9f9..2e5a627 100644 --- a/backend/api/admin.py +++ b/backend/api/admin.py @@ -52,6 +52,24 @@ _PASSWORD_DETAIL = ( ) +# ── IP extraction helper ────────────────────────────────────────────────────── + +def _ip(request: Request) -> Optional[str]: + """Extract best-effort client IP from request for audit logging. + + TRUST BOUNDARY: X-Forwarded-For is a client-controlled header and can be + forged by any caller. This value is used for forensic audit logging only — + not for authentication or access control decisions. In production, deploy + behind a trusted reverse proxy (e.g. nginx with + `proxy_set_header X-Forwarded-For $remote_addr;`) which overwrites this + header with the real remote IP before it reaches FastAPI, or use a + trusted-proxy middleware that validates the source CIDR. + """ + return request.headers.get("X-Forwarded-For") or ( + request.client.host if request.client else None + ) + + # ── Safe response helper ────────────────────────────────────────────────────── def _user_to_dict(user: User) -> dict: @@ -246,14 +264,14 @@ async def create_user( session.add(quota) await session.flush() # persist User + Quota before audit_log FK references them # D-13: admin user created event - _ip = request.headers.get("X-Forwarded-For") or (request.client.host if request.client else None) + _ip_addr = _ip(request) await write_audit_log( session, event_type="admin.user_created", user_id=new_user.id, actor_id=_admin.id, resource_id=new_user.id, - ip_address=_ip, + ip_address=_ip_addr, ) await session.commit() @@ -298,7 +316,7 @@ async def update_user_status( detail="Cannot deactivate the only admin", ) - _ip = request.headers.get("X-Forwarded-For") or (request.client.host if request.client else None) + _ip_addr = _ip(request) user.is_active = body.is_active if not body.is_active: @@ -315,7 +333,7 @@ async def update_user_status( user_id=user.id, actor_id=_admin.id, resource_id=user.id, - ip_address=_ip, + ip_address=_ip_addr, ) await session.commit() @@ -408,7 +426,7 @@ async def update_user_quota( else None ) - _ip = request.headers.get("X-Forwarded-For") or (request.client.host if request.client else None) + _ip_addr = _ip(request) old_limit = quota.limit_bytes quota.limit_bytes = body.limit_bytes session.add(quota) @@ -420,7 +438,7 @@ async def update_user_quota( user_id=user_id, actor_id=_admin.id, resource_id=None, - ip_address=_ip, + ip_address=_ip_addr, metadata_={"old_bytes": old_limit, "new_bytes": body.limit_bytes}, ) await session.commit() @@ -453,7 +471,7 @@ async def update_ai_config( if user is None: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="User not found") - _ip = request.headers.get("X-Forwarded-For") or (request.client.host if request.client else None) + _ip_addr = _ip(request) user.ai_provider = body.ai_provider user.ai_model = body.ai_model session.add(user) @@ -465,7 +483,7 @@ async def update_ai_config( user_id=user_id, actor_id=_admin.id, resource_id=None, - ip_address=_ip, + ip_address=_ip_addr, metadata_={"provider": body.ai_provider, "model": body.ai_model}, ) await session.commit() @@ -514,7 +532,7 @@ async def delete_user( detail="Cannot delete admin accounts", ) - _ip = request.headers.get("X-Forwarded-For") or (request.client.host if request.client else None) + _ip_addr = _ip(request) # SEC-09 (cloud): purge cloud-stored documents and credentials BEFORE DB delete. # Must run before MinIO cleanup so that credentials are still available to build @@ -548,7 +566,7 @@ async def delete_user( user_id=user_id, actor_id=_admin.id, resource_id=user_id, - ip_address=_ip, + ip_address=_ip_addr, metadata_={"providers": [c.provider for c in cloud_conns]}, ) @@ -572,7 +590,7 @@ async def delete_user( user_id=user_id, actor_id=_admin.id, resource_id=user_id, - ip_address=_ip, + ip_address=_ip_addr, ) await session.flush() diff --git a/backend/api/documents.py b/backend/api/documents.py index 907c648..2a119aa 100644 --- a/backend/api/documents.py +++ b/backend/api/documents.py @@ -377,6 +377,9 @@ async def confirm_upload( doc.status = "uploaded" # D-13: document uploaded event — size_bytes + storage_backend only, NO filename, NO extracted_text (T-04-07-02) + # TRUST BOUNDARY: X-Forwarded-For is client-controlled — for audit logging only, + # not for auth/access control. Use a trusted reverse proxy in production to + # overwrite this header with the real remote IP before it reaches FastAPI. _ip = request.headers.get("X-Forwarded-For") or (request.client.host if request.client else None) await write_audit_log( session, @@ -633,6 +636,9 @@ async def delete_document( is_cloud = doc.storage_backend != "minio" _doc_size = doc.size_bytes _doc_id = doc.id + # TRUST BOUNDARY: X-Forwarded-For is client-controlled — for audit logging only, + # not for auth/access control. Use a trusted reverse proxy in production to + # overwrite this header with the real remote IP before it reaches FastAPI. _ip = request.headers.get("X-Forwarded-For") or (request.client.host if request.client else None) # Cloud routing: attempt provider delete unless remove_only is set diff --git a/backend/api/shares.py b/backend/api/shares.py index e75c87d..a6dad31 100644 --- a/backend/api/shares.py +++ b/backend/api/shares.py @@ -63,7 +63,16 @@ class SharePermissionPatch(BaseModel): def _ip(request: Request) -> Optional[str]: - """Extract best-effort client IP from request (behind proxy or direct).""" + """Extract best-effort client IP from request (behind proxy or direct). + + TRUST BOUNDARY: X-Forwarded-For is a client-controlled header and can be + forged by any caller. This value is used for forensic audit logging only — + not for authentication or access control decisions. In production, deploy + behind a trusted reverse proxy (e.g. nginx with + `proxy_set_header X-Forwarded-For $remote_addr;`) which overwrites this + header with the real remote IP before it reaches FastAPI, or use a + trusted-proxy middleware that validates the source CIDR. + """ return request.headers.get("X-Forwarded-For") or ( request.client.host if request.client else None )