- Resolve merge conflicts in app/api/onedrive.py and tests/test_api_google_drive_final.py
- Fix legacy Dict[str, str] type hints in update_env_file functions to use dict[str, str]
- Add admin-only access (_require_admin dependency) to save-settings endpoints
in google_drive.py, onedrive.py, and dropbox.py
- Fix in_memory_only response field to reflect actual env_write_success status
- Update tests to override _require_admin dependency for save-settings endpoint tests
- Resolve merge conflicts in .jules/sentinel.md and app/utils/network.py
- Refactor join_url() to use urllib.parse.urlsplit/urlunsplit and posixpath
instead of sentinel-string hack, preventing corruption for any input URL
- Fix test to use pytest tmp_path fixture instead of hard-coded /tmp/workdir
- Resolve add/add conflict in tests/test_api_saved_searches.py by keeping the improved HEAD version
- Resolve content conflict in tests/test_api_advanced_filters.py by keeping HEAD (no CRUD tests)
- Remove no-op test_get_user_id_branches (was just 'pass')
- Remove unused 'from fastapi import Request' import (fixes Ruff F401)
- Fix duplicate 'session = {}' assignment in MockRequest (fixes Ruff F811)
This commit safely handles the dynamic table names in database migration queries
by leveraging `sqlalchemy.select` and `sqlalchemy.table` in `app/utils/db_migrate.py`.
It addresses the `# noqa: S608` exception that was in place for string interpolation
SQL queries which are a known security anti-pattern.
Additionally, this commit includes the latest updates to `app/views/base.py`
from the `main` branch to handle backward compatibility across Starlette
versions (<1.0 vs 1.0+) when invoking `Jinja2Templates.TemplateResponse`,
resolving previous merge conflicts in the PR.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
🚨 Severity: CRITICAL
💡 Vulnerability: The generic file hashing utility `app/utils/file_operations.py:hash_file` was vulnerable to path traversal. An attacker controlling the `filepath` argument could read arbitrary files on the system by passing relative paths like `../../../etc/passwd` or providing absolute paths directly.
🎯 Impact: This could lead to Arbitrary File Read and potential information disclosure.
🔧 Fix: Used `pathlib.Path.resolve()` to resolve both the target file path and the allowed base directory (`settings.workdir`). Added a strict check to ensure the resolved target path is strictly within the allowed boundary using `filepath_obj.relative_to(workdir_obj)`, catching the `ValueError` raised when the path is out of bounds. This safely blocks both relative traversal attacks and arbitrary absolute paths, without breaking legitimate relative application paths.
✅ Verification: Ran the test suite `pytest tests/test_path_traversal_security.py -v` successfully, which explicitly checks for `FileNotFoundError` upon traversal attempts.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
Extracted existing `TestSavedSearchesCRUD` from `tests/test_api_advanced_filters.py` into a dedicated `tests/test_api_saved_searches.py` file to better organize testing logic and reflect the application's file structure.
Significantly improved code coverage of `app/api/saved_searches.py` from 0% (missing configuration imports during tests) to 100% by testing previously untested edge cases including:
- Reaching the maximum saved search limit per user.
- Database commit errors (`HTTP_500_INTERNAL_SERVER_ERROR`) during create, update, and delete actions.
- Validation failures for `filters` field checking for non-dict types (`status.HTTP_422_UNPROCESSABLE_ENTITY`).
- Conflicting names during updates where an existing saved search matches the new name.
- Proper fallback logic across authentication methods for `_get_user_id`.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
Modified `is_private_ip` in `app/utils/network.py` to fail securely by returning True (blocking the request) when a hostname cannot be resolved. The previous implementation failed open, creating a risk for Server-Side Request Forgery (SSRF) and DNS rebinding attacks.
Updated corresponding tests to expect the secure behavior and correctly appended the security finding to the Sentinel journal.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
- Renamed `save_dropbox_settings` inside `app/api/google_drive.py` to `save_google_drive_settings` to fix a copy-paste naming error.
- Extracted duplicate `.env` file updating logic from `app/api/google_drive.py`, `app/api/onedrive.py`, and `app/api/dropbox.py` into a new reusable helper function `update_env_file` inside `app/utils/settings_service.py`.
- Refactored the three API endpoints to use the new helper function, significantly reducing complexity and code duplication.
- Updated relevant test files (`tests/test_api_google_drive_final.py`) to reflect the new function name.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
🚨 Severity: HIGH
💡 Vulnerability: User-provided IMAP `host` in `_test_imap_connection` and `pull_inbox` was not validated against private IPs, creating an SSRF risk.
🎯 Impact: Attackers could abuse the endpoints to port-scan or interact with internal/private network services.
🔧 Fix: Integrated `is_private_ip` from `app.utils.network` to block connections resolving to private, loopback, link-local, or reserved IPs.
✅ Verification: Ran `test_imap_tasks.py` and `test_api_imap_accounts.py` successfully. Checked `ruff` output and diffs. Removed all scratch files from the commit.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
🚨 Severity: HIGH
💡 Vulnerability: User-provided IMAP `host` in `_test_imap_connection` and `pull_inbox` was not validated against private IPs, creating an SSRF risk.
🎯 Impact: Attackers could abuse the endpoints to port-scan or interact with internal/private network services.
🔧 Fix: Integrated `is_private_ip` from `app.utils.network` to block connections resolving to private, loopback, link-local, or reserved IPs.
✅ Verification: Ran `test_imap_tasks.py` and `test_api_imap_accounts.py` successfully. Checked `ruff` output and diffs. Removed all scratch files from the commit.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
The `preview_migration` function in `app/utils/db_migrate.py` used string
interpolation to dynamically execute a COUNT query on the source database
(`f"SELECT COUNT(*) FROM {quoted_table}"`).
While the table name was quoted via the dialect's identifier preparer and
validated with a regex, string interpolation for raw SQL should be avoided
as it represents an anti-pattern and a theoretical risk for SQL injection
if validation controls are ever bypassed or modified.
This commit replaces the raw string interpolation with safe, parameterized
SQLAlchemy Core query construction `select(func.count()).select_from(table(table_name))`,
which automatically handles table quoting and execution safely. It also removes
the unused `text` import to keep the code clean.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
- Renamed `save_dropbox_settings` inside `app/api/google_drive.py` to `save_google_drive_settings` to fix a copy-paste naming error.
- Extracted duplicate `.env` file updating logic from `app/api/google_drive.py`, `app/api/onedrive.py`, and `app/api/dropbox.py` into a new reusable helper function `update_env_file` inside `app/utils/settings_service.py`.
- Refactored the three API endpoints to use the new helper function, significantly reducing complexity and code duplication.
- Updated relevant test files (`tests/test_api_google_drive_final.py`) to reflect the new function name.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
🚨 Severity: HIGH
💡 Vulnerability: User-provided IMAP `host` in `_test_imap_connection` and `pull_inbox` was not validated against private IPs, creating an SSRF risk.
🎯 Impact: Attackers could abuse the endpoints to port-scan or interact with internal/private network services.
🔧 Fix: Integrated `is_private_ip` from `app.utils.network` to block connections resolving to private, loopback, link-local, or reserved IPs.
✅ Verification: Ran `test_imap_tasks.py` and `test_api_imap_accounts.py` successfully. Checked `ruff` output and diffs.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
Move the valid filename regex pattern to a shared constant in `app/utils/filename_utils.py` and update both the task logic and security tests to use it. This eliminates duplication and ensures consistency across the codebase.
Normalized line endings in `app/tasks/extract_metadata_with_gpt.py` from CRLF to LF to ensure consistency and prevent CI issues.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
The transition to a newer FastAPI/Starlette version changed the signature of `Jinja2Templates.TemplateResponse` from `(name, context)` to `(request, name, context)`.
The `app/views/base.py:template_response_with_version` wrapper naively forwarded positional arguments `*args` to `original_template_response`. This caused the template name (`"files.html"`) to be passed as the `request` parameter, and the context dictionary to be passed as the `name` parameter. This resulted in Jinja2 attempting to cache the template using a dictionary as the cache key, which triggered a `TypeError: unhashable type: 'dict'`.
This commit updates the wrapper to automatically translate the legacy positional arguments `(name: str, context: dict)` into the explicit keyword arguments `request=context.get("request"), name=name, context=context` required by modern Starlette, preventing template rendering crashes across the application and restoring passing CI test suites.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
Move the valid filename regex pattern to a shared constant in `app/utils/filename_utils.py` and update both the task logic and security tests to use it. This eliminates duplication and ensures consistency across the codebase.
Also normalized line endings to LF in affected files to ensure CI compatibility.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
Added `mock_task.delay.assert_called_once_with(str(test_file))` to all integration tests involving background task enqueuing in `app/api/process.py` endpoints to ensure background tasks are called with the correct file path arguments.
Co-authored-by: christianlouis <361235+christianlouis@users.noreply.github.com>
Christian Krakau-Louis