fix(06.2): WR-07 document X-Forwarded-For trust boundary in all IP extraction code
This commit is contained in:
+29
-11
@@ -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 ──────────────────────────────────────────────────────
|
# ── Safe response helper ──────────────────────────────────────────────────────
|
||||||
|
|
||||||
def _user_to_dict(user: User) -> dict:
|
def _user_to_dict(user: User) -> dict:
|
||||||
@@ -246,14 +264,14 @@ async def create_user(
|
|||||||
session.add(quota)
|
session.add(quota)
|
||||||
await session.flush() # persist User + Quota before audit_log FK references them
|
await session.flush() # persist User + Quota before audit_log FK references them
|
||||||
# D-13: admin user created event
|
# 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(
|
await write_audit_log(
|
||||||
session,
|
session,
|
||||||
event_type="admin.user_created",
|
event_type="admin.user_created",
|
||||||
user_id=new_user.id,
|
user_id=new_user.id,
|
||||||
actor_id=_admin.id,
|
actor_id=_admin.id,
|
||||||
resource_id=new_user.id,
|
resource_id=new_user.id,
|
||||||
ip_address=_ip,
|
ip_address=_ip_addr,
|
||||||
)
|
)
|
||||||
await session.commit()
|
await session.commit()
|
||||||
|
|
||||||
@@ -298,7 +316,7 @@ async def update_user_status(
|
|||||||
detail="Cannot deactivate the only admin",
|
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
|
user.is_active = body.is_active
|
||||||
|
|
||||||
if not body.is_active:
|
if not body.is_active:
|
||||||
@@ -315,7 +333,7 @@ async def update_user_status(
|
|||||||
user_id=user.id,
|
user_id=user.id,
|
||||||
actor_id=_admin.id,
|
actor_id=_admin.id,
|
||||||
resource_id=user.id,
|
resource_id=user.id,
|
||||||
ip_address=_ip,
|
ip_address=_ip_addr,
|
||||||
)
|
)
|
||||||
await session.commit()
|
await session.commit()
|
||||||
|
|
||||||
@@ -408,7 +426,7 @@ async def update_user_quota(
|
|||||||
else None
|
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
|
old_limit = quota.limit_bytes
|
||||||
quota.limit_bytes = body.limit_bytes
|
quota.limit_bytes = body.limit_bytes
|
||||||
session.add(quota)
|
session.add(quota)
|
||||||
@@ -420,7 +438,7 @@ async def update_user_quota(
|
|||||||
user_id=user_id,
|
user_id=user_id,
|
||||||
actor_id=_admin.id,
|
actor_id=_admin.id,
|
||||||
resource_id=None,
|
resource_id=None,
|
||||||
ip_address=_ip,
|
ip_address=_ip_addr,
|
||||||
metadata_={"old_bytes": old_limit, "new_bytes": body.limit_bytes},
|
metadata_={"old_bytes": old_limit, "new_bytes": body.limit_bytes},
|
||||||
)
|
)
|
||||||
await session.commit()
|
await session.commit()
|
||||||
@@ -453,7 +471,7 @@ async def update_ai_config(
|
|||||||
if user is None:
|
if user is None:
|
||||||
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="User not found")
|
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_provider = body.ai_provider
|
||||||
user.ai_model = body.ai_model
|
user.ai_model = body.ai_model
|
||||||
session.add(user)
|
session.add(user)
|
||||||
@@ -465,7 +483,7 @@ async def update_ai_config(
|
|||||||
user_id=user_id,
|
user_id=user_id,
|
||||||
actor_id=_admin.id,
|
actor_id=_admin.id,
|
||||||
resource_id=None,
|
resource_id=None,
|
||||||
ip_address=_ip,
|
ip_address=_ip_addr,
|
||||||
metadata_={"provider": body.ai_provider, "model": body.ai_model},
|
metadata_={"provider": body.ai_provider, "model": body.ai_model},
|
||||||
)
|
)
|
||||||
await session.commit()
|
await session.commit()
|
||||||
@@ -514,7 +532,7 @@ async def delete_user(
|
|||||||
detail="Cannot delete admin accounts",
|
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.
|
# 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
|
# 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,
|
user_id=user_id,
|
||||||
actor_id=_admin.id,
|
actor_id=_admin.id,
|
||||||
resource_id=user_id,
|
resource_id=user_id,
|
||||||
ip_address=_ip,
|
ip_address=_ip_addr,
|
||||||
metadata_={"providers": [c.provider for c in cloud_conns]},
|
metadata_={"providers": [c.provider for c in cloud_conns]},
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -572,7 +590,7 @@ async def delete_user(
|
|||||||
user_id=user_id,
|
user_id=user_id,
|
||||||
actor_id=_admin.id,
|
actor_id=_admin.id,
|
||||||
resource_id=user_id,
|
resource_id=user_id,
|
||||||
ip_address=_ip,
|
ip_address=_ip_addr,
|
||||||
)
|
)
|
||||||
await session.flush()
|
await session.flush()
|
||||||
|
|
||||||
|
|||||||
@@ -377,6 +377,9 @@ async def confirm_upload(
|
|||||||
|
|
||||||
doc.status = "uploaded"
|
doc.status = "uploaded"
|
||||||
# D-13: document uploaded event — size_bytes + storage_backend only, NO filename, NO extracted_text (T-04-07-02)
|
# 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)
|
_ip = request.headers.get("X-Forwarded-For") or (request.client.host if request.client else None)
|
||||||
await write_audit_log(
|
await write_audit_log(
|
||||||
session,
|
session,
|
||||||
@@ -633,6 +636,9 @@ async def delete_document(
|
|||||||
is_cloud = doc.storage_backend != "minio"
|
is_cloud = doc.storage_backend != "minio"
|
||||||
_doc_size = doc.size_bytes
|
_doc_size = doc.size_bytes
|
||||||
_doc_id = doc.id
|
_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)
|
_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
|
# Cloud routing: attempt provider delete unless remove_only is set
|
||||||
|
|||||||
+10
-1
@@ -63,7 +63,16 @@ class SharePermissionPatch(BaseModel):
|
|||||||
|
|
||||||
|
|
||||||
def _ip(request: Request) -> Optional[str]:
|
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 (
|
return request.headers.get("X-Forwarded-For") or (
|
||||||
request.client.host if request.client else None
|
request.client.host if request.client else None
|
||||||
)
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user