diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 251de910..ce658012 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,6 +89,12 @@ jobs: echo "PostgreSQL auto-assignment concurrency test did not run" >&2 exit 1 } + - name: Test production upload migrations + working-directory: backend + env: + GOCHAT_TEST_DB: postgres + GOCHAT_TEST_DB_URL: postgres://postgres:postgres@localhost:5432/gochat_test?sslmode=disable + run: go test ./internal/database -run '^TestUploadPostgresProductionMigration' -count=1 - name: Test PostgreSQL E2E working-directory: backend env: diff --git a/backend/internal/app/bootstrap.go b/backend/internal/app/bootstrap.go index 0c29cd2d..f8b0a503 100644 --- a/backend/internal/app/bootstrap.go +++ b/backend/internal/app/bootstrap.go @@ -786,8 +786,9 @@ func Bootstrap(env string) (*App, error) { directUploadRepo := repository.NewDirectUploadRepo(db) uploadService := service.NewUploadService(directUploadRepo, cfg). WithWidgetAuth(inboxRepo, contactInboxRepo). - WithConversationRepo(conversationRepo) - uploadHandler := v1.NewUploadHandler(uploadService) + WithConversationRepo(conversationRepo). + WithAccessDB(db) + uploadHandler := v1.NewUploadHandler(uploadService).WithAccessAuth(jwtService) // Step 9: Wire handlers (HTTP presentation layer) contactMergeRepo := repository.NewContactMergeRepo(db) @@ -944,7 +945,8 @@ func Bootstrap(env string) (*App, error) { engine.Use(middleware.RateLimit(rdb)) // rate limiting (ref: Chatwoot rack-attack) engine.Use(corsMiddleware) // CORS with configurable whitelist engine.Use(middleware.SecurityHeaders(middleware.DefaultSecurityHeadersConfig())) // security headers (ref: P14 deliverable #11) - engine.StaticFS("/uploads", gin.Dir(cfg.Storage.LocalPath, false)) + engine.GET("/uploads/*filepath", uploadHandler.ServeUpload) + engine.HEAD("/uploads/*filepath", uploadHandler.ServeUpload) // CSRF protection — double-submit cookie pattern (ref: OWASP CSRF Prevention) // Applies globally; safe methods (GET/HEAD/OPTIONS) auto-set token cookie, diff --git a/backend/internal/database/upload_migration_test.go b/backend/internal/database/upload_migration_test.go new file mode 100644 index 00000000..5e926ca8 --- /dev/null +++ b/backend/internal/database/upload_migration_test.go @@ -0,0 +1,213 @@ +package database + +import ( + "bytes" + "context" + "fmt" + "mime/multipart" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/driver/postgres" + "gorm.io/gorm" + "gorm.io/gorm/logger" + + "github.com/gochat/gochat/internal/config" + "github.com/gochat/gochat/internal/model" + "github.com/gochat/gochat/internal/repository" + "github.com/gochat/gochat/internal/service" +) + +func openUploadMigrationPostgres(t *testing.T) (*gorm.DB, string) { + t.Helper() + if os.Getenv("GOCHAT_TEST_DB") == "sqlite" { + t.Skip("requires PostgreSQL migration semantics") + } + dsn := os.Getenv("GOCHAT_TEST_DB_URL") + if dsn == "" { + dsn = "postgres://postgres:postgres@localhost:5432/gochat_test?sslmode=disable" + } + admin, err := gorm.Open(postgres.Open(dsn), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + require.NoError(t, err) + adminDB, err := admin.DB() + require.NoError(t, err) + t.Cleanup(func() { _ = adminDB.Close() }) + + schema := fmt.Sprintf("upload_migration_%d", time.Now().UnixNano()) + require.NoError(t, admin.Exec("CREATE SCHEMA "+schema).Error) + t.Cleanup(func() { _ = admin.Exec("DROP SCHEMA " + schema + " CASCADE").Error }) + + migrationURL, err := url.Parse(dsn) + require.NoError(t, err) + require.NotEmpty(t, migrationURL.Scheme, "GOCHAT_TEST_DB_URL must be a PostgreSQL URL") + query := migrationURL.Query() + query.Set("search_path", schema) + migrationURL.RawQuery = query.Encode() + + db, err := gorm.Open(postgres.Open(migrationURL.String()), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + require.NoError(t, err) + sqlDB, err := db.DB() + require.NoError(t, err) + sqlDB.SetMaxOpenConns(1) + t.Cleanup(func() { _ = sqlDB.Close() }) + return db, migrationURL.String() +} + +func productionMigrationsPath(t *testing.T) string { + t.Helper() + path, err := filepath.Abs(filepath.Join("..", "..", "migrations")) + require.NoError(t, err) + return path +} + +func TestUploadPostgresProductionMigrationsFromEmptySchema(t *testing.T) { + db, dbURL := openUploadMigrationPostgres(t) + require.NoError(t, RunMigrations(dbURL, productionMigrationsPath(t))) + version, dirty, err := CurrentVersion(dbURL, productionMigrationsPath(t)) + require.NoError(t, err) + assert.Equal(t, uint(83), version) + assert.False(t, dirty) + exerciseUploadProductionSchema(t, db) +} + +func TestUploadPostgresProductionMigrationUpgradesLegacySchema(t *testing.T) { + db, dbURL := openUploadMigrationPostgres(t) + migrations := productionMigrationsPath(t) + require.NoError(t, MigrateSteps(dbURL, migrations, 82)) + var legacyAccountID, legacyInboxID uint + require.NoError(t, db.Raw("INSERT INTO accounts(name) VALUES ('legacy account') RETURNING id").Scan(&legacyAccountID).Error) + require.NoError(t, db.Raw(`INSERT INTO inboxes(account_id, name, channel_type, channel_id) + VALUES (?, 'legacy inbox', 'web_widget', 1) RETURNING id`, legacyAccountID).Scan(&legacyInboxID).Error) + require.NoError(t, db.Exec(`INSERT INTO direct_uploads(id, account_id, file_name, file_url, file_size, content_type) + VALUES (44, ?, 'legacy.png', '/uploads/account/1/legacy.png', 12, 'image/png')`, legacyAccountID).Error) + require.NoError(t, db.Exec(`INSERT INTO widget_file_uploads(id, inbox_id, enabled, max_file_size, allowed_types) + VALUES (7, ?, false, 4096, '["image/png"]')`, legacyInboxID).Error) + + require.NoError(t, MigrateSteps(dbURL, migrations, 1)) + assertLegacyUploadMapping(t, db) + exerciseUploadProductionSchema(t, db) + + require.NoError(t, MigrateSteps(dbURL, migrations, -1)) + assert.True(t, db.Migrator().HasColumn("direct_uploads", "file_name")) + assert.False(t, db.Migrator().HasColumn("direct_uploads", "upload_uuid")) + var legacyName, legacyMIME string + require.NoError(t, db.Raw("SELECT file_name, content_type FROM direct_uploads WHERE id = 44").Row().Scan(&legacyName, &legacyMIME)) + assert.Equal(t, "legacy.png", legacyName) + assert.Equal(t, "image/png", legacyMIME) + assertLegacyWidgetConfig(t, db, "widget_file_uploads") + + require.NoError(t, MigrateSteps(dbURL, migrations, 1)) + assertLegacyUploadMapping(t, db) +} + +func assertLegacyUploadMapping(t *testing.T, db *gorm.DB) { + t.Helper() + var upload model.DirectUpload + require.NoError(t, db.First(&upload, 44).Error) + assert.NotEmpty(t, upload.UploadUUID) + assert.Equal(t, model.DirectUploadStatusCompleted, upload.Status) + assert.Equal(t, model.DirectUploadSourceAccount, upload.Source) + assert.Equal(t, "legacy.png", upload.OriginalName) + assert.Equal(t, "image", upload.FileType) + assert.Equal(t, "image/png", upload.MimeType) + assert.Equal(t, int64(12), upload.FileSize) + assert.JSONEq(t, `{}`, string(upload.Metadata)) + assert.False(t, upload.ExpiresAt.IsZero()) + assertLegacyWidgetConfig(t, db, "widget_file_upload_configs") +} + +func assertLegacyWidgetConfig(t *testing.T, db *gorm.DB, table string) { + t.Helper() + var enabled bool + var maxFileSize int + var allowedTypes string + require.NoError(t, db.Raw("SELECT enabled, max_file_size, allowed_types::text FROM "+table+" WHERE id = 7").Row().Scan(&enabled, &maxFileSize, &allowedTypes)) + assert.False(t, enabled) + assert.Equal(t, 4096, maxFileSize) + assert.JSONEq(t, `["image/png"]`, allowedTypes) +} + +func exerciseUploadProductionSchema(t *testing.T, db *gorm.DB) { + t.Helper() + ctx := context.Background() + var accountID, inboxID, contactID, contactInboxID uint + require.NoError(t, db.Raw("INSERT INTO accounts(name) VALUES (?) RETURNING id", "upload migration account").Scan(&accountID).Error) + require.NoError(t, db.Raw(`INSERT INTO inboxes(account_id, name, channel_type, channel_id, enabled, channel_config) + VALUES (?, ?, 'web_widget', 1, true, ?) RETURNING id`, accountID, "upload migration inbox", `{"website_token":"migration-website"}`).Scan(&inboxID).Error) + require.NoError(t, db.Raw("INSERT INTO contacts(account_id, name) VALUES (?, ?) RETURNING id", accountID, "migration contact").Scan(&contactID).Error) + require.NoError(t, db.Raw(`INSERT INTO contact_inboxes(contact_id, inbox_id, pubsub_token) + VALUES (?, ?, ?) RETURNING id`, contactID, inboxID, "migration-widget").Scan(&contactInboxID).Error) + require.NotZero(t, contactInboxID) + + tmpDir := t.TempDir() + uploadService := service.NewUploadService(repository.NewDirectUploadRepo(db), &config.Config{ + Storage: config.StorageConfig{LocalPath: tmpDir, MaxFileSize: 20 << 20}, + }).WithWidgetAuth(repository.NewInboxRepo(db), repository.NewContactInboxRepo(db)).WithAccessDB(db) + direct, err := uploadService.AccountDirectUpload(ctx, accountID, service.AccountDirectUploadRequest{ + FileHeader: uploadMigrationFileHeader(t, "direct.png"), + }) + require.NoError(t, err) + assert.Equal(t, string(model.DirectUploadStatusPending), direct.Status) + _, ok := uploadService.ResolveAuthorizedUpload(ctx, direct.FileURL, accountID, "") + assert.True(t, ok) + _, ok = uploadService.ResolveAuthorizedUpload(ctx, direct.FileURL, accountID+1, "") + assert.False(t, ok) + + widgetService := service.NewWidgetService( + repository.NewInboxRepo(db), + repository.NewContactRepo(db), + repository.NewContactInboxRepo(db), + repository.NewConversationRepo(db), + repository.NewMessageRepo(db), + nil, nil, nil, + repository.NewWidgetFileUploadRepo(db), + nil, nil, nil, nil, + ) + widgetHeader := uploadMigrationFileHeader(t, "widget.png") + widgetReader, err := widgetHeader.Open() + require.NoError(t, err) + t.Cleanup(func() { _ = widgetReader.Close() }) + widget, err := widgetService.StageFileUpload(ctx, service.WidgetUploadRequest{ + WebsiteToken: "migration-website", + WidgetToken: "migration-widget", + FileHeader: widgetHeader, + }, widgetReader) + require.NoError(t, err) + status, err := widgetService.GetFileUploadStatus(ctx, "migration-website", widget.UploadUUID, "migration-widget") + require.NoError(t, err) + require.NotNil(t, status) + assert.Equal(t, widget.UploadUUID, status.UploadUUID) + + widgetPath := filepath.Join(tmpDir, filepath.FromSlash(strings.TrimPrefix(widget.FileURL, "/uploads/"))) + require.NoError(t, os.MkdirAll(filepath.Dir(widgetPath), 0o755)) + require.NoError(t, os.WriteFile(widgetPath, uploadMigrationPNG, 0o600)) + _, ok = uploadService.ResolveAuthorizedUpload(ctx, widget.FileURL, 0, "migration-widget") + assert.True(t, ok) + _, ok = uploadService.ResolveAuthorizedUpload(ctx, widget.FileURL, 0, "wrong-widget") + assert.False(t, ok) +} + +var uploadMigrationPNG = []byte{0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n', 0, 0, 0, 0} + +func uploadMigrationFileHeader(t *testing.T, name string) *multipart.FileHeader { + t.Helper() + var body bytes.Buffer + writer := multipart.NewWriter(&body) + part, err := writer.CreateFormFile("file", name) + require.NoError(t, err) + _, err = part.Write(uploadMigrationPNG) + require.NoError(t, err) + require.NoError(t, writer.Close()) + req := httptest.NewRequest("POST", "/", &body) + req.Header.Set("Content-Type", writer.FormDataContentType()) + require.NoError(t, req.ParseMultipartForm(1<<20)) + t.Cleanup(func() { _ = req.MultipartForm.RemoveAll() }) + return req.MultipartForm.File["file"][0] +} diff --git a/backend/internal/handler/api/v1/profile_handler_test.go b/backend/internal/handler/api/v1/profile_handler_test.go index 00afcc77..1e4be82c 100644 --- a/backend/internal/handler/api/v1/profile_handler_test.go +++ b/backend/internal/handler/api/v1/profile_handler_test.go @@ -1103,7 +1103,7 @@ func (s *ProfileHandlerTestSuite) TestUpdate_MultipartFormProfileParity() { s.Require().NoError(writer.WriteField("profile[ui_settings][editor_message_key]", "enter")) fileWriter, err := writer.CreateFormFile("profile[avatar]", "avatar.png") s.Require().NoError(err) - _, err = fileWriter.Write([]byte("fake image bytes")) + _, err = fileWriter.Write(handlerTestPNG) s.Require().NoError(err) s.Require().NoError(writer.Close()) diff --git a/backend/internal/handler/api/v1/upload_handler.go b/backend/internal/handler/api/v1/upload_handler.go index 799afef5..226329e0 100644 --- a/backend/internal/handler/api/v1/upload_handler.go +++ b/backend/internal/handler/api/v1/upload_handler.go @@ -1,6 +1,7 @@ package v1 import ( + "encoding/json" "errors" "net/http" "strings" @@ -8,13 +9,21 @@ import ( "github.com/gin-gonic/gin" "gorm.io/gorm" + "github.com/gochat/gochat/internal/auth" "github.com/gochat/gochat/internal/service" "github.com/gochat/gochat/pkg/response" ) // UploadHandler handles file upload endpoints (account-level + widget direct + account direct). type UploadHandler struct { - svc *service.UploadService + svc *service.UploadService + jwtSvc *auth.JWTService +} + +// WithAccessAuth enables authenticated serving of local private uploads. +func (h *UploadHandler) WithAccessAuth(jwtSvc *auth.JWTService) *UploadHandler { + h.jwtSvc = jwtSvc + return h } // NewUploadHandler creates a new UploadHandler. @@ -25,6 +34,7 @@ func NewUploadHandler(svc *service.UploadService) *UploadHandler { // Upload handles POST /api/v1/accounts/:id/upload — account-level file upload. // Reference: Chatwoot api/v1/accounts/:account_id/upload func (h *UploadHandler) Upload(c *gin.Context) { + h.limitBody(c) accountID := getAccountID(c) if accountID == 0 { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, "account_id is required") @@ -48,7 +58,11 @@ func (h *UploadHandler) Upload(c *gin.Context) { fileHeader, err = c.FormFile("file") } if err != nil { - c.JSON(http.StatusUnprocessableEntity, gin.H{"error": "missing input"}) + status := http.StatusUnprocessableEntity + if isUploadBodyTooLarge(err) { + status = http.StatusRequestEntityTooLarge + } + c.JSON(status, gin.H{"error": "missing input"}) return } result, svcErr = h.svc.AccountUpload(c.Request.Context(), accountID, service.AccountUploadRequest{ @@ -74,6 +88,7 @@ func (h *UploadHandler) Upload(c *gin.Context) { // DirectUpload handles POST /api/v1/widget/direct_uploads — widget direct file upload. // Reference: Chatwoot POST /widget/direct_uploads func (h *UploadHandler) DirectUpload(c *gin.Context) { + h.limitBody(c) if strings.Contains(c.GetHeader("Content-Type"), "application/json") { var req service.ActiveStorageDirectUploadRequest if err := c.ShouldBindJSON(&req); err != nil { @@ -93,12 +108,18 @@ func (h *UploadHandler) DirectUpload(c *gin.Context) { fileHeader, err := c.FormFile("file") if err != nil { - response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, "file is required") + if isUploadBodyTooLarge(err) { + c.AbortWithStatusJSON(http.StatusRequestEntityTooLarge, gin.H{"error": "upload body is too large"}) + } else { + response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, "file is required") + } return } result, svcErr := h.svc.WidgetDirectUpload(c.Request.Context(), service.WidgetDirectUploadRequest{ - FileHeader: fileHeader, + WebsiteToken: c.Query("website_token"), + AuthToken: c.GetHeader("X-Auth-Token"), + FileHeader: fileHeader, }) if svcErr != nil { handleServiceError(c, svcErr) @@ -109,7 +130,8 @@ func (h *UploadHandler) DirectUpload(c *gin.Context) { } func (h *UploadHandler) CompleteWidgetDirectUpload(c *gin.Context) { - result, svcErr := h.svc.CompleteWidgetDirectUpload(c.Request.Context(), c.Param("upload_uuid"), c.Request.Body) + h.limitBody(c) + result, svcErr := h.svc.CompleteWidgetDirectUpload(c.Request.Context(), c.Param("upload_uuid"), c.Request.Body, c.Query("token")) if svcErr != nil { handleServiceError(c, svcErr) return @@ -121,6 +143,7 @@ func (h *UploadHandler) CompleteWidgetDirectUpload(c *gin.Context) { // Returns a blob/UUID for later attachment to messages. // Reference: Chatwoot POST /api/v1/accounts/:account_id/direct_uploads func (h *UploadHandler) AccountDirectUpload(c *gin.Context) { + h.limitBody(c) accountID := getAccountID(c) if accountID == 0 { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, "account_id is required") @@ -129,7 +152,11 @@ func (h *UploadHandler) AccountDirectUpload(c *gin.Context) { fileHeader, err := c.FormFile("file") if err != nil { - response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, "file is required") + if isUploadBodyTooLarge(err) { + c.AbortWithStatusJSON(http.StatusRequestEntityTooLarge, gin.H{"error": "upload body is too large"}) + } else { + response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, "file is required") + } return } @@ -148,6 +175,7 @@ func (h *UploadHandler) AccountDirectUpload(c *gin.Context) { // endpoint used by the reused dashboard message composer. // Reference: POST /api/v1/accounts/:account_id/conversations/:conversation_id/direct_uploads func (h *UploadHandler) ConversationDirectUpload(c *gin.Context) { + h.limitBody(c) accountID := getAccountID(c) if accountID == 0 { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, "account_id is required") @@ -178,6 +206,7 @@ func (h *UploadHandler) ConversationDirectUpload(c *gin.Context) { } func (h *UploadHandler) CompleteConversationDirectUpload(c *gin.Context) { + h.limitBody(c) accountID := getAccountID(c) if accountID == 0 { response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, "account_id is required") @@ -196,3 +225,67 @@ func (h *UploadHandler) CompleteConversationDirectUpload(c *gin.Context) { } response.OK(c, result) } + +// ServeUpload replaces the public StaticFS route with account/widget-session +// authorization. Deliberately return 404 for every denied lookup to avoid an +// attachment existence oracle across tenants. +func (h *UploadHandler) ServeUpload(c *gin.Context) { + fileURL := "/uploads/" + strings.TrimPrefix(c.Param("filepath"), "/") + fullPath, ok := h.svc.ResolveAuthorizedUpload(c.Request.Context(), fileURL, h.dashboardAccountID(c), widgetAccessToken(c)) + if !ok { + c.Status(http.StatusNotFound) + return + } + c.Header("Cache-Control", "private, no-store") + c.Header("Content-Security-Policy", "sandbox") + c.Header("X-Content-Type-Options", "nosniff") + c.File(fullPath) +} + +func (h *UploadHandler) limitBody(c *gin.Context) { + if c.Request.Body != nil { + limit := int64(21 << 20) + if h.svc != nil { + limit = h.svc.MaxRequestBodySize() + } + c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, limit) + } +} + +func (h *UploadHandler) dashboardAccountID(c *gin.Context) uint { + if h.jwtSvc == nil { + return 0 + } + token := strings.TrimSpace(c.GetHeader("access-token")) + if authorization := c.GetHeader("Authorization"); strings.HasPrefix(authorization, "Bearer ") { + token = strings.TrimPrefix(authorization, "Bearer ") + } + if token == "" { + if cookie, err := c.Cookie("cw_d_session_info"); err == nil { + var session map[string]any + if json.Unmarshal([]byte(cookie), &session) == nil { + token, _ = session["access-token"].(string) + } + } + } + claims, err := h.jwtSvc.ValidateAccessToken(token) + if err != nil { + return 0 + } + return claims.AccountID +} + +func widgetAccessToken(c *gin.Context) string { + for _, token := range []string{c.GetHeader("X-Widget-Token"), c.GetHeader("X-Auth-Token")} { + if token != "" { + return token + } + } + token, _ := c.Cookie("cw_conversation") + return token +} + +func isUploadBodyTooLarge(err error) bool { + var maxBytesError *http.MaxBytesError + return errors.As(err, &maxBytesError) || strings.Contains(strings.ToLower(err.Error()), "request body too large") +} diff --git a/backend/internal/handler/api/v1/upload_handler_test.go b/backend/internal/handler/api/v1/upload_handler_test.go index c9621813..79f232be 100644 --- a/backend/internal/handler/api/v1/upload_handler_test.go +++ b/backend/internal/handler/api/v1/upload_handler_test.go @@ -11,6 +11,7 @@ import ( "strconv" "strings" "testing" + "time" "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" @@ -19,12 +20,15 @@ import ( "gorm.io/gorm" "gorm.io/gorm/logger" + "github.com/gochat/gochat/internal/auth" "github.com/gochat/gochat/internal/config" "github.com/gochat/gochat/internal/model" "github.com/gochat/gochat/internal/repository" "github.com/gochat/gochat/internal/service" ) +var handlerTestPNG = []byte{0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n', 0, 0, 0, 0} + // mockUploadService implements a mock for UploadService handler tests. // We can't easily mock UploadService because it's a concrete type, not an interface. // Instead, we test the handler layer by checking HTTP status codes and response shapes. @@ -99,7 +103,7 @@ func TestUploadHandler_Upload_ChatwootAttachmentFieldRawPayload(t *testing.T) { uploadSvc := service.NewUploadService(repository.NewDirectUploadRepo(db), &config.Config{Storage: config.StorageConfig{LocalPath: tmpDir, MaxFileSize: 50 << 20}}) router := setupUploadHandlerRouter(NewUploadHandler(uploadSvc)) - body, contentType, err := makeMultipartUploadBodyWithField("attachment", "macro.png", []byte("fake png")) + body, contentType, err := makeMultipartUploadBodyWithField("attachment", "macro.png", handlerTestPNG) require.NoError(t, err) req, _ := http.NewRequest("POST", "/api/v1/accounts/1/upload", body) req.Header.Set("Content-Type", contentType) @@ -116,7 +120,7 @@ func TestUploadHandler_Upload_ChatwootAttachmentFieldRawPayload(t *testing.T) { assert.Equal(t, payload["blob_id"], payload["blob_key"]) } -func TestUploadHandler_Upload_ChatwootExternalURLRawPayload(t *testing.T) { +func TestUploadHandler_Upload_RejectsLoopbackExternalURL(t *testing.T) { tmpDir := t.TempDir() db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) require.NoError(t, err) @@ -136,12 +140,10 @@ func TestUploadHandler_Upload_ChatwootExternalURLRawPayload(t *testing.T) { router.ServeHTTP(w, req) - require.Equal(t, http.StatusOK, w.Code) + require.Equal(t, http.StatusUnprocessableEntity, w.Code) var payload map[string]any require.NoError(t, json.Unmarshal(w.Body.Bytes(), &payload)) - assert.NotContains(t, payload, "success") - assert.NotEmpty(t, payload["file_url"]) - assert.NotEmpty(t, payload["blob_id"]) + assert.Contains(t, payload["error"], "SSRF") } func TestUploadHandler_DirectUpload_NoFile(t *testing.T) { @@ -158,6 +160,21 @@ func TestUploadHandler_DirectUpload_NoFile(t *testing.T) { assert.Equal(t, http.StatusBadRequest, w.Code) } +func TestUploadHandler_DirectUploadRejectsOversizedBody(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + require.NoError(t, err) + svc := service.NewUploadService(repository.NewDirectUploadRepo(db), &config.Config{Storage: config.StorageConfig{MaxFileSize: 1}}) + router := setupUploadHandlerRouter(NewUploadHandler(svc)) + body, contentType, err := makeMultipartUploadBody("large.png", bytes.Repeat([]byte{'x'}, 2<<20)) + require.NoError(t, err) + req := httptest.NewRequest(http.MethodPost, "/api/v1/widget/direct_uploads?website_token=test", body) + req.Header.Set("Content-Type", contentType) + w := httptest.NewRecorder() + + router.ServeHTTP(w, req) + assert.Equal(t, http.StatusRequestEntityTooLarge, w.Code) +} + func TestUploadHandler_WidgetActiveStorageDirectUploadFlow(t *testing.T) { gin.SetMode(gin.TestMode) tmpDir := t.TempDir() @@ -187,10 +204,11 @@ func TestUploadHandler_WidgetActiveStorageDirectUploadFlow(t *testing.T) { }).WithWidgetAuth(repository.NewInboxRepo(db), repository.NewContactInboxRepo(db)) router := setupUploadHandlerRouter(NewUploadHandler(uploadSvc)) + uploadContent := handlerTestPNG metadataBody, err := json.Marshal(map[string]any{ "blob": map[string]any{ "filename": "visitor.png", - "byte_size": 11, + "byte_size": len(uploadContent), "checksum": "checksum-token", "content_type": "image/png", "metadata": map[string]any{"identified": true}, @@ -212,16 +230,16 @@ func TestUploadHandler_WidgetActiveStorageDirectUploadFlow(t *testing.T) { require.NotEmpty(t, signedID) assert.Equal(t, "visitor.png", createResp["filename"]) directUpload := createResp["direct_upload"].(map[string]any) - assert.Equal(t, "/api/v1/widget/direct_uploads/"+signedID, directUpload["url"]) + assert.Contains(t, directUpload["url"], "/api/v1/widget/direct_uploads/"+signedID+"?token=") assert.Equal(t, map[string]any{"Content-Type": "image/png"}, directUpload["headers"]) assert.Equal(t, "gochat_local", createResp["service_name"]) - assert.Equal(t, float64(11), createResp["byte_size"]) + assert.Equal(t, float64(len(uploadContent)), createResp["byte_size"]) assert.Equal(t, "checksum-token", createResp["checksum"]) assert.Equal(t, true, createResp["metadata"].(map[string]any)["identified"]) assert.NotEmpty(t, createResp["key"]) wPut := httptest.NewRecorder() - reqPut, _ := http.NewRequest("PUT", directUpload["url"].(string), bytes.NewReader([]byte("hello image"))) + reqPut, _ := http.NewRequest("PUT", directUpload["url"].(string), bytes.NewReader(uploadContent)) reqPut.Header.Set("Content-Type", "image/png") router.ServeHTTP(wPut, reqPut) require.Equal(t, http.StatusOK, wPut.Code) @@ -233,14 +251,14 @@ func TestUploadHandler_WidgetActiveStorageDirectUploadFlow(t *testing.T) { assert.Equal(t, "visitor.png", completeData["original_name"]) assert.Equal(t, "image", completeData["file_type"]) assert.Equal(t, "image/png", completeData["mime_type"]) - assert.Equal(t, float64(11), completeData["file_size"]) + assert.Equal(t, float64(len(uploadContent)), completeData["file_size"]) var upload model.DirectUpload require.NoError(t, db.Where("upload_uuid = ?", signedID).First(&upload).Error) assert.Equal(t, account.ID, upload.AccountID) storedBytes, err := os.ReadFile(filepath.Join(tmpDir, "widget_direct", signedID+".png")) require.NoError(t, err) - assert.Equal(t, []byte("hello image"), storedBytes) + assert.Equal(t, uploadContent, storedBytes) } func TestUploadHandler_ConversationActiveStorageDirectUploadFlow(t *testing.T) { @@ -279,10 +297,11 @@ func TestUploadHandler_ConversationActiveStorageDirectUploadFlow(t *testing.T) { }).WithConversationRepo(repository.NewConversationRepo(db)) router := setupUploadHandlerRouter(NewUploadHandler(uploadSvc)) + uploadContent := []byte("%PDF-1.4\nhello report") metadataBody, err := json.Marshal(map[string]any{ "blob": map[string]any{ "filename": "agent-note.pdf", - "byte_size": 12, + "byte_size": len(uploadContent), "checksum": "pdf-checksum", "content_type": "application/pdf", "metadata": map[string]any{"identified": true}, @@ -304,15 +323,15 @@ func TestUploadHandler_ConversationActiveStorageDirectUploadFlow(t *testing.T) { require.NotEmpty(t, signedID) assert.Equal(t, "agent-note.pdf", createResp["filename"]) directUpload := createResp["direct_upload"].(map[string]any) - assert.Equal(t, createPath+"/"+signedID, directUpload["url"]) + assert.Contains(t, directUpload["url"], createPath+"/"+signedID+"?token=") assert.Equal(t, map[string]any{"Content-Type": "application/pdf"}, directUpload["headers"]) assert.Equal(t, "gochat_local", createResp["service_name"]) - assert.Equal(t, float64(12), createResp["byte_size"]) + assert.Equal(t, float64(len(uploadContent)), createResp["byte_size"]) assert.Equal(t, "pdf-checksum", createResp["checksum"]) assert.NotEmpty(t, createResp["key"]) wPut := httptest.NewRecorder() - reqPut, _ := http.NewRequest("PUT", directUpload["url"].(string), bytes.NewReader([]byte("hello report"))) + reqPut, _ := http.NewRequest("PUT", directUpload["url"].(string), bytes.NewReader(uploadContent)) reqPut.Header.Set("Content-Type", "application/pdf") router.ServeHTTP(wPut, reqPut) require.Equal(t, http.StatusOK, wPut.Code) @@ -324,7 +343,7 @@ func TestUploadHandler_ConversationActiveStorageDirectUploadFlow(t *testing.T) { assert.Equal(t, "agent-note.pdf", completeData["original_name"]) assert.Equal(t, "file", completeData["file_type"]) assert.Equal(t, "application/pdf", completeData["mime_type"]) - assert.Equal(t, float64(12), completeData["file_size"]) + assert.Equal(t, float64(len(uploadContent)), completeData["file_size"]) var upload model.DirectUpload require.NoError(t, db.Where("upload_uuid = ?", signedID).First(&upload).Error) @@ -332,7 +351,7 @@ func TestUploadHandler_ConversationActiveStorageDirectUploadFlow(t *testing.T) { assert.Equal(t, model.DirectUploadSourceAccount, upload.Source) storedBytes, err := os.ReadFile(filepath.Join(tmpDir, "account", strconv.FormatUint(uint64(account.ID), 10), signedID+".pdf")) require.NoError(t, err) - assert.Equal(t, []byte("hello report"), storedBytes) + assert.Equal(t, uploadContent, storedBytes) } func TestUploadHandler_ConversationActiveStorageDirectUploadRejectsUnsupportedMIME(t *testing.T) { @@ -421,7 +440,7 @@ func TestUploadHandler_AccountDirectUpload_WithFileButNilService(t *testing.T) { h := &UploadHandler{svc: nil} r := setupUploadHandlerRouter(h) - body, contentType, err := makeMultipartUploadBody("test.png", []byte("fake png")) + body, contentType, err := makeMultipartUploadBody("test.png", handlerTestPNG) assert.NoError(t, err) req, _ := http.NewRequest("POST", "/api/v1/accounts/1/direct_uploads", body) @@ -439,6 +458,54 @@ func TestUploadHandler_AccountDirectUpload_WithFileButNilService(t *testing.T) { r.ServeHTTP(w, req) } +func TestUploadHandler_PrivateUploadReturns404AcrossTenants(t *testing.T) { + gin.SetMode(gin.TestMode) + tmpDir := t.TempDir() + db, err := gorm.Open(sqlite.Open("file::memory:"), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate(&model.DirectUpload{})) + fileURL := "/uploads/account/1/private.png" + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "account", "1"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "account", "1", "private.png"), handlerTestPNG, 0o600)) + require.NoError(t, db.Create(&model.DirectUpload{ + UploadUUID: "private-upload", AccountID: 1, Status: model.DirectUploadStatusCompleted, + Source: model.DirectUploadSourceAccount, OriginalName: "private.png", FileType: "image", + MimeType: "image/png", FileSize: int64(len(handlerTestPNG)), FileURL: fileURL, ExpiresAt: time.Now().Add(time.Hour), + }).Error) + + jwtCfg := &config.JWTConfig{Secret: "upload-access-test-secret", ExpiryHours: 1, RefreshExpiryHours: 1} + jwtSvc := auth.NewJWTService(jwtCfg) + uploadSvc := service.NewUploadService(repository.NewDirectUploadRepo(db), &config.Config{Storage: config.StorageConfig{LocalPath: tmpDir}}).WithAccessDB(db) + handler := NewUploadHandler(uploadSvc).WithAccessAuth(jwtSvc) + router := gin.New() + router.GET("/uploads/*filepath", handler.ServeUpload) + + tokenFor := func(accountID uint) string { + pair, tokenErr := jwtSvc.GenerateTokenPair(&model.User{Base: model.Base{ID: 1}, Provider: "email"}, accountID, "agent") + require.NoError(t, tokenErr) + return pair.AccessToken + } + for _, test := range []struct { + name string + token string + want int + }{ + {name: "anonymous", want: http.StatusNotFound}, + {name: "other tenant", token: tokenFor(2), want: http.StatusNotFound}, + {name: "own tenant", token: tokenFor(1), want: http.StatusOK}, + } { + t.Run(test.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, fileURL, nil) + if test.token != "" { + req.Header.Set("access-token", test.token) + } + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + assert.Equal(t, test.want, w.Code) + }) + } +} + func TestUploadHandler_ResponseStructure(t *testing.T) { // Verify UploadResponse DTO structure matches expected JSON keys resp := service.UploadResponse{ diff --git a/backend/internal/handler/widget/widget_handler_test.go b/backend/internal/handler/widget/widget_handler_test.go index b5bc34a4..36af8a43 100644 --- a/backend/internal/handler/widget/widget_handler_test.go +++ b/backend/internal/handler/widget/widget_handler_test.go @@ -984,7 +984,7 @@ func TestWidgetHandler_ChatwootMessageDirectUploadAttachment(t *testing.T) { require.NoError(t, json.Unmarshal(wConfig.Body.Bytes(), &configResp)) authToken := configResp["contact"].(map[string]interface{})["pubsub_token"].(string) - fileContent := []byte("hello image") + fileContent := []byte{0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n', 0, 0, 0, 0} metadataBody, err := json.Marshal(map[string]interface{}{ "blob": map[string]interface{}{ "filename": "screenshot.png", diff --git a/backend/internal/handler/widget/widget_theme_handler.go b/backend/internal/handler/widget/widget_theme_handler.go index 79a6e2e8..1d14f509 100644 --- a/backend/internal/handler/widget/widget_theme_handler.go +++ b/backend/internal/handler/widget/widget_theme_handler.go @@ -2,6 +2,7 @@ package widget import ( "net/http" + "strings" "github.com/gin-gonic/gin" @@ -105,6 +106,7 @@ func (h *WidgetHandler) SubmitPreChatForm(c *gin.Context) { // Returns upload_uuid that can be referenced when sending a message with attachment. // Reference: Chatwoot widget SDK — file upload before sending message func (h *WidgetHandler) StageFileUpload(c *gin.Context) { + c.Request.Body = http.MaxBytesReader(c.Writer, c.Request.Body, (16<<20)+(1<<20)) websiteToken := c.Param("website_token") if websiteToken == "" { c.JSON(http.StatusBadRequest, gin.H{"error": "website_token is required"}) @@ -113,7 +115,11 @@ func (h *WidgetHandler) StageFileUpload(c *gin.Context) { file, err := c.FormFile("file") if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "file field is required", "details": err.Error()}) + status := http.StatusBadRequest + if strings.Contains(strings.ToLower(err.Error()), "request body too large") { + status = http.StatusRequestEntityTooLarge + } + c.JSON(status, gin.H{"error": "file field is required", "details": err.Error()}) return } @@ -125,6 +131,7 @@ func (h *WidgetHandler) StageFileUpload(c *gin.Context) { defer fileReader.Close() req := service.WidgetUploadRequest{ + WidgetToken: widgetTokenFromRequest(c), WebsiteToken: websiteToken, FileName: file.Filename, FileSize: file.Size, @@ -156,7 +163,7 @@ func (h *WidgetHandler) GetFileUploadStatus(c *gin.Context) { return } - resp, err := h.widgetService.GetFileUploadStatus(c.Request.Context(), websiteToken, uploadUUID) + resp, err := h.widgetService.GetFileUploadStatus(c.Request.Context(), websiteToken, uploadUUID, widgetTokenFromRequest(c)) if err != nil { applogger.L().Errorf("Failed to get upload status for uuid=%s: %v", uploadUUID, err) c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) @@ -169,4 +176,4 @@ func (h *WidgetHandler) GetFileUploadStatus(c *gin.Context) { } c.JSON(http.StatusOK, resp) -} \ No newline at end of file +} diff --git a/backend/internal/handler/widget/widget_theme_handler_test.go b/backend/internal/handler/widget/widget_theme_handler_test.go index 8018bd55..abb4f881 100644 --- a/backend/internal/handler/widget/widget_theme_handler_test.go +++ b/backend/internal/handler/widget/widget_theme_handler_test.go @@ -3,6 +3,7 @@ package widget import ( "bytes" "context" + "encoding/json" "mime/multipart" "net/http" "net/http/httptest" @@ -340,6 +341,43 @@ func (s *WidgetThemeHandlerTestSuite) TestStageFileUpload_InvalidWebsiteToken() assert.Equal(s.T(), http.StatusBadRequest, w.Code) } +func (s *WidgetThemeHandlerTestSuite) TestStageAndGetFileUpload_RequiresMatchingSession() { + account, inbox := s.seedAccountAndWidgetInbox() + s.Require().NoError(s.db.Model(inbox).Update("channel_config", `{"website_token":"secure_upload_token"}`).Error) + contact := &model.Contact{AccountID: account.ID, Name: "Secure Uploader"} + s.Require().NoError(s.db.Create(contact).Error) + s.Require().NoError(s.db.Create(&model.ContactInbox{ContactID: contact.ID, InboxID: inbox.ID, PubsubToken: "secure_widget_session"}).Error) + + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + part, err := writer.CreateFormFile("file", "secure.png") + s.Require().NoError(err) + _, err = part.Write([]byte{0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n', 0, 0, 0, 0}) + s.Require().NoError(err) + s.Require().NoError(writer.Close()) + req := httptest.NewRequest(http.MethodPost, "/widget/secure_upload_token/uploads", body) + req.Header.Set("Content-Type", writer.FormDataContentType()) + req.Header.Set("X-Auth-Token", "secure_widget_session") + w := httptest.NewRecorder() + s.router.ServeHTTP(w, req) + s.Require().Equal(http.StatusCreated, w.Code) + var upload map[string]any + s.Require().NoError(json.Unmarshal(w.Body.Bytes(), &upload)) + uuid, _ := upload["upload_uuid"].(string) + s.Require().NotEmpty(uuid) + + statusReq := httptest.NewRequest(http.MethodGet, "/widget/secure_upload_token/uploads/"+uuid, nil) + statusReq.Header.Set("X-Auth-Token", "secure_widget_session") + statusW := httptest.NewRecorder() + s.router.ServeHTTP(statusW, statusReq) + s.Require().Equal(http.StatusOK, statusW.Code) + + deniedReq := httptest.NewRequest(http.MethodGet, "/widget/secure_upload_token/uploads/"+uuid, nil) + deniedW := httptest.NewRecorder() + s.router.ServeHTTP(deniedW, deniedReq) + s.Require().Equal(http.StatusBadRequest, deniedW.Code) +} + func (s *WidgetThemeHandlerTestSuite) TestGetFileUploadStatus_MissingUploadUUID() { w := httptest.NewRecorder() req, _ := http.NewRequest("GET", "/widget/test_token/uploads/", nil) diff --git a/backend/internal/model/widget_file_upload.go b/backend/internal/model/widget_file_upload.go index 7328f6b4..9116e87e 100644 --- a/backend/internal/model/widget_file_upload.go +++ b/backend/internal/model/widget_file_upload.go @@ -55,7 +55,7 @@ func (WidgetFileUpload) TableName() string { return "widget_file_uploads" } // WidgetUploadAllowedTypes defines the allowed MIME types for widget uploads. // Reference: Chatwoot widget SDK config — allowed file types restriction var WidgetUploadAllowedTypes = map[string][]string{ - "image": {"image/png", "image/jpeg", "image/gif", "image/webp", "image/svg+xml"}, + "image": {"image/png", "image/jpeg", "image/gif", "image/webp"}, "audio": {"audio/mp3", "audio/ogg", "audio/wav", "audio/webm", "audio/mpeg"}, "video": {"video/mp4", "video/webm", "video/ogg"}, "file": {"application/pdf", "application/vnd.ms-excel", "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", @@ -69,4 +69,4 @@ var WidgetUploadMaxSizeByType = map[string]int64{ "audio": 10 * 1024 * 1024, // 10MB for audio "video": 15 * 1024 * 1024, // 15MB for video "file": 10 * 1024 * 1024, // 10MB for documents -} \ No newline at end of file +} diff --git a/backend/internal/security/security_test.go b/backend/internal/security/security_test.go index 6d0402ac..80450989 100644 --- a/backend/internal/security/security_test.go +++ b/backend/internal/security/security_test.go @@ -577,24 +577,6 @@ func TestIsBlockedIP_InvalidCIDR(t *testing.T) { assert.False(t, isBlockedIP(net.ParseIP("8.8.8.8"), []string{"invalid-cidr"})) } -func TestSameIPSets_Equal(t *testing.T) { - a := []net.IPAddr{{IP: net.ParseIP("1.2.3.4")}, {IP: net.ParseIP("5.6.7.8")}} - b := []net.IPAddr{{IP: net.ParseIP("5.6.7.8")}, {IP: net.ParseIP("1.2.3.4")}} - assert.True(t, sameIPSets(a, b)) -} - -func TestSameIPSets_DifferentLength(t *testing.T) { - a := []net.IPAddr{{IP: net.ParseIP("1.2.3.4")}} - b := []net.IPAddr{{IP: net.ParseIP("1.2.3.4")}, {IP: net.ParseIP("5.6.7.8")}} - assert.False(t, sameIPSets(a, b)) -} - -func TestSameIPSets_DifferentIPs(t *testing.T) { - a := []net.IPAddr{{IP: net.ParseIP("1.2.3.4")}} - b := []net.IPAddr{{IP: net.ParseIP("5.6.7.8")}} - assert.False(t, sameIPSets(a, b)) -} - func TestSafeRedirectCheck(t *testing.T) { check := safeRedirectCheck(2) @@ -618,6 +600,20 @@ func TestSafeRedirectCheck_IPLiteral(t *testing.T) { assert.Contains(t, err.Error(), "IP literal") } +func TestSafeRedirectCheckConfig_BlocksPrivateAndNonHTTPRedirects(t *testing.T) { + check := safeRedirectCheckConfig(DefaultSSRFConfig()) + for _, target := range []string{ + "http://169.254.169.254/latest/meta-data/", + "http://10.0.0.1/admin", + "ftp://example.com/file", + "https://example.com:8443/file", + } { + req, err := http.NewRequest(http.MethodGet, target, nil) + require.NoError(t, err) + assert.Error(t, check(req, nil), target) + } +} + func TestValidateURL_Empty(t *testing.T) { err := ValidateURL("", DefaultSSRFConfig()) assert.Error(t, err) @@ -710,6 +706,21 @@ func TestSafeHTTPClient_Do_RequireTLS(t *testing.T) { assert.Contains(t, err.Error(), "non-HTTPS") } +func TestSafeHTTPClient_Do_BlocksResolvedLoopback(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatal("blocked target must not receive a request") + })) + defer srv.Close() + + cfg := DefaultSSRFConfig() + cfg.AllowedPorts = nil + client := NewSafeHTTPClient(cfg) + req, _ := http.NewRequest(http.MethodGet, srv.URL, nil) + _, err := client.Do(req) + require.Error(t, err) + assert.Contains(t, err.Error(), "SSRF") +} + func TestSafeHTTPClient_SafeFetchURL(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) diff --git a/backend/internal/security/ssrf_protection.go b/backend/internal/security/ssrf_protection.go index dcdf3ce0..459aad5c 100644 --- a/backend/internal/security/ssrf_protection.go +++ b/backend/internal/security/ssrf_protection.go @@ -33,8 +33,9 @@ import ( // SSRFConfig holds SSRF protection configuration. type SSRFConfig struct { - AllowedDomains []string // whitelist of domains bypassing SSRF checks + AllowedDomains []string // retained trusted-domain catalog; never bypasses IP validation BlockedCIDRs []string // IP ranges forbidden (private, loopback, etc.) + AllowedPorts []string // empty permits all ports; defaults restrict fetches to HTTP(S) MaxRedirects int // limit HTTP redirect chains RequireTLS bool // enforce HTTPS for certain operations } @@ -50,22 +51,28 @@ func DefaultSSRFConfig() SSRFConfig { "web.whatsapp.com", }, BlockedCIDRs: []string{ - "10.0.0.0/8", // RFC 1918 private - "172.16.0.0/12", // RFC 1918 private - "192.168.0.0/16", // RFC 1918 private - "127.0.0.0/8", // Loopback - "0.0.0.0/8", // Current network - "100.64.0.0/10", // CGN - "169.254.0.0/16", // Link-local - "192.0.0.0/24", // IETF Protocol Assignments - "192.0.2.0/24", // TEST-NET-1 - "198.18.0.0/15", // Benchmarking - "224.0.0.0/4", // Multicast - "240.0.0.0/4", // Reserved - "::1/128", // IPv6 loopback - "fc00::/7", // IPv6 unique local - "fe80::/10", // IPv6 link-local + "10.0.0.0/8", // RFC 1918 private + "172.16.0.0/12", // RFC 1918 private + "192.168.0.0/16", // RFC 1918 private + "127.0.0.0/8", // Loopback + "0.0.0.0/8", // Current network + "100.64.0.0/10", // CGN + "169.254.0.0/16", // Link-local + "192.0.0.0/24", // IETF Protocol Assignments + "192.0.2.0/24", // TEST-NET-1 + "198.51.100.0/24", // TEST-NET-2 + "203.0.113.0/24", // TEST-NET-3 + "198.18.0.0/15", // Benchmarking + "224.0.0.0/4", // Multicast + "240.0.0.0/4", // Reserved + "::1/128", // IPv6 loopback + "::/128", // IPv6 unspecified + "fc00::/7", // IPv6 unique local + "fe80::/10", // IPv6 link-local + "ff00::/8", // IPv6 multicast + "2001:db8::/32", // IPv6 documentation }, + AllowedPorts: []string{"80", "443"}, MaxRedirects: 3, RequireTLS: false, } @@ -102,27 +109,32 @@ func NewSafeHTTPClient(cfg SSRFConfig) *SafeHTTPClient { } } - // DNS rebinding check - ips2, err := net.DefaultResolver.LookupIPAddr(ctx, host) - if err != nil { - return nil, fmt.Errorf("DNS rebinding check failed for %s: %w", host, err) + // Dial the validated address directly. Resolving the hostname again here + // would reopen a DNS-rebinding window between validation and connect. + var lastErr error + for _, ip := range ips { + conn, err := dialer.DialContext(ctx, network, net.JoinHostPort(ip.IP.String(), port)) + if err == nil { + return conn, nil + } + lastErr = err } - if !sameIPSets(ips, ips2) { - return nil, fmt.Errorf("DNS rebinding detected: %s resolved to different IPs", host) + if lastErr == nil { + lastErr = fmt.Errorf("DNS resolution returned no addresses for %s", host) } - - return dialer.DialContext(ctx, network, net.JoinHostPort(host, port)) + return nil, lastErr }, - MaxIdleConns: 100, - IdleConnTimeout: 90 * time.Second, - TLSHandshakeTimeout: 10 * time.Second, + MaxIdleConns: 100, + IdleConnTimeout: 90 * time.Second, + TLSHandshakeTimeout: 10 * time.Second, + ResponseHeaderTimeout: 10 * time.Second, } return &SafeHTTPClient{ client: &http.Client{ Transport: transport, Timeout: 30 * time.Second, - CheckRedirect: safeRedirectCheck(cfg.MaxRedirects), + CheckRedirect: safeRedirectCheckConfig(cfg), }, cfg: cfg, } @@ -130,23 +142,8 @@ func NewSafeHTTPClient(cfg SSRFConfig) *SafeHTTPClient { // Do executes an HTTP request with SSRF protection. func (c *SafeHTTPClient) Do(req *http.Request) (*http.Response, error) { - host := req.URL.Hostname() - - // Whitelist bypass - for _, allowed := range c.cfg.AllowedDomains { - if host == allowed || strings.HasSuffix(host, "."+allowed) { - return c.client.Do(req) - } - } - - if c.cfg.RequireTLS && req.URL.Scheme != "https" { - return nil, fmt.Errorf("SSRF protection: non-HTTPS request blocked for %s", req.URL) - } - - if net.ParseIP(host) != nil { - if isBlockedIP(net.ParseIP(host), c.cfg.BlockedCIDRs) { - return nil, fmt.Errorf("SSRF blocked: direct IP request to %s", host) - } + if err := validateURLTarget(req.URL, c.cfg); err != nil { + return nil, err } return c.client.Do(req) @@ -178,20 +175,13 @@ func safeRedirectCheck(maxRedirects int) func(req *http.Request, via []*http.Req } } -func sameIPSets(a, b []net.IPAddr) bool { - if len(a) != len(b) { - return false - } - setA := make(map[string]bool) - for _, ip := range a { - setA[ip.IP.String()] = true - } - for _, ip := range b { - if !setA[ip.IP.String()] { - return false +func safeRedirectCheckConfig(cfg SSRFConfig) func(req *http.Request, via []*http.Request) error { + return func(req *http.Request, via []*http.Request) error { + if len(via) >= cfg.MaxRedirects { + return fmt.Errorf("SSRF: stopped after %d redirects", cfg.MaxRedirects) } + return validateURLTarget(req.URL, cfg) } - return true } // ValidateURL checks if a URL is safe to fetch (without making a request). @@ -203,16 +193,45 @@ func ValidateURL(rawURL string, cfg SSRFConfig) error { if err != nil { return fmt.Errorf("invalid URL: %w", err) } - host := parsed.Hostname() - for _, allowed := range cfg.AllowedDomains { - if host == allowed || strings.HasSuffix(host, "."+allowed) { - return nil + return validateURLTarget(parsed, cfg) +} + +func validateURLTarget(parsed *url.URL, cfg SSRFConfig) error { + if parsed == nil || parsed.Hostname() == "" { + return fmt.Errorf("invalid URL: host is required") + } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return fmt.Errorf("SSRF: unsupported URL scheme %q", parsed.Scheme) + } + if cfg.RequireTLS && parsed.Scheme != "https" { + return fmt.Errorf("SSRF protection: non-HTTPS request blocked for %s", parsed) + } + if parsed.User != nil { + return fmt.Errorf("SSRF: URL credentials are not allowed") + } + port := parsed.Port() + if port == "" { + if parsed.Scheme == "https" { + port = "443" + } else { + port = "80" } } - if ip := net.ParseIP(host); ip != nil { - if isBlockedIP(ip, cfg.BlockedCIDRs) { - return fmt.Errorf("SSRF: URL points to private/reserved IP %s", host) + if len(cfg.AllowedPorts) > 0 { + allowed := false + for _, candidate := range cfg.AllowedPorts { + if port == candidate { + allowed = true + break + } } + if !allowed { + return fmt.Errorf("SSRF: URL port %s is not allowed", port) + } + } + host := parsed.Hostname() + if ip := net.ParseIP(host); ip != nil && isBlockedIP(ip, cfg.BlockedCIDRs) { + return fmt.Errorf("SSRF: URL points to private/reserved IP %s", host) } return nil } @@ -224,4 +243,4 @@ func (c *SafeHTTPClient) SafeFetchURL(ctx context.Context, rawurl string) (*http return nil, fmt.Errorf("invalid request: %w", err) } return c.Do(req) -} \ No newline at end of file +} diff --git a/backend/internal/service/coverage7_test.go b/backend/internal/service/coverage7_test.go index 94ddb230..e4f2e9c1 100644 --- a/backend/internal/service/coverage7_test.go +++ b/backend/internal/service/coverage7_test.go @@ -16,6 +16,7 @@ import ( "github.com/gochat/gochat/internal/llm" "github.com/gochat/gochat/internal/model" "github.com/gochat/gochat/internal/repository" + "github.com/gochat/gochat/internal/security" pkgcrypto "github.com/gochat/gochat/pkg/crypto" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -2043,7 +2044,7 @@ func TestUpload_AccountUploadFromURL_Cov7(t *testing.T) { _, svc := setupCov7Upload(t) // Create a test HTTP server - fileContent := []byte("fake file content from URL") + fileContent := testPNG server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "image/png") if _, err := w.Write(fileContent); err != nil { @@ -2051,6 +2052,7 @@ func TestUpload_AccountUploadFromURL_Cov7(t *testing.T) { } })) defer server.Close() + svc.fetchClient = security.NewSafeHTTPClient(security.SSRFConfig{MaxRedirects: 3}) result, err := svc.AccountUploadFromURL(context.Background(), 1, server.URL+"/test.png") require.NoError(t, err) @@ -2083,6 +2085,7 @@ func TestUpload_AccountUploadFromURL_ServerError_Cov7(t *testing.T) { w.WriteHeader(http.StatusInternalServerError) })) defer server.Close() + svc.fetchClient = security.NewSafeHTTPClient(security.SSRFConfig{MaxRedirects: 3}) _, err := svc.AccountUploadFromURL(context.Background(), 1, server.URL+"/test.png") require.Error(t, err) @@ -2109,6 +2112,7 @@ func TestUpload_AccountUploadFromURL_FileTooLarge_Cov7(t *testing.T) { } })) defer server.Close() + svc.fetchClient = security.NewSafeHTTPClient(security.SSRFConfig{MaxRedirects: 3}) _, err := svc.AccountUploadFromURL(context.Background(), 1, server.URL+"/test.png") require.Error(t, err) @@ -2118,7 +2122,7 @@ func TestUpload_AccountUploadFromURL_FileTooLarge_Cov7(t *testing.T) { func TestUpload_ProfileAvatarUpload_Cov7(t *testing.T) { _, svc := setupCov7Upload(t) - fileHeader := createTestFileHeader(t, "avatar.png", []byte("fake png")) + fileHeader := createTestFileHeader(t, "avatar.png", testPNG) result, err := svc.ProfileAvatarUpload(context.Background(), 1, fileHeader) require.NoError(t, err) @@ -2146,7 +2150,7 @@ func TestUpload_ProfileAvatarUpload_NoFile_Cov7(t *testing.T) { func TestUpload_ProfileAvatarUpload_NotImage_Cov7(t *testing.T) { _, svc := setupCov7Upload(t) - fileHeader := createTestFileHeader(t, "doc.pdf", []byte("fake pdf")) + fileHeader := createTestFileHeader(t, "doc.pdf", []byte("%PDF-1.4\nfile")) _, err := svc.ProfileAvatarUpload(context.Background(), 1, fileHeader) require.Error(t, err) @@ -2156,7 +2160,7 @@ func TestUpload_ProfileAvatarUpload_NotImage_Cov7(t *testing.T) { func TestUpload_CreateWidgetDirectUpload_Cov7(t *testing.T) { _, svc := setupCov7Upload(t) - resp, err := svc.CreateWidgetDirectUpload(context.Background(), ActiveStorageDirectUploadRequest{ + _, err := svc.CreateWidgetDirectUpload(context.Background(), ActiveStorageDirectUploadRequest{ WebsiteToken: "wt_upload", AuthToken: "auth_token", Blob: ActiveStorageBlobParams{ @@ -2165,12 +2169,8 @@ func TestUpload_CreateWidgetDirectUpload_Cov7(t *testing.T) { ContentType: "image/png", }, }) - require.NoError(t, err) - assert.NotZero(t, resp.ID) - assert.Equal(t, "test.png", resp.Filename) - assert.Equal(t, "image/png", resp.ContentType) - assert.NotEmpty(t, resp.SignedID) - assert.NotEmpty(t, resp.DirectUpload.URL) + require.Error(t, err) + assert.Contains(t, err.Error(), "authentication is not configured") } func TestUpload_CreateWidgetDirectUpload_NoFilename_Cov7(t *testing.T) { @@ -2308,13 +2308,14 @@ func TestUpload_CompleteWidgetDirectUpload_Cov7(t *testing.T) { OriginalName: "test.png", FileType: "image", MimeType: "image/png", - FileSize: 100, + FileSize: int64(len(testPNG)), FileURL: "/uploads/widget_direct/test.png", + Metadata: []byte(`{"active_storage_key":"complete-token"}`), ExpiresAt: time.Now().Add(24 * time.Hour), } require.NoError(t, db.Create(upload).Error) - result, err := svc.CompleteWidgetDirectUpload(context.Background(), upload.UploadUUID, strings.NewReader("file content")) + result, err := svc.CompleteWidgetDirectUpload(context.Background(), upload.UploadUUID, strings.NewReader(string(testPNG)), "complete-token") require.NoError(t, err) assert.Equal(t, upload.UploadUUID, result.UploadUUID) assert.Equal(t, "test.png", result.OriginalName) @@ -2347,13 +2348,13 @@ func TestUpload_CompleteWidgetDirectUpload_Expired_Cov7(t *testing.T) { OriginalName: "test.png", FileType: "image", MimeType: "image/png", - FileSize: 100, + FileSize: int64(len(testPNG)), FileURL: "/uploads/widget_direct/expired.png", ExpiresAt: time.Now().Add(-1 * time.Hour), // expired } require.NoError(t, db.Create(upload).Error) - _, err := svc.CompleteWidgetDirectUpload(context.Background(), upload.UploadUUID, strings.NewReader("content")) + _, err := svc.CompleteWidgetDirectUpload(context.Background(), upload.UploadUUID, strings.NewReader("content"), "missing") require.Error(t, err) assert.Contains(t, err.Error(), "expired") } @@ -2369,7 +2370,7 @@ func TestUpload_CompleteWidgetDirectUpload_SourceMismatch_Cov7(t *testing.T) { OriginalName: "test.png", FileType: "image", MimeType: "image/png", - FileSize: 100, + FileSize: int64(len(testPNG)), FileURL: "/uploads/account/test.png", ExpiresAt: time.Now().Add(24 * time.Hour), } @@ -2398,13 +2399,13 @@ func TestUpload_CompleteConversationDirectUpload_Cov7(t *testing.T) { OriginalName: "test.png", FileType: "image", MimeType: "image/png", - FileSize: 100, + FileSize: int64(len(testPNG)), FileURL: "/uploads/account/test.png", ExpiresAt: time.Now().Add(24 * time.Hour), } require.NoError(t, db.Create(upload).Error) - result, err := svc.CompleteConversationDirectUpload(context.Background(), account.ID, conv.ID, upload.UploadUUID, strings.NewReader("content")) + result, err := svc.CompleteConversationDirectUpload(context.Background(), account.ID, conv.ID, upload.UploadUUID, strings.NewReader(string(testPNG))) require.NoError(t, err) assert.Equal(t, upload.UploadUUID, result.UploadUUID) } @@ -2486,10 +2487,9 @@ func TestUpload_CompleteConversationDirectUpload_SourceMismatch_Cov7(t *testing. func TestUpload_ValidateWidgetUploadSession_Cov7(t *testing.T) { _, svc := setupCov7Upload(t) - // No inboxRepo/contactInboxRepo wired — should return (0, nil) - accountID, err := svc.validateWidgetUploadSession(context.Background(), "token", "auth") - require.NoError(t, err) - assert.Equal(t, uint(0), accountID) + _, err := svc.validateWidgetUploadSession(context.Background(), "token", "auth") + require.Error(t, err) + assert.Contains(t, err.Error(), "authentication is not configured") } func TestUpload_ValidateWidgetUploadSession_NoWebsiteToken_Cov7(t *testing.T) { diff --git a/backend/internal/service/upload_service.go b/backend/internal/service/upload_service.go index d36fd37c..668a211e 100644 --- a/backend/internal/service/upload_service.go +++ b/backend/internal/service/upload_service.go @@ -9,19 +9,23 @@ import ( "errors" "fmt" "io" + "mime" "mime/multipart" "net/http" "net/url" "os" "path/filepath" + "strconv" "strings" "time" "github.com/google/uuid" + "gorm.io/gorm" "github.com/gochat/gochat/internal/config" "github.com/gochat/gochat/internal/model" "github.com/gochat/gochat/internal/repository" + "github.com/gochat/gochat/internal/security" applogger "github.com/gochat/gochat/pkg/logger" ) @@ -31,6 +35,8 @@ type UploadService struct { conversationRepo *repository.ConversationRepo inboxRepo *repository.InboxRepo contactInboxRepo *repository.ContactInboxRepo + accessDB *gorm.DB + fetchClient *security.SafeHTTPClient cfg *config.Config } @@ -38,6 +44,7 @@ type UploadService struct { func NewUploadService(directUploadRepo *repository.DirectUploadRepo, cfg *config.Config) *UploadService { return &UploadService{ directUploadRepo: directUploadRepo, + fetchClient: security.NewSafeHTTPClient(security.DefaultSSRFConfig()), cfg: cfg, } } @@ -57,6 +64,12 @@ func (s *UploadService) WithConversationRepo(conversationRepo *repository.Conver return s } +// WithAccessDB wires the scoped lookup used by the private local-upload handler. +func (s *UploadService) WithAccessDB(db *gorm.DB) *UploadService { + s.accessDB = db + return s +} + // --- DTOs --- // AccountUploadRequest is the DTO for account-level file upload. @@ -66,7 +79,9 @@ type AccountUploadRequest struct { // WidgetDirectUploadRequest is the DTO for widget direct file upload. type WidgetDirectUploadRequest struct { - FileHeader *multipart.FileHeader `json:"-"` + WebsiteToken string `json:"-"` + AuthToken string `json:"-"` + FileHeader *multipart.FileHeader `json:"-"` } // AccountDirectUploadRequest is the DTO for account-level direct file upload (staged for message attachment). @@ -122,6 +137,18 @@ type UploadResponse struct { ExpiresAt time.Time `json:"expires_at"` } +type widgetUploadSession struct { + AccountID uint + ContactInboxID uint +} + +// MaxRequestBodySize is the single hard ceiling for upload request bodies. +// Multipart metadata gets a small allowance while file-specific limits remain +// enforced from the actual content in the service. +func (s *UploadService) MaxRequestBodySize() int64 { + return s.maxUploadSize() + (1 << 20) +} + // --- Account Upload --- // AccountUpload handles a file upload from the dashboard (account-scoped). @@ -148,16 +175,6 @@ func (s *UploadService) ProfileAvatarUpload(ctx context.Context, accountID uint, return nil, errors.New("avatar file is required") } - mimeType := fileHeader.Header.Get("Content-Type") - if mimeType == "" || mimeType == "application/octet-stream" { - mimeType = detectUploadMIMEFromFilename(fileHeader.Filename) - } - if !strings.HasPrefix(mimeType, "image/") { - return nil, fmt.Errorf("avatar must be an image, got %s", mimeType) - } - if !isUploadMIMEAllowed("image", mimeType) { - return nil, fmt.Errorf("MIME type %s is not allowed for avatar", mimeType) - } maxSize := int64(s.cfg.Storage.MaxFileSize) if maxSize <= 0 { maxSize = 20 << 20 @@ -172,7 +189,14 @@ func (s *UploadService) ProfileAvatarUpload(ctx context.Context, accountID uint, } defer src.Close() - fileURL, thumbURL, err := s.saveUploadReader(accountID, model.DirectUploadSourceAccount, filepath.Ext(fileHeader.Filename), mimeType, src) + data, mimeType, err := readValidatedUpload(src, fileHeader.Filename, fileHeader.Header.Get("Content-Type"), fileHeader.Size, maxSize, true) + if err != nil { + return nil, err + } + if !strings.HasPrefix(mimeType, "image/") || !isUploadMIMEAllowed("image", mimeType) { + return nil, fmt.Errorf("avatar must be an image with a supported format, got %s", mimeType) + } + fileURL, thumbURL, err := s.saveUploadReader(accountID, model.DirectUploadSourceAccount, extensionForUploadMIME(mimeType), mimeType, bytes.NewReader(data)) if err != nil { return nil, fmt.Errorf("failed to save avatar file: %w", err) } @@ -180,7 +204,7 @@ func (s *UploadService) ProfileAvatarUpload(ctx context.Context, accountID uint, OriginalName: fileHeader.Filename, FileType: "image", MimeType: mimeType, - FileSize: fileHeader.Size, + FileSize: int64(len(data)), FileURL: fileURL, ThumbURL: thumbURL, Status: string(model.DirectUploadStatusCompleted), @@ -191,15 +215,20 @@ func (s *UploadService) AccountUploadFromURL(ctx context.Context, accountID uint if accountID == 0 { return nil, errors.New("account_id is required") } - parsed, err := url.ParseRequestURI(externalURL) - if err != nil || (parsed.Scheme != "http" && parsed.Scheme != "https") { + parsed, err := url.Parse(externalURL) + ssrfCfg := security.DefaultSSRFConfig() + if err != nil || parsed.Hostname() == "" || (parsed.Scheme != "http" && parsed.Scheme != "https") { return nil, errors.New("invalid url") } req, err := http.NewRequestWithContext(ctx, http.MethodGet, externalURL, nil) if err != nil { return nil, err } - resp, err := http.DefaultClient.Do(req) + client := s.fetchClient + if client == nil { + client = security.NewSafeHTTPClient(ssrfCfg) + } + resp, err := client.Do(req) if err != nil { return nil, fmt.Errorf("failed to fetch external url: %w", err) } @@ -207,9 +236,9 @@ func (s *UploadService) AccountUploadFromURL(ctx context.Context, accountID uint if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { return nil, fmt.Errorf("failed to fetch external url: status %d", resp.StatusCode) } - maxSize := int64(s.cfg.Storage.MaxFileSize) - if maxSize <= 0 { - maxSize = 50 << 20 + maxSize := s.maxUploadSize() + if resp.ContentLength > maxSize { + return nil, errors.New("file too large") } data, err := io.ReadAll(io.LimitReader(resp.Body, maxSize+1)) if err != nil { @@ -253,18 +282,30 @@ func (s *UploadService) WidgetDirectUpload(ctx context.Context, req WidgetDirect if req.FileHeader == nil { return nil, errors.New("file is required") } - - // Widget direct uploads are associated with account 0 initially; - // they get linked to a real account when attached to a conversation. - return s.processUpload(ctx, 0, req.FileHeader, model.DirectUploadSourceWidget) -} - -func (s *UploadService) CreateWidgetDirectUpload(ctx context.Context, req ActiveStorageDirectUploadRequest) (*ActiveStorageDirectUploadResponse, error) { - accountID, err := s.validateWidgetUploadSession(ctx, req.WebsiteToken, req.AuthToken) + session, err := s.validateWidgetUploadSession(ctx, req.WebsiteToken, req.AuthToken) if err != nil { return nil, err } - return s.createActiveStorageDirectUpload(ctx, accountID, 0, model.DirectUploadSourceWidget, req, "/api/v1/widget/direct_uploads/") + result, err := s.processUpload(ctx, session.AccountID, req.FileHeader, model.DirectUploadSourceWidget) + if err != nil { + return nil, err + } + if err := s.addUploadSessionMetadata(ctx, result.UploadUUID, session.ContactInboxID); err != nil { + return nil, err + } + return result, nil +} + +func (s *UploadService) CreateWidgetDirectUpload(ctx context.Context, req ActiveStorageDirectUploadRequest) (*ActiveStorageDirectUploadResponse, error) { + session, err := s.validateWidgetUploadSession(ctx, req.WebsiteToken, req.AuthToken) + if err != nil { + return nil, err + } + if req.Blob.Metadata == nil { + req.Blob.Metadata = map[string]any{} + } + req.Blob.Metadata["contact_inbox_id"] = session.ContactInboxID + return s.createActiveStorageDirectUpload(ctx, session.AccountID, 0, model.DirectUploadSourceWidget, req, "/api/v1/widget/direct_uploads/") } func (s *UploadService) CreateConversationDirectUpload(ctx context.Context, accountID, conversationID uint, req ActiveStorageDirectUploadRequest) (*ActiveStorageDirectUploadResponse, error) { @@ -285,34 +326,44 @@ func (s *UploadService) CreateConversationDirectUpload(ctx context.Context, acco return s.createActiveStorageDirectUpload(ctx, accountID, accountID, model.DirectUploadSourceAccount, req, urlPrefix) } -func (s *UploadService) validateWidgetUploadSession(ctx context.Context, websiteToken, authToken string) (uint, error) { +func (s *UploadService) validateWidgetUploadSession(ctx context.Context, websiteToken, authToken string) (widgetUploadSession, error) { if websiteToken == "" { - return 0, errors.New("website_token is required") + return widgetUploadSession{}, errors.New("website_token is required") } if authToken == "" { - return 0, errors.New("widget auth token is required") + return widgetUploadSession{}, errors.New("widget auth token is required") } if s.inboxRepo == nil || s.contactInboxRepo == nil { - return 0, nil + return widgetUploadSession{}, errors.New("widget upload authentication is not configured") } inbox, err := s.inboxRepo.FindByWebsiteToken(ctx, websiteToken) if err != nil { - return 0, fmt.Errorf("invalid website_token: %w", err) + return widgetUploadSession{}, fmt.Errorf("invalid website_token: %w", err) } if !inbox.Enabled { - return 0, errors.New("inbox is disabled") + return widgetUploadSession{}, errors.New("inbox is disabled") } contactInbox, err := s.contactInboxRepo.FindByPubsubToken(ctx, authToken) if err != nil { - return 0, fmt.Errorf("invalid widget auth token: %w", err) + return widgetUploadSession{}, fmt.Errorf("invalid widget auth token: %w", err) } if contactInbox.InboxID != inbox.ID { - return 0, errors.New("widget auth token does not belong to this inbox") + return widgetUploadSession{}, errors.New("widget auth token does not belong to this inbox") } - return inbox.AccountID, nil + return widgetUploadSession{AccountID: inbox.AccountID, ContactInboxID: contactInbox.ID}, nil } -func (s *UploadService) CompleteWidgetDirectUpload(ctx context.Context, uploadUUID string, body io.Reader) (*UploadResponse, error) { +func (s *UploadService) addUploadSessionMetadata(ctx context.Context, uploadUUID string, contactInboxID uint) error { + upload, err := s.directUploadRepo.FindByUUID(ctx, uploadUUID) + if err != nil { + return err + } + metadata := map[string]any{"contact_inbox_id": contactInboxID} + upload.Metadata, _ = json.Marshal(metadata) + return s.directUploadRepo.Update(ctx, upload) +} + +func (s *UploadService) CompleteWidgetDirectUpload(ctx context.Context, uploadUUID string, body io.Reader, uploadTokens ...string) (*UploadResponse, error) { if uploadUUID == "" { return nil, errors.New("upload_uuid is required") } @@ -328,7 +379,14 @@ func (s *UploadService) CompleteWidgetDirectUpload(ctx context.Context, uploadUU _ = s.directUploadRepo.Update(ctx, upload) return nil, errors.New("direct upload has expired") } - if err := s.saveReaderToDisk(upload.FileURL, body); err != nil { + providedToken := "" + if len(uploadTokens) > 0 { + providedToken = uploadTokens[0] + } + if providedToken == "" || providedToken != directUploadToken(upload) { + return nil, errors.New("invalid direct upload token") + } + if err := s.completeDirectUpload(ctx, upload, body); err != nil { return nil, fmt.Errorf("failed to save direct upload: %w", err) } return &UploadResponse{ @@ -373,7 +431,7 @@ func (s *UploadService) CompleteConversationDirectUpload(ctx context.Context, ac _ = s.directUploadRepo.Update(ctx, upload) return nil, errors.New("direct upload has expired") } - if err := s.saveReaderToDisk(upload.FileURL, body); err != nil { + if err := s.completeDirectUpload(ctx, upload, body); err != nil { return nil, fmt.Errorf("failed to save direct upload: %w", err) } return &UploadResponse{ @@ -390,6 +448,32 @@ func (s *UploadService) CompleteConversationDirectUpload(ctx context.Context, ac }, nil } +func directUploadToken(upload *model.DirectUpload) string { + if upload == nil || len(upload.Metadata) == 0 { + return "" + } + var metadata map[string]any + if json.Unmarshal(upload.Metadata, &metadata) != nil { + return "" + } + token, _ := metadata["active_storage_key"].(string) + return token +} + +func (s *UploadService) completeDirectUpload(ctx context.Context, upload *model.DirectUpload, body io.Reader) error { + data, mimeType, err := readValidatedUpload(body, upload.OriginalName, upload.MimeType, upload.FileSize, upload.FileSize, true) + if err != nil { + return err + } + if err := s.saveReaderToDisk(upload.FileURL, bytes.NewReader(data), int64(len(data))); err != nil { + return err + } + upload.MimeType = mimeType + upload.FileType = categorizeUploadMIME(mimeType) + upload.FileSize = int64(len(data)) + return s.directUploadRepo.Update(ctx, upload) +} + // --- Internal helpers --- func (s *UploadService) createActiveStorageDirectUpload(ctx context.Context, accountID, storageAccountID uint, source model.DirectUploadSource, req ActiveStorageDirectUploadRequest, directUploadURLPrefix string) (*ActiveStorageDirectUploadResponse, error) { @@ -399,7 +483,7 @@ func (s *UploadService) createActiveStorageDirectUpload(ctx context.Context, acc if req.Blob.ByteSize <= 0 { return nil, errors.New("byte_size is required") } - mimeType := req.Blob.ContentType + mimeType := normalizeUploadMIME(req.Blob.ContentType) if mimeType == "" || mimeType == "application/octet-stream" { mimeType = detectUploadMIMEFromFilename(req.Blob.Filename) } @@ -410,16 +494,13 @@ func (s *UploadService) createActiveStorageDirectUpload(ctx context.Context, acc if !isUploadMIMEAllowed(fileCategory, mimeType) { return nil, fmt.Errorf("MIME type %s is not allowed for category %s", mimeType, fileCategory) } - maxSize := model.WidgetUploadMaxSizeByType[fileCategory] - if maxSize == 0 { - maxSize = int64(s.cfg.Storage.MaxFileSize) - } + maxSize := s.categoryUploadLimit(fileCategory) if req.Blob.ByteSize > maxSize { return nil, fmt.Errorf("file size %d exceeds maximum %d for type %s", req.Blob.ByteSize, maxSize, fileCategory) } uploadUUID := uuid.New().String() - fileURL, thumbURL := s.directUploadURL(source, storageAccountID, uploadUUID, req.Blob.Filename) + fileURL, thumbURL := s.directUploadURL(source, storageAccountID, uploadUUID, mimeType) metadata := req.Blob.Metadata if metadata == nil { metadata = map[string]any{} @@ -458,7 +539,7 @@ func (s *UploadService) createActiveStorageDirectUpload(ctx context.Context, acc CreatedAt: upload.CreatedAt, SignedID: upload.UploadUUID, DirectUpload: ActiveStorageUploadURL{ - URL: directUploadURLPrefix + upload.UploadUUID, + URL: directUploadURLPrefix + upload.UploadUUID + "?token=" + url.QueryEscape(fmt.Sprint(metadata["active_storage_key"])), Headers: map[string]string{ "Content-Type": upload.MimeType, }, @@ -477,15 +558,10 @@ func (s *UploadService) processUpload(ctx context.Context, accountID uint, fileH } func (s *UploadService) processUploadContent(ctx context.Context, accountID uint, source model.DirectUploadSource, filename, mimeType string, size int64, reader io.Reader) (*UploadResponse, error) { - if mimeType == "" || mimeType == "application/octet-stream" { - // Fall back to filename-based detection when Content-Type is empty - // or the generic default (browsers/multipart forms often send this). - detected := detectUploadMIMEFromFilename(filename) - if detected != "" && detected != "application/octet-stream" { - mimeType = detected - } + data, mimeType, err := readValidatedUpload(reader, filename, mimeType, size, s.maxUploadSize(), true) + if err != nil { + return nil, err } - fileCategory := categorizeUploadMIME(mimeType) if fileCategory == "" { return nil, fmt.Errorf("unsupported file type: %s", mimeType) @@ -495,17 +571,14 @@ func (s *UploadService) processUploadContent(ctx context.Context, accountID uint return nil, fmt.Errorf("MIME type %s is not allowed for category %s", mimeType, fileCategory) } - maxSize := model.WidgetUploadMaxSizeByType[fileCategory] - if maxSize == 0 { - maxSize = int64(s.cfg.Storage.MaxFileSize) - } - if size > maxSize { - return nil, fmt.Errorf("file size %d exceeds maximum %d for type %s", size, maxSize, fileCategory) + maxSize := s.categoryUploadLimit(fileCategory) + if int64(len(data)) > maxSize { + return nil, fmt.Errorf("file size %d exceeds maximum %d for type %s", len(data), maxSize, fileCategory) } // Step 2: Store file to disk - ext := filepath.Ext(filename) - fileURL, thumbURL, err := s.saveUploadReader(accountID, source, ext, mimeType, reader) + ext := extensionForUploadMIME(mimeType) + fileURL, thumbURL, err := s.saveUploadReader(accountID, source, ext, mimeType, bytes.NewReader(data)) if err != nil { return nil, fmt.Errorf("failed to save file: %w", err) } @@ -521,7 +594,7 @@ func (s *UploadService) processUploadContent(ctx context.Context, accountID uint OriginalName: filename, FileType: fileCategory, MimeType: mimeType, - FileSize: size, + FileSize: int64(len(data)), FileURL: fileURL, ThumbURL: thumbURL, ExpiresAt: time.Now().Add(expiryDuration), @@ -586,7 +659,7 @@ func (s *UploadService) saveUploadReader(accountID uint, source model.DirectUplo return fileURL, thumbURL, nil } -func (s *UploadService) saveReaderToDisk(fileURL string, body io.Reader) error { +func (s *UploadService) saveReaderToDisk(fileURL string, body io.Reader, expectedSizes ...int64) error { localPath := s.cfg.Storage.LocalPath if localPath == "" { localPath = "./uploads" @@ -596,21 +669,142 @@ func (s *UploadService) saveReaderToDisk(fileURL string, body io.Reader) error { if err := os.MkdirAll(filepath.Dir(fullPath), 0755); err != nil { return err } - dst, err := os.Create(fullPath) + dst, err := os.CreateTemp(filepath.Dir(fullPath), ".upload-*") if err != nil { return err } - defer dst.Close() - _, err = io.Copy(dst, body) - return err + tmpPath := dst.Name() + defer os.Remove(tmpPath) + written, copyErr := io.Copy(dst, body) + closeErr := dst.Close() + if copyErr != nil { + return copyErr + } + if closeErr != nil { + return closeErr + } + if len(expectedSizes) > 0 && written != expectedSizes[0] { + return fmt.Errorf("file size mismatch: wrote %d bytes, expected %d", written, expectedSizes[0]) + } + return os.Rename(tmpPath, fullPath) } -func (s *UploadService) directUploadURL(source model.DirectUploadSource, accountID uint, uploadUUID, filename string) (string, string) { - ext := filepath.Ext(filename) +// ResolveAuthorizedUpload maps a stored upload URL to disk only after the +// requesting account or widget conversation has been authorized. +func (s *UploadService) ResolveAuthorizedUpload(ctx context.Context, fileURL string, accountID uint, widgetToken string) (string, bool) { + if s.accessDB == nil || !strings.HasPrefix(fileURL, "/uploads/") { + return "", false + } + fileURL = "/uploads/" + strings.TrimPrefix(filepath.ToSlash(filepath.Clean(strings.TrimPrefix(fileURL, "/uploads/"))), "/") + contactInboxID, widgetInboxID, widgetContactID := s.widgetAccessIdentity(ctx, widgetToken) + + var attachment struct { + AccountID uint + ContactInboxID *uint + } + if err := s.accessDB.WithContext(ctx).Raw(` + SELECT attachments.account_id, conversations.contact_inbox_id + FROM attachments + JOIN messages ON messages.id = attachments.message_id AND messages.deleted_at IS NULL + JOIN conversations ON conversations.id = messages.conversation_id AND conversations.deleted_at IS NULL + WHERE attachments.deleted_at IS NULL AND (attachments.file_url = ? OR attachments.thumb_url = ?) + LIMIT 1`, fileURL, fileURL).Scan(&attachment).Error; err == nil && attachment.AccountID != 0 { + allowed := accountID == attachment.AccountID || + (contactInboxID != 0 && attachment.ContactInboxID != nil && *attachment.ContactInboxID == contactInboxID) + return s.localUploadPath(fileURL, allowed) + } + + var directUpload model.DirectUpload + if err := s.accessDB.WithContext(ctx). + Where("file_url = ? OR thumb_url = ?", fileURL, fileURL). + First(&directUpload).Error; err == nil { + allowed := accountID != 0 && accountID == directUpload.AccountID + if directUpload.Source == model.DirectUploadSourceWidget && contactInboxID != 0 { + allowed = allowed || metadataUint(directUpload.Metadata, "contact_inbox_id") == contactInboxID + } + return s.localUploadPath(fileURL, allowed) + } + + var widgetUpload model.WidgetFileUpload + if err := s.accessDB.WithContext(ctx). + Where("file_url = ? OR thumb_url = ?", fileURL, fileURL). + First(&widgetUpload).Error; err == nil { + var inbox model.Inbox + _ = s.accessDB.WithContext(ctx).Select("account_id").First(&inbox, widgetUpload.InboxID).Error + allowed := (accountID != 0 && accountID == inbox.AccountID) || + (widgetInboxID == widgetUpload.InboxID && widgetContactID != 0 && widgetContactID == widgetUpload.ContactID) + return s.localUploadPath(fileURL, allowed) + } + + // Durable account files such as profile avatars do not have attachment rows. + // Keep them account-private by the existing /account// storage boundary. + parts := strings.Split(strings.TrimPrefix(fileURL, "/uploads/"), "/") + if len(parts) >= 3 && parts[0] == "account" { + pathAccountID, err := strconv.ParseUint(parts[1], 10, 32) + return s.localUploadPath(fileURL, err == nil && accountID != 0 && uint(pathAccountID) == accountID) + } + return "", false +} + +func (s *UploadService) widgetAccessIdentity(ctx context.Context, token string) (uint, uint, uint) { + if token == "" || s.contactInboxRepo == nil { + return 0, 0, 0 + } + contactInbox, err := s.contactInboxRepo.FindByPubsubToken(ctx, token) + if err != nil { + return 0, 0, 0 + } + return contactInbox.ID, contactInbox.InboxID, contactInbox.ContactID +} + +func (s *UploadService) localUploadPath(fileURL string, allowed bool) (string, bool) { + if !allowed { + return "", false + } + localPath := s.cfg.Storage.LocalPath + if localPath == "" { + localPath = "./uploads" + } + base, err := filepath.Abs(localPath) + if err != nil { + return "", false + } + fullPath, err := filepath.Abs(filepath.Join(base, filepath.FromSlash(strings.TrimPrefix(fileURL, "/uploads/")))) + if err != nil { + return "", false + } + relative, err := filepath.Rel(base, fullPath) + if err != nil || relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + return "", false + } + if info, err := os.Stat(fullPath); err != nil || info.IsDir() { + return "", false + } + return fullPath, true +} + +func metadataUint(raw []byte, key string) uint { + var metadata map[string]any + if json.Unmarshal(raw, &metadata) != nil { + return 0 + } + switch value := metadata[key].(type) { + case float64: + return uint(value) + case string: + parsed, _ := strconv.ParseUint(value, 10, 32) + return uint(parsed) + default: + return 0 + } +} + +func (s *UploadService) directUploadURL(source model.DirectUploadSource, accountID uint, uploadUUID, mimeType string) (string, string) { + ext := extensionForUploadMIME(mimeType) fileName := uploadUUID + ext fileURL := s.uploadURL(source, accountID, fileName) thumbURL := "" - if strings.HasPrefix(detectUploadMIMEFromFilename(filename), "image/") { + if strings.HasPrefix(mimeType, "image/") { thumbURL = fileURL } return fileURL, thumbURL @@ -664,6 +858,134 @@ func (s *UploadService) CleanupExpiredUploads(ctx context.Context) (int64, error // --- MIME detection helpers (reuse patterns from widget_theme_service.go) --- +func (s *UploadService) maxUploadSize() int64 { + if s.cfg != nil && s.cfg.Storage.MaxFileSize > 0 { + return s.cfg.Storage.MaxFileSize + } + return 20 << 20 +} + +func (s *UploadService) categoryUploadLimit(category string) int64 { + limit := s.maxUploadSize() + if categoryLimit := model.WidgetUploadMaxSizeByType[category]; categoryLimit > 0 && categoryLimit < limit { + return categoryLimit + } + return limit +} + +func readValidatedUpload(reader io.Reader, filename, declaredMIME string, expectedSize, maxSize int64, requireExactSize bool) ([]byte, string, error) { + if reader == nil { + return nil, "", errors.New("file content is required") + } + if maxSize <= 0 { + maxSize = 20 << 20 + } + if expectedSize > maxSize { + return nil, "", fmt.Errorf("file size %d exceeds maximum %d", expectedSize, maxSize) + } + data, err := io.ReadAll(io.LimitReader(reader, maxSize+1)) + if err != nil { + return nil, "", err + } + if int64(len(data)) > maxSize { + return nil, "", fmt.Errorf("file size exceeds maximum %d", maxSize) + } + if requireExactSize && expectedSize >= 0 && int64(len(data)) != expectedSize { + return nil, "", fmt.Errorf("file size mismatch: got %d, expected %d", len(data), expectedSize) + } + if len(data) == 0 { + return nil, "", errors.New("empty file is not allowed") + } + + detectedMIME := normalizeUploadMIME(http.DetectContentType(data[:min(len(data), 512)])) + declaredMIME = normalizeUploadMIME(declaredMIME) + if declaredMIME == "" || declaredMIME == "application/octet-stream" { + declaredMIME = detectUploadMIMEFromFilename(filename) + if declaredMIME == "application/octet-stream" { + return nil, "", fmt.Errorf("unsupported file type: extension %s", filepath.Ext(filename)) + } + } + if !uploadMIMEMatches(declaredMIME, detectedMIME) { + return nil, "", fmt.Errorf("file content type %s does not match declared type %s", detectedMIME, declaredMIME) + } + if declaredMIME == "text/csv" && detectedMIME == "text/plain" { + return data, declaredMIME, nil + } + if declaredMIME != detectedMIME { + return data, declaredMIME, nil + } + return data, detectedMIME, nil +} + +func normalizeUploadMIME(value string) string { + value = strings.TrimSpace(strings.ToLower(value)) + if value == "" { + return "" + } + mediaType, _, err := mime.ParseMediaType(value) + if err == nil { + return mediaType + } + return strings.TrimSpace(strings.Split(value, ";")[0]) +} + +func uploadMIMEMatches(declared, detected string) bool { + if declared == detected || (declared == "text/csv" && detected == "text/plain") { + return true + } + switch declared { + case "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", + "application/vnd.openxmlformats-officedocument.wordprocessingml.document": + return detected == "application/zip" || detected == "application/octet-stream" + case "application/vnd.ms-excel", "application/msword": + return detected == "application/octet-stream" + } + return false +} + +func extensionForUploadMIME(mimeType string) string { + switch normalizeUploadMIME(mimeType) { + case "image/png": + return ".png" + case "image/jpeg": + return ".jpg" + case "image/gif": + return ".gif" + case "image/webp": + return ".webp" + case "audio/mpeg", "audio/mp3": + return ".mp3" + case "audio/ogg": + return ".ogg" + case "audio/wav": + return ".wav" + case "audio/webm": + return ".webm" + case "video/mp4": + return ".mp4" + case "video/webm": + return ".webm" + case "video/ogg": + return ".ogv" + case "application/pdf": + return ".pdf" + case "text/csv": + return ".csv" + case "text/plain": + return ".txt" + case "application/vnd.ms-excel": + return ".xls" + case "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet": + return ".xlsx" + case "application/msword": + return ".doc" + case "application/vnd.openxmlformats-officedocument.wordprocessingml.document": + return ".docx" + default: + return "" + } +} + func detectUploadMIMEFromFilename(filename string) string { ext := strings.ToLower(filepath.Ext(filename)) switch ext { @@ -740,9 +1062,5 @@ func isUploadMIMEAllowed(category string, mimeType string) bool { return true } } - // For "file" category, also allow application/octet-stream (unknown file types with correct extension) - if category == "file" && mimeType == "application/octet-stream" { - return true - } return false } diff --git a/backend/internal/service/upload_service_test.go b/backend/internal/service/upload_service_test.go index 255d8075..12293601 100644 --- a/backend/internal/service/upload_service_test.go +++ b/backend/internal/service/upload_service_test.go @@ -2,8 +2,10 @@ package service import ( "context" + "encoding/json" "mime/multipart" "net/http" + "os" "path/filepath" "strings" "testing" @@ -20,6 +22,11 @@ import ( "github.com/gochat/gochat/internal/repository" ) +var ( + testPNG = []byte{0x89, 'P', 'N', 'G', '\r', '\n', 0x1a, '\n', 0, 0, 0, 0} + testMP3 = []byte{'I', 'D', '3', 4, 0, 0, 0, 0, 0, 0} +) + // --- MIME detection helper tests --- func TestDetectUploadMIMEFromFilename(t *testing.T) { @@ -93,7 +100,7 @@ func TestIsUploadMIMEAllowed(t *testing.T) { assert.True(t, isUploadMIMEAllowed("video", "video/mp4")) assert.True(t, isUploadMIMEAllowed("file", "application/pdf")) assert.True(t, isUploadMIMEAllowed("file", "text/csv")) - assert.True(t, isUploadMIMEAllowed("file", "application/octet-stream")) + assert.False(t, isUploadMIMEAllowed("file", "application/octet-stream")) // Disallowed MIME types assert.False(t, isUploadMIMEAllowed("image", "audio/mpeg")) // wrong category @@ -111,7 +118,14 @@ func setupUploadServiceTest(t *testing.T) (*gorm.DB, *UploadService, string) { }) require.NoError(t, err, "failed to open test DB") - require.NoError(t, db.AutoMigrate(&model.DirectUpload{}), "failed to auto-migrate DirectUpload") + require.NoError(t, db.AutoMigrate(&model.Account{}, &model.Inbox{}, &model.Contact{}, &model.ContactInbox{}, &model.DirectUpload{}), "failed to auto-migrate upload models") + account := &model.Account{Name: "Upload Test", Status: "active"} + require.NoError(t, db.Create(account).Error) + inbox := &model.Inbox{AccountID: account.ID, Name: "Upload Inbox", ChannelType: "web_widget", ChannelConfig: `{"website_token":"test-website"}`, Enabled: true} + require.NoError(t, db.Create(inbox).Error) + contact := &model.Contact{AccountID: account.ID, Name: "Upload Contact"} + require.NoError(t, db.Create(contact).Error) + require.NoError(t, db.Create(&model.ContactInbox{ContactID: contact.ID, InboxID: inbox.ID, PubsubToken: "test-widget"}).Error) t.Cleanup(func() { sqlDB, _ := db.DB() @@ -130,7 +144,8 @@ func setupUploadServiceTest(t *testing.T) (*gorm.DB, *UploadService, string) { } directUploadRepo := repository.NewDirectUploadRepo(db) - uploadService := NewUploadService(directUploadRepo, cfg) + uploadService := NewUploadService(directUploadRepo, cfg). + WithWidgetAuth(repository.NewInboxRepo(db), repository.NewContactInboxRepo(db)) return db, uploadService, tmpDir } @@ -161,7 +176,7 @@ func createTestFileHeader(t *testing.T, filename string, content []byte) *multip func TestUploadService_AccountUpload_Success(t *testing.T) { _, svc, tmpDir := setupUploadServiceTest(t) - fileHeader := createTestFileHeader(t, "test_image.png", []byte("fake png content")) + fileHeader := createTestFileHeader(t, "test_image.png", testPNG) result, svcErr := svc.AccountUpload(context.Background(), 1, AccountUploadRequest{ FileHeader: fileHeader, @@ -203,10 +218,12 @@ func TestUploadService_AccountUpload_NoFile(t *testing.T) { func TestUploadService_WidgetDirectUpload_Success(t *testing.T) { _, svc, tmpDir := setupUploadServiceTest(t) - fileHeader := createTestFileHeader(t, "visitor_audio.mp3", []byte("fake mp3 content")) + fileHeader := createTestFileHeader(t, "visitor_audio.mp3", testMP3) result, svcErr := svc.WidgetDirectUpload(context.Background(), WidgetDirectUploadRequest{ - FileHeader: fileHeader, + WebsiteToken: "test-website", + AuthToken: "test-widget", + FileHeader: fileHeader, }) require.NoError(t, svcErr, "WidgetDirectUpload should succeed") @@ -217,7 +234,7 @@ func TestUploadService_WidgetDirectUpload_Success(t *testing.T) { assert.Equal(t, "pending", result.Status) // Widget uploads should go to "widget_direct" subdirectory - files, _ := filepath.Glob(filepath.Join(tmpDir, "widget_direct", "*.mp3")) + files, _ := filepath.Glob(filepath.Join(tmpDir, "widget_direct", "1", "*.mp3")) assert.GreaterOrEqual(t, len(files), 1, "widget file should be saved in widget_direct dir") } @@ -252,7 +269,7 @@ func TestUploadService_AccountUpload_UnsupportedFileType(t *testing.T) { func TestUploadService_AccountDirectUpload_Success(t *testing.T) { _, svc, tmpDir := setupUploadServiceTest(t) - fileHeader := createTestFileHeader(t, "staged_image.png", []byte("fake png content")) + fileHeader := createTestFileHeader(t, "staged_image.png", testPNG) result, svcErr := svc.AccountDirectUpload(context.Background(), 1, AccountDirectUploadRequest{ FileHeader: fileHeader, @@ -274,7 +291,7 @@ func TestUploadService_AccountDirectUpload_Success(t *testing.T) { func TestUploadService_AccountDirectUpload_NoAccountID(t *testing.T) { _, svc, _ := setupUploadServiceTest(t) - fileHeader := createTestFileHeader(t, "test.png", []byte("fake png")) + fileHeader := createTestFileHeader(t, "test.png", testPNG) _, svcErr := svc.AccountDirectUpload(context.Background(), 0, AccountDirectUploadRequest{ FileHeader: fileHeader, @@ -305,6 +322,142 @@ func TestUploadService_AccountDirectUpload_UnsupportedFileType(t *testing.T) { assert.Contains(t, svcErr.Error(), "unsupported file type") } +func TestUploadService_RejectsSpoofedMIMEAndSVG(t *testing.T) { + _, svc, _ := setupUploadServiceTest(t) + + for _, test := range []struct { + name string + filename string + content []byte + }{ + {name: "forged png", filename: "attack.png", content: []byte("")}, + {name: "active svg", filename: "attack.svg", content: []byte(``)}, + } { + t.Run(test.name, func(t *testing.T) { + _, err := svc.AccountUpload(context.Background(), 1, AccountUploadRequest{FileHeader: createTestFileHeader(t, test.filename, test.content)}) + require.Error(t, err) + }) + } +} + +func TestUploadService_AccountUploadFromURLRejectsPrivateTargets(t *testing.T) { + _, svc, _ := setupUploadServiceTest(t) + for _, target := range []string{ + "http://127.0.0.1/file.png", + "http://10.0.0.1/file.png", + "http://169.254.169.254/latest/meta-data/iam/security-credentials/", + } { + _, err := svc.AccountUploadFromURL(context.Background(), 1, target) + require.Error(t, err, target) + assert.Contains(t, err.Error(), "SSRF", target) + } +} + +func TestUploadService_WidgetUploadRequiresValidSession(t *testing.T) { + _, svc, _ := setupUploadServiceTest(t) + _, err := svc.WidgetDirectUpload(context.Background(), WidgetDirectUploadRequest{ + FileHeader: createTestFileHeader(t, "visitor.png", testPNG), + }) + require.Error(t, err) + assert.Contains(t, err.Error(), "website_token is required") +} + +func TestUploadService_CompleteWidgetUploadRejectsTokenAndMIMESpoof(t *testing.T) { + db, svc, tmpDir := setupUploadServiceTest(t) + content := []byte("") + upload := &model.DirectUpload{ + UploadUUID: "secured-widget-upload", + AccountID: 1, + Status: model.DirectUploadStatusPending, + Source: model.DirectUploadSourceWidget, + OriginalName: "attack.png", + FileType: "image", + MimeType: "image/png", + FileSize: int64(len(content)), + FileURL: "/uploads/widget_direct/secured-widget-upload.png", + Metadata: []byte(`{"active_storage_key":"secret-token","contact_inbox_id":1}`), + ExpiresAt: time.Now().Add(time.Hour), + } + require.NoError(t, db.Create(upload).Error) + + _, err := svc.CompleteWidgetDirectUpload(context.Background(), upload.UploadUUID, strings.NewReader(string(content)), "wrong-token") + require.ErrorContains(t, err, "invalid direct upload token") + _, err = svc.CompleteWidgetDirectUpload(context.Background(), upload.UploadUUID, strings.NewReader(string(content)), "secret-token") + require.ErrorContains(t, err, "does not match") + _, statErr := os.Stat(filepath.Join(tmpDir, "widget_direct", "secured-widget-upload.png")) + require.ErrorIs(t, statErr, os.ErrNotExist) +} + +func TestUploadService_PrivateAttachmentAccessIsTenantAndSessionScoped(t *testing.T) { + db, svc, tmpDir := setupUploadServiceTest(t) + require.NoError(t, db.AutoMigrate(&model.Conversation{}, &model.Message{}, &model.Attachment{}, &model.WidgetFileUpload{})) + var inbox model.Inbox + var contact model.Contact + var contactInbox model.ContactInbox + require.NoError(t, db.First(&inbox).Error) + require.NoError(t, db.First(&contact).Error) + require.NoError(t, db.First(&contactInbox).Error) + conversation := &model.Conversation{AccountID: 1, InboxID: inbox.ID, ContactID: contact.ID, ContactInboxID: &contactInbox.ID, Status: "open", ChannelType: "web_widget", Channel: "web_widget"} + require.NoError(t, db.Create(conversation).Error) + message := &model.Message{AccountID: 1, InboxID: inbox.ID, ConversationID: conversation.ID, MessageType: "incoming", Content: "file"} + require.NoError(t, db.Create(message).Error) + fileURL := "/uploads/account/1/private.png" + require.NoError(t, db.Create(&model.Attachment{AccountID: 1, MessageID: message.ID, FileType: "image", FileURL: fileURL, FileName: "private.png"}).Error) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "account", "1"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "account", "1", "private.png"), testPNG, 0o600)) + svc.WithAccessDB(db) + + _, ok := svc.ResolveAuthorizedUpload(context.Background(), fileURL, 2, "") + assert.False(t, ok) + _, ok = svc.ResolveAuthorizedUpload(context.Background(), fileURL, 1, "") + assert.True(t, ok) + _, ok = svc.ResolveAuthorizedUpload(context.Background(), fileURL, 0, "test-widget") + assert.True(t, ok) + _, ok = svc.ResolveAuthorizedUpload(context.Background(), fileURL, 0, "wrong-widget") + assert.False(t, ok) + + directURL := "/uploads/widget_direct/direct.png" + directMetadata, err := json.Marshal(map[string]any{"contact_inbox_id": contactInbox.ID}) + require.NoError(t, err) + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "widget_direct"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "widget_direct", "direct.png"), testPNG, 0o600)) + require.NoError(t, db.Create(&model.DirectUpload{ + UploadUUID: "private-direct", AccountID: 1, Status: model.DirectUploadStatusPending, + Source: model.DirectUploadSourceWidget, OriginalName: "direct.png", FileType: "image", + MimeType: "image/png", FileSize: int64(len(testPNG)), FileURL: directURL, + Metadata: directMetadata, ExpiresAt: time.Now().Add(time.Hour), + }).Error) + _, ok = svc.ResolveAuthorizedUpload(context.Background(), directURL, 0, "test-widget") + assert.True(t, ok) + _, ok = svc.ResolveAuthorizedUpload(context.Background(), directURL, 2, "") + assert.False(t, ok) + + widgetURL := "/uploads/widget/1/session.png" + require.NoError(t, os.MkdirAll(filepath.Join(tmpDir, "widget", "1"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "widget", "1", "session.png"), testPNG, 0o600)) + require.NoError(t, db.Create(&model.WidgetFileUpload{ + UploadUUID: "private-widget", WidgetToken: "test-widget", InboxID: inbox.ID, ContactID: contact.ID, + Status: model.WidgetFileUploadStatusPending, OriginalName: "session.png", FileType: "image", + MimeType: "image/png", FileSize: int64(len(testPNG)), FileURL: widgetURL, ExpiresAt: time.Now().Add(time.Hour), + }).Error) + _, ok = svc.ResolveAuthorizedUpload(context.Background(), widgetURL, 0, "test-widget") + assert.True(t, ok) + _, ok = svc.ResolveAuthorizedUpload(context.Background(), widgetURL, 0, "wrong-widget") + assert.False(t, ok) + otherInbox := &model.Inbox{AccountID: 1, Name: "Other Widget Inbox", ChannelType: "web_widget", ChannelConfig: `{"website_token":"other-website"}`, Enabled: true} + require.NoError(t, db.Create(otherInbox).Error) + require.NoError(t, db.Create(&model.ContactInbox{ContactID: contact.ID, InboxID: otherInbox.ID, PubsubToken: "other-widget"}).Error) + _, ok = svc.ResolveAuthorizedUpload(context.Background(), widgetURL, 0, "other-widget") + assert.False(t, ok) + + avatarURL := "/uploads/account/1/avatar.png" + require.NoError(t, os.WriteFile(filepath.Join(tmpDir, "account", "1", "avatar.png"), testPNG, 0o600)) + _, ok = svc.ResolveAuthorizedUpload(context.Background(), avatarURL, 1, "") + assert.True(t, ok) + _, ok = svc.ResolveAuthorizedUpload(context.Background(), avatarURL, 2, "") + assert.False(t, ok) +} + func TestUploadService_CleanupExpiredUploads(t *testing.T) { db, svc, _ := setupUploadServiceTest(t) @@ -347,4 +500,4 @@ func TestUploadService_CleanupExpiredUploads(t *testing.T) { var remaining model.DirectUpload require.NoError(t, db.Where("upload_uuid = ?", "active-uuid-456").First(&remaining).Error) assert.Equal(t, model.DirectUploadStatusPending, remaining.Status) -} \ No newline at end of file +} diff --git a/backend/internal/service/widget_theme_service.go b/backend/internal/service/widget_theme_service.go index e98ec8e0..b9d99ba5 100644 --- a/backend/internal/service/widget_theme_service.go +++ b/backend/internal/service/widget_theme_service.go @@ -326,12 +326,16 @@ func (s *WidgetService) StageFileUpload(ctx context.Context, req WidgetUploadReq } inboxID := inbox.ID + contactInbox, err := s.contactInboxRepo.FindByPubsubToken(ctx, req.WidgetToken) + if err != nil || contactInbox.InboxID != inboxID { + return nil, errors.New("valid widget session is required") + } // Step 2: Validate file fileHeader := req.FileHeader - mimeType := fileHeader.Header.Get("Content-Type") - if mimeType == "" { - mimeType = detectMIMEFromFilename(fileHeader.Filename) + _, mimeType, err := readValidatedUpload(fileReader, fileHeader.Filename, fileHeader.Header.Get("Content-Type"), fileHeader.Size, 15<<20, true) + if err != nil { + return nil, err } fileCategory := categorizeMIME(mimeType) @@ -361,16 +365,18 @@ func (s *WidgetService) StageFileUpload(ctx context.Context, req WidgetUploadReq expiryDuration := 24 * time.Hour uploadUUID := uuid.New().String() upload := &model.WidgetFileUpload{ - UploadUUID: uploadUUID, - InboxID: inboxID, - Status: model.WidgetFileUploadStatusPending, + UploadUUID: uploadUUID, + WidgetToken: req.WidgetToken, + InboxID: inboxID, + ContactID: contactInbox.ContactID, + Status: model.WidgetFileUploadStatusPending, OriginalName: fileHeader.Filename, - FileType: fileCategory, - MimeType: mimeType, - FileSize: fileHeader.Size, - FileURL: fileURL, - ThumbURL: thumbURL, - ExpiresAt: time.Now().Add(expiryDuration), + FileType: fileCategory, + MimeType: mimeType, + FileSize: fileHeader.Size, + FileURL: fileURL, + ThumbURL: thumbURL, + ExpiresAt: time.Now().Add(expiryDuration), } if err := s.fileUploadRepo.Create(ctx, upload); err != nil { @@ -381,20 +387,20 @@ func (s *WidgetService) StageFileUpload(ctx context.Context, req WidgetUploadReq inboxID, uploadUUID, fileHeader.Filename, fileHeader.Size) return &WidgetUploadResponse{ - UploadUUID: uploadUUID, - UploadID: upload.ID, + UploadUUID: uploadUUID, + UploadID: upload.ID, OriginalName: upload.OriginalName, - FileType: upload.FileType, - FileSize: upload.FileSize, - FileURL: upload.FileURL, - ThumbURL: upload.ThumbURL, - Status: string(upload.Status), - ExpiresAt: upload.ExpiresAt, + FileType: upload.FileType, + FileSize: upload.FileSize, + FileURL: upload.FileURL, + ThumbURL: upload.ThumbURL, + Status: string(upload.Status), + ExpiresAt: upload.ExpiresAt, }, nil } // GetFileUploadStatus retrieves the status of a staged file upload by UUID. -func (s *WidgetService) GetFileUploadStatus(ctx context.Context, websiteToken string, uploadUUID string) (*WidgetUploadResponse, error) { +func (s *WidgetService) GetFileUploadStatus(ctx context.Context, websiteToken string, uploadUUID string, widgetTokens ...string) (*WidgetUploadResponse, error) { if websiteToken == "" { return nil, errors.New("website_token is required") } @@ -403,26 +409,37 @@ func (s *WidgetService) GetFileUploadStatus(ctx context.Context, websiteToken st } // Verify website_token resolves to valid inbox - _, err := s.inboxRepo.FindByWebsiteToken(ctx, websiteToken) + inbox, err := s.inboxRepo.FindByWebsiteToken(ctx, websiteToken) if err != nil { return nil, fmt.Errorf("invalid website_token: %w", err) } + widgetToken := "" + if len(widgetTokens) > 0 { + widgetToken = widgetTokens[0] + } + contactInbox, err := s.contactInboxRepo.FindByPubsubToken(ctx, widgetToken) + if err != nil || contactInbox.InboxID != inbox.ID { + return nil, errors.New("valid widget session is required") + } upload, err := s.fileUploadRepo.FindByUUID(ctx, uploadUUID) if err != nil { return nil, nil // Not found = nil, not error } + if upload.InboxID != inbox.ID || upload.ContactID != contactInbox.ContactID { + return nil, nil + } return &WidgetUploadResponse{ - UploadUUID: upload.UploadUUID, - UploadID: upload.ID, + UploadUUID: upload.UploadUUID, + UploadID: upload.ID, OriginalName: upload.OriginalName, - FileType: upload.FileType, - FileSize: upload.FileSize, - FileURL: upload.FileURL, - ThumbURL: upload.ThumbURL, - Status: string(upload.Status), - ExpiresAt: upload.ExpiresAt, + FileType: upload.FileType, + FileSize: upload.FileSize, + FileURL: upload.FileURL, + ThumbURL: upload.ThumbURL, + Status: string(upload.Status), + ExpiresAt: upload.ExpiresAt, }, nil } @@ -454,9 +471,9 @@ func (s *WidgetService) UploadFile(ctx context.Context, req WidgetUploadRequest, // Step 2: Validate file fileHeader := req.File - mimeType := fileHeader.Header.Get("Content-Type") - if mimeType == "" { - mimeType = detectMIMEFromFilename(fileHeader.Filename) + _, mimeType, err := readValidatedUpload(fileReader, fileHeader.Filename, fileHeader.Header.Get("Content-Type"), fileHeader.Size, 15<<20, true) + if err != nil { + return nil, err } fileCategory := categorizeMIME(mimeType) @@ -629,4 +646,4 @@ func isMIMEAllowed(category string, mimeType string) bool { } } return false -} \ No newline at end of file +} diff --git a/backend/migrations/000083_align_upload_staging_tables.down.sql b/backend/migrations/000083_align_upload_staging_tables.down.sql new file mode 100644 index 00000000..fa6a9b72 --- /dev/null +++ b/backend/migrations/000083_align_upload_staging_tables.down.sql @@ -0,0 +1,29 @@ +DROP TABLE IF EXISTS widget_file_uploads; + +ALTER INDEX IF EXISTS idx_widget_file_upload_configs_inbox_id RENAME TO idx_widget_file_uploads_inbox_id; +ALTER INDEX IF EXISTS idx_widget_file_upload_configs_deleted_at RENAME TO idx_widget_file_uploads_deleted_at; +ALTER TABLE widget_file_upload_configs RENAME TO widget_file_uploads; + +ALTER TABLE direct_uploads + ADD COLUMN IF NOT EXISTS file_name VARCHAR(255), + ADD COLUMN IF NOT EXISTS content_type VARCHAR(100); + +UPDATE direct_uploads +SET file_name = original_name, + content_type = mime_type; + +DROP INDEX IF EXISTS idx_direct_uploads_upload_uuid; +DROP INDEX IF EXISTS idx_direct_uploads_status; +DROP INDEX IF EXISTS idx_direct_uploads_source; +DROP INDEX IF EXISTS idx_direct_uploads_expires_at; + +ALTER TABLE direct_uploads + DROP COLUMN IF EXISTS upload_uuid, + DROP COLUMN IF EXISTS status, + DROP COLUMN IF EXISTS source, + DROP COLUMN IF EXISTS original_name, + DROP COLUMN IF EXISTS file_type, + DROP COLUMN IF EXISTS mime_type, + DROP COLUMN IF EXISTS thumb_url, + DROP COLUMN IF EXISTS metadata, + DROP COLUMN IF EXISTS expires_at; diff --git a/backend/migrations/000083_align_upload_staging_tables.up.sql b/backend/migrations/000083_align_upload_staging_tables.up.sql new file mode 100644 index 00000000..4822fb66 --- /dev/null +++ b/backend/migrations/000083_align_upload_staging_tables.up.sql @@ -0,0 +1,79 @@ +ALTER TABLE direct_uploads + ADD COLUMN IF NOT EXISTS upload_uuid VARCHAR(36), + ADD COLUMN IF NOT EXISTS status VARCHAR(20), + ADD COLUMN IF NOT EXISTS source VARCHAR(20), + ADD COLUMN IF NOT EXISTS original_name VARCHAR(255), + ADD COLUMN IF NOT EXISTS file_type VARCHAR(50), + ADD COLUMN IF NOT EXISTS mime_type VARCHAR(100), + ADD COLUMN IF NOT EXISTS thumb_url VARCHAR(512), + ADD COLUMN IF NOT EXISTS metadata JSONB, + ADD COLUMN IF NOT EXISTS expires_at TIMESTAMPTZ; + +UPDATE direct_uploads +SET upload_uuid = COALESCE(NULLIF(upload_uuid, ''), gen_random_uuid()::text), + status = COALESCE(NULLIF(status, ''), 'completed'), + source = COALESCE(NULLIF(source, ''), 'account'), + original_name = COALESCE(NULLIF(original_name, ''), NULLIF(file_name, ''), NULLIF(regexp_replace(file_url, '^.*/', ''), ''), 'upload'), + file_type = COALESCE(NULLIF(file_type, ''), CASE + WHEN COALESCE(mime_type, content_type, '') LIKE 'image/%' THEN 'image' + WHEN COALESCE(mime_type, content_type, '') LIKE 'audio/%' THEN 'audio' + WHEN COALESCE(mime_type, content_type, '') LIKE 'video/%' THEN 'video' + ELSE 'file' + END), + mime_type = COALESCE(NULLIF(mime_type, ''), NULLIF(content_type, ''), 'application/octet-stream'), + file_size = COALESCE(file_size, 0), + metadata = COALESCE(metadata, '{}'::jsonb), + expires_at = COALESCE(expires_at, updated_at, created_at, NOW()); + +ALTER TABLE direct_uploads + ALTER COLUMN upload_uuid SET NOT NULL, + ALTER COLUMN status SET DEFAULT 'pending', + ALTER COLUMN status SET NOT NULL, + ALTER COLUMN source SET NOT NULL, + ALTER COLUMN original_name SET NOT NULL, + ALTER COLUMN file_type SET NOT NULL, + ALTER COLUMN file_size SET NOT NULL, + ALTER COLUMN expires_at SET NOT NULL, + DROP COLUMN IF EXISTS file_name, + DROP COLUMN IF EXISTS content_type; + +CREATE UNIQUE INDEX IF NOT EXISTS idx_direct_uploads_upload_uuid ON direct_uploads(upload_uuid); +CREATE INDEX IF NOT EXISTS idx_direct_uploads_status ON direct_uploads(status); +CREATE INDEX IF NOT EXISTS idx_direct_uploads_source ON direct_uploads(source); +CREATE INDEX IF NOT EXISTS idx_direct_uploads_expires_at ON direct_uploads(expires_at); + +ALTER TABLE widget_file_uploads RENAME TO widget_file_upload_configs; +ALTER INDEX IF EXISTS idx_widget_file_uploads_inbox_id RENAME TO idx_widget_file_upload_configs_inbox_id; +ALTER INDEX IF EXISTS idx_widget_file_uploads_deleted_at RENAME TO idx_widget_file_upload_configs_deleted_at; + +CREATE TABLE widget_file_uploads ( + id BIGSERIAL PRIMARY KEY, + upload_uuid VARCHAR(36) NOT NULL, + widget_token VARCHAR(255), + inbox_id BIGINT NOT NULL, + contact_id BIGINT, + status VARCHAR(20) NOT NULL DEFAULT 'pending', + original_name VARCHAR(255) NOT NULL, + file_type VARCHAR(50) NOT NULL, + mime_type VARCHAR(100), + file_size BIGINT NOT NULL, + file_url VARCHAR(512), + thumb_url VARCHAR(512), + width INTEGER DEFAULT 0, + height INTEGER DEFAULT 0, + metadata JSONB, + message_id BIGINT, + expires_at TIMESTAMPTZ NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + deleted_at TIMESTAMPTZ +); + +CREATE UNIQUE INDEX idx_widget_file_uploads_upload_uuid ON widget_file_uploads(upload_uuid); +CREATE INDEX idx_widget_file_uploads_widget_token ON widget_file_uploads(widget_token); +CREATE INDEX idx_widget_file_uploads_inbox_id ON widget_file_uploads(inbox_id); +CREATE INDEX idx_widget_file_uploads_contact_id ON widget_file_uploads(contact_id); +CREATE INDEX idx_widget_file_uploads_status ON widget_file_uploads(status); +CREATE INDEX idx_widget_file_uploads_message_id ON widget_file_uploads(message_id); +CREATE INDEX idx_widget_file_uploads_expires_at ON widget_file_uploads(expires_at); +CREATE INDEX idx_widget_file_uploads_deleted_at ON widget_file_uploads(deleted_at);