Details
## Summary
A collaborator removed from a project keeps a **live, automatic feed** of that project's contents, because nothing on any revocation path deletes the webhook they created while they had access.
Vikunja already has a revocation-cleanup routine that deletes other derived rows for exactly this reason. Its set is `{task_assignees, subscriptions}`. **`webhooks` and `link_shares` — the only two rows that carry a live channel into the project — are not in it**, and the routine is wired to one of four revocation paths.
## Details
`pkg/models/teams.go:411` — `cleanupTaskMembersAfterTeamRemoval`, added in `9358954c9`, whose commit message states the invariant: *"cleanup team memberships, assignments and subscriptions when users lose access to a project"*.
```go
canRead, _, permErr := project.CanRead(s, &user.User{ID: memberID})
...
if !canRead {
projectsToCleanup = append(projectsToCleanup, projectID)
}
...
_, err = s.In("task_id", taskIDs).And("user_id = ?", memberID).
Delete(&TaskAssginee{})
_, err = s.In("entity_id", taskIDs).
Where("entity_type = ? AND user_id = ?", SubscriptionEntityTask, memberID).
Delete(&Subscription{})
_, err = s.In("entity_id", projectsToCleanup).
Where("entity_type = ? AND user_id = ?", SubscriptionEntityProject, memberID).
Delete(&Subscription{})
```
So you have already decided that losing read access must delete rows the departing user left behind, and that the test is a live `project.CanRead`. The gap is which rows are in the set.
**And there is a second level, which is the sharper one.** That routine has exactly one caller:
```
pkg/models/listeners.go:1642 err = cleanupTaskMembersAfterTeamRemoval(s, event.Team.ID, event.Member.ID)
```
against four revocation paths:
```
pkg/models/project_team.go:151 TeamProject.Delete
pkg/models/project_users.go:141 ProjectUser.Delete <- grep -c cleanup inside: 0
pkg/models/project_users.go:248 ProjectUser.Update <- the downgrade path
pkg/models/team_members.go:91 TeamMember.Delete <- the only one wired up
```
Removing a user directly from a project — the ordinary operation — dispatches nothing at all.
## PoC
Released `vikunja/vikunja:2.6.0` Docker image.
The harness was proved first by reproducing a **known-fixed** advisory: `GHSA-qfwc` refuses the read-only member and returns the hash to the owner. Every run carries its own negative control.
The strongest form is on the **team-removal path, where your cleanup routine does fire**:
```
== OWNER removes collab from the TEAM ==
-- did the cleanup routine run? --
assignees now: [] <- POSITIVE CONTROL: it ran
collab direct read of task: HTTP 403 <- NEGATIVE CONTROL: access is gone
-- webhook still delivering? --
WEBHOOK RECEIVED task_title='post-team-removal secret'
task_desc='cleanup ran, webhook did not'
-- link share still redeemable? --
[{"title":"post-team-removal secret","description":"cleanup ran, webhook did not"}]
```
The two controls are the argument: your own routine executed and removed the assignee rows, and the collaborator's direct read is refused with 403 — and the webhook created before removal still delivers the contents of a task created *after* it.
On the `ProjectUser.Delete` path the same thing happens with no cleanup running at all.
**One lab accommodation, stated plainly**: non-routable outbound IPs were enabled so a loopback sink was reachable. In a default deployment an attacker simply uses a public URL, so this changes nothing about reachability.
## Impact
A former collaborator receives task titles and descriptions for the whole project, continuously, after their access has been revoked — including content created after revocation.
**The honest counter-argument, which we would rather state than have you find.** Both artefacts stay **visible and auditable to the owner** afterwards: `GET /projects/{p}/webhooks` still shows `{"created_by":"collab"}` and `GET /projects/{p}/shares` still shows `{"shared_by":"collab"}`. An administrator reviewing project settings after an offboarding will find them. That is genuinely weaker than a silent channel, and it argues for the low end of Medium.
The precondition is also real: the attacker held write access, so they could have taken a snapshot before leaving. **The only thing new here is access to content created after revocation** — which is precisely the line you drew yourselves in `GHSA-jp29-jrxc-92vf`, *"favourites readable after revocation"*. We think it holds. It is also the entirety of the claim, and we are not dressing it up as more.
## Affected range
**All releases from `v0.22.0` through `v2.6.0`, and current `main`.**
Determined by checking the code at each released tag:
```
v0.22.0 … v0.24.6 webhooks.go=yes cleanupRoutine=0
v1.0.0 … v2.6.0 webhooks.go=yes cleanupRoutine=1
```
Project webhooks arrive in `ad7d485eb` (2023-10-17), first released in v0.22.0. The cleanup routine arrives in `9358954c9` (2025-10-09), first released in v1.0.0, and has never included webhooks or link shares. Still present on `main` at `d822e1c58`: the routine contains only `Delete(&TaskAssginee{})` and two `Delete(&Subscription{})`, and `ProjectUser.Delete` contains zero dispatches.
Reproduced at v2.6.0; earlier versions asserted from source rather than run, and we are saying which is which.
## Recommended Fix
Two changes, and they are independent — the first is the smaller one, the second is the one that closes the class.
**Add the two row types to the cleanup set.** Alongside the existing `task_assignees` and `subscriptions` deletions, delete `webhooks` and `link_shares` whose `created_by` / `shared_by` is the departing user and whose project is in `projectsToCleanup`. The existing live `project.CanRead` test is already the right predicate.
**Dispatch the cleanup from all four revocation paths**, not only team-member removal. `ProjectUser.Delete` and `ProjectUser.Update` (the downgrade case) are the common operations and currently fire nothing; `TeamProject.Delete` unshares a whole project from a team and is equally a revocation.
A cheaper alternative for the second half, if reworking dispatch is unattractive: check `CanRead` for the webhook's `created_by` at delivery time, and for the share's `shared_by` at redemption time. That converts a cleanup problem into an authorization check on a path that already has the user id to hand.
## Severity
`CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:N/A:N` — **6.5 Medium**.
This is **the identical vector you assigned to your own webhook advisory**, `GHSA-7c2g-p23p-4jg3` (webhook BasicAuth credentials exposed to read-only members), copied rather than argued. `PR:L` because the attacker must have been provisioned with project write at some point; `C:H` because the channel carries full task titles and descriptions for the whole project continuously — the same `C:H` you assigned there for a credential leak of narrower scope.
The auditability caveat above argues for the low end of Medium and we would not contest a lower score.
### The link share is strictly more capable, and we are not leading with it
The same gap leaves a link share created by the departing collaborator redeemable after revocation, and a link share carries **read and write** — HTTP 201 demonstrated. On the same reasoning that would be `C:H/I:H` = 8.1 High.
We are deliberately **not** proposing that, and leading with the webhook instead, because a link share has a real *"it is designed to be handed out"* defence and the webhook does not. That judgement is yours to make, and if you decide the share is the more serious half we will not argue.
