Catalog-tier resources stay out of tools/list; agents discover them via
find_action (scope-filtered, schemas on demand) and invoke them through
do_action, which funnels into the same Dispatch path — schema validation
and the per-call scope re-check apply identically.
Wave 1: task labels, task relations (subtasks), team members, project
user/team shares, project views. Deliberately absent: api tokens,
webhooks, link shares, buckets and positions (v1 token scopes don't map
onto (group, op) permissions), saved filters (nested filter object).
Adds IdentityFields for records not addressed by their id (team members
go by team + username) and treats readOnly+param fields as arguments
(REST reads them from the URL; MCP has no URL).
tasks_read_all accepts filter/sort_by/order_by (from TaskCollection's
query-tagged fields) plus the generic search/page/per_page arguments;
project_id is optional so agents can list across projects. Fixture token
11 gains the tasks.read_all scope it now exercises.
Replaces the hand-written per-resource plumbing (input wrapper structs,
per-resource install functions, per-op Inputs maps) with a generic layer:
- schema.go reflects each op's input schema from the model's existing
json/doc/readOnly/valid/minLength/param/query tags — the same contract
the Huma-backed /api/v2 reads. URL-bound param fields become required
JSON arguments; models without an exposed id (task assignees) are
identified by their param fields.
- apply.go applies arguments presence-based: only keys the caller sent
are written, so an explicit zero clears a field and an omitted key
leaves it untouched — replacing the pointer tri-state wrappers.
- Registration is now a pure declaration (name, model, ops); the SDK's
low-level AddTool with runtime-built schemas replaces the generic
typed handlers, collapsing the six install functions into one loop.
- tasks_read_all now exists, backed by models.TaskCollection, exposing
the filter/sort_by/order_by query surface.
inputs.go and its wrapper structs are deleted; behavior is pinned by the
unchanged mcp webtests.
copyByJSONTag previously skipped any IsZero value, which made it
impossible for tasks_update / projects_update to flip done from true
to false, reset priority/percent_done to 0, or unarchive a project.
A non-nil pointer src is now the unambiguous "caller supplied this"
signal: dereferenced values are written through even when zero, while
value-typed src fields keep the partial-update semantics. The
affected wrapper fields (Done, IsArchived, IsFavorite, Priority,
PercentDone, RepeatAfter, RepeatMode, BucketID,
CoverImageAttachmentID, ParentProjectID, Position) move to pointer
types so the JSON Schema still marks them optional.
Registers tasks, labels, teams, task_comments and task_assignees through
the MCP tool surface, completing the v1 resource list from the plan:
* tasks : create / read_one / update / delete (read_all omitted;
models.Task.ReadAll is a stub — TaskCollection is OOS)
* labels : full CRUD
* teams : full CRUD
* tasks_comments : full CRUD, install-time gated on
config.ServiceEnableTaskComments
* tasks_assignees : create / read_all / delete only (REST exposes no
read_one or update)
Per-resource input wrappers carry the path-param fields (task_id,
user_id) explicitly so MCP callers can provide them as JSON args.
installToolsForToken fans out to one installer per resource; the
generics-bound addTool keeps per-(resource, op) call sites at compile
time. The api_tokens.yml fixture extends token 11 to cover the new
scopes; token count stays at 5 for user 1 so existing token-listing
tests are unaffected.
Integration tests per resource cover tools/list visibility, at least
one successful create or read_all, and a permission denial scenario.
Filter MCP tool visibility and invocation by the requesting API token's
(group, permission) scopes. tools/list now returns only the tools the
token's APIPermissions authorise; tools/call additionally re-checks the
scope in the dispatcher as defence-in-depth, so a session created with
one token cannot be reused to invoke tools that token never had access to.
The per-session filter runs at session-init via the StreamableHTTPHandler
getServer factory (which the SDK calls once per session, before caching
the *mcp.Server). The dispatcher check runs on every tools/call and
returns ErrScopeDenied, which the AddTool wrapper renders as an IsError
tool result.
Wires the projects resource into the MCP server end-to-end. The five
project tools (create, read_one, read_all, update, delete) are now
visible in tools/list and dispatch through handler.Do* like the REST
layer.
- Add ProjectCreateInput / ProjectUpdateInput in inputs.go with
jsonschema tags covering only the writable fields the model honours
(title, description, identifier, hex_color, parent_project_id,
position, is_archived, is_favorite); computed fields like Owner and
MaxPermission are intentionally absent so the SDK-reflected schema
stays narrow.
- Add resources.go with a sync.Once-guarded RegisterResources(), and an
installTools helper that registers tools per (resource, op) on the
*mcp.Server via a generic addTool[In inputAdapter] helper. The
handler maps domain failures (permission denials, missing rows,
validation) to IsError tool results per the SDK convention.
- Add DispatchTyped in dispatcher.go so the AddTool handler can hand a
pre-unmarshalled wrapper to the dispatcher without a JSON
round-trip. The existing Dispatch (raw JSON path) delegates to a
shared dispatchPrepared.
- Wire RegisterResources() + installTools() into newServer() so each
new MCP session inherits the static tool set.
- Add fixture token 11 (mcp:access + projects:*) for the full-scope
integration tests; bump TestAPIToken_ReadAll's expected count.
- Refresh TestMCP_ToolsListEmpty into
TestMCP_ToolsListReturnsRegisteredResources, asserting the five
projects_* tools are present (Task 6 will introduce scope-based
filtering of this list).
- Add pkg/webtests/mcp_projects_test.go covering tools/list,
create/read_one/read_all/update/delete happy paths, schema-validation
failure on missing required title, permission denial on a forbidden
project, and nonexistent-id lookup.
Define the Op bitmask, the Resource struct, the package-level Register
function, and the Dispatch entry point that future tasks will use to
expose CRUD resources over MCP. No resources are registered yet.
Op carries the CRUD-op identity, knows its api-token permission string
(matching apiTokenRoutes exactly), and knows its tool-name suffix.
Resource.Inputs maps each enabled op to a pointer-to-zero of the wrapper
type the dispatcher will allocate and unmarshal into. Register validates
the resource shape and populates a tool-name lookup table so the
dispatcher never has to string-parse names like task_comments_read_all.
Dispatch threads the user from ctx, allocates a fresh wrapper, unmarshals
arguments, asks the wrapper to copy itself onto a fresh model via the
inputAdapter seam (which Task 4 will populate with real implementations),
and forwards to the corresponding handler.Do* function. The Do* calls go
through a swappable crudFuncs struct so the unit tests can verify
dispatch routing without standing up the database.
Mount /api/v1/mcp (and /api/v1/mcp/*) inside the authenticated route
group. Reject JWT-authed requests with 401 (token-only policy), reject
API tokens without the mcp:access scope with 403, and propagate the
authed *user.User + *models.APIToken to r.Context() via typed keys so
downstream tool handlers can pull them out without depending on Echo.
The MCP protocol — JSON-RPC framing, Mcp-Session-Id management, SSE
streaming — is delegated to github.com/modelcontextprotocol/go-sdk
v1.6.1. tools/list returns {"tools": []} since no tools are registered
yet.
Adds the mcp scope group with a single access permission so it shows up
in GET /api/v1/routes (and therefore in the frontend token form).
Adds APIToken.HasMCPAccess() mirroring the caldav/feeds helpers.
The MCP endpoint will use POST, GET, and DELETE on the same path for the
streamable-HTTP transport, which CanDoAPIRoute's exact (method, path)
match cannot gate. The token middleware therefore skips the route check
for /api/v1/mcp and any sub-path; the actual authorization is delegated
to an inline HasMCPAccess() call in the MCP handler (added in the next
task).
Fixtures gain two MCP tokens for user 1: one mcp-only and one with
mcp:access plus projects read scopes for the per-tool scope filter tests.
The scenario reported in #3574: a write member creates a subproject under a
project someone else owns. The parent's owner keeps admin on it, and CanRead,
IsAdmin and expand=permissions must all agree, for a direct share and a team
share alike. Both directions are covered — an unrelated user gets neither read
nor admin and does not see the subprojects listed, and a write member is not
reported as admin on the parent.
MarshalJSON emits null for PermissionUnknown, but json.Unmarshal treats null as
a no-op for an int, so it decoded back as PermissionRead — the two did not round
trip.
Permission also backs the persisted permission on ProjectUser, TeamProject and
LinkSharing, where isValid now rejects an explicit "permission": null instead of
silently creating a read tier share. Omitting the key still defaults to read:
UnmarshalJSON is never called for an absent key.
Favorites has no row, so checkPermissionsForProjects returns nothing for it and
expand=permissions left the field nil. checkReadPermissionsForProjects already
resolves it to read; the list route now agrees instead of reporting null.
Permission's zero value is PermissionRead, so Project.MaxPermission serialized
as 0 — a real permission meaning read — on every response path that never
resolved it. GET /api/v1/projects/:project claimed read-only access on a
project you own while the x-max-permission header correctly said 2.
Typing the field *Permission makes that unrepresentable: nil marshals to null,
so "not computed" is the default rather than something each call site has to
remember. That also fixes the paths which serialize a project outside the CRUD
pipeline and had the same lie — the admin project list, the background
handlers, duplicated_project on both API versions, and the admin owner-reassign
route — and retires v2's two explicit resets, which the field type now covers.
Webhook payloads for project.created and project.updated change from 0 to null
along with it.
Fixes#3574
A write-enabled link share clears LabelTaskBulk.CanCreate, which only
checks task write access. UpdateTaskLabels then rejects any label the
share cannot see and built the error with an unchecked type assertion to
*user.User, so creator.(*user.User) yielded nil and .ID panicked - a
clean 403 became a recovered panic and a 500.
Use creator.GetID(), matching how ErrNoPermissionToSeeTask is built a
few lines up.
The creator is usually the most interested party of a task, but was not
notified about comments or changes unless they subscribed manually or got
@mentioned. Assignees have been auto-subscribed on assignment for a while,
so this makes creation consistent with that.
Subscriptions inherited from the project are resolved by Subscription.Create,
so no redundant task level subscription is created when the user is already
subscribed further up the hierarchy. Link shares can't hold subscriptions and
are skipped.
Fixes#2692
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>
Subscribing to a project promised notifications "for changes", but no
listener was registered for TaskCreatedEvent beyond mention handling, so
new tasks never reached subscribers.
Adds a TaskCreatedNotification and a listener that notifies task and
project subscribers, skipping the creator and users already notified by
the mention listener for the same event.
Fixes#3611
Vikunja leaves `oauth2.Config.Endpoint.AuthStyle` at
`AuthStyleAutoDetect`, so `golang.org/x/oauth2` tries
`client_secret_basic`, retries with `client_secret_post` on *any* error,
and reports only the second error — turning every token endpoint failure
(a wrong client secret, say) into a bogus "the client registration does
not allow client_secret_post" complaint.
Style now comes from the provider's
`token_endpoint_auth_methods_supported` discovery value. Provider
advertises neither: keep autodetect, so nothing changes for providers
with a thin discovery document.
Side effect: one token request per login instead of two, no more
spurious failed-auth entry in provider logs on every successful login.
Reported at
https://community.vikunja.io/t/authelia-could-not-authenticate-against-third-party/4770
## How to verify
1. Register a Vikunja client in an OIDC provider that only accepts
`client_secret_basic` (Authelia: `token_endpoint_auth_method:
'client_secret_basic'`).
2. Configure that provider under `auth.openid.providers` in Vikunja, but
put a deliberately wrong value in `clientsecret`.
3. Log in through that provider and read the Vikunja log.
4. **Expected:** the error names the real problem — an `invalid_client`
/ "client secret did not match" message from the provider.
**Before this PR:** the error claimed the client registration does not
allow `client_secret_post`, which was never configured on either side.
5. Fix `clientsecret` to the correct value and log in again.
6. **Expected:** login succeeds, and the provider's access log shows a
single `POST /token` per login rather than a rejected one followed by an
accepted one.
---------
Co-authored-by: kolaente <k@knt.li>
The v2 /user/password/token operation description claimed the response is
the same whether or not an account exists, but the endpoint returns 404 for
an unknown email. Correct the description to match actual behavior.
setupE2ETestEnv seeded fixtures without waiting for the previous test's
event handlers. Cancelling the test context does not stop them:
sendWebhookPayload builds its request with context.Background(), so an
in-flight webhook delivery keeps running — and keeps holding the DB
session opened by WebhookDeliveryListener.Handle — after the router shuts
down. The next test's db.LoadFixtures() then raced that session and failed
with "testfixtures: could not clean table \"webhooks\": database table is
locked: webhooks", most often in TestWebhookFailingSiblingDoesNotBlockOthers
and TestUserWebhookTasksOverdueBatchE2E.
Call events.WaitForPendingHandlers() first, the same drain the e2e testing
endpoint already does in ReplaceTableContents and TruncateAllTestingTables.
The regression test blocks a webhook target for 500ms, cancels the context
while the delivery is in flight, and asserts the next setup waits it out.
Without the drain it reproduces the CI error in ~0.1s.
Fixes incomplete team `members` arrays when a user belongs to multiple
teams included in the same response.
`addMoreInfoToTeams` queried one row per `(team_id, user_id)` membership
but stored those rows in a map keyed only by user ID. When a user was a
member of multiple teams, a later row replaced an earlier one. The
subsequent member assignment loop therefore used only the surviving
membership row.
The fix stores all queried membership rows in a slice, preserving every
team-user relation. It creates a separate `usersByID` map only for
resolving `created_by`, where one row per user is appropriate.
This affects both list paths because both use `addMoreInfoToTeams`:
- `GET /api/v1/teams`
- `GET /api/v1/projects/{projectId}/teams`
The example plugin is a Yaegi entrypoint evaluated at runtime through its
exported factory functions, so it has no func main. As a regular package main
it broke `go build ./...` with "function main is undeclared in the main
package".
Add //go:build ignore so the go tool skips it. Yaegi is unaffected: the loader
reads the file and calls interp.Eval on its source, which does not evaluate
build constraints.
Fixes#3478
Un-archiving echoes the whole project incl. a parent_project_id that may no
longer exist; that nil-dereferenced in the cycle check. A newly requested
missing parent still errors.
An archived project whose stored parent no longer exists returned 404 on
un-archive. Reparenting under an archived parent now reports
ErrParentProjectIsArchived.
Buckets, webhooks, backgrounds and task duplication checked write access
through Project.CanUpdate on a bare stub, whose zero IsArchived matched the
un-archive carve-out. With is_archived now set on descendants too, that
made whole archived subtrees writable. Use CanWrite instead.
Enforced in UpdateProject, the single write path, instead of CanUpdate,
which short-circuits for instance admins and is bypassed by direct
callers. Callers get a dedicated ErrParentProjectIsArchived (3016)
telling them to un-archive the parent first.
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.
Credentials migrators are verified synchronously before the async migration is
queued (CredentialsChecker). Routes live on v2 only; the listener learns the
migrator via a route-free RegisterMigratorForEvents, which the oauth v2
registrar uses as well. Shared status/migrate registration moved to
migration_shared.go.
Everything lands under a "Migrated from Planka" root project; projects with
several boards become a parent with one child per board. Lists become kanban
buckets (trash skipped), closed/archived cards are done, labels keep their
Planka colours, checklists, custom fields and link attachments are rendered
into the description, comments are prefixed with the author unless it is the
importing user. No assignees, memberships or other users' data.
Pages archived cards and comments (Planka requires both cursor fields, pages
until empty with a hard cap, partial results are kept), reads base custom
field definitions from the projects payload and rejects Planka v1 payloads.
Authenticates with an API key, a JWT or username + password (Planka checks
Bearer before X-Api-Key, so only one header is ever sent; downloads use the
accessToken cookie). The synchronous credential check has one 15s deadline and
no retries. Redirects to another host or to plain http are refused for the
api, followed without credentials for attachment downloads. Response bodies
are capped, url userinfo is stripped, pending login steps (totp, terms) and
non-planka endpoints are reported as distinct client errors (142xx).
Migrators that talk to a user-supplied host need a client with a redirect
policy, a cap on response bodies and no retries on permanent errors
(utils.ErrDoNotRetry). Diagnostic body reads in the shared helpers are capped.
Creating a done task puts it in the view's default done bucket; moving it into
the imported bucket flipped it back to open. The imported state (incl. done_at)
is restored in bulk after the task loop.
The handlers configured caldav-go through its setup functions, which write
package-level globals that HandleRequest then reads. Between a request's setup
and its handling another request could overwrite them, so two users syncing at
the same time could be answered from each other's storage - the wrong user, the
wrong projects.
caldav-go takes the state per request now, so pass it there instead. A test
driving eight concurrent requests per user fails against the old code both as a
race report and as user1 receiving user15's calendar.
TaskHandler never set a user or the supported components at all, so it answered
current-user-principal with whichever username the last request happened to
leave behind. It gets them like the others now.
Task uids are client chosen — the api only generates a uuid when the
field is empty (`pkg/models/tasks.go`), and the CalDAV parser stores
whatever the inbound VTODO carried (`pkg/caldav/parsing.go`). That value
went into hrefs verbatim, which lets a uid forge a path or inject
markup.
Split out of #3551 so the path-forgery half gets its own review.
## What goes wrong today
**Path forgery.** A uid containing a slash makes the href appear to live
in another collection:
```
UID:evil/../../../projects/5/y → <D:href>/dav/projects/36/evil/../../../projects/5/y.ics</D:href>
path.Clean → /dav/projects/5/y.ics
```
Anyone who can create a task in a project the victim can see can plant
one; it then renders inside the victim's collection listing pointing
elsewhere. Same class as GHSA-48ch-p4gq-x46x.
**XML injection.** `sync_collection.go` wraps hrefs in `xmlEscape`, but
PROPFIND and calendar-multiget hand `Resource.Path` to caldav-go's
`ixml.HrefTag` → `ixml.Tag`, a plain `Sprintf`. `ixml.EscapeText` sits
in the same file and is only used for prop content. A uid with `<`, `>`
or `&` therefore lands raw in the multistatus body.
Input validation cannot close either: RFC 5545 §3.3.11 puts `<`
(`%x3C`), `>` (`%x3E`) and `&` (`%x26`) all inside `TSAFE-CHAR`, so they
are legal in a TEXT value.
## The fix
Percent-encode the uid to RFC 3986 `unreserved` when building an href.
`url.PathEscape` is not enough — it leaves sub-delims alone, so `&`
survives:
```
url.PathEscape("a&b<c>d") → "a&b%3Cc%3Ed"
```
Encoding conservatively means no XML metacharacter reaches the document,
so no change to the vendored library is needed. Existing uuid uids are
entirely `unreserved`, so their hrefs are byte-for-byte unchanged and no
client resyncs.
Then decode on the way back in — neither side did:
- echo v5 hands back the still-encoded path segment (`c.Param("task")` →
`evil%2F..%2F%3Cx%3E`), so `TaskHandler` needed the decode.
- `GetResourcesByList` parses hrefs out of the REPORT body and never
decoded, so encoded hrefs the server itself emitted would silently stop
matching and tasks would vanish from multiget responses.
One more, found while checking the other call sites: caldav-go builds
`Resource.Path` from `request.URL.Path`, which Go has already decoded,
and writes it into XML unescaped. So a PROPFIND against the encoded href
echoed the forged form right back:
```
PROPFIND /dav/projects/36/evil%2F..%2F..%2F..%2Fprojects%2F5%2Fpwned%3Cx%3E%26y.ics
→ <D:href>/dav/projects/5/pwned<x>&y.ics</D:href>
```
Task requests now carry a canonical href that the storage returns
instead of echoing the client's path.
## Not fixed here
Principal hrefs interpolate the username unescaped, and
`pkg/user/user_create.go` only rejects spaces and the link-share
pattern. `isOwnPrincipalPath` limits this to the authenticated user's
own username, so it never crosses a user boundary — separate concern,
not uid-related.
## How to verify
1. Create a task over CalDAV whose VTODO carries
`UID:evil/../../../projects/<other project id>/pwned<x>&y` in a project
you own.
2. Run `curl -u <user>:<caldav-token> -X PROPFIND -H 'Depth: 1'
https://<instance>/dav/projects/<that project id>/`
3. **Expected:** the `<D:href>` for that task is percent-encoded, stays
under `/dav/projects/<that project id>/`, and the response body parses
as XML.
**Before this PR:** the href resolves to the other project's collection
and the body is malformed XML.
1. `GET` that percent-encoded href.
2. **Expected:** 200 with the task's VTODO — the uid round-trips.
1. Send a `calendar-multiget` REPORT listing that same href.
2. **Expected:** 207 containing the task.
1. Take any pre-existing task with a normal uuid uid and PROPFIND its
collection.
2. **Expected:** its href is unchanged from before this PR — encoding is
a no-op for uuids, so no client is forced to resync.
---------
Co-authored-by: kolaente <k@knt.li>
Replaces the hand-rolled timestamp check and cleanup goroutine. The old
int64 type assertion never matched on redis (Get returns a string), so
passcode replay protection was silently disabled there.
kolaente
Frederick [Bot]
emilsteixnerandGitHub