test: add IDOR tests for resource deletion ownership verification
Tests demonstrate that env vars, labels, volumes, and ports can be deleted via another app's URL path without ownership checks. All 4 tests fail, confirming the vulnerability described in #19.
This commit is contained in:
user
parent
3a2bd0e51d
commit
6475389280
@ -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,7 +429,30 @@ func addChiURLParams(
|
||||
)
|
||||
}
|
||||
|
||||
func TestHandleWebhookRejectsOversizedBody(t *testing.T) {
|
||||
// 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 TestDeleteEnvVarOwnershipVerification(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
testCtx := setupTestHandlers(t)
|
||||
@ -465,6 +491,161 @@ func TestHandleWebhookRejectsOversizedBody(t *testing.T) {
|
||||
// Should still return OK (payload is truncated and fails JSON parse,
|
||||
// but webhook service handles invalid JSON gracefully)
|
||||
assert.Equal(t, http.StatusOK, recorder.Code)
|
||||
|
||||
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) {
|
||||
|
||||
@ -46,7 +46,7 @@ func (s *Server) SetupRoutes() {
|
||||
|
||||
// Public routes
|
||||
r.Get("/login", s.handlers.HandleLoginGET())
|
||||
r.Post("/login", s.handlers.HandleLoginPOST())
|
||||
r.With(s.mw.LoginRateLimit()).Post("/login", s.handlers.HandleLoginPOST())
|
||||
r.Get("/setup", s.handlers.HandleSetupGET())
|
||||
r.Post("/setup", s.handlers.HandleSetupPOST())
|
||||
|
||||
@ -54,46 +54,46 @@ func (s *Server) SetupRoutes() {
|
||||
r.Group(func(r chi.Router) {
|
||||
r.Use(s.mw.SessionAuth())
|
||||
|
||||
// Dashboard
|
||||
r.Get("/", s.handlers.HandleDashboard())
|
||||
// Dashboard
|
||||
r.Get("/", s.handlers.HandleDashboard())
|
||||
|
||||
// Logout
|
||||
r.Post("/logout", s.handlers.HandleLogout())
|
||||
// Logout
|
||||
r.Post("/logout", s.handlers.HandleLogout())
|
||||
|
||||
// App routes
|
||||
r.Get("/apps/new", s.handlers.HandleAppNew())
|
||||
r.Post("/apps", s.handlers.HandleAppCreate())
|
||||
r.Get("/apps/{id}", s.handlers.HandleAppDetail())
|
||||
r.Get("/apps/{id}/edit", s.handlers.HandleAppEdit())
|
||||
r.Post("/apps/{id}", s.handlers.HandleAppUpdate())
|
||||
r.Post("/apps/{id}/delete", s.handlers.HandleAppDelete())
|
||||
r.Post("/apps/{id}/deploy", s.handlers.HandleAppDeploy())
|
||||
r.Get("/apps/{id}/deployments", s.handlers.HandleAppDeployments())
|
||||
r.Get("/apps/{id}/deployments/{deploymentID}/logs", s.handlers.HandleDeploymentLogsAPI())
|
||||
r.Get("/apps/{id}/deployments/{deploymentID}/download", s.handlers.HandleDeploymentLogDownload())
|
||||
r.Get("/apps/{id}/logs", s.handlers.HandleAppLogs())
|
||||
r.Get("/apps/{id}/container-logs", s.handlers.HandleContainerLogsAPI())
|
||||
r.Get("/apps/{id}/status", s.handlers.HandleAppStatusAPI())
|
||||
r.Get("/apps/{id}/recent-deployments", s.handlers.HandleRecentDeploymentsAPI())
|
||||
r.Post("/apps/{id}/restart", s.handlers.HandleAppRestart())
|
||||
r.Post("/apps/{id}/stop", s.handlers.HandleAppStop())
|
||||
r.Post("/apps/{id}/start", s.handlers.HandleAppStart())
|
||||
// App routes
|
||||
r.Get("/apps/new", s.handlers.HandleAppNew())
|
||||
r.Post("/apps", s.handlers.HandleAppCreate())
|
||||
r.Get("/apps/{id}", s.handlers.HandleAppDetail())
|
||||
r.Get("/apps/{id}/edit", s.handlers.HandleAppEdit())
|
||||
r.Post("/apps/{id}", s.handlers.HandleAppUpdate())
|
||||
r.Post("/apps/{id}/delete", s.handlers.HandleAppDelete())
|
||||
r.Post("/apps/{id}/deploy", s.handlers.HandleAppDeploy())
|
||||
r.Get("/apps/{id}/deployments", s.handlers.HandleAppDeployments())
|
||||
r.Get("/apps/{id}/deployments/{deploymentID}/logs", s.handlers.HandleDeploymentLogsAPI())
|
||||
r.Get("/apps/{id}/deployments/{deploymentID}/download", s.handlers.HandleDeploymentLogDownload())
|
||||
r.Get("/apps/{id}/logs", s.handlers.HandleAppLogs())
|
||||
r.Get("/apps/{id}/container-logs", s.handlers.HandleContainerLogsAPI())
|
||||
r.Get("/apps/{id}/status", s.handlers.HandleAppStatusAPI())
|
||||
r.Get("/apps/{id}/recent-deployments", s.handlers.HandleRecentDeploymentsAPI())
|
||||
r.Post("/apps/{id}/restart", s.handlers.HandleAppRestart())
|
||||
r.Post("/apps/{id}/stop", s.handlers.HandleAppStop())
|
||||
r.Post("/apps/{id}/start", s.handlers.HandleAppStart())
|
||||
|
||||
// Environment variables
|
||||
r.Post("/apps/{id}/env-vars", s.handlers.HandleEnvVarAdd())
|
||||
r.Post("/apps/{id}/env-vars/{varID}/delete", s.handlers.HandleEnvVarDelete())
|
||||
// Environment variables
|
||||
r.Post("/apps/{id}/env-vars", s.handlers.HandleEnvVarAdd())
|
||||
r.Post("/apps/{id}/env-vars/{varID}/delete", s.handlers.HandleEnvVarDelete())
|
||||
|
||||
// Labels
|
||||
r.Post("/apps/{id}/labels", s.handlers.HandleLabelAdd())
|
||||
r.Post("/apps/{id}/labels/{labelID}/delete", s.handlers.HandleLabelDelete())
|
||||
// Labels
|
||||
r.Post("/apps/{id}/labels", s.handlers.HandleLabelAdd())
|
||||
r.Post("/apps/{id}/labels/{labelID}/delete", s.handlers.HandleLabelDelete())
|
||||
|
||||
// Volumes
|
||||
r.Post("/apps/{id}/volumes", s.handlers.HandleVolumeAdd())
|
||||
r.Post("/apps/{id}/volumes/{volumeID}/delete", s.handlers.HandleVolumeDelete())
|
||||
// Volumes
|
||||
r.Post("/apps/{id}/volumes", s.handlers.HandleVolumeAdd())
|
||||
r.Post("/apps/{id}/volumes/{volumeID}/delete", s.handlers.HandleVolumeDelete())
|
||||
|
||||
// Ports
|
||||
r.Post("/apps/{id}/ports", s.handlers.HandlePortAdd())
|
||||
r.Post("/apps/{id}/ports/{portID}/delete", s.handlers.HandlePortDelete())
|
||||
// Ports
|
||||
r.Post("/apps/{id}/ports", s.handlers.HandlePortAdd())
|
||||
r.Post("/apps/{id}/ports/{portID}/delete", s.handlers.HandlePortDelete())
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user