diff --git a/apps/backend/internal/agent/runtime/environment/environment.go b/apps/backend/internal/agent/runtime/environment/environment.go new file mode 100644 index 0000000000..150eab4a83 --- /dev/null +++ b/apps/backend/internal/agent/runtime/environment/environment.go @@ -0,0 +1,155 @@ +// Package environment resolves the source definitions that make up a task's +// effective runtime environment. Keeping source identity until the final +// composition boundary prevents a later runtime default from silently +// replacing a repository-approved value. +package environment + +import ( + "context" + "crypto/sha256" + "encoding/hex" + "fmt" + "sort" + "strings" +) + +// Definition describes one environment value without including its plaintext +// secret value in the definition itself. +type Definition struct { + Key string + Literal string + SecretID string + Origin string + WorkspaceID string +} + +// ConflictError identifies an ambiguous environment key without including +// plaintext values or secret identifiers. +type ConflictError struct { + Key string + Origins []string +} + +func (e *ConflictError) Error() string { + return fmt.Sprintf("environment key %q has conflicting definitions from %s", e.Key, strings.Join(e.Origins, ", ")) +} + +// SecretError identifies a secret that could not be resolved while retaining a +// redacted error boundary for callers and logs. +type SecretError struct { + Key string + Origin string + err error +} + +func (e *SecretError) Error() string { + return fmt.Sprintf("environment key %q from %s could not be resolved", e.Key, e.Origin) +} + +func (e *SecretError) Unwrap() error { return e.err } + +// RevealFunc resolves a secret-backed definition. Literal definitions are not +// passed to the callback. +type RevealFunc func(context.Context, Definition) (string, error) + +// Validate checks source identity and conflicts without revealing any secret +// values. Callers may use it as an early shape check; Resolve must still run at +// the final composition boundary after all managed runtime definitions exist. +func Validate(definitions []Definition) error { + _, err := selectDefinitions(definitions) + return err +} + +// Resolve sorts definitions deterministically, deduplicates identical source +// identities, reports every origin participating in a conflict, and reveals +// secrets only after the complete conflict pass succeeds. +func Resolve(ctx context.Context, definitions []Definition, reveal RevealFunc) (map[string]string, error) { + selected, err := selectDefinitions(definitions) + if err != nil { + return nil, err + } + + resolved := make(map[string]string, len(selected)) + keys := make([]string, 0, len(selected)) + for key := range selected { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + definition := selected[key] + value := definition.Literal + if definition.SecretID != "" { + if reveal == nil { + return nil, &SecretError{Key: definition.Key, Origin: definition.Origin, err: fmt.Errorf("secret resolver unavailable")} + } + value, err = reveal(ctx, definition) + if err != nil { + return nil, &SecretError{Key: definition.Key, Origin: definition.Origin, err: err} + } + } + resolved[key] = value + } + return resolved, nil +} + +func selectDefinitions(definitions []Definition) (map[string]Definition, error) { + ordered := append([]Definition(nil), definitions...) + sort.SliceStable(ordered, func(i, j int) bool { + if ordered[i].Key != ordered[j].Key { + return ordered[i].Key < ordered[j].Key + } + if ordered[i].Origin != ordered[j].Origin { + return ordered[i].Origin < ordered[j].Origin + } + return ordered[i].SecretID < ordered[j].SecretID + }) + + selected := make(map[string]Definition, len(ordered)) + origins := make(map[string]map[string]struct{}, len(ordered)) + conflicts := make(map[string]map[string]struct{}) + for _, definition := range ordered { + if strings.TrimSpace(definition.Key) == "" { + continue + } + if prior, exists := selected[definition.Key]; exists { + if definitionIdentity(prior) != definitionIdentity(definition) { + if conflicts[definition.Key] == nil { + conflicts[definition.Key] = make(map[string]struct{}) + } + for origin := range origins[definition.Key] { + conflicts[definition.Key][origin] = struct{}{} + } + conflicts[definition.Key][definition.Origin] = struct{}{} + continue + } + origins[definition.Key][definition.Origin] = struct{}{} + continue + } + selected[definition.Key] = definition + origins[definition.Key] = map[string]struct{}{definition.Origin: {}} + } + + if len(conflicts) > 0 { + keys := make([]string, 0, len(conflicts)) + for key := range conflicts { + keys = append(keys, key) + } + sort.Strings(keys) + key := keys[0] + conflictOrigins := make([]string, 0, len(conflicts[key])) + for origin := range conflicts[key] { + conflictOrigins = append(conflictOrigins, origin) + } + sort.Strings(conflictOrigins) + return nil, &ConflictError{Key: key, Origins: conflictOrigins} + } + return selected, nil +} + +func definitionIdentity(definition Definition) string { + if definition.SecretID != "" { + return "secret:" + definition.SecretID + } + hash := sha256.Sum256([]byte(definition.Literal)) + return "literal:" + hex.EncodeToString(hash[:]) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/environment_resolution.go b/apps/backend/internal/agent/runtime/lifecycle/environment_resolution.go new file mode 100644 index 0000000000..b617a743b8 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/environment_resolution.go @@ -0,0 +1,265 @@ +package lifecycle + +import ( + "context" + "errors" + "fmt" + "sort" + "strings" + + "github.com/kandev/kandev/internal/agent/agents" + runtimeenv "github.com/kandev/kandev/internal/agent/runtime/environment" + "github.com/kandev/kandev/internal/secrets" + "github.com/kandev/kandev/internal/task/models" +) + +const ( + managedRuntimeOrigin = "managed runtime" + managedCredentialOrigin = "managed credentials" + managedAgentDefaultsOrigin = "managed agent defaults" + agentProfileOrigin = "agent profile" + executorProfileOrigin = "executor profile" +) + +// TaskEnvironmentRepositoryReader supplies durable repository bindings when a +// backend restart reconstructs a workspace-only execution. +type TaskEnvironmentRepositoryReader interface { + ListTaskRepositories(ctx context.Context, taskID string) ([]*models.TaskRepository, error) + GetRepository(ctx context.Context, id string) (*models.Repository, error) +} + +func (m *Manager) resolveStrictEnvironment( + ctx context.Context, + executionID string, + req *LaunchRequest, + agentConfig agents.Agent, + profileInfo *AgentProfileInfo, +) (map[string]string, error) { + definitions := append([]runtimeenv.Definition(nil), req.EnvironmentDefinitions...) + appendMapDefinitions(&definitions, req.Env, managedRuntimeOrigin) + appendAgentProfileDefinitions(&definitions, profileInfo) + + appendStandardDefinitions(&definitions, executionID, req) + appendAgentRuntimeDefaults(&definitions, agentConfig) + appendRequiredCredentialDefinitions(ctx, &definitions, m.credsMgr, agentConfig) + if req.managedGoCachePath != "" { + definitions = append(definitions, runtimeenv.Definition{ + Key: "GOCACHE", Literal: req.managedGoCachePath, Origin: managedRuntimeOrigin, + }) + } + + resolved, err := runtimeenv.Resolve(ctx, definitions, m.resolveEnvironmentDefinition) + if err != nil { + return nil, fmt.Errorf("resolve task environment: %w", err) + } + return resolved, nil +} + +func appendAgentProfileDefinitions(definitions *[]runtimeenv.Definition, profileInfo *AgentProfileInfo) { + if profileInfo == nil { + return + } + appendProfileDefinitions(definitions, profileInfo.EnvVars, agentProfileOrigin) + if profileInfo.Model != "" { + *definitions = append(*definitions, runtimeenv.Definition{ + Key: "AGENT_MODEL", Literal: profileInfo.Model, Origin: managedRuntimeOrigin, + }) + } + if profileInfo.AutoApprove { + *definitions = append(*definitions, runtimeenv.Definition{ + Key: "AGENTCTL_AUTO_APPROVE_PERMISSIONS", Literal: "true", Origin: managedRuntimeOrigin, + }) + } +} + +func appendAgentRuntimeDefaults(definitions *[]runtimeenv.Definition, agentConfig agents.Agent) { + if agentConfig == nil { + return + } + rt := agentConfig.Runtime() + if rt == nil { + return + } + for key, value := range rt.Env { + if hasEnvironmentDefinition(*definitions, key) { + continue + } + *definitions = append(*definitions, runtimeenv.Definition{ + Key: key, Literal: value, Origin: managedAgentDefaultsOrigin, + }) + } +} + +func appendRequiredCredentialDefinitions( + ctx context.Context, + definitions *[]runtimeenv.Definition, + credsMgr CredentialsManager, + agentConfig agents.Agent, +) { + if credsMgr == nil || agentConfig == nil { + return + } + rt := agentConfig.Runtime() + if rt == nil { + return + } + for _, key := range rt.RequiredEnv { + value, err := credsMgr.GetCredentialValue(ctx, key) + if err != nil || value == "" { + continue + } + *definitions = append(*definitions, runtimeenv.Definition{ + Key: key, Literal: value, Origin: managedCredentialOrigin, + }) + } +} + +func appendMapDefinitions(definitions *[]runtimeenv.Definition, values map[string]string, origin string) { + keys := make([]string, 0, len(values)) + for key := range values { + keys = append(keys, key) + } + sort.Strings(keys) + for _, key := range keys { + *definitions = append(*definitions, runtimeenv.Definition{Key: key, Literal: values[key], Origin: origin}) + } +} + +func appendProfileDefinitions(definitions *[]runtimeenv.Definition, values []models.ProfileEnvVar, origin string) { + for _, value := range values { + if strings.TrimSpace(value.Key) == "" { + continue + } + definition := runtimeenv.Definition{Key: value.Key, Literal: value.Value, SecretID: value.SecretID, Origin: origin} + if definition.SecretID == "" && definition.Literal == "" { + continue + } + *definitions = append(*definitions, definition) + } +} + +func appendStandardDefinitions(definitions *[]runtimeenv.Definition, executionID string, req *LaunchRequest) { + values := map[string]string{ + "KANDEV_INSTANCE_ID": executionID, + "KANDEV_TASK_ID": req.TaskID, + "KANDEV_SESSION_ID": req.SessionID, + "KANDEV_AGENT_PROFILE_ID": req.AgentProfileID, + "KANDEV_EXECUTION_PROFILE_ID": executionProfileID(req), + } + for key, value := range values { + if value == "" { + continue + } + *definitions = append(*definitions, runtimeenv.Definition{Key: key, Literal: value, Origin: managedRuntimeOrigin}) + } +} + +func hasEnvironmentDefinition(definitions []runtimeenv.Definition, key string) bool { + for _, definition := range definitions { + if definition.Key == key { + return true + } + } + return false +} + +func approvedSecretEnvironmentKeys(definitions []runtimeenv.Definition) []string { + keys := make(map[string]struct{}) + for _, definition := range definitions { + if definition.WorkspaceID != "" && strings.TrimSpace(definition.Key) != "" { + keys[definition.Key] = struct{}{} + } + } + approved := make([]string, 0, len(keys)) + for key := range keys { + approved = append(approved, key) + } + sort.Strings(approved) + return approved +} + +func (m *Manager) resolveEnvironmentDefinition(ctx context.Context, definition runtimeenv.Definition) (string, error) { + if definition.SecretID == "" { + return definition.Literal, nil + } + if m.secretStore == nil { + return "", errors.New("secret store unavailable") + } + if definition.WorkspaceID != "" { + scoped, ok := m.secretStore.(secrets.ScopedSecretStore) + if !ok { + return "", errors.New("workspace-scoped secret storage is unavailable") + } + return scoped.RevealForWorkspace(ctx, definition.SecretID, definition.WorkspaceID) + } + return revealGlobalSecret(ctx, m.secretStore, definition.SecretID) +} + +func (m *Manager) repositoryEnvironmentDefinitions(ctx context.Context, taskID, workspaceID string) ([]runtimeenv.Definition, error) { + reader, ok := m.executorProfileReader.(TaskEnvironmentRepositoryReader) + if !ok || taskID == "" { + return nil, nil + } + if strings.TrimSpace(workspaceID) == "" { + return nil, errors.New("workspace id is required to resolve repository environment") + } + taskRepositories, err := reader.ListTaskRepositories(ctx, taskID) + if err != nil { + return nil, fmt.Errorf("list task repositories: %w", err) + } + definitions := make([]runtimeenv.Definition, 0) + for _, taskRepository := range taskRepositories { + if taskRepository == nil || taskRepository.RepositoryID == "" { + continue + } + repository, err := reader.GetRepository(ctx, taskRepository.RepositoryID) + if err != nil { + return nil, fmt.Errorf("load repository %s: %w", taskRepository.RepositoryID, err) + } + if repository == nil { + return nil, fmt.Errorf("repository %s is unavailable", taskRepository.RepositoryID) + } + if repository.WorkspaceID != workspaceID { + return nil, errors.New("repository environment belongs to a different workspace") + } + origin := "repository" + if name := strings.TrimSpace(repository.Name); name != "" { + origin = "repository " + name + } + for _, binding := range repository.SecretBindings { + if strings.TrimSpace(binding.Key) == "" || strings.TrimSpace(binding.SecretID) == "" { + return nil, fmt.Errorf("repository %s has an invalid secret binding", repository.ID) + } + definitions = append(definitions, runtimeenv.Definition{ + Key: binding.Key, SecretID: binding.SecretID, Origin: origin, WorkspaceID: workspaceID, + }) + } + } + return definitions, nil +} + +func (m *Manager) executorProfileEnvironmentDefinitions(ctx context.Context, profileID string) ([]runtimeenv.Definition, error) { + if m.executorProfileReader == nil || profileID == "" { + return nil, nil + } + profile, err := m.executorProfileReader.GetExecutorProfile(ctx, profileID) + if err != nil { + return nil, fmt.Errorf("load executor profile: %w", err) + } + if profile == nil { + return nil, nil + } + definitions := make([]runtimeenv.Definition, 0, len(profile.EnvVars)) + for _, value := range profile.EnvVars { + if strings.TrimSpace(value.Key) == "" { + continue + } + if value.SecretID == "" && value.Value == "" { + continue + } + definitions = append(definitions, runtimeenv.Definition{ + Key: value.Key, Literal: value.Value, SecretID: value.SecretID, Origin: executorProfileOrigin, + }) + } + return definitions, nil +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_backend.go b/apps/backend/internal/agent/runtime/lifecycle/executor_backend.go index c432544557..8f84f594ac 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_backend.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_backend.go @@ -336,17 +336,20 @@ type RemoteStatusProvider interface { // ExecutorCreateRequest contains parameters for creating an agentctl instance. type ExecutorCreateRequest struct { - InstanceID string - TaskID string - TaskTitle string - SessionID string - TaskEnvironmentID string // Env this execution belongs to (shared across sessions in same task) - AgentProfileID string - OfficeAgentProfileID string - WorkspacePath string - WorkspaceSourceRoots []string - Protocol string - Env map[string]string + InstanceID string + TaskID string + TaskTitle string + SessionID string + TaskEnvironmentID string // Env this execution belongs to (shared across sessions in same task) + AgentProfileID string + OfficeAgentProfileID string + WorkspacePath string + WorkspaceSourceRoots []string + Protocol string + Env map[string]string + // ApprovedSecretEnvKeys contains repository binding keys explicitly + // approved for SSH forwarding. Other request env keys remain filtered. + ApprovedSecretEnvKeys []string AutoApprovePermissions bool // AutoApprovePermissionsOverride is set when a resolved profile explicitly // selected the auto-approve value. Nil preserves agentctl defaults/env fallback. diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_credentials.go b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_credentials.go index 6183706399..90825a246f 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_credentials.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_credentials.go @@ -150,11 +150,10 @@ func (r *SpritesExecutor) resolveAuthSecrets( if !ok || method.Type != "env" || method.EnvVar == "" { continue } - value, err := r.secretStore.Reveal(ctx, secretID) + value, err := revealGlobalSecret(ctx, r.secretStore, secretID) if err != nil { r.logger.Warn("failed to resolve auth secret", zap.String("method_id", methodID), - zap.String("secret_id", secretID), zap.Error(err)) continue } diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_lifecycle.go b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_lifecycle.go index f2015fe34b..975171692d 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_lifecycle.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_sprites_lifecycle.go @@ -209,7 +209,7 @@ func (r *SpritesExecutor) resolveTokenFromMetadata(ctx context.Context, instance if secretID == "" { return "" } - revealed, err := r.secretStore.Reveal(ctx, secretID) + revealed, err := revealGlobalSecret(ctx, r.secretStore, secretID) if err != nil || revealed == "" { return "" } diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go index fb57d2f174..4f2899fa3c 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_connection_test.go @@ -412,6 +412,8 @@ func TestSSHRemoteAgentEnv(t *testing.T) { // req.Env credential keys are forwarded; non-credential keys (HOME/PATH) are not. req := &ExecutorCreateRequest{Env: map[string]string{ "CLAUDE_CODE_OAUTH_TOKEN": tokenFromReq, + "NPM_TOKEN": "repository-token", + "PROFILE_ONLY": "profile-value", "HOME": nonCredentialHome, "PATH": nonCredentialPath, "OPENAI_API_KEY": openAIKey, @@ -420,7 +422,7 @@ func TestSSHRemoteAgentEnv(t *testing.T) { "GIT_CONFIG_COUNT": "1", "GIT_CONFIG_KEY_0": "credential.https://github.com.helper", "GIT_CONFIG_VALUE_0": "!agentctl git-credential", - }} + }, ApprovedSecretEnvKeys: []string{"NPM_TOKEN"}} got := sshRemoteAgentEnv(req) if got["CLAUDE_CODE_OAUTH_TOKEN"] != tokenFromReq { t.Fatalf("CLAUDE_CODE_OAUTH_TOKEN = %q, want %q", got["CLAUDE_CODE_OAUTH_TOKEN"], tokenFromReq) @@ -428,6 +430,12 @@ func TestSSHRemoteAgentEnv(t *testing.T) { if got["OPENAI_API_KEY"] != openAIKey { t.Fatalf("OPENAI_API_KEY = %q, want %q", got["OPENAI_API_KEY"], openAIKey) } + if got["NPM_TOKEN"] != "repository-token" { + t.Fatalf("NPM_TOKEN = %q, want repository-token", got["NPM_TOKEN"]) + } + if _, ok := got["PROFILE_ONLY"]; ok { + t.Fatal("unapproved profile key must not be forwarded to the remote agent") + } if got[envKeyGitHubCredentialLease] != "opaque-lease" || got["GIT_CONFIG_KEY_0"] == "" { t.Fatalf("managed GitHub broker env was not forwarded: %#v", got) } @@ -645,6 +653,22 @@ func runSSHContributionGit(t *testing.T, dir string, args ...string) string { return string(output) } +func TestSSHRemoteAgentEnvApprovedRepositoryKeyCannotReplaceManagedCredential(t *testing.T) { + got := sshRemoteAgentEnv(&ExecutorCreateRequest{ + Env: map[string]string{ + "ANTHROPIC_API_KEY": "managed-credential", + "NPM_TOKEN": "repository-token", + }, + ApprovedSecretEnvKeys: []string{"ANTHROPIC_API_KEY", "NPM_TOKEN"}, + }) + if got["ANTHROPIC_API_KEY"] != "managed-credential" { + t.Fatalf("ANTHROPIC_API_KEY = %q, want managed credential", got["ANTHROPIC_API_KEY"]) + } + if got["NPM_TOKEN"] != "repository-token" { + t.Fatalf("NPM_TOKEN = %q, want repository token", got["NPM_TOKEN"]) + } +} + func TestSSHManagedBrokerResumeForcesFreshAgentctlWithNewLease(t *testing.T) { sshExec := NewSSHExecutor(nil, nil, nil, logger.Default()) sshExec.sessions["instance-1"] = &sshSessionState{pid: 1234, remoteDir: "/remote/session"} diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials.go index e12a3ee15c..e834042401 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_credentials.go @@ -100,7 +100,7 @@ func (r *SSHExecutor) runOneAuthSetupScript( platform SSHRemotePlatform, ) { shell := sshShellForRemote(req.Metadata, platform) - envScript, err := buildSSHEnvInitScript(req.Env) + envScript, err := buildSSHEnvInitScript(sshRemoteAgentEnv(req)) if err != nil { r.logger.Warn("auth setup script skipped: invalid environment key", zap.String("display_name", displayName), @@ -162,11 +162,10 @@ func (r *SSHExecutor) resolveAuthSecrets( if !ok || method.Type != authMethodTypeEnv || method.EnvVar == "" { continue } - value, err := r.secretStore.Reveal(ctx, secretID) + value, err := revealGlobalSecret(ctx, r.secretStore, secretID) if err != nil { r.logger.Warn("failed to resolve auth secret", zap.String("method_id", methodID), - zap.String("secret_id", secretID), zap.Error(err)) continue } diff --git a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go index d486dd3293..bcec1fd2d4 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go +++ b/apps/backend/internal/agent/runtime/lifecycle/executor_ssh_operations.go @@ -781,6 +781,21 @@ func sshRemoteAgentEnv(req *ExecutorCreateRequest) map[string]string { // than a GitHub broker lease. Preserve that credential-free routing shape // for the remote agentctl process as well. copyIndexedGitConfig(req.Env, env) + + for _, key := range req.ApprovedSecretEnvKeys { + if !posixSSHEnvIdentifier.MatchString(key) { + continue + } + // Repository approval grants forwarding of an otherwise non-managed + // key; it must never replace a credential or broker value selected by + // the executor composition boundary. + if _, exists := env[key]; exists { + continue + } + if value := req.Env[key]; value != "" { + env[key] = value + } + } if len(env) == 0 { return nil } diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_auth_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_auth_test.go index cd9b60afb1..22ec5d783a 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_auth_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_auth_test.go @@ -16,6 +16,7 @@ type inMemorySecretStore struct { } var _ secrets.SecretStore = (*inMemorySecretStore)(nil) +var _ secrets.ScopedSecretStore = (*inMemorySecretStore)(nil) func newInMemorySecretStore() *inMemorySecretStore { return &inMemorySecretStore{store: make(map[string]*secrets.SecretWithValue)} @@ -55,6 +56,65 @@ func (s *inMemorySecretStore) List(_ context.Context) ([]*secrets.SecretListItem } func (s *inMemorySecretStore) Close() error { return nil } +func (s *inMemorySecretStore) ListScoped(_ context.Context, opts secrets.SecretListOptions) ([]*secrets.SecretListItem, error) { + items := make([]*secrets.SecretListItem, 0, len(s.store)) + for _, stored := range s.store { + scope := stored.Scope + if scope == "" { + scope = secrets.ScopeGlobal + } + if opts.Scope == secrets.ScopeWorkspace && scope != secrets.ScopeWorkspace { + continue + } + if opts.Scope == secrets.ScopeGlobal && scope != secrets.ScopeGlobal { + continue + } + if scope == secrets.ScopeWorkspace && stored.WorkspaceID != opts.WorkspaceID { + continue + } + items = append(items, &secrets.SecretListItem{ID: stored.ID, Name: stored.Name, Scope: scope}) + } + return items, nil +} + +func (s *inMemorySecretStore) GetForWorkspace(_ context.Context, id, workspaceID string) (*secrets.Secret, error) { + secret, err := s.Get(context.Background(), id) + if err != nil { + return nil, err + } + if secret.Scope == secrets.ScopeWorkspace && secret.WorkspaceID != workspaceID { + return nil, fmt.Errorf("workspace secret unavailable") + } + return secret, nil +} + +func (s *inMemorySecretStore) RevealGlobal(ctx context.Context, id string) (string, error) { + secret, err := s.Get(ctx, id) + if err != nil { + return "", err + } + if secret.Scope == secrets.ScopeWorkspace { + return "", fmt.Errorf("workspace secret unavailable") + } + return s.Reveal(ctx, id) +} + +func (s *inMemorySecretStore) RevealForWorkspace(ctx context.Context, id, workspaceID string) (string, error) { + if _, err := s.GetForWorkspace(ctx, id, workspaceID); err != nil { + return "", err + } + return s.Reveal(ctx, id) +} + +func (s *inMemorySecretStore) DeleteWorkspaceSecrets(_ context.Context, workspaceID string) error { + for id, stored := range s.store { + if stored.Scope == secrets.ScopeWorkspace && stored.WorkspaceID == workspaceID { + delete(s.store, id) + } + } + return nil +} + func TestPersistAuthToken(t *testing.T) { log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"}) diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go b/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go index 3f93b44ebb..745eea5cb9 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_execution.go @@ -548,16 +548,6 @@ func (m *Manager) createExecution(ctx context.Context, taskID string, info *Work return nil, fmt.Errorf("agent type %q not found in registry", info.AgentID) } - // Forward AgentProfile.EnvVars to the runtime instance. The Launch / - // ResumeSession paths merge these into req.Env via buildEnvForExecution - // before calling LaunchAgent; the lazy workspace-only path (any - // GetOrEnsureExecution* caller after backend restart) lands here directly, - // so without this merge the runtime instance gets spawned with empty env - // and CLAUDE_CONFIG_DIR (and any other workspace profile var) is lost. - // The agent subprocess inherits the instance env via agentctl, and ACP - // session/load then looks under the wrong SDK root → -32002 Resource not - // found. - env := map[string]string{} var profileInfo *AgentProfileInfo executionProfileID := workspaceExecutionProfileID(info) if executionProfileID != "" && m.profileResolver != nil { @@ -570,12 +560,34 @@ func (m *Manager) createExecution(ctx context.Context, taskID string, info *Work profileInfo = resolvedProfile } } - m.mergeAgentProfileEnvFromInfo(ctx, profileInfo, env) - managedReq := &LaunchRequest{ExecutorType: info.ExecutorType, Env: env} + managedReq := &LaunchRequest{ + TaskID: taskID, + WorkspaceID: info.WorkspaceID, + SessionID: info.SessionID, + AgentProfileID: info.AgentProfileID, + ExecutionProfileID: executionProfileID, + ExecutorType: info.ExecutorType, + Env: make(map[string]string), + } if err := m.prepareManagedGoCacheEnvironment(ctx, managedReq); err != nil { return nil, err } - env = managedReq.Env + definitions, err := m.repositoryEnvironmentDefinitions(ctx, taskID, info.WorkspaceID) + if err != nil { + return nil, err + } + managedReq.EnvironmentDefinitions = append(managedReq.EnvironmentDefinitions, definitions...) + executorDefinitions, err := m.executorProfileEnvironmentDefinitions(ctx, workspaceExecutorProfileID(info)) + if err != nil { + return nil, err + } + managedReq.EnvironmentDefinitions = append(managedReq.EnvironmentDefinitions, executorDefinitions...) + managedReq.ApprovedSecretEnvKeys = approvedSecretEnvironmentKeys(managedReq.EnvironmentDefinitions) + managedReq.EnvironmentResolutionRequired = true + env, err := m.buildEnvForExecution(ctx, executionID, managedReq, agentConfig, profileInfo) + if err != nil { + return nil, fmt.Errorf("build recovered environment: %w", err) + } autoApprove := false var autoApproveOverride *bool if profileInfo != nil { @@ -612,6 +624,7 @@ func (m *Manager) createExecution(ctx context.Context, taskID string, info *Work AutoApprovePermissionsOverride: autoApproveOverride, AgentConfig: agentConfig, Metadata: metadata, + ApprovedSecretEnvKeys: append([]string(nil), managedReq.ApprovedSecretEnvKeys...), PreviousExecutionID: info.AgentExecutionID, AuthToken: m.revealRuntimeSecret(ctx, info.Metadata, MetadataKeyAuthTokenSecret), BootstrapNonce: m.revealRuntimeSecret(ctx, info.Metadata, MetadataKeyBootstrapNonceSecret), @@ -630,12 +643,11 @@ func (m *Manager) createExecution(ctx context.Context, taskID string, info *Work execution := runtimeInstance.ToAgentExecution(req) execution.RuntimeName = rt.Name() - // Cache the resolved profile env on the execution so a subsequent - // configureAndStartAgent (when this workspace-only execution is promoted) - // reuses it via mergeAgentProfileEnvForExecution instead of doing another - // secret-store round-trip. Mirrors the Launch path (manager_launch.go). - if env != nil { - m.cacheResolvedProfileEnv(execution, env) + // Cache only agent-profile values for the best-effort configure fallback. + // The effective runtime snapshot (including repository secrets) is already + // captured by ToAgentExecution and must not be mislabeled as profile data. + if profileInfo != nil && len(profileInfo.EnvVars) > 0 { + m.cacheResolvedProfileEnv(execution, m.resolveAgentProfileEnvVars(ctx, profileInfo.EnvVars)) } // Set the ACP session ID for session resumption @@ -743,6 +755,13 @@ func workspaceExecutionProfileID(info *WorkspaceInfo) string { return info.AgentProfileID } +func workspaceExecutorProfileID(info *WorkspaceInfo) string { + if info == nil { + return "" + } + return info.ExecutorProfileID +} + // rollbackRacedExecution tears down an execution that lost a session-conflict // race in the store. Without this the runtime instance (agentctl + agent // subprocess if any) keeps running with no tracking entry, and no cleanup path @@ -821,8 +840,7 @@ func (m *Manager) persistRuntimeSecret( m.logger.Debug("persisted runtime secret in secret store", zap.String("instance_id", instance.InstanceID), - zap.String("metadata_key", metadataKey), - zap.String("secret_id", secret.ID)) + zap.String("metadata_key", metadataKey)) } func (m *Manager) revealRuntimeSecret(ctx context.Context, metadata map[string]interface{}, metadataKey string) string { @@ -833,11 +851,10 @@ func (m *Manager) revealRuntimeSecret(ctx context.Context, metadata map[string]i if secretID == "" { return "" } - value, err := m.secretStore.Reveal(ctx, secretID) + value, err := revealGlobalSecret(ctx, m.secretStore, secretID) if err != nil { m.logger.Warn("failed to reveal runtime secret", zap.String("metadata_key", metadataKey), - zap.String("secret_id", secretID), zap.Error(err)) return "" } diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_execution_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_execution_test.go index b225c14c65..7d0a351aae 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_execution_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_execution_test.go @@ -26,6 +26,7 @@ import ( settingsmodels "github.com/kandev/kandev/internal/agent/settings/models" "github.com/kandev/kandev/internal/agentruntime" "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/secrets" "github.com/kandev/kandev/internal/task/models" v1 "github.com/kandev/kandev/pkg/api/v1" ) @@ -938,6 +939,89 @@ func TestCreateExecutionResolvesProfileOnceForEnvAndAutoApprove(t *testing.T) { } } +func TestCreateExecutionRecoversRepositoryEnvironmentAndSSHApprovals(t *testing.T) { + profileResolver := &countingProfileResolver{info: &AgentProfileInfo{ + ProfileID: "agent-profile", AgentID: "auggie", EnvVars: []settingsmodels.ProfileEnvVar{{Key: "PROFILE_ONLY", Value: "profile-value"}}, + }} + mgr, backend := newEnvironmentExecutionTestManagerWithProfileResolver(t, &mockWorkspaceInfoProvider{}, profileResolver) + store := newInMemorySecretStore() + if err := store.Create(context.Background(), &secrets.SecretWithValue{Secret: secrets.Secret{ + ID: "workspace-token", Scope: secrets.ScopeWorkspace, WorkspaceID: "workspace-1", + }, Value: "repository-value"}); err != nil { + t.Fatalf("seed repository secret: %v", err) + } + mgr.secretStore = store + reader := &recoveryEnvironmentReader{ + fakeExecutorProfileReader: fakeExecutorProfileReader{ + session: &models.TaskSession{ID: "session-1", TaskID: "task-1", State: models.TaskSessionStateStarting}, + profiles: map[string]*models.ExecutorProfile{ + "executor-profile": {ID: "executor-profile", EnvVars: []models.ProfileEnvVar{{Key: "EXECUTOR_ONLY", Value: "executor-value"}}}, + }, + }, + taskRepositories: []*models.TaskRepository{{RepositoryID: "repo-1"}}, + repositories: map[string]*models.Repository{ + "repo-1": {ID: "repo-1", WorkspaceID: "workspace-1", Name: "app", SecretBindings: []models.RepositorySecretBinding{{Key: "NPM_TOKEN", SecretID: "workspace-token"}}}, + }, + } + mgr.SetExecutorProfileReader(reader) + + execution, err := mgr.createExecution(context.Background(), "task-1", &WorkspaceInfo{ + SessionID: "session-1", WorkspaceID: "workspace-1", AgentProfileID: "agent-profile", ExecutionProfileID: "agent-profile", + ExecutorProfileID: "executor-profile", + AgentID: "auggie", WorkspacePath: "/workspace/task-1", + }) + if err != nil { + t.Fatalf("createExecution returned error: %v", err) + } + if got := backend.lastRequest.Env["NPM_TOKEN"]; got != "repository-value" { + t.Fatalf("recovered NPM_TOKEN = %q, want repository value", got) + } + if got := backend.lastRequest.Env["PROFILE_ONLY"]; got != "profile-value" { + t.Fatalf("recovered profile env = %q, want profile value", got) + } + if got := backend.lastRequest.Env["EXECUTOR_ONLY"]; got != "executor-value" { + t.Fatalf("recovered executor env = %q, want executor value", got) + } + if got := reader.profileArgs; len(got) != 1 || got[0] != "executor-profile" { + t.Fatalf("executor profile lookups = %v, want [executor-profile]", got) + } + if got := backend.lastRequest.ApprovedSecretEnvKeys; len(got) != 1 || got[0] != "NPM_TOKEN" { + t.Fatalf("recovered SSH approvals = %#v, want NPM_TOKEN", got) + } + if got := execution.RuntimeEnvironment()["NPM_TOKEN"]; got != "repository-value" { + t.Fatalf("execution runtime NPM_TOKEN = %q, want repository value", got) + } +} + +func TestWorkspaceProfileIDsKeepAgentAndExecutorProfilesDistinct(t *testing.T) { + info := &WorkspaceInfo{ + AgentProfileID: "office-agent-instance", + ExecutionProfileID: "cli-profile", + ExecutorProfileID: "executor-profile", + } + + if got := workspaceExecutionProfileID(info); got != "cli-profile" { + t.Fatalf("workspaceExecutionProfileID = %q, want cli-profile", got) + } + if got := workspaceExecutorProfileID(info); got != "executor-profile" { + t.Fatalf("workspaceExecutorProfileID = %q, want executor-profile", got) + } +} + +type recoveryEnvironmentReader struct { + fakeExecutorProfileReader + taskRepositories []*models.TaskRepository + repositories map[string]*models.Repository +} + +func (r *recoveryEnvironmentReader) ListTaskRepositories(context.Context, string) ([]*models.TaskRepository, error) { + return r.taskRepositories, nil +} + +func (r *recoveryEnvironmentReader) GetRepository(_ context.Context, id string) (*models.Repository, error) { + return r.repositories[id], nil +} + func TestCreateExecutionRunsRemoteResumePreflightBeforeCreatingWorkspaceExecution(t *testing.T) { log := newTestLogger() backend := &resumeTrackingExecutor{ diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go index cd87817c5d..d13da9fc71 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch.go @@ -519,7 +519,8 @@ func invalidScratchPathID(id string) bool { } // launchPrepareRequest copies the launch request, sets the resolved workspace path, -// populates metadata from the request fields, and injects profile environment variables. +// and populates metadata from the request fields. Runtime/profile environment +// values are composed later, after every managed source has been collected. func (m *Manager) launchPrepareRequest(req *LaunchRequest, profileInfo *AgentProfileInfo, workspacePath string) (LaunchRequest, string, error) { executionID := uuid.New().String() reqWithWorktree := *req @@ -538,17 +539,6 @@ func (m *Manager) launchPrepareRequest(req *LaunchRequest, profileInfo *AgentPro reqWithWorktree.Metadata["session_id"] = req.SessionID } - if profileInfo != nil { - if reqWithWorktree.Env == nil { - reqWithWorktree.Env = make(map[string]string) - } - if profileInfo.Model != "" { - reqWithWorktree.Env["AGENT_MODEL"] = profileInfo.Model - } - if profileInfo.AutoApprove { - reqWithWorktree.Env["AGENTCTL_AUTO_APPROVE_PERMISSIONS"] = "true" - } - } if err := mergeRouteOverrideEnv(&reqWithWorktree); err != nil { return LaunchRequest{}, "", err } @@ -710,6 +700,7 @@ func (m *Manager) launchBuildExecutorRequest(ctx context.Context, executionID st AutoApprovePermissionsOverride: autoApproveOverride, Metadata: metadata, AgentConfig: agentConfig, + ApprovedSecretEnvKeys: append([]string(nil), reqWithWorktree.ApprovedSecretEnvKeys...), McpServers: mcpServers, PreviousExecutionID: reqWithWorktree.PreviousExecutionID, McpMode: reqWithWorktree.McpMode, @@ -1145,11 +1136,28 @@ func (m *Manager) launchInternal(ctx context.Context, req *LaunchRequest) (*Agen } progressRecorder := newPrepareProgressRecorder(m.newProgressCallback(req.TaskID, req.SessionID)) + // Compose the request before preparation so setup scripts receive the same + // final snapshot that the runtime, agent, shell, and terminal will use. + reqWithWorktree, executionID, err := m.launchPrepareRequest(req, profileInfo, workspacePath) + if err != nil { + m.publishLaunchPrepareCompleted(req, nil, progressRecorder, workspacePath, false, err) + return nil, err + } + finalEnv, err := m.buildEnvForExecution(ctx, executionID, &reqWithWorktree, agentConfig, profileInfo) + if err != nil { + m.publishLaunchPrepareCompleted(req, nil, progressRecorder, workspacePath, false, err) + return nil, err + } + reqWithWorktree.Env = finalEnv + reqWithWorktree.EnvironmentDefinitions = nil + reqWithWorktree.EnvironmentResolutionRequired = false + reqWithWorktree.EnvironmentFinalized = true + // 4b. Run environment preparation (if preparer registered for this executor type). // Skip on resume (ACPSessionID set) — workspace was already prepared during initial launch. var prepResult *EnvPrepareResult if req.ACPSessionID == "" { - prepResult = m.runEnvironmentPreparerWithProgress(ctx, req, workspacePath, progressRecorder.Callback(0)) + prepResult = m.runEnvironmentPreparerWithProgress(ctx, &reqWithWorktree, workspacePath, progressRecorder.Callback(0)) } else { m.logger.Debug("skipping environment preparation for resumed session", zap.String("task_id", req.TaskID), @@ -1157,16 +1165,14 @@ func (m *Manager) launchInternal(ctx context.Context, req *LaunchRequest) (*Agen } if prepResult != nil { progressRecorder.Merge(prepResult.Steps) - if err := m.launchApplyPrepareResult(req, prepResult, &workspacePath, &mainRepoGitDir, &worktreeID, &worktreeBranch); err != nil { + if err := m.launchApplyPrepareResult(&reqWithWorktree, prepResult, &workspacePath, &mainRepoGitDir, &worktreeID, &worktreeBranch); err != nil { return nil, err } - } - - // 5 & 6. Prepare the request copy with metadata and profile env - reqWithWorktree, executionID, err := m.launchPrepareRequest(req, profileInfo, workspacePath) - if err != nil { - m.publishLaunchPrepareCompleted(req, prepResult, progressRecorder, workspacePath, false, err) - return nil, err + // The preparer owns the final workspace location for worktree-backed + // launches. Keep the executor request in sync with the local launch + // state; otherwise standalone receives the repository path (or an empty + // path) that was present before preparation completed. + reqWithWorktree.WorkspacePath = workspacePath } // 6b. Deploy per-profile skills + custom prompt (ADR 0005 Wave A). diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch_test.go index 30587161c9..c89fe28469 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_launch_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch_test.go @@ -25,6 +25,7 @@ import ( "github.com/kandev/kandev/internal/agent/executor" "github.com/kandev/kandev/internal/agent/runtime/activity" agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" + runtimeenv "github.com/kandev/kandev/internal/agent/runtime/environment" settingsmodels "github.com/kandev/kandev/internal/agent/settings/models" "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/secrets" @@ -498,6 +499,64 @@ func TestBuildEnvForExecution_SeparatesOfficeAndExecutionProfiles(t *testing.T) } } +func TestBuildEnvForExecution_RejectsLateManagedValuesThatConflictWithRepositorySecrets(t *testing.T) { + store := newInMemorySecretStore() + for _, key := range []string{"AGENT_MODEL", "AGENTCTL_AUTO_APPROVE_PERMISSIONS", "GOCACHE", "ANTHROPIC_API_KEY"} { + if err := store.Create(context.Background(), &secrets.SecretWithValue{ + Secret: secrets.Secret{ID: "secret-" + key, Name: key, Scope: secrets.ScopeWorkspace, WorkspaceID: "workspace-1"}, + Value: "repository-value", + }); err != nil { + t.Fatalf("seed %s: %v", key, err) + } + } + + tests := []struct { + name string + key string + profile *AgentProfileInfo + cachePath string + required []string + credential string + }{ + {name: "model", key: "AGENT_MODEL", profile: &AgentProfileInfo{Model: "managed-model"}}, + {name: "auto approve", key: "AGENTCTL_AUTO_APPROVE_PERMISSIONS", profile: &AgentProfileInfo{AutoApprove: true}}, + {name: "go cache", key: "GOCACHE", cachePath: "/managed/cache"}, + {name: "required credential", key: "ANTHROPIC_API_KEY", required: []string{"ANTHROPIC_API_KEY"}, credential: "managed-credential"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + mgr := newTestManager(t) + mgr.secretStore = store + mgr.credsMgr = fixedCredentialManager{value: tc.credential} + req := &LaunchRequest{ + TaskID: "task-1", WorkspaceID: "workspace-1", SessionID: "session-1", + EnvironmentResolutionRequired: true, + EnvironmentDefinitions: []runtimeenv.Definition{{ + Key: tc.key, SecretID: "secret-" + tc.key, Origin: "repository app", WorkspaceID: "workspace-1", + }}, + } + req.managedGoCachePath = tc.cachePath + _, err := mgr.buildEnvForExecution(context.Background(), "exec-1", req, &testAgent{runtimeConfig: &agents.RuntimeConfig{RequiredEnv: tc.required}}, tc.profile) + if err == nil { + t.Fatal("buildEnvForExecution succeeded, want conflict") + } + var conflictErr *runtimeenv.ConflictError + if !errors.As(err, &conflictErr) { + t.Fatalf("error = %T %v, want environment conflict", err, err) + } + if len(conflictErr.Origins) != 2 { + t.Fatalf("conflict origins = %#v, want repository and managed origin", conflictErr.Origins) + } + }) + } +} + +type fixedCredentialManager struct{ value string } + +func (m fixedCredentialManager) GetCredentialValue(context.Context, string) (string, error) { + return m.value, nil +} + type recordingEnvProfileResolver struct { profileID string } @@ -1087,6 +1146,12 @@ func TestLaunch_PublishesPrepareCompletedAfterRuntimeProgress(t *testing.T) { t.Fatalf("Launch returned error: %v", err) } + if backend.lastRequest == nil { + t.Fatal("runtime did not receive a create request") + } + require.Equal(t, "/tmp/ws", backend.lastRequest.WorkspacePath, + "runtime must receive the workspace path returned by environment preparation") + completed := prepareCompletedPayloads(eventBus) require.NotEmpty(t, completed) final := completed[len(completed)-1] @@ -1095,6 +1160,30 @@ func TestLaunch_PublishesPrepareCompletedAfterRuntimeProgress(t *testing.T) { requirePrepareStep(t, final.Steps, "Waiting for Docker container") } +func TestLaunch_UsesPreparedWorkspacePathForWorktree(t *testing.T) { + profileResolver := &countingProfileResolver{info: &AgentProfileInfo{ + ProfileID: "profile-worktree", + AgentName: "auggie", + }} + mgr, backend := newEnvironmentExecutionTestManagerWithProfileResolver(t, nil, profileResolver) + mgr.preparerRegistry = NewPreparerRegistry(mgr.logger) + mgr.preparerRegistry.Register(models.ExecutorTypeWorktree, &progressPreparer{}) + + _, err := mgr.Launch(context.Background(), &LaunchRequest{ + TaskID: "task-worktree-path", + SessionID: "session-worktree-path", + AgentProfileID: "profile-worktree", + ExecutorType: string(models.ExecutorTypeWorktree), + RepositoryPath: "/tmp/repo", + UseWorktree: true, + BaseBranch: "main", + }) + require.NoError(t, err) + require.NotNil(t, backend.lastRequest) + require.Equal(t, "/tmp/ws", backend.lastRequest.WorkspacePath, + "worktree runtime must receive the path returned by preparation") +} + func TestLaunch_PublishesPrepareCompletionOnLegacyRouteEnvError(t *testing.T) { log := newTestLogger() eventBus := &MockEventBusWithTracking{} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_startup.go b/apps/backend/internal/agent/runtime/lifecycle/manager_startup.go index ff923017b8..3c3d749c72 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_startup.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_startup.go @@ -280,6 +280,24 @@ func (m *Manager) finalizeBootMessage(execution *AgentExecution, msg *models.Mes // buildEnvForExecution builds environment variables for any runtime. // This is the unified method used by the runtime interface. func (m *Manager) buildEnvForExecution(ctx context.Context, executionID string, req *LaunchRequest, agentConfig agents.Agent, profileInfo *AgentProfileInfo) (map[string]string, error) { + if req.EnvironmentFinalized { + env := cloneStringMap(req.Env) + if err := spillLargeWakePayloadEnv(env, req.WorkspacePath, m.logger.Zap()); err != nil { + return nil, err + } + return env, nil + } + if req.EnvironmentResolutionRequired { + env, err := m.resolveStrictEnvironment(ctx, executionID, req, agentConfig, profileInfo) + if err != nil { + return nil, err + } + if err := spillLargeWakePayloadEnv(env, req.WorkspacePath, m.logger.Zap()); err != nil { + return nil, err + } + return env, nil + } + env := make(map[string]string) // Copy request environment diff --git a/apps/backend/internal/agent/runtime/lifecycle/profile_env.go b/apps/backend/internal/agent/runtime/lifecycle/profile_env.go index f533b32d73..280cda2b22 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/profile_env.go +++ b/apps/backend/internal/agent/runtime/lifecycle/profile_env.go @@ -93,15 +93,13 @@ func (m *Manager) resolveAgentProfileEnvVars(ctx context.Context, envVars []sett if ev.SecretID != "" { if m.secretStore == nil { m.logger.Warn("secret store not configured for profile env var", - zap.String("key", key), - zap.String("secret_id", ev.SecretID)) + zap.String("key", key)) continue } - value, err := m.secretStore.Reveal(ctx, ev.SecretID) + value, err := m.revealGlobalSecret(ctx, ev.SecretID) if err != nil { m.logger.Warn("failed to resolve secret for profile env var", zap.String("key", key), - zap.String("secret_id", ev.SecretID), zap.Error(err)) continue } @@ -114,3 +112,7 @@ func (m *Manager) resolveAgentProfileEnvVars(ctx context.Context, envVars []sett } return resolved } + +func (m *Manager) revealGlobalSecret(ctx context.Context, secretID string) (string, error) { + return revealGlobalSecret(ctx, m.secretStore, secretID) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/profile_env_test.go b/apps/backend/internal/agent/runtime/lifecycle/profile_env_test.go index a1e09e97c7..0808ed38ea 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/profile_env_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/profile_env_test.go @@ -92,3 +92,22 @@ func TestResolveAgentProfileEnvVars_SecretAndValue(t *testing.T) { t.Fatalf("FROM_SECRET: got %q", resolved["FROM_SECRET"]) } } + +func TestResolveAgentProfileEnvVars_RejectsWorkspaceSecret(t *testing.T) { + store := newInMemorySecretStore() + if err := store.Create(context.Background(), &secrets.SecretWithValue{ + Secret: secrets.Secret{ID: "workspace-secret", Name: "workspace", Scope: secrets.ScopeWorkspace, WorkspaceID: "workspace-1"}, + Value: "must-not-resolve", + }); err != nil { + t.Fatalf("seed workspace secret: %v", err) + } + + log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"}) + m := &Manager{logger: log, secretStore: store} + resolved := m.resolveAgentProfileEnvVars(context.Background(), []settingsmodels.ProfileEnvVar{{ + Key: "WORKSPACE_ONLY", SecretID: "workspace-secret", + }}) + if _, ok := resolved["WORKSPACE_ONLY"]; ok { + t.Fatalf("workspace secret was resolved into profile environment: %#v", resolved) + } +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/secret_scope.go b/apps/backend/internal/agent/runtime/lifecycle/secret_scope.go new file mode 100644 index 0000000000..2d42608d14 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/secret_scope.go @@ -0,0 +1,14 @@ +package lifecycle + +import ( + "context" + + "github.com/kandev/kandev/internal/secrets" +) + +func revealGlobalSecret(ctx context.Context, store secrets.SecretStore, id string) (string, error) { + if scoped, ok := store.(secrets.ScopedSecretStore); ok { + return scoped.RevealGlobal(ctx, id) + } + return store.Reveal(ctx, id) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/types.go b/apps/backend/internal/agent/runtime/lifecycle/types.go index ce94be41fd..b2d4d859b6 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/types.go +++ b/apps/backend/internal/agent/runtime/lifecycle/types.go @@ -12,6 +12,7 @@ import ( "github.com/kandev/kandev/internal/agent/mcpconfig" agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" + runtimeenv "github.com/kandev/kandev/internal/agent/runtime/environment" settingsmodels "github.com/kandev/kandev/internal/agent/settings/models" "github.com/kandev/kandev/internal/agentctl/types/streams" "github.com/kandev/kandev/internal/agentruntime" @@ -646,10 +647,21 @@ type LaunchRequest struct { TaskDescription string // Task description to send via ACP prompt Attachments []MessageAttachment // Attachments (images/files) for the initial prompt Env map[string]string // Additional env vars - ACPSessionID string // ACP session ID to resume, if available - Metadata map[string]interface{} - ModelOverride string // If set, use this model instead of the profile's model - RouteOverride *RouteOverride // If set, overrides agent_id/model/mode/etc per provider routing + // ApprovedSecretEnvKeys contains repository binding keys that SSH may + // forward in addition to its managed credential allowlist. + ApprovedSecretEnvKeys []string + // EnvironmentDefinitions preserve source identity until lifecycle has added + // every managed runtime value and can perform the final strict resolution. + EnvironmentDefinitions []runtimeenv.Definition + EnvironmentResolutionRequired bool + // EnvironmentFinalized marks the immutable effective snapshot passed to the + // runtime. Later lifecycle steps must not merge profile or credential values + // over this snapshot. + EnvironmentFinalized bool + ACPSessionID string // ACP session ID to resume, if available + Metadata map[string]interface{} + ModelOverride string // If set, use this model instead of the profile's model + RouteOverride *RouteOverride // If set, overrides agent_id/model/mode/etc per provider routing // Ephemeral tasks (quick chat) get fallback workspace directories when no repo is configured. // Non-ephemeral tasks without a workspace path will not receive a fallback directory. @@ -835,6 +847,7 @@ type WorkspaceInfo struct { WorkspaceID string AgentProfileID string // Stable Office agent identity (or the execution profile for legacy sessions) ExecutionProfileID string // Concrete CLI profile selected for this execution + ExecutorProfileID string // Concrete executor profile selected for this execution AgentID string // Agent type ID (e.g., "auggie", "codex") - required for runtime creation ACPSessionID string // Agent's session ID for conversation resumption (from session metadata) // SessionMode is the persisted session permission mode (e.g. "acceptEdits") diff --git a/apps/backend/internal/agent/settings/controller/controller.go b/apps/backend/internal/agent/settings/controller/controller.go index 7434148404..e5fe0a1a79 100644 --- a/apps/backend/internal/agent/settings/controller/controller.go +++ b/apps/backend/internal/agent/settings/controller/controller.go @@ -15,6 +15,7 @@ import ( "github.com/kandev/kandev/internal/agent/settings/modelfetcher" "github.com/kandev/kandev/internal/agent/settings/store" "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/secrets" ws "github.com/kandev/kandev/pkg/websocket" "go.uber.org/zap" ) @@ -65,6 +66,13 @@ type Controller struct { maintenance *maintenanceCoordinator hub JobBroadcaster logger *logger.Logger + secretStore secrets.SecretStore +} + +// SetSecretStore wires the metadata-only validator used by shared agent +// profiles. Workspace-scoped references are intentionally rejected here. +func (c *Controller) SetSecretStore(secretStore secrets.SecretStore) { + c.secretStore = secretStore } // SetWatcherDependencyChecker wires in the watcher dependency enumerator so diff --git a/apps/backend/internal/agent/settings/controller/profile_crud.go b/apps/backend/internal/agent/settings/controller/profile_crud.go index 506bb685fe..07e5065572 100644 --- a/apps/backend/internal/agent/settings/controller/profile_crud.go +++ b/apps/backend/internal/agent/settings/controller/profile_crud.go @@ -14,6 +14,7 @@ import ( "github.com/kandev/kandev/internal/agent/settings/dto" "github.com/kandev/kandev/internal/agent/settings/models" "github.com/kandev/kandev/internal/agent/settings/profileconfig" + "github.com/kandev/kandev/internal/secrets" ) type CreateProfileRequest struct { @@ -61,6 +62,9 @@ func (c *Controller) CreateProfile(ctx context.Context, req CreateProfileRequest if err := validateProfileEnvVarDTOs(req.EnvVars); err != nil { return nil, err } + if err := c.validateGlobalSecretRefs(ctx, req.EnvVars); err != nil { + return nil, err + } if err := validateCommandPrefix(req.CommandPrefix); err != nil { return nil, err } @@ -208,6 +212,9 @@ func (c *Controller) UpdateProfile(ctx context.Context, req UpdateProfileRequest if err := validateProfileEnvVarDTOs(*req.EnvVars); err != nil { return nil, err } + if err := c.validateGlobalSecretRefs(ctx, *req.EnvVars); err != nil { + return nil, err + } profile.EnvVars = envVarsFromDTO(*req.EnvVars) } if req.CommandPrefix != nil { @@ -224,6 +231,21 @@ func (c *Controller) UpdateProfile(ctx context.Context, req UpdateProfileRequest return &result, nil } +func (c *Controller) validateGlobalSecretRefs(ctx context.Context, envVars []dto.ProfileEnvVarDTO) error { + if c.secretStore == nil { + return nil + } + for i, envVar := range envVars { + if strings.TrimSpace(envVar.SecretID) == "" { + continue + } + if err := secrets.ValidateGlobalReference(ctx, c.secretStore, envVar.SecretID); err != nil { + return fmt.Errorf("%w: env_vars[%d] secret must be global", ErrInvalidProfileEnvVars, i) + } + } + return nil +} + // validateCLIFlagDTOs rejects entries with an empty flag string or malformed // shell tokens (unterminated quotes, trailing backslash). Empty descriptions // are allowed (custom flags often don't have one). Tokenising here keeps the diff --git a/apps/backend/internal/agent/settings/controller/profile_secret_scope_test.go b/apps/backend/internal/agent/settings/controller/profile_secret_scope_test.go new file mode 100644 index 0000000000..0766d89e24 --- /dev/null +++ b/apps/backend/internal/agent/settings/controller/profile_secret_scope_test.go @@ -0,0 +1,71 @@ +package controller + +import ( + "context" + "errors" + "testing" + + "github.com/kandev/kandev/internal/agent/settings/dto" + "github.com/kandev/kandev/internal/secrets" +) + +type profileSecretStore struct { + secret *secrets.Secret +} + +func (s *profileSecretStore) Create(context.Context, *secrets.SecretWithValue) error { return nil } +func (s *profileSecretStore) Get(context.Context, string) (*secrets.Secret, error) { + if s.secret == nil { + return nil, errors.New("secret not found") + } + return s.secret, nil +} +func (s *profileSecretStore) Reveal(context.Context, string) (string, error) { + return "", errors.New("not used") +} +func (s *profileSecretStore) Update(context.Context, string, *secrets.UpdateSecretRequest) error { + return nil +} +func (s *profileSecretStore) Delete(context.Context, string) error { return nil } +func (s *profileSecretStore) List(context.Context) ([]*secrets.SecretListItem, error) { + return nil, nil +} +func (s *profileSecretStore) Close() error { return nil } + +func TestValidateGlobalSecretRefsRejectsWorkspaceSecret(t *testing.T) { + controller := &Controller{secretStore: &profileSecretStore{secret: &secrets.Secret{ + ID: "secret-1", Scope: secrets.ScopeWorkspace, WorkspaceID: "workspace-1", + }}} + + err := controller.validateGlobalSecretRefs(context.Background(), []dto.ProfileEnvVarDTO{{ + Key: "TOKEN", SecretID: "secret-1", + }}) + if err == nil { + t.Fatal("workspace secret accepted in shared profile") + } +} + +func TestValidateGlobalSecretRefsAcceptsGlobalSecret(t *testing.T) { + controller := &Controller{secretStore: &profileSecretStore{secret: &secrets.Secret{ + ID: "secret-1", Scope: secrets.ScopeGlobal, + }}} + + if err := controller.validateGlobalSecretRefs(context.Background(), []dto.ProfileEnvVarDTO{{ + Key: "TOKEN", SecretID: "secret-1", + }}); err != nil { + t.Fatalf("global secret rejected: %v", err) + } +} + +func TestValidateGlobalSecretRefsRejectsBackendOwnedIDThroughUserVisibleStore(t *testing.T) { + controller := &Controller{secretStore: secrets.NewUserVisibleStore(&profileSecretStore{secret: &secrets.Secret{ + ID: "github:user:workspace:user:access", Scope: secrets.ScopeGlobal, + }})} + + err := controller.validateGlobalSecretRefs(context.Background(), []dto.ProfileEnvVarDTO{{ + Key: "TOKEN", SecretID: "github:user:workspace:user:access", + }}) + if err == nil { + t.Fatal("backend-owned secret ID accepted in shared profile") + } +} diff --git a/apps/backend/internal/agentctl/server/api/workspace_materialize.go b/apps/backend/internal/agentctl/server/api/workspace_materialize.go index 6554e73e94..87ec9b5065 100644 --- a/apps/backend/internal/agentctl/server/api/workspace_materialize.go +++ b/apps/backend/internal/agentctl/server/api/workspace_materialize.go @@ -369,7 +369,12 @@ func materializedCheckoutExists(destination string) (bool, error) { } func matchingCheckoutIdentity(ctx context.Context, destination, locator, branch string) error { - if err := matchingCheckoutOrigin(ctx, destination, locator); err != nil { + // Read the URL stored in the checkout instead of the expanded URL returned by + // `git remote get-url`. Executor-specific insteadOf rules can rewrite the + // latter to a local transport, even though the checkout still belongs to the + // requested repository. + origin, err := materializeGitOutput(ctx, "-C", destination, "config", "--local", "--get", "remote.origin.url") + if err != nil || strings.TrimSpace(origin) != locator { return errMaterializeCollision } currentBranch, err := materializeGitOutput(ctx, "-C", destination, "branch", "--show-current") diff --git a/apps/backend/internal/agentctl/server/api/workspace_materialize_test.go b/apps/backend/internal/agentctl/server/api/workspace_materialize_test.go index c67f8a63b8..2e79b07700 100644 --- a/apps/backend/internal/agentctl/server/api/workspace_materialize_test.go +++ b/apps/backend/internal/agentctl/server/api/workspace_materialize_test.go @@ -31,6 +31,28 @@ func TestMaterializeRepository_ClonesIntoWorkspaceDestination(t *testing.T) { } } +func TestMaterializeRepository_ReusesCheckoutWithGitURLRewrite(t *testing.T) { + origin := createMaterializeOrigin(t) + configPath := filepath.Join(t.TempDir(), "gitconfig") + locator := "https://gitlab.com/fixture/repository.git" + config := "[url \"file://" + filepath.ToSlash(origin) + "\"]\n\tinsteadOf = " + locator + "\n" + if err := os.WriteFile(configPath, []byte(config), 0o600); err != nil { + t.Fatalf("write git config: %v", err) + } + t.Setenv("GIT_CONFIG_GLOBAL", configPath) + + destination := filepath.Join(t.TempDir(), "second-repo") + if reused, err := materializeRepository(context.Background(), locator, destination, "main", ""); err != nil || reused { + t.Fatalf("initial materialization = reused:%t, err:%v", reused, err) + } + + reused, err := materializeRepository(context.Background(), locator, destination, "main", "") + + if err != nil || !reused { + t.Fatalf("rewritten checkout reuse = reused:%t, err:%v; want reused checkout", reused, err) + } +} + func TestMaterializeRepository_ChecksOutBaseAsNamedTrackingBranch(t *testing.T) { origin := createMaterializeOrigin(t) workDir := t.TempDir() diff --git a/apps/backend/internal/backendapp/adapters.go b/apps/backend/internal/backendapp/adapters.go index 208b133730..6856f81643 100644 --- a/apps/backend/internal/backendapp/adapters.go +++ b/apps/backend/internal/backendapp/adapters.go @@ -13,6 +13,7 @@ import ( "github.com/kandev/kandev/internal/agent/registry" runtimeapi "github.com/kandev/kandev/internal/agent/runtime" "github.com/kandev/kandev/internal/agent/runtime/agentctl" + runtimeenv "github.com/kandev/kandev/internal/agent/runtime/environment" "github.com/kandev/kandev/internal/agent/runtime/lifecycle" "github.com/kandev/kandev/internal/agentctl/types/streams" "github.com/kandev/kandev/internal/clarification" @@ -182,47 +183,50 @@ func buildLifecycleLaunchRequest( req *executor.LaunchAgentRequest, workspacePath, officeProfileID string, ) *lifecycle.LaunchRequest { launchReq := &lifecycle.LaunchRequest{ - TaskID: req.TaskID, - WorkspaceID: req.WorkspaceID, - SessionID: req.SessionID, - TaskEnvironmentID: req.TaskEnvironmentID, - TaskTitle: req.TaskTitle, - AgentProfileID: officeProfileID, - ExecutionProfileID: req.AgentProfileID, - StartAgent: req.StartAgent, - WorkspacePath: workspacePath, - TaskDescription: req.TaskDescription, - Attachments: convertToLifecycleAttachments(req.Attachments), - Env: req.Env, - ACPSessionID: req.ACPSessionID, - Metadata: req.Metadata, - ModelOverride: req.ModelOverride, - ExecutorType: req.ExecutorType, - ExecutorConfig: req.ExecutorConfig, - PreviousExecutionID: req.PreviousExecutionID, - McpMode: req.McpMode, - McpProviders: req.McpProviders, - IsEphemeral: req.IsEphemeral, - IsPassthrough: req.IsPassthrough, - SetupScript: req.SetupScript, - CopyFiles: req.CopyFiles, - UseWorktree: req.UseWorktree, - WorktreeID: req.WorktreeID, - RepositoryID: req.RepositoryID, - RepositoryPath: req.RepositoryPath, - BaseBranch: req.BaseBranch, - DefaultBranch: req.DefaultBranch, - CheckoutBranch: req.CheckoutBranch, - PRNumber: req.PRNumber, - RemoteContribution: req.RemoteContribution, - WorktreeBranchPrefix: req.WorktreeBranchPrefix, - WorktreeBranchTemplate: req.WorktreeBranchTemplate, - WorktreeBranchTicket: req.WorktreeBranchTicket, - PullBeforeWorktree: req.PullBeforeWorktree, - TaskDirName: req.TaskDirName, - RepoName: req.RepoName, - BranchSlug: req.BranchSlug, - BranchIdentitySlug: req.BranchIdentitySlug, + TaskID: req.TaskID, + WorkspaceID: req.WorkspaceID, + SessionID: req.SessionID, + TaskEnvironmentID: req.TaskEnvironmentID, + TaskTitle: req.TaskTitle, + AgentProfileID: officeProfileID, + ExecutionProfileID: req.AgentProfileID, + StartAgent: req.StartAgent, + WorkspacePath: workspacePath, + TaskDescription: req.TaskDescription, + Attachments: convertToLifecycleAttachments(req.Attachments), + Env: req.Env, + ApprovedSecretEnvKeys: append([]string(nil), req.ApprovedSecretEnvKeys...), + EnvironmentDefinitions: append([]runtimeenv.Definition(nil), req.EnvironmentDefinitions...), + EnvironmentResolutionRequired: req.EnvironmentResolutionRequired, + ACPSessionID: req.ACPSessionID, + Metadata: req.Metadata, + ModelOverride: req.ModelOverride, + ExecutorType: req.ExecutorType, + ExecutorConfig: req.ExecutorConfig, + PreviousExecutionID: req.PreviousExecutionID, + McpMode: req.McpMode, + McpProviders: req.McpProviders, + IsEphemeral: req.IsEphemeral, + IsPassthrough: req.IsPassthrough, + SetupScript: req.SetupScript, + CopyFiles: req.CopyFiles, + UseWorktree: req.UseWorktree, + WorktreeID: req.WorktreeID, + RepositoryID: req.RepositoryID, + RepositoryPath: req.RepositoryPath, + BaseBranch: req.BaseBranch, + DefaultBranch: req.DefaultBranch, + CheckoutBranch: req.CheckoutBranch, + PRNumber: req.PRNumber, + RemoteContribution: req.RemoteContribution, + WorktreeBranchPrefix: req.WorktreeBranchPrefix, + WorktreeBranchTemplate: req.WorktreeBranchTemplate, + WorktreeBranchTicket: req.WorktreeBranchTicket, + PullBeforeWorktree: req.PullBeforeWorktree, + TaskDirName: req.TaskDirName, + RepoName: req.RepoName, + BranchSlug: req.BranchSlug, + BranchIdentitySlug: req.BranchIdentitySlug, } launchReq.WorkspaceFolders = lifecycleWorkspaceFolders(req.WorkspaceFolders) launchReq.RouteOverride = lifecycleRouteOverride(req.RouteOverride) diff --git a/apps/backend/internal/backendapp/main.go b/apps/backend/internal/backendapp/main.go index 007dd4c58b..cf7f15f1c1 100644 --- a/apps/backend/internal/backendapp/main.go +++ b/apps/backend/internal/backendapp/main.go @@ -1806,6 +1806,8 @@ func buildHTTPServer( // workspace_id with no gate of their own. No-op when auth is disabled. router.Use(integrationWorkspaceScopeMiddleware(services.Auth, services.Task)) + secretsSvc := secrets.NewService(userSecretStore, log) + secretsSvc.SetWorkspaceAuthorizer(services.Task.AuthorizeWorkspaceAccess) registerRoutes(routeParams{ router: router, gateway: gateway, @@ -1831,7 +1833,7 @@ func buildHTTPServer( promptCtrl: promptcontroller.NewController(services.Prompts), utilityCtrl: utilitycontroller.NewController(services.Utility), msgCreator: msgCreator, - secretsSvc: secrets.NewService(userSecretStore, log), + secretsSvc: secretsSvc, secretStore: userSecretStore, mcpConfigSvc: mcpconfig.NewService(repos.AgentSettings), authSvc: services.Auth, diff --git a/apps/backend/internal/backendapp/services.go b/apps/backend/internal/backendapp/services.go index 7559259a12..74e91e770d 100644 --- a/apps/backend/internal/backendapp/services.go +++ b/apps/backend/internal/backendapp/services.go @@ -55,7 +55,9 @@ func provideServices(cfg *config.Config, log *logger.Logger, repos *Repositories if err != nil { return nil, nil, err } + userSecretStore := secrets.NewUserVisibleStore(repos.Secrets) agentSettingsController := agentsettingscontroller.NewController(repos.AgentSettings, discoveryRegistry, agentRegistry, repos.Task, log) + agentSettingsController.SetSecretStore(userSecretStore) userSvc := userservice.NewService(repos.User, eventBus, log) editorSvc := editorservice.NewService(repos.Editor, repos.Task, userSvc) @@ -91,6 +93,10 @@ func provideServices(cfg *config.Config, log *logger.Logger, repos *Repositories TaskWorktreeRoots: []string{filepath.Join(cfg.ResolvedHomeDir(), "tasks")}, }, ) + taskSvc.SetSecretStore(userSecretStore) + if deleter, ok := userSecretStore.(taskservice.WorkspaceSecretDeleter); ok { + taskSvc.SetWorkspaceSecretDeleter(deleter) + } // Wire workflow step creator to task service for board creation taskSvc.SetWorkflowStepCreator(workflowSvc) diff --git a/apps/backend/internal/orchestrator/executor/executor.go b/apps/backend/internal/orchestrator/executor/executor.go index 04d27e3a5c..f25edd42ad 100644 --- a/apps/backend/internal/orchestrator/executor/executor.go +++ b/apps/backend/internal/orchestrator/executor/executor.go @@ -12,6 +12,7 @@ import ( "github.com/kandev/kandev/internal/agent/agents" "github.com/kandev/kandev/internal/agent/runtime/agentctl" + runtimeenv "github.com/kandev/kandev/internal/agent/runtime/environment" "github.com/kandev/kandev/internal/agent/runtime/lifecycle" "github.com/kandev/kandev/internal/agentctl/types/streams" "github.com/kandev/kandev/internal/agentruntime" @@ -338,15 +339,23 @@ type LaunchAgentRequest struct { Priority string Metadata map[string]interface{} Env map[string]string - ACPSessionID string // ACP session ID to resume, if available - ModelOverride string // If set, use this model instead of the profile's model - ExecutorType string // Executor type (e.g., "local", "worktree", "local_docker") - determines runtime - ExecutorConfig map[string]string // Executor config (docker_host, git_token, etc.) - PreviousExecutionID string // Previous execution ID for runtime reconnect - McpMode string // MCP tool mode: "task" (default), "config", or "office" - McpProviders []string // Normalized provider capabilities attached to the task - IsEphemeral bool // Ephemeral task (quick chat) — enables fallback workspace creation - WorkspacePath string // Optional host folder for repo-less tasks (overrides scratch fallback) + // ApprovedSecretEnvKeys contains repository binding keys that SSH may + // forward in addition to its managed credential allowlist. Values are + // still taken only from Env; the key list is the explicit repository grant. + ApprovedSecretEnvKeys []string + // EnvironmentDefinitions preserve source identity until lifecycle has added + // every managed runtime value and can perform the final strict resolution. + EnvironmentDefinitions []runtimeenv.Definition + EnvironmentResolutionRequired bool + ACPSessionID string // ACP session ID to resume, if available + ModelOverride string // If set, use this model instead of the profile's model + ExecutorType string // Executor type (e.g., "local", "worktree", "local_docker") - determines runtime + ExecutorConfig map[string]string // Executor config (docker_host, git_token, etc.) + PreviousExecutionID string // Previous execution ID for runtime reconnect + McpMode string // MCP tool mode: "task" (default), "config", or "office" + McpProviders []string // Normalized provider capabilities attached to the task + IsEphemeral bool // Ephemeral task (quick chat) — enables fallback workspace creation + WorkspacePath string // Optional host folder for repo-less tasks (overrides scratch fallback) // IsPassthrough is the session's mode snapshot (TaskSession.IsPassthrough) // at session-creation time. Forwarded to the lifecycle manager so diff --git a/apps/backend/internal/orchestrator/executor/executor_credentials.go b/apps/backend/internal/orchestrator/executor/executor_credentials.go index 827cd6df3f..56520d1023 100644 --- a/apps/backend/internal/orchestrator/executor/executor_credentials.go +++ b/apps/backend/internal/orchestrator/executor/executor_credentials.go @@ -508,11 +508,10 @@ func (e *Executor) resolveAuthSecrets(ctx context.Context, req *LaunchAgentReque continue } - value, err := e.secretStore.Reveal(ctx, secretID) + value, err := e.revealGlobalSecret(ctx, secretID) if err != nil { e.logger.Debug("failed to resolve auth secret", zap.String("method_id", methodID), - zap.String("secret_id", secretID), zap.Error(err)) continue } diff --git a/apps/backend/internal/orchestrator/executor/executor_execute.go b/apps/backend/internal/orchestrator/executor/executor_execute.go index 84969d06e5..07889a14f5 100644 --- a/apps/backend/internal/orchestrator/executor/executor_execute.go +++ b/apps/backend/internal/orchestrator/executor/executor_execute.go @@ -615,26 +615,31 @@ func (e *Executor) getSessionLock(sessionID string) *sync.Mutex { } func (e *Executor) applyPreferredShellEnv(ctx context.Context, executorType string, env map[string]string) map[string]string { + result, _ := e.applyPreferredShellEnvWithStatus(ctx, executorType, env) + return result +} + +func (e *Executor) applyPreferredShellEnvWithStatus(ctx context.Context, executorType string, env map[string]string) (map[string]string, bool) { if e.capabilities == nil || !e.capabilities.ShouldApplyPreferredShell(executorType) { - return env + return env, false } if e.shellPrefs == nil { - return env + return env, false } preferred, err := e.shellPrefs.PreferredShell(ctx) if err != nil { - return env + return env, false } preferred = strings.TrimSpace(preferred) if preferred == "" { - return env + return env, false } if env == nil { env = make(map[string]string) } env["AGENTCTL_SHELL_COMMAND"] = preferred env["SHELL"] = preferred - return env + return env, true } // Execute starts agent execution for a task @@ -972,6 +977,9 @@ func (e *Executor) LaunchPreparedSession(ctx context.Context, task *v1.Task, ses mergeEnv(req, opts.Env) if opts.RouteOverride != nil { req.RouteOverride = opts.RouteOverride + if opts.RouteOverride.ExecutionProfileID == "" { + mergeEnv(req, opts.RouteOverride.Env) + } } // Apply McpMode from options (takes precedence over session metadata check in buildLaunchAgentRequest) @@ -1012,7 +1020,9 @@ func (e *Executor) LaunchPreparedSession(ctx context.Context, task *v1.Task, ses zap.String("executor_type", req.ExecutorType), zap.Bool("use_worktree", req.UseWorktree)) - req.Env = e.applyPreferredShellEnv(ctx, req.ExecutorType, req.Env) + if err := e.resolveLaunchEnvironment(ctx, req, execCfg.ProfileEnvVars, allRepos); err != nil { + return nil, err + } // Call the AgentManager to launch the container resp, err := e.agentManager.LaunchAgent(ctx, req) @@ -1295,15 +1305,6 @@ func (e *Executor) buildLaunchAgentRequest(ctx context.Context, task *v1.Task, s req.ExecutorType = execConfig.ExecutorType req.ExecutorConfig = execConfig.ExecutorCfg req.SetupScript = execConfig.SetupScript - // Merge profile env vars into request env - if len(execConfig.ProfileEnv) > 0 { - if req.Env == nil { - req.Env = make(map[string]string) - } - for k, v := range execConfig.ProfileEnv { - req.Env[k] = v - } - } } // For remote executors (containerized *and* SSH), resolve only explicitly diff --git a/apps/backend/internal/orchestrator/executor/executor_interaction.go b/apps/backend/internal/orchestrator/executor/executor_interaction.go index c747c9be59..a85758a58e 100644 --- a/apps/backend/internal/orchestrator/executor/executor_interaction.go +++ b/apps/backend/internal/orchestrator/executor/executor_interaction.go @@ -677,7 +677,6 @@ func (e *Executor) buildSwitchModelRequest(ctx context.Context, task *models.Tas IsEphemeral: task.IsEphemeral, IsPassthrough: session.IsPassthrough, TaskEnvironmentID: session.TaskEnvironmentID, - Env: cloneStringMap(execConfig.ProfileEnv), } mcpMode, err := e.resolveTaskSessionMCPMode(ctx, task.ID, session, true) @@ -697,6 +696,13 @@ func (e *Executor) buildSwitchModelRequest(ctx context.Context, task *models.Tas req.RepositoryURL = running.WorktreePath } e.injectGitLabWorkspaceCredentials(ctx, req) + allRepos, err := e.resolveAllRepoInfo(ctx, task.ID) + if err != nil { + return nil, err + } + if err := e.resolveLaunchEnvironment(ctx, req, execConfig.ProfileEnvVars, allRepos); err != nil { + return nil, err + } return req, nil } diff --git a/apps/backend/internal/orchestrator/executor/executor_multi_repo_test.go b/apps/backend/internal/orchestrator/executor/executor_multi_repo_test.go index d0f0a50c1a..8b86f6f9fd 100644 --- a/apps/backend/internal/orchestrator/executor/executor_multi_repo_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_multi_repo_test.go @@ -107,6 +107,45 @@ func TestLaunchPreparedSession_MultiRepo_PopulatesRequestRepositories(t *testing } } +func TestLaunchPreparedSession_RepositoryEnvironmentConflictFailsBeforeLaunch(t *testing.T) { + repo := newMockRepository() + const taskID = "task-multi-env-conflict" + const sessionID = "session-multi-env-conflict" + seedMultiRepoTask(t, repo, taskID) + repo.repositories["repo-front"].WorkspaceID = "ws-1" + repo.repositories["repo-front"].SecretBindings = []models.RepositorySecretBinding{{ + Key: "PACKAGE_TOKEN", SecretID: "secret-front", + }} + repo.repositories["repo-back"].WorkspaceID = "ws-1" + repo.repositories["repo-back"].SecretBindings = []models.RepositorySecretBinding{{ + Key: "PACKAGE_TOKEN", SecretID: "secret-back", + }} + repo.sessions[sessionID] = &models.TaskSession{ + ID: sessionID, + TaskID: taskID, + AgentProfileID: "profile-123", + State: models.TaskSessionStateCreated, + StartedAt: time.Now(), + UpdatedAt: time.Now(), + } + + agentManager := &mockAgentManager{} + exec := newTestExecutor(t, agentManager, repo) + _, err := exec.LaunchPreparedSession(context.Background(), &v1.Task{ + ID: taskID, WorkspaceID: "ws-1", Title: "Multi", + }, sessionID, LaunchOptions{AgentProfileID: "profile-123", StartAgent: false}) + if err == nil { + t.Fatal("LaunchPreparedSession succeeded, want environment conflict") + } + var conflictErr *EnvironmentConflictError + if !errors.As(err, &conflictErr) { + t.Fatalf("LaunchPreparedSession error = %T %v, want EnvironmentConflictError", err, err) + } + if agentManager.launchAgentCallCount != 0 { + t.Fatalf("LaunchAgent calls = %d, want 0", agentManager.launchAgentCallCount) + } +} + func TestLaunchPreparedSession_MultiRepo_LaunchFailureReportsFailingSecondaryRepositoryID(t *testing.T) { repo := newMockRepository() const taskID = "task-multi-launch-failure" diff --git a/apps/backend/internal/orchestrator/executor/executor_resume.go b/apps/backend/internal/orchestrator/executor/executor_resume.go index 71177676b1..168470a34a 100644 --- a/apps/backend/internal/orchestrator/executor/executor_resume.go +++ b/apps/backend/internal/orchestrator/executor/executor_resume.go @@ -935,6 +935,9 @@ func (e *Executor) buildResumeRequestAtCredentialBoundary( return nil, "", execConfig, existingEnv, existingRunning, err } e.injectGitLabWorkspaceCredentials(ctx, req) + if err := e.resolveLaunchEnvironment(ctx, req, execConfig.ProfileEnvVars, allRepos); err != nil { + return nil, "", execConfig, existingEnv, existingRunning, err + } return req, repositoryID, execConfig, existingEnv, existingRunning, nil } @@ -999,15 +1002,6 @@ func (e *Executor) applyExecutorConfigToResumeRequest(ctx context.Context, req * req.ExecutorType = execConfig.ExecutorType req.ExecutorConfig = execConfig.ExecutorCfg req.SetupScript = execConfig.SetupScript - if len(execConfig.ProfileEnv) > 0 { - if req.Env == nil { - req.Env = make(map[string]string) - } - for k, v := range execConfig.ProfileEnv { - req.Env[k] = v - } - } - if executorWasEmpty && session.ExecutorID != "" { session.UpdatedAt = time.Now().UTC() if err := e.repo.UpdateTaskSession(ctx, session); err != nil { diff --git a/apps/backend/internal/orchestrator/executor/executor_state.go b/apps/backend/internal/orchestrator/executor/executor_state.go index 9ae623b350..6b906d4157 100644 --- a/apps/backend/internal/orchestrator/executor/executor_state.go +++ b/apps/backend/internal/orchestrator/executor/executor_state.go @@ -102,15 +102,15 @@ func (e *Executor) defaultExecutorID(ctx context.Context, workspaceID string) st // executorConfig holds resolved executor configuration. type executorConfig struct { - ExecutorID string - ExecutorType string - ExecutorCfg map[string]string // The executor record's Config map (docker_host, etc.) - Metadata map[string]interface{} - SetupScript string // Setup script from profile - CleanupScript string // Cleanup script from profile (terminal teardown) - ProfileEnv map[string]string // Resolved env vars from profile (secrets decrypted) - Resumable bool // Whether the executor supports session resume - RuntimeName string // Runtime name from the executor type (e.g. "local_pc") + ExecutorID string + ExecutorType string + ExecutorCfg map[string]string // The executor record's Config map (docker_host, etc.) + Metadata map[string]interface{} + SetupScript string // Setup script from profile + CleanupScript string // Cleanup script from profile (terminal teardown) + ProfileEnvVars []models.ProfileEnvVar // Source definitions resolved at launch checkpoint. + Resumable bool // Whether the executor supports session resume + RuntimeName string // Runtime name from the executor type (e.g. "local_pc") } // resolveExecutorConfig resolves executor configuration from an executor ID. @@ -200,7 +200,7 @@ func (e *Executor) applyProfile(ctx context.Context, profileID string, cfg *exec cfg.SetupScript = profile.PrepareScript cfg.CleanupScript = profile.CleanupScript - cfg.ProfileEnv = e.resolveProfileEnvVars(ctx, profile.EnvVars) + cfg.ProfileEnvVars = append([]models.ProfileEnvVar(nil), profile.EnvVars...) // Persist secret store IDs in metadata so runtimes can resolve tokens after restart // (e.g., SpritesExecutor needs SPRITES_API_TOKEN to poll remote status). for _, ev := range profile.EnvVars { @@ -279,30 +279,6 @@ func applyProfileConfigToMetadata(profileConfig map[string]string, metadata map[ } } -// resolveProfileEnvVars resolves profile env vars, dereferencing secret IDs to their values. -func (e *Executor) resolveProfileEnvVars(ctx context.Context, envVars []models.ProfileEnvVar) map[string]string { - if len(envVars) == 0 { - return nil - } - resolved := make(map[string]string, len(envVars)) - for _, ev := range envVars { - if ev.SecretID != "" && e.secretStore != nil { - value, err := e.secretStore.Reveal(ctx, ev.SecretID) - if err != nil { - e.logger.Warn("failed to resolve secret for profile env var", - zap.String("key", ev.Key), - zap.String("secret_id", ev.SecretID), - zap.Error(err)) - continue - } - resolved[ev.Key] = value - } else if ev.Value != "" { - resolved[ev.Key] = ev.Value - } - } - return resolved -} - func cloneMetadata(src map[string]interface{}) map[string]interface{} { if len(src) == 0 { return nil diff --git a/apps/backend/internal/orchestrator/executor/task_environment.go b/apps/backend/internal/orchestrator/executor/task_environment.go new file mode 100644 index 0000000000..41bee94779 --- /dev/null +++ b/apps/backend/internal/orchestrator/executor/task_environment.go @@ -0,0 +1,161 @@ +package executor + +import ( + "context" + "errors" + "fmt" + "sort" + "strings" + + runtimeenv "github.com/kandev/kandev/internal/agent/runtime/environment" + "github.com/kandev/kandev/internal/secrets" + "github.com/kandev/kandev/internal/task/models" +) + +type environmentSource struct { + key string + literal string + secretID string + origin string + workspaceID string +} + +type environmentSourceResolver func(context.Context, environmentSource) (string, error) + +// Keep the historical executor error names as aliases while the lifecycle and +// orchestrator share one source-aware resolver. +type EnvironmentConflictError = runtimeenv.ConflictError +type EnvironmentSecretError = runtimeenv.SecretError + +func resolveEnvironmentSources( + ctx context.Context, sources []environmentSource, resolve environmentSourceResolver, +) (map[string]string, error) { + definitions := make([]runtimeenv.Definition, 0, len(sources)) + for _, source := range sources { + definitions = append(definitions, runtimeenv.Definition{ + Key: source.key, Literal: source.literal, SecretID: source.secretID, + Origin: source.origin, WorkspaceID: source.workspaceID, + }) + } + return runtimeenv.Resolve(ctx, definitions, func(ctx context.Context, definition runtimeenv.Definition) (string, error) { + return resolve(ctx, environmentSource{ + key: definition.Key, literal: definition.Literal, secretID: definition.SecretID, + origin: definition.Origin, workspaceID: definition.WorkspaceID, + }) + }) +} + +func (e *Executor) taskEnvironmentSources( + workspaceID string, + managed map[string]string, + profileEnvVars []models.ProfileEnvVar, + repositories []*repoInfo, +) ([]environmentSource, error) { + // Avoid adding independent lengths to build the capacity. The sum can + // overflow before allocation when values originate in request-sized data. + sources := make([]environmentSource, 0) + managedKeys := make([]string, 0, len(managed)) + for key := range managed { + managedKeys = append(managedKeys, key) + } + sort.Strings(managedKeys) + for _, key := range managedKeys { + sources = append(sources, environmentSource{key: key, literal: managed[key], origin: "managed runtime"}) + } + for _, envVar := range profileEnvVars { + sources = append(sources, environmentSource{ + key: envVar.Key, literal: envVar.Value, secretID: envVar.SecretID, origin: "executor profile", + }) + } + for _, info := range repositories { + if info == nil || info.Repository == nil || len(info.Repository.SecretBindings) == 0 { + continue + } + if info.Repository.WorkspaceID != workspaceID { + return nil, fmt.Errorf("repository environment belongs to a different workspace") + } + origin := "repository" + if name := strings.TrimSpace(info.Repository.Name); name != "" { + origin = "repository " + name + } + for _, binding := range info.Repository.SecretBindings { + sources = append(sources, environmentSource{ + key: binding.Key, secretID: binding.SecretID, origin: origin, workspaceID: workspaceID, + }) + } + } + return sources, nil +} + +func (e *Executor) resolveLaunchEnvironment( + ctx context.Context, + req *LaunchAgentRequest, + profileEnvVars []models.ProfileEnvVar, + repositories []*repoInfo, +) error { + if req == nil { + return errors.New("launch request is required") + } + var preferredShellApplied bool + req.Env, preferredShellApplied = e.applyPreferredShellEnvWithStatus(ctx, req.ExecutorType, req.Env) + if preferredShellApplied { + profileEnvVars = withoutPreferredShellProfileEnvVars(profileEnvVars) + } + sources, err := e.taskEnvironmentSources(req.WorkspaceID, req.Env, profileEnvVars, repositories) + if err != nil { + return fmt.Errorf("build task environment sources: %w", err) + } + req.EnvironmentDefinitions = make([]runtimeenv.Definition, 0, len(sources)) + for _, source := range sources { + req.EnvironmentDefinitions = append(req.EnvironmentDefinitions, runtimeenv.Definition{ + Key: source.key, Literal: source.literal, SecretID: source.secretID, + Origin: source.origin, WorkspaceID: source.workspaceID, + }) + } + // This shape-only preflight keeps repository/repository and profile/repository + // conflicts from reaching the agent manager. Lifecycle repeats validation and + // performs the actual reveal after it has added managed runtime definitions. + if err := runtimeenv.Validate(req.EnvironmentDefinitions); err != nil { + return fmt.Errorf("validate task environment sources: %w", err) + } + req.EnvironmentResolutionRequired = true + keys := make(map[string]struct{}) + for _, info := range repositories { + if info == nil || info.Repository == nil { + continue + } + for _, binding := range info.Repository.SecretBindings { + if strings.TrimSpace(binding.Key) != "" { + keys[binding.Key] = struct{}{} + } + } + } + req.ApprovedSecretEnvKeys = make([]string, 0, len(keys)) + for key := range keys { + req.ApprovedSecretEnvKeys = append(req.ApprovedSecretEnvKeys, key) + } + sort.Strings(req.ApprovedSecretEnvKeys) + return nil +} + +func withoutPreferredShellProfileEnvVars(values []models.ProfileEnvVar) []models.ProfileEnvVar { + filtered := make([]models.ProfileEnvVar, 0, len(values)) + for _, value := range values { + if isPreferredShellEnvKey(value.Key) { + continue + } + filtered = append(filtered, value) + } + return filtered +} + +func isPreferredShellEnvKey(key string) bool { + return key == "SHELL" || key == "AGENTCTL_SHELL_COMMAND" +} + +func (e *Executor) revealGlobalSecret(ctx context.Context, secretID string) (string, error) { + if scoped, ok := e.secretStore.(secrets.ScopedSecretStore); ok { + return scoped.RevealGlobal(ctx, secretID) + } + return e.secretStore.Reveal(ctx, secretID) +} diff --git a/apps/backend/internal/orchestrator/executor/task_environment_test.go b/apps/backend/internal/orchestrator/executor/task_environment_test.go new file mode 100644 index 0000000000..ef40a1fe03 --- /dev/null +++ b/apps/backend/internal/orchestrator/executor/task_environment_test.go @@ -0,0 +1,129 @@ +package executor + +import ( + "context" + "errors" + "reflect" + "strings" + "testing" + + "github.com/kandev/kandev/internal/task/models" +) + +func TestResolveEnvironmentSources_DeduplicatesSameSecretAndIgnoresOrder(t *testing.T) { + resolver := func(_ context.Context, source environmentSource) (string, error) { + return map[string]string{"secret-a": "alpha", "secret-b": "bravo"}[source.secretID], nil + } + first := []environmentSource{ + {key: "TOKEN", secretID: "secret-a", origin: "repository app"}, + {key: "TOKEN", secretID: "secret-a", origin: "executor profile"}, + {key: "OTHER", secretID: "secret-b", origin: "repository tools"}, + } + second := []environmentSource{first[2], first[0], first[1]} + + gotFirst, err := resolveEnvironmentSources(context.Background(), first, resolver) + if err != nil { + t.Fatalf("first resolve: %v", err) + } + gotSecond, err := resolveEnvironmentSources(context.Background(), second, resolver) + if err != nil { + t.Fatalf("second resolve: %v", err) + } + if len(gotFirst) != 2 || gotFirst["TOKEN"] != "alpha" || gotFirst["OTHER"] != "bravo" { + t.Fatalf("first result = %#v", gotFirst) + } + if gotSecond["TOKEN"] != gotFirst["TOKEN"] || gotSecond["OTHER"] != gotFirst["OTHER"] { + t.Fatalf("order changed result: first=%#v second=%#v", gotFirst, gotSecond) + } +} + +func TestResolveEnvironmentSources_RejectsEveryConflictingPair(t *testing.T) { + cases := []struct { + name string + first environmentSource + last environmentSource + }{ + {name: "managed and executor literal", first: environmentSource{key: "TOKEN", literal: "managed", origin: "managed/runtime"}, last: environmentSource{key: "TOKEN", literal: "profile", origin: "executor profile"}}, + {name: "managed and repository secret", first: environmentSource{key: "TOKEN", literal: "managed", origin: "managed/runtime"}, last: environmentSource{key: "TOKEN", secretID: "secret-a", origin: "repository app"}}, + {name: "executor and repository secrets", first: environmentSource{key: "TOKEN", secretID: "secret-a", origin: "executor profile"}, last: environmentSource{key: "TOKEN", secretID: "secret-b", origin: "repository app"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := resolveEnvironmentSources(context.Background(), []environmentSource{tc.first, tc.last}, func(context.Context, environmentSource) (string, error) { + return "not-used", nil + }) + if err == nil { + t.Fatal("resolve succeeded, want conflict") + } + var conflictErr *EnvironmentConflictError + if !errors.As(err, &conflictErr) { + t.Fatalf("error = %T %v, want EnvironmentConflictError", err, err) + } + for _, leaked := range []string{"secret-a", "secret-b", "not-used"} { + if strings.Contains(err.Error(), leaked) { + t.Fatalf("error %q leaks %q", err, leaked) + } + } + }) + } +} + +func TestResolveEnvironmentSources_ReportsEveryConflictingOrigin(t *testing.T) { + _, err := resolveEnvironmentSources(context.Background(), []environmentSource{ + {key: "TOKEN", secretID: "secret-a", origin: "repository app"}, + {key: "TOKEN", secretID: "secret-b", origin: "repository tools"}, + {key: "TOKEN", literal: "profile-value", origin: "executor profile"}, + {key: "OTHER", literal: "unrelated", origin: "repository docs"}, + }, func(context.Context, environmentSource) (string, error) { + return "unused", nil + }) + if err == nil { + t.Fatal("resolve succeeded, want conflict") + } + var conflictErr *EnvironmentConflictError + if !errors.As(err, &conflictErr) { + t.Fatalf("error = %T %v, want EnvironmentConflictError", err, err) + } + want := []string{"executor profile", "repository app", "repository tools"} + if !reflect.DeepEqual(conflictErr.Origins, want) { + t.Fatalf("conflict origins = %#v, want %#v", conflictErr.Origins, want) + } +} + +func TestResolveLaunchEnvironment_PreferredShellWinsOverProfileShell(t *testing.T) { + executor := newTestExecutor(t, &mockAgentManager{}, newMockRepository()) + req := &LaunchAgentRequest{ + WorkspaceID: "workspace-1", + ExecutorType: string(models.ExecutorTypeLocal), + Env: map[string]string{}, + } + + err := executor.resolveLaunchEnvironment(context.Background(), req, []models.ProfileEnvVar{ + {Key: "SHELL", Value: "/bin/zsh"}, + {Key: "AGENTCTL_SHELL_COMMAND", Value: "/bin/fish"}, + {Key: "PROFILE_TOKEN", Value: "profile-value"}, + }, nil) + if err != nil { + t.Fatalf("resolve launch environment: %v", err) + } + if req.Env["SHELL"] != "/bin/bash" || req.Env["AGENTCTL_SHELL_COMMAND"] != "/bin/bash" { + t.Fatalf("preferred shell environment = %#v, want bash values", req.Env) + } + if len(req.EnvironmentDefinitions) != 3 { + t.Fatalf("environment definitions = %#v, want preferred shell and profile token", req.EnvironmentDefinitions) + } +} + +func TestResolveEnvironmentSources_RedactsFailedSecretReference(t *testing.T) { + _, err := resolveEnvironmentSources(context.Background(), []environmentSource{{ + key: "TOKEN", secretID: "secret-private", origin: "repository app", + }}, func(context.Context, environmentSource) (string, error) { + return "", errors.New("secret-private backend failure") + }) + if err == nil || !strings.Contains(err.Error(), "TOKEN") || !strings.Contains(err.Error(), "repository app") { + t.Fatalf("error = %v, want key and origin", err) + } + if strings.Contains(err.Error(), "secret-private") || strings.Contains(err.Error(), "backend failure") { + t.Fatalf("error %q leaks secret details", err) + } +} diff --git a/apps/backend/internal/secrets/handlers.go b/apps/backend/internal/secrets/handlers.go index 34ac6ece82..d2362f76ec 100644 --- a/apps/backend/internal/secrets/handlers.go +++ b/apps/backend/internal/secrets/handlers.go @@ -66,7 +66,15 @@ func (h *Handler) httpCreateSecret(c *gin.Context) { } func (h *Handler) httpListSecrets(c *gin.Context) { - items, err := h.service.List(c.Request.Context()) + opts := SecretListOptions{ + Scope: SecretScope(c.Query("scope")), + WorkspaceID: c.Query("workspace_id"), + IncludeGlobal: c.Query("include_global") == "true", + } + if opts.Scope == "" && opts.WorkspaceID != "" { + opts.Scope = ScopeWorkspace + } + items, err := h.service.ListScoped(c.Request.Context(), opts) if err != nil { h.logger.Error("failed to list secrets", zap.Error(err)) c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to list secrets"}) @@ -77,7 +85,7 @@ func (h *Handler) httpListSecrets(c *gin.Context) { func (h *Handler) httpGetSecret(c *gin.Context) { id := c.Param("id") - secret, err := h.service.Get(c.Request.Context(), id) + secret, err := h.getSecret(c, id) if err != nil { c.JSON(http.StatusNotFound, gin.H{"error": err.Error()}) return @@ -93,7 +101,7 @@ func (h *Handler) httpUpdateSecret(c *gin.Context) { return } - item, err := h.service.Update(c.Request.Context(), id, &req) + item, err := h.updateSecret(c, id, &req) if err != nil { h.logger.Error("failed to update secret", zap.Error(err)) c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) @@ -104,7 +112,7 @@ func (h *Handler) httpUpdateSecret(c *gin.Context) { func (h *Handler) httpDeleteSecret(c *gin.Context) { id := c.Param("id") - if err := h.service.Delete(c.Request.Context(), id); err != nil { + if err := h.deleteSecret(c, id); err != nil { c.JSON(http.StatusNotFound, gin.H{"error": err.Error()}) return } @@ -113,7 +121,7 @@ func (h *Handler) httpDeleteSecret(c *gin.Context) { func (h *Handler) httpRevealSecret(c *gin.Context) { id := c.Param("id") - value, err := h.service.Reveal(c.Request.Context(), id) + value, err := h.revealSecret(c, id) if err != nil { c.JSON(http.StatusNotFound, gin.H{"error": err.Error()}) return @@ -124,7 +132,20 @@ func (h *Handler) httpRevealSecret(c *gin.Context) { // WS handlers func (h *Handler) wsList(ctx context.Context, msg *ws.Message) (*ws.Message, error) { - items, err := h.service.List(ctx) + var req struct { + Scope SecretScope `json:"scope"` + WorkspaceID string `json:"workspace_id"` + IncludeGlobal bool `json:"include_global"` + } + if err := msg.ParsePayload(&req); err != nil { + return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "invalid payload: "+err.Error(), nil) + } + if req.Scope == "" && req.WorkspaceID != "" { + req.Scope = ScopeWorkspace + } + items, err := h.service.ListScoped(ctx, SecretListOptions{ + Scope: req.Scope, WorkspaceID: req.WorkspaceID, IncludeGlobal: req.IncludeGlobal, + }) if err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeInternalError, err.Error(), nil) } @@ -146,14 +167,15 @@ func (h *Handler) wsCreate(ctx context.Context, msg *ws.Message) (*ws.Message, e func (h *Handler) wsUpdate(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var payload struct { - ID string `json:"id"` + ID string `json:"id"` + WorkspaceID string `json:"workspace_id"` UpdateSecretRequest } if err := msg.ParsePayload(&payload); err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "invalid payload: "+err.Error(), nil) } - item, err := h.service.Update(ctx, payload.ID, &payload.UpdateSecretRequest) + item, err := h.updateSecretForWorkspace(ctx, payload.ID, payload.WorkspaceID, &payload.UpdateSecretRequest) if err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, err.Error(), nil) } @@ -162,13 +184,14 @@ func (h *Handler) wsUpdate(ctx context.Context, msg *ws.Message) (*ws.Message, e func (h *Handler) wsDelete(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var payload struct { - ID string `json:"id"` + ID string `json:"id"` + WorkspaceID string `json:"workspace_id"` } if err := msg.ParsePayload(&payload); err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "invalid payload: "+err.Error(), nil) } - if err := h.service.Delete(ctx, payload.ID); err != nil { + if err := h.deleteSecretForWorkspace(ctx, payload.ID, payload.WorkspaceID); err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeNotFound, err.Error(), nil) } return ws.NewResponse(msg.ID, msg.Action, map[string]bool{"success": true}) @@ -176,15 +199,65 @@ func (h *Handler) wsDelete(ctx context.Context, msg *ws.Message) (*ws.Message, e func (h *Handler) wsReveal(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var payload struct { - ID string `json:"id"` + ID string `json:"id"` + WorkspaceID string `json:"workspace_id"` } if err := msg.ParsePayload(&payload); err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "invalid payload: "+err.Error(), nil) } - value, err := h.service.Reveal(ctx, payload.ID) + value, err := h.revealSecretForWorkspace(ctx, payload.ID, payload.WorkspaceID) if err != nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeNotFound, err.Error(), nil) } return ws.NewResponse(msg.ID, msg.Action, RevealSecretResponse{Value: value}) } + +func (h *Handler) getSecret(c *gin.Context, id string) (*Secret, error) { + if workspaceID := c.Query("workspace_id"); workspaceID != "" { + return h.service.GetWorkspaceSecret(c.Request.Context(), id, workspaceID) + } + return h.service.Get(c.Request.Context(), id) +} + +func (h *Handler) updateSecret(c *gin.Context, id string, req *UpdateSecretRequest) (*SecretListItem, error) { + if workspaceID := c.Query("workspace_id"); workspaceID != "" { + return h.service.UpdateWorkspaceSecret(c.Request.Context(), id, workspaceID, req) + } + return h.service.Update(c.Request.Context(), id, req) +} + +func (h *Handler) updateSecretForWorkspace(ctx context.Context, id, workspaceID string, req *UpdateSecretRequest) (*SecretListItem, error) { + if workspaceID != "" { + return h.service.UpdateWorkspaceSecret(ctx, id, workspaceID, req) + } + return h.service.Update(ctx, id, req) +} + +func (h *Handler) deleteSecret(c *gin.Context, id string) error { + if workspaceID := c.Query("workspace_id"); workspaceID != "" { + return h.service.DeleteWorkspaceSecret(c.Request.Context(), id, workspaceID) + } + return h.service.Delete(c.Request.Context(), id) +} + +func (h *Handler) deleteSecretForWorkspace(ctx context.Context, id, workspaceID string) error { + if workspaceID != "" { + return h.service.DeleteWorkspaceSecret(ctx, id, workspaceID) + } + return h.service.Delete(ctx, id) +} + +func (h *Handler) revealSecret(c *gin.Context, id string) (string, error) { + if workspaceID := c.Query("workspace_id"); workspaceID != "" { + return h.service.RevealWorkspaceSecret(c.Request.Context(), id, workspaceID) + } + return h.service.Reveal(c.Request.Context(), id) +} + +func (h *Handler) revealSecretForWorkspace(ctx context.Context, id, workspaceID string) (string, error) { + if workspaceID != "" { + return h.service.RevealWorkspaceSecret(ctx, id, workspaceID) + } + return h.service.Reveal(ctx, id) +} diff --git a/apps/backend/internal/secrets/models.go b/apps/backend/internal/secrets/models.go index cb83271d70..d420587f61 100644 --- a/apps/backend/internal/secrets/models.go +++ b/apps/backend/internal/secrets/models.go @@ -2,12 +2,32 @@ package secrets import "time" +// SecretScope identifies the ownership boundary for a secret. +type SecretScope string + +const ( + // ScopeGlobal is visible to the current user in every workspace. When + // authentication is disabled it is install-global. + ScopeGlobal SecretScope = "global" + // ScopeWorkspace is visible only inside its owning workspace. + ScopeWorkspace SecretScope = "workspace" +) + +// SecretListOptions controls a metadata-only secret listing. +type SecretListOptions struct { + Scope SecretScope + WorkspaceID string + IncludeGlobal bool +} + // Secret represents stored secret metadata (without the value). type Secret struct { - ID string `json:"id" db:"id"` - Name string `json:"name" db:"name"` - CreatedAt time.Time `json:"created_at" db:"created_at"` - UpdatedAt time.Time `json:"updated_at" db:"updated_at"` + ID string `json:"id" db:"id"` + Name string `json:"name" db:"name"` + Scope SecretScope `json:"scope" db:"scope"` + WorkspaceID string `json:"workspace_id,omitempty" db:"workspace_id"` + CreatedAt time.Time `json:"created_at" db:"created_at"` + UpdatedAt time.Time `json:"updated_at" db:"updated_at"` } // SecretWithValue is used for create/update operations. @@ -18,17 +38,21 @@ type SecretWithValue struct { // SecretListItem is returned by list endpoints — never contains the value. type SecretListItem struct { - ID string `json:"id" db:"id"` - Name string `json:"name" db:"name"` - HasValue bool `json:"has_value" db:"has_value"` - CreatedAt time.Time `json:"created_at" db:"created_at"` - UpdatedAt time.Time `json:"updated_at" db:"updated_at"` + ID string `json:"id" db:"id"` + Name string `json:"name" db:"name"` + Scope SecretScope `json:"scope" db:"scope"` + WorkspaceID string `json:"workspace_id,omitempty" db:"workspace_id"` + HasValue bool `json:"has_value" db:"has_value"` + CreatedAt time.Time `json:"created_at" db:"created_at"` + UpdatedAt time.Time `json:"updated_at" db:"updated_at"` } // CreateSecretRequest is the request body for creating a secret. type CreateSecretRequest struct { - Name string `json:"name"` - Value string `json:"value"` + Name string `json:"name"` + Value string `json:"value"` + Scope SecretScope `json:"scope,omitempty"` + WorkspaceID string `json:"workspace_id,omitempty"` } // UpdateSecretRequest is the request body for updating a secret. diff --git a/apps/backend/internal/secrets/postgres_store_test.go b/apps/backend/internal/secrets/postgres_store_test.go index 5f47b517d4..dc51b14b9b 100644 --- a/apps/backend/internal/secrets/postgres_store_test.go +++ b/apps/backend/internal/secrets/postgres_store_test.go @@ -90,3 +90,56 @@ func TestPostgresStoreRoundTrip(t *testing.T) { t.Fatalf("get deleted secret error = %v, want secrets.ErrNotFound", err) } } + +func TestPostgresScopedSecretSchemaReplay(t *testing.T) { + db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) + + crypto, err := NewMasterKeyProvider(t.TempDir()) + if err != nil { + t.Fatalf("master key: %v", err) + } + store, cleanup, err := Provide(db, db, crypto) + if err != nil { + t.Fatalf("provide store: %v", err) + } + t.Cleanup(func() { _ = cleanup() }) + + ctx := context.Background() + secret := &SecretWithValue{ + Secret: Secret{ID: "postgres-scoped-secret-replay", Name: "replay-token"}, + Value: "replay-value", + } + if err := store.Create(ctx, secret); err != nil { + t.Fatalf("create secret: %v", err) + } + + // Simulate a legacy database that predates the scoped columns. Running the + // current schema initializer twice must restore both columns without + // changing the encrypted payload or row identity. + for _, column := range []string{"workspace_id", "scope"} { + if _, err := db.Exec("ALTER TABLE secrets DROP COLUMN " + column); err != nil { + t.Fatalf("drop legacy column %s: %v", column, err) + } + } + if err := store.initSchema(); err != nil { + t.Fatalf("replay scoped secret schema: %v", err) + } + if err := store.initSchema(); err != nil { + t.Fatalf("replay scoped secret schema twice: %v", err) + } + + metadata, err := store.Get(ctx, secret.ID) + if err != nil { + t.Fatalf("get migrated secret: %v", err) + } + if metadata.Scope != ScopeGlobal || metadata.WorkspaceID != "" { + t.Fatalf("migrated scope = (%q, %q), want (global, empty)", metadata.Scope, metadata.WorkspaceID) + } + value, err := store.Reveal(ctx, secret.ID) + if err != nil { + t.Fatalf("reveal migrated secret: %v", err) + } + if value != "replay-value" { + t.Fatalf("migrated secret value = %q, want %q", value, "replay-value") + } +} diff --git a/apps/backend/internal/secrets/service.go b/apps/backend/internal/secrets/service.go index c899951f8a..d235c91e81 100644 --- a/apps/backend/internal/secrets/service.go +++ b/apps/backend/internal/secrets/service.go @@ -10,8 +10,9 @@ import ( // Service provides business logic and validation for secrets. type Service struct { - store SecretStore - logger *logger.Logger + store SecretStore + logger *logger.Logger + workspaceAuthorizer func(context.Context, string) error } // NewService creates a new secrets service. @@ -22,6 +23,13 @@ func NewService(store SecretStore, log *logger.Logger) *Service { } } +// SetWorkspaceAuthorizer wires the workspace visibility check used by the +// workspace-scoped API. The callback is intentionally a function so the +// secrets package does not depend on the task service package. +func (s *Service) SetWorkspaceAuthorizer(authorizer func(context.Context, string) error) { + s.workspaceAuthorizer = authorizer +} + func (s *Service) validateCreate(req *CreateSecretRequest) error { req.Name = strings.TrimSpace(req.Name) @@ -31,6 +39,12 @@ func (s *Service) validateCreate(req *CreateSecretRequest) error { if req.Value == "" || len(req.Value) > 10000 { return fmt.Errorf("value must be 1-10000 characters") } + if req.Scope == "" { + req.Scope = ScopeGlobal + } + if err := validateSecretScope(req.Scope, req.WorkspaceID); err != nil { + return fmt.Errorf("scope validation: %w", err) + } return nil } @@ -53,10 +67,15 @@ func (s *Service) Create(ctx context.Context, req *CreateSecretRequest) (*Secret if err := s.validateCreate(req); err != nil { return nil, fmt.Errorf("validation: %w", err) } + if err := s.authorizeScope(ctx, req.Scope, req.WorkspaceID); err != nil { + return nil, err + } secret := &SecretWithValue{ Secret: Secret{ - Name: req.Name, + Name: req.Name, + Scope: req.Scope, + WorkspaceID: req.WorkspaceID, }, Value: req.Value, } @@ -65,22 +84,83 @@ func (s *Service) Create(ctx context.Context, req *CreateSecretRequest) (*Secret return nil, fmt.Errorf("create secret: %w", err) } - return &SecretListItem{ - ID: secret.ID, - Name: secret.Name, - HasValue: true, - CreatedAt: secret.CreatedAt, - UpdatedAt: secret.UpdatedAt, - }, nil + return secretListItem(&secret.Secret), nil } // Get retrieves secret metadata. func (s *Service) Get(ctx context.Context, id string) (*Secret, error) { - return s.store.Get(ctx, id) + secret, err := s.store.Get(ctx, id) + if err != nil { + return nil, err + } + if normalizeStoredScope(secret.Scope) != ScopeGlobal { + return nil, fmt.Errorf("%w: %s", ErrNotFound, id) + } + return secret, nil +} + +// GetForWorkspace retrieves a Global or same-workspace secret after checking +// the caller's workspace access. +func (s *Service) GetForWorkspace(ctx context.Context, id, workspaceID string) (*Secret, error) { + if err := s.authorizeScope(ctx, ScopeWorkspace, workspaceID); err != nil { + return nil, err + } + if scoped, ok := s.store.(ScopedSecretStore); ok { + return scoped.GetForWorkspace(ctx, id, workspaceID) + } + secret, err := s.store.Get(ctx, id) + if err != nil { + return nil, err + } + if normalizeStoredScope(secret.Scope) == ScopeGlobal || secret.WorkspaceID == workspaceID { + return secret, nil + } + return nil, fmt.Errorf("%w: %s", ErrNotFound, id) +} + +// GetWorkspaceSecret retrieves a user-visible Workspace secret. Unlike +// GetForWorkspace, this deliberately excludes Global secrets because it backs +// workspace-scoped CRUD and reveal routes rather than repository selection. +func (s *Service) GetWorkspaceSecret(ctx context.Context, id, workspaceID string) (*Secret, error) { + secret, err := s.GetForWorkspace(ctx, id, workspaceID) + if err != nil { + return nil, err + } + if normalizeStoredScope(secret.Scope) != ScopeWorkspace || secret.WorkspaceID != workspaceID { + return nil, fmt.Errorf("%w: %s", ErrNotFound, id) + } + return secret, nil } // Reveal returns the decrypted secret value. func (s *Service) Reveal(ctx context.Context, id string) (string, error) { + if _, err := s.Get(ctx, id); err != nil { + return "", err + } + if scoped, ok := s.store.(ScopedSecretStore); ok { + return scoped.RevealGlobal(ctx, id) + } + return s.store.Reveal(ctx, id) +} + +// RevealForWorkspace reveals a Global or same-workspace secret. +func (s *Service) RevealForWorkspace(ctx context.Context, id, workspaceID string) (string, error) { + if _, err := s.GetForWorkspace(ctx, id, workspaceID); err != nil { + return "", err + } + if scoped, ok := s.store.(ScopedSecretStore); ok { + return scoped.RevealForWorkspace(ctx, id, workspaceID) + } + return s.store.Reveal(ctx, id) +} + +func (s *Service) RevealWorkspaceSecret(ctx context.Context, id, workspaceID string) (string, error) { + if _, err := s.GetWorkspaceSecret(ctx, id, workspaceID); err != nil { + return "", err + } + if scoped, ok := s.store.(ScopedSecretStore); ok { + return scoped.RevealForWorkspace(ctx, id, workspaceID) + } return s.store.Reveal(ctx, id) } @@ -90,6 +170,9 @@ func (s *Service) Update(ctx context.Context, id string, req *UpdateSecretReques return nil, fmt.Errorf("validation: %w", err) } + if _, err := s.Get(ctx, id); err != nil { + return nil, err + } if err := s.store.Update(ctx, id, req); err != nil { return nil, fmt.Errorf("update secret: %w", err) } @@ -99,21 +182,114 @@ func (s *Service) Update(ctx context.Context, id string, req *UpdateSecretReques return nil, err } - return &SecretListItem{ - ID: secret.ID, - Name: secret.Name, - HasValue: true, - CreatedAt: secret.CreatedAt, - UpdatedAt: secret.UpdatedAt, - }, nil + return secretListItem(secret), nil +} + +// UpdateForWorkspace updates a Workspace secret after checking its scope and +// the caller's access. Global secrets remain valid through this endpoint too. +func (s *Service) UpdateForWorkspace(ctx context.Context, id, workspaceID string, req *UpdateSecretRequest) (*SecretListItem, error) { + if err := s.validateUpdate(req); err != nil { + return nil, fmt.Errorf("validation: %w", err) + } + if _, err := s.GetForWorkspace(ctx, id, workspaceID); err != nil { + return nil, err + } + if err := s.store.Update(ctx, id, req); err != nil { + return nil, fmt.Errorf("update secret: %w", err) + } + secret, err := s.store.Get(ctx, id) + if err != nil { + return nil, err + } + return secretListItem(secret), nil +} + +func (s *Service) UpdateWorkspaceSecret(ctx context.Context, id, workspaceID string, req *UpdateSecretRequest) (*SecretListItem, error) { + if err := s.validateUpdate(req); err != nil { + return nil, fmt.Errorf("validation: %w", err) + } + if _, err := s.GetWorkspaceSecret(ctx, id, workspaceID); err != nil { + return nil, err + } + if err := s.store.Update(ctx, id, req); err != nil { + return nil, fmt.Errorf("update secret: %w", err) + } + secret, err := s.store.Get(ctx, id) + if err != nil { + return nil, err + } + return secretListItem(secret), nil } // Delete removes a secret. func (s *Service) Delete(ctx context.Context, id string) error { + if _, err := s.Get(ctx, id); err != nil { + return err + } + return s.store.Delete(ctx, id) +} + +// DeleteForWorkspace deletes a Global or same-workspace secret. +func (s *Service) DeleteForWorkspace(ctx context.Context, id, workspaceID string) error { + if _, err := s.GetForWorkspace(ctx, id, workspaceID); err != nil { + return err + } + return s.store.Delete(ctx, id) +} + +func (s *Service) DeleteWorkspaceSecret(ctx context.Context, id, workspaceID string) error { + if _, err := s.GetWorkspaceSecret(ctx, id, workspaceID); err != nil { + return err + } return s.store.Delete(ctx, id) } // List returns all secrets without values. func (s *Service) List(ctx context.Context) ([]*SecretListItem, error) { - return s.store.List(ctx) + return s.ListScoped(ctx, SecretListOptions{Scope: ScopeGlobal}) +} + +// ListScoped lists secrets in one scope. Workspace listings require access to +// that workspace before any metadata is returned. +func (s *Service) ListScoped(ctx context.Context, opts SecretListOptions) ([]*SecretListItem, error) { + if opts.Scope == "" { + opts.Scope = ScopeGlobal + } + if err := validateListOptions(opts); err != nil { + return nil, err + } + if opts.Scope == ScopeWorkspace { + if err := s.authorizeScope(ctx, ScopeWorkspace, opts.WorkspaceID); err != nil { + return nil, err + } + } + if scoped, ok := s.store.(ScopedSecretStore); ok { + return scoped.ListScoped(ctx, opts) + } + if opts.Scope == ScopeGlobal { + return s.store.List(ctx) + } + return nil, fmt.Errorf("workspace-scoped secret storage is unavailable") +} + +func (s *Service) authorizeScope(ctx context.Context, scope SecretScope, workspaceID string) error { + if scope != ScopeWorkspace { + return nil + } + if s.workspaceAuthorizer == nil { + return fmt.Errorf("workspace secret access is unavailable") + } + return s.workspaceAuthorizer(ctx, workspaceID) +} + +func secretListItem(secret *Secret) *SecretListItem { + return &SecretListItem{ + ID: secret.ID, + Name: secret.Name, + Scope: normalizeStoredScope(secret.Scope), + WorkspaceID: secret.WorkspaceID, + HasValue: true, + CreatedAt: secret.CreatedAt, + UpdatedAt: secret.UpdatedAt, + } } diff --git a/apps/backend/internal/secrets/service_test.go b/apps/backend/internal/secrets/service_test.go new file mode 100644 index 0000000000..9434bc94d8 --- /dev/null +++ b/apps/backend/internal/secrets/service_test.go @@ -0,0 +1,142 @@ +package secrets + +import ( + "context" + "errors" + "testing" +) + +func TestService_WorkspaceSecretRequiresWorkspaceAuthorization(t *testing.T) { + store := newTestSQLiteStore(t) + svc := NewService(store, nil) + wantErr := errors.New("workspace denied") + svc.SetWorkspaceAuthorizer(func(context.Context, string) error { return wantErr }) + + _, err := svc.Create(context.Background(), &CreateSecretRequest{ + Name: "workspace token", + Value: "value", + Scope: ScopeWorkspace, + WorkspaceID: "workspace-a", + }) + if !errors.Is(err, wantErr) { + t.Fatalf("Create error = %v, want authorization error", err) + } +} + +func TestService_GlobalDefaultAndScopedOperations(t *testing.T) { + store := newTestSQLiteStore(t) + svc := NewService(store, nil) + authorized := map[string]bool{"workspace-a": true} + svc.SetWorkspaceAuthorizer(func(_ context.Context, workspaceID string) error { + if !authorized[workspaceID] { + return errors.New("workspace denied") + } + return nil + }) + + global, err := svc.Create(context.Background(), &CreateSecretRequest{Name: "global", Value: "value"}) + if err != nil { + t.Fatalf("create default global: %v", err) + } + if global.Scope != ScopeGlobal || global.WorkspaceID != "" { + t.Fatalf("global result = %+v, want global scope", global) + } + + workspace, err := svc.Create(context.Background(), &CreateSecretRequest{ + Name: "workspace", + Value: "value", + Scope: ScopeWorkspace, + WorkspaceID: "workspace-a", + }) + if err != nil { + t.Fatalf("create workspace: %v", err) + } + if workspace.Scope != ScopeWorkspace || workspace.WorkspaceID != "workspace-a" { + t.Fatalf("workspace result = %+v", workspace) + } + + items, err := svc.List(context.Background()) + if err != nil { + t.Fatalf("list global: %v", err) + } + if len(items) != 1 || items[0].ID != global.ID { + t.Fatalf("global list = %+v, want only %s", items, global.ID) + } + + workspaceItems, err := svc.ListScoped(context.Background(), SecretListOptions{ + Scope: ScopeWorkspace, + WorkspaceID: "workspace-a", + }) + if err != nil { + t.Fatalf("list workspace: %v", err) + } + if len(workspaceItems) != 1 || workspaceItems[0].ID != workspace.ID { + t.Fatalf("workspace list = %+v, want only %s", workspaceItems, workspace.ID) + } + + if _, err := svc.Get(context.Background(), workspace.ID); !errors.Is(err, ErrNotFound) { + t.Fatalf("global get of workspace secret = %v, want ErrNotFound", err) + } + if _, err := svc.GetForWorkspace(context.Background(), workspace.ID, "workspace-a"); err != nil { + t.Fatalf("workspace get: %v", err) + } + if _, err := svc.GetForWorkspace(context.Background(), workspace.ID, "workspace-b"); err == nil { + t.Fatal("workspace get from unauthorized workspace succeeded") + } +} + +func TestService_WorkspaceCRUDDoesNotMutateGlobalSecrets(t *testing.T) { + store := newTestSQLiteStore(t) + svc := NewService(store, nil) + svc.SetWorkspaceAuthorizer(func(context.Context, string) error { return nil }) + + global, err := svc.Create(context.Background(), &CreateSecretRequest{Name: "global", Value: "value"}) + if err != nil { + t.Fatalf("create global: %v", err) + } + name := "changed" + if _, err := svc.UpdateWorkspaceSecret(context.Background(), global.ID, "workspace-a", &UpdateSecretRequest{Name: &name}); !errors.Is(err, ErrNotFound) { + t.Fatalf("workspace update of global = %v, want ErrNotFound", err) + } + if _, err := svc.GetWorkspaceSecret(context.Background(), global.ID, "workspace-a"); !errors.Is(err, ErrNotFound) { + t.Fatalf("workspace get of global = %v, want ErrNotFound", err) + } + if _, err := svc.RevealWorkspaceSecret(context.Background(), global.ID, "workspace-a"); !errors.Is(err, ErrNotFound) { + t.Fatalf("workspace reveal of global = %v, want ErrNotFound", err) + } + if err := svc.DeleteWorkspaceSecret(context.Background(), global.ID, "workspace-a"); !errors.Is(err, ErrNotFound) { + t.Fatalf("workspace delete of global = %v, want ErrNotFound", err) + } + + item, err := svc.Update(context.Background(), global.ID, &UpdateSecretRequest{Name: &name}) + if err != nil { + t.Fatalf("global update: %v", err) + } + if item.Name != name { + t.Fatalf("global name = %q, want %q", item.Name, name) + } +} + +func TestService_NormalizesLegacyGlobalScopeOnUpdate(t *testing.T) { + store := newTestSQLiteStore(t) + svc := NewService(store, nil) + ctx := context.Background() + + global, err := svc.Create(ctx, &CreateSecretRequest{Name: "legacy", Value: "value"}) + if err != nil { + t.Fatalf("create global: %v", err) + } + if _, err := store.db.ExecContext(ctx, store.db.Rebind(`UPDATE secrets SET scope = '' WHERE id = ?`), global.ID); err != nil { + t.Fatalf("seed legacy scope: %v", err) + } + + updated, err := svc.Update(ctx, global.ID, &UpdateSecretRequest{Name: stringPtr("renamed")}) + if err != nil { + t.Fatalf("update legacy global: %v", err) + } + if updated.Scope != ScopeGlobal { + t.Fatalf("updated scope = %q, want %q", updated.Scope, ScopeGlobal) + } +} + +func stringPtr(value string) *string { return &value } diff --git a/apps/backend/internal/secrets/sqlite_store.go b/apps/backend/internal/secrets/sqlite_store.go index 28070d8989..2c92ccb28e 100644 --- a/apps/backend/internal/secrets/sqlite_store.go +++ b/apps/backend/internal/secrets/sqlite_store.go @@ -4,6 +4,7 @@ import ( "context" "database/sql" "fmt" + "strings" "time" "github.com/google/uuid" @@ -21,7 +22,7 @@ type sqliteStore struct { ownsDB bool } -var _ SecretStore = (*sqliteStore)(nil) +var _ ScopedSecretStore = (*sqliteStore)(nil) // Provide creates the SQLite secret store using separate writer and reader pools. func Provide(writer, reader *sqlx.DB, crypto *MasterKeyProvider) (*sqliteStore, func() error, error) { @@ -39,6 +40,8 @@ func (s *sqliteStore) initSchema() error { id TEXT PRIMARY KEY, name TEXT NOT NULL, user_id TEXT NOT NULL DEFAULT '', + scope TEXT NOT NULL DEFAULT 'global', + workspace_id TEXT NOT NULL DEFAULT '', encrypted_value %s NOT NULL, nonce %s NOT NULL, created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, @@ -52,6 +55,8 @@ func (s *sqliteStore) initSchema() error { // column must also be added via an idempotent migration (ADR 0027). migrate := db.NewMigrateLogger(s.db, nil) migrate.Apply("secrets.user_id", "ALTER TABLE secrets ADD COLUMN user_id TEXT NOT NULL DEFAULT ''") + migrate.Apply("secrets.scope", "ALTER TABLE secrets ADD COLUMN scope TEXT NOT NULL DEFAULT 'global'") + migrate.Apply("secrets.workspace_id", "ALTER TABLE secrets ADD COLUMN workspace_id TEXT NOT NULL DEFAULT ''") return nil } @@ -84,6 +89,15 @@ func (s *sqliteStore) Close() error { } func (s *sqliteStore) Create(ctx context.Context, secret *SecretWithValue) error { + if secret == nil { + return fmt.Errorf("secret is required") + } + if secret.Scope == "" { + secret.Scope = ScopeGlobal + } + if err := validateSecretScope(secret.Scope, secret.WorkspaceID); err != nil { + return err + } if secret.ID == "" { secret.ID = uuid.New().String() } @@ -97,9 +111,9 @@ func (s *sqliteStore) Create(ctx context.Context, secret *SecretWithValue) error } _, err = s.db.ExecContext(ctx, s.db.Rebind(` - INSERT INTO secrets (id, name, user_id, encrypted_value, nonce, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?)`), - secret.ID, secret.Name, scopeOwner(ctx), ciphertext, nonce, now, now, + INSERT INTO secrets (id, name, user_id, scope, workspace_id, encrypted_value, nonce, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)`), + secret.ID, secret.Name, scopeOwner(ctx), secret.Scope, secret.WorkspaceID, ciphertext, nonce, now, now, ) if err != nil { return fmt.Errorf("insert secret: %w", err) @@ -110,7 +124,7 @@ func (s *sqliteStore) Create(ctx context.Context, secret *SecretWithValue) error func (s *sqliteStore) Get(ctx context.Context, id string) (*Secret, error) { var row secretRow err := s.ro.GetContext(ctx, &row, s.ro.Rebind(` - SELECT id, name, created_at, updated_at + SELECT id, name, scope, workspace_id, created_at, updated_at FROM secrets WHERE id = ? AND (user_id = '' OR ? = '' OR user_id = ?)`), id, scopeOwner(ctx), scopeOwner(ctx)) if err != nil { @@ -162,8 +176,8 @@ func (s *sqliteStore) Update(ctx context.Context, id string, req *UpdateSecretRe } _, err = s.db.ExecContext(ctx, s.db.Rebind(` UPDATE secrets SET name = ?, encrypted_value = ?, nonce = ?, updated_at = ? - WHERE id = ?`), - existing.Name, ciphertext, nonce, now, id, + WHERE id = ? AND (user_id = '' OR ? = '' OR user_id = ?)`), + existing.Name, ciphertext, nonce, now, id, scopeOwner(ctx), scopeOwner(ctx), ) if err != nil { return fmt.Errorf("update secret: %w", err) @@ -171,8 +185,8 @@ func (s *sqliteStore) Update(ctx context.Context, id string, req *UpdateSecretRe } else { _, err = s.db.ExecContext(ctx, s.db.Rebind(` UPDATE secrets SET name = ?, updated_at = ? - WHERE id = ?`), - existing.Name, now, id, + WHERE id = ? AND (user_id = '' OR ? = '' OR user_id = ?)`), + existing.Name, now, id, scopeOwner(ctx), scopeOwner(ctx), ) if err != nil { return fmt.Errorf("update secret: %w", err) @@ -196,54 +210,182 @@ func (s *sqliteStore) Delete(ctx context.Context, id string) error { } func (s *sqliteStore) List(ctx context.Context) ([]*SecretListItem, error) { + return s.ListScoped(ctx, SecretListOptions{Scope: ScopeGlobal}) +} + +func (s *sqliteStore) ListScoped(ctx context.Context, opts SecretListOptions) ([]*SecretListItem, error) { + if err := validateListOptions(opts); err != nil { + return nil, err + } var rows []secretListRow - err := s.ro.SelectContext(ctx, &rows, s.ro.Rebind(` - SELECT id, name, 1 as has_value, created_at, updated_at - FROM secrets WHERE (user_id = '' OR ? = '' OR user_id = ?) - ORDER BY created_at DESC`), - scopeOwner(ctx), scopeOwner(ctx)) + query := ` + SELECT id, name, scope, workspace_id, 1 as has_value, created_at, updated_at + FROM secrets WHERE (user_id = '' OR ? = '' OR user_id = ?)` + args := []any{scopeOwner(ctx), scopeOwner(ctx)} + switch opts.Scope { + case ScopeGlobal: + query += " AND scope = ?" + args = append(args, ScopeGlobal) + case ScopeWorkspace: + query += " AND ((scope = ? AND workspace_id = ?)" + args = append(args, ScopeWorkspace, opts.WorkspaceID) + if opts.IncludeGlobal { + query += " OR scope = ?" + args = append(args, ScopeGlobal) + } + query += ")" + } + query += " ORDER BY created_at DESC" + err := s.ro.SelectContext(ctx, &rows, s.ro.Rebind(query), args...) if err != nil { return nil, fmt.Errorf("list secrets: %w", err) } return toSecretListItems(rows), nil } +func (s *sqliteStore) GetForWorkspace(ctx context.Context, id, workspaceID string) (*Secret, error) { + if strings.TrimSpace(workspaceID) == "" { + return nil, fmt.Errorf("workspace id is required") + } + secret, err := s.Get(ctx, id) + if err != nil { + return nil, err + } + if secret.Scope != ScopeGlobal && (secret.Scope != ScopeWorkspace || secret.WorkspaceID != workspaceID) { + return nil, fmt.Errorf("%w: %s", ErrNotFound, id) + } + return secret, nil +} + +func (s *sqliteStore) RevealGlobal(ctx context.Context, id string) (string, error) { + secret, err := s.Get(ctx, id) + if err != nil { + return "", err + } + if secret.Scope != ScopeGlobal { + return "", fmt.Errorf("%w: %s", ErrNotFound, id) + } + return s.Reveal(ctx, id) +} + +func (s *sqliteStore) RevealForWorkspace(ctx context.Context, id, workspaceID string) (string, error) { + if _, err := s.GetForWorkspace(ctx, id, workspaceID); err != nil { + return "", err + } + return s.Reveal(ctx, id) +} + +func (s *sqliteStore) DeleteWorkspaceSecrets(ctx context.Context, workspaceID string) error { + if strings.TrimSpace(workspaceID) == "" { + return fmt.Errorf("workspace id is required") + } + return s.deleteWorkspaceSecrets(ctx, s.db, workspaceID) +} + +// DeleteWorkspaceSecretsTx removes workspace-owned secrets on the supplied +// transaction so workspace and secret deletion can commit or roll back as one. +func (s *sqliteStore) DeleteWorkspaceSecretsTx(ctx context.Context, tx *sqlx.Tx, workspaceID string) error { + if tx == nil { + return fmt.Errorf("transaction is required") + } + return s.deleteWorkspaceSecrets(ctx, tx, workspaceID) +} + +func (s *sqliteStore) deleteWorkspaceSecrets(ctx context.Context, exec sqlx.ExtContext, workspaceID string) error { + if strings.TrimSpace(workspaceID) == "" { + return fmt.Errorf("workspace id is required") + } + _, err := exec.ExecContext(ctx, s.db.Rebind(` + DELETE FROM secrets WHERE scope = ? AND workspace_id = ?`), ScopeWorkspace, workspaceID) + if err != nil { + return fmt.Errorf("delete workspace secrets: %w", err) + } + return nil +} + +func validateSecretScope(scope SecretScope, workspaceID string) error { + switch scope { + case ScopeGlobal: + if strings.TrimSpace(workspaceID) != "" { + return fmt.Errorf("global secrets cannot have a workspace") + } + case ScopeWorkspace: + if strings.TrimSpace(workspaceID) == "" { + return fmt.Errorf("workspace secrets require a workspace") + } + default: + return fmt.Errorf("invalid secret scope") + } + return nil +} + +func validateListOptions(opts SecretListOptions) error { + switch opts.Scope { + case ScopeGlobal: + if strings.TrimSpace(opts.WorkspaceID) != "" { + return fmt.Errorf("global secret listing cannot have a workspace") + } + case ScopeWorkspace: + if strings.TrimSpace(opts.WorkspaceID) == "" { + return fmt.Errorf("workspace secret listing requires a workspace") + } + default: + return fmt.Errorf("invalid secret scope") + } + return nil +} + // secretRow is the DB scan target for secret metadata queries. type secretRow struct { - ID string `db:"id"` - Name string `db:"name"` - CreatedAt time.Time `db:"created_at"` - UpdatedAt time.Time `db:"updated_at"` + ID string `db:"id"` + Name string `db:"name"` + Scope SecretScope `db:"scope"` + WorkspaceID string `db:"workspace_id"` + CreatedAt time.Time `db:"created_at"` + UpdatedAt time.Time `db:"updated_at"` } func (r *secretRow) toSecret() *Secret { return &Secret{ - ID: r.ID, - Name: r.Name, - CreatedAt: r.CreatedAt, - UpdatedAt: r.UpdatedAt, + ID: r.ID, + Name: r.Name, + Scope: normalizeStoredScope(r.Scope), + WorkspaceID: r.WorkspaceID, + CreatedAt: r.CreatedAt, + UpdatedAt: r.UpdatedAt, } } // secretListRow is the DB scan target for list queries. type secretListRow struct { - ID string `db:"id"` - Name string `db:"name"` - HasValue bool `db:"has_value"` - CreatedAt time.Time `db:"created_at"` - UpdatedAt time.Time `db:"updated_at"` + ID string `db:"id"` + Name string `db:"name"` + Scope SecretScope `db:"scope"` + WorkspaceID string `db:"workspace_id"` + HasValue bool `db:"has_value"` + CreatedAt time.Time `db:"created_at"` + UpdatedAt time.Time `db:"updated_at"` } func toSecretListItems(rows []secretListRow) []*SecretListItem { items := make([]*SecretListItem, len(rows)) for i, r := range rows { items[i] = &SecretListItem{ - ID: r.ID, - Name: r.Name, - HasValue: r.HasValue, - CreatedAt: r.CreatedAt, - UpdatedAt: r.UpdatedAt, + ID: r.ID, + Name: r.Name, + Scope: normalizeStoredScope(r.Scope), + WorkspaceID: r.WorkspaceID, + HasValue: r.HasValue, + CreatedAt: r.CreatedAt, + UpdatedAt: r.UpdatedAt, } } return items } + +func normalizeStoredScope(scope SecretScope) SecretScope { + if scope == "" { + return ScopeGlobal + } + return scope +} diff --git a/apps/backend/internal/secrets/sqlite_store_test.go b/apps/backend/internal/secrets/sqlite_store_test.go index da7c1f9ade..49c0b13822 100644 --- a/apps/backend/internal/secrets/sqlite_store_test.go +++ b/apps/backend/internal/secrets/sqlite_store_test.go @@ -128,3 +128,85 @@ func TestSQLiteStore_PerUserScoping(t *testing.T) { t.Fatalf("post-claim foreign reveal: %v, want ErrNotFound", err) } } + +func TestSQLiteStore_ScopedSecretVisibility(t *testing.T) { + store := newTestSQLiteStore(t) + ctx := context.Background() + userA := authn.WithIdentity(ctx, authn.Identity{UserID: "user-a", Role: authn.RoleMember}) + userB := authn.WithIdentity(ctx, authn.Identity{UserID: "user-b", Role: authn.RoleMember}) + + global := &SecretWithValue{Secret: Secret{Name: "global"}, Value: "global-value"} + workspace := &SecretWithValue{Secret: Secret{ + Name: "workspace", + Scope: ScopeWorkspace, + WorkspaceID: "workspace-a", + }, Value: "workspace-value"} + if err := store.Create(userA, global); err != nil { + t.Fatalf("create global: %v", err) + } + if err := store.Create(userA, workspace); err != nil { + t.Fatalf("create workspace: %v", err) + } + + globalItems, err := store.ListScoped(userA, SecretListOptions{Scope: ScopeGlobal}) + if err != nil { + t.Fatalf("list global: %v", err) + } + if len(globalItems) != 1 || globalItems[0].ID != global.ID || globalItems[0].Scope != ScopeGlobal { + t.Fatalf("global items = %+v, want only %s", globalItems, global.ID) + } + + workspaceItems, err := store.ListScoped(userA, SecretListOptions{ + Scope: ScopeWorkspace, + WorkspaceID: "workspace-a", + }) + if err != nil { + t.Fatalf("list workspace: %v", err) + } + if len(workspaceItems) != 1 || workspaceItems[0].ID != workspace.ID { + t.Fatalf("workspace items = %+v, want only %s", workspaceItems, workspace.ID) + } + + if _, err := store.GetForWorkspace(userA, workspace.ID, "workspace-a"); err != nil { + t.Fatalf("workspace owner get: %v", err) + } + if _, err := store.GetForWorkspace(userB, workspace.ID, "workspace-a"); !errors.Is(err, ErrNotFound) { + t.Fatalf("foreign workspace get = %v, want ErrNotFound", err) + } + if _, err := store.GetForWorkspace(userA, workspace.ID, "workspace-b"); !errors.Is(err, ErrNotFound) { + t.Fatalf("wrong workspace get = %v, want ErrNotFound", err) + } + if _, err := store.RevealGlobal(userA, workspace.ID); !errors.Is(err, ErrNotFound) { + t.Fatalf("workspace reveal through global API = %v, want ErrNotFound", err) + } + + if err := store.DeleteWorkspaceSecrets(ctx, "workspace-a"); err != nil { + t.Fatalf("delete workspace secrets: %v", err) + } + if _, err := store.Get(ctx, workspace.ID); !errors.Is(err, ErrNotFound) { + t.Fatalf("deleted workspace secret = %v, want ErrNotFound", err) + } +} + +func TestSQLiteStore_ListScopedRequiresExplicitScope(t *testing.T) { + store := newTestSQLiteStore(t) + ctx := context.Background() + if err := store.Create(ctx, &SecretWithValue{Secret: Secret{Name: "global"}, Value: "global"}); err != nil { + t.Fatalf("create global: %v", err) + } + if err := store.Create(ctx, &SecretWithValue{Secret: Secret{ + Name: "workspace", Scope: ScopeWorkspace, WorkspaceID: "workspace-a", + }, Value: "workspace"}); err != nil { + t.Fatalf("create workspace: %v", err) + } + if _, err := store.ListScoped(ctx, SecretListOptions{}); err == nil { + t.Fatal("ListScoped with no scope succeeded, want explicit-scope error") + } + items, err := store.List(ctx) + if err != nil { + t.Fatalf("List: %v", err) + } + if len(items) != 1 || items[0].Scope != ScopeGlobal { + t.Fatalf("List returned %#v, want only global secret", items) + } +} diff --git a/apps/backend/internal/secrets/store.go b/apps/backend/internal/secrets/store.go index a89fe3d445..a8b087331a 100644 --- a/apps/backend/internal/secrets/store.go +++ b/apps/backend/internal/secrets/store.go @@ -3,6 +3,9 @@ package secrets import ( "context" "errors" + "fmt" + + "github.com/jmoiron/sqlx" ) // ErrNotFound is the sentinel returned (wrapped) by store implementations when @@ -35,3 +38,38 @@ type SecretStore interface { // Close releases resources. Close() error } + +// ValidateGlobalReference verifies that a stored reference is usable by a +// shared agent or executor profile without decrypting its value. +func ValidateGlobalReference(ctx context.Context, store SecretStore, id string) error { + if store == nil || id == "" { + return fmt.Errorf("global secret reference is required") + } + secret, err := store.Get(ctx, id) + if err != nil { + return err + } + if normalizeStoredScope(secret.Scope) != ScopeGlobal { + return fmt.Errorf("secret reference must use the global scope") + } + return nil +} + +// ScopedSecretStore is the optional scope-aware extension used by user-facing +// settings, repository binding validation, and task-environment resolution. +// SecretStore deliberately remains small so existing integration adapters and +// test doubles do not need to know about workspace ownership. +type ScopedSecretStore interface { + SecretStore + ListScoped(ctx context.Context, opts SecretListOptions) ([]*SecretListItem, error) + GetForWorkspace(ctx context.Context, id, workspaceID string) (*Secret, error) + RevealGlobal(ctx context.Context, id string) (string, error) + RevealForWorkspace(ctx context.Context, id, workspaceID string) (string, error) + DeleteWorkspaceSecrets(ctx context.Context, workspaceID string) error +} + +// WorkspaceSecretTransactionalDeleter is implemented by stores that can +// remove workspace-owned secrets on a caller-owned SQL transaction. +type WorkspaceSecretTransactionalDeleter interface { + DeleteWorkspaceSecretsTx(ctx context.Context, tx *sqlx.Tx, workspaceID string) error +} diff --git a/apps/backend/internal/secrets/user_visible_store.go b/apps/backend/internal/secrets/user_visible_store.go index 006a50a36d..2c64cdded2 100644 --- a/apps/backend/internal/secrets/user_visible_store.go +++ b/apps/backend/internal/secrets/user_visible_store.go @@ -4,6 +4,8 @@ import ( "context" "fmt" "strings" + + "github.com/jmoiron/sqlx" ) const internalGitHubSecretPrefix = "github:" @@ -18,14 +20,16 @@ func IsInternalID(id string) bool { // underlying store remains available to integration services for their // deterministic internal keys. type UserVisibleStore struct { - store SecretStore + store SecretStore + scoped ScopedSecretStore } func NewUserVisibleStore(store SecretStore) SecretStore { if store == nil { return nil } - return &UserVisibleStore{store: store} + scoped, _ := store.(ScopedSecretStore) + return &UserVisibleStore{store: store, scoped: scoped} } func (s *UserVisibleStore) Create(ctx context.Context, secret *SecretWithValue) error { @@ -39,13 +43,23 @@ func (s *UserVisibleStore) Get(ctx context.Context, id string) (*Secret, error) if IsInternalID(id) { return nil, internalSecretNotFound(id) } - return s.store.Get(ctx, id) + secret, err := s.store.Get(ctx, id) + if err != nil { + return nil, err + } + if normalizeStoredScope(secret.Scope) != ScopeGlobal { + return nil, internalSecretNotFound(id) + } + return secret, nil } func (s *UserVisibleStore) Reveal(ctx context.Context, id string) (string, error) { if IsInternalID(id) { return "", internalSecretNotFound(id) } + if s.scoped != nil { + return s.scoped.RevealGlobal(ctx, id) + } return s.store.Reveal(ctx, id) } @@ -53,6 +67,9 @@ func (s *UserVisibleStore) Update(ctx context.Context, id string, req *UpdateSec if IsInternalID(id) { return internalSecretNotFound(id) } + if _, err := s.Get(ctx, id); err != nil { + return err + } return s.store.Update(ctx, id, req) } @@ -60,10 +77,16 @@ func (s *UserVisibleStore) Delete(ctx context.Context, id string) error { if IsInternalID(id) { return internalSecretNotFound(id) } + if _, err := s.Get(ctx, id); err != nil { + return err + } return s.store.Delete(ctx, id) } func (s *UserVisibleStore) List(ctx context.Context) ([]*SecretListItem, error) { + if s.scoped != nil { + return s.ListScoped(ctx, SecretListOptions{Scope: ScopeGlobal}) + } items, err := s.store.List(ctx) if err != nil { return nil, err @@ -77,9 +100,87 @@ func (s *UserVisibleStore) List(ctx context.Context) ([]*SecretListItem, error) return visible, nil } +func (s *UserVisibleStore) ListScoped(ctx context.Context, opts SecretListOptions) ([]*SecretListItem, error) { + if opts.Scope == "" { + opts.Scope = ScopeGlobal + } + if s.scoped == nil { + if opts.Scope == ScopeGlobal { + return s.List(ctx) + } + return nil, fmt.Errorf("workspace-scoped secret storage is unavailable") + } + items, err := s.scoped.ListScoped(ctx, opts) + if err != nil { + return nil, err + } + return filterInternalSecretItems(items), nil +} + +func (s *UserVisibleStore) GetForWorkspace(ctx context.Context, id, workspaceID string) (*Secret, error) { + if IsInternalID(id) { + return nil, internalSecretNotFound(id) + } + if s.scoped == nil { + return nil, fmt.Errorf("workspace-scoped secret storage is unavailable") + } + return s.scoped.GetForWorkspace(ctx, id, workspaceID) +} + +func (s *UserVisibleStore) RevealGlobal(ctx context.Context, id string) (string, error) { + if IsInternalID(id) { + return "", internalSecretNotFound(id) + } + if s.scoped != nil { + return s.scoped.RevealGlobal(ctx, id) + } + return s.store.Reveal(ctx, id) +} + +func (s *UserVisibleStore) RevealForWorkspace(ctx context.Context, id, workspaceID string) (string, error) { + if IsInternalID(id) { + return "", internalSecretNotFound(id) + } + if s.scoped != nil { + return s.scoped.RevealForWorkspace(ctx, id, workspaceID) + } + if _, err := s.GetForWorkspace(ctx, id, workspaceID); err != nil { + return "", err + } + return s.store.Reveal(ctx, id) +} + +func (s *UserVisibleStore) DeleteWorkspaceSecrets(ctx context.Context, workspaceID string) error { + if s.scoped == nil { + return fmt.Errorf("workspace-scoped secret storage is unavailable") + } + return s.scoped.DeleteWorkspaceSecrets(ctx, workspaceID) +} + +func (s *UserVisibleStore) DeleteWorkspaceSecretsTx(ctx context.Context, tx *sqlx.Tx, workspaceID string) error { + if s.scoped == nil { + return fmt.Errorf("workspace-scoped secret storage is unavailable") + } + transactional, ok := s.scoped.(WorkspaceSecretTransactionalDeleter) + if !ok { + return fmt.Errorf("transactional workspace-scoped secret storage is unavailable") + } + return transactional.DeleteWorkspaceSecretsTx(ctx, tx, workspaceID) +} + // The wrapped store is owned and closed by the repository container. func (s *UserVisibleStore) Close() error { return nil } func internalSecretNotFound(id string) error { return fmt.Errorf("%w: %s", ErrNotFound, id) } + +func filterInternalSecretItems(items []*SecretListItem) []*SecretListItem { + visible := make([]*SecretListItem, 0, len(items)) + for _, item := range items { + if item != nil && !IsInternalID(item.ID) { + visible = append(visible, item) + } + } + return visible +} diff --git a/apps/backend/internal/secrets/user_visible_store_test.go b/apps/backend/internal/secrets/user_visible_store_test.go index e2d35e6d45..b4c949257c 100644 --- a/apps/backend/internal/secrets/user_visible_store_test.go +++ b/apps/backend/internal/secrets/user_visible_store_test.go @@ -53,3 +53,66 @@ func TestUserVisibleStoreRejectsInternalCreate(t *testing.T) { t.Fatalf("Create() error = %v, want not found", err) } } + +func TestUserVisibleStoreWorkspaceOperationsFilterInternalSecrets(t *testing.T) { + store := newTestSQLiteStore(t) + ctx := context.Background() + global := &SecretWithValue{Secret: Secret{ID: "global-visible", Name: "global"}, Value: "global-value"} + workspace := &SecretWithValue{Secret: Secret{ + ID: "workspace-visible", Name: "workspace", Scope: ScopeWorkspace, WorkspaceID: "workspace-a", + }, Value: "workspace-value"} + internal := &SecretWithValue{Secret: Secret{ + ID: "github:internal-workspace", Name: "internal", Scope: ScopeWorkspace, WorkspaceID: "workspace-a", + }, Value: "internal-value"} + for _, secret := range []*SecretWithValue{global, workspace, internal} { + if err := store.Create(ctx, secret); err != nil { + t.Fatalf("create %s: %v", secret.ID, err) + } + } + + wrapped := NewUserVisibleStore(store) + scoped, ok := wrapped.(ScopedSecretStore) + if !ok { + t.Fatal("user-visible store is not scoped") + } + items, err := scoped.ListScoped(ctx, SecretListOptions{ + Scope: ScopeWorkspace, WorkspaceID: "workspace-a", IncludeGlobal: true, + }) + if err != nil { + t.Fatalf("list workspace options: %v", err) + } + if len(items) != 2 { + t.Fatalf("visible workspace items = %#v, want global and workspace entries", items) + } + if _, err := scoped.GetForWorkspace(ctx, workspace.ID, "workspace-a"); err != nil { + t.Fatalf("get workspace secret: %v", err) + } + if _, err := scoped.GetForWorkspace(ctx, internal.ID, "workspace-a"); !errors.Is(err, ErrNotFound) { + t.Fatalf("get internal workspace secret = %v, want not found", err) + } + if got, err := scoped.RevealForWorkspace(ctx, workspace.ID, "workspace-a"); err != nil || got != workspace.Value { + t.Fatalf("reveal workspace secret = %q, %v", got, err) + } + + tx, err := store.db.BeginTxx(ctx, nil) + if err != nil { + t.Fatalf("begin cleanup transaction: %v", err) + } + t.Cleanup(func() { _ = tx.Rollback() }) + transactional, ok := wrapped.(WorkspaceSecretTransactionalDeleter) + if !ok { + t.Fatal("user-visible store is not transactional") + } + if err := transactional.DeleteWorkspaceSecretsTx(ctx, tx, "workspace-a"); err != nil { + t.Fatalf("transactional workspace cleanup: %v", err) + } + if err := tx.Commit(); err != nil { + t.Fatalf("commit cleanup transaction: %v", err) + } + if _, err := store.Get(ctx, workspace.ID); !errors.Is(err, ErrNotFound) { + t.Fatalf("workspace secret after cleanup = %v, want not found", err) + } + if _, err := store.Get(ctx, internal.ID); !errors.Is(err, ErrNotFound) { + t.Fatalf("internal workspace secret after cleanup = %v, want not found", err) + } +} diff --git a/apps/backend/internal/task/dto/dto.go b/apps/backend/internal/task/dto/dto.go index d5bc157402..709fd22544 100644 --- a/apps/backend/internal/task/dto/dto.go +++ b/apps/backend/internal/task/dto/dto.go @@ -45,28 +45,34 @@ type WorkspaceDTO struct { } type RepositoryDTO struct { - ID string `json:"id"` - WorkspaceID string `json:"workspace_id"` - Name string `json:"name"` - SourceType string `json:"source_type"` - LocalPath string `json:"local_path"` - Provider string `json:"provider"` - ProviderRepoID string `json:"provider_repo_id"` - ProviderHost string `json:"provider_host"` - ProviderOwner string `json:"provider_owner"` - ProviderName string `json:"provider_name"` - RemoteURL string `json:"remote_url"` - DefaultBranch string `json:"default_branch"` - WorktreeBranchPrefix string `json:"worktree_branch_prefix"` - WorktreeBranchTemplate string `json:"worktree_branch_template"` - PullBeforeWorktree bool `json:"pull_before_worktree"` - SetupScript string `json:"setup_script"` - CleanupScript string `json:"cleanup_script"` - DevScript string `json:"dev_script"` - CopyFiles string `json:"copy_files"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - Scripts []RepositoryScriptDTO `json:"scripts,omitempty"` + ID string `json:"id"` + WorkspaceID string `json:"workspace_id"` + Name string `json:"name"` + SourceType string `json:"source_type"` + LocalPath string `json:"local_path"` + Provider string `json:"provider"` + ProviderRepoID string `json:"provider_repo_id"` + ProviderHost string `json:"provider_host"` + ProviderOwner string `json:"provider_owner"` + ProviderName string `json:"provider_name"` + RemoteURL string `json:"remote_url"` + DefaultBranch string `json:"default_branch"` + WorktreeBranchPrefix string `json:"worktree_branch_prefix"` + WorktreeBranchTemplate string `json:"worktree_branch_template"` + PullBeforeWorktree bool `json:"pull_before_worktree"` + SetupScript string `json:"setup_script"` + CleanupScript string `json:"cleanup_script"` + DevScript string `json:"dev_script"` + CopyFiles string `json:"copy_files"` + SecretBindings []RepositorySecretBindingDTO `json:"secret_bindings,omitempty"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + Scripts []RepositoryScriptDTO `json:"scripts,omitempty"` +} + +type RepositorySecretBindingDTO struct { + Key string `json:"key"` + SecretID string `json:"secret_id"` } type RepositoryScriptDTO struct { @@ -550,6 +556,10 @@ func FromWorkspace(workspace *models.Workspace) WorkspaceDTO { } func FromRepository(repository *models.Repository) RepositoryDTO { + bindings := make([]RepositorySecretBindingDTO, 0, len(repository.SecretBindings)) + for _, binding := range repository.SecretBindings { + bindings = append(bindings, RepositorySecretBindingDTO{Key: binding.Key, SecretID: binding.SecretID}) + } return RepositoryDTO{ ID: repository.ID, WorkspaceID: repository.WorkspaceID, @@ -570,6 +580,7 @@ func FromRepository(repository *models.Repository) RepositoryDTO { CleanupScript: repository.CleanupScript, DevScript: repository.DevScript, CopyFiles: repository.CopyFiles, + SecretBindings: bindings, CreatedAt: repository.CreatedAt, UpdatedAt: repository.UpdatedAt, } diff --git a/apps/backend/internal/task/handlers/repository_handlers.go b/apps/backend/internal/task/handlers/repository_handlers.go index d93592b22a..8039d6f99d 100644 --- a/apps/backend/internal/task/handlers/repository_handlers.go +++ b/apps/backend/internal/task/handlers/repository_handlers.go @@ -302,22 +302,23 @@ func (h *RepositoryHandlers) httpLocalRepositoryStatus(c *gin.Context) { } type httpCreateRepositoryRequest struct { - Name string `json:"name"` - SourceType string `json:"source_type"` - LocalPath string `json:"local_path"` - Provider string `json:"provider"` - ProviderRepoID string `json:"provider_repo_id"` - ProviderHost string `json:"provider_host"` - ProviderOwner string `json:"provider_owner"` - ProviderName string `json:"provider_name"` - DefaultBranch string `json:"default_branch"` - WorktreeBranchPrefix string `json:"worktree_branch_prefix"` - WorktreeBranchTemplate string `json:"worktree_branch_template"` - PullBeforeWorktree *bool `json:"pull_before_worktree"` - SetupScript string `json:"setup_script"` - CleanupScript string `json:"cleanup_script"` - DevScript string `json:"dev_script"` - CopyFiles string `json:"copy_files"` + Name string `json:"name"` + SourceType string `json:"source_type"` + LocalPath string `json:"local_path"` + Provider string `json:"provider"` + ProviderRepoID string `json:"provider_repo_id"` + ProviderHost string `json:"provider_host"` + ProviderOwner string `json:"provider_owner"` + ProviderName string `json:"provider_name"` + DefaultBranch string `json:"default_branch"` + WorktreeBranchPrefix string `json:"worktree_branch_prefix"` + WorktreeBranchTemplate string `json:"worktree_branch_template"` + PullBeforeWorktree *bool `json:"pull_before_worktree"` + SetupScript string `json:"setup_script"` + CleanupScript string `json:"cleanup_script"` + DevScript string `json:"dev_script"` + CopyFiles string `json:"copy_files"` + SecretBindings []service.RepositorySecretBindingInput `json:"secret_bindings,omitempty"` } type httpInitializeLocalRepositoryRequest struct { @@ -381,6 +382,7 @@ func (h *RepositoryHandlers) httpCreateRepository(c *gin.Context) { CleanupScript: body.CleanupScript, DevScript: body.DevScript, CopyFiles: body.CopyFiles, + SecretBindings: body.SecretBindings, }) if err != nil { if errors.Is(err, service.ErrInvalidRepositorySettings) { @@ -454,22 +456,23 @@ func (h *RepositoryHandlers) refreshRepositoryBranches(ctx context.Context, repo } type httpUpdateRepositoryRequest struct { - Name *string `json:"name"` - SourceType *string `json:"source_type"` - LocalPath *string `json:"local_path"` - Provider *string `json:"provider"` - ProviderRepoID *string `json:"provider_repo_id"` - ProviderHost *string `json:"provider_host"` - ProviderOwner *string `json:"provider_owner"` - ProviderName *string `json:"provider_name"` - DefaultBranch *string `json:"default_branch"` - WorktreeBranchPrefix *string `json:"worktree_branch_prefix"` - WorktreeBranchTemplate *string `json:"worktree_branch_template"` - PullBeforeWorktree *bool `json:"pull_before_worktree"` - SetupScript *string `json:"setup_script"` - CleanupScript *string `json:"cleanup_script"` - DevScript *string `json:"dev_script"` - CopyFiles *string `json:"copy_files"` + Name *string `json:"name"` + SourceType *string `json:"source_type"` + LocalPath *string `json:"local_path"` + Provider *string `json:"provider"` + ProviderRepoID *string `json:"provider_repo_id"` + ProviderHost *string `json:"provider_host"` + ProviderOwner *string `json:"provider_owner"` + ProviderName *string `json:"provider_name"` + DefaultBranch *string `json:"default_branch"` + WorktreeBranchPrefix *string `json:"worktree_branch_prefix"` + WorktreeBranchTemplate *string `json:"worktree_branch_template"` + PullBeforeWorktree *bool `json:"pull_before_worktree"` + SetupScript *string `json:"setup_script"` + CleanupScript *string `json:"cleanup_script"` + DevScript *string `json:"dev_script"` + CopyFiles *string `json:"copy_files"` + SecretBindings *[]service.RepositorySecretBindingInput `json:"secret_bindings,omitempty"` } func (h *RepositoryHandlers) httpUpdateRepository(c *gin.Context) { @@ -495,6 +498,7 @@ func (h *RepositoryHandlers) httpUpdateRepository(c *gin.Context) { CleanupScript: body.CleanupScript, DevScript: body.DevScript, CopyFiles: body.CopyFiles, + SecretBindings: body.SecretBindings, }) if err != nil { if errors.Is(err, service.ErrInvalidRepositorySettings) { @@ -557,22 +561,23 @@ func (h *RepositoryHandlers) wsListRepositories(ctx context.Context, msg *ws.Mes } type wsCreateRepositoryRequest struct { - WorkspaceID string `json:"workspace_id"` - Name string `json:"name"` - SourceType string `json:"source_type"` - LocalPath string `json:"local_path"` - Provider string `json:"provider"` - ProviderRepoID string `json:"provider_repo_id"` - ProviderHost string `json:"provider_host"` - ProviderOwner string `json:"provider_owner"` - ProviderName string `json:"provider_name"` - DefaultBranch string `json:"default_branch"` - WorktreeBranchPrefix string `json:"worktree_branch_prefix"` - WorktreeBranchTemplate string `json:"worktree_branch_template"` - SetupScript string `json:"setup_script"` - CleanupScript string `json:"cleanup_script"` - DevScript string `json:"dev_script"` - CopyFiles string `json:"copy_files"` + WorkspaceID string `json:"workspace_id"` + Name string `json:"name"` + SourceType string `json:"source_type"` + LocalPath string `json:"local_path"` + Provider string `json:"provider"` + ProviderRepoID string `json:"provider_repo_id"` + ProviderHost string `json:"provider_host"` + ProviderOwner string `json:"provider_owner"` + ProviderName string `json:"provider_name"` + DefaultBranch string `json:"default_branch"` + WorktreeBranchPrefix string `json:"worktree_branch_prefix"` + WorktreeBranchTemplate string `json:"worktree_branch_template"` + SetupScript string `json:"setup_script"` + CleanupScript string `json:"cleanup_script"` + DevScript string `json:"dev_script"` + CopyFiles string `json:"copy_files"` + SecretBindings []service.RepositorySecretBindingInput `json:"secret_bindings,omitempty"` } func (h *RepositoryHandlers) wsCreateRepository(ctx context.Context, msg *ws.Message) (*ws.Message, error) { @@ -600,6 +605,7 @@ func (h *RepositoryHandlers) wsCreateRepository(ctx context.Context, msg *ws.Mes CleanupScript: req.CleanupScript, DevScript: req.DevScript, CopyFiles: req.CopyFiles, + SecretBindings: req.SecretBindings, }) if err != nil { if errors.Is(err, service.ErrInvalidRepositorySettings) { @@ -631,22 +637,23 @@ func (h *RepositoryHandlers) wsGetRepository(ctx context.Context, msg *ws.Messag } type wsUpdateRepositoryRequest struct { - ID string `json:"id"` - Name *string `json:"name,omitempty"` - SourceType *string `json:"source_type,omitempty"` - LocalPath *string `json:"local_path,omitempty"` - Provider *string `json:"provider,omitempty"` - ProviderRepoID *string `json:"provider_repo_id,omitempty"` - ProviderHost *string `json:"provider_host,omitempty"` - ProviderOwner *string `json:"provider_owner,omitempty"` - ProviderName *string `json:"provider_name,omitempty"` - DefaultBranch *string `json:"default_branch,omitempty"` - WorktreeBranchPrefix *string `json:"worktree_branch_prefix,omitempty"` - WorktreeBranchTemplate *string `json:"worktree_branch_template,omitempty"` - SetupScript *string `json:"setup_script,omitempty"` - CleanupScript *string `json:"cleanup_script,omitempty"` - DevScript *string `json:"dev_script,omitempty"` - CopyFiles *string `json:"copy_files,omitempty"` + ID string `json:"id"` + Name *string `json:"name,omitempty"` + SourceType *string `json:"source_type,omitempty"` + LocalPath *string `json:"local_path,omitempty"` + Provider *string `json:"provider,omitempty"` + ProviderRepoID *string `json:"provider_repo_id,omitempty"` + ProviderHost *string `json:"provider_host,omitempty"` + ProviderOwner *string `json:"provider_owner,omitempty"` + ProviderName *string `json:"provider_name,omitempty"` + DefaultBranch *string `json:"default_branch,omitempty"` + WorktreeBranchPrefix *string `json:"worktree_branch_prefix,omitempty"` + WorktreeBranchTemplate *string `json:"worktree_branch_template,omitempty"` + SetupScript *string `json:"setup_script,omitempty"` + CleanupScript *string `json:"cleanup_script,omitempty"` + DevScript *string `json:"dev_script,omitempty"` + CopyFiles *string `json:"copy_files,omitempty"` + SecretBindings *[]service.RepositorySecretBindingInput `json:"secret_bindings,omitempty"` } func (h *RepositoryHandlers) wsUpdateRepository(ctx context.Context, msg *ws.Message) (*ws.Message, error) { @@ -673,6 +680,7 @@ func (h *RepositoryHandlers) wsUpdateRepository(ctx context.Context, msg *ws.Mes CleanupScript: req.CleanupScript, DevScript: req.DevScript, CopyFiles: req.CopyFiles, + SecretBindings: req.SecretBindings, }) if err != nil { if errors.Is(err, service.ErrInvalidRepositorySettings) { diff --git a/apps/backend/internal/task/models/models.go b/apps/backend/internal/task/models/models.go index 193d9d2ddc..cbd5a30340 100644 --- a/apps/backend/internal/task/models/models.go +++ b/apps/backend/internal/task/models/models.go @@ -1114,23 +1114,35 @@ type Repository struct { // populated after the repo is cloned/synced on the agent host. LocalPath string `json:"local_path"` // Provider fields describe the upstream source (e.g. github/gitlab) for future syncing. - Provider string `json:"provider"` - ProviderRepoID string `json:"provider_repo_id"` - ProviderHost string `json:"provider_host"` - ProviderOwner string `json:"provider_owner"` - ProviderName string `json:"provider_name"` - RemoteURL string `json:"remote_url"` - DefaultBranch string `json:"default_branch"` - WorktreeBranchPrefix string `json:"worktree_branch_prefix"` - WorktreeBranchTemplate string `json:"worktree_branch_template"` - PullBeforeWorktree bool `json:"pull_before_worktree"` - SetupScript string `json:"setup_script"` - CleanupScript string `json:"cleanup_script"` - DevScript string `json:"dev_script"` - CopyFiles string `json:"copy_files"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - DeletedAt *time.Time `json:"deleted_at,omitempty"` + Provider string `json:"provider"` + ProviderRepoID string `json:"provider_repo_id"` + ProviderHost string `json:"provider_host"` + ProviderOwner string `json:"provider_owner"` + ProviderName string `json:"provider_name"` + RemoteURL string `json:"remote_url"` + DefaultBranch string `json:"default_branch"` + WorktreeBranchPrefix string `json:"worktree_branch_prefix"` + WorktreeBranchTemplate string `json:"worktree_branch_template"` + PullBeforeWorktree bool `json:"pull_before_worktree"` + SetupScript string `json:"setup_script"` + CleanupScript string `json:"cleanup_script"` + DevScript string `json:"dev_script"` + CopyFiles string `json:"copy_files"` + SecretBindings []RepositorySecretBinding `json:"secret_bindings,omitempty"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + DeletedAt *time.Time `json:"deleted_at,omitempty"` +} + +// RepositorySecretBinding maps an environment key to a secret reference. The +// value is never persisted or returned; a missing secret intentionally leaves +// this row dangling so launches can report a broken binding. +type RepositorySecretBinding struct { + RepositoryID string `json:"repository_id,omitempty"` + Key string `json:"key"` + SecretID string `json:"secret_id"` + CreatedAt time.Time `json:"created_at,omitempty"` + UpdatedAt time.Time `json:"updated_at,omitempty"` } // RepositoryScript represents a custom script for a repository diff --git a/apps/backend/internal/task/repository/interface.go b/apps/backend/internal/task/repository/interface.go index 4f66f4944d..e2e518fee8 100644 --- a/apps/backend/internal/task/repository/interface.go +++ b/apps/backend/internal/task/repository/interface.go @@ -327,6 +327,22 @@ type RepositoryEntityRepository interface { GetRepositoryByLocalPath(ctx context.Context, workspaceID, localPath string) (*models.Repository, error) } +// RepositorySecretBindingRepository stores normalized repository environment +// references. It is optional on RepositoryEntityRepository to keep legacy +// adapters source-compatible while the SQLite implementation rolls out. +type RepositorySecretBindingRepository interface { + ListRepositorySecretBindings(ctx context.Context, repositoryID string) ([]*models.RepositorySecretBinding, error) + ListRepositorySecretBindingsByRepositoryIDs(ctx context.Context, repositoryIDs []string) (map[string][]*models.RepositorySecretBinding, error) + ReplaceRepositorySecretBindings(ctx context.Context, repositoryID string, bindings []models.RepositorySecretBinding) error +} + +// RepositorySecretBindingMutator adds atomic repository-plus-binding writes. +type RepositorySecretBindingMutator interface { + RepositorySecretBindingRepository + CreateRepositoryWithSecretBindings(ctx context.Context, repository *models.Repository, bindings []models.RepositorySecretBinding) error + UpdateRepositoryWithSecretBindings(ctx context.Context, repository *models.Repository, bindings []models.RepositorySecretBinding) error +} + // RepositoryCleanupRepository performs guarded deletion of repositories // created during workspace-source attachment rollback. type RepositoryCleanupRepository interface { diff --git a/apps/backend/internal/task/repository/sqlite/base_migrations.go b/apps/backend/internal/task/repository/sqlite/base_migrations.go index 9526f4d2f2..fd4b0f0b10 100644 --- a/apps/backend/internal/task/repository/sqlite/base_migrations.go +++ b/apps/backend/internal/task/repository/sqlite/base_migrations.go @@ -110,6 +110,19 @@ func (r *Repository) runMigrations() error { // column added earlier. r.migrate.Apply("task_sessions.name", `ALTER TABLE task_sessions ADD COLUMN name TEXT DEFAULT ''`) r.migrate.Apply("repositories.copy_files", `ALTER TABLE repositories ADD COLUMN copy_files TEXT DEFAULT ''`) + r.migrate.Apply("repository_secret_bindings.table", ` + CREATE TABLE IF NOT EXISTS repository_secret_bindings ( + repository_id TEXT NOT NULL, + key TEXT NOT NULL, + secret_id TEXT NOT NULL, + created_at TIMESTAMP NOT NULL, + updated_at TIMESTAMP NOT NULL, + PRIMARY KEY (repository_id, key), + FOREIGN KEY (repository_id) REFERENCES repositories(id) ON DELETE CASCADE + )`) + r.migrate.Apply("repository_secret_bindings.index", ` + CREATE INDEX IF NOT EXISTS idx_repository_secret_bindings_repository + ON repository_secret_bindings(repository_id)`) r.migrate.Apply("repositories.remote_url", `ALTER TABLE repositories ADD COLUMN remote_url TEXT DEFAULT ''`) r.migrate.Apply("repositories.provider_host", `ALTER TABLE repositories ADD COLUMN provider_host TEXT DEFAULT ''`) r.migrate.Apply("repositories.provider_host.github_backfill", ` diff --git a/apps/backend/internal/task/repository/sqlite/base_schema.go b/apps/backend/internal/task/repository/sqlite/base_schema.go index 1174b77a1c..c6ea8a8cdf 100644 --- a/apps/backend/internal/task/repository/sqlite/base_schema.go +++ b/apps/backend/internal/task/repository/sqlite/base_schema.go @@ -336,6 +336,19 @@ func (r *Repository) initTaskSchema() error { FOREIGN KEY (workspace_id) REFERENCES workspaces(id) ON DELETE CASCADE ); + CREATE TABLE IF NOT EXISTS repository_secret_bindings ( + repository_id TEXT NOT NULL, + key TEXT NOT NULL, + secret_id TEXT NOT NULL, + created_at TIMESTAMP NOT NULL, + updated_at TIMESTAMP NOT NULL, + PRIMARY KEY (repository_id, key), + FOREIGN KEY (repository_id) REFERENCES repositories(id) ON DELETE CASCADE + ); + + CREATE INDEX IF NOT EXISTS idx_repository_secret_bindings_repository + ON repository_secret_bindings(repository_id); + CREATE TABLE IF NOT EXISTS task_repositories ( id TEXT PRIMARY KEY, task_id TEXT NOT NULL, diff --git a/apps/backend/internal/task/repository/sqlite/postgres_schema_test.go b/apps/backend/internal/task/repository/sqlite/postgres_schema_test.go index 6c8284e8d1..187231e328 100644 --- a/apps/backend/internal/task/repository/sqlite/postgres_schema_test.go +++ b/apps/backend/internal/task/repository/sqlite/postgres_schema_test.go @@ -532,6 +532,50 @@ func TestPostgresTaskEnvironmentReposMultiBranchMigration(t *testing.T) { } } +func TestPostgresRepositorySecretBindingsSchemaReplay(t *testing.T) { + db := testutil.OpenIsolatedPostgres(t, testutil.PostgresDSNFromEnv(t)) + repo, err := NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("init postgres schema: %v", err) + } + + if _, err := db.Exec("DROP TABLE repository_secret_bindings"); err != nil { + t.Fatalf("drop repository secret bindings table: %v", err) + } + if err := repo.runMigrations(); err != nil { + t.Fatalf("replay repository secret bindings migration: %v", err) + } + if err := repo.runMigrations(); err != nil { + t.Fatalf("replay repository secret bindings migration twice: %v", err) + } + + ctx := context.Background() + seedWorkspace(t, repo, "ws-postgres-secret-bindings") + repository := &models.Repository{ + ID: "repo-postgres-secret-bindings", + WorkspaceID: "ws-postgres-secret-bindings", + Name: "postgres secrets", + } + bindings := []models.RepositorySecretBinding{{Key: "NPM_TOKEN", SecretID: "secret-pg-npm"}} + if err := repo.CreateRepositoryWithSecretBindings(ctx, repository, bindings); err != nil { + t.Fatalf("create repository with bindings after replay: %v", err) + } + got, err := repo.GetRepository(ctx, repository.ID) + if err != nil { + t.Fatalf("get repository after replay: %v", err) + } + if len(got.SecretBindings) != 1 { + t.Fatalf("repository bindings = %+v, want one binding", got.SecretBindings) + } + gotBinding := got.SecretBindings[0] + if gotBinding.RepositoryID != repository.ID || gotBinding.Key != bindings[0].Key || gotBinding.SecretID != bindings[0].SecretID { + t.Fatalf("repository binding = %+v, want repository=%q key=%q secret=%q", gotBinding, repository.ID, bindings[0].Key, bindings[0].SecretID) + } + if gotBinding.CreatedAt.IsZero() || gotBinding.UpdatedAt.IsZero() { + t.Fatalf("repository binding timestamps = %+v, want persisted timestamps", gotBinding) + } +} + // openIsolatedPostgresMultiConn is like testutil.OpenIsolatedPostgres but // supports a real multi-connection pool. OpenIsolatedPostgres scopes its // isolated schema via a session-level `SET search_path` issued on one diff --git a/apps/backend/internal/task/repository/sqlite/repository_entity.go b/apps/backend/internal/task/repository/sqlite/repository_entity.go index 0fab4c6f20..894d25f401 100644 --- a/apps/backend/internal/task/repository/sqlite/repository_entity.go +++ b/apps/backend/internal/task/repository/sqlite/repository_entity.go @@ -8,6 +8,7 @@ import ( "time" "github.com/google/uuid" + "github.com/jmoiron/sqlx" "github.com/kandev/kandev/internal/db/dialect" "github.com/kandev/kandev/internal/task/models" @@ -16,6 +17,10 @@ import ( // CreateRepository creates a new repository func (r *Repository) CreateRepository(ctx context.Context, repository *models.Repository) error { + return r.insertRepository(ctx, r.db, repository) +} + +func (r *Repository) insertRepository(ctx context.Context, exec sqlx.ExtContext, repository *models.Repository) error { if repository.ID == "" { repository.ID = uuid.New().String() } @@ -23,7 +28,7 @@ func (r *Repository) CreateRepository(ctx context.Context, repository *models.Re repository.CreatedAt = now repository.UpdatedAt = now - _, err := r.db.ExecContext(ctx, r.db.Rebind(` + _, err := exec.ExecContext(ctx, r.db.Rebind(` INSERT INTO repositories ( id, workspace_id, name, source_type, local_path, provider, provider_repo_id, provider_host, provider_owner, provider_name, remote_url, default_branch, worktree_branch_prefix, worktree_branch_template, pull_before_worktree, setup_script, cleanup_script, dev_script, copy_files, created_at, updated_at, deleted_at @@ -35,6 +40,25 @@ func (r *Repository) CreateRepository(ctx context.Context, repository *models.Re return err } +// CreateRepositoryWithSecretBindings persists a repository and its complete +// binding set in one transaction. +func (r *Repository) CreateRepositoryWithSecretBindings( + ctx context.Context, repository *models.Repository, bindings []models.RepositorySecretBinding, +) error { + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + if err := r.insertRepository(ctx, tx, repository); err != nil { + return err + } + if err := insertRepositorySecretBindings(ctx, r.db, tx, repository.ID, bindings); err != nil { + return err + } + return tx.Commit() +} + // GetRepository retrieves a repository by ID func (r *Repository) GetRepository(ctx context.Context, id string) (*models.Repository, error) { repository := &models.Repository{} @@ -52,14 +76,23 @@ func (r *Repository) GetRepository(ctx context.Context, id string) (*models.Repo if err == sql.ErrNoRows { return nil, fmt.Errorf("%w: %s", repoerrors.ErrRepositoryNotFound, id) } + if err == nil { + if err := r.attachRepositorySecretBindings(ctx, repository); err != nil { + return nil, err + } + } return repository, err } // UpdateRepository updates an existing repository func (r *Repository) UpdateRepository(ctx context.Context, repository *models.Repository) error { + return r.updateRepository(ctx, r.db, repository) +} + +func (r *Repository) updateRepository(ctx context.Context, exec sqlx.ExtContext, repository *models.Repository) error { repository.UpdatedAt = time.Now().UTC() - result, err := r.db.ExecContext(ctx, r.db.Rebind(` + result, err := exec.ExecContext(ctx, r.db.Rebind(` UPDATE repositories SET name = ?, source_type = ?, local_path = ?, provider = ?, provider_repo_id = ?, provider_host = ?, provider_owner = ?, provider_name = ?, remote_url = ?, default_branch = ?, worktree_branch_prefix = ?, worktree_branch_template = ?, pull_before_worktree = ?, setup_script = ?, cleanup_script = ?, dev_script = ?, copy_files = ?, updated_at = ? @@ -78,10 +111,35 @@ func (r *Repository) UpdateRepository(ctx context.Context, repository *models.Re return nil } +// UpdateRepositoryWithSecretBindings replaces bindings atomically with the +// repository mutation. +func (r *Repository) UpdateRepositoryWithSecretBindings( + ctx context.Context, repository *models.Repository, bindings []models.RepositorySecretBinding, +) error { + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + if err := r.updateRepository(ctx, tx, repository); err != nil { + return err + } + if err := insertRepositorySecretBindings(ctx, r.db, tx, repository.ID, bindings); err != nil { + return err + } + return tx.Commit() +} + // DeleteRepository soft-deletes a repository by ID func (r *Repository) DeleteRepository(ctx context.Context, id string) error { + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + now := time.Now().UTC() - result, err := r.db.ExecContext(ctx, r.db.Rebind(` + result, err := tx.ExecContext(ctx, r.db.Rebind(` UPDATE repositories SET deleted_at = ?, updated_at = ? WHERE id = ? AND deleted_at IS NULL `), now, now, id) if err != nil { @@ -91,14 +149,23 @@ func (r *Repository) DeleteRepository(ctx context.Context, id string) error { if rows == 0 { return fmt.Errorf("repository not found: %s", id) } - return nil + if _, err := tx.ExecContext(ctx, r.db.Rebind(`DELETE FROM repository_secret_bindings WHERE repository_id = ?`), id); err != nil { + return err + } + return tx.Commit() } // DeleteRepositoryIfUnreferenced soft-deletes only an unadopted repository. // Keeping the reference check inside UPDATE closes the cleanup/adoption race. func (r *Repository) DeleteRepositoryIfUnreferenced(ctx context.Context, id string) (bool, error) { + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return false, err + } + defer func() { _ = tx.Rollback() }() + now := time.Now().UTC() - result, err := r.db.ExecContext(ctx, r.db.Rebind(` + result, err := tx.ExecContext(ctx, r.db.Rebind(` UPDATE repositories SET deleted_at = ?, updated_at = ? WHERE id = ? @@ -114,6 +181,14 @@ func (r *Repository) DeleteRepositoryIfUnreferenced(ctx context.Context, id stri if err != nil { return false, err } + if rows > 0 { + if _, err := tx.ExecContext(ctx, r.db.Rebind(`DELETE FROM repository_secret_bindings WHERE repository_id = ?`), id); err != nil { + return false, err + } + if err := tx.Commit(); err != nil { + return false, err + } + } return rows > 0, nil } @@ -122,8 +197,14 @@ func (r *Repository) DeleteRepositoryIfUnreferenced(ctx context.Context, id stri // predicate in the UPDATE prevents a session from becoming active between a // separate check and the delete. func (r *Repository) DeleteRepositoryIfNoActiveTaskSessions(ctx context.Context, id string) (bool, error) { + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return false, err + } + defer func() { _ = tx.Rollback() }() + now := time.Now().UTC() - result, err := r.db.ExecContext(ctx, r.db.Rebind(` + result, err := tx.ExecContext(ctx, r.db.Rebind(` UPDATE repositories SET deleted_at = ?, updated_at = ? WHERE id = ? @@ -145,6 +226,14 @@ func (r *Repository) DeleteRepositoryIfNoActiveTaskSessions(ctx context.Context, if err != nil { return false, err } + if rows > 0 { + if _, err := tx.ExecContext(ctx, r.db.Rebind(`DELETE FROM repository_secret_bindings WHERE repository_id = ?`), id); err != nil { + return false, err + } + if err := tx.Commit(); err != nil { + return false, err + } + } return rows > 0, nil } @@ -173,7 +262,13 @@ func (r *Repository) ListRepositories(ctx context.Context, workspaceID string) ( } result = append(result, repository) } - return result, rows.Err() + if err := rows.Err(); err != nil { + return nil, err + } + if err := r.attachRepositorySecretBindingsBatch(ctx, result); err != nil { + return nil, err + } + return result, nil } // GetRepositoryByProviderInfo finds a repository by workspace, provider, owner, and name. @@ -202,6 +297,11 @@ func (r *Repository) GetRepositoryByProviderInfo(ctx context.Context, workspaceI if err == sql.ErrNoRows { return nil, nil } + if err == nil { + if err := r.attachRepositorySecretBindings(ctx, repository); err != nil { + return nil, err + } + } return repository, err } @@ -227,9 +327,134 @@ func (r *Repository) GetRepositoryByLocalPath(ctx context.Context, workspaceID, if err == sql.ErrNoRows { return nil, nil } + if err == nil { + if err := r.attachRepositorySecretBindings(ctx, repository); err != nil { + return nil, err + } + } return repository, err } +func (r *Repository) ListRepositorySecretBindings(ctx context.Context, repositoryID string) ([]*models.RepositorySecretBinding, error) { + var rows []repositorySecretBindingRow + if err := r.ro.SelectContext(ctx, &rows, r.ro.Rebind(` + SELECT repository_id, key, secret_id, created_at, updated_at + FROM repository_secret_bindings WHERE repository_id = ? ORDER BY key`), repositoryID); err != nil { + return nil, err + } + return repositorySecretBindingsFromRows(rows), nil +} + +func (r *Repository) ListRepositorySecretBindingsByRepositoryIDs( + ctx context.Context, repositoryIDs []string, +) (map[string][]*models.RepositorySecretBinding, error) { + result := make(map[string][]*models.RepositorySecretBinding, len(repositoryIDs)) + if len(repositoryIDs) == 0 { + return result, nil + } + query, args, err := sqlx.In(` + SELECT repository_id, key, secret_id, created_at, updated_at + FROM repository_secret_bindings WHERE repository_id IN (?) ORDER BY repository_id, key`, repositoryIDs) + if err != nil { + return nil, err + } + var rows []repositorySecretBindingRow + if err := r.ro.SelectContext(ctx, &rows, r.ro.Rebind(query), args...); err != nil { + return nil, err + } + for _, binding := range repositorySecretBindingsFromRows(rows) { + result[binding.RepositoryID] = append(result[binding.RepositoryID], binding) + } + return result, nil +} + +func (r *Repository) ReplaceRepositorySecretBindings( + ctx context.Context, repositoryID string, bindings []models.RepositorySecretBinding, +) error { + tx, err := r.db.BeginTxx(ctx, nil) + if err != nil { + return err + } + defer func() { _ = tx.Rollback() }() + if err := insertRepositorySecretBindings(ctx, r.db, tx, repositoryID, bindings); err != nil { + return err + } + return tx.Commit() +} + +type repositorySecretBindingRow struct { + RepositoryID string `db:"repository_id"` + Key string `db:"key"` + SecretID string `db:"secret_id"` + CreatedAt time.Time `db:"created_at"` + UpdatedAt time.Time `db:"updated_at"` +} + +func insertRepositorySecretBindings( + ctx context.Context, db *sqlx.DB, exec *sqlx.Tx, repositoryID string, bindings []models.RepositorySecretBinding, +) error { + if _, err := exec.ExecContext(ctx, db.Rebind(`DELETE FROM repository_secret_bindings WHERE repository_id = ?`), repositoryID); err != nil { + return err + } + now := time.Now().UTC() + for _, binding := range bindings { + if _, err := exec.ExecContext(ctx, db.Rebind(` + INSERT INTO repository_secret_bindings (repository_id, key, secret_id, created_at, updated_at) + VALUES (?, ?, ?, ?, ?)`), repositoryID, binding.Key, binding.SecretID, now, now); err != nil { + return err + } + } + return nil +} + +func (r *Repository) attachRepositorySecretBindings(ctx context.Context, repository *models.Repository) error { + bindings, err := r.ListRepositorySecretBindings(ctx, repository.ID) + if err != nil { + return err + } + repository.SecretBindings = bindingsToValues(bindings) + return nil +} + +func (r *Repository) attachRepositorySecretBindingsBatch(ctx context.Context, repositories []*models.Repository) error { + ids := make([]string, 0, len(repositories)) + for _, repository := range repositories { + ids = append(ids, repository.ID) + } + bindings, err := r.ListRepositorySecretBindingsByRepositoryIDs(ctx, ids) + if err != nil { + return err + } + for _, repository := range repositories { + repository.SecretBindings = bindingsToValues(bindings[repository.ID]) + } + return nil +} + +func repositorySecretBindingsFromRows(rows []repositorySecretBindingRow) []*models.RepositorySecretBinding { + result := make([]*models.RepositorySecretBinding, 0, len(rows)) + for _, row := range rows { + result = append(result, &models.RepositorySecretBinding{ + RepositoryID: row.RepositoryID, + Key: row.Key, + SecretID: row.SecretID, + CreatedAt: row.CreatedAt, + UpdatedAt: row.UpdatedAt, + }) + } + return result +} + +func bindingsToValues(bindings []*models.RepositorySecretBinding) []models.RepositorySecretBinding { + result := make([]models.RepositorySecretBinding, 0, len(bindings)) + for _, binding := range bindings { + if binding != nil { + result = append(result, *binding) + } + } + return result +} + // CreateRepositoryScript creates a new repository script func (r *Repository) CreateRepositoryScript(ctx context.Context, script *models.RepositoryScript) error { if script.ID == "" { diff --git a/apps/backend/internal/task/repository/sqlite/repository_entity_test.go b/apps/backend/internal/task/repository/sqlite/repository_entity_test.go index 37e9deaf6e..153c5e999e 100644 --- a/apps/backend/internal/task/repository/sqlite/repository_entity_test.go +++ b/apps/backend/internal/task/repository/sqlite/repository_entity_test.go @@ -297,6 +297,115 @@ func TestRepositoryCopyFiles_RoundTrip(t *testing.T) { } } +func TestRepositorySecretBindings_RoundTripReplaceAndCascade(t *testing.T) { + repo := newRepoForEntityTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "ws-secret-bindings") + + entity := &models.Repository{ + ID: "repo-secret-bindings", + WorkspaceID: "ws-secret-bindings", + Name: "app", + } + bindings := []models.RepositorySecretBinding{ + {Key: "NPM_TOKEN", SecretID: "secret-npm"}, + {Key: "SENTRY_AUTH_TOKEN", SecretID: "secret-sentry"}, + } + if err := repo.CreateRepositoryWithSecretBindings(ctx, entity, bindings); err != nil { + t.Fatalf("create repository with bindings: %v", err) + } + + got, err := repo.GetRepository(ctx, entity.ID) + if err != nil { + t.Fatalf("get repository: %v", err) + } + if len(got.SecretBindings) != 2 || got.SecretBindings[0].SecretID == "" { + t.Fatalf("get bindings = %+v, want two references", got.SecretBindings) + } + + list, err := repo.ListRepositories(ctx, entity.WorkspaceID) + if err != nil { + t.Fatalf("list repositories: %v", err) + } + if len(list) != 1 || len(list[0].SecretBindings) != 2 { + t.Fatalf("list bindings = %+v, want two references", list) + } + + replacement := []models.RepositorySecretBinding{{Key: "NPM_TOKEN", SecretID: "secret-new"}} + if err := repo.ReplaceRepositorySecretBindings(ctx, entity.ID, replacement); err != nil { + t.Fatalf("replace bindings: %v", err) + } + got, err = repo.GetRepository(ctx, entity.ID) + if err != nil { + t.Fatalf("get after replace: %v", err) + } + if len(got.SecretBindings) != 1 || got.SecretBindings[0].SecretID != "secret-new" { + t.Fatalf("bindings after replace = %+v", got.SecretBindings) + } + + if err := repo.DeleteRepository(ctx, entity.ID); err != nil { + t.Fatalf("delete repository: %v", err) + } + remaining, err := repo.ListRepositorySecretBindings(ctx, entity.ID) + if err != nil { + t.Fatalf("list bindings after delete: %v", err) + } + if len(remaining) != 0 { + t.Fatalf("bindings after delete = %+v, want empty", remaining) + } +} + +func TestRepositoryDeleteBindingCleanupFailureRollsBackRepositoryDelete(t *testing.T) { + deleteMethods := []struct { + name string + call func(context.Context, *Repository, string) error + }{ + {name: "unconditional", call: func(ctx context.Context, repo *Repository, id string) error { + return repo.DeleteRepository(ctx, id) + }}, + {name: "unreferenced", call: func(ctx context.Context, repo *Repository, id string) error { + _, err := repo.DeleteRepositoryIfUnreferenced(ctx, id) + return err + }}, + {name: "no active sessions", call: func(ctx context.Context, repo *Repository, id string) error { + _, err := repo.DeleteRepositoryIfNoActiveTaskSessions(ctx, id) + return err + }}, + } + for _, method := range deleteMethods { + t.Run(method.name, func(t *testing.T) { + repo := newRepoForEntityTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "ws-delete-"+method.name) + entity := &models.Repository{ID: "repo-delete-" + method.name, WorkspaceID: "ws-delete-" + method.name, Name: method.name} + if err := repo.CreateRepositoryWithSecretBindings(ctx, entity, []models.RepositorySecretBinding{{Key: "TOKEN", SecretID: "secret-token"}}); err != nil { + t.Fatalf("create repository: %v", err) + } + _, err := repo.db.Exec(` + CREATE TRIGGER fail_repository_binding_delete + BEFORE DELETE ON repository_secret_bindings + BEGIN SELECT RAISE(ABORT, 'injected binding cleanup failure'); END`) + if err != nil { + t.Fatalf("create failure trigger: %v", err) + } + + if err := method.call(ctx, repo, entity.ID); err == nil { + t.Fatal("delete succeeded, want injected binding cleanup failure") + } + if _, err := repo.GetRepository(ctx, entity.ID); err != nil { + t.Fatalf("repository was soft-deleted after cleanup failure: %v", err) + } + bindings, err := repo.ListRepositorySecretBindings(ctx, entity.ID) + if err != nil { + t.Fatalf("list bindings: %v", err) + } + if len(bindings) != 1 { + t.Fatalf("bindings after failed delete = %#v, want one", bindings) + } + }) + } +} + func TestRepositoryProviderHost_RoundTrip(t *testing.T) { repo := newRepoForEntityTests(t) ctx := context.Background() diff --git a/apps/backend/internal/task/repository/sqlite/workspace.go b/apps/backend/internal/task/repository/sqlite/workspace.go index c5ccd5c4b6..6f4487244c 100644 --- a/apps/backend/internal/task/repository/sqlite/workspace.go +++ b/apps/backend/internal/task/repository/sqlite/workspace.go @@ -145,7 +145,7 @@ func (r *Repository) DeleteWorkspaceCascade( ctx context.Context, id string, ) ([]*models.Task, []*models.Workflow, error) { - return r.deleteWorkspaceCascade(ctx, id, nil) + return r.deleteWorkspaceCascade(ctx, id, nil, nil) } // DeleteWorkspaceCascadeWithName deletes a workspace and its task/workflow rows @@ -154,13 +154,34 @@ func (r *Repository) DeleteWorkspaceCascadeWithName( ctx context.Context, id, name string, ) ([]*models.Task, []*models.Workflow, error) { - return r.deleteWorkspaceCascade(ctx, id, &name) + return r.deleteWorkspaceCascade(ctx, id, &name, nil) +} + +// DeleteWorkspaceCascadeWithSecretCleanup deletes the workspace cascade and +// invokes cleanup on the same transaction before commit. +func (r *Repository) DeleteWorkspaceCascadeWithSecretCleanup( + ctx context.Context, + id string, + cleanup func(context.Context, *sqlx.Tx) error, +) ([]*models.Task, []*models.Workflow, error) { + return r.deleteWorkspaceCascade(ctx, id, nil, cleanup) +} + +// DeleteWorkspaceCascadeWithNameAndSecretCleanup is the confirmation-aware +// transactional variant of DeleteWorkspaceCascadeWithName. +func (r *Repository) DeleteWorkspaceCascadeWithNameAndSecretCleanup( + ctx context.Context, + id, name string, + cleanup func(context.Context, *sqlx.Tx) error, +) ([]*models.Task, []*models.Workflow, error) { + return r.deleteWorkspaceCascade(ctx, id, &name, cleanup) } func (r *Repository) deleteWorkspaceCascade( ctx context.Context, id string, expectedName *string, + cleanup func(context.Context, *sqlx.Tx) error, ) ([]*models.Task, []*models.Workflow, error) { tx, err := r.db.BeginTxx(ctx, nil) if err != nil { @@ -218,6 +239,11 @@ func (r *Repository) deleteWorkspaceCascade( `), id); err != nil { return nil, nil, err } + if cleanup != nil { + if err := cleanup(ctx, tx); err != nil { + return nil, nil, fmt.Errorf("workspace secret cleanup: %w", err) + } + } if err := tx.Commit(); err != nil { return nil, nil, err } diff --git a/apps/backend/internal/task/repository/sqlite/workspace_test.go b/apps/backend/internal/task/repository/sqlite/workspace_test.go index dfe9ed5972..deaa525115 100644 --- a/apps/backend/internal/task/repository/sqlite/workspace_test.go +++ b/apps/backend/internal/task/repository/sqlite/workspace_test.go @@ -5,6 +5,8 @@ import ( "errors" "testing" + "github.com/jmoiron/sqlx" + "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/orchestrator/messagequeue" "github.com/kandev/kandev/internal/task/models" @@ -43,6 +45,32 @@ func TestDeleteWorkspaceCascadeWithNameDeletesWorkspaceChildren(t *testing.T) { assertNoWorkspaceCascadeDependents(t, repo) } +func TestDeleteWorkspaceCascadeWithSecretCleanupRollsBackOnCleanupFailure(t *testing.T) { + ctx := context.Background() + repo := newRepoForHealTests(t) + seedWorkspaceCascadeRows(t, repo, "ws-delete") + + _, _, err := repo.DeleteWorkspaceCascadeWithSecretCleanup(ctx, "ws-delete", func(context.Context, *sqlx.Tx) error { + return errors.New("injected secret cleanup failure") + }) + if err == nil { + t.Fatal("delete succeeded, want cleanup failure") + } + if _, err := repo.GetWorkspace(ctx, "ws-delete"); err != nil { + t.Fatalf("workspace was deleted after cleanup failure: %v", err) + } + if _, err := repo.GetTask(ctx, "task-delete"); err != nil { + t.Fatalf("workspace task was deleted after cleanup failure: %v", err) + } + workflows, err := repo.ListWorkflows(ctx, "ws-delete", true) + if err != nil { + t.Fatalf("ListWorkflows: %v", err) + } + if len(workflows) != 1 { + t.Fatalf("workflows after cleanup failure = %#v, want one", workflows) + } +} + func TestDeleteWorkspaceCascadeDeletesWorkspaceChildren(t *testing.T) { ctx := context.Background() repo := newRepoForHealTests(t) diff --git a/apps/backend/internal/task/service/service.go b/apps/backend/internal/task/service/service.go index 057e8ec2ae..e5102176d2 100644 --- a/apps/backend/internal/task/service/service.go +++ b/apps/backend/internal/task/service/service.go @@ -8,8 +8,11 @@ import ( "strings" "sync" + "github.com/jmoiron/sqlx" + "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/events/bus" + "github.com/kandev/kandev/internal/secrets" "github.com/kandev/kandev/internal/task/models" "github.com/kandev/kandev/internal/task/repository" wfmodels "github.com/kandev/kandev/internal/workflow/models" @@ -23,6 +26,25 @@ type WorktreeCleanup interface { OnTaskDeleted(ctx context.Context, taskID string) error } +// WorkspaceSecretDeleter removes secrets owned by a workspace. It is optional +// for isolated task-service users. +type WorkspaceSecretDeleter interface { + DeleteWorkspaceSecrets(ctx context.Context, workspaceID string) error +} + +type transactionalWorkspaceCascade interface { + DeleteWorkspaceCascadeWithSecretCleanup( + ctx context.Context, + id string, + cleanup func(context.Context, *sqlx.Tx) error, + ) ([]*models.Task, []*models.Workflow, error) + DeleteWorkspaceCascadeWithNameAndSecretCleanup( + ctx context.Context, + id, name string, + cleanup func(context.Context, *sqlx.Tx) error, + ) ([]*models.Task, []*models.Workflow, error) +} + // WorktreeProvider extends WorktreeCleanup with query capabilities. // Implementations that support this can be type-asserted from WorktreeCleanup. type WorktreeProvider interface { @@ -277,6 +299,8 @@ type Service struct { repoCloneLocation RepoCloneLocation blockers BlockerRepository comments CommentRepository + secretStore secrets.SecretStore + workspaceSecretDeleter WorkspaceSecretDeleter baseBranchPusher AgentBaseBranchPusher runtimeOverridesMu sync.Mutex @@ -336,6 +360,18 @@ func (s *Service) AttachmentRepository() repository.AttachmentRepository { return s.attachments } +// SetSecretStore wires metadata-only validation for shared executor profiles. +// Workspace-scoped secret references are rejected before a profile is saved. +func (s *Service) SetSecretStore(secretStore secrets.SecretStore) { + s.secretStore = secretStore +} + +// SetWorkspaceSecretDeleter wires workspace-secret cleanup to workspace +// deletion. The callback runs only after the repository cascade succeeds. +func (s *Service) SetWorkspaceSecretDeleter(deleter WorkspaceSecretDeleter) { + s.workspaceSecretDeleter = deleter +} + // NewService creates a new task service func NewService(repos Repos, eventBus bus.EventBus, log *logger.Logger, discoveryConfig RepositoryDiscoveryConfig) *Service { return &Service{ diff --git a/apps/backend/internal/task/service/service_events.go b/apps/backend/internal/task/service/service_events.go index 91a8aaf8f4..3e427f731d 100644 --- a/apps/backend/internal/task/service/service_events.go +++ b/apps/backend/internal/task/service/service_events.go @@ -899,6 +899,10 @@ func (s *Service) publishRepositoryEvent(ctx context.Context, eventType string, if s.eventBus == nil || repository == nil { return } + bindings := make([]map[string]string, 0, len(repository.SecretBindings)) + for _, binding := range repository.SecretBindings { + bindings = append(bindings, map[string]string{"key": binding.Key, "secret_id": binding.SecretID}) + } data := map[string]interface{}{ "id": repository.ID, "workspace_id": repository.WorkspaceID, @@ -917,6 +921,7 @@ func (s *Service) publishRepositoryEvent(ctx context.Context, eventType string, "cleanup_script": repository.CleanupScript, "dev_script": repository.DevScript, "copy_files": repository.CopyFiles, + "secret_bindings": bindings, "created_at": repository.CreatedAt.Format(time.RFC3339), "updated_at": repository.UpdatedAt.Format(time.RFC3339), } diff --git a/apps/backend/internal/task/service/service_requests.go b/apps/backend/internal/task/service/service_requests.go index 5305f824a3..260d83f2b7 100644 --- a/apps/backend/internal/task/service/service_requests.go +++ b/apps/backend/internal/task/service/service_requests.go @@ -138,24 +138,32 @@ type FindOrCreateRepositoryRequest struct { // CreateRepositoryRequest contains the data for creating a new repository type CreateRepositoryRequest struct { - WorkspaceID string `json:"workspace_id"` - Name string `json:"name"` - SourceType string `json:"source_type"` - LocalPath string `json:"local_path"` - Provider string `json:"provider"` - ProviderRepoID string `json:"provider_repo_id"` - ProviderHost string `json:"provider_host"` - ProviderOwner string `json:"provider_owner"` - ProviderName string `json:"provider_name"` - RemoteURL string `json:"remote_url"` - DefaultBranch string `json:"default_branch"` - WorktreeBranchPrefix string `json:"worktree_branch_prefix"` - WorktreeBranchTemplate string `json:"worktree_branch_template"` - PullBeforeWorktree *bool `json:"pull_before_worktree"` - SetupScript string `json:"setup_script"` - CleanupScript string `json:"cleanup_script"` - DevScript string `json:"dev_script"` - CopyFiles string `json:"copy_files"` + WorkspaceID string `json:"workspace_id"` + Name string `json:"name"` + SourceType string `json:"source_type"` + LocalPath string `json:"local_path"` + Provider string `json:"provider"` + ProviderRepoID string `json:"provider_repo_id"` + ProviderHost string `json:"provider_host"` + ProviderOwner string `json:"provider_owner"` + ProviderName string `json:"provider_name"` + RemoteURL string `json:"remote_url"` + DefaultBranch string `json:"default_branch"` + WorktreeBranchPrefix string `json:"worktree_branch_prefix"` + WorktreeBranchTemplate string `json:"worktree_branch_template"` + PullBeforeWorktree *bool `json:"pull_before_worktree"` + SetupScript string `json:"setup_script"` + CleanupScript string `json:"cleanup_script"` + DevScript string `json:"dev_script"` + CopyFiles string `json:"copy_files"` + SecretBindings []RepositorySecretBindingInput `json:"secret_bindings,omitempty"` +} + +// RepositorySecretBindingInput is the write-only reference shape accepted by +// repository mutations. Secret values never cross this boundary. +type RepositorySecretBindingInput struct { + Key string `json:"key"` + SecretID string `json:"secret_id"` } // InitializeLocalRepositoryRequest contains the data for creating and registering a new local repository. @@ -183,6 +191,9 @@ type UpdateRepositoryRequest struct { CleanupScript *string `json:"cleanup_script,omitempty"` DevScript *string `json:"dev_script,omitempty"` CopyFiles *string `json:"copy_files,omitempty"` + // SecretBindings uses nil to preserve the current set and a non-nil empty + // slice to clear it. + SecretBindings *[]RepositorySecretBindingInput `json:"secret_bindings,omitempty"` } // CreateExecutorRequest contains the data for creating an executor diff --git a/apps/backend/internal/task/service/service_resources.go b/apps/backend/internal/task/service/service_resources.go index 21f0b52688..3818bf0268 100644 --- a/apps/backend/internal/task/service/service_resources.go +++ b/apps/backend/internal/task/service/service_resources.go @@ -6,14 +6,17 @@ import ( "fmt" "net/url" "os" + "regexp" "strings" "sync" "time" "github.com/google/uuid" + "github.com/jmoiron/sqlx" "go.uber.org/zap" "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/secrets" "github.com/kandev/kandev/internal/task/models" taskrepo "github.com/kandev/kandev/internal/task/repository" "github.com/kandev/kandev/internal/task/repository/repoerrors" @@ -29,6 +32,10 @@ const ( var ErrWorkspaceConfirmNameMismatch = errors.New("confirm_name does not match workspace name") +const maxRepositorySecretBindings = 100 + +var repositorySecretKeyPattern = regexp.MustCompile(`^[A-Za-z_][A-Za-z0-9_]*$`) + func normalizeProviderHost(provider, raw string) string { raw = strings.TrimSpace(raw) if raw == "" { @@ -211,15 +218,34 @@ func (s *Service) deleteWorkspace(ctx context.Context, workspace *models.Workspa var deletedTasks []*models.Task var deletedWorkflows []*models.Workflow - if confirmedName == nil { + transactionalCleanup, hasTransactionalCleanup := s.workspaceSecretDeleter.(secrets.WorkspaceSecretTransactionalDeleter) + transactionalCascade, hasTransactionalCascade := s.workspaces.(transactionalWorkspaceCascade) + switch { + case hasTransactionalCleanup && hasTransactionalCascade: + cleanup := func(cleanupCtx context.Context, tx *sqlx.Tx) error { + return transactionalCleanup.DeleteWorkspaceSecretsTx(cleanupCtx, tx, workspace.ID) + } + if confirmedName == nil { + deletedTasks, deletedWorkflows, err = transactionalCascade.DeleteWorkspaceCascadeWithSecretCleanup(ctx, workspace.ID, cleanup) + } else { + deletedTasks, deletedWorkflows, err = transactionalCascade.DeleteWorkspaceCascadeWithNameAndSecretCleanup(ctx, workspace.ID, *confirmedName, cleanup) + } + case confirmedName == nil: deletedTasks, deletedWorkflows, err = s.workspaces.DeleteWorkspaceCascade(ctx, workspace.ID) - } else { + default: deletedTasks, deletedWorkflows, err = s.workspaces.DeleteWorkspaceCascadeWithName(ctx, workspace.ID, *confirmedName) } if err != nil { s.cancelWorkspaceDeleteTaskCleanupJobs(ctx, cleanups) return s.mapWorkspaceDeleteError(workspace.ID, err) } + if s.workspaceSecretDeleter != nil && (!hasTransactionalCleanup || !hasTransactionalCascade) { + if err := s.workspaceSecretDeleter.DeleteWorkspaceSecrets(ctx, workspace.ID); err != nil { + s.cancelWorkspaceDeleteTaskCleanupJobs(ctx, cleanups) + s.logger.Error("failed to delete workspace secrets", zap.String("workspace_id", workspace.ID), zap.Error(err)) + return err + } + } cleanups = s.appendWorkspaceDeleteMissingTaskCleanups(ctx, cleanups, deletedTasks) s.publishWorkspaceDeleteChildEvents(ctx, deletedTasks, deletedWorkflows) s.runWorkspaceDeleteTaskCleanups(cleanups, deletedTasks) @@ -735,6 +761,10 @@ func (s *Service) createRepository( if err := copyfiles.ValidateSpec(req.CopyFiles); err != nil { return nil, fmt.Errorf("%w: %s", ErrInvalidRepositorySettings, err) } + bindings, err := s.validateRepositorySecretBindings(ctx, req.WorkspaceID, req.SecretBindings) + if err != nil { + return nil, err + } repository := &models.Repository{ ID: uuid.New().String(), WorkspaceID: req.WorkspaceID, @@ -755,13 +785,21 @@ func (s *Service) createRepository( CleanupScript: req.CleanupScript, DevScript: req.DevScript, CopyFiles: req.CopyFiles, + SecretBindings: bindings, } if resolveProvider { resolveRepositoryProviderIdentity(repository) } - if err := s.repoEntities.CreateRepository(ctx, repository); err != nil { + if mutator, ok := s.repoEntities.(taskrepo.RepositorySecretBindingMutator); ok { + if err := mutator.CreateRepositoryWithSecretBindings(ctx, repository, bindings); err != nil { + s.logger.Error("failed to create repository", zap.Error(err)) + return nil, err + } + } else if len(bindings) > 0 { + return nil, fmt.Errorf("%w: repository secret bindings are unavailable", ErrInvalidRepositorySettings) + } else if err := s.repoEntities.CreateRepository(ctx, repository); err != nil { s.logger.Error("failed to create repository", zap.Error(err)) return nil, err } @@ -987,9 +1025,26 @@ func (s *Service) UpdateRepository(ctx context.Context, id string, req *UpdateRe if err := applyRepositoryUpdates(repository, &updates); err != nil { return nil, err } + var replacement []models.RepositorySecretBinding + if req.SecretBindings != nil { + replacement, err = s.validateRepositorySecretBindings(ctx, repository.WorkspaceID, *req.SecretBindings) + if err != nil { + return nil, err + } + repository.SecretBindings = replacement + } repository.UpdatedAt = time.Now().UTC() - if err := s.repoEntities.UpdateRepository(ctx, repository); err != nil { + if req.SecretBindings != nil { + mutator, ok := s.repoEntities.(taskrepo.RepositorySecretBindingMutator) + if !ok { + return nil, fmt.Errorf("%w: repository secret bindings are unavailable", ErrInvalidRepositorySettings) + } + if err := mutator.UpdateRepositoryWithSecretBindings(ctx, repository, replacement); err != nil { + s.logger.Error("failed to update repository", zap.String("repository_id", id), zap.Error(err)) + return nil, err + } + } else if err := s.repoEntities.UpdateRepository(ctx, repository); err != nil { s.logger.Error("failed to update repository", zap.String("repository_id", id), zap.Error(err)) return nil, err } @@ -999,6 +1054,43 @@ func (s *Service) UpdateRepository(ctx context.Context, id string, req *UpdateRe return repository, nil } +func (s *Service) validateRepositorySecretBindings( + ctx context.Context, workspaceID string, inputs []RepositorySecretBindingInput, +) ([]models.RepositorySecretBinding, error) { + if len(inputs) > maxRepositorySecretBindings { + return nil, fmt.Errorf("%w: at most %d secret bindings are allowed", ErrInvalidRepositorySettings, maxRepositorySecretBindings) + } + if len(inputs) == 0 { + return nil, nil + } + catalog, ok := s.secretStore.(secrets.ScopedSecretStore) + if !ok { + return nil, fmt.Errorf("%w: secret binding validation is unavailable", ErrInvalidRepositorySettings) + } + seen := make(map[string]struct{}, len(inputs)) + bindings := make([]models.RepositorySecretBinding, 0, len(inputs)) + for _, input := range inputs { + key := strings.TrimSpace(input.Key) + if key != input.Key || key == "" || len(key) > 256 || !repositorySecretKeyPattern.MatchString(key) || + key == "TASK_DESCRIPTION" || strings.HasPrefix(key, "KANDEV_") { + return nil, fmt.Errorf("%w: invalid secret binding key", ErrInvalidRepositorySettings) + } + if _, exists := seen[key]; exists { + return nil, fmt.Errorf("%w: duplicate secret binding key", ErrInvalidRepositorySettings) + } + seen[key] = struct{}{} + secretID := strings.TrimSpace(input.SecretID) + if secretID == "" { + return nil, fmt.Errorf("%w: secret binding reference is required", ErrInvalidRepositorySettings) + } + if _, err := catalog.GetForWorkspace(ctx, secretID, workspaceID); err != nil { + return nil, fmt.Errorf("%w: secret binding reference is not available", ErrInvalidRepositorySettings) + } + bindings = append(bindings, models.RepositorySecretBinding{Key: key, SecretID: secretID}) + } + return bindings, nil +} + func canonicalRepositoryLocalPath(localPath string) (string, error) { if localPath == "" { return "", nil @@ -1445,6 +1537,9 @@ func (s *Service) CreateExecutorProfile(ctx context.Context, req *CreateExecutor if req.ExecutorID == "" { return nil, fmt.Errorf("executor_id is required") } + if err := s.validateGlobalProfileEnvRefs(ctx, req.EnvVars); err != nil { + return nil, err + } // Verify executor exists executor, err := s.executors.GetExecutor(ctx, req.ExecutorID) if err != nil { @@ -1498,6 +1593,9 @@ func (s *Service) UpdateExecutorProfile(ctx context.Context, id string, req *Upd profile.CleanupScript = *req.CleanupScript } if req.EnvVars != nil { + if err := s.validateGlobalProfileEnvRefs(ctx, req.EnvVars); err != nil { + return nil, err + } if executor.Type == models.ExecutorTypeSprites { req.EnvVars = mergeSpritesTokenEnvVars(profile.EnvVars, req.EnvVars) } @@ -1510,6 +1608,21 @@ func (s *Service) UpdateExecutorProfile(ctx context.Context, id string, req *Upd return profile, nil } +func (s *Service) validateGlobalProfileEnvRefs(ctx context.Context, envVars []models.ProfileEnvVar) error { + if s.secretStore == nil { + return nil + } + for i, envVar := range envVars { + if strings.TrimSpace(envVar.SecretID) == "" { + continue + } + if err := secrets.ValidateGlobalReference(ctx, s.secretStore, envVar.SecretID); err != nil { + return fmt.Errorf("profile env_vars[%d] secret must be global", i) + } + } + return nil +} + func mergeSpritesTokenEnvVars(existing, incoming []models.ProfileEnvVar) []models.ProfileEnvVar { merged := make([]models.ProfileEnvVar, 0, len(incoming)+1) var existingToken models.ProfileEnvVar diff --git a/apps/backend/internal/task/service/service_resources_test.go b/apps/backend/internal/task/service/service_resources_test.go index b6c316bda7..7cbde98b5a 100644 --- a/apps/backend/internal/task/service/service_resources_test.go +++ b/apps/backend/internal/task/service/service_resources_test.go @@ -12,8 +12,10 @@ import ( "testing" "time" + "github.com/jmoiron/sqlx" commonlogger "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/events" + "github.com/kandev/kandev/internal/secrets" "github.com/kandev/kandev/internal/task/models" "github.com/kandev/kandev/internal/task/repository" "github.com/kandev/kandev/internal/worktree" @@ -26,6 +28,16 @@ type failingWorkspaceBootstrapper struct { err error } +type failingTransactionalWorkspaceSecretDeleter struct{} + +func (failingTransactionalWorkspaceSecretDeleter) DeleteWorkspaceSecrets(context.Context, string) error { + return errors.New("legacy cleanup should not be used") +} + +func (failingTransactionalWorkspaceSecretDeleter) DeleteWorkspaceSecretsTx(context.Context, *sqlx.Tx, string) error { + return errors.New("injected transactional secret cleanup failure") +} + func (b *failingWorkspaceBootstrapper) CreateWorkspaceWithKanban( context.Context, *models.Workspace, @@ -131,6 +143,131 @@ func TestService_CreateRepositoryCanonicalizesExplicitLocalPath(t *testing.T) { } } +func TestService_RepositorySecretBindingsValidateScopeAndReplaceAtomically(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-secret-bindings", Name: "Workspace"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + secretDB := sqlx.NewDb(repo.DB(), "sqlite3") + crypto, err := secrets.NewMasterKeyProvider(t.TempDir()) + if err != nil { + t.Fatalf("master key: %v", err) + } + secretStore, closeSecrets, err := secrets.Provide(secretDB, secretDB, crypto) + if err != nil { + t.Fatalf("secret store: %v", err) + } + t.Cleanup(func() { _ = closeSecrets() }) + svc.SetSecretStore(secretStore) + + global := &secrets.SecretWithValue{Secret: secrets.Secret{Name: "global"}, Value: "global-value"} + workspace := &secrets.SecretWithValue{Secret: secrets.Secret{ + Name: "workspace", Scope: secrets.ScopeWorkspace, WorkspaceID: "ws-secret-bindings", + }, Value: "workspace-value"} + if err := secretStore.Create(ctx, global); err != nil { + t.Fatalf("create global secret: %v", err) + } + if err := secretStore.Create(ctx, workspace); err != nil { + t.Fatalf("create workspace secret: %v", err) + } + + created, err := svc.CreateRepository(ctx, &CreateRepositoryRequest{ + WorkspaceID: "ws-secret-bindings", + Name: "app", + SecretBindings: []RepositorySecretBindingInput{ + {Key: "GLOBAL_TOKEN", SecretID: global.ID}, + {Key: "WORKSPACE_TOKEN", SecretID: workspace.ID}, + }, + }) + if err != nil { + t.Fatalf("create repository: %v", err) + } + if len(created.SecretBindings) != 2 { + t.Fatalf("created bindings = %+v, want two", created.SecretBindings) + } + + bad, err := svc.CreateRepository(ctx, &CreateRepositoryRequest{ + WorkspaceID: "ws-secret-bindings", + Name: "bad", + SecretBindings: []RepositorySecretBindingInput{{Key: "BAD", SecretID: "missing-secret"}}, + }) + if err == nil || bad != nil || !errors.Is(err, ErrInvalidRepositorySettings) { + t.Fatalf("missing binding result = %v, %+v; want invalid settings", err, bad) + } + + clear := []RepositorySecretBindingInput{} + updated, err := svc.UpdateRepository(ctx, created.ID, &UpdateRepositoryRequest{SecretBindings: &clear}) + if err != nil { + t.Fatalf("clear bindings: %v", err) + } + if len(updated.SecretBindings) != 0 { + t.Fatalf("bindings after clear = %+v, want empty", updated.SecretBindings) + } + loaded, err := repo.GetRepository(ctx, created.ID) + if err != nil { + t.Fatalf("get after clear: %v", err) + } + if len(loaded.SecretBindings) != 0 { + t.Fatalf("persisted bindings after clear = %+v, want empty", loaded.SecretBindings) + } +} + +func TestService_ExecutorProfileSecretRefsRequireGlobalScope(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-profile-secrets", Name: "Workspace"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + secretDB := sqlx.NewDb(repo.DB(), "sqlite3") + crypto, err := secrets.NewMasterKeyProvider(t.TempDir()) + if err != nil { + t.Fatalf("master key: %v", err) + } + secretStore, closeSecrets, err := secrets.Provide(secretDB, secretDB, crypto) + if err != nil { + t.Fatalf("secret store: %v", err) + } + t.Cleanup(func() { _ = closeSecrets() }) + svc.SetSecretStore(secretStore) + + workspaceSecret := &secrets.SecretWithValue{Secret: secrets.Secret{ + ID: "workspace-profile-secret", Scope: secrets.ScopeWorkspace, WorkspaceID: "ws-profile-secrets", + }, Value: "workspace-value"} + if err := secretStore.Create(ctx, workspaceSecret); err != nil { + t.Fatalf("create workspace secret: %v", err) + } + if err := svc.validateGlobalProfileEnvRefs(ctx, []models.ProfileEnvVar{{Key: "TOKEN", SecretID: workspaceSecret.ID}}); err == nil { + t.Fatal("workspace secret accepted in executor profile") + } +} + +func TestService_ExecutorProfileRejectsBackendOwnedSecretID(t *testing.T) { + svc, _, repo := createTestService(t) + ctx := context.Background() + secretDB := sqlx.NewDb(repo.DB(), "sqlite3") + crypto, err := secrets.NewMasterKeyProvider(t.TempDir()) + if err != nil { + t.Fatalf("master key: %v", err) + } + secretStore, closeSecrets, err := secrets.Provide(secretDB, secretDB, crypto) + if err != nil { + t.Fatalf("secret store: %v", err) + } + t.Cleanup(func() { _ = closeSecrets() }) + svc.SetSecretStore(secrets.NewUserVisibleStore(secretStore)) + + internal := &secrets.SecretWithValue{Secret: secrets.Secret{ + ID: "github:user:workspace:user:access", Scope: secrets.ScopeGlobal, + }, Value: "backend-owned"} + if err := secretStore.Create(ctx, internal); err != nil { + t.Fatalf("create internal secret: %v", err) + } + if err := svc.validateGlobalProfileEnvRefs(ctx, []models.ProfileEnvVar{{Key: "TOKEN", SecretID: internal.ID}}); err == nil { + t.Fatal("backend-owned secret ID accepted in executor profile") + } +} + // TestService_CreateRepositoryResolvesRemoteURLForSelfHostedGitLabOrigin // covers the bug where a local repository cloned from a self-hosted GitLab // instance (e.g. gitlab.example.com, not gitlab.com) got no RemoteURL @@ -834,6 +971,37 @@ func TestService_DeleteWorkspaceDeletesWorkspaceOwnedTasksAndWorkflows(t *testin } } +func TestService_DeleteWorkspaceRollsBackCascadeWhenSecretCleanupFails(t *testing.T) { + svc, eventBus, repo := createTestService(t) + ctx := context.Background() + svc.SetWorkspaceSecretDeleter(failingTransactionalWorkspaceSecretDeleter{}) + if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-delete", Name: "Delete Me"}); err != nil { + t.Fatalf("CreateWorkspace: %v", err) + } + if err := repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-delete", WorkspaceID: "ws-delete", Name: "Doomed"}); err != nil { + t.Fatalf("CreateWorkflow: %v", err) + } + if err := repo.CreateTask(ctx, &models.Task{ + ID: "task-delete", WorkspaceID: "ws-delete", WorkflowID: "wf-delete", WorkflowStepID: "step-delete", Title: "Delete task", + }); err != nil { + t.Fatalf("CreateTask: %v", err) + } + eventBus.ClearEvents() + + if err := svc.DeleteWorkspace(ctx, "ws-delete"); err == nil { + t.Fatal("DeleteWorkspace succeeded, want secret cleanup failure") + } + if _, err := repo.GetWorkspace(ctx, "ws-delete"); err != nil { + t.Fatalf("workspace was deleted after cleanup failure: %v", err) + } + if _, err := repo.GetTask(ctx, "task-delete"); err != nil { + t.Fatalf("task was deleted after cleanup failure: %v", err) + } + if events := eventBus.GetPublishedEvents(); len(events) != 0 { + t.Fatalf("events after rolled-back delete = %#v, want none", events) + } +} + func TestService_DeleteWorkspacePublishesChildEventsAndCleansResources(t *testing.T) { svc, eventBus, repo := createTestService(t) ctx := context.Background() diff --git a/apps/backend/internal/task/service/service_turns.go b/apps/backend/internal/task/service/service_turns.go index 1ee2266afa..06ce6eeb7a 100644 --- a/apps/backend/internal/task/service/service_turns.go +++ b/apps/backend/internal/task/service/service_turns.go @@ -602,6 +602,7 @@ func (s *Service) GetWorkspaceInfoForSession(ctx context.Context, taskID, sessio WorkspacePath: workspacePath, AgentProfileID: session.AgentProfileID, ExecutionProfileID: session.ExecutionProfileID, + ExecutorProfileID: session.ExecutorProfileID, AgentID: agentID, ACPSessionID: acpSessionID, SessionMode: sessionMode, @@ -888,6 +889,9 @@ func applyTaskEnvironmentToWorkspaceInfo(info *lifecycle.WorkspaceInfo, env *mod // while the ID still pointed at the stale row — a mismatch downstream // reconcilers and progress events would key off the wrong env. info.TaskEnvironmentID = env.ID + if info.ExecutorProfileID == "" { + info.ExecutorProfileID = env.ExecutorProfileID + } if info.WorkspacePath == "" { info.WorkspacePath = env.WorkspacePath } diff --git a/apps/backend/internal/task/service/service_turns_test.go b/apps/backend/internal/task/service/service_turns_test.go index 8056d0c8e0..eb05e5f7eb 100644 --- a/apps/backend/internal/task/service/service_turns_test.go +++ b/apps/backend/internal/task/service/service_turns_test.go @@ -162,6 +162,7 @@ func TestGetWorkspaceInfoForSession_BasicFields(t *testing.T) { TaskEnvironmentID: "env-123", AgentProfileID: "profile-1", ExecutionProfileID: "claude-opus", + ExecutorProfileID: "executor-profile-1", State: models.TaskSessionStateCompleted, AgentProfileSnapshot: map[string]interface{}{ "agent_name": "auggie", @@ -212,6 +213,9 @@ func TestGetWorkspaceInfoForSession_BasicFields(t *testing.T) { if info.ExecutionProfileID != "claude-opus" { t.Errorf("expected ExecutionProfileID 'claude-opus', got %q", info.ExecutionProfileID) } + if info.ExecutorProfileID != "executor-profile-1" { + t.Errorf("expected ExecutorProfileID 'executor-profile-1', got %q", info.ExecutorProfileID) + } if info.AgentID != "auggie" { t.Errorf("expected AgentID 'auggie', got %q", info.AgentID) } @@ -220,6 +224,31 @@ func TestGetWorkspaceInfoForSession_BasicFields(t *testing.T) { } } +func TestApplyTaskEnvironmentToWorkspaceInfoUsesEnvironmentProfileAsFallback(t *testing.T) { + info := &lifecycle.WorkspaceInfo{} + applyTaskEnvironmentToWorkspaceInfo(info, &models.TaskEnvironment{ + ID: "env-1", + ExecutorProfileID: "executor-profile", + WorkspacePath: "/workspace/task-1", + }) + + if info.ExecutorProfileID != "executor-profile" { + t.Fatalf("ExecutorProfileID = %q, want executor-profile", info.ExecutorProfileID) + } + if info.WorkspacePath != "/workspace/task-1" { + t.Fatalf("WorkspacePath = %q, want /workspace/task-1", info.WorkspacePath) + } + + info.ExecutorProfileID = "session-profile" + applyTaskEnvironmentToWorkspaceInfo(info, &models.TaskEnvironment{ + ID: "env-2", + ExecutorProfileID: "stale-environment-profile", + }) + if info.ExecutorProfileID != "session-profile" { + t.Fatalf("ExecutorProfileID = %q after session value, want session-profile", info.ExecutorProfileID) + } +} + func TestGetWorkspaceInfoForSession_ProjectsPersistedWorktreeIdentity(t *testing.T) { svc, _, repo := createTestService(t) ctx := context.Background() diff --git a/apps/web/app/actions/workspace-repository-secrets.test.ts b/apps/web/app/actions/workspace-repository-secrets.test.ts new file mode 100644 index 0000000000..239186422d --- /dev/null +++ b/apps/web/app/actions/workspace-repository-secrets.test.ts @@ -0,0 +1,55 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/config", () => ({ + getBackendConfig: () => ({ apiBaseUrl: "http://backend.test" }), +})); + +import { createRepositoryAction, updateRepositoryAction } from "./workspaces"; + +describe("repository secret binding actions", () => { + const fetchSpy = vi.fn(); + + beforeEach(() => { + fetchSpy.mockReset(); + fetchSpy.mockResolvedValue( + Response.json({ id: "repo-1", secret_bindings: [] }, { status: 200 }), + ); + vi.stubGlobal("fetch", fetchSpy); + }); + + afterEach(() => vi.unstubAllGlobals()); + + it("includes bindings when creating a repository", async () => { + await createRepositoryAction({ + workspace_id: "workspace-1", + name: "repo", + source_type: "local", + local_path: "/tmp/repo", + provider: "", + provider_repo_id: "", + provider_owner: "", + provider_name: "", + default_branch: "main", + worktree_branch_prefix: "feature/", + worktree_branch_template: "feature/{title}-{suffix}", + pull_before_worktree: true, + setup_script: "", + cleanup_script: "", + dev_script: "", + copy_files: "", + secret_bindings: [{ key: "TOKEN", secret_id: "secret-1" }], + }); + + const [, init] = fetchSpy.mock.calls[0]!; + expect(JSON.parse(String(init?.body)).secret_bindings).toEqual([ + { key: "TOKEN", secret_id: "secret-1" }, + ]); + }); + + it("sends an explicit empty list when clearing bindings", async () => { + await updateRepositoryAction("repo-1", { secret_bindings: [] }); + + const [, init] = fetchSpy.mock.calls[0]!; + expect(JSON.parse(String(init?.body))).toEqual({ secret_bindings: [] }); + }); +}); diff --git a/apps/web/app/actions/workspaces.ts b/apps/web/app/actions/workspaces.ts index 73da7be477..fb9738f007 100644 --- a/apps/web/app/actions/workspaces.ts +++ b/apps/web/app/actions/workspaces.ts @@ -15,6 +15,7 @@ import type { ListWorkspacesResponse, RepositoryPathValidationResponse, Repository, + RepositorySecretBinding, RepositoryScript, StepEvents, Workspace, @@ -230,6 +231,7 @@ export async function createRepositoryAction(payload: { cleanup_script: string; dev_script: string; copy_files: string; + secret_bindings?: RepositorySecretBinding[]; }) { return fetchJson( `${apiBaseUrl}/api/v1/workspaces/${payload.workspace_id}/repositories`, @@ -252,6 +254,7 @@ export async function createRepositoryAction(payload: { cleanup_script: payload.cleanup_script, dev_script: payload.dev_script, copy_files: payload.copy_files, + secret_bindings: payload.secret_bindings, }), }, ); diff --git a/apps/web/app/settings/workspace/[id]/secrets/page.tsx b/apps/web/app/settings/workspace/[id]/secrets/page.tsx new file mode 100644 index 0000000000..7d027e5ec3 --- /dev/null +++ b/apps/web/app/settings/workspace/[id]/secrets/page.tsx @@ -0,0 +1,23 @@ +import { SecretsSettings } from "@/components/settings/secrets-settings"; +import { listSecrets } from "@/lib/api/domains/secrets-api"; +import type { SecretListItem } from "@/lib/types/http-secrets"; + +export default async function WorkspaceSecretsPage({ + params, +}: { + params: Promise<{ id: string }>; +}) { + const { id } = await params; + let initialItems: SecretListItem[] = []; + try { + initialItems = await listSecrets({ + scope: "workspace", + workspaceId: id, + cache: "no-store", + }); + } catch { + initialItems = []; + } + + return ; +} diff --git a/apps/web/app/settings/workspace/workspace-repositories-client.tsx b/apps/web/app/settings/workspace/workspace-repositories-client.tsx index e20affd0b6..095524a337 100644 --- a/apps/web/app/settings/workspace/workspace-repositories-client.tsx +++ b/apps/web/app/settings/workspace/workspace-repositories-client.tsx @@ -82,6 +82,7 @@ function buildDraftRepo( cleanup_script: "", dev_script: "", copy_files: "", + secret_bindings: [], created_at: "", updated_at: "", scripts: [], @@ -123,6 +124,7 @@ async function saveNewRepository( cleanup_script: repo.cleanup_script, dev_script: repo.dev_script, copy_files: repo.copy_files, + secret_bindings: repo.secret_bindings ?? [], }); // Like the repository name above, the seeded script name and command are // PERSISTED and sent to a shell verbatim, so both stay in English. @@ -179,6 +181,7 @@ async function saveExistingRepository({ cleanup_script: repo.cleanup_script, dev_script: repo.dev_script, copy_files: repo.copy_files, + secret_bindings: repo.secret_bindings ?? [], }); const savedScripts = savedRepositoriesById.get(repoId)?.scripts ?? []; const currentScriptIds = new Set(repo.scripts.map((s) => s.id)); diff --git a/apps/web/app/settings/workspace/workspace-repositories-dirty.test.ts b/apps/web/app/settings/workspace/workspace-repositories-dirty.test.ts index 48caea6678..c1ad41dcfa 100644 --- a/apps/web/app/settings/workspace/workspace-repositories-dirty.test.ts +++ b/apps/web/app/settings/workspace/workspace-repositories-dirty.test.ts @@ -1,26 +1,40 @@ import { describe, expect, it } from "vitest"; +import type { Repository, RepositoryScript } from "@/lib/types/http"; +import { repositoryId } from "@/lib/types/ids"; import { + areRepositorySecretBindingsEqual, + cloneRepository, isRepositoryDirty, mergeSavedRepositoryDraft, type RepositoryWithScripts, -} from "@/app/settings/workspace/workspace-repositories-dirty"; -import { repositoryId as toRepositoryId, workspaceId as toWorkspaceId } from "@/lib/types/http"; -import { defaultWorktreeBranchTemplate } from "@/lib/worktree-branch-template"; +} from "./workspace-repositories-dirty"; -function makeRepo(overrides: Partial = {}): RepositoryWithScripts { - return { - id: toRepositoryId("repo-1"), - workspace_id: toWorkspaceId("ws-1"), - name: "my-repo", +const script = (overrides: Partial = {}): RepositoryScript => ({ + id: "script-1", + repository_id: repositoryId("repo-1"), + name: "setup", + command: "echo setup", + position: 0, + created_at: "", + updated_at: "", + ...overrides, +}); + +const repository = (overrides: Partial = {}): RepositoryWithScripts => + ({ + id: repositoryId("repo-1"), + workspace_id: "workspace-1", + name: "repo", source_type: "local", - local_path: "/tmp/my-repo", + local_path: "/tmp/repo", provider: "", provider_repo_id: "", + provider_host: "", provider_owner: "", provider_name: "", default_branch: "main", worktree_branch_prefix: "feature/", - worktree_branch_template: defaultWorktreeBranchTemplate, + worktree_branch_template: "feature/{title}-{suffix}", pull_before_worktree: true, setup_script: "", cleanup_script: "", @@ -30,92 +44,59 @@ function makeRepo(overrides: Partial = {}): RepositoryWit updated_at: "", scripts: [], ...overrides, - }; -} + }) as RepositoryWithScripts; -describe("isRepositoryDirty", () => { - it("returns false when copy_files matches", () => { - const saved = makeRepo({ copy_files: ".env, .env.local" }); - const repo = makeRepo({ copy_files: ".env, .env.local" }); - expect(isRepositoryDirty(repo, saved)).toBe(false); - }); - - it("returns true when copy_files differs", () => { - const saved = makeRepo({ copy_files: "" }); - const repo = makeRepo({ copy_files: ".env" }); - expect(isRepositoryDirty(repo, saved)).toBe(true); - }); - - it("returns true when worktree branch template differs", () => { - const saved = makeRepo({ worktree_branch_template: defaultWorktreeBranchTemplate }); - const repo = makeRepo({ worktree_branch_template: "feature/{ticket}-{title}" }); - expect(isRepositoryDirty(repo, saved)).toBe(true); - }); - - it("treats an empty worktree branch template as the default", () => { - const saved = makeRepo({ worktree_branch_template: defaultWorktreeBranchTemplate }); - const repo = makeRepo({ worktree_branch_template: "" }); - expect(isRepositoryDirty(repo, saved)).toBe(false); - }); - - it("returns true when there is no saved repository", () => { - const repo = makeRepo(); - expect(isRepositoryDirty(repo, undefined)).toBe(true); - }); - - it("treats a copy_files entry with a :symlink keyword as clean when unchanged", () => { - const saved = makeRepo({ copy_files: ".env, .env.local:symlink" }); - const repo = makeRepo({ copy_files: ".env, .env.local:symlink" }); - expect(isRepositoryDirty(repo, saved)).toBe(false); - }); - - it("returns true when a copy_files entry's keyword changes", () => { - const saved = makeRepo({ copy_files: ".env.local" }); - const repo = makeRepo({ copy_files: ".env.local:symlink" }); - expect(isRepositoryDirty(repo, saved)).toBe(true); - }); -}); - -describe("mergeSavedRepositoryDraft", () => { - it("preserves client-only row state when the submitted draft is unchanged", () => { - const current = { ...makeRepo(), __autoOpen: true }; - const saved = makeRepo({ name: "saved name" }); - - const merged = mergeSavedRepositoryDraft(current, current, saved); - - expect((merged as RepositoryWithScripts & { __autoOpen?: boolean }).__autoOpen).toBe(true); - expect(merged.name).toBe("saved name"); - }); - - it("remaps created script IDs while preserving edits made during save", () => { - const submittedScript = makeScript("temp-script-1", "submitted"); - const submitted = makeRepo({ scripts: [submittedScript] }); - const current = makeRepo({ - name: "newer repository name", - scripts: [{ ...submittedScript, command: "newer command" }], +describe("repository secret binding draft state", () => { + it("compares bindings independent of row order", () => { + const left = repository({ + secret_bindings: [ + { key: "NPM_TOKEN", secret_id: "secret-npm" }, + { key: "API_TOKEN", secret_id: "secret-api" }, + ], }); - const saved = makeRepo({ - id: toRepositoryId("persisted-repo"), - scripts: [makeScript("persisted-script", "submitted")], + const right = repository({ + secret_bindings: [ + { key: "API_TOKEN", secret_id: "secret-api" }, + { key: "NPM_TOKEN", secret_id: "secret-npm" }, + ], }); + expect(areRepositorySecretBindingsEqual(left, right)).toBe(true); + expect(isRepositoryDirty(left, right)).toBe(false); + }); + + it("deep-clones bindings so draft edits do not mutate the saved row", () => { + const source = repository({ secret_bindings: [{ key: "TOKEN", secret_id: "secret-1" }] }); + const cloned = cloneRepository(source); + + cloned.secret_bindings![0]!.key = "CHANGED"; + + expect(source.secret_bindings![0]!.key).toBe("TOKEN"); + }); + + it("preserves a locally edited binding when the save response is merged", () => { + const saved = repository({ secret_bindings: [{ key: "TOKEN", secret_id: "secret-old" }] }); + const submitted = repository({ secret_bindings: [{ key: "TOKEN", secret_id: "secret-old" }] }); + const current = repository({ secret_bindings: [{ key: "TOKEN", secret_id: "secret-new" }] }); + const merged = mergeSavedRepositoryDraft(current, submitted, saved); - expect(merged.id).toBe(toRepositoryId("persisted-repo")); - expect(merged.name).toBe("newer repository name"); - expect(merged.scripts[0].id).toBe("persisted-script"); - expect(merged.scripts[0].command).toBe("newer command"); + expect(merged.secret_bindings).toEqual([{ key: "TOKEN", secret_id: "secret-new" }]); + }); + + it("detects explicit clear as dirty", () => { + const saved = repository({ secret_bindings: [{ key: "TOKEN", secret_id: "secret-1" }] }); + const draft = repository({ secret_bindings: [] }); + + expect(isRepositoryDirty(draft, saved)).toBe(true); + }); + + it("clones scripts alongside repository bindings", () => { + const source = repository({ scripts: [script()] }); + const cloned = cloneRepository(source); + + cloned.scripts[0]!.command = "changed"; + + expect(source.scripts[0]!.command).toBe("echo setup"); }); }); - -function makeScript(id: string, command: string) { - return { - id, - repository_id: toRepositoryId("repo-1"), - name: "script", - command, - position: 0, - created_at: "", - updated_at: "", - }; -} diff --git a/apps/web/app/settings/workspace/workspace-repositories-dirty.ts b/apps/web/app/settings/workspace/workspace-repositories-dirty.ts index 71f592fb20..7b3c37f6d6 100644 --- a/apps/web/app/settings/workspace/workspace-repositories-dirty.ts +++ b/apps/web/app/settings/workspace/workspace-repositories-dirty.ts @@ -21,12 +21,32 @@ const repositoryFields: Array = [ "copy_files", ]; +function normalizedSecretBindings(repo: RepositoryWithScripts) { + return (repo.secret_bindings ?? []) + .map((binding) => ({ key: binding.key.trim(), secret_id: binding.secret_id })) + .sort((left, right) => left.key.localeCompare(right.key)); +} + +export function areRepositorySecretBindingsEqual( + left: RepositoryWithScripts, + right: RepositoryWithScripts, +): boolean { + return ( + JSON.stringify(normalizedSecretBindings(left)) === + JSON.stringify(normalizedSecretBindings(right)) + ); +} + function branchTemplate(repo: RepositoryWithScripts): string { return repo.worktree_branch_template || defaultWorktreeBranchTemplate; } export function cloneRepository(repo: RepositoryWithScripts): RepositoryWithScripts { - return { ...repo, scripts: repo.scripts.map((script) => ({ ...script })) }; + return { + ...repo, + secret_bindings: repo.secret_bindings?.map((binding) => ({ ...binding })), + scripts: repo.scripts.map((script) => ({ ...script })), + }; } export function mergeSavedRepositoryDraft( @@ -34,7 +54,11 @@ export function mergeSavedRepositoryDraft( submitted: RepositoryWithScripts, saved: RepositoryWithScripts, ): RepositoryWithScripts { - if (!isRepositoryDirty(current, submitted) && !areRepositoryScriptsDirty(current, submitted)) { + if ( + !isRepositoryDirty(current, submitted) && + !areRepositoryScriptsDirty(current, submitted) && + areRepositorySecretBindingsEqual(current, submitted) + ) { return { ...current, ...saved }; } const scripts = current.scripts.map((currentScript) => { @@ -51,7 +75,17 @@ export function mergeSavedRepositoryDraft( ? savedScript : { ...savedScript, ...currentScript, id: savedScript.id, repository_id: saved.id }; }); - return { ...saved, ...current, id: saved.id, workspace_id: saved.workspace_id, scripts }; + const secretBindings = areRepositorySecretBindingsEqual(current, submitted) + ? saved.secret_bindings + : current.secret_bindings; + return { + ...saved, + ...current, + id: saved.id, + workspace_id: saved.workspace_id, + secret_bindings: secretBindings?.map((binding) => ({ ...binding })), + scripts, + }; } export function isRepositoryDirty( @@ -61,7 +95,8 @@ export function isRepositoryDirty( if (!saved) return true; return ( repositoryFields.some((field) => repo[field] !== saved[field]) || - branchTemplate(repo) !== branchTemplate(saved) + branchTemplate(repo) !== branchTemplate(saved) || + !areRepositorySecretBindingsEqual(repo, saved) ); } diff --git a/apps/web/components/app-sidebar/sections/settings/settings-tree-render.test.tsx b/apps/web/components/app-sidebar/sections/settings/settings-tree-render.test.tsx index de5e54cae6..36c4b47c9f 100644 --- a/apps/web/components/app-sidebar/sections/settings/settings-tree-render.test.tsx +++ b/apps/web/components/app-sidebar/sections/settings/settings-tree-render.test.tsx @@ -208,6 +208,28 @@ describe("SettingsTree rendering", () => { }); }); +describe("Workspace settings order", () => { + beforeEach(() => { + state.workspaces.activeId = MAIN_WORKSPACE_ID; + state.workspaces.items = [{ id: MAIN_WORKSPACE_ID, name: MAIN_WORKSPACE_NAME }]; + }); + + afterEach(() => cleanup()); + + it("places workspace secrets below automations", () => { + render(); + + const hrefs = screen.getAllByRole("link").map((link) => link.getAttribute("href")); + const automationsIndex = hrefs.indexOf("/settings/workspace/ws-1/automations"); + const secretsIndex = hrefs.indexOf("/settings/workspace/ws-1/secrets"); + + expect(automationsIndex).toBeGreaterThanOrEqual(0); + expect(secretsIndex).toBeGreaterThan(automationsIndex); + expect(screen.getByRole("link", { name: "Secrets", exact: true })).toBeTruthy(); + expect(screen.queryByRole("link", { name: "Workspace Secrets", exact: true })).toBeNull(); + }); +}); + describe("SettingsTree integration status", () => { beforeEach(() => { state.workspaces.activeId = MAIN_WORKSPACE_ID; diff --git a/apps/web/components/app-sidebar/sections/settings/workspaces-group.tsx b/apps/web/components/app-sidebar/sections/settings/workspaces-group.tsx index 3e584f8476..51adef6eb0 100644 --- a/apps/web/components/app-sidebar/sections/settings/workspaces-group.tsx +++ b/apps/web/components/app-sidebar/sections/settings/workspaces-group.tsx @@ -12,6 +12,7 @@ import { IconFolder, IconGitBranch, IconHexagon, + IconKey, IconPlugConnected, IconTicket, } from "@tabler/icons-react"; @@ -163,6 +164,7 @@ export function WorkspacesGroup({ pathname, expanded, onToggle }: WorkspacesGrou {orderedWorkspaces.map((workspace) => { const workspacePath = `${WORKSPACES_SETTINGS_HREF}/${workspace.id}`; const repositoriesPath = `${workspacePath}/repositories`; + const secretsPath = `${workspacePath}/secrets`; const workflowsPath = `${workspacePath}/workflows`; const automationsPath = `${workspacePath}/automations`; const integrationsPath = `${workspacePath}/integrations`; @@ -215,6 +217,13 @@ export function WorkspacesGroup({ pathname, expanded, onToggle }: WorkspacesGrou isActive={pathname === automationsPath} depth={2} /> + ); })} diff --git a/apps/web/components/github/pr-detail-panel.test.ts b/apps/web/components/github/pr-detail-panel.test.ts index 39c59507e2..3df8063ea3 100644 --- a/apps/web/components/github/pr-detail-panel.test.ts +++ b/apps/web/components/github/pr-detail-panel.test.ts @@ -526,8 +526,9 @@ describe("PRDetailContent persistent request expiry", () => { }), ); + const timersBeforeRequest = vi.getTimerCount(); await act(async () => reviewRequest?.reRequest("octocat")); - expect(vi.getTimerCount()).toBe(1); + expect(vi.getTimerCount()).toBe(timersBeforeRequest + 1); view.rerender( createElement(ReviewRequestHarness, { @@ -537,7 +538,7 @@ describe("PRDetailContent persistent request expiry", () => { ); await act(async () => undefined); - expect(vi.getTimerCount()).toBe(0); + expect(vi.getTimerCount()).toBe(timersBeforeRequest); vi.useRealTimers(); }); diff --git a/apps/web/components/settings/profile-edit/env-vars-card.tsx b/apps/web/components/settings/profile-edit/env-vars-card.tsx index 617a0e1156..a8991052eb 100644 --- a/apps/web/components/settings/profile-edit/env-vars-card.tsx +++ b/apps/web/components/settings/profile-edit/env-vars-card.tsx @@ -10,6 +10,7 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@ import type { ProfileEnvVar } from "@/lib/types/http"; import { SettingsCard } from "@/components/settings/settings-card"; import { useTranslation } from "react-i18next"; +import type { SecretListItem } from "@/lib/types/http-secrets"; export type EnvVarRow = { key: string; @@ -48,11 +49,13 @@ function ValueOrSecretInput({ }: { row: EnvVarRow; index: number; - secrets: { id: string; name: string }[]; + secrets: SecretListItem[]; onUpdate: (index: number, field: keyof EnvVarRow, val: string) => void; baselineRow?: EnvVarRow; }) { const { t } = useTranslation(); + const hasMissingSecret = + Boolean(row.secretId) && !secrets.some((secret) => secret.id === row.secretId); if (row.mode === "value") { return ( + {hasMissingSecret && ( + {t("executors:missingSecretReference")} + )} {secrets.map((s) => ( {s.name} @@ -93,7 +99,7 @@ function EnvVarRowComponent({ }: { row: EnvVarRow; index: number; - secrets: { id: string; name: string }[]; + secrets: SecretListItem[]; onUpdate: (index: number, field: keyof EnvVarRow, val: string) => void; onRemove: (index: number) => void; baselineRow?: EnvVarRow; @@ -158,7 +164,7 @@ function DraftValueInput({ }: { draft: EnvVarRow; valueId: string; - secrets: { id: string; name: string }[]; + secrets: SecretListItem[]; onEnter: (e: React.KeyboardEvent) => void; setDraft: React.Dispatch>; }) { @@ -182,6 +188,9 @@ function DraftValueInput({ + {draft.secretId && !secrets.some((secret) => secret.id === draft.secretId) && ( + {t("executors:missingSecretReference")} + )} {secrets.map((s) => ( {s.name} @@ -197,7 +206,7 @@ function EnvVarAddForm({ secrets, }: { onAdd: (row: EnvVarRow) => void; - secrets: { id: string; name: string }[]; + secrets: SecretListItem[]; }) { const { t } = useTranslation(); const uid = useId(); @@ -293,7 +302,7 @@ function EnvVarAddForm({ type EnvVarsFieldProps = { rows: EnvVarRow[]; baselineRows?: EnvVarRow[]; - secrets: { id: string; name: string }[]; + secrets: SecretListItem[]; onAdd: (row: EnvVarRow) => void; onUpdate: (index: number, field: keyof EnvVarRow, val: string) => void; onRemove: (index: number) => void; diff --git a/apps/web/components/settings/profile-edit/inline-secret-select.tsx b/apps/web/components/settings/profile-edit/inline-secret-select.tsx index c6a213b2cf..58f18f83ce 100644 --- a/apps/web/components/settings/profile-edit/inline-secret-select.tsx +++ b/apps/web/components/settings/profile-edit/inline-secret-select.tsx @@ -53,6 +53,9 @@ export function InlineSecretSelect({ {t("executors:none")} + {secretId && !secrets.some((secret) => secret.id === secretId) && ( + {t("executors:missingSecretReference")} + )} {secrets.map((s) => ( {s.name} diff --git a/apps/web/components/settings/profile-edit/profile-env-vars-section.tsx b/apps/web/components/settings/profile-edit/profile-env-vars-section.tsx index 669e8df8ed..b44cbecd2d 100644 --- a/apps/web/components/settings/profile-edit/profile-env-vars-section.tsx +++ b/apps/web/components/settings/profile-edit/profile-env-vars-section.tsx @@ -3,6 +3,7 @@ import { useCallback, useState } from "react"; import { useSecrets } from "@/hooks/domains/settings/use-secrets"; import type { AgentProfile, ProfileEnvVar } from "@/lib/types/http"; +import type { SecretListItem } from "@/lib/types/http-secrets"; import { EnvVarsCard, envVarsToRows, @@ -25,7 +26,7 @@ export function areEnvVarsEqual(a?: ProfileEnvVar[], b?: ProfileEnvVar[]): boole type ProfileEnvVarsEditorProps = { envVars?: ProfileEnvVar[]; baselineEnvVars?: ProfileEnvVar[]; - secrets: { id: string; name: string }[]; + secrets: SecretListItem[]; onChange: (envVars: ProfileEnvVar[]) => void; discoveryTargetId?: string; }; @@ -108,7 +109,8 @@ export function ProfileEnvVarsSection({ onChange, discoveryTargetId, }: ProfileEnvVarsSectionProps) { - const { items: secrets } = useSecrets(); + const { items } = useSecrets(); + const secrets = items.filter((secret) => secret.scope !== "workspace"); const handleChange = useCallback( (next: ProfileEnvVar[]) => onChange({ envVars: next }), [onChange], diff --git a/apps/web/components/settings/repository-card.tsx b/apps/web/components/settings/repository-card.tsx index 37b31679a1..252668c5b9 100644 --- a/apps/web/components/settings/repository-card.tsx +++ b/apps/web/components/settings/repository-card.tsx @@ -21,6 +21,10 @@ import { RepositoryBranchTemplateHelp } from "@/components/settings/repository-b import { DeleteRepositoryDialog } from "@/components/settings/repository-delete-dialog"; import { CopyFilesField } from "@/components/settings/repository-copy-files-help"; import { RepositoryCustomScripts } from "@/components/settings/repository-custom-scripts"; +import { + RepositorySecretBindings, + validateRepositorySecretBindings, +} from "@/components/settings/repository-secret-bindings"; import { getRepositoryActiveSessionCountAction } from "@/app/actions/workspaces"; import type { Repository, RepositoryScript } from "@/lib/types/http"; import { defaultWorktreeBranchTemplate } from "@/lib/worktree-branch-template"; @@ -289,6 +293,8 @@ function RepositoryEditView({ copyFiles={repository.copy_files ?? ""} /> + + , +) { + if (!validation) return undefined; + switch (validation.kind) { + case "key": + return t("workspaces:environmentSecretKeyInvalid"); + case "duplicate": + return t("workspaces:environmentSecretKeyDuplicate", { key: validation.key }); + case "reserved": + return t("workspaces:environmentSecretKeyReserved", { key: validation.key }); + case "secret": + return t("workspaces:environmentSecretRequired"); + } +} + function RepositoryPreview({ repository, isDirty, @@ -555,6 +578,8 @@ export function RepositoryCard({ const saveRequest = useRequest(() => onSave(repository.id)); const isDirty = isRepositoryDirty || areScriptsDirty; const deleteState = useRepositoryDelete(repository.id, onDelete, () => setIsEditing(false)); + const secretBindingValidation = validateRepositorySecretBindings(repository.secret_bindings); + const invalidReason = repositorySecretInvalidReason(t, secretBindingValidation); const handleSave = async () => { try { @@ -572,6 +597,8 @@ export function RepositoryCard({ id: `repository:${repository.id}`, revision: JSON.stringify(repository), isDirty, + canSave: !secretBindingValidation, + invalidReason, save: handleSave, discard: () => undefined, }); diff --git a/apps/web/components/settings/repository-secret-bindings.test.ts b/apps/web/components/settings/repository-secret-bindings.test.ts new file mode 100644 index 0000000000..9e537c97b6 --- /dev/null +++ b/apps/web/components/settings/repository-secret-bindings.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from "vitest"; +import { validateRepositorySecretBindings } from "./repository-secret-bindings"; + +describe("validateRepositorySecretBindings", () => { + it("accepts POSIX keys with a selected secret", () => { + expect( + validateRepositorySecretBindings([{ key: "NPM_TOKEN", secret_id: "secret-1" }]), + ).toBeNull(); + }); + + it.each([ + { key: "", kind: "key" }, + { key: " TOKEN", kind: "key" }, + { key: "TOKEN ", kind: "key" }, + { key: "token-name", kind: "key" }, + { key: "TASK_DESCRIPTION", kind: "reserved" }, + { key: "KANDEV_CUSTOM", kind: "reserved" }, + ])("rejects $key as $kind", ({ key, kind }) => { + expect(validateRepositorySecretBindings([{ key, secret_id: "secret-1" }])).toMatchObject({ + kind, + }); + }); + + it("enforces the environment key length boundary", () => { + const maximumKey = "A".repeat(256); + const tooLongKey = "A".repeat(257); + + expect( + validateRepositorySecretBindings([{ key: maximumKey, secret_id: "secret-1" }]), + ).toBeNull(); + expect( + validateRepositorySecretBindings([{ key: tooLongKey, secret_id: "secret-1" }]), + ).toMatchObject({ kind: "key" }); + }); + + it("rejects duplicate keys and missing references", () => { + expect( + validateRepositorySecretBindings([ + { key: "TOKEN", secret_id: "secret-1" }, + { key: "TOKEN", secret_id: "secret-2" }, + ]), + ).toMatchObject({ kind: "duplicate", key: "TOKEN" }); + expect(validateRepositorySecretBindings([{ key: "TOKEN", secret_id: "" }])).toMatchObject({ + kind: "secret", + key: "TOKEN", + }); + }); +}); diff --git a/apps/web/components/settings/repository-secret-bindings.tsx b/apps/web/components/settings/repository-secret-bindings.tsx new file mode 100644 index 0000000000..b604144f4d --- /dev/null +++ b/apps/web/components/settings/repository-secret-bindings.tsx @@ -0,0 +1,206 @@ +"use client"; + +import { IconPlus, IconTrash } from "@tabler/icons-react"; +import type { TFunction } from "i18next"; +import { useTranslation } from "react-i18next"; +import { Button } from "@kandev/ui/button"; +import { Input } from "@kandev/ui/input"; +import { Label } from "@kandev/ui/label"; +import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@kandev/ui/select"; +import { useWorkspaceSecretOptions } from "@/hooks/domains/settings/use-workspace-secret-options"; +import type { Repository, RepositorySecretBinding } from "@/lib/types/http"; +import type { SecretListItem } from "@/lib/types/http-secrets"; + +const EMPTY_SECRET = "__empty_secret__"; +const POSIX_KEY = /^[A-Za-z_][A-Za-z0-9_]*$/; + +export type RepositorySecretBindingValidation = + | { kind: "key"; key: string } + | { kind: "duplicate"; key: string } + | { kind: "reserved"; key: string } + | { kind: "secret"; key: string } + | null; + +export function validateRepositorySecretBindings( + bindings: RepositorySecretBinding[] = [], +): RepositorySecretBindingValidation { + const seen = new Set(); + for (const binding of bindings) { + const key = binding.key.trim(); + if (!key || key !== binding.key || key.length > 256 || !POSIX_KEY.test(key)) { + return { kind: "key", key }; + } + if (key === "TASK_DESCRIPTION" || key.startsWith("KANDEV_")) { + return { kind: "reserved", key }; + } + if (seen.has(key)) return { kind: "duplicate", key }; + seen.add(key); + if (!binding.secret_id.trim()) return { kind: "secret", key }; + } + return null; +} + +function secretScopeLabel(t: TFunction, secret: SecretListItem) { + return secret.scope === "workspace" + ? t("workspaces:workspaceSecretScope") + : t("workspaces:globalSecretScope"); +} + +function secretLabel(t: TFunction, secret: SecretListItem) { + return `${secret.name} — ${secretScopeLabel(t, secret)}`; +} + +type BindingRowProps = { + binding: RepositorySecretBinding; + index: number; + options: SecretListItem[]; + onUpdate: (index: number, patch: Partial) => void; + onRemove: (index: number) => void; +}; + +function BindingRow({ binding, index, options, onUpdate, onRemove }: BindingRowProps) { + const { t } = useTranslation(); + const selected = options.find((secret) => secret.id === binding.secret_id); + const hasMissingSecret = Boolean(binding.secret_id) && !selected; + return ( +
+
+ + onUpdate(index, { key: event.target.value })} + placeholder={t("workspaces:environmentSecretKeyPlaceholder")} + className="min-h-11 font-mono text-xs" + data-testid={`repository-secret-key-${index}`} + /> +
+
+ + +
+ +
+ ); +} + +type RepositorySecretBindingsProps = { + repository: Repository; + onUpdate: (repositoryId: string, updates: Partial) => void; +}; + +export function RepositorySecretBindings({ repository, onUpdate }: RepositorySecretBindingsProps) { + const { t } = useTranslation(); + const { items, loaded } = useWorkspaceSecretOptions(repository.workspace_id); + const bindings = repository.secret_bindings ?? []; + + const updateBinding = (index: number, patch: Partial) => { + onUpdate(repository.id, { + secret_bindings: bindings.map((binding, currentIndex) => + currentIndex === index ? { ...binding, ...patch } : binding, + ), + }); + }; + + const removeBinding = (index: number) => { + onUpdate(repository.id, { + secret_bindings: bindings.filter((_, currentIndex) => currentIndex !== index), + }); + }; + + const addBinding = () => { + onUpdate(repository.id, { + secret_bindings: [...bindings, { key: "", secret_id: "" }], + }); + }; + + return ( +
+
+
+ {t("workspaces:environmentSecrets")} +
+

+ {t("workspaces:environmentSecretsDescription")} +

+
+ {bindings.length === 0 ? ( +

+ {t("workspaces:noEnvironmentSecrets")} +

+ ) : ( +
+ {bindings.map((binding, index) => ( + + ))} +
+ )} + {!loaded &&

{t("settings:loadingSecrets")}

} + {loaded && items.length === 0 && bindings.length > 0 && ( +

+ {t("workspaces:noAvailableEnvironmentSecrets")} +

+ )} + +
+ ); +} diff --git a/apps/web/components/settings/secrets-settings.tsx b/apps/web/components/settings/secrets-settings.tsx index b7230d74d0..ea6ade8e19 100644 --- a/apps/web/components/settings/secrets-settings.tsx +++ b/apps/web/components/settings/secrets-settings.tsx @@ -2,8 +2,10 @@ import { useCallback, useMemo, useState } from "react"; import { Trans, useTranslation } from "react-i18next"; +import type { TFunction } from "i18next"; import { IconEdit, IconTrash, IconEye, IconEyeOff, IconKey } from "@tabler/icons-react"; import { Button } from "@kandev/ui/button"; +import { Badge } from "@kandev/ui/badge"; import { Dialog, DialogContent, @@ -16,7 +18,6 @@ import { Input } from "@kandev/ui/input"; import { Textarea } from "@kandev/ui/textarea"; import { SettingsPageTemplate } from "@/components/settings/settings-page-template"; import { useSecrets } from "@/hooks/domains/settings/use-secrets"; -import { useAppStore } from "@/components/state-provider"; import { createSecret, updateSecret, @@ -24,7 +25,7 @@ import { revealSecret, } from "@/lib/api/domains/secrets-api"; import { useRequest } from "@/lib/http/use-request"; -import type { SecretListItem } from "@/lib/types/http-secrets"; +import type { SecretListItem, SecretScope, UpdateSecretRequest } from "@/lib/types/http-secrets"; export type SecretFormState = { name: string; @@ -112,6 +113,7 @@ function SecretForm({ type SecretListItemRowProps = { secret: SecretListItem; + workspaceId?: string; onEdit: (secret: SecretListItem) => void; onDelete: (secret: SecretListItem) => void; isBusy: boolean; @@ -121,12 +123,14 @@ type SecretListItemRowProps = { function SecretListItemRow({ secret, + workspaceId, onEdit, onDelete, isBusy, showCreate, isEditing, }: SecretListItemRowProps) { + const { t } = useTranslation(); const [revealed, setRevealed] = useState(false); const [revealedValue, setRevealedValue] = useState(null); const [revealing, setRevealing] = useState(false); @@ -139,7 +143,10 @@ function SecretListItemRow({ } setRevealing(true); try { - const resp = await revealSecret(secret.id, { cache: "no-store" }); + const resp = await revealSecret(secret.id, { + cache: "no-store", + ...(workspaceId ? { workspaceId } : {}), + }); setRevealedValue(resp.value); setRevealed(true); } catch { @@ -155,6 +162,11 @@ function SecretListItemRow({
{secret.name}
+ + {secret.scope === "workspace" + ? t("settings:workspaceScope") + : t("settings:globalScope")} +
@@ -171,7 +188,8 @@ function SecretListItemRow({ size="icon" onClick={() => onEdit(secret)} disabled={isBusy || showCreate || isEditing} - className="cursor-pointer" + className="min-h-11 min-w-11 cursor-pointer" + aria-label={t("settings:editSecretNamed", { name: secret.name })} > @@ -180,7 +198,8 @@ function SecretListItemRow({ size="icon" onClick={() => onDelete(secret)} disabled={isBusy} - className="cursor-pointer" + className="min-h-11 min-w-11 cursor-pointer" + aria-label={t("settings:deleteSecretNamed", { name: secret.name })} > @@ -257,12 +276,16 @@ export function DeleteSecretDialog({ /* State + actions hooks */ /* ------------------------------------------------------------------ */ -function useSecretsState() { - const { loaded } = useSecrets(); - const items = useAppStore((s) => s.secrets.items); - const addSecret = useAppStore((s) => s.addSecret); - const updateSecretInStore = useAppStore((s) => s.updateSecret); - const removeSecret = useAppStore((s) => s.removeSecret); +function useSecretsState( + scope: SecretScope, + workspaceId?: string, + initialItems?: SecretListItem[], +) { + const { loaded, items, addSecret, updateSecret, removeSecret } = useSecrets( + scope, + workspaceId, + initialItems, + ); const [editingId, setEditingId] = useState(null); const [showCreate, setShowCreate] = useState(false); @@ -273,8 +296,10 @@ function useSecretsState() { loaded, items, addSecret, - updateSecretInStore, + updateSecretInStore: updateSecret, removeSecret, + scope, + workspaceId, editingId, setEditingId, showCreate, @@ -298,8 +323,15 @@ function useSecretsActions(state: ReturnType) { setDeleteTarget, deleteTarget, formState, + scope, + workspaceId, } = state; + const requestOptions = { + cache: "no-store" as const, + ...(scope === "workspace" && workspaceId ? { workspaceId } : {}), + }; + const resetForm = useCallback(() => { setEditingId(null); setShowCreate(false); @@ -313,23 +345,32 @@ function useSecretsActions(state: ReturnType) { }, [formState, editingId]); const createRequest = useRequest(async (s: SecretFormState) => { - const item = await createSecret({ name: s.name.trim(), value: s.value }, { cache: "no-store" }); + const item = await createSecret( + { + name: s.name.trim(), + value: s.value, + ...(scope === "workspace" && workspaceId + ? { scope: "workspace" as const, workspace_id: workspaceId } + : { scope: "global" as const }), + }, + requestOptions, + ); addToStore(item); resetForm(); }); const updateRequest = useRequest(async (id: string, s: SecretFormState) => { - const payload: Record = { + const payload: UpdateSecretRequest = { name: s.name.trim(), }; if (s.value.trim()) payload.value = s.value; - const item = await updateSecret(id, payload, { cache: "no-store" }); + const item = await updateSecret(id, payload, requestOptions); updateSecretInStore(item); resetForm(); }); const deleteRequest = useRequest(async (id: string) => { - await deleteSecret(id, { cache: "no-store" }); + await deleteSecret(id, requestOptions); removeFromStore(id); if (editingId === id) resetForm(); }); @@ -405,39 +446,60 @@ export function getSecretDraftMeta( }; } -export function SecretsSettings() { +type SecretsSettingsProps = { + scope?: SecretScope; + workspaceId?: string; + initialItems?: SecretListItem[]; +}; + +type SecretsSettingsState = ReturnType; +type SecretsSettingsActions = ReturnType; + +function secretScopeTitle(t: TFunction) { + return t("settings:secrets"); +} + +function secretScopeDescription(t: TFunction, scope: SecretScope) { + return scope === "workspace" + ? t("settings:workspaceSecretsDescription") + : t("settings:manageApiKeysAndCredentialsSecrets"); +} + +function secretDraftInvalidReason( + t: TFunction, + showCreate: boolean, + isDirty: boolean, + isValid: boolean, +) { + if (!isDirty || isValid) return undefined; + return showCreate + ? t("settings:aSecretNameAndValueAreRequired") + : t("settings:aSecretNameIsRequired"); +} + +type SecretsSettingsBodyProps = { + workspaceId?: string; + state: SecretsSettingsState; + actions: SecretsSettingsActions; + edit: ReturnType; + onFormChange: (patch: Partial) => void; +}; + +function SecretsSettingsBody({ + workspaceId, + state, + actions, + edit, + onFormChange, +}: SecretsSettingsBodyProps) { const { t } = useTranslation(); - const state = useSecretsState(); - const { loaded, editingId, showCreate, formState, setFormState, deleteTarget, items } = state; - const actions = useSecretsActions(state); + const { loaded, editingId, showCreate, formState, deleteTarget, items } = state; const { isValid, isBusy } = actions; - const { edit, isDirty, revision } = getSecretDraftMeta(items, editingId, showCreate, formState); - let invalidReason: string | undefined; - if (isDirty && !isValid) { - invalidReason = showCreate - ? t("settings:aSecretNameAndValueAreRequired") - : t("settings:aSecretNameIsRequired"); - } - - const onFormChange = (patch: Partial) => - setFormState((prev) => ({ ...prev, ...patch })); - return ( - + <>
-
{t("settings:secrets")}
+
{secretScopeTitle(t)}