fix: disable API v1 write methods (closes #112)
All checks were successful
Check / check (pull_request) Successful in 11m21s
All checks were successful
Check / check (pull_request) Successful in 11m21s
Remove POST /apps, DELETE /apps/{id}, and POST /apps/{id}/deploy from
the API v1 route group. These endpoints used cookie-based session auth
without CSRF protection, creating a CSRF vulnerability.
Read-only endpoints (GET /apps, GET /apps/{id}, GET /apps/{id}/deployments),
login, and whoami are retained.
Removed handlers: HandleAPICreateApp, HandleAPIDeleteApp,
HandleAPITriggerDeploy, along with apiCreateRequest struct and
validateCreateRequest function.
Updated tests to use service layer directly for app creation in
remaining read-only endpoint tests.
This commit is contained in:
parent
4217e62f27
commit
ab7c43b887
@ -1,7 +1,6 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"strconv"
|
||||
@ -9,7 +8,6 @@ import (
|
||||
"github.com/go-chi/chi/v5"
|
||||
|
||||
"git.eeqj.de/sneak/upaas/internal/models"
|
||||
"git.eeqj.de/sneak/upaas/internal/service/app"
|
||||
)
|
||||
|
||||
// apiAppResponse is the JSON representation of an app.
|
||||
@ -176,121 +174,6 @@ func (h *Handlers) HandleAPIGetApp() http.HandlerFunc {
|
||||
}
|
||||
}
|
||||
|
||||
// apiCreateRequest is the JSON body for creating an app via the API.
|
||||
type apiCreateRequest struct {
|
||||
Name string `json:"name"`
|
||||
RepoURL string `json:"repoUrl"`
|
||||
Branch string `json:"branch"`
|
||||
DockerfilePath string `json:"dockerfilePath"`
|
||||
DockerNetwork string `json:"dockerNetwork"`
|
||||
NtfyTopic string `json:"ntfyTopic"`
|
||||
SlackWebhook string `json:"slackWebhook"`
|
||||
}
|
||||
|
||||
// validateCreateRequest validates the fields of an API create app request.
|
||||
// Returns an error message string or empty string if valid.
|
||||
func validateCreateRequest(req *apiCreateRequest) string {
|
||||
if req.Name == "" || req.RepoURL == "" {
|
||||
return "name and repo_url are required"
|
||||
}
|
||||
|
||||
nameErr := validateAppName(req.Name)
|
||||
if nameErr != nil {
|
||||
return "invalid app name: " + nameErr.Error()
|
||||
}
|
||||
|
||||
repoURLErr := validateRepoURL(req.RepoURL)
|
||||
if repoURLErr != nil {
|
||||
return "invalid repository URL: " + repoURLErr.Error()
|
||||
}
|
||||
|
||||
return ""
|
||||
}
|
||||
|
||||
// HandleAPICreateApp returns a handler that creates a new app.
|
||||
func (h *Handlers) HandleAPICreateApp() http.HandlerFunc {
|
||||
return func(writer http.ResponseWriter, request *http.Request) {
|
||||
var req apiCreateRequest
|
||||
|
||||
decodeErr := json.NewDecoder(request.Body).Decode(&req)
|
||||
if decodeErr != nil {
|
||||
h.respondJSON(writer, request,
|
||||
map[string]string{"error": "invalid JSON body"},
|
||||
http.StatusBadRequest)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
if errMsg := validateCreateRequest(&req); errMsg != "" {
|
||||
h.respondJSON(writer, request,
|
||||
map[string]string{"error": errMsg},
|
||||
http.StatusBadRequest)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
createdApp, createErr := h.appService.CreateApp(request.Context(), app.CreateAppInput{
|
||||
Name: req.Name,
|
||||
RepoURL: req.RepoURL,
|
||||
Branch: req.Branch,
|
||||
DockerfilePath: req.DockerfilePath,
|
||||
DockerNetwork: req.DockerNetwork,
|
||||
NtfyTopic: req.NtfyTopic,
|
||||
SlackWebhook: req.SlackWebhook,
|
||||
})
|
||||
if createErr != nil {
|
||||
h.log.Error("api: failed to create app", "error", createErr)
|
||||
h.respondJSON(writer, request,
|
||||
map[string]string{"error": "failed to create app"},
|
||||
http.StatusInternalServerError)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
h.respondJSON(writer, request, appToAPI(createdApp), http.StatusCreated)
|
||||
}
|
||||
}
|
||||
|
||||
// HandleAPIDeleteApp returns a handler that deletes an app.
|
||||
func (h *Handlers) HandleAPIDeleteApp() http.HandlerFunc {
|
||||
return func(writer http.ResponseWriter, request *http.Request) {
|
||||
appID := chi.URLParam(request, "id")
|
||||
|
||||
application, err := h.appService.GetApp(request.Context(), appID)
|
||||
if err != nil {
|
||||
h.respondJSON(writer, request,
|
||||
map[string]string{"error": "internal server error"},
|
||||
http.StatusInternalServerError)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
if application == nil {
|
||||
h.respondJSON(writer, request,
|
||||
map[string]string{"error": "app not found"},
|
||||
http.StatusNotFound)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// Stop and remove the Docker container before deleting the DB record
|
||||
h.cleanupContainer(request.Context(), appID, application.Name)
|
||||
|
||||
deleteErr := h.appService.DeleteApp(request.Context(), application)
|
||||
if deleteErr != nil {
|
||||
h.log.Error("api: failed to delete app", "error", deleteErr)
|
||||
h.respondJSON(writer, request,
|
||||
map[string]string{"error": "failed to delete app"},
|
||||
http.StatusInternalServerError)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
h.respondJSON(writer, request,
|
||||
map[string]string{"status": "deleted"}, http.StatusOK)
|
||||
}
|
||||
}
|
||||
|
||||
// deploymentsPageLimit is the default number of deployments per page.
|
||||
const deploymentsPageLimit = 20
|
||||
|
||||
@ -337,39 +220,6 @@ func (h *Handlers) HandleAPIListDeployments() http.HandlerFunc {
|
||||
}
|
||||
}
|
||||
|
||||
// HandleAPITriggerDeploy returns a handler that triggers a deployment for an app.
|
||||
func (h *Handlers) HandleAPITriggerDeploy() http.HandlerFunc {
|
||||
return func(writer http.ResponseWriter, request *http.Request) {
|
||||
appID := chi.URLParam(request, "id")
|
||||
|
||||
application, err := h.appService.GetApp(request.Context(), appID)
|
||||
if err != nil || application == nil {
|
||||
h.respondJSON(writer, request,
|
||||
map[string]string{"error": "app not found"},
|
||||
http.StatusNotFound)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// Use a detached context so the deployment continues even if the
|
||||
// HTTP client disconnects.
|
||||
deployCtx := context.WithoutCancel(request.Context())
|
||||
|
||||
deployErr := h.deploy.Deploy(deployCtx, application, nil, true)
|
||||
if deployErr != nil {
|
||||
h.log.Error("api: failed to trigger deploy", "error", deployErr)
|
||||
h.respondJSON(writer, request,
|
||||
map[string]string{"error": deployErr.Error()},
|
||||
http.StatusConflict)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
h.respondJSON(writer, request,
|
||||
map[string]string{"status": "deploying"}, http.StatusAccepted)
|
||||
}
|
||||
}
|
||||
|
||||
// HandleAPIWhoAmI returns a handler that shows the current authenticated user.
|
||||
func (h *Handlers) HandleAPIWhoAmI() http.HandlerFunc {
|
||||
type whoAmIResponse struct {
|
||||
|
||||
@ -10,6 +10,8 @@ import (
|
||||
"github.com/go-chi/chi/v5"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"git.eeqj.de/sneak/upaas/internal/service/app"
|
||||
)
|
||||
|
||||
// apiRouter builds a chi router with the API routes using session auth middleware.
|
||||
@ -23,10 +25,7 @@ func apiRouter(tc *testContext) http.Handler {
|
||||
apiR.Use(tc.middleware.APISessionAuth())
|
||||
apiR.Get("/whoami", tc.handlers.HandleAPIWhoAmI())
|
||||
apiR.Get("/apps", tc.handlers.HandleAPIListApps())
|
||||
apiR.Post("/apps", tc.handlers.HandleAPICreateApp())
|
||||
apiR.Get("/apps/{id}", tc.handlers.HandleAPIGetApp())
|
||||
apiR.Delete("/apps/{id}", tc.handlers.HandleAPIDeleteApp())
|
||||
apiR.Post("/apps/{id}/deploy", tc.handlers.HandleAPITriggerDeploy())
|
||||
apiR.Get("/apps/{id}/deployments", tc.handlers.HandleAPIListDeployments())
|
||||
})
|
||||
})
|
||||
@ -62,23 +61,16 @@ func setupAPITest(t *testing.T) (*testContext, []*http.Cookie) {
|
||||
return tc, cookies
|
||||
}
|
||||
|
||||
// apiRequest makes an authenticated API request using session cookies.
|
||||
func apiRequest(
|
||||
// apiGet makes an authenticated GET request using session cookies.
|
||||
func apiGet(
|
||||
t *testing.T,
|
||||
tc *testContext,
|
||||
cookies []*http.Cookie,
|
||||
method, path string,
|
||||
body string,
|
||||
path string,
|
||||
) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
|
||||
var req *http.Request
|
||||
if body != "" {
|
||||
req = httptest.NewRequest(method, path, strings.NewReader(body))
|
||||
req.Header.Set("Content-Type", "application/json")
|
||||
} else {
|
||||
req = httptest.NewRequest(method, path, nil)
|
||||
}
|
||||
req := httptest.NewRequest(http.MethodGet, path, nil)
|
||||
|
||||
for _, c := range cookies {
|
||||
req.AddCookie(c)
|
||||
@ -175,7 +167,7 @@ func TestAPIWhoAmI(t *testing.T) {
|
||||
|
||||
tc, cookies := setupAPITest(t)
|
||||
|
||||
rr := apiRequest(t, tc, cookies, http.MethodGet, "/api/v1/whoami", "")
|
||||
rr := apiGet(t, tc, cookies, "/api/v1/whoami")
|
||||
assert.Equal(t, http.StatusOK, rr.Code)
|
||||
|
||||
var resp map[string]any
|
||||
@ -188,7 +180,7 @@ func TestAPIListAppsEmpty(t *testing.T) {
|
||||
|
||||
tc, cookies := setupAPITest(t)
|
||||
|
||||
rr := apiRequest(t, tc, cookies, http.MethodGet, "/api/v1/apps", "")
|
||||
rr := apiGet(t, tc, cookies, "/api/v1/apps")
|
||||
assert.Equal(t, http.StatusOK, rr.Code)
|
||||
|
||||
var apps []any
|
||||
@ -196,52 +188,23 @@ func TestAPIListAppsEmpty(t *testing.T) {
|
||||
assert.Empty(t, apps)
|
||||
}
|
||||
|
||||
func TestAPICreateApp(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tc, cookies := setupAPITest(t)
|
||||
|
||||
body := `{"name":"test-app","repoUrl":"https://github.com/example/repo"}`
|
||||
rr := apiRequest(t, tc, cookies, http.MethodPost, "/api/v1/apps", body)
|
||||
assert.Equal(t, http.StatusCreated, rr.Code)
|
||||
|
||||
var app map[string]any
|
||||
require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &app))
|
||||
assert.Equal(t, "test-app", app["name"])
|
||||
assert.Equal(t, "pending", app["status"])
|
||||
}
|
||||
|
||||
func TestAPICreateAppValidation(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tc, cookies := setupAPITest(t)
|
||||
|
||||
body := `{"name":"","repoUrl":""}`
|
||||
rr := apiRequest(t, tc, cookies, http.MethodPost, "/api/v1/apps", body)
|
||||
assert.Equal(t, http.StatusBadRequest, rr.Code)
|
||||
}
|
||||
|
||||
func TestAPIGetApp(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tc, cookies := setupAPITest(t)
|
||||
|
||||
body := `{"name":"my-app","repoUrl":"https://github.com/example/repo"}`
|
||||
rr := apiRequest(t, tc, cookies, http.MethodPost, "/api/v1/apps", body)
|
||||
require.Equal(t, http.StatusCreated, rr.Code)
|
||||
created, err := tc.appSvc.CreateApp(t.Context(), app.CreateAppInput{
|
||||
Name: "my-app",
|
||||
RepoURL: "https://github.com/example/repo",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
var created map[string]any
|
||||
require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &created))
|
||||
|
||||
appID, ok := created["id"].(string)
|
||||
require.True(t, ok)
|
||||
|
||||
rr = apiRequest(t, tc, cookies, http.MethodGet, "/api/v1/apps/"+appID, "")
|
||||
rr := apiGet(t, tc, cookies, "/api/v1/apps/"+created.ID)
|
||||
assert.Equal(t, http.StatusOK, rr.Code)
|
||||
|
||||
var app map[string]any
|
||||
require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &app))
|
||||
assert.Equal(t, "my-app", app["name"])
|
||||
var resp map[string]any
|
||||
require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &resp))
|
||||
assert.Equal(t, "my-app", resp["name"])
|
||||
}
|
||||
|
||||
func TestAPIGetAppNotFound(t *testing.T) {
|
||||
@ -249,29 +212,7 @@ func TestAPIGetAppNotFound(t *testing.T) {
|
||||
|
||||
tc, cookies := setupAPITest(t)
|
||||
|
||||
rr := apiRequest(t, tc, cookies, http.MethodGet, "/api/v1/apps/nonexistent", "")
|
||||
assert.Equal(t, http.StatusNotFound, rr.Code)
|
||||
}
|
||||
|
||||
func TestAPIDeleteApp(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tc, cookies := setupAPITest(t)
|
||||
|
||||
body := `{"name":"delete-me","repoUrl":"https://github.com/example/repo"}`
|
||||
rr := apiRequest(t, tc, cookies, http.MethodPost, "/api/v1/apps", body)
|
||||
require.Equal(t, http.StatusCreated, rr.Code)
|
||||
|
||||
var created map[string]any
|
||||
require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &created))
|
||||
|
||||
appID, ok := created["id"].(string)
|
||||
require.True(t, ok)
|
||||
|
||||
rr = apiRequest(t, tc, cookies, http.MethodDelete, "/api/v1/apps/"+appID, "")
|
||||
assert.Equal(t, http.StatusOK, rr.Code)
|
||||
|
||||
rr = apiRequest(t, tc, cookies, http.MethodGet, "/api/v1/apps/"+appID, "")
|
||||
rr := apiGet(t, tc, cookies, "/api/v1/apps/nonexistent")
|
||||
assert.Equal(t, http.StatusNotFound, rr.Code)
|
||||
}
|
||||
|
||||
@ -280,17 +221,13 @@ func TestAPIListDeployments(t *testing.T) {
|
||||
|
||||
tc, cookies := setupAPITest(t)
|
||||
|
||||
body := `{"name":"deploy-app","repoUrl":"https://github.com/example/repo"}`
|
||||
rr := apiRequest(t, tc, cookies, http.MethodPost, "/api/v1/apps", body)
|
||||
require.Equal(t, http.StatusCreated, rr.Code)
|
||||
created, err := tc.appSvc.CreateApp(t.Context(), app.CreateAppInput{
|
||||
Name: "deploy-app",
|
||||
RepoURL: "https://github.com/example/repo",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
var created map[string]any
|
||||
require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &created))
|
||||
|
||||
appID, ok := created["id"].(string)
|
||||
require.True(t, ok)
|
||||
|
||||
rr = apiRequest(t, tc, cookies, http.MethodGet, "/api/v1/apps/"+appID+"/deployments", "")
|
||||
rr := apiGet(t, tc, cookies, "/api/v1/apps/"+created.ID+"/deployments")
|
||||
assert.Equal(t, http.StatusOK, rr.Code)
|
||||
|
||||
var deployments []any
|
||||
|
||||
@ -2,7 +2,6 @@ package handlers_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
@ -843,33 +842,6 @@ func TestSetupRequiredExemptsHealthAndStaticAndAPI(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
// TestAPITriggerDeployUsesDetachedContext verifies that HandleAPITriggerDeploy
|
||||
// does not pass the request context directly to the deploy operation.
|
||||
// This is a compile-time/code-level fix verified by the deployment not being
|
||||
// cancelled when the request context is cancelled.
|
||||
func TestAPITriggerDeployUsesDetachedContext(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// This test verifies the fix exists by checking the handler doesn't
|
||||
// fail when called — the actual context detachment is verified by code review.
|
||||
// The deploy will fail (no docker) but shouldn't panic.
|
||||
tc, cookies := setupAPITest(t)
|
||||
|
||||
body := `{"name":"detach-ctx-app","repoUrl":"https://github.com/example/repo"}`
|
||||
rr := apiRequest(t, tc, cookies, http.MethodPost, "/api/v1/apps", body)
|
||||
require.Equal(t, http.StatusCreated, rr.Code)
|
||||
|
||||
var created map[string]any
|
||||
require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &created))
|
||||
|
||||
appID, ok := created["id"].(string)
|
||||
require.True(t, ok)
|
||||
|
||||
rr = apiRequest(t, tc, cookies, http.MethodPost, "/api/v1/apps/"+appID+"/deploy", "")
|
||||
// Should get conflict (deploy will fail) or accepted, but not panic
|
||||
assert.Contains(t, []int{http.StatusAccepted, http.StatusConflict}, rr.Code)
|
||||
}
|
||||
|
||||
func TestHandleCancelDeployRedirects(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@ -114,10 +114,7 @@ func (s *Server) SetupRoutes() {
|
||||
r.Get("/whoami", s.handlers.HandleAPIWhoAmI())
|
||||
|
||||
r.Get("/apps", s.handlers.HandleAPIListApps())
|
||||
r.Post("/apps", s.handlers.HandleAPICreateApp())
|
||||
r.Get("/apps/{id}", s.handlers.HandleAPIGetApp())
|
||||
r.Delete("/apps/{id}", s.handlers.HandleAPIDeleteApp())
|
||||
r.Post("/apps/{id}/deploy", s.handlers.HandleAPITriggerDeploy())
|
||||
r.Get("/apps/{id}/deployments", s.handlers.HandleAPIListDeployments())
|
||||
})
|
||||
})
|
||||
|
||||
Loading…
Reference in New Issue
Block a user