From e711e8d993f26550c7f1e118dd3aebb7422d1242 Mon Sep 17 00:00:00 2001 From: WaleWangPW <396218656@qq.com> Date: Fri, 21 Aug 2026 02:34:22 +0800 Subject: [PATCH] fix(credentials): reissue managed git leases (#2850) Co-authored-by: Carlos Florencio --- apps/backend/cmd/agentctl/github_cli_shim.go | 2 +- .../cmd/agentctl/github_cli_shim_test.go | 51 +++++ .../backend/cmd/agentctl/github_credential.go | 158 +++++++++++---- .../cmd/agentctl/github_credential_test.go | 148 +++++++++++++- .../runtime/lifecycle/remote_github_env.go | 1 + .../lifecycle/remote_github_env_test.go | 61 +++--- .../internal/auth/httpmw/middleware.go | 8 + .../internal/auth/httpmw/middleware_test.go | 1 + .../internal/backendapp/git_credentials.go | 7 +- .../internal/backendapp/orchestrator.go | 6 +- apps/backend/internal/backendapp/services.go | 2 +- .../backendapp/services_github_broker_test.go | 4 +- apps/backend/internal/common/config/config.go | 7 +- .../internal/common/config/config_test.go | 4 + .../backend/internal/gitcredentials/broker.go | 181 ++++++++++++++++-- .../internal/gitcredentials/reissue.go | 96 ++++++++++ .../internal/gitcredentials/reissue_test.go | 144 ++++++++++++++ apps/backend/internal/github/controller.go | 8 + .../internal/github/controller_auth.go | 100 ++++++++-- .../internal/github/credential_broker.go | 99 +++++++++- .../github/credential_broker_http_test.go | 98 +++++++++- .../internal/github/credential_broker_test.go | 40 ++++ .../internal/github/service_app_auth.go | 13 ++ .../internal/githubauth/environment.go | 27 +-- .../executor/executor_credentials.go | 57 ++++-- .../executor/executor_credentials_test.go | 36 ++++ docs/docker.md | 2 +- docs/k8s.md | 2 +- .../git-credential-lease-reissue/plan.md | 96 ++++++++++ .../task-01-capability-and-broker.md | 23 +++ .../task-02-executor-and-helper.md | 22 +++ .../task-03-regression-verification.md | 22 +++ docs/public/executors.md | 2 +- docs/specs/INDEX.md | 1 + .../git-credential-lease-reissue/spec.md | 125 ++++++++++++ 35 files changed, 1501 insertions(+), 153 deletions(-) create mode 100644 apps/backend/internal/gitcredentials/reissue.go create mode 100644 apps/backend/internal/gitcredentials/reissue_test.go create mode 100644 docs/plans/git-credential-lease-reissue/plan.md create mode 100644 docs/plans/git-credential-lease-reissue/task-01-capability-and-broker.md create mode 100644 docs/plans/git-credential-lease-reissue/task-02-executor-and-helper.md create mode 100644 docs/plans/git-credential-lease-reissue/task-03-regression-verification.md create mode 100644 docs/specs/git-credential-lease-reissue/spec.md diff --git a/apps/backend/cmd/agentctl/github_cli_shim.go b/apps/backend/cmd/agentctl/github_cli_shim.go index a59b66ace6..b5339256b4 100644 --- a/apps/backend/cmd/agentctl/github_cli_shim.go +++ b/apps/backend/cmd/agentctl/github_cli_shim.go @@ -48,7 +48,7 @@ func runGitHubCLIShim( if err != nil { return err } - credential, err := client.resolve(ctx) + credential, err := client.resolveWithReissue(ctx) if err != nil { return err } diff --git a/apps/backend/cmd/agentctl/github_cli_shim_test.go b/apps/backend/cmd/agentctl/github_cli_shim_test.go index 1e3a260e35..61fc161f95 100644 --- a/apps/backend/cmd/agentctl/github_cli_shim_test.go +++ b/apps/backend/cmd/agentctl/github_cli_shim_test.go @@ -223,6 +223,57 @@ func TestGitHubCLIShimRefreshesAndIsolatesEachInvocation(t *testing.T) { } } +func TestGitHubCLIShimReissuesInvalidLease(t *testing.T) { + var resolveCalls, reissueCalls int + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var request githubBrokerResolveRequest + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + t.Errorf("decode request: %v", err) + } + switch r.URL.Path { + case "/resolve": + resolveCalls++ + if resolveCalls == 1 { + w.WriteHeader(http.StatusUnauthorized) + _, _ = io.WriteString(w, `{"code":"github_credential_lease_revoked"}`) + return + } + if request.Lease != "replacement-lease" { + t.Errorf("replacement resolve lease = %q", request.Lease) + } + _, _ = io.WriteString(w, `{"username":"x-access-token","password":"fresh-token"}`) + case "/reissue": + reissueCalls++ + if request.Lease != "" || request.ReissueCapability != "execution-capability" { + t.Errorf("reissue request = %+v", request) + } + _, _ = io.WriteString(w, `{"token":"replacement-lease"}`) + default: + t.Errorf("unexpected endpoint %q", r.URL.Path) + } + })) + t.Cleanup(server.Close) + env := githubCredentialTestEnv(server.URL + "/resolve") + env[envGitHubCredentialReissueCapability] = "execution-capability" + env["PATH"] = "/shim:/usr/bin" + var childToken string + err := runGitHubCLIShim( + context.Background(), []string{"pr", "list"}, strings.NewReader(""), io.Discard, io.Discard, + lookupEnv(env), func() []string { return envMap(env) }, server.Client(), "/shim", + func(string, string) (string, error) { return "/usr/bin/gh", nil }, + func(_ context.Context, _ string, _ []string, childEnv []string, _ io.Reader, _, _ io.Writer) error { + childToken = envValue(childEnv, "GH_TOKEN") + return nil + }, + ) + if err != nil { + t.Fatalf("runGitHubCLIShim() error = %v", err) + } + if childToken != "fresh-token" || resolveCalls != 2 || reissueCalls != 1 { + t.Fatalf("child token/calls = %q/%d/%d, want fresh-token/2/1", childToken, resolveCalls, reissueCalls) + } +} + func TestGitHubCLIShimSelectsRepositoryLease(t *testing.T) { var got githubBrokerResolveRequest server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/apps/backend/cmd/agentctl/github_credential.go b/apps/backend/cmd/agentctl/github_credential.go index 4e9fb1ac9f..4c96eac965 100644 --- a/apps/backend/cmd/agentctl/github_credential.go +++ b/apps/backend/cmd/agentctl/github_credential.go @@ -5,6 +5,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -17,16 +18,17 @@ import ( ) type githubBrokerResolveRequest struct { - Lease string `json:"lease"` - TaskID string `json:"task_id"` - SessionID string `json:"session_id"` - RepositoryID string `json:"repository_id"` - Owner string `json:"owner"` - Repo string `json:"repo"` - Host string `json:"host"` - Path string `json:"path,omitempty"` - ProviderID string `json:"provider_id,omitempty"` - ParentProviderID string `json:"parent_provider_id,omitempty"` + Lease string `json:"lease,omitempty"` + ReissueCapability string `json:"reissue_capability,omitempty"` + TaskID string `json:"task_id"` + SessionID string `json:"session_id"` + RepositoryID string `json:"repository_id"` + Owner string `json:"owner"` + Repo string `json:"repo"` + Host string `json:"host"` + Path string `json:"path,omitempty"` + ProviderID string `json:"provider_id,omitempty"` + ParentProviderID string `json:"parent_provider_id,omitempty"` } type githubBrokerCredential struct { @@ -40,18 +42,29 @@ type githubCredentialBrokerClient struct { http *http.Client } +type githubBrokerHTTPError struct { + status int + code string + body string +} + +func (e *githubBrokerHTTPError) Error() string { + return fmt.Sprintf("broker returned HTTP %d%s", e.status, e.body) +} + func newGitHubCredentialBrokerClient( getenv func(string) string, httpClient *http.Client, ) (*githubCredentialBrokerClient, error) { request := githubBrokerResolveRequest{ - Lease: strings.TrimSpace(getenv(githubauth.CredentialLeaseEnv)), - TaskID: strings.TrimSpace(getenv(githubauth.CredentialTaskIDEnv)), - SessionID: strings.TrimSpace(getenv(githubauth.CredentialSessionIDEnv)), - RepositoryID: strings.TrimSpace(getenv(githubauth.CredentialRepositoryEnv)), - Owner: strings.TrimSpace(getenv(githubauth.CredentialOwnerEnv)), - Repo: strings.TrimSuffix(strings.TrimSpace(getenv(githubauth.CredentialRepoEnv)), ".git"), - Host: strings.ToLower(strings.TrimSpace(getenv(githubauth.CredentialHostEnv))), + Lease: strings.TrimSpace(getenv(githubauth.CredentialLeaseEnv)), + ReissueCapability: strings.TrimSpace(getenv(githubauth.CredentialReissueCapabilityEnv)), + TaskID: strings.TrimSpace(getenv(githubauth.CredentialTaskIDEnv)), + SessionID: strings.TrimSpace(getenv(githubauth.CredentialSessionIDEnv)), + RepositoryID: strings.TrimSpace(getenv(githubauth.CredentialRepositoryEnv)), + Owner: strings.TrimSpace(getenv(githubauth.CredentialOwnerEnv)), + Repo: strings.TrimSuffix(strings.TrimSpace(getenv(githubauth.CredentialRepoEnv)), ".git"), + Host: strings.ToLower(strings.TrimSpace(getenv(githubauth.CredentialHostEnv))), } endpoint := strings.TrimSpace(getenv(githubauth.CredentialBrokerURLEnv)) for name, value := range map[string]string{ @@ -102,26 +115,12 @@ func formatBrokerErrorBody(body io.Reader) string { } func (c *githubCredentialBrokerClient) resolve(ctx context.Context) (*githubBrokerCredential, error) { - body, err := json.Marshal(c.request) - if err != nil { - return nil, fmt.Errorf("encode GitHub credential request: %w", err) - } - req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.endpoint, bytes.NewReader(body)) - if err != nil { - return nil, fmt.Errorf("create GitHub credential request: %w", err) - } - req.Header.Set("Content-Type", "application/json") - resp, err := c.http.Do(req) + body, err := c.post(ctx, c.endpoint, c.request) if err != nil { return nil, fmt.Errorf("resolve GitHub credential: %w", err) } - defer func() { _ = resp.Body.Close() }() - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("resolve GitHub credential: broker returned HTTP %d%s", - resp.StatusCode, formatBrokerErrorBody(resp.Body)) - } var credential githubBrokerCredential - if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&credential); err != nil { + if err := json.Unmarshal(body, &credential); err != nil { return nil, fmt.Errorf("decode GitHub credential: %w", err) } if credential.Username == "" || credential.Password == "" { @@ -130,6 +129,97 @@ func (c *githubCredentialBrokerClient) resolve(ctx context.Context) (*githubBrok return &credential, nil } +func (c *githubCredentialBrokerClient) resolveWithReissue(ctx context.Context) (*githubBrokerCredential, error) { + credential, err := c.resolve(ctx) + if err == nil || !isReissuableLeaseError(err) || strings.TrimSpace(c.request.ReissueCapability) == "" { + return credential, err + } + if err := c.reissue(ctx); err != nil { + return nil, err + } + return c.resolve(ctx) +} + +func (c *githubCredentialBrokerClient) reissue(ctx context.Context) error { + endpoint, err := githubCredentialReissueEndpoint(c.endpoint) + if err != nil { + return err + } + request := c.request + request.Lease = "" + body, err := c.post(ctx, endpoint, request) + if err != nil { + return fmt.Errorf("reissue GitHub credential lease: %w", err) + } + var lease struct { + Token string `json:"token"` + } + if err := json.Unmarshal(body, &lease); err != nil || strings.TrimSpace(lease.Token) == "" { + return fmt.Errorf("reissue GitHub credential lease: broker returned an invalid lease") + } + c.request.Lease = lease.Token + return nil +} + +func (c *githubCredentialBrokerClient) post( + ctx context.Context, + endpoint string, + payload githubBrokerResolveRequest, +) ([]byte, error) { + body, err := json.Marshal(payload) + if err != nil { + return nil, fmt.Errorf("encode GitHub credential request: %w", err) + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(body)) + if err != nil { + return nil, fmt.Errorf("create GitHub credential request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + resp, err := c.http.Do(req) + if err != nil { + return nil, err + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode != http.StatusOK { + body := formatBrokerErrorBody(resp.Body) + return nil, &githubBrokerHTTPError{status: resp.StatusCode, code: brokerErrorCode(body), body: body} + } + return io.ReadAll(io.LimitReader(resp.Body, 1<<20)) +} + +func brokerErrorCode(body string) string { + var payload struct { + Code string `json:"code"` + } + if err := json.Unmarshal([]byte(strings.TrimPrefix(body, ": ")), &payload); err != nil { + return "" + } + return payload.Code +} + +func isReissuableLeaseError(err error) bool { + var brokerErr *githubBrokerHTTPError + if !errors.As(err, &brokerErr) || brokerErr.status != http.StatusUnauthorized { + return false + } + switch brokerErr.code { + case "github_credential_lease_invalid", "github_credential_lease_expired", "github_credential_lease_revoked", + "git_credential_lease_invalid", "git_credential_lease_expired", "git_credential_lease_revoked": + return true + default: + return false + } +} + +func githubCredentialReissueEndpoint(raw string) (string, error) { + endpoint, err := url.Parse(raw) + if err != nil || endpoint.Host == "" || !strings.HasSuffix(endpoint.Path, "/resolve") { + return "", fmt.Errorf("GitHub credential broker URL is invalid") + } + endpoint.Path = strings.TrimSuffix(endpoint.Path, "/resolve") + "/reissue" + return endpoint.String(), nil +} + func runGitHubCredentialHelper( ctx context.Context, args []string, @@ -162,7 +252,7 @@ func runGitHubCredentialHelper( return err } client.request = request - credential, err := client.resolve(ctx) + credential, err := client.resolveWithReissue(ctx) if err != nil { return err } diff --git a/apps/backend/cmd/agentctl/github_credential_test.go b/apps/backend/cmd/agentctl/github_credential_test.go index 7f349a4cf7..f69ccc732c 100644 --- a/apps/backend/cmd/agentctl/github_credential_test.go +++ b/apps/backend/cmd/agentctl/github_credential_test.go @@ -14,15 +14,16 @@ import ( ) const ( - envGitHubCredentialBrokerURL = githubauth.CredentialBrokerURLEnv - envGitHubCredentialLease = githubauth.CredentialLeaseEnv - envGitHubCredentialTaskID = githubauth.CredentialTaskIDEnv - envGitHubCredentialSessionID = githubauth.CredentialSessionIDEnv - envGitHubCredentialRepository = githubauth.CredentialRepositoryEnv - envGitHubCredentialOwner = githubauth.CredentialOwnerEnv - envGitHubCredentialRepo = githubauth.CredentialRepoEnv - envGitHubCredentialHost = githubauth.CredentialHostEnv - envGitHubCredentialScopes = githubauth.CredentialScopesEnv + envGitHubCredentialBrokerURL = githubauth.CredentialBrokerURLEnv + envGitHubCredentialLease = githubauth.CredentialLeaseEnv + envGitHubCredentialReissueCapability = githubauth.CredentialReissueCapabilityEnv + envGitHubCredentialTaskID = githubauth.CredentialTaskIDEnv + envGitHubCredentialSessionID = githubauth.CredentialSessionIDEnv + envGitHubCredentialRepository = githubauth.CredentialRepositoryEnv + envGitHubCredentialOwner = githubauth.CredentialOwnerEnv + envGitHubCredentialRepo = githubauth.CredentialRepoEnv + envGitHubCredentialHost = githubauth.CredentialHostEnv + envGitHubCredentialScopes = githubauth.CredentialScopesEnv ) func TestGitHubCredentialHelperGetsFreshCredential(t *testing.T) { @@ -63,6 +64,135 @@ func TestGitHubCredentialHelperGetsFreshCredential(t *testing.T) { } } +func TestGitHubCredentialHelperReissuesInvalidLeaseOnce(t *testing.T) { + var resolveCalls, reissueCalls int + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var request githubBrokerResolveRequest + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + t.Errorf("decode request: %v", err) + } + switch r.URL.Path { + case "/resolve": + resolveCalls++ + if resolveCalls == 1 { + w.WriteHeader(http.StatusUnauthorized) + _, _ = io.WriteString(w, `{"code":"github_credential_lease_revoked"}`) + return + } + if request.Lease != "replacement-lease" { + t.Errorf("replacement resolve lease = %q", request.Lease) + } + _, _ = io.WriteString(w, `{"username":"x-access-token","password":"fresh-token"}`) + case "/reissue": + reissueCalls++ + if request.Lease != "" { + t.Errorf("reissue used old lease %q", request.Lease) + } + if request.ReissueCapability != "execution-capability" { + t.Errorf("reissue capability = %q", request.ReissueCapability) + } + _, _ = io.WriteString(w, `{"token":"replacement-lease"}`) + default: + t.Errorf("unexpected endpoint %q", r.URL.Path) + } + })) + t.Cleanup(server.Close) + env := githubCredentialTestEnv(server.URL + "/resolve") + env[envGitHubCredentialReissueCapability] = "execution-capability" + + err := runGitHubCredentialHelper( + context.Background(), []string{"get"}, + strings.NewReader("protocol=https\nhost=github.com\npath=acme/widgets.git\n\n"), + io.Discard, lookupEnv(env), server.Client(), + ) + if err != nil { + t.Fatalf("runGitHubCredentialHelper() error = %v", err) + } + if resolveCalls != 2 || reissueCalls != 1 { + t.Fatalf("resolve/reissue calls = %d/%d, want 2/1", resolveCalls, reissueCalls) + } +} + +func TestGitHubCredentialHelperReissuesLeaseInMultiScopeMode(t *testing.T) { + var resolveCalls, reissueCalls int + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var request githubBrokerResolveRequest + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + t.Errorf("decode request: %v", err) + } + switch r.URL.Path { + case "/resolve": + resolveCalls++ + if resolveCalls == 1 { + w.WriteHeader(http.StatusUnauthorized) + _, _ = io.WriteString(w, `{"code":"github_credential_lease_revoked"}`) + return + } + if request.Lease != "replacement-lease" { + t.Errorf("replacement resolve lease = %q", request.Lease) + } + _, _ = io.WriteString(w, `{"username":"x-access-token","password":"fresh-token"}`) + case "/reissue": + reissueCalls++ + if request.Lease != "" || request.ReissueCapability != "execution-capability" { + t.Errorf("reissue request = %+v", request) + } + _, _ = io.WriteString(w, `{"token":"replacement-lease"}`) + default: + t.Errorf("unexpected endpoint %q", r.URL.Path) + } + })) + t.Cleanup(server.Close) + env := githubCredentialTestEnv(server.URL + "/resolve") + env[envGitHubCredentialScopes] = `[ + {"lease":"original-lease","reissue_capability":"execution-capability","task_id":"task-1","session_id":"session-1","repository_id":"repo-1","owner":"acme","repo":"widgets","host":"github.com"} + ]` + + err := runGitHubCredentialHelper( + context.Background(), []string{"get"}, + strings.NewReader("protocol=https\nhost=github.com\npath=acme/widgets.git\n\n"), + io.Discard, lookupEnv(env), server.Client(), + ) + if err != nil { + t.Fatalf("runGitHubCredentialHelper() error = %v", err) + } + if resolveCalls != 2 || reissueCalls != 1 { + t.Fatalf("resolve/reissue calls = %d/%d, want 2/1", resolveCalls, reissueCalls) + } +} + +func TestGitHubCredentialHelperStopsWhenReissueFails(t *testing.T) { + resolveCalls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/resolve": + resolveCalls++ + w.WriteHeader(http.StatusUnauthorized) + _, _ = io.WriteString(w, `{"code":"github_credential_lease_invalid"}`) + case "/reissue": + w.WriteHeader(http.StatusUnauthorized) + _, _ = io.WriteString(w, `{"code":"github_credential_reissue_denied"}`) + default: + t.Errorf("unexpected endpoint %q", r.URL.Path) + } + })) + t.Cleanup(server.Close) + env := githubCredentialTestEnv(server.URL + "/resolve") + env[envGitHubCredentialReissueCapability] = "execution-capability" + + err := runGitHubCredentialHelper( + context.Background(), []string{"get"}, + strings.NewReader("protocol=https\nhost=github.com\npath=acme/widgets.git\n\n"), + io.Discard, lookupEnv(env), server.Client(), + ) + if err == nil { + t.Fatal("runGitHubCredentialHelper() error = nil, want reissue failure") + } + if resolveCalls != 1 { + t.Fatalf("resolve calls = %d, want exactly 1 after reissue failure", resolveCalls) + } +} + func TestGitHubCredentialHelperRejectsRepositoryScopeMismatch(t *testing.T) { requests := 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { diff --git a/apps/backend/internal/agent/runtime/lifecycle/remote_github_env.go b/apps/backend/internal/agent/runtime/lifecycle/remote_github_env.go index 65cd3cc1bd..7c6336d8ce 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/remote_github_env.go +++ b/apps/backend/internal/agent/runtime/lifecycle/remote_github_env.go @@ -12,6 +12,7 @@ import ( var managedGitCredentialBrokerEnvKeys = []string{ githubauth.CredentialBrokerURLEnv, githubauth.CredentialLeaseEnv, + githubauth.CredentialReissueCapabilityEnv, githubauth.CredentialTaskIDEnv, githubauth.CredentialSessionIDEnv, githubauth.CredentialRepositoryEnv, diff --git a/apps/backend/internal/agent/runtime/lifecycle/remote_github_env_test.go b/apps/backend/internal/agent/runtime/lifecycle/remote_github_env_test.go index 9ebee36338..b29305d4c4 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/remote_github_env_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/remote_github_env_test.go @@ -8,45 +8,48 @@ import ( ) const ( - envKeyGitHubCredentialBrokerURL = githubauth.CredentialBrokerURLEnv - envKeyGitHubCredentialLease = githubauth.CredentialLeaseEnv - envKeyGitHubCredentialTaskID = githubauth.CredentialTaskIDEnv - envKeyGitHubCredentialSessionID = githubauth.CredentialSessionIDEnv - envKeyGitHubCredentialRepository = githubauth.CredentialRepositoryEnv - envKeyGitHubCredentialOwner = githubauth.CredentialOwnerEnv - envKeyGitHubCredentialRepo = githubauth.CredentialRepoEnv - envKeyGitHubCredentialHost = githubauth.CredentialHostEnv - envKeyGitHubCredentialScopes = githubauth.CredentialScopesEnv + envKeyGitHubCredentialBrokerURL = githubauth.CredentialBrokerURLEnv + envKeyGitHubCredentialLease = githubauth.CredentialLeaseEnv + envKeyGitHubCredentialReissueCapability = githubauth.CredentialReissueCapabilityEnv + envKeyGitHubCredentialTaskID = githubauth.CredentialTaskIDEnv + envKeyGitHubCredentialSessionID = githubauth.CredentialSessionIDEnv + envKeyGitHubCredentialRepository = githubauth.CredentialRepositoryEnv + envKeyGitHubCredentialOwner = githubauth.CredentialOwnerEnv + envKeyGitHubCredentialRepo = githubauth.CredentialRepoEnv + envKeyGitHubCredentialHost = githubauth.CredentialHostEnv + envKeyGitHubCredentialScopes = githubauth.CredentialScopesEnv ) func TestManagedGitHubBrokerEnvUsesStrictAllowlist(t *testing.T) { input := map[string]string{ - envKeyGitHubCredentialBrokerURL: "https://kandev.example/api/v1/github/credentials/resolve", - envKeyGitHubCredentialLease: "opaque-lease", - envKeyGitHubCredentialTaskID: "task-1", - envKeyGitHubCredentialSessionID: "session-1", - envKeyGitHubCredentialRepository: "repo-1", - envKeyGitHubCredentialOwner: "acme", - envKeyGitHubCredentialRepo: "widgets", - envKeyGitHubCredentialHost: "github.com", - envKeyGitHubCredentialScopes: `[{"lease":"opaque-lease"}]`, - "GIT_TERMINAL_PROMPT": "0", - "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", - "OPENAI_API_KEY": "must-not-reach-agentctl", - "GITHUB_TOKEN": "profile-token-must-not-reach-agentctl", - "HOME": "/control-plane/home", - "GIT_CONFIG_KEY_99": "credential.helper", - "GIT_CONFIG_VALUE_99": "!ambient-helper", + envKeyGitHubCredentialBrokerURL: "https://kandev.example/api/v1/github/credentials/resolve", + envKeyGitHubCredentialLease: "opaque-lease", + envKeyGitHubCredentialReissueCapability: "execution-capability", + envKeyGitHubCredentialTaskID: "task-1", + envKeyGitHubCredentialSessionID: "session-1", + envKeyGitHubCredentialRepository: "repo-1", + envKeyGitHubCredentialOwner: "acme", + envKeyGitHubCredentialRepo: "widgets", + envKeyGitHubCredentialHost: "github.com", + envKeyGitHubCredentialScopes: `[{"lease":"opaque-lease"}]`, + "GIT_TERMINAL_PROMPT": "0", + "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", + "OPENAI_API_KEY": "must-not-reach-agentctl", + "GITHUB_TOKEN": "profile-token-must-not-reach-agentctl", + "HOME": "/control-plane/home", + "GIT_CONFIG_KEY_99": "credential.helper", + "GIT_CONFIG_VALUE_99": "!ambient-helper", } got := managedGitHubBrokerEnv(input) for _, key := range []string{ envKeyGitHubCredentialBrokerURL, envKeyGitHubCredentialLease, + envKeyGitHubCredentialReissueCapability, envKeyGitHubCredentialScopes, "GIT_TERMINAL_PROMPT", "GIT_CONFIG_COUNT", diff --git a/apps/backend/internal/auth/httpmw/middleware.go b/apps/backend/internal/auth/httpmw/middleware.go index 7ea7d24caf..29f9ec7399 100644 --- a/apps/backend/internal/auth/httpmw/middleware.go +++ b/apps/backend/internal/auth/httpmw/middleware.go @@ -218,6 +218,14 @@ func isPublicPath(method, path string) bool { // readiness probe, POST redeems the lease; both self-authenticate // inside the handler, never off request identity. return method == http.MethodGet || method == http.MethodPost + case "/api/v1/github/credentials/reissue": + // The encrypted execution capability is the credential. The handler + // checks its expiry and exact task/session/repository scope. + return method == http.MethodPost + case "/api/v1/git/credentials/resolve": + return method == http.MethodGet || method == http.MethodPost + case "/api/v1/git/credentials/reissue": + return method == http.MethodPost } switch { case strings.HasPrefix(path, "/api/v1/automations/webhook/"): diff --git a/apps/backend/internal/auth/httpmw/middleware_test.go b/apps/backend/internal/auth/httpmw/middleware_test.go index b89722805d..6a84be6147 100644 --- a/apps/backend/internal/auth/httpmw/middleware_test.go +++ b/apps/backend/internal/auth/httpmw/middleware_test.go @@ -211,6 +211,7 @@ func TestEnabledModeAllowlistMatrix(t *testing.T) { {name: "github credential broker readiness", method: http.MethodGet, path: "/api/v1/github/credentials/resolve"}, {name: "github credential broker resolve", method: http.MethodPost, path: "/api/v1/github/credentials/resolve"}, + {name: "github credential broker reissue", method: http.MethodPost, path: "/api/v1/github/credentials/reissue"}, { name: "github app webhook", method: http.MethodPost, path: "/api/v1/github/app/registrations/reg1/webhook", diff --git a/apps/backend/internal/backendapp/git_credentials.go b/apps/backend/internal/backendapp/git_credentials.go index 1e43640d43..cfa658e110 100644 --- a/apps/backend/internal/backendapp/git_credentials.go +++ b/apps/backend/internal/backendapp/git_credentials.go @@ -32,6 +32,7 @@ func newGitCredentialBroker( githubSvc *githubpkg.Service, pluginSvc *plugins.Service, repo gitCredentialTaskRepository, + reissueSigningKey string, ) *gitcredentials.Broker { resolvers := make([]gitcredentials.Resolver, 0, 2) if githubSvc != nil { @@ -40,7 +41,11 @@ func newGitCredentialBroker( if pluginSvc != nil { resolvers = append(resolvers, pluginGitCredentialResolver{service: pluginCredentialServiceAdapter{service: pluginSvc}}) } - return gitcredentials.NewBroker(gitcredentials.NewCompositeResolver(resolvers...), &githubBrokerScopeAuthorizer{repo: repo}) + broker := gitcredentials.NewBroker(gitcredentials.NewCompositeResolver(resolvers...), &githubBrokerScopeAuthorizer{repo: repo}) + if signer, err := gitcredentials.NewReissueCapabilitySigner(reissueSigningKey); err == nil { + broker.SetReissueCapabilitySigner(signer) + } + return broker } // pluginGitCredentialResolver resolves only live, active manifest-declared diff --git a/apps/backend/internal/backendapp/orchestrator.go b/apps/backend/internal/backendapp/orchestrator.go index 7b6a5ba67a..e3968d5902 100644 --- a/apps/backend/internal/backendapp/orchestrator.go +++ b/apps/backend/internal/backendapp/orchestrator.go @@ -269,13 +269,13 @@ func (a githubExecutorCredentialPolicyAdapter) ResolveTaskGitCredentialPolicy( func githubCredentialBrokerEndpoint(cfg *config.Config) string { if cfg != nil { if publicBaseURL := strings.TrimRight(strings.TrimSpace(cfg.GitHubCredentialBroker.PublicBaseURL), "/"); publicBaseURL != "" { - return publicBaseURL + "/api/v1/github/credentials/resolve" + return publicBaseURL + "/api/v1/git/credentials/resolve" } if cfg.Server.Port != 0 { - return fmt.Sprintf("http://localhost:%d/api/v1/github/credentials/resolve", cfg.Server.Port) + return fmt.Sprintf("http://localhost:%d/api/v1/git/credentials/resolve", cfg.Server.Port) } } - return fmt.Sprintf("http://localhost:%d/api/v1/github/credentials/resolve", portsBackendDefault) + return fmt.Sprintf("http://localhost:%d/api/v1/git/credentials/resolve", portsBackendDefault) } // resolveQueueMaxPerSession honors the KANDEV_QUEUE_MAX_PER_SESSION env var, diff --git a/apps/backend/internal/backendapp/services.go b/apps/backend/internal/backendapp/services.go index 7b8023607a..bbf723d7c9 100644 --- a/apps/backend/internal/backendapp/services.go +++ b/apps/backend/internal/backendapp/services.go @@ -190,7 +190,7 @@ func provideServices(cfg *config.Config, log *logger.Logger, repos *Repositories pluginsSvc.SetKandevVersion(version) pluginsSvc.SetDataSources(taskSvc, taskSvc, workflowSvc, agentSettingsController, analyticsservice.New(repos.Analytics), taskSvc, pluginsTaskWriterAdapter{svc: taskSvc}) } - gitCredentialBroker := newGitCredentialBroker(githubSvc, pluginsSvc, repos.Task) + gitCredentialBroker := newGitCredentialBroker(githubSvc, pluginsSvc, repos.Task, cfg.GitHubCredentialBroker.ReissueSigningKey) if pluginsSvc != nil { pluginsSvc.SetGitCredentialLeaseRevoker(gitCredentialBroker.RevokeProvider) } diff --git a/apps/backend/internal/backendapp/services_github_broker_test.go b/apps/backend/internal/backendapp/services_github_broker_test.go index 64abf6adf3..5bbee21b7a 100644 --- a/apps/backend/internal/backendapp/services_github_broker_test.go +++ b/apps/backend/internal/backendapp/services_github_broker_test.go @@ -173,11 +173,11 @@ func TestGitHubCredentialBrokerEndpoint(t *testing.T) { dedicated := &config.Config{ GitHubCredentialBroker: config.GitHubCredentialBrokerConfig{PublicBaseURL: "https://broker.example/"}, } - if got, want := githubCredentialBrokerEndpoint(dedicated), "https://broker.example/api/v1/github/credentials/resolve"; got != want { + if got, want := githubCredentialBrokerEndpoint(dedicated), "https://broker.example/api/v1/git/credentials/resolve"; got != want { t.Fatalf("dedicated endpoint = %q, want %q", got, want) } local := &config.Config{Server: config.ServerConfig{Port: 49123}} - if got, want := githubCredentialBrokerEndpoint(local), "http://localhost:49123/api/v1/github/credentials/resolve"; got != want { + if got, want := githubCredentialBrokerEndpoint(local), "http://localhost:49123/api/v1/git/credentials/resolve"; got != want { t.Fatalf("local endpoint = %q, want %q", got, want) } } diff --git a/apps/backend/internal/common/config/config.go b/apps/backend/internal/common/config/config.go index 12b857d4c0..6ba8fdbc6f 100644 --- a/apps/backend/internal/common/config/config.go +++ b/apps/backend/internal/common/config/config.go @@ -51,7 +51,8 @@ type Config struct { // used by managed remote executors. It is independent of GitHub App setup so // PAT and named gh CLI workspaces can use remote executors. type GitHubCredentialBrokerConfig struct { - PublicBaseURL string `mapstructure:"publicBaseUrl" json:"-"` + PublicBaseURL string `mapstructure:"publicBaseUrl" json:"-"` + ReissueSigningKey string `mapstructure:"reissueSigningKey" json:"-"` } func (c GitHubCredentialBrokerConfig) validate() error { @@ -673,6 +674,10 @@ func LoadWithPath(configPath string) (*Config, error) { "githubCredentialBroker.publicBaseUrl", "KANDEV_GITHUB_CREDENTIAL_BROKER_PUBLIC_BASE_URL", ) + _ = v.BindEnv( + "githubCredentialBroker.reissueSigningKey", + "KANDEV_GITHUB_CREDENTIAL_BROKER_REISSUE_SIGNING_KEY", + ) // Configure config file v.SetConfigName("config") diff --git a/apps/backend/internal/common/config/config_test.go b/apps/backend/internal/common/config/config_test.go index 99b876a39f..52b72d5a31 100644 --- a/apps/backend/internal/common/config/config_test.go +++ b/apps/backend/internal/common/config/config_test.go @@ -23,6 +23,7 @@ func TestGitHubCredentialBrokerConfigValidation(t *testing.T) { func TestGitHubCredentialBrokerConfigEnvironmentBinding(t *testing.T) { t.Setenv("KANDEV_GITHUB_CREDENTIAL_BROKER_PUBLIC_BASE_URL", "https://kandev.example.com") + t.Setenv("KANDEV_GITHUB_CREDENTIAL_BROKER_REISSUE_SIGNING_KEY", "test-signing-key") cfg, err := LoadWithPath(t.TempDir()) if err != nil { t.Fatalf("LoadWithPath: %v", err) @@ -30,6 +31,9 @@ func TestGitHubCredentialBrokerConfigEnvironmentBinding(t *testing.T) { if got := cfg.GitHubCredentialBroker.PublicBaseURL; got != "https://kandev.example.com" { t.Fatalf("broker public base URL = %q", got) } + if got := cfg.GitHubCredentialBroker.ReissueSigningKey; got != "test-signing-key" { + t.Fatalf("broker reissue signing key was not bound") + } } func TestWebTitlePrefixEnvironmentBinding(t *testing.T) { diff --git a/apps/backend/internal/gitcredentials/broker.go b/apps/backend/internal/gitcredentials/broker.go index 3035e0b6b8..ac68959223 100644 --- a/apps/backend/internal/gitcredentials/broker.go +++ b/apps/backend/internal/gitcredentials/broker.go @@ -23,12 +23,15 @@ const ( ) var ( - ErrLeaseInvalid = errors.New("git credential lease is invalid") - ErrLeaseExpired = errors.New("git credential lease is expired") - ErrLeaseRevoked = errors.New("git credential lease was revoked") - ErrLeaseLimit = errors.New("git credential lease limit reached") - ErrScopeDenied = errors.New("git credential scope denied") - ErrUnsupported = errors.New("git credential provider is unsupported") + ErrLeaseInvalid = errors.New("git credential lease is invalid") + ErrLeaseExpired = errors.New("git credential lease is expired") + ErrLeaseRevoked = errors.New("git credential lease was revoked") + ErrLeaseLimit = errors.New("git credential lease limit reached") + ErrScopeDenied = errors.New("git credential scope denied") + ErrUnsupported = errors.New("git credential provider is unsupported") + ErrReissueUnavailable = errors.New("git credential lease reissue is unavailable") + ErrReissueCapabilityInvalid = errors.New("git credential reissue capability is invalid") + ErrReissueCapabilityExpired = errors.New("git credential reissue capability is expired") ) // Scope is the host-verified identity a lease may redeem for. Path is the @@ -70,6 +73,27 @@ type Lease struct { ExpiresAt time.Time } +// ReissueCapability is an opaque, execution-scoped bearer capability. It is +// distinct from a lease: a helper may use it only to obtain another lease for +// the exact scope it was launched with. +type ReissueCapability struct { + Token string + ExpiresAt time.Time +} + +// ReissueRequest carries the helper-supplied non-secret scope. Lease is +// intentionally absent: an invalid lease must never authenticate reissue. +type ReissueRequest struct { + Capability string + TaskID string + SessionID string + RepositoryID string + Host string + Path string + IdentityProviderID string + ParentProviderID string +} + // Credential is transient material returned only after a successful lease // redemption. Metadata is never retained in a broker lease record. type Credential struct { @@ -94,6 +118,13 @@ type Resolver interface { Resolve(context.Context, Scope) (Credential, error) } +// ScopeRefresher updates provider-owned binding data in a reissue scope while +// keeping the execution and repository identity unchanged. It is optional: +// providers without rotating binding data can reuse the sealed scope as-is. +type ScopeRefresher interface { + RefreshScope(context.Context, Scope) (Scope, error) +} + // CompositeResolver selects the first live resolver that declares a provider. // Resolvers may make their declaration dynamic, so disabled plugins stop // resolving immediately without a restart or cached credential. @@ -131,6 +162,18 @@ func (r *CompositeResolver) Resolve(ctx context.Context, scope Scope) (Credentia return resolver.Resolve(ctx, scope) } +func (r *CompositeResolver) RefreshScope(ctx context.Context, scope Scope) (Scope, error) { + resolver := r.resolver(scope.ProviderID) + if resolver == nil { + return Scope{}, ErrUnsupported + } + refresher, ok := resolver.(ScopeRefresher) + if !ok { + return scope, nil + } + return refresher.RefreshScope(ctx, scope) +} + func (r *CompositeResolver) resolver(providerID string) Resolver { if r == nil { return nil @@ -144,16 +187,19 @@ func (r *CompositeResolver) resolver(providerID string) Resolver { } type leaseRecord struct { - scope Scope - binding string - ttl time.Duration - expiresAt time.Time + scope Scope + binding string + ttl time.Duration + expiresAt time.Time + reissueKey [sha256.Size]byte + hasReissueKey bool } // Broker stores only hashed leases plus non-secret scope and revision data. type Broker struct { - resolver Resolver - authorizer Authorizer + resolver Resolver + authorizer Authorizer + reissueSigner *ReissueCapabilitySigner mu sync.Mutex leases map[[sha256.Size]byte]leaseRecord @@ -167,9 +213,102 @@ func NewBroker(resolver Resolver, authorizer Authorizer) *Broker { } } +// SetReissueCapabilitySigner enables restart-safe reissue. A nil signer +// leaves the existing lease-only behavior intact and fails reissue closed. +func (b *Broker) SetReissueCapabilitySigner(signer *ReissueCapabilitySigner) { + if b == nil { + return + } + b.mu.Lock() + defer b.mu.Unlock() + b.reissueSigner = signer +} + +// IssueWithReissueCapability creates a normal lease and, when configured, an +// opaque capability that may later replace that lease after a broker restart. +func (b *Broker) IssueWithReissueCapability(ctx context.Context, scope Scope) (Lease, ReissueCapability, error) { + if b == nil { + return Lease{}, ReissueCapability{}, ErrReissueUnavailable + } + b.mu.Lock() + hasSigner := b.reissueSigner != nil + b.mu.Unlock() + if !hasSigner { + return Lease{}, ReissueCapability{}, ErrReissueUnavailable + } + capability, err := b.IssueReissueCapability(scope) + if err != nil { + return Lease{}, ReissueCapability{}, err + } + reissueKey := sha256.Sum256([]byte(capability.Token)) + lease, err := b.issue(ctx, scope, &reissueKey) + if err != nil { + return Lease{}, ReissueCapability{}, err + } + return lease, capability, nil +} + +// IssueReissueCapability seals a normalized scope with the stable signer. +func (b *Broker) IssueReissueCapability(scope Scope) (ReissueCapability, error) { + normalized, err := normalizeScope(scope) + if err != nil { + return ReissueCapability{}, err + } + if b == nil { + return ReissueCapability{}, ErrReissueUnavailable + } + b.mu.Lock() + signer := b.reissueSigner + now := b.now + b.mu.Unlock() + if signer == nil { + return ReissueCapability{}, ErrReissueUnavailable + } + return signer.Issue(normalized, now()) +} + +// Reissue validates an execution capability against the exact helper scope, +// then performs normal issuance so live authorization and binding checks are +// never bypassed by a pre-restart capability. +func (b *Broker) Reissue(ctx context.Context, request ReissueRequest) (Lease, error) { + if b == nil { + return Lease{}, ErrUnsupported + } + b.mu.Lock() + signer := b.reissueSigner + now := b.now + b.mu.Unlock() + if signer == nil { + return Lease{}, ErrReissueUnavailable + } + scope, err := signer.Validate(request.Capability, now()) + if err != nil { + return Lease{}, err + } + if !matchesRedemption(scope, Redemption{ + TaskID: request.TaskID, SessionID: request.SessionID, RepositoryID: request.RepositoryID, + Host: request.Host, Path: request.Path, IdentityProviderID: request.IdentityProviderID, + ParentProviderID: request.ParentProviderID, + }) { + return Lease{}, ErrScopeDenied + } + if refresher, ok := b.resolver.(ScopeRefresher); ok { + scope, err = refresher.RefreshScope(ctx, scope) + if err != nil { + return Lease{}, err + } + } + reissueKey := sha256.Sum256([]byte(strings.TrimSpace(request.Capability))) + return b.issue(ctx, scope, &reissueKey) +} + // Issue creates a new opaque lease after validating scope ownership and the // live provider binding. It never resolves or stores a credential. func (b *Broker) Issue(ctx context.Context, scope Scope) (Lease, error) { + return b.issue(ctx, scope, nil) +} + +func (b *Broker) issue(ctx context.Context, scope Scope, reissueKey *[sha256.Size]byte) (Lease, error) { normalized, err := normalizeScope(scope) if err != nil { return Lease{}, err @@ -190,10 +329,10 @@ func (b *Broker) Issue(ctx context.Context, scope Scope) (Lease, error) { if strings.TrimSpace(binding) == "" { return Lease{}, ErrLeaseRevoked } - return b.storeLease(normalized, binding) + return b.storeLease(normalized, binding, reissueKey) } -func (b *Broker) storeLease(scope Scope, binding string) (Lease, error) { +func (b *Broker) storeLease(scope Scope, binding string, reissueKey *[sha256.Size]byte) (Lease, error) { raw := make([]byte, leaseTokenBytes) if _, err := rand.Read(raw); err != nil { return Lease{}, fmt.Errorf("create Git credential lease: %w", err) @@ -207,10 +346,22 @@ func (b *Broker) storeLease(scope Scope, binding string) (Lease, error) { b.mu.Lock() defer b.mu.Unlock() b.sweepExpiredLocked(now) + if reissueKey != nil { + for hash, record := range b.leases { + if record.hasReissueKey && record.reissueKey == *reissueKey { + delete(b.leases, hash) + } + } + } if b.workspaceLeaseCountLocked(scope.WorkspaceID) >= maxLeasesPerWorkspace { return Lease{}, fmt.Errorf("%w for workspace %s", ErrLeaseLimit, scope.WorkspaceID) } - b.leases[hash] = leaseRecord{scope: scope, binding: binding, ttl: ttl, expiresAt: expiresAt} + record := leaseRecord{scope: scope, binding: binding, ttl: ttl, expiresAt: expiresAt} + if reissueKey != nil { + record.reissueKey = *reissueKey + record.hasReissueKey = true + } + b.leases[hash] = record return Lease{Token: token, ExpiresAt: expiresAt}, nil } diff --git a/apps/backend/internal/gitcredentials/reissue.go b/apps/backend/internal/gitcredentials/reissue.go new file mode 100644 index 0000000000..f577b3a691 --- /dev/null +++ b/apps/backend/internal/gitcredentials/reissue.go @@ -0,0 +1,96 @@ +package gitcredentials + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/rand" + "crypto/sha256" + "encoding/base64" + "encoding/json" + "fmt" + "strings" + "time" +) + +const ( + reissueCapabilityTTL = 30 * 24 * time.Hour + reissueCapabilityAAD = "kandev.gitcredentials.reissue.v1" +) + +type reissueCapabilityClaims struct { + Scope Scope `json:"scope"` + ExpiresAt int64 `json:"expires_at"` +} + +// ReissueCapabilitySigner encrypts scope claims. Its key must remain stable +// across backend restarts; callers must not log, persist, or expose that key. +type ReissueCapabilitySigner struct { + key [sha256.Size]byte +} + +func NewReissueCapabilitySigner(key string) (*ReissueCapabilitySigner, error) { + key = strings.TrimSpace(key) + if key == "" { + return nil, ErrReissueUnavailable + } + return &ReissueCapabilitySigner{key: sha256.Sum256([]byte(reissueCapabilityAAD + ":" + key))}, nil +} + +func (s *ReissueCapabilitySigner) Issue(scope Scope, now time.Time) (ReissueCapability, error) { + if s == nil { + return ReissueCapability{}, ErrReissueUnavailable + } + expiresAt := now.UTC().Add(reissueCapabilityTTL) + payload, err := json.Marshal(reissueCapabilityClaims{Scope: scope, ExpiresAt: expiresAt.Unix()}) + if err != nil { + return ReissueCapability{}, fmt.Errorf("encode Git credential reissue capability: %w", err) + } + block, err := aes.NewCipher(s.key[:]) + if err != nil { + return ReissueCapability{}, fmt.Errorf("initialize Git credential reissue capability: %w", err) + } + gcm, err := cipher.NewGCM(block) + if err != nil { + return ReissueCapability{}, fmt.Errorf("initialize Git credential reissue capability: %w", err) + } + nonce := make([]byte, gcm.NonceSize()) + if _, err := rand.Read(nonce); err != nil { + return ReissueCapability{}, fmt.Errorf("create Git credential reissue capability: %w", err) + } + sealed := gcm.Seal(nil, nonce, payload, []byte(reissueCapabilityAAD)) + return ReissueCapability{Token: base64.RawURLEncoding.EncodeToString(append(nonce, sealed...)), ExpiresAt: expiresAt}, nil +} + +func (s *ReissueCapabilitySigner) Validate(raw string, now time.Time) (Scope, error) { + if s == nil || strings.TrimSpace(raw) == "" { + return Scope{}, ErrReissueCapabilityInvalid + } + encoded, err := base64.RawURLEncoding.DecodeString(raw) + if err != nil { + return Scope{}, ErrReissueCapabilityInvalid + } + block, err := aes.NewCipher(s.key[:]) + if err != nil { + return Scope{}, ErrReissueCapabilityInvalid + } + gcm, err := cipher.NewGCM(block) + if err != nil || len(encoded) < gcm.NonceSize() { + return Scope{}, ErrReissueCapabilityInvalid + } + payload, err := gcm.Open(nil, encoded[:gcm.NonceSize()], encoded[gcm.NonceSize():], []byte(reissueCapabilityAAD)) + if err != nil { + return Scope{}, ErrReissueCapabilityInvalid + } + var claims reissueCapabilityClaims + if err := json.Unmarshal(payload, &claims); err != nil || claims.ExpiresAt == 0 { + return Scope{}, ErrReissueCapabilityInvalid + } + if !time.Unix(claims.ExpiresAt, 0).After(now) { + return Scope{}, ErrReissueCapabilityExpired + } + scope, err := normalizeScope(claims.Scope) + if err != nil { + return Scope{}, ErrReissueCapabilityInvalid + } + return scope, nil +} diff --git a/apps/backend/internal/gitcredentials/reissue_test.go b/apps/backend/internal/gitcredentials/reissue_test.go new file mode 100644 index 0000000000..efeba99018 --- /dev/null +++ b/apps/backend/internal/gitcredentials/reissue_test.go @@ -0,0 +1,144 @@ +package gitcredentials + +import ( + "errors" + "testing" + "time" +) + +func TestBrokerReissueRestoresLeaseAfterBrokerRestart(t *testing.T) { + provider := &rotatingProvider{binding: "generation-1", username: "user", secret: "secret"} + authorizer := &recordingAuthorizer{} + signer, err := NewReissueCapabilitySigner("stable-test-key") + if err != nil { + t.Fatalf("NewReissueCapabilitySigner() error = %v", err) + } + beforeRestart := NewBroker(provider, authorizer) + beforeRestart.SetReissueCapabilitySigner(signer) + lease, capability, err := beforeRestart.IssueWithReissueCapability(t.Context(), testScope()) + if err != nil { + t.Fatalf("IssueWithReissueCapability() error = %v", err) + } + if _, err := beforeRestart.Redeem(t.Context(), testRedemption(lease.Token)); err != nil { + t.Fatalf("Redeem() before restart error = %v", err) + } + + afterRestart := NewBroker(provider, authorizer) + afterRestart.SetReissueCapabilitySigner(signer) + reissued, err := afterRestart.Reissue(t.Context(), ReissueRequest{ + Capability: capability.Token, TaskID: "task-1", SessionID: "session-1", RepositoryID: "repository-1", + Host: "bitbucket.example", Path: "/scm/ENG/widgets.git", + }) + if err != nil { + t.Fatalf("Reissue() after restart error = %v", err) + } + if reissued.Token == "" || reissued.Token == lease.Token { + t.Fatalf("reissued lease = %#v, original = %#v", reissued, lease) + } + if _, err := afterRestart.Redeem(t.Context(), testRedemption(reissued.Token)); err != nil { + t.Fatalf("Redeem() reissued lease error = %v", err) + } +} + +func TestBrokerReissueRecoversRotatedBinding(t *testing.T) { + provider := &rotatingProvider{binding: "generation-1", username: "user", secret: "secret"} + broker := NewBroker(provider, &recordingAuthorizer{}) + signer, err := NewReissueCapabilitySigner("stable-test-key") + if err != nil { + t.Fatalf("NewReissueCapabilitySigner() error = %v", err) + } + broker.SetReissueCapabilitySigner(signer) + lease, capability, err := broker.IssueWithReissueCapability(t.Context(), testScope()) + if err != nil { + t.Fatalf("IssueWithReissueCapability() error = %v", err) + } + provider.mu.Lock() + provider.binding = "generation-2" + provider.mu.Unlock() + if _, err := broker.Redeem(t.Context(), testRedemption(lease.Token)); !errors.Is(err, ErrLeaseRevoked) { + t.Fatalf("Redeem() after rotation error = %v, want revoked lease", err) + } + reissued, err := broker.Reissue(t.Context(), ReissueRequest{ + Capability: capability.Token, TaskID: "task-1", SessionID: "session-1", RepositoryID: "repository-1", + Host: "bitbucket.example", Path: "/scm/ENG/widgets.git", + }) + if err != nil { + t.Fatalf("Reissue() after rotation error = %v", err) + } + if _, err := broker.Redeem(t.Context(), testRedemption(reissued.Token)); err != nil { + t.Fatalf("Redeem() reissued lease error = %v", err) + } +} + +func TestBrokerReissueReplacesPreviousCapabilityLease(t *testing.T) { + provider := &rotatingProvider{binding: "generation-1", username: "user", secret: "secret"} + broker := NewBroker(provider, &recordingAuthorizer{}) + signer, err := NewReissueCapabilitySigner("stable-test-key") + if err != nil { + t.Fatalf("NewReissueCapabilitySigner() error = %v", err) + } + broker.SetReissueCapabilitySigner(signer) + _, capability, err := broker.IssueWithReissueCapability(t.Context(), testScope()) + if err != nil { + t.Fatalf("IssueWithReissueCapability() error = %v", err) + } + request := ReissueRequest{ + Capability: capability.Token, TaskID: "task-1", SessionID: "session-1", RepositoryID: "repository-1", + Host: "bitbucket.example", Path: "/scm/ENG/widgets.git", + } + first, err := broker.Reissue(t.Context(), request) + if err != nil { + t.Fatalf("first Reissue() error = %v", err) + } + second, err := broker.Reissue(t.Context(), request) + if err != nil { + t.Fatalf("second Reissue() error = %v", err) + } + if first.Token == second.Token { + t.Fatal("two reissues returned the same lease token") + } + if got, want := broker.ActiveLeaseCount(), 1; got != want { + t.Fatalf("active leases after repeated reissue = %d, want %d", got, want) + } + if _, err := broker.Redeem(t.Context(), testRedemption(first.Token)); !errors.Is(err, ErrLeaseInvalid) { + t.Fatalf("Redeem(first replacement) error = %v, want invalid lease", err) + } + if _, err := broker.Redeem(t.Context(), testRedemption(second.Token)); err != nil { + t.Fatalf("Redeem(second replacement) error = %v", err) + } +} + +func TestBrokerReissueRejectsForgedExpiredAndMismatchedCapabilities(t *testing.T) { + provider := &rotatingProvider{binding: "generation-1", username: "user", secret: "secret"} + broker := NewBroker(provider, &recordingAuthorizer{}) + signer, err := NewReissueCapabilitySigner("stable-test-key") + if err != nil { + t.Fatalf("NewReissueCapabilitySigner() error = %v", err) + } + broker.SetReissueCapabilitySigner(signer) + _, capability, err := broker.IssueWithReissueCapability(t.Context(), testScope()) + if err != nil { + t.Fatalf("IssueWithReissueCapability() error = %v", err) + } + request := ReissueRequest{Capability: capability.Token, TaskID: "task-1", SessionID: "session-1", RepositoryID: "repository-1", Host: "bitbucket.example", Path: "/scm/ENG/widgets.git"} + + request.Capability += "forged" + if _, err := broker.Reissue(t.Context(), request); !errors.Is(err, ErrReissueCapabilityInvalid) { + t.Fatalf("forged Reissue() error = %v, want invalid capability", err) + } + request.Capability = capability.Token + request.RepositoryID = "other-repository" + if _, err := broker.Reissue(t.Context(), request); !errors.Is(err, ErrScopeDenied) { + t.Fatalf("mismatched Reissue() error = %v, want scope denied", err) + } + + expired, err := signer.Issue(testScope(), time.Now().Add(-reissueCapabilityTTL-time.Second)) + if err != nil { + t.Fatalf("Issue() expired capability error = %v", err) + } + request.Capability = expired.Token + request.RepositoryID = "repository-1" + if _, err := broker.Reissue(t.Context(), request); !errors.Is(err, ErrReissueCapabilityExpired) { + t.Fatalf("expired Reissue() error = %v, want expired capability", err) + } +} diff --git a/apps/backend/internal/github/controller.go b/apps/backend/internal/github/controller.go index b65db9ccd7..1ce8f69862 100644 --- a/apps/backend/internal/github/controller.go +++ b/apps/backend/internal/github/controller.go @@ -68,6 +68,14 @@ func (c *Controller) RegisterHTTPRoutes(router *gin.Engine) { api.DELETE("/personal-connection", c.httpDisconnectPersonalAuth) api.GET("/credentials/resolve", c.httpCredentialBrokerReady) api.POST("/credentials/resolve", c.httpResolveCredentialLease) + api.POST("/credentials/reissue", c.httpReissueCredentialLease) + + // Git credential broker routes are provider-neutral. Keep the GitHub paths + // above as compatibility aliases for existing executors and integrations. + gitAPI := router.Group("/api/v1/git") + gitAPI.GET("/credentials/resolve", c.httpCredentialBrokerReady) + gitAPI.POST("/credentials/resolve", c.httpResolveCredentialLease) + gitAPI.POST("/credentials/reissue", c.httpReissueCredentialLease) api.GET("/task-prs", c.httpListTaskPRs) api.POST("/task-prs", c.httpCreateTaskPR) diff --git a/apps/backend/internal/github/controller_auth.go b/apps/backend/internal/github/controller_auth.go index 3bf56d4fe8..a3818873ee 100644 --- a/apps/backend/internal/github/controller_auth.go +++ b/apps/backend/internal/github/controller_auth.go @@ -240,21 +240,32 @@ func (c *Controller) httpGitHubAppWebhook(ctx *gin.Context) { ctx.JSON(http.StatusOK, result) } -func (c *Controller) httpResolveCredentialLease(ctx *gin.Context) { - var request struct { - Lease string `json:"lease"` - TaskID string `json:"task_id"` - SessionID string `json:"session_id"` - RepositoryID string `json:"repository_id"` - Owner string `json:"owner"` - Repo string `json:"repo"` - Host string `json:"host"` - Path string `json:"path"` - ProviderID string `json:"provider_id"` - ParentProviderID string `json:"parent_provider_id"` - } +type credentialBrokerHTTPRequest struct { + Lease string `json:"lease"` + Capability string `json:"capability"` + TaskID string `json:"task_id"` + SessionID string `json:"session_id"` + RepositoryID string `json:"repository_id"` + Owner string `json:"owner"` + Repo string `json:"repo"` + Host string `json:"host"` + Path string `json:"path"` + ProviderID string `json:"provider_id"` + ParentProviderID string `json:"parent_provider_id"` +} + +func decodeCredentialBrokerHTTPRequest(ctx *gin.Context) (credentialBrokerHTTPRequest, bool) { + var request credentialBrokerHTTPRequest if err := ctx.ShouldBindJSON(&request); err != nil { - ctx.JSON(http.StatusBadRequest, gin.H{"code": "github_invalid_request", "error": "invalid payload"}) + ctx.JSON(http.StatusBadRequest, gin.H{"code": credentialBrokerInvalidRequestCode(ctx), "error": "invalid payload"}) + return credentialBrokerHTTPRequest{}, false + } + return request, true +} + +func (c *Controller) httpResolveCredentialLease(ctx *gin.Context) { + request, ok := decodeCredentialBrokerHTTPRequest(ctx) + if !ok { return } credential, err := c.service.ResolveGitHubCredential(ctx.Request.Context(), BrokerCredentialRequest{ @@ -270,6 +281,25 @@ func (c *Controller) httpResolveCredentialLease(ctx *gin.Context) { ctx.JSON(http.StatusOK, credential) } +func (c *Controller) httpReissueCredentialLease(ctx *gin.Context) { + request, ok := decodeCredentialBrokerHTTPRequest(ctx) + if !ok { + return + } + lease, err := c.service.ReissueGitHubCredentialLease(ctx.Request.Context(), CredentialLeaseReissueRequest{ + Capability: request.Capability, TaskID: request.TaskID, SessionID: request.SessionID, + RepositoryID: request.RepositoryID, Owner: request.Owner, Repo: request.Repo, + Host: request.Host, Path: request.Path, ProviderID: request.ProviderID, + ParentProviderID: request.ParentProviderID, + }) + if err != nil { + writeGitHubAuthError(ctx, err) + return + } + ctx.Header("Cache-Control", "no-store") + ctx.JSON(http.StatusOK, lease) +} + func (c *Controller) httpCredentialBrokerReady(ctx *gin.Context) { if c == nil || c.service == nil || !c.service.CredentialBrokerReady() { ctx.Status(http.StatusServiceUnavailable) @@ -281,6 +311,9 @@ func (c *Controller) httpCredentialBrokerReady(ctx *gin.Context) { func writeGitHubAuthError(ctx *gin.Context, err error) { status, code := githubAuthErrorResponse(err) + if isGenericCredentialBrokerPath(ctx) { + code = genericCredentialBrokerErrorCode(err) + } message := strings.TrimSpace(err.Error()) payload := gin.H{"code": code, "error": message} var importError *AppRegistrationImportError @@ -295,6 +328,34 @@ func writeGitHubAuthError(ctx *gin.Context, err error) { ctx.JSON(status, payload) } +func isGenericCredentialBrokerPath(ctx *gin.Context) bool { + return ctx != nil && strings.HasPrefix(ctx.Request.URL.Path, "/api/v1/git/credentials/") +} + +func credentialBrokerInvalidRequestCode(ctx *gin.Context) string { + if isGenericCredentialBrokerPath(ctx) { + return "git_credential_invalid_request" + } + return "github_invalid_request" +} + +func genericCredentialBrokerErrorCode(err error) string { + switch { + case errors.Is(err, ErrCredentialLeaseInvalid): + return "git_credential_lease_invalid" + case errors.Is(err, ErrCredentialLeaseExpired): + return "git_credential_lease_expired" + case errors.Is(err, ErrCredentialLeaseRevoked): + return "git_credential_lease_revoked" + case errors.Is(err, ErrCredentialReissueCapabilityInvalid), errors.Is(err, ErrCredentialReissueCapabilityExpired): + return "git_credential_reissue_denied" + case errors.Is(err, ErrCredentialScopeDenied): + return "git_credential_denied" + default: + return "git_credential_internal_error" + } +} + func githubAuthErrorCode(err error) string { _, code := githubAuthErrorResponse(err) return code @@ -325,8 +386,15 @@ func githubAuthErrorResponse(err error) (int, string) { status, code = http.StatusBadRequest, "github_invalid_callback" case errors.Is(err, ErrInvalidWebhookSignature): status, code = http.StatusUnauthorized, "github_invalid_webhook_signature" - case errors.Is(err, ErrCredentialLeaseInvalid), errors.Is(err, ErrCredentialLeaseExpired), - errors.Is(err, ErrCredentialLeaseRevoked), errors.Is(err, ErrCredentialScopeDenied): + case errors.Is(err, ErrCredentialLeaseInvalid): + status, code = http.StatusUnauthorized, "github_credential_lease_invalid" + case errors.Is(err, ErrCredentialLeaseExpired): + status, code = http.StatusUnauthorized, "github_credential_lease_expired" + case errors.Is(err, ErrCredentialLeaseRevoked): + status, code = http.StatusUnauthorized, "github_credential_lease_revoked" + case errors.Is(err, ErrCredentialReissueCapabilityInvalid), errors.Is(err, ErrCredentialReissueCapabilityExpired): + status, code = http.StatusUnauthorized, "github_credential_reissue_denied" + case errors.Is(err, ErrCredentialScopeDenied): status, code = http.StatusUnauthorized, "github_credential_denied" case errors.Is(err, ErrInvalidToken): status, code = http.StatusBadRequest, "github_invalid_token" diff --git a/apps/backend/internal/github/credential_broker.go b/apps/backend/internal/github/credential_broker.go index e2898c9ff8..10913f6371 100644 --- a/apps/backend/internal/github/credential_broker.go +++ b/apps/backend/internal/github/credential_broker.go @@ -14,11 +14,13 @@ import ( const gitHubTokenUsername = "x-access-token" var ( - ErrCredentialLeaseInvalid = gitcredentials.ErrLeaseInvalid - ErrCredentialLeaseExpired = gitcredentials.ErrLeaseExpired - ErrCredentialLeaseRevoked = gitcredentials.ErrLeaseRevoked - ErrCredentialLeaseLimit = gitcredentials.ErrLeaseLimit - ErrCredentialScopeDenied = gitcredentials.ErrScopeDenied + ErrCredentialLeaseInvalid = gitcredentials.ErrLeaseInvalid + ErrCredentialLeaseExpired = gitcredentials.ErrLeaseExpired + ErrCredentialLeaseRevoked = gitcredentials.ErrLeaseRevoked + ErrCredentialLeaseLimit = gitcredentials.ErrLeaseLimit + ErrCredentialScopeDenied = gitcredentials.ErrScopeDenied + ErrCredentialReissueCapabilityInvalid = gitcredentials.ErrReissueCapabilityInvalid + ErrCredentialReissueCapabilityExpired = gitcredentials.ErrReissueCapabilityExpired ) // BrokerScopeAuthorizer verifies task/workspace/repository ownership. It is @@ -73,6 +75,21 @@ type BrokerCredentialRequest struct { ParentProviderID string } +// CredentialLeaseReissueRequest is authorized only by its opaque execution +// capability; it deliberately has no lease field. +type CredentialLeaseReissueRequest struct { + Capability string + TaskID string + SessionID string + RepositoryID string + Owner string + Repo string + Host string + Path string + ProviderID string + ParentProviderID string +} + type BrokerCredential struct { Username string `json:"username"` Password string `json:"password"` @@ -156,6 +173,25 @@ func (b *CredentialBroker) Resolve(ctx context.Context, req BrokerCredentialRequ }, nil } +func (b *CredentialBroker) Reissue(ctx context.Context, req CredentialLeaseReissueRequest) (*CredentialLease, error) { + if b == nil || b.broker == nil { + return nil, ErrGitHubNotConfigured + } + path := req.Path + if strings.TrimSpace(path) == "" { + path = githubCredentialPath(req.Owner, req.Repo) + } + lease, err := b.broker.Reissue(ctx, gitcredentials.ReissueRequest{ + Capability: req.Capability, TaskID: req.TaskID, SessionID: req.SessionID, + RepositoryID: req.RepositoryID, Host: req.Host, Path: path, + IdentityProviderID: req.ProviderID, ParentProviderID: req.ParentProviderID, + }) + if err != nil { + return nil, err + } + return &CredentialLease{Token: lease.Token, ExpiresAt: lease.ExpiresAt}, nil +} + func (b *CredentialBroker) RevokeTask(taskID string) { if b != nil && b.broker != nil { b.broker.RevokeTask(taskID) @@ -231,6 +267,40 @@ func (p *gitHubCredentialProvider) Binding(ctx context.Context, scope gitcredent return githubCredentialBindingFor(connection, appGeneration) } +func (p *gitHubCredentialProvider) RefreshScope(ctx context.Context, scope gitcredentials.Scope) (gitcredentials.Scope, error) { + rawBinding := strings.TrimSpace(scope.CredentialBinding) + if rawBinding == "" { + return scope, nil + } + var binding taskmodels.ContributionDestinationCredentialBinding + if err := json.Unmarshal([]byte(rawBinding), &binding); err != nil { + return gitcredentials.Scope{}, fmt.Errorf("%w: invalid destination credential binding", ErrCredentialScopeDenied) + } + if err := binding.Validate(); err != nil { + return gitcredentials.Scope{}, fmt.Errorf("%w: invalid destination credential binding", ErrCredentialScopeDenied) + } + connection, appGeneration, err := p.issueConnection(ctx, scope.WorkspaceID) + if err != nil { + return gitcredentials.Scope{}, err + } + if !destinationBindingIdentityMatchesConnection(&binding, connection) { + return gitcredentials.Scope{}, fmt.Errorf("%w: destination credential binding does not match the active workspace connection", ErrCredentialScopeDenied) + } + binding.CredentialGeneration = connection.CredentialGeneration + if connection.Source == ConnectionSourceGitHubAppInstallation { + binding.AppCredentialGeneration = appGeneration + } else { + binding.AppCredentialGeneration = 0 + binding.InstallationID = 0 + } + encoded, err := json.Marshal(binding) + if err != nil { + return gitcredentials.Scope{}, fmt.Errorf("encode contribution destination credential binding: %w", err) + } + scope.CredentialBinding = string(encoded) + return scope, nil +} + func (p *gitHubCredentialProvider) Resolve(ctx context.Context, scope gitcredentials.Scope) (gitcredentials.Credential, error) { if p == nil || p.resolver == nil { return gitcredentials.Credential{}, ErrGitHubNotConfigured @@ -321,9 +391,8 @@ func destinationBindingMatchesConnection( connection *WorkspaceConnection, appCredentialGeneration int64, ) bool { - if binding == nil || connection == nil || binding.Source != string(connection.Source) || - binding.CredentialGeneration != connection.CredentialGeneration || - binding.AppRegistrationID != connection.AppRegistrationID { + if !destinationBindingIdentityMatchesConnection(binding, connection) || + binding.CredentialGeneration != connection.CredentialGeneration { return false } if connection.Source == ConnectionSourceGitHubAppInstallation { @@ -335,3 +404,17 @@ func destinationBindingMatchesConnection( } return binding.AppCredentialGeneration == 0 && binding.InstallationID == 0 } + +func destinationBindingIdentityMatchesConnection( + binding *taskmodels.ContributionDestinationCredentialBinding, + connection *WorkspaceConnection, +) bool { + if binding == nil || connection == nil || binding.Source != string(connection.Source) || + binding.AppRegistrationID != connection.AppRegistrationID { + return false + } + if connection.Source == ConnectionSourceGitHubAppInstallation { + return connection.InstallationID != nil && binding.InstallationID == *connection.InstallationID + } + return binding.Login == "" || strings.EqualFold(binding.Login, connection.Login) +} diff --git a/apps/backend/internal/github/credential_broker_http_test.go b/apps/backend/internal/github/credential_broker_http_test.go index 0e6cb93a98..dfeb0baa46 100644 --- a/apps/backend/internal/github/credential_broker_http_test.go +++ b/apps/backend/internal/github/credential_broker_http_test.go @@ -16,6 +16,7 @@ import ( "github.com/kandev/kandev/internal/auth/httpmw" authstore "github.com/kandev/kandev/internal/auth/store" "github.com/kandev/kandev/internal/common/config" + "github.com/kandev/kandev/internal/gitcredentials" userstore "github.com/kandev/kandev/internal/user/store" ) @@ -32,6 +33,16 @@ type resolveCredentialLeaseBody struct { Host string `json:"host"` } +type reissueCredentialLeaseBody struct { + Capability string `json:"capability"` + TaskID string `json:"task_id"` + SessionID string `json:"session_id"` + RepositoryID string `json:"repository_id"` + Owner string `json:"owner"` + Repo string `json:"repo"` + Host string `json:"host"` +} + // newEnabledAuthService builds a real auth.Service in enabled mode (flag on, // admin provisioned) so the broker HTTP test below exercises the production // httpmw.Middleware, not a stub — this is what pins that the middleware, not @@ -146,8 +157,91 @@ func TestCredentialBrokerRoutesPassRealAuthMiddleware(t *testing.T) { if rec.Code != http.StatusUnauthorized { t.Fatalf("status = %d, want %d", rec.Code, http.StatusUnauthorized) } - if !bytes.Contains(rec.Body.Bytes(), []byte("github_credential_denied")) { - t.Fatalf("body = %s, want broker code github_credential_denied", rec.Body.String()) + if !bytes.Contains(rec.Body.Bytes(), []byte("github_credential_lease_invalid")) { + t.Fatalf("body = %s, want broker code github_credential_lease_invalid", rec.Body.String()) } }) } + +func TestGenericCredentialBrokerRoutesUseProviderNeutralContract(t *testing.T) { + broker, _, _ := newPATCredentialBroker(t) + lease, err := broker.Issue(context.Background(), brokerLeaseRequest()) + if err != nil { + t.Fatalf("issue lease: %v", err) + } + router := newBrokerHTTPTestRouter(t, broker) + + ready := httptest.NewRequest(http.MethodGet, "/api/v1/git/credentials/resolve", nil) + readyRec := httptest.NewRecorder() + router.ServeHTTP(readyRec, ready) + if readyRec.Code != http.StatusNoContent { + t.Fatalf("generic readiness status = %d body=%s, want %d", readyRec.Code, readyRec.Body.String(), http.StatusNoContent) + } + + body, err := json.Marshal(resolveCredentialLeaseBody{ + Lease: lease.Token, TaskID: "task-1", SessionID: "session-1", + RepositoryID: "repository-1", Owner: "kdlbs", Repo: "kandev", Host: "github.com", + }) + if err != nil { + t.Fatalf("marshal body: %v", err) + } + request := httptest.NewRequest(http.MethodPost, "/api/v1/git/credentials/resolve", bytes.NewReader(body)) + request.Header.Set("Content-Type", "application/json") + record := httptest.NewRecorder() + router.ServeHTTP(record, request) + if record.Code != http.StatusOK { + t.Fatalf("generic resolve status = %d body=%s, want %d", record.Code, record.Body.String(), http.StatusOK) + } + + invalidBody, err := json.Marshal(resolveCredentialLeaseBody{ + Lease: "invalid", TaskID: "task-1", SessionID: "session-1", + RepositoryID: "repository-1", Owner: "kdlbs", Repo: "kandev", Host: "github.com", + }) + if err != nil { + t.Fatalf("marshal invalid body: %v", err) + } + invalidRequest := httptest.NewRequest(http.MethodPost, "/api/v1/git/credentials/resolve", bytes.NewReader(invalidBody)) + invalidRequest.Header.Set("Content-Type", "application/json") + invalidRecord := httptest.NewRecorder() + router.ServeHTTP(invalidRecord, invalidRequest) + if invalidRecord.Code != http.StatusUnauthorized || + !bytes.Contains(invalidRecord.Body.Bytes(), []byte("git_credential_lease_invalid")) { + t.Fatalf("generic invalid resolve status/body = %d/%s", invalidRecord.Code, invalidRecord.Body.String()) + } +} + +func TestCredentialBrokerReissueRouteAcceptsOnlyScopedCapability(t *testing.T) { + broker, _, _ := newPATCredentialBroker(t) + signer, err := gitcredentials.NewReissueCapabilitySigner("test-signing-key") + if err != nil { + t.Fatalf("NewReissueCapabilitySigner() error = %v", err) + } + broker.broker.SetReissueCapabilitySigner(signer) + _, capability, err := broker.broker.IssueWithReissueCapability(t.Context(), gitcredentials.Scope{ + ProviderID: "github", WorkspaceID: "workspace-1", TaskID: "task-1", SessionID: "session-1", + RepositoryID: "repository-1", Host: "github.com", Path: "/kdlbs/kandev.git", + }) + if err != nil { + t.Fatalf("IssueWithReissueCapability() error = %v", err) + } + router := newBrokerHTTPTestRouter(t, broker) + post := func(body reissueCredentialLeaseBody) *httptest.ResponseRecorder { + raw, marshalErr := json.Marshal(body) + if marshalErr != nil { + t.Fatalf("marshal reissue request: %v", marshalErr) + } + req := httptest.NewRequest(http.MethodPost, "/api/v1/github/credentials/reissue", bytes.NewReader(raw)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + return rec + } + request := reissueCredentialLeaseBody{Capability: capability.Token, TaskID: "task-1", SessionID: "session-1", RepositoryID: "repository-1", Owner: "kdlbs", Repo: "kandev", Host: "github.com"} + if rec := post(request); rec.Code != http.StatusOK { + t.Fatalf("valid reissue status = %d body=%s", rec.Code, rec.Body.String()) + } + request.Capability += "forged" + if rec := post(request); rec.Code != http.StatusUnauthorized || !bytes.Contains(rec.Body.Bytes(), []byte("github_credential_reissue_denied")) { + t.Fatalf("forged reissue status/body = %d/%s", rec.Code, rec.Body.String()) + } +} diff --git a/apps/backend/internal/github/credential_broker_test.go b/apps/backend/internal/github/credential_broker_test.go index a8040370e2..bcc11cd51d 100644 --- a/apps/backend/internal/github/credential_broker_test.go +++ b/apps/backend/internal/github/credential_broker_test.go @@ -2,10 +2,12 @@ package github import ( "context" + "encoding/json" "errors" "fmt" "testing" + "github.com/kandev/kandev/internal/gitcredentials" taskmodels "github.com/kandev/kandev/internal/task/models" ) @@ -225,6 +227,44 @@ func TestCredentialBrokerRejectsDestinationLeaseForChangedWorkspaceConnection(t } } +func TestCredentialBrokerReissueRefreshesDestinationBindingGeneration(t *testing.T) { + broker, connection, _ := newPATCredentialBroker(t) + signer, err := gitcredentials.NewReissueCapabilitySigner("stable-test-key") + if err != nil { + t.Fatalf("NewReissueCapabilitySigner() error = %v", err) + } + broker.broker.SetReissueCapabilitySigner(signer) + binding, err := json.Marshal(taskmodels.ContributionDestinationCredentialBinding{ + Source: string(ConnectionSourcePAT), Login: "octocat", CredentialGeneration: connection.CredentialGeneration, + }) + if err != nil { + t.Fatalf("marshal destination binding: %v", err) + } + scope := gitcredentials.Scope{ + ProviderID: "github", WorkspaceID: "workspace-1", TaskID: "task-1", SessionID: "session-1", + RepositoryID: "repository-1", Host: "github.com", Path: "/kdlbs/kandev.git", + CredentialBinding: string(binding), + } + _, capability, err := broker.broker.IssueWithReissueCapability(t.Context(), scope) + if err != nil { + t.Fatalf("IssueWithReissueCapability() error = %v", err) + } + connection.CredentialGeneration++ + reissued, err := broker.broker.Reissue(t.Context(), gitcredentials.ReissueRequest{ + Capability: capability.Token, TaskID: "task-1", SessionID: "session-1", RepositoryID: "repository-1", + Host: "github.com", Path: "/kdlbs/kandev.git", + }) + if err != nil { + t.Fatalf("Reissue() after destination connection rotation error = %v", err) + } + if _, err := broker.broker.Redeem(t.Context(), gitcredentials.Redemption{ + Lease: reissued.Token, TaskID: "task-1", SessionID: "session-1", RepositoryID: "repository-1", + Host: "github.com", Path: "/kdlbs/kandev.git", + }); err != nil { + t.Fatalf("Redeem() reissued destination lease error = %v", err) + } +} + func TestCredentialBrokerAppRegistrationChangeRevokesLease(t *testing.T) { installationID := int64(42) connection := &WorkspaceConnection{ diff --git a/apps/backend/internal/github/service_app_auth.go b/apps/backend/internal/github/service_app_auth.go index 49e090a918..07e702f881 100644 --- a/apps/backend/internal/github/service_app_auth.go +++ b/apps/backend/internal/github/service_app_auth.go @@ -420,6 +420,19 @@ func (s *Service) ResolveGitHubCredential( return broker.Resolve(ctx, request) } +func (s *Service) ReissueGitHubCredentialLease( + ctx context.Context, + request CredentialLeaseReissueRequest, +) (*CredentialLease, error) { + s.mu.Lock() + broker := s.credentialBroker + s.mu.Unlock() + if broker == nil { + return nil, ErrGitHubNotConfigured + } + return broker.Reissue(ctx, request) +} + type WorkspaceAutomationStatus struct { *WorkspaceConnection Actor *AuthPrincipal `json:"actor,omitempty"` diff --git a/apps/backend/internal/githubauth/environment.go b/apps/backend/internal/githubauth/environment.go index eec422349a..14ad29cf42 100644 --- a/apps/backend/internal/githubauth/environment.go +++ b/apps/backend/internal/githubauth/environment.go @@ -3,19 +3,20 @@ package githubauth import "strings" const ( - CredentialBrokerURLEnv = "KANDEV_GITHUB_CREDENTIAL_BROKER_URL" - CredentialHelperPathEnv = "KANDEV_GITHUB_CREDENTIAL_HELPER_PATH" - CredentialCLIShimDirEnv = "KANDEV_GITHUB_CLI_SHIM_DIR" - CredentialCLIBashEnvEnv = "KANDEV_GITHUB_CLI_BASH_ENV" - CredentialParentBashEnv = "KANDEV_GITHUB_PARENT_BASH_ENV" - CredentialLeaseEnv = "KANDEV_GITHUB_CREDENTIAL_LEASE" - CredentialTaskIDEnv = "KANDEV_GITHUB_CREDENTIAL_TASK_ID" - CredentialSessionIDEnv = "KANDEV_GITHUB_CREDENTIAL_SESSION_ID" - CredentialRepositoryEnv = "KANDEV_GITHUB_CREDENTIAL_REPOSITORY_ID" - CredentialOwnerEnv = "KANDEV_GITHUB_CREDENTIAL_OWNER" - CredentialRepoEnv = "KANDEV_GITHUB_CREDENTIAL_REPO" - CredentialHostEnv = "KANDEV_GITHUB_CREDENTIAL_HOST" - CredentialScopesEnv = "KANDEV_GITHUB_CREDENTIAL_SCOPES" + CredentialBrokerURLEnv = "KANDEV_GITHUB_CREDENTIAL_BROKER_URL" + CredentialHelperPathEnv = "KANDEV_GITHUB_CREDENTIAL_HELPER_PATH" + CredentialCLIShimDirEnv = "KANDEV_GITHUB_CLI_SHIM_DIR" + CredentialCLIBashEnvEnv = "KANDEV_GITHUB_CLI_BASH_ENV" + CredentialParentBashEnv = "KANDEV_GITHUB_PARENT_BASH_ENV" + CredentialLeaseEnv = "KANDEV_GITHUB_CREDENTIAL_LEASE" + CredentialReissueCapabilityEnv = "KANDEV_GITHUB_CREDENTIAL_REISSUE_CAPABILITY" + CredentialTaskIDEnv = "KANDEV_GITHUB_CREDENTIAL_TASK_ID" + CredentialSessionIDEnv = "KANDEV_GITHUB_CREDENTIAL_SESSION_ID" + CredentialRepositoryEnv = "KANDEV_GITHUB_CREDENTIAL_REPOSITORY_ID" + CredentialOwnerEnv = "KANDEV_GITHUB_CREDENTIAL_OWNER" + CredentialRepoEnv = "KANDEV_GITHUB_CREDENTIAL_REPO" + CredentialHostEnv = "KANDEV_GITHUB_CREDENTIAL_HOST" + CredentialScopesEnv = "KANDEV_GITHUB_CREDENTIAL_SCOPES" ManagedGitCredentialHelper = `!"${KANDEV_GITHUB_CREDENTIAL_HELPER_PATH}" git-credential` LegacyShimGitCredentialHelper = `!"${KANDEV_GITHUB_CLI_SHIM_DIR}/agentctl" git-credential` diff --git a/apps/backend/internal/orchestrator/executor/executor_credentials.go b/apps/backend/internal/orchestrator/executor/executor_credentials.go index 66167238d5..7aac4f3b95 100644 --- a/apps/backend/internal/orchestrator/executor/executor_credentials.go +++ b/apps/backend/internal/orchestrator/executor/executor_credentials.go @@ -44,16 +44,17 @@ const ( var ErrGitHubCredentialBrokerURL = errors.New("invalid Git credential broker URL") type githubCredentialScope struct { - Lease string `json:"lease"` - TaskID string `json:"task_id"` - SessionID string `json:"session_id"` - RepositoryID string `json:"repository_id"` - Owner string `json:"owner"` - Repo string `json:"repo"` - Host string `json:"host"` - Path string `json:"path"` - ProviderID string `json:"provider_id,omitempty"` - ParentProviderID string `json:"parent_provider_id,omitempty"` + Lease string `json:"lease"` + ReissueCapability string `json:"reissue_capability,omitempty"` + TaskID string `json:"task_id"` + SessionID string `json:"session_id"` + RepositoryID string `json:"repository_id"` + Owner string `json:"owner"` + Repo string `json:"repo"` + Host string `json:"host"` + Path string `json:"path"` + ProviderID string `json:"provider_id,omitempty"` + ParentProviderID string `json:"parent_provider_id,omitempty"` } // GitCredentialLeaseIssuer creates opaque helper leases. *gitcredentials.Broker @@ -62,6 +63,12 @@ type GitCredentialLeaseIssuer interface { Issue(context.Context, gitcredentials.Scope) (gitcredentials.Lease, error) } +// GitCredentialLeaseReissuer is optionally implemented by brokers that can +// give a launched helper a restart-safe, scope-bound replacement capability. +type GitCredentialLeaseReissuer interface { + IssueWithReissueCapability(context.Context, gitcredentials.Scope) (gitcredentials.Lease, gitcredentials.ReissueCapability, error) +} + // GitHubCredentialLease preserves the historical executor-facing lease name // while the broker uses the provider-neutral gitcredentials.Lease contract. type GitHubCredentialLease = gitcredentials.Lease @@ -376,6 +383,7 @@ func (e *Executor) configureGitCredentialEnvironment( primary := scopes[0] req.Env[githubauth.CredentialBrokerURLEnv] = e.gitCredentialBrokerURL req.Env[githubauth.CredentialLeaseEnv] = primary.Lease + req.Env[githubauth.CredentialReissueCapabilityEnv] = primary.ReissueCapability req.Env[githubauth.CredentialTaskIDEnv] = primary.TaskID req.Env[githubauth.CredentialSessionIDEnv] = primary.SessionID req.Env[githubauth.CredentialRepositoryEnv] = primary.RepositoryID @@ -443,6 +451,7 @@ func removeManagedGitCredentials(req *LaunchAgentRequest) error { githubauth.CredentialBrokerURLEnv, githubauth.CredentialHelperPathEnv, githubauth.CredentialLeaseEnv, + githubauth.CredentialReissueCapabilityEnv, githubauth.CredentialTaskIDEnv, githubauth.CredentialSessionIDEnv, githubauth.CredentialRepositoryEnv, @@ -500,7 +509,7 @@ func (e *Executor) issueGitCredentialScope( if err := validateGitHubCredentialBrokerURL(e.gitCredentialBrokerURL, req.ExecutorType); err != nil { return nil, err } - lease, err := e.gitCredentialIssuer.Issue(ctx, gitcredentials.Scope{ + scope := gitcredentials.Scope{ ProviderID: providerID, WorkspaceID: req.WorkspaceID, TaskID: req.TaskID, SessionID: req.SessionID, RepositoryID: info.RepositoryID, Host: host, Path: path, IdentityProviderID: func() string { @@ -509,7 +518,8 @@ func (e *Executor) issueGitCredentialScope( } return "" }(), - }) + } + lease, capability, err := e.issueGitCredentialLease(ctx, scope) if err != nil { return nil, fmt.Errorf("issue Git credential lease: %w", err) } @@ -517,7 +527,7 @@ func (e *Executor) issueGitCredentialScope( return nil, fmt.Errorf("issue Git credential lease: empty lease") } return &githubCredentialScope{ - Lease: lease.Token, TaskID: req.TaskID, SessionID: req.SessionID, + Lease: lease.Token, ReissueCapability: capability.Token, TaskID: req.TaskID, SessionID: req.SessionID, RepositoryID: info.RepositoryID, Owner: owner, Repo: repo, Host: host, Path: path, ProviderID: func() string { if providerID == gitHubProviderID { @@ -629,7 +639,7 @@ func (e *Executor) issueGitHubCredentialScopeForIdentity( } scope.CredentialBinding = string(encodedBinding) } - lease, err := e.gitCredentialIssuer.Issue(ctx, scope) + lease, capability, err := e.issueGitCredentialLease(ctx, scope) if err != nil { return nil, fmt.Errorf("issue Git credential lease: %w", err) } @@ -637,12 +647,29 @@ func (e *Executor) issueGitHubCredentialScopeForIdentity( return nil, fmt.Errorf("issue Git credential lease: empty lease") } return &githubCredentialScope{ - Lease: lease.Token, TaskID: req.TaskID, SessionID: req.SessionID, + Lease: lease.Token, ReissueCapability: capability.Token, TaskID: req.TaskID, SessionID: req.SessionID, RepositoryID: repositoryID, Owner: owner, Repo: repo, Host: host, Path: path, ProviderID: providerID, ParentProviderID: parentProviderID, }, nil } +func (e *Executor) issueGitCredentialLease( + ctx context.Context, + scope gitcredentials.Scope, +) (gitcredentials.Lease, gitcredentials.ReissueCapability, error) { + if issuer, ok := e.gitCredentialIssuer.(GitCredentialLeaseReissuer); ok { + lease, capability, err := issuer.IssueWithReissueCapability(ctx, scope) + if err == nil { + return lease, capability, nil + } + if !errors.Is(err, gitcredentials.ErrReissueUnavailable) { + return gitcredentials.Lease{}, gitcredentials.ReissueCapability{}, err + } + } + lease, err := e.gitCredentialIssuer.Issue(ctx, scope) + return lease, gitcredentials.ReissueCapability{}, err +} + func managedGitCredentialProvider(repository *models.Repository, githubManaged bool, env map[string]string) string { if repository.SourceType == sourceTypeLocal && strings.TrimSpace(repository.Provider) == "" { return "" diff --git a/apps/backend/internal/orchestrator/executor/executor_credentials_test.go b/apps/backend/internal/orchestrator/executor/executor_credentials_test.go index 8756386e8e..e098c2c594 100644 --- a/apps/backend/internal/orchestrator/executor/executor_credentials_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_credentials_test.go @@ -38,6 +38,19 @@ type fakeGitHubCredentialLeaseIssuer struct { calls int } +type fakeGitCredentialLeaseReissuer struct { + fakeGitHubCredentialLeaseIssuer + capability gitcredentials.ReissueCapability +} + +func (f *fakeGitCredentialLeaseReissuer) IssueWithReissueCapability( + ctx context.Context, + req gitcredentials.Scope, +) (gitcredentials.Lease, gitcredentials.ReissueCapability, error) { + lease, err := f.Issue(ctx, req) + return lease, f.capability, err +} + type fakeTaskGitCredentialPolicyResolver struct { policy TaskGitCredentialPolicy err error @@ -128,6 +141,29 @@ func TestConfigureGitHubCredentialBroker(t *testing.T) { } } +func TestConfigureGitHubCredentialBrokerPublishesReissueCapability(t *testing.T) { + issuer := &fakeGitCredentialLeaseReissuer{ + fakeGitHubCredentialLeaseIssuer: fakeGitHubCredentialLeaseIssuer{lease: gitcredentials.Lease{Token: "opaque-lease"}}, + capability: gitcredentials.ReissueCapability{Token: "execution-capability"}, + } + exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) + exec.SetGitHubCredentialBroker(issuer, "https://kandev.example/api/github/credentials/resolve") + req := &LaunchAgentRequest{ + TaskID: "task-1", WorkspaceID: "workspace-1", SessionID: "session-1", + ExecutorType: string(models.ExecutorTypeRemoteDocker), Env: map[string]string{}, + } + info := &repoInfo{RepositoryID: "repo-1", Repository: &models.Repository{Provider: "github", ProviderOwner: "acme", ProviderName: "widgets"}} + if err := exec.configureGitHubCredentialBroker(context.Background(), req, info); err != nil { + t.Fatalf("configureGitHubCredentialBroker() error = %v", err) + } + if got := req.Env[githubauth.CredentialReissueCapabilityEnv]; got != "execution-capability" { + t.Fatalf("reissue capability env = %q", got) + } + if got := req.Env[githubauth.CredentialScopesEnv]; !strings.Contains(got, `"reissue_capability":"execution-capability"`) { + t.Fatalf("credential scopes = %q, want reissue capability", got) + } +} + func TestApplyGitCredentialSnapshotOmitsManagedSourceWithoutBroker(t *testing.T) { exec := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) exec.SetTaskGitCredentialPolicyResolver(fakeTaskGitCredentialPolicyResolver{ diff --git a/docs/docker.md b/docs/docker.md index 1407ddf03e..064f6d7726 100644 --- a/docs/docker.md +++ b/docs/docker.md @@ -82,7 +82,7 @@ A one-time `docker exec -it kandev gh auth login` makes that named GitHub accoun > Workspace PATs are stored in Kandev's encrypted secret store. A workspace configured for GitHub CLI resolves the exact selected host/login without changing the active CLI account. Host-active `gh`, backend `GITHUB_TOKEN`/`GH_TOKEN`, and old globally named secrets are consulted only by migration-only **Legacy shared** connections. -Set `KANDEV_GITHUB_CREDENTIAL_BROKER_PUBLIC_BASE_URL` to an HTTPS Kandev URL reachable from agent containers and remote executors. This is independent of GitHub App setup and is required for managed PAT or CLI credentials outside local/worktree execution. The reverse proxy must pass both `GET` and `POST` on `/api/v1/github/credentials/resolve`; executors require the non-secret `GET` readiness response before startup, while credential redemption uses the lease-authenticated `POST`. +Set `KANDEV_GITHUB_CREDENTIAL_BROKER_PUBLIC_BASE_URL` to an HTTPS Kandev URL reachable from agent containers and remote executors. Set the stable secret `KANDEV_GITHUB_CREDENTIAL_BROKER_REISSUE_SIGNING_KEY` to let a running executor recover its managed lease after a backend restart; changing this key invalidates outstanding execution capabilities. Drain or quiesce active agent sessions before key rotation. This is independent of GitHub App setup and is required for managed PAT or CLI credentials outside local/worktree execution. The reverse proxy must pass `GET` and `POST` on `/api/v1/git/credentials/resolve` and `POST` on `/api/v1/git/credentials/reissue`; the older `/api/v1/github/credentials/*` paths remain compatibility aliases. Executors require the non-secret `GET` readiness response before startup, credential redemption uses the lease-authenticated `POST`, and reissue uses an execution-scoped capability. ## Configuration diff --git a/docs/k8s.md b/docs/k8s.md index e813e6cb7c..deaf65a6de 100644 --- a/docs/k8s.md +++ b/docs/k8s.md @@ -117,7 +117,7 @@ So a one-time `kubectl exec -it deployment/kandev -- gh auth login` makes that n > Workspace PATs are stored in Kandev's encrypted secret store. A workspace configured for GitHub CLI resolves the exact selected host/login without changing the active CLI account. Host-active `gh`, backend `GITHUB_TOKEN`/`GH_TOKEN`, and old globally named secrets are consulted only by migration-only **Legacy shared** connections. -Set `KANDEV_GITHUB_CREDENTIAL_BROKER_PUBLIC_BASE_URL` to the HTTPS Kandev service URL reachable from agent containers and remote executors. This is independent of GitHub App setup and is required for managed PAT or CLI credentials outside local/worktree execution. Ingress must pass both `GET` and `POST` on `/api/v1/github/credentials/resolve`; executors require the non-secret `GET` readiness response before startup, while credential redemption uses the lease-authenticated `POST`. +Set `KANDEV_GITHUB_CREDENTIAL_BROKER_PUBLIC_BASE_URL` to the HTTPS Kandev service URL reachable from agent containers and remote executors. Set the stable secret `KANDEV_GITHUB_CREDENTIAL_BROKER_REISSUE_SIGNING_KEY` to let a running executor recover its managed lease after a backend restart; changing this key invalidates outstanding execution capabilities. Drain or quiesce active agent sessions before key rotation. This is independent of GitHub App setup and is required for managed PAT or CLI credentials outside local/worktree execution. Ingress must pass `GET` and `POST` on `/api/v1/git/credentials/resolve` and `POST` on `/api/v1/git/credentials/reissue`; the older `/api/v1/github/credentials/*` paths remain compatibility aliases. Executors require the non-secret `GET` readiness response before startup, credential redemption uses the lease-authenticated `POST`, and reissue uses an execution-scoped capability. ## Configuration diff --git a/docs/plans/git-credential-lease-reissue/plan.md b/docs/plans/git-credential-lease-reissue/plan.md new file mode 100644 index 0000000000..288612f982 --- /dev/null +++ b/docs/plans/git-credential-lease-reissue/plan.md @@ -0,0 +1,96 @@ +--- +spec: docs/specs/git-credential-lease-reissue/spec.md +created: 2026-08-20 +status: completed +--- + +# Implementation Plan: Git Credential Lease Reissue + +## Overview + +Add an encrypted, execution-scoped reissue capability alongside each managed Git +credential lease. The generic broker validates that capability and issues a +fresh lease from live authorization and provider binding; the existing helper +then performs one replacement redemption. A capability reissue replaces the +previous lease for that capability, so short-lived helper processes cannot +consume the workspace lease limit. This retains in-memory leases while making +a running execution recover after a credential-generation change or backend +restart. + +## Backend + +### Capability and broker contracts + +- Add sealing, validation, expiry, and exact-scope comparison to + `apps/backend/internal/gitcredentials/` without retaining credentials. +- Extend `Broker` with capability issue/reissue operations that call its + existing authorization path, preserving live authorizer and resolver checks. +- Let providers refresh rotating binding generations during reissue without + changing the sealed task, session, or repository identity. +- Wire a stable signer from backend configuration in + `apps/backend/internal/backendapp/` and leave capability issuance disabled + when no stable key exists. + +### Executor environment injection + +- Extend `apps/backend/internal/orchestrator/executor/executor_credentials.go` + scope records and managed-environment cleanup to carry a capability matching + each issued scope. +- Extend `apps/backend/internal/githubauth/environment.go` and + `apps/backend/internal/agent/runtime/lifecycle/remote_github_env.go` so + Docker, SSH, and local managed helper environments receive only the required + opaque capability values. + +### HTTP and helper recovery + +- Add provider-neutral `/api/v1/git/credentials/*` broker routes and precise + reissuable error codes. Keep the GitHub paths as compatibility aliases and + preserve public-route self-authentication in + `apps/backend/internal/auth/httpmw/middleware.go`. +- Update `apps/backend/cmd/agentctl/github_credential.go` to recognize only + eligible lease errors, request one replacement lease with the matching + capability, and retry the original redemption once. The same command powers + the Git helper and `gh` shim. + +## Tests + +- **Capability contract:** `internal/gitcredentials/*_test.go` covers valid + reissue, exact-scope mismatch, forged token, and expiry. +- **Broker HTTP:** `internal/github/*_test.go` verifies self-authenticated + reissue, backend-restart-equivalent new broker recovery, and no lease for + forged/expired capabilities. +- **Executor wiring:** `internal/orchestrator/executor/*_test.go` asserts every + single and multi-repository helper scope receives only its matching opaque + capability. +- **Helper integration:** `cmd/agentctl/github_credential_test.go` asserts + credential-generation revoked and missing-lease responses reissue once, + retry successfully in single- and multi-scope mode, and reject + non-reissuable/fake/expired paths. The `gh` shim uses the same one-shot flow. +- **Lease lifecycle:** repeated reissue keeps one active lease for a + capability. Contribution destination reissue refreshes the current binding + generation after an authorized connection rotation. + +## Verification Results + +- `go build ./...` and affected-package `go vet` passed locally. +- Focused regressions cover credential-generation rotation, an empty lease map + after restart, forged/expired/scope-mismatched capabilities, helper retry + limits, HTTP middleware admission, executor injection, and remote executor + environment forwarding. +- `gofmt -l`, `git diff --check`, and public-doc validation passed locally. +- `golangci-lint --new-from-rev` was not run because `golangci-lint` is not + installed in this worktree environment. + +## Implementation Waves + +1. [task-01-capability-and-broker](task-01-capability-and-broker.md) +2. [task-02-executor-and-helper](task-02-executor-and-helper.md) +3. [task-03-regression-verification](task-03-regression-verification.md) + +## Risks + +- The reissue route is publicly reachable, so its only authority is the + capability plus repeated live scope authorization. It must never accept an + old lease as reissue proof. +- A non-stable signer cannot safely support cross-restart recovery. Its + disabled path must remain explicit and fail closed. diff --git a/docs/plans/git-credential-lease-reissue/task-01-capability-and-broker.md b/docs/plans/git-credential-lease-reissue/task-01-capability-and-broker.md new file mode 100644 index 0000000000..36e68844e1 --- /dev/null +++ b/docs/plans/git-credential-lease-reissue/task-01-capability-and-broker.md @@ -0,0 +1,23 @@ +--- +id: "01-capability-and-broker" +title: "Add credential reissue capability" +status: completed +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/git-credential-lease-reissue/spec.md" +--- + +# Task 01: Add credential reissue capability + +- **Acceptance:** Capability claims have an exact scope and expiry, reject + forged/expired/mismatched requests, and reissue only through live broker + authorization and binding validation. +- **Acceptance:** The HTTP reissue endpoint returns only a lease and remains + self-authenticating under the existing public-route middleware boundary. +- **Verification:** `cd apps/backend && go test -count=1 ./internal/gitcredentials ./internal/github ./internal/backendapp` + +Likely files: `internal/gitcredentials`, `internal/github`, +`internal/backendapp`, and `internal/auth/httpmw`. + +Risk: no old lease may be treated as reissue authority. diff --git a/docs/plans/git-credential-lease-reissue/task-02-executor-and-helper.md b/docs/plans/git-credential-lease-reissue/task-02-executor-and-helper.md new file mode 100644 index 0000000000..a61c202b81 --- /dev/null +++ b/docs/plans/git-credential-lease-reissue/task-02-executor-and-helper.md @@ -0,0 +1,22 @@ +--- +id: "02-executor-and-helper" +title: "Wire helper lease recovery" +status: completed +wave: 2 +depends_on: ["01-capability-and-broker"] +plan: "plan.md" +spec: "../../specs/git-credential-lease-reissue/spec.md" +--- + +# Task 02: Wire helper lease recovery + +- **Acceptance:** Each managed launch scope includes only its matching opaque + capability in local, Docker, and SSH helper environments. +- **Acceptance:** Git helper and `gh` shim reissue and retry exactly once for + eligible lease failures, without scope fallback. +- **Verification:** `cd apps/backend && go test -count=1 ./internal/orchestrator/executor ./cmd/agentctl ./internal/agent/runtime/lifecycle` + +Likely files: `internal/orchestrator/executor`, `internal/githubauth`, +`internal/agent/runtime/lifecycle`, and `cmd/agentctl`. + +Risk: multi-repository scope selection must retain exact path matching. diff --git a/docs/plans/git-credential-lease-reissue/task-03-regression-verification.md b/docs/plans/git-credential-lease-reissue/task-03-regression-verification.md new file mode 100644 index 0000000000..8f2c05fca5 --- /dev/null +++ b/docs/plans/git-credential-lease-reissue/task-03-regression-verification.md @@ -0,0 +1,22 @@ +--- +id: "03-regression-verification" +title: "Verify lease recovery contracts" +status: completed +wave: 3 +depends_on: ["01-capability-and-broker", "02-executor-and-helper"] +plan: "plan.md" +spec: "../../specs/git-credential-lease-reissue/spec.md" +--- + +# Task 03: Verify lease recovery contracts + +- **Acceptance:** Rotation, backend-restart-equivalent, forged capability, and + expired capability scenarios have focused regression coverage. +- **Acceptance:** Formatting, diff checks, targeted tests, build, vet, and the + changed-file linter are recorded accurately. +- **Verification:** `cd apps/backend && go build ./... && go vet ./internal/gitcredentials ./internal/github ./internal/orchestrator/executor ./cmd/agentctl` + +Likely files: the tests added by tasks 01 and 02 plus this plan's result +sections. + +Risk: do not claim E2E or remote runtime evidence from local unit tests. diff --git a/docs/public/executors.md b/docs/public/executors.md index d1c369808d..10df8e3c98 100644 --- a/docs/public/executors.md +++ b/docs/public/executors.md @@ -277,7 +277,7 @@ docker ps -a --filter label=kandev.managed=true 3. Select that secret for the required `SPRITES_API_TOKEN` profile environment variable. 4. Review remote credential methods, Git identity, prepare/cleanup scripts, and network policy. -Sprites profiles do not copy the host-active `gh` CLI token. Kandev may copy explicitly selected agent credential files, resolve selected Kandev secrets into agent environment variables, or run an agent auth setup script. A profile-selected GitHub token is an unmanaged override; otherwise an attached GitHub repository uses the workspace credential broker. Set `githubCredentialBroker.publicBaseUrl` (or `KANDEV_GITHUB_CREDENTIAL_BROKER_PUBLIC_BASE_URL`) to an HTTPS Kandev URL reachable from remote executors; this setting is independent of GitHub App registration. Credential upload is best-effort: provisioning can continue while later agent authentication fails. The remote sandbox receives highly sensitive data; use a scoped provider token and least-privilege repository credentials. +Sprites profiles do not copy the host-active `gh` CLI token. Kandev may copy explicitly selected agent credential files, resolve selected Kandev secrets into agent environment variables, or run an agent auth setup script. A profile-selected GitHub token is an unmanaged override; otherwise an attached GitHub repository uses the workspace credential broker. Set `githubCredentialBroker.publicBaseUrl` (or `KANDEV_GITHUB_CREDENTIAL_BROKER_PUBLIC_BASE_URL`) to an HTTPS Kandev URL reachable from remote executors. To recover managed Git credentials after a backend restart, configure the stable secret `githubCredentialBroker.reissueSigningKey` (or `KANDEV_GITHUB_CREDENTIAL_BROKER_REISSUE_SIGNING_KEY`); changing it invalidates outstanding execution capabilities. Drain or quiesce active agent sessions before key rotation. The broker uses `/api/v1/git/credentials/resolve` and `/api/v1/git/credentials/reissue`; the older GitHub paths remain compatibility aliases. These settings are independent of GitHub App registration. Credential upload is best-effort: provisioning can continue while later agent authentication fails. The remote sandbox receives highly sensitive data; use a scoped provider token and least-privilege repository credentials. Network rules are stored in `sprites_network_policy_rules` as JSON entries with `domain`, `action` (`allow` or `deny`), and optional `include`. Kandev applies them only on fresh sandbox creation, and currently does so after credential upload, prepare, controller startup, and agent-instance creation. Bootstrap traffic can therefore occur before the profile policy is installed. A parse/provider failure is reported as skipped and does not abort launch. Provider semantics remain authoritative; do not treat this late, best-effort step as a security boundary, and test the resulting policy. diff --git a/docs/specs/INDEX.md b/docs/specs/INDEX.md index fc7f79a728..4ea71c3c9a 100644 --- a/docs/specs/INDEX.md +++ b/docs/specs/INDEX.md @@ -55,6 +55,7 @@ Product-wide capabilities that are not tied to a single feature area. | [semantic-notifications](platform/notifications.md) | shipped | | [workspace-git-status](platform/workspace-git-status.md) | shipped | | [git-subprocess-admission](platform/git-subprocess-admission.md) | building | +| [git-credential-lease-reissue](git-credential-lease-reissue/spec.md) | shipped | | [bounded-task-status-delivery](platform/bounded-task-status-delivery.md) | approved | | [diagnostic-logging](platform/diagnostic-logging.md) | approved | | [provider-error-recovery](platform/provider-error-recovery.md) | draft | diff --git a/docs/specs/git-credential-lease-reissue/spec.md b/docs/specs/git-credential-lease-reissue/spec.md new file mode 100644 index 0000000000..4b72ae1b6d --- /dev/null +++ b/docs/specs/git-credential-lease-reissue/spec.md @@ -0,0 +1,125 @@ +--- +status: implemented +created: 2026-08-20 +owner: kandev +--- + +# Git Credential Lease Reissue + +## Why + +A long-running execution can outlive the in-memory Git credential lease held by +the backend. A workspace credential rotation or backend restart then leaves Git +and the `gh` shim permanently unable to redeem credentials even though the task, +session, repository, and current workspace connection remain valid. + +## What + +- A managed Git credential helper SHALL make at most one reissue attempt for a + single credential request after the broker reports a reissuable lease failure. +- A reissue uses a dedicated execution capability, never the failed lease as + proof of authority. +- The broker SHALL issue the replacement lease only after it verifies the exact + task, session, repository, provider, host, path, and optional provider-parent + identity carried by the capability against live state. +- A replacement lease SHALL bind to the current provider credential generation. + A live execution therefore recovers after an authorized workspace credential + rotation or after a backend restart discarded the old in-memory lease map. +- The helper SHALL retry only lease-invalid, lease-expired, or lease-revoked + responses. Lease-revoked is included because a credential-generation change + deliberately revokes the old lease. Scope denial, malformed requests, + capability failures, provider failures, and transport failures remain + fail-closed with no retry. +- Capabilities and leases are opaque bearer values. They SHALL never be logged, + returned by browser-facing APIs, or stored with plaintext Git credentials. + +## Data model + +`GitCredentialReissueCapability` is an encrypted, authenticated opaque execution capability. Its +claims are the exact non-secret `gitcredentials.Scope` identity, an issue time, +and an expiry. The capability is injected only into the managed runtime helper +environment with the matching lease scope. No capability, lease, or credential +plaintext is written to the task database. + +The capability signer uses a stable configured backend signing key. When no +stable signer is configured, reissue capability issuance is unavailable and the +existing managed-lease path remains fail-closed; a process restart does not gain +an unauthenticated recovery path. + +## API surface + +- `POST /api/v1/github/credentials/resolve` continues to redeem an opaque + lease for a credential. Its error code distinguishes lease-invalid, + lease-expired, and lease-revoked responses so a helper can decide whether a + reissue is eligible. +- `POST /api/v1/github/credentials/reissue` accepts a reissue capability and + the helper's exact non-secret repository request. It returns a new opaque + lease and expiry, never a Git credential. The route is unauthenticated at + HTTP middleware level because the capability self-authenticates in the + handler. +- `KANDEV_GITHUB_CREDENTIAL_REISSUE_CAPABILITY` carries the default helper + capability. Multi-repository `KANDEV_GITHUB_CREDENTIAL_SCOPES` entries carry + the matching capability beside each lease. + +## State machine + +1. Launch issues a scoped lease and a scoped reissue capability. +2. Helper redeems the lease. +3. On an eligible lease failure, helper validates its local Git input against + the issued scope, exchanges the capability for one replacement lease, and + retries redemption once. +4. The replacement redemption succeeds or fails closed. The helper does not + loop or fall back to another repository scope. + +## Permissions + +The capability grants no broader permission than the launch-time scope. On +every reissue the broker re-runs live task/session/repository authorization and +the provider binding check. A terminal session, removed task repository, +disabled connection, changed repository identity, forged capability, expired +capability, or mismatched helper request is denied. + +## Failure modes + +- A malformed, forged, expired, or scope-mismatched capability returns an + authorization failure and issues no lease. +- A terminal session or changed task/repository binding returns an authorization + failure and issues no lease. +- A disabled, disconnected, or otherwise unavailable provider returns its + existing failure; the helper does not retry another credential source. +- A failed reissue or replacement redemption is returned to Git/`gh` after the + single allowed retry. + +## Persistence guarantees + +Leases remain intentionally in-memory and disappear on backend restart. +Reissue capabilities survive only in the still-running execution environment +and remain verifiable across a backend restart when the configured signer is +stable. They expire and are rendered useless by live authorization once their +task/session/repository is no longer eligible. + +## Scenarios + +- **GIVEN** a running execution with a valid reissue capability, **WHEN** the + workspace credential generation rotates and the old lease is redeemed, + **THEN** the helper obtains one lease bound to the new generation and the + same Git request succeeds. +- **GIVEN** a running execution with a valid reissue capability, **WHEN** the + backend restarts and has no prior lease records, **THEN** the helper obtains + one newly issued lease and the same Git request succeeds. +- **GIVEN** a forged or expired capability, **WHEN** the helper requests a + lease, **THEN** the broker issues no lease and returns an authorization + failure. +- **GIVEN** a capability for repository A, **WHEN** a helper requests + repository B or a terminal session's scope, **THEN** the broker issues no + lease. +- **GIVEN** a lease scope error or a failed replacement redemption, **WHEN** + the helper handles the response, **THEN** it performs no further reissue + attempt or fallback. + +## Out of scope + +- Persisting lease records or Git credentials. +- Reissuing executor-profile, GitLab, Azure DevOps, or user-supplied tokens. +- Repairing an execution whose agent process or helper environment has stopped. +- Retrying arbitrary broker, network, provider, or authorization failures.