Verify resource ownership before deletion to prevent IDOR (closes #3) #8

Closed
clawbot wants to merge 1 commits from (deleted):fix/issue-3 into main
2 changed files with 88 additions and 0 deletions
Showing only changes of commit 839d83ab8b - Show all commits

View File

@ -807,6 +807,13 @@ func (h *Handlers) HandleEnvVarDelete() http.HandlerFunc {
return return
} }
// Verify the env var belongs to this app
if envVar.AppID != appID {
http.NotFound(writer, request)
return
}
deleteErr := envVar.Delete(request.Context()) deleteErr := envVar.Delete(request.Context())
if deleteErr != nil { if deleteErr != nil {
h.log.Error("failed to delete env var", "error", deleteErr) h.log.Error("failed to delete env var", "error", deleteErr)
@ -854,6 +861,13 @@ func (h *Handlers) HandleLabelDelete() http.HandlerFunc {
return return
} }
// Verify the label belongs to this app
if label.AppID != appID {
http.NotFound(writer, request)
return
}
deleteErr := label.Delete(request.Context()) deleteErr := label.Delete(request.Context())
if deleteErr != nil { if deleteErr != nil {
h.log.Error("failed to delete label", "error", deleteErr) h.log.Error("failed to delete label", "error", deleteErr)
@ -932,6 +946,13 @@ func (h *Handlers) HandleVolumeDelete() http.HandlerFunc {
return return
} }
// Verify the volume belongs to this app
if volume.AppID != appID {
http.NotFound(writer, request)
return
}
deleteErr := volume.Delete(request.Context()) deleteErr := volume.Delete(request.Context())
if deleteErr != nil { if deleteErr != nil {
h.log.Error("failed to delete volume", "error", deleteErr) h.log.Error("failed to delete volume", "error", deleteErr)
@ -1022,6 +1043,13 @@ func (h *Handlers) HandlePortDelete() http.HandlerFunc {
return return
} }
// Verify the port belongs to this app
if port.AppID != appID {
http.NotFound(writer, request)
return
}
deleteErr := port.Delete(request.Context()) deleteErr := port.Delete(request.Context())
if deleteErr != nil { if deleteErr != nil {
h.log.Error("failed to delete port", "error", deleteErr) h.log.Error("failed to delete port", "error", deleteErr)

View File

@ -5,6 +5,7 @@ import (
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"net/url" "net/url"
"strconv"
"strings" "strings"
"testing" "testing"
"time" "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) { func TestHandleWebhookReturns404ForUnknownSecret(t *testing.T) {
t.Parallel() t.Parallel()