diff --git a/apps/backend/cmd/agentctl/github_cli_shim.go b/apps/backend/cmd/agentctl/github_cli_shim.go index a4dc035ac8..246e5ba0a3 100644 --- a/apps/backend/cmd/agentctl/github_cli_shim.go +++ b/apps/backend/cmd/agentctl/github_cli_shim.go @@ -267,6 +267,10 @@ func installGitHubCLIShim(agentctlExecutable, tempRoot string) (string, func(), cleanup() return "", nil, fmt.Errorf("install gh shim: %w", err) } + if err := linkOrCopyExecutable(agentctlExecutable, filepath.Join(dir, "agentctl")); err != nil { + cleanup() + return "", nil, fmt.Errorf("install agentctl helper: %w", err) + } return dir, cleanup, nil } diff --git a/apps/backend/cmd/agentctl/github_cli_shim_test.go b/apps/backend/cmd/agentctl/github_cli_shim_test.go index 03838df194..d45701b615 100644 --- a/apps/backend/cmd/agentctl/github_cli_shim_test.go +++ b/apps/backend/cmd/agentctl/github_cli_shim_test.go @@ -12,7 +12,7 @@ import ( "testing" ) -func TestInstallGitHubCLIShimCreatesGHEntrypoint(t *testing.T) { +func TestInstallGitHubCLIShimCreatesManagedTools(t *testing.T) { root := t.TempDir() binary := filepath.Join(root, "agentctl") if err := os.WriteFile(binary, []byte("agentctl"), 0o700); err != nil { @@ -24,12 +24,14 @@ func TestInstallGitHubCLIShimCreatesGHEntrypoint(t *testing.T) { t.Fatalf("installGitHubCLIShim() error = %v", err) } t.Cleanup(cleanup) - entrypoint := filepath.Join(shimDir, githubCLIShimName()) - if _, err := os.Stat(entrypoint); err != nil { - t.Fatalf("stat gh shim: %v", err) - } - if filepath.Dir(entrypoint) != shimDir { - t.Fatalf("entrypoint = %q, want inside %q", entrypoint, shimDir) + for _, name := range []string{githubCLIShimName(), "agentctl"} { + entrypoint := filepath.Join(shimDir, name) + if _, err := os.Stat(entrypoint); err != nil { + t.Fatalf("stat %s managed tool: %v", name, err) + } + if filepath.Dir(entrypoint) != shimDir { + t.Fatalf("entrypoint = %q, want inside %q", entrypoint, shimDir) + } } } diff --git a/apps/backend/cmd/agentctl/main.go b/apps/backend/cmd/agentctl/main.go index 12ef23331a..aa39d5ac3a 100644 --- a/apps/backend/cmd/agentctl/main.go +++ b/apps/backend/cmd/agentctl/main.go @@ -18,7 +18,6 @@ import ( "os/exec" "os/signal" "strconv" - "strings" "syscall" "time" @@ -28,7 +27,6 @@ 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" @@ -119,9 +117,6 @@ func runGitHubUtilityCommand() (int, bool) { } func prepareGitHubCLIShim() (func(), error) { - if os.Getenv(githubauth.CredentialBrokerURLEnv) == "" { - return func() {}, nil - } executable, err := os.Executable() if err != nil { return nil, fmt.Errorf("resolve agentctl executable for gh shim: %w", err) @@ -134,12 +129,6 @@ func prepareGitHubCLIShim() (func(), error) { cleanup() return nil, fmt.Errorf("configure gh shim directory: %w", err) } - path := pathWithoutDirectory(os.Getenv("PATH"), shimDir) - path = strings.Join([]string{shimDir, path}, string(os.PathListSeparator)) - if err := os.Setenv("PATH", path); err != nil { - cleanup() - return nil, fmt.Errorf("configure gh shim PATH: %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 471e15001a..2caec90de1 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/container.go +++ b/apps/backend/internal/agent/runtime/lifecycle/container.go @@ -8,7 +8,6 @@ import ( "encoding/hex" "fmt" "os" - "strconv" "strings" "time" @@ -18,12 +17,14 @@ import ( "github.com/kandev/kandev/internal/agent/docker" agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/gitconfigenv" ) const ( - dockerAgentctlInstancePortBase = 41001 - dockerAgentctlInstancePortMax = 41100 - boolStringTrue = "true" + dockerAgentctlInstancePortBase = 41001 + dockerAgentctlInstancePortMax = 41100 + boolStringTrue = "true" + gitHubCredentialHelperConfigKey = "credential.https://github.com.helper" ) // ContainerConfig holds configuration for launching a Docker container @@ -451,7 +452,10 @@ func (cm *ContainerManager) buildContainerConfig(config ContainerConfig) (docker } // Build environment variables - env := cm.buildEnvVars(config) + env, err := cm.buildEnvVars(config) + if err != nil { + return docker.ContainerConfig{}, err + } // Calculate resource limits memoryBytes := rt.ResourceLimits.MemoryMB * 1024 * 1024 @@ -607,7 +611,7 @@ func (cm *ContainerManager) expandMountSource(source, workspacePath string) stri } // buildEnvVars builds environment variables for the container -func (cm *ContainerManager) buildEnvVars(config ContainerConfig) []string { +func (cm *ContainerManager) buildEnvVars(config ContainerConfig) ([]string, error) { ag := config.AgentConfig rt := ag.Runtime() env := make([]string, 0) @@ -632,40 +636,32 @@ func (cm *ContainerManager) buildEnvVars(config ContainerConfig) []string { // - Trust all directories (for mounted workspaces) // - URL rewriting: SSH → HTTPS for GitHub (enables token auth) // - Credential helper for GitHub HTTPS auth (uses GH_TOKEN env var) - gitConfigCount := 3 - env = append(env, - "GIT_CONFIG_KEY_0=safe.directory", - "GIT_CONFIG_VALUE_0=*", - "GIT_CONFIG_KEY_1=url.https://github.com/.insteadOf", - "GIT_CONFIG_VALUE_1=git@github.com:", - "GIT_CONFIG_KEY_2=url.https://github.com/.insteadOf", - "GIT_CONFIG_VALUE_2=ssh://git@github.com/", - ) + gitConfig := map[string]string{ + "GIT_CONFIG_COUNT": "3", + "GIT_CONFIG_KEY_0": "safe.directory", + "GIT_CONFIG_VALUE_0": "*", + "GIT_CONFIG_KEY_1": "url.https://github.com/.insteadOf", + "GIT_CONFIG_VALUE_1": "git@github.com:", + "GIT_CONFIG_KEY_2": "url.https://github.com/.insteadOf", + "GIT_CONFIG_VALUE_2": "ssh://git@github.com/", + } // If GitHub token is provided, add credential helper // Use ${GH_TOKEN:-${GITHUB_TOKEN}} to support either env var being set if config.Credentials["GH_TOKEN"] != "" || config.Credentials["GITHUB_TOKEN"] != "" { - env = append(env, - "GIT_CONFIG_KEY_3=credential.https://github.com.helper", - `GIT_CONFIG_VALUE_3=!f() { echo "username=x-access-token"; echo "password=${GH_TOKEN:-${GITHUB_TOKEN}}"; }; f`, - ) - gitConfigCount = 4 + gitConfig["GIT_CONFIG_COUNT"] = "4" + gitConfig["GIT_CONFIG_KEY_3"] = gitHubCredentialHelperConfigKey + gitConfig["GIT_CONFIG_VALUE_3"] = `!f() { echo "username=x-access-token"; echo "password=${GH_TOKEN:-${GITHUB_TOKEN}}"; }; f` } - if incomingCount, err := strconv.Atoi(config.Credentials["GIT_CONFIG_COUNT"]); err == nil { - for i := 0; i < incomingCount; i++ { - key, keyOK := config.Credentials[fmt.Sprintf("GIT_CONFIG_KEY_%d", i)] - value, valueOK := config.Credentials[fmt.Sprintf("GIT_CONFIG_VALUE_%d", i)] - if !keyOK || !valueOK { - continue - } - env = append(env, - fmt.Sprintf("GIT_CONFIG_KEY_%d=%s", gitConfigCount, key), - fmt.Sprintf("GIT_CONFIG_VALUE_%d=%s", gitConfigCount, value), - ) - gitConfigCount++ - } + gitConfig, err := gitconfigenv.Merge(gitConfig, config.Credentials) + if err != nil { + return nil, fmt.Errorf("compose container Git config: %w", err) } - env = append(env, fmt.Sprintf("GIT_CONFIG_COUNT=%d", gitConfigCount)) + gitConfigEntries, err := gitconfigenv.EnvironmentEntries(gitConfig) + if err != nil { + return nil, fmt.Errorf("serialize container Git config: %w", err) + } + env = append(env, gitConfigEntries...) // Inject credentials from the provided credentials map for k, v := range config.Credentials { @@ -691,7 +687,7 @@ func (cm *ContainerManager) buildEnvVars(config ContainerConfig) []string { env = append(env, "AGENTCTL_BOOTSTRAP_NONCE="+config.BootstrapNonce) } - return env + return env, nil } // generateBootstrapNonce creates a cryptographically random 32-byte hex-encoded nonce. diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_docker_test.go b/apps/backend/internal/agent/runtime/lifecycle/executor_docker_test.go index d991e42a05..621ac1c28b 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_docker_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_docker_test.go @@ -909,10 +909,13 @@ func TestContainerBuildEnvVarsMergesEphemeralGitLabCredentialHelper(t *testing.T "GIT_CONFIG_KEY_0": "credential.http://gitlab.internal:8080.helper", "GIT_CONFIG_VALUE_0": `!f() { echo "username=oauth2"; echo "password=$GITLAB_TOKEN"; }; f`, } - env := (&ContainerManager{}).buildEnvVars(ContainerConfig{ + env, err := (&ContainerManager{}).buildEnvVars(ContainerConfig{ AgentConfig: agents.NewMockAgent(), Credentials: credentials, }) + if err != nil { + t.Fatalf("buildEnvVars() error = %v", err) + } joined := strings.Join(env, "\n") if !strings.Contains(joined, "GIT_CONFIG_COUNT=4") || !strings.Contains(joined, "GIT_CONFIG_KEY_3=credential.http://gitlab.internal:8080.helper") { t.Fatalf("GitLab credential helper was not merged:\n%s", joined) @@ -925,6 +928,19 @@ func TestContainerBuildEnvVarsMergesEphemeralGitLabCredentialHelper(t *testing.T } } +func TestContainerBuildEnvVarsRejectsMalformedIndexedGitConfig(t *testing.T) { + _, err := (&ContainerManager{}).buildEnvVars(ContainerConfig{ + AgentConfig: agents.NewMockAgent(), + Credentials: map[string]string{ + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "core.hooksPath", + }, + }) + if err == nil { + t.Fatal("buildEnvVars() error = nil, want malformed indexed Git config error") + } +} + func TestLocalPathFromCloneURL(t *testing.T) { tests := []struct { name string diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go index b130638d5d..0c44311ed0 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go @@ -19,6 +19,7 @@ import ( "github.com/kandev/kandev/internal/agent/runtime/activity" "github.com/kandev/kandev/internal/agent/settings/cliflags" "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/gitconfigenv" storageworkspaces "github.com/kandev/kandev/internal/system/storage/workspaces" "github.com/kandev/kandev/internal/task/models" "github.com/kandev/kandev/internal/worktree" @@ -467,7 +468,7 @@ func invalidScratchPathID(id string) bool { // launchPrepareRequest copies the launch request, sets the resolved workspace path, // populates metadata from the request fields, and injects profile environment variables. -func (m *Manager) launchPrepareRequest(req *LaunchRequest, profileInfo *AgentProfileInfo, workspacePath string) (LaunchRequest, string) { +func (m *Manager) launchPrepareRequest(req *LaunchRequest, profileInfo *AgentProfileInfo, workspacePath string) (LaunchRequest, string, error) { executionID := uuid.New().String() reqWithWorktree := *req reqWithWorktree.WorkspacePath = workspacePath @@ -496,22 +497,27 @@ func (m *Manager) launchPrepareRequest(req *LaunchRequest, profileInfo *AgentPro reqWithWorktree.Env["AGENTCTL_AUTO_APPROVE_PERMISSIONS"] = "true" } } - mergeRouteOverrideEnv(&reqWithWorktree) - return reqWithWorktree, executionID + if err := mergeRouteOverrideEnv(&reqWithWorktree); err != nil { + return LaunchRequest{}, "", err + } + return reqWithWorktree, executionID, nil } // mergeRouteOverrideEnv preserves legacy model-only routing overlays. // Concrete execution profiles own their complete environment. -func mergeRouteOverrideEnv(req *LaunchRequest) { +func mergeRouteOverrideEnv(req *LaunchRequest) error { if req == nil || hasConcreteRouteExecutionProfile(req) || req.RouteOverride == nil || len(req.RouteOverride.Env) == 0 { - return + return nil } if req.Env == nil { req.Env = make(map[string]string, len(req.RouteOverride.Env)) } - for k, v := range req.RouteOverride.Env { - req.Env[k] = v + merged, err := gitconfigenv.Merge(req.Env, req.RouteOverride.Env) + if err != nil { + return fmt.Errorf("compose legacy route environment: %w", err) } + req.Env = merged + return nil } // newProgressCallback builds a PrepareProgressCallback that publishes progress events for a task/session. @@ -1069,7 +1075,11 @@ func (m *Manager) launchInternal(ctx context.Context, req *LaunchRequest) (*Agen } // 5 & 6. Prepare the request copy with metadata and profile env - reqWithWorktree, executionID := m.launchPrepareRequest(req, profileInfo, workspacePath) + reqWithWorktree, executionID, err := m.launchPrepareRequest(req, profileInfo, workspacePath) + if err != nil { + m.publishLaunchPrepareCompleted(req, prepResult, progressRecorder, workspacePath, false, err) + return nil, err + } // 6b. Deploy per-profile skills + custom prompt (ADR 0005 Wave A). // Best-effort: a deploy failure is logged but does not abort the launch diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch_test.go index 9477b377e7..33598fad58 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_launch_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch_test.go @@ -553,7 +553,9 @@ func TestConcreteExecutionProfileIgnoresLegacyRouteFlagsAndEnv(t *testing.T) { if len(flags) != 1 || flags[0] != "--profile-flag" { t.Fatalf("flags = %v, want execution profile flags only", flags) } - mergeRouteOverrideEnv(req) + if err := mergeRouteOverrideEnv(req); err != nil { + t.Fatalf("mergeRouteOverrideEnv() error = %v", err) + } if req.Env["PROFILE_ENV"] != "profile" { t.Fatalf("PROFILE_ENV = %q, want execution profile value", req.Env["PROFILE_ENV"]) } @@ -575,12 +577,60 @@ func TestLegacyRouteStillAppliesFlagsAndEnv(t *testing.T) { if len(flags) != 2 || flags[1] != "--legacy-route-flag" { t.Fatalf("flags = %v, want legacy route flag appended", flags) } - mergeRouteOverrideEnv(req) + if err := mergeRouteOverrideEnv(req); err != nil { + t.Fatalf("mergeRouteOverrideEnv() error = %v", err) + } if req.Env["ROUTE_ONLY"] != "value" { t.Fatalf("legacy route env missing: %v", req.Env) } } +func TestLegacyRouteComposesIndexedGitConfig(t *testing.T) { + req := &LaunchRequest{ + Env: map[string]string{ + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "core.hooksPath", + "GIT_CONFIG_VALUE_0": "/opt/locstat/hooks", + }, + RouteOverride: &RouteOverride{Env: map[string]string{ + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": gitHubCredentialHelperConfigKey, + "GIT_CONFIG_VALUE_0": "!agentctl git-credential", + }}, + } + + if err := mergeRouteOverrideEnv(req); err != nil { + t.Fatalf("mergeRouteOverrideEnv() error = %v", err) + } + + if got := req.Env["GIT_CONFIG_COUNT"]; got != "2" { + t.Fatalf("GIT_CONFIG_COUNT = %q, want 2", got) + } + if got := req.Env["GIT_CONFIG_KEY_0"]; got != "core.hooksPath" { + t.Fatalf("GIT_CONFIG_KEY_0 = %q, want base entry", got) + } + if got := req.Env["GIT_CONFIG_VALUE_0"]; got != "/opt/locstat/hooks" { + t.Fatalf("GIT_CONFIG_VALUE_0 = %q, want base value", got) + } + if got := req.Env["GIT_CONFIG_KEY_1"]; got != gitHubCredentialHelperConfigKey { + t.Fatalf("GIT_CONFIG_KEY_1 = %q, want route entry", got) + } + if got := req.Env["GIT_CONFIG_VALUE_1"]; got != "!agentctl git-credential" { + t.Fatalf("GIT_CONFIG_VALUE_1 = %q, want route value", got) + } +} + +func TestLegacyRouteRejectsMalformedIndexedGitConfig(t *testing.T) { + req := &LaunchRequest{RouteOverride: &RouteOverride{Env: map[string]string{ + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "core.hooksPath", + }}} + + if err := mergeRouteOverrideEnv(req); err == nil { + t.Fatal("mergeRouteOverrideEnv() error = nil, want malformed Git config error") + } +} + func TestBuildEnvForExecution_DoesNotCopyTaskDescriptionToEnv(t *testing.T) { mgr := newTestManager(t) env, err := mgr.buildEnvForExecution( @@ -1041,6 +1091,35 @@ func TestLaunch_PublishesPrepareCompletedAfterRuntimeProgress(t *testing.T) { requirePrepareStep(t, final.Steps, "Waiting for Docker container") } +func TestLaunch_PublishesPrepareCompletionOnLegacyRouteEnvError(t *testing.T) { + log := newTestLogger() + eventBus := &MockEventBusWithTracking{} + mgr := NewManager( + newTestRegistry(), eventBus, nil, + &MockCredentialsManager{}, &MockProfileResolver{}, nil, + ExecutorFallbackWarn, "", log, + ) + cleanupManagerStopCh(t, mgr) + + _, err := mgr.Launch(context.Background(), &LaunchRequest{ + TaskID: "task-route-env-error", + SessionID: "session-route-env-error", + AgentProfileID: "profile-1", + ExecutorType: string(models.ExecutorTypeLocal), + IsEphemeral: true, + RouteOverride: &RouteOverride{Env: map[string]string{ + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "core.hooksPath", + }}, + }) + require.ErrorContains(t, err, "compose legacy route environment") + + completed := prepareCompletedPayloads(eventBus) + require.Len(t, completed, 1) + require.False(t, completed[0].Success) + require.Contains(t, completed[0].ErrorMessage, "compose legacy route environment") +} + func prepareCompletedPayloads(eventBus *MockEventBusWithTracking) []*PrepareCompletedEventPayload { eventBus.mu.Lock() defer eventBus.mu.Unlock() diff --git a/apps/backend/internal/agent/runtime/lifecycle/profile_env.go b/apps/backend/internal/agent/runtime/lifecycle/profile_env.go index b49035a6a5..f533b32d73 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/profile_env.go +++ b/apps/backend/internal/agent/runtime/lifecycle/profile_env.go @@ -6,6 +6,7 @@ import ( "go.uber.org/zap" settingsmodels "github.com/kandev/kandev/internal/agent/settings/models" + "github.com/kandev/kandev/internal/gitconfigenv" ) // metadataKeyProfileEnvResolved caches resolved profile env vars on an execution @@ -62,13 +63,17 @@ func mergeEnvFillMissing(dst, src map[string]string) { return } for k, v := range src { - if v == "" { + if v == "" || gitconfigenv.IsIndexedKey(k) { continue } if _, exists := dst[k]; !exists { dst[k] = v } } + merged, err := gitconfigenv.Merge(src, dst) + if err == nil { + gitconfigenv.CopyIndexed(dst, merged) + } } // resolveAgentProfileEnvVars resolves profile env entries. SecretID wins over diff --git a/apps/backend/internal/agent/runtime/lifecycle/profile_env_test.go b/apps/backend/internal/agent/runtime/lifecycle/profile_env_test.go index 035a1c5000..a1e09e97c7 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/profile_env_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/profile_env_test.go @@ -20,6 +20,58 @@ func TestMergeEnvFillMissing(t *testing.T) { } } +func TestMergeEnvFillMissingComposesIndexedGitConfig(t *testing.T) { + dst := map[string]string{ + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "credential.https://github.com.helper", + "GIT_CONFIG_VALUE_0": "!agentctl git-credential", + } + src := map[string]string{ + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "core.hooksPath", + "GIT_CONFIG_VALUE_0": "/opt/locstat/hooks", + } + + mergeEnvFillMissing(dst, src) + + if got := dst["GIT_CONFIG_COUNT"]; got != "2" { + t.Fatalf("GIT_CONFIG_COUNT = %q, want 2", got) + } + if got := dst["GIT_CONFIG_KEY_0"]; got != "core.hooksPath" { + t.Fatalf("GIT_CONFIG_KEY_0 = %q, want inherited profile entry", got) + } + if got := dst["GIT_CONFIG_KEY_1"]; got != "credential.https://github.com.helper" { + t.Fatalf("GIT_CONFIG_KEY_1 = %q, want request entry", got) + } +} + +func TestMergeEnvFillMissingComposesIndexedGitConfigMismatchedCount(t *testing.T) { + dst := map[string]string{ + "GIT_CONFIG_COUNT": "1", + "GIT_CONFIG_KEY_0": "credential.https://github.com.helper", + "GIT_CONFIG_VALUE_0": "!agentctl git-credential", + } + src := map[string]string{ + "GIT_CONFIG_COUNT": "2", + "GIT_CONFIG_KEY_0": "core.hooksPath", + "GIT_CONFIG_VALUE_0": "/opt/locstat/hooks", + "GIT_CONFIG_KEY_1": "core.autocrlf", + "GIT_CONFIG_VALUE_1": "input", + } + + mergeEnvFillMissing(dst, src) + + if got := dst["GIT_CONFIG_COUNT"]; got != "3" { + t.Fatalf("GIT_CONFIG_COUNT = %q, want 3", got) + } + if got := dst["GIT_CONFIG_KEY_0"]; got != "core.hooksPath" { + t.Fatalf("GIT_CONFIG_KEY_0 = %q, want inherited profile entry", got) + } + if got := dst["GIT_CONFIG_KEY_2"]; got != "credential.https://github.com.helper" { + t.Fatalf("GIT_CONFIG_KEY_2 = %q, want request entry", got) + } +} + func TestResolveAgentProfileEnvVars_SecretAndValue(t *testing.T) { store := newInMemorySecretStore() _ = store.Create(context.Background(), &secrets.SecretWithValue{ diff --git a/apps/backend/internal/agentctl/server/config/config.go b/apps/backend/internal/agentctl/server/config/config.go index 3e0423ccf1..8f7ea0327f 100644 --- a/apps/backend/internal/agentctl/server/config/config.go +++ b/apps/backend/internal/agentctl/server/config/config.go @@ -22,6 +22,7 @@ import ( "sync" "time" + "github.com/kandev/kandev/internal/gitconfigenv" "github.com/kandev/kandev/pkg/agent" ) @@ -494,6 +495,29 @@ func ValidateCommandArgs(args []string) error { // CollectAgentEnv collects environment variables to pass to the agent. // It filters out AGENTCTL_* variables and optionally merges additional env vars. func CollectAgentEnv(additional map[string]string) []string { + env, err := CollectAgentEnvWithError(additional) + if err == nil { + return env + } + // Legacy callers cannot surface an error. Preserve their filtered parent + // environment rather than returning nil when an optional overlay is bad. + return CollectAgentEnvWithoutAdditional() +} + +func CollectAgentEnvWithoutAdditional() []string { + env := make([]string, 0, len(os.Environ())) + for _, value := range os.Environ() { + if idx := strings.Index(value, "="); idx > 0 && + !strings.HasPrefix(value[:idx], "AGENTCTL_") && !isNpmEnvVar(value[:idx]) { + env = append(env, value) + } + } + return env +} + +// CollectAgentEnvWithError collects environment variables for an agent instance. +// It rejects malformed indexed Git configuration rather than changing Git's effective config. +func CollectAgentEnvWithError(additional map[string]string) ([]string, error) { envMap := make(map[string]string) // Start with current environment, excluding AGENTCTL_* and npm_config_* vars. @@ -508,9 +532,12 @@ func CollectAgentEnv(additional map[string]string) []string { } } - // Merge additional env vars - for k, v := range additional { - envMap[k] = v + // Merge additional env vars. Git's indexed configuration environment is a + // single ordered protocol, so it must not be overlaid key by key. + var err error + envMap, err = gitconfigenv.Merge(envMap, additional) + 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"]) @@ -521,7 +548,7 @@ func CollectAgentEnv(additional map[string]string) []string { for k, v := range envMap { result = append(result, k+"="+v) } - return result + return result, nil } func prependPathEntry(env map[string]string, entry string) { diff --git a/apps/backend/internal/agentctl/server/config/config_test.go b/apps/backend/internal/agentctl/server/config/config_test.go index e3b581fa33..2eb264c931 100644 --- a/apps/backend/internal/agentctl/server/config/config_test.go +++ b/apps/backend/internal/agentctl/server/config/config_test.go @@ -2,6 +2,8 @@ package config import ( "os" + "os/exec" + "path/filepath" "strings" "testing" ) @@ -16,6 +18,74 @@ func TestCollectAgentEnvKeepsGitHubCLIShimAheadOfProfilePath(t *testing.T) { } } +func TestCollectAgentEnvPreservesParentIndexedGitConfig(t *testing.T) { + t.Setenv("GIT_CONFIG_COUNT", "2") + t.Setenv("GIT_CONFIG_KEY_0", "core.hooksPath") + t.Setenv("GIT_CONFIG_VALUE_0", "/opt/locstat/hooks") + t.Setenv("GIT_CONFIG_KEY_1", "notes.augment.mergeStrategy") + t.Setenv("GIT_CONFIG_VALUE_1", "cat_sort_uniq") + + env := CollectAgentEnv(map[string]string{ + "GIT_CONFIG_COUNT": "2", + "GIT_CONFIG_KEY_0": "credential.https://github.com.helper", + "GIT_CONFIG_VALUE_0": "!agentctl git-credential", + "GIT_CONFIG_KEY_1": "credential.useHttpPath", + "GIT_CONFIG_VALUE_1": "true", + }) + + if got := envSliceValue(env, "GIT_CONFIG_COUNT"); got != "4" { + t.Fatalf("GIT_CONFIG_COUNT = %q, want 4", got) + } + if got := envSliceValue(env, "GIT_CONFIG_KEY_0"); got != "core.hooksPath" { + t.Fatalf("GIT_CONFIG_KEY_0 = %q, want core.hooksPath", got) + } + if got := envSliceValue(env, "GIT_CONFIG_KEY_2"); got != "credential.https://github.com.helper" { + t.Fatalf("GIT_CONFIG_KEY_2 = %q, want managed helper", got) + } +} + +func TestCollectAgentEnvPreservesParentGitConfigHook(t *testing.T) { + repository := t.TempDir() + hooks := t.TempDir() + marker := filepath.Join(t.TempDir(), "hook-ran") + hookPath := filepath.Join(hooks, "pre-commit") + if err := os.WriteFile(hookPath, []byte("#!/bin/sh\ntouch '"+marker+"'\n"), 0o755); err != nil { + t.Fatalf("write hook: %v", err) + } + runGit(t, nil, "init", repository) + runGit(t, nil, "-C", repository, "config", "user.name", "Kandev Test") + runGit(t, nil, "-C", repository, "config", "user.email", "kandev@example.test") + + t.Setenv("GIT_CONFIG_COUNT", "2") + t.Setenv("GIT_CONFIG_KEY_0", "core.hooksPath") + t.Setenv("GIT_CONFIG_VALUE_0", hooks) + t.Setenv("GIT_CONFIG_KEY_1", "notes.augment.mergeStrategy") + t.Setenv("GIT_CONFIG_VALUE_1", "cat_sort_uniq") + env := CollectAgentEnv(map[string]string{ + "GIT_CONFIG_COUNT": "2", + "GIT_CONFIG_KEY_0": "credential.https://github.com.helper", + "GIT_CONFIG_VALUE_0": "!agentctl git-credential", + "GIT_CONFIG_KEY_1": "credential.useHttpPath", + "GIT_CONFIG_VALUE_1": "true", + }) + + runGit(t, env, "-C", repository, "commit", "--allow-empty", "-m", "verify inherited hook") + if _, err := os.Stat(marker); err != nil { + t.Fatalf("inherited pre-commit hook did not run: %v", err) + } +} + +func runGit(t *testing.T, env []string, args ...string) { + t.Helper() + command := exec.Command("git", args...) + if env != nil { + command.Env = env + } + if output, err := command.CombinedOutput(); err != nil { + t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, output) + } +} + func envSliceValue(env []string, key string) string { prefix := key + "=" for _, entry := range env { diff --git a/apps/backend/internal/agentctl/server/instance/manager.go b/apps/backend/internal/agentctl/server/instance/manager.go index b62ee44f8e..b197f9bed7 100644 --- a/apps/backend/internal/agentctl/server/instance/manager.go +++ b/apps/backend/internal/agentctl/server/instance/manager.go @@ -81,6 +81,10 @@ func (m *Manager) SetServerFactory(factory ServerFactory) { func (m *Manager) CreateInstance(ctx context.Context, req *CreateRequest) (*CreateResponse, error) { m.mu.Lock() defer m.mu.Unlock() + agentEnv, err := config.CollectAgentEnvWithError(req.Env) + if err != nil { + return nil, fmt.Errorf("prepare agent environment: %w", err) + } id := req.ID if id == "" { @@ -109,7 +113,7 @@ func (m *Manager) CreateInstance(ctx context.Context, req *CreateRequest) (*Crea AgentCommand: agentCmd, WorkDir: req.WorkspacePath, AutoStart: &autoStart, - Env: config.CollectAgentEnv(req.Env), + Env: agentEnv, AutoApprovePermissions: req.AutoApprovePermissions, AgentType: req.AgentType, McpServers: mcpServers, diff --git a/apps/backend/internal/backendapp/orchestrator.go b/apps/backend/internal/backendapp/orchestrator.go index 06b360d946..9da8aa7b1b 100644 --- a/apps/backend/internal/backendapp/orchestrator.go +++ b/apps/backend/internal/backendapp/orchestrator.go @@ -99,6 +99,7 @@ func provideOrchestrator( githubExecutorCredentialLeaseAdapter{service: githubSvc}, githubCredentialBrokerEndpoint(cfg), ) + orchestratorSvc.SetTaskGitCredentialPolicyResolver(githubExecutorCredentialPolicyAdapter{service: githubSvc}) } taskSvc.SetExecutionStopper(orchestratorSvc) taskSvc.SetGitArchiveCapture(orchestratorSvc) @@ -178,6 +179,26 @@ func provideOrchestrator( type githubCredentialLeaseService interface { IssueGitHubCredentialLease(context.Context, githubpkg.CredentialLeaseRequest) (*githubpkg.CredentialLease, error) + DescribeTaskGitCredentialPolicy(context.Context, string) (githubpkg.TaskGitCredentialPolicy, error) +} + +type githubExecutorCredentialPolicyAdapter struct { + service githubCredentialLeaseService +} + +func (a githubExecutorCredentialPolicyAdapter) ResolveTaskGitCredentialPolicy( + ctx context.Context, + workspaceID string, +) (executorpkg.TaskGitCredentialPolicy, error) { + policy, err := a.service.DescribeTaskGitCredentialPolicy(ctx, workspaceID) + if err != nil { + return executorpkg.TaskGitCredentialPolicy{}, err + } + return executorpkg.TaskGitCredentialPolicy{ + Mode: policy.Mode, + WorkspaceMethod: policy.WorkspaceMethod, + WorkspaceActor: policy.WorkspaceActor, + }, nil } type githubExecutorCredentialLeaseAdapter struct { diff --git a/apps/backend/internal/backendapp/services_github_broker_test.go b/apps/backend/internal/backendapp/services_github_broker_test.go index 17386d26e8..79f39ceffe 100644 --- a/apps/backend/internal/backendapp/services_github_broker_test.go +++ b/apps/backend/internal/backendapp/services_github_broker_test.go @@ -23,6 +23,13 @@ func (s *fakeGitHubCredentialLeaseService) IssueGitHubCredentialLease( return &githubpkg.CredentialLease{Token: "lease-token"}, nil } +func (*fakeGitHubCredentialLeaseService) DescribeTaskGitCredentialPolicy( + context.Context, + string, +) (githubpkg.TaskGitCredentialPolicy, error) { + return githubpkg.TaskGitCredentialPolicy{Mode: githubpkg.TaskGitCredentialsModeManaged}, nil +} + func TestGitHubExecutorCredentialLeaseAdapterMapsScope(t *testing.T) { service := &fakeGitHubCredentialLeaseService{} adapter := githubExecutorCredentialLeaseAdapter{service: service} diff --git a/apps/backend/internal/gitconfigenv/environment.go b/apps/backend/internal/gitconfigenv/environment.go new file mode 100644 index 0000000000..87046e9cad --- /dev/null +++ b/apps/backend/internal/gitconfigenv/environment.go @@ -0,0 +1,211 @@ +// Package gitconfigenv composes Git's indexed environment configuration. +package gitconfigenv + +import ( + "fmt" + "strconv" + "strings" +) + +const ( + countKey = "GIT_CONFIG_COUNT" + keyPrefix = "GIT_CONFIG_KEY_" + valuePrefix = "GIT_CONFIG_VALUE_" + maxEntries = 256 +) + +// Entry is one Git configuration key/value pair supplied through the +// GIT_CONFIG_COUNT environment protocol. +type Entry struct { + Key string + Value string +} + +// Merge combines ordinary environment variables with Git's indexed configuration block. +// Overlay values take precedence for ordinary variables; indexed Git entries retain source order. +func Merge(base, overlay map[string]string) (map[string]string, error) { + baseEntries, err := entriesFrom(base) + if err != nil { + return nil, fmt.Errorf("base Git config: %w", err) + } + overlayEntries, err := entriesFrom(overlay) + if err != nil { + return nil, fmt.Errorf("overlay Git config: %w", err) + } + result := make(map[string]string) + for key, value := range base { + if !isIndexedKey(key) { + result[key] = value + } + } + for key, value := range overlay { + if !isIndexedKey(key) { + result[key] = value + } + } + entries := removeBoundaryOverlap(baseEntries, overlayEntries) + writeEntries(result, entries) + return result, nil +} + +// CopyIndexed replaces target's indexed Git configuration with source's validated block. +func CopyIndexed(target, source map[string]string) { + for key := range target { + if isIndexedKey(key) { + delete(target, key) + } + } + for key, value := range source { + if isIndexedKey(key) { + target[key] = value + } + } +} + +// EnvironmentEntries serializes a validated indexed Git configuration block. +func EnvironmentEntries(env map[string]string) ([]string, error) { + entries, err := entriesFrom(env) + if err != nil { + return nil, err + } + if len(entries) == 0 { + return nil, nil + } + result := make([]string, 0, len(entries)*2+1) + result = append(result, fmt.Sprintf("%s=%d", countKey, len(entries))) + for index, entry := range entries { + result = append(result, + fmt.Sprintf("%s%d=%s", keyPrefix, index, entry.Key), + fmt.Sprintf("%s%d=%s", valuePrefix, index, entry.Value), + ) + } + return result, nil +} + +// Filter returns a copy of env with indexed Git configuration entries that do +// not satisfy keep removed. The complete ordered block is provided so callers +// can remove a related pair without disturbing unrelated host configuration. +// Ordinary environment variables are preserved. +func Filter(env map[string]string, keep func(index int, entries []Entry) bool) (map[string]string, error) { + entries, err := entriesFrom(env) + if err != nil { + return nil, err + } + result := make(map[string]string, len(env)) + for key, value := range env { + if !isIndexedKey(key) { + result[key] = value + } + } + filtered := make([]Entry, 0, len(entries)) + for index, entry := range entries { + if keep(index, entries) { + filtered = append(filtered, entry) + } + } + writeEntries(result, filtered) + return result, nil +} + +func isIndexedKey(key string) bool { + return key == countKey || strings.HasPrefix(key, keyPrefix) || strings.HasPrefix(key, valuePrefix) +} + +// IsIndexedKey reports whether key belongs to Git's indexed environment +// configuration block. +func IsIndexedKey(key string) bool { + return isIndexedKey(key) +} + +func entriesFrom(env map[string]string) ([]Entry, error) { + if len(env) == 0 { + return nil, nil + } + countValue, hasCount := env[countKey] + if !hasCount { + if hasIndexedEntry(env) { + return nil, fmt.Errorf("%s is required when indexed entries are set", countKey) + } + return nil, nil + } + count, err := strconv.Atoi(countValue) + if err != nil || count < 0 || count > maxEntries { + return nil, fmt.Errorf("%s must be between 0 and %d", countKey, maxEntries) + } + entries := make([]Entry, 0, count) + for index := 0; index < count; index++ { + key, keyOK := env[fmt.Sprintf("%s%d", keyPrefix, index)] + value, valueOK := env[fmt.Sprintf("%s%d", valuePrefix, index)] + if !keyOK || !valueOK || key == "" { + return nil, fmt.Errorf("entry %d must include a key and value", index) + } + entries = append(entries, Entry{Key: key, Value: value}) + } + if hasOutOfRangeIndexedEntry(env, count) { + return nil, fmt.Errorf("indexed entry exceeds %s", countKey) + } + return entries, nil +} + +func hasIndexedEntry(env map[string]string) bool { + for key := range env { + if strings.HasPrefix(key, keyPrefix) || strings.HasPrefix(key, valuePrefix) { + return true + } + } + return false +} + +func hasOutOfRangeIndexedEntry(env map[string]string, count int) bool { + for key := range env { + index, ok := indexedEntryIndex(key) + if ok && index >= count { + return true + } + } + return false +} + +func indexedEntryIndex(key string) (int, bool) { + prefix := keyPrefix + if strings.HasPrefix(key, valuePrefix) { + prefix = valuePrefix + } else if !strings.HasPrefix(key, keyPrefix) { + return 0, false + } + index, err := strconv.Atoi(strings.TrimPrefix(key, prefix)) + return index, err == nil && index >= 0 +} + +func removeBoundaryOverlap(base, overlay []Entry) []Entry { + maxOverlap := min(len(base), len(overlay)) + for overlap := maxOverlap; overlap > 0; overlap-- { + if entriesEqual(base[len(base)-overlap:], overlay[:overlap]) { + return append(append([]Entry{}, base...), overlay[overlap:]...) + } + } + return append(append([]Entry{}, base...), overlay...) +} + +func entriesEqual(left, right []Entry) bool { + if len(left) != len(right) { + return false + } + for index := range left { + if left[index] != right[index] { + return false + } + } + return true +} + +func writeEntries(env map[string]string, entries []Entry) { + if len(entries) == 0 { + return + } + env[countKey] = strconv.Itoa(len(entries)) + for index, entry := range entries { + env[fmt.Sprintf("%s%d", keyPrefix, index)] = entry.Key + env[fmt.Sprintf("%s%d", valuePrefix, index)] = entry.Value + } +} diff --git a/apps/backend/internal/gitconfigenv/environment_test.go b/apps/backend/internal/gitconfigenv/environment_test.go new file mode 100644 index 0000000000..dba04b71e5 --- /dev/null +++ b/apps/backend/internal/gitconfigenv/environment_test.go @@ -0,0 +1,111 @@ +package gitconfigenv + +import "testing" + +func TestIndexedGitConfigMergePreservesOrderedEntries(t *testing.T) { + merged, err := Merge(map[string]string{ + "GIT_CONFIG_COUNT": "2", + "GIT_CONFIG_KEY_0": "core.hooksPath", + "GIT_CONFIG_VALUE_0": "/opt/locstat/hooks", + "GIT_CONFIG_KEY_1": "notes.augment.mergeStrategy", + "GIT_CONFIG_VALUE_1": "cat_sort_uniq", + }, map[string]string{ + "GIT_CONFIG_COUNT": "2", + "GIT_CONFIG_KEY_0": "credential.https://github.com.helper", + "GIT_CONFIG_VALUE_0": "!agentctl git-credential", + "GIT_CONFIG_KEY_1": "credential.useHttpPath", + "GIT_CONFIG_VALUE_1": "true", + }) + if err != nil { + t.Fatalf("Merge() error = %v", err) + } + + if got := merged["GIT_CONFIG_COUNT"]; got != "4" { + t.Fatalf("GIT_CONFIG_COUNT = %q, want 4", got) + } + if got := merged["GIT_CONFIG_KEY_1"]; got != "notes.augment.mergeStrategy" { + t.Fatalf("GIT_CONFIG_KEY_1 = %q, want host entry", got) + } + if got := merged["GIT_CONFIG_KEY_2"]; got != "credential.https://github.com.helper" { + t.Fatalf("GIT_CONFIG_KEY_2 = %q, want managed entry", got) + } +} + +func TestIndexedGitConfigMergeCollapsesOnlyExactBoundaryOverlap(t *testing.T) { + managedHelper := Entry{Key: "credential.https://github.com.helper", Value: "!agentctl git-credential"} + merged, err := Merge(environmentWithEntries( + Entry{Key: "safe.directory", Value: "*"}, + managedHelper, + ), environmentWithEntries( + managedHelper, + Entry{Key: "credential.useHttpPath", Value: "true"}, + )) + if err != nil { + t.Fatalf("Merge() error = %v", err) + } + + if got := merged["GIT_CONFIG_COUNT"]; got != "3" { + t.Fatalf("GIT_CONFIG_COUNT = %q, want 3", got) + } + if got := merged["GIT_CONFIG_KEY_2"]; got != "credential.useHttpPath" { + t.Fatalf("GIT_CONFIG_KEY_2 = %q, want trailing overlay entry", got) + } +} + +func TestIndexedGitConfigMergeRetainsMeaningfulRepeatedEntries(t *testing.T) { + merged, err := Merge(environmentWithEntries( + Entry{Key: "url.https://github.com/.insteadOf", Value: "git@github.com:"}, + ), environmentWithEntries( + Entry{Key: "url.https://github.com/.insteadOf", Value: "ssh://git@github.com/"}, + )) + if err != nil { + t.Fatalf("Merge() error = %v", err) + } + + if got := merged["GIT_CONFIG_COUNT"]; got != "2" { + t.Fatalf("GIT_CONFIG_COUNT = %q, want 2", got) + } +} + +func TestIndexedGitConfigMergeRejectsMalformedBlock(t *testing.T) { + _, err := Merge(map[string]string{ + "GIT_CONFIG_COUNT": "2", + "GIT_CONFIG_KEY_0": "core.hooksPath", + }, nil) + if err == nil { + t.Fatal("Merge() error = nil, want malformed Git config error") + } +} + +func TestFilterRemovesOnlyRequestedIndexedEntries(t *testing.T) { + env := map[string]string{ + "GIT_CONFIG_COUNT": "2", + "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": "!agentctl git-credential", + "UNCHANGED": "yes", + } + + filtered, err := Filter(env, func(index int, entries []Entry) bool { + return entries[index].Value != "!agentctl git-credential" + }) + if err != nil { + t.Fatalf("Filter() error = %v", err) + } + if got, want := filtered["GIT_CONFIG_COUNT"], "1"; got != want { + t.Fatalf("GIT_CONFIG_COUNT = %q, want %q", got, want) + } + if got, want := filtered["GIT_CONFIG_KEY_0"], "core.hooksPath"; got != want { + t.Fatalf("GIT_CONFIG_KEY_0 = %q, want %q", got, want) + } + if got, want := filtered["UNCHANGED"], "yes"; got != want { + t.Fatalf("UNCHANGED = %q, want %q", got, want) + } +} + +func environmentWithEntries(entries ...Entry) map[string]string { + env := map[string]string{} + writeEntries(env, entries) + return env +} diff --git a/apps/backend/internal/github/auth_resolver.go b/apps/backend/internal/github/auth_resolver.go index 8040cfe897..69083724ad 100644 --- a/apps/backend/internal/github/auth_resolver.go +++ b/apps/backend/internal/github/auth_resolver.go @@ -71,6 +71,11 @@ type credentialCacheEpoch struct { workspace uint64 } +type credentialCacheEntry struct { + resolved *ResolvedCredential + cachedAt time.Time +} + // CredentialResolver owns credential selection for all GitHub operations. // Its cache is keyed by workspace and generation so replacement or revocation // cannot reuse a client created for another principal. @@ -86,7 +91,7 @@ type CredentialResolver struct { now func() time.Time mu sync.Mutex - cache map[credentialCacheKey]*ResolvedCredential + cache map[credentialCacheKey]credentialCacheEntry allEpoch uint64 workspaceEpochs map[string]uint64 } @@ -103,7 +108,7 @@ func NewCredentialResolver(connections workspaceConnectionReader, secrets authSe }, ghToken: ResolveGHAccountToken, now: time.Now, - cache: make(map[credentialCacheKey]*ResolvedCredential), + cache: make(map[credentialCacheKey]credentialCacheEntry), workspaceEpochs: make(map[string]uint64), } } @@ -443,10 +448,15 @@ func (r *CredentialResolver) resolveLegacy( func (r *CredentialResolver) cached(key credentialCacheKey) *ResolvedCredential { r.mu.Lock() defer r.mu.Unlock() - resolved := r.cache[key] - if resolved == nil { + entry, ok := r.cache[key] + if !ok || entry.resolved == nil { return nil } + if key.source == ConnectionSourceGHCLI && !entry.cachedAt.Add(credentialCacheRefreshMargin).After(r.now()) { + delete(r.cache, key) + return nil + } + resolved := entry.resolved if !resolved.ExpiresAt.IsZero() && !resolved.ExpiresAt.After(r.now().Add(credentialCacheRefreshMargin)) { delete(r.cache, key) return nil @@ -469,7 +479,7 @@ func (r *CredentialResolver) storeCached( delete(r.cache, existing) } } - r.cache[key] = cloneResolvedCredential(resolved) + r.cache[key] = credentialCacheEntry{resolved: cloneResolvedCredential(resolved), cachedAt: r.now()} return true } diff --git a/apps/backend/internal/github/auth_resolver_test.go b/apps/backend/internal/github/auth_resolver_test.go index 65d1e91f1b..0d2431c768 100644 --- a/apps/backend/internal/github/auth_resolver_test.go +++ b/apps/backend/internal/github/auth_resolver_test.go @@ -3,6 +3,7 @@ package github import ( "context" "errors" + "fmt" "sync" "testing" "time" @@ -289,6 +290,39 @@ func TestCredentialResolverNamedCLIUsesSelectedLogin(t *testing.T) { } } +func TestCredentialResolverNamedCLIRefreshesAfterFiveMinutes(t *testing.T) { + now := time.Date(2026, 7, 27, 12, 0, 0, 0, time.UTC) + connections := &fakeConnectionReader{workspaces: map[string]*WorkspaceConnection{ + "work": {WorkspaceID: "work", Source: ConnectionSourceGHCLI, GitHubHost: "github.com", Login: "work-user", Status: ConnectionStatusActive, CredentialGeneration: 2}, + }} + resolver := NewCredentialResolver(connections, nil) + resolver.now = func() time.Time { return now } + calls := 0 + resolver.ghToken = func(_ context.Context, _, _ string) (string, error) { + calls++ + return fmt.Sprintf("cli-token-%d", calls), nil + } + request := ResolveCredentialRequest{WorkspaceID: "work", Purpose: CredentialPurposeAutomation} + + first, err := resolver.Resolve(context.Background(), request) + if err != nil { + t.Fatal(err) + } + now = now.Add(4*time.Minute + 59*time.Second) + stillCached, err := resolver.Resolve(context.Background(), request) + if err != nil { + t.Fatal(err) + } + now = now.Add(time.Second) + refreshed, err := resolver.Resolve(context.Background(), request) + if err != nil { + t.Fatal(err) + } + if calls != 2 || first.credential != stillCached.credential || refreshed.credential == first.credential { + t.Fatalf("calls=%d credentials=%q/%q/%q", calls, first.credential, stillCached.credential, refreshed.credential) + } +} + func TestCredentialResolverAppRequiresPersonalIdentityForMyGitHub(t *testing.T) { installationID := int64(42) connections := &fakeConnectionReader{workspaces: map[string]*WorkspaceConnection{ diff --git a/apps/backend/internal/github/copy.go b/apps/backend/internal/github/copy.go index be180a2440..f272bba9b3 100644 --- a/apps/backend/internal/github/copy.go +++ b/apps/backend/internal/github/copy.go @@ -40,12 +40,13 @@ func (s *Service) CopyWorkspaceSettingsToWorkspace(ctx context.Context, sourceWo return nil, fmt.Errorf("read source github settings: %w", err) } target := &WorkspaceSettings{ - WorkspaceID: targetWorkspaceID, - RepoScopeMode: source.RepoScopeMode, - RepoScopeOrgs: append([]string(nil), source.RepoScopeOrgs...), - RepoScopeRepos: append([]RepoFilter(nil), source.RepoScopeRepos...), - SavedPresets: cloneRawMessage(source.SavedPresets), - DefaultQueryPresets: cloneRawMessage(source.DefaultQueryPresets), + WorkspaceID: targetWorkspaceID, + TaskGitCredentialsMode: source.TaskGitCredentialsMode, + RepoScopeMode: source.RepoScopeMode, + RepoScopeOrgs: append([]string(nil), source.RepoScopeOrgs...), + RepoScopeRepos: append([]RepoFilter(nil), source.RepoScopeRepos...), + SavedPresets: cloneRawMessage(source.SavedPresets), + DefaultQueryPresets: cloneRawMessage(source.DefaultQueryPresets), } // Copy the action presets before the workspace-settings write. These are two // separate store writes without a shared transaction, so ordering the diff --git a/apps/backend/internal/github/copy_test.go b/apps/backend/internal/github/copy_test.go index 72db40f89b..f261cbdf43 100644 --- a/apps/backend/internal/github/copy_test.go +++ b/apps/backend/internal/github/copy_test.go @@ -20,11 +20,12 @@ func TestCopyWorkspaceSettingsToWorkspace_CopiesSettings(t *testing.T) { const src, dst = "ws-src", "ws-dst" if err := svc.UpsertWorkspaceSettings(ctx, &WorkspaceSettings{ - WorkspaceID: src, - RepoScopeMode: RepoScopeModeRepos, - RepoScopeRepos: []RepoFilter{{Owner: "kdlbs", Name: "kandev"}}, - SavedPresets: []byte(`[{"id":"p1","kind":"pr","label":"Mine"}]`), - DefaultQueryPresets: []byte(`{"pr":[],"issue":[]}`), + WorkspaceID: src, + TaskGitCredentialsMode: TaskGitCredentialsModeExecutor, + RepoScopeMode: RepoScopeModeRepos, + RepoScopeRepos: []RepoFilter{{Owner: "kdlbs", Name: "kandev"}}, + SavedPresets: []byte(`[{"id":"p1","kind":"pr","label":"Mine"}]`), + DefaultQueryPresets: []byte(`{"pr":[],"issue":[]}`), }); err != nil { t.Fatalf("seed source: %v", err) } @@ -42,6 +43,9 @@ func TestCopyWorkspaceSettingsToWorkspace_CopiesSettings(t *testing.T) { if got.WorkspaceID != dst || got.RepoScopeMode != RepoScopeModeRepos { t.Errorf("copied settings identity/scope mismatch: %+v", got) } + if got.TaskGitCredentialsMode != TaskGitCredentialsModeExecutor { + t.Errorf("task Git credential mode not copied: %q", got.TaskGitCredentialsMode) + } if len(got.RepoScopeRepos) != 1 || got.RepoScopeRepos[0].Owner != "kdlbs" || got.RepoScopeRepos[0].Name != "kandev" { t.Errorf("repo scope not copied: %+v", got.RepoScopeRepos) diff --git a/apps/backend/internal/github/models.go b/apps/backend/internal/github/models.go index 73b5643baf..d677e469dd 100644 --- a/apps/backend/internal/github/models.go +++ b/apps/backend/internal/github/models.go @@ -890,39 +890,43 @@ type UpdateIssueWatchRequest struct { // --- Workspace GitHub scope/settings --- const ( - RepoScopeModeAll = "all" - RepoScopeModeOrgs = "orgs" - RepoScopeModeRepos = "repos" + RepoScopeModeAll = "all" + RepoScopeModeOrgs = "orgs" + RepoScopeModeRepos = "repos" + TaskGitCredentialsModeManaged = "managed" + TaskGitCredentialsModeExecutor = "executor" ) // WorkspaceSettings stores per-workspace GitHub operational settings. // Authentication is stored separately and is intentionally not copied with // these operational settings. type WorkspaceSettings struct { - WorkspaceID string `json:"workspace_id" db:"workspace_id"` - RepoScopeMode string `json:"repo_scope_mode" db:"repo_scope_mode"` - RepoScopeOrgs []string `json:"repo_scope_orgs" db:"-"` - RepoScopeRepos []RepoFilter `json:"repo_scope_repos" db:"-"` - RepoScopeOrgsJSON string `json:"-" db:"repo_scope_orgs"` - RepoScopeReposJSON string `json:"-" db:"repo_scope_repos"` - SavedPresets json.RawMessage `json:"saved_presets,omitempty" db:"saved_presets"` - DefaultQueryPresets json.RawMessage `json:"default_query_presets,omitempty" db:"default_query_presets"` - CreatedAt time.Time `json:"created_at" db:"created_at"` - UpdatedAt time.Time `json:"updated_at" db:"updated_at"` + WorkspaceID string `json:"workspace_id" db:"workspace_id"` + TaskGitCredentialsMode string `json:"task_git_credentials_mode" db:"task_git_credentials_mode"` + RepoScopeMode string `json:"repo_scope_mode" db:"repo_scope_mode"` + RepoScopeOrgs []string `json:"repo_scope_orgs" db:"-"` + RepoScopeRepos []RepoFilter `json:"repo_scope_repos" db:"-"` + RepoScopeOrgsJSON string `json:"-" db:"repo_scope_orgs"` + RepoScopeReposJSON string `json:"-" db:"repo_scope_repos"` + SavedPresets json.RawMessage `json:"saved_presets,omitempty" db:"saved_presets"` + DefaultQueryPresets json.RawMessage `json:"default_query_presets,omitempty" db:"default_query_presets"` + CreatedAt time.Time `json:"created_at" db:"created_at"` + UpdatedAt time.Time `json:"updated_at" db:"updated_at"` } // UpdateWorkspaceSettingsRequest replaces the workspace GitHub scope and/or // preference blobs. Nil blobs leave values unchanged; explicit JSON null clears // default query presets back to built-ins. type UpdateWorkspaceSettingsRequest struct { - WorkspaceID string `json:"workspace_id"` - RepoScopeMode *string `json:"repo_scope_mode,omitempty"` - RepoScopeOrgs *[]string `json:"repo_scope_orgs,omitempty"` - RepoScopeRepos *[]RepoFilter `json:"repo_scope_repos,omitempty"` - SavedPresets *json.RawMessage `json:"saved_presets,omitempty"` - DefaultQueryPresets *json.RawMessage `json:"default_query_presets,omitempty"` - SavedPresetsSet bool `json:"-"` - DefaultQueriesSet bool `json:"-"` + WorkspaceID string `json:"workspace_id"` + TaskGitCredentialsMode *string `json:"task_git_credentials_mode,omitempty"` + RepoScopeMode *string `json:"repo_scope_mode,omitempty"` + RepoScopeOrgs *[]string `json:"repo_scope_orgs,omitempty"` + RepoScopeRepos *[]RepoFilter `json:"repo_scope_repos,omitempty"` + SavedPresets *json.RawMessage `json:"saved_presets,omitempty"` + DefaultQueryPresets *json.RawMessage `json:"default_query_presets,omitempty"` + SavedPresetsSet bool `json:"-"` + DefaultQueriesSet bool `json:"-"` } func (r *UpdateWorkspaceSettingsRequest) UnmarshalJSON(data []byte) error { diff --git a/apps/backend/internal/github/store.go b/apps/backend/internal/github/store.go index ac195a8938..f0e2edf0d6 100644 --- a/apps/backend/internal/github/store.go +++ b/apps/backend/internal/github/store.go @@ -211,6 +211,7 @@ const createTablesSQL = ` CREATE TABLE IF NOT EXISTS github_workspace_settings ( workspace_id TEXT PRIMARY KEY, + task_git_credentials_mode TEXT NOT NULL DEFAULT 'managed', repo_scope_mode TEXT NOT NULL DEFAULT 'all', repo_scope_orgs TEXT NOT NULL DEFAULT '[]', repo_scope_repos TEXT NOT NULL DEFAULT '[]', @@ -486,6 +487,9 @@ func (s *Store) applyIdempotentSchemaColumns() { } func (s *Store) initSchemaUpgrades() error { + if err := s.addTaskGitCredentialsMode(); err != nil { + return err + } // Watcher self-heal columns: when the dispatch pipeline detects an // orphaned watcher (e.g. its agent profile has been soft-deleted), it // disables the row and stamps a human-readable cause + timestamp here @@ -510,6 +514,20 @@ func (s *Store) initSchemaUpgrades() error { return nil } +func (s *Store) addTaskGitCredentialsMode() error { + columns, err := s.tableColumns("github_workspace_settings") + if err != nil { + return fmt.Errorf("read github_workspace_settings columns: %w", err) + } + if _, ok := columns["task_git_credentials_mode"]; ok { + return nil + } + if _, err := s.db.Exec(`ALTER TABLE github_workspace_settings ADD COLUMN task_git_credentials_mode TEXT NOT NULL DEFAULT 'managed'`); err != nil { + return fmt.Errorf("add github_workspace_settings.task_git_credentials_mode: %w", err) + } + return nil +} + func (s *Store) initSchemaData(legacyUpgrade bool) error { if err := s.backfillGitHubUserConnectionVersions(); err != nil { return err @@ -2504,14 +2522,15 @@ func (s *Store) ResetIssueWatchState(ctx context.Context, watchID string) error func defaultWorkspaceSettings(workspaceID string) *WorkspaceSettings { now := time.Now().UTC() return &WorkspaceSettings{ - WorkspaceID: workspaceID, - RepoScopeMode: RepoScopeModeAll, - RepoScopeOrgs: []string{}, - RepoScopeRepos: []RepoFilter{}, - SavedPresets: json.RawMessage("[]"), - DefaultQueryPresets: nil, - CreatedAt: now, - UpdatedAt: now, + WorkspaceID: workspaceID, + TaskGitCredentialsMode: TaskGitCredentialsModeManaged, + RepoScopeMode: RepoScopeModeAll, + RepoScopeOrgs: []string{}, + RepoScopeRepos: []RepoFilter{}, + SavedPresets: json.RawMessage("[]"), + DefaultQueryPresets: nil, + CreatedAt: now, + UpdatedAt: now, } } @@ -2536,12 +2555,20 @@ func normalizeRepoScopeMode(mode string) string { } } +func normalizeTaskGitCredentialsMode(mode string) string { + if strings.EqualFold(strings.TrimSpace(mode), TaskGitCredentialsModeExecutor) { + return TaskGitCredentialsModeExecutor + } + return TaskGitCredentialsModeManaged +} + func normalizeWorkspaceSettings(settings *WorkspaceSettings) *WorkspaceSettings { if settings == nil { return nil } out := *settings out.WorkspaceID = strings.TrimSpace(out.WorkspaceID) + out.TaskGitCredentialsMode = normalizeTaskGitCredentialsMode(out.TaskGitCredentialsMode) out.RepoScopeMode = normalizeRepoScopeMode(out.RepoScopeMode) if out.RepoScopeMode != RepoScopeModeOrgs { out.RepoScopeOrgs = nil @@ -2597,17 +2624,18 @@ func (s *Store) GetWorkspaceSettings(ctx context.Context, workspaceID string) (* return nil, fmt.Errorf("workspace_id is required") } var row struct { - WorkspaceID string `db:"workspace_id"` - RepoScopeMode string `db:"repo_scope_mode"` - RepoScopeOrgsJSON string `db:"repo_scope_orgs"` - RepoScopeReposJSON string `db:"repo_scope_repos"` - SavedPresets string `db:"saved_presets"` - DefaultQueryPresets sql.NullString `db:"default_query_presets"` - CreatedAt time.Time `db:"created_at"` - UpdatedAt time.Time `db:"updated_at"` + WorkspaceID string `db:"workspace_id"` + TaskGitCredentialsMode string `db:"task_git_credentials_mode"` + RepoScopeMode string `db:"repo_scope_mode"` + RepoScopeOrgsJSON string `db:"repo_scope_orgs"` + RepoScopeReposJSON string `db:"repo_scope_repos"` + SavedPresets string `db:"saved_presets"` + DefaultQueryPresets sql.NullString `db:"default_query_presets"` + CreatedAt time.Time `db:"created_at"` + UpdatedAt time.Time `db:"updated_at"` } err := s.ro.GetContext(ctx, &row, ` - SELECT workspace_id, repo_scope_mode, repo_scope_orgs, repo_scope_repos, + SELECT workspace_id, task_git_credentials_mode, repo_scope_mode, repo_scope_orgs, repo_scope_repos, saved_presets, default_query_presets, created_at, updated_at FROM github_workspace_settings WHERE workspace_id = ?`, workspaceID) @@ -2618,13 +2646,14 @@ func (s *Store) GetWorkspaceSettings(ctx context.Context, workspaceID string) (* return nil, err } settings := &WorkspaceSettings{ - WorkspaceID: row.WorkspaceID, - RepoScopeMode: row.RepoScopeMode, - RepoScopeOrgsJSON: row.RepoScopeOrgsJSON, - RepoScopeReposJSON: row.RepoScopeReposJSON, - SavedPresets: json.RawMessage(row.SavedPresets), - CreatedAt: row.CreatedAt, - UpdatedAt: row.UpdatedAt, + WorkspaceID: row.WorkspaceID, + TaskGitCredentialsMode: row.TaskGitCredentialsMode, + RepoScopeMode: row.RepoScopeMode, + RepoScopeOrgsJSON: row.RepoScopeOrgsJSON, + RepoScopeReposJSON: row.RepoScopeReposJSON, + SavedPresets: json.RawMessage(row.SavedPresets), + CreatedAt: row.CreatedAt, + UpdatedAt: row.UpdatedAt, } if row.DefaultQueryPresets.Valid { settings.DefaultQueryPresets = json.RawMessage(row.DefaultQueryPresets.String) @@ -2660,17 +2689,18 @@ func (s *Store) UpsertWorkspaceSettings(ctx context.Context, settings *Workspace } _, err = s.db.ExecContext(ctx, ` INSERT INTO github_workspace_settings ( - workspace_id, repo_scope_mode, repo_scope_orgs, repo_scope_repos, + workspace_id, task_git_credentials_mode, repo_scope_mode, repo_scope_orgs, repo_scope_repos, saved_presets, default_query_presets, created_at, updated_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?) + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(workspace_id) DO UPDATE SET + task_git_credentials_mode = excluded.task_git_credentials_mode, repo_scope_mode = excluded.repo_scope_mode, repo_scope_orgs = excluded.repo_scope_orgs, repo_scope_repos = excluded.repo_scope_repos, saved_presets = excluded.saved_presets, default_query_presets = excluded.default_query_presets, updated_at = excluded.updated_at`, - settings.WorkspaceID, settings.RepoScopeMode, string(orgsJSON), string(reposJSON), + settings.WorkspaceID, settings.TaskGitCredentialsMode, settings.RepoScopeMode, string(orgsJSON), string(reposJSON), string(settings.SavedPresets), defaults, now, now) return err } @@ -2685,10 +2715,10 @@ func (s *Store) PatchWorkspaceSettings(ctx context.Context, req *UpdateWorkspace now := time.Now().UTC() if _, err := s.db.ExecContext(ctx, ` INSERT OR IGNORE INTO github_workspace_settings ( - workspace_id, repo_scope_mode, repo_scope_orgs, repo_scope_repos, + workspace_id, task_git_credentials_mode, repo_scope_mode, repo_scope_orgs, repo_scope_repos, saved_presets, default_query_presets, created_at, updated_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?)`, - workspaceID, RepoScopeModeAll, "[]", "[]", "[]", nil, now, now); err != nil { + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`, + workspaceID, TaskGitCredentialsModeManaged, RepoScopeModeAll, "[]", "[]", "[]", nil, now, now); err != nil { return nil, err } @@ -2699,6 +2729,9 @@ func (s *Store) PatchWorkspaceSettings(ctx context.Context, req *UpdateWorkspace if err := appendWorkspaceScopePatch(&patch, req); err != nil { return nil, err } + if req.TaskGitCredentialsMode != nil { + patch.add("task_git_credentials_mode = ?", normalizeTaskGitCredentialsMode(*req.TaskGitCredentialsMode)) + } if err := appendWorkspacePresetPatch(&patch, req); err != nil { return nil, err } diff --git a/apps/backend/internal/github/store_multi_repo_test.go b/apps/backend/internal/github/store_multi_repo_test.go index e274eb2d43..a211f8a659 100644 --- a/apps/backend/internal/github/store_multi_repo_test.go +++ b/apps/backend/internal/github/store_multi_repo_test.go @@ -24,6 +24,9 @@ func newTestStore(t *testing.T) *Store { if _, err := sqlxDB.Exec(`CREATE TABLE tasks (id TEXT PRIMARY KEY, workspace_id TEXT, archived_at DATETIME)`); err != nil { t.Fatalf("create tasks table: %v", err) } + if _, err := sqlxDB.Exec(`CREATE TABLE workspaces (id TEXT PRIMARY KEY)`); err != nil { + t.Fatalf("create workspaces table: %v", err) + } store, err := NewStore(sqlxDB, sqlxDB) if err != nil { t.Fatalf("new store: %v", err) diff --git a/apps/backend/internal/github/workspace_settings_service.go b/apps/backend/internal/github/workspace_settings_service.go index 61b2423a1b..829433f834 100644 --- a/apps/backend/internal/github/workspace_settings_service.go +++ b/apps/backend/internal/github/workspace_settings_service.go @@ -14,6 +14,40 @@ const ( githubSearchResultLimit = 1000 ) +// TaskGitCredentialPolicy describes non-secret credential routing information +// for one workspace. It is safe to persist in task-session metadata and render +// in the Changes panel. +type TaskGitCredentialPolicy struct { + Mode string `json:"mode"` + WorkspaceMethod string `json:"workspace_method,omitempty"` + WorkspaceActor string `json:"workspace_actor,omitempty"` +} + +// DescribeTaskGitCredentialPolicy returns routing and known automation identity +// without resolving a token, lease, or credential helper. +func (s *Service) DescribeTaskGitCredentialPolicy(ctx context.Context, workspaceID string) (TaskGitCredentialPolicy, error) { + settings, err := s.GetWorkspaceSettings(ctx, workspaceID) + if err != nil { + return TaskGitCredentialPolicy{}, err + } + policy := TaskGitCredentialPolicy{Mode: settings.TaskGitCredentialsMode} + if s.store == nil { + return policy, nil + } + connection, err := s.store.GetWorkspaceConnection(ctx, workspaceID) + if err != nil || connection == nil { + return policy, nil + } + policy.WorkspaceMethod = string(connection.Source) + switch connection.Source { + case ConnectionSourcePAT, ConnectionSourceGHCLI: + policy.WorkspaceActor = connection.Login + case ConnectionSourceGitHubAppInstallation: + policy.WorkspaceActor = connection.InstallationAccountLogin + } + return policy, nil +} + // GetWorkspaceSettings returns the GitHub operational settings for a workspace. func (s *Service) GetWorkspaceSettings(ctx context.Context, workspaceID string) (*WorkspaceSettings, error) { if err := s.authorizeWorkspaceAccess(ctx, workspaceID); err != nil { @@ -54,12 +88,24 @@ func (s *Service) UpdateWorkspaceSettings(ctx context.Context, req *UpdateWorksp if req.RepoScopeMode != nil && !isValidRepoScopeMode(*req.RepoScopeMode) { return nil, fmt.Errorf("%w: invalid repo_scope_mode %q", ErrWorkspaceSettingsValidation, *req.RepoScopeMode) } + if req.TaskGitCredentialsMode != nil && !isValidTaskGitCredentialsMode(*req.TaskGitCredentialsMode) { + return nil, fmt.Errorf("%w: invalid task_git_credentials_mode %q", ErrWorkspaceSettingsValidation, *req.TaskGitCredentialsMode) + } if s.store == nil { return nil, fmt.Errorf("github store not configured") } return s.store.PatchWorkspaceSettings(ctx, req) } +func isValidTaskGitCredentialsMode(mode string) bool { + switch strings.TrimSpace(strings.ToLower(mode)) { + case TaskGitCredentialsModeManaged, TaskGitCredentialsModeExecutor: + return true + default: + return false + } +} + func (s *Service) SearchUserPRsPagedForWorkspace( ctx context.Context, workspaceID string, diff --git a/apps/backend/internal/github/workspace_settings_test.go b/apps/backend/internal/github/workspace_settings_test.go index cedcd5aec5..c12f134058 100644 --- a/apps/backend/internal/github/workspace_settings_test.go +++ b/apps/backend/internal/github/workspace_settings_test.go @@ -8,6 +8,7 @@ import ( "sort" "strings" "testing" + "time" ) func TestStore_GitHubWorkspaceSettingsRoundTrip(t *testing.T) { @@ -47,6 +48,90 @@ func TestStore_GitHubWorkspaceSettingsRoundTrip(t *testing.T) { } } +func TestTaskGitCredentialModeDefaultsManaged(t *testing.T) { + store := newTestStore(t) + settings, err := store.GetWorkspaceSettings(context.Background(), "ws-credential-mode") + if err != nil { + t.Fatalf("GetWorkspaceSettings() error = %v", err) + } + raw, err := json.Marshal(settings) + if err != nil { + t.Fatalf("marshal workspace settings: %v", err) + } + if !strings.Contains(string(raw), `"task_git_credentials_mode":"managed"`) { + t.Fatalf("settings JSON = %s, want managed task Git credential mode", raw) + } +} + +func TestDescribeTaskGitCredentialPolicyUsesConnectionIdentity(t *testing.T) { + store := newTestStore(t) + ctx := context.Background() + installationID := int64(42) + for _, tc := range []struct { + name string + conn WorkspaceConnection + method string + actor string + }{ + { + name: "PAT", conn: WorkspaceConnection{WorkspaceID: "ws-pat", Source: ConnectionSourcePAT, GitHubHost: defaultGitHubHost, Login: "alice", Status: ConnectionStatusActive}, + method: "pat", actor: "alice", + }, + { + name: "GitHub App", conn: WorkspaceConnection{WorkspaceID: "ws-app", Source: ConnectionSourceGitHubAppInstallation, GitHubHost: defaultGitHubHost, InstallationID: &installationID, InstallationAccountLogin: "acme-bot", InstallationAccountType: "Organization", AppRegistrationID: "app-1", Status: ConnectionStatusActive}, + method: "github_app_installation", actor: "acme-bot", + }, + } { + t.Run(tc.name, func(t *testing.T) { + if _, err := store.db.ExecContext(ctx, `INSERT INTO workspaces (id) VALUES (?)`, tc.conn.WorkspaceID); err != nil { + t.Fatalf("create workspace: %v", err) + } + if tc.conn.AppRegistrationID != "" { + registration := newAppRegistration(tc.conn.AppRegistrationID, 99, "Acme automation", time.Now().UTC()) + if err := store.InsertAppRegistration(ctx, registration); err != nil { + t.Fatalf("create app registration: %v", err) + } + } + if err := store.UpsertWorkspaceConnection(ctx, &tc.conn); err != nil { + t.Fatalf("UpsertWorkspaceConnection() error = %v", err) + } + svc := newWorkspaceAuthenticatedTestService(t, NewMockClient(), store, tc.conn.WorkspaceID) + policy, err := svc.DescribeTaskGitCredentialPolicy(ctx, tc.conn.WorkspaceID) + if err != nil { + t.Fatalf("DescribeTaskGitCredentialPolicy() error = %v", err) + } + if policy.Mode != TaskGitCredentialsModeManaged || policy.WorkspaceMethod != tc.method || policy.WorkspaceActor != tc.actor { + t.Fatalf("policy = %+v", policy) + } + }) + } +} + +func TestService_UpdateWorkspaceSettings_TaskGitCredentialMode(t *testing.T) { + store := newTestStore(t) + svc := newWorkspaceAuthenticatedTestService(t, NewMockClient(), store, "ws-credential-mode") + executor := TaskGitCredentialsModeExecutor + updated, err := svc.UpdateWorkspaceSettings(context.Background(), &UpdateWorkspaceSettingsRequest{ + WorkspaceID: "ws-credential-mode", + TaskGitCredentialsMode: &executor, + }) + if err != nil { + t.Fatalf("UpdateWorkspaceSettings() error = %v", err) + } + if got := updated.TaskGitCredentialsMode; got != TaskGitCredentialsModeExecutor { + t.Fatalf("TaskGitCredentialsMode = %q, want executor", got) + } + + invalid := "host" + _, err = svc.UpdateWorkspaceSettings(context.Background(), &UpdateWorkspaceSettingsRequest{ + WorkspaceID: "ws-credential-mode", + TaskGitCredentialsMode: &invalid, + }) + if !errors.Is(err, ErrWorkspaceSettingsValidation) { + t.Fatalf("UpdateWorkspaceSettings() error = %v, want validation error", err) + } +} + func TestService_SearchUserPRsPagedForWorkspace_FiltersToSelectedRepos(t *testing.T) { client := NewMockClient() client.AddPR(&PR{RepoOwner: "kdlbs", RepoName: "kandev", Number: 1, Title: "in scope"}) diff --git a/apps/backend/internal/gitlab/poller_health_test.go b/apps/backend/internal/gitlab/poller_health_test.go index 4d68f015e3..fd8dd6a83a 100644 --- a/apps/backend/internal/gitlab/poller_health_test.go +++ b/apps/backend/internal/gitlab/poller_health_test.go @@ -25,11 +25,15 @@ func TestPollerStartRecordsHealthForEveryConfiguredWorkspace(t *testing.T) { t.Cleanup(func() { cancel(); poller.Stop() }) deadline := time.Now().Add(time.Second) for { - cfg, err := store.GetConfigForWorkspace(context.Background(), "workspace-a") - if err != nil { - t.Fatalf("get config workspace-a: %v", err) + allProbed := true + for _, workspaceID := range []string{"workspace-a", "workspace-b"} { + cfg, err := store.GetConfigForWorkspace(context.Background(), workspaceID) + if err != nil { + t.Fatalf("get config %s: %v", workspaceID, err) + } + allProbed = allProbed && cfg.LastCheckedAt != nil } - if cfg.LastCheckedAt != nil { + if allProbed { break } if time.Now().After(deadline) { diff --git a/apps/backend/internal/orchestrator/executor/executor.go b/apps/backend/internal/orchestrator/executor/executor.go index 0815931156..e6d5038065 100644 --- a/apps/backend/internal/orchestrator/executor/executor.go +++ b/apps/backend/internal/orchestrator/executor/executor.go @@ -646,8 +646,9 @@ type Executor struct { gitlabCredentials GitLabCredentialResolver logger *logger.Logger - githubCredentialIssuer GitHubCredentialLeaseIssuer - githubCredentialBrokerURL string + githubCredentialIssuer GitHubCredentialLeaseIssuer + githubCredentialBrokerURL string + githubCredentialPolicyResolver TaskGitCredentialPolicyResolver // Configuration retryLimit int diff --git a/apps/backend/internal/orchestrator/executor/executor_credentials.go b/apps/backend/internal/orchestrator/executor/executor_credentials.go index c7d5ab07ec..860beb87a5 100644 --- a/apps/backend/internal/orchestrator/executor/executor_credentials.go +++ b/apps/backend/internal/orchestrator/executor/executor_credentials.go @@ -9,10 +9,13 @@ import ( "net/url" "strconv" "strings" + "time" "go.uber.org/zap" + "github.com/kandev/kandev/internal/gitconfigenv" "github.com/kandev/kandev/internal/githubauth" + "github.com/kandev/kandev/internal/task/models" ) const ( @@ -24,9 +27,10 @@ const ( envGitLabHost = "GITLAB_HOST" envKandevGitLabHost = "KANDEV_GITLAB_HOST" - gitHubCredentialHelper = "!agentctl git-credential" - defaultGitHubHost = "github.com" - gitLabCredentialHelper = `!f() { echo "username=oauth2"; echo "password=$GITLAB_TOKEN"; }; f` + gitHubCredentialHelper = "!agentctl git-credential" + defaultGitHubHost = "github.com" + gitLabCredentialHelper = `!f() { echo "username=oauth2"; echo "password=$GITLAB_TOKEN"; }; f` + taskGitCredentialsModeExecutor = "executor" ) var ErrGitHubCredentialBrokerURL = errors.New("invalid GitHub credential broker URL") @@ -59,6 +63,18 @@ type GitHubCredentialLeaseIssuer interface { IssueGitHubCredentialLease(context.Context, GitHubCredentialLeaseRequest) (GitHubCredentialLease, error) } +// TaskGitCredentialPolicy is the non-secret routing contract resolved for a workspace. +type TaskGitCredentialPolicy struct { + Mode string + WorkspaceMethod string + WorkspaceActor string +} + +// TaskGitCredentialPolicyResolver resolves the workspace policy without exposing credentials. +type TaskGitCredentialPolicyResolver interface { + ResolveTaskGitCredentialPolicy(context.Context, string) (TaskGitCredentialPolicy, error) +} + // SetGitHubCredentialBroker configures renewable workspace automation credentials. // brokerURL is the full credential-resolution endpoint URL. func (e *Executor) SetGitHubCredentialBroker(issuer GitHubCredentialLeaseIssuer, brokerURL string) { @@ -66,6 +82,59 @@ func (e *Executor) SetGitHubCredentialBroker(issuer GitHubCredentialLeaseIssuer, e.githubCredentialBrokerURL = strings.TrimSpace(brokerURL) } +// SetTaskGitCredentialPolicyResolver configures workspace-specific task Git routing. +func (e *Executor) SetTaskGitCredentialPolicyResolver(resolver TaskGitCredentialPolicyResolver) { + e.githubCredentialPolicyResolver = resolver +} + +func (e *Executor) applyGitCredentialSnapshot( + ctx context.Context, + req *LaunchAgentRequest, + session *models.TaskSession, +) error { + if req == nil || session == nil { + return nil + } + policy := TaskGitCredentialPolicy{Mode: "managed"} + if e.githubCredentialPolicyResolver != nil { + resolved, err := e.githubCredentialPolicyResolver.ResolveTaskGitCredentialPolicy(ctx, req.WorkspaceID) + if err != nil { + return fmt.Errorf("resolve task Git credential policy: %w", err) + } + policy = resolved + } + snapshot := models.GitCredentialSnapshot{ + Version: 1, + Policy: policy.Mode, + ExecutorType: req.ExecutorType, + CapturedAt: time.Now().UTC(), + } + switch { + case req.Env[envGitHubToken] != "" || req.Env[envGHToken] != "": + snapshot.Source = "executor_profile" + snapshot.Transport = "profile_token" + case policy.Mode == taskGitCredentialsModeExecutor: + snapshot.Source = "executor" + snapshot.Transport = "executor_selected" + default: + if req.Env[githubauth.CredentialBrokerURLEnv] == "" { + return nil + } + snapshot.Source = "workspace" + snapshot.WorkspaceMethod = policy.WorkspaceMethod + snapshot.Actor = policy.WorkspaceActor + if snapshot.Actor == "" { + snapshot.Actor = "runtime_selected" + } + snapshot.Transport = "managed_https" + } + if session.Metadata == nil { + session.Metadata = make(map[string]interface{}) + } + session.Metadata[models.SessionMetaKeyGitCredentialSnapshot] = snapshot + return nil +} + func (e *Executor) configureGitHubCredentialBroker( ctx context.Context, req *LaunchAgentRequest, @@ -82,6 +151,15 @@ func (e *Executor) configureGitHubCredentialBrokerForRepositories( if e.githubCredentialIssuer == nil || len(infos) == 0 { return nil } + if e.githubCredentialPolicyResolver != nil { + policy, err := e.githubCredentialPolicyResolver.ResolveTaskGitCredentialPolicy(ctx, req.WorkspaceID) + if err != nil { + return fmt.Errorf("resolve task Git credential policy: %w", err) + } + if policy.Mode == taskGitCredentialsModeExecutor { + return removeManagedGitHubCredentials(req) + } + } if req.Env == nil { req.Env = make(map[string]string) } @@ -116,11 +194,47 @@ func (e *Executor) configureGitHubCredentialBrokerForRepositories( req.Env[githubauth.CredentialHostEnv] = primary.Host req.Env[githubauth.CredentialScopesEnv] = string(encodedScopes) req.Env["GIT_TERMINAL_PROMPT"] = "0" + // 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", "") appendGitConfig(req.Env, "credential.https://github.com.helper", gitHubCredentialHelper) appendGitConfig(req.Env, "credential.useHttpPath", "true") return nil } +func removeManagedGitHubCredentials(req *LaunchAgentRequest) error { + if req == nil || req.Env == nil { + return nil + } + for _, key := range []string{ + githubauth.CredentialBrokerURLEnv, + githubauth.CredentialLeaseEnv, + githubauth.CredentialTaskIDEnv, + githubauth.CredentialSessionIDEnv, + githubauth.CredentialRepositoryEnv, + githubauth.CredentialOwnerEnv, + githubauth.CredentialRepoEnv, + githubauth.CredentialHostEnv, + githubauth.CredentialScopesEnv, + } { + delete(req.Env, key) + } + 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 { + 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 + }) + if err != nil { + return fmt.Errorf("remove managed GitHub credential helper: %w", err) + } + req.Env = entries + return nil +} + 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 820b9ff4f1..697ba5e692 100644 --- a/apps/backend/internal/orchestrator/executor/executor_credentials_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_credentials_test.go @@ -36,6 +36,18 @@ type fakeGitHubCredentialLeaseIssuer struct { calls int } +type fakeTaskGitCredentialPolicyResolver struct { + policy TaskGitCredentialPolicy + err error +} + +func (r fakeTaskGitCredentialPolicyResolver) ResolveTaskGitCredentialPolicy( + context.Context, + string, +) (TaskGitCredentialPolicy, error) { + return r.policy, r.err +} + func (f *fakeGitHubCredentialLeaseIssuer) IssueGitHubCredentialLease( _ context.Context, req GitHubCredentialLeaseRequest, @@ -89,11 +101,13 @@ func TestConfigureGitHubCredentialBroker(t *testing.T) { envGitHubCredentialOwner: "acme", envGitHubCredentialRepo: "widgets", envGitHubCredentialHost: "github.com", - "GIT_CONFIG_COUNT": "3", + "GIT_CONFIG_COUNT": "4", "GIT_CONFIG_KEY_1": "credential.https://github.com.helper", - "GIT_CONFIG_VALUE_1": "!agentctl git-credential", - "GIT_CONFIG_KEY_2": "credential.useHttpPath", - "GIT_CONFIG_VALUE_2": "true", + "GIT_CONFIG_VALUE_1": "", + "GIT_CONFIG_KEY_2": "credential.https://github.com.helper", + "GIT_CONFIG_VALUE_2": "!agentctl git-credential", + "GIT_CONFIG_KEY_3": "credential.useHttpPath", + "GIT_CONFIG_VALUE_3": "true", "GIT_TERMINAL_PROMPT": "0", } if got := req.Env[envGitHubCredentialScopes]; !strings.Contains(got, `"repository_id":"repo-1"`) { @@ -112,6 +126,75 @@ func TestConfigureGitHubCredentialBroker(t *testing.T) { } } +func TestApplyGitCredentialSnapshotOmitsManagedSourceWithoutBroker(t *testing.T) { + exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) + exec.SetTaskGitCredentialPolicyResolver(fakeTaskGitCredentialPolicyResolver{ + policy: TaskGitCredentialPolicy{Mode: "managed", WorkspaceMethod: "gh_cli"}, + }) + session := &models.TaskSession{ID: "session-1"} + req := &LaunchAgentRequest{WorkspaceID: "workspace-1", Env: map[string]string{}} + + if err := exec.applyGitCredentialSnapshot(context.Background(), req, session); err != nil { + t.Fatalf("applyGitCredentialSnapshot() error = %v", err) + } + if _, ok := session.Metadata[models.SessionMetaKeyGitCredentialSnapshot]; ok { + t.Fatalf("snapshot = %#v, want no managed snapshot without broker", session.Metadata) + } +} + +func TestApplyGitCredentialSnapshotUsesExecutorSources(t *testing.T) { + for _, tc := range []struct { + name string + policy TaskGitCredentialPolicy + env map[string]string + wantSource string + wantActor string + wantTransit string + }{ + {name: "profile token", env: map[string]string{envGHToken: "token"}, wantSource: "executor_profile", wantTransit: "profile_token"}, + {name: "executor inheritance", policy: TaskGitCredentialPolicy{Mode: taskGitCredentialsModeExecutor}, env: map[string]string{}, wantSource: "executor", wantTransit: "executor_selected"}, + } { + t.Run(tc.name, func(t *testing.T) { + exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) + exec.SetTaskGitCredentialPolicyResolver(fakeTaskGitCredentialPolicyResolver{policy: tc.policy}) + session := &models.TaskSession{ID: "session-1"} + if err := exec.applyGitCredentialSnapshot(context.Background(), &LaunchAgentRequest{WorkspaceID: "workspace-1", Env: tc.env}, session); err != nil { + t.Fatalf("applyGitCredentialSnapshot() error = %v", err) + } + snapshot, ok := session.Metadata[models.SessionMetaKeyGitCredentialSnapshot].(models.GitCredentialSnapshot) + if !ok || snapshot.Source != tc.wantSource || snapshot.Actor != tc.wantActor || snapshot.Transport != tc.wantTransit { + t.Fatalf("snapshot = %#v", session.Metadata[models.SessionMetaKeyGitCredentialSnapshot]) + } + }) + } +} + +func TestApplyGitCredentialSnapshotUsesManagedWorkspaceIdentity(t *testing.T) { + exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) + exec.SetTaskGitCredentialPolicyResolver(fakeTaskGitCredentialPolicyResolver{ + policy: TaskGitCredentialPolicy{Mode: "managed", WorkspaceMethod: "github_app_installation", WorkspaceActor: "acme-bot"}, + }) + session := &models.TaskSession{ID: "session-1"} + req := &LaunchAgentRequest{WorkspaceID: "workspace-1", Env: map[string]string{githubauth.CredentialBrokerURLEnv: "http://broker"}} + + if err := exec.applyGitCredentialSnapshot(context.Background(), req, session); err != nil { + t.Fatalf("applyGitCredentialSnapshot() error = %v", err) + } + snapshot := session.Metadata[models.SessionMetaKeyGitCredentialSnapshot].(models.GitCredentialSnapshot) + if snapshot.Source != "workspace" || snapshot.Transport != "managed_https" || snapshot.WorkspaceMethod != "github_app_installation" || snapshot.Actor != "acme-bot" { + t.Fatalf("snapshot = %#v", snapshot) + } +} + +func TestApplyGitCredentialSnapshotReturnsResolverError(t *testing.T) { + exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) + exec.SetTaskGitCredentialPolicyResolver(fakeTaskGitCredentialPolicyResolver{err: errors.New("resolver unavailable")}) + err := exec.applyGitCredentialSnapshot(context.Background(), &LaunchAgentRequest{WorkspaceID: "workspace-1"}, &models.TaskSession{}) + if err == nil || !strings.Contains(err.Error(), "resolver unavailable") { + t.Fatalf("applyGitCredentialSnapshot() error = %v", err) + } +} + func TestConfigureGitHubCredentialBrokerIssuesOneLeasePerRepository(t *testing.T) { issuer := &fakeGitHubCredentialLeaseIssuer{} exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) @@ -178,6 +261,57 @@ func TestConfigureGitHubCredentialBrokerPreservesExplicitProfileToken(t *testing } } +func TestConfigureGitHubCredentialBrokerSkipsExecutorInheritedPolicy(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.SetTaskGitCredentialPolicyResolver(fakeTaskGitCredentialPolicyResolver{ + policy: TaskGitCredentialPolicy{Mode: "executor"}, + }) + req := &LaunchAgentRequest{WorkspaceID: "workspace-1", Env: map[string]string{ + githubauth.CredentialBrokerURLEnv: "http://broker.example/resolve", + githubauth.CredentialLeaseEnv: "lease", + githubauth.CredentialTaskIDEnv: "task-1", + githubauth.CredentialSessionIDEnv: "session-1", + githubauth.CredentialRepositoryEnv: "repo-1", + githubauth.CredentialOwnerEnv: "acme", + githubauth.CredentialRepoEnv: "widgets", + githubauth.CredentialHostEnv: "github.com", + githubauth.CredentialScopesEnv: `[{"repo":"widgets"}]`, + "GIT_CONFIG_COUNT": "3", + "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", + }} + 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 issuer.calls != 0 { + t.Fatalf("IssueGitHubCredentialLease calls = %d, want 0", issuer.calls) + } + 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} { + if _, ok := req.Env[key]; ok { + t.Fatalf("Env[%q] still contains a managed credential value", key) + } + } + if got, want := req.Env["GIT_CONFIG_COUNT"], "1"; got != want { + t.Fatalf("GIT_CONFIG_COUNT = %q, want %q", got, want) + } + if got, want := req.Env["GIT_CONFIG_KEY_0"], "core.hooksPath"; got != want { + t.Fatalf("GIT_CONFIG_KEY_0 = %q, want %q", got, want) + } +} + func TestConfigureGitHubCredentialBrokerRejectsRemoteLoopbackURL(t *testing.T) { issuer := &fakeGitHubCredentialLeaseIssuer{lease: GitHubCredentialLease{Token: "opaque-lease"}} exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) diff --git a/apps/backend/internal/orchestrator/executor/executor_execute.go b/apps/backend/internal/orchestrator/executor/executor_execute.go index adcc2cb286..7b58e7a5b6 100644 --- a/apps/backend/internal/orchestrator/executor/executor_execute.go +++ b/apps/backend/internal/orchestrator/executor/executor_execute.go @@ -1231,6 +1231,9 @@ func (e *Executor) buildLaunchAgentRequest(ctx context.Context, task *v1.Task, s if err := e.configureGitHubCredentialBrokerForRepositories(ctx, req, allRepos); err != nil { return nil, execConfig, err } + if err := e.applyGitCredentialSnapshot(ctx, req, session); err != nil { + return nil, execConfig, err + } // Multi-repo: when more than one repository is associated with the task, // populate req.Repositories so the lifecycle preparer creates one worktree diff --git a/apps/backend/internal/orchestrator/executor/executor_resume.go b/apps/backend/internal/orchestrator/executor/executor_resume.go index de0fda7130..6d472f2748 100644 --- a/apps/backend/internal/orchestrator/executor/executor_resume.go +++ b/apps/backend/internal/orchestrator/executor/executor_resume.go @@ -632,6 +632,16 @@ func (e *Executor) buildResumeRequest(ctx context.Context, task *v1.Task, sessio if err != nil { return nil, "", execConfig, nil, nil, err } + allRepos, err := e.resolveAllRepoInfo(ctx, task.ID) + if err != nil { + return nil, "", execConfig, nil, nil, err + } + if err := e.configureGitHubCredentialBrokerForRepositories(ctx, req, allRepos); err != nil { + return nil, "", execConfig, nil, nil, err + } + if err := e.applyGitCredentialSnapshot(ctx, req, session); err != nil { + return nil, "", execConfig, nil, nil, err + } e.reuseExistingEnvironment(ctx, req, existingEnv) diff --git a/apps/backend/internal/orchestrator/service.go b/apps/backend/internal/orchestrator/service.go index 5843736b44..301c3f77d0 100644 --- a/apps/backend/internal/orchestrator/service.go +++ b/apps/backend/internal/orchestrator/service.go @@ -735,6 +735,11 @@ func (s *Service) SetGitHubCredentialBroker(issuer executor.GitHubCredentialLeas s.executor.SetGitHubCredentialBroker(issuer, endpoint) } +// SetTaskGitCredentialPolicyResolver configures non-secret workspace policy lookup. +func (s *Service) SetTaskGitCredentialPolicyResolver(resolver executor.TaskGitCredentialPolicyResolver) { + s.executor.SetTaskGitCredentialPolicyResolver(resolver) +} + // SetTurnService sets the turn service for tracking conversation turns. // // A "turn" represents a single conversation round-trip: user prompt → agent response. diff --git a/apps/backend/internal/task/models/models.go b/apps/backend/internal/task/models/models.go index fb0d213a65..61fb8f05bc 100644 --- a/apps/backend/internal/task/models/models.go +++ b/apps/backend/internal/task/models/models.go @@ -132,6 +132,23 @@ const SessionMetaKeyACPConfigBaseline = "acp_config_baseline" // reconnection. It is display metadata and is not replayed to the provider. const SessionMetaKeyACPModelState = "acp_model_state" +// SessionMetaKeyGitCredentialSnapshot records the non-secret Git credential +// routing contract that successfully launched or resumed a session. +const SessionMetaKeyGitCredentialSnapshot = "git_credential_snapshot" + +// GitCredentialSnapshot is launch-time display metadata. It never contains a +// token, broker lease, helper command, credential file, or SSH key detail. +type GitCredentialSnapshot struct { + Version int `json:"version"` + Policy string `json:"policy"` + Source string `json:"source"` + WorkspaceMethod string `json:"workspace_method,omitempty"` + Actor string `json:"actor"` + Transport string `json:"transport"` + ExecutorType string `json:"executor_type,omitempty"` + CapturedAt time.Time `json:"captured_at"` +} + // TurnMetaKeyRuntimeConfigSnapshot stores the immutable effective runtime // configuration attributed to one prompt/response turn. const TurnMetaKeyRuntimeConfigSnapshot = "runtime_config_snapshot" diff --git a/apps/web/components/github/github-auth-method-list.tsx b/apps/web/components/github/github-auth-method-list.tsx index 49ed641475..fd702a1635 100644 --- a/apps/web/components/github/github-auth-method-list.tsx +++ b/apps/web/components/github/github-auth-method-list.tsx @@ -9,19 +9,22 @@ const methods = [ { value: "pat" as const, label: "Personal access token", - description: "Simple and portable, but automation is attributed to your personal account.", + description: + "Stored encrypted by Kandev. Workspace automation and managed task GitHub HTTPS/gh act as your account.", icon: IconKey, }, { value: "cli" as const, label: "GitHub CLI account", - description: "Reuse one named account already authenticated on this Kandev host.", + description: + "Resolved from one named host account when needed. Managed tasks receive a scoped brokered identity; host Git is not inherited unless selected below.", icon: IconTerminal2, }, { value: "app" as const, label: "GitHub App", - description: "Isolated, short-lived credentials with explicit repository access.", + description: + "Stored App credentials mint short-lived installation tokens. Managed tasks receive a scoped brokered App identity for allowed repositories.", icon: IconBrandGithub, }, ]; diff --git a/apps/web/components/github/github-settings.tsx b/apps/web/components/github/github-settings.tsx index 2132ba68da..562e9e8bbe 100644 --- a/apps/web/components/github/github-settings.tsx +++ b/apps/web/components/github/github-settings.tsx @@ -17,6 +17,7 @@ import { IssueWatchDialog } from "./issue-watch-dialog"; import { ActionPresetsSection } from "./action-presets-section"; import { DefaultQueriesSection } from "./default-queries-section"; import { GitHubRepoScopeSection } from "./github-repo-scope-section"; +import { GitHubTaskCredentialsSection } from "./github-task-credentials-section"; import { PRStatsPanel } from "./pr-stats"; import { useReviewWatches } from "@/hooks/domains/github/use-review-watches"; import { useIssueWatches } from "@/hooks/domains/github/use-issue-watches"; @@ -274,6 +275,7 @@ function PerWorkspaceSection({ workspaceId }: { workspaceId: string }) { + diff --git a/apps/web/components/github/github-task-credentials-section.tsx b/apps/web/components/github/github-task-credentials-section.tsx new file mode 100644 index 0000000000..0c0d5d8980 --- /dev/null +++ b/apps/web/components/github/github-task-credentials-section.tsx @@ -0,0 +1,162 @@ +"use client"; + +import { useCallback, useEffect, useState } from "react"; +import { IconInfoCircle } from "@tabler/icons-react"; +import { Button } from "@kandev/ui/button"; +import { CardContent } from "@kandev/ui/card"; +import { + Drawer, + DrawerContent, + DrawerDescription, + DrawerHeader, + DrawerTitle, + DrawerTrigger, +} from "@kandev/ui/drawer"; +import { RadioGroup, RadioGroupItem } from "@kandev/ui/radio-group"; +import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; +import { SettingsCard } from "@/components/settings/settings-card"; +import { SettingsSection } from "@/components/settings/settings-section"; +import { useSettingsSaveContributor } from "@/components/settings/settings-save-provider"; +import { useToast } from "@/components/toast-provider"; +import { useTouchDrawer } from "@/hooks/use-compact-task-chrome"; +import { + fetchGitHubWorkspaceSettings, + updateGitHubWorkspaceSettings, +} from "@/lib/api/domains/github-api"; +import type { TaskGitCredentialsMode } from "@/lib/types/github"; + +const deliveryHelp = + "PAT, named GitHub CLI, and GitHub App connections choose Kandev's workspace automation identity. In managed mode Kandev brokers that identity to the task for GitHub HTTPS and gh. Inherit executor credentials leaves task Git and gh to the selected executor. An explicit executor-profile GH_TOKEN or GITHUB_TOKEN takes precedence in managed mode."; + +function TaskCredentialsHelp() { + const usesDrawer = useTouchDrawer(); + const [open, setOpen] = useState(false); + const button = ( + + ); + const trigger = {button}; + return ( + + {usesDrawer ? ( + trigger + ) : ( + + {trigger} + + {deliveryHelp} + + + )} + + + How task Git credentials work + {deliveryHelp} + + + + ); +} + +export function GitHubTaskCredentialsSection({ workspaceId }: { workspaceId: string }) { + const { toast } = useToast(); + const [mode, setMode] = useState("managed"); + const [baseline, setBaseline] = useState("managed"); + const [loading, setLoading] = useState(true); + useEffect(() => { + let cancelled = false; + setLoading(true); + void fetchGitHubWorkspaceSettings(workspaceId) + .then((settings) => { + if (!cancelled) { + const next = settings.task_git_credentials_mode ?? "managed"; + setMode(next); + setBaseline(next); + } + }) + .catch(() => { + if (!cancelled) + toast({ description: "Failed to load task Git credential settings", variant: "error" }); + }) + .finally(() => { + if (!cancelled) setLoading(false); + }); + return () => { + cancelled = true; + }; + }, [toast, workspaceId]); + const save = useCallback(async () => { + try { + const updated = await updateGitHubWorkspaceSettings({ + workspace_id: workspaceId, + task_git_credentials_mode: mode, + }); + const next = updated.task_git_credentials_mode ?? "managed"; + setBaseline(next); + setMode((current) => (current === mode ? next : current)); + toast({ description: "Task Git credential settings saved", variant: "success" }); + } catch { + toast({ description: "Failed to save task Git credential settings", variant: "error" }); + throw new Error("Failed to save task Git credential settings"); + } + }, [mode, toast, workspaceId]); + const dirty = mode !== baseline; + useSettingsSaveContributor({ + id: `github-task-credentials:${workspaceId}`, + revision: mode, + isDirty: dirty, + canSave: !loading, + save, + discard: () => setMode(baseline), + }); + return ( + } + > + + + setMode(value as TaskGitCredentialsMode)} + disabled={loading} + data-testid="github-task-credentials-mode" + > + + + +

+ An executor-profile GH_TOKEN or GITHUB_TOKEN overrides managed workspace credentials for + that task. +

+
+
+
+ ); +} diff --git a/apps/web/components/task/changes-git-credential-display.test.ts b/apps/web/components/task/changes-git-credential-display.test.ts new file mode 100644 index 0000000000..6a81105515 --- /dev/null +++ b/apps/web/components/task/changes-git-credential-display.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from "vitest"; +import { getGitCredentialDisplay } from "./changes-git-credential-display"; + +describe("getGitCredentialDisplay", () => { + it("renders the managed workspace method without exposing credentials", () => { + expect( + getGitCredentialDisplay({ + git_credential_snapshot: { + source: "workspace", + workspace_method: "gh_cli", + transport: "managed_https", + }, + }), + ).toEqual({ + source: "Managed workspace identity", + detail: "workspace GitHub CLI account", + transport: "Kandev-managed GitHub HTTPS and gh", + }); + }); + + it("renders executor profile precedence", () => { + expect( + getGitCredentialDisplay({ + git_credential_snapshot: { + source: "executor_profile", + actor: "runtime_selected", + transport: "profile_token", + }, + }), + ).toEqual({ + source: "Executor-profile token", + detail: "runtime_selected", + transport: "executor-profile token", + }); + }); + + it("does not display absent or malformed snapshots", () => { + expect(getGitCredentialDisplay(null)).toBeNull(); + expect(getGitCredentialDisplay({ git_credential_snapshot: "secret" })).toBeNull(); + }); +}); diff --git a/apps/web/components/task/changes-git-credential-display.ts b/apps/web/components/task/changes-git-credential-display.ts new file mode 100644 index 0000000000..6f5728ae09 --- /dev/null +++ b/apps/web/components/task/changes-git-credential-display.ts @@ -0,0 +1,66 @@ +export type GitCredentialDisplay = { + source: string; + detail: string; + transport: string; +}; + +type GitCredentialSnapshot = { + source?: unknown; + workspace_method?: unknown; + actor?: unknown; + transport?: unknown; +}; + +const workspaceMethodLabels: Record = { + pat: "workspace personal access token", + gh_cli: "workspace GitHub CLI account", + github_app_installation: "workspace GitHub App", +}; + +const transportLabels: Record = { + managed_https: "Kandev-managed GitHub HTTPS and gh", + profile_token: "executor-profile token", + executor_selected: "executor Git and gh credentials", +}; + +function readSnapshot( + metadata: Record | null | undefined, +): GitCredentialSnapshot | null { + const snapshot = metadata?.git_credential_snapshot; + if (!snapshot || typeof snapshot !== "object") return null; + return snapshot as GitCredentialSnapshot; +} + +/** Returns safe, launch-time credential routing copy for the Changes panel. */ +export function getGitCredentialDisplay( + metadata: Record | null | undefined, +): GitCredentialDisplay | null { + const snapshot = readSnapshot(metadata); + if (!snapshot || typeof snapshot.source !== "string") return null; + const actor = typeof snapshot.actor === "string" ? snapshot.actor : ""; + const workspaceMethod = + typeof snapshot.workspace_method === "string" ? snapshot.workspace_method : ""; + const transport = typeof snapshot.transport === "string" ? snapshot.transport : ""; + switch (snapshot.source) { + case "workspace": + return { + source: "Managed workspace identity", + detail: workspaceMethodLabels[workspaceMethod] ?? "workspace GitHub credentials", + transport: transportLabels[transport] ?? "Kandev-managed task credentials", + }; + case "executor_profile": + return { + source: "Executor-profile token", + detail: actor || "An explicit GH_TOKEN or GITHUB_TOKEN", + transport: transportLabels[transport] ?? "executor-profile token", + }; + case "executor": + return { + source: "Executor credentials", + detail: actor || "Credentials available in the selected executor", + transport: transportLabels[transport] ?? "executor Git and gh credentials", + }; + default: + return null; + } +} diff --git a/apps/web/components/task/changes-panel-data.tsx b/apps/web/components/task/changes-panel-data.tsx index 5e5281cc67..d3cf850d67 100644 --- a/apps/web/components/task/changes-panel-data.tsx +++ b/apps/web/components/task/changes-panel-data.tsx @@ -39,6 +39,7 @@ import { } from "./changes-panel-helpers"; import type { OpenDiffOptions } from "./changes-diff-target"; import type { PRDiffFile, TaskPR } from "@/lib/types/github"; +import { getGitCredentialDisplay } from "./changes-git-credential-display"; function useChangesPanelStoreData() { const activeTaskId = useAppStore((state) => state.tasks.activeTaskId); @@ -62,7 +63,21 @@ function useChangesPanelStoreData() { if (Array.isArray(taskPRs) && taskPRs.length > 1) return undefined; return state.pendingPrUrlByTaskId.byTaskId[taskId]?.[""]; }); - return { activeTaskId, activeSessionId, taskTitle, baseBranch, existingPrUrl }; + const activeSessionMetadata = useAppStore((state) => + activeSessionId ? state.taskSessions.items[activeSessionId]?.metadata : undefined, + ); + const gitCredentialDisplay = useMemo( + () => getGitCredentialDisplay(activeSessionMetadata), + [activeSessionMetadata], + ); + return { + activeTaskId, + activeSessionId, + taskTitle, + baseBranch, + existingPrUrl, + gitCredentialDisplay, + }; } type DialogsType = ReturnType & ReturnType; @@ -298,7 +313,8 @@ function hasCumulativeFiles(files: Record | null | undefined): } export function useChangesPanelData() { - const { activeTaskId, activeSessionId, baseBranch, existingPrUrl } = useChangesPanelStoreData(); + const { activeTaskId, activeSessionId, baseBranch, existingPrUrl, gitCredentialDisplay } = + useChangesPanelStoreData(); const baseBranchByRepo = useBaseBranchByRepo(activeTaskId); const git = useSessionGit(activeSessionId); const { toast } = useToast(); @@ -364,6 +380,7 @@ export function useChangesPanelData() { repoDisplayName, prByRepo, existingPrUrl, + gitCredentialDisplay, walkthroughRequestReady, ...prData, }; diff --git a/apps/web/components/task/changes-panel-header.tsx b/apps/web/components/task/changes-panel-header.tsx index b4e626ddba..dde133f7c0 100644 --- a/apps/web/components/task/changes-panel-header.tsx +++ b/apps/web/components/task/changes-panel-header.tsx @@ -19,6 +19,14 @@ import { Label } from "@kandev/ui/label"; import { Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle } from "@kandev/ui/dialog"; import { HoverCard, HoverCardContent, HoverCardTrigger } from "@kandev/ui/hover-card"; import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip"; +import { + Drawer, + DrawerContent, + DrawerDescription, + DrawerHeader, + DrawerTitle, + DrawerTrigger, +} from "@kandev/ui/drawer"; import { DropdownMenu, DropdownMenuContent, @@ -29,6 +37,8 @@ import { } from "@kandev/ui/dropdown-menu"; import { PanelHeaderBarSplit } from "./panel-primitives"; import { BaseBranchPicker } from "./base-branch-picker"; +import type { GitCredentialDisplay } from "./changes-git-credential-display"; +import { useTouchDrawer } from "@/hooks/use-compact-task-chrome"; type PerRepoStatus = { repository_name: string; @@ -208,6 +218,7 @@ function BranchHoverCard({ taskId, onRenameBranch, isRenaming, + credentialDisplay, }: { displayBranch: string; baseBranchDisplay: string; @@ -218,50 +229,79 @@ function BranchHoverCard({ taskId: string | null; onRenameBranch?: (newName: string, repo: string) => Promise; isRenaming: boolean; + credentialDisplay: GitCredentialDisplay | null; }) { + const usesTouchDrawer = useTouchDrawer(); + const [open, setOpen] = useState(false); const isMulti = rows && rows.length > 0; const headerLabel = isMulti ? "Your branches:" : "Your code lives in:"; const trailerLabel = "comparing against:"; - return ( - - - - - -
-
- {headerLabel} - {trailerLabel} -
- {isMulti ? ( -
- {rows!.map((row) => ( - - ))} -
- ) : ( + const trigger = ( + + ); + const content = ( +
+
+ {headerLabel} + {trailerLabel} +
+ {isMulti ? ( +
+ {rows!.map((row) => ( - )} + ))}
+ ) : ( + + )} + {credentialDisplay && ( +
+

{credentialDisplay.source}

+

{credentialDisplay.detail}

+

{credentialDisplay.transport}

+
+ )} +
+ ); + if (usesTouchDrawer) { + return ( + + {trigger} + + + Branch details + Branch comparison and task Git credentials. + +
{content}
+
+
+ ); + } + return ( + + {trigger} + + {content} ); @@ -517,6 +557,7 @@ export function ChangesPanelHeader({ repoDisplayName, taskId, onRenameBranch, + credentialDisplay, }: { hasChanges: boolean; hasCommits: boolean; @@ -540,6 +581,7 @@ export function ChangesPanelHeader({ onRepoRebase: (repo: string) => void; onRepoMerge: (repo: string) => void; onRenameBranch?: (newName: string, repo: string) => Promise; + credentialDisplay: GitCredentialDisplay | null; repoDisplayName?: (repositoryName: string) => string | undefined; /** Active task id; piped into the base-branch picker so it can resolve * the right task_repositories row to PATCH. Null while task data is @@ -574,6 +616,7 @@ export function ChangesPanelHeader({ taskId={taskId} onRenameBranch={onRenameBranch} isRenaming={loadingOperation === "rename_branch"} + credentialDisplay={credentialDisplay} /> )} diff --git a/apps/web/components/task/mobile/mobile-changes-panel.tsx b/apps/web/components/task/mobile/mobile-changes-panel.tsx index 9c12ccadfd..c4f5ddd6fc 100644 --- a/apps/web/components/task/mobile/mobile-changes-panel.tsx +++ b/apps/web/components/task/mobile/mobile-changes-panel.tsx @@ -117,6 +117,7 @@ export const MobileChangesPanel = memo(function MobileChangesPanel({ onRenameBranch={data.git.renameBranch} repoDisplayName={data.repoDisplayName} taskId={data.activeTaskId} + credentialDisplay={data.gitCredentialDisplay} /> diff --git a/apps/web/e2e/tests/integrations/github-workspace-settings.spec.ts b/apps/web/e2e/tests/integrations/github-workspace-settings.spec.ts index bca8a9f103..4817764224 100644 --- a/apps/web/e2e/tests/integrations/github-workspace-settings.spec.ts +++ b/apps/web/e2e/tests/integrations/github-workspace-settings.spec.ts @@ -1,6 +1,32 @@ import { test, expect } from "../../fixtures/test-base"; test.describe("GitHub workspace settings", () => { + test("saves the explicit task Git credential policy", async ({ + testPage, + apiClient, + seedData, + prCapture, + }) => { + await testPage.goto(`/settings/workspace/${seedData.workspaceId}/integrations/github`); + await expect(testPage.getByRole("heading", { name: "Task Git credentials" })).toBeVisible(); + + await testPage.getByRole("radio", { name: "Inherit executor Git credentials" }).click(); + await prCapture.screenshot("desktop-task-git-credentials", { + caption: "Workspace task Git credential policy", + }); + await testPage.getByTestId("settings-floating-save").getByRole("button").click(); + await expect(testPage.getByText("Task Git credential settings saved")).toBeVisible({ + timeout: 10_000, + }); + + const response = await apiClient.rawRequest( + "GET", + `/api/v1/github/workspace-settings?workspace_id=${seedData.workspaceId}`, + ); + expect(response.status).toBe(200); + expect(await response.json()).toMatchObject({ task_git_credentials_mode: "executor" }); + }); + test("repository scope is saved per workspace and filters the GitHub PR list", async ({ testPage, apiClient, diff --git a/apps/web/e2e/tests/integrations/mobile-github-workspace-settings.spec.ts b/apps/web/e2e/tests/integrations/mobile-github-workspace-settings.spec.ts index ac9dbcda3b..232c729c58 100644 --- a/apps/web/e2e/tests/integrations/mobile-github-workspace-settings.spec.ts +++ b/apps/web/e2e/tests/integrations/mobile-github-workspace-settings.spec.ts @@ -1,6 +1,36 @@ import { test, expect } from "../../fixtures/test-base"; test.describe("GitHub workspace settings on mobile", () => { + test("explains and saves task Git credential inheritance", async ({ + testPage, + apiClient, + seedData, + prCapture, + }) => { + await testPage.goto(`/settings/workspace/${seedData.workspaceId}/integrations/github`); + + await testPage.getByRole("button", { name: "Explain task Git credentials" }).click(); + await expect( + testPage.getByRole("dialog", { name: "How task Git credentials work" }), + ).toContainText("GH_TOKEN or GITHUB_TOKEN"); + await prCapture.screenshot("mobile-task-git-credentials-help", { + caption: "Mobile task Git credential explanation", + }); + await testPage.keyboard.press("Escape"); + + await testPage.getByRole("radio", { name: "Inherit executor Git credentials" }).click(); + await testPage.getByTestId("settings-floating-save").getByRole("button").click(); + await expect(testPage.getByText("Task Git credential settings saved")).toBeVisible({ + timeout: 10_000, + }); + + const response = await apiClient.rawRequest( + "GET", + `/api/v1/github/workspace-settings?workspace_id=${seedData.workspaceId}`, + ); + expect(await response.json()).toMatchObject({ task_git_credentials_mode: "executor" }); + }); + test("explains repository scope below issue watches without requiring hover", async ({ testPage, seedData, diff --git a/apps/web/e2e/tests/settings/copy-files-symlink-helpers.ts b/apps/web/e2e/tests/settings/copy-files-symlink-helpers.ts index 519acdabf8..0427195a73 100644 --- a/apps/web/e2e/tests/settings/copy-files-symlink-helpers.ts +++ b/apps/web/e2e/tests/settings/copy-files-symlink-helpers.ts @@ -11,6 +11,7 @@ export async function configureSymlinkAndCreateTask(options: { }) { const { page, apiClient, seedData, title } = options; await page.goto(`/settings/workspace/${seedData.workspaceId}/repositories`); + await expect(page.getByRole("button", { name: "Add Local Repository" })).toBeVisible(); await page.getByRole("button", { name: "Edit" }).first().click(); const input = page.getByTestId(`copy-files-input-${seedData.repositoryId}`); diff --git a/apps/web/e2e/tests/settings/github-app-registration.spec.ts b/apps/web/e2e/tests/settings/github-app-registration.spec.ts index bdaf238228..124910c8a8 100644 --- a/apps/web/e2e/tests/settings/github-app-registration.spec.ts +++ b/apps/web/e2e/tests/settings/github-app-registration.spec.ts @@ -216,7 +216,7 @@ test.describe("Workspace GitHub App onboarding", () => { await connection.getByLabel("Name in Kandev").fill("Existing company App"); await connection.getByLabel("GitHub App slug").fill("company-kandev"); await connection.getByLabel("GitHub owner login").fill("acme"); - await connection.getByLabel("App ID").fill("404"); + await connection.getByRole("textbox", { name: "App ID" }).fill("404"); await connection.getByLabel("Client ID").fill("Iv1.client-id"); await connection.getByLabel("Client secret").fill("client-secret-value"); await connection.getByLabel("Webhook secret").fill("webhook-secret-value"); diff --git a/apps/web/lib/types/github.ts b/apps/web/lib/types/github.ts index 6bac724b09..d968bbcd1b 100644 --- a/apps/web/lib/types/github.ts +++ b/apps/web/lib/types/github.ts @@ -312,9 +312,11 @@ export type RepoFilter = { }; export type GitHubRepoScopeMode = "all" | "orgs" | "repos"; +export type TaskGitCredentialsMode = "managed" | "executor"; export type GitHubWorkspaceSettings = { workspace_id: string; + task_git_credentials_mode?: TaskGitCredentialsMode; repo_scope_mode: GitHubRepoScopeMode; repo_scope_orgs: string[]; repo_scope_repos: RepoFilter[]; @@ -326,6 +328,7 @@ export type GitHubWorkspaceSettings = { export type UpdateGitHubWorkspaceSettingsRequest = { workspace_id: string; + task_git_credentials_mode?: TaskGitCredentialsMode; repo_scope_mode?: GitHubRepoScopeMode; repo_scope_orgs?: string[]; repo_scope_repos?: RepoFilter[]; diff --git a/docs/decisions/0047-github-authentication-ownership.md b/docs/decisions/0047-github-authentication-ownership.md index ff77f02739..b32bb0234a 100644 --- a/docs/decisions/0047-github-authentication-ownership.md +++ b/docs/decisions/0047-github-authentication-ownership.md @@ -4,6 +4,12 @@ > [ADR-2026-07-21-workspace-selectable-github-app-registrations](2026-07-21-workspace-selectable-github-app-registrations.md): > Kandev stores multiple App registrations and every workspace explicitly selects one registration > and installation. The automation and personal identity boundaries remain unchanged. +> +> Amended by +> [ADR-2026-07-27-task-git-credential-policy](2026-07-27-task-git-credential-policy.md): +> workspace automation identity and task Git credential routing are separate settings. Background +> automation still uses the workspace connection; tasks may explicitly inherit executor +> credentials instead of receiving the managed broker contract. - Status: accepted - Date: 2026-07-19 @@ -40,7 +46,8 @@ GitHub authentication has three ownership layers and is resolved by purpose: creating separate registrations provides independent root credentials and bot identities. 2. **Workspace-owned automation connection.** Each workspace selects exactly one automation source: PAT, a named `gh` CLI host/login, a verified App installation, or the temporary - `legacy_shared` migration source. Background work and agents always use this identity. + `legacy_shared` migration source. Background work and managed-mode task agents use this + identity; executor-inherited tasks deliberately use executor-visible Git credentials instead. 3. **User-owned personal connection within a workspace.** At most one verified App user connection exists per `(workspace_id, user_id)`. It provides authenticated-viewer semantics and human attribution. It is never injected into an agent. The current runtime uses `default-user`; future diff --git a/docs/decisions/2026-07-27-task-git-credential-policy.md b/docs/decisions/2026-07-27-task-git-credential-policy.md new file mode 100644 index 0000000000..80d0c1cef9 --- /dev/null +++ b/docs/decisions/2026-07-27-task-git-credential-policy.md @@ -0,0 +1,115 @@ +# ADR-2026-07-27-task-git-credential-policy: Separate GitHub Automation From Task Git Credential Policy + +**Status:** accepted +**Date:** 2026-07-27 +**Area:** backend, frontend, security + +## Context + +A workspace GitHub connection currently selects both the identity used by Kandev background +automation and the credentials injected into task processes. That coupling is surprising for local +users who expect Git to use their host credential manager or SSH setup, and selecting a named +`gh` account does not mean that its host configuration should be inherited by every executor. +Managed task access also depends on a broker-aware `agentctl` and `gh` tool path; a standalone +control process can start successfully while leaving the child Git helper unable to find +`agentctl`. + +## Decision + +Workspace GitHub automation identity and task Git credential policy are separate workspace-owned +settings: + +- `managed` is the default. Kandev injects task/repository-bound broker leases for attached GitHub + repositories. PAT, named GitHub CLI, GitHub App, and migration-only legacy connections remain + automation methods rather than task-policy values. +- `executor` is the explicit inheritance mode. Kandev injects no GitHub broker helper or `gh` shim. + Local and Worktree tasks use credentials visible on the Kandev host; Docker, SSH, Sprites, and + other remote tasks use credentials configured in that executor. +- An explicit executor-profile `GITHUB_TOKEN` or `GH_TOKEN` remains an unmanaged override. It takes + precedence when the selected policy is `managed` and is disclosed as an executor-profile token + whose actor is selected at runtime. + +The policy is stored with the non-secret `github_workspace_settings` operational settings, is +copied by workspace-settings copy, and does not create another +`github_workspace_connections.source`. Missing or invalid stored values normalize to `managed` for +backward compatibility. + +Managed runtime tools are installed once by the `agentctl` control process and activated only for +an instance carrying a valid broker contract. The managed tool directory contains both the +`agentctl` Git credential helper and the broker-aware `gh` shim. GitHub HTTPS helper configuration +resets the inherited helper chain before adding Kandev's helper, disables terminal prompts, and +fails closed when lease redemption or the helper fails. The contract does not claim to prevent an +arbitrary agent from manually changing a remote to SSH or invoking another credential-bearing +tool; executor isolation and agent permissions remain the boundary for that behavior. + +`GIT_CONFIG_COUNT` and its indexed `GIT_CONFIG_KEY_` / `GIT_CONFIG_VALUE_` variables are one +ordered protocol, not independent environment variables. Every task-launch boundary that combines +environment sources must parse, compose, and contiguously reindex valid blocks. Lower-precedence +executor or host entries come first and higher-precedence task entries follow; the longest exact +suffix/prefix overlap is emitted once when a block has already been forwarded through a control +process. Kandev may reset the GitHub credential-helper chain with a later empty helper entry, but it +must preserve unrelated entries such as `core.hooksPath`, `safe.directory`, URL rewrites, and notes +configuration. Malformed indexed blocks fail task environment preparation with a credential-safe +diagnostic instead of being partially overlaid. Repository-clone subprocesses remain an explicit +exception: they intentionally build an isolated Git environment and strip ambient indexed config. + +Initial launch and resume use the same policy resolver. After each successful launch or resume, +Kandev stores a versioned, non-secret `git_credential_snapshot` in task-session metadata. It +records the selected policy, effective source (`workspace`, `executor_profile`, or `executor`), +workspace method when known, known actor label or `runtime_selected`, transport, executor type, +and capture time. It never stores a token, lease, helper path, or credential-file path. The Changes +panel reads this launch-time snapshot; it does not probe the current host or executor and does not +rewrite history when workspace settings later change. + +The branch disclosure shows the snapshot alongside branch/base-branch information. Fine-pointer +desktop users receive hover and keyboard-focus access; coarse-pointer users receive the same +content in a tappable drawer. GitHub settings explain every automation method in visible text and +provide a supplementary information disclosure describing how each method reaches tasks under +both policies. + +Named GitHub CLI credentials remain deterministic host/login selections. Their derived bearer +tokens are cached in memory for no more than five minutes before Kandev asks the host CLI for that +exact account again; cache invalidation on connection generation changes remains immediate. + +## Consequences + +- Users can keep Kandev background automation on a PAT, named CLI account, or App while choosing + traditional executor Git behavior for tasks. +- Selecting GitHub CLI authentication no longer implies host credential inheritance. Managed mode + still resolves the named account on the Kandev host and brokers its bearer token to tasks. +- Existing workspaces remain managed without a migration-time behavior change. +- Host and executor Git tooling expressed through indexed environment config survives Kandev + credential injection; Kandev's later GitHub helper entries retain their intended precedence. +- The UI can truthfully identify managed actors while labeling inherited and profile-token actors + as runtime-selected instead of guessing. +- Resume may pick up a changed workspace policy or automation connection, and its new snapshot + replaces the prior launch snapshot only after the resume succeeds. +- Managed HTTPS Git and `gh` fail closed, but a local/Worktree agent still runs with host process + authority; this setting is credential routing, not a sandbox. +- Environment composition becomes fallible when an indexed Git config block is malformed, so + launch errors can identify the invalid source instead of silently producing a different Git + configuration. + +## Alternatives Considered + +### Treat executor inheritance as a fourth GitHub connection source + +Rejected. It would conflate Kandev's background automation identity with task transport again and +would leave watches and repository discovery without a defined credential. + +### Infer host inheritance from the named GitHub CLI method + +Rejected. The named CLI selection is a deterministic automation identity. Remote executors cannot +inherit the Kandev host's CLI configuration, and local inference would make the same setting mean +different security policies by executor. + +### Display the current workspace connection instead of a session snapshot + +Rejected. Settings can change after a task starts, so current status can misrepresent the +credential contract under which the visible session actually launched. + +### Probe inherited credentials to discover the actor + +Rejected. Git credential managers and SSH agents may be interactive, may choose by URL at command +time, and should not be invoked merely to render UI. Unknown inherited actors are labeled +`runtime_selected`. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 89f28c0ae9..2a1e79a1ee 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -80,3 +80,4 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 2026-07-24-semantic-notification-events | [Semantic notification events come from domain occurrences](2026-07-24-semantic-notification-events.md) | accepted | backend, frontend, desktop | 2026-07-24 | | 2026-07-26-user-managed-agent-runtime-updates | [User-Managed Agent Runtime Updates](2026-07-26-user-managed-agent-runtime-updates.md) | accepted | backend, frontend, protocol | 2026-07-26 | | 2026-07-26-single-session-model-switching | [Single-Session Model Switching](2026-07-26-single-session-model-switching.md) | accepted (amended 2026-07-27) | workflow, infra | 2026-07-26 | +| 2026-07-27-task-git-credential-policy | [Separate GitHub Automation From Task Git Credential Policy](2026-07-27-task-git-credential-policy.md) | accepted | backend, frontend, security | 2026-07-27 | diff --git a/docs/plans/task-git-credential-policy/plan.md b/docs/plans/task-git-credential-policy/plan.md new file mode 100644 index 0000000000..3b6e81e0d3 --- /dev/null +++ b/docs/plans/task-git-credential-policy/plan.md @@ -0,0 +1,272 @@ +--- +spec: docs/specs/integrations/github-authentication.md +created: 2026-07-27 +status: complete +--- + +# Implementation Plan: Task Git Credential Policy And Identity + +## Overview + +Fix the confirmed standalone managed-helper wiring failure while separating workspace GitHub +automation from task Git credential routing. Persist the policy first, then make initial launch and +resume resolve the same effective contract and record a truthful session snapshot. Finish with +self-documenting settings, responsive Changes-panel identity disclosure, focused E2E coverage, and +public documentation. + +Confirmed root causes: + +1. The standalone launcher resolves and starts the `agentctl` control binary by absolute path, but + the per-task Git config stores `!agentctl git-credential`. The control process only + creates/prepends its `gh` shim directory when broker environment exists at control-process + startup; standalone broker data arrives later in the per-instance environment. Therefore child + Git cannot find `agentctl`. +2. Task environments currently merge `GIT_CONFIG_COUNT` and indexed keys as ordinary map entries. + A Kandev block with count 2 overwrites a host/executor block's count and first two entries, so + unrelated tools such as hooks and Git notes disappear. The same defect can hide Docker's + `safe.directory` and URL rewrites. A focused reproduction against `CollectAgentEnv` produced + count 2 where the correct composed result is count 4. +3. Resume currently does not rebuild the broker contract at all. + +--- + +## Backend + +### Ordered indexed Git configuration + +- Add a shared backend utility for parsing, validating, composing, and contiguously serializing the + `GIT_CONFIG_COUNT` / `GIT_CONFIG_KEY_` / `GIT_CONFIG_VALUE_` protocol. +- Composition preserves source order, appends higher-precedence entries, and removes only the + longest exact suffix/prefix overlap when the same block has already passed through a control + process. It never deduplicates arbitrary repeated Git keys because repeated keys can be + meaningful. +- Use structured composition in agent-profile/request merges, route overrides, container + environment construction, remote forwarding, and agentctl's parent-plus-instance collection. + Ordinary scalar environment variables retain their documented precedence. +- Keep `internal/repoclone` isolated: managed clone subprocesses intentionally strip ambient Git + configuration and build a closed credential environment. +- Add a real Git subprocess regression that starts with locstat-shaped `core.hooksPath` and + `notes.augment.mergeStrategy` entries, adds Kandev's helper entries, makes a commit, and verifies + the hook ran. Add focused standalone, Docker, SSH/remote, overlap, precedence, malformed-block, + and bounded-count tests. + +### Workspace policy persistence and description + +- In `apps/backend/internal/github/models.go`, add + `TaskGitCredentialsModeManaged`, `TaskGitCredentialsModeExecutor`, and + `WorkspaceSettings.TaskGitCredentialsMode` / + `UpdateWorkspaceSettingsRequest.TaskGitCredentialsMode`. +- In `apps/backend/internal/github/store.go`, add + `github_workspace_settings.task_git_credentials_mode TEXT NOT NULL DEFAULT 'managed'` to fresh + schema and a fail-loud, idempotent startup migration. Include it in defaults, normalization, + reads, upserts, partial patches, and workspace-settings copy. +- In `apps/backend/internal/github/workspace_settings_service.go`, reject unknown policy values + without mutating the row. +- Add a non-secret service description that combines the policy with + `github_workspace_connections` and, for Apps, registration display identity. It returns the + selected mode, method, and known actor context without resolving or exposing a credential. +- Extend the existing `/api/v1/github/workspace-settings` response/update contract. Authorization + remains the existing workspace-settings authorization path. + +### Launch/resume resolution and session snapshot + +- In `apps/backend/internal/task/models/models.go`, add + `SessionMetaKeyGitCredentialSnapshot`, a versioned `GitCredentialSnapshot` type, and a + JSON-rehydration-safe loader. +- In `apps/backend/internal/orchestrator/executor/executor.go` and + `executor_credentials.go`, add a task Git credential policy resolver wired beside + `GitHubCredentialLeaseIssuer`. Resolve one of: + `workspace`, `executor_profile`, or `executor`. +- Move effective GitHub credential configuration out of the middle of + `buildLaunchAgentRequest` so it runs after profile/request environment is final. Reuse the same + function from `buildResumeRequest` after `resolveAllRepoInfo`. +- For managed workspace credentials, clear stale broker keys, reset the GitHub HTTPS helper chain + with an empty helper before Kandev's helper, set `credential.useHttpPath=true`, and disable + terminal prompts. Executor inheritance removes/omits Kandev broker/helper/shim configuration. +- Put the candidate snapshot on the in-memory session before lifecycle launch, but persist it only + through the existing successful `persistLaunchState` / `persistResumeState` full-row write. A + failed operation leaves the prior durable snapshot unchanged. +- In `apps/backend/internal/backendapp/orchestrator.go`, adapt the GitHub service's non-secret + policy descriptor into the executor interface alongside the existing lease adapter. + +### Managed runtime tools and CLI refresh + +- Refactor `apps/backend/cmd/agentctl/github_cli_shim.go` so the control process always creates one + private managed GitHub tool directory containing both `gh` and `agentctl` links/copies, records + its path, and does not globally prepend it for every instance. +- In `apps/backend/internal/agentctl/server/config/config.go`, prepend that directory only when the + instance's additional environment contains the complete broker contract. All agent processes, + shells, background commands, and agentctl Git operations then share the same per-instance PATH. +- Preserve `runGitHubCLIShim` lookup of the real `gh` outside the managed directory. Executor + inheritance and profile-token override instances must see the normal executor PATH. +- Add a real subprocess regression around the standalone shape: control `agentctl` starts without + broker env, a broker-enabled instance receives it later, and both `git credential fill` and + `gh` resolve through the managed tools. Missing broker/helper fails instead of consulting an + ambient helper. +- In `apps/backend/internal/github/auth_resolver.go`, give named-CLI cached credentials a separate + five-minute cache deadline without pretending the provider token itself has an expiry. + Generation invalidation remains immediate. + +--- + +## Frontend + +### Task Git credentials settings + +- Extend `GitHubWorkspaceSettings` and `UpdateGitHubWorkspaceSettingsRequest` in + `apps/web/lib/types/github.ts`. +- Add `apps/web/components/github/github-task-credentials-section.tsx`, registered with + `useSettingsSaveContributor`, with two explicit choices: + **Managed workspace credentials** and **Inherit executor Git credentials**. +- Visible copy must explain local/Worktree host behavior, remote executor behavior, profile-token + precedence, and when each choice applies. Reuse the repository-scope responsive help pattern: + tooltip/focus on a fine pointer and a 44px information-button Drawer on a coarse pointer. +- Update `github-auth-method-list.tsx`, `github-connection-dialog.tsx`, and `github-settings.tsx` + so PAT, named CLI, and App cards state where their credential is stored/resolved and how managed + tasks receive it. The information disclosure supplements rather than replaces this text. + +### Changes branch credential disclosure + +- Add a pure parser/view-model module beside the Changes panel that reads + `git_credential_snapshot` from the active `TaskSession.metadata`, tolerates malformed/legacy + values, and produces display labels without guessing unknown actors. +- Extract the current branch-card body from + `apps/web/components/task/changes-panel-header.tsx`. Keep the compact fine-pointer trigger, but + make it keyboard-focus accessible and include policy, effective method/source, actor, and + transport below the existing branch/base-branch rows. +- Use `useTouchDrawer` for coarse pointers. The mobile trigger is at least 44px and opens a Drawer + containing the same branch editing/base-branch controls and credential truth; desktop keeps the + hover/focus card. +- Select the active session snapshot in `useChangesPanelData` and pass the normalized disclosure + through both desktop `changes-panel.tsx` and `mobile/mobile-changes-panel.tsx`. + +--- + +## Tests + +- **What:** fresh/replayed schemas default existing and new workspaces to `managed`; policy + round-trips, partial updates reject invalid values, and settings copy includes policy but not + authentication. + **File:** `apps/backend/internal/github/workspace_settings_test.go`, + `copy_test.go`, and focused migration tests. + **How:** real SQLite store/service tests, including same-database schema replay. +- **What:** the internal policy description identifies PAT/CLI humans and App registration context + without resolving tokens, while executor actors remain unknown. + **File:** focused tests under `apps/backend/internal/github/`. + **How:** table-driven service tests with fake connections/registration store. +- **What:** initial launch and resume produce the same managed/executor/profile-override contract; + managed helper chains reset, inheritance injects nothing, and only successful lifecycle launches + persist a new session snapshot. + **File:** `apps/backend/internal/orchestrator/executor/executor_credentials_test.go`, + `executor_resume_test.go`, and launch-path tests. + **How:** table-driven unit tests plus mocked lifecycle success/failure integration tests. +- **What:** host/executor/profile/task indexed Git config blocks compose in precedence order, + already-forwarded suffixes occur once, malformed blocks fail clearly, and unrelated settings + remain active beside Kandev's managed helper. + **File:** new shared utility tests, + `apps/backend/internal/agentctl/server/config/config_test.go`, + `apps/backend/internal/agent/runtime/lifecycle/executor_docker_test.go`, and focused remote tests. + **How:** table-driven block composition plus a temp-repository real Git commit/hook subprocess + regression using locstat-shaped entries. +- **What:** standalone per-instance broker activation makes `agentctl` and `gh` discoverable even + though the control process started without broker env; inherit/override instances do not activate + the tool directory. + **File:** `apps/backend/cmd/agentctl/github_cli_shim_test.go`, + `apps/backend/internal/agentctl/server/config/config_test.go`. + **How:** temp executable/subprocess and fake HTTP broker tests; invoke real Git credential + plumbing where supported. +- **What:** named CLI cache entries re-resolve after five minutes and still invalidate immediately + on generation changes. + **File:** `apps/backend/internal/github/auth_resolver_test.go`. + **How:** deterministic fake clock and call counter. +- **What:** settings drafts load, save, discard, and explain both policies and every automation + method. + **File:** new component/model tests beside + `apps/web/components/github/github-task-credentials-section.tsx`. + **How:** mocked API promises and settings-save provider; keyboard-focus help assertion. +- **What:** valid/malformed/missing session snapshots produce truthful disclosure labels, never an + inferred actor. + **File:** new Changes credential view-model test and focused header component test. + **How:** table-driven Vitest plus keyboard-focus component test under `TooltipProvider`. + +Targeted pre-PR commands are recorded in the task files. + +--- + +## E2E Tests + +- **Scenario:** GIVEN workspace settings, WHEN the user switches to executor inheritance and saves, + THEN the API persists `executor`; discard/reload and switching back to managed remain coherent, + and the desktop help names PAT/CLI/App delivery and profile-token precedence. + **File:** extend + `apps/web/e2e/tests/integrations/github-workspace-settings.spec.ts`. +- **Scenario:** GIVEN a coarse-pointer settings viewport, WHEN the task-credential information icon + is tapped, THEN a Drawer contains the same delivery explanation, controls meet 44px targets, and + the page has no horizontal overflow. + **File:** extend + `apps/web/e2e/tests/integrations/mobile-github-auth-settings.spec.ts`. +- **Scenario:** GIVEN a session seeded with a managed CLI or App snapshot, WHEN a desktop user + hovers/focuses the Changes branch trigger, THEN branch/base information and launch-time actor, + method, and managed transport are visible. + **File:** `apps/web/e2e/tests/git/git-credential-identity.spec.ts`. +- **Scenario:** GIVEN an inherited or profile-token snapshot on a coarse-pointer task view, WHEN the + branch trigger is tapped, THEN a Drawer labels the actor runtime-selected, has a 44px trigger, and + does not overflow. + **File:** `apps/web/e2e/tests/git/mobile-git-credential-identity.spec.ts`. + +Production E2E runs only after rebuilding backend and Vite artifacts. + +--- + +## Public Documentation + +- Update `docs/public/integrations.md`, `docs/public/executors.md`, and + `docs/public/use-kandev.md` to separate workspace automation method from task credential policy, + describe named CLI brokerage, explain executor inheritance, and document the Changes snapshot. +- Update troubleshooting so `agentctl: command not found` is a managed-runtime defect rather than a + prompt to fall back to personal SSH, and distinguish managed-helper failure from missing executor + credentials. + +--- + +## Implementation Waves And Parallel Candidates + +Wave 1: + +- [x] [task-00-indexed-git-config-composition](task-00-indexed-git-config-composition.md) +- [x] [task-01-policy-persistence](task-01-policy-persistence.md) + +Wave 2 (parallel candidates — user authorization required): + +- [x] [task-02-launch-resume-snapshot](task-02-launch-resume-snapshot.md) +- [x] [task-04-settings-explanation](task-04-settings-explanation.md) + +Wave 3 (parallel candidates — user authorization required): + +- [x] [task-03-managed-runtime-tools](task-03-managed-runtime-tools.md) +- [x] [task-05-changes-identity-disclosure](task-05-changes-identity-disclosure.md) + +Wave 4: + +- [x] [task-06-e2e-and-documentation](task-06-e2e-and-documentation.md) + +The default execution is sequential in this primary conversation. Waves identify dependency-safe +opportunities only and do not authorize subagents. + +## Risks + +- Local and Worktree agents retain host process authority. Managed routing can fail closed for + Kandev-injected GitHub HTTPS/`gh`, but cannot prevent a capable agent from selecting SSH or + another host tool. +- The helper-reset entry must be ordered before Kandev's helper without deleting unrelated + caller-supplied Git config. +- Indexed Git config keys form one structured protocol. Plain map overlay at any launch boundary + can regress host tooling; arbitrary de-duplication can also change the meaning of multi-valued + Git settings. +- Docker and remote control processes can already contain the task suffix before it is forwarded + to an instance, so composition must remove only an exact boundary overlap. +- Resume must resolve every attached repository, not only the legacy primary fields. +- The snapshot is launch-time truth. Rendering current workspace status in its place would + reintroduce misleading identity. +- Agentctl tool-directory activation must be per instance so executor inheritance and explicit + profile-token overrides do not accidentally invoke the broker shim. diff --git a/docs/plans/task-git-credential-policy/task-00-indexed-git-config-composition.md b/docs/plans/task-git-credential-policy/task-00-indexed-git-config-composition.md new file mode 100644 index 0000000000..d24521a37f --- /dev/null +++ b/docs/plans/task-git-credential-policy/task-00-indexed-git-config-composition.md @@ -0,0 +1,66 @@ +--- +id: "00-indexed-git-config-composition" +title: "Preserve ordered indexed Git configuration across task environments" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 00: Preserve Ordered Indexed Git Configuration + +## Acceptance + +- Valid `GIT_CONFIG_COUNT` / `GIT_CONFIG_KEY_` / `GIT_CONFIG_VALUE_` blocks from the host, + executor, profile, task, and Kandev compose into one contiguous ordered block. +- Higher-precedence entries follow lower-precedence entries. Only the longest exact suffix/prefix + overlap is emitted once; arbitrary repeated entries remain untouched. +- A locstat-shaped host block followed by Kandev's two managed credential entries produces count 4, + preserves hooks and notes settings, and a real Git commit executes the configured hook. +- Docker retains `safe.directory` and SSH-to-HTTPS rewrites, while standalone and remote agentctl + instance creation do not clobber or duplicate forwarded entries. +- Malformed or unreasonably large blocks fail environment preparation with a sanitized error. + `internal/repoclone` remains intentionally isolated from ambient Git config. + +## Verification + +```bash +cd apps/backend && go test ./internal/gitconfigenv ./internal/agentctl/server/config ./internal/agent/runtime/lifecycle ./internal/orchestrator/executor -run 'Test.*(IndexedGitConfig|CollectAgentEnv.*GitConfig|BuildEnvVars.*GitConfig|GitConfigPreservesHook)' +``` + +## Files likely touched + +- new shared package under `apps/backend/internal/gitconfigenv/` +- `apps/backend/internal/agentctl/server/config/config.go` +- `apps/backend/internal/agentctl/server/config/config_test.go` +- `apps/backend/internal/agent/runtime/lifecycle/profile_env.go` +- `apps/backend/internal/agent/runtime/lifecycle/container.go` +- `apps/backend/internal/agent/runtime/lifecycle/remote_github_env.go` +- focused standalone, Docker, and SSH/remote tests +- `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 + +Parallel-safe with Task 01: this task owns environment composition/runtime plumbing while Task 01 +owns workspace settings persistence. + +## Inputs + +- The reproduced `CollectAgentEnv` failure: parent count 2 plus task count 2 returns count 2 instead + of 4. +- Existing Docker composition in `container.go`, remote allowlisting in + `remote_github_env.go`, and managed clone isolation in `internal/repoclone`. +- Spec scenarios for locstat-shaped hooks/notes preservation and exact boundary overlap. + +## Output contract + +Report the parser validity rules and count bound, precedence and overlap algorithm, every updated +merge boundary, real Git subprocess RED/GREEN evidence, executor-specific focused results, +confirmed `repoclone` isolation, files changed, and update task/plan status. diff --git a/docs/plans/task-git-credential-policy/task-01-policy-persistence.md b/docs/plans/task-git-credential-policy/task-01-policy-persistence.md new file mode 100644 index 0000000000..7febd5c0ce --- /dev/null +++ b/docs/plans/task-git-credential-policy/task-01-policy-persistence.md @@ -0,0 +1,58 @@ +--- +id: "01-policy-persistence" +title: "Persist the task Git credential policy" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 01: Persist The Task Git Credential Policy + +## Acceptance + +- `github_workspace_settings.task_git_credentials_mode` defaults/normalizes to `managed`, accepts + only `managed` or `executor`, and survives fresh schema, replay, read, upsert, partial update, and + settings copy. +- The existing workspace-settings API exposes the policy without changing its authorization model. +- A non-secret GitHub service descriptor returns the policy plus known workspace method/actor + context without resolving a token. + +## Verification + +```bash +cd apps/backend && go test ./internal/github ./internal/backendapp -run 'Test(Store_GitHubWorkspaceSettings|Service_UpdateWorkspaceSettings|CopyWorkspaceSettings|TaskGitCredential|GitHubWorkspaceSettings)' +``` + +## Files likely touched + +- `apps/backend/internal/github/models.go` +- `apps/backend/internal/github/store.go` +- `apps/backend/internal/github/workspace_settings_service.go` +- `apps/backend/internal/github/copy.go` +- `apps/backend/internal/github/controller.go` +- `apps/backend/internal/github/workspace_settings_test.go` +- `apps/backend/internal/github/copy_test.go` +- focused GitHub schema/descriptor tests +- `apps/backend/internal/backendapp/services_github_broker_test.go` +- this task file and `plan.md` + +## Dependencies + +None. + +## Parallelism + +Sequential. It owns the shared schema and API contract. + +## Inputs + +- Spec sections `What`, `Data Model`, `API Surface`, and task-policy scenarios. +- ADR-2026-07-27-task-git-credential-policy. +- Existing repository-scope partial-update and workspace-copy patterns. + +## Output contract + +Report the migration/replay behavior, normalized enum contract, descriptor shape, RED/GREEN command +results, files changed, residual risks, and update this task plus `plan.md` status. diff --git a/docs/plans/task-git-credential-policy/task-02-launch-resume-snapshot.md b/docs/plans/task-git-credential-policy/task-02-launch-resume-snapshot.md new file mode 100644 index 0000000000..7f286025c8 --- /dev/null +++ b/docs/plans/task-git-credential-policy/task-02-launch-resume-snapshot.md @@ -0,0 +1,62 @@ +--- +id: "02-launch-resume-snapshot" +title: "Resolve launch and resume credentials with a session snapshot" +status: done +wave: 2 +depends_on: ["00-indexed-git-config-composition", "01-policy-persistence"] +plan: "plan.md" +spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 02: Resolve Launch And Resume Credentials With A Session Snapshot + +## Acceptance + +- Initial launch and resume share one resolver for managed, executor-inherited, and explicit + profile-token outcomes across every attached repository. +- Managed GitHub HTTPS resets inherited helpers and fails closed; executor inheritance injects no + Kandev broker/helper/shim contract. +- Only a successful launch/resume persists the versioned, non-secret session credential snapshot; + actor labels are known only for workspace identities Kandev can describe. + +## Verification + +```bash +cd apps/backend && go test ./internal/orchestrator/executor ./internal/backendapp ./internal/task/models -run 'Test.*(GitHubCredential|GitCredential|Resume.*Credential|Launch.*Credential)' +``` + +## Files likely touched + +- `apps/backend/internal/task/models/models.go` +- `apps/backend/internal/task/models/models_test.go` +- `apps/backend/internal/orchestrator/executor/executor.go` +- `apps/backend/internal/orchestrator/executor/executor_credentials.go` +- `apps/backend/internal/orchestrator/executor/executor_execute.go` +- `apps/backend/internal/orchestrator/executor/executor_resume.go` +- `apps/backend/internal/orchestrator/executor/executor_credentials_test.go` +- focused launch/resume integration tests +- `apps/backend/internal/backendapp/orchestrator.go` +- `apps/backend/internal/backendapp/services_github_broker_test.go` +- this task file and `plan.md` + +## Dependencies + +Tasks 00 and 01. + +## Parallelism + +Parallel-safe with Task 04 after Task 01: backend orchestration/model files and frontend +settings files are disjoint. + +## Inputs + +- Spec `Task Git credential resolution`, snapshot data model, failure modes, and scenarios. +- Existing `resolveAllRepoInfo`, `persistLaunchState`, and `persistResumeState` paths. +- Task 01's policy/actor descriptor. +- Task 00's structured Git config composer; managed helper reset/additions must append to the + existing block rather than assigning an unrelated `GIT_CONFIG_COUNT`. + +## Output contract + +Report the exact precedence table, launch/resume call sites, snapshot JSON shape, durable-write +point, helper reset ordering, RED/GREEN results, files changed, and update task/plan status. diff --git a/docs/plans/task-git-credential-policy/task-03-managed-runtime-tools.md b/docs/plans/task-git-credential-policy/task-03-managed-runtime-tools.md new file mode 100644 index 0000000000..ecd52d3c1d --- /dev/null +++ b/docs/plans/task-git-credential-policy/task-03-managed-runtime-tools.md @@ -0,0 +1,63 @@ +--- +id: "03-managed-runtime-tools" +title: "Repair managed agentctl and gh runtime tools" +status: done +wave: 3 +depends_on: ["00-indexed-git-config-composition", "02-launch-resume-snapshot"] +plan: "plan.md" +spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 03: Repair Managed Agentctl And Gh Runtime Tools + +## Acceptance + +- A standalone control `agentctl` started without broker env exposes both managed `agentctl` and + `gh` to a later broker-enabled instance; managed Git and `gh` commands do not depend on the + backend shell PATH. +- The managed tool directory is not activated for executor inheritance or explicit profile-token + override instances, and a managed helper/broker failure does not fall back. +- Agentctl parent-plus-instance collection preserves host/executor indexed Git configuration and + emits an already-forwarded task suffix only once. +- Named GitHub CLI credentials are re-resolved after at most five minutes and invalidate + immediately on connection-generation changes. + +## Verification + +```bash +cd apps/backend && go test ./cmd/agentctl ./internal/agentctl/server/config ./internal/github -run 'Test.*(GitHubCLIShim|GitHubTool|CollectAgentEnv|CredentialResolver.*CLI|CredentialCache|IndexedGitConfig)' +``` + +## Files likely touched + +- `apps/backend/cmd/agentctl/main.go` +- `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/github/auth_resolver.go` +- `apps/backend/internal/github/auth_resolver_test.go` +- focused subprocess/fake-broker test helpers +- this task file and `plan.md` + +## Dependencies + +Tasks 00 and 02. + +## Parallelism + +Parallel-safe with Task 05: backend runtime/auth files and frontend Changes-panel files are +disjoint. + +## Inputs + +- Confirmed standalone root cause in `plan.md`. +- Spec failure/security scenarios and five-minute named-CLI refresh contract. +- Existing `installGitHubCLIShim`, `CollectAgentEnv`, and credential resolver cache patterns. +- Task 00's composition and overlap rules. + +## Output contract + +Report per-instance activation conditions, parent/instance Git-config composition evidence, actual +subprocess proof for `git` and `gh`, CLI cache deadline semantics, RED/GREEN results, files changed, +portability limits, and update task/plan status. diff --git a/docs/plans/task-git-credential-policy/task-04-settings-explanation.md b/docs/plans/task-git-credential-policy/task-04-settings-explanation.md new file mode 100644 index 0000000000..944cca0b7e --- /dev/null +++ b/docs/plans/task-git-credential-policy/task-04-settings-explanation.md @@ -0,0 +1,56 @@ +--- +id: "04-settings-explanation" +title: "Add task credential settings and method explanations" +status: done +wave: 2 +depends_on: ["01-policy-persistence"] +plan: "plan.md" +spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 04: Add Task Credential Settings And Method Explanations + +## Acceptance + +- Workspace GitHub settings load/save/discard the separate managed/executor task credential policy + through the shared settings-save coordinator. +- PAT, named CLI, and App choices visibly explain storage/resolution and managed task delivery; + policy copy visibly explains local/Worktree, remote executor, and profile-token precedence. +- Fine pointers get hover/focus help and coarse pointers get the same content in a 44px-target + Drawer without horizontal overflow. + +## Verification + +```bash +cd apps && pnpm --filter @kandev/web test -- --run components/github/github-task-credentials-section.test.tsx components/github/github-auth-method-list.test.tsx +cd apps/web && pnpm run typecheck +``` + +## Files likely touched + +- `apps/web/lib/types/github.ts` +- `apps/web/components/github/github-task-credentials-section.tsx` +- focused component/model tests beside it +- `apps/web/components/github/github-auth-method-list.tsx` +- `apps/web/components/github/github-connection-dialog.tsx` +- `apps/web/components/github/github-settings.tsx` +- this task file and `plan.md` + +## Dependencies + +Task 01. + +## Parallelism + +Parallel-safe with Task 02 after Task 01: it owns frontend settings files only. + +## Inputs + +- Spec `Choosing A Method`, `UX And Mobile Contract`, and policy scenarios. +- Task 01 workspace-settings API field. +- `GitHubRepoScopeSection` responsive information-help and save-contributor patterns. + +## Output contract + +Report visible option copy, help content, responsive primitive choice, save/discard behavior, +RED/GREEN/typecheck results, files changed, and update task/plan status. diff --git a/docs/plans/task-git-credential-policy/task-05-changes-identity-disclosure.md b/docs/plans/task-git-credential-policy/task-05-changes-identity-disclosure.md new file mode 100644 index 0000000000..30f99594e2 --- /dev/null +++ b/docs/plans/task-git-credential-policy/task-05-changes-identity-disclosure.md @@ -0,0 +1,57 @@ +--- +id: "05-changes-identity-disclosure" +title: "Show launch credential identity in the Changes branch disclosure" +status: done +wave: 3 +depends_on: ["02-launch-resume-snapshot"] +plan: "plan.md" +spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 05: Show Launch Credential Identity In The Changes Branch Disclosure + +## Acceptance + +- The active session's valid launch snapshot renders policy, effective method/source, known actor + or runtime-selected truth, transport, and executor alongside the existing branch comparison. +- Missing/malformed/legacy metadata degrades safely without inventing an actor or breaking branch + editing/base-branch controls. +- Fine-pointer desktop supports hover and keyboard focus; coarse-pointer/mobile uses the same + content in a Drawer with a 44px trigger. + +## Verification + +```bash +cd apps && pnpm --filter @kandev/web test -- --run components/task/changes-git-credential-display.test.ts +cd apps/web && pnpm run typecheck +``` + +## Files likely touched + +- `apps/web/components/task/changes-git-credential-display.ts` +- focused parser/view-model tests +- `apps/web/components/task/changes-panel-header.tsx` +- focused header component tests +- `apps/web/components/task/changes-panel-data.tsx` +- `apps/web/components/task/changes-panel.tsx` +- `apps/web/components/task/mobile/mobile-changes-panel.tsx` +- this task file and `plan.md` + +## Dependencies + +Task 02. + +## Parallelism + +Parallel-safe with Task 03: it owns frontend Changes-panel files only. + +## Inputs + +- Spec task-session snapshot and Changes-panel UX scenarios. +- Task 02 snapshot JSON contract. +- `useTouchDrawer` and existing responsive GitHub disclosure patterns. + +## Output contract + +Report snapshot parsing rules, exact displayed labels for every effective source, desktop/mobile +access behavior, RED/GREEN/typecheck results, files changed, and update task/plan status. diff --git a/docs/plans/task-git-credential-policy/task-06-e2e-and-documentation.md b/docs/plans/task-git-credential-policy/task-06-e2e-and-documentation.md new file mode 100644 index 0000000000..573fce6908 --- /dev/null +++ b/docs/plans/task-git-credential-policy/task-06-e2e-and-documentation.md @@ -0,0 +1,63 @@ +--- +id: "06-e2e-and-documentation" +title: "Prove credential policy UX and document the boundary" +status: done +wave: 4 +depends_on: + - "03-managed-runtime-tools" + - "04-settings-explanation" + - "05-changes-identity-disclosure" +plan: "plan.md" +spec: "../../specs/integrations/github-authentication.md" +--- + +# Task 06: Prove Credential Policy UX And Document The Boundary + +## Acceptance + +- Desktop and mobile E2E prove policy persistence/help plus managed and runtime-selected Changes + disclosures, including keyboard/touch access, 44px targets, and no horizontal overflow. +- Public GitHub/executor/getting-started documentation separates automation method from task + credential policy and gives accurate managed-versus-inherited troubleshooting. +- E2E runs use rebuilt production backend/Vite artifacts and record exact results. + +## Verification + +```bash +make -C apps/backend build +cd apps && pnpm --filter @kandev/web build:vite +cd apps/web && pnpm e2e:run --project chromium tests/integrations/github-workspace-settings.spec.ts tests/git/git-credential-identity.spec.ts +cd apps/web && pnpm e2e:run --project mobile-chrome tests/integrations/mobile-github-workspace-settings.spec.ts tests/git/mobile-git-credential-identity.spec.ts +``` + +## Files likely touched + +- `apps/web/e2e/tests/integrations/github-workspace-settings.spec.ts` +- `apps/web/e2e/tests/integrations/mobile-github-workspace-settings.spec.ts` +- `apps/web/e2e/tests/git/git-credential-identity.spec.ts` +- `apps/web/e2e/tests/git/mobile-git-credential-identity.spec.ts` +- `apps/web/e2e/pages/github-auth-settings-page.ts` +- `apps/web/e2e/helpers/api-client.ts` only if a typed snapshot seed helper is needed +- `docs/public/integrations.md` +- `docs/public/executors.md` +- `docs/public/use-kandev.md` +- this task file and `plan.md` + +## Dependencies + +Tasks 03, 04, and 05. + +## Parallelism + +Sequential. It integrates all backend/frontend behavior and owns production E2E evidence. + +## Inputs + +- All task-policy spec scenarios. +- Completed Task 03 runtime proof, Task 04 settings UI, and Task 05 Changes disclosure. +- Existing GitHub settings page object and `_test/task-sessions` metadata seeding. + +## Output contract + +Report the exact desktop/mobile E2E scenarios, build and test results, public-doc wording changes, +residual out-of-scope security limits, files changed, and update task/plan status. diff --git a/docs/public/executors.md b/docs/public/executors.md index 7a29ebbb43..1f384763aa 100644 --- a/docs/public/executors.md +++ b/docs/public/executors.md @@ -68,6 +68,14 @@ Two current preparation exceptions are easy to miss: ## Managed GitHub credentials +The workspace's GitHub connection and the task's Git transport policy are separate. **Managed +workspace credentials** is the default and provides the broker behavior below. Select **Inherit +executor Git credentials** in the workspace GitHub settings to leave Git and `gh` to the host or +selected executor; Kandev then injects no GitHub broker helper or shim. Local and Worktree use +host-visible Git/SSH credentials, while Docker, SSH, and cloud require executor-configured +credentials. An explicit executor-profile `GH_TOKEN` or `GITHUB_TOKEN` overrides the managed +workspace route. + For attached GitHub repositories, Kandev normally gives the task an opaque lease for each repository instead of placing the workspace PAT, selected CLI token, or App installation token in its ambient environment. Git's credential helper selects the lease whose HTTPS host and path diff --git a/docs/public/integrations.md b/docs/public/integrations.md index 223c227733..3602417192 100644 --- a/docs/public/integrations.md +++ b/docs/public/integrations.md @@ -92,6 +92,24 @@ backend actions for another attached repository. PAT/CLI `gh` commands still rec bearer grant described above. Explicit executor-profile tokens bypass these managed guarantees and must be scoped and rotated independently. +### Choose task Git credentials + +The **Task Git credentials** setting is separate from the workspace automation connection. It +controls how GitHub HTTPS and `gh` authenticate *inside newly launched task processes*: + +- **Managed workspace credentials** (the default) uses the selected workspace PAT, named GitHub + CLI account, or GitHub App through Kandev's short-lived, task/repository-scoped broker. The task + receives neither the stored PAT nor an App private key. An executor-profile `GH_TOKEN` or + `GITHUB_TOKEN` deliberately takes precedence for that task. +- **Inherit executor Git credentials** does not install Kandev's broker helper or `gh` shim. Local + and Worktree tasks use credentials already visible to the host Git process (including SSH). + Docker, SSH, and cloud tasks use only credentials intentionally configured in that executor. + +Selecting a GitHub CLI workspace connection does not implicitly select host Git credentials; use +the explicit inheritance mode when that is the intended boundary. The Changes panel's branch +details show the non-secret identity and delivery route captured for a successfully launched +session. + ### Use a GitHub App Choose a GitHub App when automation should not depend on one person's long-lived credential, when diff --git a/docs/public/use-kandev.md b/docs/public/use-kandev.md index 212482f831..b7efadf9e2 100644 --- a/docs/public/use-kandev.md +++ b/docs/public/use-kandev.md @@ -120,7 +120,7 @@ See [Sessions and review](sessions-and-review.md) for the workbench and [Tasks a - **Auto Approve**, permission-skipping flags, and unrestricted passthrough remove human gates inside the agent CLI. They are security decisions. - Local and worktree sessions run on the Kandev host. Worktree separation does not limit process, filesystem, network, or credential access. - Containers and remote executors change the boundary, but their mounted files, copied credentials, environment, daemon/socket access, and prepare scripts can reintroduce host or provider access. -- Managed GitHub task access uses task/repository-bound broker leases from the workspace automation connection. GitHub App tokens are minted for the redeemed repository; PAT and named-CLI bearer tokens retain all provider-granted scopes once delivered to the trusted agent subprocess. Personal tokens and the App private key never enter executors. +- Managed GitHub task access (the default) uses task/repository-bound broker leases from the workspace automation connection. GitHub App tokens are minted for the redeemed repository; PAT and named-CLI bearer tokens retain all provider-granted scopes once delivered to the trusted agent subprocess. Personal tokens and the App private key never enter executors. Choose **Inherit executor Git credentials** in the workspace GitHub settings only when task Git and `gh` should use host-visible credentials for Local/Worktree or intentionally configured credentials for remote executors. - A profile-supplied `GITHUB_TOKEN` or `GH_TOKEN` is an explicit unmanaged override and bypasses workspace broker selection. Ambient backend tokens and the host-active `gh` account are used only by migration-only **Legacy shared** workspace connections. - Repository, executor, and task action scripts are executable configuration. Review them like code. - The web app, HTTP API, WebSocket, and external MCP routes currently have no Kandev user-login boundary. Treat anyone who can reach the backend as an operator; keep the whole origin on loopback or a trusted network, or put it behind an authenticated TLS proxy. diff --git a/docs/specs/integrations/github-authentication.md b/docs/specs/integrations/github-authentication.md index 72c7504971..b100668bed 100644 --- a/docs/specs/integrations/github-authentication.md +++ b/docs/specs/integrations/github-authentication.md @@ -1,7 +1,7 @@ --- status: draft created: 2026-07-19 -amended: 2026-07-21 +amended: 2026-07-27 owner: Kandev --- @@ -43,14 +43,27 @@ automation under different GitHub Apps without operating separate Kandev deploym - User-triggered mutations prefer the workspace's verified personal connection, then a human PAT/CLI automation connection, then the App installation. The UI always identifies the effective actor and never labels an App mutation as human-attributed. -- Background watches, cleanup, repository discovery, workflow sync, managed agent git transport, - and agent-initiated GitHub operations always use the workspace automation connection. Personal - credentials are never exposed to agents or executors. -- Managed executors receive task/repository/generation-bound broker leases. App installation tokens - are minted for the requested repository and cached only in memory. PAT/CLI tokens retain their - provider-granted scope once delivered to a trusted agent subprocess. +- Background watches, cleanup, repository discovery, and workflow sync always use the workspace + automation connection. Personal credentials are never exposed to agents or executors. +- Task Git credential routing is a separate workspace policy. **Managed workspace credentials** + gives attached GitHub repositories task/repository/generation-bound broker leases from the + workspace automation connection. **Inherit executor Git credentials** injects no GitHub broker + helper or `gh` shim: Local and Worktree tasks use host-visible Git/SSH credentials, while remote + tasks use credentials configured in that executor. +- Under managed routing, App installation tokens are minted for the requested repository and cached + 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. +- Kandev composes the indexed `GIT_CONFIG_COUNT` / `GIT_CONFIG_KEY_` / + `GIT_CONFIG_VALUE_` protocol across host, executor, profile, task, and agentctl boundaries. + Unrelated entries such as hooks, notes, safe-directory, and URL-rewrite settings survive in their + original order; later Kandev credential entries affect only their intended GitHub credential + keys. An already-forwarded suffix is not duplicated. - Explicit executor-profile `GITHUB_TOKEN` or `GH_TOKEN` values remain unmanaged operator overrides - and take precedence over the workspace broker. + and take precedence over the workspace broker when managed routing is selected. +- Every successful task launch or resume records a non-secret session snapshot of the selected + policy, effective credential source, known method and actor, transport, executor, and capture + time. Inherited and profile-token actors are labeled runtime-selected instead of being probed. - The unpublished `KANDEV_GITHUB_APP_*` configuration introduced on this branch is removed. Setting those variables does not create or configure a registration; operators use the guided import flow for an App they already own. @@ -75,12 +88,18 @@ The workspace UI explains that an App is recommended for background jobs and man it does not claim agents can only use the selected method. Explicit executor credentials and other unmanaged tools remain outside Kandev's workspace credential contract. +The automation method and task Git credential policy are independent. Selecting a named `gh` +account does not inherit the host's Git configuration: in managed mode Kandev resolves that exact +host/login and brokers its token to the task. Users who want traditional host or executor behavior +select **Inherit executor Git credentials** explicitly. + ## Identity And Routing | Purpose | First choice | Fallback | Attribution | | --- | --- | --- | --- | | Background reads and writes | Workspace automation | None | Automation principal | -| Managed agent git and GitHub access | Workspace automation | None | Automation principal | +| Managed task git and `gh` access | Workspace automation | Explicit profile token | Automation principal, or runtime-selected override | +| Inherited task Git access | Executor-visible Git/SSH credentials | None | Runtime-selected | | `My GitHub` reads | Personal connection | Human PAT/CLI automation | Human principal | | User-triggered mutation | Personal connection | Human PAT/CLI, then App installation | Effective principal shown in UI | @@ -154,6 +173,34 @@ One row per configured workspace automation identity. Existing fields remain, wi For an App connection, `installation_id`, verified account login/type, and `app_registration_id` must all be present. PAT/CLI/legacy rows must have no registration ID. +### `github_workspace_settings` + +The non-secret operational settings row adds `task_git_credentials_mode`, with allowed values: + +| Value | Behavior | +| --- | --- | +| `managed` | Default. Inject the workspace broker contract for attached GitHub repositories unless an explicit executor-profile token overrides it. | +| `executor` | Inject no Kandev GitHub helper or `gh` shim; use credentials available where the selected executor runs. | + +Missing or invalid persisted values normalize to `managed`. Workspace-settings copy includes this +policy because it is operational configuration, not authentication material. + +### Task-session credential snapshot + +After a successful launch or resume, task-session metadata contains a versioned +`git_credential_snapshot` with: + +- selected policy (`managed` or `executor`); +- effective source (`workspace`, `executor_profile`, or `executor`); +- workspace method when known (`pat`, `gh_cli`, `github_app_installation`, or + `legacy_shared`); +- a known human/App actor label, or `runtime_selected` when Kandev does not inspect the credential; +- transport (`managed_https`, `profile_token`, or `executor_selected`), executor type, and capture + timestamp. + +The snapshot contains no token, lease, helper path, credential-file path, or SSH key detail. A +failed launch/resume does not replace the last successful snapshot. + ### `github_user_connections` One optional personal identity per `(workspace_id, user_id)`. Add required @@ -212,6 +259,10 @@ before exposing registration management. - `GET /api/v1/github/status?workspace_id=` returns automation, personal identity, effective actors, App registration metadata, capabilities, missing permissions, and migration state. +- `GET /api/v1/github/workspace-settings?workspace_id=` returns repository scope, saved + preferences, and `task_git_credentials_mode`. +- `PUT /api/v1/github/workspace-settings` accepts a partial + `task_git_credentials_mode` update and rejects unknown values without changing the prior policy. - `GET /api/v1/github/auth/gh-cli/accounts?workspace_id=` lists exact local host/login choices. - `PUT /api/v1/github/workspace-connection?workspace_id=` configures validated PAT or named CLI auth. App connections can only be committed by the verified installation callback. @@ -266,6 +317,17 @@ post-signature processing failures produce `failing`; a later valid successful d connections matching both registration ID and installation ID. - Disconnect removes workspace-owned secrets and leaves the reusable registration untouched. +### Task Git credential resolution + +- Missing settings start at `managed`. +- `managed + explicit profile GITHUB_TOKEN/GH_TOKEN -> executor_profile`. +- `managed + attached GitHub repository + active workspace connection -> workspace broker`. +- `executor -> executor-visible credentials`, regardless of PAT/CLI/App automation method. +- Initial launch and resume run the same resolution. A successful operation replaces the session + snapshot; a failed operation leaves the previous snapshot unchanged. +- Changing the workspace policy affects new launches and the next resume, not an already-running + process. + ## Permissions And Security - Registration list/create/import/delete requires registration-manager authority plus access to the @@ -279,12 +341,22 @@ post-signature processing failures produce `failing`; a later valid successful d - Runtime and token-cache keys include registration ID, registration generation, installation ID, workspace ID, and repository scope as applicable. No lookup may fall back to another registration or workspace. +- Named GitHub CLI bearer tokens remain memory-only and are re-resolved from the exact selected + host/login after at most five minutes. Connection-generation invalidation remains immediate. - Public base URL validation requires a canonical HTTPS origin with no credentials, query, or fragment and rejects loopback, private, link-local, or non-globally-routable DNS results. Kandev does not fetch the supplied URL as validation. - App private keys, client secrets, webhook secrets, personal tokens, and live installation tokens never enter executor environments. Only brokered PAT/CLI tokens or repository-restricted - installation tokens reach the trusted child operation. + installation tokens reach a managed trusted child operation; explicit executor-profile + `GITHUB_TOKEN`/`GH_TOKEN` values are an unmanaged exception and can reach that child instead. +- Managed helper configuration resets the inherited GitHub HTTPS helper chain, disables terminal + 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. +- Indexed Git configuration is validated and composed as a single ordered block at environment + merge boundaries. Kandev never replaces a complete inherited block merely by assigning its own + `GIT_CONFIG_COUNT`; managed helper reset semantics are expressed as later Git config entries. ## Failure Modes @@ -299,6 +371,13 @@ post-signature processing failures produce `failing`; a later valid successful d after structured status and multi-account flags. Genuine discovery failures are shown as errors, not as an empty account list. A CLI without named-token support may resolve only the active account; selecting another stored login fails with guidance to activate it or upgrade the CLI. +- If the managed `agentctl` helper, broker, or `gh` shim is unavailable, the command fails with a + managed-credential error and does not fall through to another HTTPS helper or interactive prompt. +- If any environment source supplies a malformed or unreasonably large indexed Git configuration, + task environment preparation fails with a sanitized configuration error rather than silently + truncating, partially merging, or executing a different block. +- If executor inheritance is selected but no usable credential exists in that executor, Git/SSH + reports its normal authentication failure. Kandev does not probe or guess the actor. - Deleting a registration with any workspace or personal reference returns `github_app_registration_in_use` with a non-secret binding count. - Changing workspace auth while a flow is open makes the stale callback fail without reverting the @@ -306,10 +385,11 @@ post-signature processing failures produce `failing`; a later valid successful d ## Persistence Guarantees -Registrations, workspace/personal bindings, credential generations, health, auth flows, and webhook -dedupe survive restart. Installation tokens, App JWTs, CLI-derived tokens, and broker lease plaintext -remain memory-only. Active encrypted-bundle pointers are crash consistent; orphan inactive bundles -are reconciled after restart. Restart rebuilds runtime clients independently for every valid stored +Registrations, workspace/personal bindings, task Git credential policy, credential generations, +health, auth flows, webhook dedupe, and successful task-session credential snapshots survive +restart. Installation tokens, App JWTs, CLI-derived tokens, and broker lease plaintext remain +memory-only. Active encrypted-bundle pointers are crash consistent; orphan inactive bundles are +reconciled after restart. Restart rebuilds runtime clients independently for every valid stored registration and never creates a global default. ## UX And Mobile Contract @@ -317,6 +397,11 @@ registration and never creates a global default. - Workspace GitHub settings lead with the active automation identity and a **Change connection** command. The method chooser uses a menu/list with PAT, GitHub CLI, and GitHub App descriptions, not a segmented tab control. +- Method descriptions state where the credential is stored/resolved and how managed tasks receive + it. A separate **Task Git credentials** setting visibly explains **Managed workspace + credentials** and **Inherit executor Git credentials**, including local/Worktree versus remote + behavior and explicit profile-token precedence. An information icon provides the complete + per-method delivery explanation as supplementary help. - GitHub App selection first explains when to use it and the sharing/isolation trade-off, then lists known registrations and actions to **Add existing App** or **Create new App**. - The import guide provides copyable callback, setup, and webhook URLs; required permissions/events; @@ -331,6 +416,10 @@ registration and never creates a global default. uses a single-column sheet/page, one scroll owner, safe-area padding, 44px targets, no fixed footer, and no horizontal overflow. External GitHub navigation is deliberate and returns to the same workspace settings route. +- The Changes panel branch disclosure includes the active session's launch-time Git credential + snapshot. Desktop supports hover and keyboard focus; coarse-pointer/mobile users open the same + information in a 44px-target drawer. Unknown inherited/profile actors are explicitly labeled + runtime-selected. ## Scenarios @@ -356,6 +445,26 @@ registration and never creates a global default. - **GIVEN** a PAT or named CLI workspace, **WHEN** an agent uses the managed credential helper, **THEN** it receives that workspace's automation token and the UI does not promise provider-side repository narrowing. +- **GIVEN** a named CLI workspace in managed mode, **WHEN** a task launches, **THEN** Kandev resolves + the selected host/login, makes both managed `git` and `gh` available in standalone and remote + runtimes, and does not depend on the host's currently active CLI account. +- **GIVEN** a workspace selects executor inheritance, **WHEN** a Local/Worktree or remote task + launches, **THEN** Kandev injects no broker helper/shim and the task uses host-visible or + executor-configured credentials respectively. +- **GIVEN** managed mode and an explicit executor-profile GitHub token, **WHEN** a task launches, + **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** 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 + Kandev entries afterward, and a real Git commit still runs the configured hook. +- **GIVEN** Docker or a remote control process already contains the same task Git config suffix, + **WHEN** that suffix is forwarded again while creating or configuring an agent instance, + **THEN** Kandev emits it once while retaining executor-added `safe.directory` and URL rewrites. +- **GIVEN** a workspace policy or automation connection changes after launch, **WHEN** the user + views the running session, **THEN** the Changes disclosure still shows its launch snapshot; a + successful resume records and shows the newly resolved contract. - **GIVEN** an authenticated host GitHub CLI without structured status or named-token flags, **WHEN** an operator selects its sole account, **THEN** Kandev discovers and validates that account without requiring a CLI upgrade. @@ -365,6 +474,9 @@ registration and never creates a global default. exposing the token. - **GIVEN** desktop and mobile viewports, **WHEN** users complete every App flow, **THEN** actions and disclosures remain usable without clipping, overlap, or desktop-only capability. +- **GIVEN** desktop fine-pointer and mobile coarse-pointer task views, **WHEN** the branch + disclosure is opened, **THEN** both show the same credential policy, method, actor truth, and + transport without horizontal overflow. ## Success Criteria @@ -374,6 +486,11 @@ registration and never creates a global default. - Secret scans find no PAT, private key, client secret, webhook secret, personal token, refresh token, or live installation token in logs, API snapshots, redirects, process arguments, or executor environments. +- Standalone, container, and remote task tests prove the managed helper is discoverable only for + broker-enabled instances, while executor inheritance receives no Kandev GitHub helper/shim. +- A real Git subprocess test proves that host/executor indexed hooks and notes config survive + managed credential injection, and focused tests prove ordered composition and overlap handling + across standalone, container, and remote launch shapes. ## Out Of Scope @@ -384,11 +501,19 @@ registration and never creates a global default. - GitHub Enterprise Server, enterprise-owned Apps, or hosts other than `github.com`. - Kandev multi-user login, workspace membership, or RBAC implementation. - Publishing Apps to GitHub Marketplace. +- Discovering or verifying the actor behind inherited credential managers, SSH agents, or explicit + profile tokens. +- Preventing a host-authority agent from manually selecting another Git transport outside + Kandev-injected managed HTTPS and `gh` commands. ## Implementation Plan -See [the implementation plan](../../plans/github-authentication/plan.md). +See [the original authentication implementation plan](../../plans/github-authentication/plan.md) +and the +[task Git credential policy follow-up plan](../../plans/task-git-credential-policy/plan.md). ## Decision -See [ADR-2026-07-21-workspace-selectable-github-app-registrations](../../decisions/2026-07-21-workspace-selectable-github-app-registrations.md). +See [ADR-2026-07-21-workspace-selectable-github-app-registrations](../../decisions/2026-07-21-workspace-selectable-github-app-registrations.md) +and +[ADR-2026-07-27-task-git-credential-policy](../../decisions/2026-07-27-task-git-credential-policy.md).