diff --git a/apps/backend/cmd/agentctl/github_cli_shim.go b/apps/backend/cmd/agentctl/github_cli_shim.go index 246e5ba0a3..1862bf4771 100644 --- a/apps/backend/cmd/agentctl/github_cli_shim.go +++ b/apps/backend/cmd/agentctl/github_cli_shim.go @@ -16,7 +16,7 @@ import ( "github.com/kandev/kandev/internal/githubauth" ) -const envGitHubCLIShimDir = "KANDEV_GITHUB_CLI_SHIM_DIR" +const envGitHubCLIShimDir = githubauth.CredentialCLIShimDirEnv const windowsOS = "windows" @@ -271,9 +271,30 @@ func installGitHubCLIShim(agentctlExecutable, tempRoot string) (string, func(), cleanup() return "", nil, fmt.Errorf("install agentctl helper: %w", err) } + if err := installGitHubCLIBashEnv(dir); err != nil { + cleanup() + return "", nil, fmt.Errorf("install Bash environment: %w", err) + } return dir, cleanup, nil } +func installGitHubCLIBashEnv(shimDir string) error { + path := filepath.Join(shimDir, githubauth.CLIBashEnvFilename) + const script = `#!/bin/sh +if [ -n "${KANDEV_GITHUB_PARENT_BASH_ENV:-}" ] && + [ "${KANDEV_GITHUB_PARENT_BASH_ENV}" != "${KANDEV_GITHUB_CLI_BASH_ENV:-}" ]; then + . "$KANDEV_GITHUB_PARENT_BASH_ENV" +fi +if [ -n "${KANDEV_GITHUB_CLI_SHIM_DIR:-}" ]; then + case ":${PATH:-}:" in + *:"${KANDEV_GITHUB_CLI_SHIM_DIR}":*) ;; + *) PATH="${KANDEV_GITHUB_CLI_SHIM_DIR}:${PATH:-}"; export PATH ;; + esac +fi +` + return os.WriteFile(path, []byte(script), 0o700) +} + func linkOrCopyExecutable(source, target string) error { if err := os.Symlink(source, target); err == nil { return nil diff --git a/apps/backend/cmd/agentctl/github_cli_shim_test.go b/apps/backend/cmd/agentctl/github_cli_shim_test.go index d45701b615..522f068ef6 100644 --- a/apps/backend/cmd/agentctl/github_cli_shim_test.go +++ b/apps/backend/cmd/agentctl/github_cli_shim_test.go @@ -7,7 +7,9 @@ import ( "net/http" "net/http/httptest" "os" + osExec "os/exec" "path/filepath" + "runtime" "strings" "testing" ) @@ -35,6 +37,112 @@ func TestInstallGitHubCLIShimCreatesManagedTools(t *testing.T) { } } +func TestPrepareGitHubCLIShimPublishesCredentialHelperExecutable(t *testing.T) { + t.Setenv("KANDEV_GITHUB_CREDENTIAL_HELPER_PATH", "") + executable, err := os.Executable() + if err != nil { + t.Fatalf("resolve test executable: %v", err) + } + cleanup, err := prepareGitHubCLIShim() + if err != nil { + t.Fatalf("prepareGitHubCLIShim() error = %v", err) + } + t.Cleanup(cleanup) + if got := os.Getenv("KANDEV_GITHUB_CREDENTIAL_HELPER_PATH"); got != executable { + t.Fatalf("credential helper executable = %q, want %q", got, executable) + } +} + +func TestInstallGitHubCLIShimCreatesLoginShellEnvironment(t *testing.T) { + root := t.TempDir() + binary := filepath.Join(root, "agentctl") + if err := os.WriteFile(binary, []byte("agentctl"), 0o700); err != nil { + t.Fatal(err) + } + + shimDir, cleanup, err := installGitHubCLIShim(binary, root) + if err != nil { + t.Fatalf("installGitHubCLIShim() error = %v", err) + } + t.Cleanup(cleanup) + + bashEnv := filepath.Join(shimDir, "bash-env.sh") + info, err := os.Stat(bashEnv) + if err != nil { + t.Fatalf("stat managed Bash environment: %v", err) + } + if got := info.Mode().Perm(); got != 0o700 { + t.Fatalf("Bash environment mode = %o, want 700", got) + } + content, err := os.ReadFile(bashEnv) + if err != nil { + t.Fatalf("read managed Bash environment: %v", err) + } + for _, want := range []string{ + "KANDEV_GITHUB_CLI_SHIM_DIR", + "KANDEV_GITHUB_PARENT_BASH_ENV", + "PATH=", + } { + if !strings.Contains(string(content), want) { + t.Fatalf("Bash environment missing %q: %s", want, content) + } + } +} + +func TestInstallGitHubCLIShimLoginShellRestoresManagedTools(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Bash login-shell behavior is Unix-specific") + } + root := t.TempDir() + binary := filepath.Join(root, "agentctl") + if err := os.WriteFile(binary, []byte("agentctl"), 0o700); err != nil { + t.Fatal(err) + } + shimDir, cleanup, err := installGitHubCLIShim(binary, root) + if err != nil { + t.Fatalf("installGitHubCLIShim() error = %v", err) + } + t.Cleanup(cleanup) + + marker := filepath.Join(t.TempDir(), "parent-bash-hook-ran") + parentEnv := filepath.Join(t.TempDir(), "parent-bash-env.sh") + if err := os.WriteFile(parentEnv, []byte("#!/bin/sh\nprintf hook-ran > \"$KANDEV_BASH_HOOK_MARKER\"\n"), 0o700); err != nil { + t.Fatalf("write parent Bash environment: %v", err) + } + env := make(map[string]string, len(os.Environ())+5) + for _, entry := range os.Environ() { + key, value, ok := strings.Cut(entry, "=") + if ok && key != "BASH_ENV" && key != "PATH" { + env[key] = value + } + } + env["KANDEV_GITHUB_CREDENTIAL_BROKER_URL"] = "https://kandev.example/resolve" + env["KANDEV_GITHUB_CLI_SHIM_DIR"] = shimDir + env["KANDEV_GITHUB_CLI_BASH_ENV"] = filepath.Join(shimDir, "bash-env.sh") + env["KANDEV_GITHUB_PARENT_BASH_ENV"] = parentEnv + env["KANDEV_BASH_HOOK_MARKER"] = marker + env["BASH_ENV"] = env["KANDEV_GITHUB_CLI_BASH_ENV"] + env["PATH"] = "/usr/bin:/bin" + commandEnv := make([]string, 0, len(env)) + for key, value := range env { + commandEnv = append(commandEnv, key+"="+value) + } + + command := osExec.Command("bash", "-lc", "printf 'agentctl=%s\\ngh=%s\\n' \"$(command -v agentctl || true)\" \"$(command -v gh || true)\"") + command.Env = commandEnv + output, err := command.CombinedOutput() + if err != nil { + t.Fatalf("Bash login shell failed: %v\n%s", err, output) + } + if !strings.Contains(string(output), "agentctl="+filepath.Join(shimDir, "agentctl")) || + !strings.Contains(string(output), "gh="+filepath.Join(shimDir, "gh")) { + t.Fatalf("managed tools = %q", output) + } + if hook, err := os.ReadFile(marker); err != nil || string(hook) != "hook-ran" { + t.Fatalf("parent Bash hook marker = %q, error = %v", hook, err) + } +} + func TestPathWithoutDirectoryPreservesPathList(t *testing.T) { path := strings.Join([]string{"/bin", "/shim", "/usr/bin"}, string(os.PathListSeparator)) want := strings.Join([]string{"/bin", "/usr/bin"}, string(os.PathListSeparator)) diff --git a/apps/backend/cmd/agentctl/main.go b/apps/backend/cmd/agentctl/main.go index 67fafebebc..0936314faa 100644 --- a/apps/backend/cmd/agentctl/main.go +++ b/apps/backend/cmd/agentctl/main.go @@ -17,6 +17,7 @@ import ( "os" "os/exec" "os/signal" + "path/filepath" "strconv" "syscall" "time" @@ -27,6 +28,7 @@ import ( "github.com/kandev/kandev/internal/agentctl/server/instance" "github.com/kandev/kandev/internal/agentctl/server/process" "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/githubauth" mcpserver "github.com/kandev/kandev/internal/mcp/server" "github.com/kandev/kandev/pkg/agent" "go.uber.org/zap" @@ -121,6 +123,10 @@ func prepareGitHubCLIShim() (func(), error) { if err != nil { return nil, fmt.Errorf("resolve agentctl executable for gh shim: %w", err) } + executable, err = filepath.Abs(executable) + if err != nil { + return nil, fmt.Errorf("resolve absolute agentctl executable for GitHub tools: %w", err) + } shimDir, cleanup, err := installGitHubCLIShim(executable, "") if err != nil { return nil, err @@ -129,6 +135,17 @@ func prepareGitHubCLIShim() (func(), error) { cleanup() return nil, fmt.Errorf("configure gh shim directory: %w", err) } + if err := os.Setenv( + githubauth.CredentialCLIBashEnvEnv, + filepath.Join(shimDir, githubauth.CLIBashEnvFilename), + ); err != nil { + cleanup() + return nil, fmt.Errorf("configure Bash environment: %w", err) + } + if err := os.Setenv(githubauth.CredentialHelperPathEnv, executable); err != nil { + cleanup() + return nil, fmt.Errorf("configure Git credential helper executable: %w", err) + } return cleanup, nil } diff --git a/apps/backend/internal/agent/runtime/lifecycle/container.go b/apps/backend/internal/agent/runtime/lifecycle/container.go index 2caec90de1..1da438c603 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/container.go +++ b/apps/backend/internal/agent/runtime/lifecycle/container.go @@ -18,6 +18,7 @@ import ( agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/gitconfigenv" + "github.com/kandev/kandev/internal/githubauth" ) const ( @@ -25,6 +26,7 @@ const ( dockerAgentctlInstancePortMax = 41100 boolStringTrue = "true" gitHubCredentialHelperConfigKey = "credential.https://github.com.helper" + remoteAgentctlExecutablePath = "/usr/local/bin/agentctl" ) // ContainerConfig holds configuration for launching a Docker container @@ -665,11 +667,15 @@ func (cm *ContainerManager) buildEnvVars(config ContainerConfig) ([]string, erro // Inject credentials from the provided credentials map for k, v := range config.Credentials { - if k == "GIT_CONFIG_COUNT" || strings.HasPrefix(k, "GIT_CONFIG_KEY_") || strings.HasPrefix(k, "GIT_CONFIG_VALUE_") { + if k == "GIT_CONFIG_COUNT" || strings.HasPrefix(k, "GIT_CONFIG_KEY_") || + strings.HasPrefix(k, "GIT_CONFIG_VALUE_") || k == githubauth.CredentialHelperPathEnv { continue } env = append(env, fmt.Sprintf("%s=%s", k, v)) } + if hasManagedGitHubBrokerEnv(config.Credentials) { + env = append(env, githubauth.CredentialHelperPathEnv+"="+remoteAgentctlExecutablePath) + } // Add profile-specific label if available if config.ProfileInfo != nil && config.ProfileInfo.ProfileID != "" { diff --git a/apps/backend/internal/agent/runtime/lifecycle/container_config_test.go b/apps/backend/internal/agent/runtime/lifecycle/container_config_test.go index a59078c752..9c2a7b9e9e 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/container_config_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/container_config_test.go @@ -125,6 +125,38 @@ func TestBuildContainerConfigPreflightsBrokerBeforePrepareClone(t *testing.T) { } } +func TestBuildContainerConfigPublishesManagedGitCredentialHelperBeforeAgentctlStartup(t *testing.T) { + cm := newCMTest(t) + cfg := ContainerConfig{ + AgentConfig: newConfigStubAgent(), + InstanceID: "0123456789abcdef", + TaskID: "task-1", + Credentials: map[string]string{ + envKeyGitHubCredentialBrokerURL: "https://kandev.example/api/v1/github/credentials/resolve", + envKeyGitHubCredentialLease: "lease", + }, + PrepareScript: "git clone https://github.com/acme/widgets.git /workspace", + } + + got, err := cm.buildContainerConfig(cfg) + if err != nil { + t.Fatalf("buildContainerConfig: %v", err) + } + want := "KANDEV_GITHUB_CREDENTIAL_HELPER_PATH=/usr/local/bin/agentctl" + if !containsExactString(got.Env, want) { + t.Fatalf("container env missing pre-start credential helper %q: %#v", want, got.Env) + } +} + +func containsExactString(values []string, want string) bool { + for _, value := range values { + if value == want { + return true + } + } + return false +} + func TestBuildContainerConfig_ImageDefaultsToRuntime(t *testing.T) { cm := newCMTest(t) cfg := ContainerConfig{ diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_clone_auth_test.go b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_clone_auth_test.go index c815730bcb..bae0b5eb9a 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_clone_auth_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_clone_auth_test.go @@ -100,6 +100,16 @@ func TestIsTransientUploadError(t *testing.T) { require.False(t, isTransientUploadError(errors.New("upload failed: HTTP 400"))) } +func TestBuildSpriteEnvPublishesManagedGitCredentialHelperBeforeAgentctlStartup(t *testing.T) { + exec := &SpritesExecutor{} + got := exec.buildSpriteEnv(map[string]string{ + envKeyGitHubCredentialBrokerURL: "https://kandev.example/api/v1/github/credentials/resolve", + envKeyGitHubCredentialLease: "lease", + }) + want := "KANDEV_GITHUB_CREDENTIAL_HELPER_PATH=/usr/local/bin/agentctl" + require.Contains(t, got, want) +} + func TestSpriteCreateInstanceRequestCarriesRefreshedEnvironment(t *testing.T) { req := &ExecutorCreateRequest{ InstanceID: "instance-1", diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_helpers.go b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_helpers.go index 47e38551af..7ae8c2e8b7 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_helpers.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_helpers.go @@ -12,6 +12,8 @@ import ( sprites "github.com/superfly/sprites-go" "go.uber.org/zap" + + "github.com/kandev/kandev/internal/githubauth" ) var uploadHTTPStatusRE = regexp.MustCompile(`(?i)\b(?:http|status)\s*:?\s*(\d{3})\b`) @@ -129,8 +131,14 @@ func extractUploadHTTPStatus(msg string) int { func (r *SpritesExecutor) buildSpriteEnv(env map[string]string) []string { result := make([]string, 0, len(env)) for k, v := range env { + if k == githubauth.CredentialHelperPathEnv { + continue + } result = append(result, k+"="+v) } + if hasManagedGitHubBrokerEnv(env) { + result = append(result, githubauth.CredentialHelperPathEnv+"="+remoteAgentctlExecutablePath) + } return result } diff --git a/apps/backend/internal/agentctl/server/config/config.go b/apps/backend/internal/agentctl/server/config/config.go index 8f7ea0327f..26aed753e1 100644 --- a/apps/backend/internal/agentctl/server/config/config.go +++ b/apps/backend/internal/agentctl/server/config/config.go @@ -17,12 +17,14 @@ import ( "fmt" "os" "path/filepath" + "runtime" "strconv" "strings" "sync" "time" "github.com/kandev/kandev/internal/gitconfigenv" + "github.com/kandev/kandev/internal/githubauth" "github.com/kandev/kandev/pkg/agent" ) @@ -539,8 +541,9 @@ func CollectAgentEnvWithError(additional map[string]string) ([]string, error) { if err != nil { return nil, fmt.Errorf("compose indexed Git config: %w", err) } - if envMap["KANDEV_GITHUB_CREDENTIAL_BROKER_URL"] != "" { - prependPathEntry(envMap, envMap["KANDEV_GITHUB_CLI_SHIM_DIR"]) + if envMap[githubauth.CredentialBrokerURLEnv] != "" { + prependPathEntry(envMap, envMap[githubauth.CredentialCLIShimDirEnv]) + configureGitHubCLIStartupEnv(envMap) } // Convert back to slice @@ -551,6 +554,83 @@ func CollectAgentEnvWithError(additional map[string]string) ([]string, error) { return result, nil } +func configureGitHubCLIStartupEnv(env map[string]string) { + if runtime.GOOS == "windows" { + return + } + startupEnv := env[githubauth.CredentialCLIBashEnvEnv] + if startupEnv == "" { + return + } + parentEnv := env["BASH_ENV"] + resolvedParentEnv := expandBashEnvParameters(parentEnv, env) + if resolvedParentEnv != "" && filepath.Clean(resolvedParentEnv) != filepath.Clean(startupEnv) { + env[githubauth.CredentialParentBashEnv] = resolvedParentEnv + } else { + delete(env, githubauth.CredentialParentBashEnv) + } + env["BASH_ENV"] = startupEnv +} + +func expandBashEnvParameters(value string, env map[string]string) string { + var expanded strings.Builder + for index := 0; index < len(value); { + dollarOffset := strings.IndexByte(value[index:], '$') + if dollarOffset < 0 { + expanded.WriteString(value[index:]) + break + } + dollar := index + dollarOffset + expanded.WriteString(value[index:dollar]) + name, end, ok := bashEnvParameterAt(value, dollar) + if !ok { + expanded.WriteByte('$') + index = dollar + 1 + continue + } + expanded.WriteString(env[name]) + index = end + } + return expanded.String() +} + +func bashEnvParameterAt(value string, dollar int) (string, int, bool) { + if dollar+1 >= len(value) { + return "", 0, false + } + if value[dollar+1] == '{' { + closeOffset := strings.IndexByte(value[dollar+2:], '}') + if closeOffset < 0 { + return "", 0, false + } + end := dollar + 2 + closeOffset + name := value[dollar+2 : end] + return name, end + 1, isBashEnvName(name) + } + end := dollar + 1 + for end < len(value) && isBashEnvNameByte(value[end], end == dollar+1) { + end++ + } + if end == dollar+1 { + return "", 0, false + } + return value[dollar+1 : end], end, true +} + +func isBashEnvName(name string) bool { + for index := range len(name) { + if !isBashEnvNameByte(name[index], index == 0) { + return false + } + } + return name != "" +} + +func isBashEnvNameByte(value byte, first bool) bool { + letter := value >= 'a' && value <= 'z' || value >= 'A' && value <= 'Z' + return letter || value == '_' || !first && value >= '0' && value <= '9' +} + func prependPathEntry(env map[string]string, entry string) { if entry == "" { return diff --git a/apps/backend/internal/agentctl/server/config/config_test.go b/apps/backend/internal/agentctl/server/config/config_test.go index bd78b615dd..27d525dee2 100644 --- a/apps/backend/internal/agentctl/server/config/config_test.go +++ b/apps/backend/internal/agentctl/server/config/config_test.go @@ -4,6 +4,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "strings" "testing" ) @@ -18,6 +19,171 @@ func TestCollectAgentEnvKeepsGitHubCLIShimAheadOfProfilePath(t *testing.T) { } } +func TestCollectAgentEnvGitHubCLIShimSurvivesLoginShell(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Bash login-shell behavior is Unix-specific") + } + shimDir := filepath.Join(t.TempDir(), "managed github shim") + if err := os.MkdirAll(shimDir, 0o700); err != nil { + t.Fatalf("create shim directory: %v", err) + } + for _, name := range []string{"agentctl", "gh"} { + path := filepath.Join(shimDir, name) + if err := os.WriteFile(path, []byte("#!/bin/sh\n"), 0o700); err != nil { + t.Fatalf("write %s shim: %v", name, err) + } + } + marker := filepath.Join(t.TempDir(), "bash-hook-ran") + parentBashEnv := filepath.Join(t.TempDir(), "parent-bash-env.sh") + if err := os.WriteFile(parentBashEnv, []byte("#!/bin/sh\nprintf hook-ran > \"$KANDEV_BASH_HOOK_MARKER\"\n"), 0o700); err != nil { + t.Fatalf("write parent Bash environment: %v", err) + } + bashEnv := filepath.Join(shimDir, "bash-env.sh") + bootstrap := "#!/bin/sh\n" + + "if [ -n \"${KANDEV_GITHUB_PARENT_BASH_ENV:-}\" ]; then . \"$KANDEV_GITHUB_PARENT_BASH_ENV\"; fi\n" + + "case \":${PATH:-}:\" in *:\"${KANDEV_GITHUB_CLI_SHIM_DIR}\":*) ;; *) PATH=\"${KANDEV_GITHUB_CLI_SHIM_DIR}:${PATH:-}\"; export PATH ;; esac\n" + if err := os.WriteFile(bashEnv, []byte(bootstrap), 0o700); err != nil { + t.Fatalf("write Bash environment fixture: %v", err) + } + + env, err := CollectAgentEnvWithError(map[string]string{ + "KANDEV_GITHUB_CREDENTIAL_BROKER_URL": "https://kandev.example/resolve", + "KANDEV_GITHUB_CLI_SHIM_DIR": shimDir, + "KANDEV_GITHUB_CLI_BASH_ENV": bashEnv, + "BASH_ENV": parentBashEnv, + "KANDEV_BASH_HOOK_MARKER": marker, + "PATH": "/usr/bin:/bin", + }) + if err != nil { + t.Fatalf("CollectAgentEnvWithError() error = %v", err) + } + if got := envSliceValue(env, "BASH_ENV"); got != bashEnv { + t.Fatalf("BASH_ENV = %q, want generated environment %q", got, bashEnv) + } + if got := envSliceValue(env, "KANDEV_GITHUB_PARENT_BASH_ENV"); got != parentBashEnv { + t.Fatalf("parent Bash environment = %q, want %q", got, parentBashEnv) + } + + command := exec.Command("bash", "-lc", "printf 'agentctl=%s\\ngh=%s\\n' \"$(command -v agentctl || true)\" \"$(command -v gh || true)\"") + command.Env = env + output, err := command.CombinedOutput() + if err != nil { + t.Fatalf("bash login shell failed: %v\n%s", err, output) + } + if !strings.Contains(string(output), "agentctl="+filepath.Join(shimDir, "agentctl")) || + !strings.Contains(string(output), "gh="+filepath.Join(shimDir, "gh")) { + t.Fatalf("login-shell tool resolution = %q", output) + } + if hook, err := os.ReadFile(marker); err != nil || string(hook) != "hook-ran" { + t.Fatalf("parent Bash hook marker = %q, error = %v", hook, err) + } +} + +func TestCollectAgentEnvResolvesParameterizedBashEnv(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Bash startup behavior is Unix-specific") + } + shimDir := t.TempDir() + hookDir := t.TempDir() + marker := filepath.Join(hookDir, "parent-hook-ran") + parentBashEnv := filepath.Join(hookDir, "parent-bash-env.sh") + if err := os.WriteFile(parentBashEnv, []byte("#!/bin/sh\nprintf hook-ran > \"$KANDEV_TEST_HOOK_MARKER\"\n"), 0o700); err != nil { + t.Fatalf("write parent Bash environment: %v", err) + } + bashEnv := filepath.Join(shimDir, "bash-env.sh") + if err := os.WriteFile(bashEnv, []byte("#!/bin/sh\n. \"$KANDEV_GITHUB_PARENT_BASH_ENV\"\n"), 0o700); err != nil { + t.Fatalf("write managed Bash environment: %v", err) + } + + env, err := CollectAgentEnvWithError(map[string]string{ + "KANDEV_GITHUB_CREDENTIAL_BROKER_URL": "https://kandev.example/resolve", + "KANDEV_GITHUB_CLI_SHIM_DIR": shimDir, + "KANDEV_GITHUB_CLI_BASH_ENV": bashEnv, + "KANDEV_TEST_HOOK_ROOT": hookDir, + "KANDEV_TEST_HOOK_MARKER": marker, + "BASH_ENV": "${KANDEV_TEST_HOOK_ROOT}/parent-bash-env.sh", + }) + if err != nil { + t.Fatalf("CollectAgentEnvWithError() error = %v", err) + } + if got := envSliceValue(env, "KANDEV_GITHUB_PARENT_BASH_ENV"); got != parentBashEnv { + t.Fatalf("parent Bash environment = %q, want resolved path %q", got, parentBashEnv) + } + command := exec.Command("bash", "-c", "true") + command.Env = env + if output, err := command.CombinedOutput(); err != nil { + t.Fatalf("parameterized Bash environment failed: %v\n%s", err, output) + } + if hook, err := os.ReadFile(marker); err != nil || string(hook) != "hook-ran" { + t.Fatalf("parent Bash hook marker = %q, error = %v", hook, err) + } +} + +func TestCollectAgentEnvAvoidsManagedBashEnvSelfSourcing(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Bash startup behavior is Unix-specific") + } + startupEnv := filepath.Join(t.TempDir(), "managed-bash-env.sh") + env, err := CollectAgentEnvWithError(map[string]string{ + "KANDEV_GITHUB_CREDENTIAL_BROKER_URL": "https://kandev.example/resolve", + "KANDEV_GITHUB_CLI_BASH_ENV": startupEnv, + "KANDEV_GITHUB_PARENT_BASH_ENV": "/stale/parent-bash-env.sh", + "BASH_ENV": startupEnv, + }) + if err != nil { + t.Fatalf("CollectAgentEnvWithError() error = %v", err) + } + if got := envSliceValue(env, "BASH_ENV"); got != startupEnv { + t.Fatalf("BASH_ENV = %q, want generated environment %q", got, startupEnv) + } + if got := envSliceValue(env, "KANDEV_GITHUB_PARENT_BASH_ENV"); got != "" { + t.Fatalf("parent Bash environment = %q, want unset for self-sourcing hook", got) + } +} + +func TestExpandBashEnvParameters(t *testing.T) { + env := map[string]string{"HOME": "/home/agent", "HOOK_ROOT": "/opt/hooks"} + for _, tc := range []struct { + name string + value string + want string + }{ + {name: "unbraced", value: "$HOME/bash-env.sh", want: "/home/agent/bash-env.sh"}, + {name: "braced", value: "${HOOK_ROOT}/bash-env.sh", want: "/opt/hooks/bash-env.sh"}, + {name: "unsupported expansion remains literal", value: "$(printf /tmp/hook)", want: "$(printf /tmp/hook)"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := expandBashEnvParameters(tc.value, env); got != tc.want { + t.Fatalf("expandBashEnvParameters(%q) = %q, want %q", tc.value, got, tc.want) + } + }) + } +} + +func TestCollectAgentEnvLeavesGitHubStartupHookUntouchedWithoutBroker(t *testing.T) { + t.Setenv("KANDEV_GITHUB_CREDENTIAL_BROKER_URL", "") + parentBashEnv := filepath.Join(t.TempDir(), "parent-bash-env.sh") + startupEnv := filepath.Join(t.TempDir(), "managed-bash-env.sh") + env, err := CollectAgentEnvWithError(map[string]string{ + "KANDEV_GITHUB_CLI_SHIM_DIR": " /managed/shims ", + "KANDEV_GITHUB_CLI_BASH_ENV": startupEnv, + "BASH_ENV": parentBashEnv, + "PATH": "/usr/bin:/bin", + }) + if err != nil { + t.Fatalf("CollectAgentEnvWithError() error = %v", err) + } + if got := envSliceValue(env, "BASH_ENV"); got != parentBashEnv { + t.Fatalf("BASH_ENV = %q, want existing hook %q", got, parentBashEnv) + } + if got := envSliceValue(env, "KANDEV_GITHUB_PARENT_BASH_ENV"); got != "" { + t.Fatalf("parent Bash environment = %q, want unset without broker", got) + } + if got := envSliceValue(env, "PATH"); got != "/usr/bin:/bin" { + t.Fatalf("PATH = %q, want unchanged without broker", got) + } +} + func TestCollectAgentEnvPreservesParentIndexedGitConfig(t *testing.T) { clearParentIndexedGitConfig(t) t.Setenv("GIT_CONFIG_COUNT", "2") diff --git a/apps/backend/internal/agentctl/server/process/shell_test.go b/apps/backend/internal/agentctl/server/process/shell_test.go index bdfab3c741..4eb95b5ca2 100644 --- a/apps/backend/internal/agentctl/server/process/shell_test.go +++ b/apps/backend/internal/agentctl/server/process/shell_test.go @@ -2,7 +2,10 @@ package process import ( "os" + osExec "os/exec" + "path/filepath" "runtime" + "strings" "testing" ) @@ -52,15 +55,69 @@ func TestShellExecArgs(t *testing.T) { if len(args) == 0 { t.Fatal("expected non-empty args") } - // The command string should appear in the args - found := false - for _, a := range args { - if a == "echo hello" { - found = true - break + if runtime.GOOS == "windows" { + if args[len(args)-1] != "echo hello" { + t.Errorf("expected command string in args, got prog=%q args=%v", prog, args) } + return } - if !found { - t.Errorf("expected command string in args, got prog=%q args=%v", prog, args) + if !strings.Contains(args[len(args)-1], "echo hello") { + t.Errorf("expected command string in shell wrapper, got prog=%q args=%v", prog, args) + } +} + +func TestShellExecArgsRestoresManagedGitHubPath(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Unix shell wrapper only") + } + _, args := shellExecArgs("echo hello") + command := args[len(args)-1] + for _, want := range []string{ + "KANDEV_GITHUB_CREDENTIAL_BROKER_URL", + "KANDEV_GITHUB_CLI_SHIM_DIR", + "PATH=\"${KANDEV_GITHUB_CLI_SHIM_DIR}:${PATH:-}\"", + "echo hello", + } { + if !strings.Contains(command, want) { + t.Fatalf("shell command missing %q: %s", want, command) + } + } +} + +func TestShellExecArgsRestoresManagedGitHubPathInProcess(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Unix shell wrapper only") + } + root := t.TempDir() + shimDir := filepath.Join(root, "managed github shim") + homeDir := filepath.Join(root, "home") + if err := os.MkdirAll(shimDir, 0o700); err != nil { + t.Fatalf("create shim directory: %v", err) + } + if err := os.MkdirAll(homeDir, 0o700); err != nil { + t.Fatalf("create home directory: %v", err) + } + marker := filepath.Join(shimDir, "kandev-marker") + if err := os.WriteFile(marker, []byte("#!/bin/sh\n"), 0o700); err != nil { + t.Fatalf("write marker executable: %v", err) + } + if err := os.WriteFile(filepath.Join(homeDir, ".profile"), []byte("PATH=/usr/bin:/bin\nexport PATH\n"), 0o700); err != nil { + t.Fatalf("write login profile: %v", err) + } + + prog, args := shellExecArgs("command -v kandev-marker") + command := osExec.Command(prog, args...) + command.Env = []string{ + "HOME=" + homeDir, + "PATH=" + shimDir + ":/usr/bin:/bin", + "KANDEV_GITHUB_CREDENTIAL_BROKER_URL=https://kandev.example/resolve", + "KANDEV_GITHUB_CLI_SHIM_DIR=" + shimDir, + } + output, err := command.CombinedOutput() + if err != nil { + t.Fatalf("managed shell command failed: %v\n%s", err, output) + } + if strings.TrimSpace(string(output)) != marker { + t.Fatalf("marker resolution = %q, want %q", output, marker) } } diff --git a/apps/backend/internal/agentctl/server/process/shell_unix.go b/apps/backend/internal/agentctl/server/process/shell_unix.go index 5443e64994..46d1e12c9a 100644 --- a/apps/backend/internal/agentctl/server/process/shell_unix.go +++ b/apps/backend/internal/agentctl/server/process/shell_unix.go @@ -40,5 +40,13 @@ func resolveShellPath(value string) string { // string through the system shell. // On Unix: sh -lc "command" func shellExecArgs(command string) (prog string, args []string) { - return "sh", []string{"-lc", command} + return "sh", []string{"-lc", managedGitHubPathRestore + command} } + +const managedGitHubPathRestore = `if [ -n "${KANDEV_GITHUB_CREDENTIAL_BROKER_URL:-}" ] && [ -n "${KANDEV_GITHUB_CLI_SHIM_DIR:-}" ]; then + case ":${PATH:-}:" in + *:"${KANDEV_GITHUB_CLI_SHIM_DIR}":*) ;; + *) PATH="${KANDEV_GITHUB_CLI_SHIM_DIR}:${PATH:-}"; export PATH ;; + esac +fi +` diff --git a/apps/backend/internal/backendapp/main.go b/apps/backend/internal/backendapp/main.go index 77dab320e5..3b52827d1e 100644 --- a/apps/backend/internal/backendapp/main.go +++ b/apps/backend/internal/backendapp/main.go @@ -503,6 +503,7 @@ func startAgentInfrastructure( log.Error("Failed to initialize orchestrator", zap.Error(err)) return false } + orchestratorSvc.SetAgentctlBinaryPath(agentctlBinaryPath) // Wire the soft-deleted-profile pre-flight into the watcher dispatch. // Orphan watchers (their agent profile was soft-deleted by the diff --git a/apps/backend/internal/githubauth/environment.go b/apps/backend/internal/githubauth/environment.go index 6512ddc33e..7b136ef2de 100644 --- a/apps/backend/internal/githubauth/environment.go +++ b/apps/backend/internal/githubauth/environment.go @@ -2,6 +2,10 @@ package githubauth const ( CredentialBrokerURLEnv = "KANDEV_GITHUB_CREDENTIAL_BROKER_URL" + CredentialHelperPathEnv = "KANDEV_GITHUB_CREDENTIAL_HELPER_PATH" + CredentialCLIShimDirEnv = "KANDEV_GITHUB_CLI_SHIM_DIR" + CredentialCLIBashEnvEnv = "KANDEV_GITHUB_CLI_BASH_ENV" + CredentialParentBashEnv = "KANDEV_GITHUB_PARENT_BASH_ENV" CredentialLeaseEnv = "KANDEV_GITHUB_CREDENTIAL_LEASE" CredentialTaskIDEnv = "KANDEV_GITHUB_CREDENTIAL_TASK_ID" CredentialSessionIDEnv = "KANDEV_GITHUB_CREDENTIAL_SESSION_ID" @@ -10,4 +14,9 @@ const ( CredentialRepoEnv = "KANDEV_GITHUB_CREDENTIAL_REPO" CredentialHostEnv = "KANDEV_GITHUB_CREDENTIAL_HOST" CredentialScopesEnv = "KANDEV_GITHUB_CREDENTIAL_SCOPES" + + ManagedGitCredentialHelper = `!"${KANDEV_GITHUB_CREDENTIAL_HELPER_PATH}" git-credential` + LegacyShimGitCredentialHelper = `!"${KANDEV_GITHUB_CLI_SHIM_DIR}/agentctl" git-credential` + LegacyGitCredentialHelper = "!agentctl git-credential" + CLIBashEnvFilename = "bash-env.sh" ) diff --git a/apps/backend/internal/orchestrator/executor/executor.go b/apps/backend/internal/orchestrator/executor/executor.go index cb4ac4c27d..2927339e42 100644 --- a/apps/backend/internal/orchestrator/executor/executor.go +++ b/apps/backend/internal/orchestrator/executor/executor.go @@ -649,6 +649,7 @@ type Executor struct { githubCredentialIssuer GitHubCredentialLeaseIssuer githubCredentialBrokerURL string githubCredentialPolicyResolver TaskGitCredentialPolicyResolver + agentctlBinaryPath string // Configuration retryLimit int diff --git a/apps/backend/internal/orchestrator/executor/executor_credentials.go b/apps/backend/internal/orchestrator/executor/executor_credentials.go index b226b16b0d..e4b64be1df 100644 --- a/apps/backend/internal/orchestrator/executor/executor_credentials.go +++ b/apps/backend/internal/orchestrator/executor/executor_credentials.go @@ -27,7 +27,9 @@ const ( envGitLabHost = "GITLAB_HOST" envKandevGitLabHost = "KANDEV_GITLAB_HOST" - gitHubCredentialHelper = "!agentctl git-credential" + gitHubCredentialHelper = githubauth.ManagedGitCredentialHelper + legacyShimGitCredentialHelper = githubauth.LegacyShimGitCredentialHelper + legacyGitHubCredentialHelper = githubauth.LegacyGitCredentialHelper defaultGitHubHost = "github.com" gitLabCredentialHelper = `!f() { echo "username=oauth2"; echo "password=$GITLAB_TOKEN"; }; f` taskGitCredentialsModeManaged = "managed" @@ -83,6 +85,12 @@ func (e *Executor) SetGitHubCredentialBroker(issuer GitHubCredentialLeaseIssuer, e.githubCredentialBrokerURL = strings.TrimSpace(brokerURL) } +// SetAgentctlBinaryPath configures the launcher-owned helper executable used +// by host-side Local and Worktree preparation before agentctl startup. +func (e *Executor) SetAgentctlBinaryPath(path string) { + e.agentctlBinaryPath = strings.TrimSpace(path) +} + // SetTaskGitCredentialPolicyResolver configures workspace-specific task Git routing. func (e *Executor) SetTaskGitCredentialPolicyResolver(resolver TaskGitCredentialPolicyResolver) { e.githubCredentialPolicyResolver = resolver @@ -195,6 +203,12 @@ func (e *Executor) configureGitHubCredentialBrokerForRepositories( req.Env[githubauth.CredentialHostEnv] = primary.Host req.Env[githubauth.CredentialScopesEnv] = string(encodedScopes) req.Env["GIT_TERMINAL_PROMPT"] = "0" + switch models.ExecutorType(req.ExecutorType) { + case "", models.ExecutorTypeLocal, models.ExecutorTypeWorktree: + if e.agentctlBinaryPath != "" { + req.Env[githubauth.CredentialHelperPathEnv] = e.agentctlBinaryPath + } + } // An empty helper resets inherited GitHub HTTPS helpers before the scoped // broker helper is appended. Other indexed Git configuration remains intact. appendGitConfig(req.Env, "credential.https://github.com.helper", "") @@ -209,6 +223,7 @@ func removeManagedGitHubCredentials(req *LaunchAgentRequest) error { } for _, key := range []string{ githubauth.CredentialBrokerURLEnv, + githubauth.CredentialHelperPathEnv, githubauth.CredentialLeaseEnv, githubauth.CredentialTaskIDEnv, githubauth.CredentialSessionIDEnv, @@ -222,12 +237,12 @@ func removeManagedGitHubCredentials(req *LaunchAgentRequest) error { } entries, err := gitconfigenv.Filter(req.Env, func(index int, entries []gitconfigenv.Entry) bool { entry := entries[index] - if entry.Key == "credential.https://github.com.helper" && entry.Value == gitHubCredentialHelper { + if entry.Key == "credential.https://github.com.helper" && isManagedGitHubCredentialHelper(entry.Value) { return false } return entry.Key != "credential.https://github.com.helper" || entry.Value != "" || index+1 >= len(entries) || entries[index+1].Key != entry.Key || - entries[index+1].Value != gitHubCredentialHelper + !isManagedGitHubCredentialHelper(entries[index+1].Value) }) if err != nil { return fmt.Errorf("remove managed GitHub credential helper: %w", err) @@ -236,6 +251,11 @@ func removeManagedGitHubCredentials(req *LaunchAgentRequest) error { return nil } +func isManagedGitHubCredentialHelper(value string) bool { + return value == gitHubCredentialHelper || value == legacyShimGitCredentialHelper || + value == legacyGitHubCredentialHelper +} + func (e *Executor) issueGitHubCredentialScope( ctx context.Context, req *LaunchAgentRequest, diff --git a/apps/backend/internal/orchestrator/executor/executor_credentials_test.go b/apps/backend/internal/orchestrator/executor/executor_credentials_test.go index 697ba5e692..36ef939220 100644 --- a/apps/backend/internal/orchestrator/executor/executor_credentials_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_credentials_test.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "os" + osExec "os/exec" "path/filepath" "strings" "testing" @@ -105,7 +106,7 @@ func TestConfigureGitHubCredentialBroker(t *testing.T) { "GIT_CONFIG_KEY_1": "credential.https://github.com.helper", "GIT_CONFIG_VALUE_1": "", "GIT_CONFIG_KEY_2": "credential.https://github.com.helper", - "GIT_CONFIG_VALUE_2": "!agentctl git-credential", + "GIT_CONFIG_VALUE_2": gitHubCredentialHelper, "GIT_CONFIG_KEY_3": "credential.useHttpPath", "GIT_CONFIG_VALUE_3": "true", "GIT_TERMINAL_PROMPT": "0", @@ -195,6 +196,93 @@ func TestApplyGitCredentialSnapshotReturnsResolverError(t *testing.T) { } } +func TestConfigureGitHubCredentialBrokerHelperSurvivesPathReset(t *testing.T) { + helperDir := filepath.Join(t.TempDir(), "managed github helper") + if err := os.MkdirAll(helperDir, 0o700); err != nil { + t.Fatalf("create helper directory: %v", err) + } + helper := filepath.Join(helperDir, "agentctl") + const helperScript = `#!/bin/sh +if [ "$1" != "git-credential" ] || [ "$2" != "get" ]; then + exit 2 +fi +printf 'username=x-access-token\npassword=fake-token\n' +` + if err := os.WriteFile(helper, []byte(helperScript), 0o700); err != nil { + t.Fatalf("write helper: %v", err) + } + + issuer := &fakeGitHubCredentialLeaseIssuer{lease: GitHubCredentialLease{Token: "opaque-lease"}} + exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) + exec.SetGitHubCredentialBroker(issuer, "https://kandev.example/api/github/credentials/resolve") + exec.SetAgentctlBinaryPath(helper) + req := &LaunchAgentRequest{ + TaskID: "task-1", WorkspaceID: "workspace-1", SessionID: "session-1", + ExecutorType: string(models.ExecutorTypeWorktree), Env: map[string]string{}, + } + info := &repoInfo{RepositoryID: "repo-1", Repository: &models.Repository{ + Provider: "github", ProviderOwner: "acme", ProviderName: "widgets", + }} + if err := exec.configureGitHubCredentialBroker(context.Background(), req, info); err != nil { + t.Fatalf("configureGitHubCredentialBroker() error = %v", err) + } + + env := make(map[string]string, len(os.Environ())+len(req.Env)+1) + for _, entry := range os.Environ() { + key, value, ok := strings.Cut(entry, "=") + if ok && !strings.HasPrefix(key, "GIT_CONFIG_") && key != githubauth.CredentialCLIShimDirEnv { + env[key] = value + } + } + for key, value := range req.Env { + env[key] = value + } + env[githubauth.CredentialBrokerURLEnv] = "https://kandev.example/api/github/credentials/resolve" + env["PATH"] = "/usr/bin:/bin" + commandEnv := make([]string, 0, len(env)) + for key, value := range env { + commandEnv = append(commandEnv, key+"="+value) + } + + command := osExec.Command("git", "credential", "fill") + command.Env = commandEnv + command.Stdin = strings.NewReader("protocol=https\nhost=github.com\npath=acme/widgets\n\n") + output, err := command.CombinedOutput() + if err != nil { + t.Fatalf("git credential fill failed with PATH reset: %v\n%s", err, output) + } + if !strings.Contains(string(output), "username=x-access-token") || + !strings.Contains(string(output), "password=fake-token") { + t.Fatalf("credential output = %q, want fake helper credentials", output) + } +} + +func TestConfigureGitHubCredentialBrokerPublishesLocalHelperBeforePreparation(t *testing.T) { + helperPath := filepath.Join(t.TempDir(), "managed agentctl") + for _, executorType := range []models.ExecutorType{models.ExecutorTypeLocal, models.ExecutorTypeWorktree} { + t.Run(string(executorType), func(t *testing.T) { + issuer := &fakeGitHubCredentialLeaseIssuer{lease: GitHubCredentialLease{Token: "opaque-lease"}} + exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) + exec.SetGitHubCredentialBroker(issuer, "https://kandev.example/api/github/credentials/resolve") + exec.SetAgentctlBinaryPath(helperPath) + req := &LaunchAgentRequest{ + TaskID: "task-1", WorkspaceID: "workspace-1", SessionID: "session-1", + ExecutorType: string(executorType), Env: map[string]string{}, + } + info := &repoInfo{RepositoryID: "repo-1", Repository: &models.Repository{ + Provider: "github", ProviderOwner: "acme", ProviderName: "widgets", + }} + + if err := exec.configureGitHubCredentialBroker(context.Background(), req, info); err != nil { + t.Fatalf("configureGitHubCredentialBroker() error = %v", err) + } + if got := req.Env[githubauth.CredentialHelperPathEnv]; got != helperPath { + t.Fatalf("credential helper path = %q, want %q before preparation", got, helperPath) + } + }) + } +} + func TestConfigureGitHubCredentialBrokerIssuesOneLeasePerRepository(t *testing.T) { issuer := &fakeGitHubCredentialLeaseIssuer{} exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) @@ -270,6 +358,7 @@ func TestConfigureGitHubCredentialBrokerSkipsExecutorInheritedPolicy(t *testing. }) req := &LaunchAgentRequest{WorkspaceID: "workspace-1", Env: map[string]string{ githubauth.CredentialBrokerURLEnv: "http://broker.example/resolve", + githubauth.CredentialHelperPathEnv: "/opt/kandev/agentctl", githubauth.CredentialLeaseEnv: "lease", githubauth.CredentialTaskIDEnv: "task-1", githubauth.CredentialSessionIDEnv: "session-1", @@ -278,13 +367,17 @@ func TestConfigureGitHubCredentialBrokerSkipsExecutorInheritedPolicy(t *testing. githubauth.CredentialRepoEnv: "widgets", githubauth.CredentialHostEnv: "github.com", githubauth.CredentialScopesEnv: `[{"repo":"widgets"}]`, - "GIT_CONFIG_COUNT": "3", + "GIT_CONFIG_COUNT": "5", "GIT_CONFIG_KEY_0": "core.hooksPath", "GIT_CONFIG_VALUE_0": "/work/hooks", "GIT_CONFIG_KEY_1": "credential.https://github.com.helper", "GIT_CONFIG_VALUE_1": "", "GIT_CONFIG_KEY_2": "credential.https://github.com.helper", - "GIT_CONFIG_VALUE_2": "!agentctl git-credential", + "GIT_CONFIG_VALUE_2": gitHubCredentialHelper, + "GIT_CONFIG_KEY_3": "credential.https://github.com.helper", + "GIT_CONFIG_VALUE_3": legacyShimGitCredentialHelper, + "GIT_CONFIG_KEY_4": "credential.https://github.com.helper", + "GIT_CONFIG_VALUE_4": legacyGitHubCredentialHelper, }} info := &repoInfo{RepositoryID: "repo-1", Repository: &models.Repository{ Provider: "github", ProviderOwner: "acme", ProviderName: "widgets", @@ -299,7 +392,7 @@ func TestConfigureGitHubCredentialBrokerSkipsExecutorInheritedPolicy(t *testing. if got := req.Env[githubauth.CredentialBrokerURLEnv]; got != "" { t.Fatalf("broker URL = %q, want none for executor inheritance", got) } - for _, key := range []string{githubauth.CredentialLeaseEnv, githubauth.CredentialTaskIDEnv, githubauth.CredentialSessionIDEnv, githubauth.CredentialRepositoryEnv, githubauth.CredentialOwnerEnv, githubauth.CredentialRepoEnv, githubauth.CredentialHostEnv, githubauth.CredentialScopesEnv} { + for _, key := range []string{githubauth.CredentialHelperPathEnv, githubauth.CredentialLeaseEnv, githubauth.CredentialTaskIDEnv, githubauth.CredentialSessionIDEnv, githubauth.CredentialRepositoryEnv, githubauth.CredentialOwnerEnv, githubauth.CredentialRepoEnv, githubauth.CredentialHostEnv, githubauth.CredentialScopesEnv} { if _, ok := req.Env[key]; ok { t.Fatalf("Env[%q] still contains a managed credential value", key) } diff --git a/apps/backend/internal/orchestrator/service.go b/apps/backend/internal/orchestrator/service.go index 05c1a773d9..ae664cd965 100644 --- a/apps/backend/internal/orchestrator/service.go +++ b/apps/backend/internal/orchestrator/service.go @@ -769,6 +769,12 @@ func (s *Service) SetGitHubCredentialBroker(issuer executor.GitHubCredentialLeas s.executor.SetGitHubCredentialBroker(issuer, endpoint) } +// SetAgentctlBinaryPath configures the host agentctl executable used by +// managed Git during Local and Worktree preparation. +func (s *Service) SetAgentctlBinaryPath(path string) { + s.executor.SetAgentctlBinaryPath(path) +} + // SetTaskGitCredentialPolicyResolver configures non-secret workspace policy lookup. func (s *Service) SetTaskGitCredentialPolicyResolver(resolver executor.TaskGitCredentialPolicyResolver) { s.executor.SetTaskGitCredentialPolicyResolver(resolver) diff --git a/docs/plans/github-managed-tools-login-shell/plan.md b/docs/plans/github-managed-tools-login-shell/plan.md new file mode 100644 index 0000000000..71892c320c --- /dev/null +++ b/docs/plans/github-managed-tools-login-shell/plan.md @@ -0,0 +1,154 @@ +--- +spec: docs/specs/integrations/github-authentication.md +created: 2026-08-01 +status: completed +--- + +# Implementation Plan: Managed GitHub Tools Across Login Shells + +## Overview + +The managed GitHub environment reaches the agent process correctly, but a later non-interactive +login shell can replace `PATH` and hide Kandev's temporary `agentctl` and `gh` shims. Repair the +exact Git failure first by making the credential helper resolve an absolute Kandev-owned +`agentctl` executable, then restore the CLI shim directory after Unix login-shell initialization so +broker-aware `gh` remains selected as well. Both changes stay conditional on a valid broker +contract and preserve executor-inheritance behavior. + +## Confirmed Root Cause + +`Executor.configureGitHubCredentialBrokerForRepositories` injects the managed helper and +`CollectAgentEnvWithError` prepends `KANDEV_GITHUB_CLI_SHIM_DIR` to the agent process `PATH`. The +running failing session contained that correct environment. Commands launched with `bash -lc` then +sourced `/etc/profile`, which replaced `PATH`; Git's `!agentctl git-credential` helper consequently +exited 127 before contacting the broker. The existing config test proves only the pre-shell +environment and therefore does not cover the failing boundary. + +## Backend + +### Make the Git credential helper path independent + +Files: + +- `apps/backend/internal/githubauth/environment.go` +- `apps/backend/cmd/agentctl/main.go` +- `apps/backend/internal/agent/runtime/lifecycle/container.go` +- `apps/backend/internal/agent/runtime/lifecycle/executor_sprites_helpers.go` +- `apps/backend/internal/orchestrator/executor/executor_credentials.go` +- `apps/backend/internal/orchestrator/executor/executor_credentials_test.go` + +Changes: + +- Define the managed helper executable and helper command as shared GitHub runtime-environment + constants. The helper uses a shell-quoted absolute executable variable instead of resolving + `agentctl` through `PATH`. +- Bind the helper executable before Docker and Sprites preparation to the installed + `/usr/local/bin/agentctl`, then let a running `agentctl` refresh it from `os.Executable` for child + processes. The temporary shim directory remains the `gh`/shell-PATH mechanism, not the Git helper + lifetime boundary. +- Keep the empty helper reset, managed helper, and `credential.useHttpPath=true` ordering unchanged. +- When executor inheritance removes managed credentials, recognize both the new helper and the + legacy `!agentctl git-credential` value so stale runtime blocks cannot retain either helper. +- Keep the shim path runtime-only; do not persist it in the task-session credential snapshot or + expose it in logs, errors, browser payloads, or process arguments. + +### Restore managed tools after login-shell initialization + +Files: + +- `apps/backend/cmd/agentctl/github_cli_shim.go` +- `apps/backend/cmd/agentctl/github_cli_shim_test.go` +- `apps/backend/internal/agentctl/server/config/config.go` +- `apps/backend/internal/agentctl/server/config/config_test.go` +- `apps/backend/internal/agentctl/server/process/shell_unix.go` +- `apps/backend/internal/agentctl/server/process/shell_test.go` + +Changes: + +- Install a mode-restricted Bash startup fragment beside the existing `agentctl` and `gh` shims. + `prepareGitHubCLIShim` publishes its path through an internal runtime environment variable. +- When and only when an instance has a broker URL, compose `BASH_ENV` so the generated fragment + runs after Bash login/profile initialization. Preserve an existing `BASH_ENV` through a separate + internal variable, resolve simple `$VAR` and `${VAR}` hook-path references from the effective + child environment, source it once, avoid recursive composition, and prepend the shim directory + idempotently. +- Prefix agentctl's Unix `sh -lc` command body with a broker-conditional, shell-quoted PATH restore. + This covers task-scoped process launches whose `/bin/sh` does not honor `BASH_ENV`; Windows keeps + its existing non-login command path. +- Broker-disabled instances, executor-inheritance sessions, and explicit profile-token sessions + retain their original `PATH` and shell-hook variables. + +## Frontend + +No frontend or browser-contract changes are required. This repair restores the already documented +managed task credential behavior. + +## Tests + +- **What:** the managed Git helper works when `PATH` deliberately excludes the helper executable and + fails closed when the managed helper is absent. + **File:** `apps/backend/internal/orchestrator/executor/executor_credentials_test.go`. + **How:** configure a broker request, publish a fake absolute `agentctl` path containing whitespace, + and run a real `git credential fill` subprocess with an ambient-only `PATH`. The RED test must + fail with a PATH- or shim-dependent helper and pass after the helper becomes path independent. +- **What:** executor inheritance removes new and legacy managed helper forms without disturbing + unrelated indexed Git configuration. + **File:** `apps/backend/internal/orchestrator/executor/executor_credentials_test.go`. + **How:** table-driven filtering tests over ordered indexed configuration blocks. +- **What:** a non-interactive Bash login shell that replaces `PATH` runs an existing `BASH_ENV` + hook and then resolves both managed shims ahead of ambient tools. + **Files:** `apps/backend/cmd/agentctl/github_cli_shim_test.go` and + `apps/backend/internal/agentctl/server/config/config_test.go`. + **How:** install real temporary shims plus a marker hook, collect a broker-enabled instance + environment, and execute a short-lived Bash login subprocess. Assert the marker and resolved + executable paths without invoking the credential broker. +- **What:** agentctl Unix command processes restore the managed shim path after login initialization, + while broker-disabled commands preserve their existing runtime behavior apart from the + broker-conditional wrapper. + **File:** `apps/backend/internal/agentctl/server/process/shell_test.go`. + **How:** unit-test the broker-conditional shell prelude and run a short-lived `sh -lc` process + with a login profile that deliberately resets `PATH` and a non-secret marker executable. + +## E2E Tests + +No Playwright test is planned. There is no UI change, and a browser fixture would either expose a +credential environment or merely duplicate the subprocess boundary exercised by focused backend +tests. + +## Verification Results + +- Task 01 RED/GREEN and focused helper checks passed; see + `task-01-path-independent-git-helper.md`. +- Task 02 RED/GREEN, generated-startup-script, affected-package, lifecycle, and Git-config checks + passed; see `task-02-login-shell-shim-restoration.md`. +- Local semantic review found two lifecycle edge cases after Task 02: remote preparation ran before + the shim directory existed, and parameterized parent `BASH_ENV` paths were sourced literally. + Task 03 records their TDD remediation. Its RED checks failed at all four intended boundaries; + the focused remediation suite passed 73 tests and the complete affected-package suite passed + 2,000 tests across six packages. +- PR #2141 review found that Local and Worktree preparation also precedes task-instance creation, + so those host paths still lacked the launcher-owned helper executable. It also found an + overflow-prone Sprites slice-capacity expression. Task 04 records the TDD and CodeQL remediation; + seven focused regressions and 2,969 tests across five affected packages passed, and the CI-style + changed-file Go lint reported no issues. +- `git diff --check` passed. No temporary artifacts remain. + +## Implementation Waves And Parallel Candidates + +Execute sequentially in the primary conversation. The tasks change the shared managed GitHub +runtime-environment contract, so they are not parallel-safe. + +- [x] [Task 01: Path-independent Git credential helper](task-01-path-independent-git-helper.md) +- [x] [Task 02: Login-shell shim restoration](task-02-login-shell-shim-restoration.md) +- [x] [Task 03: Review blocker remediation](task-03-review-blocker-remediation.md) +- [x] [Task 04: PR review fixup](task-04-pr-review-fixup.md) + +## Risks And Out Of Scope + +- Shell startup hooks are an execution boundary. The repair must compose an existing `BASH_ENV`, + avoid recursive sourcing, and keep all secret broker values out of script contents and argv. +- Git helper resolution is shell-independent after Task 01. Task 02 covers non-interactive Bash + launched by agents and agentctl's Unix `sh -lc` processes; it does not rewrite user profile files + or attempt to override a command that deliberately changes `PATH` after startup. +- Windows does not use the Unix login-shell wrapper and retains its current managed PATH behavior. +- No database, API, UI, credential-policy, lease, token-cache, or persistence changes are included. diff --git a/docs/plans/github-managed-tools-login-shell/task-01-path-independent-git-helper.md b/docs/plans/github-managed-tools-login-shell/task-01-path-independent-git-helper.md new file mode 100644 index 0000000000..27b972b1a0 --- /dev/null +++ b/docs/plans/github-managed-tools-login-shell/task-01-path-independent-git-helper.md @@ -0,0 +1,75 @@ +--- +id: "01-path-independent-git-helper" +title: "Path-independent Git credential helper" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 01: Path-Independent Git Credential Helper + +## Acceptance + +- Managed Git invokes the instance-owned `agentctl` helper even when the active shell `PATH` does + not contain the helper executable, including an absolute helper path containing whitespace. +- The helper reset/order and fail-closed behavior remain unchanged; no ambient helper or prompt is + used when the managed helper is missing or fails. +- Executor inheritance removes both current and legacy managed-helper entries while preserving + unrelated indexed Git configuration in order. + +## Verification + +RED first: + +```bash +cd apps/backend && rtk go test ./internal/orchestrator/executor -run 'TestConfigureGitHubCredentialBrokerHelperSurvivesPathReset' -count=1 +``` + +GREEN and focused regression checks: + +```bash +cd apps/backend && rtk go test ./internal/orchestrator/executor -run 'Test.*(GitHubCredential|ManagedGitHubHelper)' -count=1 +``` + +## Files likely touched + +- `apps/backend/internal/githubauth/environment.go` +- `apps/backend/internal/orchestrator/executor/executor_credentials.go` +- `apps/backend/internal/orchestrator/executor/executor_credentials_test.go` +- this task file and `plan.md` + +## Dependencies + +None. + +## Parallelism + +Sequential. Task 02 consumes the shared managed-helper and shim-environment constants established +here. + +## Inputs + +- Spec: managed runtime-tool behavior, security constraints, failure modes, and login-PATH scenarios. +- Plan: confirmed root cause and path-independent helper design. +- Existing patterns: `configureGitHubCredentialBrokerForRepositories`, + `removeManagedGitHubCredentials`, and `gitconfigenv.Filter`. + +## Output contract + +Report the RED failure reason, final helper command and quoting behavior, legacy-removal behavior, +files changed, exact test results, security/secret boundary, and synchronized task/plan status. + +## Results + +- RED: `cd apps/backend && rtk go test ./internal/orchestrator/executor -run 'TestConfigureGitHubCredentialBrokerHelperSurvivesPathReset' -count=1` + failed as expected with `agentctl: not found` and Git's terminal-prompt-disabled error. +- GREEN: the same regression test initially passed after the helper changed to the shim-directory + variable. Task 03 superseded that lifetime assumption with a dedicated absolute helper-executable + variable so preparation-time Git works before the shim directory exists. +- GREEN: `cd apps/backend && rtk go test ./internal/orchestrator/executor -run 'Test.*(GitHubCredential|ManagedGitHubHelper)' -count=1` + passed (7 tests). +- The regression uses a temporary helper path containing whitespace and fake non-secret credentials; + the test process owns and removes its temporary directory automatically. No credential values, + leases, or tokens were persisted. Final `git diff --check` passed after Task 02 completed. diff --git a/docs/plans/github-managed-tools-login-shell/task-02-login-shell-shim-restoration.md b/docs/plans/github-managed-tools-login-shell/task-02-login-shell-shim-restoration.md new file mode 100644 index 0000000000..5f7add3165 --- /dev/null +++ b/docs/plans/github-managed-tools-login-shell/task-02-login-shell-shim-restoration.md @@ -0,0 +1,81 @@ +--- +id: "02-login-shell-shim-restoration" +title: "Login-shell shim restoration" +status: done +wave: 2 +depends_on: ["01-path-independent-git-helper"] +plan: "plan.md" +spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 02: Login-Shell Shim Restoration + +## Acceptance + +- A broker-enabled non-interactive Bash login shell preserves an existing `BASH_ENV` hook and + restores the instance-owned `agentctl` and `gh` shims ahead of ambient tools after profile + initialization resets `PATH`. +- Agentctl Unix `sh -lc` task processes restore the same managed path immediately before the + requested command, including shells that do not honor `BASH_ENV`. +- Broker-disabled, executor-inheritance, explicit profile-token, and Windows launch paths receive + no managed Bash hook or shell prelude behavior change. + +## Verification + +RED first: + +```bash +cd apps/backend && rtk go test ./cmd/agentctl ./internal/agentctl/server/config -run 'Test.*GitHubCLIShim.*LoginShell' -count=1 +``` + +GREEN and focused regression checks: + +```bash +cd apps/backend && rtk go test ./cmd/agentctl ./internal/agentctl/server/config ./internal/agentctl/server/process -run 'Test.*(GitHubCLIShim|CollectAgentEnv|ShellExec|ManagedGitHubPath|BashEnv)' -count=1 +``` + +## Files likely touched + +- `apps/backend/cmd/agentctl/github_cli_shim.go` +- `apps/backend/cmd/agentctl/github_cli_shim_test.go` +- `apps/backend/internal/agentctl/server/config/config.go` +- `apps/backend/internal/agentctl/server/config/config_test.go` +- `apps/backend/internal/agentctl/server/process/shell_unix.go` +- `apps/backend/internal/agentctl/server/process/shell_test.go` +- this task file and `plan.md` + +## Dependencies + +Task 01 establishes the shared managed-helper and shim-environment contract. + +## Parallelism + +Sequential. + +## Inputs + +- Spec: managed runtime-tool behavior, runtime-only environment security, and login-PATH scenarios. +- Plan: Bash startup-fragment composition and agentctl `sh -lc` restoration design. +- Existing patterns: `installGitHubCLIShim`, `prepareGitHubCLIShim`, + `CollectAgentEnvWithError`, `prependPathEntry`, and `shellExecArgs`. + +## Output contract + +Report the RED failure reason, startup-hook precedence and recursion handling, broker activation +conditions, Unix/Windows behavior, files changed, exact test results, cleanup evidence, remaining +shell portability limits, and synchronized task/plan status. + +## Results + +- RED: `cd apps/backend && rtk go test ./cmd/agentctl ./internal/agentctl/server/config -run 'Test.*GitHubCLIShim.*LoginShell' -count=1` + failed as expected: the generated Bash environment was absent and the collected `BASH_ENV` + remained the parent hook. +- GREEN: `cd apps/backend && rtk go test ./cmd/agentctl ./internal/agentctl/server/config ./internal/agentctl/server/process -run 'Test.*(GitHubCLIShim|CollectAgentEnv|ShellExec|ManagedGitHubPath|BashEnv)' -count=1` + passed (14 tests), including the actual generated startup script, existing-hook preservation, + and the Unix shell wrapper. +- Additional focused checks passed: `go test ./cmd/agentctl -run 'TestInstallGitHubCLIShim.*' -count=1` + (3 tests); the affected backend package set (`cmd/agentctl`, agentctl config/process, + orchestrator executor, and `githubauth`) passed; lifecycle GitHub/environment checks passed (52 + tests); and `internal/gitconfigenv` passed (5 tests). +- All temporary shim directories and hook files are test-owned and cleaned up by `t.TempDir`. + No broker values or credentials were persisted. Final `git diff --check` passed. diff --git a/docs/plans/github-managed-tools-login-shell/task-03-review-blocker-remediation.md b/docs/plans/github-managed-tools-login-shell/task-03-review-blocker-remediation.md new file mode 100644 index 0000000000..5ed9533c1b --- /dev/null +++ b/docs/plans/github-managed-tools-login-shell/task-03-review-blocker-remediation.md @@ -0,0 +1,84 @@ +--- +id: "03-review-blocker-remediation" +title: "Review blocker remediation" +status: done +wave: 3 +depends_on: ["01-path-independent-git-helper", "02-login-shell-shim-restoration"] +plan: "plan.md" +spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 03: Review Blocker Remediation + +## Acceptance + +- Docker and Sprites preparation publish an absolute Kandev-owned credential-helper executable + before a prepare-script clone can invoke the managed indexed Git configuration. +- A running `agentctl` publishes its own absolute executable for agent, terminal, and task-command + child processes; Git helper execution never depends on ambient `PATH` or the later-created CLI + shim directory. +- Executor inheritance removes the current executable-variable helper plus both prior managed + helper forms without disturbing unrelated indexed Git configuration. +- An inherited `BASH_ENV` path containing `$VAR` or `${VAR}` references is resolved from the + effective child environment before the generated startup fragment sources it. + +## Verification + +RED first: + +```bash +cd apps/backend && rtk go test ./internal/agent/runtime/lifecycle ./internal/agentctl/server/config -run 'Test.*(ManagedGitCredentialHelperBeforeAgentctl|ParameterizedBashEnv)' -count=1 +``` + +GREEN and focused regression checks: + +```bash +cd apps/backend && rtk go test ./cmd/agentctl ./internal/agentctl/server/config ./internal/agentctl/server/process ./internal/agent/runtime/lifecycle ./internal/orchestrator/executor ./internal/githubauth -run 'Test.*(GitHub|ManagedGit|BashEnv|ShellExec|ContainerConfig|SpriteEnv)' -count=1 +``` + +## Files likely touched + +- `apps/backend/internal/githubauth/environment.go` +- `apps/backend/cmd/agentctl/main.go` +- `apps/backend/internal/agent/runtime/lifecycle/container.go` +- `apps/backend/internal/agent/runtime/lifecycle/container_config_test.go` +- `apps/backend/internal/agent/runtime/lifecycle/executor_sprites_helpers.go` +- `apps/backend/internal/agent/runtime/lifecycle/executor_sprites_clone_auth_test.go` +- `apps/backend/internal/agentctl/server/config/config.go` +- `apps/backend/internal/agentctl/server/config/config_test.go` +- `apps/backend/internal/orchestrator/executor/executor_credentials.go` +- `apps/backend/internal/orchestrator/executor/executor_credentials_test.go` +- this task file, `plan.md`, and the linked spec + +## Dependencies + +Tasks 01 and 02 establish the helper and login-shell contracts being corrected. + +## Parallelism + +Sequential. Both regressions alter the shared managed GitHub environment contract and its tests. + +## Output contract + +Report both RED failure reasons, the final helper executable lifetime, inherited-hook expansion +behavior, exact targeted test results, secret/runtime-only boundaries, and synchronized task/plan +status. + +## Results + +- RED: the lifecycle/config command failed in three intended tests because Docker and Sprites did + not publish a pre-start helper executable and the parent `BASH_ENV` remained literal. The real + Git regression failed with `/agentctl: not found`; the agentctl startup regression observed an + empty helper-executable variable. +- GREEN: `rtk go test ./cmd/agentctl ./internal/agentctl/server/config + ./internal/agentctl/server/process ./internal/agent/runtime/lifecycle + ./internal/orchestrator/executor ./internal/githubauth -run + 'Test.*(GitHub|ManagedGit|BashEnv|ShellExec|ContainerConfig|SpriteEnv)' -count=1` passed 73 tests. +- Full affected packages: the same six packages without `-run` passed 2,000 tests. +- Docker and Sprites bind `/usr/local/bin/agentctl` only for broker-enabled preparation. Agentctl + replaces that boundary value with its absolute `os.Executable` path for children. The helper + remains fail-closed, and no lease, token, scope, or credential value is written to a file or + process argument. +- Parent Bash hooks now resolve simple `$VAR` and `${VAR}` path references from the effective child + environment without `eval`; unsupported shell expressions remain literal rather than being + executed by Kandev's composition logic. diff --git a/docs/plans/github-managed-tools-login-shell/task-04-pr-review-fixup.md b/docs/plans/github-managed-tools-login-shell/task-04-pr-review-fixup.md new file mode 100644 index 0000000000..27d148800c --- /dev/null +++ b/docs/plans/github-managed-tools-login-shell/task-04-pr-review-fixup.md @@ -0,0 +1,55 @@ +--- +id: "04-pr-review-fixup" +title: "PR review fixup" +status: done +wave: 4 +depends_on: ["03-review-blocker-remediation"] +plan: "plan.md" +spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 04: PR Review Fixup + +## Acceptance + +- Broker-enabled Local and Worktree requests publish the standalone launcher's absolute + `agentctl` path before checkout, worktree creation, or repository setup can invoke Git. +- Executor-inheritance requests remove that managed helper path together with the broker lease and + indexed helper configuration. +- Sprites environment construction avoids overflow-prone capacity arithmetic without changing its + filtering or managed-helper output. +- A managed Bash startup environment never records itself as its inherited parent hook. + +## Verification + +RED first: + +```bash +cd apps/backend && rtk go test ./internal/orchestrator/executor -run '^TestConfigureGitHubCredentialBrokerPublishesLocalHelperBeforePreparation$' -count=1 +cd apps/backend && rtk go test ./internal/orchestrator/executor -run '^TestConfigureGitHubCredentialBrokerSkipsExecutorInheritedPolicy$' -count=1 +``` + +GREEN and affected-package checks: + +```bash +cd apps/backend && rtk go test ./internal/orchestrator/executor ./internal/agent/runtime/lifecycle -run '^Test(ConfigureGitHubCredentialBroker(HelperSurvivesPathReset|PublishesLocalHelperBeforePreparation|SkipsExecutorInheritedPolicy)|BuildSpriteEnvPublishesManagedGitCredentialHelperBeforeAgentctlStartup)$' -count=1 +cd apps/backend && rtk go test ./internal/agentctl/server/config -run '^TestCollectAgentEnvAvoidsManagedBashEnvSelfSourcing$' -count=1 +cd apps/backend && rtk go test ./internal/backendapp ./internal/orchestrator ./internal/orchestrator/executor ./internal/agent/runtime/lifecycle ./internal/agentctl/server/config -count=1 +cd apps/backend && rtk golangci-lint run ./... --new-from-rev="$(git merge-base origin/main HEAD)" --timeout=5m +``` + +## Results + +- The Local/Worktree RED test failed because the executor had no launcher-path seam. The + executor-inheritance RED test failed because the managed path remained in the request. +- GitHub CodeQL alert 301 supplied the static RED evidence for `len(env)+1`; reproducing an integer + overflow dynamically would require an infeasibly max-sized Go map. +- The final focused commands passed six tests across the executor and lifecycle packages plus the + reviewer-requested Bash self-source guard. The five complete affected-package suites passed + 2,969 tests, and changed-file Go lint reported no issues. +- The launcher path is injected only for host-side Local, Worktree, and legacy-empty executor + types. Docker and Sprites retain their installed remote path binding, and executor inheritance + removes the host path. No lease, token, scope, or credential value is persisted or logged. +- CodeRabbit's optional shared-shell-constant refactor was not adopted: the two call sites have + different broker and parent-hook guards, and moving their partial overlap into `githubauth` would + add raw-shell coupling without changing the managed path contract. diff --git a/docs/specs/integrations/github-authentication.md b/docs/specs/integrations/github-authentication.md index be474e4435..ed20337b97 100644 --- a/docs/specs/integrations/github-authentication.md +++ b/docs/specs/integrations/github-authentication.md @@ -1,7 +1,7 @@ --- -status: draft +status: building created: 2026-07-19 -amended: 2026-07-31 +amended: 2026-08-01 owner: Kandev --- @@ -78,6 +78,16 @@ automation under different GitHub Apps without operating separate Kandev deploym only in memory. PAT/CLI tokens retain their provider-granted scope once delivered to a trusted agent subprocess. GitHub HTTPS and the broker-aware `gh` shim fail closed rather than consulting another ambient helper after a managed-helper failure. +- Managed Git helper execution does not depend on the post-startup `PATH`: Git resolves an + absolute Kandev-owned `agentctl` executable published before the first managed Git operation. + Local and Worktree preparation binds the helper to the standalone launcher's absolute executable + before checkout or setup scripts run. Remote preparation binds it to the installed executor + binary before cloning, and a running `agentctl` publishes its own executable for child processes. + Non-interactive Unix login shells that replace their inherited `PATH` restore the managed + CLI-shim directory after profile initialization for broker-enabled tasks, while preserving + pre-existing Bash environment hooks, including hook paths containing `$VAR` or `${VAR}` + references from the effective child environment. Broker-disabled and executor-inheritance + processes receive no shell hook or managed-tool path. - Under managed routing, every authorized task execution surface receives the same current task-scoped Git environment: the agent subprocess, terminal shells, passthrough-agent PTYs, and task-scoped command processes. This includes the broker contract, managed indexed Git @@ -386,6 +396,12 @@ post-signature processing failures produce `failing`; a later valid successful d prompts, and activates Kandev's `agentctl`/`gh` tool directory only for broker-enabled task instances. It does not claim to prevent a host-authority agent from manually switching a remote to SSH or invoking another credential-bearing tool. +- The helper command uses a Kandev-owned absolute executable variable rather than searching ambient + `PATH`. The variable is bound before executor preparation and refreshed from `os.Executable` by + a running `agentctl`. Unix shell-startup restoration composes with an inherited `BASH_ENV`, + resolves simple environment-variable references in that hook path from the effective child + environment, remains conditional on the broker contract, and never places broker leases, tokens, + or credential scopes in shell arguments or startup files. - The effective task Git environment is runtime-only. It is copied only after the existing task/session or task-environment ownership check, is never persisted in task metadata or terminal records, and is never written to logs, errors, browser payloads, or process arguments. @@ -595,6 +611,25 @@ registration and never creates a global default. **THEN** the profile token wins and the session disclosure labels its actor runtime-selected. - **GIVEN** a managed helper cannot execute or redeem its lease, **WHEN** Git requests GitHub HTTPS credentials, **THEN** the command fails without falling through to a personal helper or prompt. +- **GIVEN** a broker-enabled managed task whose login profile replaces its inherited `PATH`, + **WHEN** Git requests GitHub HTTPS credentials, **THEN** the configured helper invokes the + instance-owned `agentctl` directly and does not search or fall through to an ambient helper. +- **GIVEN** a broker-enabled Local or Worktree task whose checkout or setup script invokes Git + before the task instance is created, **WHEN** Git requests GitHub HTTPS credentials, **THEN** the + configured helper invokes the standalone launcher's absolute `agentctl` executable without + consulting `PATH` or an ambient helper. +- **GIVEN** a broker-enabled Docker or Sprites task whose prepare script clones before `agentctl` + starts, **WHEN** Git requests GitHub HTTPS credentials during that clone, **THEN** the configured + helper invokes the already-installed absolute executor binary and redeems the task lease without + consulting `PATH` or an ambient helper. +- **GIVEN** a broker-enabled managed task with an existing Bash environment hook, **WHEN** a + non-interactive login shell replaces `PATH`, **THEN** the existing hook still runs and the + Kandev-managed `agentctl` and `gh` shims are restored ahead of ambient tools before the requested + command starts. +- **GIVEN** that existing Bash environment hook is expressed as `$HOME/hook.sh` or + `${KANDEV_HOOK_ROOT}/hook.sh`, **WHEN** Kandev composes its managed startup fragment, **THEN** it + resolves the reference from the effective child environment and sources the intended hook rather + than a filename containing literal dollar-sign text. - **GIVEN** the host or executor exports indexed Git config for `core.hooksPath` and `notes.augment.mergeStrategy`, **WHEN** Kandev appends its managed GitHub helper configuration, **THEN** the agent receives one contiguous block containing the original entries first and the @@ -659,6 +694,8 @@ the [executor clone transport repair plan](../../plans/github-executor-clone-transport/plan.md), and the [managed task terminal environment plan](../../plans/task-terminal-git-environment/plan.md), and the +[managed GitHub login-shell repair plan](../../plans/github-managed-tools-login-shell/plan.md), +and the [system-service identity guardrails repair plan](../../plans/system-service-identity-guardrails/plan.md). ## Decision