From 839d83ab8bdd4e8f694c9a96b8ed1f6b73596a4d Mon Sep 17 00:00:00 2001 From: clawbot Date: Sun, 8 Feb 2026 12:04:09 -0800 Subject: [PATCH] fix: verify resource ownership before deletion to prevent IDOR (closes #3) --- internal/handlers/app.go | 28 ++++++++++++++ internal/handlers/handlers_test.go | 60 ++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/internal/handlers/app.go b/internal/handlers/app.go index 2887139..cfbc56a 100644 --- a/internal/handlers/app.go +++ b/internal/handlers/app.go @@ -807,6 +807,13 @@ func (h *Handlers) HandleEnvVarDelete() http.HandlerFunc { return } + // Verify the env var belongs to this app + if envVar.AppID != appID { + http.NotFound(writer, request) + + return + } + deleteErr := envVar.Delete(request.Context()) if deleteErr != nil { h.log.Error("failed to delete env var", "error", deleteErr) @@ -854,6 +861,13 @@ func (h *Handlers) HandleLabelDelete() http.HandlerFunc { return } + // Verify the label belongs to this app + if label.AppID != appID { + http.NotFound(writer, request) + + return + } + deleteErr := label.Delete(request.Context()) if deleteErr != nil { h.log.Error("failed to delete label", "error", deleteErr) @@ -932,6 +946,13 @@ func (h *Handlers) HandleVolumeDelete() http.HandlerFunc { return } + // Verify the volume belongs to this app + if volume.AppID != appID { + http.NotFound(writer, request) + + return + } + deleteErr := volume.Delete(request.Context()) if deleteErr != nil { h.log.Error("failed to delete volume", "error", deleteErr) @@ -1022,6 +1043,13 @@ func (h *Handlers) HandlePortDelete() http.HandlerFunc { return } + // Verify the port belongs to this app + if port.AppID != appID { + http.NotFound(writer, request) + + return + } + deleteErr := port.Delete(request.Context()) if deleteErr != nil { h.log.Error("failed to delete port", "error", deleteErr) diff --git a/internal/handlers/handlers_test.go b/internal/handlers/handlers_test.go index adec558..780c906 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" @@ -426,6 +427,65 @@ func addChiURLParams( ) } +func TestHandleEnvVarDeleteRejectsWrongApp(t *testing.T) { + t.Parallel() + + testCtx := setupTestHandlers(t) + + // Create two apps + app1, err := testCtx.appSvc.CreateApp( + context.Background(), + app.CreateAppInput{ + Name: "app-owner", + RepoURL: "git@example.com:user/repo1.git", + }, + ) + require.NoError(t, err) + + app2, err := testCtx.appSvc.CreateApp( + context.Background(), + app.CreateAppInput{ + Name: "app-other", + RepoURL: "git@example.com:user/repo2.git", + }, + ) + require.NoError(t, err) + + // Add env var to app1 + err = testCtx.appSvc.AddEnvVar( + context.Background(), app1.ID, "SECRET", "value", + ) + require.NoError(t, err) + + envVars, err := app1.GetEnvVars(context.Background()) + require.NoError(t, err) + require.Len(t, envVars, 1) + + envVarID := strconv.FormatInt(envVars[0].ID, 10) + + // Try to delete app1's env var via app2's URL - should return 404 + request := httptest.NewRequest( + http.MethodPost, + "/apps/"+app2.ID+"/env-vars/"+envVarID+"/delete", + nil, + ) + request = addChiURLParams(request, map[string]string{ + "id": app2.ID, + "envID": envVarID, + }) + recorder := httptest.NewRecorder() + + handler := testCtx.handlers.HandleEnvVarDelete() + handler.ServeHTTP(recorder, request) + + assert.Equal(t, http.StatusNotFound, recorder.Code) + + // Verify env var was NOT deleted + envVars, err = app1.GetEnvVars(context.Background()) + require.NoError(t, err) + assert.Len(t, envVars, 1) +} + func TestHandleWebhookReturns404ForUnknownSecret(t *testing.T) { t.Parallel()