SQLite and MySQL dumps store bools as 0/1. Since switching to pgx, restoring
such a dump into PostgreSQL fails with "unable to encode 0 into binary format
for bool".
Each project gets a monotonic high-water counter row, backfilled from the
highest existing task index, and an append-only alias table recording
retired (project, index) addresses. Project creation seeds the counter,
project deletion removes both.
One row per project and each ancestor including itself, so the tree
resolves with a join instead of a recursive walk. CreateProject inserts
the chain, UpdateProject re-links the subtree on a parent change and
Project.Delete drops the rows; the relink reads take FOR UPDATE so
concurrent reparents of overlapping subtrees serialize, and a project
without its self row fails loudly instead of silently losing access.
Descendant archiving reads from the table. RepairOrphanedProjects
rebuilds the whole table so drift and rows of deleted projects go away.
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.
One GET /tasks/{id} loaded the task twice (Task.CanRead, then ReadOne), its
project twice and users by id several times, each a round trip.
GetTaskByIDSimple, GetProjectSimpleByID, GetProjectsMapByIDs,
user.GetUserByID and user.GetUsersByIDs now go through the per-session memo,
which drops itself as soon as that session writes anything.
db.Remember and db.RememberEach hold the get-or-fetch dance, mirroring
keyvalue.Remember; the two pre-existing memo sites in project_access.go use
them too. Callers get copies: Project.memoCopy re-allocates ParentProjectID,
and the user memo holds the raw row so the disabled/locked gate runs on
every hit rather than only on the first load.
Every API-token request ran pbkdf2.Key(token, salt, 10000, 50, sha256):
~3 ms CPU per request. Tokens are 160-bit CSPRNG values, so a slow KDF
and salt add nothing; sessions.go already stores refresh tokens as
plain SHA-256 for the same reason. Lookup is now one indexed seek on
token_sha256, falling back to the legacy PBKDF2 scan only for rows
that have not been backfilled yet. A legacy match backfills the column
on its own autocommit session, since callers roll theirs back.
The first version kept the regexp for WITH, but every recursive CTE
(project access, subscriptions, hierarchy) is a WITH statement of several
kilobytes, and the second profile still showed 5 % of CPU in regexp
backtracking there. A whole-word scan with the same boundary rules
replaces it.
The write-invalidation hook classifies every statement, and the two regexps
it used for that were 5 % of the API host's CPU in the load test. A byte scan
for the leading keyword does the same; the CTE regexp stays for WITH only.
ParadeDB's ||| operator needs the search term while the statement is planned.
Named prepared statements are planned at Parse, with the term still an unbound
parameter, so every search fails with:
The right-hand side of the `|||` operator must be a text or text array value
lib/pq never hit this because unnamed prepared statements are planned at Bind,
with the parameter values in hand. Probe for the extension while connecting and
fall back to pgx's exec mode, which uses unnamed statements, when it is present.
ParadeDB installations therefore keep working search and give up the statement
cache; everyone else keeps it.
Upstream fixed the same failure for === in paradedb/paradedb#5907 (v0.25.2) but
not for |||, still absent as of v0.25.6. This can go once that lands.
pgx encodes parameters using the type the statement description reports, where
lib/pq sent everything as text and let PostgreSQL coerce it. Three fixtures
relied on that coercion and are rejected outright:
unable to encode 0x1 into binary format for bool (OID 16)
unable to encode 0xe into text format for varchar (OID 1043)
Cross-checked every fixture key against the Go field type it maps to; these
were the only mismatches.
lib/pq sends every query as an unnamed prepared statement, so PostgreSQL
plans each statement from scratch. pgx via database/sql keeps a per-connection
statement cache and reuses named prepared statements instead, removing that
planning cost from the hot path.
xorm already ships a "pgx" driver mapping, so the switch is just the driver
name plus the blank import. The connection string format is unchanged: pgconn
parses the same URL and forwards unknown parameters (search_path) as runtime
parameters.
Named prepared statements do not work through a connection pooler in
transaction pooling mode unless it supports them (PgBouncer 1.21 and later,
with max_prepared_statements above 0). Those setups can raise that setting or
connect to PostgreSQL directly, which is no loss since Vikunja already pools
via database.maxopenconnections.
lib/pq was only used for QuoteIdentifier, which is now inlined.
labelSearchCond matched titles with a raw ILIKE while projectSearchCond went
through db.MultiFieldSearch, so label search missed the ParadeDB path. Moving
it over widens what label search matches to descriptions as well as titles,
and on Postgres with ParadeDB it becomes a fuzzy match rather than a substring
one - the same asymmetry projects and tasks already have.
Fields are qualified with the labels alias: the cond is spliced into the
per-task query, which joins label_tasks, so a future column collision would
otherwise fail at runtime on MySQL and PostgreSQL only.
The numeric branch is untouched - a search whose terms all parse as integers
still matches label ids.
labels becomes a ParadeDB consumer with this, and on Postgres with pg_search
installed the ||| operator requires the table to carry a USING bm25 index, so
CreateParadeDBIndexes grows one for labels alongside tasks, projects and
time_entries.
GET /labels ran the label list as `labels LEFT JOIN label_tasks ... WHERE
label_tasks.task_id IN (SELECT id FROM tasks WHERE project_id IN (<every
accessible project>)) GROUP BY labels.id`, fanning every label out over its
label_tasks rows only to collapse them again, and then repeated the whole
thing as a `count(DISTINCT labels.id)`.
The label list is now a plain select over labels whose task branch is an
uncorrelated `labels.id IN (SELECT label_id FROM label_tasks INNER JOIN
tasks ...)` subquery. The count query only runs when the returned page is
full, since a short page means the offset plus what we got is already the
total. The same applies to the per-task query when it is unpaginated, which
is how task lists load their labels.
The old query resolved its projects through getRawProjectsForUser, which
leaves out archived projects, so a label used only on tasks in an archived
project was missing from the list even though CanRead - and therefore
GET /labels/{id} - happily returned it. Reading the accessible projects
from the session access memo drops that accident and makes the list agree
with the permission check.
Both helpers compared a.GetID() against users.id, but *LinkSharing also
satisfies web.Auth and its GetID() is a link_shares.id - a different id
namespace. Passing one would have emitted users.id = <share id> and lent
that share another user's identity, which is the confusion behind
GHSA-vvcv-vpph-h844. Guarded at every call site, but the helpers moved
out of models and into user, so the guard drifted away from the code
needing it. Taking *User makes the case unrepresentable.
Also fail closed on a zero id: humans store bot_owner_id = 0, so an
unguarded zero would have matched all of them.
The doc claimed JWT and CalDAV auth carry no BotOwnerID, but bots can
reach neither path, and claimed bots-cannot-own-bots as an invariant when
the guard is inert for JWT principals. Reworded to chain-safety by
construction, which holds regardless.
The label access condition covered owner-to-bot and bot-to-owner, so two bots
of the same human could not use each other's labels: bot A seeded a label and
bot B got a 403 on it.
Replacing it with SameBotIdentityCond resolves every caller to an identity root
- a bot's owner, or a human themselves - and matches everyone under that root,
which covers all three directions in a single condition. Write access is
unchanged: only the creator and the human owner can update or delete a label.
Resolving the same thing repeatedly within one request is the cost this exists to
remove, but a memo that outlives a write is a correctness bug — a permission check
after a share is revoked must not answer from the pre-write snapshot.
An xorm hook marks the session dirty as soon as it writes, and the memo stops
serving from that point on, so staleness is not expressible rather than something
every writer has to remember. Sessions built outside db.NewSession never memoize.
Testfixtures writes through the raw *sql.DB, which no hook sees, so reloading
fixtures drops every live memo too.
The memo rides in a weak map keyed by the session and in the session context: the
map is how readers find it, the context is how the hook reaches it. Replacing that
context detaches invalidation, hence SetSessionContext.
Only an expired session or a disabled account invalidates the cookie. A transient failure must not clear it, and neither must a token rotated away by a concurrent refresh: that would delete the cookie the winning refresh just set.
Capturing a config value before init returns "", and Key.Set writes at viper's
override level, which outranks defaults. Restoring a captured value therefore
pinned database.path to "" for the rest of the process, and TestRestore resolved
it to the bare rootpath and failed with "is a directory".
Only visible when VIKUNJA_TESTS_USE_CONFIG=1, which CI sets for every database
except sqlite-in-memory, which is why every test-api job failed while local runs
passed. ResetForTests restores genuine fresh-process state instead.
ResolvedDatabasePath exists so callers can report the path a diagnostic would
use. Resolving it must therefore create nothing, which took three things:
getUserDataDir ended in os.MkdirAll, so merely reporting the path created the
user data directory. resolveUserDataDir computes it; getUserDataDir creates it.
resolveDatabasePath then chose its rootpath fallback from whether the injected
getter errored, which only the creating variant could do — so the reporting and
engine paths disagreed, and doctor failed a working install by naming a path the
server never opened. Creation now lives inside the injected getter: resolution
passes existingUserDataDir, the engine passes createdUserDataDir, and both fall
back to rootpath on failure.
Creation stays scoped to the user data directory. An operator-configured
database.path never gets its parents created, and an unwritable data directory
still degrades to rootpath rather than refusing to start.
getUserDataDir, the resolve-and-create variant, has no caller left. Keeping one
around invites reintroducing the bug where a reporting path creates the
directory it reports on.
Label with zero task attachments unreachable by anyone but creator, so
label seeded by human unusable by that human's own bot — every attach
403s forever until someone else attaches it once.
Refs #3592
`hasAccessToLabel` grants non-creators access only through existing
`label_tasks` row on readable task. Bot owners already inherit access to
labels their bots created (c9c2c58c1); this adds missing reverse
direction so both sides of bot/owner pair share label access.
Human-to-human access unchanged: another user's labels stay invisible
until they show up on a task you can read — the restriction
GHSA-hj5c-mhh2-g7jq introduced. Bots inherit read/attach only; renaming
and deleting the owner's labels still requires being the owner.
### Scope: does not close#3592
Deliberately `Refs`, not `Fixes`. The issue also reports the
human-teammate case — someone you shared a project with still cannot
attach a label that has never been used. That stays broken here, because
the fix the issue proposes for it (allow attach whenever the caller can
write the target task) would let any user attach any label id to a task
they own and read the title back, re-opening GHSA-hj5c-mhh2-g7jq in a
new shape. Whether labels should become project-scoped or explicitly
shareable is a design question worth its own issue — leaving #3592 open
to track it.
Note: the commit trailer still reads `Fixes #3592`, so adjust the
message on squash-merge if you want the issue left open.
### Operator note: widened bot/owner scope
A bot token can now enumerate every label its owner has created via `GET
/api/v2/labels`, including labels only used on projects the bot was
never given access to, and can attach one to a task in any project it
can write (which then makes that label visible to that project's
members). Read-only — rename and delete still require being the owner.
Worth knowing if you hand bot tokens to third-party integrations.
## How to verify
1. As a human user, create a label and do not attach it to any task:
`POST /api/v2/labels {"title":"seeded"}` → note the returned id `N`.
2. Create a bot owned by that user (`PUT /api/v1/user/bots`), issue an
API token owned by the bot with the `tasks_labels` create scope, and
give the bot access to a project.
3. As the bot, create a task in that project, then attach the label:
`POST /api/v2/tasks/{taskID}/labels {"label_id": N}`.
4. **Expected:** the request returns 201 and the label appears on the
task. `GET /api/v2/labels` as the bot also lists label `N`.
**Before this PR:** step 3 returned 403 on every attempt, and label `N`
was missing from the bot's label listing, until some other user attached
the label to a task first.
5. As a second, unrelated user (not the bot's owner), attach the same
label to a task you can write. **Expected:** still 403 — the label
remains private to its creator until it is visible through a shared
task.
6. As the bot, try to rename and delete label `N` (`PUT` and `DELETE
/api/v2/labels/N`). **Expected:** 403 on both.
---------
Co-authored-by: kolaente <k@knt.li>
is_archived is written down the whole subtree on archive/unarchive and
backfilled for pre-existing rows, so CheckIsArchived, ReadOne and the
project list CTE no longer need to derive it from ancestors. The list
CTE previously only saw ancestors the user could access, so list and
single reads could disagree. task_overdue_reminder already trusted the
column and is now correct for old data too.
Project 21 now carries is_archived=1 like the backfill migration produces.
Un-archiving a project whose parent is still archived is rejected in
CanUpdate explicitly instead of relying on the unflagged child.
Huma's AutoPatch implements PATCH as an internal GET + PUT re-dispatched through
the router, so both legs re-enter the API token middleware. Scoping the GET leg
like a client request made every /api/v2 PATCH additionally demand the
resource's read_one permission, so a token scoped to tasks: update could never
patch a task.
Mark the re-dispatched requests with the route the client request was matched
against, and skip the scope check only for a GET leg that carries no query
string and resolves to that exact route. Anything looser is exploitable: echo
routes on the raw path while autopatch re-dispatches the decoded one, so an
encoded slash steers the unchecked leg onto a deeper route and an encoded
question mark smuggles a query onto it.
Fixes#3528
Dumps store datetime columns as RFC3339 strings (e.g. 2026-03-27T11:27:01Z).
On restore these were passed verbatim to the database, which MySQL and
MariaDB reject with 'Error 1292 Incorrect datetime value'. Parse them into
time.Time so the driver formats them for the target database.
Also guard against a nil pointer dereference when a dump contains a column
that no longer exists in the current schema - such columns are now dropped
with a warning instead of crashing.
Fixes https://github.com/go-vikunja/vikunja/issues/3375
Users with access to parent project could remove labels from tasks in
child projects but not add them back — 403 "Tried to create while not
having the permissions for it". Label attach check used direct shares
only; task write permission and label picker both walk project
hierarchy. Fix: label access now uses same recursive subquery.
Reported: https://community.vikunja.io/t/permissions-on-labels/4460
## How to verify
1. As user A, create a parent project with a child project, and add a
task in the child project.
2. Share the parent project with a team that has write access and
contains user B.
3. As user A, add a label to the task in the child project.
4. As user B, open that task, remove the label, then try to add it back.
5. **Expected:** the label can be added again; the API returns 201.
**Before this PR:** step 4 failed with 403 even though user B could edit
the task and remove the label.
Co-authored-by: kolaente <k@knt.li>
xorm connects lazily, so nothing touched the database until the ParadeDB probe
ran, and a connection failure surfaced as "could not check for paradedb
extension" followed by "Schema check failed: could not determine the current
schema" - which is how #3287 came in labelled as a schema bug. Ping right after
the engine is built and let the existing "Could not connect to db" callers report
it. The ParadeDB probe moves up alongside it so it no longer runs first.
With constant-score relevance, 1.5 ranks a title match above a description
match while two matched words (2.0) still beat a single boosted title word,
preserving "more matched words wins". ParadeDB-only; the substring fallback
has no scoring.
The fuzzy query shape makes pdb.score a constant-score sum (1.0 per
exact/prefix word-field match, 0.5 per one-edit match), not BM25, and no
field boosting is configured. Correct the comments claiming otherwise.
MySQL/MariaDB/Postgres cannot bind a table name as a ? placeholder, so the non-SQLite branch failed with a syntax error. Interpolate the already-validated identifier with x.Quote (per-dialect quoting) instead. validateTableName restricts to registered table names, so this is injection-safe — the same trust model the SQLite branch already relies on. Latent bug surfaced by the new cross-engine testing webtest, which is the first to exercise this path on MySQL/MariaDB.
Port the per-user webhook endpoints (/user/settings/webhooks) from /api/v1 to
the Huma-backed /api/v2: list, available events, create, update, delete. They
are the project-less sibling of the project webhooks (#2858) and share the
webhooks.enabled gate, checked inside the registrar.
Webhook.ReadAll is extended to serve the user-level list (scoped to the
authenticated user) so the v2 list handler can go through handler.DoReadAll like
the project list; the project branch is unchanged. Credentials are masked on
read via the model's existing maskCredentials, matching #2858.
Bot owners inherit read/update/delete permission on labels created by
bots they own, mirroring the bot-owner branch already used by API tokens
(see api_tokens_permissions.go). Without this, a label a bot creates is
permanently locked to that bot and the human owner cannot maintain it.
https://claude.ai/code/session_016x6mUPJuuQEeXpHY814iLh
Ports the v1 DatabaseNotifications routes to the Huma /api/v2 API:
- GET /notifications lists the caller's own notifications (paginated)
- PUT /notifications/{notificationid} marks one (un-)read
- POST /notifications is a custom action marking all as read; the
link-share guard, session and commit live in the handler since there
is no CRUDable Do* for a bulk mark.
Adds fixture rows and a webtest matrix mirroring the v1 model behaviour
(own-only visibility, mark-(un)read, link-share refusal on every route).
Port the v1 webhook webtest to /api/v2 and extend it to the full
permission gradient the model enforces: list needs read access while
create/update/delete need write (Project.CanWrite), exercised across an
owned project and read/write/admin shares plus a no-access project. Also
assert credential masking, events-only updates, the 412 validation path,
and that the routes 404 when webhooks.enabled is false.
Add fixture webhooks 2-5 in projects 9/10/11/2 to back the matrix; they
do not collide with the e2e tests, which scope to project 1.
A comment whose body contains <blockquote data-comment-id="…"> nodes
now triggers the same task-comment mention notification for the
quoted comments' authors, respecting CanRead, subscription, and
existing dedup. Self-quotes, wrong-task quotes, and malformed ids
are silently skipped.
kolaenteandTink
kolaenteandkolaente
Tea ReggiandGitHub
Claudeandkolaente