fix(dav): reject ambiguous nested authorization encodings

[seonghobae]

Current authority

This PR is the Naruon owner for the existing workspace-document organization boundary and the DAV/WebDAV single-decode/capability-truth repair. It does not own LLM-provider routing or provider-network policy.

• protected base: develop@042b0c70531b229af3acbd0421a2f23098d848b3
• branch: fix/dav-single-decode-authorization
• exact head: 9a019892f7d53f72415fa5633a0facd73815b8f4
• lifecycle: open / Draft / not merge-ready
• effective delta: 6 files

The prior head 8146c56587acea5c4aa859ba9366eef0f39540d7 is retained in ancestry. It carried an unrelated local-LLM-provider network-policy slice. The ordinary child 9a019892... restores backend/services/llm_provider_urls.py and backend/tests/test_llm_provider_urls.py to their exact protected-develop blobs without force-pushing or rewriting history, and rewrites the doctoring note to the DAV/document boundary only.

Bounded fixes retained

1. DAV single-decode authorization — ASGI/framework-decoded paths are not recursively decoded; residual encodings that another decode could turn structural fail closed, backslashes are normalized consistently, controls/NUL are rejected, and normalized path identity is propagated through owner checks and route handling.
2. DAV capability truthfulness — authenticated OPTIONS advertises DAV Level 1 and only the methods implemented by this endpoint; unsupported richer/writeback verbs remain unregistered rather than being advertised and then rejected downstream.
3. Workspace-document tenant isolation — _get_workspace_document and the Data quality-surface document list scope persisted Document.organization_id together with signed workspace_id; personal sessions require organization_id IS NULL.

Current effective files are:

• backend/api/data.py
• backend/api/dav.py
• backend/tests/test_data_document_authorization.py
• backend/tests/test_dav_api.py
• backend/tests/test_dav_normalized_route.py
• docs/doctoring/dav-and-local-provider-network-boundaries.md

The doctoring filename is retained for history, but its content now explicitly limits this decision to DAV/WebDAV and workspace-document authorization. It records RFC 3986 §§2.1/2.4 and RFC 4918 §10.1 traceability plus the external owner boundary.

External owner boundary

LLM-provider selection, provider URL/routing policy, provider-network SSRF policy, DNS-rebinding controls, credentials, and fallback behavior belong to the released contextual-orchestrator owner path. This PR does not expand Naruon's protected legacy direct-provider implementation. Broader migration/removal of that legacy surface remains the canonical #1548/docs-governance + contextual-orchestrator integration lane.

Consumer/descendant boundary

Draft #1404 introduces a new _find_workspace_document preview read path. That path must preserve this organization invariant, but #1404 must not become a second authority for the inherited _get_workspace_document or quality-surface scope. #1404 remains blocked on normal protected integration of this PR and must ordinary-restack afterward while retaining only its preview-specific regression. Preview/HWPX behavior remains owned by #1404; it is not added here.

Exact-head evidence

The owner-boundary repair changed the exact head, so predecessor workflow and review results are development history only and are not inherited as merge evidence.

Fresh repository workflows created for 9a019892f7d53f72415fa5633a0facd73815b8f4:

• Application CI 34065084229: queued
• Security Scan 34065084250: queued
• SAST Semgrep 34065084194: queued
• Bandit Security Scan 34065084189: queued
• Build and Publish Docker Images 34065084471: queued
• CodeQL PR 34065084193: queued

Current-head independent robot review is not yet established. Keep Draft until the unchanged final head has terminal required execution and review evidence under the live repository policy.

Merge boundary

Merge only when this unchanged exact head (or a later ordinary descendant adopted after inspecting intervening deltas) satisfies every then-live repository/organization-required check and all valid review findings are resolved. Do not manufacture a dummy/no-op commit, weaken required contexts, self-approve, dismiss valid evidence, add Naruon-local provider fallbacks, force-push, or bypass protection.

Refs #1344.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds bounded DAV authorization-path normalization, restricts allowlisted local provider hostnames to approved private networks, and adds organization scoping to document queries. Tests and boundary documentation cover the updated behavior.

Changes

DAV authorization path validation

Layer / File(s)	Summary
Bounded authorization path normalization backend/api/dav.py	The API validates path length and control characters, normalizes backslashes, rejects ambiguous nested encodings, and removes iterative percent-decoding.
Normalized path authorization and routing backend/api/dav.py	Authorization, logging, PROPFIND, and PUT handling use the normalized path.
Normalization and route validation tests backend/tests/test_dav_api.py, backend/tests/test_dav_normalized_route.py	Tests cover valid percent data, nested structural encodings, control characters, backslashes, length limits, encoded traversal, logging rejection, route handling, and PROPFIND output.

Local provider network boundaries

Layer / File(s)	Summary
Private network resolution validation backend/services/llm_provider_urls.py, backend/tests/test_llm_provider_urls.py	Allowlisted local provider hostnames are limited to RFC 1918 IPv4 and RFC 4193 IPv6 networks. Tests reject disallowed special-purpose addresses and accept IPv6 unique-local addresses.
Boundary decision documentation docs/doctoring/dav-and-local-provider-network-boundaries.md	The decision document defines DAV path rules, local-provider network rules, verification requirements, limitations, rollback procedures, and RFC references.

Document organization authorization

Layer / File(s)	Summary
Organization-scoped document access backend/api/data.py, backend/tests/test_data_document_authorization.py	Workspace and quality-surface document queries apply organization ownership filters. Tests cover cross-organization rejection, same-organization access, and personal-scope rejection.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Merge Risk: 🟡 Moderate · up to 7cfa1

This PR tightens DAV authorization decoding, local-provider network restrictions, and organization-scoped document access. It is not merge-ready until qualifying fresh independent review and every live required check succeed on the unchanged current head; the remaining branch-coverage request is a non-blocking follow-up.

Sequence Diagram(s)

sequenceDiagram
  participant DAVClient
  participant DAVHandler
  participant PathNormalizer
  participant DAVAuthorization
  participant PROPFINDHandler
  DAVClient->>DAVHandler: send DAV request with authorization path
  DAVHandler->>PathNormalizer: validate and normalize path
  PathNormalizer-->>DAVHandler: normalized path or HTTP error
  DAVHandler->>DAVAuthorization: authorize normalized path
  DAVAuthorization-->>DAVHandler: authorization result
  DAVHandler->>PROPFINDHandler: process normalized path
  PROPFINDHandler-->>DAVClient: WebDAV response

Loading

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the DAV authorization change. It identifies the rejection of ambiguous nested encodings, which is a primary security fix in the pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/dav-single-decode-authorization

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

PR governance metadata gate is not ready for 8c6a51e6777203ff3d0db84ebfa837f0c399cf6e:

• 2 unresolved current review thread(s) remain.
• Required check strix is FAILURE on the current head.

[seonghobae]

Current-head review disposition for the remaining DAV test comment: not a valid code/test defect on 0a897dbac65a903901fada99c55051c3b6a41a61. The exact-head Application CI run 31837061501, backend Python 3.14 job 94885594338, checked out this head and ran the unfiltered python -m pytest -q; it completed successfully with the route regression expecting 403. At the route boundary the ASGI/TestClient stack has already decoded the nested request target to a literal traversal segment. The production path therefore rejects the literal .. through owner/traversal validation and returns the observed fail-closed 403; _normalize_dav_authorization_path() does not receive the residual %2e%2e string assumed by the review. Changing the assertion to 400 would contradict the exercised production route. No source change is warranted for that finding. A fresh current-head automated review is still required before merge; this disposition is not an approval or a protection bypass.

@coderabbitai review

[coderabbitai]

@seonghobae: The disposition is valid. The route receives a literal .. segment after ASGI/TestClient decoding. The owner/traversal validation then rejects the request with HTTP 403. The prior expectation of HTTP 400 does not apply to this route-level case.

I have withdrawn that finding. I will run a fresh automated review of the current head.

🐇

---

✏️ Learnings added

Learnt from: seonghobae
URL: https://github.com/ContextualWisdomLab/naruon/pull/1345

Timestamp: 2026-08-14T20:27:16.019Z
Learning: In the DAV route tests, the ASGI/TestClient stack can decode a nested encoded request target before application authorization-path processing. For an encoded traversal sequence that becomes a literal `..` segment at the route boundary, the DAV owner/traversal validation rejects the request fail-closed with HTTP 403. `_normalize_dav_authorization_path()` does not receive the residual percent-encoded traversal text in that case.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

⚠️ Action not completed

Already reviewed.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[seonghobae]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

No files to review.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[seonghobae]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

No files to review.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Superseded by later exact-head fixes. The DAV normalized-path finding is resolved and its review thread records it as addressed. This dismissal clears stale predecessor-head review state only; a fresh qualifying current-head independent approval remains required.

[seonghobae]

@coderabbitai review

Please review the unchanged exact current head ff6e47554a76992f8166d28aca5ffc12bd5e2088 (normal merge of live develop@dd8d1519). Predecessor bde6998f evidence does not transfer. Local DAV/SSRF/tenant tests: 82 passed. Independent non-author APPROVE is still required; this comment is not an approval.

[coderabbitai]

@seonghobae I will review pull request #1345 at exact head ff6e47554a76992f8166d28aca5ffc12bd5e2088. I will treat predecessor-head evidence as non-current. I will not approve the pull request from this request.

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[opencode-agent]

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

• Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

• Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

• Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

• Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

• Result: REQUEST_CHANGES

• Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head ff6e47554a76992f8166d28aca5ffc12bd5e2088.

• Head SHA: ff6e47554a76992f8166d28aca5ffc12bd5e2088

• Workflow run: 32116669104

• Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Backend (7 files)"]
  S1 --> I1["API and service runtime"]
  I1 --> R1["Review risk: Backend (7 files)"]
  R1 --> V1["backend tests"]
  Evidence --> S2["Docs: dav-and-local-provider-network-boundaries.md"]
  S2 --> I2["operator or user guidance"]
  I2 --> R2["Review risk: Docs: dav-and-local-provider-network-boundaries.md"]
  R2 --> V2["docs review"]

Loading

[opencode-agent]

OpenCode Review Overview

• Head SHA: 89d885084fec5510c53c8cab992bf1f41c6abd55
• Workflow run: 32420608708
• Workflow attempt: 1
• Gate result: REQUEST_CHANGES (approval step)

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

• Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

• Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

• Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

• Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

• Result: REQUEST_CHANGES

• Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 89d885084fec5510c53c8cab992bf1f41c6abd55.

• Head SHA: 89d885084fec5510c53c8cab992bf1f41c6abd55

• Workflow run: 32420608708

• Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Backend (7 files)"]
  S1 --> I1["API and service runtime"]
  I1 --> R1["Review risk: Backend (7 files)"]
  R1 --> V1["backend tests"]
  Evidence --> S2["Docs: dav-and-local-provider-network-boundaries.md"]
  S2 --> I2["operator or user guidance"]
  I2 --> R2["Review risk: Docs: dav-and-local-provider-network-boundaries.md"]
  R2 --> V2["docs review"]

Loading

[opencode-agent]

Pull request overview

OpenCode cannot approve yet because required coverage evidence did not pass.

Review outcome

1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence

• Problem: The required coverage-evidence job result was failure, so OpenCode cannot establish approval sufficiency for this head.

• Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.

• Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports success with required evidence or explicit no-source not-applicable evidence.

• Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present.

• Result: REQUEST_CHANGES

• Reason: coverage-evidence result was failure, so required test/docstring evidence was not proven for current head 89d885084fec5510c53c8cab992bf1f41c6abd55.

• Head SHA: 89d885084fec5510c53c8cab992bf1f41c6abd55

• Workflow run: 32420608708

• Workflow attempt: 1

Coverage evidence

Coverage evidence job did not run or did not publish coverage evidence.

Changed-File Evidence Map

flowchart LR
  PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
  Evidence --> S1["Backend (7 files)"]
  S1 --> I1["API and service runtime"]
  I1 --> R1["Review risk: Backend (7 files)"]
  R1 --> V1["backend tests"]
  Evidence --> S2["Docs: dav-and-local-provider-network-boundaries.md"]
  S2 --> I2["operator or user guidance"]
  I2 --> R2["Review risk: Docs: dav-and-local-provider-network-boundaries.md"]
  R2 --> V2["docs review"]

Loading

Verified: coverage-evidence and other non-metadata-gate required checks on the current head now report pass (gh pr checks). The coverage-evidence failure this review cited has since cleared on rerun. Dismissing as superseded.

[devin-ai-integration]

Devin Review found 2 potential issues.

[seonghobae]

Strix finding disposition (check-run 97109339574, model nvidia_nim/nvidia/nemotron-3-super-120b-a12b):

MEDIUM "Authentication Bypass via Session Verifier Override in AuthContext" — REBUTTED (false positive on unchanged trusted context).

The report itself is conditional ("If there exists a code path where..."); no such path exists:

1. Sole production constructor: _auth_context_from_session_payload (backend/api/auth.py:507-527) always passes session_verifier explicitly (session_verifier=cast(SessionVerifier, session_verifier), line 526) from verified-token material.
2. That value originates only from _verify_signed_session_token (backend/api/auth.py:370-407), which returns exactly "oidc" (line 379, RS256 JWKS path) or "hmac" (line 407, HS256 path). No JWT claim feeds it: claims are extracted through fixed allowlist helpers (sub, role, org, groups, workspace), and crit headers are rejected.
3. Therefore every real HMAC/OIDC session fails the tenant-admin gate at backend/api/auth.py:515 (hmac/oidc ∉ {"server", "override"}) — deny-by-default, matching repo policy that signed sessions cannot carry tenant-admin roles.
4. Direct AuthContext(...) constructions exist only under backend/tests/** (conftest.py:70, test_auth_real.py, test_dav_api.py, test_security_api.py, test_observability_api.py, test_data_document_authorization.py) as documented FastAPI dependency-override fixtures (see docstring backend/api/auth.py:181-189). No dataclasses.replace/kwargs construction paths exist.
5. This file is not part of PR fix(dav): reject ambiguous nested authorization encodings #1345's diff (dav/data/llm_provider_urls only); the flag came from whole-context scanning of pre-existing code. Removing the documented test-fixture default would churn 8+ test files without closing any reachable path.

[seonghobae]

Fresh authority correction: the live protected base is develop@042b0c70531b229af3acbd0421a2f23098d848b3 and the actual PR head is now 8146c56587acea5c4aa859ba9366eef0f39540d7, not the older ff6e475... / dd8d151... pair still described in the long-form body. A fresh compare against protected develop is ahead 32 / behind 0 and remains bounded to the eight DAV/data/LLM-provider production/test/doctoring files, so the intervening lineage is preserved rather than destructively rewritten. Exact-head Application CI 33934455424, Bandit 33934455446, Semgrep 33934455467, Security Scan 33934455482, CodeQL PR 33934455461, and Docker 33934455595 are all queued. The current review list has no qualifying independent current-head APPROVED review, while all currently returned inline review threads are resolved. I restored the PR to Draft. Treat the older body wording that calls ff6e475... current and says to keep Ready as historical until the long-form body is reconciled; predecessor checks/reviews do not transfer.