-
Notifications
You must be signed in to change notification settings - Fork 3
Introduce a new persistent execution model #47
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
faeb4e8
4c876a3
b4657f1
ddf5864
ded3c2a
2b542c6
fe1a794
ee4e503
11af74b
4c9a4c5
d62d549
60dec15
937370f
7f7d5cb
f8ac359
dec432c
8686408
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,14 +51,26 @@ type CapturedFile struct { | |
| } | ||
|
|
||
| // HandleExecRequest handles the execution of scripts and streams output via SSE | ||
| func HandleExecRequest(registry *ExecutableRegistry, runbookPath string, useExecutableRegistry bool, cliOutputPath string) gin.HandlerFunc { | ||
| func HandleExecRequest(registry *ExecutableRegistry, runbookPath string, useExecutableRegistry bool, cliOutputPath string, sessionManager *SessionManager) gin.HandlerFunc { | ||
| return func(c *gin.Context) { | ||
| var req ExecRequest | ||
| if err := c.ShouldBindJSON(&req); err != nil { | ||
| c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) | ||
| return | ||
| } | ||
|
|
||
| // Validate session token if Authorization header is provided | ||
| var session *Session | ||
| token := extractBearerToken(c) | ||
| if token != "" { | ||
| var valid bool | ||
| session, valid = sessionManager.ValidateToken(token) | ||
| if !valid { | ||
| c.JSON(http.StatusUnauthorized, gin.H{"error": "Invalid or expired session token. Try refreshing the page or restarting Runbooks."}) | ||
| return | ||
| } | ||
| } | ||
|
|
||
| var executable *Executable | ||
| var err error | ||
|
|
||
|
|
@@ -137,6 +149,28 @@ func HandleExecRequest(registry *ExecutableRegistry, runbookPath string, useExec | |
| c.Header("Connection", "keep-alive") | ||
| c.Header("Transfer-Encoding", "chunked") | ||
|
|
||
| // Create temp files for environment capture (used to capture env changes after script execution) | ||
| var envCapturePath, pwdCapturePath string | ||
| if session != nil { | ||
| envFile, err := os.CreateTemp("", "runbook-env-capture-*.txt") | ||
| if err != nil { | ||
| sendSSEError(c, fmt.Sprintf("Failed to create env capture file: %v", err)) | ||
| return | ||
| } | ||
| envCapturePath = envFile.Name() | ||
| envFile.Close() | ||
| defer os.Remove(envCapturePath) | ||
|
|
||
| pwdFile, err := os.CreateTemp("", "runbook-pwd-capture-*.txt") | ||
| if err != nil { | ||
| sendSSEError(c, fmt.Sprintf("Failed to create pwd capture file: %v", err)) | ||
| return | ||
| } | ||
| pwdCapturePath = pwdFile.Name() | ||
| pwdFile.Close() | ||
| defer os.Remove(pwdCapturePath) | ||
| } | ||
|
|
||
| // Create a temporary file for the script | ||
| tmpFile, err := os.CreateTemp("", "runbook-check-*.sh") | ||
| if err != nil { | ||
|
|
@@ -145,8 +179,23 @@ func HandleExecRequest(registry *ExecutableRegistry, runbookPath string, useExec | |
| } | ||
| defer os.Remove(tmpFile.Name()) | ||
|
|
||
| // Detect interpreter from shebang or use language from executable | ||
| // We need this BEFORE deciding whether to wrap, so we can skip wrapping for non-bash scripts | ||
| interpreter, args := detectInterpreter(scriptContent, executable.Language) | ||
|
|
||
| // Write script content to temp file | ||
| if _, err := tmpFile.WriteString(scriptContent); err != nil { | ||
| // If we have a session AND the script is bash-compatible, wrap to capture environment changes. | ||
| // Non-bash scripts (Python, Ruby, etc.) cannot have their environment changes captured because: | ||
| // 1. The wrapper is bash code that wouldn't be valid in other interpreters | ||
| // 2. Even if we ran non-bash scripts separately, their os.environ changes only affect | ||
| // their own subprocess and wouldn't propagate back to the session | ||
| scriptToWrite := scriptContent | ||
| isBashCompatible := isBashInterpreter(interpreter) | ||
| if session != nil && isBashCompatible { | ||
| scriptToWrite = wrapScriptForEnvCapture(scriptContent, envCapturePath, pwdCapturePath) | ||
| } | ||
|
|
||
| if _, err := tmpFile.WriteString(scriptToWrite); err != nil { | ||
| tmpFile.Close() | ||
| sendSSEError(c, fmt.Sprintf("Failed to write script: %v", err)) | ||
| return | ||
|
|
@@ -160,9 +209,6 @@ func HandleExecRequest(registry *ExecutableRegistry, runbookPath string, useExec | |
| } | ||
| tmpFile.Close() | ||
|
|
||
| // Detect interpreter from shebang or use language from executable | ||
| interpreter, args := detectInterpreter(scriptContent, executable.Language) | ||
|
|
||
| // Create context with 5 minute timeout | ||
| ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) | ||
| defer cancel() | ||
|
|
@@ -171,16 +217,20 @@ func HandleExecRequest(registry *ExecutableRegistry, runbookPath string, useExec | |
| cmdArgs := append(args, tmpFile.Name()) | ||
| cmd := exec.CommandContext(ctx, interpreter, cmdArgs...) | ||
|
|
||
| // Pass through all environment variables | ||
| // This feels dirty, but the point of a Runbook is to streamline what a user would otherwise do | ||
| // in their local environment! For users who want more security/control here, a commercial version | ||
| // of Runbooks is probably the answer. | ||
| cmd.Env = os.Environ() | ||
| // Set environment variables | ||
| // If we have a session, use the session's environment; otherwise use the process environment | ||
| if session != nil { | ||
| cmd.Env = session.EnvSlice() | ||
| } else { | ||
| cmd.Env = os.Environ() | ||
| } | ||
|
|
||
| // Set working directory if capturing files | ||
| // This isolates the script so all relative file writes are captured | ||
| // Set working directory | ||
| // Priority: captureFiles workDir > session workDir > default | ||
| if req.CaptureFiles && workDir != "" { | ||
| cmd.Dir = workDir | ||
| } else if session != nil { | ||
| cmd.Dir = session.WorkingDir | ||
josh-padnick marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| // Get stdout and stderr pipes | ||
|
|
@@ -270,6 +320,30 @@ func HandleExecRequest(registry *ExecutableRegistry, runbookPath string, useExec | |
| sendSSEStatus(c, status, exitCode) | ||
| flusher.Flush() | ||
|
|
||
| // Update session environment if we have a session, the script is bash-compatible, and execution succeeded. | ||
| // We capture env even on warnings since the script may have made partial changes. | ||
| // Non-bash scripts (Python, Ruby, etc.) don't get environment capture - their env changes | ||
| // only affect their own subprocess and can't propagate back to the session. | ||
| if session != nil && isBashCompatible && (status == "success" || status == "warn") { | ||
| capturedEnv, capturedPwd := parseEnvCapture(envCapturePath, pwdCapturePath) | ||
| if capturedEnv != nil { | ||
| // Filter out shell internals | ||
| filteredEnv := FilterCapturedEnv(capturedEnv) | ||
| // Determine new working directory | ||
| newWorkDir := session.WorkingDir | ||
| if capturedPwd != "" { | ||
| newWorkDir = capturedPwd | ||
| } | ||
| // Update session (ignore errors, non-critical) | ||
| if err := sessionManager.UpdateSessionEnv(filteredEnv, newWorkDir); err != nil { | ||
| // If the session was deleted concurrently, we can't update it. | ||
| // Log a warning to the user's console. | ||
| // TODO: Surface this warning in the UI, perhaps with a toaster notification | ||
| sendSSELog(c, fmt.Sprintf("Warning: could not persist environment changes: %v", err)) | ||
| } | ||
| } | ||
josh-padnick marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
josh-padnick marked this conversation as resolved.
Show resolved
Hide resolved
josh-padnick marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // Capture files if enabled and execution was successful (or warning) | ||
| if req.CaptureFiles && workDir != "" && (status == "success" || status == "warn") { | ||
| capturedFiles, captureErr := captureFilesFromWorkDir(workDir, captureOutputDir, cliOutputPath) | ||
|
|
@@ -291,6 +365,72 @@ func HandleExecRequest(registry *ExecutableRegistry, runbookPath string, useExec | |
| } | ||
| } | ||
|
|
||
| // wrapScriptForEnvCapture wraps a script to capture environment changes after execution. | ||
| // The wrapper appends commands to dump the environment and working directory to temp files. | ||
| func wrapScriptForEnvCapture(script, envCapturePath, pwdCapturePath string) string { | ||
| // We wrap the script to capture environment after it runs | ||
| // The wrapper: | ||
| // 1. Sources/executes the original script | ||
| // 2. Captures the resulting environment to a temp file | ||
| // 3. Captures the working directory to another temp file | ||
| // | ||
| // We use a subshell to run the user script so that 'exit' calls don't skip our capture | ||
| // but we need the environment changes to propagate, so we use 'source' instead | ||
josh-padnick marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| // | ||
| // The wrapper preserves the exit code of the original script | ||
| wrapper := fmt.Sprintf(`#!/bin/bash | ||
| # Runbooks environment capture wrapper | ||
| # This wrapper captures environment changes after the user script runs | ||
|
|
||
| __RUNBOOKS_ENV_CAPTURE_PATH=%q | ||
| __RUNBOOKS_PWD_CAPTURE_PATH=%q | ||
|
|
||
| # Run the user script in the current shell context so env changes propagate | ||
| # We use a function and trap to ensure we capture env even if script calls 'exit' | ||
| __runbooks_capture_env() { | ||
| env > "$__RUNBOOKS_ENV_CAPTURE_PATH" 2>/dev/null | ||
| pwd > "$__RUNBOOKS_PWD_CAPTURE_PATH" 2>/dev/null | ||
| } | ||
|
|
||
| trap __runbooks_capture_env EXIT | ||
josh-padnick marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| # Execute the user script inline (not sourced, to preserve proper error handling) | ||
| # --- BEGIN USER SCRIPT --- | ||
| %s | ||
| # --- END USER SCRIPT --- | ||
| `, envCapturePath, pwdCapturePath, script) | ||
|
|
||
| return wrapper | ||
| } | ||
|
|
||
| // parseEnvCapture reads the captured environment and working directory from temp files. | ||
| func parseEnvCapture(envCapturePath, pwdCapturePath string) (map[string]string, string) { | ||
| env := make(map[string]string) | ||
|
|
||
| // Read environment capture | ||
| if envData, err := os.ReadFile(envCapturePath); err == nil { | ||
| for _, line := range strings.Split(string(envData), "\n") { | ||
| if idx := strings.Index(line, "="); idx != -1 { | ||
| key := line[:idx] | ||
| value := line[idx+1:] | ||
| env[key] = value | ||
| } | ||
| } | ||
josh-padnick marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| // Read working directory capture | ||
| var pwd string | ||
| if pwdData, err := os.ReadFile(pwdCapturePath); err == nil { | ||
| pwd = strings.TrimSpace(string(pwdData)) | ||
| } | ||
|
|
||
| if len(env) == 0 { | ||
| return nil, pwd | ||
| } | ||
|
|
||
| return env, pwd | ||
| } | ||
josh-padnick marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // detectInterpreter detects the interpreter from the shebang line or uses provided language | ||
| func detectInterpreter(script string, providedLang string) (string, []string) { | ||
| // If language is explicitly provided, use it | ||
|
|
@@ -328,6 +468,18 @@ func detectInterpreter(script string, providedLang string) (string, []string) { | |
| return "bash", []string{} | ||
| } | ||
|
|
||
| // isBashInterpreter returns true if the interpreter is bash or sh compatible. | ||
| // Only bash-compatible scripts can have their environment changes captured, | ||
| // because the environment capture wrapper is written in bash. | ||
| func isBashInterpreter(interpreter string) bool { | ||
| switch interpreter { | ||
| case "bash", "sh", "/bin/bash", "/bin/sh", "/usr/bin/bash", "/usr/bin/sh": | ||
| return true | ||
| default: | ||
| return false | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bash wrapper executed with sh fails on non-bash systemsHigh Severity The Additional Locations (1) |
||
|
|
||
| // streamOutput reads from a pipe and sends lines to the output channel | ||
| func streamOutput(pipe io.ReadCloser, outputChan chan<- string) { | ||
| scanner := bufio.NewScanner(pipe) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.