From 5673c53fa8c869e56fa3713f8c94706fd0f9a224 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 4 Aug 2026 16:29:52 +0900 Subject: [PATCH 01/19] fix(security,api): harden CardDAV paths and opaque prompt identifiers --- backend/api/data.py | 16 ++++- backend/api/emails.py | 12 +++- backend/api/prompts.py | 1 - backend/services/carddav_discovery.py | 33 +++++++--- backend/tests/test_carddav_discovery.py | 30 +++++++++ ...t_carddav_encoded_path_canonicalization.py | 13 ++++ .../tests/test_carddav_unicode_controls.py | 5 ++ backend/tests/test_data_api.py | 29 +++++++- .../test_data_api_document_org_isolation.py | 66 +++++++++++++++++++ backend/tests/test_prompts_api.py | 11 +++- 10 files changed, 202 insertions(+), 14 deletions(-) create mode 100644 backend/tests/test_carddav_encoded_path_canonicalization.py create mode 100644 backend/tests/test_carddav_unicode_controls.py create mode 100644 backend/tests/test_data_api_document_org_isolation.py diff --git a/backend/api/data.py b/backend/api/data.py index dccd85890..83b9fa6d5 100644 --- a/backend/api/data.py +++ b/backend/api/data.py @@ -2489,10 +2489,16 @@ async def _get_workspace_document( auth_context: AuthContext, document_id: str, ) -> Document: + organization_filter = ( + Document.organization_id == auth_context.organization_id + if auth_context.organization_id is not None + else Document.organization_id.is_(None) + ) result = await db.execute( select(Document).where( Document.document_id == document_id, Document.workspace_id == auth_context.workspace_id, + organization_filter, ) ) document = result.scalar_one_or_none() @@ -3928,10 +3934,18 @@ async def get_data_quality_surface( ProjectFolder.folder_uid.asc(), ), ) + document_organization_filter = ( + Document.organization_id == auth_context.organization_id + if auth_context.organization_id is not None + else Document.organization_id.is_(None) + ) documents = await _scoped_rows( db, select(Document) - .where(Document.workspace_id == auth_context.workspace_id) + .where( + Document.workspace_id == auth_context.workspace_id, + document_organization_filter, + ) .order_by(Document.created_at.desc(), Document.document_id.asc()) .limit(8), ) diff --git a/backend/api/emails.py b/backend/api/emails.py index 223ebf040..49d0ce0e9 100644 --- a/backend/api/emails.py +++ b/backend/api/emails.py @@ -5,7 +5,7 @@ from sqlalchemy import func, or_, select from db.session import get_db from db.models import Email -from pydantic import BaseModel, EmailStr, Field +from pydantic import BaseModel, EmailStr, Field, field_validator import datetime import time from typing import Literal @@ -693,6 +693,16 @@ class SendEmailRequest(BaseModel): in_reply_to: str | None = None # O3: email threading support references: str | None = None + @field_validator("to", "subject", "in_reply_to", "references", mode="before") + @classmethod + def reject_crlf(cls, value: object) -> object: + """Reject SMTP header injection via CR/LF before pattern validation.""" + if value is None: + return value + if isinstance(value, str) and ("\r" in value or "\n" in value): + raise ValueError("Email header fields must not contain newlines") + return value + @router.post("/send") async def send_email_endpoint( diff --git a/backend/api/prompts.py b/backend/api/prompts.py index c4d7008ea..408b096de 100644 --- a/backend/api/prompts.py +++ b/backend/api/prompts.py @@ -36,7 +36,6 @@ class PromptCreate(BaseModel): class PromptResponse(BaseModel): - id: int prompt_uid: str title: str description: Optional[str] = None diff --git a/backend/services/carddav_discovery.py b/backend/services/carddav_discovery.py index 70c742685..c1a475a18 100644 --- a/backend/services/carddav_discovery.py +++ b/backend/services/carddav_discovery.py @@ -29,7 +29,8 @@ import socket from dataclasses import dataclass from typing import Any, Awaitable, Callable -from urllib.parse import urljoin, urlsplit, urlunsplit +from unicodedata import category +from urllib.parse import unquote, urljoin, urlsplit, urlunsplit import httpx @@ -43,6 +44,8 @@ TxtResolver = Callable[[str], list[str]] HttpClientFactory = Callable[[], Any] +_MAX_CONTEXT_PATH_DECODE_ROUNDS = 5 + @dataclass(frozen=True) class CarddavDiscoveryResult: @@ -224,18 +227,30 @@ def _txt_context_path(records: list[str]) -> str | None: if key.strip().lower() != "path": continue path = value.strip() + decoded_path = path + for _ in range(_MAX_CONTEXT_PATH_DECODE_ROUNDS): + next_path = unquote(decoded_path) + if next_path == decoded_path: + break + decoded_path = next_path + else: + # Reject values that still change after the decode budget. This + # keeps over-encoded traversal payloads from hiding another + # interpretation beyond the validation boundary. + if unquote(decoded_path) != decoded_path: + continue if ( - path.startswith("/") - and "://" not in path - and "\\" not in path - and "?" not in path - and "#" not in path + decoded_path.startswith("/") + and "://" not in decoded_path + and "\\" not in decoded_path + and "?" not in decoded_path + and "#" not in decoded_path and all( - segment not in {".", ".."} for segment in path.split("/") + segment not in {".", ".."} for segment in decoded_path.split("/") ) - and all(ord(ch) >= 32 and ord(ch) != 127 for ch in path) + and all(category(ch) != "Cc" for ch in decoded_path) ): - return path + return decoded_path return None diff --git a/backend/tests/test_carddav_discovery.py b/backend/tests/test_carddav_discovery.py index f8bbd2987..50e483fa9 100644 --- a/backend/tests/test_carddav_discovery.py +++ b/backend/tests/test_carddav_discovery.py @@ -163,6 +163,36 @@ def txt_resolver(name): assert result.base_url == "https://dav.example.com/" +@pytest.mark.asyncio +@pytest.mark.parametrize( + "txt_path", + [ + "/%2e%2e%2fescape", + "/%252e%252e%252fescape", + "/%5c..%5cescape", + "/%255c..%255cescape", + "/safe%0aheader", + ], +) +@pytest.mark.asyncio +async def test_encoded_unsafe_txt_path_is_ignored(txt_path): + response = FakeResponse(404) + + def resolver(name): + if name == "_carddavs._tcp.example.com": + return [("dav.example.com", 443)] + return [] + + result = await discover_carddav( + "example.com", + http_client_factory=_factory(response), + srv_resolver=resolver, + txt_resolver=lambda name: [f"path={txt_path}"], + ) + assert result is not None + assert result.base_url == "https://dav.example.com/" + + @pytest.mark.asyncio async def test_no_discovery_returns_none(): response = FakeResponse(404) diff --git a/backend/tests/test_carddav_encoded_path_canonicalization.py b/backend/tests/test_carddav_encoded_path_canonicalization.py new file mode 100644 index 000000000..17b8625e3 --- /dev/null +++ b/backend/tests/test_carddav_encoded_path_canonicalization.py @@ -0,0 +1,13 @@ +"""Regression tests for canonical CardDAV TXT context-path execution.""" + +from services.carddav_discovery import _txt_context_path + + +def test_fully_encoded_leading_slash_is_canonicalized() -> None: + """Execute the same decoded representation that passed validation.""" + assert _txt_context_path(["path=%2Fsafe"]) == "/safe" + + +def test_percent_encoded_unicode_path_is_canonicalized() -> None: + """Preserve a safe Unicode path after bounded decoding.""" + assert _txt_context_path(["path=/%EC%A3%BC%EC%86%8C%EB%A1%9D"]) == "/주소록" diff --git a/backend/tests/test_carddav_unicode_controls.py b/backend/tests/test_carddav_unicode_controls.py new file mode 100644 index 000000000..dabea83fb --- /dev/null +++ b/backend/tests/test_carddav_unicode_controls.py @@ -0,0 +1,5 @@ +from services.carddav_discovery import _txt_context_path + + +def test_txt_context_path_rejects_unicode_c1_control(): + assert _txt_context_path(["path=/safe%C2%85header"]) is None diff --git a/backend/tests/test_data_api.py b/backend/tests/test_data_api.py index cd0b7bf37..eb67f894a 100644 --- a/backend/tests/test_data_api.py +++ b/backend/tests/test_data_api.py @@ -101,11 +101,27 @@ async def execute(self, query): ), None, ) + organization_ids = [ + value + for key, value in params.items() + if key.startswith("organization_id") + ] + organization_is_null = ( + "workspace_documents.organization_id is null" in rendered_query_lower + ) rows = [ document for document in self.documents if (document_id is None or document.document_id == document_id) and (workspace_id is None or document.workspace_id == workspace_id) + and ( + ( + organization_ids + and document.organization_id == organization_ids[0] + ) + or (organization_is_null and document.organization_id is None) + or (not organization_ids and not organization_is_null) + ) ] if "order by" in rendered_query_lower: return MockResult(rows) @@ -2484,6 +2500,7 @@ def test_data_quality_surface_includes_workspace_document_assets(mock_db): Document( document_id="doc_owned", workspace_id="workspace-org-acme", + organization_id="org-acme", document_name="roadmap.md", document_type="text/markdown", document_content="# Roadmap", @@ -2493,6 +2510,7 @@ def test_data_quality_surface_includes_workspace_document_assets(mock_db): Document( document_id="doc_rival", workspace_id="workspace-rival", + organization_id="org-rival", document_name="rival.md", document_type="text/markdown", document_content="rival", @@ -2586,6 +2604,7 @@ def test_data_document_actions_are_workspace_scoped_and_intent_only(mock_db): document = Document( document_id="doc_owned", workspace_id="workspace-org-acme", + organization_id="org-acme", document_name="source.hwp", document_type="application/x-hwp", document_content="opaque hwp extraction placeholder", @@ -2594,7 +2613,8 @@ def test_data_document_actions_are_workspace_scoped_and_intent_only(mock_db): ) rival_document = Document( document_id="doc_rival", - workspace_id="workspace-rival", + workspace_id="workspace-org-acme", + organization_id="org-rival", document_name="rival.md", document_type="text/markdown", document_content="rival", @@ -2638,6 +2658,7 @@ def test_data_document_actions_are_workspace_scoped_and_intent_only(mock_db): assert rival_response.status_code == 404 assert "doc_rival" not in rival_response.text + assert rival_document.document_status == "uploaded" def test_data_document_webdav_materialization_executes_source_backed_write( @@ -2648,6 +2669,7 @@ def test_data_document_webdav_materialization_executes_source_backed_write( Document( document_id="doc_owned", workspace_id="workspace-org-acme", + organization_id="org-acme", document_name="../roadmap.md", document_type="text/markdown", document_content="# Roadmap\nPhase 10", @@ -2743,6 +2765,7 @@ def test_data_document_webdav_materialization_rejects_empty_document(mock_db): Document( document_id="doc_empty", workspace_id="workspace-org-acme", + organization_id="org-acme", document_name="empty.md", document_type="text/markdown", document_content=" ", @@ -2777,6 +2800,7 @@ def test_data_document_webdav_materialization_rejects_pending_pdf(mock_db): Document( document_id="doc_pending", workspace_id="workspace-org-acme", + organization_id="org-acme", document_name="contract.pdf", document_type="pdf", document_content="JVBERi0xLjcK", # base64 %PDF-1.7\n @@ -2807,6 +2831,7 @@ def test_data_pdf_dom_recognition_intent_rejects_non_pdf_document(mock_db): Document( document_id="doc_text", workspace_id="workspace-org-acme", + organization_id="org-acme", document_name="notes.md", document_type="text/markdown", document_content="# Notes", @@ -2830,6 +2855,7 @@ def test_data_pdf_dom_recognition_intent_rejects_non_pdf_document(mock_db): Document( document_id="doc_pdf", workspace_id="workspace-org-acme", + organization_id="org-acme", document_name="contract.pdf", document_type="pdf", document_content="JVBERi0xLjcK", @@ -2855,6 +2881,7 @@ def test_data_pdf_dom_recognition_intent_rejects_invalid_stored_payload(mock_db) Document( document_id="doc_invalid_pdf", workspace_id="workspace-org-acme", + organization_id="org-acme", document_name="contract.pdf", document_type="pdf", document_content=base64.b64encode(b"not a PDF").decode("ascii"), diff --git a/backend/tests/test_data_api_document_org_isolation.py b/backend/tests/test_data_api_document_org_isolation.py new file mode 100644 index 000000000..8fc4456f3 --- /dev/null +++ b/backend/tests/test_data_api_document_org_isolation.py @@ -0,0 +1,66 @@ +import pytest + +from db.models import Document +from tests.test_data_api import ( + _now, + _restore_overrides, + _signed_session_token, + _valid_session_payload, + _with_signed_auth, + mock_db as _data_api_mock_db, +) + + +@pytest.fixture +def mock_db(): + """Reuse the data-quality API session fixture in this focused module.""" + return _data_api_mock_db.__wrapped__() + + +def test_data_quality_surface_excludes_cross_org_document_in_same_workspace(mock_db): + mock_db.documents.extend( + [ + Document( + document_id="doc_owned_same_workspace", + workspace_id="workspace-org-acme", + organization_id="org-acme", + document_name="owned.md", + document_type="text/markdown", + document_content="owned", + document_status="uploaded", + created_at=_now(), + ), + Document( + document_id="doc_rival_same_workspace", + workspace_id="workspace-org-acme", + organization_id="org-rival", + document_name="rival.md", + document_type="text/markdown", + document_content="rival", + document_status="uploaded", + created_at=_now(), + ), + ] + ) + token = _signed_session_token(_valid_session_payload()) + client, previous_secret, original_overrides = _with_signed_auth(mock_db, token) + try: + response = client.get("/api/data/quality-surface") + finally: + client.close() + _restore_overrides(previous_secret, original_overrides) + + assert response.status_code == 200, response.text + data = response.json() + document_repository = next( + repository + for repository in data["repositories"] + if repository["repository_type"] == "document_repository" + ) + assert document_repository["object_count"] == 1 + document_asset_keys = { + asset["asset_key"] + for asset in data["repository_assets"] + if asset["asset_type"] == "workspace_document" + } + assert document_asset_keys == {"doc_owned_same_workspace"} diff --git a/backend/tests/test_prompts_api.py b/backend/tests/test_prompts_api.py index 2480031f4..3bc388a0e 100644 --- a/backend/tests/test_prompts_api.py +++ b/backend/tests/test_prompts_api.py @@ -187,13 +187,22 @@ def test_prompt_crud(auth_client): data = resp.json() assert data["title"] == "Test Prompt" assert data["prompt_uid"].startswith("prompt_") + # Identity is the opaque prompt_uid; the sequential DB surrogate must never + # be exposed on the API surface (see CLAUDE.md "never expose sequential + # database ids"). + assert "id" not in data assert mock_session.items[0].organization_id == "org-acme" assert mock_session.items[0].workspace_id == "workspace-org-acme" # List resp = auth_client.get("/api/prompts") assert resp.status_code == 200 - assert len(resp.json()) == 1 + listed = resp.json() + assert len(listed) == 1 + assert all( + "id" not in item and item["prompt_uid"].startswith("prompt_") + for item in listed + ) def test_prompt_list_scopes_shared_prompts_to_current_workspace(auth_client): From 6b8a5af5bd6630b5076eb1f5c1ac15ca7d2e1b2f Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Tue, 4 Aug 2026 18:14:04 +0900 Subject: [PATCH 02/19] ci: revalidate security hardening on current head From afe981be6b1e015c2e1699df1b0b8fdb0d80e05e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 21:08:58 +0900 Subject: [PATCH 03/19] test(carddav): reject ambiguous nested path encoding --- ...t_carddav_encoded_path_canonicalization.py | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/backend/tests/test_carddav_encoded_path_canonicalization.py b/backend/tests/test_carddav_encoded_path_canonicalization.py index 17b8625e3..ce70a95b2 100644 --- a/backend/tests/test_carddav_encoded_path_canonicalization.py +++ b/backend/tests/test_carddav_encoded_path_canonicalization.py @@ -1,13 +1,39 @@ """Regression tests for canonical CardDAV TXT context-path execution.""" +import pytest + from services.carddav_discovery import _txt_context_path def test_fully_encoded_leading_slash_is_canonicalized() -> None: - """Execute the same decoded representation that passed validation.""" + """Execute the same singly decoded representation that passed validation.""" assert _txt_context_path(["path=%2Fsafe"]) == "/safe" def test_percent_encoded_unicode_path_is_canonicalized() -> None: - """Preserve a safe Unicode path after bounded decoding.""" + """Preserve a safe Unicode path after one percent-decoding pass.""" assert _txt_context_path(["path=/%EC%A3%BC%EC%86%8C%EB%A1%9D"]) == "/주소록" + + +@pytest.mark.parametrize( + "txt_path", + [ + "/literal%252Fsegment", + "/%252e%252e%252fescape", + "/safe%2525control", + ], +) +def test_nested_percent_encoding_is_rejected(txt_path: str) -> None: + """Reject values whose meaning would change under a second decode pass.""" + assert _txt_context_path([f"path={txt_path}"]) is None + + +@pytest.mark.parametrize("txt_path", ["/safe%", "/safe%2", "/safe%2G"]) +def test_malformed_percent_triplets_are_rejected(txt_path: str) -> None: + """Reject malformed URI percent encodings instead of forwarding ambiguity.""" + assert _txt_context_path([f"path={txt_path}"]) is None + + +def test_encoded_literal_percent_is_preserved_after_one_decode() -> None: + """Allow a single encoded percent when it does not form another triplet.""" + assert _txt_context_path(["path=/discount-100%25"]) == "/discount-100%" From 071573365bb19c990d76caaa9d01edfa05ee4a39 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 21:11:15 +0900 Subject: [PATCH 04/19] test(carddav): reject invalid UTF-8 path octets --- backend/tests/test_carddav_encoded_path_canonicalization.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/backend/tests/test_carddav_encoded_path_canonicalization.py b/backend/tests/test_carddav_encoded_path_canonicalization.py index ce70a95b2..025f276bc 100644 --- a/backend/tests/test_carddav_encoded_path_canonicalization.py +++ b/backend/tests/test_carddav_encoded_path_canonicalization.py @@ -34,6 +34,11 @@ def test_malformed_percent_triplets_are_rejected(txt_path: str) -> None: assert _txt_context_path([f"path={txt_path}"]) is None +def test_invalid_utf8_percent_octet_is_rejected() -> None: + """Reject invalid UTF-8 rather than accepting a replacement-character path.""" + assert _txt_context_path(["path=/safe%FF"]) is None + + def test_encoded_literal_percent_is_preserved_after_one_decode() -> None: """Allow a single encoded percent when it does not form another triplet.""" assert _txt_context_path(["path=/discount-100%25"]) == "/discount-100%" From 952f359ffd089b8d2fbf270ee12fcd4dfbab9e82 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 21:12:02 +0900 Subject: [PATCH 05/19] fix(carddav): enforce single-decode TXT path semantics --- backend/services/carddav_discovery.py | 28 +++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/backend/services/carddav_discovery.py b/backend/services/carddav_discovery.py index c1a475a18..0629d5dea 100644 --- a/backend/services/carddav_discovery.py +++ b/backend/services/carddav_discovery.py @@ -26,6 +26,7 @@ import ipaddress import logging +import re import socket from dataclasses import dataclass from typing import Any, Awaitable, Callable @@ -44,7 +45,8 @@ TxtResolver = Callable[[str], list[str]] HttpClientFactory = Callable[[], Any] -_MAX_CONTEXT_PATH_DECODE_ROUNDS = 5 +_MALFORMED_PERCENT_TRIPLET = re.compile(r"%(?![0-9A-Fa-f]{2})") +_REMAINING_PERCENT_TRIPLET = re.compile(r"%[0-9A-Fa-f]{2}") @dataclass(frozen=True) @@ -220,25 +222,23 @@ def _default_txt_resolver(name: str) -> list[str]: def _txt_context_path(records: list[str]) -> str | None: - """Extract and validate the RFC 6764 Section 6 TXT ``path`` hint.""" + """Extract and validate a singly decoded RFC 6764 TXT ``path`` hint.""" for record in records: for part in record.split(";"): key, _, value = part.strip().partition("=") if key.strip().lower() != "path": continue path = value.strip() - decoded_path = path - for _ in range(_MAX_CONTEXT_PATH_DECODE_ROUNDS): - next_path = unquote(decoded_path) - if next_path == decoded_path: - break - decoded_path = next_path - else: - # Reject values that still change after the decode budget. This - # keeps over-encoded traversal payloads from hiding another - # interpretation beyond the validation boundary. - if unquote(decoded_path) != decoded_path: - continue + if _MALFORMED_PERCENT_TRIPLET.search(path): + continue + try: + decoded_path = unquote(path, errors="strict") + except UnicodeDecodeError: + continue + if _REMAINING_PERCENT_TRIPLET.search(decoded_path): + # A second decode would change the request target. Reject the + # ambiguous value instead of inventing a recursive decode count. + continue if ( decoded_path.startswith("/") and "://" not in decoded_path From d5e4286f48b9828f4e0d3116ea3305dd0b7f1791 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 21:12:39 +0900 Subject: [PATCH 06/19] docs(carddav): record TXT path decode boundary --- .../carddav-txt-path-canonicalization.md | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 docs/doctoring/carddav-txt-path-canonicalization.md diff --git a/docs/doctoring/carddav-txt-path-canonicalization.md b/docs/doctoring/carddav-txt-path-canonicalization.md new file mode 100644 index 000000000..798efd0aa --- /dev/null +++ b/docs/doctoring/carddav-txt-path-canonicalization.md @@ -0,0 +1,43 @@ +# CardDAV TXT path canonicalization + +## Scope + +Naruon consumes the optional `path` key advertised by a secure `_carddavs._tcp` TXT record during CardDAV discovery. The value becomes part of an outbound HTTPS request target, so validation and execution must use one unambiguous representation. + +## Decision + +The parser applies the following fail-closed contract: + +1. Reject malformed percent triplets before decoding. +2. Percent-decode the TXT value exactly once with strict UTF-8 handling. +3. Reject the value when a valid percent triplet remains after that pass, because a second decoder could observe a different request target. +4. Reject traversal segments, backslashes, query or fragment delimiters, absolute-URI syntax, and Unicode control characters. +5. Return and execute the same validated representation. + +This replaces the previous arbitrary five-round recursive decoding budget. Recursive decoding changed legitimate literal-percent paths and left the security meaning dependent on a chosen iteration count. A single-pass contract follows the URI processing rule that a component must not be percent-decoded more than once, while rejecting nested encodings that would remain ambiguous at another HTTP or provider boundary. + +An encoded literal percent remains supported when its decoded form does not begin another percent triplet. Invalid UTF-8 is rejected rather than normalized through the Unicode replacement character. + +## Product boundary + +This decision protects CardDAV auto-discovery only. It does not grant authorization to arbitrary paths, weaken the existing HTTPS/global-address SSRF controls, or treat TXT records as trusted credentials. Provider account authorization and resource ownership remain separate checks. + +## Verification + +The focused regression suite covers: + +- a singly encoded leading slash; +- Korean UTF-8 path text; +- nested encoded slash and traversal forms; +- nested encoded percent forms; +- incomplete and non-hex percent triplets; +- invalid UTF-8 octets; +- a safe encoded literal percent. + +## References + +Berners-Lee, T., Fielding, R., & Masinter, L. (2005). *Uniform resource identifier (URI): Generic syntax* (RFC 3986). RFC Editor. https://doi.org/10.17487/RFC3986 + +Daboo, C. (2012). *Locating CalDAV and CardDAV services* (RFC 6764). RFC Editor. https://doi.org/10.17487/RFC6764 + +MITRE. (2025). *CWE-174: Double decoding of the same data*. Common Weakness Enumeration. https://cwe.mitre.org/data/definitions/174.html From 4c8776e9c3ff7052401b11b809414ab22e052ec2 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 15 Aug 2026 21:20:16 +0900 Subject: [PATCH 07/19] refactor(carddav): leave document isolation to canonical security lane --- backend/api/data.py | 16 +---- backend/tests/test_data_api.py | 29 +------- .../test_data_api_document_org_isolation.py | 66 ------------------- 3 files changed, 2 insertions(+), 109 deletions(-) delete mode 100644 backend/tests/test_data_api_document_org_isolation.py diff --git a/backend/api/data.py b/backend/api/data.py index 83b9fa6d5..dccd85890 100644 --- a/backend/api/data.py +++ b/backend/api/data.py @@ -2489,16 +2489,10 @@ async def _get_workspace_document( auth_context: AuthContext, document_id: str, ) -> Document: - organization_filter = ( - Document.organization_id == auth_context.organization_id - if auth_context.organization_id is not None - else Document.organization_id.is_(None) - ) result = await db.execute( select(Document).where( Document.document_id == document_id, Document.workspace_id == auth_context.workspace_id, - organization_filter, ) ) document = result.scalar_one_or_none() @@ -3934,18 +3928,10 @@ async def get_data_quality_surface( ProjectFolder.folder_uid.asc(), ), ) - document_organization_filter = ( - Document.organization_id == auth_context.organization_id - if auth_context.organization_id is not None - else Document.organization_id.is_(None) - ) documents = await _scoped_rows( db, select(Document) - .where( - Document.workspace_id == auth_context.workspace_id, - document_organization_filter, - ) + .where(Document.workspace_id == auth_context.workspace_id) .order_by(Document.created_at.desc(), Document.document_id.asc()) .limit(8), ) diff --git a/backend/tests/test_data_api.py b/backend/tests/test_data_api.py index eb67f894a..cd0b7bf37 100644 --- a/backend/tests/test_data_api.py +++ b/backend/tests/test_data_api.py @@ -101,27 +101,11 @@ async def execute(self, query): ), None, ) - organization_ids = [ - value - for key, value in params.items() - if key.startswith("organization_id") - ] - organization_is_null = ( - "workspace_documents.organization_id is null" in rendered_query_lower - ) rows = [ document for document in self.documents if (document_id is None or document.document_id == document_id) and (workspace_id is None or document.workspace_id == workspace_id) - and ( - ( - organization_ids - and document.organization_id == organization_ids[0] - ) - or (organization_is_null and document.organization_id is None) - or (not organization_ids and not organization_is_null) - ) ] if "order by" in rendered_query_lower: return MockResult(rows) @@ -2500,7 +2484,6 @@ def test_data_quality_surface_includes_workspace_document_assets(mock_db): Document( document_id="doc_owned", workspace_id="workspace-org-acme", - organization_id="org-acme", document_name="roadmap.md", document_type="text/markdown", document_content="# Roadmap", @@ -2510,7 +2493,6 @@ def test_data_quality_surface_includes_workspace_document_assets(mock_db): Document( document_id="doc_rival", workspace_id="workspace-rival", - organization_id="org-rival", document_name="rival.md", document_type="text/markdown", document_content="rival", @@ -2604,7 +2586,6 @@ def test_data_document_actions_are_workspace_scoped_and_intent_only(mock_db): document = Document( document_id="doc_owned", workspace_id="workspace-org-acme", - organization_id="org-acme", document_name="source.hwp", document_type="application/x-hwp", document_content="opaque hwp extraction placeholder", @@ -2613,8 +2594,7 @@ def test_data_document_actions_are_workspace_scoped_and_intent_only(mock_db): ) rival_document = Document( document_id="doc_rival", - workspace_id="workspace-org-acme", - organization_id="org-rival", + workspace_id="workspace-rival", document_name="rival.md", document_type="text/markdown", document_content="rival", @@ -2658,7 +2638,6 @@ def test_data_document_actions_are_workspace_scoped_and_intent_only(mock_db): assert rival_response.status_code == 404 assert "doc_rival" not in rival_response.text - assert rival_document.document_status == "uploaded" def test_data_document_webdav_materialization_executes_source_backed_write( @@ -2669,7 +2648,6 @@ def test_data_document_webdav_materialization_executes_source_backed_write( Document( document_id="doc_owned", workspace_id="workspace-org-acme", - organization_id="org-acme", document_name="../roadmap.md", document_type="text/markdown", document_content="# Roadmap\nPhase 10", @@ -2765,7 +2743,6 @@ def test_data_document_webdav_materialization_rejects_empty_document(mock_db): Document( document_id="doc_empty", workspace_id="workspace-org-acme", - organization_id="org-acme", document_name="empty.md", document_type="text/markdown", document_content=" ", @@ -2800,7 +2777,6 @@ def test_data_document_webdav_materialization_rejects_pending_pdf(mock_db): Document( document_id="doc_pending", workspace_id="workspace-org-acme", - organization_id="org-acme", document_name="contract.pdf", document_type="pdf", document_content="JVBERi0xLjcK", # base64 %PDF-1.7\n @@ -2831,7 +2807,6 @@ def test_data_pdf_dom_recognition_intent_rejects_non_pdf_document(mock_db): Document( document_id="doc_text", workspace_id="workspace-org-acme", - organization_id="org-acme", document_name="notes.md", document_type="text/markdown", document_content="# Notes", @@ -2855,7 +2830,6 @@ def test_data_pdf_dom_recognition_intent_rejects_non_pdf_document(mock_db): Document( document_id="doc_pdf", workspace_id="workspace-org-acme", - organization_id="org-acme", document_name="contract.pdf", document_type="pdf", document_content="JVBERi0xLjcK", @@ -2881,7 +2855,6 @@ def test_data_pdf_dom_recognition_intent_rejects_invalid_stored_payload(mock_db) Document( document_id="doc_invalid_pdf", workspace_id="workspace-org-acme", - organization_id="org-acme", document_name="contract.pdf", document_type="pdf", document_content=base64.b64encode(b"not a PDF").decode("ascii"), diff --git a/backend/tests/test_data_api_document_org_isolation.py b/backend/tests/test_data_api_document_org_isolation.py deleted file mode 100644 index 8fc4456f3..000000000 --- a/backend/tests/test_data_api_document_org_isolation.py +++ /dev/null @@ -1,66 +0,0 @@ -import pytest - -from db.models import Document -from tests.test_data_api import ( - _now, - _restore_overrides, - _signed_session_token, - _valid_session_payload, - _with_signed_auth, - mock_db as _data_api_mock_db, -) - - -@pytest.fixture -def mock_db(): - """Reuse the data-quality API session fixture in this focused module.""" - return _data_api_mock_db.__wrapped__() - - -def test_data_quality_surface_excludes_cross_org_document_in_same_workspace(mock_db): - mock_db.documents.extend( - [ - Document( - document_id="doc_owned_same_workspace", - workspace_id="workspace-org-acme", - organization_id="org-acme", - document_name="owned.md", - document_type="text/markdown", - document_content="owned", - document_status="uploaded", - created_at=_now(), - ), - Document( - document_id="doc_rival_same_workspace", - workspace_id="workspace-org-acme", - organization_id="org-rival", - document_name="rival.md", - document_type="text/markdown", - document_content="rival", - document_status="uploaded", - created_at=_now(), - ), - ] - ) - token = _signed_session_token(_valid_session_payload()) - client, previous_secret, original_overrides = _with_signed_auth(mock_db, token) - try: - response = client.get("/api/data/quality-surface") - finally: - client.close() - _restore_overrides(previous_secret, original_overrides) - - assert response.status_code == 200, response.text - data = response.json() - document_repository = next( - repository - for repository in data["repositories"] - if repository["repository_type"] == "document_repository" - ) - assert document_repository["object_count"] == 1 - document_asset_keys = { - asset["asset_key"] - for asset in data["repository_assets"] - if asset["asset_type"] == "workspace_document" - } - assert document_asset_keys == {"doc_owned_same_workspace"} From d235e7c7a5c5fbeadc2a47f8e0ffd67b5f4c4d90 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 16 Aug 2026 08:49:54 +0900 Subject: [PATCH 08/19] test(auth): prove trusted OIDC admin sessions --- backend/tests/test_auth_oidc_admin_roles.py | 92 +++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 backend/tests/test_auth_oidc_admin_roles.py diff --git a/backend/tests/test_auth_oidc_admin_roles.py b/backend/tests/test_auth_oidc_admin_roles.py new file mode 100644 index 000000000..e36e3eb17 --- /dev/null +++ b/backend/tests/test_auth_oidc_admin_roles.py @@ -0,0 +1,92 @@ +"""Focused contracts for high-privilege OIDC session authority.""" + +import time + +import pytest +from fastapi import HTTPException + +from api import auth as auth_module +from core.config import settings + + +ADMIN_ROLES = ( + "system_admin", + "platform_admin", + "tenant_admin", + "organization_admin", +) + + +def _oidc_payload(role: str) -> dict[str, object]: + """Build a short-lived payload from the configured authoritative OIDC issuer.""" + return { + "iss": "https://login.example.test/realms/naruon", + "aud": "naruon-api", + "sub": "alice", + "role": role, + "org": "org-acme", + "groups": ["group-1"], + "workspace": "workspace-org-acme", + "exp": int(time.time()) + 300, + } + + +def _hmac_payload(role: str) -> dict[str, object]: + """Build compatibility-session metadata without granting membership authority.""" + return { + "ver": 1, + "iss": auth_module.SESSION_ISSUER, + "aud": auth_module.SESSION_AUDIENCE, + "sub": "alice", + "role": role, + "org": "org-acme", + "groups": ["group-1"], + "workspace": "workspace-org-acme", + "exp": int(time.time()) + 300, + } + + +@pytest.mark.parametrize("admin_role", ADMIN_ROLES) +def test_trusted_oidc_session_can_supply_admin_role(monkeypatch, admin_role: str) -> None: + """A configured JWKS-backed IdP remains usable for authorized administrators.""" + previous_issuer_url = settings.OIDC_ISSUER_URL + previous_client_id = settings.OIDC_CLIENT_ID + settings.OIDC_ISSUER_URL = "https://login.example.test/realms/naruon" + settings.OIDC_CLIENT_ID = "naruon-api" + payload = _oidc_payload(admin_role) + + monkeypatch.setattr(auth_module, "jwks_client", object()) + monkeypatch.setattr( + auth_module, + "_decode_cached_oidc_session_payload", + lambda _token: payload, + ) + + try: + verified_payload, verifier = auth_module._verify_signed_session_token( + "trusted-idp-token" + ) + context = auth_module._auth_context_from_session_payload( + verified_payload, verifier + ) + finally: + settings.OIDC_ISSUER_URL = previous_issuer_url + settings.OIDC_CLIENT_ID = previous_client_id + + assert verifier == "oidc" + assert context.role == admin_role + assert context.organization_id == "org-acme" + assert context.workspace_id == "workspace-org-acme" + assert context.session_verifier == "oidc" + + +@pytest.mark.parametrize("admin_role", ADMIN_ROLES) +def test_hmac_compatibility_session_cannot_supply_admin_role(admin_role: str) -> None: + """HMAC compatibility credentials never become authoritative admin membership.""" + with pytest.raises(HTTPException) as exc: + auth_module._auth_context_from_session_payload( + _hmac_payload(admin_role), "hmac" + ) + + assert exc.value.status_code == 401 + assert exc.value.detail == "Authentication required" From eefca3d43461efc0346f38ffdb4c1d903b4d4148 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 16 Aug 2026 08:51:23 +0900 Subject: [PATCH 09/19] fix(auth): trust admin roles only from verified OIDC --- backend/api/auth.py | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/backend/api/auth.py b/backend/api/auth.py index bd188351c..2c56c92e8 100644 --- a/backend/api/auth.py +++ b/backend/api/auth.py @@ -196,11 +196,12 @@ def build_auth_context(authorization: str | None = None) -> AuthContext: """ Build runtime identity from verified signed session material. - Client-supplied identity metadata is not authentication material. Only a - bearer token signed by the configured control-plane HMAC secret can supply - identity, role, organization, group, and workspace claims in the runtime - dependency path. Endpoint tests that need fixture identities must continue to - use explicit FastAPI dependency overrides. + Client-supplied identity metadata is not authentication material. Runtime + identity comes only from a bearer token verified by the configured OIDC/JWKS + provider or the control-plane HMAC compatibility secret. Admin roles are + accepted only from the verified OIDC authority; HMAC compatibility sessions + cannot assert admin membership. Endpoint tests that need fixture identities + must continue to use explicit FastAPI dependency overrides. """ payload, session_verifier = _verify_signed_session_payload(authorization) return _auth_context_from_session_payload(payload, session_verifier) @@ -374,7 +375,6 @@ def _verify_signed_session_token(token: str) -> tuple[dict[str, Any], SessionVer raise _authentication_error() try: payload = _decode_cached_oidc_session_payload(token) - _reject_signed_session_admin_payload(payload) return payload, "oidc" except Exception: raise _authentication_error() from None @@ -403,16 +403,15 @@ def _verify_signed_session_token(token: str) -> tuple[dict[str, Any], SessionVer raise _authentication_error() if not isinstance(payload, dict): raise _authentication_error() - _reject_signed_session_admin_payload(payload) + _reject_hmac_admin_payload(payload) return payload, "hmac" -def _reject_signed_session_admin_payload(payload: dict[str, Any]) -> None: +def _reject_hmac_admin_payload(payload: dict[str, Any]) -> None: + """Reject admin membership claims from the HMAC compatibility credential.""" role_claim = payload.get("role") if not isinstance(role_claim, str): raise _authentication_error() - # Admin roles require explicit server-side assignment, not externally - # supplied HMAC or enterprise OIDC session claims. if role_claim in ADMIN_ROLES: raise _authentication_error() @@ -512,7 +511,7 @@ def _auth_context_from_session_payload( if role_value not in ALLOWED_ROLES: raise _authentication_error() role = cast(RoleName, role_value) - if role in TENANT_ADMIN_ROLES and session_verifier not in ("server", "override"): + if role in ADMIN_ROLES and session_verifier not in ("oidc", "server", "override"): raise _authentication_error() organization_id = _optional_string_claim(payload, "org") if organization_id is None: @@ -542,4 +541,4 @@ async def get_current_workspace_id( async def get_current_user_role( auth_context: AuthContext = Depends(get_auth_context), ) -> str: - return auth_context.role + return auth_context.role \ No newline at end of file From 59299a7d2aa8324462ec304350ef0e454d853864 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 16 Aug 2026 13:48:46 +0900 Subject: [PATCH 10/19] test(security): prove system admin source-policy boundary --- ...test_security_source_policy_admin_roles.py | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 backend/tests/test_security_source_policy_admin_roles.py diff --git a/backend/tests/test_security_source_policy_admin_roles.py b/backend/tests/test_security_source_policy_admin_roles.py new file mode 100644 index 000000000..4382065fa --- /dev/null +++ b/backend/tests/test_security_source_policy_admin_roles.py @@ -0,0 +1,53 @@ +"""Regression tests for system-level roles in source access policies.""" + +import pytest + +from api.auth import AuthContext +from api.security import _access_request, _source_policy +from services.access_policy import evaluate_access + + +@pytest.mark.parametrize("role", ["system_admin", "platform_admin"]) +def test_source_policy_allows_system_admin_roles_in_current_organization(role): + auth_context = AuthContext( + user_id="global-admin", + role=role, + organization_id="org-acme", + group_ids=(), + workspace_id="workspace-org-acme", + ) + policy = _source_policy( + auth_context, + owner_id="source-owner", + organization_id="org-acme", + workspace_id="workspace-org-acme", + writeback_enabled=False, + ) + + decision = evaluate_access(_access_request(auth_context), policy) + + assert decision.allowed is True + assert decision.reason == "allowed" + + +@pytest.mark.parametrize("role", ["system_admin", "platform_admin"]) +def test_source_policy_keeps_system_admin_roles_inside_current_organization(role): + auth_context = AuthContext( + user_id="global-admin", + role=role, + organization_id="org-acme", + group_ids=(), + workspace_id="workspace-org-acme", + ) + policy = _source_policy( + auth_context, + owner_id="source-owner", + organization_id="org-rival", + workspace_id="workspace-org-acme", + writeback_enabled=False, + ) + + decision = evaluate_access(_access_request(auth_context), policy) + + assert decision.allowed is False + assert decision.reason == "organization_denied" From cc90e1edd9c9a7e1922f6248921ec4d2ed919cfd Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 16 Aug 2026 13:50:53 +0900 Subject: [PATCH 11/19] fix(security): admit system admins inside source tenant boundary --- backend/api/security.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/backend/api/security.py b/backend/api/security.py index 7925284b3..ce5b86416 100644 --- a/backend/api/security.py +++ b/backend/api/security.py @@ -269,6 +269,7 @@ def _source_policy( workspace_id: str, writeback_enabled: bool, ) -> ResourcePolicy: + """Build a source policy that preserves tenant scope for every admin tier.""" delegated_user_ids: tuple[str, ...] = ( (auth_context.user_id,) if ( @@ -281,7 +282,14 @@ def _source_policy( return ResourcePolicy( owner_id=owner_id, organization_id=organization_id, - permitted_roles=("tenant_admin", "organization_admin", "group_admin", "member"), + permitted_roles=( + "system_admin", + "platform_admin", + "tenant_admin", + "organization_admin", + "group_admin", + "member", + ), permitted_group_ids=auth_context.group_ids, data_region=settings.DATA_REGION, required_consent_scopes=required_consent, From 4cae8cd26a37ac51573ab64301da86d74e66af74 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 00:36:20 +0900 Subject: [PATCH 12/19] test(prompts): pin opaque response identifier schema --- .../test_prompt_response_naming_contract.py | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 backend/tests/test_prompt_response_naming_contract.py diff --git a/backend/tests/test_prompt_response_naming_contract.py b/backend/tests/test_prompt_response_naming_contract.py new file mode 100644 index 000000000..7bb7afa96 --- /dev/null +++ b/backend/tests/test_prompt_response_naming_contract.py @@ -0,0 +1,47 @@ +"""Naming and public-identifier contract for prompt responses.""" + +from __future__ import annotations + +import datetime +from types import SimpleNamespace + +from api.prompts import PromptResponse + + +def _prompt_record() -> SimpleNamespace: + """Return an ORM-shaped prompt record that still contains its private row id.""" + now = datetime.datetime(2026, 9, 1, tzinfo=datetime.timezone.utc) + return SimpleNamespace( + id=17, + prompt_uid="prompt-example", + title="Example", + description=None, + content="Summarize {{email}}", + is_shared=False, + created_by="user-example", + created_at=now, + updated_at=now, + ) + + +def test_prompt_response_uses_only_opaque_public_identifier() -> None: + """Sequential database identity must not enter the owned API response model.""" + assert "prompt_uid" in PromptResponse.model_fields + assert "id" not in PromptResponse.model_fields + assert "prompt_record_id" not in PromptResponse.model_fields + + prompt_response = PromptResponse.model_validate(_prompt_record()) + serialized_response = prompt_response.model_dump() + + assert serialized_response["prompt_uid"] == "prompt-example" + assert "id" not in serialized_response + assert "prompt_record_id" not in serialized_response + + +def test_prompt_response_json_schema_does_not_advertise_sequential_database_id() -> None: + """FastAPI's response schema must advertise the opaque UID as the sole identifier.""" + response_properties = PromptResponse.model_json_schema()["properties"] + + assert "prompt_uid" in response_properties + assert "id" not in response_properties + assert "prompt_record_id" not in response_properties From 2533f3a5540a48045dddff54b4ca3f8a07fa101b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 00:36:40 +0900 Subject: [PATCH 13/19] docs(prompts): record opaque response identifier contract --- .../prompt-response-semantic-identifiers.md | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 docs/doctoring/prompt-response-semantic-identifiers.md diff --git a/docs/doctoring/prompt-response-semantic-identifiers.md b/docs/doctoring/prompt-response-semantic-identifiers.md new file mode 100644 index 000000000..754f33dec --- /dev/null +++ b/docs/doctoring/prompt-response-semantic-identifiers.md @@ -0,0 +1,40 @@ +# Prompt response semantic identifiers + +## Decision + +Naruon's Prompt Catalog bounded context already has an opaque public identifier, `prompt_uid`, and a separate sequential database row identity, `PromptTemplate.id`. The public `/api/prompts` list and create responses expose only `prompt_uid`; the sequential row identity is not part of `PromptResponse`. + +A naming-only approach that aliases the sequential database identity under a more descriptive internal name is insufficient because it still exports an unnecessary enumerable identifier. The canonical public contract therefore removes both bare `id` and any renamed row-id alias instead of merely recasing or relabeling them. + +| Previous public field | Current public field | Meaning | +| --- | --- | --- | +| `id` (sequential database row id) | removed | private persistence identity | +| `prompt_uid` | `prompt_uid` | opaque public prompt identity | + +## DDD, security, and compatibility boundary + +- **Bounded context:** Prompt Catalog. +- **Entity:** persisted prompt-template record. +- **Public identity:** `prompt_uid` is the sole prompt identifier in list/create response contracts. +- **Persistence identity:** `PromptTemplate.id` remains private to persistence and may still exist on ORM records; Pydantic `from_attributes=True` ignores it because it is not a response field. +- **Authorization invariant:** organization/workspace ownership filters on `list_prompts` and creation ownership assignments remain unchanged. Opaque identifiers are defense-in-depth and do not replace object-level authorization. +- **Public contract:** the redundant sequential `id` response property is intentionally absent. Consumers use the already-present `prompt_uid` for prompt identity. +- **Persistence:** unchanged. No database migration, backfill, index change, new lock, UPSERT change, partition change, or read/write split is introduced. + +OWASP API Security Top 10 API1:2023 notes that object identifiers, including sequential integers, are common BOLA attack inputs and recommends random, unpredictable record identifiers together with proper object-level authorization. Naruon already has the unpredictable `prompt_uid`, so retaining a second sequential public identifier has no buyer-visible product benefit and widens the identifier surface unnecessarily. + +## Verification contract + +`backend/tests/test_prompts_api.py` verifies that create/list responses omit `id` while returning opaque `prompt_uid`. `backend/tests/test_prompt_response_naming_contract.py` additionally validates an ORM-shaped record that still contains private `id=17` and requires both runtime serialization and generated JSON schema to omit `id` and `prompt_record_id` while retaining `prompt_uid`. Exact-head repository CI, security workflows, review threads, and branch protection remain authoritative merge evidence. + +## Research traceability + +Empirical software-engineering research supports treating identifier names as program-comprehension artifacts rather than cosmetic style. Feitelson et al. found that explicitly choosing the concepts represented in a name improved judged name quality and tended to produce names containing more concepts; later replication work corroborated that model and found that merely making names longer was not equivalent to selecting meaningful concepts. Here the stronger domain conclusion is that the public concept is already fully represented by `prompt_uid`; a second database-row identifier should not be renamed and exported when it is not part of the public domain language. + +### References + +Alpern, R., Lazer, I., Tzachor, I., Hakim, H., Weissbuch, S., & Feitelson, D. G. (2024). *Reproducing, extending, and analyzing naming experiments*. arXiv. https://doi.org/10.48550/arXiv.2402.10022 + +Feitelson, D. G., Mizrahi, A., Noy, N., Ben Shabat, A., Eliyahu, O., & Sheffer, R. (2022). How developers choose names. *IEEE Transactions on Software Engineering, 48*(1), 37–52. https://doi.org/10.1109/TSE.2020.2976920 + +OWASP Foundation. (2023). *API1:2023 Broken Object Level Authorization*. OWASP API Security Top 10. https://owasp.org/API-Security/editions/2023/en/0xa1-broken-object-level-authorization/ From 2389f0b5d8e54e6968b5b9b9f71b35e29e11e254 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 01:51:32 +0900 Subject: [PATCH 14/19] test(security): reject implicit orgless admin delegation --- ...test_security_source_policy_admin_roles.py | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/backend/tests/test_security_source_policy_admin_roles.py b/backend/tests/test_security_source_policy_admin_roles.py index 4382065fa..67b322f0b 100644 --- a/backend/tests/test_security_source_policy_admin_roles.py +++ b/backend/tests/test_security_source_policy_admin_roles.py @@ -51,3 +51,27 @@ def test_source_policy_keeps_system_admin_roles_inside_current_organization(role assert decision.allowed is False assert decision.reason == "organization_denied" + + +@pytest.mark.parametrize("role", ["system_admin", "platform_admin"]) +def test_source_policy_does_not_delegate_orgless_legacy_sources(role): + """Missing organization identity must not become an implicit admin delegation.""" + auth_context = AuthContext( + user_id="global-admin", + role=role, + organization_id=None, + group_ids=(), + workspace_id="workspace-legacy", + ) + policy = _source_policy( + auth_context, + owner_id="source-owner", + organization_id=None, + workspace_id="workspace-legacy", + writeback_enabled=False, + ) + + decision = evaluate_access(_access_request(auth_context), policy) + + assert decision.allowed is False + assert decision.reason == "ownership_denied" From 5eab848690d942474997a6e62f6ab62aa50dfc00 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 01:54:38 +0900 Subject: [PATCH 15/19] fix(security): require concrete tenant for admin delegation --- backend/api/security.py | 1 + 1 file changed, 1 insertion(+) diff --git a/backend/api/security.py b/backend/api/security.py index ce5b86416..801c98880 100644 --- a/backend/api/security.py +++ b/backend/api/security.py @@ -274,6 +274,7 @@ def _source_policy( (auth_context.user_id,) if ( is_admin_role(auth_context.role) + and organization_id is not None and organization_id == auth_context.organization_id ) else () From aba2a03f3ca87914fcf1ca1c751b173097852bca Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 01:56:16 +0900 Subject: [PATCH 16/19] test(carddav): remove duplicate asyncio marker --- backend/tests/test_carddav_discovery.py | 1 - 1 file changed, 1 deletion(-) diff --git a/backend/tests/test_carddav_discovery.py b/backend/tests/test_carddav_discovery.py index 50e483fa9..8656b6e6d 100644 --- a/backend/tests/test_carddav_discovery.py +++ b/backend/tests/test_carddav_discovery.py @@ -163,7 +163,6 @@ def txt_resolver(name): assert result.base_url == "https://dav.example.com/" -@pytest.mark.asyncio @pytest.mark.parametrize( "txt_path", [ From 43467ed10fb3ec6c3f2075a38acf9a934342ca02 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 10:16:49 +0900 Subject: [PATCH 17/19] test(carddav): preserve encoded reserved path identity --- .../test_carddav_encoded_path_canonicalization.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/backend/tests/test_carddav_encoded_path_canonicalization.py b/backend/tests/test_carddav_encoded_path_canonicalization.py index 025f276bc..9325fda89 100644 --- a/backend/tests/test_carddav_encoded_path_canonicalization.py +++ b/backend/tests/test_carddav_encoded_path_canonicalization.py @@ -15,6 +15,21 @@ def test_percent_encoded_unicode_path_is_canonicalized() -> None: assert _txt_context_path(["path=/%EC%A3%BC%EC%86%8C%EB%A1%9D"]) == "/주소록" +@pytest.mark.parametrize( + ("txt_path", "expected_path"), + [ + ("/users/alice%2Fcalendar", "/users/alice%2Fcalendar"), + ("/collections/a%3Bb", "/collections/a%3Bb"), + ], +) +def test_encoded_reserved_path_characters_preserve_wire_identity( + txt_path: str, + expected_path: str, +) -> None: + """Validate reserved escapes without turning them into path delimiters.""" + assert _txt_context_path([f"path={txt_path}"]) == expected_path + + @pytest.mark.parametrize( "txt_path", [ From 882c1dda08276d11bb346774dcc3119f64bc51b9 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 10:17:41 +0900 Subject: [PATCH 18/19] fix(carddav): preserve encoded reserved path characters --- backend/services/carddav_discovery.py | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/backend/services/carddav_discovery.py b/backend/services/carddav_discovery.py index 0629d5dea..28d451846 100644 --- a/backend/services/carddav_discovery.py +++ b/backend/services/carddav_discovery.py @@ -46,7 +46,9 @@ HttpClientFactory = Callable[[], Any] _MALFORMED_PERCENT_TRIPLET = re.compile(r"%(?![0-9A-Fa-f]{2})") +_PERCENT_TRIPLET = re.compile(r"%([0-9A-Fa-f]{2})") _REMAINING_PERCENT_TRIPLET = re.compile(r"%[0-9A-Fa-f]{2}") +_RFC3986_RESERVED_CHARACTERS = frozenset(":/?#[]@!$&'()*+,;=") @dataclass(frozen=True) @@ -221,8 +223,27 @@ def _default_txt_resolver(name: str) -> list[str]: return records +def _execution_path_preserving_reserved_escapes(path: str) -> str: + """Decode safe path octets while retaining encoded RFC 3986 reserved data.""" + + def protect_reserved(match: re.Match[str]) -> str: + octet = int(match.group(1), 16) + character = chr(octet) + if character not in _RFC3986_RESERVED_CHARACTERS: + return match.group(0) + # The context path itself must start with a structural slash. An + # encoded leading slash is therefore canonicalized, while later + # reserved escapes remain data exactly as the provider advertised. + if match.start() == 0 and character == "/": + return match.group(0) + return f"%25{match.group(1).upper()}" + + protected_path = _PERCENT_TRIPLET.sub(protect_reserved, path) + return unquote(protected_path, errors="strict") + + def _txt_context_path(records: list[str]) -> str | None: - """Extract and validate a singly decoded RFC 6764 TXT ``path`` hint.""" + """Validate one decode of an RFC 6764 TXT ``path`` and preserve wire identity.""" for record in records: for part in record.split(";"): key, _, value = part.strip().partition("=") @@ -233,6 +254,7 @@ def _txt_context_path(records: list[str]) -> str | None: continue try: decoded_path = unquote(path, errors="strict") + execution_path = _execution_path_preserving_reserved_escapes(path) except UnicodeDecodeError: continue if _REMAINING_PERCENT_TRIPLET.search(decoded_path): @@ -250,7 +272,7 @@ def _txt_context_path(records: list[str]) -> str | None: ) and all(category(ch) != "Cc" for ch in decoded_path) ): - return decoded_path + return execution_path return None From 80a56bee03dd04c3605ae58c39c86d8d9f74f935 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 10:18:12 +0900 Subject: [PATCH 19/19] docs(carddav): distinguish validation from reserved wire identity --- .../carddav-txt-path-canonicalization.md | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/docs/doctoring/carddav-txt-path-canonicalization.md b/docs/doctoring/carddav-txt-path-canonicalization.md index 798efd0aa..bf5f11fbe 100644 --- a/docs/doctoring/carddav-txt-path-canonicalization.md +++ b/docs/doctoring/carddav-txt-path-canonicalization.md @@ -2,19 +2,20 @@ ## Scope -Naruon consumes the optional `path` key advertised by a secure `_carddavs._tcp` TXT record during CardDAV discovery. The value becomes part of an outbound HTTPS request target, so validation and execution must use one unambiguous representation. +Naruon consumes the optional `path` key advertised by a secure `_carddavs._tcp` TXT record during CardDAV discovery. The value becomes part of an outbound HTTPS request target, so validation must reject ambiguous encodings without changing the provider-advertised identity of reserved path data. ## Decision The parser applies the following fail-closed contract: 1. Reject malformed percent triplets before decoding. -2. Percent-decode the TXT value exactly once with strict UTF-8 handling. -3. Reject the value when a valid percent triplet remains after that pass, because a second decoder could observe a different request target. -4. Reject traversal segments, backslashes, query or fragment delimiters, absolute-URI syntax, and Unicode control characters. -5. Return and execute the same validated representation. +2. Percent-decode the TXT value exactly once with strict UTF-8 handling for security validation. +3. Reject the value when a valid percent triplet remains after that validation pass, because a second decoder could observe a different request target. +4. Reject traversal segments, backslashes, query or fragment delimiters, absolute-URI syntax, and Unicode control characters in the decoded validation representation. +5. Normalize percent-encoded unreserved/non-ASCII text through the existing single-pass UTF-8 path contract, while preserving percent-encoded RFC 3986 reserved characters in the executed path. The sole structural exception is an encoded leading `/`, which is canonicalized to the required leading path delimiter. +6. Uppercase the hexadecimal digits of preserved reserved escapes so equivalent percent encodings have one wire representation. -This replaces the previous arbitrary five-round recursive decoding budget. Recursive decoding changed legitimate literal-percent paths and left the security meaning dependent on a chosen iteration count. A single-pass contract follows the URI processing rule that a component must not be percent-decoded more than once, while rejecting nested encodings that would remain ambiguous at another HTTP or provider boundary. +This replaces the previous arbitrary five-round recursive decoding budget. Recursive decoding changed legitimate literal-percent paths and left the security meaning dependent on a chosen iteration count. The current contract performs one security decode and rejects nested encodings, while retaining the distinction RFC 3986 makes between a reserved character and its percent-encoded octet. For example, `/users/alice%2Fcalendar` remains distinct from `/users/alice/calendar`, and `/collections/a%3Bb` remains distinct from `/collections/a;b` after validation. An encoded literal percent remains supported when its decoded form does not begin another percent triplet. Invalid UTF-8 is rejected rather than normalized through the Unicode replacement character. @@ -28,16 +29,19 @@ The focused regression suite covers: - a singly encoded leading slash; - Korean UTF-8 path text; +- percent-encoded reserved `/` and `;` data whose wire identity must survive validation; - nested encoded slash and traversal forms; - nested encoded percent forms; - incomplete and non-hex percent triplets; - invalid UTF-8 octets; - a safe encoded literal percent. +The RED regression was added first at `43467ed10fb3ec6c3f2075a38acf9a934342ca02`: the previous unconditional `unquote` returned structural `/` and `;` characters for the two reserved-escape cases. The minimal production repair is `882c1dda08276d11bb346774dcc3119f64bc51b9`, which keeps the fully decoded representation for security checks and derives a separate execution representation that protects RFC 3986 reserved escapes. Hosted exact-head execution remains required before the repair is classified GREEN. + ## References Berners-Lee, T., Fielding, R., & Masinter, L. (2005). *Uniform resource identifier (URI): Generic syntax* (RFC 3986). RFC Editor. https://doi.org/10.17487/RFC3986 -Daboo, C. (2012). *Locating CalDAV and CardDAV services* (RFC 6764). RFC Editor. https://doi.org/10.17487/RFC6764 +Daboo, C. (2013). *Locating services for calendaring extensions to WebDAV (CalDAV) and vCard extensions to WebDAV (CardDAV)* (RFC 6764). RFC Editor. https://doi.org/10.17487/RFC6764 MITRE. (2025). *CWE-174: Double decoding of the same data*. Common Weakness Enumeration. https://cwe.mitre.org/data/definitions/174.html