fj/cmd/root.go

206 lines
5.7 KiB
Go
Raw Normal View History

2025-12-08 09:49:07 +01:00
package cmd
import (
"fmt"
"io"
2025-12-08 09:49:07 +01:00
"os"
"path/filepath"
"strconv"
2026-01-05 12:47:28 +01:00
"strings"
2025-12-08 09:49:07 +01:00
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
"forgejo.zerova.net/public/fj/internal/config"
"forgejo.zerova.net/public/fj/internal/git"
2025-12-08 09:49:07 +01:00
"github.com/spf13/cobra"
"github.com/spf13/viper"
)
var cfgFile string
var jsonErrors bool
2025-12-08 09:49:07 +01:00
var rootCmd = &cobra.Command{
Use: "fj",
2025-12-08 09:49:07 +01:00
Short: "Forgejo CLI tool - work seamlessly with Forgejo from the command line",
Long: `fj is a command line tool for Forgejo instances (including Codeberg).
2025-12-08 09:49:07 +01:00
It brings pull requests, issues, and other Forgejo concepts to the terminal.`,
2026-04-26 08:33:17 -06:00
Version: "0.3.2",
SilenceErrors: true,
}
// JSONErrors reports whether the --json-errors flag is set.
func JSONErrors() bool {
return jsonErrors
2025-12-08 09:49:07 +01:00
}
func Execute() error {
return rootCmd.Execute()
}
func init() {
cobra.OnInitialize(initConfig)
rootCmd.PersistentFlags().StringVar(&cfgFile, "config", "", "config file (default is $HOME/.config/fj/config.yaml)")
rootCmd.PersistentFlags().BoolVar(&jsonErrors, "json-errors", false, "output errors as structured JSON to stderr")
2025-12-08 09:49:07 +01:00
rootCmd.PersistentFlags().String("hostname", "", "Forgejo instance hostname")
2025-12-08 10:00:50 +01:00
_ = viper.BindPFlag("hostname", rootCmd.PersistentFlags().Lookup("hostname"))
2025-12-08 09:49:07 +01:00
}
func initConfig() {
if cfgFile != "" {
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
// Tell viper to load this file for env-style overrides AND make
// internal/config.Load()/.Save() use it (this is the load-bearing
// half — without SetExplicitConfigPath, --config was silently
// ignored by every auth-touching command).
2025-12-08 09:49:07 +01:00
viper.SetConfigFile(cfgFile)
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
config.SetExplicitConfigPath(cfgFile)
2025-12-08 09:49:07 +01:00
} else {
home, err := os.UserHomeDir()
if err != nil {
fmt.Fprintln(ios.ErrOut, err)
2025-12-08 09:49:07 +01:00
os.Exit(1)
}
configDir := home + "/.config/fj"
legacyDir := home + "/.config/fgj"
// Migrate from ~/.config/fgj/ if the new dir doesn't exist yet.
if _, err := os.Stat(configDir); os.IsNotExist(err) {
if info, err := os.Stat(legacyDir); err == nil && info.IsDir() {
if copyErr := migrateConfigDir(legacyDir, configDir); copyErr == nil {
fmt.Fprintln(ios.ErrOut, "notice: migrated config from ~/.config/fgj/ to ~/.config/fj/")
fmt.Fprintln(ios.ErrOut, " you can remove ~/.config/fgj/ when ready")
}
}
}
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
_ = os.MkdirAll(configDir, 0700)
2025-12-08 09:49:07 +01:00
viper.AddConfigPath(configDir)
viper.SetConfigType("yaml")
viper.SetConfigName("config")
}
viper.AutomaticEnv()
viper.SetEnvPrefix("FJ")
2025-12-08 09:49:07 +01:00
2025-12-08 10:00:50 +01:00
_ = viper.ReadInConfig()
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 the resolved config exists with overly permissive mode, warn — the
// file holds API tokens. Don't fail-close; just nudge the user.
if path, err := config.GetConfigPath(); err == nil {
if info, statErr := os.Stat(path); statErr == nil && info.Mode()&0o077 != 0 {
fmt.Fprintf(ios.ErrOut, "warning: %s mode %o is world/group readable; tokens may leak. chmod 600 it.\n", path, info.Mode().Perm())
}
}
2025-12-08 09:49:07 +01:00
}
2026-01-05 12:47:28 +01:00
// parseRepo parses the repository string in the format "owner/name".
// If not provided, it attempts to auto-detect from the git repository.
func parseRepo(repo string) (string, string, error) {
// If repo flag is provided, use it
if repo != "" {
parts := strings.Split(repo, "/")
if len(parts) != 2 {
return "", "", fmt.Errorf("invalid repository format: %s (expected: owner/name)", repo)
}
return parts[0], parts[1], nil
}
// Try to auto-detect from git
owner, name, err := git.DetectRepo()
if err != nil {
return "", "", fmt.Errorf("repository flag is required (use -R owner/name) or run from a git repository: %w", err)
}
return owner, name, nil
}
// getDetectedHost attempts to auto-detect the Forgejo instance hostname.
// Returns empty string if detection fails, which will fall back to other methods.
func getDetectedHost() string {
host, err := git.DetectHost()
if err != nil {
return ""
}
return host
}
// getCwd returns the current working directory, or "" on error.
func getCwd() string {
cwd, err := os.Getwd()
if err != nil {
return ""
}
return cwd
}
// promptLine prints a prompt to stderr and reads a line from stdin.
func promptLine(prompt string) (string, error) {
fmt.Fprint(ios.ErrOut, prompt)
var buf [1024]byte
n, err := ios.In.Read(buf[:])
if err != nil {
return "", fmt.Errorf("reading input: %w", err)
}
return strings.TrimSpace(string(buf[:n])), nil
}
// parseIssueArg parses an issue/PR number from various formats:
// "123", "#123", "https://host/owner/repo/pulls/123", "https://host/owner/repo/issues/123"
func parseIssueArg(arg string) (int64, error) {
arg = strings.TrimPrefix(arg, "#")
// Try URL format
if strings.HasPrefix(arg, "http") {
parts := strings.Split(strings.TrimRight(arg, "/"), "/")
arg = parts[len(parts)-1]
}
return strconv.ParseInt(arg, 10, 64)
}
// migrateConfigDir copies all files from src to dst (one level, no subdirs).
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
// Uses O_TRUNC so a partially-pre-existing dst file is fully replaced rather
// than having the legacy contents overwrite a prefix and leaving stale tail
// bytes — which for a YAML token store would silently corrupt config.
func migrateConfigDir(src, dst 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 err := os.MkdirAll(dst, 0700); err != nil {
return err
}
entries, err := os.ReadDir(src)
if err != nil {
return err
}
for _, e := range entries {
if e.IsDir() {
continue
}
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 err := copyOneConfigFile(filepath.Join(src, e.Name()), filepath.Join(dst, e.Name())); err != nil {
return err
}
}
return nil
}
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
func copyOneConfigFile(srcPath, dstPath string) (retErr error) {
in, err := os.Open(srcPath)
if err != nil {
return err
}
defer func() {
if cerr := in.Close(); retErr == nil {
retErr = cerr
}
}()
out, err := os.OpenFile(dstPath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600)
if err != nil {
return err
}
defer func() {
if cerr := out.Close(); retErr == nil {
retErr = cerr
}
}()
_, err = io.Copy(out, in)
return err
}