The proxy dial exemption was keyed on addresses learned lazily from earlier
proxied requests, so a direct request (loopback, NO_PROXY or an unproxied
scheme) or a redirect to the proxy's host:port was dialed unguarded. Derive
the exempt addresses once from the configured or environment proxy URLs and
reject direct requests to them with ssrf.ErrProhibitedIP.
Address pr-swarm finding: SSRF guard bypass via the proxy address
Both `usernamefallback` + `emailfallback` on: lookup required username
AND email match, so local user with different username never linked,
duplicate account created. Now verified-email-only lookup always tried
last.
Fixes#2022
## How to verify
1. Create a local user `localuser` with email `me@example.com`.
2. Configure an OIDC provider with `usernamefallback: true` and
`emailfallback: true`. In the IdP, create a user whose email is
`me@example.com` (sent with `email_verified: true`) and whose
username/sub is different from `localuser`.
3. Log in to Vikunja through that OIDC provider.
4. **Expected:** You are logged in as `localuser`, and `vikunja user
list` shows no new user for the provider's issuer.
**Before this PR:** a second user with a random or IdP-provided username
was created.
---------
Co-authored-by: kolaente <k@knt.li>
The files row is inserted before the blob is written, and the local backend creates the file before copying into it, so a failed copy left a partial blob behind once the row was rolled back.
The vikunja-file import checks the user's stored files against
migration.vikunjafile.maxuserstorage. The upload is now one of those
files, so an import counted its own zip against the quota. Exclude files
referenced as a migration upload.
The upload was spooled to the receiving instance's temp dir and only its
name travelled with the event. That holds while events stay in-process,
but it ties the job to the instance that accepted the upload.
Store it through the files package on the configured backend and record
its id on the migration status, so the event carries only the status id
and any instance can run the import. The job copies it to a local temp
file first: the zip importer needs random access, which an S3 object body
cannot give.
The upload is removed once the import is over. An hourly cron deletes
leftovers from events lost on restart or instances that died mid-import.
The config was unmarshalled up front purely to produce a 400, then parsed
again by SetOptions, and a third time in the background job. Drop the
throwaway parse and give SetOptions a domain error that maps to 400 on
its own.
A file import ran inside the upload request. A big export takes minutes,
far longer than a reverse proxy holds a request open, so the client got a
502 while the import kept running and kept the user's migration slot, and
there was no way to cancel or retry it.
The upload request now only validates what is cheap to validate, claims
the slot, spools the upload into a per-process temp directory and queues a
FileMigrationRequestedEvent. FileMigrationListener runs the import through
the lifecycle it now shares with the OAuth migrations: stale event skip,
panic recovery, heartbeat, outcome and notifications. The vikunja-file
migrator rejects a non-zip or structurally broken export before queueing;
ticktick, wekan and csv would have to parse the whole upload, so they fail
in the job and report through the status and failure mail. The csv
migrator receives its column mapping as options carried with the event.
Split opening the zip and indexing its entries out of Migrate, so every
structural check that doesn't need entry contents runs in one place before
anything is imported. The missing VERSION check moves into that scan.
A finished migration_status row looked the same whether the import worked
or failed, so the client could only guess. Rows now carry error_kind, a
closed set the client translates (reported, interrupted, credentials,
queue, upload, detail), and error_message, set only for "detail": the
error caused by the user's own data, the same text the failure mail sends.
A failure we report to Sentry stores just the kind, so server internals
never reach the status.
The migration was already finished and its claim released when the done
notification failed, yet the error was returned and the listener then sent
a failure mail and overwrote the finished row. Log it instead. The failure
mail's own log line also printed the migration error instead of the
notification error.
The claim timeout was measured from started_at, so a long import either
kept a dead claim for a day (the old 24h default) or had its slot taken
over while still running. A running migration now refreshes heartbeat_at
at a tenth of the timeout, clamped to 1s..30s, and a claim only counts as
stale once its last beat is older than the timeout. Rows that never beat
fall back to started_at. The default timeout drops to 5m accordingly.
Also bind the stale cutoff in the configured timezone: xorm writes times in
the engine's zone but binds query args as-is, which broke the comparison on
SQLite.
A second import sent while the first claim was still being written could
fail the insert with whatever the driver reports under contention, which
surfaced as a 500. The claim now reads the running row before writing so a
losing request stays read-only, re-reads the live claim whenever the insert
fails, retries lock collisions a few times, and probes for a stale claim
before taking a write lock to release it.
SQLite dumps can contain empty strings in json columns (e.g. project_views.filter),
which PostgreSQL rejects with "invalid input syntax for type json".
Fixes#3895
mcp.New builds the tool index from the v2 document and serves a stateless
per-request server: JWTs are rejected, mcp:access is required, tools/list is
filtered by the caller's token, and cross-origin browser requests are
refused unless the CORS configuration trusts the origin.
Tools are built from the finished document: path and query parameters plus
the JSON body become one input schema, update tools take their schema from
the sibling PUT because AutoPatch strips $ref, and an explicit allow-list
decides which operations are typed tools, catalog actions, or hidden.
Top-level and related tasks are collected first and created in one batch
so preset indexes survive, related references resolve to the created
struct instead of a mutable id lookup, untitled relation stubs are skipped,
and positions and buckets remap through one old-id map.
After a wipe the init-schema path marks every migration as applied, so
the backfill never runs on the restored rows and every project would be
invisible.
`POST /api/v1/oauth/authorize` and `POST /api/v1/oauth/token` are served
but missing from the generated OpenAPI docs: the handlers in
`pkg/modules/auth/oauth2server` carry no swagger annotations, so neither
path is in `swagger.json` — 126 paths, no `oauth` among them.
Found while building a third-party client. The API answers fine; the
docs say the endpoints don't exist, so a client that validates its calls
against them can't use OAuth at all.
Adds annotations for both handlers and regenerates `pkg/swagger`. No
behaviour change — comments only, plus generated output. `mage
generate:swagger-docs` produces 521 insertions and 0 deletions, so
nothing else in the spec moved.
Checked against a 2.5.0 instance: `OPTIONS` on both paths returns
`Allow: OPTIONS, POST`, and `POST /api/v1/oauth/token` with no grant
type returns code 17007 naming `authorization_code` and `refresh_token`.
One judgement call worth a look: I annotated the token endpoint `@Accept
json`, matching the comment on `TokenRequest` that v1 binds JSON and v2
takes the form-encoded body. In practice v1 binds a form body too —
`Content-Type: application/x-www-form-urlencoded` with `grant_type=nope`
returns the same 17007 — so if you'd rather document both, say the word
and I'll widen it.
The `/api/v2` routes are untouched — happy to cover those too if you'd
like them in.
AI-assisted: annotations drafted with Claude, reviewed and tested by me.
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
db.NewSession begins a transaction, so every request holds one pooled
connection for its whole life: across the API's own work and every round
trip in between. At 20,000 simulated users that filled a 192-connection
pool with 170 backends idle in transaction while 13 were active, and the
database sat at 40 % CPU with p99 at 94 ms.
NewReadSession attaches the session memo but does not Begin; each statement
borrows a connection only for itself. Commit and Rollback are no-ops on it,
so DoReadOne, DoReadAll and ValidateAPITokenString keep their shape. A read
path that writes (position healing) now commits per statement.
GetAuthFromClaims opened a session and loaded the token owner by id on
every call, although the token middleware had already put the user into
the context. With the metrics middleware and the handler both resolving
the auth, that was two extra transactions and user lookups per request
authenticated with an API token.
The api error handler, the poison queue logger and the migration listener all
captured through the same frame, so API-OSS-2P collected six unrelated MySQL
errors under one title. Set a derived fingerprint and an error.type tag on the
scope before capturing.
Unsafe paths, unusable file ids, a missing or unparseable VERSION and
unreadable data/filter json all describe a broken upload, not a server
fault, but they were raw fmt.Errorf values and surfaced as a 500. They now
return ErrInvalidImportFile; an export from a version we can no longer
read gets its own ErrImportFromUnsupportedVersion (14013) so clients can
tell the two apart. The http messages stay generic and never echo a path
from the archive.
ErrCodeNotAZipFile and ErrCodeFileIsEmpty shared 14001 and 14002 with
ErrCodeAPITokenInvalid and ErrCodeInvalidAPITokenPermission, so clients
mapping codes to messages rendered the api token text for a failed import.
The api token codes are the older ones, so the migration errors move to
14011 and 14012.
BREAKING CHANGE: a failed import now returns 14011 instead of 14001 and
14012 instead of 14002.
Errors like planka's ErrInvalidCredentials or ErrNoPlankaAtURL surface
inside the migration listener and map to a 4xx, so they are the user's
problem to fix, not ours.
encoding/json reports a document that ends mid-value as io.ErrUnexpectedEOF
rather than a *json.SyntaxError, so those still reached the error handler as a
500.
A WeKan JSON export saved with a UTF-8 BOM made encoding/json return a raw
*json.SyntaxError from the migrator, which the shared error handler turned into
a 500 and forwarded to Sentry. Map decode failures of the uploaded file to a
400 in RunFileMigration (shared by v1 and v2), and skip a leading BOM so an
otherwise valid export imports instead of being rejected.
Fixes API-CLOUD-4B
A 4xx from the migration source (disabled mailbox, expired token, deleted
item) is not a bug on our side, but every one of them was captured as an
exception. Those users now get the failure notification with the upstream
message instead of the "we have been notified" one.
Non-2xx responses from the service we migrate from were plain fmt.Errorf
values, so callers could not tell an upstream problem apart from a bug.
ErrUpstreamRequestFailed carries the migrator, status code and a truncated
body and is now returned by the shared http helpers, microsoft-todo,
todoist and planka.
A file row whose blob is gone from storage (Sentry API-OSS-50) made
LoadFileByID return the raw storage path error, which the error handler
turned into a 500 and reported to Sentry. Return ErrFileDoesNotExist
instead and update the callers that matched on the path error.
The metrics middleware called GetAuthFromClaims on every request and logged
its error when no JWT was in context. Unauthenticated routes (login, token
refresh, /info, websocket) hit that path constantly, flooding the log at
WARNING level and burying real errors.
Skip the active-user update when the request carries no credentials.
Preflight over the declared uncompressed sizes (overflow-safe), a file
count cap and a per-user storage quota bound the import before
anything is read; the actual decompressed bytes are still counted
against the same budget while reading. Attachments and backgrounds
now stream through a lazy provider: one zip entry is decompressed
into a private temp file at a time. A failed import deletes the file
blobs it wrote, which were orphaned for every migrator before.
kolaenteandTink
Wouter BlancquaertandGitHub
kolaenteandkolaente