Pkce v2.18.25

.github/workflows/dev.yml

@@ -39,7 +39,7 @@ jobs:
 
       - name: Add PRO implementation
         run: |
-          git clone -b main https://${{ secrets.GH_TOKEN }}@github.com/semaphoreui/semaphorepro-module.git pro_impl
+          git clone -b 2-18-stable https://${{ secrets.GH_TOKEN }}@github.com/semaphoreui/semaphorepro-module.git pro_impl
           go work init . ./pro_impl
 
       - name: Run build

.github/workflows/pro_selfhosted_beta.yml

@@ -39,7 +39,7 @@ jobs:
 
       - name: Add PRO implementation
         run: |
-          git clone -b main https://${{ secrets.GH_TOKEN }}@github.com/semaphoreui/semaphorepro-module.git pro_impl
+          git clone -b 2-18-stable https://${{ secrets.GH_TOKEN }}@github.com/semaphoreui/semaphorepro-module.git pro_impl
           go work init . ./pro_impl
 
       - name: Install deps

.github/workflows/pro_selfhosted_release.yml

@@ -38,7 +38,7 @@ jobs:
 
       - name: Add PRO implementation
         run: |
-          git clone -b main https://${{ secrets.GH_TOKEN }}@github.com/semaphoreui/semaphorepro-module.git pro_impl
+          git clone -b 2-18-stable https://${{ secrets.GH_TOKEN }}@github.com/semaphoreui/semaphorepro-module.git pro_impl
           go work init . ./pro_impl
 
       - name: Install deps

api-docs.yml

@@ -855,6 +855,8 @@ definitions:
         type: array
         items:
           type: string
+      skip_galaxy_install:
+        type: boolean
 
   TerraformTaskParams:
     type: object

api/auth.go

@@ -3,6 +3,7 @@ package api
 import (
 	"errors"
 	"net/http"
+	"net/url"
 	"strings"
 	"time"
 
@@ -322,3 +323,90 @@ func adminMiddleware(next http.Handler) http.Handler {
 		next.ServeHTTP(w, r)
 	})
 }
+
+// isStateChangingMethod reports whether an HTTP method can modify server state
+// and therefore requires CSRF protection. Safe methods (GET, HEAD, OPTIONS,
+// TRACE) are excluded.
+func isStateChangingMethod(method string) bool {
+	switch method {
+	case http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete:
+		return true
+	default:
+		return false
+	}
+}
+
+// requestOriginHost extracts the origin host (host[:port]) of the request from
+// the Origin header, falling back to the Referer header. The boolean is false
+// when neither header is present or parseable.
+func requestOriginHost(r *http.Request) (string, bool) {
+	for _, header := range []string{"Origin", "Referer"} {
+		value := r.Header.Get(header)
+		if value == "" {
+			continue
+		}
+
+		u, err := url.Parse(value)
+		if err != nil || u.Host == "" {
+			continue
+		}
+
+		return u.Host, true
+	}
+
+	return "", false
+}
+
+// isSameOriginHost reports whether host belongs to Semaphore itself. Both the
+// configured public web host and the host the request was addressed to are
+// accepted, so reverse-proxy deployments keep working.
+func isSameOriginHost(host string, r *http.Request) bool {
+	if host == r.Host {
+		return true
+	}
+
+	if util.WebHostURL != nil && host == util.WebHostURL.Host {
+		return true
+	}
+
+	return false
+}
+
+// csrfProtectionMiddleware blocks cross-site state-changing requests that rely
+// on the session cookie, providing defense-in-depth against CSRF on top of the
+// SameSite=Lax session cookie.
+//
+// Requests authenticated with an API token (Authorization: bearer) are exempt:
+// browsers never attach such tokens automatically, so token-based clients are
+// not vulnerable to CSRF and must keep working without an Origin header.
+//
+// When neither Origin nor Referer is present (e.g. non-browser clients using a
+// cookie), the request is allowed — the SameSite=Lax cookie already prevents a
+// browser from sending the session cookie cross-site in that case.
+func csrfProtectionMiddleware(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if !isStateChangingMethod(r.Method) {
+			next.ServeHTTP(w, r)
+			return
+		}
+
+		authHeader := strings.ToLower(r.Header.Get("authorization"))
+		if strings.Contains(authHeader, "bearer") {
+			next.ServeHTTP(w, r)
+			return
+		}
+
+		if origin, ok := requestOriginHost(r); ok && !isSameOriginHost(origin, r) {
+			log.WithFields(log.Fields{
+				"origin": origin,
+				"host":   r.Host,
+				"path":   r.URL.Path,
+				"method": r.Method,
+			}).Warn("Blocked cross-origin request (possible CSRF)")
+			helpers.WriteErrorStatus(w, "CROSS_ORIGIN_REQUEST_BLOCKED", http.StatusForbidden)
+			return
+		}
+
+		next.ServeHTTP(w, r)
+	})
+}

api/auth_test.go

@@ -0,0 +1,189 @@
+package api
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+
+	"github.com/semaphoreui/semaphore/util"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestIsStateChangingMethod(t *testing.T) {
+	tests := []struct {
+		method   string
+		expected bool
+	}{
+		{http.MethodGet, false},
+		{http.MethodHead, false},
+		{http.MethodOptions, false},
+		{http.MethodPost, true},
+		{http.MethodPut, true},
+		{http.MethodPatch, true},
+		{http.MethodDelete, true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.method, func(t *testing.T) {
+			assert.Equal(t, tt.expected, isStateChangingMethod(tt.method))
+		})
+	}
+}
+
+func TestRequestOriginHost(t *testing.T) {
+	t.Run("from Origin header", func(t *testing.T) {
+		r := httptest.NewRequest(http.MethodPost, "/api/users/1/password", nil)
+		r.Header.Set("Origin", "https://semaphore.example.com")
+
+		host, ok := requestOriginHost(r)
+		assert.True(t, ok)
+		assert.Equal(t, "semaphore.example.com", host)
+	})
+
+	t.Run("falls back to Referer", func(t *testing.T) {
+		r := httptest.NewRequest(http.MethodPost, "/api/users/1/password", nil)
+		r.Header.Set("Referer", "https://semaphore.example.com/project/1")
+
+		host, ok := requestOriginHost(r)
+		assert.True(t, ok)
+		assert.Equal(t, "semaphore.example.com", host)
+	})
+
+	t.Run("Origin takes precedence over Referer", func(t *testing.T) {
+		r := httptest.NewRequest(http.MethodPost, "/api/users/1/password", nil)
+		r.Header.Set("Origin", "https://attacker.com")
+		r.Header.Set("Referer", "https://semaphore.example.com/")
+
+		host, ok := requestOriginHost(r)
+		assert.True(t, ok)
+		assert.Equal(t, "attacker.com", host)
+	})
+
+	t.Run("no headers", func(t *testing.T) {
+		r := httptest.NewRequest(http.MethodPost, "/api/users/1/password", nil)
+
+		_, ok := requestOriginHost(r)
+		assert.False(t, ok)
+	})
+}
+
+// newRecordingHandler returns an http.Handler that records whether it was
+// called, used to assert that the middleware did or did not pass the request
+// through.
+func newRecordingHandler(called *bool) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		*called = true
+		w.WriteHeader(http.StatusNoContent)
+	})
+}
+
+func TestCsrfProtectionMiddleware(t *testing.T) {
+	orig := util.WebHostURL
+	defer func() { util.WebHostURL = orig }()
+
+	webHost, err := url.Parse("https://semaphore.example.com")
+	require.NoError(t, err)
+	util.WebHostURL = webHost
+
+	tests := []struct {
+		name         string
+		method       string
+		host         string
+		origin       string
+		referer      string
+		authHeader   string
+		wantStatus   int
+		wantForwPass bool
+	}{
+		{
+			name:         "safe method is always allowed",
+			method:       http.MethodGet,
+			host:         "semaphore.example.com",
+			origin:       "https://attacker.com",
+			wantStatus:   http.StatusNoContent,
+			wantForwPass: true,
+		},
+		{
+			name:         "same origin POST is allowed",
+			method:       http.MethodPost,
+			host:         "semaphore.example.com",
+			origin:       "https://semaphore.example.com",
+			wantStatus:   http.StatusNoContent,
+			wantForwPass: true,
+		},
+		{
+			name:         "cross origin POST is blocked",
+			method:       http.MethodPost,
+			host:         "semaphore.example.com",
+			origin:       "https://attacker.com",
+			wantStatus:   http.StatusForbidden,
+			wantForwPass: false,
+		},
+		{
+			name:         "cross origin DELETE is blocked",
+			method:       http.MethodDelete,
+			host:         "semaphore.example.com",
+			origin:       "https://attacker.com:1337",
+			wantStatus:   http.StatusForbidden,
+			wantForwPass: false,
+		},
+		{
+			name:         "cross origin via Referer is blocked",
+			method:       http.MethodPost,
+			host:         "semaphore.example.com",
+			referer:      "https://attacker.com/evil",
+			wantStatus:   http.StatusForbidden,
+			wantForwPass: false,
+		},
+		{
+			name:         "missing origin and referer is allowed",
+			method:       http.MethodPost,
+			host:         "semaphore.example.com",
+			wantStatus:   http.StatusNoContent,
+			wantForwPass: true,
+		},
+		{
+			name:         "bearer token bypasses origin check",
+			method:       http.MethodPost,
+			host:         "semaphore.example.com",
+			origin:       "https://attacker.com",
+			authHeader:   "Bearer sometoken",
+			wantStatus:   http.StatusNoContent,
+			wantForwPass: true,
+		},
+		{
+			name:         "origin matching request host is allowed",
+			method:       http.MethodPost,
+			host:         "internal-proxy:3000",
+			origin:       "http://internal-proxy:3000",
+			wantStatus:   http.StatusNoContent,
+			wantForwPass: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r := httptest.NewRequest(tt.method, "/api/users/1/password", nil)
+			r.Host = tt.host
+			if tt.origin != "" {
+				r.Header.Set("Origin", tt.origin)
+			}
+			if tt.referer != "" {
+				r.Header.Set("Referer", tt.referer)
+			}
+			if tt.authHeader != "" {
+				r.Header.Set("Authorization", tt.authHeader)
+			}
+
+			var forwarded bool
+			w := httptest.NewRecorder()
+
+			csrfProtectionMiddleware(newRecordingHandler(&forwarded)).ServeHTTP(w, r)
+
+			assert.Equal(t, tt.wantStatus, w.Code)
+			assert.Equal(t, tt.wantForwPass, forwarded)
+		})
+	}
+}