github-starred/vikunja
2
0
Fork 0
mirror of https://github.com/go-vikunja/vikunja.git synced 2026-09-04 18:48:13 -05:00
Code Issues 690 Packages Projects Releases 13 Wiki Activity

[PR #2186] [MERGED] fix(files): require io.ReadSeeker for S3 uploads to prevent permission denied errors #3982

New Issue
Closed
opened 2026-03-22 14:58:08 -05:00 by GiteaMirror · 0 comments
GiteaMirror commented 2026-03-22 14:58:08 -05:00
Owner
Copy Link
Copy Source

📋 Pull Request Information

Original PR: https://github.com/go-vikunja/vikunja/pull/2186
Author: @kolaente
Created: 2/1/2026
Status: ✅ Merged
Merged: 2/8/2026
Merged by: @kolaente

Base: main ← Head: fix/s3-avatar-upload-permission-denied

📝 Commits (10+)

  • 2a10d6a fix(files): require io.ReadSeeker for S3 uploads, remove temp file fallback
  • d8278f7 fix(files): update all callers to provide seekable readers for S3 uploads
  • 753817a test(files): update tests for io.ReadSeeker API
  • c1d9c5f fix(dump): stream files during restore to avoid memory pressure
  • 49512a5 fix(files): seek to start before writing for consistent behavior
  • ccab67a fix(backgrounds): enforce max file size for unsplash downloads
  • afae368 fix(dump): limit copy size to prevent decompression bombs
  • 132a825 fix(backgrounds): avoid integer overflow in max size calculation
  • 06cebf0 refactor(files): remove redundant seek operations in writeToStorage
  • 3c82426 fix(backgrounds): stream unsplash download to temp file instead of memory

📊 Changes

10 files changed (+106 additions, -240 deletions)

View changed files

📝 pkg/files/files.go (+16 -96)
📝 pkg/files/files_test.go (+3 -27)
📝 pkg/files/s3_test.go (+7 -75)
📝 pkg/models/task_attachment.go (+1 -2)
📝 pkg/models/task_attachment_test.go (+2 -23)
📝 pkg/modules/avatar/upload/upload.go (+1 -1)
📝 pkg/modules/background/handler/background.go (+1 -1)
📝 pkg/modules/background/unsplash/unsplash.go (+33 -1)
📝 pkg/modules/dump/restore.go (+41 -11)
📝 pkg/modules/migration/create_from_structure.go (+1 -3)

📄 Description

When uploading avatars (and background images), the resized image was passed as a *bytes.Buffer to the file storage layer. Since *bytes.Buffer does not implement io.ReadSeeker, the S3 upload code fell back to creating temporary files (vikunja-s3-upload-*) on the local filesystem. In Docker containers with restrictive permissions, this temp file creation failed. Regular file attachments worked because multipart.File already implements io.ReadSeeker.

Fix: Changed the file storage API (Create, CreateWithMime, CreateWithMimeAndSession, Save) to require io.ReadSeeker instead of io.Reader. This:

  • Eliminates the temp file fallback entirely — no more local filesystem writes during S3 uploads
  • Enforces at the type level that all callers provide seekable readers, preventing this class of bug from recurring
  • Updated all callers (avatar upload, background upload, unsplash, dump restore, migration) to provide *bytes.Reader or equivalent seekable readers

Fixes #2185

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/go-vikunja/vikunja/pull/2186 **Author:** [@kolaente](https://github.com/kolaente) **Created:** 2/1/2026 **Status:** ✅ Merged **Merged:** 2/8/2026 **Merged by:** [@kolaente](https://github.com/kolaente) **Base:** `main` ← **Head:** `fix/s3-avatar-upload-permission-denied` --- ### 📝 Commits (10+) - [`2a10d6a`](https://github.com/go-vikunja/vikunja/commit/2a10d6a6474ada5b91e11557b82a05fab5c9b684) fix(files): require io.ReadSeeker for S3 uploads, remove temp file fallback - [`d8278f7`](https://github.com/go-vikunja/vikunja/commit/d8278f7d337e2e3e5189c280a01ff29fe420307c) fix(files): update all callers to provide seekable readers for S3 uploads - [`753817a`](https://github.com/go-vikunja/vikunja/commit/753817a5594ad4fc60d295c85a4dc90f7e1c0591) test(files): update tests for io.ReadSeeker API - [`c1d9c5f`](https://github.com/go-vikunja/vikunja/commit/c1d9c5fa63700ee02070a0f59cac97a8d00ba3eb) fix(dump): stream files during restore to avoid memory pressure - [`49512a5`](https://github.com/go-vikunja/vikunja/commit/49512a5969b59e7fa6333be69b7533d286afcfd9) fix(files): seek to start before writing for consistent behavior - [`ccab67a`](https://github.com/go-vikunja/vikunja/commit/ccab67a86acd7550344630d39f8bf68f39c83ae1) fix(backgrounds): enforce max file size for unsplash downloads - [`afae368`](https://github.com/go-vikunja/vikunja/commit/afae3681a41658b8d61fd2fbe01bde5487559e96) fix(dump): limit copy size to prevent decompression bombs - [`132a825`](https://github.com/go-vikunja/vikunja/commit/132a8258551e18643f488f3b2391a78c15572341) fix(backgrounds): avoid integer overflow in max size calculation - [`06cebf0`](https://github.com/go-vikunja/vikunja/commit/06cebf0156d4f26313aa6db403fc4df919a114a1) refactor(files): remove redundant seek operations in writeToStorage - [`3c82426`](https://github.com/go-vikunja/vikunja/commit/3c824269c879ef227f3bf273dd6bbd5573dc30a5) fix(backgrounds): stream unsplash download to temp file instead of memory ### 📊 Changes **10 files changed** (+106 additions, -240 deletions) <details> <summary>View changed files</summary> 📝 `pkg/files/files.go` (+16 -96) 📝 `pkg/files/files_test.go` (+3 -27) 📝 `pkg/files/s3_test.go` (+7 -75) 📝 `pkg/models/task_attachment.go` (+1 -2) 📝 `pkg/models/task_attachment_test.go` (+2 -23) 📝 `pkg/modules/avatar/upload/upload.go` (+1 -1) 📝 `pkg/modules/background/handler/background.go` (+1 -1) 📝 `pkg/modules/background/unsplash/unsplash.go` (+33 -1) 📝 `pkg/modules/dump/restore.go` (+41 -11) 📝 `pkg/modules/migration/create_from_structure.go` (+1 -3) </details> ### 📄 Description When uploading avatars (and background images), the resized image was passed as a `*bytes.Buffer` to the file storage layer. Since `*bytes.Buffer` does not implement `io.ReadSeeker`, the S3 upload code fell back to creating temporary files (`vikunja-s3-upload-*`) on the local filesystem. In Docker containers with restrictive permissions, this temp file creation failed. Regular file attachments worked because `multipart.File` already implements `io.ReadSeeker`. **Fix:** Changed the file storage API (`Create`, `CreateWithMime`, `CreateWithMimeAndSession`, `Save`) to require `io.ReadSeeker` instead of `io.Reader`. This: - Eliminates the temp file fallback entirely — no more local filesystem writes during S3 uploads - Enforces at the **type level** that all callers provide seekable readers, preventing this class of bug from recurring - Updated all callers (avatar upload, background upload, unsplash, dump restore, migration) to provide `*bytes.Reader` or equivalent seekable readers Fixes #2185 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
GiteaMirror added the pull-request label 2026-03-22 14:58:08 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area/api
area/attachments
area/auth
area/avatars
area/backup-restore
area/caldav
area/calendar-view
area/comments
area/config
area/database
area/desktop
area/docker
area/email
area/favorites
area/filters
area/frontend
area/gantt
area/i18n
area/import-export
area/internal-code
area/kanban
area/labels
area/list-view
area/mobile
area/notifications
area/permissions
area/projects
area/pwa
area/recurring-tasks
area/reminders
area/search
area/shortcuts
area/subtasks
area/sync
area/table-view
area/task-editor
area/task-metadata
area/task-relations
area/teams
area/theming
area/time-tracking
area/typesense
area/views
area/webhooks
bug
changes requested
concern/accessibility
concern/performance
concern/regression
concern/ux
confirmed
db/mysql
dependencies
enhancement
good first issue
help wanted
integration/inbound
integration/outbound
kind/bug
kind/feature
needs reproduction
pull-request
Mirrored from GitHub Pull Request
 pull-request
Mirrored from GitHub Pull Request
 pull-request
Mirrored from GitHub Pull Request
 pull-request
Mirrored from GitHub Pull Request
 question
security
support
upstream issue
waiting for reply
wontfix
No labels pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
GiteaMirror ninjasurge (Kaehlen Rog)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/vikunja#3982