Add enrollment deletion, security validations, and thread-safe metrics by ahmetabdullahgultekin · Pull Request #18 · Rollingcat-Software/biometric-processor

ahmetabdullahgultekin · 2026-03-12T14:25:49Z

Summary

This PR adds face enrollment deletion functionality, enhances security with file type validation, implements thread-safe metrics tracking, and adds a UniFace liveness detector placeholder.

Key Changes

New Features

Enrollment Deletion: Added DELETE /enroll/{user_id} endpoint with DeleteEnrollmentUseCase to remove user face embeddings from the repository
UniFace Liveness Detector: Added UniFaceLivenessDetector class that wraps texture-based analysis as a placeholder for MiniFASNet model integration

Security Enhancements

File Type Validation: Added magic byte validation in verification, liveness, and search endpoints to prevent file type spoofing attacks (validates actual file content, not just Content-Type headers)
Batch Size Limits: Added validation in batch verification to prevent DoS attacks by enforcing maximum batch size and total payload size limits
Tenant ID Validation: Added validation for tenant ID header in proctor endpoint

Code Quality & Reliability

Thread-Safe Metrics: Wrapped all metrics recording functions (record_activity, record_api_call, record_verification, record_search, record_liveness_check) with mutex locks to prevent race conditions in concurrent environments
Error Handling: Improved base64 decoding error handling in WebSocket frame handler with descriptive error messages
Exception Handling: Fixed bare except clause in live_analysis WebSocket handler to catch specific Exception type

Implementation Details

Enrollment deletion validates user_id input and returns appropriate HTTP status codes (404 if not found, 500 on server errors)
File validation uses validate_image_file() utility with configurable allowed formats from settings
Metrics locking uses threading.Lock() for in-memory metrics (noted as placeholder for Redis in production)
All new endpoints follow existing patterns with dependency injection and comprehensive logging

https://claude.ai/code/session_014adzmvdwnihxwoQUpFaeyW

The identity-core-api's BiometricServiceAdapter calls DELETE /enroll/{userId} but this endpoint was missing from the biometric-processor. Added: - DeleteEnrollmentUseCase that delegates to repository.delete() - DELETE /enroll/{user_id} route in enrollment.py - Factory function in container.py https://claude.ai/code/session_014adzmvdwnihxwoQUpFaeyW

Creates uniface_liveness_detector.py that was referenced in container.py and __init__.py but never existed, causing pytest collection failures on 7 test files. Falls back to TextureLivenessDetector until the native uniface module is available. https://claude.ai/code/session_014adzmvdwnihxwoQUpFaeyW

Replace bare except: with except Exception: in live_analysis.py WebSocket handler. Add validate_image_file() magic-byte validation to verification.py, liveness.py, and search.py routes to match the security pattern already used in enrollment.py, preventing file type confusion attacks. https://claude.ai/code/session_014adzmvdwnihxwoQUpFaeyW

- Add batch size and total size validation to batch_verify endpoint, matching the protection already in batch_enroll (prevents memory exhaustion via unlimited file uploads) - Add thread-safe locking to admin metrics recording functions to prevent race conditions in multi-worker deployments - Add tenant ID validation in proctor routes using validate_tenant_id() to prevent injection attacks via X-Tenant-ID header https://claude.ai/code/session_014adzmvdwnihxwoQUpFaeyW

Wraps base64.b64decode() in try/except to prevent unhandled binascii.Error from crashing WebSocket connections when clients send invalid base64 data. https://claude.ai/code/session_014adzmvdwnihxwoQUpFaeyW

Copilot

Pull request overview

This PR expands the biometric API with enrollment deletion, adds content-based image validation to reduce file-type spoofing risk, introduces mutex-protected in-memory metrics recording, and adds a UniFace liveness-detector placeholder wired into the liveness backend selection.

Changes:

Added DELETE /enroll/{user_id} via a new DeleteEnrollmentUseCase and DI wiring.
Added magic-byte image validation for verification/search/liveness uploads and additional batch verification size limits.
Made admin metrics writers thread-safe via a global lock and added UniFace liveness backend placeholder.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
app/infrastructure/ml/liveness/uniface_liveness_detector.py	Adds UniFace liveness detector placeholder delegating to texture fallback.
app/core/container.py	Wires `DeleteEnrollmentUseCase` and UniFace liveness backend into DI container.
app/application/use_cases/delete_enrollment.py	Implements the enrollment deletion use case over `IEmbeddingRepository.delete()`.
app/api/websocket/frame_handler.py	Improves base64 frame decode error handling for websocket frames.
app/api/routes/verification.py	Adds magic-byte image validation (but currently imports a non-existent `get_settings`).
app/api/routes/search.py	Adds magic-byte image validation (but currently imports a non-existent `get_settings`).
app/api/routes/proctor.py	Validates tenant ID header via `validate_tenant_id`.
app/api/routes/liveness.py	Adds magic-byte image validation (but currently imports a non-existent `get_settings`).
app/api/routes/live_analysis.py	Replaces bare `except` with `except Exception`.
app/api/routes/enrollment.py	Adds `DELETE /enroll/{user_id}` endpoint.
app/api/routes/batch.py	Adds batch verification count/size guardrails.
app/api/routes/admin.py	Adds a lock and wraps metrics writes for thread-safety.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-12T14:31:56Z

+from app.core.config import get_settings
+from app.core.validation import ValidationError, validate_image_file
 from app.domain.interfaces.file_storage import IFileStorage

+settings = get_settings()
+


app.core.config does not define get_settings() (it only exposes the settings = Settings() singleton). This import will fail and prevent the liveness routes from loading. Use the existing settings singleton (or add get_settings() to app.core.config).

Suggested change

from app.core.config import get_settings

from app.core.validation import ValidationError, validate_image_file

from app.domain.interfaces.file_storage import IFileStorage

settings = get_settings()

from app.core.config import settings

from app.core.validation import ValidationError, validate_image_file

from app.domain.interfaces.file_storage import IFileStorage

Copilot · 2026-03-12T14:31:57Z

+            try:
+                frame_bytes = base64.b64decode(frame_b64)
+            except Exception as e:
+                raise ValueError(f"Invalid base64 frame data: {e}")


base64.b64decode() without validate=True will ignore non-base64 characters, and catching generic Exception makes it harder to distinguish decode failures. Consider decoding with strict validation and catching the specific base64/binascii exceptions; also re-raise with exception chaining (raise ... from e) so the original cause is preserved.

Copilot · 2026-03-12T14:31:57Z

+@router.delete("/enroll/{user_id}", status_code=200)
+async def delete_enrollment(
+    user_id: str,
+    use_case: DeleteEnrollmentUseCase = Depends(get_delete_enrollment_use_case),
+) -> dict:


This delete endpoint doesn’t accept or validate tenant_id, and IEmbeddingRepository.delete() will delete across all tenants when tenant_id is None (see pgvector repo query using ($2 IS NULL OR tenant_id = $2)). In a multi-tenant setup this can delete data outside the caller’s tenant. Consider requiring tenant_id (or at minimum accepting/validating it and passing it through to the use case).

Copilot · 2026-03-12T14:31:57Z

+from app.core.config import get_settings
+from app.core.validation import ValidationError, validate_image_file, validate_user_id, validate_tenant_id
 from app.domain.interfaces.file_storage import IFileStorage

+settings = get_settings()


app.core.config does not define get_settings() (it only exposes the settings = Settings() singleton). Importing get_settings here will raise ImportError and prevent the API from starting. Switch to importing settings directly or add an actual get_settings() helper in app.core.config and use that consistently.

Suggested change

from app.core.config import get_settings

from app.core.validation import ValidationError, validate_image_file, validate_user_id, validate_tenant_id

from app.domain.interfaces.file_storage import IFileStorage

settings = get_settings()

from app.core.config import settings as app_settings

from app.core.validation import ValidationError, validate_image_file, validate_user_id, validate_tenant_id

from app.domain.interfaces.file_storage import IFileStorage

settings = app_settings

Copilot · 2026-03-12T14:31:58Z

+from app.core.config import get_settings
+from app.core.validation import ValidationError, validate_image_file
 from app.domain.interfaces.file_storage import IFileStorage

+settings = get_settings()
+


app.core.config does not define get_settings() (it only exposes the settings = Settings() singleton). This import will fail at runtime and break the /search endpoint module import. Use from app.core.config import settings (or implement get_settings() in config).

Suggested change

from app.core.config import get_settings

from app.core.validation import ValidationError, validate_image_file

from app.domain.interfaces.file_storage import IFileStorage

settings = get_settings()

from app.core.config import settings

from app.core.validation import ValidationError, validate_image_file

from app.domain.interfaces.file_storage import IFileStorage

Copilot · 2026-03-12T14:31:58Z

+    total_size_bytes = 0
+    for file in files:
+        if hasattr(file, 'size') and file.size:
+            total_size_bytes += file.size
+        else:
+            total_size_bytes += 2 * 1024 * 1024
+    max_total_bytes = settings.BATCH_MAX_TOTAL_SIZE_MB * 1024 * 1024
+    if total_size_bytes > max_total_bytes:
+        raise HTTPException(
+            status_code=400,
+            detail=f"Total batch size exceeds maximum ({settings.BATCH_MAX_TOTAL_SIZE_MB}MB)"
+        )


UploadFile (FastAPI/Starlette) does not expose a reliable .size attribute, so this total-size check will usually fall back to the 2MB estimate and can be bypassed with very large uploads (defeating the DoS protection). Consider calculating actual size (e.g., via the underlying file object or Content-Length) or enforcing per-file limits in the storage layer before processing.

Copilot · 2026-03-12T14:31:58Z

+    with _metrics_lock:
+        _metrics["activities"].insert(0, activity)
+        # Keep only last 100 activities
+        _metrics["activities"] = _metrics["activities"][:100]


 def record_api_call(response_time_ms: float):
-    """Record an API call for metrics."""
-    _metrics["api_calls_today"] += 1
-    _metrics["api_calls_this_week"] += 1
-    _metrics["response_times"].append(response_time_ms)
-    # Keep only last 1000 response times
-    _metrics["response_times"] = _metrics["response_times"][-1000:]
+    """Record an API call for metrics (thread-safe)."""
+    with _metrics_lock:
+        _metrics["api_calls_today"] += 1
+        _metrics["api_calls_this_week"] += 1
+        _metrics["response_times"].append(response_time_ms)
+        # Keep only last 1000 response times
+        _metrics["response_times"] = _metrics["response_times"][-1000:]


Locking writes is a good start, but /admin/stats and /admin/activity read _metrics without acquiring _metrics_lock. That means the metrics are still not fully thread-safe (reads can observe partial updates / inconsistent lists while another request mutates them). Consider taking the lock while reading or copying a snapshot under lock before computing aggregates.

Copilot · 2026-03-12T14:31:59Z

+import threading
+
 # Track metrics in memory (would use Redis in production)


import threading is placed after router = APIRouter(...). Keeping imports grouped at the top of the module avoids surprising side effects and follows the style used elsewhere in this file.

Copilot · 2026-03-12T14:31:59Z

+    """Extract and validate tenant ID from header."""
+    from app.core.validation import ValidationError, validate_tenant_id
+    try:
+        return validate_tenant_id(x_tenant_id)


validate_tenant_id() can return None for empty/whitespace input, but this dependency is annotated to return str and is used as a required header. As-is, an empty X-Tenant-ID would be treated as valid and could propagate None into downstream logic. Consider explicitly rejecting empty/whitespace tenant IDs here (or add a validate_required_tenant_id helper).

Suggested change

return validate_tenant_id(x_tenant_id)

tenant_id = validate_tenant_id(x_tenant_id)

if tenant_id is None:

raise HTTPException(

status_code=400,

detail="Invalid tenant ID: Tenant ID is required and cannot be empty",

)

return tenant_id

Copilot · 2026-03-12T14:31:59Z

+@router.delete("/enroll/{user_id}", status_code=200)
+async def delete_enrollment(
+    user_id: str,
+    use_case: DeleteEnrollmentUseCase = Depends(get_delete_enrollment_use_case),
+) -> dict:
+    """Delete a user's face enrollment.
+
+    Args:
+        user_id: User identifier whose enrollment should be deleted
+        use_case: Injected delete enrollment use case
+
+    Returns:
+        Success response with deletion status
+
+    Raises:
+        HTTPException 404: If no enrollment found for user
+        HTTPException 500: Internal server error
+    """
+    try:
+        # Validate user_id
+        try:
+            user_id = validate_user_id(user_id)
+        except ValidationError as e:
+            logger.warning(f"Input validation failed: {str(e)}")
+            raise HTTPException(status_code=400, detail=f"Invalid input: {str(e)}")
+
+        logger.info(f"Delete enrollment request: user_id={user_id}")
+
+        deleted = await use_case.execute(user_id=user_id)
+
+        if not deleted:
+            raise HTTPException(
+                status_code=404,
+                detail=f"No enrollment found for user: {user_id}",
+            )
+
+        return {
+            "success": True,
+            "user_id": user_id,
+            "message": "Face data deleted successfully",
+        }


There are integration tests for the existing enrollment endpoints (e.g., tests/integration/test_api_routes.py::TestEnrollmentEndpoint), but this new DELETE /api/v1/enroll/{user_id} behavior isn’t covered. Add tests for success, invalid user_id (400), and not-found (404) (and tenant behavior if tenant_id is added) to prevent regressions.

claude added 5 commits March 12, 2026 06:23

Add error handling for base64 frame decoding in WebSocket handler

3c2bd49

Wraps base64.b64decode() in try/except to prevent unhandled binascii.Error from crashing WebSocket connections when clients send invalid base64 data. https://claude.ai/code/session_014adzmvdwnihxwoQUpFaeyW

Copilot AI review requested due to automatic review settings March 12, 2026 14:25

Copilot started reviewing on behalf of ahmetabdullahgultekin March 12, 2026 14:26 View session

ahmetabdullahgultekin merged commit ee74db3 into main Mar 12, 2026
6 of 9 checks passed

Copilot AI reviewed Mar 12, 2026

View reviewed changes

ahmetabdullahgultekin deleted the claude/audit-client-backend-flows-jdo2n branch March 28, 2026 08:27

ahmetabdullahgultekin mentioned this pull request May 12, 2026

fix(verify): enforce anti-spoof block + EAR + aged-threshold + SHA-pin + verify-challenge (2026-05-12 ML review) #102

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add enrollment deletion, security validations, and thread-safe metrics#18

Add enrollment deletion, security validations, and thread-safe metrics#18
ahmetabdullahgultekin merged 5 commits into
mainfrom
claude/audit-client-backend-flows-jdo2n

ahmetabdullahgultekin commented Mar 12, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Copilot AI Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		import threading

		# Track metrics in memory (would use Redis in production)

-        return validate_tenant_id(x_tenant_id)
+        tenant_id = validate_tenant_id(x_tenant_id)
+        if tenant_id is None:
+            raise HTTPException(
+                status_code=400,
+                detail="Invalid tenant ID: Tenant ID is required and cannot be empty",
+            )
+        return tenant_id

Uh oh!

Conversation

ahmetabdullahgultekin commented Mar 12, 2026

Summary

Key Changes

New Features

Security Enhancements

Code Quality & Reliability

Implementation Details

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants