Merge pull request #1 from GoCodeAlone/fix/ws02-github-plugin

Fix GitHub plugin: MaxBytesReader, webhook routing, dynamic step fields

Author: intel352

internal/module_webhook.go

@@ -6,7 +6,9 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"io"
 	"net/http"
 	"strings"
 	"time"
@@ -98,12 +100,9 @@ func (m *webhookModule) SetMessageSubscriber(_ sdk.MessageSubscriber) {}
 // Init is a no-op; the module is ready after construction.
 func (m *webhookModule) Init() error { return nil }
 
-// Start registers the HTTP webhook handler with the global mux.
-// The engine must be running its HTTP server for this to receive requests.
-func (m *webhookModule) Start(_ context.Context) error {
-	http.HandleFunc("/webhooks/github", m.handleWebhook)
-	return nil
-}
+// Start is a no-op; the webhook route is declared via ConfigFragment so the
+// engine's HTTP server registers it through the normal config pipeline.
+func (m *webhookModule) Start(_ context.Context) error { return nil }
 
 // Stop is a no-op.
 func (m *webhookModule) Stop(_ context.Context) error { return nil }
@@ -306,27 +305,16 @@ func normalizeGenericEvent(event *GitEvent, payload map[string]any) {
 }
 
 // readLimitedBody reads up to maxBytes from the request body.
+// It uses io.LimitReader to cap reads safely without requiring a ResponseWriter.
+// If the body is exactly maxBytes, an extra byte is attempted to detect overflow.
 func readLimitedBody(r *http.Request, maxBytes int64) ([]byte, error) {
-	r.Body = http.MaxBytesReader(nil, r.Body, maxBytes)
-	buf := make([]byte, 0, 4096)
-	tmp := make([]byte, 4096)
-	var total int64
-	for {
-		n, err := r.Body.Read(tmp)
-		if n > 0 {
-			total += int64(n)
-			if total > maxBytes {
-				return nil, fmt.Errorf("request body exceeds %d bytes", maxBytes)
-			}
-			buf = append(buf, tmp[:n]...)
-		}
-		if err != nil {
-			if err.Error() == "EOF" || err.Error() == "http: request body too large" {
-				break
-			}
-			// io.EOF is expected at end of body
-			break
-		}
+	lr := io.LimitReader(r.Body, maxBytes+1)
+	buf, err := io.ReadAll(lr)
+	if err != nil && !errors.Is(err, io.EOF) {
+		return nil, fmt.Errorf("failed to read request body: %w", err)
+	}
+	if int64(len(buf)) > maxBytes {
+		return nil, fmt.Errorf("request body exceeds %d bytes", maxBytes)
 	}
 	return buf, nil
 }

internal/plugin.go

@@ -6,6 +6,20 @@ import (
 	sdk "github.com/GoCodeAlone/workflow/plugin/external/sdk"
 )
 
+// webhookRouteConfig is the config fragment YAML that declares the GitHub
+// webhook HTTP route so the engine's HTTP server registers it via the normal
+// config pipeline instead of the unreachable global DefaultServeMux.
+const webhookRouteConfig = `
+workflows:
+  github-webhook-receiver:
+    triggers:
+      - type: http
+        config:
+          path: /webhooks/github
+          method: POST
+    steps: []
+`
+
 // githubPlugin implements sdk.PluginProvider, sdk.ModuleProvider, and sdk.StepProvider.
 type githubPlugin struct{}
 
@@ -61,3 +75,11 @@ func (p *githubPlugin) CreateStep(typeName, name string, config map[string]any)
 		return nil, fmt.Errorf("github plugin: unknown step type %q", typeName)
 	}
 }
+
+// ConfigFragment implements sdk.ConfigProvider.
+// It returns a config fragment that declares the /webhooks/github HTTP route
+// so the engine registers it through the normal config pipeline rather than
+// through the global DefaultServeMux (which is unreachable in a gRPC plugin).
+func (p *githubPlugin) ConfigFragment() ([]byte, error) {
+	return []byte(webhookRouteConfig), nil
+}

internal/resolve.go

@@ -0,0 +1,92 @@
+package internal
+
+import (
+	"fmt"
+	"strings"
+)
+
+// resolveField performs basic template resolution on value, replacing
+// {{.field}} references with values looked up from triggerData, stepOutputs,
+// and current (in that priority order).
+//
+// Supported reference forms:
+//
+//	{{.field}}                     — look up "field" in triggerData
+//	{{.steps.stepName.field}}      — look up stepOutputs["stepName"]["field"]
+//	{{.current.field}}             — look up "field" in current
+//
+// If the placeholder cannot be resolved the original placeholder text is left
+// in place so misconfiguration is visible rather than silently swallowed.
+func resolveField(value string, triggerData map[string]any, stepOutputs map[string]map[string]any, current map[string]any) string {
+	if !strings.Contains(value, "{{") {
+		return value
+	}
+
+	result := value
+	// Iterate until no more replacements can be made (handles multiple refs).
+	for strings.Contains(result, "{{") {
+		start := strings.Index(result, "{{")
+		end := strings.Index(result, "}}")
+		if end < start {
+			break
+		}
+		placeholder := result[start : end+2]
+		inner := strings.TrimSpace(result[start+2 : end])
+
+		resolved, ok := lookupRef(inner, triggerData, stepOutputs, current)
+		if ok {
+			result = strings.Replace(result, placeholder, fmt.Sprintf("%v", resolved), 1)
+		} else {
+			// Leave the unresolvable placeholder and stop to avoid an infinite loop.
+			break
+		}
+	}
+	return result
+}
+
+// lookupRef resolves a single template reference (the content between {{ and }}).
+func lookupRef(ref string, triggerData map[string]any, stepOutputs map[string]map[string]any, current map[string]any) (any, bool) {
+	// Strip leading dot.
+	ref = strings.TrimPrefix(ref, ".")
+
+	parts := strings.SplitN(ref, ".", 3)
+
+	switch parts[0] {
+	case "steps":
+		// {{.steps.<stepName>.<field>}}
+		if len(parts) < 3 {
+			return nil, false
+		}
+		stepName, field := parts[1], parts[2]
+		if stepOutputs == nil {
+			return nil, false
+		}
+		outputs, ok := stepOutputs[stepName]
+		if !ok {
+			return nil, false
+		}
+		v, ok := outputs[field]
+		return v, ok
+
+	case "current":
+		// {{.current.<field>}}
+		if len(parts) < 2 {
+			return nil, false
+		}
+		field := strings.Join(parts[1:], ".")
+		if current == nil {
+			return nil, false
+		}
+		v, ok := current[field]
+		return v, ok
+
+	default:
+		// {{.field}} — look up directly in triggerData.
+		field := strings.Join(parts, ".")
+		if triggerData == nil {
+			return nil, false
+		}
+		v, ok := triggerData[field]
+		return v, ok
+	}
+}

internal/step_action_status.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"strconv"
+	"strings"
 	"time"
 
 	sdk "github.com/GoCodeAlone/workflow/plugin/external/sdk"
@@ -33,6 +34,7 @@ type actionStatusConfig struct {
 	Owner        string        `yaml:"owner"`
 	Repo         string        `yaml:"repo"`
 	RunID        int64         `yaml:"run_id"`
+	RunIDRaw     string        // raw string value for dynamic {{.field}} resolution
 	Token        string        `yaml:"token"`
 	Wait         bool          `yaml:"wait"`
 	PollInterval time.Duration `yaml:"poll_interval"`
@@ -70,6 +72,8 @@ func parseActionStatusConfig(raw map[string]any) (actionStatusConfig, error) {
 	}
 
 	// run_id can be provided as int, int64, float64, or string.
+	// When the string contains a template reference (e.g. {{.steps.trigger.run_id}})
+	// the literal value is stored in RunIDRaw and resolved at Execute time.
 	switch v := raw["run_id"].(type) {
 	case int:
 		cfg.RunID = int64(v)
@@ -79,14 +83,18 @@ func parseActionStatusConfig(raw map[string]any) (actionStatusConfig, error) {
 		cfg.RunID = int64(v)
 	case string:
 		if v != "" {
-			n, err := strconv.ParseInt(v, 10, 64)
-			if err != nil {
-				return cfg, fmt.Errorf("config.run_id is not a valid integer: %w", err)
+			if strings.Contains(v, "{{") {
+				cfg.RunIDRaw = v
+			} else {
+				n, err := strconv.ParseInt(v, 10, 64)
+				if err != nil {
+					return cfg, fmt.Errorf("config.run_id is not a valid integer: %w", err)
+				}
+				cfg.RunID = n
 			}
-			cfg.RunID = n
 		}
 	}
-	if cfg.RunID == 0 {
+	if cfg.RunID == 0 && cfg.RunIDRaw == "" {
 		return cfg, fmt.Errorf("config.run_id is required")
 	}
 
@@ -118,27 +126,47 @@ func parseActionStatusConfig(raw map[string]any) (actionStatusConfig, error) {
 }
 
 // Execute checks the status of the configured workflow run.
+// triggerData, stepOutputs, and current are used to resolve dynamic field
+// references (e.g. {{.steps.trigger.run_id}}) in owner, repo, and run_id.
 // When wait=true it polls until the run completes or the timeout elapses.
 func (s *actionStatusStep) Execute(
 	ctx context.Context,
-	_ map[string]any,
-	_ map[string]map[string]any,
-	_ map[string]any,
+	triggerData map[string]any,
+	stepOutputs map[string]map[string]any,
+	current map[string]any,
 	_ map[string]any,
 ) (*sdk.StepResult, error) {
 	token := s.config.Token
 	if token == "" {
 		return errorResult("GITHUB_TOKEN is not configured"), nil
 	}
 
+	// Resolve dynamic owner / repo.
+	owner := resolveField(s.config.Owner, triggerData, stepOutputs, current)
+	repo := resolveField(s.config.Repo, triggerData, stepOutputs, current)
+
+	// Resolve run_id — may be a static int or a dynamic template reference.
+	runID := s.config.RunID
+	if s.config.RunIDRaw != "" {
+		resolved := resolveField(s.config.RunIDRaw, triggerData, stepOutputs, current)
+		n, err := strconv.ParseInt(resolved, 10, 64)
+		if err != nil {
+			return errorResult(fmt.Sprintf("run_id resolved to non-integer value %q: %v", resolved, err)), nil
+		}
+		runID = n
+	}
+	if runID == 0 {
+		return errorResult("run_id resolved to zero — check pipeline context"), nil
+	}
+
 	if !s.config.Wait {
-		return s.fetchStatus(ctx, token)
+		return s.fetchStatusDynamic(ctx, owner, repo, runID, token)
 	}
 
 	// Poll with timeout.
 	deadline := time.Now().Add(s.config.Timeout)
 	for {
-		result, err := s.fetchStatus(ctx, token)
+		result, err := s.fetchStatusDynamic(ctx, owner, repo, runID, token)
 		if err != nil {
 			return nil, err
 		}
@@ -149,7 +177,7 @@ func (s *actionStatusStep) Execute(
 		}
 
 		if time.Now().After(deadline) {
-			return errorResult(fmt.Sprintf("timeout waiting for workflow run %d after %s", s.config.RunID, s.config.Timeout)), nil
+			return errorResult(fmt.Sprintf("timeout waiting for workflow run %d after %s", runID, s.config.Timeout)), nil
 		}
 
 		select {
@@ -160,9 +188,10 @@ func (s *actionStatusStep) Execute(
 	}
 }
 
-// fetchStatus retrieves the current state of the workflow run from the GitHub API.
-func (s *actionStatusStep) fetchStatus(ctx context.Context, token string) (*sdk.StepResult, error) {
-	run, err := s.ghClient.GetWorkflowRun(ctx, s.config.Owner, s.config.Repo, s.config.RunID, token)
+// fetchStatusDynamic retrieves the current state of a workflow run from the
+// GitHub API using caller-supplied (already-resolved) owner, repo, and runID.
+func (s *actionStatusStep) fetchStatusDynamic(ctx context.Context, owner, repo string, runID int64, token string) (*sdk.StepResult, error) {
+	run, err := s.ghClient.GetWorkflowRun(ctx, owner, repo, runID, token)
 	if err != nil {
 		return errorResult(fmt.Sprintf("failed to get workflow run: %v", err)), nil
 	}

internal/step_action_trigger.go

@@ -92,39 +92,43 @@ func parseActionTriggerConfig(raw map[string]any) (actionTriggerConfig, error) {
 }
 
 // Execute triggers the configured GitHub Actions workflow.
-// It returns the trigger confirmation and stops on error.
+// triggerData, stepOutputs, and current are used to resolve dynamic field
+// references (e.g. {{.owner}}, {{.steps.prev.ref}}) in the config values.
 func (s *actionTriggerStep) Execute(
 	ctx context.Context,
-	_ map[string]any,
-	_ map[string]map[string]any,
-	_ map[string]any,
+	triggerData map[string]any,
+	stepOutputs map[string]map[string]any,
+	current map[string]any,
 	_ map[string]any,
 ) (*sdk.StepResult, error) {
 	token := s.config.Token
 	if token == "" {
 		return errorResult("GITHUB_TOKEN is not configured"), nil
 	}
 
-	err := s.ghClient.TriggerWorkflow(
-		ctx,
-		s.config.Owner,
-		s.config.Repo,
-		s.config.Workflow,
-		s.config.Ref,
-		s.config.Inputs,
-		token,
-	)
+	owner := resolveField(s.config.Owner, triggerData, stepOutputs, current)
+	repo := resolveField(s.config.Repo, triggerData, stepOutputs, current)
+	workflow := resolveField(s.config.Workflow, triggerData, stepOutputs, current)
+	ref := resolveField(s.config.Ref, triggerData, stepOutputs, current)
+
+	// Resolve template references in each input value.
+	inputs := make(map[string]string, len(s.config.Inputs))
+	for k, v := range s.config.Inputs {
+		inputs[k] = resolveField(v, triggerData, stepOutputs, current)
+	}
+
+	err := s.ghClient.TriggerWorkflow(ctx, owner, repo, workflow, ref, inputs, token)
 	if err != nil {
 		return errorResult(fmt.Sprintf("failed to trigger workflow: %v", err)), nil
 	}
 
 	return &sdk.StepResult{
 		Output: map[string]any{
 			"triggered": true,
-			"owner":     s.config.Owner,
-			"repo":      s.config.Repo,
-			"workflow":  s.config.Workflow,
-			"ref":       s.config.Ref,
+			"owner":     owner,
+			"repo":      repo,
+			"workflow":  workflow,
+			"ref":       ref,
 		},
 	}, nil
 }