Merge pull request #52 from github/copilot/fix-51

Enhance HMAC auth plugin to support structured signature headers (Tailscale-style)

Author: GrantBirki

.rubocop.yml

@@ -15,6 +15,7 @@ AllCops:
 GitHub/InsecureHashAlgorithm:
   Exclude:
     - "spec/unit/lib/hooks/plugins/auth/hmac_spec.rb"
+    - "spec/acceptance/acceptance_tests.rb"
 
 GitHub/AvoidObjectSendWithDynamicMethod:
   Exclude:

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    hooks-ruby (0.2.0)
+    hooks-ruby (0.2.1)
       dry-schema (~> 1.14, >= 1.14.1)
       grape (~> 2.3)
       puma (~> 6.6)

docs/auth_plugins.md

@@ -82,7 +82,49 @@ The maximum age (in seconds) allowed for timestamped requests. Only used when `t
 
 A template for constructing the payload used in signature generation when timestamp validation is enabled. Use placeholders like `{version}`, `{timestamp}`, and `{body}`.
 
-**Example:** `{version}:{timestamp}:{body}`
+**Example:** `{version}:{timestamp}:{body}` (Slack-style), `{timestamp}.{body}` (Tailscale-style)
+
+##### `header_format` (optional)
+
+The format of the signature header content. Use "structured" for headers containing comma-separated key-value pairs.
+
+**Default:** `simple`  
+**Valid values:**
+
+- `simple` - Standard single-value headers like "sha256=abc123..." or "abc123..."
+- `structured` - Comma-separated key-value pairs like "t=1663781880,v1=abc123..."
+
+##### `signature_key` (optional)
+
+When `header_format` is "structured", this specifies the key name for the signature value in the header.
+
+**Default:** `v1`  
+**Example:** `signature`
+
+##### `timestamp_key` (optional)
+
+When `header_format` is "structured", this specifies the key name for the timestamp value in the header.
+
+**Default:** `t`  
+**Example:** `timestamp`
+
+##### `structured_header_separator` (optional)
+
+When `header_format` is "structured", this specifies the separator used between the unique keys in the structured header.
+
+For example, if the header is `t=1663781880,v1=abc123`, the `structured_header_separator` would be `,`. It defaults to `,` but can be changed if needed.
+
+**Example:** `.`
+**Default:** `,`
+
+##### `key_value_separator` (optional)
+
+When `header_format` is "structured", this specifies the separator used between the key and value in the structured header.
+
+For example, in the header `t=1663781880,v1=abc123`, the `key_value_separator` would be `=`. It defaults to `=` but can be changed if needed.
+
+**Example:** `:`
+**Default:** `=`
 
 #### HMAC Examples
 
@@ -218,6 +260,59 @@ curl -X POST "$WEBHOOK_URL" \
 
 This approach provides strong security through timestamp validation while using a simpler format than the Slack-style implementation. The signing payload becomes `1609459200:{"event":"deployment","status":"success"}` and the resulting signature format is `sha256=computed_hmac_hash`.
 
+**Tailscale-style HMAC with structured headers:**
+
+This configuration supports providers like Tailscale that include both timestamp and signature in a single header using comma-separated key-value pairs.
+
+```yaml
+auth:
+  type: hmac
+  secret_env_key: TAILSCALE_WEBHOOK_SECRET
+  header: Tailscale-Webhook-Signature
+  algorithm: sha256
+  format: "signature_only"  # produces "abc123..." (no prefix)
+  header_format: "structured"  # enables parsing of "t=123,v1=abc" format
+  signature_key: "v1"  # key for signature in structured header
+  timestamp_key: "t"   # key for timestamp in structured header
+  payload_template: "{timestamp}.{body}"  # dot-separated format
+  timestamp_tolerance: 300  # 5 minutes
+```
+
+**How it works:**
+
+1. The signature header contains both timestamp and signature: `Tailscale-Webhook-Signature: t=1663781880,v1=0123456789abcdef`
+2. The timestamp and signature are extracted from the structured header
+3. The HMAC is calculated over the payload using the template: `{timestamp}.{body}`
+4. For example, if timestamp is "1663781880" and body is `{"event":"test"}`, the signed payload becomes: `1663781880.{"event":"test"}`
+5. The signature is validated as a raw hex string (no prefix)
+
+**Example curl request:**
+
+```bash
+#!/bin/bash
+
+# Configuration
+WEBHOOK_URL="https://your-hooks-server.com/webhooks/tailscale"
+SECRET="your_tailscale_webhook_secret"
+TIMESTAMP=$(date +%s)
+PAYLOAD='{"nodeId":"n123","event":"nodeCreated"}'
+
+# Construct the signing payload (timestamp.body format)
+SIGNING_PAYLOAD="${TIMESTAMP}.${PAYLOAD}"
+
+# Generate HMAC signature
+SIGNATURE=$(echo -n "$SIGNING_PAYLOAD" | openssl dgst -sha256 -hmac "$SECRET" -hex | cut -d' ' -f2)
+STRUCTURED_SIGNATURE="t=${TIMESTAMP},v1=${SIGNATURE}"
+
+# Send the request
+curl -X POST "$WEBHOOK_URL" \
+  -H "Content-Type: application/json" \
+  -H "Tailscale-Webhook-Signature: $STRUCTURED_SIGNATURE" \
+  -d "$PAYLOAD"
+```
+
+This format is particularly useful for providers that want to include multiple pieces of metadata in a single header while maintaining strong security through timestamp validation.
+
 ### Shared Secret Authentication
 
 The SharedSecret plugin provides simple secret-based authentication by comparing a secret value sent in an HTTP header. While simpler than HMAC, it provides less security since the secret is transmitted directly in the request header.

docs/design.md

@@ -100,7 +100,7 @@ Core configuration options can be provided via environment variables:
 export HOOKS_CONFIG=./config/config.yaml
 
 # Runtime settings (override config file)
-export HOOKS_REQUEST_LIMIT=1048576
+export HOOKS_REQUEST_LIMIT=1048576 # 1 MB
 export HOOKS_REQUEST_TIMEOUT=15
 export HOOKS_GRACEFUL_SHUTDOWN_TIMEOUT=30
 export HOOKS_ROOT_PATH="/webhooks"

lib/hooks/core/config_validator.rb

@@ -45,6 +45,11 @@ class ValidationError < StandardError; end
           optional(:format).filled(:string)
           optional(:version_prefix).filled(:string)
           optional(:payload_template).filled(:string)
+          optional(:header_format).filled(:string)
+          optional(:signature_key).filled(:string)
+          optional(:timestamp_key).filled(:string)
+          optional(:structured_header_separator).filled(:string)
+          optional(:key_value_separator).filled(:string)
         end
 
         optional(:opts).hash

lib/hooks/plugins/auth/base.rb

@@ -14,6 +14,10 @@ module Auth
       class Base
         extend Hooks::Core::ComponentAccess
 
+        # Security constants shared across auth validators
+        MAX_HEADER_VALUE_LENGTH = ENV.fetch("HOOKS_MAX_HEADER_VALUE_LENGTH", 1024).to_i # Prevent DoS attacks via large header values
+        MAX_PAYLOAD_SIZE = ENV.fetch("HOOKS_MAX_PAYLOAD_SIZE", 10 * 1024 * 1024).to_i # 10MB limit for payload validation
+
         # Validate request
         #
         # @param payload [String] Raw request body
@@ -67,6 +71,61 @@ def self.find_header_value(headers, header_name)
           end
           nil
         end
+
+        # Validate headers object for security issues
+        #
+        # @param headers [Object] Headers to validate
+        # @return [Boolean] true if headers are valid
+        def self.valid_headers?(headers)
+          unless headers.respond_to?(:each)
+            log.warn("Auth validation failed: Invalid headers object")
+            return false
+          end
+          true
+        end
+
+        # Validate payload size for security issues
+        #
+        # @param payload [String] Payload to validate
+        # @return [Boolean] true if payload is valid
+        def self.valid_payload_size?(payload)
+          return true if payload.nil?
+
+          if payload.bytesize > MAX_PAYLOAD_SIZE
+            log.warn("Auth validation failed: Payload size exceeds maximum limit of #{MAX_PAYLOAD_SIZE} bytes")
+            return false
+          end
+          true
+        end
+
+        # Validate header value for security issues
+        #
+        # @param header_value [String] Header value to validate
+        # @param header_name [String] Header name for logging
+        # @return [Boolean] true if header value is valid
+        def self.valid_header_value?(header_value, header_name)
+          return false if header_value.nil? || header_value.empty?
+
+          # Check length to prevent DoS
+          if header_value.length > MAX_HEADER_VALUE_LENGTH
+            log.warn("Auth validation failed: #{header_name} exceeds maximum length")
+            return false
+          end
+
+          # Check for whitespace tampering
+          if header_value != header_value.strip
+            log.warn("Auth validation failed: #{header_name} contains leading/trailing whitespace")
+            return false
+          end
+
+          # Check for control characters
+          if header_value.match?(/[\u0000-\u001f\u007f-\u009f]/)
+            log.warn("Auth validation failed: #{header_name} contains control characters")
+            return false
+          end
+
+          true
+        end
       end
     end
   end