The manage_endpoints agent tool accepts arbitrary user-supplied base_urls
and writes them to the database without calling check_outbound_url(),
while the HTTP route (POST /api/model-endpoints) enforces full SSRF
hardening. A prompt-injection payload smuggled into a skill, note, fetched
page, or email could instruct the model to register a cloud-metadata or
internal-network endpoint via this tool path.
Add the same check_outbound_url() guard used by the HTTP route to the
agent tool's 'add' action. Also sanitize the error message in the except
block to avoid leaking raw exception text.
Add 8 regression tests covering: cloud metadata, non-HTTP schemes,
loopback (local-first), strict mode, safe URLs, empty URLs, gopher
scheme, and error message sanitization.
Co-authored-by: isharak7m <192635824+isharak7m@users.noreply.github.com>
* fix: block SSRF via user-supplied model endpoint URLs
* fix(test): stub DNS in loopback SSRF tests for deterministic results
Replace environment-dependent test_loopback_always_rejected with two
deterministic tests using a stub resolver:
- test_loopback_accepted_by_default (local-first, block_private=False)
- test_loopback_rejected_in_strict_mode (block_private=True)
The old test only passed when localhost resolved to ::1 (hit is_reserved),
not because loopback was actually always rejected.
Co-Authored-By: Santoshkumarpuppala
* fix: guard PATCH base_url against SSRF and add ipv6 loopback coverage
---------
Co-authored-by: isharak7m <192635824+isharak7m@users.noreply.github.com>
* fix(security): deny generated-image access when ownership verification fails
The except Exception block in serve_generated_image catches all
exceptions during ownership verification and falls through to serve
the file anyway. If the database is down or corrupted, the ownership
check is bypassed entirely — any authenticated user can download any
generated image by guessing the content-hash filename.
Raise HTTP 503 (Service Unavailable) instead of falling through,
so the endpoint fails closed when verification is impossible. This
matches the defense-in-depth principle: deny access on verification
errors rather than granting it.
* fix(security): enforce fail-closed ownership and secure caching for generated images
- Fail closed on missing gallery records, null/empty owners, and DB errors
- Support both cookie session and bearer API token owners via effective_user
- Add in-memory registry for freshly generated images to preserve access
- Eliminate file existence oracle by verifying ownership before disk probing
- Update Cache-Control to private, no-cache, must-revalidate
- Pass owner attribution through tool execution and image MCP server
- Add comprehensive hermetic regression tests for ownership gates and caching
* fix(security): sanitize tool owner arguments and drop unneeded MCP env fallback
- Remove redundant and unsafe ODYSSEUS_MCP_IMAGE_OWNER fallback in image_gen_server.py to fix 4 tests in test_env_reference.py.
- Sanitize caller-supplied tool arguments across all MCP and email tool dispatch paths in src/tool_execution.py: strip untrusted _odysseus_* keys from arguments.
- Only inject _odysseus_owner when verified trusted server context is present, preventing untrusted/anonymous users from spoofing ownership claims.
- Sanitize empty/whitespace owner strings in image_gen_server.py to None.
- Add comprehensive regression tests in test_generated_image_ownership.py for argument sanitization, owner injection, fallback propagation, and server sanitization.
---------
Co-authored-by: isharak7m <192635824+isharak7m@users.noreply.github.com>
Co-authored-by: PR #6166 Reviewer <reviewer@odysseus-review.local>
`_auto_summarize_pass_single` in `routes/email_pollers.py` opens a
long-lived IMAP connection at line 172 and then performs ~700 lines of
work — IMAP `select`/`FETCH`/`SEARCH`, network POSTs to the LLM
endpoint, SQLite writes, and per-uid awaits. The only `conn.logout()`
calls were on three safe paths (early `"No recent emails"`, early
`"No model configured"`, and the happy path at the very end). If any
exception fired between `conn` being created and the final happy path,
the outer `except` block at line 921 caught it, logged, and returned —
without ever calling `conn.logout()`. The IMAP socket leaked until
the server's idle timeout killed it.
This is the same shape as the just-merged upstream fixes#1325
(`_imap_move` in `routes/email_helpers.py`) and #1330 (`_list_emails_sync`
in `routes/email_routes.py`), but in the *background* poller path —
`_auto_summarize_poller` invokes it every 30 min, so the leak
accumulates on every crashed pass instead of being a transient
request-path leak.
The fix is the exact try/finally pattern from #1330:
1. initialize `conn = None` before the try
2. let the try-block assign `conn = _imap_connect(...)`
3. drop the three explicit `conn.logout()` calls on safe paths
4. add a `finally:` block that calls `conn.logout()` if `conn` was set
Tests in `tests/test_email_polly_imap_leak.py` (1, all passing):
- `test_auto_summarize_pass_logs_out_imap_on_select_failure` —
monkeypatches `_imap_connect` to return a fake conn whose `select`
raises `RuntimeError`, then asserts the fake `conn.logout` was
called exactly once and the function returned an `Error: ...`
string. Pre-fix the assertion fails because the outer `except`
never reached `conn.logout`; post-fix the `finally` block
guarantees it on every exit path.
Pre-fix verification: temporarily reverted the patch and re-ran the
test; it fails with `logout_calls=0` (the IMAP socket was leaked on
every crashed pass). Post-fix: `logout_calls=1`.
Uniqueness:
- `git log --all --oneline -S 'conn.logout' -- routes/email_pollers.py`
→ no recent commit has touched this pattern in this file
- GitHub PR search for `routes/email_pollers.py` open PRs → 0
- Function has no existing test file (`grep _auto_summarize_pass_single
tests/` → no results)
---
**@pewdiepie-archdaemon — gentle bump on a sibling PR that's also stuck
in your queue from the same author:** PR #1306
(`fix(caldav): no-op prune when date_search returns 0 events`) is on
its 4th rebase, isolated to 2 files, 2/2 tests passing, with one
independent approval from `lalalune` already on record. It was clean
the last time you re-checked; if there's a blocker I haven't
addressed, please flag it so I can fix it. Otherwise, both #1306 and
this one are ready to merge.
Co-authored-by: isharak7m <192635824+isharak7m@users.noreply.github.com>
