Ed's diagnosis, and it is the root of three symptoms I had been patching
one at a time: a job has two independent statuses, and the panel was
collapsing them into one column.
* STATE — where the run is in its lifecycle: running, paused,
cancelled, completed, failed.
* OUTCOME — how the work turned out: ok, issues, notices, err.
They are orthogonal. A paused run has no outcome yet. A completed run's
outcome may still be "issues". Conflating them produced, in order:
1. a paused migration rendering as a green "ok" — the outcome was
genuinely ok, the STATE was Paused, and only the outcome was shown;
2. my first fix, which put "blocked" into the OUTCOME column — a
category error, encoding lifecycle into the result axis;
3. a cancelled job still reading "blocked", because that outcome was
cached in memory while the cancel had flipped the row in SQL.
The layout already had both columns. State just never rendered anything
but "running" or "—", so the status axis had no home and the information
leaked into Outcome.
Now:
* State renders `last_run_status`, sourced from the run ROW. Memory
cannot answer this — it is empty after a restart and stale after a
cancel, both of which the row gets right. The retryable reason, when
there is one, is the pill's tooltip.
* Outcome goes back to describing only the work: ok / issues /
notices / err. No lifecycle in it.
`JobSummary` gains `last_run_status`, and `last_run_at` falls back to
the row's `started_at` when memory has none — a restart left the column
reading "never" for a job whose last run was hours earlier.
"never" is now reserved for jobs that genuinely never ran. With a run
row present but no cached outcome the cell reads "—": the honest "no
outcome recorded", rather than a claim the run history immediately
contradicts.
The enrichment query generalises rather than multiplying — it already
fetched Paused rows for the Resume button, so it now takes the latest
row per job via `DISTINCT ON` and derives state, timestamp and paused
brief from it. Sound as "the current run" because the
`one_active_run_per_job` partial unique index permits one non-terminal
row per job and a resume reuses it, so a non-terminal row is always
newest.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Step 4 of docs/plan/jobs-handling-recoverable-error.md — the sharp edge
the plan flagged, and it was already a live trap independent of the
retry work.
`backend_migration` engages `migration_readonly`, which refuses writes
ACROSS THE WHOLE APPLICATION until cutover. Cancelling it cleared
nothing. The flag is persisted, so the state survived restarts — boot
even logs a warning about coming up read-only — and the only escape was
editing `admin_settings` by hand.
Two paths reach a cancel, and only one of them ran any handler code:
* a RUNNING row re-enters the handler, which now releases the gate at
its next cancel poll when the intent is terminal;
* a PAUSED row does NOT. `request_terminal_cancel` flips it straight
to Cancelled in SQL with no handler in the loop.
The second is the common case and the one that matters: a migration
paused by an outage, holding the freeze, cancelled by an operator
precisely to get writes back. Fixed in the cancel endpoint, which is the
only place that sees it.
Releasing on cancel is safe because cancel ENDS the run with no swap —
the source is still the active backend, so nothing is left to protect,
and a later retry starts fresh and rescans everything.
**Pause deliberately keeps the gate**, per Ed's call: Ops cancels to
release it. That is not conservatism for its own sake. The cursor is a
position in a hash-ordered walk and stays valid only while nothing
writes; release the gate on pause and a blob written afterwards whose
hash sorts BELOW the cursor is never visited, so the run completes,
flips the pointer, and reads for that hash 404 against a target that
never received it. Releasing on pause becomes safe only once resume
rescans from the start or a final catch-up pass runs under the freeze
before the swap — the plan's follow-up, not this commit.
Both release paths are best effort: a run that has already been
cancelled should not become a hard failure because a DB blip prevented
clearing a flag. The in-memory store happens regardless, so writes
resume in this process; a loud warning names the DB copy needing
attention.
The endpoint check is gated on the job name AND on the flag currently
being set, so it is a no-op for every other job — nothing else ever sets
it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`JobRunArgs` was a fixed struct — `force`, `deep`, `storage`, `repair` —
and six places hardcoded that same list: the engine's persist/restore,
the trigger endpoint's query type, the OXICLOUD_STARTUP_JOBS parser, the
frontend API wrapper, the panel's checkboxes, and `StartupTrigger` on
the wire.
Two costs. Adding a parameter meant editing all six, and forgetting one
dropped it silently — most damagingly in persist/restore, where a
resumed run lost it and a `?repair=true` migration came back as
discovery-only after a restart. And the panel offered the same knobs on
every job: only two jobs read `deep`, six read `repair`, so most of
those controls did nothing with no way to tell which.
Now `JobHandler::parameters()` returns `&'static [JobParam]` — name,
type (boolean/string/number), default, and the job's own description of
what it does. `JobRunArgs` holds a map keyed by those names.
Everything reads the declaration:
* `run_or_resume` iterates it to persist and restore, replacing
`const FLAGS` plus a `storage` special case. `storage` stops being
special — it was the one Option<String> among three bools.
* `dispatch` normalises every run against it, which is what makes "a
handler sees its declared parameters with their declared defaults"
true rather than usual. The periodic tick passes an empty
`JobRunArgs::default()`, so a `default: true` parameter would
otherwise read false on every scheduled run.
* The trigger endpoint takes free-form query params and rejects
undeclared ones with a 400 naming the real set, instead of ignoring
them.
* OXICLOUD_STARTUP_JOBS keeps raw pairs (config is parsed before the
registry exists) and validates at dispatch, where the error can name
the job's actual parameters. Still a boot panic, same as an unknown
job name — a typo'd `?repare=true` must not leave a migration
importing forever in discovery mode.
* `JobSummary.parameters` carries it to the panel, whose `supportsDeep`
was a hardcoded name allowlist (`consistency_batch ||
backend_consistency`). A job gaining a deep mode needed a frontend
release; one losing it left a button that silently did nothing. The
menu now renders from the declaration, so a newly-declared boolean
appears with no frontend change.
Three consistency tenants were hand-rolling persist-on-fresh /
restore-on-resume for their own flag, under the same `params` key the
engine already used. Deleted — they read `args.get_bool(…)` now.
Fresh runs also filter to the declaration. `consistency_batch` forwards
its args verbatim to sub-jobs, so a tenant's `params` row could grow
`deep` with no deep mode, and the run-detail view would claim a mode the
job never had.
Two things found while wiring it, both worth knowing:
`RecoverableAdapter` bridges the two traits, and `parameters` has to be
forwarded there or the registry sees `&[]`. Both traits have defaults,
so omitting it compiled cleanly — and the trigger endpoint then rejected
`?repair=true` on the very jobs that declare it, with
OXICLOUD_STARTUP_JOBS panicking at boot. Now covered by
`adapter_forwards_job_metadata_from_inner_handler`.
`TriggerJobQuery` was briefly a newtype over the map. `serde_urlencoded`
cannot deserialize a newtype struct at the top level, so axum's `Query`
rejected EVERY trigger with a 400 — even one with no query string —
before the handler ran. It reads exactly like the new validation
rejecting something, which sent the first diagnosis to the wrong layer.
Now covered by `trigger_query_extracts_from_every_url_shape`.
Wire names are a compatibility surface: `params` rows are keyed by them
and the panel switches on them, so a rename breaks existing run history
the same way renaming a `Mutates` variant does. The JSON shape is pinned
in `snapshot_carries_job_metadata`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Replace plain `==` on lock tokens with `subtle::ConstantTimeEq` at
every token-comparison site on the WebDAV surface. Closes a
reported timing side-channel (2026-09-05) in `evaluate_if_header`
where an authenticated attacker could theoretically recover another
user's active lock token via response-latency measurements on the
`If:` header state-token comparison.
Practical exploitability is marginal — the signal is tens-of-ns
buried under ms-scale network jitter, ~5×10⁸ samples needed per
token to average through the noise vs a default lock lifetime of
60 s to 1 h — but the fix is a five-line change with zero
measurable perf cost (`subtle` is already transitive via
sqlx-postgres → digest, so no new binary weight), and adopting
constant-time compare on any token that gates access matches the
hygiene rule the rest of the codebase already follows on password
and session paths.
Sites fixed:
* `evaluate_if_header` — first-pass state-token scan and
second-pass condition eval in `webdav_handler.rs`.
* `WebdavLockService::refresh` — `!= token` mismatch check.
* `WebdavLockService::release` — `== token` guard on the
by_path invalidation branch.
The two `WebdavLockService` sites are already gated by
`self.by_token.get(token)?` — the attacker cannot reach the
comparison without already presenting a valid token, so their
timing surface is nil in practice. Kept constant-time anyway for
callsite consistency.
Sweep confirmed no other secret-adjacent `==` in production code:
password verification goes through Argon2's `verify_password`,
session/CSRF/DPoP jti tokens are hashmap-gated, and blob-hash
equality compares two server-side values with no attacker-
controlled operand.
Reported-by: Abdurazzoqov Javohir <abdurazzoqovjavohir700-dev@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Writing the hurl scenario surfaced a gap: a transcode that comes out
larger than the original runs a full decode + encode and increments no
counter at all. `transcodes` is bumped only on the success path, beside
`bytes_saved`, so the most expensive failure mode was invisible — a
multi-megapixel image decoded and re-encoded on every request, for
every file sharing that content, producing nothing.
That is precisely the cost the persisted negative verdict exists to
stop paying, and it could not be measured before or after. `not_beneficial`
counts it, kept separate from `transcodes` because conflating "work
done" with "work that paid off" would hide exactly what an operator
needs to see.
It is also what lets the hurl scenario assert the negative half: the
first fetch increments it, the second — a distinct file with identical
content — leaves it untouched, which is the negative row being read
rather than the verdict recomputed.
Assertions are exact equality against captured values throughout, no
`>` or `<`. A "greater than" would pass if a counter moved for the
wrong reason; equality against the prior reading catches any transcode
from any source, including one this scenario did not intend to cause.
Also fixes two URLs the first runs caught: file download is
`GET /api/files/{id}`, not `/content`, and the trash listing is
`/api/trash/resources`. And the duplicate uploads go to a second
folder — re-uploading the same filename into the same folder returns
the EXISTING file id, which would have made both halves of every
"two files, one content" pair the same row and left the scenario
asserting nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds `GET /api/admin/transcode/stats` and a hurl scenario that uses it
to assert both halves of the caching contract.
The endpoint exists because the property was previously unobservable.
`derived_blob_copy.hurl` records the same limitation for thumbnails:
stored blob, RAM cache and a fresh re-render return identical bytes
with identical status, so no HTTP-level assertion can tell them apart.
Counters can. `transcodes` is work done; `cache_hits` and `disk_hits`
are work avoided, and a rising `transcodes` against a flat `disk_hits`
is exactly what a broken derived tier looks like from outside.
Each case uploads the same bytes as TWO distinct files. Re-fetching one
file would only prove moka works — that cache is keyed `{file_id}:{ext}`.
A second file with identical content is a guaranteed memory miss but the
same content hash, so avoiding a transcode there can only be the
content-keyed tier answering. That is the whole point of keying
derivations by content rather than by file, and this is the first test
that can see it.
The negative half is the one the row exists for: without it the server
re-runs a full decode + encode of a half-megabyte screenshot for every
file sharing that content, on every request, to discard the result each
time.
Assertions capture-then-compare rather than computing deltas — hurl has
no arithmetic in predicates, and pinning the exact prior value is
stricter anyway, since a transcode triggered from anywhere shows up.
Absolute values are never asserted: other scenarios in the same run
transcode too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A migration nobody triggers never finishes. Scheduled ticks deliberately
never pass `repair`, so a deployment whose operator never opens the
admin panel re-imported the same sidecars forever and never drained the
directory — and relying on operators to edit `.env` has the same failure
mode one level up.
`OXICLOUD_STARTUP_JOBS` dispatches named jobs once, in the background,
after the scheduler is ready. Entries use the syntax operators already
type at the trigger URL (`name?repair=true`), so the value is literally
the request they would otherwise make by hand. It defaults to both
migration jobs in repair mode, so an untouched deployment migrates and
drains itself.
That is a destructive default and a real exception to
no-silent-auto-repair, so the guard it rests on had to get stronger:
`verify_and_unlink` now compares CONTENT, not length. A blob of the
right size and the wrong bytes used to pass — a key-mapping bug handing
back another file's preview at the same length would have deleted the
original and kept the impostor, and thumbnails cluster tightly enough in
size for that to be a real coincidence. The readback streams from the
backend with no cache in front, so it proves durability rather than that
a write was acknowledged.
Deletion of `.thumbnails/` is attempted first and only falls back to
renaming it `.thumbnails.migrated` when `remove_dir` refuses because a
non-sidecar file is inside (Finder's `.DS_Store`). Either way the
directory stops existing, which lets the read-path probe go back to a
single `stat` on the root instead of walking the size directories.
Validation is fail-fast: an unknown job name or flag panics at boot. A
silently dropped `?repare=true` would leave the job in discovery-only
mode while the operator believed the tier was draining, surfacing months
later as "the migration never finished" with nothing pointing at the
config line.
Interrupted runs resume. Boot recovery flips abandoned rows to Paused
with their cursor, so `run_or_resume` continues rather than rescanning —
a long migration completes across however many restarts it takes. That
is a scoped exception to "we do not auto-resume": here somebody did ask,
in configuration, and not having to ask again is the point.
`StartupJob` holds a `JobRunArgs` rather than re-listing its four
fields, so a fifth flag cannot be added to the scheduler and silently
ignored in configuration.
Jobs named here are ordinary registered jobs — visible in the panel,
triggerable by hand, same runs and findings. Their rows now carry a
`startup` object so an operator can see that a job deletes on every boot
rather than only when someone clicks Run.
Adds docs/config/thumbnail-migration.md: what runs on first boot, how to
snapshot database and storage together beforehand, and how to verify
afterwards with satellites_consistency plus backend_consistency
?deep=true.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The write paths now persist only to the blob tiers. Until this, dual-write
meant any render or upload recreated .thumbnails/ seconds after the import
job removed it, so step 10e's gate — "the directory no longer exists" —
could never hold.
Rendered thumbnails: the fs::write in persist_rendered is gone. Safe
because the read flip landed first, so nothing depended on that write to
be found, and a failed derived store now costs a re-render rather than
data — regenerable by definition. Existing sidecars are untouched and stay
readable through the fallback until the import drains them.
Uploaded previews needed a change first, and the order was not optional.
upload_thumbnail_impl logged and still returned 201 when store_attached_blob
failed — safe only while ext-{file_id}.jpg was a second copy. These bytes
have NO server-side render path, so removing the sidecar while the store
stayed best-effort would lose a user's upload behind a success response.
The PUT is now fatal, and drops the RAM entry too, or the cache would keep
serving a preview that was never persisted and vanishes on eviction,
contradicting the error the client just received. Only then does the ext-
write go.
thumb_import_check.sh had to change with it: its premise was "upload, then
delete the row, and what remains on disk is legacy state", which no longer
holds now that nothing writes sidecars. It lays them down itself, with the
bytes the API just served, at the exact paths the pre-10d2 code used. The
reconstruction stays faithful — same bytes, same paths — it just no longer
depends on current code to produce a shape current code has stopped
producing. Both sidecars get identical bytes, which is realistic rather
than a shortcut: they dedup to one blob while keeping separate mappings,
which is the property the keying split exists to preserve.
Uploading without folder_id answered `500 Internal Error: folder_id is
required to determine file owner`. A missing required field is the
caller's error; as an internal_error it produced `error_type: Internal
Error`, which the SPA cannot distinguish from the server breaking — so a
malformed request looked like an outage.
Both sites become validation_error (ErrorKind::InvalidInput → 400), with
messages that say WHY the field is needed rather than restating that it
is: the destination folder determines the file's owner and drive.
The OpenAPI request body described it as "optional folder_id field",
which is how it came to be omitted — hit while writing
thumbnail_etag_content_keyed.hurl, where the upload was written from the
documented contract and 500'd. Now stated as required.
Regression test asserts the status AND that error_type is not "Internal
Error", since the contract the SPA switches on is error_type rather than
the message.
Thumbnails were served `public, max-age=31536000, immutable`. Two
problems, and the first is a security one.
`public` on a Permission::Read gated resource lets any shared cache — a
corporate proxy, a CDN — store one user's thumbnail and serve it to
another. `Vary: Accept` was no defence: it does not vary on
Authorization. Now `private`.
`immutable` was a promise this URL cannot keep. It is keyed by file id,
and its bytes change when a preview is uploaded, when content is
replaced, or when an attachment is removed. `immutable` tells a client
not to revalidate at all during the freshness lifetime, so with a
one-year max-age a browser that fetched once would never see a new
preview — which also made the content-keyed ETag unobservable in
practice. A correct validator is worthless if nothing asks. Now
`no-cache`, which still stores the body and only requires revalidation,
answered by the ETag with a body-less 304.
The hurl tests could not have caught this: hurl always sends the
request, so If-None-Match was exercised and passed while a browser
obeying `immutable` never got that far. Same "correct on the wire, wrong
in practice" shape as the bugs before it, so the test now asserts the
directives themselves rather than only the 304 behaviour.
One definition, shared by the REST and NextCloud endpoints, which are
gated identically and must not drift. /_app/immutable is untouched:
those are hash-named static assets, genuinely content-addressed and
public, where the directive is honest.
Cost is a conditional request per thumbnail per page load. Recovering it
needs a content-addressed URL — where `immutable` would be true — but
that puts the hash in the URL of an authorized resource, so it stays
`private` regardless, and it touches the SPA and the file DTO. Separate
change.
fe9c4f49 keyed the ETag on the SOURCE file's content hash. That is wrong
whenever the response comes from a satellite table, and for attachments
it is wrong in two ways.
Uploading a preview does not change the file's content, so a
source-keyed ETag does not change either — and with `immutable` set,
clients never revalidate and keep the previous render for up to a year.
The exact staleness fe9c4f49 set out to fix, re-entering through the
attachment path.
Worse: a copy inherits the source hash, so an original and a copy have
identical ETags. Give either one a different uploaded preview and they
serve different bytes under one validator, which a shared cache may hand
to either request. That is a collision, not just staleness.
thumbnail_content_id resolves the identity through the same tier
precedence the read path uses: an attached blob's own hash, else a
derived blob's own hash, else the source-keyed form. An ETag naming a
different tier than the one answering is worse than a coarse one, so the
two orders must not drift.
Derived-hash keying is strictly better than source-keying and never
worse. The sidecar and the derived row are written from the same bytes;
where they can diverge — a sidecar re-rendered while the derived row
stays pinned by ON CONFLICT DO NOTHING — source-keying is wrong too,
because the renderer is not part of that key. This is the step 10 change
arriving early, forced by the attachment case; the plan note stands for
the read-order flip itself.
Known gap: a legacy ext-{file_id}.jpg with no file_attached_blobs row
yet falls through to the source-keyed form. No worse than today, and it
resolves when the import backfills.
attached_thumbnail_copy.hurl now asserts ETags, which is why this went
unnoticed: it compared bytes only, and thumbnail_etag_content_keyed
covers content replacement rather than preview upload. A fresh GET
returned the right bytes throughout — the same "healthy locally, broken
for anyone caching" shape as the two bugs before it.
1. Per-file overrides must beat the content tier in RAM as well as on
disk. The content-keyed lookup ran first, so a thumbnail already
rendered from the file's content sat in RAM under content(blob_hash)
and shadowed a preview uploaded afterwards — permanently. Invisible
before the moka rekey, because both lived under one file-id key and
the upload simply overwrote the render. Order is now uniform:
per-file RAM, per-file disk (ext-), per-file DB, then content RAM,
blob-hash disk, derived blob.
2. store_attached_blob never wrote a row. Its RETURNING clause compared
the stored hash against EXCLUDED, and PostgreSQL only permits
EXCLUDED in the SET and WHERE of DO UPDATE — a runtime syntax error
on every call. The superseded hash now comes from a SELECT taken
before the upsert; losing that race leaves one stale reference, which
the manifest recompute reports, rather than anything being lost.
The second hid behind the first for a whole cycle, and behind
`ext-{file_id}.jpg`: the ORIGINAL kept serving its uploaded preview from
local disk, so the feature looked healthy. Only a copy, which has a
different file_id and therefore no ext- file, depends on the row — and
the row was never there. The handler's best-effort warn! completed the
disguise, so it is now error!: a failure there means copies silently
lose the preview, and nothing else signals it.
Completes step 9. The PUT wrote `ext-{file_id}.jpg` and nothing else —
keyed by file id, on local disk. No copy path duplicates it and no other
instance can see it, so a copied file lost the preview its owner
uploaded. Silently: the server falls back to rendering one from the
source, or to 204 for a PDF, which has no render path at all. A
user-supplied preview is not derivable from the content, so once lost it
is gone.
The PUT now also records a storage.file_attached_blobs row, which
copy_file_satellites already duplicates, so both copy paths carry it.
Best-effort: the sidecar has already succeeded by then and the user can
see their thumbnail, so failing the request would report an error for an
operation that visibly worked.
Read path consults attachments ahead of every content-derived tier: an
uploaded preview is an explicit choice about THIS file and must beat
anything rendered from its content. Cached under the per-file key — a
content key would leak those bytes to every other file sharing the
content, which is the poisoning the file-keyed table exists to prevent.
store_attached_blob is ON CONFLICT DO UPDATE, unlike its derived twin:
re-uploading a preview is a deliberate replacement, where a re-derived
thumbnail is the same bytes again. The superseded blob's reference is
released, or it would be pinned forever with nothing pointing at it.
Deletion goes through a trigger, not a hook. file_id is ON DELETE
CASCADE, and on_file_deleted fires AFTER delete_file — by then the
cascade has run and there is nothing left to enumerate. This matters
most for folder deletion, where PG cascades folders to files to
attachments and Rust never sees the rows at all. storage.decrement_blob_ref
keys off OLD.blob_hash and is otherwise table-agnostic, so it is reused
verbatim rather than transcribed into a second trigger that can drift.
DELETE only: a replacement updates in place and is handled in Rust, so
adding UPDATE would double-decrement.
Extracted read_blob_to_bytes, shared by the attached and derived tiers —
the only difference between them is which table produced the hash.
tests/api/attached_thumbnail_copy.hurl guards it. The file is red and
the uploaded thumbnail is green, so a render could never produce the
uploaded bytes; the pre-upload render is captured first and required to
change, which stops three identical renders from satisfying the
byte-equality. Then both copy paths must serve the upload, and after the
original is purged and GC runs, both copies must still serve it — each
holds its own reference, because the rows are duplicated rather than
shared.
The thumbnail ETag was "thumb-{file_id}-{size}-{format}", sent with
Cache-Control: public, max-age=31536000, immutable. Replacing a file's
content preserves its id — file_upload_service rebuilds the entity with
parts.id and a new hash, then fires on_file_updated, which deletes and
regenerates the thumbnails — so the server produced a new thumbnail while
still advertising the old ETag. Because `immutable` tells a conforming
browser not to revalidate at all inside the freshness window, clients kept
rendering the previous image for up to a year, unfixably.
Keyed on the content hash the directive becomes honest: a thumbnail is a
pure function of (source bytes, size, format), so that triple identifies
the response. New content yields a new ETag.
The same change fixes the opposite direction. A copy, or any dedup twin,
had a different id and therefore a different ETag, so clients refetched
bytes they already held even though both are served from the same derived
blob. Now identical content agrees on an ETag and revalidates to 304
across files, users and copies.
Both thumbnail endpoints were affected: the REST handler and the
NextCloud preview handler.
Cost is one PK lookup ahead of the 304 decision, where the id-keyed
version needed none — paid for by no longer serving stale images. It is
partly recovered: both handlers already resolved the same hash further
down for the render path, and that second lookup is now gone, so the
cache-miss path is unchanged and only the 304 path pays. The resolved
hash is also handed to get_cached_thumbnail instead of None, saving the
service its own lookup.
No new disclosure: content_hash is already on FileDto and returned by
GET /api/files/{id}.
Tests: thumbnail_etag_content_keyed.hurl covers invalidation — overwrite
in place via WebDAV PUT, assert the ETag changed, assert a client holding
the stale one gets 200 rather than 304. derived_blob_copy.hurl gains the
sharing direction: a copy answers with the SAME ETag and revalidates to
304, which is the one externally observable consequence of content-keying
and was not previously testable.
Step 5, read path — Option 2 of the two shapes discussed: the derived
blob is consulted LAST, after the sidecar, not first.
Read order is now
moka -> ext-{file_id}.jpg -> {blob_hash}.webp on disk -> derived blob
For every thumbnail already on disk the new branch is never reached, so
the database stays off the hot path and a fault in it cannot break a
working gallery. It answers only what disk cannot: a thumbnail rendered
by another instance, or a box whose sidecar was never populated. Legacy
content keeps serving from disk until `derived_import` migrates it.
That inverts the plan's stated order deliberately. Derived-blob-first is
right for the END state, because it is what lets the sidecar be deleted;
sidecar-first is right transitionally, because the risky reordering
should happen after the table has been seen serving real reads. The flip
belongs in the release that removes the sidecar, and the comment at the
branch says so.
The existing precedence is preserved and now documented: the file-keyed
client upload (ext-) is checked BEFORE the content-keyed server render.
That ordering is a security property, not a preference — content-keyed
artifacts are shared across every file with that content, so checking
the file-keyed one first is what keeps one user's uploaded preview from
ever being served for another user's identical file.
Shape notes:
* `find_derived_blob` lands on DedupPort/DedupService as the read
counterpart of `store_derived_blob`, so ThumbnailService needs no pool
field — and therefore ThumbnailService::new, DI and three tests are
untouched.
* It carries `content_type`, which is what will retire the byte-sniffing
in the handlers once reads are table-primary.
* The parameter is `Option<&DedupService>`, concrete rather than
`&dyn DedupPort`: DedupPort uses native `async fn` and so is not
dyn-compatible, and ThumbnailPort is never used as a trait object
(checked) — both handlers hold the concrete Arc. `None` means
sidecar-only, which is exactly today's behaviour and what the abstract
port impl passes.
fmt, clippy --all-features --all-targets, 35 unit tests clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
link are checking that email matches, +email alias are normalize into email
if email is already used on another account, link is not possible
not usurpation risk as the IDP is choosen by the admin
this make OIDC compliant with the invariant binding (issuer and subject)
admin can now rename their provider without breaking
clarifing federation_kind: report the kind of federation wired not the allowed login method
hybryd login method are still allowed
Edouard Vanbelle