8f23b8a19e
* fix: prevent command injection via untrusted git branch names
Untrusted git branch names (e.g. a fork PR head branch) were substituted
unescaped into executor prepare scripts and run via `eval` in a shell,
yielding command injection (CRITICAL RCE) in the Docker/Sprites/remote
executors. git check-ref-format permits `;`, `|`, `$`, backticks and
`()`, so a branch like `$(touch pwned)` or `main;touch pwned` executed.
- scriptengine: shell-single-quote every data placeholder that lands in
shell text (repository.path/name/branch/clone_url/ssh_url,
workspace.path, worktree.base_path/path/branch/base_branch). Script
fragments (setup_script, git.identity_setup, github.auth_setup,
kandev.*) are left untouched.
- default_scripts: wrap each data placeholder in single quotes in the
postlude, clone lines, and the Sprites echo (double quotes still allow
$(...) substitution). `'<escaped>'` round-trips to the literal.
- orchestrator: defense-in-depth — reject a PR head branch that fails
securityutil.IsValidBranchName before it reaches CheckoutBranch, while
still creating the review task without a checkout.
- tests: PoC-derived regression asserting the injected command no longer
runs through the real sh -c/eval sink, provider escaping unit tests,
and ingestion-layer validation coverage.
* fix: harden branch-name escaping for legacy stored prepare scripts
Automated review (Codex, CodeRabbit) flagged that shell-single-quoting the
value while relying on the templates to add the surrounding quotes only
protects the templates we ship. Profiles snapshot the default prepare
script at creation time, so an OLD stored script referencing a placeholder
bare (e.g. `git clone --depth=1 --branch {{repository.branch}} ...`) still
resolved with an unquoted, injectable branch value.
- scriptengine: add shellQuote, which emits a fully self-contained
single-quoted shell token (surrounding quotes included). Every data
placeholder (repository.path/name/branch/clone_url/ssh_url,
workspace.path, worktree.base_path/path/branch/base_branch) now carries
its own quoting, so a bare `{{placeholder}}` in any template — including
legacy stored scripts — is safe. shellSingleQuote is kept for the git
identity fragment, which supplies its own quotes.
- default_scripts: reference data placeholders bare (the value self-quotes);
the Sprites echo becomes printf with the values as separate args so a
hostile URL/branch can't expand inside a double-quoted string.
- tests: assert providers emit fully-quoted tokens that round-trip through a
real shell; cover base_path/path/repository.path/name/ssh_url; add a
regression proving a legacy stored script with bare placeholders no longer
executes injected commands.
* test: strengthen branch-injection regression assertions
Address Greptile review feedback on the new tests.
- assertNoUnquotedPayload now tracks real single-quote state char-by-char
instead of "some quote before and after", so a payload in an unquoted
context on a line that happens to contain other quotes is caught.
- Drop the unused wantHasRepoRow table field; repo-row presence is already
asserted via len(got) != 1.
---------
Co-authored-by: Kandev Agent <agent@kandev.dev>
241 lines
8.5 KiB
Go
241 lines
8.5 KiB
Go
package scriptengine
|
|
|
|
import (
|
|
"fmt"
|
|
"strings"
|
|
)
|
|
|
|
// RepositoryProvider returns git-related placeholders from metadata and environment.
|
|
// Parameters:
|
|
// - metadata: executor create request metadata (contains "repository_path", "base_branch", etc.)
|
|
// - env: environment variables (contains "GITHUB_TOKEN", etc.)
|
|
// - repoURLResolver: resolves a local repo path to its remote URL (e.g., `git remote get-url origin`)
|
|
// - tokenInjector: injects auth token into a clone URL
|
|
func RepositoryProvider(
|
|
metadata map[string]any,
|
|
env map[string]string,
|
|
repoURLResolver func(string) (string, error),
|
|
tokenInjector func(string, map[string]string) string,
|
|
) PlaceholderProvider {
|
|
return func() map[string]string {
|
|
vars := make(map[string]string)
|
|
|
|
// SECURITY: repository.path/name/branch/clone_url/ssh_url are DATA that
|
|
// land in shell text (git clone args, etc.). shellQuote emits a fully
|
|
// single-quoted, self-contained shell token so a hostile value (e.g. a
|
|
// fork PR branch named "$(...)") is a literal string even when a
|
|
// template (including an OLD stored prepare_script) references the
|
|
// placeholder BARE, e.g. `--branch {{repository.branch}}`.
|
|
repoPath := getMetaString(metadata, "repository_path")
|
|
if repoPath != "" {
|
|
vars["repository.path"] = shellQuote(repoPath)
|
|
vars["repository.name"] = shellQuote(repoNameFromPath(repoPath))
|
|
}
|
|
|
|
branch := getMetaString(metadata, "base_branch")
|
|
if branch == "" {
|
|
branch = getMetaString(metadata, "repository_branch")
|
|
}
|
|
vars["repository.branch"] = shellQuote(branch)
|
|
|
|
// repository.setup_script is a script FRAGMENT (intentional multi-line
|
|
// shell), not data — do NOT quote it.
|
|
setupScript := getMetaString(metadata, "repository_setup_script")
|
|
vars["repository.setup_script"] = setupScript
|
|
|
|
// Clone URL: prefer explicit metadata, fall back to resolving from local repo
|
|
cloneURL := getMetaString(metadata, "repository_clone_url")
|
|
if cloneURL == "" && repoPath != "" && repoURLResolver != nil {
|
|
if remoteURL, err := repoURLResolver(repoPath); err == nil && remoteURL != "" {
|
|
vars["repository.ssh_url"] = shellQuote(remoteURL)
|
|
cloneURL = remoteURL
|
|
}
|
|
}
|
|
if cloneURL != "" {
|
|
vars["repository.clone_url"] = shellQuote(injectToken(cloneURL, env, tokenInjector))
|
|
}
|
|
|
|
return vars
|
|
}
|
|
}
|
|
|
|
// AgentctlProvider returns kandev agentctl-related placeholders.
|
|
func AgentctlProvider(agentctlPort int, workspacePath string) PlaceholderProvider {
|
|
return func() map[string]string {
|
|
portStr := fmt.Sprintf("%d", agentctlPort)
|
|
return map[string]string{
|
|
"kandev.agentctl.port": portStr,
|
|
"kandev.agentctl.install": "chmod +x /usr/local/bin/agentctl",
|
|
"kandev.agentctl.start": fmt.Sprintf(
|
|
"nohup agentctl --port %s --workdir %s > /tmp/agentctl.log 2>&1 &\nsleep 1",
|
|
portStr, workspacePath,
|
|
),
|
|
}
|
|
}
|
|
}
|
|
|
|
// WorkspaceProvider returns workspace path placeholder.
|
|
//
|
|
// SECURITY: workspace.path is DATA in shell text; shellQuote emits a
|
|
// self-contained single-quoted token so it stays a literal even if it ever
|
|
// carries metacharacters and even when referenced bare in a template.
|
|
func WorkspaceProvider(workspacePath string) PlaceholderProvider {
|
|
return func() map[string]string {
|
|
return map[string]string{
|
|
"workspace.path": shellQuote(workspacePath),
|
|
}
|
|
}
|
|
}
|
|
|
|
// WorktreeProvider returns placeholders that describe the selected worktree context.
|
|
//
|
|
// SECURITY: worktree paths and the (untrusted, possibly fork-PR-controlled)
|
|
// branch names are DATA that land in shell text. shellQuote emits a
|
|
// self-contained single-quoted token so a hostile branch like "$(touch pwned)"
|
|
// cannot inject commands even when a stored/legacy template references the
|
|
// placeholder bare. worktree.id is a kandev-generated UUID (not shell data),
|
|
// left as-is.
|
|
func WorktreeProvider(basePath, path, id, branch, baseBranch string) PlaceholderProvider {
|
|
return func() map[string]string {
|
|
return map[string]string{
|
|
"worktree.base_path": shellQuote(basePath),
|
|
"worktree.path": shellQuote(path),
|
|
"worktree.id": id,
|
|
"worktree.branch": shellQuote(branch),
|
|
"worktree.base_branch": shellQuote(baseBranch),
|
|
}
|
|
}
|
|
}
|
|
|
|
// GitIdentityProvider returns placeholders for git identity setup in remote executors.
|
|
func GitIdentityProvider(metadata map[string]any) PlaceholderProvider {
|
|
return func() map[string]string {
|
|
name := getMetaString(metadata, "git_user_name")
|
|
email := getMetaString(metadata, "git_user_email")
|
|
|
|
vars := map[string]string{
|
|
"git.user_name": name,
|
|
"git.user_email": email,
|
|
"git.identity_setup": "",
|
|
}
|
|
if name == "" || email == "" {
|
|
return vars
|
|
}
|
|
|
|
lines := []string{
|
|
fmt.Sprintf("git config --global user.name '%s'", shellSingleQuote(name)),
|
|
fmt.Sprintf("git config --global user.email '%s'", shellSingleQuote(email)),
|
|
}
|
|
vars["git.identity_setup"] = strings.Join(lines, "\n")
|
|
return vars
|
|
}
|
|
}
|
|
|
|
// injectToken applies token injection to a URL if an injector is provided.
|
|
func injectToken(url string, env map[string]string, injector func(string, map[string]string) string) string {
|
|
if injector != nil {
|
|
return injector(url, env)
|
|
}
|
|
return url
|
|
}
|
|
|
|
// getMetaString extracts a string value from a metadata map.
|
|
func getMetaString(metadata map[string]any, key string) string {
|
|
if metadata == nil {
|
|
return ""
|
|
}
|
|
if v, ok := metadata[key].(string); ok {
|
|
return v
|
|
}
|
|
return ""
|
|
}
|
|
|
|
// repoNameFromPath extracts the repository name from a file path.
|
|
func repoNameFromPath(repoPath string) string {
|
|
if repoPath == "" {
|
|
return ""
|
|
}
|
|
// Find last path component
|
|
for i := len(repoPath) - 1; i >= 0; i-- {
|
|
if repoPath[i] == '/' {
|
|
return repoPath[i+1:]
|
|
}
|
|
}
|
|
return repoPath
|
|
}
|
|
|
|
// AgentInstallProvider returns a placeholder with combined install scripts for multiple agents.
|
|
func AgentInstallProvider(installScripts []string) PlaceholderProvider {
|
|
return func() map[string]string {
|
|
seen := map[string]bool{}
|
|
var lines []string
|
|
for _, s := range installScripts {
|
|
s = strings.TrimSpace(s)
|
|
if s != "" && !seen[s] {
|
|
seen[s] = true
|
|
lines = append(lines, s)
|
|
}
|
|
}
|
|
return map[string]string{
|
|
"kandev.agents.install": strings.Join(lines, "\n"),
|
|
}
|
|
}
|
|
}
|
|
|
|
// shellSingleQuote escapes embedded single quotes for use INSIDE an existing
|
|
// pair of single quotes supplied by the caller/template (e.g. '%s'). It does
|
|
// NOT add the surrounding quotes.
|
|
func shellSingleQuote(value string) string {
|
|
return strings.ReplaceAll(value, "'", `'"'"'`)
|
|
}
|
|
|
|
// shellQuote returns value as a complete, self-contained single-quoted shell
|
|
// token (surrounding quotes included, embedded quotes escaped). The result is
|
|
// safe to drop into a script UNQUOTED — `--branch <shellQuote(v)>` cannot be
|
|
// broken out of by shell metacharacters ($(), backticks, ;, |, whitespace).
|
|
//
|
|
// Prefer this over shellSingleQuote for any DATA placeholder, because stored
|
|
// prepare_script templates (snapshotted before a template was hardened) may
|
|
// reference the placeholder bare, so the value itself must carry its quoting.
|
|
func shellQuote(value string) string {
|
|
return "'" + shellSingleQuote(value) + "'"
|
|
}
|
|
|
|
// GitHubAuthProvider returns placeholders for GitHub authentication setup.
|
|
// It configures both the gh CLI and git credential helper to use the GitHub token.
|
|
func GitHubAuthProvider(env map[string]string) PlaceholderProvider {
|
|
return func() map[string]string {
|
|
vars := map[string]string{
|
|
"github.auth_setup": "",
|
|
}
|
|
|
|
// Check for GitHub token in env (either GH_TOKEN or GITHUB_TOKEN)
|
|
token := env["GH_TOKEN"]
|
|
if token == "" {
|
|
token = env["GITHUB_TOKEN"]
|
|
}
|
|
if token == "" {
|
|
// No token - output fallback commands that may work if gh is pre-authenticated
|
|
vars["github.auth_setup"] = `# No GitHub token configured - trying fallback methods
|
|
gh auth setup-git 2>/dev/null || true
|
|
gh config set git_protocol https --host github.com 2>/dev/null || true`
|
|
return vars
|
|
}
|
|
|
|
// Build auth setup script that configures both gh CLI and git
|
|
// Use /bin/sh (not /bin/bash) for Alpine compatibility
|
|
// Use ${GH_TOKEN:-${GITHUB_TOKEN}} to support either env var being set
|
|
lines := []string{
|
|
"# GitHub token authentication",
|
|
"# Configure git credential helper for GitHub HTTPS authentication",
|
|
`git config --global credential.https://github.com.helper '!/bin/sh -c "echo username=x-access-token; echo password=${GH_TOKEN:-${GITHUB_TOKEN}}"'`,
|
|
"# Configure gh CLI to use HTTPS protocol",
|
|
"gh config set git_protocol https --host github.com 2>/dev/null || true",
|
|
"# Register gh as git credential helper (backup method)",
|
|
"gh auth setup-git 2>/dev/null || true",
|
|
}
|
|
vars["github.auth_setup"] = strings.Join(lines, "\n")
|
|
return vars
|
|
}
|
|
}
|