fj/internal/config/config.go

271 lines
6.6 KiB
Go
Raw Permalink Normal View History

2025-12-08 09:49:07 +01:00
package config
import (
"fmt"
"os"
"path/filepath"
"strings"
2025-12-08 09:49:07 +01:00
"github.com/spf13/viper"
"gopkg.in/yaml.v3"
)
type Config struct {
Hosts map[string]HostConfig `yaml:"hosts"`
}
fix(cmd): correctness + audit hardening across cmd/ + internal/ Addresses audit findings from a tri-partite review (codex + 2 Claude agents). Multiple distinct fixes here because they touched overlapping files; happy to split via interactive rebase if a reviewer prefers. ## Correctness bugs (HIGH) * `--config` is now actually honored. cmd/root.initConfig fed Viper but every command that mattered loaded config via `internal/config.Load()` which always read the default path. Added `config.SetExplicitConfigPath` consulted by `GetConfigPath`; `--config other.yaml auth login` now writes to other.yaml. - internal/config/config.go, cmd/root.go * `--json` now works on `fj run …`, `fj workflow …`, and `fj wiki view`. cmd/aliases.go registered `--json` as a Bool but the handlers call `wantJSON()` which does `GetString("json")` and silently ignores the type-error return. cmd/wiki.go did the inverse (`GetBool("json")` against a string-registered flag). Both now use `addJSONFlags`/`wantJSON`/ `outputJSON` consistently. - cmd/aliases.go, cmd/wiki.go * `fj api` no longer lets endpoints escape the /api/v1 base via path-traversal. `fj api '/../admin/users'` previously normalized to `/admin/users` because `http.NewRequest` resolves `..` segments — silently sending authenticated traffic to non-API routes. Endpoint is now parsed, `..` segments are rejected, and JoinPath is used. - cmd/api.go ## Design rework (BREAKING — gets rid of the `--json=fields` quirk) * `--json` flag rebuilt from a string-with-NoOptDefVal=" " sentinel into a plain Bool. `--json-fields` keeps comma-separated projection. The two are mutually exclusive (`MarkFlagsMutuallyExclusive`). `--jq` composes with either or neither. The previous design produced a `--json string[=" "]` in --help and required `--json=fields` (with literal "=") because `--json fields` was parsed as the bare flag plus a positional. Gone. - cmd/json.go: addJSONFlags / wantJSON / outputJSON - cmd/api.go: example block reflects the new shape Migration: `--json=fields` → `--json-fields fields`. Bare `--json` still means "everything as JSON". * `fj api` now uses `internal/api.SharedHTTPClient` (30 s timeout, pooled) instead of constructing a zero-value `&http.Client{}` with no timeout. A hung Forgejo no longer pins the CLI indefinitely. Response body is also bounded by `io.LimitReader` at 64 MB to prevent OOM-on-self. - internal/api/client.go (export SharedHTTPClient), cmd/api.go * `--hostname` declared as a persistent flag on rootCmd is now the only declaration. cmd/auth.go re-declared `--hostname` on three subcommands, shadowing the persistent flag — meaning `fj --hostname=X auth login` and `fj auth login --hostname=X` went through different code paths (viper read vs. local flag read). Local declarations removed. - cmd/auth.go ## Hardening (MEDIUM/LOW) * `--token` on `auth login` now emits a stderr warning when used, since it puts the PAT on argv (visible in `ps auxe`/shell history). Flag not removed — too disruptive — but discoverable now. - cmd/auth.go * Error handling no longer regex-matches "401"/"403" against rendered error strings (would have triggered "auth login" hint for an error that just mentioned issue #403). Now relies on typed `*api.APIError`. Hints moved to a separate `Hint` field on `CLIError`, so JSON-error consumers get clean structure and the human renderer still appends "\nHint: …". - cmd/errors.go * `migrateConfigDir` now opens dst with `O_TRUNC` instead of just `O_CREATE|O_WRONLY`. Previously a partially-pre-existing dst file would have legacy contents overwrite a prefix and leave stale tail bytes — silent YAML/token corruption. - cmd/root.go (extracted into copyOneConfigFile with proper close handling) * Config dir created with mode 0700 instead of 0755. `initConfig` warns on stderr if the resolved config file is world/group readable (`mode & 0o077 != 0`); doesn't fail-close. - cmd/root.go * Network errors (`no such host`, `connection refused`, `i/o timeout`) now return a structured `CLIError` with code `ErrNetworkError` and a hint, instead of a fmt.Errorf chain. - cmd/errors.go Verified: `go build ./...` and `go test ./...` clean. Live integration tested against forgejo.zerova.net. Out of scope, deferred to follow-up commits: - Pagination unification across `repo list`/`pr list`/`issue list` (only `release list` walks pages today; others silently truncate). - `fj api --paginate` to follow pages like `gh api --paginate`. - De-duplicating cmd/aliases.go ↔ cmd/actions.go subtrees.
2026-05-02 15:41:48 -06:00
// explicitConfigPath, when non-empty, overrides the default config file
// location for both Load() and Save(). It's set by cmd/root.initConfig when
// the user passes --config <path>. Stored at package scope so existing
// call sites of config.Load()/c.Save() continue to work without each one
// having to know about the flag.
var explicitConfigPath string
// SetExplicitConfigPath wires a user-supplied --config path through to
// Load/Save. Pass "" to clear.
func SetExplicitConfigPath(p string) { explicitConfigPath = p }
2025-12-08 09:49:07 +01:00
type HostConfig struct {
Hostname string `yaml:"hostname"`
Token string `yaml:"token"`
User string `yaml:"user,omitempty"`
GitProtocol string `yaml:"git_protocol,omitempty"`
MatchDirs []string `yaml:"match_dirs,omitempty"`
Order int `yaml:"-"` // config file order, set at load time
2025-12-08 09:49:07 +01:00
}
func GetConfigDir() (string, error) {
2026-03-12 15:44:24 +01:00
if xdgConfigHome := os.Getenv("XDG_CONFIG_HOME"); xdgConfigHome != "" {
return filepath.Join(xdgConfigHome, "fj"), nil
2026-03-12 15:44:24 +01:00
}
2025-12-08 09:49:07 +01:00
home, err := os.UserHomeDir()
if err != nil {
return "", err
}
return filepath.Join(home, ".config", "fj"), nil
2025-12-08 09:49:07 +01:00
}
func GetConfigPath() (string, error) {
fix(cmd): correctness + audit hardening across cmd/ + internal/ Addresses audit findings from a tri-partite review (codex + 2 Claude agents). Multiple distinct fixes here because they touched overlapping files; happy to split via interactive rebase if a reviewer prefers. ## Correctness bugs (HIGH) * `--config` is now actually honored. cmd/root.initConfig fed Viper but every command that mattered loaded config via `internal/config.Load()` which always read the default path. Added `config.SetExplicitConfigPath` consulted by `GetConfigPath`; `--config other.yaml auth login` now writes to other.yaml. - internal/config/config.go, cmd/root.go * `--json` now works on `fj run …`, `fj workflow …`, and `fj wiki view`. cmd/aliases.go registered `--json` as a Bool but the handlers call `wantJSON()` which does `GetString("json")` and silently ignores the type-error return. cmd/wiki.go did the inverse (`GetBool("json")` against a string-registered flag). Both now use `addJSONFlags`/`wantJSON`/ `outputJSON` consistently. - cmd/aliases.go, cmd/wiki.go * `fj api` no longer lets endpoints escape the /api/v1 base via path-traversal. `fj api '/../admin/users'` previously normalized to `/admin/users` because `http.NewRequest` resolves `..` segments — silently sending authenticated traffic to non-API routes. Endpoint is now parsed, `..` segments are rejected, and JoinPath is used. - cmd/api.go ## Design rework (BREAKING — gets rid of the `--json=fields` quirk) * `--json` flag rebuilt from a string-with-NoOptDefVal=" " sentinel into a plain Bool. `--json-fields` keeps comma-separated projection. The two are mutually exclusive (`MarkFlagsMutuallyExclusive`). `--jq` composes with either or neither. The previous design produced a `--json string[=" "]` in --help and required `--json=fields` (with literal "=") because `--json fields` was parsed as the bare flag plus a positional. Gone. - cmd/json.go: addJSONFlags / wantJSON / outputJSON - cmd/api.go: example block reflects the new shape Migration: `--json=fields` → `--json-fields fields`. Bare `--json` still means "everything as JSON". * `fj api` now uses `internal/api.SharedHTTPClient` (30 s timeout, pooled) instead of constructing a zero-value `&http.Client{}` with no timeout. A hung Forgejo no longer pins the CLI indefinitely. Response body is also bounded by `io.LimitReader` at 64 MB to prevent OOM-on-self. - internal/api/client.go (export SharedHTTPClient), cmd/api.go * `--hostname` declared as a persistent flag on rootCmd is now the only declaration. cmd/auth.go re-declared `--hostname` on three subcommands, shadowing the persistent flag — meaning `fj --hostname=X auth login` and `fj auth login --hostname=X` went through different code paths (viper read vs. local flag read). Local declarations removed. - cmd/auth.go ## Hardening (MEDIUM/LOW) * `--token` on `auth login` now emits a stderr warning when used, since it puts the PAT on argv (visible in `ps auxe`/shell history). Flag not removed — too disruptive — but discoverable now. - cmd/auth.go * Error handling no longer regex-matches "401"/"403" against rendered error strings (would have triggered "auth login" hint for an error that just mentioned issue #403). Now relies on typed `*api.APIError`. Hints moved to a separate `Hint` field on `CLIError`, so JSON-error consumers get clean structure and the human renderer still appends "\nHint: …". - cmd/errors.go * `migrateConfigDir` now opens dst with `O_TRUNC` instead of just `O_CREATE|O_WRONLY`. Previously a partially-pre-existing dst file would have legacy contents overwrite a prefix and leave stale tail bytes — silent YAML/token corruption. - cmd/root.go (extracted into copyOneConfigFile with proper close handling) * Config dir created with mode 0700 instead of 0755. `initConfig` warns on stderr if the resolved config file is world/group readable (`mode & 0o077 != 0`); doesn't fail-close. - cmd/root.go * Network errors (`no such host`, `connection refused`, `i/o timeout`) now return a structured `CLIError` with code `ErrNetworkError` and a hint, instead of a fmt.Errorf chain. - cmd/errors.go Verified: `go build ./...` and `go test ./...` clean. Live integration tested against forgejo.zerova.net. Out of scope, deferred to follow-up commits: - Pagination unification across `repo list`/`pr list`/`issue list` (only `release list` walks pages today; others silently truncate). - `fj api --paginate` to follow pages like `gh api --paginate`. - De-duplicating cmd/aliases.go ↔ cmd/actions.go subtrees.
2026-05-02 15:41:48 -06:00
if explicitConfigPath != "" {
return explicitConfigPath, nil
}
2025-12-08 09:49:07 +01:00
dir, err := GetConfigDir()
if err != nil {
return "", err
}
return filepath.Join(dir, "config.yaml"), nil
}
func Load() (*Config, error) {
path, err := GetConfigPath()
if err != nil {
return nil, err
}
2025-12-08 10:14:47 +01:00
return LoadFromPath(path)
}
2025-12-08 09:49:07 +01:00
2025-12-08 10:14:47 +01:00
func LoadFromPath(path string) (*Config, error) {
2025-12-08 09:49:07 +01:00
data, err := os.ReadFile(path)
if err != nil {
if os.IsNotExist(err) {
return &Config{Hosts: make(map[string]HostConfig)}, nil
}
return nil, err
}
var cfg Config
if err := yaml.Unmarshal(data, &cfg); err != nil {
return nil, err
}
if cfg.Hosts == nil {
cfg.Hosts = make(map[string]HostConfig)
}
// Parse again with yaml.Node to capture config file order for hosts
assignHostOrder(&cfg, data)
2025-12-08 09:49:07 +01:00
return &cfg, nil
}
// assignHostOrder walks the YAML document tree to find the "hosts" mapping
// and stamps each HostConfig.Order with its position in the file.
func assignHostOrder(cfg *Config, data []byte) {
var doc yaml.Node
if err := yaml.Unmarshal(data, &doc); err != nil || len(doc.Content) == 0 {
return
}
root := doc.Content[0] // mapping node
if root.Kind != yaml.MappingNode {
return
}
for i := 0; i+1 < len(root.Content); i += 2 {
if root.Content[i].Value == "hosts" {
hostsNode := root.Content[i+1]
if hostsNode.Kind != yaml.MappingNode {
return
}
order := 0
for j := 0; j+1 < len(hostsNode.Content); j += 2 {
key := hostsNode.Content[j].Value
if h, ok := cfg.Hosts[key]; ok {
h.Order = order
cfg.Hosts[key] = h
order++
}
}
return
}
}
}
2025-12-08 09:49:07 +01:00
func (c *Config) Save() error {
path, err := GetConfigPath()
if err != nil {
return err
}
2025-12-08 10:14:47 +01:00
return c.SaveToPath(path)
}
2025-12-08 09:49:07 +01:00
2025-12-08 10:14:47 +01:00
func (c *Config) SaveToPath(path string) error {
2025-12-08 09:49:07 +01:00
dir := filepath.Dir(path)
if err := os.MkdirAll(dir, 0755); err != nil {
return err
}
data, err := yaml.Marshal(c)
if err != nil {
return err
}
return os.WriteFile(path, data, 0600)
}
2026-01-05 12:47:28 +01:00
// GetHost resolves the hostname to use for API client creation.
// Priority order:
// 1. Explicitly provided hostname parameter
// 2. CLI flag (--hostname)
// 3. Environment variable (FJ_HOST, with FGJ_HOST fallback)
2026-01-05 12:47:28 +01:00
// 4. Auto-detected hostname from git remote
// 5. match_dirs lookup (longest prefix match)
// 6. Default to codeberg.org
func (c *Config) GetHost(hostname string, detectedHost string, cwd string) (HostConfig, error) {
2025-12-08 09:49:07 +01:00
if hostname == "" {
hostname = viper.GetString("hostname")
}
if hostname == "" {
hostname = EnvWithFallback("FJ_HOST", "FGJ_HOST")
2025-12-08 09:49:07 +01:00
}
2026-01-05 12:47:28 +01:00
if hostname == "" {
hostname = detectedHost
}
if hostname == "" {
hostname = c.ResolveHostByPath(cwd)
}
2025-12-08 09:49:07 +01:00
if hostname == "" {
hostname = "codeberg.org"
}
host, ok := c.Hosts[hostname]
if !ok {
return HostConfig{}, fmt.Errorf("no configuration found for host %s", hostname)
}
return host, nil
}
// ResolveHostByPath finds the host whose match_dirs entry is the longest
// prefix of cwd. Returns "" if no match is found.
// Both cwd and match_dirs entries are resolved through filepath.EvalSymlinks
// to handle symlinks (e.g. macOS /tmp → /private/tmp).
// On ties (same prefix length from multiple hosts), the host appearing first
// in the config file wins and a warning is printed to stderr.
func (c *Config) ResolveHostByPath(cwd string) string {
if cwd == "" {
return ""
}
// Resolve symlinks in cwd so /tmp becomes /private/tmp on macOS, etc.
if resolved, err := filepath.EvalSymlinks(cwd); err == nil {
cwd = resolved
}
bestHost := ""
bestLen := 0
bestOrder := 0
tied := false
for hostname, host := range c.Hosts {
for _, dir := range host.MatchDirs {
if dir == "" {
continue
}
// Expand ~ to home directory
dir = expandHome(dir)
// Resolve symlinks in the configured dir as well
if resolved, err := filepath.EvalSymlinks(dir); err == nil {
dir = resolved
}
// Normalize: ensure trailing slash for prefix matching
prefix := dir
if !strings.HasSuffix(prefix, "/") {
prefix += "/"
}
// Match if cwd equals dir exactly or is under it
if cwd == dir || strings.HasPrefix(cwd, prefix) {
if len(dir) > bestLen {
bestLen = len(dir)
bestHost = hostname
bestOrder = host.Order
tied = false
} else if len(dir) == bestLen && hostname != bestHost {
// Tie — pick the host with the lower Order (earlier in config)
if host.Order < bestOrder {
bestHost = hostname
bestOrder = host.Order
}
tied = true
}
}
}
}
if tied {
fmt.Fprintf(os.Stderr, "warning: multiple hosts match directory %q with the same specificity; using %s (first in config)\n", cwd, bestHost)
}
return bestHost
}
// expandHome replaces a leading ~ with the user's home directory.
// EnvWithFallback returns the value of the primary env var, falling back to
// the legacy name if the primary is unset. This eases the FGJ_ → FJ_ rename.
func EnvWithFallback(primary, legacy string) string {
if v := os.Getenv(primary); v != "" {
return v
}
return os.Getenv(legacy)
}
func expandHome(path string) string {
if path == "~" || strings.HasPrefix(path, "~/") {
home, err := os.UserHomeDir()
if err != nil {
return path
}
return filepath.Join(home, path[1:])
}
return path
}
2025-12-08 09:49:07 +01:00
func (c *Config) SetHost(hostname string, host HostConfig) {
if c.Hosts == nil {
c.Hosts = make(map[string]HostConfig)
}
c.Hosts[hostname] = host
}