Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 5b41d32f90 |
@@ -200,5 +200,3 @@ cython_debug/
|
||||
# Build metadata files - generated at build time
|
||||
GIT_SHA
|
||||
RUNTIME_INFO
|
||||
node_modules
|
||||
frontend/node_modules
|
||||
|
||||
+4
-30
@@ -1,30 +1,4 @@
|
||||
## 2024-05-24 - SSRF in WebDAV connection test
|
||||
**Vulnerability:** The `_test_webdav_connection` function had a custom SSRF check that failed to resolve DNS names, allowing attackers to bypass the check by providing a domain that resolves to an internal IP (e.g., `127.0.0.1`).
|
||||
**Learning:** DNS resolution is required for robust SSRF protection when validating URLs provided by users.
|
||||
**Prevention:** Use a centralized `is_private_ip` function (now in `app/utils/network.py`) that resolves the hostname to its IPs and checks if any are private.
|
||||
## 2026-03-22 - B310: urllib.request.urlopen replaced with httpx
|
||||
**Vulnerability:** The `_test_webdav_connection` function used `urllib.request.urlopen`, which natively supports dangerous schemes like `file://` or `ftp://` and follows redirects by default, potentially allowing SSRF bypasses or Local File Inclusion.
|
||||
**Learning:** `urllib.request` should be avoided for user-supplied URLs. Even when URL schemes are manually validated, `urllib`'s default redirect following behavior can bypass SSRF protections (e.g. redirecting to `127.0.0.1`).
|
||||
**Prevention:** Use a modern, safer HTTP client like `httpx` with `follow_redirects=False` when testing user-provided URLs.
|
||||
|
||||
## 2026-03-20 - Safe Path Traversal Prevention in Low-Level Utilities
|
||||
**Vulnerability:** The generic file utility `hash_file` in `app/utils/file_operations.py` accepted any file path and was vulnerable to reading arbitrary files via path traversal (e.g., `../../../etc/passwd`) or absolute paths if an attacker could control the `filepath` argument.
|
||||
**Learning:** Naively checking for `".." in path` breaks legitimate relative paths used internally by the application. Blocking absolute paths entirely also breaks functionality. Input validation should occur at the API boundary, but for defense-in-depth, low-level utilities must enforce expected boundaries (e.g., the application's `workdir`).
|
||||
**Prevention:** Use `pathlib.Path.resolve()` on both the target path and the allowed base directory (`settings.workdir`). Ensure the resolved target path is strictly within the allowed boundary using `filepath_obj.relative_to(workdir_obj)`, catching the `ValueError` that is raised when the path is out of bounds. This safely blocks both relative traversal attacks and arbitrary absolute paths.
|
||||
## 2025-05-18 - [SSRF Bypass via DNS Resolution Failure]
|
||||
**Vulnerability:** The `is_private_ip` function in `app/utils/network.py` failed open (returned `False`) when a hostname could not be resolved (`socket.gaierror`).
|
||||
**Learning:** This fail-open pattern was originally added to allow external domains in tests, but in production, it created a severe SSRF risk. An attacker could bypass SSRF protections by providing a URL that fails to resolve during the security check but resolves later (DNS rebinding), or by exploiting internal routing behaviors via unresolvable addresses.
|
||||
**Prevention:** Always fail securely in network authorization functions. If a domain cannot be resolved to verify its safety, the request must be blocked (`return True` / default-deny). Tests should mock DNS resolution correctly instead of compromising production security logic.
|
||||
## 2026-03-26 - SSRF in Integration Connection Tests
|
||||
**Vulnerability:** The `_test_imap_connection` and `_test_s3_connection` functions in `app/api/integrations.py` did not validate user-provided `host` and `endpoint_url` variables against `is_private_ip()`. This allowed an attacker to test the presence of internal IMAP servers or direct S3 SDK API calls to internal infrastructure via SSRF.
|
||||
**Learning:** Any time a new generic connection or integration test is added, SSRF validation may be forgotten if the core network utility (`is_private_ip`) is not systematically applied to all outbound network operations, regardless of the protocol (e.g., IMAP, S3).
|
||||
**Prevention:** Establish a pattern where any user-configurable host or endpoint URL is immediately passed through the centralized `is_private_ip` validation function before any network call or third-party client initialization.
|
||||
|
||||
## 2024-05-27 - SSRF Bypass via HTTP Redirects
|
||||
**Vulnerability:** In `app/api/url_upload.py`, the `validate_url_safety` function was correctly verifying the initially requested URL to prevent fetching internal IPs or cloud metadata endpoints. However, the subsequent `httpx.AsyncClient` was configured with `follow_redirects=True` without validating the destination of those redirects. An attacker could bypass SSRF protections by providing a URL to an attacker-controlled server that responds with a 301/302 redirect pointing to an internal target (e.g., `http://127.0.0.1` or `http://169.254.169.254`).
|
||||
**Learning:** Checking the URL before sending the request is insufficient if the HTTP client automatically follows redirects. The target of every single redirect must be subject to the same strict validation as the initial request.
|
||||
**Prevention:** Avoid `follow_redirects=True` for user-provided URLs when possible. If redirects must be followed, attach an event hook (e.g., `event_hooks={"response": [hook_function]}`) to the `httpx` client to intercept the response, calculate the redirect destination from the `Location` header, and run the URL safety validation logic before the redirect is actually followed.
|
||||
## 2026-03-27 - SSRF Bypass via HTTP Redirects in httpx
|
||||
**Vulnerability:** The `/process-url` endpoint used `httpx.AsyncClient(follow_redirects=True)` after validating the initial user-provided URL against SSRF protections. However, it did not validate the target URLs of any subsequent HTTP redirects, allowing an attacker to provide a safe URL that redirects to an internal/private IP, bypassing the security check.
|
||||
**Learning:** Initial URL validation is insufficient when the HTTP client is configured to follow redirects automatically. The client must be explicitly configured to validate every redirect target.
|
||||
**Prevention:** When using `httpx.AsyncClient(follow_redirects=True)` for user-provided URLs, always implement a redirect validator hook function (e.g., using `event_hooks={'response': [validate_redirect]}`) that resolves the `Location` header and passes it through the same SSRF validation logic before the redirect is followed.
|
||||
## 2025-02-14 - SSRF vulnerability in webhook outgoing requests
|
||||
**Vulnerability:** Found an SSRF vulnerability where outgoing webhook requests could hit private IPs or metadata endpoints (e.g. 169.254.169.254).
|
||||
**Learning:** This existed because the `url` parameter provided for webhooks (`app/utils/webhook.py` and `app/utils/user_notification.py`) was not being checked before being passed to `requests.post()` or `httpx.post()`.
|
||||
**Prevention:** Make sure to always validate URL scheme and hostname with `is_private_ip()` and block known cloud metadata endpoints before doing outgoing network requests based on dynamic values.
|
||||
|
||||
+1
-1
@@ -1 +1 @@
|
||||
2026-05-17T14:20:08Z
|
||||
2026-04-07T09:34:53Z
|
||||
|
||||
@@ -10,50 +10,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
<!-- version list -->
|
||||
|
||||
## v0.172.12 (2026-05-17)
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
- Validate webhook targets before delivery
|
||||
([#846](https://github.com/christianlouis/DocuElevate/pull/846),
|
||||
[`e2fa963`](https://github.com/christianlouis/DocuElevate/commit/e2fa96318f5bd45607baa0fe08a0bf14e1ca83d4))
|
||||
|
||||
### Testing
|
||||
|
||||
- Cover webhook SSRF validation ([#846](https://github.com/christianlouis/DocuElevate/pull/846),
|
||||
[`e2fa963`](https://github.com/christianlouis/DocuElevate/commit/e2fa96318f5bd45607baa0fe08a0bf14e1ca83d4))
|
||||
|
||||
|
||||
## v0.172.11 (2026-05-17)
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
- Escape search result template values
|
||||
([#853](https://github.com/christianlouis/DocuElevate/pull/853),
|
||||
[`1a02187`](https://github.com/christianlouis/DocuElevate/commit/1a0218799b9a1eb4154e2f4fbb2572cb3922106a))
|
||||
|
||||
### Documentation
|
||||
|
||||
- **changelog**: Update changelog [skip ci]
|
||||
([`048f28a`](https://github.com/christianlouis/DocuElevate/commit/048f28a6717fa7f5cf4b235f9142e625e80e5d59))
|
||||
|
||||
|
||||
## Unreleased
|
||||
|
||||
|
||||
## v0.172.10 (2026-05-17)
|
||||
|
||||
### Bug Fixes
|
||||
|
||||
- **url-upload**: Handle unsafe redirects as client errors
|
||||
([`871f788`](https://github.com/christianlouis/DocuElevate/commit/871f788f0bd782ba8ad3a7d70e5cd4ccd24f749b))
|
||||
|
||||
### Documentation
|
||||
|
||||
- **changelog**: Update changelog [skip ci]
|
||||
([`58b14ae`](https://github.com/christianlouis/DocuElevate/commit/58b14ae769b85e25290126256de936743609af06))
|
||||
|
||||
|
||||
## Unreleased
|
||||
|
||||
|
||||
|
||||
+6
-6
@@ -1,10 +1,10 @@
|
||||
DocuElevate Build Information
|
||||
==============================
|
||||
Version: 0.172.12
|
||||
Build Date: 2026-05-17T14:20:08Z
|
||||
Git Commit: e2fa96318f5bd45607baa0fe08a0bf14e1ca83d4
|
||||
Git Short SHA: e2fa963
|
||||
Version: 0.172.9
|
||||
Build Date: 2026-04-07T09:34:53Z
|
||||
Git Commit: 3bd8a52ea201b33d6071c9b3a7fdace582e65fd5
|
||||
Git Short SHA: 3bd8a52
|
||||
Git Branch: main
|
||||
Commit Date: 2026-05-17T16:19:41+02:00
|
||||
Build Timestamp: 2026-05-17T14:20:08Z
|
||||
Commit Date: 2026-04-07T11:34:28+02:00
|
||||
Build Timestamp: 2026-04-07T09:34:53Z
|
||||
==============================
|
||||
|
||||
+4
-13
@@ -28,10 +28,6 @@ logger = logging.getLogger(__name__)
|
||||
router = APIRouter()
|
||||
|
||||
|
||||
class UnsafeRedirectError(httpx.RequestError):
|
||||
"""Raised when a redirect target fails URL safety checks."""
|
||||
|
||||
|
||||
class URLUploadRequest(BaseModel):
|
||||
"""Request model for URL-based file upload"""
|
||||
|
||||
@@ -124,10 +120,9 @@ async def verify_redirect(response: httpx.Response) -> None:
|
||||
try:
|
||||
validate_url_safety(new_url)
|
||||
except HTTPException as e:
|
||||
raise UnsafeRedirectError(
|
||||
f"Redirect to unsafe URL blocked: {e.detail}",
|
||||
request=response.request,
|
||||
) from e
|
||||
# Map the validation error to an httpx exception so it can be handled
|
||||
# properly by the caller, avoiding raw HTTPExceptions escaping the client scope
|
||||
raise httpx.RequestError(f"Redirect to unsafe URL blocked: {e.detail}", request=response.request) from e
|
||||
|
||||
|
||||
@router.post("/process-url")
|
||||
@@ -186,10 +181,10 @@ async def process_url(
|
||||
async with httpx.AsyncClient(
|
||||
timeout=settings.http_request_timeout,
|
||||
follow_redirects=True,
|
||||
event_hooks={"response": [verify_redirect]},
|
||||
headers={
|
||||
"User-Agent": "DocuElevate/1.0", # Identify ourselves
|
||||
},
|
||||
event_hooks={"response": [verify_redirect]},
|
||||
) as client:
|
||||
async with client.stream("GET", url) as response:
|
||||
response.raise_for_status()
|
||||
@@ -276,10 +271,6 @@ async def process_url(
|
||||
logger.error(f"HTTP error while downloading file from URL: {url} - {str(e)}")
|
||||
raise HTTPException(status_code=e.response.status_code, detail=f"HTTP error: {str(e)}")
|
||||
|
||||
except UnsafeRedirectError as e:
|
||||
logger.warning(f"Unsafe redirect blocked while downloading file from URL: {url} - {str(e)}")
|
||||
raise HTTPException(status_code=400, detail=str(e))
|
||||
|
||||
except httpx.RequestError as e:
|
||||
logger.error(f"Error downloading file from URL: {url} - {str(e)}")
|
||||
raise HTTPException(status_code=500, detail=f"Failed to download file: {str(e)}")
|
||||
|
||||
@@ -18,7 +18,6 @@ import httpx
|
||||
|
||||
from app.database import SessionLocal
|
||||
from app.models import InAppNotification, UserNotificationPreference, UserNotificationTarget
|
||||
from app.utils.network import is_private_ip
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -30,11 +29,6 @@ USER_EVENT_LABELS: dict[str, str] = {
|
||||
EVENT_DOCUMENT_PROCESSED: "Document Processed",
|
||||
EVENT_DOCUMENT_FAILED: "Document Processing Failed",
|
||||
}
|
||||
METADATA_ENDPOINTS = {
|
||||
"169.254.169.254",
|
||||
"169.254.169.253",
|
||||
"metadata.google.internal",
|
||||
}
|
||||
|
||||
|
||||
def create_in_app_notification(
|
||||
@@ -135,18 +129,29 @@ def _send_webhook_notification(target_config: dict[str, Any], event_type: str, t
|
||||
logger.warning("Webhook notification target missing url")
|
||||
return False
|
||||
|
||||
from app.utils.network import is_private_ip
|
||||
|
||||
parsed_url = urlparse(url)
|
||||
if parsed_url.scheme not in {"http", "https"}:
|
||||
logger.warning("Webhook notification to %s blocked: invalid scheme %s", url, parsed_url.scheme)
|
||||
if parsed_url.scheme not in ("http", "https"):
|
||||
logger.warning("Webhook notification to %s blocked: Invalid scheme %s", url, parsed_url.scheme)
|
||||
return False
|
||||
|
||||
hostname = parsed_url.hostname
|
||||
if not hostname:
|
||||
logger.warning("Webhook notification to %s blocked: missing hostname", url)
|
||||
logger.warning("Webhook notification to %s blocked: No hostname", url)
|
||||
return False
|
||||
|
||||
if hostname in METADATA_ENDPOINTS or is_private_ip(hostname):
|
||||
logger.warning("Webhook notification to %s blocked: private or metadata endpoint", url)
|
||||
if is_private_ip(hostname):
|
||||
logger.warning("Webhook notification to %s blocked: Private IP", url)
|
||||
return False
|
||||
|
||||
metadata_endpoints = [
|
||||
"169.254.169.254", # AWS, Azure, GCP metadata
|
||||
"metadata.google.internal", # GCP
|
||||
"169.254.169.253", # AWS link-local
|
||||
]
|
||||
if hostname in metadata_endpoints:
|
||||
logger.warning("Webhook notification to %s blocked: Metadata endpoint", url)
|
||||
return False
|
||||
|
||||
payload = {
|
||||
|
||||
+16
-11
@@ -24,7 +24,6 @@ import requests
|
||||
|
||||
from app.database import SessionLocal
|
||||
from app.models import WebhookConfig
|
||||
from app.utils.network import is_private_ip
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -42,11 +41,6 @@ VALID_EVENTS: frozenset[str] = frozenset(
|
||||
|
||||
#: Timeout (seconds) for outgoing webhook HTTP requests.
|
||||
WEBHOOK_TIMEOUT = 10
|
||||
METADATA_ENDPOINTS = {
|
||||
"169.254.169.254",
|
||||
"169.254.169.253",
|
||||
"metadata.google.internal",
|
||||
}
|
||||
|
||||
|
||||
def compute_signature(payload_bytes: bytes, secret: str) -> str:
|
||||
@@ -74,18 +68,29 @@ def deliver_webhook(url: str, payload: dict[str, Any], secret: str | None = None
|
||||
Returns:
|
||||
``True`` when the remote server responds with a 2xx status.
|
||||
"""
|
||||
from app.utils.network import is_private_ip
|
||||
|
||||
parsed_url = urlparse(url)
|
||||
if parsed_url.scheme not in {"http", "https"}:
|
||||
logger.warning("Webhook to %s blocked: invalid scheme %s", url, parsed_url.scheme)
|
||||
if parsed_url.scheme not in ("http", "https"):
|
||||
logger.warning("Webhook to %s blocked: Invalid scheme %s", url, parsed_url.scheme)
|
||||
return False
|
||||
|
||||
hostname = parsed_url.hostname
|
||||
if not hostname:
|
||||
logger.warning("Webhook to %s blocked: missing hostname", url)
|
||||
logger.warning("Webhook to %s blocked: No hostname", url)
|
||||
return False
|
||||
|
||||
if hostname in METADATA_ENDPOINTS or is_private_ip(hostname):
|
||||
logger.warning("Webhook to %s blocked: private or metadata endpoint", url)
|
||||
if is_private_ip(hostname):
|
||||
logger.warning("Webhook to %s blocked: Private IP", url)
|
||||
return False
|
||||
|
||||
metadata_endpoints = [
|
||||
"169.254.169.254", # AWS, Azure, GCP metadata
|
||||
"metadata.google.internal", # GCP
|
||||
"169.254.169.253", # AWS link-local
|
||||
]
|
||||
if hostname in metadata_endpoints:
|
||||
logger.warning("Webhook to %s blocked: Metadata endpoint", url)
|
||||
return False
|
||||
|
||||
body = json.dumps(payload, default=str, sort_keys=True)
|
||||
|
||||
@@ -1538,28 +1538,6 @@
|
||||
});
|
||||
}
|
||||
|
||||
function escapeHtml(str) {
|
||||
if (!str) return '';
|
||||
return String(str)
|
||||
.replace(/&/g, '&')
|
||||
.replace(/</g, '<')
|
||||
.replace(/>/g, '>')
|
||||
.replace(/"/g, '"')
|
||||
.replace(/'/g, ''');
|
||||
}
|
||||
|
||||
function sanitizeHighlight(html) {
|
||||
if (!html) return '';
|
||||
let safe = String(html)
|
||||
.replace(/<mark>/gi, '\x00MARK_OPEN\x00')
|
||||
.replace(/<\/mark>/gi, '\x00MARK_CLOSE\x00');
|
||||
safe = escapeHtml(safe);
|
||||
safe = safe
|
||||
.replace(/\x00MARK_OPEN\x00/g, '<mark>')
|
||||
.replace(/\x00MARK_CLOSE\x00/g, '</mark>');
|
||||
return safe;
|
||||
}
|
||||
|
||||
function renderSearchResults(data, q) {
|
||||
const panel = document.getElementById('search-results-panel');
|
||||
const list = document.getElementById('search-results-list');
|
||||
@@ -1577,31 +1555,25 @@
|
||||
|
||||
list.innerHTML = results.map(hit => {
|
||||
const fmt = hit._formatted || {};
|
||||
const titleRaw = fmt.document_title || hit.document_title || hit.original_filename || __i18n.untitled;
|
||||
const filenameRaw = fmt.original_filename || hit.original_filename || '';
|
||||
const snippetRaw = fmt.ocr_text || '';
|
||||
const tagsRaw = Array.isArray(hit.tags) ? hit.tags.join(', ') : (hit.tags || '');
|
||||
const docTypeRaw = hit.document_type || '';
|
||||
|
||||
const safeTitle = fmt.document_title ? sanitizeHighlight(titleRaw) : escapeHtml(titleRaw);
|
||||
const safeFilename = escapeHtml(filenameRaw);
|
||||
const safeSnippet = sanitizeHighlight(snippetRaw);
|
||||
const safeTags = escapeHtml(tagsRaw);
|
||||
const safeDocType = escapeHtml(docTypeRaw);
|
||||
const title = fmt.document_title || hit.document_title || hit.original_filename || __i18n.untitled;
|
||||
const filename = fmt.original_filename || hit.original_filename || '';
|
||||
const snippet = fmt.ocr_text || '';
|
||||
const tags = Array.isArray(hit.tags) ? hit.tags.join(', ') : (hit.tags || '');
|
||||
const docType = hit.document_type || '';
|
||||
|
||||
return `<div style="padding: 0.75rem 1rem; border-bottom: 1px solid #f3f4f6; display: flex; gap: 0.75rem; align-items: flex-start;">
|
||||
<div style="flex-shrink: 0; color: #3b82f6; font-size: 1.25rem; padding-top: 0.1rem;">
|
||||
<i class="fas fa-file-pdf"></i>
|
||||
</div>
|
||||
<div style="flex: 1; min-width: 0;">
|
||||
<div style="font-weight: 600; font-size: 0.9rem; color: #111827;">${safeTitle}</div>
|
||||
${safeFilename ? `<div style="font-size: 0.8rem; color: #6b7280; margin-top: 0.15rem;">${safeFilename}</div>` : ''}
|
||||
${safeDocType ? `<span style="display: inline-block; margin-top: 0.25rem; padding: 0.1rem 0.5rem; background: #eff6ff; color: #1d4ed8; border-radius: 9999px; font-size: 0.75rem;">${safeDocType}</span>` : ''}
|
||||
${safeTags ? `<span style="display: inline-block; margin-top: 0.25rem; margin-left: 0.25rem; padding: 0.1rem 0.5rem; background: #f0fdf4; color: #15803d; border-radius: 9999px; font-size: 0.75rem;">${safeTags}</span>` : ''}
|
||||
${safeSnippet ? `<div style="margin-top: 0.4rem; font-size: 0.8rem; color: #374151; white-space: pre-wrap; word-break: break-word;">…${safeSnippet}…</div>` : ''}
|
||||
<div style="font-weight: 600; font-size: 0.9rem; color: #111827;">${title}</div>
|
||||
${filename ? `<div style="font-size: 0.8rem; color: #6b7280; margin-top: 0.15rem;">${filename}</div>` : ''}
|
||||
${docType ? `<span style="display: inline-block; margin-top: 0.25rem; padding: 0.1rem 0.5rem; background: #eff6ff; color: #1d4ed8; border-radius: 9999px; font-size: 0.75rem;">${docType}</span>` : ''}
|
||||
${tags ? `<span style="display: inline-block; margin-top: 0.25rem; margin-left: 0.25rem; padding: 0.1rem 0.5rem; background: #f0fdf4; color: #15803d; border-radius: 9999px; font-size: 0.75rem;">${tags}</span>` : ''}
|
||||
${snippet ? `<div style="margin-top: 0.4rem; font-size: 0.8rem; color: #374151; white-space: pre-wrap; word-break: break-word;">…${snippet}…</div>` : ''}
|
||||
</div>
|
||||
<div style="flex-shrink: 0;">
|
||||
<a href="/files/${escapeHtml(hit.file_id)}" style="padding: 0.25rem 0.6rem; background: #f3f4f6; color: #374151; border-radius: 0.25rem; font-size: 0.8rem; text-decoration: none; white-space: nowrap;" title="${__i18n.viewFile}">
|
||||
<a href="/files/${hit.file_id}" style="padding: 0.25rem 0.6rem; background: #f3f4f6; color: #374151; border-radius: 0.25rem; font-size: 0.8rem; text-decoration: none; white-space: nowrap;" title="${__i18n.viewFile}">
|
||||
<i class="fas fa-external-link-alt"></i>
|
||||
</a>
|
||||
</div>
|
||||
|
||||
+1
-1
@@ -1,5 +1,5 @@
|
||||
[build-system]
|
||||
requires = ["setuptools>=82.0.1", "wheel"]
|
||||
requires = ["setuptools>=45", "wheel"]
|
||||
build-backend = "setuptools.build_meta"
|
||||
|
||||
[project]
|
||||
|
||||
+26
-13
@@ -465,19 +465,6 @@ class TestURLUploadEndpoint:
|
||||
data = response.json()
|
||||
assert "Failed to download file" in data["detail"]
|
||||
|
||||
@patch("app.api.url_upload.httpx.AsyncClient.stream")
|
||||
def test_process_url_unsafe_redirect_returns_400(self, mock_stream, client):
|
||||
"""Test unsafe redirects are reported as a client error instead of HTTP 500."""
|
||||
from app.api.url_upload import UnsafeRedirectError
|
||||
|
||||
mock_stream.side_effect = UnsafeRedirectError("Redirect to unsafe URL blocked: Unsafe URL")
|
||||
|
||||
response = client.post("/api/process-url", json={"url": "https://example.com/file.pdf"})
|
||||
|
||||
assert response.status_code == 400
|
||||
data = response.json()
|
||||
assert "Redirect to unsafe URL blocked" in data["detail"]
|
||||
|
||||
@patch("app.api.url_upload.httpx.AsyncClient.stream")
|
||||
def test_process_url_oserror_during_save(self, mock_stream, client, tmp_path, monkeypatch):
|
||||
"""Test handling of OSError when saving file"""
|
||||
@@ -937,3 +924,29 @@ class TestURLUploadCoverageGaps:
|
||||
|
||||
# Should not raise any exception and should ignore missing Location header
|
||||
await verify_redirect(resp)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_verify_redirect_coverage():
|
||||
from app.api.url_upload import verify_redirect
|
||||
|
||||
response = MagicMock(spec=httpx.Response)
|
||||
response.status_code = 301
|
||||
response.headers = httpx.Headers({"Location": "ftp://example.com"})
|
||||
response.url = httpx.URL("http://test.com")
|
||||
response.request = MagicMock(spec=httpx.Request)
|
||||
|
||||
with pytest.raises(httpx.RequestError):
|
||||
await verify_redirect(response)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_verify_redirect_coverage2():
|
||||
from app.api.url_upload import verify_redirect
|
||||
|
||||
response = MagicMock(spec=httpx.Response)
|
||||
response.status_code = 301
|
||||
response.headers = httpx.Headers({"Location": "http://example.com"})
|
||||
response.url = httpx.URL("http://test.com")
|
||||
response.request = MagicMock(spec=httpx.Request)
|
||||
|
||||
# Should be fine
|
||||
await verify_redirect(response)
|
||||
|
||||
@@ -227,7 +227,8 @@ class TestSendEmailNotification:
|
||||
class TestSendWebhookNotification:
|
||||
"""Tests for _send_webhook_notification()."""
|
||||
|
||||
def test_success_with_secret_header(self):
|
||||
def test_success_with_secret_header(self, mocker):
|
||||
mocker.patch("app.utils.network.is_private_ip", return_value=False)
|
||||
"""Webhook sent and X-DocuElevate-Secret header set when secret provided."""
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
|
||||
@@ -235,10 +236,7 @@ class TestSendWebhookNotification:
|
||||
mock_response.status_code = 200
|
||||
mock_response.raise_for_status = MagicMock()
|
||||
|
||||
with (
|
||||
patch("app.utils.user_notification.is_private_ip", return_value=False),
|
||||
patch("app.utils.user_notification.httpx.post", return_value=mock_response) as mock_post,
|
||||
):
|
||||
with patch("app.utils.user_notification.httpx.post", return_value=mock_response) as mock_post:
|
||||
result = _send_webhook_notification(
|
||||
{"url": "https://hook.example.com/test", "secret": "mysecret"},
|
||||
"document.processed",
|
||||
@@ -251,7 +249,8 @@ class TestSendWebhookNotification:
|
||||
assert kwargs["headers"]["X-DocuElevate-Secret"] == "mysecret"
|
||||
assert kwargs["json"]["event"] == "document.processed"
|
||||
|
||||
def test_success_without_secret(self):
|
||||
def test_success_without_secret(self, mocker):
|
||||
mocker.patch("app.utils.network.is_private_ip", return_value=False)
|
||||
"""Webhook sent without X-DocuElevate-Secret header when no secret."""
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
|
||||
@@ -259,10 +258,7 @@ class TestSendWebhookNotification:
|
||||
mock_response.status_code = 200
|
||||
mock_response.raise_for_status = MagicMock()
|
||||
|
||||
with (
|
||||
patch("app.utils.user_notification.is_private_ip", return_value=False),
|
||||
patch("app.utils.user_notification.httpx.post", return_value=mock_response) as mock_post,
|
||||
):
|
||||
with patch("app.utils.user_notification.httpx.post", return_value=mock_response) as mock_post:
|
||||
result = _send_webhook_notification(
|
||||
{"url": "https://hook.example.com/test"},
|
||||
"document.failed",
|
||||
@@ -278,12 +274,9 @@ class TestSendWebhookNotification:
|
||||
"""_send_webhook_notification returns False when httpx raises."""
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
|
||||
with (
|
||||
patch("app.utils.user_notification.is_private_ip", return_value=False),
|
||||
patch(
|
||||
"app.utils.user_notification.httpx.post",
|
||||
side_effect=Exception("connection error"),
|
||||
),
|
||||
with patch(
|
||||
"app.utils.user_notification.httpx.post",
|
||||
side_effect=Exception("connection error"),
|
||||
):
|
||||
result = _send_webhook_notification(
|
||||
{"url": "https://hook.example.com/test"},
|
||||
@@ -309,10 +302,7 @@ class TestSendWebhookNotification:
|
||||
)
|
||||
)
|
||||
|
||||
with (
|
||||
patch("app.utils.user_notification.is_private_ip", return_value=False),
|
||||
patch("app.utils.user_notification.httpx.post", return_value=mock_response),
|
||||
):
|
||||
with patch("app.utils.user_notification.httpx.post", return_value=mock_response):
|
||||
result = _send_webhook_notification(
|
||||
{"url": "https://hook.example.com/test"},
|
||||
"document.processed",
|
||||
@@ -322,69 +312,6 @@ class TestSendWebhookNotification:
|
||||
|
||||
assert result is False
|
||||
|
||||
def test_blocks_private_webhook_target(self):
|
||||
"""Webhook delivery is skipped for private network targets."""
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
|
||||
with (
|
||||
patch("app.utils.user_notification.is_private_ip", return_value=True),
|
||||
patch("app.utils.user_notification.httpx.post") as mock_post,
|
||||
):
|
||||
result = _send_webhook_notification(
|
||||
{"url": "https://10.0.0.5/test"},
|
||||
"document.processed",
|
||||
"T",
|
||||
"M",
|
||||
)
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
def test_blocks_metadata_webhook_target(self):
|
||||
"""Webhook delivery is skipped for cloud metadata endpoints."""
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
|
||||
with patch("app.utils.user_notification.httpx.post") as mock_post:
|
||||
result = _send_webhook_notification(
|
||||
{"url": "http://169.254.169.254/latest/meta-data"},
|
||||
"document.processed",
|
||||
"T",
|
||||
"M",
|
||||
)
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
def test_blocks_invalid_webhook_scheme(self):
|
||||
"""Webhook delivery is skipped for unsupported URL schemes."""
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
|
||||
with patch("app.utils.user_notification.httpx.post") as mock_post:
|
||||
result = _send_webhook_notification(
|
||||
{"url": "file:///etc/passwd"},
|
||||
"document.processed",
|
||||
"T",
|
||||
"M",
|
||||
)
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
def test_blocks_webhook_without_hostname(self):
|
||||
"""Webhook delivery is skipped when the URL has no hostname."""
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
|
||||
with patch("app.utils.user_notification.httpx.post") as mock_post:
|
||||
result = _send_webhook_notification(
|
||||
{"url": "https:///missing-host"},
|
||||
"document.processed",
|
||||
"T",
|
||||
"M",
|
||||
)
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# dispatch_user_notification – preference loop
|
||||
@@ -708,3 +635,33 @@ class TestNotifyUserDocumentHelpers:
|
||||
assert "broken.pdf" in notifs[0].title
|
||||
assert "Timeout" in notifs[0].message
|
||||
assert notifs[0].event_type == "document.failed"
|
||||
|
||||
def test_webhook_coverage():
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
assert _send_webhook_notification({"url": "http://169.254.169.254"}, "test", "test", "test") == False
|
||||
assert _send_webhook_notification({"url": "http://localhost"}, "test", "test", "test") == False
|
||||
assert _send_webhook_notification({"url": "ftp://example.com"}, "test", "test", "test") == False
|
||||
assert _send_webhook_notification({"url": "http://"}, "test", "test", "test") == False
|
||||
assert _send_webhook_notification({"url": "http://foo.bar.baz"}, "test", "test", "test") == False
|
||||
assert _send_webhook_notification({"url": ""}, "test", "test", "test") == False
|
||||
|
||||
def test_webhook_coverage2(mocker):
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
mocker.patch("app.utils.network.is_private_ip", return_value=False)
|
||||
assert _send_webhook_notification({"url": "http://127.0.0.1"}, "test", "test", "test") == False
|
||||
|
||||
def test_webhook_coverage3(mocker):
|
||||
mocker.patch("app.utils.network.is_private_ip", return_value=False)
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
assert _send_webhook_notification({"url": "http://127.0.0.1"}, "test", "test", "test") == False
|
||||
assert _send_webhook_notification({"url": "http://169.254.169.253"}, "test", "test", "test") == False
|
||||
assert _send_webhook_notification({"url": "http://metadata.google.internal"}, "test", "test", "test") == False
|
||||
|
||||
def test_webhook_coverage5():
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
assert _send_webhook_notification({}, "test", "test", "test") == False
|
||||
|
||||
def test_webhook_coverage4(mocker):
|
||||
from app.utils.user_notification import _send_webhook_notification
|
||||
mocker.patch("app.utils.network.is_private_ip", return_value=True)
|
||||
assert _send_webhook_notification({"url": "http://127.0.0.1"}, "test", "test", "test") == False
|
||||
|
||||
+30
-42
@@ -81,7 +81,6 @@ class TestDeliverWebhook:
|
||||
|
||||
def test_success_returns_true(self, mocker):
|
||||
"""A 200 response returns True."""
|
||||
mocker.patch("app.utils.webhook.is_private_ip", return_value=False)
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
mock_post.return_value = MagicMock(ok=True, status_code=200)
|
||||
|
||||
@@ -91,7 +90,6 @@ class TestDeliverWebhook:
|
||||
|
||||
def test_non_2xx_returns_false(self, mocker):
|
||||
"""A non-2xx response returns False."""
|
||||
mocker.patch("app.utils.webhook.is_private_ip", return_value=False)
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
mock_post.return_value = MagicMock(ok=False, status_code=500)
|
||||
|
||||
@@ -102,7 +100,6 @@ class TestDeliverWebhook:
|
||||
"""A network error returns False."""
|
||||
import requests
|
||||
|
||||
mocker.patch("app.utils.webhook.is_private_ip", return_value=False)
|
||||
mocker.patch("app.utils.webhook.requests.post", side_effect=requests.ConnectionError("fail"))
|
||||
|
||||
result = deliver_webhook("https://example.com/hook", {"event": "test"})
|
||||
@@ -110,7 +107,6 @@ class TestDeliverWebhook:
|
||||
|
||||
def test_signature_header_included_when_secret(self, mocker):
|
||||
"""X-Webhook-Signature header is present when a secret is supplied."""
|
||||
mocker.patch("app.utils.webhook.is_private_ip", return_value=False)
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
mock_post.return_value = MagicMock(ok=True, status_code=200)
|
||||
|
||||
@@ -122,7 +118,6 @@ class TestDeliverWebhook:
|
||||
|
||||
def test_no_signature_header_without_secret(self, mocker):
|
||||
"""X-Webhook-Signature header is absent when no secret is supplied."""
|
||||
mocker.patch("app.utils.webhook.is_private_ip", return_value=False)
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
mock_post.return_value = MagicMock(ok=True, status_code=200)
|
||||
|
||||
@@ -131,43 +126,6 @@ class TestDeliverWebhook:
|
||||
headers = call_kwargs.kwargs.get("headers") or call_kwargs[1].get("headers")
|
||||
assert "X-Webhook-Signature" not in headers
|
||||
|
||||
def test_private_target_is_blocked(self, mocker):
|
||||
"""Private network webhook targets are not called."""
|
||||
mocker.patch("app.utils.webhook.is_private_ip", return_value=True)
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
|
||||
result = deliver_webhook("https://10.0.0.5/hook", {"event": "test"})
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
def test_metadata_target_is_blocked(self, mocker):
|
||||
"""Cloud metadata webhook targets are not called."""
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
|
||||
result = deliver_webhook("http://169.254.169.254/latest/meta-data", {"event": "test"})
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
def test_invalid_scheme_is_blocked(self, mocker):
|
||||
"""Unsupported webhook URL schemes are not called."""
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
|
||||
result = deliver_webhook("file:///etc/passwd", {"event": "test"})
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
def test_missing_hostname_is_blocked(self, mocker):
|
||||
"""Webhook URLs without a hostname are not called."""
|
||||
mock_post = mocker.patch("app.utils.webhook.requests.post")
|
||||
|
||||
result = deliver_webhook("https:///missing-host", {"event": "test"})
|
||||
|
||||
assert result is False
|
||||
mock_post.assert_not_called()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Unit tests – get_active_webhooks_for_event (DB)
|
||||
@@ -480,6 +438,7 @@ class TestDeliverWebhookTask:
|
||||
"""Tests for the Celery webhook delivery task."""
|
||||
|
||||
def test_success_returns_status_dict(self, mocker):
|
||||
mocker.patch("app.utils.network.is_private_ip", return_value=False)
|
||||
"""Task returns a dict on successful delivery."""
|
||||
mocker.patch("app.tasks.webhook_tasks.deliver_webhook", return_value=True)
|
||||
|
||||
@@ -502,3 +461,32 @@ class TestDeliverWebhookTask:
|
||||
|
||||
with pytest.raises(RuntimeError, match="Webhook delivery.*failed"):
|
||||
deliver_webhook_task.__wrapped__("https://example.com/hook", {"event": "test"}, None)
|
||||
|
||||
def test_webhook_ssrf_coverage():
|
||||
from app.utils.webhook import deliver_webhook
|
||||
assert deliver_webhook("ftp://example.com", {"data": 1}) == False
|
||||
assert deliver_webhook("http://", {"data": 1}) == False
|
||||
assert deliver_webhook("http://localhost", {"data": 1}) == False
|
||||
assert deliver_webhook("http://169.254.169.254", {"data": 1}) == False
|
||||
assert deliver_webhook("", {"data": 1}) == False
|
||||
|
||||
def test_webhook_ssrf_coverage2(mocker):
|
||||
from app.utils.webhook import deliver_webhook
|
||||
mocker.patch("app.utils.network.is_private_ip", return_value=False)
|
||||
assert deliver_webhook("http://127.0.0.1", {"data": 1}) == False
|
||||
|
||||
def test_webhook_ssrf_coverage3(mocker):
|
||||
mocker.patch("app.utils.network.is_private_ip", return_value=False)
|
||||
from app.utils.webhook import deliver_webhook
|
||||
assert deliver_webhook("http://127.0.0.1", {"data": 1}) == False
|
||||
assert deliver_webhook("http://169.254.169.253", {"data": 1}) == False
|
||||
assert deliver_webhook("http://metadata.google.internal", {"data": 1}) == False
|
||||
|
||||
def test_webhook_ssrf_coverage5():
|
||||
from app.utils.webhook import deliver_webhook
|
||||
assert deliver_webhook("http://example.com/test", {"data": 1}) == False
|
||||
|
||||
def test_webhook_ssrf_coverage4(mocker):
|
||||
from app.utils.webhook import deliver_webhook
|
||||
mocker.patch("app.utils.network.is_private_ip", return_value=True)
|
||||
assert deliver_webhook("http://127.0.0.1", {"data": 1}) == False
|
||||
|
||||
Reference in New Issue
Block a user