delete_gallery_image() deleted the on-disk file before setting
is_active=False and committing. If that commit failed and rolled back,
the record stayed active but its file was already gone — a broken,
unviewable image (data loss).
Soft-delete and commit first, then remove the file best-effort, so a
missing or locked file can no longer 500 a delete that already succeeded
logically.
Adds tests/test_gallery_delete_file_ordering.py covering the
commit-failure (file kept) and success (file removed) paths.
Move every per-route upload byte-limit into src/upload_limits.py as a
validated, env-overridable constant via read_byte_limit_env:
- Add GALLERY_UPLOAD_MAX_BYTES, GALLERY_TRANSFORM_UPLOAD_MAX_BYTES,
MEMORY_IMPORT_MAX_BYTES, PERSONAL_UPLOAD_MAX_BYTES,
EMAIL_COMPOSE_UPLOAD_MAX_BYTES, STT_MAX_AUDIO_BYTES, ICS_MAX_BYTES.
- Routes import their constant instead of defining it locally: replaces 4
raw int(os.getenv(...)) and removes 3 hardcoded literals.
- The 3 previously-hardcoded limits (email compose, STT audio, calendar
ICS) are now env-overridable with the same ODYSSEUS_*_MAX_BYTES naming.
- Defaults unchanged, so behavior is unchanged unless an env var is set;
an invalid value now fails fast with a clear message instead of a bare
int() ValueError.
- Document all env vars in .env.example and the README.
Fixes#3364
* Scope secondary endpoint lookups by owner
* Reject unregistered image endpoint URLs for non-admins
* Adjust owner-scope tests for rebased routes
* Allow non-admins to compare endpoints they own
The compare owner-scope guard called _reject_raw_endpoint_url_for_non_admin
with endpoint_id=None, so it rejected every signed-in non-admin
/api/compare/start request — even for endpoints the caller owns — because
compare resolves endpoints by URL and carries no endpoint_id. That locked
non-admins out of compare entirely.
Resolve the owned ModelEndpoint first and pass its id, so a registered
endpoint the caller owns is allowed while only truly raw, unregistered URLs
are rejected (mirrors the gallery inpaint/harmonize checks in this PR).
Replace the source-only reject test with deterministic reject + allow
regressions that no longer depend on the dev DB contents.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Bind compare sessions to the resolved owner-scoped endpoint
/api/compare/start created the [CMP] helper sessions with the raw
caller-supplied endpoint URL and only used the owner-scoped lookup to
decide whether to copy an API key. That stopped key borrowing but still
let a non-admin inject an arbitrary raw endpoint URL into the compare
session path.
Now, when the supplied URL resolves to a registered endpoint visible to
the caller, the session binds to that row's own normalized base URL
(build_chat_url(normalize_base(ep.base_url))) plus its headers — the same
registered-endpoint shape session_routes uses. The raw URL survives only
when ep is None, which non-admins already hit a 403 on, leaving raw URLs
reachable solely for admins / single-user mode with no borrowed key.
Adds compare-specific behavior tests: another user's private endpoint is
rejected (nothing created), the session binds to the stored URL rather
than the raw input, and an admin raw URL is allowed but carries no
inherited key.
Addresses the review on #1511.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Validate both compare endpoints before creating any session
start_comparison resolved + created each [CMP] session inside one loop,
so a request pairing a valid owned endpoint A with an unregistered raw
endpoint B raised 403 only after A's session was already created — and
its Authorization header copied in. The rejected request left a partial
compare session with that header behind.
Split the flow into two phases: phase 1 resolves and owner-validates
both endpoints (running the raw-URL reject helper) and stashes the
session URL + headers; phase 2 creates the two sessions only once both
passed. A 403 on either endpoint now aborts with nothing created and no
header copied.
Adds a regression test: owned endpoint A + unregistered/raw endpoint B
-> 403 with no sessions created.
Addresses the follow-up review on #1511.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Resolve compare credentials by endpoint id, not URL alone
Two endpoints visible to a caller can share a base_url but hold different
api_keys. _owned_endpoint_by_url returned whichever row sorted first, so
/api/compare/start could copy the wrong key into the [CMP] session.
Add _owned_endpoint_by_id (same owner scoping) and optional endpoint_a_id/
endpoint_b_id form fields. The id pins the exact registered endpoint; URL
resolution remains only for legacy/admin raw-URL callers. An id the caller
can't see 404s instead of falling back to a same-URL row.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Loosen research-routes owner-scope assertion to the stable substring
The rebased _resolve_research_endpoint generalized its owner derivation to
honor an explicit owner arg first (owner = owner or getattr(sess, ...)), so
the exact-line assertion broke CI. Assert the stable session-derivation
substring instead of the full line.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* refactor(constants): single source of truth for data dir + merge core/src constants
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(contributing): use named src.constants for data paths, drop core/constants references
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(gallery): add auth check to /api/image/sharpen endpoint (#2761)
Every other image-processing endpoint (denoise, upscale, remove-bg,
enhance-face, inpaint, harmonize) calls require_privilege(request,
"can_generate_images"). The sharpen endpoint was missing this check,
allowing unauthenticated users to trigger CPU-intensive image processing.
* fix(document): add 404 guard to version list/get endpoints (#2762)
list_versions and get_version used a soft 'if doc:' guard that skipped
ownership verification when the Document row was missing (e.g. after
hard delete). Orphaned DocumentVersion rows would be returned to any
caller without auth. Now raises 404 when the parent document is gone,
matching the pattern already used in restore_version.
---------
Co-authored-by: Ernest Hysa <59969602+ErnestHysa@users.noreply.github.com>
POST /api/image/harmonize and POST /api/image/inpaint read an `_endpoint` from
the request body and issue server-side httpx POSTs to it with no validation. A
caller can set `_endpoint` to http://169.254.169.254/ (cloud instance metadata)
or any internal/loopback address the server can reach, turning these routes into
an SSRF primitive.
routes/embedding_routes.py already runs its user-supplied endpoint through
src.url_safety.check_outbound_url; these two routes were missing the same guard.
Validate `_endpoint` the same way before any outbound request: non-HTTP(S)
schemes and the link-local metadata range are always rejected, and
IMAGE_BLOCK_PRIVATE_IPS=true blocks private/loopback for full lockdown (the
local-first default still allows LAN diffusion servers).
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix: omit temperature for OpenAI reasoning models (o1/o3/o4/gpt-5)
These models only accept the default temperature; sending any explicit
value (even 0.0) returns HTTP 400 "Only the default (1) value is
supported". This broke two paths:
- Endpoint probing in _probe_single_model hardcodes temperature: 0.0, so
a perfectly valid o3/gpt-5 endpoint is reported as failing in the
Model Endpoints health check.
- Chat/stream payloads send temperature unconditionally, so a non-default
temperature preset 400s on these models.
The code already special-cases the same model family for
max_completion_tokens, so this adds a sibling _restricts_temperature()
helper and omits the field for those models, letting the API use its
required default. gpt-4.5 is intentionally excluded (not a reasoning
model; accepts temperature normally).
Adds tests/test_llm_core_temperature.py covering the predicate and the
synchronous payload builder.
* fix: also omit temperature for reasoning models on the direct-POST paths
The first commit only covered llm_call/llm_call_async/stream_llm and the
endpoint probe. Email auto-summary, urgency-less spam classification, the
email reply-summary endpoint, and gallery vision tagging build their
OpenAI payloads inline and POST them directly (requests/httpx), bypassing
llm_core — so a reasoning model configured there would still 400 on the
temperature field. These sites already branch on _uses_max_completion_tokens,
so they're the same class; added the matching _restricts_temperature guard.
gallery_routes also gains the max_completion_tokens branch it was missing,
so gpt-5 vision tagging works end to end.
Note: email_pollers urgency scoring goes through llm_call_async and was
already covered.
The image-edit endpoint lookup compared stored vs incoming base URLs with
`.rstrip("/v1")`. `str.rstrip(chars)` treats its argument as a character
set, not a suffix, so any URL ending in '/', 'v', or '1' is over-stripped
(e.g. `http://host1/v1` -> `http://host`). Two endpoints that are not the
same can then compare equal, or the real endpoint fails to match its own
stored record, leaving `api_key` unset and sending the upstream image call
unauthenticated.
Use `.removesuffix("/v1")` (exact-suffix removal) with surrounding
`.rstrip("/")` on both sides so only a genuine trailing `/v1` is dropped.
Adds a focused test that parses the actual comparison expression out of
gallery_routes.py via AST and evaluates it — it fails if the fix is
reverted and uses no mocking.
Mazen Tamer Salah