From fb20667521029222fe33067cd014d40bbecad272 Mon Sep 17 00:00:00 2001 From: benbenbang Date: Fri, 9 Oct 2026 15:37:54 +0800 Subject: [PATCH] feat(config): add safe initialization and view helpers Add file helpers that stage templates with 0600 permissions, create new parents with 0700 permissions, publish through hardlinks without replacing existing entries, clean their own staging files, and verify context. Use shared pure configuration validation to reject unsafe URL fields and invalid types. View ignores environment overrides, comments, and unknown keys while redacting only api_key. Parent storage must be trusted because mode bits do not enforce ACL privacy. Add helper tests and update the example configuration and command plan. --- config.example.toml | 1 + docs/internal/config-commands.md | 9 ++- internal/config/config.go | 11 ++- internal/config/files.go | 118 +++++++++++++++++++++++++++++++ internal/config/files_test.go | 104 +++++++++++++++++++++++++++ internal/config/template.toml | 7 ++ 6 files changed, 247 insertions(+), 3 deletions(-) create mode 100644 internal/config/files.go create mode 100644 internal/config/files_test.go create mode 100644 internal/config/template.toml diff --git a/config.example.toml b/config.example.toml index 1bed9de..2bd6957 100644 --- a/config.example.toml +++ b/config.example.toml @@ -5,3 +5,4 @@ base_url = "https://api.typesafe.ai" model = "jev-latest" provider = "jev" timeout = "10s" +editor = "vim" diff --git a/docs/internal/config-commands.md b/docs/internal/config-commands.md index 0e04fb4..4a3c974 100644 --- a/docs/internal/config-commands.md +++ b/docs/internal/config-commands.md @@ -1,10 +1,15 @@ # Config command plan - 27: centralize discovery: explicit `--config` > current-directory `jevwise.toml` > existing XDG/HOME `ts-jev/config.toml`; selected files replace, not merge. Preserve flags/environment precedence and credential-free help. Bound reads and reject nonregular local files. Never search ancestor directories. -- 28: add `config init` and redacted `config view`; init defaults global, `--local` creates `./jevwise.toml`, explicit paths supported, conflicting selectors rejected. Publish a private template without overwriting any existing file/link; view shows known file settings/defaults, not environment overrides or arbitrary extra keys/comments. -- 29: add `config edit`, default installed `vim` on PATH for macOS/Linux; configurable `editor` executable name/path, no shell or embedded editor. Forward terminal streams, test with injected/fake processes, safe errors and cancellation. No Windows editor support promise; preserve cross-builds. +- 28: add tested file helpers for initialization and redacted viewing; init defaults global, `--local` creates `./jevwise.toml`, explicit paths supported, conflicting selectors rejected. Publish a mode-restricted template in trusted parent storage without overwriting any existing file/link; view shows known file settings/defaults, not environment overrides or arbitrary extra keys/comments. +- 29: wire `config init`/`view` commands and CLI tests/docs. +- 30: add `config edit`, default installed `vim` on PATH for macOS/Linux; configurable `editor` executable name/path, no shell or embedded editor. Forward terminal streams, test with injected/fake processes, safe errors and cancellation. No Windows editor support promise; preserve cross-builds. - Worktrees under `.agent/worktrees`, linear `virtual-main` branches; soft 300/hard 450 aggregate lines each. Local commits via approved git ch/OpenAI/luna; no push/merge/tag/release without approval. - Local auto-load requires trusting the current directory, especially its base URL. Do not copy environment API keys into templates or show them in view; add local file to project ignore rules before storing credentials. Existing provider/SDK/config namespace and Windows dependency deferral remain unchanged. - Root and independent QA: fresh normal/race tests, tidy/vet/build, offline Python fixtures, formatting/diff checks, six platform builds and synthetic credential-free CLI smoke. Never launch a real editor/agent or use normal user config during tests. Private spec remains unstaged. - 27 local QA passed: root normal/race/tidy/vet/build, 11 offline Python fixtures, formatting/diff/docs checks and six platform builds; independent full tests and config race repeated 20 times. No network/model/editor calls. + +- Implementation split adjusted to four small branches so tests and documentation remain below the 300-line target; behavior unchanged. + +- 28 local QA passed: root fresh full gates and six builds; independent full tests and config race repeated 20 times. Initialization preserves existing entries and cleans staging; file view ignores environment and redacts only api_key. Mode bits are not ACL enforcement, so require trusted parent ownership/ACLs in public CLI docs. diff --git a/internal/config/config.go b/internal/config/config.go index 17cb4be..91ab7d6 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -26,7 +26,7 @@ type Config struct { // Neither global Viper state nor the supplied flags are mutated. func Load(path string, flags *pflag.FlagSet) (Config, error) { v := viper.New() - defaults := map[string]string{"api_key": "", "base_url": "https://api.typesafe.ai", "model": "jev-latest", "provider": "jev", "timeout": "10s"} + defaults := defaultValues() for key, value := range defaults { v.SetDefault(key, value) } @@ -71,7 +71,16 @@ func Load(path string, flags *pflag.FlagSet) (Config, error) { } values[key] = strings.TrimSpace(value) } + return fromValues(values) +} + +func defaultValues() map[string]string { + return map[string]string{"api_key": "", "base_url": "https://api.typesafe.ai", "model": "jev-latest", "provider": "jev", "timeout": "10s"} +} + +func fromValues(values map[string]string) (Config, error) { c := Config{APIKey: values["api_key"], BaseURL: values["base_url"], Model: values["model"], Provider: values["provider"]} + var err error c.Timeout, err = time.ParseDuration(values["timeout"]) if err != nil { return Config{}, errors.New("timeout must be a positive duration") diff --git a/internal/config/files.go b/internal/config/files.go new file mode 100644 index 0000000..f50ea19 --- /dev/null +++ b/internal/config/files.go @@ -0,0 +1,118 @@ +package config + +import ( + "bytes" + "context" + "crypto/rand" + _ "embed" + "encoding/json" + "errors" + "io" + "os" + "path/filepath" + "strings" + + "github.com/spf13/viper" +) + +//go:embed template.toml +var template []byte + +// Init publishes a mode-restricted template without copying environment credentials or +// replacing any existing entry. Root confinement assumes trusted parent storage. +func Init(ctx context.Context, path string) (err error) { + if ctx == nil { + return errors.New("configuration context is required") + } + if err := ctx.Err(); err != nil { + return err + } + parent := filepath.Dir(path) + if err := os.MkdirAll(parent, 0700); err != nil { + return err + } + root, err := os.OpenRoot(parent) + if err != nil { + return err + } + defer root.Close() + stage := ".config-" + rand.Text() + ".tmp" + file, err := root.OpenFile(stage, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0600) + if err != nil { + return err + } + published := false + defer func() { + _ = file.Close() + if cleanup := root.Remove(stage); cleanup != nil { + if published { + err = errors.New("configuration created but staging cleanup failed") + } else { + err = errors.Join(err, errors.New("cannot clean configuration staging file")) + } + } + }() + if n, err := file.Write(template); err != nil { + return err + } else if n != len(template) { + return io.ErrShortWrite + } + if err := file.Sync(); err != nil { + return err + } + if err := file.Close(); err != nil { + return err + } + if err := ctx.Err(); err != nil { + return err + } + if err := root.Link(stage, filepath.Base(path)); err != nil { + return err + } + published = true + return nil +} + +func fileValues(path string) (map[string]string, error) { + data, err := ReadFile(path) + if err != nil { + return nil, errors.New("cannot read configuration file; initialize it first") + } + v := viper.New() + v.SetConfigType("toml") + if err := v.ReadConfig(bytes.NewReader(data)); err != nil { + return nil, errors.New("cannot read configuration: invalid TOML") + } + defaults := defaultValues() + defaults["editor"] = "vim" + values := make(map[string]string) + for key, value := range defaults { + v.SetDefault(key, value) + value, ok := v.Get(key).(string) + if !ok { + return nil, errors.New("configuration settings must be strings") + } + values[key] = strings.TrimSpace(value) + } + return values, nil +} + +// View shows known file values/defaults, never environment overrides, arbitrary +// extra keys or comments. Only api_key is redacted; URL userinfo/query/fragment +// are refused before display. Other settings must not contain credentials. +func View(path string) (string, error) { + values, err := fileValues(path) + if err != nil { + return "", err + } + if _, err := fromValues(values); err != nil { + return "", err + } + values["api_key"] = "[redacted]" + var out strings.Builder + for _, key := range []string{"api_key", "base_url", "model", "provider", "timeout", "editor"} { + value, _ := json.Marshal(values[key]) + out.WriteString(key + " = " + string(value) + "\n") + } + return out.String(), nil +} diff --git a/internal/config/files_test.go b/internal/config/files_test.go new file mode 100644 index 0000000..76f2924 --- /dev/null +++ b/internal/config/files_test.go @@ -0,0 +1,104 @@ +package config + +import ( + "context" + "errors" + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +func TestInitPrivateTemplateNoOverwriteOrCredentialCopy(t *testing.T) { + cleanEnv(t) + t.Setenv("TYPESAFE_API_KEY", "environment-secret") + path := filepath.Join(t.TempDir(), "new", "config.toml") + if err := Init(context.Background(), path); err != nil { + t.Fatal(err) + } + data, _ := os.ReadFile(path) + info, _ := os.Stat(path) + parent, _ := os.Stat(filepath.Dir(path)) + cfg, err := Load(path, nil) + if err != nil || cfg.Model != "jev-latest" || strings.Contains(string(data), "environment-secret") { + t.Fatal("template invalid or copied credentials", err) + } + if runtime.GOOS != "windows" && (info.Mode().Perm() != 0600 || parent.Mode().Perm() != 0700) { + t.Fatal("template or new parent is not private") + } + if err := Init(context.Background(), path); !errors.Is(err, os.ErrExist) { + t.Fatal("existing file accepted", err) + } + after, _ := os.ReadFile(path) + entries, _ := os.ReadDir(filepath.Dir(path)) + if string(after) != string(data) || len(entries) != 1 { + t.Fatal("existing file changed or staging leaked") + } + for _, kind := range []string{"directory", "link"} { + target := filepath.Join(filepath.Dir(path), kind) + if kind == "link" { + if runtime.GOOS == "windows" { + continue + } + if err := os.Symlink(path, target); err != nil { + t.Fatal(err) + } + } else { + _ = os.Mkdir(target, 0700) + } + if err := Init(context.Background(), target); err == nil { + t.Fatal("existing entry replaced") + } + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + missing := filepath.Join(t.TempDir(), "absent", "config.toml") + if err := Init(ctx, missing); !errors.Is(err, context.Canceled) { + t.Fatal("cancellation ignored", err) + } + if _, err := os.Stat(filepath.Dir(missing)); !errors.Is(err, os.ErrNotExist) { + t.Fatal("cancelled init changed storage") + } +} + +func TestViewOnlyKnownFileValuesAndRedactsSecrets(t *testing.T) { + cleanEnv(t) + t.Setenv("TS_JEV_MODEL", "environment-model") + path := writeConfig(t, filepath.Join(t.TempDir(), "config.toml"), "API_KEY='file-secret'\nmodel='file-model'\nextra='extra-secret'\n# comment-secret\n") + text, err := View(path) + if err != nil || !strings.Contains(text, `model = "file-model"`) || !strings.Contains(text, `editor = "vim"`) || !strings.Contains(text, "[redacted]") { + t.Fatal("wrong file view", err) + } + for _, secret := range []string{"file-secret", "extra-secret", "comment-secret", "environment-model"} { + if strings.Contains(text, secret) { + t.Fatal("view leaked non-file or secret data") + } + } + for _, content := range []string{"api_key='parse-secret'\n[", "base_url='https://user:url-secret@example.com'", "editor=42"} { + writeConfig(t, path, content) + if out, err := View(path); out != "" || err == nil || strings.Contains(err.Error(), "secret") { + t.Fatal("invalid file accepted or leaked", err) + } + } +} + +func TestReadFileBoundaryAndExplicitReadOnlyLink(t *testing.T) { + cleanEnv(t) + path := writeConfig(t, filepath.Join(t.TempDir(), "config.toml"), "#"+strings.Repeat("x", MaxFileBytes-2)+"\n") + if data, err := ReadFile(path); err != nil || len(data) != MaxFileBytes { + t.Fatal("exact boundary refused", err) + } + if _, err := Load(path, nil); err != nil { + t.Fatal("boundary TOML refused", err) + } + if runtime.GOOS != "windows" { + link := path + ".link" + if err := os.Symlink(path, link); err != nil { + t.Fatal(err) + } + if _, err := Load(link, nil); err != nil { + t.Fatal("explicit read-only link compatibility lost", err) + } + } +} diff --git a/internal/config/template.toml b/internal/config/template.toml new file mode 100644 index 0000000..92a7cf9 --- /dev/null +++ b/internal/config/template.toml @@ -0,0 +1,7 @@ +# Optional CLI configuration. Prefer environment credentials. +api_key = "" +base_url = "https://api.typesafe.ai" +model = "jev-latest" +provider = "jev" +timeout = "10s" +editor = "vim"