diff --git a/internal/handlers/app.go b/internal/handlers/app.go index a1fe09e..8a94b42 100644 --- a/internal/handlers/app.go +++ b/internal/handlers/app.go @@ -824,7 +824,7 @@ func (h *Handlers) HandleEnvVarDelete() http.HandlerFunc { } envVar, findErr := models.FindEnvVar(request.Context(), h.db, envVarID) - if findErr != nil || envVar == nil { + if findErr != nil || envVar == nil || envVar.AppID != appID { http.NotFound(writer, request) return @@ -871,7 +871,7 @@ func (h *Handlers) HandleLabelDelete() http.HandlerFunc { } label, findErr := models.FindLabel(request.Context(), h.db, labelID) - if findErr != nil || label == nil { + if findErr != nil || label == nil || label.AppID != appID { http.NotFound(writer, request) return @@ -949,7 +949,7 @@ func (h *Handlers) HandleVolumeDelete() http.HandlerFunc { } volume, findErr := models.FindVolume(request.Context(), h.db, volumeID) - if findErr != nil || volume == nil { + if findErr != nil || volume == nil || volume.AppID != appID { http.NotFound(writer, request) return @@ -1039,7 +1039,7 @@ func (h *Handlers) HandlePortDelete() http.HandlerFunc { } port, findErr := models.FindPort(request.Context(), h.db, portID) - if findErr != nil || port == nil { + if findErr != nil || port == nil || port.AppID != appID { http.NotFound(writer, request) return diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index 58266fb..03de641 100644 --- a/internal/handlers/handlers_test.go +++ b/internal/handlers/handlers_test.go @@ -5,6 +5,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strconv" "strings" "testing" "time" @@ -14,6 +15,8 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/fx" + "git.eeqj.de/sneak/upaas/internal/models" + "git.eeqj.de/sneak/upaas/internal/config" "git.eeqj.de/sneak/upaas/internal/database" "git.eeqj.de/sneak/upaas/internal/docker" @@ -426,6 +429,29 @@ func addChiURLParams( ) } +// createTestApp creates an app using the app service and returns it. +func createTestApp( + t *testing.T, + tc *testContext, + name string, +) *models.App { + t.Helper() + + createdApp, err := tc.appSvc.CreateApp( + context.Background(), + app.CreateAppInput{ + Name: name, + RepoURL: "git@example.com:user/" + name + ".git", + Branch: "main", + }, + ) + require.NoError(t, err) + + return createdApp +} + +// TestDeleteEnvVarOwnershipVerification tests that deleting an env var +// via another app's URL path returns 404 (IDOR prevention). func TestHandleWebhookRejectsOversizedBody(t *testing.T) { t.Parallel() @@ -467,6 +493,169 @@ func TestHandleWebhookRejectsOversizedBody(t *testing.T) { assert.Equal(t, http.StatusOK, recorder.Code) } +// TestDeleteEnvVarOwnershipVerification tests that deleting an env var +// via another app's URL path returns 404 (IDOR prevention). +func TestDeleteEnvVarOwnershipVerification(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + app1 := createTestApp(t, testCtx, "envvar-owner-app") + app2 := createTestApp(t, testCtx, "envvar-other-app") + + // Create env var belonging to app1 + envVar := models.NewEnvVar(testCtx.database) + envVar.AppID = app1.ID + envVar.Key = "SECRET" + envVar.Value = "hunter2" + require.NoError(t, envVar.Save(context.Background())) + + // Try to delete app1's env var using app2's URL path + request := httptest.NewRequest( + http.MethodPost, + "/apps/"+app2.ID+"/env/"+strconv.FormatInt(envVar.ID, 10)+"/delete", + nil, + ) + request = addChiURLParams(request, map[string]string{ + "id": app2.ID, + "envID": strconv.FormatInt(envVar.ID, 10), + }) + recorder := httptest.NewRecorder() + + handler := testCtx.handlers.HandleEnvVarDelete() + handler.ServeHTTP(recorder, request) + + // Should return 404 because the env var doesn't belong to app2 + assert.Equal(t, http.StatusNotFound, recorder.Code) + + // Verify the env var was NOT deleted + found, err := models.FindEnvVar(context.Background(), testCtx.database, envVar.ID) + require.NoError(t, err) + assert.NotNil(t, found, "env var should still exist after IDOR attempt") +} + +// TestDeleteLabelOwnershipVerification tests that deleting a label +// via another app's URL path returns 404 (IDOR prevention). +func TestDeleteLabelOwnershipVerification(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + app1 := createTestApp(t, testCtx, "label-owner-app") + app2 := createTestApp(t, testCtx, "label-other-app") + + // Create label belonging to app1 + label := models.NewLabel(testCtx.database) + label.AppID = app1.ID + label.Key = "traefik.enable" + label.Value = "true" + require.NoError(t, label.Save(context.Background())) + + // Try to delete app1's label using app2's URL path + request := httptest.NewRequest( + http.MethodPost, + "/apps/"+app2.ID+"/labels/"+strconv.FormatInt(label.ID, 10)+"/delete", + nil, + ) + request = addChiURLParams(request, map[string]string{ + "id": app2.ID, + "labelID": strconv.FormatInt(label.ID, 10), + }) + recorder := httptest.NewRecorder() + + handler := testCtx.handlers.HandleLabelDelete() + handler.ServeHTTP(recorder, request) + + assert.Equal(t, http.StatusNotFound, recorder.Code) + + // Verify the label was NOT deleted + found, err := models.FindLabel(context.Background(), testCtx.database, label.ID) + require.NoError(t, err) + assert.NotNil(t, found, "label should still exist after IDOR attempt") +} + +// TestDeleteVolumeOwnershipVerification tests that deleting a volume +// via another app's URL path returns 404 (IDOR prevention). +func TestDeleteVolumeOwnershipVerification(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + app1 := createTestApp(t, testCtx, "volume-owner-app") + app2 := createTestApp(t, testCtx, "volume-other-app") + + // Create volume belonging to app1 + volume := models.NewVolume(testCtx.database) + volume.AppID = app1.ID + volume.HostPath = "/data/app1" + volume.ContainerPath = "/app/data" + volume.ReadOnly = false + require.NoError(t, volume.Save(context.Background())) + + // Try to delete app1's volume using app2's URL path + request := httptest.NewRequest( + http.MethodPost, + "/apps/"+app2.ID+"/volumes/"+strconv.FormatInt(volume.ID, 10)+"/delete", + nil, + ) + request = addChiURLParams(request, map[string]string{ + "id": app2.ID, + "volumeID": strconv.FormatInt(volume.ID, 10), + }) + recorder := httptest.NewRecorder() + + handler := testCtx.handlers.HandleVolumeDelete() + handler.ServeHTTP(recorder, request) + + assert.Equal(t, http.StatusNotFound, recorder.Code) + + // Verify the volume was NOT deleted + found, err := models.FindVolume(context.Background(), testCtx.database, volume.ID) + require.NoError(t, err) + assert.NotNil(t, found, "volume should still exist after IDOR attempt") +} + +// TestDeletePortOwnershipVerification tests that deleting a port +// via another app's URL path returns 404 (IDOR prevention). +func TestDeletePortOwnershipVerification(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + app1 := createTestApp(t, testCtx, "port-owner-app") + app2 := createTestApp(t, testCtx, "port-other-app") + + // Create port belonging to app1 + port := models.NewPort(testCtx.database) + port.AppID = app1.ID + port.HostPort = 8080 + port.ContainerPort = 80 + port.Protocol = models.PortProtocolTCP + require.NoError(t, port.Save(context.Background())) + + // Try to delete app1's port using app2's URL path + request := httptest.NewRequest( + http.MethodPost, + "/apps/"+app2.ID+"/ports/"+strconv.FormatInt(port.ID, 10)+"/delete", + nil, + ) + request = addChiURLParams(request, map[string]string{ + "id": app2.ID, + "portID": strconv.FormatInt(port.ID, 10), + }) + recorder := httptest.NewRecorder() + + handler := testCtx.handlers.HandlePortDelete() + handler.ServeHTTP(recorder, request) + + assert.Equal(t, http.StatusNotFound, recorder.Code) + + // Verify the port was NOT deleted + found, err := models.FindPort(context.Background(), testCtx.database, port.ID) + require.NoError(t, err) + assert.NotNil(t, found, "port should still exist after IDOR attempt") +} + func TestHandleWebhookReturns404ForUnknownSecret(t *testing.T) { t.Parallel()