Enforce webhook project and receiver authorization boundaries

.github/release-notes/v0.10.2.md

@@ -0,0 +1,9 @@
+# Webhook Automation Service v0.10.2 candidate
+
+This candidate builds on the published v0.10.1 release. It is not yet a published release.
+
+- Require the control-plane project header to match the requested project on receiver management routes, and enforce read-only roles in comma-separated role lists.
+- Restrict receiver listing, lookup, deletion, name checks, and key or JWT execution validation to `webhookReceiver` generic objects.
+- Add regression tests for project boundaries, unrelated generic-object kinds, and valid receiver execution.
+
+The candidate must pass the security release gate and isolated-VM acceptance checks before publication. The Server 8080 QA path has not been claimed as verified by this note.

.github/workflows/security-release-gate.yml

@@ -19,7 +19,7 @@ jobs:
     runs-on: ubuntu-24.04
     timeout-minutes: 90
     env:
-      CANDIDATE_VERSION: 0.10.1
+      CANDIDATE_VERSION: 0.10.2
       GO_VERSION: 1.27.0
       GO_LINUX_AMD64_SHA256: 675c26c449cbb18fc24b74650de1eabbae6e16f64326fd85a283fb3b58280685
       TRIVY_IMAGE: aquasec/trivy:0.74.0@sha256:62b1e65e8869bc4b4c6aa4fa2b21595256c7c2f6018a9d9ad61caf87187c1969

README.md

@@ -10,6 +10,7 @@ PastureStack is an independent community effort to preserve, audit, and moderniz
 
 The current public GitHub Release is
 [`v0.10.1`](https://github.com/PastureStack/webhook-automation-service/releases/tag/v0.10.1).
+The next candidate is `v0.10.2`; see the [candidate release notes](.github/release-notes/v0.10.2.md).
 
 ## Runtime boundary
 
@@ -32,7 +33,7 @@ make build
 make integration-test
 ```
 
-`make package` creates the deterministic flat GitHub Release asset `webhook-automation-service-0.10.1-linux-amd64.tar.xz`. The archive contains the executable plus compatibility, source, notice, and composite license files. The Server release verifies its SHA-256 digest before installation, so operators do not need an artifact mirror.
+`make package` creates the deterministic flat candidate asset `webhook-automation-service-0.10.2-linux-amd64.tar.xz`. The archive contains the executable plus compatibility, source, notice, and composite license files. The Server release verifies its SHA-256 digest before installation, so operators do not need an artifact mirror.
 
 See [COMPATIBILITY.md](COMPATIBILITY.md), [ORIGIN.md](ORIGIN.md), [MODIFICATIONS.md](MODIFICATIONS.md), and [THIRD-PARTY-NOTICES.md](THIRD-PARTY-NOTICES.md).
 

scripts/integration-test

@@ -33,7 +33,8 @@ RSA_PRIVATE_KEY_CONTENTS='must-not-be-read-or-logged' \
 service_pid=$!
 
 for _ in $(seq 1 100); do
-    if curl --silent --fail "http://${listen_address}/v1-webhooks?projectId=1a1" >"${work_root}/version.json"; then
+    if curl --silent --fail -H 'X-API-Project-Id: 1a1' \
+        "http://${listen_address}/v1-webhooks?projectId=1a1" >"${work_root}/version.json"; then
         break
     fi
     if ! kill -0 "${service_pid}" 2>/dev/null; then
@@ -44,6 +45,12 @@ for _ in $(seq 1 100); do
 done
 grep -F 'v1-webhooks' "${work_root}/version.json" >/dev/null
 
+# A directly reached management route must not trust its query alone. The
+# production engine supplies this header only after authorizing the project.
+status="$(curl --silent --output /dev/null --write-out '%{http_code}' \
+    "http://${listen_address}/v1-webhooks?projectId=1a1")"
+test "${status}" = "403"
+
 status="$(curl --silent --output "${work_root}/invalid-token.json" --write-out '%{http_code}' \
     -X POST "http://${listen_address}/v1-webhooks/endpoint?token=invalid")"
 test "${status}" = "400"

scripts/version

@@ -3,7 +3,7 @@
 version_script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 version_repo_root="$(cd "${version_script_dir}/.." && pwd)"
 
-VERSION="${VERSION:-0.10.1}"
+VERSION="${VERSION:-0.10.2}"
 COMMIT="${COMMIT:-$(git -c safe.directory="${version_repo_root}" -C "${version_repo_root}" rev-parse HEAD)}"
 ARCH="${ARCH:-amd64}"
 

service/construct_handler.go

@@ -21,7 +21,7 @@ import (
 const maximumConfigurationBodyBytes = 1 << 20
 
 func (rh *RouteHandler) ConstructPayload(w http.ResponseWriter, r *http.Request) (int, error) {
-	if readonlyRoles[getRoles(r)] {
+	if hasReadonlyRole(r) {
 		return http.StatusMethodNotAllowed, fmt.Errorf("user doesn't have the access to create webhook")
 	}
 	apiContext := api.GetApiContext(r)

service/execute_handler.go

@@ -85,18 +85,26 @@ func (rh *RouteHandler) ExecuteWithKey(uuid string, projectID string, request *h
 
 	filters := make(map[string]interface{})
 	filters["key"] = uuid
+	filters["kind"] = webhookReceiverKind
 	goCollection, err := apiClient.GenericObject.List(&client.ListOpts{
 		Filters: filters,
 	})
 	if err != nil {
 		return 500, fmt.Errorf("Error %v filtering genericObjects by key", err)
 	}
 
-	if len(goCollection.Data) == 0 {
+	var webhook *client.GenericObject
+	for i := range goCollection.Data {
+		if goCollection.Data[i].Kind == webhookReceiverKind {
+			webhook = &goCollection.Data[i]
+			break
+		}
+	}
+	if webhook == nil {
 		return 403, fmt.Errorf("Requested webhook has been revoked/does not exist for this account")
 	}
 
-	resourceData := goCollection.Data[0].ResourceData
+	resourceData := webhook.ResourceData
 	driverID, ok := resourceData["driver"].(string)
 	if !ok {
 		return 400, fmt.Errorf("No driver provided")
@@ -123,14 +131,17 @@ func (rh *RouteHandler) ExecuteWithKey(uuid string, projectID string, request *h
 func validateWebhook(uuid string, apiClient *client.RancherClient) (int, error) {
 	filters := make(map[string]interface{})
 	filters["key"] = uuid
+	filters["kind"] = webhookReceiverKind
 	webhookCollection, err := apiClient.GenericObject.List(&client.ListOpts{
 		Filters: filters,
 	})
 	if err != nil {
 		return 500, err
 	}
-	if len(webhookCollection.Data) > 0 {
-		return 0, nil
+	for _, webhook := range webhookCollection.Data {
+		if webhook.Kind == webhookReceiverKind {
+			return 0, nil
+		}
 	}
 	return 403, fmt.Errorf("Requested webhook has been revoked")
 }

service/execute_kind_test.go

@@ -0,0 +1,133 @@
+package service
+
+import (
+	"crypto/rand"
+	"crypto/rsa"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/PastureStack/webhook-automation-service/model"
+	"github.com/rancher/go-rancher/v2"
+)
+
+// The mock honors the key filter but intentionally ignores kind, so the
+// receiver boundary is checked even if the API returns other object kinds.
+type executeKindObjectList struct {
+	client.GenericObjectOperations
+	objects []client.GenericObject
+	filters map[string]interface{}
+}
+
+func (m *executeKindObjectList) List(opts *client.ListOpts) (*client.GenericObjectCollection, error) {
+	m.filters = opts.Filters
+	var matched []client.GenericObject
+	for _, object := range m.objects {
+		if object.Key == opts.Filters["key"] {
+			matched = append(matched, object)
+		}
+	}
+	return &client.GenericObjectCollection{Data: matched}, nil
+}
+
+type executeKindClientFactory struct {
+	objects *executeKindObjectList
+}
+
+func (f *executeKindClientFactory) GetClient(string) (*client.RancherClient, error) {
+	return &client.RancherClient{GenericObject: f.objects}, nil
+}
+
+func newExecuteKindHandler(objects ...client.GenericObject) (*RouteHandler, *executeKindObjectList) {
+	list := &executeKindObjectList{objects: objects}
+	return &RouteHandler{ClientFactory: &executeKindClientFactory{objects: list}}, list
+}
+
+func executeKindReceiver(kind string) client.GenericObject {
+	return client.GenericObject{
+		Key:  "receiver-key",
+		Kind: kind,
+		ResourceData: map[string]interface{}{
+			"driver": "scaleService",
+			"config": model.ScaleService{
+				ServiceID:   "id",
+				ScaleAction: "up",
+				ScaleChange: 1,
+			},
+		},
+	}
+}
+
+func TestExecuteWithKeyRejectsForeignGenericObject(t *testing.T) {
+	handler, list := newExecuteKindHandler(executeKindReceiver("otherGenericObject"))
+	request := httptest.NewRequest(http.MethodPost, "/v1-webhooks/endpoint", nil)
+	code, err := handler.ExecuteWithKey("receiver-key", "1a1", request)
+	if code != http.StatusForbidden || err == nil {
+		t.Fatalf("foreign object returned (%d, %v), want 403", code, err)
+	}
+	if list.filters["key"] != "receiver-key" || list.filters["kind"] != webhookReceiverKind {
+		t.Fatalf("list filters = %#v, want receiver key and kind", list.filters)
+	}
+}
+
+func TestExecuteWithSignedJWTRejectsForeignGenericObject(t *testing.T) {
+	privateKey, err := rsa.GenerateKey(rand.Reader, 2048)
+	if err != nil {
+		t.Fatal(err)
+	}
+	handler, list := newExecuteKindHandler(executeKindReceiver("otherGenericObject"))
+	handler.PublicKey = &privateKey.PublicKey
+	token := signTestToken(t, privateKey, "RS256", map[string]interface{}{
+		"driver":    "scaleService",
+		"projectId": "1a1",
+		"uuid":      "receiver-key",
+		"exp":       time.Now().Add(time.Minute).Unix(),
+	})
+	request := httptest.NewRequest(http.MethodPost, "/v1-webhooks/endpoint?token="+token, nil)
+	code, err := handler.Execute(httptest.NewRecorder(), request)
+	if code != http.StatusForbidden || err == nil {
+		t.Fatalf("signed JWT for foreign object returned (%d, %v), want 403", code, err)
+	}
+	if list.filters["key"] != "receiver-key" || list.filters["kind"] != webhookReceiverKind {
+		t.Fatalf("list filters = %#v, want receiver key and kind", list.filters)
+	}
+}
+
+func TestExecuteWithKeyUsesReceiverAfterForeignGenericObject(t *testing.T) {
+	foreign := executeKindReceiver("otherGenericObject")
+	foreign.ResourceData["driver"] = "unregistered"
+	handler, _ := newExecuteKindHandler(
+		foreign,
+		executeKindReceiver(webhookReceiverKind),
+	)
+	request := httptest.NewRequest(http.MethodPost, "/v1-webhooks/endpoint", nil)
+	code, err := handler.ExecuteWithKey("receiver-key", "1a1", request)
+	if code != http.StatusOK || err != nil {
+		t.Fatalf("valid receiver returned (%d, %v), want 200", code, err)
+	}
+}
+
+func TestValidateWebhookRequiresReceiverKind(t *testing.T) {
+	for _, test := range []struct {
+		name    string
+		objects []client.GenericObject
+		want    int
+	}{
+		{"foreign only", []client.GenericObject{executeKindReceiver("otherGenericObject")}, http.StatusForbidden},
+		{"valid receiver", []client.GenericObject{executeKindReceiver(webhookReceiverKind)}, 0},
+		{"foreign then receiver", []client.GenericObject{executeKindReceiver("otherGenericObject"), executeKindReceiver(webhookReceiverKind)}, 0},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			list := &executeKindObjectList{objects: test.objects}
+			apiClient := &client.RancherClient{GenericObject: list}
+			code, err := validateWebhook("receiver-key", apiClient)
+			if code != test.want || (err == nil) != (test.want == 0) {
+				t.Fatalf("validateWebhook returned (%d, %v), want status %d", code, err, test.want)
+			}
+			if list.filters["key"] != "receiver-key" || list.filters["kind"] != webhookReceiverKind {
+				t.Fatalf("list filters = %#v, want receiver key and kind", list.filters)
+			}
+		})
+	}
+}

service/forward_post_test.go

@@ -59,7 +59,7 @@ func TestCreateUpdateExecuteListAndDelete(t *testing.T) {
 	constructURL := fmt.Sprintf("%s/v1-webhooks/receivers?projectId=1a1", server.URL)
 	jsonStr := []byte(`{"driver":"forwardPost","name":"wh-name",
 		"forwardPostConfig": {"projectId": "1a5","serviceName": "pipeline-server", "port": "60080", "path": "/v1"}}`)
-	request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr))
+	request, err := managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr))
 	requireNoError(t, err)
 
 	request.Header.Set("Content-Type", "application/json")
@@ -79,7 +79,7 @@ func TestCreateUpdateExecuteListAndDelete(t *testing.T) {
 
 	// Test getting the created webhook by id
 	byID := fmt.Sprintf("%s/v1-webhooks/receivers/1?projectId=1a1", server.URL)
-	request, err = http.NewRequest("GET", byID, nil)
+	request, err = managementRequest("GET", byID, nil)
 	requireNoError(t, err)
 
 	request.Header.Set("Content-Type", "application/json")
@@ -105,7 +105,7 @@ func TestCreateUpdateExecuteListAndDelete(t *testing.T) {
 	requireEqual(t, "execute status", response.Code, http.StatusOK)
 
 	//List webhooks
-	requestList, err := http.NewRequest("GET", constructURL, nil)
+	requestList, err := managementRequest("GET", constructURL, nil)
 	requireNoError(t, err)
 
 	requestList.Header.Set("Content-Type", "application/json")
@@ -126,7 +126,7 @@ func TestCreateUpdateExecuteListAndDelete(t *testing.T) {
 	requireSelfLink(t, wh.Links["self"])
 
 	//Delete
-	request, err = http.NewRequest("DELETE", byID, nil)
+	request, err = managementRequest("DELETE", byID, nil)
 	requireNoError(t, err)
 
 	request.Header.Set("Content-Type", "application/json")

service/framework_test.go

@@ -5,6 +5,7 @@ import (
 	"crypto/rand"
 	"crypto/rsa"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -22,6 +23,19 @@ var server *httptest.Server
 var router *mux.Router
 var r *RouteHandler
 
+// managementRequest models the control plane's authenticated proxy. Public
+// webhook endpoint tests may carry this header, but do not depend on it.
+func managementRequest(method, url string, body io.Reader) (*http.Request, error) {
+	request, err := http.NewRequest(method, url, body)
+	if err != nil {
+		return nil, err
+	}
+	if projectID := request.URL.Query().Get("projectId"); projectID != "" {
+		request.Header.Set(projectAPIHeader, projectID)
+	}
+	return request, nil
+}
+
 // TODO Refactor this test to use gocheck
 func init() {
 	drivers.Drivers = map[string]drivers.WebhookDriver{}
@@ -86,7 +100,7 @@ func init() {
 
 func TestMissingProjectIdHeader(t *testing.T) {
 	constructURL := fmt.Sprintf("%s/v1-webhooks", server.URL)
-	request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer([]byte(`{}`)))
+	request, err := managementRequest("POST", constructURL, bytes.NewBuffer([]byte(`{}`)))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -107,7 +121,7 @@ func TestMissingProjectIdHeader(t *testing.T) {
 
 func TestMissingContentTypeHeader(t *testing.T) {
 	constructURL := fmt.Sprintf("%s/v1-webhooks?projectId=1a1", server.URL)
-	request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer([]byte(`{}`)))
+	request, err := managementRequest("POST", constructURL, bytes.NewBuffer([]byte(`{}`)))
 	if err != nil {
 		t.Fatal(err)
 	}