Address review decision: SaveAvatar removed the old file's blob from
storage inside the transaction, so a later rollback left the user on an
avatar whose content was gone. The files row is still deleted in the
session; the blob and its previews go once the session commits.
Address pr-swarm finding: Open went through OpenWithSize, which failed on S3-compatible backends that answer without a Content-Length, breaking every original download there.
Previews live under previews/<file id>_<size>, next to the files they
are derived from but apart from them, so nothing that reads files by
their database id ever sees one.
A file was created under its final name and then filled, so a reader
could open it half written and a failed write left a truncated file
behind. That did not matter for uploads, which are only readable once
their database row is committed, but a stored preview is looked up by
its name alone and would be served truncated from then on.
File storage needs to know every size a preview can be stored in so it
can remove them all without listing the storage. The models package
keeps its names as aliases.
ErrCodeFileIsTooLarge shared 4013 with ErrCodeInvalidSortParam, which has
held the code since the sort order feature landed, so an oversized upload
rendered "The task sort param is invalid." in clients.
BREAKING CHANGE: uploading a file larger than the configured limit now
returns 4035 instead of 4013.
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.
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.
Local creates a missing base path and is idempotent; ValidateBasePath rejects a
missing path without creating it and rejects a regular file. Mem and s3 are
no-ops that never touch a client.
filehandling.go decided "is this local storage?" in three places: backend
selection, ensureLocalBasePath, and ValidateFileStorage. The latter two
re-derived a decision their caller had already made, and ensureLocalBasePath's
whole body was guarded by a check its only caller had just performed.
Two code paths independently deriving the same fact is what produced the path
divergence and the over-broad directory creation earlier in this series. The
backend now owns both answers: Ensure creates what it needs, ValidateBasePath
checks it. No-ops for s3 and mem, beside the existing MkdirAll no-op.
config.FilesType is now read once, at backend selection. Pure refactor: error
strings and creation behaviour are unchanged.
The S3 backend passed context.Background() to every AWS call, so an unreachable
but well-formed endpoint blocked until the OS TCP timeout with no way to
interrupt it. Write, Stat and Remove now take the caller's context.
TestInitFileHandler_S3Configuration and TestInitFileHandler_LocalFilesystem
replaced the package-level mem storage and never put it back, so any test
ordered after them failed. Pre-existing; surfaced by running with -shuffle=on.
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.
Running `vikunja doctor` outside the install directory created a `files`
directory in the current working directory and then reported it as existing and
writable, which reads as a passing check against storage doctor made itself.
Use the read-only file handler init. A missing base directory is now reported
as a failed check instead of being silently created.
ValidateFileStorage created the local base directory as a side effect of
checking it. Move that into ensureLocalBasePath so callers that only want to
inspect storage can do so without creating it.
InitFileHandler now calls both, so server and restore behavior is unchanged.
InitFileHandlerReadOnly sets up the storage backend and nothing else.
Querying the global engine while the request already holds an open
transaction acquires a second pool connection. Under concurrent
attachment downloads (e.g. a kanban board full of image previews) all
pool connections end up held by transactions that each wait for an
extra connection that can never be freed, hanging every request until
restart.
The redundant meta load in the upload avatar provider is removed
entirely; the decoded image is all that path uses.
Authoritative size now comes from the reader instead of the caller's
claim in CreateWithMimeAndSession. The migration import path accepts
attacker-controlled metadata (GHSA-qh78-rvg3-cv54), so trusting
realsize for the limit check allowed oversized uploads to be accepted
and stored.
measureReaderSize leaves the reader seeked to 0 so the measured value
matches the bytes storage backends will actually write.
Replace the github.com/spf13/afero dependency with a purpose-built
FileStorage interface (Open, Write, Stat, Remove, MkdirAll) with three
implementations: localStorage (with basePath), s3Storage (with key
prefix), and memStorage (for tests).
Each implementation owns its base path — callers pass only file IDs.
Delete s3fs.go, change File.File from afero.File to io.ReadCloser,
and fix duplication flows to buffer content for seeking.
http.ServeContent requires io.ReadSeeker which S3 files don't support.
Add S3 branch using io.Copy with explicit headers, matching the pattern
already used in task attachment downloads.
Refs: #2347
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the third-party afero-s3 library (and its temporary fork) with
a minimal in-tree afero.Fs implementation (~200 lines) that only supports
the three S3 operations Vikunja actually uses: Open (GetObject), Remove
(DeleteObject), and Stat (HeadObject).
The s3File implementation lazily opens a GetObject stream on first Read
and supports Seek by closing and re-opening the stream from the new offset,
matching the behavior of the fixed afero-s3 fork.
All other afero.Fs/File methods return ErrS3NotSupported since they are
never called in the S3 code path (writes already use direct PutObject).
This removes the fclairamb/afero-s3 dependency and the temporary replace
directive in go.mod entirely.
Closes#2347
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
File.Delete() had s.Commit() and s.Rollback() calls that could
prematurely commit or abort an outer transaction when using a shared
session. The caller is now responsible for transaction management.
Refactor functions that created their own sessions when called from
within existing transactions, which caused "database table is locked"
errors in SQLite's shared-cache mode.
Changes:
- Add files.CreateWithSession() to reuse caller's session
- Refactor DeleteBackgroundFileIfExists() to accept session parameter
- Add variadic session parameter to notifications.Notify() and
Notifiable.ShouldNotify() interface
- Update all Notify callers (~17 sites) to pass their session through
- Use files.CreateWithSession in SaveBackgroundFile and NewAttachment
- Fix test code to commit sessions before assertions
Add a CLI command that finds all files in the database with no MIME type
set, detects it from the stored file content, and updates the database.
This is useful for backfilling MIME types on files created before MIME
detection was added on upload.
Use mimetype.DetectReader in files.Create to automatically detect and
store the MIME type from file content. This fixes task attachment
previews, S3 Content-Type headers, and UI display for newly uploaded
files.
Adds `files.s3.disablesigning` config option that sends
`UNSIGNED-PAYLOAD` instead of computing SHA256 hashes for S3 request
signing which fixes `XAmzContentSHA256Mismatch` errors with
S3-compatible providers like Ceph RadosGW and Clever Cloud Cellar
Resolves https://github.com/go-vikunja/vikunja/issues/2181
Use a temp file instead of io.ReadAll to avoid buffering the entire
Unsplash image in RAM, which could cause OOM with large images or
high maxsize configuration.
Move the seek-to-start into the local file branch only and simplify
contentLengthFromReadSeeker to seek to end then back to 0 instead of
saving/restoring the original position. This reduces the S3 upload
path from 5 seek operations to 2.
- Replace custom testfile structs with bytes.NewReader
- Remove readerOnly wrapper and non-seekable reader tests (no longer
possible at the type level)
- Update S3 unit tests to remove temp file assertions
The S3 upload path used temp files (vikunja-s3-upload-*) to buffer
non-seekable readers. In Docker containers with restrictive permissions,
these temp files could not be created, causing "permission denied"
errors for avatar and background image uploads.
By changing the file storage API (Create, CreateWithMime,
CreateWithMimeAndSession, Save) to require io.ReadSeeker instead of
io.Reader, the temp file fallback is no longer needed and is removed.
This enforces at the type level that all callers provide seekable
readers, preventing this class of bug from recurring.
Closesgo-vikunja/vikunja#2185
This PR adds support for detecting and handling Linux user namespaces (commonly used in rootless Docker containers) and improves error diagnostics when file storage validation fails.
Docs PR: https://github.com/go-vikunja/website/pull/289
---------
Co-authored-by: Claude <noreply@anthropic.com>
kolaente
Robert SzulistandGitHub
XiangCanyandGitHub
renovate[bot]andGitHub