diff --git a/README.md b/README.md index 744bff2..e50cce8 100644 --- a/README.md +++ b/README.md @@ -121,17 +121,23 @@ Precedence: **changed flags > nonblank environment > TOML > defaults**. - Trust the current directory before running decision/MCP commands: local settings can change the upstream destination. Environment/changed flags still win. Invalid, oversized (over 1 MiB), linked or nonregular local files fail without global fallback; help/version do not load them. - Start with [config.example.toml](config.example.toml). Prefer environment credentials rather than command-line secrets. -### Initialize and inspect +### Initialize, inspect and edit ```sh jevwise config init # Global XDG/HOME config, even if local exists jevwise config init --local # ./jevwise.toml, automatically selected in this cwd jevwise config view # Selected file/defaults, API key redacted jevwise config view --config /path/to/config.toml +jevwise config edit # Selected local/global file, vim by default +jevwise config edit --editor nvim # One-shot override, also for repairs ``` - Init never overwrites existing files, directories or links. It writes a template, not your environment credentials; Unix file permissions are requested as `0600`, new parent directories as `0700`. Use a trusted parent with safe ownership/ACLs: mode bits alone do not guarantee privacy, including inherited Unix ACLs. Windows users must manage ACLs separately. - `--config PATH` also selects an init destination; do not combine it with `--local` or pass a blank path. View ignores environment/decision flag overrides, unknown keys and comments; invalid files fail safely. It is not a dump of effective runtime settings. Only the `api_key` field is automatically redacted; do not store credentials in other displayed settings. +- Set `editor = "nvim"` (or another executable name/path) in TOML to change the editor. Selection is `config edit --editor` > selected file's `editor` > `vim` on PATH; `$EDITOR`/`$VISUAL` are not read. No editor is bundled and no shell is used. Arguments are not parsed: use a trusted wrapper executable if options such as `--wait` are needed. +- Edit forwards terminal input/output and waits for the direct editor process. It uses the command context, not the decision timeout; missing editors/nonzero exits fail safely. Invalid TOML falls back to Vim for repair; wrong-typed editor settings or oversized files can be repaired using `--editor`. It refuses missing files, directories and final symlinks. +- Go-copy pipe waits are bounded after process exit/cancellation. Only the direct editor process is managed, not wrapper descendants; arbitrary blocking custom readers/writers cannot be forcibly interrupted. +- Trust local editor settings and wrappers before editing: they select a program to execute. Editor terminal output intentionally may show the file you are editing. macOS/Linux support assumes an installed editor; no Windows editor availability/behavior is promised. - Add `jevwise.toml` to your project's ignore rules before storing credentials. Prefer environment API keys. ## Skill setup diff --git a/docs/internal/config-commands.md b/docs/internal/config-commands.md index 6ee6e2d..b5369c0 100644 --- a/docs/internal/config-commands.md +++ b/docs/internal/config-commands.md @@ -16,3 +16,5 @@ - 29 local QA passed after correcting blank selectors and short output writes: root fresh full gates, six builds and isolated native init/view smoke; independent full tests and CLI config race repeated 20 times. Public docs qualify ACL/privacy assumptions and field-only redaction. - Follow-up outside this scope: shared Cobra help-template write errors may be printed directly to stderr; harden the shared HelpFunc separately, not just config commands. + +- 30 local QA passed: root fresh full gates/six builds and fake-editor native smoke (PATH/default/custom spaced path/repair/override/help); independent final full tests and editor/config race repeated 10 times. Bounded inherited-pipe waits and owned-helper completion are regression-tested. This cohesive security regression justifies a small soft-300 exception; hard 450 remains satisfied. No real editor, API, agent, push, tag or release used. diff --git a/internal/cli/config.go b/internal/cli/config.go index a5ec855..15b6a71 100644 --- a/internal/cli/config.go +++ b/internal/cli/config.go @@ -90,6 +90,6 @@ func newConfig() *cobra.Command { } return nil }} - cmd.AddCommand(init, view) + cmd.AddCommand(init, view, newConfigEdit(runConfigEditor)) return cmd } diff --git a/internal/cli/config_edit.go b/internal/cli/config_edit.go new file mode 100644 index 0000000..0abecf7 --- /dev/null +++ b/internal/cli/config_edit.go @@ -0,0 +1,65 @@ +package cli + +import ( + "context" + "errors" + "io" + "os" + "os/exec" + "strings" + "time" + + "github.com/bitbrew-dev/jevwise/internal/config" + "github.com/spf13/cobra" +) + +type configEditor func(context.Context, string, string, io.Reader, io.Writer, io.Writer) error + +func runConfigEditor(ctx context.Context, editor, path string, in io.Reader, out, stderr io.Writer) error { + process := exec.CommandContext(ctx, editor, path) // No shell; absolute file is one argument. + process.WaitDelay = 250 * time.Millisecond // Bound inherited Go-copy pipe waits. + process.Stdin, process.Stdout, process.Stderr = in, out, stderr + err := process.Run() + if ctx.Err() != nil { + return ctx.Err() + } + return err +} + +func newConfigEdit(run configEditor) *cobra.Command { + var override string + cmd := &cobra.Command{Use: "edit", Short: "Edit selected configuration (default: vim on PATH)", Args: configNoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + if err := cmd.Context().Err(); err != nil { + return err + } + path, err := configPathFlag(cmd) + if err != nil { + return err + } + path, _, err = config.ResolvePath(path) + if err != nil { + return err + } + info, err := os.Lstat(path) + if err != nil || !info.Mode().IsRegular() { + return errors.New("config edit requires an existing regular file, not a link; initialize it first") + } + editor := strings.TrimSpace(override) + if cmd.Flags().Changed("editor") && editor == "" { + return errors.New("editor executable must not be blank") + } + if editor == "" { + editor, err = config.Editor(path) + if err != nil { + return err + } + } + if err := run(cmd.Context(), editor, path, cmd.InOrStdin(), cmd.OutOrStdout(), cmd.ErrOrStderr()); err != nil { + return &decisionError{"cannot run editor; install vim or select one editor executable name/path", err} + } + return nil + }} + cmd.Flags().StringVar(&override, "editor", "", "Override editor executable for this edit (no shell or arguments)") + return cmd +} diff --git a/internal/cli/config_edit_test.go b/internal/cli/config_edit_test.go new file mode 100644 index 0000000..61e345f --- /dev/null +++ b/internal/cli/config_edit_test.go @@ -0,0 +1,185 @@ +package cli + +import ( + "bytes" + "context" + "errors" + "io" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/bitbrew-dev/jevwise/internal/config" + "github.com/spf13/cobra" +) + +func editorRoot(t *testing.T, run configEditor) *cobra.Command { + root := configRoot(t) + group, _, _ := root.Find([]string{"config"}) + edit, _, _ := root.Find([]string{"config", "edit"}) + group.RemoveCommand(edit) + group.AddCommand(newConfigEdit(run)) + return root +} + +func TestConfigEditSelectionStreamsAndRepair(t *testing.T) { + for _, mode := range []string{"default", "local", "global", "explicit", "malformed", "override"} { + t.Run(mode, func(t *testing.T) { + configEnvironment(t) + path, _ := filepath.Abs(config.LocalName) + content, want := "", "vim" + args := []string{"config", "edit"} + if mode == "global" { + path, _ = config.GlobalPath() + } + if mode == "explicit" { + path = filepath.Join(t.TempDir(), "selected file.toml") + args = append(args, "--config", path) + _ = os.WriteFile(config.LocalName, []byte("editor='wrong-local'"), 0600) + } + if mode == "local" || mode == "global" || mode == "explicit" { + content, want = "editor='configured editor'", "configured editor" + } + if mode == "malformed" { + content = "api_key='private-secret'\n[" + } + if mode == "override" { + content, want = "editor=42", "override-editor" + args = append(args, "--editor", want) + } + _ = os.MkdirAll(filepath.Dir(path), 0700) + _ = os.WriteFile(path, []byte(content), 0600) + calls := 0 + root := editorRoot(t, func(ctx context.Context, editor, selected string, in io.Reader, out, stderr io.Writer) error { + calls++ + input, _ := io.ReadAll(in) + if editor != want || selected != path || string(input) != "typed" || ctx.Err() != nil { + t.Fatal("wrong editor/path/streams/context") + } + _, _ = io.WriteString(out, "editor-output") + _, _ = io.WriteString(stderr, "editor-diagnostic") + return nil + }) + root.SetIn(strings.NewReader("typed")) + out, stderr, err := execute(root, args...) + if err != nil || calls != 1 || out != "editor-output" || stderr != "editor-diagnostic" { + t.Fatal("editor not invoked correctly", err) + } + }) + } +} + +func TestConfigEditInvalidRequestsHelpAndSafeFailure(t *testing.T) { + configEnvironment(t) + for _, args := range [][]string{{"config", "edit", "--help", "--config", "/missing"}, {"config", "edit"}, {"config", "edit", "private-secret"}, {"config", "edit", "--config="}} { + out, _, err := execute(editorRoot(t, func(context.Context, string, string, io.Reader, io.Writer, io.Writer) error { + t.Fatal("invalid/help started editor") + return nil + }), args...) + if strings.Contains(strings.Join(args, " "), "--help") { + if err != nil || !strings.Contains(out, "--editor") { + t.Fatal("edit help failed", err) + } + } else if err == nil || out != "" || strings.Contains(err.Error(), "private-secret") { + t.Fatal("invalid edit accepted or leaked", err) + } + } + _ = os.WriteFile(config.LocalName, []byte("editor='vim'"), 0600) + if _, _, err := execute(editorRoot(t, func(context.Context, string, string, io.Reader, io.Writer, io.Writer) error { + return errors.New("private-secret") + }), "config", "edit"); err == nil || strings.Contains(err.Error(), "private-secret") { + t.Fatal("editor error leaked", err) + } + if runtime.GOOS != "windows" { + _ = os.Symlink(config.LocalName, "linked.toml") + if _, _, err := execute(editorRoot(t, func(context.Context, string, string, io.Reader, io.Writer, io.Writer) error { + t.Fatal("linked editor target accepted") + return nil + }), "config", "edit", "--config", "linked.toml"); err == nil { + t.Fatal("symlink accepted") + } + } +} + +func TestConfigEditorProcessNoShellAndCancellation(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("installed editor behavior supported/tested on Unix") + } + directory := t.TempDir() + editor := filepath.Join(directory, "vim") + t.Setenv("PATH", directory+string(os.PathListSeparator)+os.Getenv("PATH")) + _ = os.WriteFile(editor, []byte("#!/bin/sh\nprintf '%s\\n' \"$#\" \"$1\"\ncat\nprintf 'diagnostic' >&2\n"), 0700) + path := filepath.Join(directory, "config ; not a command.toml") + var out, stderr bytes.Buffer + if err := runConfigEditor(context.Background(), "vim", path, strings.NewReader("typed"), &out, &stderr); err != nil || out.String() != "1\n"+path+"\ntyped" || stderr.String() != "diagnostic" { + t.Fatal("process arguments/streams changed", err) + } + marker := filepath.Join(directory, "not-created") + if err := runConfigEditor(context.Background(), "sh -c 'touch "+marker+"'", path, nil, io.Discard, io.Discard); err == nil { + t.Fatal("shell fragment executed") + } + if _, err := os.Stat(marker); !errors.Is(err, os.ErrNotExist) { + t.Fatal("shell side effect") + } + _ = os.WriteFile(editor, []byte("#!/bin/sh\nexit 7\n"), 0700) + if err := runConfigEditor(context.Background(), editor, path, nil, io.Discard, io.Discard); err == nil { + t.Fatal("nonzero editor exit accepted") + } + _ = os.WriteFile(editor, []byte("#!/bin/sh\nexec sleep 30\n"), 0700) + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + if err := runConfigEditor(ctx, editor, path, nil, io.Discard, io.Discard); !errors.Is(err, context.DeadlineExceeded) { + t.Fatal("editor cancellation ignored", err) + } +} + +func TestConfigEditRejectsBadEditorAndCancelledRequests(t *testing.T) { + configEnvironment(t) + _ = os.WriteFile(config.LocalName, []byte("editor=42"), 0600) + for _, args := range [][]string{{"config", "edit"}, {"config", "edit", "--editor="}, {"config", "edit", "--config", "."}} { + if _, _, err := execute(editorRoot(t, func(context.Context, string, string, io.Reader, io.Writer, io.Writer) error { + t.Fatal("invalid editor request launched") + return nil + }), args...); err == nil { + t.Fatal("invalid editor accepted") + } + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + root := editorRoot(t, func(context.Context, string, string, io.Reader, io.Writer, io.Writer) error { + t.Fatal("cancelled edit launched") + return nil + }) + root.SetArgs([]string{"config", "edit", "--editor", "vim"}) + if err := root.ExecuteContext(ctx); !errors.Is(err, context.Canceled) { + t.Fatal("cancelled edit accepted", err) + } +} + +func TestConfigEditorBoundsInheritedPipeWait(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Unix synthetic editor helper") + } + dir := t.TempDir() + path, editor := filepath.Join(dir, "config.toml"), filepath.Join(dir, "helper") + _ = os.WriteFile(editor, []byte("#!/bin/sh\n(sleep 1; printf done > \"$1.finished\") &\nexit 0\n"), 0700) + var out, stderr bytes.Buffer + if err := runConfigEditor(context.Background(), editor, path, nil, &out, &stderr); !errors.Is(err, exec.ErrWaitDelay) { + t.Fatal("inherited output pipe was not bounded", err) + } + // Wait for the owned, bounded helper to finish before deleting its files. + deadline := time.Now().Add(5 * time.Second) + for { + if _, err := os.Stat(path + ".finished"); err == nil { + break + } + if time.Now().After(deadline) { + t.Fatal("synthetic helper did not finish") + } + time.Sleep(10 * time.Millisecond) + } +} diff --git a/internal/config/editor.go b/internal/config/editor.go new file mode 100644 index 0000000..3a54960 --- /dev/null +++ b/internal/config/editor.go @@ -0,0 +1,35 @@ +package config + +import ( + "bytes" + "errors" + "strings" + + "github.com/spf13/viper" +) + +// Editor reads only the selected file's editor setting, not environment/SDK +// configuration. Invalid TOML falls back to vim so the file can be repaired. +func Editor(path string) (string, error) { + data, err := ReadFile(path) + if err != nil { + return "", errors.New("cannot read configuration to choose editor; use --editor to repair") + } + v := viper.New() + v.SetConfigType("toml") + if err := v.ReadConfig(bytes.NewReader(data)); err != nil { + return "vim", nil + } + value := v.Get("editor") + if value == nil { + return "vim", nil + } + editor, ok := value.(string) + if !ok { + return "", errors.New("editor must be one executable name or path; use --editor to repair") + } + if strings.TrimSpace(editor) == "" { + return "vim", nil + } + return strings.TrimSpace(editor), nil +} diff --git a/internal/config/editor_test.go b/internal/config/editor_test.go new file mode 100644 index 0000000..cdfdcda --- /dev/null +++ b/internal/config/editor_test.go @@ -0,0 +1,29 @@ +package config + +import ( + "path/filepath" + "strings" + "testing" +) + +func TestEditorUsesOnlyFileAndCanRepairInvalidTOML(t *testing.T) { + cleanEnv(t) + t.Setenv("EDITOR", "environment-editor") + t.Setenv("VISUAL", "environment-visual") + t.Setenv("TS_JEV_EDITOR", "environment-editor") + for _, tc := range []struct { + content, want string + invalid bool + }{ + {"", "vim", false}, {"editor='nvim'", "nvim", false}, {"editor=' '", "vim", false}, + {"editor='/some/editor with spaces'", "/some/editor with spaces", false}, + {"model=42\neditor='nano'", "nano", false}, {"api_key='private-secret'\n[", "vim", false}, + {"editor=42", "", true}, + } { + path := writeConfig(t, filepath.Join(t.TempDir(), "config.toml"), tc.content) + got, err := Editor(path) + if got != tc.want || (err != nil) != tc.invalid || err != nil && strings.Contains(err.Error(), "private-secret") { + t.Fatal("editor selection or safe error failed", err) + } + } +}