diff --git a/CHANGELOG.md b/CHANGELOG.md index 0565379..170c5ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,107 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.4.0] - 2026-05-02 + +Audit-driven hardening pass. Three reviewers (Codex + two Claude agents +with non-overlapping focuses) found 13 issues across cmd/ and internal/; +this release ships fixes for all 13. + +### BREAKING + +- `--json=fields` syntax removed. The flag was a string with + `NoOptDefVal=" "` sentinel — `--json` alone meant "everything", + `--json=fields` projected. That produced `--json string[=" "]` in + `--help` and required a literal `=` because `--json fields` was parsed + as the bare flag plus a positional. **Migration**: `--json=fields` → + `--json-fields fields`. Bare `--json` still means "all fields as JSON". + `--json` and `--json-fields` are mutually exclusive; `--jq` composes + with either. + +### Added + +- `fj api --json` / `--json-fields` / `--jq` — projection and jq filtering + for raw API responses. Routes through the same `addJSONFlags` helpers + as the other list commands. Closes the inconsistency where `fj api` + was the only command returning raw JSON without these knobs. +- `fj api --paginate` — follows RFC 5988 `Link: rel="next"` headers and + concatenates JSON array pages, gh-compatible. Validates same-origin + before forwarding the bearer token to the next URL. +- `cmd/paginate.go` — generic `paginateGitea[T any]` helper. Applied to + `repo list`, `pr list`, `issue list`. Previously only `release list` + walked pages; the others passed `PageSize: limit` directly to the + gitea SDK, which silently caps PageSize at 50, so `--limit > 50` was + truncated without warning. +- `CLAUDE.md` — guide for Claude Code sessions: layout, codex review + pattern, release process, homebrew tap update steps. + +### Changed + +- `--json` flag rebuilt as a plain `Bool`. `--json-fields` keeps + comma-separated projection. Both registered via `addJSONFlags` and + marked `MutuallyExclusive`. +- `cmd/actions.go` — `run` and `workflow` subtrees converted from + package-level `var`s to factory functions (`newRunCmd`, + `newWorkflowCmd`, ...). `cmd/aliases.go` shrank from 142 → 17 lines + and now calls those same factories with a `parentLabel` parameter that + disambiguates the alias variant. Result: `diff` of `fj run list + --help` flags vs `fj actions run list --help` flags is now empty. + Drift between the two paths is structurally impossible. +- `fj api` now uses `internal/api.SharedHTTPClient` (30s timeout, pooled + connections) instead of a zero-value `&http.Client{}` with no timeout. + A hung Forgejo no longer pins the CLI indefinitely. +- `fj api` response body bounded by `io.LimitReader` at 64 MB to prevent + OOM-on-self. +- `cmd/auth.go` removed redundant local `--hostname` declarations on + three subcommands. The persistent flag on rootCmd is now the only + declaration; previously local declarations shadowed it, so + `fj --hostname=X auth login` and `fj auth login --hostname=X` went + through different code paths. +- `--token` on `auth login` emits a stderr warning when used (visible + in `ps auxe` and shell history). Flag not removed; just discoverable. +- Error handling: `Hint` is now a structured field on `CLIError`. + JSON-error consumers get clean structure; the human renderer still + appends `\nHint: ...`. Dropped substring matching of `"401"`/`"403"` + against rendered error strings (would match issue #403); now relies + exclusively on typed `*api.APIError`. +- Network errors (`no such host`, `connection refused`, `i/o timeout`) + return a structured `CLIError` with code `ErrNetworkError` and a hint. +- Config dir created with mode 0700 instead of 0755. + +### Fixed + +- `--config ` now actually honored. Previously fed only into + Viper; every command that touched config went through + `internal/config.Load()` / `Save()` which always read the default + path. So `fj --config other.yaml auth login` writes to other.yaml now. +- `fj run list --json`, `fj workflow list --json`, `fj wiki view --json` + now produce JSON. `cmd/aliases.go` registered `--json` as `Bool` but + handlers called `wantJSON()` which does `GetString("json")` — pflag + returned a type-error that `wantJSON` silently swallowed. + `cmd/wiki.go` had the inverse bug (`GetBool` against an + `addJSONFlags`-registered string flag). Both routed through + `addJSONFlags`/`wantJSON`/`outputJSON` consistently now. +- `migrateConfigDir` opens dst with `O_TRUNC`. Previously a partially- + pre-existing dst file would have legacy contents overwrite a prefix + and leave stale tail bytes — silent YAML/token corruption. Refactored + close handling into `copyOneConfigFile`. + +### Security + +- `fj api` endpoint path traversal closed. `fj api '/../admin/users'` + previously normalized through `http.NewRequest` to + `https://host/admin/users` — silently sending authenticated traffic + to non-API paths. Endpoint is now parsed via `url.Parse`, `..` + segments rejected, then `JoinPath` onto the `/api/v1` base. + URL-encoded `%2E%2E` is also caught because Go decodes before our + split. +- `fj api --paginate` validates same-origin before forwarding the + bearer token to a `Link: rel="next"` URL. Refuses to reattach + `Authorization` if the next URL's scheme isn't `https` or its host + doesn't match the configured one. +- `initConfig` warns on stderr if the resolved config file is world or + group readable (`mode & 0o077 != 0`). + ## [0.3.0c] - 2026-03-21 ### Added diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..dfece55 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,166 @@ +# fj — guide for Claude Code sessions + +`fj` is a personal Forgejo/Gitea CLI tool, modeled on GitHub's `gh`. It targets `forgejo.zerova.net` (and Codeberg). The user (sid) owns it; the canonical repo is `public/fj` on forgejo.zerova.net (mirrored from there to nowhere else). + +This file is read first by Claude Code when working in `~/repos/fj`. Goal: get a session productive quickly without re-deriving the dev workflow each time. + +## Layout + +``` +~/repos/fj/ +├── cmd/ cobra command definitions, one file per subject area +│ ├── root.go rootCmd, --config plumbing, OnInitialize +│ ├── auth.go login/status/logout/token (uses persistent --hostname) +│ ├── api.go raw API access; --json/--json-fields/--jq/--paginate +│ ├── json.go shared JSON output helpers (addJSONFlags/wantJSON/outputJSON) +│ ├── paginate.go generic paginateGitea[T] helper for list commands +│ ├── errors.go CLIError with structured Hint field +│ ├── actions.go Forgejo Actions; runs/workflows via factory functions +│ ├── aliases.go top-level `fj run` / `fj workflow` aliases — calls actions.go factories +│ ├── repo.go pr.go issue.go release.go wiki.go label.go milestone.go +│ └── ... +├── internal/ +│ ├── api/client.go SharedHTTPClient (30s timeout); GetJSON/DoJSON/DownloadFile +│ ├── config/config.go YAML config; honors --config via SetExplicitConfigPath +│ ├── git/ repo + host detection from `git remote` +│ ├── iostreams/ wrapped stdin/stdout/stderr + spinner + pager + colors +│ └── text/ formatting helpers +├── main.go thin entrypoint; ContextualError + JSON-error rendering +├── Makefile build / lint / test (no release automation) +├── CHANGELOG.md Keep-a-Changelog format +└── README.md +``` + +## Build, install, test + +```bash +go build ./... # quick build check +go test ./... # unit tests +go install . # build + install to ~/go/bin/fj (the binary that's on PATH) +make lint # golangci-lint, if you have it +``` + +After any change in cmd/ or internal/, run `go install .` and the global `fj` reflects it immediately. There's no daemon/restart. + +## Auth + +The user is authenticated as `sid` on `forgejo.zerova.net`. Token lives in `~/.config/fj/config.yaml` (mode 0600). For HTTPS git pushes from this host, the token can be injected via `git -c "http.extraHeader=Authorization: token " push` — the local SSH key (`sid@debian` on forgejo) is also registered, so `git@forgejo.zerova.net:public/fj.git` works directly. + +## Code review pattern (use this for non-trivial changes) + +For audits or significant refactors, run **three reviewers in parallel** with non-overlapping focuses (we did this in the v0.4.0 cycle and it found bugs none would have caught alone): + +- **Codex** — read-only sandbox, peer-AI cross-check + ```bash + codex exec --skip-git-repo-check --sandbox read-only \ + -m gpt-5.4-mini --config model_reasoning_effort="medium" "" 2>/dev/null + ``` + For follow-up rounds resume the same session: `echo "" | codex exec --skip-git-repo-check resume --last 2>/dev/null`. Codex remembers prior critique. + +- **Claude general-purpose agent A** — architecture / UX / code-quality +- **Claude general-purpose agent B** — security / correctness / error handling + +Tell each reviewer what the **siblings** are covering so they don't duplicate. Cap reports at ~600 words. Consolidate findings by severity (HIGH / MEDIUM / LOW) before presenting to the user. + +## Release process + +We use semver. **Pre-1.0**: breaking change → minor bump (e.g. v0.3.x → v0.4.0). + +1. **Bump version** + ```go + // cmd/root.go + Version: "0.4.0", + ``` + +2. **Update CHANGELOG.md** — prepend a new section. Format: + ```markdown + ## [0.4.0] - YYYY-MM-DD + + ### BREAKING + - + + ### Added + - ... + ### Changed + - ... + ### Fixed + - ... + ### Security + - ... + ``` + +3. **Commit** the version+changelog bump as a single commit: + ```bash + git commit -m "chore: bump version to 0.4.0" + ``` + +4. **Tag** the commit: + ```bash + git tag -a v0.4.0 -m "Release v0.4.0: " + ``` + +5. **Push** commits and tag: + ```bash + git push origin main + git push origin v0.4.0 + ``` + +6. **Create the Forgejo release page** via fj itself: + ```bash + fj release create v0.4.0 \ + --title "v0.4.0: " \ + --notes "$(awk '/^## \[0.4.0\]/{flag=1;next} /^## /{flag=0} flag' CHANGELOG.md)" + ``` + (The awk one-liner extracts the just-added CHANGELOG section as release notes.) + +7. **Update the homebrew tap** — see the next section. + +## Updating the homebrew tap (`public/homebrew-sid`) + +The tap lives at `~/repos/homebrew-sid` (or `git@forgejo.zerova.net:public/homebrew-sid.git`). The `Formula/fj.rb` formula references the source by `tag:` + `revision:` (SHA), so a release bump touches three lines: + +```ruby +url "ssh://git@forgejo.zerova.net/public/fj.git", + tag: "v0.4.0", # was v0.3.2 + revision: "" # update + +test do + assert_match "0.4.0", shell_output("#{bin}/fj --version") # update +end +``` + +To get the SHA: +```bash +git -C ~/repos/fj rev-parse v0.4.0 +``` + +Then in `~/repos/homebrew-sid`: +```bash +# edit Formula/fj.rb (the three lines above) +git commit -am "fj: bump to v0.4.0" +git push origin main +``` + +After push, users can `brew update && brew upgrade fj` to pick up the new version. + +## Common footguns + +- **`fj` reads the current dir's `git origin`** to detect the host. In a directory whose origin points at github.com (e.g. /opt/stacks/claude-code-proxy/build), bare `fj api ...` errors with "no configuration found for host github.com". Pass `--hostname forgejo.zerova.net` explicitly, or `cd` somewhere else. +- **`--json=fields` was removed in v0.4.0** in favor of `--json-fields fields` (or `--json-fields=fields`). The old `=fields` form was a `NoOptDefVal=" "` sentinel hack. `--json` is now a plain Bool meaning "as JSON". +- **`--config` was silently ignored before v0.4.0.** Old fj versions read --config into Viper but `internal/config.Load()` always read the default path. Fixed; `fj --config other.yaml auth login` now writes to other.yaml. +- **The `actions` and `run`/`workflow` command trees share factory functions** in `cmd/actions.go` (`newRunCmd`, `newWorkflowCmd`). Don't add flags directly to `runListCmd` style globals — they don't exist anymore. Edit the factory and both `fj actions run list` and `fj run list` get the change. + +## Useful commands + +```bash +# Live test against forgejo (using the new flags) +fj --hostname forgejo.zerova.net api repos/public/fj --json-fields full_name,description + +# Walk paginated endpoints +fj --hostname forgejo.zerova.net api 'repos/public/fj/commits?limit=10' --paginate --jq '.[].sha[0:8]' + +# Confirm both command trees stay in sync after edits +diff <(fj run list --help | grep -E "^ -|^ --" | sort) \ + <(fj actions run list --help | grep -E "^ -|^ --" | sort) +# Empty diff = trees agree. Any output = factory drift. +``` diff --git a/README.md b/README.md index 656d90b..da5bac5 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ ### macOS (Homebrew) ```bash -brew tap sid/fj https://forgejo.zerova.net/sid/homebrew-fj.git +brew tap public/sid git@forgejo.zerova.net:public/homebrew-sid.git brew install fj ``` diff --git a/cmd/actions.go b/cmd/actions.go index 0b58455..172831c 100644 --- a/cmd/actions.go +++ b/cmd/actions.go @@ -87,18 +87,34 @@ var actionsCmd = &cobra.Command{ Long: "View and manage workflows, runs, secrets, and variables for Forgejo Actions in your repositories.", } -// Run commands (compatible with gh run) -var runCmd = &cobra.Command{ - Use: "run", - Short: "View and manage workflow runs", - Long: "List, view, and manage workflow runs.", +// Run and Workflow command trees are built via factory functions +// (newRunCmd / newWorkflowCmd) so cmd/aliases.go can build an identical +// top-level tree under rootCmd without duplicating Use/Short/Long/Example/ +// flag declarations. Single source of truth — drift impossible. + +// newRunCmd builds the `run` subtree. parentLabel is interpolated into the +// parent's Short/Long so the alias-tree variant can advertise itself as +// "alias for 'actions run'" without diverging on the children. +func newRunCmd(parentLabel string) *cobra.Command { + cmd := &cobra.Command{ + Use: "run", + Short: "View and manage workflow runs" + parentLabel, + Long: "List, view, and manage workflow runs." + parentLabel, + } + cmd.AddCommand(newRunListCmd()) + cmd.AddCommand(newRunViewCmd()) + cmd.AddCommand(newRunWatchCmd()) + cmd.AddCommand(newRunRerunCmd()) + cmd.AddCommand(newRunCancelCmd()) + return cmd } -var runListCmd = &cobra.Command{ - Use: "list", - Short: "List recent workflow runs", - Long: "List recent workflow runs for a repository.", - Example: ` # List recent workflow runs +func newRunListCmd() *cobra.Command { + c := &cobra.Command{ + Use: "list", + Short: "List recent workflow runs", + Long: "List recent workflow runs for a repository.", + Example: ` # List recent workflow runs fj actions run list # List runs with a custom limit @@ -106,14 +122,20 @@ var runListCmd = &cobra.Command{ # Output as JSON fj actions run list --json`, - RunE: runRunList, + RunE: runRunList, + } + addRepoFlags(c) + c.Flags().IntP("limit", "L", 20, "Maximum number of runs to list") + addJSONFlags(c, "Output workflow runs as JSON") + return c } -var runViewCmd = &cobra.Command{ - Use: "view ", - Short: "View a workflow run", - Long: "View details about a specific workflow run.", - Example: ` # View a workflow run +func newRunViewCmd() *cobra.Command { + c := &cobra.Command{ + Use: "view ", + Short: "View a workflow run", + Long: "View details about a specific workflow run.", + Example: ` # View a workflow run fj actions run view 123 # View with job details @@ -124,55 +146,86 @@ var runViewCmd = &cobra.Command{ # View only failed logs fj actions run view 123 --log-failed`, - Args: cobra.ExactArgs(1), - RunE: runRunView, + Args: cobra.ExactArgs(1), + RunE: runRunView, + } + addRepoFlags(c) + c.Flags().BoolP("verbose", "v", false, "Show job steps") + c.Flags().BoolP("log", "", false, "View full log for either a run or specific job") + c.Flags().StringP("job", "j", "", "View a specific job ID from a run") + c.Flags().BoolP("log-failed", "", false, "View the log for any failed steps in a run or specific job") + addJSONFlags(c, "Output workflow run as JSON") + return c } -var runWatchCmd = &cobra.Command{ - Use: "watch ", - Short: "Watch a workflow run", - Long: "Poll a workflow run until it completes.", - Example: ` # Watch a run until it completes +func newRunWatchCmd() *cobra.Command { + c := &cobra.Command{ + Use: "watch ", + Short: "Watch a workflow run", + Long: "Poll a workflow run until it completes.", + Example: ` # Watch a run until it completes fj actions run watch 123 # Watch with a custom polling interval fj actions run watch 123 -i 10s`, - Args: cobra.ExactArgs(1), - RunE: runRunWatch, + Args: cobra.ExactArgs(1), + RunE: runRunWatch, + } + addRepoFlags(c) + c.Flags().DurationP("interval", "i", 5*time.Second, "Polling interval") + return c } -var runRerunCmd = &cobra.Command{ - Use: "rerun ", - Short: "Rerun a workflow run", - Long: "Trigger a rerun for a specific workflow run.", - Example: ` # Rerun a failed workflow run +func newRunRerunCmd() *cobra.Command { + c := &cobra.Command{ + Use: "rerun ", + Short: "Rerun a workflow run", + Long: "Trigger a rerun for a specific workflow run.", + Example: ` # Rerun a failed workflow run fj actions run rerun 123`, - Args: cobra.ExactArgs(1), - RunE: runRunRerun, + Args: cobra.ExactArgs(1), + RunE: runRunRerun, + } + addRepoFlags(c) + return c } -var runCancelCmd = &cobra.Command{ - Use: "cancel ", - Short: "Cancel a workflow run", - Long: "Cancel a running workflow run.", - Example: ` # Cancel a running workflow +func newRunCancelCmd() *cobra.Command { + c := &cobra.Command{ + Use: "cancel ", + Short: "Cancel a workflow run", + Long: "Cancel a running workflow run.", + Example: ` # Cancel a running workflow fj actions run cancel 123`, - Args: cobra.ExactArgs(1), - RunE: runRunCancel, + Args: cobra.ExactArgs(1), + RunE: runRunCancel, + } + addRepoFlags(c) + return c } -// Workflow commands -var workflowCmd = &cobra.Command{ - Use: "workflow", - Short: "Manage workflows", - Long: "List, view, and run workflows.", +// newWorkflowCmd builds the `workflow` subtree. parentLabel is interpolated +// the same way as newRunCmd's, so the alias variant can self-identify. +func newWorkflowCmd(parentLabel string) *cobra.Command { + cmd := &cobra.Command{ + Use: "workflow", + Short: "Manage workflows" + parentLabel, + Long: "List, view, and run workflows." + parentLabel, + } + cmd.AddCommand(newWorkflowListCmd()) + cmd.AddCommand(newWorkflowViewCmd()) + cmd.AddCommand(newWorkflowRunCmd()) + cmd.AddCommand(newWorkflowEnableCmd()) + cmd.AddCommand(newWorkflowDisableCmd()) + return cmd } -var workflowListCmd = &cobra.Command{ - Use: "list", - Short: "List workflows", - Long: "List all workflows in a repository.", - Example: ` # List all workflows +func newWorkflowListCmd() *cobra.Command { + c := &cobra.Command{ + Use: "list", + Short: "List workflows", + Long: "List all workflows in a repository.", + Example: ` # List all workflows fj actions workflow list # List workflows as JSON @@ -180,53 +233,78 @@ var workflowListCmd = &cobra.Command{ # List workflows for a specific repo fj actions workflow list -R owner/repo`, - RunE: runWorkflowList, + RunE: runWorkflowList, + } + addRepoFlags(c) + c.Flags().IntP("limit", "L", 20, "Maximum number of workflows to list") + addJSONFlags(c, "Output workflows as JSON") + return c } -var workflowViewCmd = &cobra.Command{ - Use: "view ", - Short: "View a workflow", - Long: "View details about a specific workflow. You can specify the workflow by name or filename.", - Example: ` # View a workflow by filename +func newWorkflowViewCmd() *cobra.Command { + c := &cobra.Command{ + Use: "view ", + Short: "View a workflow", + Long: "View details about a specific workflow. You can specify the workflow by name or filename.", + Example: ` # View a workflow by filename fj actions workflow view ci.yml # View as JSON fj actions workflow view ci.yml --json`, - Args: cobra.ExactArgs(1), - RunE: runWorkflowView, + Args: cobra.ExactArgs(1), + RunE: runWorkflowView, + } + addRepoFlags(c) + addJSONFlags(c, "Output workflow as JSON") + return c } -var workflowRunCmd = &cobra.Command{ - Use: "run ", - Short: "Run a workflow", - Long: "Trigger a workflow_dispatch event for a workflow. The workflow must support the workflow_dispatch trigger.", - Example: ` # Trigger a workflow on the default branch +func newWorkflowRunCmd() *cobra.Command { + c := &cobra.Command{ + Use: "run ", + Short: "Run a workflow", + Long: "Trigger a workflow_dispatch event for a workflow. The workflow must support the workflow_dispatch trigger.", + Example: ` # Trigger a workflow on the default branch fj actions workflow run deploy.yml # Trigger on a specific branch with input parameters fj actions workflow run deploy.yml -r staging -f environment=staging -f version=1.2.3`, - Args: cobra.ExactArgs(1), - RunE: runWorkflowRun, + Args: cobra.ExactArgs(1), + RunE: runWorkflowRun, + } + addRepoFlags(c) + c.Flags().StringP("ref", "r", "", "Branch or tag name to run the workflow on (defaults to repository's default branch)") + c.Flags().StringSliceP("field", "f", nil, "Add a string parameter in key=value format (can be used multiple times)") + c.Flags().StringSliceP("raw-field", "F", nil, "Add a string parameter in key=value format, reading from file if value starts with @ (can be used multiple times)") + return c } -var workflowEnableCmd = &cobra.Command{ - Use: "enable ", - Short: "Enable a workflow", - Long: "Enable a workflow so it can be triggered.\n\nNote: This feature requires Forgejo 15.0+ or Gitea 1.24+.\nFor older versions, use the web UI to enable workflows.", - Example: ` # Enable a workflow +func newWorkflowEnableCmd() *cobra.Command { + c := &cobra.Command{ + Use: "enable ", + Short: "Enable a workflow", + Long: "Enable a workflow so it can be triggered.\n\nNote: This feature requires Forgejo 15.0+ or Gitea 1.24+.\nFor older versions, use the web UI to enable workflows.", + Example: ` # Enable a workflow fj actions workflow enable ci.yml`, - Args: cobra.ExactArgs(1), - RunE: runWorkflowEnable, + Args: cobra.ExactArgs(1), + RunE: runWorkflowEnable, + } + addRepoFlags(c) + return c } -var workflowDisableCmd = &cobra.Command{ - Use: "disable ", - Short: "Disable a workflow", - Long: "Disable a workflow so it cannot be triggered.\n\nNote: This feature requires Forgejo 15.0+ or Gitea 1.24+.\nFor older versions, use the web UI to disable workflows.", - Example: ` # Disable a workflow +func newWorkflowDisableCmd() *cobra.Command { + c := &cobra.Command{ + Use: "disable ", + Short: "Disable a workflow", + Long: "Disable a workflow so it cannot be triggered.\n\nNote: This feature requires Forgejo 15.0+ or Gitea 1.24+.\nFor older versions, use the web UI to disable workflows.", + Example: ` # Disable a workflow fj actions workflow disable ci.yml`, - Args: cobra.ExactArgs(1), - RunE: runWorkflowDisable, + Args: cobra.ExactArgs(1), + RunE: runWorkflowDisable, + } + addRepoFlags(c) + return c } // Secret commands @@ -336,21 +414,10 @@ var actionsVariableDeleteCmd = &cobra.Command{ func init() { rootCmd.AddCommand(actionsCmd) - // Add run commands (gh run compatible) - actionsCmd.AddCommand(runCmd) - runCmd.AddCommand(runListCmd) - runCmd.AddCommand(runViewCmd) - runCmd.AddCommand(runWatchCmd) - runCmd.AddCommand(runRerunCmd) - runCmd.AddCommand(runCancelCmd) - - // Add workflow commands (gh workflow compatible) - actionsCmd.AddCommand(workflowCmd) - workflowCmd.AddCommand(workflowListCmd) - workflowCmd.AddCommand(workflowViewCmd) - workflowCmd.AddCommand(workflowRunCmd) - workflowCmd.AddCommand(workflowEnableCmd) - workflowCmd.AddCommand(workflowDisableCmd) + // Run and Workflow trees come from the factory functions defined above + // so cmd/aliases.go can build identical top-level trees under rootCmd. + actionsCmd.AddCommand(newRunCmd("")) + actionsCmd.AddCommand(newWorkflowCmd("")) // Add secret commands actionsCmd.AddCommand(actionsSecretCmd) @@ -366,34 +433,6 @@ func init() { actionsVariableCmd.AddCommand(actionsVariableUpdateCmd) actionsVariableCmd.AddCommand(actionsVariableDeleteCmd) - // Add flags for run commands - addRepoFlags(runListCmd) - runListCmd.Flags().IntP("limit", "L", 20, "Maximum number of runs to list") - addJSONFlags(runListCmd, "Output workflow runs as JSON") - addRepoFlags(runViewCmd) - runViewCmd.Flags().BoolP("verbose", "v", false, "Show job steps") - runViewCmd.Flags().BoolP("log", "", false, "View full log for either a run or specific job") - runViewCmd.Flags().StringP("job", "j", "", "View a specific job ID from a run") - runViewCmd.Flags().BoolP("log-failed", "", false, "View the log for any failed steps in a run or specific job") - addJSONFlags(runViewCmd, "Output workflow run as JSON") - addRepoFlags(runWatchCmd) - runWatchCmd.Flags().DurationP("interval", "i", 5*time.Second, "Polling interval") - addRepoFlags(runRerunCmd) - addRepoFlags(runCancelCmd) - - // Add flags for workflow commands - addRepoFlags(workflowListCmd) - workflowListCmd.Flags().IntP("limit", "L", 20, "Maximum number of workflows to list") - addJSONFlags(workflowListCmd, "Output workflows as JSON") - addRepoFlags(workflowViewCmd) - addJSONFlags(workflowViewCmd, "Output workflow as JSON") - addRepoFlags(workflowRunCmd) - addRepoFlags(workflowEnableCmd) - addRepoFlags(workflowDisableCmd) - workflowRunCmd.Flags().StringP("ref", "r", "", "Branch or tag name to run the workflow on (defaults to repository's default branch)") - workflowRunCmd.Flags().StringSliceP("field", "f", nil, "Add a string parameter in key=value format (can be used multiple times)") - workflowRunCmd.Flags().StringSliceP("raw-field", "F", nil, "Add a string parameter in key=value format, reading from file if value starts with @ (can be used multiple times)") - // Add flags for secret commands addRepoFlags(actionsSecretListCmd) addRepoFlags(actionsSecretCreateCmd) diff --git a/cmd/aliases.go b/cmd/aliases.go index 522c540..46f0a24 100644 --- a/cmd/aliases.go +++ b/cmd/aliases.go @@ -1,142 +1,16 @@ package cmd -import ( - "time" - - "github.com/spf13/cobra" -) - -// Top-level aliases for "actions run" and "actions workflow" commands, -// matching gh CLI's command structure (e.g., "fj run list" instead of "fj actions run list"). +// Top-level aliases for "actions run" and "actions workflow" — matches gh +// CLI's ergonomics so users can type `fj run list` and `fj workflow list` +// instead of `fj actions run list`. +// +// Both trees are built from the same factory functions defined in +// `cmd/actions.go` (newRunCmd / newWorkflowCmd), which means flags and +// help text are guaranteed identical between the two paths. Previously +// this file rebuilt parallel trees by hand and silently drifted (the +// `--json` Bool/string mismatch was the symptom that surfaced). func init() { - // --- run alias --- - runAliasCmd := &cobra.Command{ - Use: "run", - Short: "View and manage workflow runs (alias for 'actions run')", - Long: "List, view, and manage workflow runs.\n\nThis is a top-level alias for 'actions run'.", - } - - runAliasListCmd := &cobra.Command{ - Use: "list", - Short: "List recent workflow runs", - Long: "List recent workflow runs for a repository.", - RunE: runRunList, - } - addRepoFlags(runAliasListCmd) - runAliasListCmd.Flags().IntP("limit", "L", 20, "Maximum number of runs to list") - runAliasListCmd.Flags().Bool("json", false, "Output workflow runs as JSON") - - runAliasViewCmd := &cobra.Command{ - Use: "view ", - Short: "View a workflow run", - Long: "View details about a specific workflow run.", - Args: cobra.ExactArgs(1), - RunE: runRunView, - } - addRepoFlags(runAliasViewCmd) - runAliasViewCmd.Flags().BoolP("verbose", "v", false, "Show job steps") - runAliasViewCmd.Flags().BoolP("log", "", false, "View full log for either a run or specific job") - runAliasViewCmd.Flags().StringP("job", "j", "", "View a specific job ID from a run") - runAliasViewCmd.Flags().BoolP("log-failed", "", false, "View the log for any failed steps in a run or specific job") - runAliasViewCmd.Flags().Bool("json", false, "Output workflow run as JSON") - - runAliasWatchCmd := &cobra.Command{ - Use: "watch ", - Short: "Watch a workflow run", - Long: "Poll a workflow run until it completes.", - Args: cobra.ExactArgs(1), - RunE: runRunWatch, - } - addRepoFlags(runAliasWatchCmd) - runAliasWatchCmd.Flags().DurationP("interval", "i", 5*time.Second, "Polling interval") - - runAliasRerunCmd := &cobra.Command{ - Use: "rerun ", - Short: "Rerun a workflow run", - Long: "Trigger a rerun for a specific workflow run.", - Args: cobra.ExactArgs(1), - RunE: runRunRerun, - } - addRepoFlags(runAliasRerunCmd) - - runAliasCancelCmd := &cobra.Command{ - Use: "cancel ", - Short: "Cancel a workflow run", - Long: "Cancel a running workflow run.", - Args: cobra.ExactArgs(1), - RunE: runRunCancel, - } - addRepoFlags(runAliasCancelCmd) - - runAliasCmd.AddCommand(runAliasListCmd) - runAliasCmd.AddCommand(runAliasViewCmd) - runAliasCmd.AddCommand(runAliasWatchCmd) - runAliasCmd.AddCommand(runAliasRerunCmd) - runAliasCmd.AddCommand(runAliasCancelCmd) - rootCmd.AddCommand(runAliasCmd) - - // --- workflow alias --- - workflowAliasCmd := &cobra.Command{ - Use: "workflow", - Short: "Manage workflows (alias for 'actions workflow')", - Long: "List, view, and run workflows.\n\nThis is a top-level alias for 'actions workflow'.", - } - - workflowAliasListCmd := &cobra.Command{ - Use: "list", - Short: "List workflows", - Long: "List all workflows in a repository.", - RunE: runWorkflowList, - } - addRepoFlags(workflowAliasListCmd) - workflowAliasListCmd.Flags().IntP("limit", "L", 20, "Maximum number of workflows to list") - workflowAliasListCmd.Flags().Bool("json", false, "Output workflows as JSON") - - workflowAliasViewCmd := &cobra.Command{ - Use: "view ", - Short: "View a workflow", - Long: "View details about a specific workflow. You can specify the workflow by name or filename.", - Args: cobra.ExactArgs(1), - RunE: runWorkflowView, - } - addRepoFlags(workflowAliasViewCmd) - workflowAliasViewCmd.Flags().Bool("json", false, "Output workflow as JSON") - - workflowAliasRunCmd := &cobra.Command{ - Use: "run ", - Short: "Run a workflow", - Long: "Trigger a workflow_dispatch event for a workflow. The workflow must support the workflow_dispatch trigger.", - Args: cobra.ExactArgs(1), - RunE: runWorkflowRun, - } - addRepoFlags(workflowAliasRunCmd) - workflowAliasRunCmd.Flags().StringP("ref", "r", "", "Branch or tag name to run the workflow on (defaults to repository's default branch)") - workflowAliasRunCmd.Flags().StringSliceP("field", "f", nil, "Add a string parameter in key=value format (can be used multiple times)") - workflowAliasRunCmd.Flags().StringSliceP("raw-field", "F", nil, "Add a string parameter in key=value format, reading from file if value starts with @ (can be used multiple times)") - - workflowAliasEnableCmd := &cobra.Command{ - Use: "enable ", - Short: "Enable a workflow", - Long: "Enable a workflow so it can be triggered.\n\nNote: This feature requires Forgejo 15.0+ or Gitea 1.24+.\nFor older versions, use the web UI to enable workflows.", - Args: cobra.ExactArgs(1), - RunE: runWorkflowEnable, - } - addRepoFlags(workflowAliasEnableCmd) - - workflowAliasDisableCmd := &cobra.Command{ - Use: "disable ", - Short: "Disable a workflow", - Long: "Disable a workflow so it cannot be triggered.\n\nNote: This feature requires Forgejo 15.0+ or Gitea 1.24+.\nFor older versions, use the web UI to disable workflows.", - Args: cobra.ExactArgs(1), - RunE: runWorkflowDisable, - } - addRepoFlags(workflowAliasDisableCmd) - - workflowAliasCmd.AddCommand(workflowAliasListCmd) - workflowAliasCmd.AddCommand(workflowAliasViewCmd) - workflowAliasCmd.AddCommand(workflowAliasRunCmd) - workflowAliasCmd.AddCommand(workflowAliasEnableCmd) - workflowAliasCmd.AddCommand(workflowAliasDisableCmd) - rootCmd.AddCommand(workflowAliasCmd) + rootCmd.AddCommand(newRunCmd(" (alias for 'actions run')")) + rootCmd.AddCommand(newWorkflowCmd(" (alias for 'actions workflow')")) } diff --git a/cmd/api.go b/cmd/api.go index fe0fd85..753ff49 100644 --- a/cmd/api.go +++ b/cmd/api.go @@ -6,15 +6,23 @@ import ( "fmt" "io" "net/http" + "net/url" "os" "strconv" "strings" + "forgejo.zerova.net/public/fj/internal/api" "forgejo.zerova.net/public/fj/internal/config" "forgejo.zerova.net/public/fj/internal/git" "github.com/spf13/cobra" ) +// maxAPIResponseBytes caps response bodies for `fj api`. Forgejo responses +// are normally <1 MB; 64 MB is enough for any sane payload while preventing +// a runaway body from OOMing the CLI when combined with the 30 s client +// timeout. +const maxAPIResponseBytes = 64 << 20 + var apiCmd = &cobra.Command{ Use: "api [flags]", Short: "Make an authenticated API request", @@ -35,7 +43,13 @@ If --field is used and no --method is specified, the method defaults to POST.`, fj api /users/johndoe # Use raw body from stdin - echo '{"title":"test"}' | fj api /repos/{owner}/{repo}/issues --input -`, + echo '{"title":"test"}' | fj api /repos/{owner}/{repo}/issues --input - + + # Filter the response with a jq expression + fj api /repos/{owner}/{repo}/issues --jq '.[].title' + + # Project the response down to specific fields + fj api /repos/{owner}/{repo} --json-fields full_name,description,private`, Args: cobra.ExactArgs(1), RunE: runAPI, } @@ -50,6 +64,40 @@ func init() { apiCmd.Flags().StringArrayP("header", "H", nil, "Add an HTTP request header (key:value)") apiCmd.Flags().Bool("silent", false, "Do not print the response body") apiCmd.Flags().BoolP("include", "i", false, "Include HTTP response headers in the output") + apiCmd.Flags().Bool("paginate", false, "Follow rel=\"next\" Link headers and concatenate JSON array pages (gh-compatible)") + addJSONFlags(apiCmd, "Output the response as JSON") +} + +// parseLinkHeaderNext extracts the URL with rel="next" from an RFC 5988 +// Link header. Returns "" if not present. +func parseLinkHeaderNext(link string) string { + for _, segment := range strings.Split(link, ",") { + segment = strings.TrimSpace(segment) + if !strings.Contains(segment, `rel="next"`) { + continue + } + start := strings.Index(segment, "<") + end := strings.Index(segment, ">") + if start >= 0 && end > start { + return segment[start+1 : end] + } + } + return "" +} + +// concatPaginatedJSON parses each body as a JSON array and merges them. +// Errors if any body isn't an array (e.g. an object response means the +// endpoint isn't paginated and --paginate doesn't apply). +func concatPaginatedJSON(bodies [][]byte) ([]byte, error) { + merged := make([]json.RawMessage, 0) + for i, b := range bodies { + var page []json.RawMessage + if err := json.Unmarshal(b, &page); err != nil { + return nil, fmt.Errorf("--paginate requires JSON array responses; page %d wasn't an array: %w", i+1, err) + } + merged = append(merged, page...) + } + return json.Marshal(merged) } func runAPI(cmd *cobra.Command, args []string) error { @@ -139,15 +187,28 @@ func runAPI(cmd *cobra.Command, args []string) error { body = bytes.NewReader(bodyBytes) } - // Build URL - baseURL := "https://" + host.Hostname + "/api/v1" - if !strings.HasPrefix(endpoint, "/") { - endpoint = "/" + endpoint + // Build the request URL safely. Naive concatenation lets endpoints like + // "/../admin/users" escape the /api/v1 base via Go's URL normalization + // of `..` segments — silently sending authenticated traffic to non-API + // paths. Parse the endpoint, reject `..`, then JoinPath onto the base. + endpointURL, err := url.Parse(endpoint) + if err != nil { + return fmt.Errorf("invalid endpoint %q: %w", endpoint, err) } - url := baseURL + endpoint + if endpointURL.Scheme != "" || endpointURL.Host != "" { + return fmt.Errorf("endpoint must be a path, not a full URL: %s", endpoint) + } + for _, seg := range strings.Split(strings.Trim(endpointURL.Path, "/"), "/") { + if seg == ".." { + return fmt.Errorf("endpoint contains forbidden '..' segment: %s", endpoint) + } + } + base := &url.URL{Scheme: "https", Host: host.Hostname, Path: "/api/v1"} + final := base.JoinPath(endpointURL.Path) + final.RawQuery = endpointURL.RawQuery // Create HTTP request - req, err := http.NewRequest(method, url, body) + req, err := http.NewRequest(method, final.String(), body) if err != nil { return fmt.Errorf("failed to create request: %w", err) } @@ -170,20 +231,42 @@ func runAPI(cmd *cobra.Command, args []string) error { req.Header.Set(strings.TrimSpace(key), strings.TrimSpace(value)) } - // Execute request - ios.StartSpinner("Requesting...") - httpClient := &http.Client{} - resp, err := httpClient.Do(req) - ios.StopSpinner() - if err != nil { - return fmt.Errorf("failed to perform request: %w", err) + paginate, _ := cmd.Flags().GetBool("paginate") + if paginate && method != http.MethodGet { + return fmt.Errorf("--paginate only supports GET requests") + } + + // doOnce executes a single request via the shared client (30 s timeout, + // pooled connections), reads the body bounded by maxAPIResponseBytes, + // and closes the body before returning. Previous zero-value http.Client{} + // had no timeout, pinning the CLI on a hung Forgejo indefinitely. + doOnce := func(r *http.Request) (body []byte, header http.Header, status int, proto string, statusText string, retErr error) { + ios.StartSpinner("Requesting...") + resp, err := api.SharedHTTPClient.Do(r) + ios.StopSpinner() + if err != nil { + return nil, nil, 0, "", "", fmt.Errorf("failed to perform request: %w", err) + } + defer func() { _ = resp.Body.Close() }() + + body, err = io.ReadAll(io.LimitReader(resp.Body, maxAPIResponseBytes+1)) + if err != nil { + return nil, nil, 0, "", "", fmt.Errorf("failed to read response body: %w", err) + } + if int64(len(body)) > maxAPIResponseBytes { + return nil, nil, 0, "", "", fmt.Errorf("response body exceeded %d bytes (use a different tool for bulk transfers)", maxAPIResponseBytes) + } + return body, resp.Header, resp.StatusCode, resp.Proto, resp.Status, nil + } + + respBody, respHeader, statusCode, proto, status, err := doOnce(req) + if err != nil { + return err } - defer func() { _ = resp.Body.Close() }() - // Print response headers if requested if include { - fmt.Fprintf(ios.Out, "%s %s\n", resp.Proto, resp.Status) - for key, values := range resp.Header { + fmt.Fprintf(ios.Out, "%s %s\n", proto, status) + for key, values := range respHeader { for _, v := range values { fmt.Fprintf(ios.Out, "%s: %s\n", key, v) } @@ -191,39 +274,99 @@ func runAPI(cmd *cobra.Command, args []string) error { fmt.Fprintln(ios.Out) } - // Read response body - respBody, err := io.ReadAll(resp.Body) - if err != nil { - return fmt.Errorf("failed to read response body: %w", err) - } - - // Handle non-2xx status codes - if resp.StatusCode < 200 || resp.StatusCode >= 300 { + if statusCode < 200 || statusCode >= 300 { if !silent { fmt.Fprint(ios.ErrOut, string(respBody)) if len(respBody) > 0 && respBody[len(respBody)-1] != '\n' { fmt.Fprintln(ios.ErrOut) } } - return fmt.Errorf("API request failed with status %d", resp.StatusCode) + return fmt.Errorf("API request failed with status %d", statusCode) + } + + // Follow `Link: rel="next"` headers when --paginate is set, accumulating + // each page's body. After the loop, concatPaginatedJSON merges them into + // a single JSON array. Endpoint must be paginatable (returns an array). + if paginate { + bodies := [][]byte{respBody} + nextURL := parseLinkHeaderNext(respHeader.Get("Link")) + for nextURL != "" { + // Forgejo emits same-origin next-links in practice, but a buggy + // or hostile upstream could redirect us to a foreign host — at + // which point we'd leak the bearer token. Validate origin (and + // resolve relative URLs against `base`) before forwarding auth. + parsedNext, err := url.Parse(nextURL) + if err != nil { + return fmt.Errorf("invalid Link rel=\"next\" URL %q: %w", nextURL, err) + } + if !parsedNext.IsAbs() { + parsedNext = base.ResolveReference(parsedNext) + } + if parsedNext.Scheme != "https" || parsedNext.Host != host.Hostname { + return fmt.Errorf("paginated next URL %s is not same-origin as https://%s; refusing to forward credentials", parsedNext.String(), host.Hostname) + } + + nextReq, err := http.NewRequest(http.MethodGet, parsedNext.String(), nil) + if err != nil { + return fmt.Errorf("failed to build paginated request: %w", err) + } + if host.Token != "" { + nextReq.Header.Set("Authorization", "token "+host.Token) + } + nextReq.Header.Set("Accept", "application/json") + for _, h := range headers { + key, value, found := strings.Cut(h, ":") + if !found { + continue + } + nextReq.Header.Set(strings.TrimSpace(key), strings.TrimSpace(value)) + } + pageBody, pageHeader, pageStatus, _, _, err := doOnce(nextReq) + if err != nil { + return err + } + if pageStatus < 200 || pageStatus >= 300 { + return fmt.Errorf("paginated request to %s failed with status %d", parsedNext.String(), pageStatus) + } + bodies = append(bodies, pageBody) + nextURL = parseLinkHeaderNext(pageHeader.Get("Link")) + } + merged, err := concatPaginatedJSON(bodies) + if err != nil { + return err + } + respBody = merged } if silent || len(respBody) == 0 { return nil } - // Pretty-print JSON, or output raw if not JSON - contentType := resp.Header.Get("Content-Type") - if strings.Contains(contentType, "json") || json.Valid(respBody) { + contentType := respHeader.Get("Content-Type") + isJSON := strings.Contains(contentType, "json") || json.Valid(respBody) + + // If the user asked for JSON projection or jq filtering, route through + // the shared JSON output helpers so the API command is consistent with + // `fj repo list`, `fj pr list`, etc. + if wantJSON(cmd) { + if !isJSON { + return fmt.Errorf("--json/--json-fields/--jq requires a JSON response, but the server returned %s", contentType) + } + var parsed any + if err := json.Unmarshal(respBody, &parsed); err != nil { + return fmt.Errorf("response is not valid JSON: %w", err) + } + return outputJSON(cmd, parsed) + } + + // Pretty-print JSON by default, otherwise emit raw bytes. + if isJSON { var parsed any if err := json.Unmarshal(respBody, &parsed); err == nil { - enc := json.NewEncoder(ios.Out) - enc.SetIndent("", " ") - return enc.Encode(parsed) + return writeJSON(parsed) } } - // Raw output for non-JSON responses _, err = ios.Out.Write(respBody) return err } diff --git a/cmd/auth.go b/cmd/auth.go index 16034f0..53ce982 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -55,16 +55,25 @@ func init() { authCmd.AddCommand(authLogoutCmd) authCmd.AddCommand(authTokenCmd) - authLoginCmd.Flags().String("hostname", "", "Forgejo instance hostname (e.g., codeberg.org)") - authLoginCmd.Flags().StringP("token", "t", "", "Personal access token") - authLogoutCmd.Flags().String("hostname", "", "Forgejo instance hostname (e.g., codeberg.org)") - authTokenCmd.Flags().String("hostname", "", "Forgejo instance hostname (e.g., codeberg.org)") + // --hostname is a persistent flag on rootCmd (cmd/root.go). Don't + // re-declare it on auth subcommands — local flags shadow the persistent + // one, so `fj --hostname=X auth login` and `fj auth login --hostname=X` + // went through different code paths (viper vs. local). + authLoginCmd.Flags().StringP("token", "t", "", "Personal access token (DEPRECATED: visible in `ps auxe`; pipe via stdin instead)") } func runAuthLogin(cmd *cobra.Command, args []string) error { hostname, _ := cmd.Flags().GetString("hostname") token, _ := cmd.Flags().GetString("token") + // Tokens passed via --token end up on the process command line and + // therefore in `ps auxe` and shell history. Warn loudly so users notice. + // (Don't refuse the flag — too disruptive for scripts that already use it.) + if cmd.Flags().Changed("token") { + fmt.Fprintln(ios.ErrOut, "warning: --token puts the token on the command line (visible in `ps auxe` and shell history)") + fmt.Fprintln(ios.ErrOut, " prefer omitting --token and pasting at the prompt, or piping via stdin.") + } + reader := bufio.NewReader(os.Stdin) if hostname == "" { diff --git a/cmd/errors.go b/cmd/errors.go index bec3dd9..7617430 100644 --- a/cmd/errors.go +++ b/cmd/errors.go @@ -3,7 +3,6 @@ package cmd import ( "encoding/json" "errors" - "fmt" "strings" "forgejo.zerova.net/public/fj/internal/api" @@ -25,9 +24,15 @@ type CLIError struct { Message string `json:"message"` Detail string `json:"detail,omitempty"` Status int `json:"status,omitempty"` + // Hint is a separate field so JSON consumers get clean structure and + // the human renderer can append "Hint: ..." without polluting Message. + Hint string `json:"hint,omitempty"` } func (e *CLIError) Error() string { + if e.Hint != "" { + return e.Message + "\nHint: " + e.Hint + } return e.Message } @@ -42,46 +47,59 @@ func NewAPIError(status int, message string) *CLIError { } // ContextualError wraps common errors with helpful hints. +// +// Auth/404 hints come exclusively from a typed *api.APIError now — we used +// to substring-match "401"/"403" against the rendered error string, which +// would trigger an "auth login" hint for any error mentioning issue #403. +// If the API client doesn't surface APIError, no hint is added; that's a +// signal to fix the client wrapper, not to layer regex on top. func ContextualError(err error) error { if err == nil { return nil } - msg := err.Error() - - // Check for API errors with status codes - var apiErr *api.APIError - if errors.As(err, &apiErr) { - switch { - case apiErr.StatusCode == 401 || apiErr.StatusCode == 403: - return fmt.Errorf("%w\nHint: Try authenticating with: fj auth login", err) - case apiErr.StatusCode == 404: - return fmt.Errorf("%w\nHint: Resource not found. Check the repository and number are correct.", err) - } + // If the error chain already holds a CLIError, leave it — it owns its + // Code/Hint already. + var cErr *CLIError + if errors.As(err, &cErr) { return err } - // Check for network/connection errors - switch { - case strings.Contains(msg, "no such host"): - return fmt.Errorf("%w\nHint: Check your internet connection and that the host is correct.", err) - case strings.Contains(msg, "connection refused"): - return fmt.Errorf("%w\nHint: Check your internet connection and that the host is correct.", err) + var apiErr *api.APIError + if errors.As(err, &apiErr) { + c := &CLIError{ + Code: ErrAPIError, + Message: err.Error(), + Status: apiErr.StatusCode, + Detail: apiErr.Body, + } + switch apiErr.StatusCode { + case 401, 403: + c.Code = ErrAuthRequired + c.Hint = "Try authenticating with: fj auth login" + case 404: + c.Code = ErrNotFound + c.Hint = "Resource not found. Check the repository and number are correct." + } + return c } - // Check for string-based status code patterns (from wrapped errors) + // Plain network errors come back as fmt.Errorf strings from net/http. + msg := err.Error() switch { - case strings.Contains(msg, "401") || strings.Contains(msg, "403"): - if strings.Contains(msg, "authentication") || strings.Contains(msg, "unauthorized") || strings.Contains(msg, "forbidden") { - return fmt.Errorf("%w\nHint: Try authenticating with: fj auth login", err) + case strings.Contains(msg, "no such host"), + strings.Contains(msg, "connection refused"), + strings.Contains(msg, "i/o timeout"): + return &CLIError{ + Code: ErrNetworkError, + Message: msg, + Hint: "Check your internet connection and that the host is correct.", } } return err } -// writeJSONError writes a structured JSON error to stderr. -// It attempts to extract structured info from known error types. // WriteJSONError writes a structured JSON error to stderr. // It is exported for use from main.go. func WriteJSONError(err error) { @@ -90,7 +108,9 @@ func WriteJSONError(err error) { Message: err.Error(), } - // Try to extract structured info from the error chain. + // Try to extract structured info from the error chain. Prefer CLIError + // (which carries Hint cleanly) over APIError so a wrapped CLIError + // keeps its structured fields. var apiErr *api.APIError var cErr *CLIError @@ -105,8 +125,6 @@ func WriteJSONError(err error) { cliErr.Code = ErrAuthRequired case apiErr.StatusCode == 404: cliErr.Code = ErrNotFound - default: - cliErr.Code = ErrAPIError } } @@ -114,3 +132,6 @@ func WriteJSONError(err error) { enc.SetIndent("", " ") _ = enc.Encode(cliErr) } + +// Compile-time check that CLIError satisfies the standard error interface. +var _ error = (*CLIError)(nil) diff --git a/cmd/issue.go b/cmd/issue.go index f5e2517..25f54f3 100644 --- a/cmd/issue.go +++ b/cmd/issue.go @@ -221,13 +221,24 @@ func runIssueList(cmd *cobra.Command, args []string) error { } ios.StartSpinner("Fetching issues...") - issues, _, err := client.ListRepoIssues(owner, name, gitea.ListIssueOption{ - State: stateType, - Labels: labels, - KeyWord: search, - CreatedBy: author, - AssignedBy: assignee, - ListOptions: gitea.ListOptions{PageSize: limit}, + // ListRepoIssues returns both issues AND PRs (we filter PRs out below). + // Pull more than `limit` so post-filter we still have `limit` real issues + // — overshoot 2x as a heuristic. paginateGitea(0, ...) would be safer + // but spends extra round-trips; keep it bounded. + fetchLimit := limit * 2 + if fetchLimit < 50 { + fetchLimit = 50 + } + issues, err := paginateGitea(fetchLimit, func(page, pageSize int) ([]*gitea.Issue, error) { + batch, _, err := client.ListRepoIssues(owner, name, gitea.ListIssueOption{ + State: stateType, + Labels: labels, + KeyWord: search, + CreatedBy: author, + AssignedBy: assignee, + ListOptions: gitea.ListOptions{Page: page, PageSize: pageSize}, + }) + return batch, err }) ios.StopSpinner() if err != nil { @@ -240,6 +251,9 @@ func runIssueList(cmd *cobra.Command, args []string) error { nonPRIssues = append(nonPRIssues, issue) } } + if limit > 0 && len(nonPRIssues) > limit { + nonPRIssues = nonPRIssues[:limit] + } if wantJSON(cmd) { return outputJSON(cmd, nonPRIssues) diff --git a/cmd/json.go b/cmd/json.go index 2472449..70ac0be 100644 --- a/cmd/json.go +++ b/cmd/json.go @@ -10,47 +10,48 @@ import ( ) // addJSONFlags adds --json, --json-fields, and --jq flags to a command. -// --json is an optional-value string flag: -// - --json (no value) → output all fields as JSON -// - --json title,state → output only those fields (gh-compatible) // -// --json-fields is kept as a backwards-compatible alias. +// Flag design (BREAKING CHANGE — the previous --json was a string with +// NoOptDefVal=" " so `--json=fields` projected and `--json` alone meant +// "everything". That sentinel produced a `--json string[=" "]` in --help +// and left users guessing about the equals sign). Now: +// +// - --json : Bool. "Output the response as JSON." (all fields) +// - --json-fields … : String. Comma-separated projection. +// - --jq … : String. jq expression filter. +// +// --json and --json-fields are mutually exclusive — pick one. --jq composes +// with either (or neither, in which case it implies "as JSON"). func addJSONFlags(cmd *cobra.Command, jsonDesc string) { f := cmd.Flags() - f.String("json", "", jsonDesc) - f.Lookup("json").NoOptDefVal = " " // space sentinel: flag present with no value - f.String("json-fields", "", "Comma-separated list of JSON fields to include") + f.Bool("json", false, jsonDesc) + f.String("json-fields", "", "Output as JSON, projecting only these comma-separated fields") f.String("jq", "", "Filter JSON output using a jq expression") + cmd.MarkFlagsMutuallyExclusive("json", "json-fields") } -// wantJSON returns true if the user requested JSON output via --json, --json-fields, or --jq. +// wantJSON returns true if the user requested JSON output via --json, +// --json-fields, or --jq. func wantJSON(cmd *cobra.Command) bool { - if j, _ := cmd.Flags().GetString("json"); j != "" { - return true - } - if jq, _ := cmd.Flags().GetString("jq"); jq != "" { + if b, _ := cmd.Flags().GetBool("json"); b { return true } if f, _ := cmd.Flags().GetString("json-fields"); f != "" { return true } + if jq, _ := cmd.Flags().GetString("jq"); jq != "" { + return true + } return false } -// outputJSON writes a value as JSON, respecting --json, --json-fields, and --jq flags. +// outputJSON writes a value as JSON, respecting --json-fields and --jq. +// --json (the bool) is the "no projection, no filter" signal handled +// implicitly: when neither --json-fields nor --jq is set, the whole value +// is emitted. func outputJSON(cmd *cobra.Command, value any) error { - jsonVal, _ := cmd.Flags().GetString("json") - jsonFields, _ := cmd.Flags().GetString("json-fields") + fields, _ := cmd.Flags().GetString("json-fields") jqExpr, _ := cmd.Flags().GetString("jq") - - fields := "" - jsonVal = strings.TrimSpace(jsonVal) - if jsonVal != "" { - fields = jsonVal - } else if jsonFields != "" { - fields = jsonFields - } - return writeJSONFiltered(value, fields, jqExpr) } diff --git a/cmd/paginate.go b/cmd/paginate.go new file mode 100644 index 0000000..8c9737e --- /dev/null +++ b/cmd/paginate.go @@ -0,0 +1,43 @@ +package cmd + +// paginateGitea walks pages of a gitea SDK list method until the response +// is short (last page) or we hit limit. limit=0 means unlimited. +// +// Forgejo/Gitea caps PageSize at 50, so naive `PageSize: limit` for limit > 50 +// silently truncated results across most `fj * list` commands. This helper +// centralizes the loop so every list command paginates consistently. +// +// fetch is called with (page, pageSize) and returns the items for that page. +// The 1-based `page` matches the gitea SDK convention. +func paginateGitea[T any](limit int, fetch func(page, pageSize int) ([]T, error)) ([]T, error) { + const maxPageSize = 50 + pageSize := maxPageSize + if limit > 0 && limit < pageSize { + pageSize = limit + } + + var all []T + for page := 1; ; page++ { + if limit > 0 && len(all) >= limit { + break + } + batch, err := fetch(page, pageSize) + if err != nil { + return all, err + } + if len(batch) == 0 { + break + } + all = append(all, batch...) + // A short page (less than the requested size) is the conventional + // "you've reached the end" signal — saves one extra round-trip. + if len(batch) < pageSize { + break + } + } + + if limit > 0 && len(all) > limit { + all = all[:limit] + } + return all, nil +} diff --git a/cmd/pr.go b/cmd/pr.go index 1210fe1..e38faac 100644 --- a/cmd/pr.go +++ b/cmd/pr.go @@ -252,39 +252,32 @@ func runPRList(cmd *cobra.Command, args []string) error { needsClientFilter := assignee != "" || author != "" || len(labels) > 0 || search != "" || draft || head != "" || base != "" ios.StartSpinner("Fetching pull requests...") + // When client-side filtering is needed, pull pages until exhausted (no + // limit) so we can apply filters; otherwise paginate up to the user's + // limit. Either way, paginate — `PageSize: limit` capped at 50 silently. + fetchPage := func(page, pageSize int) ([]*gitea.PullRequest, error) { + batch, _, err := client.ListRepoPullRequests(owner, name, gitea.ListPullRequestsOptions{ + State: stateType, + ListOptions: gitea.ListOptions{Page: page, PageSize: pageSize}, + }) + return batch, err + } var prs []*gitea.PullRequest if needsClientFilter { - page := 1 - for { - batch, _, err := client.ListRepoPullRequests(owner, name, gitea.ListPullRequestsOptions{ - State: stateType, - ListOptions: gitea.ListOptions{Page: page, PageSize: 50}, - }) - if err != nil { - ios.StopSpinner() - return fmt.Errorf("failed to list pull requests: %w", err) + prs, err = paginateGitea(0, fetchPage) // pull all, then filter + limit + if err == nil { + prs = filterPRs(prs, author, assignee, labels, search, draft, head, base) + if limit > 0 && len(prs) > limit { + prs = prs[:limit] } - prs = append(prs, batch...) - if len(batch) < 50 { - break - } - page++ - } - prs = filterPRs(prs, author, assignee, labels, search, draft, head, base) - if len(prs) > limit { - prs = prs[:limit] } } else { - prs, _, err = client.ListRepoPullRequests(owner, name, gitea.ListPullRequestsOptions{ - State: stateType, - ListOptions: gitea.ListOptions{PageSize: limit}, - }) - if err != nil { - ios.StopSpinner() - return fmt.Errorf("failed to list pull requests: %w", err) - } + prs, err = paginateGitea(limit, fetchPage) } ios.StopSpinner() + if err != nil { + return fmt.Errorf("failed to list pull requests: %w", err) + } if wantJSON(cmd) { return outputJSON(cmd, prs) diff --git a/cmd/repo.go b/cmd/repo.go index b9bd7fc..0514c4d 100644 --- a/cmd/repo.go +++ b/cmd/repo.go @@ -216,17 +216,18 @@ func runRepoList(cmd *cobra.Command, args []string) error { return fmt.Errorf("failed to get user info: %w", err) } - repos, _, err := client.ListUserRepos(user.UserName, gitea.ListReposOptions{}) + limit, _ := cmd.Flags().GetInt("limit") + repos, err := paginateGitea(limit, func(page, pageSize int) ([]*gitea.Repository, error) { + batch, _, err := client.ListUserRepos(user.UserName, gitea.ListReposOptions{ + ListOptions: gitea.ListOptions{Page: page, PageSize: pageSize}, + }) + return batch, err + }) ios.StopSpinner() if err != nil { return fmt.Errorf("failed to list repositories: %w", err) } - limit, _ := cmd.Flags().GetInt("limit") - if limit > 0 && len(repos) > limit { - repos = repos[:limit] - } - if wantJSON(cmd) { return outputJSON(cmd, repos) } diff --git a/cmd/root.go b/cmd/root.go index 0ee2cf0..8b4dcef 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -8,6 +8,7 @@ import ( "strconv" "strings" + "forgejo.zerova.net/public/fj/internal/config" "forgejo.zerova.net/public/fj/internal/git" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -21,7 +22,7 @@ var rootCmd = &cobra.Command{ Short: "Forgejo CLI tool - work seamlessly with Forgejo from the command line", Long: `fj is a command line tool for Forgejo instances (including Codeberg). It brings pull requests, issues, and other Forgejo concepts to the terminal.`, - Version: "0.3.2", + Version: "0.4.0", SilenceErrors: true, } @@ -45,7 +46,12 @@ func init() { func initConfig() { if cfgFile != "" { + // 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). viper.SetConfigFile(cfgFile) + config.SetExplicitConfigPath(cfgFile) } else { home, err := os.UserHomeDir() if err != nil { @@ -66,7 +72,7 @@ func initConfig() { } } - _ = os.MkdirAll(configDir, 0755) + _ = os.MkdirAll(configDir, 0700) viper.AddConfigPath(configDir) viper.SetConfigType("yaml") @@ -77,6 +83,14 @@ func initConfig() { viper.SetEnvPrefix("FJ") _ = viper.ReadInConfig() + + // 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()) + } + } } // parseRepo parses the repository string in the format "owner/name". @@ -143,8 +157,11 @@ func parseIssueArg(arg string) (int64, error) { } // migrateConfigDir copies all files from src to dst (one level, no subdirs). +// 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 { - if err := os.MkdirAll(dst, 0755); err != nil { + if err := os.MkdirAll(dst, 0700); err != nil { return err } entries, err := os.ReadDir(src) @@ -155,21 +172,34 @@ func migrateConfigDir(src, dst string) error { if e.IsDir() { continue } - in, err := os.Open(filepath.Join(src, e.Name())) - if err != nil { - return err - } - out, err := os.OpenFile(filepath.Join(dst, e.Name()), os.O_CREATE|os.O_WRONLY, 0600) - if err != nil { - in.Close() - return err - } - _, err = io.Copy(out, in) - in.Close() - out.Close() - if err != nil { + if err := copyOneConfigFile(filepath.Join(src, e.Name()), filepath.Join(dst, e.Name())); err != nil { return err } } return nil } + +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 +} diff --git a/cmd/wiki.go b/cmd/wiki.go index 97d2b13..63cb504 100644 --- a/cmd/wiki.go +++ b/cmd/wiki.go @@ -266,10 +266,9 @@ func runWikiView(cmd *cobra.Command, args []string) error { return fmt.Errorf("wiki page has no HTML URL") } - jsonFlag, _ := cmd.Flags().GetBool("json") - if jsonFlag { + if wantJSON(cmd) { page.Content = string(content) - return writeJSON(page) + return outputJSON(cmd, page) } if err := ios.StartPager(); err != nil { diff --git a/internal/api/client.go b/internal/api/client.go index 112d0c3..0666357 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -12,10 +12,16 @@ import ( "forgejo.zerova.net/public/fj/internal/config" ) -var sharedHTTPClient = &http.Client{ +// SharedHTTPClient is the package-wide HTTP client. Exported so other +// packages (notably cmd/api.go) can reuse the same timeout and connection +// pooling instead of constructing zero-value clients with no timeout. +var SharedHTTPClient = &http.Client{ Timeout: 30 * time.Second, } +// Internal alias kept so existing call sites compile unchanged. +var sharedHTTPClient = SharedHTTPClient + type Client struct { *gitea.Client hostname string diff --git a/internal/config/config.go b/internal/config/config.go index e0abfca..79c5c0b 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -14,6 +14,17 @@ type Config struct { Hosts map[string]HostConfig `yaml:"hosts"` } +// 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 . 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 } + type HostConfig struct { Hostname string `yaml:"hostname"` Token string `yaml:"token"` @@ -35,6 +46,9 @@ func GetConfigDir() (string, error) { } func GetConfigPath() (string, error) { + if explicitConfigPath != "" { + return explicitConfigPath, nil + } dir, err := GetConfigDir() if err != nil { return "", err