diff --git a/pkg/cli/templates/github-agentic-workflows.md b/pkg/cli/templates/github-agentic-workflows.md index 30ddce78ae..f062b022d8 100644 --- a/pkg/cli/templates/github-agentic-workflows.md +++ b/pkg/cli/templates/github-agentic-workflows.md @@ -77,7 +77,7 @@ The YAML frontmatter supports these fields: - **`on:`** - Workflow triggers (required) - String: `"push"`, `"issues"`, etc. - Object: Complex trigger configuration - - Special: `command:` for /mention triggers + - Special: `slash_command:` for /mention triggers (replaces deprecated `command:`) - **`forks:`** - Fork allowlist for `pull_request` triggers (array or string). By default, workflows block all forks and only allow same-repo PRs. Use `["*"]` to allow all forks, or specify patterns like `["org/*", "user/repo"]` - **`stop-after:`** - Can be included in the `on:` object to set a deadline for workflow execution. Supports absolute timestamps ("YYYY-MM-DD HH:MM:SS") or relative time deltas (+25h, +3d, +1d12h). The minimum unit for relative deltas is hours (h). Uses precise date calculations that account for varying month lengths. - **`reaction:`** - Add emoji reactions to triggering items @@ -104,6 +104,13 @@ The YAML frontmatter supports these fields: - **`description:`** - Human-readable workflow description (string) - **`source:`** - Workflow origin tracking in format `owner/repo/path@ref` (string) +- **`labels:`** - Array of labels to categorize and organize workflows (array) + - Labels filter workflows in status/list commands + - Example: `labels: [automation, security, daily]` +- **`metadata:`** - Custom key-value pairs compatible with custom agent spec (object) + - Key names limited to 64 characters + - Values limited to 1024 characters + - Example: `metadata: { team: "platform", priority: "high" }` - **`github-token:`** - Default GitHub token for workflow (must use `${{ secrets.* }}` syntax) - **`roles:`** - Repository access roles that can trigger workflow (array or "all") - Default: `[admin, maintainer, write]` @@ -282,8 +289,11 @@ The YAML frontmatter supports these fields: labels: [automation, agentic] # Optional: labels to attach to issues assignees: [user1, copilot] # Optional: assignees (use 'copilot' for bot) max: 5 # Optional: maximum number of issues (default: 1) + expires: 7 # Optional: auto-close after 7 days (supports: 7d, 2w, 1m, 1y) target-repo: "owner/repo" # Optional: cross-repository ``` + + **Auto-Expiration**: The `expires` field auto-closes issues after a time period. Supports integers (days) or relative formats (7d, 2w, 1m, 1y). Generates daily `agentics-maintenance.yml` workflow to close expired items. When using `safe-outputs.create-issue`, the main job does **not** need `issues: write` permission since issue creation is handled by a separate job with appropriate permissions. **Temporary IDs and Sub-Issues:** @@ -333,8 +343,13 @@ The YAML frontmatter supports these fields: max: 3 # Optional: maximum number of comments (default: 1) target: "*" # Optional: target for comments (default: "triggering") discussion: true # Optional: target discussions + hide-older-comments: true # Optional: minimize previous comments from same workflow + allowed-reasons: [outdated] # Optional: restrict hiding reasons (default: outdated) target-repo: "owner/repo" # Optional: cross-repository ``` + + **Hide Older Comments**: Set `hide-older-comments: true` to minimize previous comments from the same workflow before posting new ones. Useful for status updates. Allowed reasons: `spam`, `abuse`, `off_topic`, `outdated` (default), `resolved`. + When using `safe-outputs.add-comment`, the main job does **not** need `issues: write` or `pull-requests: write` permissions since comment creation is handled by a separate job with appropriate permissions. - `create-pull-request:` - Safe pull request creation with git patches ```yaml @@ -554,10 +569,11 @@ The YAML frontmatter supports these fields: github-token: ${{ secrets.CUSTOM_PAT }} # Use custom PAT instead of GITHUB_TOKEN ``` Useful when you need additional permissions or want to perform actions across repositories. - -- **`command:`** - Command trigger configuration for /mention workflows + +- **`slash_command:`** - Command trigger configuration for /mention workflows (replaces deprecated `command:`) - **`cache:`** - Cache configuration for workflow dependencies (object or array) - **`cache-memory:`** - Memory MCP server with persistent cache storage (boolean or object) +- **`repo-memory:`** - Repository-specific memory storage (boolean) ### Cache Configuration @@ -658,6 +674,17 @@ Cache-memory configurations can be imported from shared agentic workflows using The memory MCP server is automatically configured when `cache-memory` is enabled and works with both Claude and Custom engines. +### Repo Memory Configuration + +The `repo-memory:` field enables repository-specific memory storage for maintaining context across executions: + +```yaml +tools: + repo-memory: +``` + +This provides persistent memory storage specific to the repository, useful for maintaining workflow-specific context and state across runs. + ## Output Processing and Issue Creation ### Automatic GitHub Issue Creation @@ -731,17 +758,19 @@ on: ### Command Triggers (/mentions) ```yaml on: - command: + slash_command: name: my-bot # Responds to /my-bot in issues/comments ``` +**Note**: The `command:` trigger field is deprecated. Use `slash_command:` instead. The old syntax still works but may show deprecation warnings. + This automatically creates conditions to match `/my-bot` mentions in issue bodies and comments. You can restrict where commands are active using the `events:` field: ```yaml on: - command: + slash_command: name: my-bot events: [issues, issue_comment] # Only in issue bodies and issue comments ``` @@ -1208,7 +1237,7 @@ Research latest developments in ${{ github.repository }}: ```markdown --- on: - command: + slash_command: name: helper-bot permissions: contents: read @@ -1219,7 +1248,7 @@ safe-outputs: # Helper Bot -Respond to /helper-bot mentions with helpful information realted to ${{ github.repository }}. The request is "${{ needs.activation.outputs.text }}". +Respond to /helper-bot mentions with helpful information related to ${{ github.repository }}. The request is "${{ needs.activation.outputs.text }}". ``` ### Workflow Improvement Bot diff --git a/pkg/workflow/safe_inputs.go b/pkg/workflow/safe_inputs.go deleted file mode 100644 index 87dd3623f4..0000000000 --- a/pkg/workflow/safe_inputs.go +++ /dev/null @@ -1,837 +0,0 @@ -package workflow - -import ( - "encoding/json" - "fmt" - "sort" - "strings" - - "github.com/githubnext/gh-aw/pkg/constants" - "github.com/githubnext/gh-aw/pkg/logger" -) - -var safeInputsLog = logger.New("workflow:safe_inputs") - -// sanitizeParameterName converts a parameter name to a safe JavaScript identifier -// by replacing non-alphanumeric characters with underscores -func sanitizeParameterName(name string) string { - // Replace dashes and other non-alphanumeric chars with underscores - result := strings.Map(func(r rune) rune { - if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_' || r == '$' { - return r - } - return '_' - }, name) - - // Ensure it doesn't start with a number - if len(result) > 0 && result[0] >= '0' && result[0] <= '9' { - result = "_" + result - } - - return result -} - -// SafeInputsConfig holds the configuration for safe-inputs custom tools -type SafeInputsConfig struct { - Mode string // Transport mode: "http" (default) or "stdio" - Tools map[string]*SafeInputToolConfig -} - -// SafeInputToolConfig holds the configuration for a single safe-input tool -type SafeInputToolConfig struct { - Name string // Tool name (key from the config) - Description string // Required: tool description - Inputs map[string]*SafeInputParam // Optional: input parameters - Script string // JavaScript implementation (mutually exclusive with Run and Py) - Run string // Shell script implementation (mutually exclusive with Script and Py) - Py string // Python script implementation (mutually exclusive with Script and Run) - Env map[string]string // Environment variables (typically for secrets) - Timeout int // Timeout in seconds for tool execution (default: 60) -} - -// SafeInputParam holds the configuration for a tool input parameter -type SafeInputParam struct { - Type string // JSON schema type (string, number, boolean, array, object) - Description string // Description of the parameter - Required bool // Whether the parameter is required - Default any // Default value -} - -// SafeInputsMode constants define the available transport modes -const ( - SafeInputsModeHTTP = "http" -) - -// HasSafeInputs checks if safe-inputs are configured -func HasSafeInputs(safeInputs *SafeInputsConfig) bool { - return safeInputs != nil && len(safeInputs.Tools) > 0 -} - -// IsSafeInputsHTTPMode checks if safe-inputs is configured to use HTTP mode -// Note: All safe-inputs configurations now use HTTP mode exclusively -func IsSafeInputsHTTPMode(safeInputs *SafeInputsConfig) bool { - return safeInputs != nil -} - -// IsSafeInputsEnabled checks if safe-inputs are configured. -// Safe-inputs are enabled by default when configured in the workflow. -// The workflowData parameter is kept for backward compatibility but is not used. -func IsSafeInputsEnabled(safeInputs *SafeInputsConfig, workflowData *WorkflowData) bool { - return HasSafeInputs(safeInputs) -} - -// parseSafeInputsMap parses safe-inputs configuration from a map. -// This is the shared implementation used by both ParseSafeInputs and extractSafeInputsConfig. -// Returns the config and a boolean indicating whether any tools were found. -func parseSafeInputsMap(safeInputsMap map[string]any) (*SafeInputsConfig, bool) { - config := &SafeInputsConfig{ - Mode: "http", // Only HTTP mode is supported - Tools: make(map[string]*SafeInputToolConfig), - } - - // Mode field is ignored - only HTTP mode is supported - // All safe-inputs configurations use HTTP transport - - for toolName, toolValue := range safeInputsMap { - // Skip the "mode" field as it's not a tool definition - if toolName == "mode" { - continue - } - - toolMap, ok := toolValue.(map[string]any) - if !ok { - continue - } - - toolConfig := &SafeInputToolConfig{ - Name: toolName, - Inputs: make(map[string]*SafeInputParam), - Env: make(map[string]string), - Timeout: 60, // Default timeout: 60 seconds - } - - // Parse description (required) - if desc, exists := toolMap["description"]; exists { - if descStr, ok := desc.(string); ok { - toolConfig.Description = descStr - } - } - - // Parse inputs (optional) - if inputs, exists := toolMap["inputs"]; exists { - if inputsMap, ok := inputs.(map[string]any); ok { - for paramName, paramValue := range inputsMap { - if paramMap, ok := paramValue.(map[string]any); ok { - param := &SafeInputParam{ - Type: "string", // default type - } - - if t, exists := paramMap["type"]; exists { - if tStr, ok := t.(string); ok { - param.Type = tStr - } - } - - if desc, exists := paramMap["description"]; exists { - if descStr, ok := desc.(string); ok { - param.Description = descStr - } - } - - if req, exists := paramMap["required"]; exists { - if reqBool, ok := req.(bool); ok { - param.Required = reqBool - } - } - - if def, exists := paramMap["default"]; exists { - param.Default = def - } - - toolConfig.Inputs[paramName] = param - } - } - } - } - - // Parse script (JavaScript implementation) - if script, exists := toolMap["script"]; exists { - if scriptStr, ok := script.(string); ok { - toolConfig.Script = scriptStr - } - } - - // Parse run (shell script implementation) - if run, exists := toolMap["run"]; exists { - if runStr, ok := run.(string); ok { - toolConfig.Run = runStr - } - } - - // Parse py (Python script implementation) - if py, exists := toolMap["py"]; exists { - if pyStr, ok := py.(string); ok { - toolConfig.Py = pyStr - } - } - - // Parse env (environment variables) - if env, exists := toolMap["env"]; exists { - if envMap, ok := env.(map[string]any); ok { - for envName, envValue := range envMap { - if envStr, ok := envValue.(string); ok { - toolConfig.Env[envName] = envStr - } - } - } - } - - // Parse timeout (optional, default is 60 seconds) - if timeout, exists := toolMap["timeout"]; exists { - switch t := timeout.(type) { - case int: - toolConfig.Timeout = t - case uint64: - toolConfig.Timeout = int(t) - case float64: - toolConfig.Timeout = int(t) - case string: - // Try to parse string as integer - _, _ = fmt.Sscanf(t, "%d", &toolConfig.Timeout) - } - } - - config.Tools[toolName] = toolConfig - } - - return config, len(config.Tools) > 0 -} - -// ParseSafeInputs parses safe-inputs configuration from frontmatter (standalone function for testing) -func ParseSafeInputs(frontmatter map[string]any) *SafeInputsConfig { - if frontmatter == nil { - return nil - } - - safeInputs, exists := frontmatter["safe-inputs"] - if !exists { - return nil - } - - safeInputsMap, ok := safeInputs.(map[string]any) - if !ok { - return nil - } - - config, _ := parseSafeInputsMap(safeInputsMap) - return config -} - -// extractSafeInputsConfig extracts safe-inputs configuration from frontmatter -func (c *Compiler) extractSafeInputsConfig(frontmatter map[string]any) *SafeInputsConfig { - safeInputsLog.Print("Extracting safe-inputs configuration from frontmatter") - - safeInputs, exists := frontmatter["safe-inputs"] - if !exists { - return nil - } - - safeInputsMap, ok := safeInputs.(map[string]any) - if !ok { - return nil - } - - config, hasTools := parseSafeInputsMap(safeInputsMap) - if !hasTools { - return nil - } - - safeInputsLog.Printf("Extracted %d safe-input tools", len(config.Tools)) - return config -} - -// SafeInputsDirectory is the directory where safe-inputs files are generated -const SafeInputsDirectory = "/tmp/gh-aw/safe-inputs" - -// SafeInputsToolJSON represents a tool configuration for the tools.json file -type SafeInputsToolJSON struct { - Name string `json:"name"` - Description string `json:"description"` - InputSchema map[string]any `json:"inputSchema"` - Handler string `json:"handler,omitempty"` - Env map[string]string `json:"env,omitempty"` - Timeout int `json:"timeout,omitempty"` -} - -// SafeInputsConfigJSON represents the tools.json configuration file structure -type SafeInputsConfigJSON struct { - ServerName string `json:"serverName"` - Version string `json:"version"` - LogDir string `json:"logDir,omitempty"` - Tools []SafeInputsToolJSON `json:"tools"` -} - -// generateSafeInputsToolsConfig generates the tools.json configuration for the safe-inputs MCP server -func generateSafeInputsToolsConfig(safeInputs *SafeInputsConfig) string { - config := SafeInputsConfigJSON{ - ServerName: "safeinputs", - Version: constants.SafeInputsMCPVersion, - LogDir: SafeInputsDirectory + "/logs", - Tools: []SafeInputsToolJSON{}, - } - - // Sort tool names for stable output - toolNames := make([]string, 0, len(safeInputs.Tools)) - for toolName := range safeInputs.Tools { - toolNames = append(toolNames, toolName) - } - sort.Strings(toolNames) - - for _, toolName := range toolNames { - toolConfig := safeInputs.Tools[toolName] - - // Build input schema - inputSchema := map[string]any{ - "type": "object", - "properties": make(map[string]any), - } - - props := inputSchema["properties"].(map[string]any) - var required []string - - // Sort input names for stable output - inputNames := make([]string, 0, len(toolConfig.Inputs)) - for paramName := range toolConfig.Inputs { - inputNames = append(inputNames, paramName) - } - sort.Strings(inputNames) - - for _, paramName := range inputNames { - param := toolConfig.Inputs[paramName] - propDef := map[string]any{ - "type": param.Type, - "description": param.Description, - } - if param.Default != nil { - propDef["default"] = param.Default - } - props[paramName] = propDef - if param.Required { - required = append(required, paramName) - } - } - - sort.Strings(required) - if len(required) > 0 { - inputSchema["required"] = required - } - - // Determine handler path based on script type - var handler string - if toolConfig.Script != "" { - handler = toolName + ".cjs" - } else if toolConfig.Run != "" { - handler = toolName + ".sh" - } else if toolConfig.Py != "" { - handler = toolName + ".py" - } - - // Build env list of required environment variables (not actual secrets) - // This documents which env vars the tool needs, but doesn't store secret values - // The actual values are passed as environment variables and accessed via process.env - var envRefs map[string]string - if len(toolConfig.Env) > 0 { - envRefs = make(map[string]string) - // Sort env var names for stable output - envVarNames := make([]string, 0, len(toolConfig.Env)) - for envVarName := range toolConfig.Env { - envVarNames = append(envVarNames, envVarName) - } - sort.Strings(envVarNames) - - for _, envVarName := range envVarNames { - // Store just the environment variable name without $ prefix or secret value - // Handlers access the actual value via process.env[envVarName] at runtime - envRefs[envVarName] = envVarName - } - } - - config.Tools = append(config.Tools, SafeInputsToolJSON{ - Name: toolName, - Description: toolConfig.Description, - InputSchema: inputSchema, - Handler: handler, - Env: envRefs, - Timeout: toolConfig.Timeout, - }) - } - - jsonBytes, err := json.MarshalIndent(config, "", " ") - if err != nil { - safeInputsLog.Printf("Error marshaling tools config: %v", err) - return "{}" - } - return string(jsonBytes) -} - -// generateSafeInputsMCPServerScript generates the entry point script for the safe-inputs MCP server -// This script uses HTTP transport exclusively -func generateSafeInputsMCPServerScript(safeInputs *SafeInputsConfig) string { - var sb strings.Builder - - // HTTP transport - server started in separate step - sb.WriteString(`// @ts-check -// Auto-generated safe-inputs MCP server entry point (HTTP transport) -// This script uses the reusable safe_inputs_mcp_server_http module - -const path = require("path"); -const { startHttpServer } = require("./safe_inputs_mcp_server_http.cjs"); - -// Configuration file path (generated alongside this script) -const configPath = path.join(__dirname, "tools.json"); - -// Get port and API key from environment variables -const port = parseInt(process.env.GH_AW_SAFE_INPUTS_PORT || "3000", 10); -const apiKey = process.env.GH_AW_SAFE_INPUTS_API_KEY || ""; - -// Start the HTTP server -startHttpServer(configPath, { - port: port, - stateless: false, - logDir: "/tmp/gh-aw/safe-inputs/logs" -}).catch(error => { - console.error("Failed to start safe-inputs HTTP server:", error); - process.exit(1); -}); -`) - - return sb.String() -} - -// generateSafeInputJavaScriptToolScript generates the JavaScript tool file for a safe-input tool -// The user's script code is automatically wrapped in a function with module.exports, -// so users can write simple code without worrying about exports. -// Input parameters are destructured and available as local variables. -func generateSafeInputJavaScriptToolScript(toolConfig *SafeInputToolConfig) string { - var sb strings.Builder - - sb.WriteString("// @ts-check\n") - sb.WriteString("// Auto-generated safe-input tool: " + toolConfig.Name + "\n\n") - sb.WriteString("/**\n") - sb.WriteString(" * " + toolConfig.Description + "\n") - sb.WriteString(" * @param {Object} inputs - Input parameters\n") - // Sort input names for stable code generation in JSDoc - inputNamesForDoc := make([]string, 0, len(toolConfig.Inputs)) - for paramName := range toolConfig.Inputs { - inputNamesForDoc = append(inputNamesForDoc, paramName) - } - sort.Strings(inputNamesForDoc) - for _, paramName := range inputNamesForDoc { - param := toolConfig.Inputs[paramName] - fmt.Fprintf(&sb, " * @param {%s} inputs.%s - %s\n", param.Type, paramName, param.Description) - } - sb.WriteString(" * @returns {Promise} Tool result\n") - sb.WriteString(" */\n") - sb.WriteString("async function execute(inputs) {\n") - - // Destructure inputs to make parameters available as local variables - if len(toolConfig.Inputs) > 0 { - var paramNames []string - for paramName := range toolConfig.Inputs { - safeName := sanitizeParameterName(paramName) - if safeName != paramName { - // If sanitized, use alias - paramNames = append(paramNames, fmt.Sprintf("%s: %s", paramName, safeName)) - } else { - paramNames = append(paramNames, paramName) - } - } - sort.Strings(paramNames) - fmt.Fprintf(&sb, " const { %s } = inputs || {};\n\n", strings.Join(paramNames, ", ")) - } - - // Indent the user's script code - sb.WriteString(" " + strings.ReplaceAll(toolConfig.Script, "\n", "\n ") + "\n") - sb.WriteString("}\n\n") - sb.WriteString("module.exports = { execute };\n") - - return sb.String() -} - -// generateSafeInputShellToolScript generates the shell script for a safe-input tool -func generateSafeInputShellToolScript(toolConfig *SafeInputToolConfig) string { - var sb strings.Builder - - sb.WriteString("#!/bin/bash\n") - sb.WriteString("# Auto-generated safe-input tool: " + toolConfig.Name + "\n") - sb.WriteString("# " + toolConfig.Description + "\n\n") - sb.WriteString("set -euo pipefail\n\n") - sb.WriteString(toolConfig.Run + "\n") - - return sb.String() -} - -// generateSafeInputPythonToolScript generates the Python script for a safe-input tool -// Python scripts receive inputs as a dictionary (parsed from JSON stdin): -// - Input parameters are available as a pre-parsed 'inputs' dictionary -// - Individual parameters can be destructured: param = inputs.get('param', default) -// - Outputs are printed to stdout as JSON -// - Environment variables from env: field are available via os.environ -func generateSafeInputPythonToolScript(toolConfig *SafeInputToolConfig) string { - var sb strings.Builder - - sb.WriteString("#!/usr/bin/env python3\n") - sb.WriteString("# Auto-generated safe-input tool: " + toolConfig.Name + "\n") - sb.WriteString("# " + toolConfig.Description + "\n\n") - sb.WriteString("import json\n") - sb.WriteString("import os\n") - sb.WriteString("import sys\n\n") - - // Add wrapper code to read inputs from stdin - sb.WriteString("# Read inputs from stdin (JSON format)\n") - sb.WriteString("try:\n") - sb.WriteString(" inputs = json.loads(sys.stdin.read()) if not sys.stdin.isatty() else {}\n") - sb.WriteString("except (json.JSONDecodeError, Exception):\n") - sb.WriteString(" inputs = {}\n\n") - - // Add helper comment about input parameters - if len(toolConfig.Inputs) > 0 { - sb.WriteString("# Input parameters available in 'inputs' dictionary:\n") - // Sort input names for stable code generation - inputNames := make([]string, 0, len(toolConfig.Inputs)) - for paramName := range toolConfig.Inputs { - inputNames = append(inputNames, paramName) - } - sort.Strings(inputNames) - for _, paramName := range inputNames { - param := toolConfig.Inputs[paramName] - defaultValue := "" - if param.Default != nil { - defaultValue = fmt.Sprintf(", default=%v", param.Default) - } - fmt.Fprintf(&sb, "# %s = inputs.get('%s'%s) # %s\n", - sanitizePythonVariableName(paramName), paramName, defaultValue, param.Description) - } - sb.WriteString("\n") - } - - // Add user's Python code - sb.WriteString("# User code:\n") - sb.WriteString(toolConfig.Py + "\n") - - return sb.String() -} - -// sanitizePythonVariableName converts a parameter name to a valid Python identifier -func sanitizePythonVariableName(name string) string { - // Replace dashes and other non-alphanumeric chars with underscores - result := strings.Map(func(r rune) rune { - if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_' { - return r - } - return '_' - }, name) - - // Ensure it doesn't start with a number - if len(result) > 0 && result[0] >= '0' && result[0] <= '9' { - result = "_" + result - } - - return result -} - -// getSafeInputsEnvVars returns the list of environment variables needed for safe-inputs -func getSafeInputsEnvVars(safeInputs *SafeInputsConfig) []string { - envVars := []string{} - seen := make(map[string]bool) - - if safeInputs == nil { - return envVars - } - - for _, toolConfig := range safeInputs.Tools { - for envName := range toolConfig.Env { - if !seen[envName] { - envVars = append(envVars, envName) - seen[envName] = true - } - } - } - - sort.Strings(envVars) - return envVars -} - -// collectSafeInputsSecrets collects all secrets from safe-inputs configuration -func collectSafeInputsSecrets(safeInputs *SafeInputsConfig) map[string]string { - secrets := make(map[string]string) - - if safeInputs == nil { - return secrets - } - - // Sort tool names for consistent behavior when same env var appears in multiple tools - toolNames := make([]string, 0, len(safeInputs.Tools)) - for toolName := range safeInputs.Tools { - toolNames = append(toolNames, toolName) - } - sort.Strings(toolNames) - - for _, toolName := range toolNames { - toolConfig := safeInputs.Tools[toolName] - // Sort env var names for consistent order within each tool - envNames := make([]string, 0, len(toolConfig.Env)) - for envName := range toolConfig.Env { - envNames = append(envNames, envName) - } - sort.Strings(envNames) - - for _, envName := range envNames { - secrets[envName] = toolConfig.Env[envName] - } - } - - return secrets -} - -// renderSafeInputsMCPConfigWithOptions generates the Safe Inputs MCP server configuration with engine-specific options -// Only supports HTTP transport mode -func renderSafeInputsMCPConfigWithOptions(yaml *strings.Builder, safeInputs *SafeInputsConfig, isLast bool, includeCopilotFields bool) { - envVars := getSafeInputsEnvVars(safeInputs) - - yaml.WriteString(" \"" + constants.SafeInputsMCPServerID + "\": {\n") - - // HTTP transport configuration - server started in separate step - // Add type field for HTTP (required by MCP specification for HTTP transport) - yaml.WriteString(" \"type\": \"http\",\n") - - // HTTP URL using environment variable - // Use host.docker.internal to allow access from firewall container - if includeCopilotFields { - // Copilot format: backslash-escaped shell variable reference - yaml.WriteString(" \"url\": \"http://host.docker.internal:\\${GH_AW_SAFE_INPUTS_PORT}\",\n") - } else { - // Claude/Custom format: direct shell variable reference - yaml.WriteString(" \"url\": \"http://host.docker.internal:$GH_AW_SAFE_INPUTS_PORT\",\n") - } - - // Add Authorization header with API key - yaml.WriteString(" \"headers\": {\n") - if includeCopilotFields { - // Copilot format: backslash-escaped shell variable reference - yaml.WriteString(" \"Authorization\": \"Bearer \\${GH_AW_SAFE_INPUTS_API_KEY}\"\n") - } else { - // Claude/Custom format: direct shell variable reference - yaml.WriteString(" \"Authorization\": \"Bearer $GH_AW_SAFE_INPUTS_API_KEY\"\n") - } - yaml.WriteString(" },\n") - - // Add tools field for Copilot - if includeCopilotFields { - yaml.WriteString(" \"tools\": [\"*\"],\n") - } - - // Add env block for environment variable passthrough - envVarsWithServerConfig := append([]string{"GH_AW_SAFE_INPUTS_PORT", "GH_AW_SAFE_INPUTS_API_KEY"}, envVars...) - yaml.WriteString(" \"env\": {\n") - - // Write environment variables with appropriate escaping - for i, envVar := range envVarsWithServerConfig { - isLastEnvVar := i == len(envVarsWithServerConfig)-1 - comma := "" - if !isLastEnvVar { - comma = "," - } - - if includeCopilotFields { - // Copilot format: backslash-escaped shell variable reference - yaml.WriteString(" \"" + envVar + "\": \"\\${" + envVar + "}\"" + comma + "\n") - } else { - // Claude/Custom format: direct shell variable reference - yaml.WriteString(" \"" + envVar + "\": \"$" + envVar + "\"" + comma + "\n") - } - } - - yaml.WriteString(" }\n") - - if isLast { - yaml.WriteString(" }\n") - } else { - yaml.WriteString(" },\n") - } -} - -// mergeSafeInputs merges safe-inputs configuration from imports into the main configuration -func (c *Compiler) mergeSafeInputs(main *SafeInputsConfig, importedConfigs []string) *SafeInputsConfig { - if main == nil { - main = &SafeInputsConfig{ - Mode: "http", // Default to HTTP mode - Tools: make(map[string]*SafeInputToolConfig), - } - } - - for _, configJSON := range importedConfigs { - if configJSON == "" || configJSON == "{}" { - continue - } - - // Merge the imported JSON config - var importedMap map[string]any - if err := json.Unmarshal([]byte(configJSON), &importedMap); err != nil { - safeInputsLog.Printf("Warning: failed to parse imported safe-inputs config: %v", err) - continue - } - - // Mode field is ignored - only HTTP mode is supported - // All safe-inputs configurations use HTTP transport - - // Merge each tool from the imported config - for toolName, toolValue := range importedMap { - // Skip mode field as it's already handled - if toolName == "mode" { - continue - } - - // Skip if tool already exists in main config (main takes precedence) - if _, exists := main.Tools[toolName]; exists { - safeInputsLog.Printf("Skipping imported tool '%s' - already defined in main config", toolName) - continue - } - - toolMap, ok := toolValue.(map[string]any) - if !ok { - continue - } - - toolConfig := &SafeInputToolConfig{ - Name: toolName, - Inputs: make(map[string]*SafeInputParam), - Env: make(map[string]string), - Timeout: 60, // Default timeout: 60 seconds - } - - // Parse description - if desc, exists := toolMap["description"]; exists { - if descStr, ok := desc.(string); ok { - toolConfig.Description = descStr - } - } - - // Parse inputs - if inputs, exists := toolMap["inputs"]; exists { - if inputsMap, ok := inputs.(map[string]any); ok { - for paramName, paramValue := range inputsMap { - if paramMap, ok := paramValue.(map[string]any); ok { - param := &SafeInputParam{ - Type: "string", - } - if t, exists := paramMap["type"]; exists { - if tStr, ok := t.(string); ok { - param.Type = tStr - } - } - if desc, exists := paramMap["description"]; exists { - if descStr, ok := desc.(string); ok { - param.Description = descStr - } - } - if req, exists := paramMap["required"]; exists { - if reqBool, ok := req.(bool); ok { - param.Required = reqBool - } - } - if def, exists := paramMap["default"]; exists { - param.Default = def - } - toolConfig.Inputs[paramName] = param - } - } - } - } - - // Parse script - if script, exists := toolMap["script"]; exists { - if scriptStr, ok := script.(string); ok { - toolConfig.Script = scriptStr - } - } - - // Parse run - if run, exists := toolMap["run"]; exists { - if runStr, ok := run.(string); ok { - toolConfig.Run = runStr - } - } - - // Parse py - if py, exists := toolMap["py"]; exists { - if pyStr, ok := py.(string); ok { - toolConfig.Py = pyStr - } - } - - // Parse env - if env, exists := toolMap["env"]; exists { - if envMap, ok := env.(map[string]any); ok { - for envName, envValue := range envMap { - if envStr, ok := envValue.(string); ok { - toolConfig.Env[envName] = envStr - } - } - } - } - - // Parse timeout (optional, default is 60 seconds) - if timeout, exists := toolMap["timeout"]; exists { - switch t := timeout.(type) { - case int: - toolConfig.Timeout = t - case uint64: - toolConfig.Timeout = int(t) - case float64: - toolConfig.Timeout = int(t) - case string: - // Try to parse string as integer - _, _ = fmt.Sscanf(t, "%d", &toolConfig.Timeout) - } - } - - main.Tools[toolName] = toolConfig - safeInputsLog.Printf("Merged imported safe-input tool: %s", toolName) - } - } - - return main -} - -// Public wrapper functions for CLI use - -// GenerateSafeInputsToolsConfigForInspector generates the tools.json configuration for the safe-inputs MCP server -// This is a public wrapper for use by the CLI inspector command -func GenerateSafeInputsToolsConfigForInspector(safeInputs *SafeInputsConfig) string { - return generateSafeInputsToolsConfig(safeInputs) -} - -// GenerateSafeInputsMCPServerScriptForInspector generates the MCP server entry point script -// This is a public wrapper for use by the CLI inspector command -func GenerateSafeInputsMCPServerScriptForInspector(safeInputs *SafeInputsConfig) string { - return generateSafeInputsMCPServerScript(safeInputs) -} - -// GenerateSafeInputJavaScriptToolScriptForInspector generates a JavaScript tool handler script -// This is a public wrapper for use by the CLI inspector command -func GenerateSafeInputJavaScriptToolScriptForInspector(toolConfig *SafeInputToolConfig) string { - return generateSafeInputJavaScriptToolScript(toolConfig) -} - -// GenerateSafeInputShellToolScriptForInspector generates a shell script tool handler -// This is a public wrapper for use by the CLI inspector command -func GenerateSafeInputShellToolScriptForInspector(toolConfig *SafeInputToolConfig) string { - return generateSafeInputShellToolScript(toolConfig) -} - -// GenerateSafeInputPythonToolScriptForInspector generates a Python script tool handler -// This is a public wrapper for use by the CLI inspector command -func GenerateSafeInputPythonToolScriptForInspector(toolConfig *SafeInputToolConfig) string { - return generateSafeInputPythonToolScript(toolConfig) -} diff --git a/pkg/workflow/safe_inputs_generator.go b/pkg/workflow/safe_inputs_generator.go new file mode 100644 index 0000000000..3205eb635a --- /dev/null +++ b/pkg/workflow/safe_inputs_generator.go @@ -0,0 +1,311 @@ +package workflow + +import ( + "encoding/json" + "fmt" + "sort" + "strings" + + "github.com/githubnext/gh-aw/pkg/constants" +) + +// SafeInputsToolJSON represents a tool configuration for the tools.json file +type SafeInputsToolJSON struct { + Name string `json:"name"` + Description string `json:"description"` + InputSchema map[string]any `json:"inputSchema"` + Handler string `json:"handler,omitempty"` + Env map[string]string `json:"env,omitempty"` + Timeout int `json:"timeout,omitempty"` +} + +// SafeInputsConfigJSON represents the tools.json configuration file structure +type SafeInputsConfigJSON struct { + ServerName string `json:"serverName"` + Version string `json:"version"` + LogDir string `json:"logDir,omitempty"` + Tools []SafeInputsToolJSON `json:"tools"` +} + +// generateSafeInputsToolsConfig generates the tools.json configuration for the safe-inputs MCP server +func generateSafeInputsToolsConfig(safeInputs *SafeInputsConfig) string { + config := SafeInputsConfigJSON{ + ServerName: "safeinputs", + Version: constants.SafeInputsMCPVersion, + LogDir: SafeInputsDirectory + "/logs", + Tools: []SafeInputsToolJSON{}, + } + + // Sort tool names for stable output + toolNames := make([]string, 0, len(safeInputs.Tools)) + for toolName := range safeInputs.Tools { + toolNames = append(toolNames, toolName) + } + sort.Strings(toolNames) + + for _, toolName := range toolNames { + toolConfig := safeInputs.Tools[toolName] + + // Build input schema + inputSchema := map[string]any{ + "type": "object", + "properties": make(map[string]any), + } + + props := inputSchema["properties"].(map[string]any) + var required []string + + // Sort input names for stable output + inputNames := make([]string, 0, len(toolConfig.Inputs)) + for paramName := range toolConfig.Inputs { + inputNames = append(inputNames, paramName) + } + sort.Strings(inputNames) + + for _, paramName := range inputNames { + param := toolConfig.Inputs[paramName] + propDef := map[string]any{ + "type": param.Type, + "description": param.Description, + } + if param.Default != nil { + propDef["default"] = param.Default + } + props[paramName] = propDef + if param.Required { + required = append(required, paramName) + } + } + + sort.Strings(required) + if len(required) > 0 { + inputSchema["required"] = required + } + + // Determine handler path based on script type + var handler string + if toolConfig.Script != "" { + handler = toolName + ".cjs" + } else if toolConfig.Run != "" { + handler = toolName + ".sh" + } else if toolConfig.Py != "" { + handler = toolName + ".py" + } + + // Build env list of required environment variables (not actual secrets) + // This documents which env vars the tool needs, but doesn't store secret values + // The actual values are passed as environment variables and accessed via process.env + var envRefs map[string]string + if len(toolConfig.Env) > 0 { + envRefs = make(map[string]string) + // Sort env var names for stable output + envVarNames := make([]string, 0, len(toolConfig.Env)) + for envVarName := range toolConfig.Env { + envVarNames = append(envVarNames, envVarName) + } + sort.Strings(envVarNames) + + for _, envVarName := range envVarNames { + // Store just the environment variable name without $ prefix or secret value + // Handlers access the actual value via process.env[envVarName] at runtime + envRefs[envVarName] = envVarName + } + } + + config.Tools = append(config.Tools, SafeInputsToolJSON{ + Name: toolName, + Description: toolConfig.Description, + InputSchema: inputSchema, + Handler: handler, + Env: envRefs, + Timeout: toolConfig.Timeout, + }) + } + + jsonBytes, err := json.MarshalIndent(config, "", " ") + if err != nil { + safeInputsLog.Printf("Error marshaling tools config: %v", err) + return "{}" + } + return string(jsonBytes) +} + +// generateSafeInputsMCPServerScript generates the entry point script for the safe-inputs MCP server +// This script uses HTTP transport exclusively +func generateSafeInputsMCPServerScript(safeInputs *SafeInputsConfig) string { + var sb strings.Builder + + // HTTP transport - server started in separate step + sb.WriteString(`// @ts-check +// Auto-generated safe-inputs MCP server entry point (HTTP transport) +// This script uses the reusable safe_inputs_mcp_server_http module + +const path = require("path"); +const { startHttpServer } = require("./safe_inputs_mcp_server_http.cjs"); + +// Configuration file path (generated alongside this script) +const configPath = path.join(__dirname, "tools.json"); + +// Get port and API key from environment variables +const port = parseInt(process.env.GH_AW_SAFE_INPUTS_PORT || "3000", 10); +const apiKey = process.env.GH_AW_SAFE_INPUTS_API_KEY || ""; + +// Start the HTTP server +startHttpServer(configPath, { + port: port, + stateless: false, + logDir: "/tmp/gh-aw/safe-inputs/logs" +}).catch(error => { + console.error("Failed to start safe-inputs HTTP server:", error); + process.exit(1); +}); +`) + + return sb.String() +} + +// generateSafeInputJavaScriptToolScript generates the JavaScript tool file for a safe-input tool +// The user's script code is automatically wrapped in a function with module.exports, +// so users can write simple code without worrying about exports. +// Input parameters are destructured and available as local variables. +func generateSafeInputJavaScriptToolScript(toolConfig *SafeInputToolConfig) string { + var sb strings.Builder + + sb.WriteString("// @ts-check\n") + sb.WriteString("// Auto-generated safe-input tool: " + toolConfig.Name + "\n\n") + sb.WriteString("/**\n") + sb.WriteString(" * " + toolConfig.Description + "\n") + sb.WriteString(" * @param {Object} inputs - Input parameters\n") + // Sort input names for stable code generation in JSDoc + inputNamesForDoc := make([]string, 0, len(toolConfig.Inputs)) + for paramName := range toolConfig.Inputs { + inputNamesForDoc = append(inputNamesForDoc, paramName) + } + sort.Strings(inputNamesForDoc) + for _, paramName := range inputNamesForDoc { + param := toolConfig.Inputs[paramName] + fmt.Fprintf(&sb, " * @param {%s} inputs.%s - %s\n", param.Type, paramName, param.Description) + } + sb.WriteString(" * @returns {Promise} Tool result\n") + sb.WriteString(" */\n") + sb.WriteString("async function execute(inputs) {\n") + + // Destructure inputs to make parameters available as local variables + if len(toolConfig.Inputs) > 0 { + var paramNames []string + for paramName := range toolConfig.Inputs { + safeName := sanitizeParameterName(paramName) + if safeName != paramName { + // If sanitized, use alias + paramNames = append(paramNames, fmt.Sprintf("%s: %s", paramName, safeName)) + } else { + paramNames = append(paramNames, paramName) + } + } + sort.Strings(paramNames) + fmt.Fprintf(&sb, " const { %s } = inputs || {};\n\n", strings.Join(paramNames, ", ")) + } + + // Indent the user's script code + sb.WriteString(" " + strings.ReplaceAll(toolConfig.Script, "\n", "\n ") + "\n") + sb.WriteString("}\n\n") + sb.WriteString("module.exports = { execute };\n") + + return sb.String() +} + +// generateSafeInputShellToolScript generates the shell script for a safe-input tool +func generateSafeInputShellToolScript(toolConfig *SafeInputToolConfig) string { + var sb strings.Builder + + sb.WriteString("#!/bin/bash\n") + sb.WriteString("# Auto-generated safe-input tool: " + toolConfig.Name + "\n") + sb.WriteString("# " + toolConfig.Description + "\n\n") + sb.WriteString("set -euo pipefail\n\n") + sb.WriteString(toolConfig.Run + "\n") + + return sb.String() +} + +// generateSafeInputPythonToolScript generates the Python script for a safe-input tool +// Python scripts receive inputs as a dictionary (parsed from JSON stdin): +// - Input parameters are available as a pre-parsed 'inputs' dictionary +// - Individual parameters can be destructured: param = inputs.get('param', default) +// - Outputs are printed to stdout as JSON +// - Environment variables from env: field are available via os.environ +func generateSafeInputPythonToolScript(toolConfig *SafeInputToolConfig) string { + var sb strings.Builder + + sb.WriteString("#!/usr/bin/env python3\n") + sb.WriteString("# Auto-generated safe-input tool: " + toolConfig.Name + "\n") + sb.WriteString("# " + toolConfig.Description + "\n\n") + sb.WriteString("import json\n") + sb.WriteString("import os\n") + sb.WriteString("import sys\n\n") + + // Add wrapper code to read inputs from stdin + sb.WriteString("# Read inputs from stdin (JSON format)\n") + sb.WriteString("try:\n") + sb.WriteString(" inputs = json.loads(sys.stdin.read()) if not sys.stdin.isatty() else {}\n") + sb.WriteString("except (json.JSONDecodeError, Exception):\n") + sb.WriteString(" inputs = {}\n\n") + + // Add helper comment about input parameters + if len(toolConfig.Inputs) > 0 { + sb.WriteString("# Input parameters available in 'inputs' dictionary:\n") + // Sort input names for stable code generation + inputNames := make([]string, 0, len(toolConfig.Inputs)) + for paramName := range toolConfig.Inputs { + inputNames = append(inputNames, paramName) + } + sort.Strings(inputNames) + for _, paramName := range inputNames { + param := toolConfig.Inputs[paramName] + defaultValue := "" + if param.Default != nil { + defaultValue = fmt.Sprintf(", default=%v", param.Default) + } + fmt.Fprintf(&sb, "# %s = inputs.get('%s'%s) # %s\n", + sanitizePythonVariableName(paramName), paramName, defaultValue, param.Description) + } + sb.WriteString("\n") + } + + // Add user's Python code + sb.WriteString("# User code:\n") + sb.WriteString(toolConfig.Py + "\n") + + return sb.String() +} + +// Public wrapper functions for CLI use + +// GenerateSafeInputsToolsConfigForInspector generates the tools.json configuration for the safe-inputs MCP server +// This is a public wrapper for use by the CLI inspector command +func GenerateSafeInputsToolsConfigForInspector(safeInputs *SafeInputsConfig) string { + return generateSafeInputsToolsConfig(safeInputs) +} + +// GenerateSafeInputsMCPServerScriptForInspector generates the MCP server entry point script +// This is a public wrapper for use by the CLI inspector command +func GenerateSafeInputsMCPServerScriptForInspector(safeInputs *SafeInputsConfig) string { + return generateSafeInputsMCPServerScript(safeInputs) +} + +// GenerateSafeInputJavaScriptToolScriptForInspector generates a JavaScript tool handler script +// This is a public wrapper for use by the CLI inspector command +func GenerateSafeInputJavaScriptToolScriptForInspector(toolConfig *SafeInputToolConfig) string { + return generateSafeInputJavaScriptToolScript(toolConfig) +} + +// GenerateSafeInputShellToolScriptForInspector generates a shell script tool handler +// This is a public wrapper for use by the CLI inspector command +func GenerateSafeInputShellToolScriptForInspector(toolConfig *SafeInputToolConfig) string { + return generateSafeInputShellToolScript(toolConfig) +} + +// GenerateSafeInputPythonToolScriptForInspector generates a Python script tool handler +// This is a public wrapper for use by the CLI inspector command +func GenerateSafeInputPythonToolScriptForInspector(toolConfig *SafeInputToolConfig) string { + return generateSafeInputPythonToolScript(toolConfig) +} diff --git a/pkg/workflow/safe_inputs_generator_test.go b/pkg/workflow/safe_inputs_generator_test.go new file mode 100644 index 0000000000..f4d810465f --- /dev/null +++ b/pkg/workflow/safe_inputs_generator_test.go @@ -0,0 +1,406 @@ +package workflow + +import ( + "strings" + "testing" +) + +func TestGenerateSafeInputsMCPServerScript(t *testing.T) { + config := &SafeInputsConfig{ + Tools: map[string]*SafeInputToolConfig{ + "search-issues": { + Name: "search-issues", + Description: "Search for issues in the repository", + Script: "return 'hello';", + Inputs: map[string]*SafeInputParam{ + "query": { + Type: "string", + Description: "Search query", + Required: true, + }, + }, + }, + "echo-message": { + Name: "echo-message", + Description: "Echo a message", + Run: "echo $INPUT_MESSAGE", + Inputs: map[string]*SafeInputParam{ + "message": { + Type: "string", + Description: "Message to echo", + Default: "Hello", + }, + }, + }, + "analyze-data": { + Name: "analyze-data", + Description: "Analyze data with Python", + Py: "import json\nprint(json.dumps({'result': 'success'}))", + Inputs: map[string]*SafeInputParam{ + "data": { + Type: "string", + Description: "Data to analyze", + Required: true, + }, + }, + }, + }, + } + + // Test the entry point script + script := generateSafeInputsMCPServerScript(config) + + // Check for HTTP server entry point structure + if !strings.Contains(script, "safe_inputs_mcp_server_http.cjs") { + t.Error("Script should reference the HTTP MCP server module") + } + + if !strings.Contains(script, "startHttpServer") { + t.Error("Script should use startHttpServer function") + } + + if !strings.Contains(script, "tools.json") { + t.Error("Script should reference tools.json configuration file") + } + + if !strings.Contains(script, "/tmp/gh-aw/safe-inputs/logs") { + t.Error("Script should specify log directory") + } + + if !strings.Contains(script, "GH_AW_SAFE_INPUTS_PORT") { + t.Error("Script should reference GH_AW_SAFE_INPUTS_PORT environment variable") + } + + if !strings.Contains(script, "GH_AW_SAFE_INPUTS_API_KEY") { + t.Error("Script should reference GH_AW_SAFE_INPUTS_API_KEY environment variable") + } + + // Test the tools configuration JSON + toolsJSON := generateSafeInputsToolsConfig(config) + + if !strings.Contains(toolsJSON, `"serverName": "safeinputs"`) { + t.Error("Tools config should contain server name 'safeinputs'") + } + + if !strings.Contains(toolsJSON, `"name": "search-issues"`) { + t.Error("Tools config should contain search-issues tool") + } + + if !strings.Contains(toolsJSON, `"name": "echo-message"`) { + t.Error("Tools config should contain echo-message tool") + } + + if !strings.Contains(toolsJSON, `"name": "analyze-data"`) { + t.Error("Tools config should contain analyze-data tool") + } + + // Check for JavaScript tool handler + if !strings.Contains(toolsJSON, `"handler": "search-issues.cjs"`) { + t.Error("Tools config should reference JavaScript tool handler file") + } + + // Check for shell tool handler + if !strings.Contains(toolsJSON, `"handler": "echo-message.sh"`) { + t.Error("Tools config should reference shell script handler file") + } + + // Check for Python tool handler + if !strings.Contains(toolsJSON, `"handler": "analyze-data.py"`) { + t.Error("Tools config should reference Python script handler file") + } + + // Check for input schema + if !strings.Contains(toolsJSON, `"description": "Search query"`) { + t.Error("Tools config should contain input descriptions") + } + + if !strings.Contains(toolsJSON, `"required"`) { + t.Error("Tools config should contain required fields array") + } +} + +func TestGenerateSafeInputsToolsConfigWithEnv(t *testing.T) { + config := &SafeInputsConfig{ + Tools: map[string]*SafeInputToolConfig{ + "github-query": { + Name: "github-query", + Description: "Query GitHub with authentication", + Run: "gh repo view $INPUT_REPO", + Inputs: map[string]*SafeInputParam{ + "repo": { + Type: "string", + Required: true, + }, + }, + Env: map[string]string{ + "GH_TOKEN": "${{ secrets.GITHUB_TOKEN }}", + "API_KEY": "${{ secrets.API_KEY }}", + }, + }, + }, + } + + toolsJSON := generateSafeInputsToolsConfig(config) + + // Verify that env field is present in tools.json + if !strings.Contains(toolsJSON, `"env"`) { + t.Error("Tools config should contain env field") + } + + // Verify that env contains environment variable names (not secrets or $ prefixes) + // The values should be just the variable names like "GH_TOKEN": "GH_TOKEN" + if !strings.Contains(toolsJSON, `"GH_TOKEN": "GH_TOKEN"`) { + t.Error("Tools config should contain GH_TOKEN env variable name") + } + + if !strings.Contains(toolsJSON, `"API_KEY": "API_KEY"`) { + t.Error("Tools config should contain API_KEY env variable name") + } + + // Verify that actual secret expressions are NOT in tools.json + if strings.Contains(toolsJSON, "secrets.GITHUB_TOKEN") { + t.Error("Tools config should NOT contain secret expressions") + } + + if strings.Contains(toolsJSON, "secrets.API_KEY") { + t.Error("Tools config should NOT contain secret expressions") + } + + // Verify that $ prefix is not used (which might suggest variable expansion) + if strings.Contains(toolsJSON, `"$GH_TOKEN"`) { + t.Error("Tools config should NOT contain $ prefix in env values") + } +} + +func TestGenerateSafeInputJavaScriptToolScript(t *testing.T) { + config := &SafeInputToolConfig{ + Name: "test-tool", + Description: "A test tool", + Script: "return inputs.value * 2;", + Inputs: map[string]*SafeInputParam{ + "value": { + Type: "number", + Description: "Value to double", + }, + }, + } + + script := generateSafeInputJavaScriptToolScript(config) + + if !strings.Contains(script, "test-tool") { + t.Error("Script should contain tool name") + } + + if !strings.Contains(script, "A test tool") { + t.Error("Script should contain description") + } + + if !strings.Contains(script, "return inputs.value * 2;") { + t.Error("Script should contain the tool script") + } + + if !strings.Contains(script, "module.exports") { + t.Error("Script should export execute function") + } +} + +func TestGenerateSafeInputShellToolScript(t *testing.T) { + config := &SafeInputToolConfig{ + Name: "test-shell", + Description: "A shell test tool", + Run: "echo $INPUT_MESSAGE", + } + + script := generateSafeInputShellToolScript(config) + + if !strings.Contains(script, "#!/bin/bash") { + t.Error("Script should have bash shebang") + } + + if !strings.Contains(script, "test-shell") { + t.Error("Script should contain tool name") + } + + if !strings.Contains(script, "set -euo pipefail") { + t.Error("Script should have strict mode") + } + + if !strings.Contains(script, "echo $INPUT_MESSAGE") { + t.Error("Script should contain the run command") + } +} + +func TestGenerateSafeInputPythonToolScript(t *testing.T) { + config := &SafeInputToolConfig{ + Name: "test-python", + Description: "A Python test tool", + Py: "result = {'message': 'Hello from Python'}\nprint(json.dumps(result))", + Inputs: map[string]*SafeInputParam{ + "message": { + Type: "string", + Description: "Message to process", + }, + "count": { + Type: "number", + Description: "Number of times", + }, + }, + } + + script := generateSafeInputPythonToolScript(config) + + if !strings.Contains(script, "#!/usr/bin/env python3") { + t.Error("Script should have python3 shebang") + } + + if !strings.Contains(script, "test-python") { + t.Error("Script should contain tool name") + } + + if !strings.Contains(script, "import json") { + t.Error("Script should import json module") + } + + if !strings.Contains(script, "import sys") { + t.Error("Script should import sys module") + } + + if !strings.Contains(script, "inputs = json.loads(sys.stdin.read())") { + t.Error("Script should parse inputs from stdin") + } + + if !strings.Contains(script, "result = {'message': 'Hello from Python'}") { + t.Error("Script should contain the Python code") + } + + // Check for input parameter documentation + if !strings.Contains(script, "# message = inputs.get('message'") { + t.Error("Script should document message parameter access") + } + + if !strings.Contains(script, "# count = inputs.get('count'") { + t.Error("Script should document count parameter access") + } +} + +// TestSafeInputsStableCodeGeneration verifies that code generation produces stable, deterministic output +// when called multiple times with the same input. This ensures tools and inputs are sorted properly. +func TestSafeInputsStableCodeGeneration(t *testing.T) { + // Create a config with multiple tools and inputs to ensure sorting is tested + config := &SafeInputsConfig{ + Tools: map[string]*SafeInputToolConfig{ + "zebra-tool": { + Name: "zebra-tool", + Description: "A tool that starts with Z", + Run: "echo zebra", + Inputs: map[string]*SafeInputParam{ + "zebra-input": {Type: "string", Description: "Zebra input"}, + "alpha-input": {Type: "number", Description: "Alpha input"}, + "beta-input": {Type: "boolean", Description: "Beta input"}, + }, + Env: map[string]string{ + "ZEBRA_SECRET": "${{ secrets.ZEBRA }}", + "ALPHA_SECRET": "${{ secrets.ALPHA }}", + }, + }, + "alpha-tool": { + Name: "alpha-tool", + Description: "A tool that starts with A", + Script: "return 'alpha';", + Inputs: map[string]*SafeInputParam{ + "charlie-param": {Type: "string", Description: "Charlie param"}, + "alpha-param": {Type: "string", Description: "Alpha param"}, + }, + }, + "middle-tool": { + Name: "middle-tool", + Description: "A tool in the middle", + Run: "echo middle", + }, + }, + } + + // Generate the entry point script multiple times and verify identical output + iterations := 10 + entryScripts := make([]string, iterations) + + for i := 0; i < iterations; i++ { + entryScripts[i] = generateSafeInputsMCPServerScript(config) + } + + // All entry point script iterations should produce identical output + for i := 1; i < iterations; i++ { + if entryScripts[i] != entryScripts[0] { + t.Errorf("generateSafeInputsMCPServerScript produced different output on iteration %d", i+1) + } + } + + // Generate the tools config JSON multiple times and verify identical output + toolsConfigs := make([]string, iterations) + + for i := 0; i < iterations; i++ { + toolsConfigs[i] = generateSafeInputsToolsConfig(config) + } + + // All tools config iterations should produce identical output + for i := 1; i < iterations; i++ { + if toolsConfigs[i] != toolsConfigs[0] { + t.Errorf("generateSafeInputsToolsConfig produced different output on iteration %d", i+1) + // Find first difference for debugging + for j := 0; j < len(toolsConfigs[0]) && j < len(toolsConfigs[i]); j++ { + if toolsConfigs[0][j] != toolsConfigs[i][j] { + start := j - 50 + if start < 0 { + start = 0 + } + end := j + 50 + if end > len(toolsConfigs[0]) { + end = len(toolsConfigs[0]) + } + if end > len(toolsConfigs[i]) { + end = len(toolsConfigs[i]) + } + t.Errorf("First difference at position %d:\n Expected: %q\n Got: %q", j, toolsConfigs[0][start:end], toolsConfigs[i][start:end]) + break + } + } + } + } + + // Verify tools appear in sorted order in tools.json (alpha-tool before middle-tool before zebra-tool) + alphaPos := strings.Index(toolsConfigs[0], `"name": "alpha-tool"`) + middlePos := strings.Index(toolsConfigs[0], `"name": "middle-tool"`) + zebraPos := strings.Index(toolsConfigs[0], `"name": "zebra-tool"`) + + if alphaPos == -1 || middlePos == -1 || zebraPos == -1 { + t.Error("Tools config should contain all tools") + } + + if alphaPos >= middlePos || middlePos >= zebraPos { + t.Errorf("Tools should be sorted alphabetically: alpha(%d) < middle(%d) < zebra(%d)", alphaPos, middlePos, zebraPos) + } + + // Test JavaScript tool script stability + jsScripts := make([]string, iterations) + for i := 0; i < iterations; i++ { + jsScripts[i] = generateSafeInputJavaScriptToolScript(config.Tools["alpha-tool"]) + } + + for i := 1; i < iterations; i++ { + if jsScripts[i] != jsScripts[0] { + t.Errorf("generateSafeInputJavaScriptToolScript produced different output on iteration %d", i+1) + } + } + + // Verify inputs in JSDoc are sorted + alphaParamPos := strings.Index(jsScripts[0], "inputs.alpha-param") + charlieParamPos := strings.Index(jsScripts[0], "inputs.charlie-param") + + if alphaParamPos == -1 || charlieParamPos == -1 { + t.Error("JavaScript script should contain all input parameters in JSDoc") + } + + if alphaParamPos >= charlieParamPos { + t.Errorf("Input parameters should be sorted alphabetically in JSDoc: alpha(%d) < charlie(%d)", alphaParamPos, charlieParamPos) + } +} diff --git a/pkg/workflow/safe_inputs_parser.go b/pkg/workflow/safe_inputs_parser.go new file mode 100644 index 0000000000..ceddb33ee3 --- /dev/null +++ b/pkg/workflow/safe_inputs_parser.go @@ -0,0 +1,413 @@ +package workflow + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/githubnext/gh-aw/pkg/logger" +) + +var safeInputsLog = logger.New("workflow:safe_inputs") + +// SafeInputsConfig holds the configuration for safe-inputs custom tools +type SafeInputsConfig struct { + Mode string // Transport mode: "http" (default) or "stdio" + Tools map[string]*SafeInputToolConfig +} + +// SafeInputToolConfig holds the configuration for a single safe-input tool +type SafeInputToolConfig struct { + Name string // Tool name (key from the config) + Description string // Required: tool description + Inputs map[string]*SafeInputParam // Optional: input parameters + Script string // JavaScript implementation (mutually exclusive with Run and Py) + Run string // Shell script implementation (mutually exclusive with Script and Py) + Py string // Python script implementation (mutually exclusive with Script and Run) + Env map[string]string // Environment variables (typically for secrets) + Timeout int // Timeout in seconds for tool execution (default: 60) +} + +// SafeInputParam holds the configuration for a tool input parameter +type SafeInputParam struct { + Type string // JSON schema type (string, number, boolean, array, object) + Description string // Description of the parameter + Required bool // Whether the parameter is required + Default any // Default value +} + +// SafeInputsMode constants define the available transport modes +const ( + SafeInputsModeHTTP = "http" +) + +// SafeInputsDirectory is the directory where safe-inputs files are generated +const SafeInputsDirectory = "/tmp/gh-aw/safe-inputs" + +// HasSafeInputs checks if safe-inputs are configured +func HasSafeInputs(safeInputs *SafeInputsConfig) bool { + return safeInputs != nil && len(safeInputs.Tools) > 0 +} + +// IsSafeInputsHTTPMode checks if safe-inputs is configured to use HTTP mode +// Note: All safe-inputs configurations now use HTTP mode exclusively +func IsSafeInputsHTTPMode(safeInputs *SafeInputsConfig) bool { + return safeInputs != nil +} + +// IsSafeInputsEnabled checks if safe-inputs are configured. +// Safe-inputs are enabled by default when configured in the workflow. +// The workflowData parameter is kept for backward compatibility but is not used. +func IsSafeInputsEnabled(safeInputs *SafeInputsConfig, workflowData *WorkflowData) bool { + return HasSafeInputs(safeInputs) +} + +// sanitizeParameterName converts a parameter name to a safe JavaScript identifier +// by replacing non-alphanumeric characters with underscores +func sanitizeParameterName(name string) string { + // Replace dashes and other non-alphanumeric chars with underscores + result := strings.Map(func(r rune) rune { + if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_' || r == '$' { + return r + } + return '_' + }, name) + + // Ensure it doesn't start with a number + if len(result) > 0 && result[0] >= '0' && result[0] <= '9' { + result = "_" + result + } + + return result +} + +// sanitizePythonVariableName converts a parameter name to a valid Python identifier +func sanitizePythonVariableName(name string) string { + // Replace dashes and other non-alphanumeric chars with underscores + result := strings.Map(func(r rune) rune { + if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_' { + return r + } + return '_' + }, name) + + // Ensure it doesn't start with a number + if len(result) > 0 && result[0] >= '0' && result[0] <= '9' { + result = "_" + result + } + + return result +} + +// parseSafeInputsMap parses safe-inputs configuration from a map. +// This is the shared implementation used by both ParseSafeInputs and extractSafeInputsConfig. +// Returns the config and a boolean indicating whether any tools were found. +func parseSafeInputsMap(safeInputsMap map[string]any) (*SafeInputsConfig, bool) { + config := &SafeInputsConfig{ + Mode: "http", // Only HTTP mode is supported + Tools: make(map[string]*SafeInputToolConfig), + } + + // Mode field is ignored - only HTTP mode is supported + // All safe-inputs configurations use HTTP transport + + for toolName, toolValue := range safeInputsMap { + // Skip the "mode" field as it's not a tool definition + if toolName == "mode" { + continue + } + + toolMap, ok := toolValue.(map[string]any) + if !ok { + continue + } + + toolConfig := &SafeInputToolConfig{ + Name: toolName, + Inputs: make(map[string]*SafeInputParam), + Env: make(map[string]string), + Timeout: 60, // Default timeout: 60 seconds + } + + // Parse description (required) + if desc, exists := toolMap["description"]; exists { + if descStr, ok := desc.(string); ok { + toolConfig.Description = descStr + } + } + + // Parse inputs (optional) + if inputs, exists := toolMap["inputs"]; exists { + if inputsMap, ok := inputs.(map[string]any); ok { + for paramName, paramValue := range inputsMap { + if paramMap, ok := paramValue.(map[string]any); ok { + param := &SafeInputParam{ + Type: "string", // default type + } + + if t, exists := paramMap["type"]; exists { + if tStr, ok := t.(string); ok { + param.Type = tStr + } + } + + if desc, exists := paramMap["description"]; exists { + if descStr, ok := desc.(string); ok { + param.Description = descStr + } + } + + if req, exists := paramMap["required"]; exists { + if reqBool, ok := req.(bool); ok { + param.Required = reqBool + } + } + + if def, exists := paramMap["default"]; exists { + param.Default = def + } + + toolConfig.Inputs[paramName] = param + } + } + } + } + + // Parse script (JavaScript implementation) + if script, exists := toolMap["script"]; exists { + if scriptStr, ok := script.(string); ok { + toolConfig.Script = scriptStr + } + } + + // Parse run (shell script implementation) + if run, exists := toolMap["run"]; exists { + if runStr, ok := run.(string); ok { + toolConfig.Run = runStr + } + } + + // Parse py (Python script implementation) + if py, exists := toolMap["py"]; exists { + if pyStr, ok := py.(string); ok { + toolConfig.Py = pyStr + } + } + + // Parse env (environment variables) + if env, exists := toolMap["env"]; exists { + if envMap, ok := env.(map[string]any); ok { + for envName, envValue := range envMap { + if envStr, ok := envValue.(string); ok { + toolConfig.Env[envName] = envStr + } + } + } + } + + // Parse timeout (optional, default is 60 seconds) + if timeout, exists := toolMap["timeout"]; exists { + switch t := timeout.(type) { + case int: + toolConfig.Timeout = t + case uint64: + toolConfig.Timeout = int(t) + case float64: + toolConfig.Timeout = int(t) + case string: + // Try to parse string as integer + _, _ = fmt.Sscanf(t, "%d", &toolConfig.Timeout) + } + } + + config.Tools[toolName] = toolConfig + } + + return config, len(config.Tools) > 0 +} + +// ParseSafeInputs parses safe-inputs configuration from frontmatter (standalone function for testing) +func ParseSafeInputs(frontmatter map[string]any) *SafeInputsConfig { + if frontmatter == nil { + return nil + } + + safeInputs, exists := frontmatter["safe-inputs"] + if !exists { + return nil + } + + safeInputsMap, ok := safeInputs.(map[string]any) + if !ok { + return nil + } + + config, _ := parseSafeInputsMap(safeInputsMap) + return config +} + +// extractSafeInputsConfig extracts safe-inputs configuration from frontmatter +func (c *Compiler) extractSafeInputsConfig(frontmatter map[string]any) *SafeInputsConfig { + safeInputsLog.Print("Extracting safe-inputs configuration from frontmatter") + + safeInputs, exists := frontmatter["safe-inputs"] + if !exists { + return nil + } + + safeInputsMap, ok := safeInputs.(map[string]any) + if !ok { + return nil + } + + config, hasTools := parseSafeInputsMap(safeInputsMap) + if !hasTools { + return nil + } + + safeInputsLog.Printf("Extracted %d safe-input tools", len(config.Tools)) + return config +} + +// mergeSafeInputs merges safe-inputs configuration from imports into the main configuration +func (c *Compiler) mergeSafeInputs(main *SafeInputsConfig, importedConfigs []string) *SafeInputsConfig { + if main == nil { + main = &SafeInputsConfig{ + Mode: "http", // Default to HTTP mode + Tools: make(map[string]*SafeInputToolConfig), + } + } + + for _, configJSON := range importedConfigs { + if configJSON == "" || configJSON == "{}" { + continue + } + + // Merge the imported JSON config + var importedMap map[string]any + if err := json.Unmarshal([]byte(configJSON), &importedMap); err != nil { + safeInputsLog.Printf("Warning: failed to parse imported safe-inputs config: %v", err) + continue + } + + // Mode field is ignored - only HTTP mode is supported + // All safe-inputs configurations use HTTP transport + + // Merge each tool from the imported config + for toolName, toolValue := range importedMap { + // Skip mode field as it's already handled + if toolName == "mode" { + continue + } + + // Skip if tool already exists in main config (main takes precedence) + if _, exists := main.Tools[toolName]; exists { + safeInputsLog.Printf("Skipping imported tool '%s' - already defined in main config", toolName) + continue + } + + toolMap, ok := toolValue.(map[string]any) + if !ok { + continue + } + + toolConfig := &SafeInputToolConfig{ + Name: toolName, + Inputs: make(map[string]*SafeInputParam), + Env: make(map[string]string), + Timeout: 60, // Default timeout: 60 seconds + } + + // Parse description + if desc, exists := toolMap["description"]; exists { + if descStr, ok := desc.(string); ok { + toolConfig.Description = descStr + } + } + + // Parse inputs + if inputs, exists := toolMap["inputs"]; exists { + if inputsMap, ok := inputs.(map[string]any); ok { + for paramName, paramValue := range inputsMap { + if paramMap, ok := paramValue.(map[string]any); ok { + param := &SafeInputParam{ + Type: "string", + } + if t, exists := paramMap["type"]; exists { + if tStr, ok := t.(string); ok { + param.Type = tStr + } + } + if desc, exists := paramMap["description"]; exists { + if descStr, ok := desc.(string); ok { + param.Description = descStr + } + } + if req, exists := paramMap["required"]; exists { + if reqBool, ok := req.(bool); ok { + param.Required = reqBool + } + } + if def, exists := paramMap["default"]; exists { + param.Default = def + } + toolConfig.Inputs[paramName] = param + } + } + } + } + + // Parse script + if script, exists := toolMap["script"]; exists { + if scriptStr, ok := script.(string); ok { + toolConfig.Script = scriptStr + } + } + + // Parse run + if run, exists := toolMap["run"]; exists { + if runStr, ok := run.(string); ok { + toolConfig.Run = runStr + } + } + + // Parse py + if py, exists := toolMap["py"]; exists { + if pyStr, ok := py.(string); ok { + toolConfig.Py = pyStr + } + } + + // Parse env + if env, exists := toolMap["env"]; exists { + if envMap, ok := env.(map[string]any); ok { + for envName, envValue := range envMap { + if envStr, ok := envValue.(string); ok { + toolConfig.Env[envName] = envStr + } + } + } + } + + // Parse timeout (optional, default is 60 seconds) + if timeout, exists := toolMap["timeout"]; exists { + switch t := timeout.(type) { + case int: + toolConfig.Timeout = t + case uint64: + toolConfig.Timeout = int(t) + case float64: + toolConfig.Timeout = int(t) + case string: + // Try to parse string as integer + _, _ = fmt.Sscanf(t, "%d", &toolConfig.Timeout) + } + } + + main.Tools[toolName] = toolConfig + safeInputsLog.Printf("Merged imported safe-input tool: %s", toolName) + } + } + + return main +} diff --git a/pkg/workflow/safe_inputs_parser_test.go b/pkg/workflow/safe_inputs_parser_test.go new file mode 100644 index 0000000000..4248692bae --- /dev/null +++ b/pkg/workflow/safe_inputs_parser_test.go @@ -0,0 +1,448 @@ +package workflow + +import ( + "testing" +) + +func TestParseSafeInputs(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + expectedTools int + expectedNil bool + }{ + { + name: "nil frontmatter", + frontmatter: nil, + expectedNil: true, + }, + { + name: "empty frontmatter", + frontmatter: map[string]any{}, + expectedNil: true, + }, + { + name: "single javascript tool", + frontmatter: map[string]any{ + "safe-inputs": map[string]any{ + "search-issues": map[string]any{ + "description": "Search for issues", + "script": "return 'hello';", + "inputs": map[string]any{ + "query": map[string]any{ + "type": "string", + "description": "Search query", + "required": true, + }, + }, + }, + }, + }, + expectedTools: 1, + }, + { + name: "single shell tool", + frontmatter: map[string]any{ + "safe-inputs": map[string]any{ + "echo-message": map[string]any{ + "description": "Echo a message", + "run": "echo $INPUT_MESSAGE", + "inputs": map[string]any{ + "message": map[string]any{ + "type": "string", + "description": "Message to echo", + "default": "Hello", + }, + }, + }, + }, + }, + expectedTools: 1, + }, + { + name: "single python tool", + frontmatter: map[string]any{ + "safe-inputs": map[string]any{ + "analyze-data": map[string]any{ + "description": "Analyze data with Python", + "py": "import json\nprint(json.dumps({'result': 'success'}))", + "inputs": map[string]any{ + "data": map[string]any{ + "type": "string", + "description": "Data to analyze", + "required": true, + }, + }, + }, + }, + }, + expectedTools: 1, + }, + { + name: "multiple tools", + frontmatter: map[string]any{ + "safe-inputs": map[string]any{ + "tool1": map[string]any{ + "description": "Tool 1", + "script": "return 1;", + }, + "tool2": map[string]any{ + "description": "Tool 2", + "run": "echo 2", + }, + }, + }, + expectedTools: 2, + }, + { + name: "tool with env secrets", + frontmatter: map[string]any{ + "safe-inputs": map[string]any{ + "api-call": map[string]any{ + "description": "Call API", + "script": "return fetch(url);", + "env": map[string]any{ + "API_KEY": "${{ secrets.API_KEY }}", + }, + }, + }, + }, + expectedTools: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := ParseSafeInputs(tt.frontmatter) + + if tt.expectedNil { + if result != nil { + t.Errorf("Expected nil, got %+v", result) + } + return + } + + if result == nil { + t.Error("Expected non-nil result") + return + } + + if len(result.Tools) != tt.expectedTools { + t.Errorf("Expected %d tools, got %d", tt.expectedTools, len(result.Tools)) + } + }) + } +} + +func TestHasSafeInputs(t *testing.T) { + tests := []struct { + name string + config *SafeInputsConfig + expected bool + }{ + { + name: "nil config", + config: nil, + expected: false, + }, + { + name: "empty tools", + config: &SafeInputsConfig{Tools: map[string]*SafeInputToolConfig{}}, + expected: false, + }, + { + name: "with tools", + config: &SafeInputsConfig{ + Tools: map[string]*SafeInputToolConfig{ + "test": {Name: "test", Description: "Test tool"}, + }, + }, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := HasSafeInputs(tt.config) + if result != tt.expected { + t.Errorf("Expected %v, got %v", tt.expected, result) + } + }) + } +} + +func TestIsSafeInputsEnabled(t *testing.T) { + // Test config with tools + configWithTools := &SafeInputsConfig{ + Tools: map[string]*SafeInputToolConfig{ + "test": {Name: "test", Description: "Test tool"}, + }, + } + + tests := []struct { + name string + config *SafeInputsConfig + workflowData *WorkflowData + expected bool + }{ + { + name: "nil config - not enabled", + config: nil, + workflowData: nil, + expected: false, + }, + { + name: "empty tools - not enabled", + config: &SafeInputsConfig{Tools: map[string]*SafeInputToolConfig{}}, + workflowData: nil, + expected: false, + }, + { + name: "with tools - enabled by default", + config: configWithTools, + workflowData: nil, + expected: true, + }, + { + name: "with tools and feature flag enabled - enabled (backward compat)", + config: configWithTools, + workflowData: &WorkflowData{ + Features: map[string]any{"safe-inputs": true}, + }, + expected: true, + }, + { + name: "with tools and feature flag disabled - still enabled (feature flag ignored)", + config: configWithTools, + workflowData: &WorkflowData{ + Features: map[string]any{"safe-inputs": false}, + }, + expected: true, + }, + { + name: "with tools and other features - enabled", + config: configWithTools, + workflowData: &WorkflowData{ + Features: map[string]any{"other-feature": true}, + }, + expected: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := IsSafeInputsEnabled(tt.config, tt.workflowData) + if result != tt.expected { + t.Errorf("Expected %v, got %v", tt.expected, result) + } + }) + } +} + +func TestIsSafeInputsEnabledWithEnv(t *testing.T) { + // Test config with tools + configWithTools := &SafeInputsConfig{ + Tools: map[string]*SafeInputToolConfig{ + "test": {Name: "test", Description: "Test tool"}, + }, + } + + // Safe-inputs are enabled by default when configured, environment variable no longer needed + t.Run("with tools - enabled regardless of GH_AW_FEATURES", func(t *testing.T) { + t.Setenv("GH_AW_FEATURES", "safe-inputs") + result := IsSafeInputsEnabled(configWithTools, nil) + if !result { + t.Errorf("Expected true, got false") + } + }) + + t.Run("with tools and GH_AW_FEATURES=other - still enabled", func(t *testing.T) { + t.Setenv("GH_AW_FEATURES", "other") + result := IsSafeInputsEnabled(configWithTools, nil) + if !result { + t.Errorf("Expected true, got false") + } + }) +} + +// TestParseSafeInputsAndExtractSafeInputsConfigConsistency verifies that ParseSafeInputs +// and extractSafeInputsConfig produce identical results for the same input. +// This ensures both functions use the shared helper correctly. +func TestParseSafeInputsAndExtractSafeInputsConfigConsistency(t *testing.T) { + tests := []struct { + name string + frontmatter map[string]any + }{ + { + name: "nil frontmatter", + frontmatter: nil, + }, + { + name: "empty frontmatter", + frontmatter: map[string]any{}, + }, + { + name: "single tool with all fields", + frontmatter: map[string]any{ + "safe-inputs": map[string]any{ + "search-issues": map[string]any{ + "description": "Search for issues", + "script": "return 'hello';", + "inputs": map[string]any{ + "query": map[string]any{ + "type": "string", + "description": "Search query", + "required": true, + "default": "test", + }, + }, + "env": map[string]any{ + "API_KEY": "${{ secrets.API_KEY }}", + }, + }, + }, + }, + }, + { + name: "multiple tools with different types", + frontmatter: map[string]any{ + "safe-inputs": map[string]any{ + "js-tool": map[string]any{ + "description": "JavaScript tool", + "script": "return 1;", + }, + "shell-tool": map[string]any{ + "description": "Shell tool", + "run": "echo hello", + }, + "python-tool": map[string]any{ + "description": "Python tool", + "py": "print('hello')", + }, + }, + }, + }, + { + name: "tool with complex inputs", + frontmatter: map[string]any{ + "safe-inputs": map[string]any{ + "complex-tool": map[string]any{ + "description": "Complex tool", + "script": "return inputs;", + "inputs": map[string]any{ + "string-param": map[string]any{ + "type": "string", + "description": "A string parameter", + }, + "number-param": map[string]any{ + "type": "number", + "description": "A number parameter", + "default": 42, + }, + "bool-param": map[string]any{ + "type": "boolean", + "description": "A boolean parameter", + "required": true, + }, + }, + }, + }, + }, + }, + } + + compiler := &Compiler{} + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result1 := ParseSafeInputs(tt.frontmatter) + result2 := compiler.extractSafeInputsConfig(tt.frontmatter) + + // Both should be nil or both non-nil + if (result1 == nil) != (result2 == nil) { + t.Errorf("Inconsistent nil results: ParseSafeInputs=%v, extractSafeInputsConfig=%v", result1 == nil, result2 == nil) + return + } + + if result1 == nil { + return + } + + // Compare number of tools + if len(result1.Tools) != len(result2.Tools) { + t.Errorf("Different number of tools: ParseSafeInputs=%d, extractSafeInputsConfig=%d", len(result1.Tools), len(result2.Tools)) + return + } + + // Compare each tool + for toolName, tool1 := range result1.Tools { + tool2, exists := result2.Tools[toolName] + if !exists { + t.Errorf("Tool %s not found in extractSafeInputsConfig result", toolName) + continue + } + + if tool1.Name != tool2.Name { + t.Errorf("Tool %s: Name mismatch: %s vs %s", toolName, tool1.Name, tool2.Name) + } + if tool1.Description != tool2.Description { + t.Errorf("Tool %s: Description mismatch: %s vs %s", toolName, tool1.Description, tool2.Description) + } + if tool1.Script != tool2.Script { + t.Errorf("Tool %s: Script mismatch: %s vs %s", toolName, tool1.Script, tool2.Script) + } + if tool1.Run != tool2.Run { + t.Errorf("Tool %s: Run mismatch: %s vs %s", toolName, tool1.Run, tool2.Run) + } + if tool1.Py != tool2.Py { + t.Errorf("Tool %s: Py mismatch: %s vs %s", toolName, tool1.Py, tool2.Py) + } + + // Compare inputs + if len(tool1.Inputs) != len(tool2.Inputs) { + t.Errorf("Tool %s: Different number of inputs: %d vs %d", toolName, len(tool1.Inputs), len(tool2.Inputs)) + continue + } + + for inputName, input1 := range tool1.Inputs { + input2, exists := tool2.Inputs[inputName] + if !exists { + t.Errorf("Tool %s: Input %s not found in extractSafeInputsConfig result", toolName, inputName) + continue + } + + if input1.Type != input2.Type { + t.Errorf("Tool %s, Input %s: Type mismatch: %s vs %s", toolName, inputName, input1.Type, input2.Type) + } + if input1.Description != input2.Description { + t.Errorf("Tool %s, Input %s: Description mismatch: %s vs %s", toolName, inputName, input1.Description, input2.Description) + } + if input1.Required != input2.Required { + t.Errorf("Tool %s, Input %s: Required mismatch: %v vs %v", toolName, inputName, input1.Required, input2.Required) + } + // Compare defaults (handle nil case) + if (input1.Default == nil) != (input2.Default == nil) { + t.Errorf("Tool %s, Input %s: Default nil mismatch: %v vs %v", toolName, inputName, input1.Default, input2.Default) + } + } + + // Compare env + if len(tool1.Env) != len(tool2.Env) { + t.Errorf("Tool %s: Different number of env vars: %d vs %d", toolName, len(tool1.Env), len(tool2.Env)) + continue + } + + for envName, envValue1 := range tool1.Env { + envValue2, exists := tool2.Env[envName] + if !exists { + t.Errorf("Tool %s: Env %s not found in extractSafeInputsConfig result", toolName, envName) + continue + } + if envValue1 != envValue2 { + t.Errorf("Tool %s, Env %s: Value mismatch: %s vs %s", toolName, envName, envValue1, envValue2) + } + } + } + }) + } +} diff --git a/pkg/workflow/safe_inputs_renderer.go b/pkg/workflow/safe_inputs_renderer.go new file mode 100644 index 0000000000..84dc5b074d --- /dev/null +++ b/pkg/workflow/safe_inputs_renderer.go @@ -0,0 +1,129 @@ +package workflow + +import ( + "sort" + "strings" + + "github.com/githubnext/gh-aw/pkg/constants" +) + +// getSafeInputsEnvVars returns the list of environment variables needed for safe-inputs +func getSafeInputsEnvVars(safeInputs *SafeInputsConfig) []string { + envVars := []string{} + seen := make(map[string]bool) + + if safeInputs == nil { + return envVars + } + + for _, toolConfig := range safeInputs.Tools { + for envName := range toolConfig.Env { + if !seen[envName] { + envVars = append(envVars, envName) + seen[envName] = true + } + } + } + + sort.Strings(envVars) + return envVars +} + +// collectSafeInputsSecrets collects all secrets from safe-inputs configuration +func collectSafeInputsSecrets(safeInputs *SafeInputsConfig) map[string]string { + secrets := make(map[string]string) + + if safeInputs == nil { + return secrets + } + + // Sort tool names for consistent behavior when same env var appears in multiple tools + toolNames := make([]string, 0, len(safeInputs.Tools)) + for toolName := range safeInputs.Tools { + toolNames = append(toolNames, toolName) + } + sort.Strings(toolNames) + + for _, toolName := range toolNames { + toolConfig := safeInputs.Tools[toolName] + // Sort env var names for consistent order within each tool + envNames := make([]string, 0, len(toolConfig.Env)) + for envName := range toolConfig.Env { + envNames = append(envNames, envName) + } + sort.Strings(envNames) + + for _, envName := range envNames { + secrets[envName] = toolConfig.Env[envName] + } + } + + return secrets +} + +// renderSafeInputsMCPConfigWithOptions generates the Safe Inputs MCP server configuration with engine-specific options +// Only supports HTTP transport mode +func renderSafeInputsMCPConfigWithOptions(yaml *strings.Builder, safeInputs *SafeInputsConfig, isLast bool, includeCopilotFields bool) { + envVars := getSafeInputsEnvVars(safeInputs) + + yaml.WriteString(" \"" + constants.SafeInputsMCPServerID + "\": {\n") + + // HTTP transport configuration - server started in separate step + // Add type field for HTTP (required by MCP specification for HTTP transport) + yaml.WriteString(" \"type\": \"http\",\n") + + // HTTP URL using environment variable + // Use host.docker.internal to allow access from firewall container + if includeCopilotFields { + // Copilot format: backslash-escaped shell variable reference + yaml.WriteString(" \"url\": \"http://host.docker.internal:\\${GH_AW_SAFE_INPUTS_PORT}\",\n") + } else { + // Claude/Custom format: direct shell variable reference + yaml.WriteString(" \"url\": \"http://host.docker.internal:$GH_AW_SAFE_INPUTS_PORT\",\n") + } + + // Add Authorization header with API key + yaml.WriteString(" \"headers\": {\n") + if includeCopilotFields { + // Copilot format: backslash-escaped shell variable reference + yaml.WriteString(" \"Authorization\": \"Bearer \\${GH_AW_SAFE_INPUTS_API_KEY}\"\n") + } else { + // Claude/Custom format: direct shell variable reference + yaml.WriteString(" \"Authorization\": \"Bearer $GH_AW_SAFE_INPUTS_API_KEY\"\n") + } + yaml.WriteString(" },\n") + + // Add tools field for Copilot + if includeCopilotFields { + yaml.WriteString(" \"tools\": [\"*\"],\n") + } + + // Add env block for environment variable passthrough + envVarsWithServerConfig := append([]string{"GH_AW_SAFE_INPUTS_PORT", "GH_AW_SAFE_INPUTS_API_KEY"}, envVars...) + yaml.WriteString(" \"env\": {\n") + + // Write environment variables with appropriate escaping + for i, envVar := range envVarsWithServerConfig { + isLastEnvVar := i == len(envVarsWithServerConfig)-1 + comma := "" + if !isLastEnvVar { + comma = "," + } + + if includeCopilotFields { + // Copilot format: backslash-escaped shell variable reference + yaml.WriteString(" \"" + envVar + "\": \"\\${" + envVar + "}\"" + comma + "\n") + } else { + // Claude/Custom format: direct shell variable reference + yaml.WriteString(" \"" + envVar + "\": \"$" + envVar + "\"" + comma + "\n") + } + } + + yaml.WriteString(" }\n") + + if isLast { + yaml.WriteString(" }\n") + } else { + yaml.WriteString(" },\n") + } +} diff --git a/pkg/workflow/safe_inputs_renderer_test.go b/pkg/workflow/safe_inputs_renderer_test.go new file mode 100644 index 0000000000..e44be0a16a --- /dev/null +++ b/pkg/workflow/safe_inputs_renderer_test.go @@ -0,0 +1,192 @@ +package workflow + +import ( + "testing" +) + +func TestGetSafeInputsEnvVars(t *testing.T) { + tests := []struct { + name string + config *SafeInputsConfig + expectedLen int + contains []string + }{ + { + name: "nil config", + config: nil, + expectedLen: 0, + }, + { + name: "tool with env", + config: &SafeInputsConfig{ + Tools: map[string]*SafeInputToolConfig{ + "test": { + Name: "test", + Env: map[string]string{ + "API_KEY": "${{ secrets.API_KEY }}", + "TOKEN": "${{ secrets.TOKEN }}", + }, + }, + }, + }, + expectedLen: 2, + contains: []string{"API_KEY", "TOKEN"}, + }, + { + name: "multiple tools with shared env", + config: &SafeInputsConfig{ + Tools: map[string]*SafeInputToolConfig{ + "tool1": { + Name: "tool1", + Env: map[string]string{"API_KEY": "key1"}, + }, + "tool2": { + Name: "tool2", + Env: map[string]string{"API_KEY": "key2"}, + }, + }, + }, + expectedLen: 1, // Should deduplicate + contains: []string{"API_KEY"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := getSafeInputsEnvVars(tt.config) + + if len(result) != tt.expectedLen { + t.Errorf("Expected %d env vars, got %d: %v", tt.expectedLen, len(result), result) + } + + for _, expected := range tt.contains { + found := false + for _, v := range result { + if v == expected { + found = true + break + } + } + if !found { + t.Errorf("Expected to contain %s, got %v", expected, result) + } + } + }) + } +} + +func TestCollectSafeInputsSecrets(t *testing.T) { + tests := []struct { + name string + config *SafeInputsConfig + expectedLen int + }{ + { + name: "nil config", + config: nil, + expectedLen: 0, + }, + { + name: "tool with secrets", + config: &SafeInputsConfig{ + Tools: map[string]*SafeInputToolConfig{ + "test": { + Name: "test", + Env: map[string]string{ + "API_KEY": "${{ secrets.API_KEY }}", + }, + }, + }, + }, + expectedLen: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := collectSafeInputsSecrets(tt.config) + + if len(result) != tt.expectedLen { + t.Errorf("Expected %d secrets, got %d", tt.expectedLen, len(result)) + } + }) + } +} + +func TestCollectSafeInputsSecretsStability(t *testing.T) { + config := &SafeInputsConfig{ + Tools: map[string]*SafeInputToolConfig{ + "zebra-tool": { + Name: "zebra-tool", + Env: map[string]string{ + "ZEBRA_SECRET": "${{ secrets.ZEBRA }}", + "ALPHA_SECRET": "${{ secrets.ALPHA }}", + }, + }, + "alpha-tool": { + Name: "alpha-tool", + Env: map[string]string{ + "BETA_SECRET": "${{ secrets.BETA }}", + }, + }, + }, + } + + // Test collectSafeInputsSecrets stability + iterations := 10 + secretResults := make([]map[string]string, iterations) + for i := 0; i < iterations; i++ { + secretResults[i] = collectSafeInputsSecrets(config) + } + + // All iterations should produce same key set + for i := 1; i < iterations; i++ { + if len(secretResults[i]) != len(secretResults[0]) { + t.Errorf("collectSafeInputsSecrets produced different number of secrets on iteration %d", i+1) + } + for key, val := range secretResults[0] { + if secretResults[i][key] != val { + t.Errorf("collectSafeInputsSecrets produced different value for key %s on iteration %d", key, i+1) + } + } + } +} + +func TestGetSafeInputsEnvVarsStability(t *testing.T) { + config := &SafeInputsConfig{ + Tools: map[string]*SafeInputToolConfig{ + "zebra-tool": { + Name: "zebra-tool", + Env: map[string]string{ + "ZEBRA_SECRET": "${{ secrets.ZEBRA }}", + "ALPHA_SECRET": "${{ secrets.ALPHA }}", + }, + }, + "alpha-tool": { + Name: "alpha-tool", + Env: map[string]string{ + "BETA_SECRET": "${{ secrets.BETA }}", + }, + }, + }, + } + + // Test getSafeInputsEnvVars stability + iterations := 10 + envResults := make([][]string, iterations) + for i := 0; i < iterations; i++ { + envResults[i] = getSafeInputsEnvVars(config) + } + + for i := 1; i < iterations; i++ { + if len(envResults[i]) != len(envResults[0]) { + t.Errorf("getSafeInputsEnvVars produced different number of env vars on iteration %d", i+1) + } + for j := range envResults[0] { + if envResults[i][j] != envResults[0][j] { + t.Errorf("getSafeInputsEnvVars produced different value at position %d on iteration %d: expected %s, got %s", + j, i+1, envResults[0][j], envResults[i][j]) + } + } + } +} diff --git a/pkg/workflow/safe_inputs_test.go b/pkg/workflow/safe_inputs_test.go deleted file mode 100644 index c98766199a..0000000000 --- a/pkg/workflow/safe_inputs_test.go +++ /dev/null @@ -1,994 +0,0 @@ -package workflow - -import ( - "strings" - "testing" -) - -func TestParseSafeInputs(t *testing.T) { - tests := []struct { - name string - frontmatter map[string]any - expectedTools int - expectedNil bool - }{ - { - name: "nil frontmatter", - frontmatter: nil, - expectedNil: true, - }, - { - name: "empty frontmatter", - frontmatter: map[string]any{}, - expectedNil: true, - }, - { - name: "single javascript tool", - frontmatter: map[string]any{ - "safe-inputs": map[string]any{ - "search-issues": map[string]any{ - "description": "Search for issues", - "script": "return 'hello';", - "inputs": map[string]any{ - "query": map[string]any{ - "type": "string", - "description": "Search query", - "required": true, - }, - }, - }, - }, - }, - expectedTools: 1, - }, - { - name: "single shell tool", - frontmatter: map[string]any{ - "safe-inputs": map[string]any{ - "echo-message": map[string]any{ - "description": "Echo a message", - "run": "echo $INPUT_MESSAGE", - "inputs": map[string]any{ - "message": map[string]any{ - "type": "string", - "description": "Message to echo", - "default": "Hello", - }, - }, - }, - }, - }, - expectedTools: 1, - }, - { - name: "single python tool", - frontmatter: map[string]any{ - "safe-inputs": map[string]any{ - "analyze-data": map[string]any{ - "description": "Analyze data with Python", - "py": "import json\nprint(json.dumps({'result': 'success'}))", - "inputs": map[string]any{ - "data": map[string]any{ - "type": "string", - "description": "Data to analyze", - "required": true, - }, - }, - }, - }, - }, - expectedTools: 1, - }, - { - name: "multiple tools", - frontmatter: map[string]any{ - "safe-inputs": map[string]any{ - "tool1": map[string]any{ - "description": "Tool 1", - "script": "return 1;", - }, - "tool2": map[string]any{ - "description": "Tool 2", - "run": "echo 2", - }, - }, - }, - expectedTools: 2, - }, - { - name: "tool with env secrets", - frontmatter: map[string]any{ - "safe-inputs": map[string]any{ - "api-call": map[string]any{ - "description": "Call API", - "script": "return fetch(url);", - "env": map[string]any{ - "API_KEY": "${{ secrets.API_KEY }}", - }, - }, - }, - }, - expectedTools: 1, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := ParseSafeInputs(tt.frontmatter) - - if tt.expectedNil { - if result != nil { - t.Errorf("Expected nil, got %+v", result) - } - return - } - - if result == nil { - t.Error("Expected non-nil result") - return - } - - if len(result.Tools) != tt.expectedTools { - t.Errorf("Expected %d tools, got %d", tt.expectedTools, len(result.Tools)) - } - }) - } -} - -func TestHasSafeInputs(t *testing.T) { - tests := []struct { - name string - config *SafeInputsConfig - expected bool - }{ - { - name: "nil config", - config: nil, - expected: false, - }, - { - name: "empty tools", - config: &SafeInputsConfig{Tools: map[string]*SafeInputToolConfig{}}, - expected: false, - }, - { - name: "with tools", - config: &SafeInputsConfig{ - Tools: map[string]*SafeInputToolConfig{ - "test": {Name: "test", Description: "Test tool"}, - }, - }, - expected: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := HasSafeInputs(tt.config) - if result != tt.expected { - t.Errorf("Expected %v, got %v", tt.expected, result) - } - }) - } -} - -func TestIsSafeInputsEnabled(t *testing.T) { - // Test config with tools - configWithTools := &SafeInputsConfig{ - Tools: map[string]*SafeInputToolConfig{ - "test": {Name: "test", Description: "Test tool"}, - }, - } - - tests := []struct { - name string - config *SafeInputsConfig - workflowData *WorkflowData - expected bool - }{ - { - name: "nil config - not enabled", - config: nil, - workflowData: nil, - expected: false, - }, - { - name: "empty tools - not enabled", - config: &SafeInputsConfig{Tools: map[string]*SafeInputToolConfig{}}, - workflowData: nil, - expected: false, - }, - { - name: "with tools - enabled by default", - config: configWithTools, - workflowData: nil, - expected: true, - }, - { - name: "with tools and feature flag enabled - enabled (backward compat)", - config: configWithTools, - workflowData: &WorkflowData{ - Features: map[string]any{"safe-inputs": true}, - }, - expected: true, - }, - { - name: "with tools and feature flag disabled - still enabled (feature flag ignored)", - config: configWithTools, - workflowData: &WorkflowData{ - Features: map[string]any{"safe-inputs": false}, - }, - expected: true, - }, - { - name: "with tools and other features - enabled", - config: configWithTools, - workflowData: &WorkflowData{ - Features: map[string]any{"other-feature": true}, - }, - expected: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := IsSafeInputsEnabled(tt.config, tt.workflowData) - if result != tt.expected { - t.Errorf("Expected %v, got %v", tt.expected, result) - } - }) - } -} - -func TestIsSafeInputsEnabledWithEnv(t *testing.T) { - // Test config with tools - configWithTools := &SafeInputsConfig{ - Tools: map[string]*SafeInputToolConfig{ - "test": {Name: "test", Description: "Test tool"}, - }, - } - - // Safe-inputs are enabled by default when configured, environment variable no longer needed - t.Run("with tools - enabled regardless of GH_AW_FEATURES", func(t *testing.T) { - t.Setenv("GH_AW_FEATURES", "safe-inputs") - result := IsSafeInputsEnabled(configWithTools, nil) - if !result { - t.Errorf("Expected true, got false") - } - }) - - t.Run("with tools and GH_AW_FEATURES=other - still enabled", func(t *testing.T) { - t.Setenv("GH_AW_FEATURES", "other") - result := IsSafeInputsEnabled(configWithTools, nil) - if !result { - t.Errorf("Expected true, got false") - } - }) -} - -// TestParseSafeInputsAndExtractSafeInputsConfigConsistency verifies that ParseSafeInputs -// and extractSafeInputsConfig produce identical results for the same input. -// This ensures both functions use the shared helper correctly. -func TestParseSafeInputsAndExtractSafeInputsConfigConsistency(t *testing.T) { - tests := []struct { - name string - frontmatter map[string]any - }{ - { - name: "nil frontmatter", - frontmatter: nil, - }, - { - name: "empty frontmatter", - frontmatter: map[string]any{}, - }, - { - name: "single tool with all fields", - frontmatter: map[string]any{ - "safe-inputs": map[string]any{ - "search-issues": map[string]any{ - "description": "Search for issues", - "script": "return 'hello';", - "inputs": map[string]any{ - "query": map[string]any{ - "type": "string", - "description": "Search query", - "required": true, - "default": "test", - }, - }, - "env": map[string]any{ - "API_KEY": "${{ secrets.API_KEY }}", - }, - }, - }, - }, - }, - { - name: "multiple tools with different types", - frontmatter: map[string]any{ - "safe-inputs": map[string]any{ - "js-tool": map[string]any{ - "description": "JavaScript tool", - "script": "return 1;", - }, - "shell-tool": map[string]any{ - "description": "Shell tool", - "run": "echo hello", - }, - "python-tool": map[string]any{ - "description": "Python tool", - "py": "print('hello')", - }, - }, - }, - }, - { - name: "tool with complex inputs", - frontmatter: map[string]any{ - "safe-inputs": map[string]any{ - "complex-tool": map[string]any{ - "description": "Complex tool", - "script": "return inputs;", - "inputs": map[string]any{ - "string-param": map[string]any{ - "type": "string", - "description": "A string parameter", - }, - "number-param": map[string]any{ - "type": "number", - "description": "A number parameter", - "default": 42, - }, - "bool-param": map[string]any{ - "type": "boolean", - "description": "A boolean parameter", - "required": true, - }, - }, - }, - }, - }, - }, - } - - compiler := &Compiler{} - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result1 := ParseSafeInputs(tt.frontmatter) - result2 := compiler.extractSafeInputsConfig(tt.frontmatter) - - // Both should be nil or both non-nil - if (result1 == nil) != (result2 == nil) { - t.Errorf("Inconsistent nil results: ParseSafeInputs=%v, extractSafeInputsConfig=%v", result1 == nil, result2 == nil) - return - } - - if result1 == nil { - return - } - - // Compare number of tools - if len(result1.Tools) != len(result2.Tools) { - t.Errorf("Different number of tools: ParseSafeInputs=%d, extractSafeInputsConfig=%d", len(result1.Tools), len(result2.Tools)) - return - } - - // Compare each tool - for toolName, tool1 := range result1.Tools { - tool2, exists := result2.Tools[toolName] - if !exists { - t.Errorf("Tool %s not found in extractSafeInputsConfig result", toolName) - continue - } - - if tool1.Name != tool2.Name { - t.Errorf("Tool %s: Name mismatch: %s vs %s", toolName, tool1.Name, tool2.Name) - } - if tool1.Description != tool2.Description { - t.Errorf("Tool %s: Description mismatch: %s vs %s", toolName, tool1.Description, tool2.Description) - } - if tool1.Script != tool2.Script { - t.Errorf("Tool %s: Script mismatch: %s vs %s", toolName, tool1.Script, tool2.Script) - } - if tool1.Run != tool2.Run { - t.Errorf("Tool %s: Run mismatch: %s vs %s", toolName, tool1.Run, tool2.Run) - } - if tool1.Py != tool2.Py { - t.Errorf("Tool %s: Py mismatch: %s vs %s", toolName, tool1.Py, tool2.Py) - } - - // Compare inputs - if len(tool1.Inputs) != len(tool2.Inputs) { - t.Errorf("Tool %s: Different number of inputs: %d vs %d", toolName, len(tool1.Inputs), len(tool2.Inputs)) - continue - } - - for inputName, input1 := range tool1.Inputs { - input2, exists := tool2.Inputs[inputName] - if !exists { - t.Errorf("Tool %s: Input %s not found in extractSafeInputsConfig result", toolName, inputName) - continue - } - - if input1.Type != input2.Type { - t.Errorf("Tool %s, Input %s: Type mismatch: %s vs %s", toolName, inputName, input1.Type, input2.Type) - } - if input1.Description != input2.Description { - t.Errorf("Tool %s, Input %s: Description mismatch: %s vs %s", toolName, inputName, input1.Description, input2.Description) - } - if input1.Required != input2.Required { - t.Errorf("Tool %s, Input %s: Required mismatch: %v vs %v", toolName, inputName, input1.Required, input2.Required) - } - // Compare defaults (handle nil case) - if (input1.Default == nil) != (input2.Default == nil) { - t.Errorf("Tool %s, Input %s: Default nil mismatch: %v vs %v", toolName, inputName, input1.Default, input2.Default) - } - } - - // Compare env - if len(tool1.Env) != len(tool2.Env) { - t.Errorf("Tool %s: Different number of env vars: %d vs %d", toolName, len(tool1.Env), len(tool2.Env)) - continue - } - - for envName, envValue1 := range tool1.Env { - envValue2, exists := tool2.Env[envName] - if !exists { - t.Errorf("Tool %s: Env %s not found in extractSafeInputsConfig result", toolName, envName) - continue - } - if envValue1 != envValue2 { - t.Errorf("Tool %s, Env %s: Value mismatch: %s vs %s", toolName, envName, envValue1, envValue2) - } - } - } - }) - } -} - -func TestGetSafeInputsEnvVars(t *testing.T) { - tests := []struct { - name string - config *SafeInputsConfig - expectedLen int - contains []string - }{ - { - name: "nil config", - config: nil, - expectedLen: 0, - }, - { - name: "tool with env", - config: &SafeInputsConfig{ - Tools: map[string]*SafeInputToolConfig{ - "test": { - Name: "test", - Env: map[string]string{ - "API_KEY": "${{ secrets.API_KEY }}", - "TOKEN": "${{ secrets.TOKEN }}", - }, - }, - }, - }, - expectedLen: 2, - contains: []string{"API_KEY", "TOKEN"}, - }, - { - name: "multiple tools with shared env", - config: &SafeInputsConfig{ - Tools: map[string]*SafeInputToolConfig{ - "tool1": { - Name: "tool1", - Env: map[string]string{"API_KEY": "key1"}, - }, - "tool2": { - Name: "tool2", - Env: map[string]string{"API_KEY": "key2"}, - }, - }, - }, - expectedLen: 1, // Should deduplicate - contains: []string{"API_KEY"}, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := getSafeInputsEnvVars(tt.config) - - if len(result) != tt.expectedLen { - t.Errorf("Expected %d env vars, got %d: %v", tt.expectedLen, len(result), result) - } - - for _, expected := range tt.contains { - found := false - for _, v := range result { - if v == expected { - found = true - break - } - } - if !found { - t.Errorf("Expected to contain %s, got %v", expected, result) - } - } - }) - } -} - -func TestCollectSafeInputsSecrets(t *testing.T) { - tests := []struct { - name string - config *SafeInputsConfig - expectedLen int - }{ - { - name: "nil config", - config: nil, - expectedLen: 0, - }, - { - name: "tool with secrets", - config: &SafeInputsConfig{ - Tools: map[string]*SafeInputToolConfig{ - "test": { - Name: "test", - Env: map[string]string{ - "API_KEY": "${{ secrets.API_KEY }}", - }, - }, - }, - }, - expectedLen: 1, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := collectSafeInputsSecrets(tt.config) - - if len(result) != tt.expectedLen { - t.Errorf("Expected %d secrets, got %d", tt.expectedLen, len(result)) - } - }) - } -} - -func TestGenerateSafeInputsMCPServerScript(t *testing.T) { - config := &SafeInputsConfig{ - Tools: map[string]*SafeInputToolConfig{ - "search-issues": { - Name: "search-issues", - Description: "Search for issues in the repository", - Script: "return 'hello';", - Inputs: map[string]*SafeInputParam{ - "query": { - Type: "string", - Description: "Search query", - Required: true, - }, - }, - }, - "echo-message": { - Name: "echo-message", - Description: "Echo a message", - Run: "echo $INPUT_MESSAGE", - Inputs: map[string]*SafeInputParam{ - "message": { - Type: "string", - Description: "Message to echo", - Default: "Hello", - }, - }, - }, - "analyze-data": { - Name: "analyze-data", - Description: "Analyze data with Python", - Py: "import json\nprint(json.dumps({'result': 'success'}))", - Inputs: map[string]*SafeInputParam{ - "data": { - Type: "string", - Description: "Data to analyze", - Required: true, - }, - }, - }, - }, - } - - // Test the entry point script - script := generateSafeInputsMCPServerScript(config) - - // Check for HTTP server entry point structure - if !strings.Contains(script, "safe_inputs_mcp_server_http.cjs") { - t.Error("Script should reference the HTTP MCP server module") - } - - if !strings.Contains(script, "startHttpServer") { - t.Error("Script should use startHttpServer function") - } - - if !strings.Contains(script, "tools.json") { - t.Error("Script should reference tools.json configuration file") - } - - if !strings.Contains(script, "/tmp/gh-aw/safe-inputs/logs") { - t.Error("Script should specify log directory") - } - - if !strings.Contains(script, "GH_AW_SAFE_INPUTS_PORT") { - t.Error("Script should reference GH_AW_SAFE_INPUTS_PORT environment variable") - } - - if !strings.Contains(script, "GH_AW_SAFE_INPUTS_API_KEY") { - t.Error("Script should reference GH_AW_SAFE_INPUTS_API_KEY environment variable") - } - - // Test the tools configuration JSON - toolsJSON := generateSafeInputsToolsConfig(config) - - if !strings.Contains(toolsJSON, `"serverName": "safeinputs"`) { - t.Error("Tools config should contain server name 'safeinputs'") - } - - if !strings.Contains(toolsJSON, `"name": "search-issues"`) { - t.Error("Tools config should contain search-issues tool") - } - - if !strings.Contains(toolsJSON, `"name": "echo-message"`) { - t.Error("Tools config should contain echo-message tool") - } - - if !strings.Contains(toolsJSON, `"name": "analyze-data"`) { - t.Error("Tools config should contain analyze-data tool") - } - - // Check for JavaScript tool handler - if !strings.Contains(toolsJSON, `"handler": "search-issues.cjs"`) { - t.Error("Tools config should reference JavaScript tool handler file") - } - - // Check for shell tool handler - if !strings.Contains(toolsJSON, `"handler": "echo-message.sh"`) { - t.Error("Tools config should reference shell script handler file") - } - - // Check for Python tool handler - if !strings.Contains(toolsJSON, `"handler": "analyze-data.py"`) { - t.Error("Tools config should reference Python script handler file") - } - - // Check for input schema - if !strings.Contains(toolsJSON, `"description": "Search query"`) { - t.Error("Tools config should contain input descriptions") - } - - if !strings.Contains(toolsJSON, `"required"`) { - t.Error("Tools config should contain required fields array") - } -} - -func TestGenerateSafeInputsToolsConfigWithEnv(t *testing.T) { - config := &SafeInputsConfig{ - Tools: map[string]*SafeInputToolConfig{ - "github-query": { - Name: "github-query", - Description: "Query GitHub with authentication", - Run: "gh repo view $INPUT_REPO", - Inputs: map[string]*SafeInputParam{ - "repo": { - Type: "string", - Required: true, - }, - }, - Env: map[string]string{ - "GH_TOKEN": "${{ secrets.GITHUB_TOKEN }}", - "API_KEY": "${{ secrets.API_KEY }}", - }, - }, - }, - } - - toolsJSON := generateSafeInputsToolsConfig(config) - - // Verify that env field is present in tools.json - if !strings.Contains(toolsJSON, `"env"`) { - t.Error("Tools config should contain env field") - } - - // Verify that env contains environment variable names (not secrets or $ prefixes) - // The values should be just the variable names like "GH_TOKEN": "GH_TOKEN" - if !strings.Contains(toolsJSON, `"GH_TOKEN": "GH_TOKEN"`) { - t.Error("Tools config should contain GH_TOKEN env variable name") - } - - if !strings.Contains(toolsJSON, `"API_KEY": "API_KEY"`) { - t.Error("Tools config should contain API_KEY env variable name") - } - - // Verify that actual secret expressions are NOT in tools.json - if strings.Contains(toolsJSON, "secrets.GITHUB_TOKEN") { - t.Error("Tools config should NOT contain secret expressions") - } - - if strings.Contains(toolsJSON, "secrets.API_KEY") { - t.Error("Tools config should NOT contain secret expressions") - } - - // Verify that $ prefix is not used (which might suggest variable expansion) - if strings.Contains(toolsJSON, `"$GH_TOKEN"`) { - t.Error("Tools config should NOT contain $ prefix in env values") - } -} - -func TestGenerateSafeInputJavaScriptToolScript(t *testing.T) { - config := &SafeInputToolConfig{ - Name: "test-tool", - Description: "A test tool", - Script: "return inputs.value * 2;", - Inputs: map[string]*SafeInputParam{ - "value": { - Type: "number", - Description: "Value to double", - }, - }, - } - - script := generateSafeInputJavaScriptToolScript(config) - - if !strings.Contains(script, "test-tool") { - t.Error("Script should contain tool name") - } - - if !strings.Contains(script, "A test tool") { - t.Error("Script should contain description") - } - - if !strings.Contains(script, "return inputs.value * 2;") { - t.Error("Script should contain the tool script") - } - - if !strings.Contains(script, "module.exports") { - t.Error("Script should export execute function") - } -} - -func TestGenerateSafeInputShellToolScript(t *testing.T) { - config := &SafeInputToolConfig{ - Name: "test-shell", - Description: "A shell test tool", - Run: "echo $INPUT_MESSAGE", - } - - script := generateSafeInputShellToolScript(config) - - if !strings.Contains(script, "#!/bin/bash") { - t.Error("Script should have bash shebang") - } - - if !strings.Contains(script, "test-shell") { - t.Error("Script should contain tool name") - } - - if !strings.Contains(script, "set -euo pipefail") { - t.Error("Script should have strict mode") - } - - if !strings.Contains(script, "echo $INPUT_MESSAGE") { - t.Error("Script should contain the run command") - } -} - -func TestGenerateSafeInputPythonToolScript(t *testing.T) { - config := &SafeInputToolConfig{ - Name: "test-python", - Description: "A Python test tool", - Py: "result = {'message': 'Hello from Python'}\nprint(json.dumps(result))", - Inputs: map[string]*SafeInputParam{ - "message": { - Type: "string", - Description: "Message to process", - }, - "count": { - Type: "number", - Description: "Number of times", - }, - }, - } - - script := generateSafeInputPythonToolScript(config) - - if !strings.Contains(script, "#!/usr/bin/env python3") { - t.Error("Script should have python3 shebang") - } - - if !strings.Contains(script, "test-python") { - t.Error("Script should contain tool name") - } - - if !strings.Contains(script, "import json") { - t.Error("Script should import json module") - } - - if !strings.Contains(script, "import sys") { - t.Error("Script should import sys module") - } - - if !strings.Contains(script, "inputs = json.loads(sys.stdin.read())") { - t.Error("Script should parse inputs from stdin") - } - - if !strings.Contains(script, "result = {'message': 'Hello from Python'}") { - t.Error("Script should contain the Python code") - } - - // Check for input parameter documentation - if !strings.Contains(script, "# message = inputs.get('message'") { - t.Error("Script should document message parameter access") - } - - if !strings.Contains(script, "# count = inputs.get('count'") { - t.Error("Script should document count parameter access") - } -} - -// TestSafeInputsStableCodeGeneration verifies that code generation produces stable, deterministic output -// when called multiple times with the same input. This ensures tools and inputs are sorted properly. -func TestSafeInputsStableCodeGeneration(t *testing.T) { - // Create a config with multiple tools and inputs to ensure sorting is tested - config := &SafeInputsConfig{ - Tools: map[string]*SafeInputToolConfig{ - "zebra-tool": { - Name: "zebra-tool", - Description: "A tool that starts with Z", - Run: "echo zebra", - Inputs: map[string]*SafeInputParam{ - "zebra-input": {Type: "string", Description: "Zebra input"}, - "alpha-input": {Type: "number", Description: "Alpha input"}, - "beta-input": {Type: "boolean", Description: "Beta input"}, - }, - Env: map[string]string{ - "ZEBRA_SECRET": "${{ secrets.ZEBRA }}", - "ALPHA_SECRET": "${{ secrets.ALPHA }}", - }, - }, - "alpha-tool": { - Name: "alpha-tool", - Description: "A tool that starts with A", - Script: "return 'alpha';", - Inputs: map[string]*SafeInputParam{ - "charlie-param": {Type: "string", Description: "Charlie param"}, - "alpha-param": {Type: "string", Description: "Alpha param"}, - }, - }, - "middle-tool": { - Name: "middle-tool", - Description: "A tool in the middle", - Run: "echo middle", - }, - }, - } - - // Generate the entry point script multiple times and verify identical output - iterations := 10 - entryScripts := make([]string, iterations) - - for i := 0; i < iterations; i++ { - entryScripts[i] = generateSafeInputsMCPServerScript(config) - } - - // All entry point script iterations should produce identical output - for i := 1; i < iterations; i++ { - if entryScripts[i] != entryScripts[0] { - t.Errorf("generateSafeInputsMCPServerScript produced different output on iteration %d", i+1) - } - } - - // Generate the tools config JSON multiple times and verify identical output - toolsConfigs := make([]string, iterations) - - for i := 0; i < iterations; i++ { - toolsConfigs[i] = generateSafeInputsToolsConfig(config) - } - - // All tools config iterations should produce identical output - for i := 1; i < iterations; i++ { - if toolsConfigs[i] != toolsConfigs[0] { - t.Errorf("generateSafeInputsToolsConfig produced different output on iteration %d", i+1) - // Find first difference for debugging - for j := 0; j < len(toolsConfigs[0]) && j < len(toolsConfigs[i]); j++ { - if toolsConfigs[0][j] != toolsConfigs[i][j] { - start := j - 50 - if start < 0 { - start = 0 - } - end := j + 50 - if end > len(toolsConfigs[0]) { - end = len(toolsConfigs[0]) - } - if end > len(toolsConfigs[i]) { - end = len(toolsConfigs[i]) - } - t.Errorf("First difference at position %d:\n Expected: %q\n Got: %q", j, toolsConfigs[0][start:end], toolsConfigs[i][start:end]) - break - } - } - } - } - - // Verify tools appear in sorted order in tools.json (alpha-tool before middle-tool before zebra-tool) - alphaPos := strings.Index(toolsConfigs[0], `"name": "alpha-tool"`) - middlePos := strings.Index(toolsConfigs[0], `"name": "middle-tool"`) - zebraPos := strings.Index(toolsConfigs[0], `"name": "zebra-tool"`) - - if alphaPos == -1 || middlePos == -1 || zebraPos == -1 { - t.Error("Tools config should contain all tools") - } - - if alphaPos >= middlePos || middlePos >= zebraPos { - t.Errorf("Tools should be sorted alphabetically: alpha(%d) < middle(%d) < zebra(%d)", alphaPos, middlePos, zebraPos) - } - - // Test JavaScript tool script stability - jsScripts := make([]string, iterations) - for i := 0; i < iterations; i++ { - jsScripts[i] = generateSafeInputJavaScriptToolScript(config.Tools["alpha-tool"]) - } - - for i := 1; i < iterations; i++ { - if jsScripts[i] != jsScripts[0] { - t.Errorf("generateSafeInputJavaScriptToolScript produced different output on iteration %d", i+1) - } - } - - // Verify inputs in JSDoc are sorted - alphaParamPos := strings.Index(jsScripts[0], "inputs.alpha-param") - charlieParamPos := strings.Index(jsScripts[0], "inputs.charlie-param") - - if alphaParamPos == -1 || charlieParamPos == -1 { - t.Error("JavaScript script should contain all input parameters in JSDoc") - } - - if alphaParamPos >= charlieParamPos { - t.Errorf("Input parameters should be sorted alphabetically in JSDoc: alpha(%d) < charlie(%d)", alphaParamPos, charlieParamPos) - } - - // Test collectSafeInputsSecrets stability - secretResults := make([]map[string]string, iterations) - for i := 0; i < iterations; i++ { - secretResults[i] = collectSafeInputsSecrets(config) - } - - // All iterations should produce same key set - for i := 1; i < iterations; i++ { - if len(secretResults[i]) != len(secretResults[0]) { - t.Errorf("collectSafeInputsSecrets produced different number of secrets on iteration %d", i+1) - } - for key, val := range secretResults[0] { - if secretResults[i][key] != val { - t.Errorf("collectSafeInputsSecrets produced different value for key %s on iteration %d", key, i+1) - } - } - } - - // Test getSafeInputsEnvVars stability - envResults := make([][]string, iterations) - for i := 0; i < iterations; i++ { - envResults[i] = getSafeInputsEnvVars(config) - } - - for i := 1; i < iterations; i++ { - if len(envResults[i]) != len(envResults[0]) { - t.Errorf("getSafeInputsEnvVars produced different number of env vars on iteration %d", i+1) - } - for j := range envResults[0] { - if envResults[i][j] != envResults[0][j] { - t.Errorf("getSafeInputsEnvVars produced different value at position %d on iteration %d: expected %s, got %s", - j, i+1, envResults[0][j], envResults[i][j]) - } - } - } -}