* 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>
* chore: remove orphan executor tracing file and deprecated ResolveScript wrappers
- Delete internal/agentctl/tracing/executor.go (7 TraceExecutor* funcs)
- Delete internal/orchestrator/executor/script_template.go (deprecated wrapper)
- Delete scriptengine.ResolveScript convenience function
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: remove orphan setter methods across healthpoll, jira, linear, slack, updates
- Delete Poller.SetInterval from integrations/healthpoll (no callers)
- Delete Poller.SetIssueTickHook from jira and linear (no callers)
- Delete Trigger.SetInterval and Trigger.SetMatchHook from slack (no callers)
- Delete Service.SetPollInterval from system/updates (no callers)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: remove orphan wrapper functions and accessor method
- Delete updates.FetchLatestRelease shim (wraps FetchLatestReleaseFrom, no callers)
- Delete usage.NewClaudeUsageClient and NewCodexUsageClient shims (no callers)
- Delete database.Service.Path() accessor (no callers)
- Update ErrGitHubRateLimited godoc to reference FetchLatestReleaseFrom
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: remove unwired handleListExecutors MCP handler
The sibling profile handlers (list/create/update/delete) are registered
via RegisterFunc in handlers.go. handleListExecutors never was — dead flow.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: remove stale hook-field comments after setter deletion
SetIssueTickHook (jira, linear) and SetMatchHook (slack) were removed
in the deadcode cleanup. Their struct fields still claimed "tests use
this" — no test or caller can set these fields now.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* chore: remove orphan hook fields and fire methods after setter deletion
issueTickHook (jira/linear) and matchHook (slack) are unexported and
never set after their public setters were removed; fireIssueTickHook /
fireHook were permanent no-ops.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* feat: add GitHub token injection for remote executors and Docker session resume
- Add global GitHub access token configuration (backend + frontend)
- Inject GitHub token into Docker and Sprites executors via env vars and Git credential helper
- Transition Docker executor from host mounting to repository cloning
- Implement Docker session resume (reconnect to existing containers after backend restart)
- Add shell validation using exec.LookPath in agentctl
- Add scriptengine providers for agent install and agentctl placeholders
* fix: format github-status.tsx with prettier
* fix: resolve lint issues (goconst, errcheck, file-length)
* fix: address code review findings
- Fix resolveGHCLIToken to check both GITHUB_TOKEN and GH_TOKEN guards
- Fix credential helper to use \\ fallback
- Change /bin/bash to /bin/sh in credential helper for Alpine compat
- Strip embedded token from .git/config after clone in prepare scripts
- Extract buildLaunchAgentRequest helpers to stay under 80-line limit
- Add tests for GitHubAuthProvider, injectTokenIntoURL, reconnect validation
- Add GH_TOKEN-only test case for resolveGHCLIToken
* refactor(executor): extract GitHub credential resolution to separate file
Move injectGitHubToken, injectGitHubTokenFromCLI, resolveRemoteCredentials,
resolveAuthSecrets, methodIDToEnvVar, resolveGHCLIToken, and detectGHToken
out of executor_execute.go (was 803 logical lines, over the 800-line limit
after merging main) into a new executor_credentials.go file. Also adds the
nil-guard on secretStore in resolveAuthSecrets that was flagged by review.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
---------
Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Carlos Florêncio