Around 38 events are registered for audit logging, including every admin
action, but a full personal data export left no trace at all. It is
dispatched from both v1 and v2, so one registration covers both.
The license servers are hardcoded, but the check client followed redirects
without any policy and dialed without the SSRF guard, so a hijacked or
poisoned license host could forward the license key to an internal address.
Redirects are refused outright rather than capped: the check is a POST to a
fixed JSON API that never redirects.
avatar.gravatarbaseurl is operator-configurable, so the request destination was
never a fixed constant - pkg/utils/avatar.go already uses the SSRF-safe client
for the same job. The previous 5s timeout is kept as a context deadline so the
configured (30s by default) client timeout does not apply here.
doGet built its own http.Client, unlike its siblings in the same file which
already use utils.NewSSRFSafeHTTPClient(). The previous 10s timeout is kept as
a context deadline so the configured (30s by default) client timeout does not
apply here.
The webhook target URL is user-configured, so a hostile target can answer
a delivery with a 4xx/5xx carrying an arbitrarily large body. That body
was read whole into memory and written whole to the log.
Cap the read at 4KiB, which is plenty for a diagnostic log line, matching
the LimitReader already used for the license server response.
v1 only registers /login when local or LDAP auth is enabled, but v2
registered it unconditionally. With auth.local.enabled=false a
pre-existing local password still authenticated on /api/v2/login.
/logout stays unconditional - it terminates any session, OIDC included.
The job pushes with the SSH deploy key configured by actions/checkout's
ssh-key input, so GITHUB_TOKEN is never used for writes. Scope it down to
contents: read.
GetTokenFromTokenString sliced token[len(token)-8:] without checking the
length, so any string with the "tk_" prefix but shorter than 8 characters
panicked with "slice bounds out of range".
The helper is reachable with attacker-controlled input from three
unauthenticated call sites: the main API bearer-token middleware
(/api/v1 and /api/v2) and CalDAV basic auth on /dav/ and
/.well-known/caldav. Each request was turned into a 500 by the global
panic recovery, at the cost of a full stack unwind, an error-level log
line and a Sentry event. On the main API the token middleware also runs
before the rate limiter, and the CalDAV routes are not rate limited at
all.
The same guard already existed in the feeds auth path; fixing it at the
shared choke point closes all sites at once. Real tokens are the prefix
plus 40 hex characters, so no legitimate token is affected — a too-short
token now gets the same rejection as any other invalid one.
The listener runs after the deleting transaction committed, so the task
is already soft-deleted when it looks up who to notify. Every task
subscription lookup filters `t.deleted_at IS NULL`, so the subscriber
list came back empty and nobody was notified - not even users with full
access.
The `IsErrTaskDoesNotExist` fallback to project subscribers never
covered for this: the lookup returns an empty slice with a nil error, so
the branch could not fire. Removed rather than repaired - with
soft-deleted tasks included, the CTE resolves project and parent-project
subscriptions on its own, which is strictly more than the fallback did.
Soft-deleted tasks are opt-in, so the reminder crons keep ignoring them.
Permissions come from the project, since the task can no longer carry
them - a subscriber who lost access still gets nothing.
Asserting only "not 500" let the test pass on a 404, 401 or 403, so it
would have kept passing if the public v2 endpoints broke another way.
Assert 200 per path plus the remaining budget counting down across them,
which is what proves both unauthenticated requests share one ip key.
The default branch of the kind switch logged the misconfiguration and
then continued with an empty key, putting every request of the whole
instance into one shared bucket. Fall back to per-ip limiting instead,
so a typo in ratelimit.kind degrades to the "ip" behaviour.
With ratelimit.kind at its default value of "user", the rate limit
middleware logged the error from GetAuthFromClaims and then dereferenced
the nil web.Auth anyway. Every unauthenticated /api/v2 request produces
exactly that state, since v2 attaches the limiter to the single group
serving its public routes too - so enabling rate limiting turned
/api/v2/info, /api/v2/health and /api/v2/login into 500s. v1 is
unaffected because it splits its unauthenticated routes into their own
ip-keyed subgroups before the "user" limiter is attached.
Fall back to keying by IP, matching the "ip" kind and v1's
unauthenticated groups. Authenticated requests are unchanged.
service.secret had an unconditional random default, and viper's IsSet
reports defaults as set. The deprecation branch therefore always took the
"both keys are set" path and never copied service.jwtsecret over, so
instances configured with the old key silently got a fresh random secret
on every start — flaky 401s across replicas.
The random fallback is now generated after the config file and env are
read, so an empty service.secret unambiguously means "not configured".
Recreates model-declared indexes that are missing, matching by column
set so converted databases with differently-named equivalent indexes
are left alone. Fails with an actionable message if duplicate values
block recreating a unique index.
Fixes#3244
xorm's Sync drops every index and unique constraint the synced struct
doesn't declare (#3244). Adds a partialSync helper (no drops) and
converts all 65 existing-table struct syncs to it — the four v2.4.0
migrations that wiped users/tasks indexes on every upgrade, plus every
historical one back to 2019, each a latent index wipe and a pgloader
2BP01 abort point for installs upgrading from versions predating it.
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.
Route-level tests on both api versions, since the pseudo-projects only get
their parent through the read path. The v2 read body embeds models.Project and
Huma's $schema wrapper copies its fields into a generated struct, so a custom
MarshalJSON on the model would not have applied there — assert the raw JSON
instead of the struct.
781ffac19 (GHSA-44v6-7fxq-vgf4) turned ParentProjectID into a *int64 with
`omitempty` so a write can tell an omitted field from an explicit 0 (detach,
admin only). That also dropped the key from every response where the pointer is
nil: the Favorites and saved-filter pseudo-projects, which are built in Go and
never set it, plus rows whose nullable parent_project_id column was never
backfilled. Clients parse the field as a plain int, so the whole project list
fails to decode (go-vikunja/app#295).
Keep the pointer and its three-way write semantics, but never hand out a nil:
normalize DB rows in AfterLoad and set the field where the pseudo-projects are
built.
Closesgo-vikunja/app#295
kolaente
TinkandGitHub
renovate[bot]andkolaente
Frederick [Bot]