- 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>
copilot-swe-agent[bot]