diff --git a/cmd/context/create.go b/cmd/context/create.go index 91b891e5c4..3daa79f8b1 100644 --- a/cmd/context/create.go +++ b/cmd/context/create.go @@ -46,36 +46,27 @@ func NewCreateCmd(flags *flags.GlobalFlags) *cobra.Command { // Run runs the command logic. func (cmd *CreateCmd) Run(ctx context.Context, context string) error { - devsyConfig, err := config.LoadConfig("", cmd.Provider) - if err != nil { - return err - } else if devsyConfig.Contexts[context] != nil { - return fmt.Errorf("context %q already exists", context) - } - - // verify name - if provider2.ProviderNameRegEx.MatchString(context) { - return fmt.Errorf("context name can only include lower case letters, numbers or dashes") - } else if len(context) > 48 { - return fmt.Errorf("context name cannot be longer than 48 characters") - } - devsyConfig.Contexts[context] = &config.ContextConfig{} + return config.UpdateConfig("", cmd.Provider, func(devsyConfig *config.Config) error { + if devsyConfig.Contexts[context] != nil { + return fmt.Errorf("context %q already exists", context) + } - // check if there are create options set - if len(cmd.Options) > 0 { - err = setOptions(devsyConfig, context, cmd.Options) - if err != nil { - return err + if provider2.ProviderNameRegEx.MatchString(context) { + return fmt.Errorf("context name can only include lower case letters, numbers or dashes") + } else if len(context) > 48 { + return fmt.Errorf("context name cannot be longer than 48 characters") } - } + devsyConfig.Contexts[context] = &config.ContextConfig{} - devsyConfig.DefaultContext = context - err = config.SaveConfig(devsyConfig) - if err != nil { - return fmt.Errorf("save config: %w", err) - } + if len(cmd.Options) > 0 { + if err := setOptions(devsyConfig, context, cmd.Options); err != nil { + return err + } + } - return nil + devsyConfig.DefaultContext = context + return nil + }) } func setOptions(devsyConfig *config.Config, context string, options []string) error { diff --git a/cmd/context/delete.go b/cmd/context/delete.go index d622e50d34..b5ec4d0ad8 100644 --- a/cmd/context/delete.go +++ b/cmd/context/delete.go @@ -44,31 +44,27 @@ func NewDeleteCmd(flags *flags.GlobalFlags) *cobra.Command { // Run runs the command logic. func (cmd *DeleteCmd) Run(ctx context.Context, context string) error { - devsyConfig, err := config.LoadConfig(context, cmd.Provider) - if err != nil { - return err - } - - if context == "" { - context = devsyConfig.DefaultContext - } else if devsyConfig.Contexts[context] == nil { - return fmt.Errorf("context %q doesn't exist", context) - } - - if context == "default" { - return fmt.Errorf("cannot delete 'default' context") - } + err := config.UpdateConfig(context, cmd.Provider, func(devsyConfig *config.Config) error { + if context == "" { + context = devsyConfig.DefaultContext + } else if devsyConfig.Contexts[context] == nil { + return fmt.Errorf("context %q doesn't exist", context) + } - if err := deleteContextSecrets(devsyConfig, context); err != nil { - return err - } + if context == config.DefaultContext { + return fmt.Errorf("cannot delete 'default' context") + } - delete(devsyConfig.Contexts, context) - resetContextReferences(devsyConfig, context) + if err := deleteContextSecrets(devsyConfig, context); err != nil { + return err + } - err = config.SaveConfig(devsyConfig) + delete(devsyConfig.Contexts, context) + resetContextReferences(devsyConfig, context) + return nil + }) if err != nil { - return fmt.Errorf("save config: %w", err) + return err } return removeContextDir(context) @@ -91,10 +87,10 @@ func removeContextDir(contextName string) error { func resetContextReferences(devsyConfig *config.Config, context string) { if devsyConfig.DefaultContext == context { - devsyConfig.DefaultContext = "default" + devsyConfig.DefaultContext = config.DefaultContext } if devsyConfig.OriginalContext == context { - devsyConfig.OriginalContext = "default" + devsyConfig.OriginalContext = config.DefaultContext } } diff --git a/cmd/context/set_options.go b/cmd/context/set_options.go index 18e8bf3e07..c94040b30d 100644 --- a/cmd/context/set_options.go +++ b/cmd/context/set_options.go @@ -50,30 +50,19 @@ func NewSetOptionsCmd(flags *flags.GlobalFlags) *cobra.Command { // Run runs the command logic. func (cmd *SetOptionsCmd) Run(ctx context.Context, context string) error { - devsyConfig, err := config.LoadConfig("", cmd.Provider) - if err != nil { - return err - } - - // check for context - if context == "" { - context = devsyConfig.DefaultContext - } else if devsyConfig.Contexts[context] == nil { - return fmt.Errorf("context %q doesn't exist", context) - } - - // check if there are setOptions options set - if len(cmd.Options) > 0 { - err = setOptions(devsyConfig, context, cmd.Options) - if err != nil { - return err + return config.UpdateConfig("", cmd.Provider, func(devsyConfig *config.Config) error { + if context == "" { + context = devsyConfig.DefaultContext + } else if devsyConfig.Contexts[context] == nil { + return fmt.Errorf("context %q doesn't exist", context) } - } - err = config.SaveConfig(devsyConfig) - if err != nil { - return fmt.Errorf("save config: %w", err) - } + if len(cmd.Options) > 0 { + if err := setOptions(devsyConfig, context, cmd.Options); err != nil { + return err + } + } - return nil + return nil + }) } diff --git a/cmd/context/use.go b/cmd/context/use.go index 19971c71bf..33af291b3a 100644 --- a/cmd/context/use.go +++ b/cmd/context/use.go @@ -45,26 +45,18 @@ func NewUseCmd(flags *flags.GlobalFlags) *cobra.Command { // Run runs the command logic. func (cmd *UseCmd) Run(ctx context.Context, context string) error { - devsyConfig, err := config.LoadConfig("", cmd.Provider) - if err != nil { - return err - } else if devsyConfig.Contexts[context] == nil { - return fmt.Errorf("context %q doesn't exist", context) - } - - // check if there are use options set - if len(cmd.Options) > 0 { - err = setOptions(devsyConfig, context, cmd.Options) - if err != nil { - return err + return config.UpdateConfig("", cmd.Provider, func(devsyConfig *config.Config) error { + if devsyConfig.Contexts[context] == nil { + return fmt.Errorf("context %q doesn't exist", context) } - } - devsyConfig.DefaultContext = context - err = config.SaveConfig(devsyConfig) - if err != nil { - return fmt.Errorf("save config: %w", err) - } + if len(cmd.Options) > 0 { + if err := setOptions(devsyConfig, context, cmd.Options); err != nil { + return err + } + } - return nil + devsyConfig.DefaultContext = context + return nil + }) } diff --git a/cmd/ide/set.go b/cmd/ide/set.go index b9e563f77a..7d1ed02aa5 100644 --- a/cmd/ide/set.go +++ b/cmd/ide/set.go @@ -55,23 +55,13 @@ with 'devsy ide list'.`, // Run runs the command logic. func (cmd *SetCmd) Run(_ context.Context, ideName string) error { - devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) - if err != nil { - return err - } - ideName = strings.ToLower(ideName) ideOptions, err := ideparse.GetIDEOptions(ideName) if err != nil { return err } - if err := setOptions(devsyConfig, ideName, cmd.Options, ideOptions); err != nil { - return err - } - - if err := config.SaveConfig(devsyConfig); err != nil { - return fmt.Errorf("save config: %w", err) - } - return nil + return config.UpdateConfig(cmd.Context, cmd.Provider, func(devsyConfig *config.Config) error { + return setOptions(devsyConfig, ideName, cmd.Options, ideOptions) + }) } diff --git a/cmd/ide/use.go b/cmd/ide/use.go index 01d261b894..f83b8c76c1 100644 --- a/cmd/ide/use.go +++ b/cmd/ide/use.go @@ -2,7 +2,6 @@ package ide import ( "context" - "fmt" "maps" "strings" @@ -52,29 +51,24 @@ Available IDEs can be listed with 'devsy ide list'`, // Run runs the command logic. func (cmd *UseCmd) Run(ctx context.Context, ide string) error { - devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) - if err != nil { - return err - } - ide = strings.ToLower(ide) ideOptions, err := ideparse.GetIDEOptions(ide) if err != nil { return err } - // check if there are user options set - if len(cmd.Options) > 0 { - err = setOptions(devsyConfig, ide, cmd.Options, ideOptions) - if err != nil { - return err + err = config.UpdateConfig(cmd.Context, cmd.Provider, func(devsyConfig *config.Config) error { + if len(cmd.Options) > 0 { + if err := setOptions(devsyConfig, ide, cmd.Options, ideOptions); err != nil { + return err + } } - } - devsyConfig.Current().DefaultIDE = ide - err = config.SaveConfig(devsyConfig) + devsyConfig.Current().DefaultIDE = ide + return nil + }) if err != nil { - return fmt.Errorf("save config: %w", err) + return err } log.Infof("default IDE set to %q", ide) diff --git a/cmd/mcp/tools_provider.go b/cmd/mcp/tools_provider.go index 363b533f2d..9198bf93b4 100644 --- a/cmd/mcp/tools_provider.go +++ b/cmd/mcp/tools_provider.go @@ -142,6 +142,11 @@ func runProviderAdd(ctx context.Context, g *flags.GlobalFlags, in providerAddInp } func runProviderDelete(ctx context.Context, g *flags.GlobalFlags, name string) error { + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() devsyConfig, err := config.LoadConfig(g.Context, g.Provider) if err != nil { return err @@ -150,9 +155,5 @@ func runProviderDelete(ctx context.Context, g *flags.GlobalFlags, name string) e } func runProviderUse(_ context.Context, g *flags.GlobalFlags, name string) error { - devsyConfig, err := config.LoadConfig(g.Context, g.Provider) - if err != nil { - return err - } - return cmdprovider.UseProvider(devsyConfig, name) + return cmdprovider.UseProvider(g.Context, g.Provider, name) } diff --git a/cmd/pro/login.go b/cmd/pro/login.go index b176d518c5..4fdd9d05b7 100644 --- a/cmd/pro/login.go +++ b/cmd/pro/login.go @@ -108,17 +108,29 @@ func (cmd *LoginCmd) Run(ctx context.Context, fullURL string) error { return err } - devsyConfig, currentInstance, err := cmd.resolveInstance(fullURL) + devsyConfig, err := cmd.prepareProvider(ctx, fullURL) if err != nil { return err } - devsyConfig, err = cmd.ensureProvider(ctx, devsyConfig, currentInstance, fullURL) + return cmd.loginAndConfigure(ctx, devsyConfig, fullURL) +} + +// prepareProvider applies the login-related config changes under the config +// lock; the interactive browser login itself runs unlocked. +func (cmd *LoginCmd) prepareProvider(ctx context.Context, fullURL string) (*config.Config, error) { + unlock, err := config.LockConfig() if err != nil { - return err + return nil, err } + defer unlock() - return cmd.loginAndConfigure(ctx, devsyConfig, fullURL) + devsyConfig, currentInstance, err := cmd.resolveInstance(fullURL) + if err != nil { + return nil, err + } + + return cmd.ensureProvider(ctx, devsyConfig, currentInstance, fullURL) } func (cmd *LoginCmd) normalizeURL(fullURL string) (string, error) { @@ -279,8 +291,14 @@ func (cmd *LoginCmd) loginAndConfigure( } if cmd.Use { + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + // Post-login: preserve user values; resolver prunes anything stale. - err := providercmd.ConfigureProvider(ctx, providercmd.ProviderOptionsConfig{ + err = providercmd.ConfigureProvider(ctx, providercmd.ProviderOptionsConfig{ Provider: providerConfig, ContextName: devsyConfig.DefaultContext, UserOptions: cmd.Options, diff --git a/cmd/pro/logout.go b/cmd/pro/logout.go index c9dfd6f050..505e8a35cf 100644 --- a/cmd/pro/logout.go +++ b/cmd/pro/logout.go @@ -118,8 +118,17 @@ func (cmd *LogoutCmd) Run(ctx context.Context, args []string) error { } } - // delete the provider config - err = providercmd.DeleteProviderConfig(devsyConfig, proInstanceConfig.Provider, true) + // delete the provider config, reloading it under the config lock: the + // earlier load in this flow predates the daemon shutdown above + unlock, err := config.LockConfig() + if err != nil { + return err + } + freshConfig, err := config.LoadConfig(devsyConfig.DefaultContext, "") + if err == nil { + err = providercmd.DeleteProviderConfig(freshConfig, proInstanceConfig.Provider, true) + } + unlock() if err != nil { return err } diff --git a/cmd/pro/update_provider.go b/cmd/pro/update_provider.go index e7a77e5d97..8694589482 100644 --- a/cmd/pro/update_provider.go +++ b/cmd/pro/update_provider.go @@ -49,6 +49,12 @@ func (cmd *UpdateProviderCmd) Run(ctx context.Context, args []string) error { } newVersion := args[0] + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/cmd/provider/add.go b/cmd/provider/add.go index 11d15105ad..5d1e485552 100644 --- a/cmd/provider/add.go +++ b/cmd/provider/add.go @@ -48,6 +48,12 @@ func NewAddCmd(f *flags.GlobalFlags) *cobra.Command { }, RunE: func(cobraCmd *cobra.Command, args []string) error { ctx := cobraCmd.Context() + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/cmd/provider/delete.go b/cmd/provider/delete.go index 8323c3af9b..4e9da10689 100644 --- a/cmd/provider/delete.go +++ b/cmd/provider/delete.go @@ -61,6 +61,12 @@ func NewDeleteCmd(flags *flags.GlobalFlags) *cobra.Command { } func (cmd *DeleteCmd) Run(ctx context.Context, args []string) error { + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/cmd/provider/init.go b/cmd/provider/init.go index d7c33c83ee..731df238d1 100644 --- a/cmd/provider/init.go +++ b/cmd/provider/init.go @@ -29,6 +29,12 @@ func NewInitCmd(f *flags.GlobalFlags) *cobra.Command { Short: "Run or re-run init and option resolution for an existing provider", Args: cobra.MaximumNArgs(1), RunE: func(cobraCmd *cobra.Command, args []string) error { + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/cmd/provider/rename.go b/cmd/provider/rename.go index b8b867df6c..58235aee83 100644 --- a/cmd/provider/rename.go +++ b/cmd/provider/rename.go @@ -48,6 +48,14 @@ func (cmd *RenameCmd) Run(ctx context.Context, args []string) error { return err } + // The rename rewrites provider, workspace, and machine state across several + // config saves with rollback, so the whole transaction runs under the lock. + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/cmd/provider/set.go b/cmd/provider/set.go index d658b7051d..6eb67dc136 100644 --- a/cmd/provider/set.go +++ b/cmd/provider/set.go @@ -63,6 +63,12 @@ func NewSetCmd(f *flags.GlobalFlags) *cobra.Command { } func (cmd *SetCmd) Run(ctx context.Context, args []string) error { + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, providerWithOptions, err := cmd.loadProvider(args) if err != nil { return err diff --git a/cmd/provider/set_source.go b/cmd/provider/set_source.go index 7b9dbc140f..e1caa348bb 100644 --- a/cmd/provider/set_source.go +++ b/cmd/provider/set_source.go @@ -35,6 +35,12 @@ func NewSetSourceCmd(flags *flags.GlobalFlags) *cobra.Command { Short: "Set or change a provider's source (replaces the registered name, repo, URL, or path)", RunE: func(cobraCmd *cobra.Command, args []string) error { ctx := cobraCmd.Context() + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/cmd/provider/use.go b/cmd/provider/use.go index 67903547bf..19e38e0455 100644 --- a/cmd/provider/use.go +++ b/cmd/provider/use.go @@ -1,8 +1,6 @@ package provider import ( - "fmt" - "github.com/devsy-org/devsy/cmd/completion" "github.com/devsy-org/devsy/cmd/flags" "github.com/devsy-org/devsy/pkg/config" @@ -11,17 +9,27 @@ import ( "github.com/spf13/cobra" ) -// UseProvider sets the named provider as the default for the active config context. -func UseProvider(devsyConfig *config.Config, name string) error { - p, err := workspace.FindProvider(devsyConfig, name) +// UseProvider sets the named provider as the default for the addressed config +// context, loading and saving config.yaml under the config lock. +func UseProvider(contextOverride, providerOverride, name string) error { + var resolved string + err := config.UpdateConfig( + contextOverride, + providerOverride, + func(devsyConfig *config.Config) error { + p, err := workspace.FindProvider(devsyConfig, name) + if err != nil { + return err + } + devsyConfig.Current().DefaultProvider = p.Config.Name + resolved = p.Config.Name + return nil + }, + ) if err != nil { return err } - devsyConfig.Current().DefaultProvider = p.Config.Name - if err := config.SaveConfig(devsyConfig); err != nil { - return fmt.Errorf("save config: %w", err) - } - log.Infof("default provider: %s", p.Config.Name) + log.Infof("default provider: %s", resolved) return nil } @@ -38,20 +46,7 @@ func NewUseCmd(f *flags.GlobalFlags) *cobra.Command { Short: "Set the default provider for the active context", Args: cobra.ExactArgs(1), RunE: func(cobraCmd *cobra.Command, args []string) error { - devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) - if err != nil { - return err - } - p, err := workspace.FindProvider(devsyConfig, args[0]) - if err != nil { - return err - } - devsyConfig.Current().DefaultProvider = p.Config.Name - if err := config.SaveConfig(devsyConfig); err != nil { - return fmt.Errorf("save config: %w", err) - } - log.Infof("default provider: %s", p.Config.Name) - return nil + return UseProvider(cmd.Context, cmd.Provider, args[0]) }, ValidArgsFunction: func( rootCmd *cobra.Command, diff --git a/cmd/secrets/list.go b/cmd/secrets/list.go index 0d4bb71961..ad80dabb22 100644 --- a/cmd/secrets/list.go +++ b/cmd/secrets/list.go @@ -4,9 +4,11 @@ import ( "context" "encoding/json" "fmt" + "slices" "time" "github.com/devsy-org/devsy/cmd/flags" + "github.com/devsy-org/devsy/pkg/config" "github.com/devsy-org/devsy/pkg/output" "github.com/devsy-org/devsy/pkg/secrets" "github.com/devsy-org/devsy/pkg/table" @@ -41,23 +43,22 @@ type secretEntry struct { Created string `json:"created,omitempty"` LastUsed string `json:"lastUsed,omitempty"` Orphaned bool `json:"orphaned,omitempty"` + Attached bool `json:"attached"` } func (cmd *ListCmd) Run(_ context.Context) error { - contextName, store, err := resolveContext(cmd.GlobalFlags) + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err } - - all, err := store.List(contextName) + contextName := devsyConfig.DefaultContext + store, err := secrets.NewStoreForConfig(devsyConfig) if err != nil { return err } - metas := make([]secrets.SecretMeta, 0, len(all)) - for _, m := range all { - if m.Sensitive() { - metas = append(metas, m) - } + entries, err := listEntries(devsyConfig, store, contextName) + if err != nil { + return err } mode, err := output.ResolveMode(cmd.ResultFormat) @@ -67,30 +68,32 @@ func (cmd *ListCmd) Run(_ context.Context) error { switch mode { case output.ModePlain: - renderPlain(metas) + renderPlain(entries) case output.ModeJSON: - return renderJSON(metas) + return renderJSON(entries) } return nil } -func renderPlain(metas []secrets.SecretMeta) { - tableEntries := [][]string{} - for _, m := range metas { - tableEntries = append(tableEntries, []string{ - m.Name, - formatTime(m.Created), - formatTime(m.LastUsed), - orphanLabel(m.Orphaned), - }) +func listEntries( + devsyConfig *config.Config, + store secrets.Store, + contextName string, +) ([]secretEntry, error) { + all, err := store.List(contextName) + if err != nil { + return nil, err } - table.Print([]string{"Name", "Created", "Last Used", "Status"}, tableEntries) -} - -func renderJSON(metas []secrets.SecretMeta) error { - entries := []secretEntry{} - for _, m := range metas { + var attachedNames []string + if ctxConfig := devsyConfig.Contexts[contextName]; ctxConfig != nil { + attachedNames = ctxConfig.Secrets + } + entries := make([]secretEntry, 0, len(all)) + for _, m := range all { + if !m.Sensitive() { + continue + } entries = append(entries, secretEntry{ Name: m.Name, Context: m.Context, @@ -98,8 +101,27 @@ func renderJSON(metas []secrets.SecretMeta) error { Created: formatTime(m.Created), LastUsed: formatTime(m.LastUsed), Orphaned: m.Orphaned, + Attached: slices.Contains(attachedNames, m.Name), + }) + } + return entries, nil +} + +func renderPlain(entries []secretEntry) { + tableEntries := [][]string{} + for _, e := range entries { + tableEntries = append(tableEntries, []string{ + e.Name, + e.Created, + e.LastUsed, + orphanLabel(e.Orphaned), + fmt.Sprint(e.Attached), }) } + table.Print([]string{"Name", "Created", "Last Used", "Status", "Attached"}, tableEntries) +} + +func renderJSON(entries []secretEntry) error { out, err := json.MarshalIndent(entries, "", " ") if err != nil { return err diff --git a/cmd/secrets/list_test.go b/cmd/secrets/list_test.go new file mode 100644 index 0000000000..266e0f4dc6 --- /dev/null +++ b/cmd/secrets/list_test.go @@ -0,0 +1,66 @@ +package secrets + +import ( + "testing" + "time" + + "github.com/devsy-org/devsy/pkg/config" + devsysecrets "github.com/devsy-org/devsy/pkg/secrets" + "github.com/stretchr/testify/suite" +) + +type ListTestSuite struct { + suite.Suite +} + +func TestListSuite(t *testing.T) { + suite.Run(t, new(ListTestSuite)) +} + +type listTestStore struct { + metas []devsysecrets.SecretMeta +} + +func (s *listTestStore) Set(string, string, string, devsysecrets.Kind) error { return nil } +func (s *listTestStore) Get(string, string) (string, error) { return "", nil } +func (s *listTestStore) Meta(string, string) (devsysecrets.SecretMeta, error) { + return devsysecrets.SecretMeta{}, nil +} +func (s *listTestStore) List(string) ([]devsysecrets.SecretMeta, error) { return s.metas, nil } +func (s *listTestStore) Delete(string, string) error { return nil } + +func (s *ListTestSuite) TestListEntriesMarksAttachedSecrets() { + created := time.Date(2026, 9, 24, 12, 0, 0, 0, time.UTC) + store := &listTestStore{metas: []devsysecrets.SecretMeta{ + { + Name: "ATTACHED", + Context: config.DefaultContext, + Kind: devsysecrets.KindSecret, + Created: created, + }, + {Name: "DETACHED", Context: config.DefaultContext, Kind: devsysecrets.KindSecret}, + {Name: "ENV_VAR", Context: config.DefaultContext, Kind: devsysecrets.KindEnv}, + }} + cfg := deleteTestConfig([]string{"ATTACHED", "sops:project/API_TOKEN"}) + + entries, err := listEntries(cfg, store, config.DefaultContext) + s.Require().NoError(err) + s.Require().Len(entries, 2) + s.Require().Equal("ATTACHED", entries[0].Name) + s.Require().True(entries[0].Attached) + s.Require().Equal(created.Format(time.RFC3339), entries[0].Created) + s.Require().Equal("DETACHED", entries[1].Name) + s.Require().False(entries[1].Attached) +} + +func (s *ListTestSuite) TestListEntriesDetachedWhenContextHasNoBindings() { + store := &listTestStore{metas: []devsysecrets.SecretMeta{ + {Name: "TOKEN", Context: config.DefaultContext, Kind: devsysecrets.KindSecret}, + }} + cfg := deleteTestConfig(nil) + + entries, err := listEntries(cfg, store, config.DefaultContext) + s.Require().NoError(err) + s.Require().Len(entries, 1) + s.Require().False(entries[0].Attached) +} diff --git a/cmd/workspace/import.go b/cmd/workspace/import.go index cd3c7f7bb1..bce4b5b2de 100644 --- a/cmd/workspace/import.go +++ b/cmd/workspace/import.go @@ -109,6 +109,12 @@ func (cmd *ImportCmd) execute(ctx context.Context) error { if parseErr != nil { return parseErr } + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := config.LoadConfig(cmd.Context, cmd.Provider) if err != nil { return err diff --git a/desktop/e2e/workspaces.e2e.ts b/desktop/e2e/workspaces.e2e.ts index 6220ffde2f..47cee6408f 100644 --- a/desktop/e2e/workspaces.e2e.ts +++ b/desktop/e2e/workspaces.e2e.ts @@ -99,7 +99,7 @@ test.describe("Workspace lifecycle badges", () => { await expect(main).toContainText("Deleting", { timeout: 3000 }) await expect(main.locator("text=deleteprobe")).not.toBeVisible({ - timeout: 5000, + timeout: 10000, }) await waitForDeleteToSettle() } catch (error) { diff --git a/desktop/src/main/__tests__/ipc-provider-jobs.test.ts b/desktop/src/main/__tests__/ipc-provider-jobs.test.ts index d1b49ef840..82d22071f0 100644 --- a/desktop/src/main/__tests__/ipc-provider-jobs.test.ts +++ b/desktop/src/main/__tests__/ipc-provider-jobs.test.ts @@ -90,7 +90,13 @@ function invoke(channel: string, args: Record) { } function statusLine(phase: string) { - return JSON.stringify({ kind: "status", schemaVersion: 1, pipeline: "provider", phase, state: "started" }) + return JSON.stringify({ + kind: "status", + schemaVersion: 1, + pipeline: "provider", + phase, + state: "started", + }) } describe("provider job lifecycle over IPC", () => { @@ -99,6 +105,40 @@ describe("provider job lifecycle over IPC", () => { vi.clearAllMocks() }) + it("forwards managed secret attachment intent to the CLI", async () => { + const { cli } = setup() + await invoke("secret_attach", { name: "DB_PASSWORD", context: "staging" }) + await invoke("secret_detach", { name: "DB_PASSWORD", context: "staging" }) + expect(cli.runRaw).toHaveBeenCalledWith([ + "--context", + "staging", + "secret", + "attach", + "DB_PASSWORD", + ]) + expect(cli.runRaw).toHaveBeenCalledWith([ + "--context", + "staging", + "secret", + "detach", + "DB_PASSWORD", + ]) + }) + + it("rejects managed secret attachment without a context", async () => { + const { cli } = setup() + const result = (await invoke("secret_attach", { + name: "DB_PASSWORD", + context: "", + })) as { ok: boolean; message: string; cliError?: unknown } + expect(result).toEqual({ + ok: false, + message: "context is required", + cliError: undefined, + }) + expect(cli.runRaw).not.toHaveBeenCalled() + }) + it("forwards managed environment attachment intent to the CLI", async () => { const { cli } = setup() await invoke("env_attach", { name: "LOG_LEVEL", context: "staging" }) @@ -225,15 +265,21 @@ describe("provider job lifecycle over IPC", () => { expect(providerJobs.get("docker")?.error).toBeTruthy() }) - it("streams update phases and completes the provider job", async () => { const seen: string[][] = [] const { providerJobs, send } = setup((cliArgs) => { seen.push(cliArgs) - return { lines: [statusLine(cliArgs[1] === "init" ? "running_init" : "downloading")], code: 0 } + return { + lines: [ + statusLine(cliArgs[1] === "init" ? "running_init" : "downloading"), + ], + code: 0, + } }) - const commandId = await invoke("provider_update_streaming", { name: "docker" }) + const commandId = await invoke("provider_update_streaming", { + name: "docker", + }) expect(commandId).toEqual(expect.any(String)) expect(providerJobs.get("docker")?.activity).toBe("updating") @@ -252,18 +298,23 @@ describe("provider job lifecycle over IPC", () => { })) await invoke("provider_update_streaming", { name: "docker" }) - await vi.waitFor(() => expect(providerJobs.get("docker")?.error).toBeTruthy()) + await vi.waitFor(() => + expect(providerJobs.get("docker")?.error).toBeTruthy(), + ) }) - it("terminates a streaming update when provider refresh fails", async () => { const { providerJobs, send } = setup(() => ({ lines: [], code: 0 })) providerJobs.setRefresh(() => Promise.reject(new Error("refresh boom"))) - const commandId = await invoke("provider_update_streaming", { name: "docker" }) + const commandId = await invoke("provider_update_streaming", { + name: "docker", + }) await vi.waitFor(() => - expect(providerJobs.get("docker")?.errorCode).toBe("provider_refresh_failed"), + expect(providerJobs.get("docker")?.errorCode).toBe( + "provider_refresh_failed", + ), ) expect(send).toHaveBeenCalledWith( "command-progress", @@ -271,7 +322,6 @@ describe("provider job lifecycle over IPC", () => { ) }) - it("does not blame a successful init for a refresh failure afterward", async () => { const { providerJobs } = setup(() => ({ lines: [statusLine("running_init"), statusLine("ready")], @@ -303,7 +353,9 @@ describe("provider job lifecycle over IPC", () => { it("returns failure and retains recovery when provider refresh still fails", async () => { const { providerJobs } = setup(() => ({ lines: [], code: 0 })) - providerJobs.setRefresh(() => Promise.reject(new Error("still unavailable"))) + providerJobs.setRefresh(() => + Promise.reject(new Error("still unavailable")), + ) providerJobs.start("docker", "updating") await providerJobs.finish("docker", { code: "provider_refresh_failed", @@ -313,7 +365,8 @@ describe("provider job lifecycle over IPC", () => { const result = await invoke("provider_refresh_state", { name: "docker" }) expect(result).toEqual({ ok: false, message: "still unavailable" }) - expect(providerJobs.get("docker")?.errorCode).toBe("provider_refresh_failed") + expect(providerJobs.get("docker")?.errorCode).toBe( + "provider_refresh_failed", + ) }) - }) diff --git a/desktop/src/main/ipc.ts b/desktop/src/main/ipc.ts index 68bfd08238..c65b31d507 100644 --- a/desktop/src/main/ipc.ts +++ b/desktop/src/main/ipc.ts @@ -17,10 +17,7 @@ import { loadCatalog } from "./image-catalog.js" import type { LogStore } from "./log-store.js" import type { MachineDiagnosticsStore } from "./machine-diagnostics-store.js" import type { MachineDiagnosticsManager } from "./machine-diagnostics-manager.js" -import type { - ProviderActivity, - ProviderJobs, -} from "./provider-jobs.js" +import type { ProviderActivity, ProviderJobs } from "./provider-jobs.js" import type { PtyManager } from "./pty.js" import { sanitizeAppSettingsPatch } from "./app-settings.js" import type { SettingsService } from "./settings-service.js" @@ -51,6 +48,7 @@ interface SecretEntry { lastUsed?: string orphaned?: boolean backend?: "keyring" | "file" + attached?: boolean } interface EnvEntry { @@ -106,8 +104,8 @@ interface IpcDependencies { cli: CliRunner state: DaemonState logStore: LogStore - machineDiagnosticsStore?: MachineDiagnosticsStore - machineDiagnosticsManager?: MachineDiagnosticsManager + machineDiagnosticsStore?: MachineDiagnosticsStore + machineDiagnosticsManager?: MachineDiagnosticsManager pty: PtyManager getMainWindow: () => BrowserWindow | null providerJobs: ProviderJobs @@ -142,13 +140,19 @@ interface ProgressSink { function redactSensitiveText(value: string): string { const secrets = Object.entries(process.env) - .filter(([name, secret]) => - secret && - /(PASSWORD|PASSWD|TOKEN|SECRET|API_KEY|APIKEY|AUTH|CREDENTIAL|PRIVATE_KEY|ACCESS_KEY)/i.test(name), + .filter( + ([name, secret]) => + secret && + /(PASSWORD|PASSWD|TOKEN|SECRET|API_KEY|APIKEY|AUTH|CREDENTIAL|PRIVATE_KEY|ACCESS_KEY)/i.test( + name, + ), ) .map(([, secret]) => secret as string) .sort((a, b) => b.length - a.length) - const redacted = secrets.reduce((text, secret) => text.split(secret).join("***"), value) + const redacted = secrets.reduce( + (text, secret) => text.split(secret).join("***"), + value, + ) return redacted .replace(/(https?:\/\/)[^\s/@]+@/gi, "$1***@") .replace(/(authorization\s*[:=]\s*(?:bearer|basic)\s+)[^\s,]+/gi, "$1***") @@ -184,9 +188,13 @@ function redactOperationStatus(value: OperationStatus): OperationStatus { error: value.error ? { ...value.error, - code: value.error.code ? redactSensitiveText(value.error.code) : undefined, + code: value.error.code + ? redactSensitiveText(value.error.code) + : undefined, message: redactSensitiveText(value.error.message), - hint: value.error.hint ? redactSensitiveText(value.error.hint) : undefined, + hint: value.error.hint + ? redactSensitiveText(value.error.hint) + : undefined, context: value.error.context ? Object.fromEntries( Object.entries(value.error.context).map(([key, text]) => [ @@ -237,8 +245,12 @@ function createLogSink( ...(extra ? { ...extra, - message: extra.message ? redactSensitiveText(extra.message) : undefined, - cliError: extra.cliError ? redactCLIError(extra.cliError) : undefined, + message: extra.message + ? redactSensitiveText(extra.message) + : undefined, + cliError: extra.cliError + ? redactCLIError(extra.cliError) + : undefined, } : {}), }) @@ -280,7 +292,17 @@ export function registerIpcHandlers(deps: IpcDependencies): { start: (workspaceId: string) => Promise } } { - const { cli, state, logStore, pty, providerJobs, workspaceJobs, machineDiagnosticsStore, machineDiagnosticsManager, getMainWindow } = deps + const { + cli, + state, + logStore, + pty, + providerJobs, + workspaceJobs, + machineDiagnosticsStore, + machineDiagnosticsManager, + getMainWindow, + } = deps const tunnelProcesses = new Map< string, import("node:child_process").ChildProcess @@ -360,18 +382,29 @@ export function registerIpcHandlers(deps: IpcDependencies): { tunnelProc.kill("SIGTERM") await tunnelExit // If process did not close in time, forcefully kill and suppress any late callbacks - if (!settled || (tunnelProc.exitCode === null && tunnelProc.signalCode === null)) { + if ( + !settled || + (tunnelProc.exitCode === null && tunnelProc.signalCode === null) + ) { // Suppress workspace callbacks from the onLine handler - const suppressWorkspaceFn = (tunnelProc as unknown as { _suppressWorkspaceCallbacks?: () => void })._suppressWorkspaceCallbacks + const suppressWorkspaceFn = ( + tunnelProc as unknown as { _suppressWorkspaceCallbacks?: () => void } + )._suppressWorkspaceCallbacks if (suppressWorkspaceFn) suppressWorkspaceFn() // Suppress callbacks at the readline level - const suppressFn = (tunnelProc as unknown as { _suppressCallbacks?: () => void })._suppressCallbacks + const suppressFn = ( + tunnelProc as unknown as { _suppressCallbacks?: () => void } + )._suppressCallbacks if (suppressFn) suppressFn() // Close readline interfaces - const rlStdout = (tunnelProc as unknown as { _rlStdout?: { close: () => void } })._rlStdout - const rlStderr = (tunnelProc as unknown as { _rlStderr?: { close: () => void } })._rlStderr + const rlStdout = ( + tunnelProc as unknown as { _rlStdout?: { close: () => void } } + )._rlStdout + const rlStderr = ( + tunnelProc as unknown as { _rlStderr?: { close: () => void } } + )._rlStderr if (rlStdout) rlStdout.close() if (rlStderr) rlStderr.close() @@ -428,7 +461,13 @@ export function registerIpcHandlers(deps: IpcDependencies): { if (!workspaceJobs.owns(workspaceId, commandId)) return true const status = redactOperationStatus(normalizeOperationStatus(envelope)) workspaceJobs.progress(workspaceId, commandId, status) - deps.getMainWindow()?.webContents.send("workspace-status", { commandId, workspaceId, ...status }) + deps + .getMainWindow() + ?.webContents.send("workspace-status", { + commandId, + workspaceId, + ...status, + }) return true } @@ -455,7 +494,11 @@ export function registerIpcHandlers(deps: IpcDependencies): { : undefined await providerJobs.finish( name, - cliError ?? redactCLIError({ code: "provider_failed", message: errorMessage(error) }), + cliError ?? + redactCLIError({ + code: "provider_failed", + message: errorMessage(error), + }), ) throw error } @@ -480,7 +523,9 @@ export function registerIpcHandlers(deps: IpcDependencies): { if (stream !== "stdout") return const envelope = parseCliEnvelope(line) if (envelope?.kind === "status") { - const status = redactOperationStatus(normalizeOperationStatus(envelope)) + const status = redactOperationStatus( + normalizeOperationStatus(envelope), + ) providerJobs.reportStatus(name, status) } }, @@ -546,10 +591,18 @@ export function registerIpcHandlers(deps: IpcDependencies): { // ── Workspaces ── ipcMain.handle("workspace_list", () => state.workspaceList()) - ipcMain.handle("workspace_snapshot", () => deps.workspaceSnapshot?.() ?? { - workspaces: state.workspaceList(), jobs: workspaceJobs.snapshot(), revision: workspaceJobs.revision, - }) - ipcMain.handle("workspace_refresh", (_event, args: { workspaceId: string }) => workspaceJobs.retryRefresh(args.workspaceId)) + ipcMain.handle( + "workspace_snapshot", + () => + deps.workspaceSnapshot?.() ?? { + workspaces: state.workspaceList(), + jobs: workspaceJobs.snapshot(), + revision: workspaceJobs.revision, + }, + ) + ipcMain.handle("workspace_refresh", (_event, args: { workspaceId: string }) => + workspaceJobs.retryRefresh(args.workspaceId), + ) ipcMain.handle( "workspace_status", @@ -566,7 +619,10 @@ export function registerIpcHandlers(deps: IpcDependencies): { if (args.recovery) cliArgs.push("--recovery") const generation = workspaceJobs.generation(args.workspaceId) const raw = await cli.runRaw(cliArgs) - if (!args.recovery && generation === workspaceJobs.generation(args.workspaceId)) { + if ( + !args.recovery && + generation === workspaceJobs.generation(args.workspaceId) + ) { const status = normalizeWorkspaceStatus(raw) if (status && state.updateWorkspaceStatus(args.workspaceId, status)) { // The watcher owns renderer broadcasts; this update still keeps @@ -641,7 +697,10 @@ export function registerIpcHandlers(deps: IpcDependencies): { await providerJobs.finish( args.name, redactCLIError( - cliError ?? { code: "provider_failed", message: errorMessage(error) }, + cliError ?? { + code: "provider_failed", + message: errorMessage(error), + }, ), ) throw error @@ -706,7 +765,9 @@ export function registerIpcHandlers(deps: IpcDependencies): { if (stream === "stdout") { const envelope = parseCliEnvelope(line) if (envelope?.kind === "status") { - const status = redactOperationStatus(normalizeOperationStatus(envelope)) + const status = redactOperationStatus( + normalizeOperationStatus(envelope), + ) providerJobs.reportStatus(args.name, status) return } @@ -733,10 +794,9 @@ export function registerIpcHandlers(deps: IpcDependencies): { }, ), ) - const exitMsg = redactSensitiveText(formatLogLine( - `Exit code: ${code}`, - code === 0 ? "INFO" : "ERROR", - )) + const exitMsg = redactSensitiveText( + formatLogLine(`Exit code: ${code}`, code === 0 ? "INFO" : "ERROR"), + ) win?.webContents.send("command-progress", { commandId: cmdId, message: exitMsg, @@ -787,36 +847,39 @@ export function registerIpcHandlers(deps: IpcDependencies): { const runStep = (cliArgs: string[]): Promise => new Promise((resolve, reject) => { - cli.runStreaming( - cliArgs, - (line, stream, meta) => { - if (stream === "stdout") { - const envelope = parseCliEnvelope(line) - if (envelope?.kind === "status") { - providerJobs.reportStatus( - args.name, - redactOperationStatus(normalizeOperationStatus(envelope)), - ) + cli + .runStreaming( + cliArgs, + (line, stream, meta) => { + if (stream === "stdout") { + const envelope = parseCliEnvelope(line) + if (envelope?.kind === "status") { + providerJobs.reportStatus( + args.name, + redactOperationStatus(normalizeOperationStatus(envelope)), + ) + return + } + } + sendProgress(line, meta?.level) + }, + (code, cliError) => { + if (code === 0) { + resolve() return } - } - sendProgress(line, meta?.level) - }, - (code, cliError) => { - if (code === 0) { - resolve() - return - } - reject( - Object.assign( - new Error( - cliError?.message ?? `${cliArgs.join(" ")} exited with ${code}`, + reject( + Object.assign( + new Error( + cliError?.message ?? + `${cliArgs.join(" ")} exited with ${code}`, + ), + { cliError }, ), - { cliError }, - ), - ) - }, - ).catch(reject) + ) + }, + ) + .catch(reject) }) void (async () => { @@ -840,7 +903,8 @@ export function registerIpcHandlers(deps: IpcDependencies): { } catch (error) { failure = { code: "provider_refresh_failed", - message: "The provider updated, but its current state could not be refreshed.", + message: + "The provider updated, but its current state could not be refreshed.", hint: "Refresh provider status to try again.", context: { cause: errorMessage(error) }, } @@ -865,14 +929,17 @@ export function registerIpcHandlers(deps: IpcDependencies): { }, ) - ipcMain.handle("provider_refresh_state", async (_event, args: { name: string }) => { - try { - await providerJobs.retryRefresh(args.name) - return { ok: true } as const - } catch (error) { - return { ok: false, message: errorMessage(error) } as const - } - }) + ipcMain.handle( + "provider_refresh_state", + async (_event, args: { name: string }) => { + try { + await providerJobs.retryRefresh(args.name) + return { ok: true } as const + } catch (error) { + return { ok: false, message: errorMessage(error) } as const + } + }, + ) ipcMain.handle("provider_options", async (_event, args: { name: string }) => { return cli.run(["provider", "get", args.name]) @@ -1022,7 +1089,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { const cliArgs = ["machine", "delete", args.id, "--context", key.context] if (args.force) cliArgs.push("--force") await cli.runRaw(cliArgs) - machineDiagnosticsManager?.delete(key) + machineDiagnosticsManager?.delete(key) }, ) @@ -1033,7 +1100,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { ipcMain.handle("machine_stop", async (_event, args: { id: string }) => { const key = { context: state.currentContext(), machineId: args.id } await cli.runRaw(["machine", "stop", args.id, "--context", key.context]) - machineDiagnosticsManager?.markStopped(key) + machineDiagnosticsManager?.markStopped(key) }) ipcMain.handle("machine_status", async (_event, args: { id: string }) => { @@ -1041,16 +1108,29 @@ export function registerIpcHandlers(deps: IpcDependencies): { }) ipcMain.handle("machine_diagnostics_get", (_event, args: { id: string }) => { - return machineDiagnosticsManager?.getCached({ context: state.currentContext(), machineId: args.id }) ?? null + return ( + machineDiagnosticsManager?.getCached({ + context: state.currentContext(), + machineId: args.id, + }) ?? null + ) }) - ipcMain.handle("machine_diagnostics_refresh", async (_event, args: { id: string }) => { - if (!machineDiagnosticsManager) throw new Error("machine diagnostics manager is unavailable") - const key = { context: state.currentContext(), machineId: args.id } - const merged = await machineDiagnosticsManager.refresh(key) - getMainWindow()?.webContents.send("machine-diagnostics-changed", { machineId: args.id, context: key.context, diagnostics: merged }) - return merged - }) + ipcMain.handle( + "machine_diagnostics_refresh", + async (_event, args: { id: string }) => { + if (!machineDiagnosticsManager) + throw new Error("machine diagnostics manager is unavailable") + const key = { context: state.currentContext(), machineId: args.id } + const merged = await machineDiagnosticsManager.refresh(key) + getMainWindow()?.webContents.send("machine-diagnostics-changed", { + machineId: args.id, + context: key.context, + diagnostics: merged, + }) + return merged + }, + ) // ── Contexts ── ipcMain.handle("context_list", () => state.contextList()) @@ -1130,6 +1210,50 @@ export function registerIpcHandlers(deps: IpcDependencies): { } }) + ipcMain.handle( + "secret_attach", + async (_event, args: { name: string; context: string }) => { + trackEvent("secret_attach") + try { + if (!args.context) throw new Error("context is required") + await cli.runRaw([ + "--context", + args.context, + "secret", + "attach", + args.name, + ]) + return { ok: true } as const + } catch (err) { + const cliError = (err as { cliError?: CLIError }).cliError + const message = err instanceof Error ? err.message : String(err) + return { ok: false, message, cliError } as const + } + }, + ) + + ipcMain.handle( + "secret_detach", + async (_event, args: { name: string; context: string }) => { + trackEvent("secret_detach") + try { + if (!args.context) throw new Error("context is required") + await cli.runRaw([ + "--context", + args.context, + "secret", + "detach", + args.name, + ]) + return { ok: true } as const + } catch (err) { + const cliError = (err as { cliError?: CLIError }).cliError + const message = err instanceof Error ? err.message : String(err) + return { ok: false, message, cliError } as const + } + }, + ) + ipcMain.handle("env_list", async () => cli.run(["env", "list"])) // Returns an envelope rather than throwing so a structured cliError survives @@ -1155,7 +1279,13 @@ export function registerIpcHandlers(deps: IpcDependencies): { trackEvent("env_delete") try { if (!args.context) throw new Error("context is required") - await cli.runRaw(["--context", args.context, "env", "delete", args.name]) + await cli.runRaw([ + "--context", + args.context, + "env", + "delete", + args.name, + ]) return { ok: true } as const } catch (err) { const cliError = (err as { cliError?: CLIError }).cliError @@ -1165,31 +1295,49 @@ export function registerIpcHandlers(deps: IpcDependencies): { }, ) - ipcMain.handle("env_attach", async (_event, args: { name: string; context: string }) => { - trackEvent("env_attach") - try { - if (!args.context) throw new Error("context is required") - await cli.runRaw(["--context", args.context, "env", "attach", args.name]) - return { ok: true } as const - } catch (err) { - const cliError = (err as { cliError?: CLIError }).cliError - const message = err instanceof Error ? err.message : String(err) - return { ok: false, message, cliError } as const - } - }) + ipcMain.handle( + "env_attach", + async (_event, args: { name: string; context: string }) => { + trackEvent("env_attach") + try { + if (!args.context) throw new Error("context is required") + await cli.runRaw([ + "--context", + args.context, + "env", + "attach", + args.name, + ]) + return { ok: true } as const + } catch (err) { + const cliError = (err as { cliError?: CLIError }).cliError + const message = err instanceof Error ? err.message : String(err) + return { ok: false, message, cliError } as const + } + }, + ) - ipcMain.handle("env_detach", async (_event, args: { name: string; context: string }) => { - trackEvent("env_detach") - try { - if (!args.context) throw new Error("context is required") - await cli.runRaw(["--context", args.context, "env", "detach", args.name]) - return { ok: true } as const - } catch (err) { - const cliError = (err as { cliError?: CLIError }).cliError - const message = err instanceof Error ? err.message : String(err) - return { ok: false, message, cliError } as const - } - }) + ipcMain.handle( + "env_detach", + async (_event, args: { name: string; context: string }) => { + trackEvent("env_detach") + try { + if (!args.context) throw new Error("context is required") + await cli.runRaw([ + "--context", + args.context, + "env", + "detach", + args.name, + ]) + return { ok: true } as const + } catch (err) { + const cliError = (err as { cliError?: CLIError }).cliError + const message = err instanceof Error ? err.message : String(err) + return { ok: false, message, cliError } as const + } + }, + ) // ── System ── ipcMain.handle("devsy_version", async () => { @@ -1443,8 +1591,7 @@ export function registerIpcHandlers(deps: IpcDependencies): { }, wsId, ) - // Expose a method to suppress callbacks from cancelActiveUp - ;( + ;( child as unknown as { _suppressWorkspaceCallbacks?: () => void } )._suppressWorkspaceCallbacks = () => { suppressCallbacks = true @@ -1475,9 +1622,10 @@ export function registerIpcHandlers(deps: IpcDependencies): { }) return { commandId: cmdId, completion } } - - ipcMain.handle("workspace_up", (_event, args: Parameters[0]) => - runWorkspaceUp(args).commandId, + ipcMain.handle( + "workspace_up", + (_event, args: Parameters[0]) => + runWorkspaceUp(args).commandId, ) async function reconcileDetachedTask( @@ -2035,14 +2183,14 @@ export function registerIpcHandlers(deps: IpcDependencies): { runInitialProviderUpdateCheck: runUpdateCheck, workspaceActions: { async stop(workspaceId: string): Promise { - const { completion } = await startWorkspaceStop( - { workspaceId }, - "tray", - ) + const { completion } = await startWorkspaceStop({ workspaceId }, "tray") await completion }, async start(workspaceId: string): Promise { - await runWorkspaceUp({ source: workspaceId, commandId: crypto.randomUUID() }).completion + await runWorkspaceUp({ + source: workspaceId, + commandId: crypto.randomUUID(), + }).completion }, }, } diff --git a/desktop/src/renderer/src/lib/ipc/commands.test.ts b/desktop/src/renderer/src/lib/ipc/commands.test.ts index 3fd4e0c64c..1e454ab5af 100644 --- a/desktop/src/renderer/src/lib/ipc/commands.test.ts +++ b/desktop/src/renderer/src/lib/ipc/commands.test.ts @@ -10,6 +10,8 @@ import { envDelete, envAttach, envDetach, + secretAttach, + secretDetach, machineCreate, machineDelete, machineStatus, @@ -145,6 +147,26 @@ describe("IPC commands", () => { }) }) + describe("managed secret commands", () => { + it("secretAttach sends the secret name and context", async () => { + mockInvoke.mockResolvedValue({ ok: true }) + await secretAttach("DB_PASSWORD", "staging") + expect(mockInvoke).toHaveBeenCalledWith("secret_attach", { + name: "DB_PASSWORD", + context: "staging", + }) + }) + + it("secretDetach sends the secret name and context", async () => { + mockInvoke.mockResolvedValue({ ok: true }) + await secretDetach("DB_PASSWORD", "staging") + expect(mockInvoke).toHaveBeenCalledWith("secret_detach", { + name: "DB_PASSWORD", + context: "staging", + }) + }) + }) + describe("managed environment commands", () => { it("envAttach sends the variable name and context", async () => { mockInvoke.mockResolvedValue({ ok: true }) diff --git a/desktop/src/renderer/src/lib/ipc/commands.ts b/desktop/src/renderer/src/lib/ipc/commands.ts index cdab0d0dfe..a4bcd7e3c6 100644 --- a/desktop/src/renderer/src/lib/ipc/commands.ts +++ b/desktop/src/renderer/src/lib/ipc/commands.ts @@ -19,7 +19,11 @@ import type { MachineDiagnosticsCache } from "$shared/machine-diagnostics-types. type CommandEnvelope = | { ok: true } - | { ok: false; message: string; cliError?: import("$shared/cli-error.js").CLIError } + | { + ok: false + message: string + cliError?: import("$shared/cli-error.js").CLIError + } /** Unwrap a structured command envelope, rethrowing failures as an Error with .cliError attached. */ function unwrapEnvelope(result: CommandEnvelope): void { @@ -166,7 +170,9 @@ export async function providerUpdateStreaming(name: string): Promise { } export async function providerRefreshState(name: string): Promise { - unwrapEnvelope(await invoke("provider_refresh_state", { name })) + unwrapEnvelope( + await invoke("provider_refresh_state", { name }), + ) } export async function providerOptions( @@ -193,18 +199,24 @@ export async function providerRename( } export async function providerListVersions(name: string, noCache?: boolean) { - return invoke<{ versions: ProviderVersion[]; unsupported: boolean; error?: string }>( - "provider_list_versions", - { name, noCache }, - ) + return invoke<{ + versions: ProviderVersion[] + unsupported: boolean + error?: string + }>("provider_list_versions", { name, noCache }) } -export async function providerSetVersion(name: string, tag: string): Promise { +export async function providerSetVersion( + name: string, + tag: string, +): Promise { return invoke("provider_set_version", { name, tag }) } export async function providerCheckUpdates() { - return invoke>("provider_check_updates") + return invoke>( + "provider_check_updates", + ) } export async function providerGetUpdateCache() { @@ -271,11 +283,17 @@ export async function machineStatus(id: string): Promise { } } -export async function machineDiagnosticsGet(id: string): Promise { - return invoke("machine_diagnostics_get", { id }) +export async function machineDiagnosticsGet( + id: string, +): Promise { + return invoke("machine_diagnostics_get", { + id, + }) } -export async function machineDiagnosticsRefresh(id: string): Promise { +export async function machineDiagnosticsRefresh( + id: string, +): Promise { return invoke("machine_diagnostics_refresh", { id }) } @@ -324,6 +342,24 @@ export async function secretDelete(name: string): Promise { unwrapEnvelope(await invoke("secret_delete", { name })) } +export async function secretAttach( + name: string, + context: string, +): Promise { + unwrapEnvelope( + await invoke("secret_attach", { name, context }), + ) +} + +export async function secretDetach( + name: string, + context: string, +): Promise { + unwrapEnvelope( + await invoke("secret_detach", { name, context }), + ) +} + export async function envList(): Promise { return invoke("env_list") } @@ -333,9 +369,7 @@ export async function envSet(name: string, value: string): Promise { } export async function envDelete(name: string, context: string): Promise { - unwrapEnvelope( - await invoke("env_delete", { name, context }), - ) + unwrapEnvelope(await invoke("env_delete", { name, context })) } export async function envAttach(name: string, context: string): Promise { @@ -422,7 +456,9 @@ export async function getReleaseChannel(): Promise { return invoke("get_release_channel") } -export async function setReleaseChannel(channel: ReleaseChannel): Promise { +export async function setReleaseChannel( + channel: ReleaseChannel, +): Promise { return invoke("set_release_channel", { channel }) } diff --git a/desktop/src/renderer/src/lib/types/index.ts b/desktop/src/renderer/src/lib/types/index.ts index 3632667a9c..c176968e66 100644 --- a/desktop/src/renderer/src/lib/types/index.ts +++ b/desktop/src/renderer/src/lib/types/index.ts @@ -156,6 +156,7 @@ export interface Secret { lastUsed?: string orphaned?: boolean backend?: "keyring" | "file" + attached?: boolean } export interface EnvVar { diff --git a/desktop/src/renderer/src/pages/SecretsPage.svelte b/desktop/src/renderer/src/pages/SecretsPage.svelte index 75f4f0e6d2..15d8666c19 100644 --- a/desktop/src/renderer/src/pages/SecretsPage.svelte +++ b/desktop/src/renderer/src/pages/SecretsPage.svelte @@ -3,14 +3,26 @@ import { KeyRound, Plus, Search, Trash2 } from "@lucide/svelte" import { Button } from "$lib/components/ui/button/index.js" import { Input } from "$lib/components/ui/input/index.js" import { Label } from "$lib/components/ui/label/index.js" +import { Switch } from "$lib/components/ui/switch/index.js" import { badgeVariants } from "$lib/components/ui/badge/index.js" import * as Dialog from "$lib/components/ui/dialog/index.js" import ConfirmDialog from "$lib/components/layout/ConfirmDialog.svelte" import CardSkeleton from "$lib/components/ui/skeleton/CardSkeleton.svelte" -import { secrets, secretsError, secretsLoading, refreshSecrets } from "$lib/stores/secrets.js" -import { secretSet, secretDelete } from "$lib/ipc/commands.js" +import { + secrets, + secretsError, + secretsLoading, + refreshSecrets, +} from "$lib/stores/secrets.js" +import { + secretSet, + secretDelete, + secretAttach, + secretDetach, +} from "$lib/ipc/commands.js" import { toasts } from "$lib/stores/toasts.js" import { extractErrorMessage } from "$lib/utils/error.js" +import type { Secret } from "$lib/types/index.js" const NAME_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/ @@ -22,6 +34,9 @@ let saving = $state(false) let confirmDeleteOpen = $state(false) let pendingDelete = $state("") let deleting = $state(false) +let updatingAttachment = $state>({}) +let attachmentResets = $state>({}) +let attachmentErrors = $state>({}) let searchTerm = $state("") let filteredSecrets = $derived.by(() => { @@ -34,9 +49,7 @@ let filteredSecrets = $derived.by(() => { }) let nameValid = $derived(NAME_PATTERN.test(newName)) -let nameExists = $derived( - $secrets.some((s) => s.name === newName.trim()), -) +let nameExists = $derived($secrets.some((s) => s.name === newName.trim())) $effect(() => { if (!createDialogOpen) { @@ -58,11 +71,30 @@ async function handleCreate() { return } createDialogOpen = false - toasts.success(replacing ? `Secret "${name}" replaced` : `Secret "${name}" saved`) + toasts.success( + replacing ? `Secret "${name}" replaced` : `Secret "${name}" saved`, + ) saving = false await refreshSecrets().catch(() => {}) } +async function setAttached(secret: Secret, attached: boolean) { + const key = `${secret.context}\x00${secret.name}` + if (updatingAttachment[key]) return + updatingAttachment = { ...updatingAttachment, [key]: true } + attachmentErrors = { ...attachmentErrors, [key]: "" } + try { + if (attached) await secretAttach(secret.name, secret.context) + else await secretDetach(secret.name, secret.context) + await refreshSecrets() + } catch (err) { + attachmentErrors = { ...attachmentErrors, [key]: extractErrorMessage(err) } + attachmentResets = { ...attachmentResets, [key]: (attachmentResets[key] ?? 0) + 1 } + } finally { + updatingAttachment = { ...updatingAttachment, [key]: false } + } +} + function requestDelete(e: Event, name: string) { e.stopPropagation() pendingDelete = name @@ -184,7 +216,7 @@ async function confirmDelete() { {:else}
- {#each filteredSecrets as secret (secret.name)} + {#each filteredSecrets as secret (`${secret.context}\x00${secret.name}`)}
@@ -200,9 +232,25 @@ async function confirmDelete() {
-

- Context: {secret.context} -

+
+
+

Inject into workspaces

+

Context: {secret.context}. Delivered through the protected secret path when a workspace starts or is recreated.

+
+ {#key attachmentResets[`${secret.context}\x00${secret.name}`] ?? 0} + setAttached(secret, checked)} + /> + {/key} +
+ {#if attachmentErrors[`${secret.context}\x00${secret.name}`]} +

+ Failed to update attachment: {attachmentErrors[`${secret.context}\x00${secret.name}`]} +

+ {/if}
{/each}
diff --git a/desktop/src/renderer/src/pages/SecretsPage.test.ts b/desktop/src/renderer/src/pages/SecretsPage.test.ts new file mode 100644 index 0000000000..44a1c3b376 --- /dev/null +++ b/desktop/src/renderer/src/pages/SecretsPage.test.ts @@ -0,0 +1,88 @@ +import { + cleanup, + fireEvent, + render, + screen, + waitFor, +} from "@testing-library/svelte" +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" + +const mocks = vi.hoisted(() => ({ + secretAttach: vi.fn().mockResolvedValue(undefined), + secretDetach: vi.fn().mockResolvedValue(undefined), + secretDelete: vi.fn().mockResolvedValue(undefined), + refreshSecrets: vi.fn().mockResolvedValue(undefined), +})) +vi.mock("$lib/ipc/commands.js", () => ({ + secretAttach: mocks.secretAttach, + secretDelete: mocks.secretDelete, + secretDetach: mocks.secretDetach, + secretSet: vi.fn(), +})) +vi.mock("$lib/stores/secrets.js", async () => { + const { writable } = await import("svelte/store") + return { + secretsLoading: writable(false), + secretsError: writable(null), + secrets: writable([ + { name: "ATTACHED", context: "default", attached: true }, + { name: "DETACHED", context: "default", attached: false }, + { name: "STAGING_ONLY", context: "staging", attached: false }, + ]), + refreshSecrets: mocks.refreshSecrets, + } +}) + +import SecretsPage from "./SecretsPage.svelte" + +describe("SecretsPage managed secret attachments", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + afterEach(() => { + cleanup() + }) + + it("renders attachment state and sends attach/detach intent", async () => { + render(SecretsPage) + + expect( + screen + .getByRole("switch", { name: "Inject ATTACHED into workspaces" }) + .getAttribute("aria-checked"), + ).toBe("true") + const detached = screen.getByRole("switch", { + name: "Inject DETACHED into workspaces", + }) + expect(detached.getAttribute("aria-checked")).toBe("false") + + await fireEvent.click(detached) + await waitFor(() => + expect(mocks.secretAttach).toHaveBeenCalledWith("DETACHED", "default"), + ) + expect(mocks.refreshSecrets).toHaveBeenCalled() + + await fireEvent.click( + screen.getByRole("switch", { name: "Inject ATTACHED into workspaces" }), + ) + await waitFor(() => + expect(mocks.secretDetach).toHaveBeenCalledWith("ATTACHED", "default"), + ) + }) + + it("uses the context displayed on a stale row", async () => { + render(SecretsPage) + + await fireEvent.click( + screen.getByRole("switch", { + name: "Inject STAGING_ONLY into workspaces", + }), + ) + await waitFor(() => + expect(mocks.secretAttach).toHaveBeenCalledWith( + "STAGING_ONLY", + "staging", + ), + ) + }) +}) diff --git a/e2e/tests/machineprovider/machineprovider.go b/e2e/tests/machineprovider/machineprovider.go index b57288245e..3ba6a2cdfc 100644 --- a/e2e/tests/machineprovider/machineprovider.go +++ b/e2e/tests/machineprovider/machineprovider.go @@ -182,7 +182,7 @@ var _ = ginkgo.Describe( framework.ExpectNoError(err) ginkgo.DeferCleanup(framework.CleanupTempDir, initialDir, tempDir) - // create provider (same 5s inactivity timeout as machineprovider2) + // create provider with a short timeout while leaving startup headroom _ = f.DevsyProviderDelete(ctx, "docker123") err = f.DevsyProviderAdd(ctx, filepath.Join(tempDir, "provider.yaml")) framework.ExpectNoError(err) @@ -213,14 +213,12 @@ var _ = ginkgo.Describe( err = f.DevsyUp(ctx, tempDir, "--daemon-interval=3s") framework.ExpectNoError(err) - // verify workspace stays running well past the 5s timeout. - // The timeout would fire within ~15s (5s timeout + 10s ticker). - // We assert RUNNING for 30s to give ample margin. + // Verify workspace stays running past the 30s timeout and its 10s ticker. gomega.Consistently(func() string { status, err := f.DevsyStatus(ctx, tempDir, "--container-status=false") framework.ExpectNoError(err) return strings.ToUpper(status.State) - }, 30*time.Second, 2*time.Second).Should( + }, 50*time.Second, 2*time.Second).Should( gomega.Equal("RUNNING"), "workspace should stay running when shutdownAction is none", ) diff --git a/e2e/tests/machineprovider/testdata/machineprovider3/provider.yaml b/e2e/tests/machineprovider/testdata/machineprovider3/provider.yaml index 1b14a88b76..2e0c50805f 100644 --- a/e2e/tests/machineprovider/testdata/machineprovider3/provider.yaml +++ b/e2e/tests/machineprovider/testdata/machineprovider3/provider.yaml @@ -8,7 +8,7 @@ options: default: devsy-e2e INACTIVITY_TIMEOUT: description: The timeout until the pod will be stopped - default: 5s + default: 30s agent: path: /usr/local/bin/devsy inactivityTimeout: ${INACTIVITY_TIMEOUT} diff --git a/e2e/tests/up/helper.go b/e2e/tests/up/helper.go index 7cd789f04a..4bc3117950 100644 --- a/e2e/tests/up/helper.go +++ b/e2e/tests/up/helper.go @@ -17,6 +17,7 @@ import ( provider2 "github.com/devsy-org/devsy/pkg/provider" "github.com/devsy-org/devsy/pkg/scanner" "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" ) const ( @@ -265,3 +266,51 @@ func setupWorkspaceAndUp( } return tempDir, f.DevsyUp(ctx, append([]string{tempDir}, args...)...) } + +type contextAttachmentCase struct { + contextPrefix string + testdataDir string + store func(context.Context, string, string) + command string + name string + checkFile string +} + +// verifyContextAttachment proves a context-attached managed value reaches the +// workspace at up and is gone after detach and recreate. +func (dtc *dockerTestContext) verifyContextAttachment( + ctx context.Context, + tc contextAttachmentCase, +) { + useFileSecretsBackend() + contextName := fmt.Sprintf("%s-%d", tc.contextPrefix, time.Now().UnixNano()) + framework.ExpectNoError(dtc.f.DevsyContextCreate(ctx, contextName)) + ginkgo.DeferCleanup(func(cleanupCtx context.Context) { + _ = dtc.f.DevsyContextUse(cleanupCtx, "default") + _ = dtc.f.DevsyContextDelete(cleanupCtx, contextName) + }) + framework.ExpectNoError(dtc.f.DevsyContextUse(ctx, contextName)) + framework.ExpectNoError( + dtc.f.DevsyProviderAdd(ctx, "docker", "-o", "DOCKER_PATH=docker"), + ) + framework.ExpectNoError(dtc.f.DevsyProviderUse(ctx, "docker")) + + tempDir, err := setupWorkspace(tc.testdataDir, dtc.initialDir, dtc.f) + framework.ExpectNoError(err) + tc.store(ctx, tc.name, "expected-value") + _, err = dtc.f.ExecCommandOutput(ctx, []string{tc.command, "attach", tc.name}) + framework.ExpectNoError(err) + + // Intentionally no --env/--secret argument: the context binding is the source. + framework.ExpectNoError(dtc.f.DevsyUp(ctx, tempDir)) + out, err := dtc.execSSH(ctx, tempDir, "cat "+tc.checkFile) + framework.ExpectNoError(err) + gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("expected-value")) + + _, err = dtc.f.ExecCommandOutput(ctx, []string{tc.command, "detach", tc.name}) + framework.ExpectNoError(err) + framework.ExpectNoError(dtc.f.DevsyUpRecreate(ctx, tempDir)) + out, err = dtc.execSSH(ctx, tempDir, "cat "+tc.checkFile) + framework.ExpectNoError(err) + gomega.Expect(strings.TrimSpace(out)).To(gomega.BeEmpty()) +} diff --git a/e2e/tests/up/provider_docker.go b/e2e/tests/up/provider_docker.go index 9a0197dc2f..f7a822d0a5 100644 --- a/e2e/tests/up/provider_docker.go +++ b/e2e/tests/up/provider_docker.go @@ -768,46 +768,29 @@ var _ = ginkgo.Describe( ginkgo.It( "context-attached managed env var injects without --env and is removed after detach", func(ctx context.Context) { - useFileSecretsBackend() - contextName := fmt.Sprintf("managed-env-%d", time.Now().UnixNano()) - framework.ExpectNoError(dtc.f.DevsyContextCreate(ctx, contextName)) - ginkgo.DeferCleanup(func(cleanupCtx context.Context) { - _ = dtc.f.DevsyContextUse(cleanupCtx, "default") - _ = dtc.f.DevsyContextDelete(cleanupCtx, contextName) + dtc.verifyContextAttachment(ctx, contextAttachmentCase{ + contextPrefix: "managed-env", + testdataDir: "tests/up/testdata/docker-managed-env-attached", + store: dtc.storeEnv, + command: envCmd, + name: "ATTACHED_ENV", + checkFile: "/tmp/attached-env-check.out", }) - framework.ExpectNoError(dtc.f.DevsyContextUse(ctx, contextName)) - framework.ExpectNoError( - dtc.f.DevsyProviderAdd( - ctx, - "docker", - "-o", - "DOCKER_PATH=docker", - ), - ) - framework.ExpectNoError(dtc.f.DevsyProviderUse(ctx, "docker")) - - tempDir, err := setupWorkspace( - "tests/up/testdata/docker-managed-env-attached", - dtc.initialDir, - dtc.f, - ) - framework.ExpectNoError(err) - dtc.storeEnv(ctx, "ATTACHED_ENV", "expected-value") - _, err = dtc.f.ExecCommandOutput(ctx, []string{envCmd, "attach", "ATTACHED_ENV"}) - framework.ExpectNoError(err) - - // Intentionally no --env argument: the context binding is the source. - framework.ExpectNoError(dtc.f.DevsyUp(ctx, tempDir)) - out, err := dtc.execSSH(ctx, tempDir, "cat /tmp/attached-env-check.out") - framework.ExpectNoError(err) - gomega.Expect(strings.TrimSpace(out)).To(gomega.Equal("expected-value")) + }, + ginkgo.SpecTimeout(framework.TimeoutShort()), + ) - _, err = dtc.f.ExecCommandOutput(ctx, []string{envCmd, "detach", "ATTACHED_ENV"}) - framework.ExpectNoError(err) - framework.ExpectNoError(dtc.f.DevsyUpRecreate(ctx, tempDir)) - out, err = dtc.execSSH(ctx, tempDir, "cat /tmp/attached-env-check.out") - framework.ExpectNoError(err) - gomega.Expect(strings.TrimSpace(out)).To(gomega.BeEmpty()) + ginkgo.It( + "context-attached managed secret injects without --secret and is removed after detach", + func(ctx context.Context) { + dtc.verifyContextAttachment(ctx, contextAttachmentCase{ + contextPrefix: "managed-secret", + testdataDir: "tests/up/testdata/docker-managed-secret-attached", + store: dtc.storeSecret, + command: secretCmd, + name: "ATTACHED_SECRET", + checkFile: "/tmp/attached-secret-check.out", + }) }, ginkgo.SpecTimeout(framework.TimeoutShort()), ) diff --git a/e2e/tests/up/testdata/docker-managed-secret-attached/.devcontainer.json b/e2e/tests/up/testdata/docker-managed-secret-attached/.devcontainer.json new file mode 100644 index 0000000000..f8a019893e --- /dev/null +++ b/e2e/tests/up/testdata/docker-managed-secret-attached/.devcontainer.json @@ -0,0 +1,4 @@ +{ + "image": "ghcr.io/devsy-org/test-images/go:1", + "postCreateCommand": "printf '%s' \"${ATTACHED_SECRET-}\" > /tmp/attached-secret-check.out" +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 1798f52681..fb33e9cf4e 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -358,6 +358,25 @@ func SaveConfig(config *Config) error { return writeConfigAtomic(configOrigin, out) } +// UpdateConfig runs a read/modify/write cycle on config.yaml under LockConfig: +// the config is loaded after the lock is acquired and saved before it is +// released, so concurrent mutations cannot lose each other's changes. +func UpdateConfig(contextOverride, providerOverride string, mutate func(*Config) error) error { + unlock, err := LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err := LoadConfig(contextOverride, providerOverride) + if err != nil { + return err + } + if err := mutate(devsyConfig); err != nil { + return err + } + return SaveConfig(devsyConfig) +} + // LockConfig serializes read/modify/write mutations to config.yaml across // processes. Callers must acquire it before loading the config and hold it // until every related persistent operation is complete. diff --git a/pkg/workspace/provider_update.go b/pkg/workspace/provider_update.go index 429be2895d..c88d780641 100644 --- a/pkg/workspace/provider_update.go +++ b/pkg/workspace/provider_update.go @@ -86,16 +86,46 @@ func applyProviderUpdate( devsyConfig *config.Config, providerName, newVersion string, ) error { - providerSource, err := ResolveProviderSource(devsyConfig, providerName) + originalProvider, err := FindProvider(devsyConfig, providerName) if err != nil { - return fmt.Errorf("resolve provider source %s: %w", providerName, err) + return fmt.Errorf("find provider %s: %w", providerName, err) } - - splitted := strings.Split(providerSource, "@") - if len(splitted) == 0 { + originalSource := originalProvider.Config.Source + providerSource := provider2.GetProviderSource(originalSource, originalProvider.Config.Name) + sourceBase, _ := provider2.SplitSourceAndTag(providerSource) + if sourceBase == "" { return fmt.Errorf("no provider source found %s", providerSource) } - providerSource = splitted[0] + "@" + newVersion + providerSource = sourceBase + "@" + newVersion + + // The caller's config predates the update check; reload it under the + // config lock so the update cannot lose a concurrent mutation. + unlock, err := config.LockConfig() + if err != nil { + return err + } + defer unlock() + devsyConfig, err = config.LoadConfig(devsyConfig.DefaultContext, "") + if err != nil { + return err + } + + // Another command may have updated the provider while this one waited on + // the lock; never replace an equal or newer version with this one, and + // never undo a source change made in the meantime. + currentProvider, err := FindProvider(devsyConfig, providerName) + if err != nil { + return fmt.Errorf("find provider %s: %w", providerName, err) + } + if reason := providerUpdateSkipReason( + originalSource, + currentProvider.Config.Source, + currentProvider.Config.Version, + newVersion, + ); reason != "" { + log.Infof("%s, skipping update: provider=%s", reason, providerName) + return nil + } _, err = UpdateProvider(ctx, devsyConfig, providerName, providerSource) if err != nil { @@ -106,6 +136,21 @@ func applyProviderUpdate( return nil } +func providerUpdateSkipReason( + originalSource, currentSource provider2.ProviderSource, + currentVersion, newVersion string, +) string { + if originalSource != currentSource { + return "provider source changed" + } + newV, newErr := semver.Parse(strings.TrimPrefix(newVersion, "v")) + currentV, currentErr := semver.Parse(strings.TrimPrefix(currentVersion, "v")) + if newErr == nil && currentErr == nil && currentV.GTE(newV) { + return "provider already up to date" + } + return "" +} + // GetProInstance returns the ProInstance associated with the given provider name, or nil if not found. func GetProInstance( devsyConfig *config.Config, diff --git a/pkg/workspace/provider_update_test.go b/pkg/workspace/provider_update_test.go index e079a9194c..a4eecf934e 100644 --- a/pkg/workspace/provider_update_test.go +++ b/pkg/workspace/provider_update_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/devsy-org/devsy/pkg/config" + "github.com/devsy-org/devsy/pkg/provider" "github.com/stretchr/testify/assert" ) @@ -114,3 +115,76 @@ func TestProviderVersionNeedsUpdate(t *testing.T) { }) } } + +func TestProviderUpdateSkipReasonResolvedGitHubSource(t *testing.T) { + original := provider.ProviderSource{Github: "org/provider", Raw: "org/provider@v1.0.0"} + changed := provider.ProviderSource{Github: "org/provider", Raw: "org/provider@v1.1.0"} + assert.Equal( + t, + provider.GetProviderSource(original, "provider"), + provider.GetProviderSource(changed, "provider"), + ) + assert.Equal(t, "provider source changed", providerUpdateSkipReason( + original, changed, "v1.0.0", "v1.2.0", + )) + assert.Equal(t, "provider already up to date", providerUpdateSkipReason( + original, original, "v1.3.0", "v1.2.0", + )) +} + +func TestProviderUpdateSkipReason(t *testing.T) { + const ( + originalSource = "github.com/org/provider@v1.0.0" + updatedPin = "github.com/org/provider@v1.1.0" + otherSource = "github.com/other/provider@v1.0.0" + version100 = "v1.0.0" + version110 = "v1.1.0" + ) + tests := []struct { + name string + originalSource provider.ProviderSource + currentSource provider.ProviderSource + currentVersion string + newVersion string + wantReason string + }{ + { + name: "unchanged source", + originalSource: provider.ProviderSource{Raw: originalSource}, + currentSource: provider.ProviderSource{Raw: originalSource}, + currentVersion: version100, + newVersion: version110, + }, + { + name: "pin changed on same repository", + originalSource: provider.ProviderSource{Raw: originalSource}, + currentSource: provider.ProviderSource{Raw: updatedPin}, + currentVersion: version110, + newVersion: "v1.2.0", + wantReason: "provider source changed", + }, + { + name: "repository changed", + originalSource: provider.ProviderSource{Raw: originalSource}, + currentSource: provider.ProviderSource{Raw: otherSource}, + currentVersion: version100, + newVersion: "v1.2.0", + wantReason: "provider source changed", + }, + { + name: "current version is newer", + originalSource: provider.ProviderSource{Raw: updatedPin}, + currentSource: provider.ProviderSource{Raw: updatedPin}, + currentVersion: version110, + newVersion: version100, + wantReason: "provider already up to date", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.wantReason, providerUpdateSkipReason( + tt.originalSource, tt.currentSource, tt.currentVersion, tt.newVersion, + )) + }) + } +} diff --git a/sites/docs-devsy-sh/content/docs/developing-in-workspaces/secrets.mdx b/sites/docs-devsy-sh/content/docs/developing-in-workspaces/secrets.mdx index ad6fdd21fb..c130c59e3e 100644 --- a/sites/docs-devsy-sh/content/docs/developing-in-workspaces/secrets.mdx +++ b/sites/docs-devsy-sh/content/docs/developing-in-workspaces/secrets.mdx @@ -262,6 +262,10 @@ devsy secret detach DB_PASSWORD devsy secret detach sops:project/API_TOKEN ``` +Attachments are context-scoped and contain only the secret reference, never +the value. The Desktop Secrets page shows and changes the same attachment +state. + Devsy stores only the reference for an external secret. Repository-owned `customizations.devsy` can declare project-specific automatic bindings with its `secrets` list. If any requested or attached value cannot be @@ -378,3 +382,7 @@ detached before converting it into an environment variable. This keeps the non-sensitive environment path separate from protected secret delivery. If a stored value is deleted, Devsy removes its context attachment as part of the same operation. + +Sensitive values use `devsy secret attach` instead; attached secrets are +injected through the protected secret-delivery paths described above, and the +Desktop Secrets page manages their attachment state.