diff --git a/apps/backend/cmd/plugin-fixture/fixture-package/manifest.yaml b/apps/backend/cmd/plugin-fixture/fixture-package/manifest.yaml index 5eba8e4b53..0219a9ee87 100644 --- a/apps/backend/cmd/plugin-fixture/fixture-package/manifest.yaml +++ b/apps/backend/cmd/plugin-fixture/fixture-package/manifest.yaml @@ -39,6 +39,12 @@ actions: - key: "connection-status" scope: "workspace" max_body_bytes: 4096 + - key: "repositories.inspect" + scope: "workspace" + max_body_bytes: 16384 + - key: "repositories.branches" + scope: "workspace" + max_body_bytes: 16384 - key: "link-pull-request" scope: "task" max_body_bytes: 4096 diff --git a/apps/backend/cmd/plugin-fixture/fixture_package_test.go b/apps/backend/cmd/plugin-fixture/fixture_package_test.go index 1033bec0ae..fe94026b68 100644 --- a/apps/backend/cmd/plugin-fixture/fixture_package_test.go +++ b/apps/backend/cmd/plugin-fixture/fixture_package_test.go @@ -32,8 +32,13 @@ func TestFixtureManifest_ParsesAndValidates(t *testing.T) { require.Equal(t, []string{"fixture-source-control"}, m.RepositoryProviders) require.Equal(t, "connection-status", m.Actions[0].Key) require.Equal(t, "workspace", m.Actions[0].ResourceScope) - require.Equal(t, "link-pull-request", m.Actions[1].Key) - require.Equal(t, "task", m.Actions[1].ResourceScope) + actions := make(map[string]manifest.Action, len(m.Actions)) + for _, action := range m.Actions { + actions[action.Key] = action + } + require.Equal(t, "workspace", actions[repositoryInspectActionKey].ResourceScope) + require.Equal(t, "workspace", actions[repositoryBranchesActionKey].ResourceScope) + require.Equal(t, "task", actions["link-pull-request"].ResourceScope) require.Len(t, m.ReferenceSources, 1) require.Equal(t, "fixture-pull-requests", m.ReferenceSources[0].Source) require.Equal(t, "fixture-source-control", m.ReferenceSources[0].Provider) diff --git a/apps/backend/cmd/plugin-fixture/main_test.go b/apps/backend/cmd/plugin-fixture/main_test.go index decc15c213..91014e2107 100644 --- a/apps/backend/cmd/plugin-fixture/main_test.go +++ b/apps/backend/cmd/plugin-fixture/main_test.go @@ -154,6 +154,29 @@ func TestHandleAction_ReturnsAuthenticatedConnectionStatus(t *testing.T) { require.JSONEq(t, `{"connected":true,"workspace_id":"workspace-42"}`, string(resp.Body)) } +func TestHandleAction_InspectsFixtureRepository(t *testing.T) { + p := &fixturePlugin{dataDir: t.TempDir()} + + resp, err := p.HandleAction(context.Background(), &pluginsdk.PluginActionRequest{ + ActionKey: repositoryInspectActionKey, + Context: pluginsdk.VerifiedActionContext{WorkspaceID: "workspace-42"}, + Body: []byte(`{"url":"https://bitbucket.example.test/projects/TEAM/repos/fixture"}`), + }) + require.NoError(t, err) + require.JSONEq(t, `{"repository":{"provider_id":"fixture-source-control","provider_host":"bitbucket.example.test","provider_scope":"","provider_repository_id":"fixture-repository","owner_or_project":"TEAM","name":"fixture","clone_url":"https://bitbucket.example.test/scm/TEAM/fixture.git","default_branch":"main"}}`, string(resp.Body)) +} + +func TestHandleAction_ListsFixtureRepositoryBranches(t *testing.T) { + p := &fixturePlugin{dataDir: t.TempDir()} + + resp, err := p.HandleAction(context.Background(), &pluginsdk.PluginActionRequest{ + ActionKey: repositoryBranchesActionKey, + Context: pluginsdk.VerifiedActionContext{WorkspaceID: "workspace-42"}, + }) + require.NoError(t, err) + require.JSONEq(t, `{"branches":[{"name":"main","is_default":true},{"name":"feature/provider-contract"}]}`, string(resp.Body)) +} + func TestHandleAction_CreatesPluginOwnedWatchTask(t *testing.T) { p := &fixturePlugin{dataDir: t.TempDir()} host := &fakeHost{createdTaskID: "watch-task-42"} diff --git a/apps/backend/cmd/plugin-fixture/plugin.go b/apps/backend/cmd/plugin-fixture/plugin.go index 5d74771e5a..664363a250 100644 --- a/apps/backend/cmd/plugin-fixture/plugin.go +++ b/apps/backend/cmd/plugin-fixture/plugin.go @@ -7,6 +7,7 @@ import ( "context" "encoding/json" "fmt" + "net/url" "os" "path/filepath" "strings" @@ -26,17 +27,19 @@ const ( // writeProbeWebhookKey triggers the Host data API write round-trip // (CreateTask + CreateComment). Gated on this key so unrelated webhook // deliveries don't attempt writes. - writeProbeWebhookKey = "write" - fixtureReferenceSource = "fixture-pull-requests" - fixturePullRequestID = "pull-request-42" - revokedPullRequestID = "pull-request-revoked" - fixtureProviderID = "fixture-source-control" - fixtureCredentialHost = "bitbucket.example.test" - fixtureCredentialPath = "/scm/TEAM/fixture" - connectionStatusAction = "connection-status" - searchPurpose = "search" - submissionPurpose = "submission" - fixtureTaskIDKey = "task_id" + writeProbeWebhookKey = "write" + fixtureReferenceSource = "fixture-pull-requests" + fixturePullRequestID = "pull-request-42" + revokedPullRequestID = "pull-request-revoked" + fixtureProviderID = "fixture-source-control" + fixtureCredentialHost = "bitbucket.example.test" + fixtureCredentialPath = "/scm/TEAM/fixture" + connectionStatusAction = "connection-status" + repositoryInspectActionKey = "repositories.inspect" + repositoryBranchesActionKey = "repositories.branches" + searchPurpose = "search" + submissionPurpose = "submission" + fixtureTaskIDKey = "task_id" ) // deliveryRecord is one recorded OnEvent delivery, appended as a JSON line @@ -184,6 +187,10 @@ func (p *fixturePlugin) HandleAction(ctx context.Context, req *pluginsdk.PluginA response["pull_request_id"] = fixturePullRequestID case "watch-create-task": return p.createWatchTask(ctx, req.Context.WorkspaceID) + case repositoryInspectActionKey: + return p.inspectRepository(req.Body) + case repositoryBranchesActionKey: + return p.listRepositoryBranches() default: return nil, fmt.Errorf("plugin-fixture: unknown action %q", req.ActionKey) } @@ -194,6 +201,25 @@ func (p *fixturePlugin) HandleAction(ctx context.Context, req *pluginsdk.PluginA return &pluginsdk.PluginActionResponse{Body: body}, nil } +func (p *fixturePlugin) inspectRepository(body []byte) (*pluginsdk.PluginActionResponse, error) { + var request struct { + URL string `json:"url"` + } + if err := json.Unmarshal(body, &request); err != nil || !fixtureRepositoryURL(request.URL) { + return &pluginsdk.PluginActionResponse{Body: []byte(`{"matched":false}`)}, nil + } + return &pluginsdk.PluginActionResponse{Body: []byte(`{"repository":{"provider_id":"fixture-source-control","provider_host":"bitbucket.example.test","provider_scope":"","provider_repository_id":"fixture-repository","owner_or_project":"TEAM","name":"fixture","clone_url":"https://bitbucket.example.test/scm/TEAM/fixture.git","default_branch":"main"}}`)}, nil +} + +func fixtureRepositoryURL(raw string) bool { + parsed, err := url.Parse(strings.TrimSpace(raw)) + return err == nil && strings.EqualFold(parsed.Hostname(), fixtureCredentialHost) +} + +func (p *fixturePlugin) listRepositoryBranches() (*pluginsdk.PluginActionResponse, error) { + return &pluginsdk.PluginActionResponse{Body: []byte(`{"branches":[{"name":"main","is_default":true},{"name":"feature/provider-contract"}]}`)}, nil +} + func (p *fixturePlugin) createWatchTask(ctx context.Context, workspaceID string) (*pluginsdk.PluginActionResponse, error) { host := p.Host() if host == nil { diff --git a/apps/backend/internal/backendapp/repository_selection.go b/apps/backend/internal/backendapp/repository_selection.go new file mode 100644 index 0000000000..a44cc9b667 --- /dev/null +++ b/apps/backend/internal/backendapp/repository_selection.go @@ -0,0 +1,70 @@ +package backendapp + +import ( + "context" + "errors" + "strings" + + "github.com/kandev/kandev/internal/plugins" + taskservice "github.com/kandev/kandev/internal/task/service" +) + +type repositoryProviderInspector interface { + InspectRepositoryProvider(context.Context, string, plugins.RepositoryProviderInspectionRequest) (*plugins.RepositoryProviderInspection, error) +} + +type pluginRepositorySelectionResolver struct { + inspector repositoryProviderInspector +} + +func (r pluginRepositorySelectionResolver) ResolveRepositorySelection( + ctx context.Context, workspaceID string, input taskservice.TaskRepositoryInput, +) (taskservice.TaskRepositoryInput, error) { + if r.inspector == nil { + return taskservice.TaskRepositoryInput{}, taskservice.NewRepositorySelectionError( + taskservice.RepositorySelectionErrorUnavailable, nil, + ) + } + url := strings.TrimSpace(input.RemoteURL) + if url == "" { + url = strings.TrimSpace(input.GitHubURL) + } + inspection, err := r.inspector.InspectRepositoryProvider(ctx, workspaceID, plugins.RepositoryProviderInspectionRequest{ + Provider: input.Provider, URL: url, ProviderScope: input.ProviderScope, + ProviderRepositoryID: input.ProviderRepoID, + }) + if err != nil { + return taskservice.TaskRepositoryInput{}, mapRepositoryProviderError(err) + } + if inspection == nil { + return taskservice.TaskRepositoryInput{}, taskservice.NewRepositorySelectionError( + taskservice.RepositorySelectionErrorUnavailable, nil, + ) + } + return taskservice.TaskRepositoryInput{ + BaseBranch: input.BaseBranch, CheckoutBranch: input.CheckoutBranch, + BranchPolicyID: input.BranchPolicyID, PRNumber: input.PRNumber, + RemoteContribution: input.RemoteContribution, ContributionDestination: input.ContributionDestination, + PreserveBaseBranch: input.PreserveBaseBranch, ResolveProviderDefaults: input.ResolveProviderDefaults, + RemoteURL: inspection.CloneURL, Provider: inspection.ProviderID, + ProviderHost: inspection.ProviderHost, ProviderScope: inspection.ProviderScope, + ProviderRepoID: inspection.ProviderRepositoryID, ProviderOwner: inspection.OwnerOrProject, + ProviderName: inspection.Name, DefaultBranch: inspection.DefaultBranch, + TrustedProviderDescriptor: true, + }, nil +} + +func mapRepositoryProviderError(err error) error { + var providerErr *plugins.RepositoryProviderError + if !errors.As(err, &providerErr) { + return taskservice.NewRepositorySelectionError(taskservice.RepositorySelectionErrorUnavailable, err) + } + code := taskservice.RepositorySelectionErrorUnavailable + switch providerErr.Code { + case plugins.RepositoryProviderErrorInvalid: + code = taskservice.RepositorySelectionErrorInvalid + case plugins.RepositoryProviderErrorNotFound: + code = taskservice.RepositorySelectionErrorNotFound + } + return taskservice.NewRepositorySelectionError(code, err) +} diff --git a/apps/backend/internal/backendapp/repository_selection_test.go b/apps/backend/internal/backendapp/repository_selection_test.go new file mode 100644 index 0000000000..68db65fdec --- /dev/null +++ b/apps/backend/internal/backendapp/repository_selection_test.go @@ -0,0 +1,77 @@ +package backendapp + +import ( + "context" + "errors" + "testing" + + "github.com/kandev/kandev/internal/plugins" + taskservice "github.com/kandev/kandev/internal/task/service" +) + +type repositoryProviderInspectorStub struct { + inspection *plugins.RepositoryProviderInspection + err error + request plugins.RepositoryProviderInspectionRequest +} + +func (s *repositoryProviderInspectorStub) InspectRepositoryProvider( + _ context.Context, _ string, request plugins.RepositoryProviderInspectionRequest, +) (*plugins.RepositoryProviderInspection, error) { + s.request = request + return s.inspection, s.err +} + +func TestPluginRepositorySelectionResolverUsesAuthoritativeInspection(t *testing.T) { + inspector := &repositoryProviderInspectorStub{inspection: &plugins.RepositoryProviderInspection{ + ProviderID: "fixture-source-control", ProviderHost: "https://bitbucket.example.test", + ProviderScope: "workspace-a", ProviderRepositoryID: "repo-42", OwnerOrProject: "TEAM", + Name: "fixture", CloneURL: "https://bitbucket.example.test/scm/TEAM/fixture.git", DefaultBranch: "main", + }} + resolver := pluginRepositorySelectionResolver{inspector: inspector} + input := taskservice.TaskRepositoryInput{ + RemoteURL: "https://bitbucket.example.test/projects/TEAM/fixture", + Provider: "fixture-source-control", ProviderScope: "workspace-a", ProviderRepoID: "repo-42", + ProviderHost: "https://attacker.example.test", ProviderOwner: "attacker", ProviderName: "wrong", + DefaultBranch: "attacker-default", BaseBranch: "main", CheckoutBranch: "feature/provider-contract", + } + resolved, err := resolver.ResolveRepositorySelection(context.Background(), "workspace-1", input) + if err != nil { + t.Fatalf("ResolveRepositorySelection: %v", err) + } + if inspector.request.Provider != input.Provider || inspector.request.URL != input.RemoteURL || + inspector.request.ProviderScope != input.ProviderScope || inspector.request.ProviderRepositoryID != input.ProviderRepoID { + t.Fatalf("inspection request = %+v", inspector.request) + } + if resolved.ProviderHost != "https://bitbucket.example.test" || resolved.ProviderOwner != "TEAM" || + resolved.ProviderName != "fixture" || resolved.ProviderRepoID != "repo-42" || + resolved.RemoteURL != "https://bitbucket.example.test/scm/TEAM/fixture.git" || resolved.DefaultBranch != "main" || + !resolved.TrustedProviderDescriptor { + t.Fatalf("resolved input = %+v", resolved) + } +} + +func TestPluginRepositorySelectionResolverMapsTypedProviderFailures(t *testing.T) { + for _, test := range []struct { + name string + providerCode plugins.RepositoryProviderErrorCode + want taskservice.RepositorySelectionErrorCode + }{ + {name: "invalid", providerCode: plugins.RepositoryProviderErrorInvalid, want: taskservice.RepositorySelectionErrorInvalid}, + {name: "not found", providerCode: plugins.RepositoryProviderErrorNotFound, want: taskservice.RepositorySelectionErrorNotFound}, + {name: "unavailable", providerCode: plugins.RepositoryProviderErrorUnavailable, want: taskservice.RepositorySelectionErrorUnavailable}, + } { + t.Run(test.name, func(t *testing.T) { + resolver := pluginRepositorySelectionResolver{inspector: &repositoryProviderInspectorStub{ + err: &plugins.RepositoryProviderError{Code: test.providerCode}, + }} + _, err := resolver.ResolveRepositorySelection(context.Background(), "workspace-1", taskservice.TaskRepositoryInput{ + Provider: "fixture-source-control", RemoteURL: "https://bitbucket.example.test/TEAM/fixture", + }) + var selectionErr *taskservice.RepositorySelectionError + if !errors.As(err, &selectionErr) || selectionErr.Code != test.want { + t.Fatalf("error = %v, want code %q", err, test.want) + } + }) + } +} diff --git a/apps/backend/internal/backendapp/services.go b/apps/backend/internal/backendapp/services.go index f557b0a2db..260cf35043 100644 --- a/apps/backend/internal/backendapp/services.go +++ b/apps/backend/internal/backendapp/services.go @@ -238,6 +238,7 @@ func provideServices(cfg *config.Config, log *logger.Logger, repos *Repositories if githubSvc != nil { pluginsSvc.SetTaskPRSource(githubSvc) } + taskSvc.SetRepositorySelectionResolver(pluginRepositorySelectionResolver{inspector: pluginsSvc}) } gitCredentialBroker := newGitCredentialBroker(githubSvc, pluginsSvc, repos.Task, cfg.GitHubCredentialBroker.ReissueSigningKey) if pluginsSvc != nil { diff --git a/apps/backend/internal/mcp/handlers/handlers.go b/apps/backend/internal/mcp/handlers/handlers.go index 193feab6c7..415eadc84e 100644 --- a/apps/backend/internal/mcp/handlers/handlers.go +++ b/apps/backend/internal/mcp/handlers/handlers.go @@ -59,6 +59,8 @@ type workspaceSourceJSON struct { GitHubURL string `json:"github_url"` RemoteURL string `json:"remote_url"` Provider string `json:"provider"` + ProviderHost string `json:"provider_host"` + ProviderScope string `json:"provider_scope"` ProviderRepoID string `json:"provider_repo_id"` ProviderOwner string `json:"provider_owner"` ProviderName string `json:"provider_name"` @@ -2071,7 +2073,7 @@ func parseWorkspaceSources(raw []json.RawMessage) ([]service.WorkspaceSourceInpu allowed := map[string]bool{"kind": true, "local_path": true} switch kind { case string(service.WorkspaceSourceRepository): - for _, key := range []string{"repository_id", "remote_url", "github_url", "provider", "provider_repo_id", "provider_owner", "provider_name", "base_branch", "checkout_branch"} { + for _, key := range []string{"repository_id", "remote_url", "github_url", "provider", "provider_host", "provider_scope", "provider_repo_id", "provider_owner", "provider_name", "base_branch", "checkout_branch"} { allowed[key] = true } case string(service.WorkspaceSourceFolder): @@ -2088,7 +2090,7 @@ func parseWorkspaceSources(raw []json.RawMessage) ([]service.WorkspaceSourceInpu if err := json.Unmarshal(item, &source); err != nil { return nil, err } - sources = append(sources, service.WorkspaceSourceInput{Kind: service.WorkspaceSourceKind(source.Kind), RepositoryID: source.RepositoryID, LocalPath: source.LocalPath, GitHubURL: source.GitHubURL, RemoteURL: source.RemoteURL, Provider: source.Provider, ProviderRepoID: source.ProviderRepoID, ProviderOwner: source.ProviderOwner, ProviderName: source.ProviderName, BaseBranch: source.BaseBranch, CheckoutBranch: source.CheckoutBranch, DisplayName: source.DisplayName}) + sources = append(sources, service.WorkspaceSourceInput{Kind: service.WorkspaceSourceKind(source.Kind), RepositoryID: source.RepositoryID, LocalPath: source.LocalPath, GitHubURL: source.GitHubURL, RemoteURL: source.RemoteURL, Provider: source.Provider, ProviderHost: source.ProviderHost, ProviderScope: source.ProviderScope, ProviderRepoID: source.ProviderRepoID, ProviderOwner: source.ProviderOwner, ProviderName: source.ProviderName, BaseBranch: source.BaseBranch, CheckoutBranch: source.CheckoutBranch, DisplayName: source.DisplayName}) } return sources, nil } diff --git a/apps/backend/internal/plugins/repository_provider_inspect.go b/apps/backend/internal/plugins/repository_provider_inspect.go new file mode 100644 index 0000000000..12aaaf22dd --- /dev/null +++ b/apps/backend/internal/plugins/repository_provider_inspect.go @@ -0,0 +1,314 @@ +package plugins + +import ( + "context" + "encoding/json" + "errors" + "strings" + "time" + + "github.com/kandev/kandev/internal/repoclone" + "github.com/kandev/kandev/pkg/pluginsdk" + "go.uber.org/zap" +) + +const ( + repositoryInspectActionKey = "repositories.inspect" + maxRepositoryInspectResponseSize = 1 << 20 + repositoryInspectActionTimeout = 15 * time.Second + maxRepositoryProviderScopeBytes = 512 + nullJSONValue = "null" + repositoryProviderInspectionNone = "none" +) + +// RepositoryProviderInspectionRequest contains the untrusted URL and optional +// immutable identity hints supplied by a task-create request. The hints are +// checked against the plugin result but are never sent to the plugin. +type RepositoryProviderInspectionRequest struct { + Provider string + URL string + ProviderScope string + ProviderRepositoryID string +} + +// RepositoryProviderInspection is the authoritative, credential-free +// repository identity returned by an active repository-provider plugin. +type RepositoryProviderInspection struct { + ProviderID string `json:"provider_id"` + ProviderHost string `json:"provider_host"` + ProviderScope string `json:"provider_scope"` + ProviderRepositoryID string `json:"provider_repository_id"` + OwnerOrProject string `json:"owner_or_project"` + Name string `json:"name"` + CloneURL string `json:"clone_url"` + DefaultBranch string `json:"default_branch"` +} + +// RepositoryProviderErrorCode classifies a repository-provider inspection +// failure without exposing plugin response bodies or upstream error text. +type RepositoryProviderErrorCode string + +const ( + RepositoryProviderErrorInvalid RepositoryProviderErrorCode = "invalid" + RepositoryProviderErrorNotFound RepositoryProviderErrorCode = "not_found" + RepositoryProviderErrorUnavailable RepositoryProviderErrorCode = "unavailable" +) + +// RepositoryProviderError is a safe, typed failure from repository-provider +// inspection. The wrapped cause is available to internal callers for +// cancellation checks, while Error never includes it. +type RepositoryProviderError struct { + Code RepositoryProviderErrorCode + cause error +} + +func (e *RepositoryProviderError) Error() string { + switch e.Code { + case RepositoryProviderErrorInvalid: + return "repository provider returned an invalid repository descriptor" + case RepositoryProviderErrorNotFound: + return "repository was not found by the repository provider" + case RepositoryProviderErrorUnavailable: + return "repository provider is unavailable" + default: + return "repository provider inspection failed" + } +} + +func (e *RepositoryProviderError) Unwrap() error { return e.cause } + +// InspectRepositoryProvider asks the active manifest owner of the provider to +// resolve a repository URL into a validated descriptor. +func (s *Service) InspectRepositoryProvider( + ctx context.Context, workspaceID string, request RepositoryProviderInspectionRequest, +) (*RepositoryProviderInspection, error) { + return s.inspectRepositoryProvider(ctx, workspaceID, request, s.InvokeAction) +} + +func (s *Service) inspectRepositoryProvider( + ctx context.Context, + workspaceID string, + request RepositoryProviderInspectionRequest, + invoke repositoryActionInvoker, +) (inspection *RepositoryProviderInspection, err error) { + started := time.Now() + providerID := strings.TrimSpace(request.Provider) + pluginID := "" + responseShape := "not_sent" + defer func() { + if s.log == nil { + return + } + failureCategory := repositoryProviderInspectionNone + if err != nil { + failureCategory = repositoryProviderInspectionFailureCategory(err) + } + s.log.Info("plugin repository inspection", + zap.String("provider_id", providerID), + zap.String("plugin_id", pluginID), + zap.String("workspace_id", workspaceID), + zap.Duration("duration", time.Since(started)), + zap.String("response_shape", responseShape), + zap.String("failure_category", failureCategory)) + }() + + request.Provider = strings.TrimSpace(request.Provider) + request.URL = strings.TrimSpace(request.URL) + if strings.TrimSpace(workspaceID) == "" || request.Provider == "" || request.URL == "" { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, nil) + } + record, action, err := s.repositoryProviderAction(request.Provider, repositoryInspectActionKey) + if err != nil { + return nil, newRepositoryProviderError(RepositoryProviderErrorUnavailable, err) + } + pluginID = record.ID + body, err := json.Marshal(map[string]string{"url": request.URL}) + if err != nil || action.MaxBodyBytes <= 0 || len(body) > action.MaxBodyBytes { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, err) + } + invokeCtx, cancel := context.WithTimeout(ctx, repositoryInspectActionTimeout) + defer cancel() + response, err := invoke(invokeCtx, record.ID, dispatchGeneration(record), &pluginsdk.PluginActionRequest{ + ActionKey: repositoryInspectActionKey, + Context: pluginsdk.VerifiedActionContext{WorkspaceID: workspaceID}, + Body: body, + }) + responseShape = repositoryProviderInspectionResponseShape(response) + if err != nil { + return nil, newRepositoryProviderError(RepositoryProviderErrorUnavailable, err) + } + return parseRepositoryProviderActionResponse(response, request) +} + +func parseRepositoryProviderActionResponse( + response *pluginsdk.PluginActionResponse, request RepositoryProviderInspectionRequest, +) (*RepositoryProviderInspection, error) { + if response != nil && response.Status != 0 && (response.Status < 200 || response.Status >= 300) { + return nil, repositoryProviderStatusError(response.Status) + } + return parseRepositoryProviderInspection(response, request) +} + +func repositoryProviderInspectionFailureCategory(err error) string { + var providerErr *RepositoryProviderError + if errors.As(err, &providerErr) && providerErr.Code != "" { + return string(providerErr.Code) + } + return "unknown" +} + +func repositoryProviderInspectionResponseShape(response *pluginsdk.PluginActionResponse) string { + if response == nil { + return repositoryProviderInspectionNone + } + if response.Status != 0 && (response.Status < 200 || response.Status >= 300) { + return "http_status" + } + if len(response.Body) == 0 { + return "empty" + } + if len(response.Body) > maxRepositoryInspectResponseSize { + return "oversized" + } + var raw map[string]json.RawMessage + if err := json.Unmarshal(response.Body, &raw); err != nil { + return "invalid_json" + } + if matched, exists := raw["matched"]; exists { + var value bool + if err := json.Unmarshal(matched, &value); err != nil { + return "invalid_matched" + } + if !value { + return "matched_false" + } + } + if _, nested := raw["repository"]; nested { + return "nested_descriptor" + } + return "direct_descriptor" +} + +func parseRepositoryProviderInspection( + response *pluginsdk.PluginActionResponse, request RepositoryProviderInspectionRequest, +) (*RepositoryProviderInspection, error) { + if response == nil || len(response.Body) == 0 || len(response.Body) > maxRepositoryInspectResponseSize { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, nil) + } + var raw map[string]json.RawMessage + if err := json.Unmarshal(response.Body, &raw); err != nil { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, err) + } + if matched, exists := raw["matched"]; exists { + var value bool + if err := json.Unmarshal(matched, &value); err != nil { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, err) + } + if !value { + if repository, hasRepository := raw["repository"]; hasRepository && string(repository) != nullJSONValue { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, nil) + } + return nil, newRepositoryProviderError(RepositoryProviderErrorNotFound, nil) + } + } + + candidate, err := inspectionCandidate(raw) + if err != nil { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, err) + } + return validateRepositoryProviderInspection(candidate, request) +} + +func inspectionCandidate(raw map[string]json.RawMessage) (*RepositoryProviderInspection, error) { + if repository, nested := raw["repository"]; nested { + if string(repository) == nullJSONValue { + return nil, errors.New("repository descriptor is null") + } + var candidate RepositoryProviderInspection + if err := json.Unmarshal(repository, &candidate); err != nil { + return nil, err + } + return &candidate, nil + } + var candidate RepositoryProviderInspection + body, err := json.Marshal(raw) + if err != nil { + return nil, err + } + if err := json.Unmarshal(body, &candidate); err != nil { + return nil, err + } + return &candidate, nil +} + +func validateRepositoryProviderInspection( + candidate *RepositoryProviderInspection, request RepositoryProviderInspectionRequest, +) (*RepositoryProviderInspection, error) { + if candidate == nil || !strings.EqualFold(strings.TrimSpace(candidate.ProviderID), request.Provider) { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, nil) + } + host, err := repoclone.HTTPSProviderOrigin(candidate.ProviderHost) + if err != nil { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, err) + } + scope, err := normalizeRepositoryProviderScope(candidate.ProviderScope) + if err != nil { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, err) + } + repositoryID := strings.TrimSpace(candidate.ProviderRepositoryID) + owner := strings.TrimSpace(candidate.OwnerOrProject) + name := strings.TrimSpace(candidate.Name) + cloneURL := strings.TrimSpace(candidate.CloneURL) + defaultBranch := strings.TrimSpace(candidate.DefaultBranch) + if repositoryID == "" || owner == "" || name == "" || cloneURL == "" || defaultBranch == "" || + strings.ContainsAny(repositoryID+owner+name+defaultBranch, "\x00") { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, nil) + } + if err := repoclone.ValidateHTTPSCloneOrigin(cloneURL, host); err != nil { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, err) + } + if err := validateRepositoryProviderHint(request.ProviderScope, scope); err != nil { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, err) + } + if hint := strings.TrimSpace(request.ProviderRepositoryID); hint != "" && hint != repositoryID { + return nil, newRepositoryProviderError(RepositoryProviderErrorInvalid, nil) + } + return &RepositoryProviderInspection{ + ProviderID: request.Provider, ProviderHost: host, ProviderScope: scope, + ProviderRepositoryID: repositoryID, OwnerOrProject: owner, Name: name, + CloneURL: cloneURL, DefaultBranch: defaultBranch, + }, nil +} + +func normalizeRepositoryProviderScope(raw string) (string, error) { + scope := strings.TrimSpace(raw) + if len(scope) > maxRepositoryProviderScopeBytes || strings.ContainsRune(scope, '\x00') { + return "", errors.New("repository provider scope is invalid") + } + return scope, nil +} + +func validateRepositoryProviderHint(hint, value string) error { + normalized, err := normalizeRepositoryProviderScope(hint) + if err != nil { + return err + } + if normalized != "" && normalized != value { + return errors.New("repository provider scope does not match the request hint") + } + return nil +} + +func repositoryProviderStatusError(status int) error { + if status == 404 { + return newRepositoryProviderError(RepositoryProviderErrorNotFound, nil) + } + if status >= 400 && status < 500 && status != 408 && status != 429 { + return newRepositoryProviderError(RepositoryProviderErrorInvalid, nil) + } + return newRepositoryProviderError(RepositoryProviderErrorUnavailable, nil) +} + +func newRepositoryProviderError(code RepositoryProviderErrorCode, cause error) error { + return &RepositoryProviderError{Code: code, cause: cause} +} diff --git a/apps/backend/internal/plugins/repository_provider_inspect_test.go b/apps/backend/internal/plugins/repository_provider_inspect_test.go new file mode 100644 index 0000000000..940bd32782 --- /dev/null +++ b/apps/backend/internal/plugins/repository_provider_inspect_test.go @@ -0,0 +1,295 @@ +package plugins + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "runtime" + "strings" + "testing" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/plugins/pkgtar/pkgtartest" + "github.com/kandev/kandev/pkg/pluginsdk" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" +) + +func TestInspectRepositoryProviderUsesOwnedDeclaredActionAndVerifiedWorkspace(t *testing.T) { + svc, _, _ := newTestService(t) + if _, err := svc.Install(t.Context(), repositoryInspectPackage(t, "workspace")); err != nil { + t.Fatalf("install provider plugin: %v", err) + } + request := RepositoryProviderInspectionRequest{ + Provider: "bitbucket", URL: "https://bitbucket.example.test/projects/acme/widgets", + ProviderScope: "workspace-a", ProviderRepositoryID: "repo-42", + } + inspection, err := svc.inspectRepositoryProvider(t.Context(), "workspace-1", request, + func(_ context.Context, id string, _ pluginDispatchGeneration, action *pluginsdk.PluginActionRequest) (*pluginsdk.PluginActionResponse, error) { + if id != "kandev-plugin-bitbucket" || action.ActionKey != repositoryInspectActionKey { + t.Fatalf("unexpected dispatch: id=%q action=%q", id, action.ActionKey) + } + if action.Context.WorkspaceID != "workspace-1" { + t.Fatalf("workspace context = %q", action.Context.WorkspaceID) + } + var body map[string]string + if err := json.Unmarshal(action.Body, &body); err != nil { + t.Fatalf("decode request body: %v", err) + } + if len(body) != 1 || body["url"] != request.URL { + t.Fatalf("request body = %#v, want only URL", body) + } + return &pluginsdk.PluginActionResponse{Body: validInspectionResponse()}, nil + }) + if err != nil { + t.Fatalf("inspectRepositoryProvider: %v", err) + } + if inspection.ProviderID != "bitbucket" || inspection.ProviderHost != "https://bitbucket.example.test" || + inspection.ProviderScope != "workspace-a" || inspection.ProviderRepositoryID != "repo-42" || + inspection.OwnerOrProject != "acme" || inspection.Name != "widgets" || + inspection.CloneURL != "https://bitbucket.example.test/scm/acme/widgets.git" || inspection.DefaultBranch != "main" { + t.Fatalf("inspection = %+v", inspection) + } +} + +func TestInspectRepositoryProviderAcceptsDirectDescriptorCompatibilityResponse(t *testing.T) { + request := RepositoryProviderInspectionRequest{Provider: "bitbucket", URL: "https://bitbucket.example.test/acme/widgets"} + inspection, err := parseRepositoryProviderInspection(&pluginsdk.PluginActionResponse{Body: directInspectionResponse()}, request) + if err != nil { + t.Fatalf("parse direct inspection: %v", err) + } + if inspection.ProviderRepositoryID != "repo-42" || inspection.DefaultBranch != "main" { + t.Fatalf("inspection = %+v", inspection) + } +} + +func TestInspectRepositoryProviderRejectsInvalidContractResponses(t *testing.T) { + request := RepositoryProviderInspectionRequest{Provider: "bitbucket", URL: "https://bitbucket.example.test/acme/widgets"} + tests := []struct { + name string + body []byte + }{ + {name: "malformed", body: []byte(`{"repository":`)}, + {name: "provider mismatch", body: []byte(`{"repository":{"provider_id":"github","provider_host":"bitbucket.example.test","provider_repository_id":"repo-42","owner_or_project":"acme","name":"widgets","clone_url":"https://bitbucket.example.test/scm/acme/widgets.git","default_branch":"main"}}`)}, + {name: "credential-bearing clone", body: []byte(`{"repository":{"provider_id":"bitbucket","provider_host":"bitbucket.example.test","provider_repository_id":"repo-42","owner_or_project":"acme","name":"widgets","clone_url":"https://token@bitbucket.example.test/scm/acme/widgets.git","default_branch":"main"}}`)}, + {name: "foreign clone origin", body: []byte(`{"repository":{"provider_id":"bitbucket","provider_host":"bitbucket.example.test","provider_repository_id":"repo-42","owner_or_project":"acme","name":"widgets","clone_url":"https://attacker.example.test/acme/widgets.git","default_branch":"main"}}`)}, + {name: "missing immutable field", body: []byte(`{"repository":{"provider_id":"bitbucket","provider_host":"bitbucket.example.test","provider_repository_id":"repo-42","owner_or_project":"acme","clone_url":"https://bitbucket.example.test/scm/acme/widgets.git","default_branch":"main"}}`)}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + _, err := parseRepositoryProviderInspection(&pluginsdk.PluginActionResponse{Body: test.body}, request) + assertRepositoryProviderError(t, err, RepositoryProviderErrorInvalid) + }) + } + + _, err := parseRepositoryProviderInspection(&pluginsdk.PluginActionResponse{Body: []byte(`{"matched":false}`)}, request) + assertRepositoryProviderError(t, err, RepositoryProviderErrorNotFound) +} + +func TestInspectRepositoryProviderRejectsMissingOwnerAndWrongActionScope(t *testing.T) { + for _, test := range []struct { + name string + pkg func(*testing.T) *bytes.Buffer + }{ + {name: "missing owner", pkg: func(t *testing.T) *bytes.Buffer { + return testPackageWithRepositoryProvider(t, "kandev-plugin-bitbucket", "bitbucket") + }}, + {name: "wrong action scope", pkg: func(t *testing.T) *bytes.Buffer { + return repositoryInspectPackage(t, "task") + }}, + } { + t.Run(test.name, func(t *testing.T) { + svc, _, _ := newTestService(t) + if _, err := svc.Install(t.Context(), test.pkg(t)); err != nil { + t.Fatalf("install provider plugin: %v", err) + } + _, err := svc.inspectRepositoryProvider(t.Context(), "workspace-1", RepositoryProviderInspectionRequest{ + Provider: "bitbucket", URL: "https://bitbucket.example.test/acme/widgets", + }, nil) + assertRepositoryProviderError(t, err, RepositoryProviderErrorUnavailable) + }) + } +} + +func TestInspectRepositoryProviderBoundsResponseAndTimeout(t *testing.T) { + request := RepositoryProviderInspectionRequest{Provider: "bitbucket", URL: "https://bitbucket.example.test/acme/widgets"} + tooLarge := bytes.Repeat([]byte("x"), maxRepositoryInspectResponseSize+1) + _, err := parseRepositoryProviderInspection(&pluginsdk.PluginActionResponse{Body: tooLarge}, request) + assertRepositoryProviderError(t, err, RepositoryProviderErrorInvalid) + + svc, _, _ := newTestService(t) + if _, err := svc.Install(t.Context(), repositoryInspectPackage(t, "workspace")); err != nil { + t.Fatalf("install provider plugin: %v", err) + } + _, err = svc.inspectRepositoryProvider(t.Context(), "workspace-1", request, + func(ctx context.Context, _ string, _ pluginDispatchGeneration, _ *pluginsdk.PluginActionRequest) (*pluginsdk.PluginActionResponse, error) { + if _, ok := ctx.Deadline(); !ok { + return nil, errors.New("inspection context has no deadline") + } + return nil, context.DeadlineExceeded + }) + assertRepositoryProviderError(t, err, RepositoryProviderErrorUnavailable) +} + +func TestInspectRepositoryProviderRejectsPinMismatch(t *testing.T) { + request := RepositoryProviderInspectionRequest{ + Provider: "bitbucket", URL: "https://bitbucket.example.test/acme/widgets", + ProviderScope: "workspace-b", ProviderRepositoryID: "repo-other", + } + _, err := parseRepositoryProviderInspection(&pluginsdk.PluginActionResponse{Body: validInspectionResponse()}, request) + assertRepositoryProviderError(t, err, RepositoryProviderErrorInvalid) +} + +func TestInspectRepositoryProviderRejectsRedirectResponse(t *testing.T) { + svc, _, _ := newTestService(t) + if _, err := svc.Install(t.Context(), repositoryInspectPackage(t, "workspace")); err != nil { + t.Fatalf("install provider plugin: %v", err) + } + request := RepositoryProviderInspectionRequest{Provider: "bitbucket", URL: "https://bitbucket.example.test/acme/widgets"} + _, err := svc.inspectRepositoryProvider(t.Context(), "workspace-1", request, + func(context.Context, string, pluginDispatchGeneration, *pluginsdk.PluginActionRequest) (*pluginsdk.PluginActionResponse, error) { + return &pluginsdk.PluginActionResponse{Status: 302, Body: validInspectionResponse()}, nil + }) + assertRepositoryProviderError(t, err, RepositoryProviderErrorUnavailable) +} + +func TestInspectRepositoryProviderLogsSanitizedOutcome(t *testing.T) { + svc, _, _ := newTestService(t) + if _, err := svc.Install(t.Context(), repositoryInspectPackage(t, "workspace")); err != nil { + t.Fatalf("install provider plugin: %v", err) + } + core, observed := observer.New(zapcore.InfoLevel) + log, err := logger.NewFromZap(zap.New(core)) + if err != nil { + t.Fatalf("create observed logger: %v", err) + } + svc.log = log + + request := RepositoryProviderInspectionRequest{ + Provider: "bitbucket", URL: "https://bitbucket.example.test/acme/widgets", + } + tests := []struct { + name string + invoke repositoryActionInvoker + wantShape string + wantFailure string + wantError RepositoryProviderErrorCode + }{ + { + name: "success", + invoke: func(context.Context, string, pluginDispatchGeneration, *pluginsdk.PluginActionRequest) (*pluginsdk.PluginActionResponse, error) { + return &pluginsdk.PluginActionResponse{Body: validInspectionResponse()}, nil + }, + wantShape: "nested_descriptor", + wantFailure: "none", + }, + { + name: "invalid descriptor", + invoke: func(context.Context, string, pluginDispatchGeneration, *pluginsdk.PluginActionRequest) (*pluginsdk.PluginActionResponse, error) { + return &pluginsdk.PluginActionResponse{Body: []byte(`{"repository":{"provider_id":"bitbucket"}}`)}, nil + }, + wantShape: "nested_descriptor", + wantFailure: "invalid", + wantError: RepositoryProviderErrorInvalid, + }, + { + name: "provider unavailable", + invoke: func(context.Context, string, pluginDispatchGeneration, *pluginsdk.PluginActionRequest) (*pluginsdk.PluginActionResponse, error) { + return nil, context.DeadlineExceeded + }, + wantShape: "none", + wantFailure: "unavailable", + wantError: RepositoryProviderErrorUnavailable, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + before := observed.Len() + _, err := svc.inspectRepositoryProvider(t.Context(), "workspace-1", request, test.invoke) + if test.wantError == "" { + if err != nil { + t.Fatalf("inspectRepositoryProvider: %v", err) + } + } else { + assertRepositoryProviderError(t, err, test.wantError) + } + entries := observed.All() + if len(entries) != before+1 { + t.Fatalf("log entries = %d, want %d", len(entries), before+1) + } + fields := entries[before].ContextMap() + for key, want := range map[string]string{ + "provider_id": "bitbucket", + "plugin_id": "kandev-plugin-bitbucket", + "workspace_id": "workspace-1", + "response_shape": test.wantShape, + "failure_category": test.wantFailure, + } { + if got := fmt.Sprint(fields[key]); got != want { + t.Errorf("%s = %q, want %q", key, got, want) + } + } + if _, ok := fields["duration"]; !ok { + t.Error("duration field is missing") + } + if strings.Contains(entries[before].Message, request.URL) || strings.Contains(fmt.Sprint(fields), request.URL) { + t.Errorf("inspection log leaked repository URL: %s", request.URL) + } + }) + } +} + +func assertRepositoryProviderError(t *testing.T, err error, want RepositoryProviderErrorCode) { + t.Helper() + if err == nil { + t.Fatalf("expected repository provider error with code %q", want) + } + var providerErr *RepositoryProviderError + if !errors.As(err, &providerErr) { + t.Fatalf("error = %v, want RepositoryProviderError", err) + } + if providerErr.Code != want { + t.Fatalf("error code = %q, want %q", providerErr.Code, want) + } + if strings.Contains(err.Error(), "token") || strings.Contains(err.Error(), "attacker.example.test") { + t.Fatalf("error leaked upstream details: %v", err) + } +} + +func validInspectionResponse() []byte { + return []byte(`{"repository":{"provider_id":"bitbucket","provider_host":"bitbucket.example.test","provider_scope":"workspace-a","provider_repository_id":"repo-42","owner_or_project":"acme","name":"widgets","clone_url":"https://bitbucket.example.test/scm/acme/widgets.git","default_branch":"main"}}`) +} + +func directInspectionResponse() []byte { + return []byte(`{"provider_id":"bitbucket","provider_host":"bitbucket.example.test","provider_repository_id":"repo-42","owner_or_project":"acme","name":"widgets","clone_url":"https://bitbucket.example.test/scm/acme/widgets.git","default_branch":"main"}`) +} + +func repositoryInspectPackage(t *testing.T, scope string) *bytes.Buffer { + t.Helper() + manifestYAML := fmt.Sprintf(` +id: kandev-plugin-bitbucket +api_version: 1 +version: "1.0.0" +display_name: Bitbucket +repository_providers: [bitbucket] +actions: + - key: repositories.inspect + scope: %s + max_body_bytes: 16384 +runtime: + type: binary + executables: + %s-%s: server/plugin +`, scope, runtime.GOOS, runtime.GOARCH) + var buffer bytes.Buffer + if err := pkgtartest.WritePackage(&buffer, map[string][]byte{ + "manifest.yaml": []byte(manifestYAML), + "server/plugin": []byte("#!/bin/sh\necho fake\n"), + }); err != nil { + t.Fatalf("WritePackage: %v", err) + } + return &buffer +} diff --git a/apps/backend/internal/task/dto/requests.go b/apps/backend/internal/task/dto/requests.go index e511b35a77..e18edd15f2 100644 --- a/apps/backend/internal/task/dto/requests.go +++ b/apps/backend/internal/task/dto/requests.go @@ -266,6 +266,8 @@ type TaskRepositoryInput struct { GitHubURL string RemoteURL string Provider string + ProviderHost string + ProviderScope string ProviderRepoID string ProviderOwner string ProviderName string diff --git a/apps/backend/internal/task/handlers/errors.go b/apps/backend/internal/task/handlers/errors.go index 9a1f2e4cdb..5d33d8edb3 100644 --- a/apps/backend/internal/task/handlers/errors.go +++ b/apps/backend/internal/task/handlers/errors.go @@ -10,6 +10,7 @@ import ( "github.com/kandev/kandev/internal/common/logger" taskrepo "github.com/kandev/kandev/internal/task/repository/sqlite" "github.com/kandev/kandev/internal/task/service" + ws "github.com/kandev/kandev/pkg/websocket" "go.uber.org/zap" ) @@ -30,6 +31,10 @@ func handleNotFound(c *gin.Context, log *logger.Logger, err error, fallback stri abortClientDisconnect(c) return } + if status, ok := repositorySelectionHTTPStatus(err); ok { + c.JSON(status, taskErrorBody(err)) + return + } if isNotFound(err) { c.JSON(http.StatusNotFound, gin.H{"error": fallback}) return @@ -55,12 +60,50 @@ func taskErrorBody(err error) gin.H { } func taskErrorDetails(err error) map[string]interface{} { + var selectionErr *service.RepositorySelectionError + if errors.As(err, &selectionErr) { + return map[string]interface{}{"error_code": string(selectionErr.Code)} + } if errors.Is(err, service.ErrRepositoryBranchPolicyStale) { return map[string]interface{}{"error_code": service.BranchPolicyStaleErrorCode} } return nil } +func repositorySelectionHTTPStatus(err error) (int, bool) { + var selectionErr *service.RepositorySelectionError + if !errors.As(err, &selectionErr) { + return 0, false + } + switch selectionErr.Code { + case service.RepositorySelectionErrorInvalid: + return http.StatusBadRequest, true + case service.RepositorySelectionErrorNotFound: + return http.StatusNotFound, true + case service.RepositorySelectionErrorUnavailable: + return http.StatusServiceUnavailable, true + default: + return http.StatusInternalServerError, true + } +} + +func repositorySelectionWSCode(err error) (string, bool) { + var selectionErr *service.RepositorySelectionError + if !errors.As(err, &selectionErr) { + return "", false + } + switch selectionErr.Code { + case service.RepositorySelectionErrorInvalid: + return ws.ErrorCodeValidation, true + case service.RepositorySelectionErrorNotFound: + return ws.ErrorCodeNotFound, true + case service.RepositorySelectionErrorUnavailable: + return ws.ErrorCodeUnavailable, true + default: + return ws.ErrorCodeInternalError, true + } +} + func handleSelectedMoveError(c *gin.Context, log *logger.Logger, err error) { switch { case isClientDisconnect(err): diff --git a/apps/backend/internal/task/handlers/errors_test.go b/apps/backend/internal/task/handlers/errors_test.go index 99e8375c16..2ba851aae9 100644 --- a/apps/backend/internal/task/handlers/errors_test.go +++ b/apps/backend/internal/task/handlers/errors_test.go @@ -8,6 +8,7 @@ import ( "net" "net/http" "net/http/httptest" + "strings" "testing" "github.com/gin-gonic/gin" @@ -15,6 +16,7 @@ import ( taskrepo "github.com/kandev/kandev/internal/task/repository/sqlite" "github.com/kandev/kandev/internal/task/service" wfmodels "github.com/kandev/kandev/internal/workflow/models" + ws "github.com/kandev/kandev/pkg/websocket" ) // TestErrorsAreClassifiable verifies that the package's error-classification @@ -115,6 +117,67 @@ func TestErrorsAreClassifiable(t *testing.T) { }) } +func TestTaskCreateRepositorySelectionErrorsMapToTransports(t *testing.T) { + tests := []struct { + name string + errorCode service.RepositorySelectionErrorCode + httpStatus int + wsCode string + }{ + { + name: "invalid", + errorCode: service.RepositorySelectionErrorInvalid, + httpStatus: http.StatusBadRequest, + wsCode: ws.ErrorCodeValidation, + }, + { + name: "not found", + errorCode: service.RepositorySelectionErrorNotFound, + httpStatus: http.StatusNotFound, + wsCode: ws.ErrorCodeNotFound, + }, + { + name: "unavailable", + errorCode: service.RepositorySelectionErrorUnavailable, + httpStatus: http.StatusServiceUnavailable, + wsCode: ws.ErrorCodeUnavailable, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + err := service.NewRepositorySelectionError(test.errorCode, errors.New("plugin response secret")) + + status, ok := repositorySelectionHTTPStatus(err) + if !ok || status != test.httpStatus { + t.Fatalf("repositorySelectionHTTPStatus() = (%d, %t), want (%d, true)", status, ok, test.httpStatus) + } + code, ok := repositorySelectionWSCode(err) + if !ok || code != test.wsCode { + t.Fatalf("repositorySelectionWSCode() = (%q, %t), want (%q, true)", code, ok, test.wsCode) + } + + gin.SetMode(gin.TestMode) + rec := httptest.NewRecorder() + ctx, _ := gin.CreateTestContext(rec) + handleNotFound(ctx, newTestLogger(t), err, "task not created") + if rec.Code != test.httpStatus { + t.Fatalf("HTTP status = %d, want %d", rec.Code, test.httpStatus) + } + var payload map[string]string + if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil { + t.Fatalf("decode HTTP response: %v", err) + } + if payload["error_code"] != string(test.errorCode) { + t.Fatalf("error_code = %q, want %q", payload["error_code"], test.errorCode) + } + if strings.Contains(rec.Body.String(), "plugin response secret") { + t.Fatal("HTTP response leaked the wrapped provider error") + } + }) + } +} + func TestMoveConflictCode(t *testing.T) { cases := []struct { name string diff --git a/apps/backend/internal/task/handlers/task_handlers.go b/apps/backend/internal/task/handlers/task_handlers.go index 118246be3c..086c66f6ff 100644 --- a/apps/backend/internal/task/handlers/task_handlers.go +++ b/apps/backend/internal/task/handlers/task_handlers.go @@ -237,6 +237,8 @@ func convertToServiceRepos(repos []dto.TaskRepositoryInput) []service.TaskReposi GitHubURL: r.GitHubURL, RemoteURL: r.RemoteURL, Provider: r.Provider, + ProviderHost: r.ProviderHost, + ProviderScope: r.ProviderScope, ProviderRepoID: r.ProviderRepoID, ProviderOwner: r.ProviderOwner, ProviderName: r.ProviderName, diff --git a/apps/backend/internal/task/handlers/task_http_handlers.go b/apps/backend/internal/task/handlers/task_http_handlers.go index ee992aadec..c90089add2 100644 --- a/apps/backend/internal/task/handlers/task_http_handlers.go +++ b/apps/backend/internal/task/handlers/task_http_handlers.go @@ -41,6 +41,8 @@ type workspaceSourceJSON struct { GitHubURL string `json:"github_url"` RemoteURL string `json:"remote_url"` Provider string `json:"provider"` + ProviderHost string `json:"provider_host"` + ProviderScope string `json:"provider_scope"` ProviderRepoID string `json:"provider_repo_id"` ProviderOwner string `json:"provider_owner"` ProviderName string `json:"provider_name"` @@ -85,7 +87,7 @@ func parseHTTPWorkspaceSources(raw []json.RawMessage) ([]service.WorkspaceSource allowed := map[string]bool{"kind": true, "local_path": true} switch kind { case string(service.WorkspaceSourceRepository): - for _, key := range []string{"repository_id", "remote_url", "github_url", "provider", "provider_repo_id", "provider_owner", "provider_name", "base_branch", "checkout_branch"} { + for _, key := range []string{"repository_id", "remote_url", "github_url", "provider", "provider_host", "provider_scope", "provider_repo_id", "provider_owner", "provider_name", "base_branch", "checkout_branch"} { allowed[key] = true } case string(service.WorkspaceSourceFolder): @@ -102,7 +104,7 @@ func parseHTTPWorkspaceSources(raw []json.RawMessage) ([]service.WorkspaceSource if err := json.Unmarshal(item, &source); err != nil { return nil, err } - sources = append(sources, service.WorkspaceSourceInput{Kind: service.WorkspaceSourceKind(source.Kind), RepositoryID: source.RepositoryID, LocalPath: source.LocalPath, GitHubURL: source.GitHubURL, RemoteURL: source.RemoteURL, Provider: source.Provider, ProviderRepoID: source.ProviderRepoID, ProviderOwner: source.ProviderOwner, ProviderName: source.ProviderName, BaseBranch: source.BaseBranch, CheckoutBranch: source.CheckoutBranch, DisplayName: source.DisplayName}) + sources = append(sources, service.WorkspaceSourceInput{Kind: service.WorkspaceSourceKind(source.Kind), RepositoryID: source.RepositoryID, LocalPath: source.LocalPath, GitHubURL: source.GitHubURL, RemoteURL: source.RemoteURL, Provider: source.Provider, ProviderHost: source.ProviderHost, ProviderScope: source.ProviderScope, ProviderRepoID: source.ProviderRepoID, ProviderOwner: source.ProviderOwner, ProviderName: source.ProviderName, BaseBranch: source.BaseBranch, CheckoutBranch: source.CheckoutBranch, DisplayName: source.DisplayName}) } return sources, nil } @@ -701,6 +703,8 @@ type httpTaskRepositoryInput struct { GitHubURL string `json:"github_url"` RemoteURL string `json:"remote_url"` Provider string `json:"provider"` + ProviderHost string `json:"provider_host"` + ProviderScope string `json:"provider_scope"` ProviderRepoID string `json:"provider_repo_id"` ProviderOwner string `json:"provider_owner"` ProviderName string `json:"provider_name"` @@ -1362,6 +1366,8 @@ func convertCreateTaskRepositories(c *gin.Context, inputs []httpTaskRepositoryIn GitHubURL: r.GitHubURL, RemoteURL: r.RemoteURL, Provider: r.Provider, + ProviderHost: r.ProviderHost, + ProviderScope: r.ProviderScope, ProviderRepoID: r.ProviderRepoID, ProviderOwner: r.ProviderOwner, ProviderName: r.ProviderName, @@ -1590,6 +1596,8 @@ func (h *TaskHandlers) httpUpdateTask(c *gin.Context) { GitHubURL: r.GitHubURL, RemoteURL: r.RemoteURL, Provider: r.Provider, + ProviderHost: r.ProviderHost, + ProviderScope: r.ProviderScope, ProviderRepoID: r.ProviderRepoID, ProviderOwner: r.ProviderOwner, ProviderName: r.ProviderName, diff --git a/apps/backend/internal/task/handlers/task_http_handlers_test.go b/apps/backend/internal/task/handlers/task_http_handlers_test.go index a68b5c815e..d895756000 100644 --- a/apps/backend/internal/task/handlers/task_http_handlers_test.go +++ b/apps/backend/internal/task/handlers/task_http_handlers_test.go @@ -101,12 +101,14 @@ func TestWorkspaceSourceHTTPStatusMapsRepositoryNotFound(t *testing.T) { } func TestParseHTTPWorkspaceSourcesPreservesSnakeCaseFields(t *testing.T) { - sources, err := parseHTTPWorkspaceSources([]json.RawMessage{json.RawMessage(`{"kind":"repository","repository_id":"repo-1","base_branch":"main","checkout_branch":"feature/x"}`)}) + sources, err := parseHTTPWorkspaceSources([]json.RawMessage{json.RawMessage(`{"kind":"repository","repository_id":"repo-1","base_branch":"main","checkout_branch":"feature/x","provider_host":"https://provider.example.test","provider_scope":"account-1"}`)}) require.NoError(t, err) require.Len(t, sources, 1) assert.Equal(t, "repo-1", sources[0].RepositoryID) assert.Equal(t, "main", sources[0].BaseBranch) assert.Equal(t, "feature/x", sources[0].CheckoutBranch) + assert.Equal(t, "https://provider.example.test", sources[0].ProviderHost) + assert.Equal(t, "account-1", sources[0].ProviderScope) } func TestQuickChatResolveParamsForcesWorktreeForRepositoryContext(t *testing.T) { diff --git a/apps/backend/internal/task/handlers/task_ws_handlers.go b/apps/backend/internal/task/handlers/task_ws_handlers.go index 842869dc3f..587fe95597 100644 --- a/apps/backend/internal/task/handlers/task_ws_handlers.go +++ b/apps/backend/internal/task/handlers/task_ws_handlers.go @@ -141,6 +141,8 @@ func (h *TaskHandlers) wsCreateTask(ctx context.Context, msg *ws.Message) (*ws.M GitHubURL: r.GitHubURL, RemoteURL: r.RemoteURL, Provider: r.Provider, + ProviderHost: r.ProviderHost, + ProviderScope: r.ProviderScope, ProviderRepoID: r.ProviderRepoID, ProviderOwner: r.ProviderOwner, ProviderName: r.ProviderName, @@ -192,6 +194,9 @@ func (h *TaskHandlers) wsCreateTask(ctx context.Context, msg *ws.Message) (*ws.M }) if err != nil { h.logger.Error("failed to create task", zap.Error(err)) + if code, ok := repositorySelectionWSCode(err); ok { + return ws.NewError(msg.ID, msg.Action, code, err.Error(), taskErrorDetails(err)) + } if errors.Is(err, service.ErrWIPLimitExceeded) { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeConflict, err.Error(), nil) } @@ -310,6 +315,8 @@ func (h *TaskHandlers) wsUpdateTask(ctx context.Context, msg *ws.Message) (*ws.M GitHubURL: r.GitHubURL, RemoteURL: r.RemoteURL, Provider: r.Provider, + ProviderHost: r.ProviderHost, + ProviderScope: r.ProviderScope, ProviderRepoID: r.ProviderRepoID, ProviderOwner: r.ProviderOwner, ProviderName: r.ProviderName, @@ -341,6 +348,9 @@ func (h *TaskHandlers) wsUpdateTask(ctx context.Context, msg *ws.Message) (*ws.M }) if err != nil { h.logger.Error("failed to update task", zap.Error(err)) + if code, ok := repositorySelectionWSCode(err); ok { + return ws.NewError(msg.ID, msg.Action, code, err.Error(), taskErrorDetails(err)) + } if isValidationError(err) { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, err.Error(), nil) } diff --git a/apps/backend/internal/task/service/repository_selection.go b/apps/backend/internal/task/service/repository_selection.go new file mode 100644 index 0000000000..7b2fb2c8bd --- /dev/null +++ b/apps/backend/internal/task/service/repository_selection.go @@ -0,0 +1,149 @@ +package service + +import ( + "context" + "errors" + "strings" +) + +// RepositorySelectionResolver verifies a first-use, plugin-owned repository +// URL and returns the complete trusted descriptor to persist. It must not +// perform repository or task writes. +type RepositorySelectionResolver interface { + ResolveRepositorySelection(context.Context, string, TaskRepositoryInput) (TaskRepositoryInput, error) +} + +// RepositorySelectionErrorCode is the stable transport classification for a +// failed server-side repository selection. +type RepositorySelectionErrorCode string + +const ( + RepositorySelectionErrorInvalid RepositorySelectionErrorCode = "repository_selection_invalid" + RepositorySelectionErrorNotFound RepositorySelectionErrorCode = "repository_selection_not_found" + RepositorySelectionErrorUnavailable RepositorySelectionErrorCode = "repository_selection_unavailable" +) + +// RepositorySelectionError is safe to expose to task transports. Its Error +// method is bounded and never includes plugin response data. +type RepositorySelectionError struct { + Code RepositorySelectionErrorCode + cause error +} + +func (e *RepositorySelectionError) Error() string { + switch e.Code { + case RepositorySelectionErrorInvalid: + return "The selected repository could not be verified." + case RepositorySelectionErrorNotFound: + return "The selected repository was not found." + case RepositorySelectionErrorUnavailable: + return "The repository provider is unavailable. Check the connection and try again." + default: + return "The selected repository could not be resolved." + } +} + +func (e *RepositorySelectionError) Unwrap() error { return e.cause } + +// NewRepositorySelectionError creates a safe typed error for adapters that +// translate provider-specific failures into the task-service contract. +func NewRepositorySelectionError(code RepositorySelectionErrorCode, cause error) error { + return &RepositorySelectionError{Code: code, cause: cause} +} + +func (s *Service) preflightRepositorySelections(ctx context.Context, req *CreateTaskRequest) error { + return s.preflightRepositoryInputs(ctx, req.WorkspaceID, req.Repositories) +} + +func (s *Service) preflightRepositoryInputs( + ctx context.Context, workspaceID string, inputs []TaskRepositoryInput, +) error { + for index, input := range inputs { + needsInspection, err := repositorySelectionNeedsInspection(input) + if err != nil { + return err + } + if !needsInspection { + continue + } + if s.repositorySelectionResolver == nil { + return NewRepositorySelectionError(RepositorySelectionErrorUnavailable, nil) + } + resolved, err := s.repositorySelectionResolver.ResolveRepositorySelection(ctx, workspaceID, input) + if err != nil { + return normalizeRepositorySelectionError(err) + } + resolved, err = trustResolvedRepositorySelection(input, resolved) + if err != nil { + return err + } + inputs[index] = resolved + } + return nil +} + +func repositorySelectionNeedsInspection(input TaskRepositoryInput) (bool, error) { + if input.RepositoryID != "" || input.TrustedProviderDescriptor || effectiveRemoteURL(input) == "" { + return false, nil + } + + provider := strings.TrimSpace(input.Provider) + if provider != "" && + !strings.EqualFold(provider, providerGitHub) && + !strings.EqualFold(provider, providerGitLab) && + !strings.EqualFold(provider, providerAzureDevOps) { + return true, nil + } + + parsedProvider, _, _, _, err := parseRemoteRepositoryURL(effectiveRemoteURL(input), provider) + if err != nil { + return false, NewRepositorySelectionError(RepositorySelectionErrorInvalid, nil) + } + if provider != "" && !strings.EqualFold(parsedProvider, provider) { + return false, NewRepositorySelectionError(RepositorySelectionErrorInvalid, nil) + } + return false, nil +} + +func trustResolvedRepositorySelection( + requested, resolved TaskRepositoryInput, +) (TaskRepositoryInput, error) { + if resolved.RepositoryID != "" || !resolved.TrustedProviderDescriptor { + return TaskRepositoryInput{}, NewRepositorySelectionError(RepositorySelectionErrorInvalid, nil) + } + if !strings.EqualFold(strings.TrimSpace(requested.Provider), strings.TrimSpace(resolved.Provider)) { + return TaskRepositoryInput{}, NewRepositorySelectionError(RepositorySelectionErrorInvalid, nil) + } + if hint := strings.TrimSpace(requested.ProviderScope); hint != "" && hint != strings.TrimSpace(resolved.ProviderScope) { + return TaskRepositoryInput{}, NewRepositorySelectionError(RepositorySelectionErrorInvalid, nil) + } + if hint := strings.TrimSpace(requested.ProviderRepoID); hint != "" && hint != strings.TrimSpace(resolved.ProviderRepoID) { + return TaskRepositoryInput{}, NewRepositorySelectionError(RepositorySelectionErrorInvalid, nil) + } + if err := validateTrustedRemoteRepository(resolved); err != nil || strings.TrimSpace(resolved.DefaultBranch) == "" { + return TaskRepositoryInput{}, NewRepositorySelectionError(RepositorySelectionErrorInvalid, err) + } + // Branches and policy selectors belong to the task request. The resolver + // may only replace provider identity and the exact clone URL. + resolved.RepositoryID = "" + resolved.BaseBranch = requested.BaseBranch + resolved.CheckoutBranch = requested.CheckoutBranch + resolved.BranchPolicyID = requested.BranchPolicyID + resolved.PRNumber = requested.PRNumber + resolved.RemoteContribution = requested.RemoteContribution + resolved.ContributionDestination = requested.ContributionDestination + resolved.PreserveBaseBranch = requested.PreserveBaseBranch + resolved.ResolveProviderDefaults = requested.ResolveProviderDefaults + resolved.TrustedProviderDescriptor = true + resolved.GitHubURL = "" + resolved.LocalPath = "" + return resolved, nil +} + +func normalizeRepositorySelectionError(err error) error { + var selectionErr *RepositorySelectionError + if errors.As(err, &selectionErr) { + return err + } + return NewRepositorySelectionError(RepositorySelectionErrorUnavailable, err) +} diff --git a/apps/backend/internal/task/service/repository_selection_test.go b/apps/backend/internal/task/service/repository_selection_test.go new file mode 100644 index 0000000000..19f7a5ebcb --- /dev/null +++ b/apps/backend/internal/task/service/repository_selection_test.go @@ -0,0 +1,310 @@ +package service + +import ( + "context" + "errors" + "strings" + "testing" + + "github.com/kandev/kandev/internal/task/models" +) + +type repositorySelectionResolverStub struct { + calls []TaskRepositoryInput + resolve func(TaskRepositoryInput) (TaskRepositoryInput, error) +} + +func (r *repositorySelectionResolverStub) ResolveRepositorySelection( + _ context.Context, _ string, input TaskRepositoryInput, +) (TaskRepositoryInput, error) { + r.calls = append(r.calls, input) + if r.resolve == nil { + return input, nil + } + return r.resolve(input) +} + +func TestCreateTaskPreflightsPluginRepositoryBeforePersistence(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + createRepositorySelectionWorkspace(t, repo) + resolver := &repositorySelectionResolverStub{resolve: authoritativeRepositoryInput} + svc.SetRepositorySelectionResolver(resolver) + + result, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: "plugin repo", + Repositories: []TaskRepositoryInput{{ + RemoteURL: "https://bitbucket.example.test/projects/TEAM/fixture", + Provider: "fixture-source-control", ProviderHost: "https://attacker.example.test", + ProviderRepoID: "repo-42", ProviderOwner: "attacker", ProviderName: "wrong", + DefaultBranch: "browser-default", BaseBranch: "main", CheckoutBranch: "feature/provider-contract", + }}, + }) + if err != nil { + t.Fatalf("CreateTask: %v", err) + } + if len(resolver.calls) != 1 || resolver.calls[0].ProviderOwner != "attacker" { + t.Fatalf("resolver calls = %+v, want one call with untrusted browser input", resolver.calls) + } + if result.Task == nil || len(result.Task.Repositories) != 1 { + t.Fatalf("created task repositories = %+v", result.Task) + } + stored, err := repo.GetRepository(ctx, result.Task.Repositories[0].RepositoryID) + if err != nil { + t.Fatalf("GetRepository: %v", err) + } + if stored.Provider != "fixture-source-control" || stored.ProviderHost != "https://bitbucket.example.test" || + stored.ProviderRepoID != "repo-42" || stored.ProviderOwner != "TEAM" || stored.ProviderName != "fixture" || + stored.RemoteURL != "https://bitbucket.example.test/scm/TEAM/fixture.git" || stored.DefaultBranch != "main" { + t.Fatalf("stored repository = %+v, want authoritative plugin identity", stored) + } +} + +func TestCreateTaskPluginRepositoryResolutionFailureLeavesNoWrites(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + createRepositorySelectionWorkspace(t, repo) + resolver := &repositorySelectionResolverStub{resolve: func(TaskRepositoryInput) (TaskRepositoryInput, error) { + return TaskRepositoryInput{}, errors.New("upstream plugin response contains secret details") + }} + svc.SetRepositorySelectionResolver(resolver) + + _, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: "plugin repo failure", + Repositories: []TaskRepositoryInput{{ + RemoteURL: "https://bitbucket.example.test/projects/TEAM/fixture", + Provider: "fixture-source-control", BaseBranch: "main", + }}, + }) + assertRepositorySelectionError(t, err, RepositorySelectionErrorUnavailable, "secret") + tasks, listErr := repo.ListTasks(ctx, "wf-1") + if listErr != nil { + t.Fatalf("ListTasks: %v", listErr) + } + if len(tasks) != 0 { + t.Fatalf("tasks after failed preflight = %d, want zero", len(tasks)) + } + repositories, listErr := repo.ListRepositories(ctx, "ws-1") + if listErr != nil { + t.Fatalf("ListRepositories: %v", listErr) + } + if len(repositories) != 0 { + t.Fatalf("repositories after failed preflight = %d, want zero", len(repositories)) + } +} + +func TestCreateTaskPreflightsEveryPluginRepositoryBeforeAnyWrite(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + createRepositorySelectionWorkspace(t, repo) + resolver := &repositorySelectionResolverStub{resolve: func(input TaskRepositoryInput) (TaskRepositoryInput, error) { + if input.RemoteURL == "https://bitbucket.example.test/projects/TEAM/second" { + return TaskRepositoryInput{}, errors.New("second provider unavailable") + } + return authoritativeRepositoryInput(input) + }} + svc.SetRepositorySelectionResolver(resolver) + + _, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: "multi plugin repos", + Repositories: []TaskRepositoryInput{ + {RemoteURL: "https://bitbucket.example.test/projects/TEAM/first", Provider: "fixture-source-control"}, + {RemoteURL: "https://bitbucket.example.test/projects/TEAM/second", Provider: "fixture-source-control"}, + }, + }) + assertRepositorySelectionError(t, err, RepositorySelectionErrorUnavailable, "") + if len(resolver.calls) != 2 { + t.Fatalf("resolver calls = %d, want both repository selections preflighted", len(resolver.calls)) + } + tasks, listErr := repo.ListTasks(ctx, "wf-1") + if listErr != nil { + t.Fatalf("ListTasks: %v", listErr) + } + if len(tasks) != 0 { + t.Fatalf("tasks after multi-repository preflight failure = %d, want zero", len(tasks)) + } +} + +func TestCreateTaskSettledExternalIDSkipsPluginRepositoryInspection(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + createRepositorySelectionWorkspace(t, repo) + first, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: "first", ExternalID: "external-3066", + }) + if err != nil { + t.Fatalf("first CreateTask: %v", err) + } + if first.Task == nil { + t.Fatal("first CreateTask returned no task") + } + resolver := &repositorySelectionResolverStub{resolve: func(TaskRepositoryInput) (TaskRepositoryInput, error) { + return TaskRepositoryInput{}, errors.New("inspection must not run") + }} + svc.SetRepositorySelectionResolver(resolver) + second, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: "retry", ExternalID: "external-3066", + Repositories: []TaskRepositoryInput{{RemoteURL: "https://bitbucket.example.test/TEAM/fixture", Provider: "fixture-source-control"}}, + }) + if err != nil { + t.Fatalf("retry CreateTask: %v", err) + } + if second.Task == nil || second.Task.ID != first.Task.ID || len(resolver.calls) != 0 { + t.Fatalf("retry result = %+v, resolver calls = %d", second, len(resolver.calls)) + } +} + +func TestCreateTaskTrustedPluginDescriptorSkipsRepositoryInspection(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + createRepositorySelectionWorkspace(t, repo) + resolver := &repositorySelectionResolverStub{resolve: func(TaskRepositoryInput) (TaskRepositoryInput, error) { + return TaskRepositoryInput{}, errors.New("trusted descriptor must not be inspected again") + }} + svc.SetRepositorySelectionResolver(resolver) + + input, err := authoritativeRepositoryInput(TaskRepositoryInput{BaseBranch: "main"}) + if err != nil { + t.Fatalf("authoritativeRepositoryInput: %v", err) + } + result, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: "trusted plugin task", + Repositories: []TaskRepositoryInput{input}, + }) + if err != nil { + t.Fatalf("CreateTask: %v", err) + } + if result.Task == nil || len(resolver.calls) != 0 { + t.Fatalf("result = %+v, resolver calls = %d, want trusted path without inspection", result, len(resolver.calls)) + } +} + +func TestCreateTaskRejectsUnresolvedPluginURLWithMissingOrBuiltinProviderBeforePersistence(t *testing.T) { + tests := []struct { + name string + provider string + }{ + {name: "missing provider", provider: ""}, + {name: "builtin provider", provider: providerGitHub}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + createRepositorySelectionWorkspace(t, repo) + + _, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: "invalid plugin selection", + Repositories: []TaskRepositoryInput{{ + RemoteURL: "https://bitbucket.example.test/projects/TEAM/fixture", + Provider: tt.provider, BaseBranch: "main", + }}, + }) + assertRepositorySelectionError(t, err, RepositorySelectionErrorInvalid, "") + + tasks, listErr := repo.ListTasks(ctx, "wf-1") + if listErr != nil { + t.Fatalf("ListTasks: %v", listErr) + } + if len(tasks) != 0 { + t.Fatalf("tasks after invalid selection = %d, want zero", len(tasks)) + } + repositories, listErr := repo.ListRepositories(ctx, "ws-1") + if listErr != nil { + t.Fatalf("ListRepositories: %v", listErr) + } + if len(repositories) != 0 { + t.Fatalf("repositories after invalid selection = %d, want zero", len(repositories)) + } + }) + } +} + +func TestUpdateTaskPreflightsPluginRepositoryBeforeReplacingExistingRepositories(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + createRepositorySelectionWorkspace(t, repo) + if err := repo.CreateRepository(ctx, &models.Repository{ID: "repo-existing", WorkspaceID: "ws-1", Name: "existing", DefaultBranch: "main"}); err != nil { + t.Fatalf("CreateRepository: %v", err) + } + created, err := svc.CreateTask(ctx, &CreateTaskRequest{ + WorkspaceID: "ws-1", WorkflowID: "wf-1", WorkflowStepID: "step-1", Title: "original", + Repositories: []TaskRepositoryInput{{RepositoryID: "repo-existing", BaseBranch: "main"}}, + }) + if err != nil { + t.Fatalf("CreateTask: %v", err) + } + + resolver := &repositorySelectionResolverStub{resolve: func(TaskRepositoryInput) (TaskRepositoryInput, error) { + return TaskRepositoryInput{}, errors.New("provider response contains a secret") + }} + svc.SetRepositorySelectionResolver(resolver) + updatedTitle := "must not be written" + _, err = svc.UpdateTask(ctx, created.Task.ID, &UpdateTaskRequest{ + Title: &updatedTitle, + Repositories: []TaskRepositoryInput{{ + RemoteURL: "https://bitbucket.example.test/projects/TEAM/fixture", + Provider: "fixture-source-control", BaseBranch: "main", + }}, + }) + assertRepositorySelectionError(t, err, RepositorySelectionErrorUnavailable, "secret") + if len(resolver.calls) != 1 { + t.Fatalf("resolver calls = %d, want one", len(resolver.calls)) + } + + storedTask, err := repo.GetTask(ctx, created.Task.ID) + if err != nil { + t.Fatalf("GetTask: %v", err) + } + if storedTask.Title != "original" { + t.Fatalf("stored title = %q, want original", storedTask.Title) + } + associated, err := repo.ListTaskRepositories(ctx, created.Task.ID) + if err != nil { + t.Fatalf("ListTaskRepositories: %v", err) + } + if len(associated) != 1 || associated[0].RepositoryID != "repo-existing" { + t.Fatalf("task repositories after failed update = %+v, want existing association", associated) + } +} + +func createRepositorySelectionWorkspace(t *testing.T, repo interface { + CreateWorkspace(context.Context, *models.Workspace) error + CreateWorkflow(context.Context, *models.Workflow) error +}) { + t.Helper() + ctx := context.Background() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-1", Name: "Workspace"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + if err := repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-1", WorkspaceID: "ws-1", Name: "Workflow"}); err != nil { + t.Fatalf("CreateWorkflow: %v", err) + } +} + +func authoritativeRepositoryInput(input TaskRepositoryInput) (TaskRepositoryInput, error) { + return TaskRepositoryInput{ + RemoteURL: "https://bitbucket.example.test/scm/TEAM/fixture.git", + Provider: "fixture-source-control", ProviderHost: "https://bitbucket.example.test", + ProviderScope: "workspace-a", ProviderRepoID: "repo-42", ProviderOwner: "TEAM", ProviderName: "fixture", + DefaultBranch: "main", BaseBranch: input.BaseBranch, CheckoutBranch: input.CheckoutBranch, + TrustedProviderDescriptor: true, + }, nil +} + +func assertRepositorySelectionError(t *testing.T, err error, want RepositorySelectionErrorCode, leaked string) { + t.Helper() + if err == nil { + t.Fatalf("expected repository selection error") + } + var selectionErr *RepositorySelectionError + if !errors.As(err, &selectionErr) { + t.Fatalf("error = %v, want RepositorySelectionError", err) + } + if selectionErr.Code != want { + t.Fatalf("error code = %q, want %q", selectionErr.Code, want) + } + if leaked != "" && strings.Contains(err.Error(), leaked) { + t.Fatalf("error leaked resolver detail: %v", err) + } +} diff --git a/apps/backend/internal/task/service/service.go b/apps/backend/internal/task/service/service.go index f7ec8f4e63..99975ca5e2 100644 --- a/apps/backend/internal/task/service/service.go +++ b/apps/backend/internal/task/service/service.go @@ -351,6 +351,7 @@ type Service struct { sshTaskDirReclaimer SSHTaskDirReclaimer sessionRunningChecker SessionRunningChecker remoteBranchLister RemoteBranchLister + repositorySelectionResolver RepositorySelectionResolver repoCloneLocation RepoCloneLocation blockers BlockerRepository // dependencyEdgeMu serializes validate-then-insert for dependency edges so @@ -644,6 +645,13 @@ func (s *Service) SetRemoteBranchLister(lister RemoteBranchLister) { s.remoteBranchLister = lister } +// SetRepositorySelectionResolver wires server-side inspection for first-use +// plugin repository selections. The resolver is optional for focused callers; +// plugin selections fail closed when it is not wired. +func (s *Service) SetRepositorySelectionResolver(resolver RepositorySelectionResolver) { + s.repositorySelectionResolver = resolver +} + // RepoCloneLocation reports the base path the orchestrator clones repos into // (e.g. ~/.kandev/repos or KANDEV_REPOCLONE_BASEPATH). Listing local branches // for a cloned repo requires that path to be allow-listed by diff --git a/apps/backend/internal/task/service/service_tasks.go b/apps/backend/internal/task/service/service_tasks.go index 86a2d11a84..bf43c08816 100644 --- a/apps/backend/internal/task/service/service_tasks.go +++ b/apps/backend/internal/task/service/service_tasks.go @@ -252,6 +252,9 @@ func (s *Service) prepareTaskForCreation(ctx context.Context, req *CreateTaskReq if err := s.inheritParentRepositories(ctx, req); err != nil { return nil, err } + if err := s.preflightRepositorySelections(ctx, req); err != nil { + return nil, err + } if err := s.validateTaskRepositoryPolicies(ctx, req.WorkspaceID, req.Repositories); err != nil { return nil, err } @@ -1605,6 +1608,11 @@ func (s *Service) UpdateTask(ctx context.Context, id string, req *UpdateTaskRequ if err != nil { return nil, err } + if req.Repositories != nil { + if err := s.preflightRepositoryInputs(ctx, task.WorkspaceID, req.Repositories); err != nil { + return nil, err + } + } oldWorkflowStepID := task.WorkflowStepID var oldState *v1.TaskState stateChanged := false diff --git a/apps/backend/internal/task/service/service_workspace_sources.go b/apps/backend/internal/task/service/service_workspace_sources.go index 816d404e0a..6559de5604 100644 --- a/apps/backend/internal/task/service/service_workspace_sources.go +++ b/apps/backend/internal/task/service/service_workspace_sources.go @@ -27,9 +27,9 @@ const ( ) type WorkspaceSourceInput struct { - Kind WorkspaceSourceKind - RepositoryID, LocalPath, GitHubURL, RemoteURL, Provider, ProviderRepoID, ProviderOwner, ProviderName string - BaseBranch, CheckoutBranch, DisplayName string + Kind WorkspaceSourceKind + RepositoryID, LocalPath, GitHubURL, RemoteURL, Provider, ProviderHost, ProviderScope, ProviderRepoID, ProviderOwner, ProviderName string + BaseBranch, CheckoutBranch, DisplayName string } type AttachWorkspaceSourcesRequest struct { @@ -391,7 +391,7 @@ func (s *Service) validateRepositoryWorkspaceSourceInput(ctx context.Context, ta } func (s *Service) resolveRepositoryWorkspaceSource(ctx context.Context, task *models.Task, input WorkspaceSourceInput) (string, string, string, error) { - id, base, created, err := s.ResolveRepositoryRef(ctx, task.WorkspaceID, TaskRepositoryInput{RepositoryID: input.RepositoryID, LocalPath: input.LocalPath, GitHubURL: input.GitHubURL, RemoteURL: input.RemoteURL, Provider: input.Provider, ProviderRepoID: input.ProviderRepoID, ProviderOwner: input.ProviderOwner, ProviderName: input.ProviderName, BaseBranch: input.BaseBranch, ResolveProviderDefaults: true}) + id, base, created, err := s.ResolveRepositoryRef(ctx, task.WorkspaceID, TaskRepositoryInput{RepositoryID: input.RepositoryID, LocalPath: input.LocalPath, GitHubURL: input.GitHubURL, RemoteURL: input.RemoteURL, Provider: input.Provider, ProviderHost: input.ProviderHost, ProviderScope: input.ProviderScope, ProviderRepoID: input.ProviderRepoID, ProviderOwner: input.ProviderOwner, ProviderName: input.ProviderName, BaseBranch: input.BaseBranch, ResolveProviderDefaults: true}) if err != nil { return "", "", "", classifyWorkspaceRepositoryError(err) } diff --git a/apps/backend/pkg/websocket/actions.go b/apps/backend/pkg/websocket/actions.go index 35d9e8d28e..aa089227b9 100644 --- a/apps/backend/pkg/websocket/actions.go +++ b/apps/backend/pkg/websocket/actions.go @@ -693,5 +693,6 @@ const ( ErrorCodeForbidden = "FORBIDDEN" ErrorCodeValidation = "VALIDATION_ERROR" ErrorCodeConflict = "CONFLICT" + ErrorCodeUnavailable = "UNAVAILABLE" ErrorCodeUnknownAction = "UNKNOWN_ACTION" ) diff --git a/apps/web/components/task-create-dialog-footer.tsx b/apps/web/components/task-create-dialog-footer.tsx index e2b78a97f1..2fdd009495 100644 --- a/apps/web/components/task-create-dialog-footer.tsx +++ b/apps/web/components/task-create-dialog-footer.tsx @@ -32,7 +32,7 @@ function UpdateButton({ isCreatingTask, hasTitle, onUpdate }: UpdateButtonProps) diff --git a/apps/web/components/task-create-dialog-submit.test.tsx b/apps/web/components/task-create-dialog-submit.test.tsx index 7e0aa0afe4..ad0b0c7c50 100644 --- a/apps/web/components/task-create-dialog-submit.test.tsx +++ b/apps/web/components/task-create-dialog-submit.test.tsx @@ -15,6 +15,7 @@ const RENAMED_TITLE = "Renamed task"; const ORIGINAL_PROMPT = "Original prompt"; const UPDATED_PROMPT = "Updated prompt"; const ORIGINAL_TITLE = "Original title"; +const RAW_REPOSITORY_PROVIDER_FAILURE = "raw repository provider failure"; const MAIN_BRANCH = "main"; const TODO_STATE = "TODO" as const; vi.mock("@/lib/routing/client-router", () => ({ @@ -107,7 +108,7 @@ vi.mock("@/components/task-create-dialog-fresh-branch-consent", () => ({ }), })); -import { useTaskSubmitHandlers } from "./task-create-dialog-submit"; +import { taskSubmitErrorMessage, useTaskSubmitHandlers } from "./task-create-dialog-submit"; import { readQueuedTaskCreateLastUsedState, resetTaskCreateLastUsedSync, @@ -367,6 +368,91 @@ describe("useTaskSubmitHandlers — started task edits", () => { }); }); +describe("useTaskSubmitHandlers — repository selection edit failures", () => { + const failures = [ + { + code: "repository_selection_invalid", + description: "The selected repository could not be verified.", + }, + { + code: "repository_selection_not_found", + description: "The selected repository was not found.", + }, + { + code: "repository_selection_unavailable", + description: "The repository provider is unavailable. Check the connection and try again.", + }, + ] as const; + + for (const failure of failures) { + it(`keeps the dialog open for ${failure.code} in the edit submit handler`, async () => { + const onOpenChange = vi.fn(); + updateTaskMock.mockRejectedValueOnce( + new ApiError(RAW_REPOSITORY_PROVIDER_FAILURE, 400, { + error: RAW_REPOSITORY_PROVIDER_FAILURE, + error_code: failure.code, + }), + ); + const deps = makeDeps({ + isEditMode: true, + taskName: ORIGINAL_TITLE, + editingTask: { + id: TASK_ID, + title: ORIGINAL_TITLE, + description: ORIGINAL_PROMPT, + workflowStepId: "step-1", + state: TODO_STATE, + }, + descriptionInputRef: makeRef(ORIGINAL_PROMPT), + onOpenChange, + }); + const { result } = renderHook(() => useTaskSubmitHandlers(deps)); + + await act(async () => { + await result.current.handleSubmit({ preventDefault: () => {} } as never); + }); + + expect(onOpenChange).not.toHaveBeenCalled(); + expect(toastMock).toHaveBeenCalledWith( + expect.objectContaining({ description: failure.description }), + ); + }); + + it(`keeps the dialog open for ${failure.code} in the update-only handler`, async () => { + const onOpenChange = vi.fn(); + updateTaskMock.mockRejectedValueOnce( + new ApiError(RAW_REPOSITORY_PROVIDER_FAILURE, 400, { + error: RAW_REPOSITORY_PROVIDER_FAILURE, + error_code: failure.code, + }), + ); + const deps = makeDeps({ + isEditMode: true, + taskName: ORIGINAL_TITLE, + editingTask: { + id: TASK_ID, + title: ORIGINAL_TITLE, + description: ORIGINAL_PROMPT, + workflowStepId: "step-1", + state: TODO_STATE, + }, + descriptionInputRef: makeRef(ORIGINAL_PROMPT), + onOpenChange, + }); + const { result } = renderHook(() => useTaskSubmitHandlers(deps)); + + await act(async () => { + await result.current.handleUpdateWithoutAgent(); + }); + + expect(onOpenChange).not.toHaveBeenCalled(); + expect(toastMock).toHaveBeenCalledWith( + expect.objectContaining({ description: failure.description }), + ); + }); + } +}); + // eslint-disable-next-line max-lines-per-function -- create-mode parity cases share transport setup. describe("useTaskSubmitHandlers — handleCreateSubmit (CLI-mode parity)", () => { it("refreshes stale policy options and keeps the dialog open", async () => { @@ -534,3 +620,20 @@ describe("useTaskSubmitHandlers — handleCreateWithoutAgent", () => { ); }); }); + +describe("taskSubmitErrorMessage", () => { + it("uses localized copy for repository selection error codes", () => { + expect( + taskSubmitErrorMessage( + new ApiError("raw upstream provider failure", 503, { + error: "raw upstream provider failure", + error_code: "repository_selection_unavailable", + }), + ), + ).toBe("The repository provider is unavailable. Check the connection and try again."); + }); + + it("preserves non-repository error messages", () => { + expect(taskSubmitErrorMessage(new Error("ordinary failure"))).toBe("ordinary failure"); + }); +}); diff --git a/apps/web/components/task-create-dialog-submit.tsx b/apps/web/components/task-create-dialog-submit.tsx index 9964419eae..8b7e846f28 100644 --- a/apps/web/components/task-create-dialog-submit.tsx +++ b/apps/web/components/task-create-dialog-submit.tsx @@ -67,6 +67,26 @@ function isStaleBranchPolicyError(error: unknown): boolean { ); } +const REPOSITORY_SELECTION_ERROR_KEYS: Record = { + repository_selection_invalid: "task:repositorySelectionInvalid", + repository_selection_not_found: "task:repositorySelectionNotFound", + repository_selection_unavailable: "task:repositorySelectionUnavailable", +}; + +export function taskSubmitErrorMessage(error: unknown): string { + if (error instanceof ApiError) { + const key = REPOSITORY_SELECTION_ERROR_KEYS[error.errorCode ?? ""]; + if (key) return t(key); + } + return error instanceof Error ? error.message : t(GENERIC_ERROR_KEY); +} + +function isRepositorySelectionError(error: unknown): boolean { + return ( + error instanceof ApiError && Boolean(REPOSITORY_SELECTION_ERROR_KEYS[error.errorCode ?? ""]) + ); +} + // eslint-disable-next-line max-lines-per-function export function useTaskSubmitHandlers({ isSessionMode, @@ -303,7 +323,7 @@ export function useTaskSubmitHandlers({ } catch (error) { toast({ title: t("task:failedToCreateSession"), - description: error instanceof Error ? error.message : t(GENERIC_ERROR_KEY), + description: taskSubmitErrorMessage(error), variant: "error", }); } finally { @@ -376,10 +396,12 @@ export function useTaskSubmitHandlers({ onSuccess?.(updatedTask, "edit", { taskSessionId }); } catch (error) { - closeDialog = !(await refreshStaleBranchPolicies(error)); + closeDialog = !( + isRepositorySelectionError(error) || (await refreshStaleBranchPolicies(error)) + ); toast({ title: t("task:failedToUpdateTask"), - description: error instanceof Error ? error.message : t(GENERIC_ERROR_KEY), + description: taskSubmitErrorMessage(error), variant: "error", }); } finally { @@ -408,10 +430,12 @@ export function useTaskSubmitHandlers({ if (!result) return; onSuccess?.(result.updatedTask, "edit"); } catch (error) { - closeDialog = !(await refreshStaleBranchPolicies(error)); + closeDialog = !( + isRepositorySelectionError(error) || (await refreshStaleBranchPolicies(error)) + ); toast({ title: t("task:failedToUpdateTask"), - description: error instanceof Error ? error.message : t(GENERIC_ERROR_KEY), + description: taskSubmitErrorMessage(error), variant: "error", }); } finally { @@ -574,7 +598,7 @@ export function useTaskSubmitHandlers({ await refreshStaleBranchPolicies(error); toast({ title: t("task:failedToStartTaskPlanMode"), - description: error instanceof Error ? error.message : t(GENERIC_ERROR_KEY), + description: taskSubmitErrorMessage(error), variant: "error", }); } finally { @@ -606,7 +630,7 @@ export function useTaskSubmitHandlers({ await refreshStaleBranchPolicies(error); toast({ title: t("task:failedToStartTaskPlanMode"), - description: error instanceof Error ? error.message : t(GENERIC_ERROR_KEY), + description: taskSubmitErrorMessage(error), variant: "error", }); } finally { @@ -676,7 +700,7 @@ export function useTaskSubmitHandlers({ await refreshStaleBranchPolicies(error); toast({ title: t("task:failedToCreateTask"), - description: error instanceof Error ? error.message : t(GENERIC_ERROR_KEY), + description: taskSubmitErrorMessage(error), variant: "error", }); } finally { @@ -746,7 +770,7 @@ export function useTaskSubmitHandlers({ await refreshStaleBranchPolicies(error); toast({ title: t("task:failedToCreateTask"), - description: error instanceof Error ? error.message : t(GENERIC_ERROR_KEY), + description: taskSubmitErrorMessage(error), variant: "error", }); } finally { diff --git a/apps/web/e2e/tests/task/mobile-plugin-repository-task-create.spec.ts b/apps/web/e2e/tests/task/mobile-plugin-repository-task-create.spec.ts new file mode 100644 index 0000000000..8dbaf56801 --- /dev/null +++ b/apps/web/e2e/tests/task/mobile-plugin-repository-task-create.spec.ts @@ -0,0 +1,130 @@ +import type { Page } from "@playwright/test"; +import { expect, test } from "../../fixtures/test-base"; +import { installFixturePlugin, PLUGIN_ID } from "../../helpers/plugin-fixture"; +import type { ApiClient } from "../../helpers/api-client"; +import { MobileKanbanPage } from "../../pages/mobile-kanban-page"; +import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions"; + +const FIXTURE_PROVIDER = "fixture-source-control"; +const FIXTURE_REPOSITORY_ID = "fixture-repository"; +const FIXTURE_BRANCH = "feature/provider-contract"; + +type RepositorySummary = { + id: string; + provider?: string; + provider_repo_id?: string; +}; + +async function fixtureRepositories(apiClient: ApiClient, workspaceId: string) { + const response = await apiClient.rawRequest( + "GET", + `/api/v1/workspaces/${workspaceId}/repositories`, + ); + const bodyText = await response.text(); + expect(response.ok, bodyText).toBe(true); + const body = JSON.parse(bodyText) as { repositories?: RepositorySummary[] }; + return (body.repositories ?? []).filter( + (repository) => + repository.provider === FIXTURE_PROVIDER && + repository.provider_repo_id === FIXTURE_REPOSITORY_ID, + ); +} + +async function removeFixtureRepositories(apiClient: ApiClient, workspaceId: string): Promise { + for (const repository of await fixtureRepositories(apiClient, workspaceId)) { + await apiClient + .rawRequest("DELETE", `/api/v1/repositories/${repository.id}`) + .catch(() => undefined); + } +} + +async function selectFixtureRepository(page: Page): Promise { + await page.getByTestId("source-mode-remote").tap(); + const repositoryTrigger = page.getByTestId("remote-repo-chip-trigger"); + await expect(repositoryTrigger).toHaveCount(1); + await repositoryTrigger.tap(); + const repositoryOption = page + .getByTestId("remote-repo-option") + .filter({ hasText: "TEAM/fixture" }); + await expect(repositoryOption).toHaveCount(1); + await expect(repositoryOption).toBeVisible({ timeout: 15_000 }); + await repositoryOption.tap(); + await expect(repositoryTrigger).toContainText("TEAM/fixture"); +} + +async function selectFixtureBranch(page: Page): Promise { + const branch = page.getByTestId("remote-branch-chip-trigger"); + await expect(branch).toHaveCount(1); + await expect(branch).toBeEnabled({ timeout: 15_000 }); + await branch.tap(); + const option = page.getByRole("option", { name: FIXTURE_BRANCH, exact: false }); + await expect(option).toHaveCount(1); + await expect(option).toBeVisible({ timeout: 15_000 }); + await option.tap(); + await expect(branch).toContainText(FIXTURE_BRANCH); +} + +test.describe("first-use plugin repository task creation on mobile", () => { + test.afterEach(async ({ apiClient, seedData }) => { + await apiClient.rawRequest("DELETE", `/api/plugins/${PLUGIN_ID}`).catch(() => undefined); + await removeFixtureRepositories(apiClient, seedData.workspaceId); + }); + + test("creates the same authoritative repository through the phone dialog", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + await removeFixtureRepositories(apiClient, seedData.workspaceId); + expect(await fixtureRepositories(apiClient, seedData.workspaceId)).toHaveLength(0); + await installFixturePlugin(testPage); + + const mobile = new MobileKanbanPage(testPage); + await mobile.goto(); + await mobile.mobileFab.tap(); + const dialog = testPage.getByTestId("create-task-dialog"); + await expect(dialog).toBeVisible(); + await selectFixtureRepository(testPage); + await selectFixtureBranch(testPage); + await dialog.getByTestId("task-title-input").fill("Mobile first-use plugin repository task"); + await dialog + .getByTestId("task-description-input") + .fill("Create this task from a plugin repository on a phone."); + + const createOnly = dialog.getByRole("button", { name: "Create only", exact: true }); + await expect(createOnly).toBeEnabled(); + const createBox = await createOnly.boundingBox(); + expect(createBox).not.toBeNull(); + expect(createBox!.height).toBeGreaterThanOrEqual(44); + expect(createBox!.width).toBeGreaterThanOrEqual(44); + await assertNoDocumentHorizontalOverflow( + testPage, + "plugin repository task dialog before submit", + ); + + const responsePromise = testPage.waitForResponse( + (response) => + response.url().endsWith("/api/v1/tasks") && response.request().method() === "POST", + ); + await createOnly.tap(); + const response = await responsePromise; + const responseBody = await response.text(); + expect(response.status(), responseBody).toBe(200); + const created = JSON.parse(responseBody) as { id: string }; + expect(created.id).toBeTruthy(); + await expect(dialog).toBeHidden({ timeout: 15_000 }); + await assertNoDocumentHorizontalOverflow( + testPage, + "plugin repository task dialog after submit", + ); + + const task = await apiClient.getTask(created.id); + expect(task.repositories).toHaveLength(1); + expect(task.repositories?.[0]).toMatchObject({ + base_branch: "main", + checkout_branch: FIXTURE_BRANCH, + }); + expect(await fixtureRepositories(apiClient, seedData.workspaceId)).toHaveLength(1); + }); +}); diff --git a/apps/web/e2e/tests/task/plugin-repository-task-create.spec.ts b/apps/web/e2e/tests/task/plugin-repository-task-create.spec.ts new file mode 100644 index 0000000000..b333c3ad00 --- /dev/null +++ b/apps/web/e2e/tests/task/plugin-repository-task-create.spec.ts @@ -0,0 +1,181 @@ +import type { Page } from "@playwright/test"; +import { expect, test } from "../../fixtures/test-base"; +import { installFixturePlugin, PLUGIN_ID } from "../../helpers/plugin-fixture"; +import type { ApiClient } from "../../helpers/api-client"; +import { KanbanPage } from "../../pages/kanban-page"; + +const FIXTURE_PROVIDER = "fixture-source-control"; +const FIXTURE_REPOSITORY_ID = "fixture-repository"; +const FIXTURE_REPOSITORY_URL = "https://bitbucket.example.test/projects/TEAM/repos/fixture"; +const FIXTURE_BRANCH = "feature/provider-contract"; + +type RepositorySummary = { + id: string; + provider?: string; + provider_repo_id?: string; +}; + +async function fixtureRepositories(apiClient: ApiClient, workspaceId: string) { + const response = await apiClient.rawRequest( + "GET", + `/api/v1/workspaces/${workspaceId}/repositories`, + ); + const bodyText = await response.text(); + expect(response.ok, bodyText).toBe(true); + const body = JSON.parse(bodyText) as { repositories?: RepositorySummary[] }; + return (body.repositories ?? []).filter( + (repository) => + repository.provider === FIXTURE_PROVIDER && + repository.provider_repo_id === FIXTURE_REPOSITORY_ID, + ); +} + +async function removeFixtureRepositories(apiClient: ApiClient, workspaceId: string): Promise { + for (const repository of await fixtureRepositories(apiClient, workspaceId)) { + const response = await apiClient.rawRequest("DELETE", `/api/v1/repositories/${repository.id}`); + if (!response.ok && response.status !== 404) { + throw new Error( + `fixture repository cleanup failed (${response.status}): ${await response.text()}`, + ); + } + } +} + +async function selectFixtureRepository(page: Page): Promise { + await page.getByTestId("source-mode-remote").click(); + const repositoryTrigger = page.getByTestId("remote-repo-chip-trigger"); + await expect(repositoryTrigger).toHaveCount(1); + await repositoryTrigger.click(); + const repositoryOption = page + .getByTestId("remote-repo-option") + .filter({ hasText: "TEAM/fixture" }); + await expect(repositoryOption).toHaveCount(1); + await expect(repositoryOption).toBeVisible({ timeout: 15_000 }); + await repositoryOption.click(); + await expect(repositoryTrigger).toContainText("TEAM/fixture"); +} + +async function selectFixtureBranch(page: Page): Promise { + const branch = page.getByTestId("remote-branch-chip-trigger"); + await expect(branch).toHaveCount(1); + await expect(branch).toBeEnabled({ timeout: 15_000 }); + await branch.click(); + const option = page.getByRole("option", { name: FIXTURE_BRANCH, exact: false }); + await expect(option).toHaveCount(1); + await expect(option).toBeVisible({ timeout: 15_000 }); + await option.click(); + await expect(branch).toContainText(FIXTURE_BRANCH); +} + +async function fillTaskForm(page: Page, title: string): Promise { + await page.getByTestId("task-title-input").fill(title); + await page + .getByTestId("task-description-input") + .fill("Create this task from a plugin repository."); +} + +async function createWithoutAgent(page: Page): Promise { + const responsePromise = page.waitForResponse( + (response) => + response.url().endsWith("/api/v1/tasks") && response.request().method() === "POST", + ); + await page.getByTestId("submit-start-agent-chevron").click(); + await page.getByTestId("submit-create-without-agent").click(); + return responsePromise; +} + +test.describe("first-use plugin repository task creation", () => { + test.afterEach(async ({ apiClient, seedData }) => { + await apiClient.rawRequest("DELETE", `/api/plugins/${PLUGIN_ID}`).catch(() => undefined); + await removeFixtureRepositories(apiClient, seedData.workspaceId).catch(() => undefined); + }); + + test("creates and persists the authoritative repository on desktop", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + await removeFixtureRepositories(apiClient, seedData.workspaceId); + expect(await fixtureRepositories(apiClient, seedData.workspaceId)).toHaveLength(0); + await installFixturePlugin(testPage); + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + await kanban.createTaskButton.first().click(); + const dialog = testPage.getByTestId("create-task-dialog"); + await expect(dialog).toBeVisible(); + await selectFixtureRepository(testPage); + await selectFixtureBranch(testPage); + await fillTaskForm(testPage, "First-use plugin repository task"); + + const response = await createWithoutAgent(testPage); + const responseBody = await response.text(); + expect(response.status(), responseBody).toBe(200); + const created = JSON.parse(responseBody) as { id: string }; + expect(created.id).toBeTruthy(); + await expect(dialog).toBeHidden({ timeout: 15_000 }); + + const task = await apiClient.getTask(created.id); + expect(task.repositories).toHaveLength(1); + expect(task.repositories?.[0]).toMatchObject({ + base_branch: "main", + checkout_branch: FIXTURE_BRANCH, + }); + const repositories = await fixtureRepositories(apiClient, seedData.workspaceId); + expect(repositories).toHaveLength(1); + const repositoryResponse = await apiClient.rawRequest( + "GET", + `/api/v1/repositories/${repositories[0]!.id}`, + ); + const repositoryBody = await repositoryResponse.text(); + expect(repositoryResponse.ok, repositoryBody).toBe(true); + expect(JSON.parse(repositoryBody)).toMatchObject({ + provider: FIXTURE_PROVIDER, + provider_repo_id: FIXTURE_REPOSITORY_ID, + provider_owner: "TEAM", + provider_name: "fixture", + remote_url: "https://bitbucket.example.test/scm/TEAM/fixture.git", + default_branch: "main", + }); + }); + + test("keeps the form open when the provider becomes inactive", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + await removeFixtureRepositories(apiClient, seedData.workspaceId); + expect(await fixtureRepositories(apiClient, seedData.workspaceId)).toHaveLength(0); + await installFixturePlugin(testPage); + + const kanban = new KanbanPage(testPage); + await kanban.goto(); + await kanban.createTaskButton.first().click(); + const dialog = testPage.getByTestId("create-task-dialog"); + await expect(dialog).toBeVisible(); + await selectFixtureRepository(testPage); + await selectFixtureBranch(testPage); + const title = "Inactive plugin repository task"; + await fillTaskForm(testPage, title); + + const disable = await apiClient.rawRequest("POST", `/api/plugins/${PLUGIN_ID}/disable`); + expect(disable.ok, await disable.text()).toBe(true); + const response = await createWithoutAgent(testPage); + expect(response.status()).toBe(503); + await expect(dialog).toBeVisible(); + await expect( + testPage.getByText( + "The repository provider is unavailable. Check the connection and try again.", + { + exact: true, + }, + ), + ).toBeVisible(); + const tasks = await apiClient.listTasks(seedData.workspaceId); + expect(tasks.tasks.some((task) => task.title === title)).toBe(false); + expect(await fixtureRepositories(apiClient, seedData.workspaceId)).toHaveLength(0); + expect(testPage.url()).not.toContain(FIXTURE_REPOSITORY_URL); + }); +}); diff --git a/apps/web/src/locales/en/task.json b/apps/web/src/locales/en/task.json index c50d36c560..86d1603ed8 100644 --- a/apps/web/src/locales/en/task.json +++ b/apps/web/src/locales/en/task.json @@ -1701,6 +1701,9 @@ "repositoryStillBeingVerified": "Repository is still being verified", "repositoryProviderVerificationFailed": "Kandev could not verify this provider URL. Use Retry on the repository row.", "repositoryProviderVerificationPending": "Wait for provider inspection to finish before creating the task.", + "repositorySelectionInvalid": "The selected repository could not be verified.", + "repositorySelectionNotFound": "The selected repository was not found.", + "repositorySelectionUnavailable": "The repository provider is unavailable. Check the connection and try again.", "failedToCreateFile": "Failed to create file", "failedToCreateSession": "Failed to create session", "failedToCreateTask": "Failed to create task", diff --git a/apps/web/src/locales/pseudo/task.json b/apps/web/src/locales/pseudo/task.json index 816076ffe8..d264b73f75 100644 --- a/apps/web/src/locales/pseudo/task.json +++ b/apps/web/src/locales/pseudo/task.json @@ -1701,6 +1701,9 @@ "repositoryStillBeingVerified": "Ŕēƥōśĩţōŕŷ ĩś śţĩĺĺ ƀēĩńĝ vēŕĩƒĩēď", "repositoryProviderVerificationFailed": "Ķàńďēv ćōũĺď ńōţ vēŕĩƒŷ ţĥĩś ƥŕōvĩďēŕ ŨŔĹ. Ũśē Ŕēţŕŷ ōń ţĥē ŕēƥōśĩţōŕŷ ŕōŵ.", "repositoryProviderVerificationPending": "Ŵàĩţ ƒōŕ ƥŕōvĩďēŕ ĩńśƥēćţĩōń ţō ƒĩńĩśĥ ƀēƒōŕē ćŕēàţĩńĝ ţĥē ţàśķ.", + "repositorySelectionInvalid": "Ţĥē śēĺēćţēď ŕēƥōśĩţōŕŷ ćōũĺď ńōţ ƀē vēŕĩƒĩēď.", + "repositorySelectionNotFound": "Ţĥē śēĺēćţēď ŕēƥōśĩţōŕŷ ŵàś ńōţ ƒōũńď.", + "repositorySelectionUnavailable": "Ţĥē ŕēƥōśĩţōŕŷ ƥŕōvĩďēŕ ĩś ũńàvàĩĺàƀĺē. Ćĥēćķ ţĥē ćōńńēćţĩōń àńď ţŕŷ àĝàĩń.", "failedToCreateFile": "Ƒàĩĺēď ţō ćŕēàţē ƒĩĺē", "failedToCreateSession": "Ƒàĩĺēď ţō ćŕēàţē śēśśĩōń", "failedToCreateTask": "Ƒàĩĺēď ţō ćŕēàţē ţàśķ", diff --git a/apps/web/src/locales/pt-pt/task.json b/apps/web/src/locales/pt-pt/task.json index 1fe105513a..b4e44a576e 100644 --- a/apps/web/src/locales/pt-pt/task.json +++ b/apps/web/src/locales/pt-pt/task.json @@ -1656,6 +1656,9 @@ "repositoryStillBeingVerified": "O repositório ainda está a ser verificado", "repositoryProviderVerificationFailed": "O Kandev não conseguiu verificar este URL do fornecedor. Use Repetir na linha do repositório.", "repositoryProviderVerificationPending": "Aguarde que a inspeção do fornecedor termine antes de criar a tarefa.", + "repositorySelectionInvalid": "Não foi possível verificar o repositório selecionado.", + "repositorySelectionNotFound": "O repositório selecionado não foi encontrado.", + "repositorySelectionUnavailable": "O fornecedor do repositório não está disponível. Verifique a ligação e tente novamente.", "failedToCreateFile": "Falha ao criar o ficheiro", "failedToCreateSession": "Falha ao criar a sessão", "failedToCreateTask": "Falha ao criar a tarefa", diff --git a/apps/web/src/locales/zh-cn/task.json b/apps/web/src/locales/zh-cn/task.json index 9c7e6cd2b5..a3bea9dc56 100644 --- a/apps/web/src/locales/zh-cn/task.json +++ b/apps/web/src/locales/zh-cn/task.json @@ -1655,6 +1655,9 @@ "repositoryStillBeingVerified": "仍在验证仓库", "repositoryProviderVerificationFailed": "Kandev 无法验证此提供商 URL。请在仓库行中使用重试。", "repositoryProviderVerificationPending": "请等待提供商检查完成后再创建任务。", + "repositorySelectionInvalid": "无法验证所选仓库。", + "repositorySelectionNotFound": "找不到所选仓库。", + "repositorySelectionUnavailable": "仓库提供商不可用。请检查连接后重试。", "failedToCreateFile": "创建文件失败", "failedToCreateSession": "创建会话失败", "failedToCreateTask": "创建任务失败", diff --git a/apps/web/src/locales/zh-hk/task.json b/apps/web/src/locales/zh-hk/task.json index f73a73c01e..9a2fb36bd6 100644 --- a/apps/web/src/locales/zh-hk/task.json +++ b/apps/web/src/locales/zh-hk/task.json @@ -1655,6 +1655,9 @@ "repositoryStillBeingVerified": "仍在驗證儲存庫", "repositoryProviderVerificationFailed": "Kandev 無法驗證此提供者 URL。請在儲存庫行中使用重試。", "repositoryProviderVerificationPending": "請等待提供者檢查完成後再建立任務。", + "repositorySelectionInvalid": "無法驗證所選儲存庫。", + "repositorySelectionNotFound": "找不到所選儲存庫。", + "repositorySelectionUnavailable": "儲存庫提供者不可用。請檢查連線後重試。", "failedToCreateFile": "建立檔案失敗", "failedToCreateSession": "建立工作階段失敗", "failedToCreateTask": "建立任務失敗", diff --git a/apps/web/src/locales/zh-tw/task.json b/apps/web/src/locales/zh-tw/task.json index f79061b4a4..608136b173 100644 --- a/apps/web/src/locales/zh-tw/task.json +++ b/apps/web/src/locales/zh-tw/task.json @@ -1655,6 +1655,9 @@ "repositoryStillBeingVerified": "仍在驗證儲存庫", "repositoryProviderVerificationFailed": "Kandev 無法驗證此提供者 URL。請在儲存庫行中使用重試。", "repositoryProviderVerificationPending": "請等待提供者檢查完成後再建立任務。", + "repositorySelectionInvalid": "無法驗證所選儲存庫。", + "repositorySelectionNotFound": "找不到所選儲存庫。", + "repositorySelectionUnavailable": "儲存庫提供者不可用。請檢查連線後重試。", "failedToCreateFile": "建立檔案失敗", "failedToCreateSession": "建立工作階段失敗", "failedToCreateTask": "建立任務失敗", diff --git a/docs/decisions/2026-08-26-server-owned-plugin-repository-task-resolution.md b/docs/decisions/2026-08-26-server-owned-plugin-repository-task-resolution.md new file mode 100644 index 0000000000..39d99c6366 --- /dev/null +++ b/docs/decisions/2026-08-26-server-owned-plugin-repository-task-resolution.md @@ -0,0 +1,88 @@ +# ADR-2026-08-26-server-owned-plugin-repository-task-resolution: Resolve First-Use Plugin Repositories on the Server + +**Status:** accepted +**Date:** 2026-08-26 +**Area:** backend, frontend, protocol, security, plugins + +## Context + +The native task dialog can list a repository from a plugin before Kandev has a +repository row for it. The browser currently submits a complete provider +descriptor. Normal REST task creation correctly refuses to trust that +descriptor, then sends the URL through the built-in parser. A Bitbucket URL is +unsupported there. The failure happens after the task row is inserted, so the +user sees an error and an empty task remains. + +The browser cannot become repository authority. If a request could choose both +the provider host and clone URL, a later credential lookup could send managed +credentials to an attacker-controlled host. Requiring a separate registration +step would preserve the trust boundary but would break the native first-use +workflow already presented by the plugin picker. + +The active code-host plugin already owns the external connection and can +inspect a repository URL within verified workspace context. + +## Decision + +Kandev will resolve a first-use plugin repository on the backend through a +standard workspace-scoped `repositories.inspect` action owned by the active +plugin manifest. + +Task-create request fields are selection hints only. Kandev uses the provider +ID to find the active manifest owner and forwards the untrusted URL to that +plugin under server-verified workspace context. The plugin proves the URL +belongs to its configured connection and returns a complete, credential-free +repository descriptor. Kandev validates provider ownership, immutable +repository identity, provider scope, HTTPS clone origin, and response bounds. +Only this validated server result can enter the existing trusted-descriptor +repository resolver. + +Kandev resolves and validates every first-use plugin selection before it +creates a task or repository row. An inactive provider, unavailable action, +timeout, no-match result, or invalid response returns a typed, bounded error +without partial persistence. + +Existing repository IDs and built-in provider URL parsing remain unchanged. +The authenticated plugin Host `Tasks.Create` path also remains valid because +the plugin server, not the browser, supplies its descriptor. REST, WebSocket, +and MCP callers cannot set the internal trusted-descriptor marker. + +## Consequences + +Users can create a task directly from a repository listed by a connected +Bitbucket or future code-host plugin. The normal native task dialog remains the +single workflow on desktop and phone. + +Plugin authors that support first-use task creation must implement the +workspace-scoped inspect action. Already-persisted plugin repositories continue +to work without it. Kandev gains one backend adapter between task creation and +the plugin service, but keeps credentials and provider-specific API logic out +of the task service. + +Task creation now waits for one bounded provider inspection per first-use +plugin repository. Multi-repository inspection is sequential initially so +error order is deterministic and one plugin connection is not flooded. + +This decision removes the known provider-resolution partial-write case. It +does not make every later task-create write transactional. + +## Alternatives Considered + +- Trust the complete repository descriptor from the browser. Rejected because + a forged provider host and clone URL can redirect managed credentials. +- Require users to register the repository before opening the task dialog. + Rejected because the picker already offers a direct first-use workflow and + the extra step is unnecessary when the connected plugin can resolve it. +- Require each plugin to replace normal REST task creation with an + authenticated plugin action. Rejected because it duplicates the host's + native creation transport and makes provider behavior depend on which screen + opened the same task dialog. +- Teach the built-in URL parser about Bitbucket and every future plugin host. + Rejected because host-specific parsing and connection authority belong to + the plugin, including self-managed origins and immutable provider IDs. + +## References + +- [Issue #3066](https://github.com/kdlbs/kandev/issues/3066) +- [Plugin Repository Task Creation Requirements](../specs/plugins/requirements/repository-provider-task-creation.md) +- [Plugin Repository Task Creation System Design](../specs/plugins/system-design/repository-provider-task-creation.md) diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 6b7ba8e5f7..16f9fd9b8b 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -109,6 +109,7 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 2026-07-31-authenticated-plugin-actions | [Authenticated Plugin Actions](2026-07-31-authenticated-plugin-actions.md) | accepted | backend, frontend, protocol, security | 2026-07-31 | | 2026-07-31-plugin-repository-provider-extensions | [Plugin Repository Provider Extensions](2026-07-31-plugin-repository-provider-extensions.md) | accepted | frontend, backend, protocol | 2026-07-31 | | 2026-07-31-provider-neutral-git-credential-broker | [Provider-Neutral Git Credential Broker](2026-07-31-provider-neutral-git-credential-broker.md) | accepted | backend, protocol, security | 2026-07-31 | +| 2026-08-26-server-owned-plugin-repository-task-resolution | [Resolve First-Use Plugin Repositories on the Server](2026-08-26-server-owned-plugin-repository-task-resolution.md) | accepted | backend, frontend, protocol, security, plugins | 2026-08-26 | | 2026-08-01-bitbucket-initial-release-remains-unsigned | [Bitbucket Initial Release Remains Unsigned](2026-08-01-bitbucket-initial-release-remains-unsigned.md) | accepted | infra, workflow, security | 2026-08-01 | | 2026-07-31-npm-nightly-release-channel | [Publish deterministic npm-only nightlies](2026-07-31-npm-nightly-release-channel.md) | accepted (amended 2026-08-03) | workflow, backend, frontend, cli | 2026-07-31 | | 2026-07-31-isolate-manual-pr-review-content | [Isolate Manual PR Review Content](2026-07-31-isolate-manual-pr-review-content.md) | accepted | infra, workflow, security | 2026-07-31 | diff --git a/docs/plans/plugin-repository-task-resolution/plan.md b/docs/plans/plugin-repository-task-resolution/plan.md new file mode 100644 index 0000000000..03e7c7796c --- /dev/null +++ b/docs/plans/plugin-repository-task-resolution/plan.md @@ -0,0 +1,151 @@ +--- +created: 2026-08-26 +status: implemented +requirements: + - REQ-PLUGINS-REPOSITORY-TASK-CREATION-001 +system_design: + - ../../specs/plugins/system-design/repository-provider-task-creation.md +legacy_specs: [] +--- + +# Implementation Plan: Server-Resolved Plugin Repository Task Creation + +## Overview + +Let the native task dialog create a task from a first-use plugin repository. +The backend asks the active plugin to inspect the selection, validates the +returned descriptor, and completes all provider preflight before it writes the +task. The work preserves the current dialog on desktop and phone and documents +the standardized plugin action. + +## Confirmed root cause + +- The plugin repository picker returns a complete descriptor for repositories + that are not yet stored by Kandev. +- Normal REST task creation cannot set the internal trusted-descriptor marker. + This is an intentional security boundary. +- The task service therefore passes the Bitbucket URL to the built-in parser, + which supports only built-in providers and rejects `bitbucket.org`. +- The task row is inserted before repository resolution, so this error leaves + a task without its selected repository. +- A pre-registered repository ID works because that path reads an authorized, + persisted descriptor. +- The plugin action registry and the Bitbucket plugin already provide the + foundations for a server-to-server `repositories.inspect` action. + +## Scope + +### In scope + +- Standardize server-side plugin repository inspection. +- Validate the returned credential-free descriptor against manifest ownership + and clone-origin rules. +- Preflight all first-use plugin selections before task or repository writes. +- Keep built-in providers, persisted repository IDs, and Host `Tasks.Create` + behavior compatible. +- Return typed, bounded errors through task-create transports. +- Extend the in-tree plugin fixture with deterministic inspect and branch + actions. +- Prove first-use success and failure in desktop and phone Playwright projects. +- Update public plugin-authoring and internal frontend host contracts. + +### Out of scope + +- Full transactionality for every task-create side effect. +- A separate repository registration interface. +- New provider-specific parsing in the task service. +- Changes to external plugin credential storage or OAuth. +- A new task-dialog layout or provider capability screen. + +## Technical approach + +Add a bounded `repositories.inspect` call beside the existing standardized +branch action in `internal/plugins`. It resolves the active manifest owner, +uses verified workspace context, parses the compatible direct or nested +response, and validates the complete provider descriptor. + +Expose that capability to the task service through a narrow resolver seam +wired in backend composition. After task-create authorization and external-ID +deduplication, the task service resolves all first-use plugin selections into +an in-memory list. It marks only those server-returned values as trusted. The +existing trusted repository resolver then persists canonical repository rows +and task associations. + +Provider resolution and validation errors occur before the task insert and use +typed categories. The current task dialog already keeps the form open and +shows a failed-create toast, so no new production UI is expected. Playwright +coverage uses the real packaged fixture and the shared dialog to prove both +responsive presentations. + +## Work orders + +- [done] [Task 01: Add Server Repository Inspection](task-01-add-server-repository-inspection.md) +- [done] [Task 02: Preflight Task Repository Selections](task-02-preflight-task-repository-selections.md) +- [done] [Task 03: Prove Native Desktop and Phone Flows](task-03-prove-native-desktop-and-phone-flows.md) +- [done] [Task 04: Publish the Plugin Action Contract](task-04-publish-plugin-action-contract.md) + +## Dependency order + +```text +Task 01 -> Task 02 -> Task 03 -> Task 04 +``` + +The initial package is sequential. Task 02 consumes the inspection contract. +Task 03 proves the combined host and task-service behavior. Task 04 documents +the verified final shapes and limits. + +## Verification strategy + +- Plugin-service unit tests prove active ownership, workspace scope, action + limits, response shapes, descriptor validation, timeout, and safe errors. +- Task-service unit tests prove external-ID ordering, request distrust, + multi-repository preflight, no partial persistence, and compatibility paths. +- Handler tests prove stable REST and WebSocket error mapping. +- Fixture tests prove its manifest and action responses match the contract. +- Playwright proves successful first-use creation, safe failure, branch + selection, persistence, phone reachability, and no horizontal overflow. +- Public docs, specification lint, backend race tests, frontend typecheck, and + focused lint cover the combined change. + +## Risks + +- Reusing browser descriptor fields can accidentally re-open the credential + redirect fixed in the trusted Host path. Persist only the plugin response. +- Calling inspect after task insertion recreates the empty-task bug. Pin the + ordering with repository write-spy tests. +- A loose response parser can accept a mismatched provider or clone origin. + Share the existing trusted descriptor validator and test malicious shapes. +- Resolving every retry before external-ID deduplication adds provider side + effects and can turn a safe retry into a failure. Preserve dedupe ordering. +- The fixture UI already lists a plugin repository, but its backend manifest + does not declare inspect or branch actions. Keep the fixture's UI and backend + descriptors identical. +- A generic internal error would hide a recoverable disconnect. Preserve typed + plugin action categories without exposing upstream response text. + +## Package handoff + +Implement the four work orders in order with TDD. Update each work order's +status and results after its focused checks pass. Change this plan to +`implemented` only after desktop and phone E2E, public docs validation, +specification lint, and the affected backend checks all pass. + +## Results + +- Server inspection validates active manifest ownership, verified workspace + context, bounded responses, credential-free clone origin, immutable hints, + and safe typed errors. The fixture implements the inspect and branch actions. +- Task creation preflights all first-use plugin selections before persistence, + preserves idempotent and trusted Host paths, and maps typed errors through + REST and WebSocket transports. +- Desktop and phone Playwright flows pass without retries. The phone flow also + verifies native reachability, 44px submit targets, and no horizontal + overflow. +- Public plugin contracts document the server-owned inspection boundary and + nested response shape. +- Verification passed: affected backend race tests (3,122 tests), backend + lint, frontend typecheck and lint, i18n checks, specification lint, public + docs validation, diff check, formatting, and focused E2E sleep lint. +- The repository-wide E2E-sleep ESLint command remains blocked by 188 existing + errors and 305 existing warnings in unrelated files. The changed E2E files + pass that configuration directly, and the new-code sleep ratchet passes. diff --git a/docs/plans/plugin-repository-task-resolution/task-01-add-server-repository-inspection.md b/docs/plans/plugin-repository-task-resolution/task-01-add-server-repository-inspection.md new file mode 100644 index 0000000000..60732adb36 --- /dev/null +++ b/docs/plans/plugin-repository-task-resolution/task-01-add-server-repository-inspection.md @@ -0,0 +1,118 @@ +--- +id: "01-add-server-repository-inspection" +title: "Add server repository inspection" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-PLUGINS-REPOSITORY-TASK-CREATION-001 +acceptance_criteria: + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.2 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.4 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.5 +system_design: + - ../../specs/plugins/system-design/repository-provider-task-creation.md +--- + +# Task 01: Add Server Repository Inspection + +## Summary + +Add a standardized backend action that asks the active manifest owner to turn +an untrusted repository URL into a validated, credential-free descriptor. +Extend the packaged fixture to implement the same contract. + +## In scope + +- Start with failing plugin-service tests for success, no match, missing + ownership, wrong action scope, timeout, oversized or malformed output, + provider mismatch, credential-bearing URLs, and clone-origin mismatch. +- Add the `repositories.inspect` action key, request and response models, + action deadline, and response bounds beside `repositories.branches`. +- Resolve the plugin record only through active manifest provider ownership. +- Accept the preferred nested repository response and the compatible direct + descriptor response. +- Return typed failures suitable for task transport mapping. +- Add deterministic fixture action declarations and handlers for repository + inspection and stored-repository branch listing. +- Keep fixture frontend and backend provider identity, scope, immutable ID, + owner, name, clone URL, and default branch identical. + +## Out of scope + +- Task-service invocation or write ordering. +- Task-create HTTP and WebSocket error mapping. +- External Bitbucket plugin changes. +- Browser-invoked repository inspection. + +## Acceptance + +- The active manifest owner receives one bounded workspace-scoped inspect + request containing only the untrusted URL and verified action context. +- A valid direct or nested response becomes one normalized authoritative + descriptor. +- Invalid ownership, action declarations, response content, or origin fails + closed with no credential or raw response leakage. +- The fixture package passes manifest validation and returns descriptors that + satisfy the production parser. + +## Verification + +Create the failing parser and invocation tests first. Confirm at least one +fails because no inspect action exists. Then run: + +```bash +# From apps/backend: +rtk go test ./internal/plugins -run 'RepositoryProvider.*Inspect|InspectRepositoryProvider' -race +rtk go test ./cmd/plugin-fixture -run 'Repository|Action|FixturePackage' -race +``` + +## Files likely touched + +- `apps/backend/internal/plugins/repository_provider_inspect.go` +- `apps/backend/internal/plugins/repository_provider_inspect_test.go` +- `apps/backend/cmd/plugin-fixture/plugin.go` +- `apps/backend/cmd/plugin-fixture/main_test.go` +- `apps/backend/cmd/plugin-fixture/fixture_package_test.go` +- `apps/backend/cmd/plugin-fixture/fixture-package/manifest.yaml` + +## Dependencies + +None. + +## Risks + +- Treating the request URL as an allowed credential origin would break the + security boundary. The plugin must establish connection ownership first. +- Supporting two response envelopes can become open-ended. Limit compatibility + to the documented direct and nested repository shapes. +- Error strings from provider APIs can contain sensitive URLs or account data. + Categorize and sanitize at the plugin action boundary. + +## Parallelism + +`sequential` + +## Inputs + +- `REQ-PLUGINS-REPOSITORY-TASK-CREATION-001`. +- `docs/specs/plugins/system-design/repository-provider-task-creation.md`. +- `docs/decisions/2026-07-31-authenticated-plugin-actions.md`. +- Existing `repositories.branches` implementation and tests. + +## Results + +Implemented `repositories.inspect` with active manifest ownership, verified +workspace action context, direct and nested response compatibility, bounded +response parsing, descriptor and clone-origin validation, immutable hint +checks, timeout handling, redirect rejection, and safe typed errors. The +fixture package now declares and serves deterministic inspect and branch +actions. + +Verification passed: + +```text +go test ./internal/plugins -run 'RepositoryProvider.*Inspect|InspectRepositoryProvider' -race +go test ./cmd/plugin-fixture -run 'Repository|Action|FixturePackage' -race +``` diff --git a/docs/plans/plugin-repository-task-resolution/task-02-preflight-task-repository-selections.md b/docs/plans/plugin-repository-task-resolution/task-02-preflight-task-repository-selections.md new file mode 100644 index 0000000000..029b890a65 --- /dev/null +++ b/docs/plans/plugin-repository-task-resolution/task-02-preflight-task-repository-selections.md @@ -0,0 +1,136 @@ +--- +id: "02-preflight-task-repository-selections" +title: "Preflight task repository selections" +status: done +wave: 2 +depends_on: + - "01-add-server-repository-inspection" +plan: "plan.md" +requirements: + - REQ-PLUGINS-REPOSITORY-TASK-CREATION-001 +acceptance_criteria: + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.1 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.3 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.5 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.6 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.7 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.8 +system_design: + - ../../specs/plugins/system-design/repository-provider-task-creation.md +--- + +# Task 02: Preflight Task Repository Selections + +## Summary + +Wire plugin inspection into task creation. Resolve every first-use plugin +selection before persistence, ignore mutable browser descriptor values, and +map typed failures to safe transport responses. + +## In scope + +- Start with failing task-service tests that prove the current code inserts a + task before plugin URL resolution fails. +- Add a narrow `RepositorySelectionResolver` interface and optional service + setter for focused tests. +- Wire a production adapter from the task service to the plugin service. +- Preserve workspace authorization and external-ID deduplication before any + plugin action. +- Resolve all eligible plugin selections into an in-memory trusted list before + `createTaskWithCapacity` or repository persistence. +- Ignore request host, owner, name, clone URL, and default branch when applying + the authoritative plugin result. Check immutable pinning hints when present. +- Reuse `validateTrustedRemoteRepository`, canonical repository identity, and + the existing trusted persistence path. +- Add typed task-service errors and stable error codes for invalid, not-found, + and unavailable resolution outcomes. +- Map the errors in REST and WebSocket handlers. Confirm the existing frontend + submit path keeps the dialog open and displays the safe message. + +## Out of scope + +- Refactoring all task-create side effects into one database transaction. +- Changing task-create payload fields. +- Adding a new dialog control or translation key unless testing proves the + existing failed-create message cannot express the error. +- Resolving already-persisted repository IDs through a plugin. + +## Acceptance + +- A first-use plugin repository is persisted and attached through the existing + canonical trusted-descriptor path. +- Tampered request descriptors cannot change the provider host, clone URL, + owner, name, default branch, or credential destination. +- One failed plugin selection in a multi-repository request produces no task, + repository, task-repository, event, or last-used write. +- An already-settled external-ID retry returns its task without an inspect call. +- Built-in URLs, repository IDs, and authenticated Host task creation keep + their existing behavior. +- Client errors are bounded and classified without raw plugin response text. + +## Verification + +Create write-spy regressions first and confirm the failure occurs after a task +insert on the old path. Then run: + +```bash +# From apps/backend: +rtk go test ./internal/task/service -run 'CreateTask.*Plugin|RepositorySelection|TrustedRemote' -race +rtk go test ./internal/task/handlers -run 'CreateTask.*Repository|TaskCreate.*Error' -race +rtk go test ./internal/backendapp -run 'Plugin.*Repository|CodeHost' -race +``` + +## Files likely touched + +- `apps/backend/internal/task/service/service.go` +- `apps/backend/internal/task/service/service_requests.go` +- `apps/backend/internal/task/service/service_tasks.go` +- `apps/backend/internal/task/service/service_tasks_test.go` +- `apps/backend/internal/task/handlers/errors.go` +- `apps/backend/internal/task/handlers/errors_test.go` +- `apps/backend/internal/task/handlers/task_http_handlers_test.go` +- `apps/backend/internal/task/handlers/task_ws_handlers_test.go` +- `apps/backend/internal/backendapp/services.go` +- `apps/backend/internal/backendapp/plugins_test.go` + +## Dependencies + +- Task 01 supplies the normalized inspection result and typed plugin errors. + +## Risks + +- Preflight before external-ID lookup would break idempotent retries. +- Preflight after task insertion would preserve the reported bug. +- Parent-repository inheritance and Office identifier allocation share the + current preparation function. Keep provider preflight before task insertion + and pin any unavoidable non-row ordering with tests. +- A nil resolver in focused tests must fail closed only for plugin selections, + while unrelated creation tests remain isolated. + +## Parallelism + +`sequential` + +## Inputs + +- Task 01 normalized descriptor and error contract. +- Existing `TrustedProviderDescriptor` and Host `Tasks.Create` adapter. +- External-ID idempotency create sequence. +- `ADR-2026-08-26-server-owned-plugin-repository-task-resolution`. + +## Results + +Implemented the task-service resolver seam and backend adapter. First-use +plugin selections are resolved after authorization, parent inheritance, and +external-ID deduplication, then validated in memory before task or repository +writes. Browser identity fields are discarded, trusted Host task creation is +preserved, and invalid, not-found, and unavailable failures map to bounded +REST and WebSocket responses. + +Verification passed: + +```text +go test ./internal/task/service -run 'CreateTask.*Plugin|RepositorySelection|TrustedRemote' -race +go test ./internal/task/handlers -run 'CreateTask.*Repository|TaskCreate.*Error' -race +go test ./internal/backendapp -run 'Plugin.*Repository|CodeHost' -race +``` diff --git a/docs/plans/plugin-repository-task-resolution/task-03-prove-native-desktop-and-phone-flows.md b/docs/plans/plugin-repository-task-resolution/task-03-prove-native-desktop-and-phone-flows.md new file mode 100644 index 0000000000..4438fca53c --- /dev/null +++ b/docs/plans/plugin-repository-task-resolution/task-03-prove-native-desktop-and-phone-flows.md @@ -0,0 +1,120 @@ +--- +id: "03-prove-native-desktop-and-phone-flows" +title: "Prove native desktop and phone flows" +status: done +wave: 3 +depends_on: + - "02-preflight-task-repository-selections" +plan: "plan.md" +requirements: + - REQ-PLUGINS-REPOSITORY-TASK-CREATION-001 +acceptance_criteria: + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.1 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.5 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.8 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.9 +system_design: + - ../../specs/plugins/system-design/repository-provider-task-creation.md +--- + +# Task 03: Prove Native Desktop and Phone Flows + +## Summary + +Use the real packaged fixture to prove that the native task dialog creates a +task from an unregistered plugin repository and fails cleanly when inspection +is unavailable. Run the same user value on desktop and phone. + +## In scope + +- Add a desktop Playwright test that installs the fixture, opens the native task + dialog, selects the plugin repository and a non-default branch, creates the + task, and verifies the persisted repository identity and branch. +- Prove the selected repository did not exist before submission and exists + after successful creation. +- Add a failure case for an inactive or unavailable inspect action and verify + the dialog stays open, shows a safe error, and creates no task. +- Add a `mobile-*.spec.ts` test for the same first-use success path through the + phone dialog. +- Verify phone repository and branch controls are reachable, the create action + is touch-sized, and the dialog has no horizontal overflow. +- Use semantic roles, labels, and existing plugin fixture helpers. Do not add + fixed sleeps or repeated blind retries. + +## Out of scope + +- A packaged external Bitbucket plugin dependency. +- Screenshots as the primary assertion. +- Re-testing every generic task-dialog field. +- New mobile-only product behavior. + +## Acceptance + +- Desktop creation from the fixture's first-use repository succeeds and opens + a task attached to the canonical plugin repository and selected branch. +- A provider inspection failure leaves no task and keeps the user's form + available with a bounded error. +- The phone flow provides the same selection and submit capability without + overflow or undersized primary actions. +- Tests pass without retries in the focused desktop and mobile projects. + +## Verification + +Write the desktop first-use test before enabling the server resolver and +confirm that it reproduces the unsupported provider failure. Then run: + +```bash +# From apps/web: +rtk pnpm e2e:raw --project=chromium e2e/tests/task/plugin-repository-task-create.spec.ts --retries=0 +rtk pnpm e2e:raw --project=mobile-chrome e2e/tests/task/mobile-plugin-repository-task-create.spec.ts --retries=0 +``` + +Run the fixture packaging command required by the existing E2E helper before +the focused specs when the helper does not build it automatically. + +## Files likely touched + +- `apps/web/e2e/tests/task/plugin-repository-task-create.spec.ts` +- `apps/web/e2e/tests/task/mobile-plugin-repository-task-create.spec.ts` +- `apps/web/e2e/helpers/plugin-fixture.ts` +- `apps/web/e2e/README.md` only if the fixture setup contract changes + +## Dependencies + +- Task 02 supplies the complete server-resolved task-create flow. + +## Risks + +- Reusing a persisted fixture repository can turn the test into the fast path. + Assert absence before submit and isolate workspace state. +- Clicking a desktop-hidden control in the phone project gives false parity. + Use the phone dialog's visible native controls. +- An optional external plugin package makes CI coverage disappear. Use the + repository-owned fixture package. + +## Parallelism + +`sequential` + +## Inputs + +- The fixture action contract from Task 01. +- The task preflight contract from Task 02. +- Existing task-dialog and plugin fixture Playwright helpers. +- Mobile UI language and E2E cleanup guidance. + +## Results + +Added desktop and phone Playwright coverage using the packaged fixture. The +success cases prove first-use repository persistence, authoritative provider +identity, and selected branch handling. The desktop failure case proves the +dialog remains open with a bounded provider-unavailable message and no task or +repository write. The phone case also proves reachable controls, 44px submit +targets, and no horizontal overflow. + +Verification passed without retries: + +```text +pnpm e2e:raw --project=chromium e2e/tests/task/plugin-repository-task-create.spec.ts --retries=0 +pnpm e2e:raw --project=mobile-chrome e2e/tests/task/mobile-plugin-repository-task-create.spec.ts --retries=0 +``` diff --git a/docs/plans/plugin-repository-task-resolution/task-04-publish-plugin-action-contract.md b/docs/plans/plugin-repository-task-resolution/task-04-publish-plugin-action-contract.md new file mode 100644 index 0000000000..a12d58cb7d --- /dev/null +++ b/docs/plans/plugin-repository-task-resolution/task-04-publish-plugin-action-contract.md @@ -0,0 +1,120 @@ +--- +id: "04-publish-plugin-action-contract" +title: "Publish the plugin action contract" +status: done +wave: 4 +depends_on: + - "03-prove-native-desktop-and-phone-flows" +plan: "plan.md" +requirements: + - REQ-PLUGINS-REPOSITORY-TASK-CREATION-001 +acceptance_criteria: + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.2 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.3 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.4 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.5 + - AC-PLUGINS-REPOSITORY-TASK-CREATION-001.7 +system_design: + - ../../specs/plugins/system-design/repository-provider-task-creation.md +--- + +# Task 04: Publish the Plugin Action Contract + +## Summary + +Document the manifest declaration, request, response, validation, failure, and +security rules for server-side repository inspection. Keep public and internal +author contracts aligned with the verified implementation. + +## In scope + +- Add a public `repositories.inspect` section beside the persisted repository + branch action. +- Explain that frontend `inspectURL` supports browser display and URL entry, + while the backend action is required for first-use task persistence. +- Document the preferred nested repository response and any bounded direct + response compatibility. +- Document verified workspace context, manifest owner selection, timeout and + size limits, no-match behavior, typed action errors, and credential-safe URL + handling. +- Update the internal plugin API contract and any manifest examples that show a + repository provider capable of native first-use task creation. +- Run public documentation and specification validators. + +## Out of scope + +- General plugin tutorials unrelated to repository providers. +- External plugin release notes. +- A new public REST task-create field. + +## Acceptance + +- A plugin author can implement the inspect action without reading Kandev + source code. +- The docs clearly separate untrusted browser selection from authoritative + server resolution. +- Examples do not include credentials and do not imply that the request URL is + an allowed credential origin. +- Public and internal action shapes match production tests. + +## Verification + +Run the relevant documentation checks from their documented working +directories, then run: + +```bash +rtk python3 scripts/lint-spec-files.py --all +rtk git diff --check +``` + +Also run the public docs link, heading, and build commands required by +`docs/public/README.md`. + +## Files likely touched + +- `docs/public/plugins-authoring.md` +- `docs/public/plugins-manifest.md` if action examples need manifest context +- `docs/plans/plugins/PLUGIN-API.md` +- `docs/specs/plugins/requirements/repository-provider-task-creation.md` +- `docs/specs/plugins/system-design/repository-provider-task-creation.md` +- `docs/decisions/2026-08-26-server-owned-plugin-repository-task-resolution.md` + +## Dependencies + +- Task 03 confirms the final request, response, errors, and limits. + +## Risks + +- Calling frontend `inspectURL` authoritative would reintroduce the browser + trust error. Name each boundary explicitly. +- Publishing a response shape before compatibility behavior is tested can lock + in ambiguity. Document only the shapes accepted by focused tests. +- Copying provider-specific Bitbucket rules into the host contract would make + the action less reusable. Keep the contract provider-neutral. + +## Parallelism + +`sequential` + +## Inputs + +- Verified implementation and E2E results from Tasks 01 through 03. +- `docs/public/plugins-authoring.md` persisted branch action section. +- `docs/plans/plugins/PLUGIN-API.md` repository provider contract. +- `ADR-2026-08-26-server-owned-plugin-repository-task-resolution`. + +## Results + +Updated the public manifest and authoring references plus the internal plugin +API contract. The docs distinguish browser display hints from authoritative +server inspection and document the request body, preferred nested response, +compatibility response, validation rules, limits, and safe failure behavior. + +Verification passed: + +```text +python3 scripts/lint-spec-files.py --all +node --test scripts/validate-public-docs.test.mjs +node scripts/validate-public-docs.mjs +git diff --check +``` diff --git a/docs/plans/plugins/PLUGIN-API.md b/docs/plans/plugins/PLUGIN-API.md index 31f19f27ba..8d90cb0dfb 100644 --- a/docs/plans/plugins/PLUGIN-API.md +++ b/docs/plans/plugins/PLUGIN-API.md @@ -416,6 +416,57 @@ Radix/portal/context-based would split React context across instances and break refs/`asChild`. Pure-React libs (e.g. `@tabler/icons-react`) bundle fine. +### First-use repository inspection action + +A manifest-owned repository provider that supports native task creation from a +new URL declares this workspace-scoped action: + +```yaml +actions: + - key: "repositories.inspect" + scope: "workspace" + max_body_bytes: 16384 +``` + +The host invokes the active manifest owner from the backend. The plugin receives +the verified workspace context and this bounded body. Browser descriptor fields +are not forwarded: + +```json +{"url":"https://code.example.com/owner/repository"} +``` + +Return the preferred nested response: + +```json +{ + "repository": { + "provider_id": "example-provider", + "provider_host": "https://code.example.com", + "provider_scope": "https://code.example.com/context-a", + "provider_repository_id": "owner/repository", + "owner_or_project": "owner", + "name": "repository", + "clone_url": "https://code.example.com/owner/repository.git", + "default_branch": "main" + } +} +``` + +The host accepts the descriptor at the top level for compatibility and accepts +`{"matched":false}` for a URL the provider does not own. A successful response +must use the requested provider ID and include a provider host, provider scope, +repository ID, owner or project, name, credential-free HTTPS clone URL, and +default branch. The clone origin must match the HTTPS provider origin. Provider +scope is limited to 512 bytes and identity fields cannot contain NUL bytes. Any +scope or repository ID hint from the task request must match the response. + +The host enforces the manifest body limit, a 15-second timeout, and a 1 MiB +response limit. It validates the response before it writes a repository or task. +Malformed output is invalid, `matched:false` is not found, and provider or +transport failures are unavailable. Error responses do not include plugin body +content or upstream credentials. + ### Persisted repository branch action A manifest-owned repository provider that participates in Kandev's native task branch @@ -806,8 +857,8 @@ interface PluginRegistry { registerKeybinding(id: string, handler: (event: KeyboardEvent) => void): void; // Requires manifest ownership of provider.id. One active plugin owns one provider; - // unload revokes it and aborts in-flight provider work. inspectURL returns a complete - // credential-free HTTPS descriptor—host does not parse plugin provider URLs. + // unload revokes it and aborts in-flight provider work. inspectURL returns browser + // picker data only. Native first-use task creation also requires repositories.inspect. registerRepositoryProvider(provider: RepositoryProviderRegistration): void; // Native task-menu contribution. placement "link" renders in Link menus on desktop @@ -878,6 +929,7 @@ interface RepositoryProviderRegistration { repository: RepositoryInspection; signal: AbortSignal; }): Promise; +// Browser picker callback. Its result is not authoritative for a native task write. inspectURL(context: { workspaceId: string; url: string; diff --git a/docs/public/plugins-authoring.md b/docs/public/plugins-authoring.md index 9b341409af..0cd60d61f2 100644 --- a/docs/public/plugins-authoring.md +++ b/docs/public/plugins-authoring.md @@ -937,6 +937,58 @@ does not own. Exactly one structured inspection may succeed. Multiple successes are rejected as ambiguous instead of selecting registration order, while a real inspection failure is surfaced when no provider establishes ownership. +### First-use repository task creation + +The browser `inspectURL` callback supports the repository picker. Its result is +display data and is not trusted for a native task write. To support a native task +that uses a repository not yet saved in the workspace, declare this action: + +```yaml +actions: + - key: "repositories.inspect" + scope: "workspace" + max_body_bytes: 16384 +``` + +Kandev invokes the active manifest owner from the backend. It supplies a verified +workspace context and a body with only the submitted URL: + +```json +{"url":"https://code.example.com/owner/repository"} +``` + +The preferred response nests the complete descriptor under `repository`: + +```json +{ + "repository": { + "provider_id": "acme", + "provider_host": "https://code.example.com", + "provider_scope": "workspace-a", + "provider_repository_id": "owner/repository", + "owner_or_project": "owner", + "name": "repository", + "clone_url": "https://code.example.com/owner/repository.git", + "default_branch": "main" + } +} +``` + +The host also accepts the descriptor fields at the top level for compatibility. +Return `{"matched":false}` when the provider does not own the URL. The host +requires a matching `provider_id`, a valid HTTPS provider origin, a scope of at +most 512 bytes, and complete repository identity fields. The clone URL must be +HTTPS, contain no credentials, and use the same origin as `provider_host`. +Provider and repository identity fields must not contain NUL bytes. The host +also checks any provider scope or repository ID hint that came from the picker. + +The action has a 15-second deadline and a 1 MiB response limit. Do not return +credentials or provider error details. Invalid output maps to a validation error, +`{"matched":false}` maps to not found, and provider or transport failures map to +an unavailable error. After successful inspection, Kandev persists the validated +descriptor with the normal native task flow. Existing repository IDs and built-in +provider URLs continue to use their existing paths. + A code-host review provider may publish normalized task chrome on each snapshot: ```ts diff --git a/docs/public/plugins-manifest.md b/docs/public/plugins-manifest.md index 6633dca2b2..612088cbd9 100644 --- a/docs/public/plugins-manifest.md +++ b/docs/public/plugins-manifest.md @@ -61,11 +61,17 @@ webhooks: access: public # public (default) or authenticated max_body_bytes: 4194304 # public maximum 4 MiB; authenticated maximum 16 MiB -actions: # authenticated browser-to-plugin calls +actions: # authenticated manifest-declared actions; some are backend-invoked - key: "connection.save" scope: "workspace" # workspace | task | repository access: admin # authenticated (default) | admin max_body_bytes: 65536 # 1 through 1048576 + - key: "repositories.inspect" + scope: "workspace" + max_body_bytes: 16384 + - key: "repositories.branches" + scope: "workspace" + max_body_bytes: 16384 repository_providers: ["acme"] # optional provider ids owned while active reference_sources: # optional dynamic composer sources @@ -172,7 +178,7 @@ ui: # optional native frontend plugin | `actions[].scope` | yes\* | `workspace` \| `task` \| `repository` | Resource selector the host authorizes. A task-scoped invocation may additionally name one repository attached to that verified task. Use the canonical field name `scope`; `resource_scope` is only a legacy read-compatibility spelling and must not be authored. | | `actions[].access` | no | `authenticated` \| `admin` | Defaults to `authenticated`. `admin` is enforced before the action envelope is read and requires `min_kandev_version: "0.91.1"` or later so older hosts cannot silently treat it as authenticated. | | `actions[].max_body_bytes` | yes\* | int | Maximum size of the decoded untrusted `body`, from 1 through 1,048,576 bytes. The whole HTTP envelope has a slightly larger hard cap. | -| `repository_providers` | no | string[] | Provider IDs this plugin owns while active. An active provider can register native repository discovery/URL inspection and may supply transient Git credentials. Native task creation must use the authenticated plugin task-create path so the plugin reauthorizes the repository descriptor before Host Tasks.Create. Duplicate ownership is rejected. | +| `repository_providers` | no | string[] | Provider IDs this plugin owns while active. An active provider can register native repository discovery/URL inspection and may supply transient Git credentials. Native first-use task creation requires a workspace-scoped `repositories.inspect` action; Kandev invokes the active owner on the server and validates its descriptor before persistence. Plugin-originated task creation must use the authenticated plugin task-create path so the plugin reauthorizes the descriptor before Host Tasks.Create. Duplicate ownership is rejected. | | `reference_sources[]` | no | object[] | Dynamic composer-reference source. Each entry declares `source`, `provider`, `kind`, `display_name`, and `kind_label`; `order` is optional. Candidate identity is not trusted: Kandev reauthorizes the canonical reference when it is submitted. | | `auth_providers[]` | no | object[] | Login buttons this plugin contributes to the pre-auth login screen (needs `capabilities.auth`). Each is `{ id, display_name, initiate }`, where `initiate` names one of the plugin's `webhooks[].key` values; the button navigates to that webhook, which 302-redirects to the IdP. Surfaced anonymously in the boot payload as `auth.ssoProviders`. | | `config_schema` | no | object | JSON-Schema-like object driving the settings form at **Settings > Plugins > ``** (`GET /api/plugins/{id}/config` and `PATCH /api/plugins/{id}`). See "Config schema validation and secret fields" below. | diff --git a/docs/specs/plugins/README.md b/docs/specs/plugins/README.md index f3577e9454..050f0c999d 100644 --- a/docs/specs/plugins/README.md +++ b/docs/specs/plugins/README.md @@ -37,6 +37,7 @@ plugin security boundaries. - [Plugin Marketplace](requirements/marketplace.md) - [Plugin nav items in the sidebar footer icon row](requirements/plugin-nav-sidebar-footer.md) - [Plugin System](requirements/plugins.md) +- [Plugin Repository Task Creation](requirements/repository-provider-task-creation.md) - [Voice Plugin Host Prerequisites](requirements/voice-extraction-host.md) - [Voice Mode Leaves Core](requirements/voice-extraction.md) @@ -53,6 +54,7 @@ plugin security boundaries. - [Plugin System System Design Part 2](system-design/plugins-02.md) - [Plugin System System Design Part 3](system-design/plugins-03.md) - [Plugin System System Design Part 4](system-design/plugins-04.md) +- [Plugin Repository Task Creation](system-design/repository-provider-task-creation.md) ## Migration record diff --git a/docs/specs/plugins/requirements/repository-provider-task-creation.md b/docs/specs/plugins/requirements/repository-provider-task-creation.md new file mode 100644 index 0000000000..994a9ca137 --- /dev/null +++ b/docs/specs/plugins/requirements/repository-provider-task-creation.md @@ -0,0 +1,98 @@ +--- +status: active +system: plugins +created: 2026-08-26 +owners: + - kandev +--- + +# Plugin Repository Task Creation Requirements + +## Overview + +The native task dialog can list repositories from an installed code-host +plugin before Kandev has stored those repositories. Today, the browser sends a +complete repository description during task creation. The task service does +not trust that description, so it tries to parse the URL as a built-in +provider. A Bitbucket URL then fails after the task row exists and leaves an +empty task. + +Kandev must resolve a first-use plugin repository through the active plugin on +the server. The browser can select a repository, but it cannot define the +repository identity or the host that can receive credentials. + +## Terminology + +- **Repository selection:** Untrusted request data that identifies what the + user selected. It is a lookup hint, not authority. +- **Authoritative descriptor:** A complete, credential-free repository + identity returned by the active manifest owner through a verified workspace + action. +- **First-use repository:** A provider repository that is visible in the + picker but does not yet have a Kandev repository row. + +## Requirements + +### REQ-PLUGINS-REPOSITORY-TASK-CREATION-001: Server-Resolved Plugin Repository Selection + +**Intent:** Let users create tasks from first-use plugin repositories without +trusting repository identity supplied by the browser or leaving partial tasks +when resolution fails. + +**User story:** As a workspace user, I want to select a repository from a +connected code-host plugin and create a task immediately, so that I do not have +to register the repository in a separate step. + +#### Acceptance criteria + +- **AC-PLUGINS-REPOSITORY-TASK-CREATION-001.1:** When a user selects a + first-use repository from an active plugin provider in the native task + dialog, Kandev shall resolve, register, and attach that repository during the + same task-create operation. +- **AC-PLUGINS-REPOSITORY-TASK-CREATION-001.2:** Kandev shall resolve an + unregistered plugin repository by invoking the active manifest owner's + workspace-scoped `repositories.inspect` action from the backend. +- **AC-PLUGINS-REPOSITORY-TASK-CREATION-001.3:** For a first-use plugin + selection, Kandev shall treat every provider host, scope, repository ID, + owner, name, clone URL, and default branch received from REST, WebSocket, or + MCP task-create input as untrusted. It shall persist only the descriptor + returned by the verified plugin action. Existing repository IDs, valid + built-in provider URLs, and authorized plugin Host `Tasks.Create` calls shall + continue through their existing server-owned paths. +- **AC-PLUGINS-REPOSITORY-TASK-CREATION-001.4:** Before persistence, Kandev + shall verify manifest ownership, workspace scope, provider identity, + immutable repository identity, a credential-free HTTPS clone URL, and clone + origin agreement with the returned provider host. If request pinning hints + are present, the returned immutable values shall match them. +- **AC-PLUGINS-REPOSITORY-TASK-CREATION-001.5:** When the provider is unknown, + inactive, disconnected, or missing the inspect action, or when inspection + times out, finds no match, or returns an invalid descriptor, Kandev shall + return a safe, actionable error before it creates a task row, repository row, + or task-repository association. +- **AC-PLUGINS-REPOSITORY-TASK-CREATION-001.6:** For a multi-repository task, + Kandev shall resolve and validate every first-use plugin selection before it + creates the task or any repository. One failed selection shall not leave a + partial task or a partially registered repository set. +- **AC-PLUGINS-REPOSITORY-TASK-CREATION-001.7:** Existing workspace repository + IDs, built-in provider URLs, and authorized plugin Host `Tasks.Create` calls + shall keep their current resolution paths. A retry with an already-settled + external task ID shall return the existing task without invoking a plugin. +- **AC-PLUGINS-REPOSITORY-TASK-CREATION-001.8:** The native task dialog shall + keep the user's input open and show the bounded provider-resolution error. + It shall not show plugin secrets, raw authenticated URLs, or unrestricted + upstream error text. +- **AC-PLUGINS-REPOSITORY-TASK-CREATION-001.9:** Desktop and phone users shall + have the same repository selection, branch selection, submit, success, and + failure capabilities through the shared native task dialog. + +## Out of scope + +- Making every task-create write fully transactional after provider resolution. +- Adding a separate repository-registration wizard. +- Trusting a browser descriptor when the inspect action is unavailable. +- Changing built-in GitHub, GitLab, or Azure DevOps URL parsing. +- Changing how a plugin authenticates to its external code host. + +## System design + +- [Plugin Repository Task Creation System Design](../system-design/repository-provider-task-creation.md) diff --git a/docs/specs/plugins/system-design/repository-provider-task-creation.md b/docs/specs/plugins/system-design/repository-provider-task-creation.md new file mode 100644 index 0000000000..35074747ef --- /dev/null +++ b/docs/specs/plugins/system-design/repository-provider-task-creation.md @@ -0,0 +1,270 @@ +--- +status: current +system: plugins +requirements: + - REQ-PLUGINS-REPOSITORY-TASK-CREATION-001 +--- + +# Plugin Repository Task Creation System Design + +## Purpose and boundaries + +The plugin system owns the server-side contract that turns an untrusted plugin +repository selection into an authoritative repository descriptor. The task +service consumes that contract before it writes a new task. + +The task system continues to own task validation, idempotency, repository +attachment, branch policies, and task persistence. The workspace system +continues to own repository rows. The plugin keeps ownership of provider API +calls and external credentials. + +## Requirement mapping + +| Requirement | Design section | +| --- | --- | +| `AC-PLUGINS-REPOSITORY-TASK-CREATION-001.1` | [Create sequence](#create-sequence) | +| `AC-PLUGINS-REPOSITORY-TASK-CREATION-001.2` | [Inspection action](#inspection-action) | +| `AC-PLUGINS-REPOSITORY-TASK-CREATION-001.3` | [Trust boundary](#trust-boundary) | +| `AC-PLUGINS-REPOSITORY-TASK-CREATION-001.4` | [Descriptor validation](#descriptor-validation) | +| `AC-PLUGINS-REPOSITORY-TASK-CREATION-001.5` | [Failure contract](#failure-contract) | +| `AC-PLUGINS-REPOSITORY-TASK-CREATION-001.6` | [Multi-repository preflight](#multi-repository-preflight) | +| `AC-PLUGINS-REPOSITORY-TASK-CREATION-001.7` | [Compatibility](#compatibility) | +| `AC-PLUGINS-REPOSITORY-TASK-CREATION-001.8` | [Failure contract](#failure-contract) | +| `AC-PLUGINS-REPOSITORY-TASK-CREATION-001.9` | [Responsive behavior](#responsive-behavior) | + +## Components and responsibilities + +- `internal/plugins.Service` resolves the active manifest owner and invokes its + standardized workspace-scoped `repositories.inspect` action. It limits, + parses, and validates the response. +- A narrow task-service `RepositorySelectionResolver` interface accepts the + verified workspace ID and an untrusted repository selection. Production + composition adapts the plugin service to this interface. Focused task tests + use a stub. +- `internal/task/service.Service` preflights all first-use plugin selections, + replaces each one with an internal trusted descriptor, and only then enters + the existing create and repository-attachment sequence. +- Task HTTP and WebSocket handlers map typed inspection failures to bounded + client errors. The existing task-dialog submit path keeps the dialog open and + shows the returned message in its error toast. +- The plugin fixture supplies deterministic list, inspect, and branch action + results for backend and Playwright tests. It contains no real credentials. + +## Data and contracts + +No database migration or public task-create schema change is required. The +existing task-create repository fields remain selection hints. The internal +`TrustedProviderDescriptor` marker remains unavailable to JSON callers. + +The task service classifies each selection before persistence: + +- An existing `repository_id` or an internal trusted descriptor uses its + existing server-owned path. +- A remote URL with a non-built-in provider uses the plugin resolver. +- A remote URL with a blank or built-in provider is parsed through the existing + built-in path. An unsupported, malformed, or provider-mismatched URL fails + closed before any task or repository write. + +The resolver returns the existing internal repository input shape with a +complete descriptor and `TrustedProviderDescriptor` set by server code. It +does not return credentials. + +## Inspection action + +A plugin that supports first-use repository selection declares this action: + +```yaml +actions: + - key: "repositories.inspect" + scope: "workspace" + max_body_bytes: 16384 +``` + +Kandev selects the active plugin from manifest ownership of the requested +provider ID. It does not accept a plugin ID from the request. It invokes the +action with verified workspace context and a small body: + +```json +{ + "url": "https://code.example.com/owner/repository" +} +``` + +The URL is untrusted. The plugin must first prove that the URL belongs to its +configured provider connection. It must not send connection credentials to an +origin selected only by this value. + +A successful response contains either a `repository` object or the same object +as the top-level response for compatibility with existing provider actions: + +```json +{ + "repository": { + "provider_id": "example-provider", + "provider_host": "https://code.example.com", + "provider_scope": "https://code.example.com/context-a", + "provider_repository_id": "immutable-repository-id", + "owner_or_project": "owner", + "name": "repository", + "clone_url": "https://code.example.com/owner/repository.git", + "default_branch": "main" + } +} +``` + +`{"matched":false}` is a normal no-match response. Pull-request inspection +envelopes may contain additional change-request fields, but task repository +resolution reads only the nested repository descriptor. + +The host applies the declared request limit, a 15-second deadline, a 1 MiB +response limit, and typed action error classification. It does not retry the +action inside one task-create attempt. + +## Trust boundary + +The request's provider ID is used only to select a manifest ownership slot. The +active registry determines the plugin record. The remote URL and any other +request fields are lookup or pinning hints. They never become persisted +authority and never enter the Git credential broker. + +The plugin response is authoritative because it arrives server-to-server from +the active manifest owner under verified workspace context. Installed plugin +code is trusted to use its own connection safely, but Kandev still validates +the returned descriptor before persistence. + +The browser cannot set `TrustedProviderDescriptor`. The existing plugin Host +`Tasks.Create` adapter can still set it after its separate authenticated Host +authorization. These are the only two server-owned paths to the trusted +resolver. + +## Descriptor validation + +The plugin service and task service jointly enforce these rules before writes: + +- The response provider ID equals the manifest-owned provider ID. +- Provider host is a normalized HTTPS origin without user information. +- Provider scope is either empty or a valid normalized scope. +- Immutable provider repository ID, owner or project, name, clone URL, and + default branch are present. +- Clone URL is credential-free HTTPS and its origin matches the provider host. +- Optional immutable request hints, such as provider scope or repository ID, + match the response. Mutable owner, name, host, clone URL, and branch values + from the browser are ignored. +- The response is within size limits and contains one repository descriptor. + +The existing trusted remote repository validator remains the final persistence +guard. Validation logic should be shared instead of creating a weaker plugin +action path. + +## Create sequence + +```mermaid +sequenceDiagram + actor User + participant UI as Native task dialog + participant Task as Task service + participant Plugins as Plugin service + participant Plugin as Connected code-host plugin + participant DB as Kandev database + + User->>UI: Select plugin repository and create task + UI->>Task: Task fields plus repository selection + Task->>Task: Authorize workspace and check external ID + Task->>Plugins: Resolve selection in verified workspace + Plugins->>Plugins: Find active manifest owner + Plugins->>Plugin: repositories.inspect with untrusted URL + Plugin-->>Plugins: Authoritative credential-free descriptor + Plugins->>Plugins: Validate identity, origin, scope, and limits + Plugins-->>Task: Internal trusted descriptor + Task->>Task: Preflight all selected repositories + Task->>DB: Create task, canonical repository, and association + Task-->>UI: Created task + UI-->>User: Open created task +``` + +Task-create idempotency stays first. If an external ID already names an +existing task, Kandev returns that task without contacting a plugin. For a new +task, plugin preflight runs after workspace authorization and request +normalization but before the task row insert. + +After preflight, the existing `resolveTrustedRemoteRepository` and +`FindOrCreateRepository` path preserves canonical provider identity and +deduplicates a repository that another request registered concurrently. + +## Multi-repository preflight + +Kandev resolves repository selections in request order into an in-memory list. +It validates every plugin descriptor before it creates a task or repository +row. A failure discards the list and returns an error. + +The service can resolve selections sequentially in the first implementation. +This avoids concurrent calls to the same plugin connection and keeps error +ordering deterministic. Repository database insertion continues through the +existing canonical identity lock after all external work succeeds. + +This boundary prevents the issue's empty task and partially registered plugin +repository set. It does not make later blocker, branch-policy, attachment, or +database failures transactional. That broader create-atomicity work remains +separate. + +## Failure contract + +Inspection failures use typed categories instead of string matching: + +| Failure | REST result | WebSocket result | User result | +| --- | --- | --- | --- | +| Invalid selection or descriptor | `400` | validation error | Correct the repository selection. | +| Provider or repository not found | `404` | not-found or validation error | Reconnect or select another repository. | +| Plugin inactive, timeout, rate limit, or upstream unavailable | `503` or typed retryable status | unavailable error | Retry after the provider is available. | + +The response includes a stable error code for plugin repository resolution and +a bounded message. Logs can include workspace ID, provider ID, plugin ID, +action key, duration, and failure category. Logs and client responses exclude +secrets, authenticated URLs, plugin configuration, and raw response bodies. + +Cancellation stops the action and returns without writes. A provider-specific +failure occurs before the task-created event and before task-create last-used +settings are recorded. + +## Compatibility + +- A non-empty workspace `repository_id` keeps the current authorized lookup. +- GitHub, GitLab, and Azure DevOps URLs keep the built-in parser. +- Plugin Host `Tasks.Create` keeps its authenticated trusted-descriptor path. +- REST, WebSocket, and MCP callers cannot set the internal trust marker. +- A provider without a valid workspace-scoped `repositories.inspect` action + cannot create a task from a first-use repository. Its already-persisted + repositories remain usable. +- Direct and nested repository action responses are accepted during the + compatibility period. Public documentation defines the nested form as the + preferred shape. + +## Responsive behavior + +The feature uses the existing shared task dialog and repository picker. It +adds no desktop-only control, compressed table, hover-only action, or new phone +navigation. The same submit error keeps the dialog open on both presentations. + +Playwright must prove the complete first-use flow in a desktop project and a +phone project. The phone test also verifies that the repository and branch +controls remain reachable, the submit action has a touch-sized target, and the +dialog has no horizontal overflow. + +## Observability + +Structured logs record one inspection outcome with provider ID, plugin ID, +workspace ID, duration, response shape, and typed failure category. Success +logs do not include repository URLs. Existing task-created logs remain the +evidence that persistence followed a successful preflight. + +No new metric is required for the first implementation. If operational data +shows repeated provider-specific failures, a later change can add bounded +outcome counters without changing this contract. + +## Related decisions + +- [Resolve First-Use Plugin Repositories on the Server](../../../decisions/2026-08-26-server-owned-plugin-repository-task-resolution.md) +- [Plugin Repository Provider Extensions](../../../decisions/2026-07-31-plugin-repository-provider-extensions.md) +- [Authenticated Plugin Actions](../../../decisions/2026-07-31-authenticated-plugin-actions.md) +- [Provider-Neutral Git Credential Broker](../../../decisions/2026-07-31-provider-neutral-git-credential-broker.md) +- [Code-Host Plugins Reuse Native Dashboard Primitives](../../../decisions/2026-08-06-plugin-code-host-dashboard-parity.md)