fix(dav): harden authorization and bounded PROPFIND semantics

backend/api/dav.py

@@ -1,6 +1,6 @@
 import logging
+import re
 from html import escape as escape_xml_text
-from urllib.parse import unquote
 
 from fastapi import APIRouter, Depends, HTTPException, Request, Response
 from sqlalchemy.ext.asyncio import AsyncSession
@@ -14,14 +14,45 @@
 router = APIRouter(prefix="/dav", tags=["dav"])
 
 
+_INVALID_RAW_PERCENT_ESCAPE = re.compile(br"%(?![0-9A-Fa-f]{2})")
+_NESTED_RAW_PERCENT_ESCAPE = re.compile(br"%25(?=[0-9A-Fa-f]{2})")
+_ENCODED_CONTROL_CHARACTER = re.compile(br"%(?:0[0-9A-Fa-f]|1[0-9A-Fa-f]|7[Ff])")
+
+
+def _validate_dav_raw_request_path(request: Request, decoded_path: str) -> None:
+    """Validate wire encoding before trusting the framework-decoded DAV path."""
+    raw_path = request.scope.get("raw_path")
+    if raw_path is None:
+        if "%" in decoded_path:
+            raise HTTPException(
+                status_code=400,
+                detail="DAV raw path required for percent-bearing path",
+            )
+        return
+    if not isinstance(raw_path, bytes):
+        raise HTTPException(status_code=400, detail="DAV raw path is unavailable")
+    if any(byte < 0x20 or byte == 0x7F for byte in raw_path):
+        raise HTTPException(status_code=400, detail="DAV path contains control characters")
+    if _INVALID_RAW_PERCENT_ESCAPE.search(raw_path):
+        raise HTTPException(
+            status_code=400, detail="DAV path contains invalid percent encoding"
+        )
+    if _ENCODED_CONTROL_CHARACTER.search(raw_path):
+        raise HTTPException(status_code=400, detail="DAV path contains control characters")
+    if _NESTED_RAW_PERCENT_ESCAPE.search(raw_path):
+        raise HTTPException(
+            status_code=400, detail="DAV path contains nested percent encoding"
+        )
+
+
 def _normalize_dav_authorization_path(path: str) -> str:
+    """Normalize the path value after ASGI routing has already decoded the target."""
     normalized_path = path.replace("\\", "/")
-    for _ in range(100):
-        decoded_path = unquote(normalized_path).replace("\\", "/")
-        if decoded_path == normalized_path:
-            return normalized_path
-        normalized_path = decoded_path
-    raise HTTPException(status_code=400, detail="DAV path decoding limit exceeded")
+    if "\ufffd" in normalized_path or any(
+        0xD800 <= ord(character) <= 0xDFFF for character in normalized_path
+    ):
+        raise HTTPException(status_code=400, detail="DAV path contains invalid Unicode")
+    return normalized_path
 
 
 def _dav_path_owner_user_id(path: str) -> str | None:
@@ -173,6 +204,7 @@ async def dav_handler(
     Provider-backed writeback stays fail-closed until source capability and
     ETag/If-Match enforcement are available through signed writeback intents.
     """
+    _validate_dav_raw_request_path(request, path)
     _ensure_dav_owner_scope(path, auth_context)
     safe_path = repr(path)[1:-1]
     logger.info("DAV Request: %s /%s", request.method, safe_path)

backend/tests/test_dav_api.py

@@ -1,8 +1,11 @@
 import defusedxml.ElementTree as ET
 
 import pytest
+from fastapi import HTTPException
 from fastapi.testclient import TestClient
 
+from api.dav import _normalize_dav_authorization_path
+
 from main import app
 from services.webdav_service import webdav_service
 
@@ -156,21 +159,16 @@ def test_dav_unsupported_method_logs_reason(dev_auth_dependency_overrides, caplo
         for record in caplog.records
     )
 
+
 def test_dav_log_injection_prevention(dev_auth_dependency_overrides, caplog):
-    """
-    Test that DAV handlers safely encode control characters in the requested path,
-    preventing log injection vulnerabilities.
-    """
+    """Keep control characters escaped if a direct handler test reaches logging."""
+    import asyncio
     import logging
 
-    caplog.set_level(logging.INFO)
-    malicious_path = "user123/projects/test\x1b[31minjected\n\r"
-
-    # Since HTTP clients block raw control chars and starlette unquotes but might reject it before reaching our route,
-    # we test the handler directly to ensure the logger is using repr().
-    import asyncio
     from fastapi import Request
 
+    caplog.set_level(logging.INFO)
+    malicious_path = "user123/projects/test\x1b[31minjected\n\r"
     scope = {
         "type": "http",
         "method": "OPTIONS",
@@ -180,24 +178,188 @@ def test_dav_log_injection_prevention(dev_auth_dependency_overrides, caplog):
     async def run_handler():
         req = Request(scope)
         from api.auth import AuthContext
-        auth_ctx = AuthContext(user_id="user123", organization_id="org1", role="user", group_ids=[], workspace_id="ws1")
+
+        auth_ctx = AuthContext(
+            user_id="user123",
+            organization_id="org1",
+            role="user",
+            group_ids=[],
+            workspace_id="ws1",
+        )
 
         from api.dav import dav_handler
+
         await dav_handler(request=req, path=malicious_path, auth_context=auth_ctx)
 
     asyncio.run(run_handler())
 
-    # In some fastapi versions, returning an unexpected path might return 404. Let's just assert the log was captured.
-    # The vulnerability is about the logger.
-
-    # Assert that the raw ansi escape / newline was not logged, but encoded
     raw_ansi = "\x1b[31m"
     found_in_logs = False
     for record in caplog.records:
         if "DAV Request" in record.message:
             assert raw_ansi not in record.message, "Raw ANSI escape sequence found in logs!"
             assert "\n" not in record.message[12:], "Raw newline found in log message body!"
-            assert "\\x1b[31minjected\\n\\r" in record.message or "\\x1b[31minjected\\r\\n" in record.message, "Escaped characters missing from log message!"
+            assert (
+                "\\x1b[31minjected\\n\\r" in record.message
+                or "\\x1b[31minjected\\r\\n" in record.message
+            ), "Escaped characters missing from log message!"
             found_in_logs = True
 
     assert found_in_logs, "DAV Request log was not found"
+
+
+@pytest.mark.parametrize(
+    "request_path",
+    [
+        "/dav/user123/projects/%252e%252e",
+        "/dav/user123/projects/%25252e%25252e",
+        "/dav/user123/projects/alice%255c..%255cbob",
+        "/dav/user123/projects/%2525",
+    ],
+)
+def test_dav_route_rejects_ambiguous_nested_encoding(
+    dev_auth_dependency_overrides,
+    request_path: str,
+) -> None:
+    with TestClient(app) as client:
+        response = client.request("PROPFIND", request_path, headers=AUTH_HEADERS)
+
+    assert response.status_code == 400
+    assert response.json()["detail"] == "DAV path contains nested percent encoding"
+
+
+@pytest.mark.parametrize(
+    "request_path",
+    [
+        "/dav/user123/projects/report%25",
+        "/dav/user123/projects/%25report",
+    ],
+)
+def test_dav_route_preserves_encoded_percent_as_data(
+    dev_auth_dependency_overrides,
+    stub_dav_project_folders,
+    request_path: str,
+) -> None:
+    with TestClient(app) as client:
+        response = client.request("PROPFIND", request_path, headers=AUTH_HEADERS)
+
+    assert response.status_code == 207
+    assert "%" in response.text
+
+
+@pytest.mark.parametrize(
+    "request_path",
+    [
+        "/dav/user123/projects/%GG",
+        "/dav/user123/projects/%2",
+    ],
+)
+def test_dav_route_rejects_malformed_raw_percent_escape(
+    dev_auth_dependency_overrides,
+    request_path: str,
+) -> None:
+    with TestClient(app) as client:
+        response = client.request("PROPFIND", request_path, headers=AUTH_HEADERS)
+
+    assert response.status_code == 400
+    assert response.json()["detail"] == "DAV path contains invalid percent encoding"
+
+
+@pytest.mark.parametrize(
+    "request_path",
+    [
+        "/dav/user123/projects/%00",
+        "/dav/user123/projects/%0A",
+        "/dav/user123/projects/%7F",
+    ],
+)
+def test_dav_route_rejects_percent_encoded_control_character(
+    dev_auth_dependency_overrides,
+    request_path: str,
+) -> None:
+    with TestClient(app) as client:
+        response = client.request("PROPFIND", request_path, headers=AUTH_HEADERS)
+
+    assert response.status_code == 400
+    assert response.json()["detail"] == "DAV path contains control characters"
+
+
+def test_dav_route_rejects_invalid_utf8_replacement(
+    dev_auth_dependency_overrides,
+) -> None:
+    with TestClient(app) as client:
+        response = client.request(
+            "PROPFIND",
+            "/dav/user123/projects/%FF",
+            headers=AUTH_HEADERS,
+        )
+
+    assert response.status_code == 400
+    assert response.json()["detail"] == "DAV path contains invalid Unicode"
+
+
+@pytest.mark.parametrize(
+    "request_path",
+    [
+        "/dav/user123/projects/%2e%2e",
+        "/dav/user123/projects/%5c..%5c",
+    ],
+)
+def test_dav_route_rejects_single_decode_traversal(
+    dev_auth_dependency_overrides,
+    request_path: str,
+) -> None:
+    with TestClient(app) as client:
+        response = client.request("PROPFIND", request_path, headers=AUTH_HEADERS)
+
+    assert response.status_code == 403
+    assert response.json()["detail"] == "DAV path must include an owner user"
+
+
+def test_dav_missing_raw_path_fails_closed_for_residual_percent(
+    dev_auth_dependency_overrides,
+) -> None:
+    import asyncio
+
+    from fastapi import Request
+
+    from api.auth import AuthContext
+    from api.dav import dav_handler
+
+    scope = {
+        "type": "http",
+        "method": "OPTIONS",
+        "headers": [],
+        "path": "/dav/user123/projects/report%",
+    }
+    request = Request(scope)
+    auth_context = AuthContext(
+        user_id="user123",
+        organization_id="org1",
+        role="user",
+        group_ids=[],
+        workspace_id="ws1",
+    )
+
+    with pytest.raises(HTTPException) as exc_info:
+        asyncio.run(
+            dav_handler(
+                request=request,
+                path="user123/projects/report%",
+                auth_context=auth_context,
+            )
+        )
+
+    assert exc_info.value.status_code == 400
+    assert exc_info.value.detail == "DAV raw path required for percent-bearing path"
+
+
+def test_normalize_dav_authorization_path_treats_route_path_as_already_decoded() -> None:
+    assert _normalize_dav_authorization_path("/alice/docs") == "/alice/docs"
+    assert _normalize_dav_authorization_path("/%2e%2e/bob") == "/%2e%2e/bob"
+    assert _normalize_dav_authorization_path("/alice%/docs") == "/alice%/docs"
+
+
+def test_normalize_dav_authorization_path_has_no_recursive_input_amplification() -> None:
+    path = "/alice/" + ("segment-" * 8192)
+    assert _normalize_dav_authorization_path(path) == path

docs/doctoring/dav-authorization-path-decoding.md

@@ -0,0 +1,53 @@
+# DAV authorization-path decoding boundary
+
+## Decision
+
+Naruon authorizes DAV paths against the Unicode `path` value supplied by ASGI routing and does not percent-decode that value again. The original request-target bytes, when supplied as ASGI `raw_path`, are used only to validate wire-level percent syntax and reject encodings whose first decode would leave another `%HH` sequence. Backslashes are normalized to `/` before owner/traversal checks.
+
+This separates two representations that must not be conflated:
+
+- `raw_path`: original request-target path bytes. It is the only representation that can distinguish a literal percent encoded as `%25` from malformed raw `%` syntax or an ambiguous nested encoding such as `%252e`.
+- `path`: framework-decoded Unicode path. Authorization, owner extraction and traversal checks consume this representation without another percent-decoding pass.
+
+The ASGI HTTP specification defines `path` as having percent-encoded and UTF-8 byte sequences decoded into characters. It defines `raw_path` as the original path bytes and notes that `raw_path` is optional. Therefore a second application `unquote()` over a route parameter is not a neutral normalization step: it can create a second interpretation of data that the framework has already decoded.
+
+RFC 3986 §2.4 warns that implementations must not decode the same string more than once. CWE-174 describes the corresponding weakness as double decoding that can introduce dangerous input after an earlier validation step.
+
+## Failure lineage
+
+Issue #1344 identified the prior recursive-unquote loop as the wrong authorization boundary. PR #1645 initially replaced the loop with one explicit `unquote()`, but its helper-only tests did not account for framework decoding. That intermediate state also rejected legitimate encoded-percent data because `%25` became `%` and was then classified as a malformed escape.
+
+The current repair is intentionally narrower:
+
+1. `c91988398414db3cd0226749aaa336c268543dbc` adds route-level RED cases for framework-decoded nested traversal, literal encoded percent data, malformed raw escapes, encoded control characters, invalid UTF-8 replacement, single-decode traversal, and a large non-recursive path.
+2. `eafcc8d4720e58bb04d3029774e5c31f38a6e1e0` removes application percent-decoding from authorization normalization and validates raw request-target syntax before owner checks.
+3. ASGI specifies that `raw_path` may be absent. `b6931bf55a4a3320e103b417ee384c82f12444ac` therefore adds a second RED case: if raw provenance is unavailable and the decoded path still contains `%`, Naruon must not guess whether that percent came from valid encoded data, malformed wire syntax or a nested encoding.
+4. `42b5af38f21231aa4cad5f2e0ce0768f81fc1ee4` makes that fallback fail closed while permitting percent-free decoded paths on ASGI servers that omit `raw_path`.
+
+The old `0bdaf4fedc001fe43326fa67390f05f83a718238` checks were admitted while #1645 targeted `develop`; they are historical after the PR was retargeted to canonical parent #1417 and do not certify the current `(PR, base ref/SHA, head SHA)` identity.
+
+## Invariants
+
+- Authorization never recursively percent-decodes a framework route value.
+- Malformed raw percent triplets fail with HTTP 400 when `raw_path` is available.
+- A raw `%25` that would decode to a literal percent is allowed when it does not form a second `%HH` sequence.
+- A raw encoding such as `%252e` or `%2525` that would expose another valid percent triplet is rejected as ambiguous before authorization.
+- Percent-encoded C0/DEL control characters fail before owner or DAV operation handling.
+- Invalid UTF-8 replacement/surrogate values in the framework-decoded path fail before authorization.
+- `.` and `..` segments, including those produced by the framework's single decode and Windows-separator normalization, remain unauthorized.
+- If `raw_path` is unavailable, a residual `%` in the decoded path fails closed because its wire provenance cannot be established. Percent-free decoded paths remain supported.
+- The normalization path is linear in input length; there is no recursive or fixed-round decode loop.
+
+## Reproducible acceptance
+
+The owned executable acceptance is `backend/tests/test_dav_api.py`. Required evidence includes raw, singly encoded and nested traversal cases; encoded-percent data; malformed triplets; slash/backslash variants; encoded controls and invalid Unicode; route-level TestClient behavior; and a large input that exercises the non-recursive path. The current exact branch must run this test plus Ruff and the repository security/CI gates after every source or document change. A predecessor-head pass is not current-head evidence.
+
+Issue #1344 stays open until current-base exact-head hosted checks, current-head independent review and all valid findings are complete. This document does not claim protected integration, release, deployment, broader DAV writeback support, or a security certification.
+
+## References
+
+ASGI Team. (n.d.). *HTTP & WebSocket ASGI message format*. ASGI 3.0 documentation. Retrieved September 10, 2026, from https://asgi.readthedocs.io/en/latest/specs/www.html
+
+Berners-Lee, T., Fielding, R., & Masinter, L. (2005). *Uniform Resource Identifier (URI): Generic Syntax* (RFC 3986). Internet Engineering Task Force. https://doi.org/10.17487/RFC3986
+
+The MITRE Corporation. (n.d.). *CWE-174: Double Decoding of the Same Data* (CWE List Version 4.20). Common Weakness Enumeration. Retrieved September 10, 2026, from https://cwe.mitre.org/data/definitions/174.html