rickub
rickub/clipublic Fork
⑂ b7e1d1f
Branches
feat/10-repo-rename-transfer feat/7-edit-title-and-body feat/8-run-logs-job-filter feat/staleness-notice main test/create-body-dash-stdin
Tags
v0.1.0 v0.1.0-rc.11 v0.1.0-rc.14 v0.1.0-rc.16 v0.1.0-rc.20 v0.1.0-rc.26 v0.1.0-rc.36 v0.1.0-rc.43 v0.1.0-rc.50 v0.1.0-rc.64 v0.2.0 View all branches → View all tags →
Commits
⬇ Clone ▾
git clone https://git.rickub.com/rickub/cli.git
git clone ssh://git@rickub.com/rickub/cli.git

Host key fingerprint (ed25519): SHA256:iycHnxEyq0Q7uyVpB7JlznP0G7JrTPXLYRcAU5CSLhc — verify it before your first connect.

Download ZIP

security: harden the client, config writes, git auth scope, and human outputUnverified

Findings from a security review of the 0.1.0 code, each with a test:

- HTTP: redirects are followed only on the same host and never https→http,
  so the bearer token cannot be carried to another origin (Go keeps
  Authorization for subdomains and same-host downgrades); response bodies
  are capped at 128 MiB so a hostile host cannot exhaust memory.
- Config: saves are atomic (temp file + rename) and always 0600, so a
  crash cannot truncate the token file and a world-readable file left by
  another tool is fixed on the next save instead of inherited.
- repo clone --protocol https: the token's extraHeader is scoped to the
  clone origin (http.<origin>/.extraHeader) so git never sends it to a
  redirect target or a submodule on another host.
- Output: server-provided titles, bodies, and names are stripped of
  terminal control sequences before human display; tables go through a
  filtering writer. JSON and run logs are untouched.
- gosec/staticcheck clean: tabwriter Flush errors are handled, the dead
  ctx helper is gone, and the two shell-outs plus the config read carry
  #nosec annotations with their justification in place.
- go.mod pins toolchain go1.26.8: govulncheck reports stdlib CVEs fixed in
  1.26.6 (net/http, crypto/tls, net/url, encoding/asn1) against older
  patch releases; the 0.1.0 release was built with 1.26.8 and is not
  affected, but a local 1.26.4 build would have been.

Adds .github/workflows/security.yml (push, PR, weekly): go vet, go mod
verify + tidy check, govulncheck, gosec, staticcheck, and a clean-tree
release build, with scanner versions pinned. Adds SECURITY.md.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FgkcT3UPxwxukBKojB5eg4
Olivier Girardot committed 2026-09-03T08:01:25+02:00 Browse files
b7e1d1f parent: fe71cc6
added .github/workflows/security.yml +82 -0
new file mode 100644
@@ -0,0 +1,82 @@
1+name: Security
2+
3+# Static and dependency security checks, separate from the fast CI gate so a
4+# scanner outage or a new advisory cannot block an ordinary build, and so the
5+# weekly run keeps checking main against new advisories between commits.
6+#
7+# NOTE: no workflow_dispatch here — the rickub dispatch API fires *every*
8+# dispatchable workflow on a ref, so only release.yml may declare it. See
9+# RELEASING.md.
10+on:
11+ push:
12+ branches:
13+ - main
14+ pull_request:
15+ schedule:
16+ # Mondays 06:00 UTC: catch advisories published since the last push.
17+ - cron: "0 6 * * 1"
18+
19+permissions:
20+ contents: read
21+
22+concurrency:
23+ group: security-${{ github.ref }}
24+ cancel-in-progress: true
25+
26+# Scanner versions are pinned; bumping one is a reviewed change, not a surprise.
27+env:
28+ GOVULNCHECK_VERSION: v1.7.0
29+ GOSEC_VERSION: v2.29.0
30+ STATICCHECK_VERSION: v0.8.1
31+
32+jobs:
33+ scan:
34+ name: vet / vuln / gosec / staticcheck / modules
35+ runs-on: ubuntu-latest
36+ steps:
37+ - name: Checkout
38+ uses: actions/checkout@v4
39+
40+ - name: Set up Go
41+ uses: actions/setup-go@v5
42+ with:
43+ go-version-file: go.mod
44+ cache: true
45+
46+ - name: go vet
47+ run: go vet ./...
48+
49+ - name: Module integrity
50+ # go.sum must match what the proxy serves, and go.mod/go.sum must be
51+ # tidy: an untracked requirement is a dependency nobody reviewed.
52+ run: |
53+ set -euo pipefail
54+ go mod verify
55+ go mod tidy
56+ if ! git diff --exit-code -- go.mod go.sum; then
57+ echo "::error::go.mod/go.sum are not tidy; run 'go mod tidy' and commit"
58+ exit 1
59+ fi
60+
61+ - name: govulncheck
62+ # Known vulnerabilities in the standard library and in every module the
63+ # binary links, reported only where our code actually reaches them.
64+ run: go run golang.org/x/vuln/cmd/govulncheck@${GOVULNCHECK_VERSION} ./...
65+
66+ - name: gosec
67+ # Static analysis for insecure patterns (command injection, file modes,
68+ # unchecked errors…). Every accepted finding carries a '#nosec' with a
69+ # reason next to the code; there is no global exclude list.
70+ run: go run github.com/securego/gosec/v2/cmd/gosec@${GOSEC_VERSION} -quiet ./...
71+
72+ - name: staticcheck
73+ run: go run honnef.co/go/tools/cmd/staticcheck@${STATICCHECK_VERSION} ./...
74+
75+ - name: Release binary is reproducible from a clean tree
76+ # The same flags as scripts/build-dist.sh. Guards against a build that
77+ # only works with something untracked lying around.
78+ run: |
79+ set -euo pipefail
80+ git diff --quiet || { echo "::error::working tree dirty before build"; git status --short; exit 1; }
81+ CGO_ENABLED=0 go build -trimpath -ldflags "-s -w -X rickub.com/rickub/cli/cmd.Version=security-scan" -o "${RUNNER_TEMP}/rickub" .
82+ "${RUNNER_TEMP}/rickub" version
new file mode 100644
@@ -0,0 +1,82 @@
1+name: Security
2+
3+# Static and dependency security checks, separate from the fast CI gate so a
4+# scanner outage or a new advisory cannot block an ordinary build, and so the
5+# weekly run keeps checking main against new advisories between commits.
6+#
7+# NOTE: no workflow_dispatch here — the rickub dispatch API fires *every*
8+# dispatchable workflow on a ref, so only release.yml may declare it. See
9+# RELEASING.md.
10+on:
11+ push:
12+ branches:
13+ - main
14+ pull_request:
15+ schedule:
16+ # Mondays 06:00 UTC: catch advisories published since the last push.
17+ - cron: "0 6 * * 1"
18+
19+permissions:
20+ contents: read
21+
22+concurrency:
23+ group: security-${{ github.ref }}
24+ cancel-in-progress: true
25+
26+# Scanner versions are pinned; bumping one is a reviewed change, not a surprise.
27+env:
28+ GOVULNCHECK_VERSION: v1.7.0
29+ GOSEC_VERSION: v2.29.0
30+ STATICCHECK_VERSION: v0.8.1
31+
32+jobs:
33+ scan:
34+ name: vet / vuln / gosec / staticcheck / modules
35+ runs-on: ubuntu-latest
36+ steps:
37+ - name: Checkout
38+ uses: actions/checkout@v4
39+
40+ - name: Set up Go
41+ uses: actions/setup-go@v5
42+ with:
43+ go-version-file: go.mod
44+ cache: true
45+
46+ - name: go vet
47+ run: go vet ./...
48+
49+ - name: Module integrity
50+ # go.sum must match what the proxy serves, and go.mod/go.sum must be
51+ # tidy: an untracked requirement is a dependency nobody reviewed.
52+ run: |
53+ set -euo pipefail
54+ go mod verify
55+ go mod tidy
56+ if ! git diff --exit-code -- go.mod go.sum; then
57+ echo "::error::go.mod/go.sum are not tidy; run 'go mod tidy' and commit"
58+ exit 1
59+ fi
60+
61+ - name: govulncheck
62+ # Known vulnerabilities in the standard library and in every module the
63+ # binary links, reported only where our code actually reaches them.
64+ run: go run golang.org/x/vuln/cmd/govulncheck@${GOVULNCHECK_VERSION} ./...
65+
66+ - name: gosec
67+ # Static analysis for insecure patterns (command injection, file modes,
68+ # unchecked errors…). Every accepted finding carries a '#nosec' with a
69+ # reason next to the code; there is no global exclude list.
70+ run: go run github.com/securego/gosec/v2/cmd/gosec@${GOSEC_VERSION} -quiet ./...
71+
72+ - name: staticcheck
73+ run: go run honnef.co/go/tools/cmd/staticcheck@${STATICCHECK_VERSION} ./...
74+
75+ - name: Release binary is reproducible from a clean tree
76+ # The same flags as scripts/build-dist.sh. Guards against a build that
77+ # only works with something untracked lying around.
78+ run: |
79+ set -euo pipefail
80+ git diff --quiet || { echo "::error::working tree dirty before build"; git status --short; exit 1; }
81+ CGO_ENABLED=0 go build -trimpath -ldflags "-s -w -X rickub.com/rickub/cli/cmd.Version=security-scan" -o "${RUNNER_TEMP}/rickub" .
82+ "${RUNNER_TEMP}/rickub" version
modified README.md +9 -0
@@ -247,12 +247,21 @@ rickub repo list --user <you>
247247
248248 The dev token is stored separately from your rickub.com token; both stay put.
249249
250+## Security
251+
252+See [SECURITY.md](SECURITY.md): where the token lives and how it is bound to
253+its host, the redirect and body-size policy, how `git` and the browser are
254+invoked, and what the security workflow checks.
255+
250256 ## Development
251257
252258 ```sh
253259 go build ./... && go test ./... && go vet ./... && gofmt -l .
254260 ```
255261
262+CI runs those on every push and pull request (`ci.yml`), and `security.yml`
263+adds `go mod verify`, `govulncheck`, `gosec`, and `staticcheck`.
264+
256265 Layout:
257266
258267 ```
@@ -247,12 +247,21 @@ rickub repo list --user <you>
247 247
248 The dev token is stored separately from your rickub.com token; both stay put.248 The dev token is stored separately from your rickub.com token; both stay put.
249 249
250+## Security
251+
252+See [SECURITY.md](SECURITY.md): where the token lives and how it is bound to
253+its host, the redirect and body-size policy, how `git` and the browser are
254+invoked, and what the security workflow checks.
255+
250 ## Development256 ## Development
251 257
252 ```sh258 ```sh
253 go build ./... && go test ./... && go vet ./... && gofmt -l .259 go build ./... && go test ./... && go vet ./... && gofmt -l .
254 ```260 ```
255 261
262+CI runs those on every push and pull request (`ci.yml`), and `security.yml`
263+adds `go mod verify`, `govulncheck`, `gosec`, and `staticcheck`.
264+
256 Layout:265 Layout:
257 266
258 ```267 ```
added SECURITY.md +62 -0
new file mode 100644
@@ -0,0 +1,62 @@
1+# Security
2+
3+## What the CLI holds
4+
5+The only secret `rickub` handles is your personal access token (PAT).
6+
7+- It is stored in `~/.config/rickub/config.yaml` (honouring `XDG_CONFIG_HOME`),
8+ written atomically with mode `0600` in a `0700` directory. A save always
9+ re-applies `0600`, even over a file something else made world-readable.
10+- A stored token is bound to the host it was minted for and is only ever sent
11+ there. `--host` or `RICKUB_HOST` pointing elsewhere gets "no token stored
12+ for that host", never your production credential.
13+- A token given explicitly (`--token`, `RICKUB_TOKEN`) is sent to whatever host
14+ is in effect; the CLI warns before sending it over plain `http://` to a
15+ non-loopback address.
16+- `auth status` prints only the non-secret `rickub_pat_` prefix. `--token` is
17+ discouraged in the help text because arguments are visible to other
18+ processes and end up in shell history; prefer `auth login --with-token`
19+ (stdin) or the environment variable.
20+
21+## Network behaviour
22+
23+- Every request goes to `<host>/api/v1/…` with the token as a bearer header.
24+- Redirects are followed only on the same host and never from https to http;
25+ anything else is refused, so a redirect cannot carry the token away.
26+- Response bodies are capped (128 MiB) so a hostile host cannot exhaust memory.
27+- No TLS verification is ever disabled.
28+
29+## Shelling out
30+
31+- `repo clone` runs `git` with a URL assembled from a fixed scheme and host and
32+ the `owner/repo` you typed; anything after `--` is passed to git unchanged.
33+ With `--protocol https` the token reaches git through its environment as a
34+ header scoped to that one origin (`http.<origin>/.extraHeader`): never on
35+ the command line, never written to `.git/config`, never sent to a redirect
36+ target or another host's submodule.
37+- `browse` and the device login hand a URL to the platform opener only after
38+ checking it is `http`/`https` with a host, so no other scheme handler can be
39+ reached.
40+- Repo inference runs `git remote get-url origin` in the current directory.
41+
42+## Output
43+
44+Text that comes from the server (titles, bodies, names) is stripped of terminal
45+control sequences before it is printed for a human, so a crafted issue title
46+cannot repaint your terminal. `--json` output and `run logs` are passed through
47+untouched.
48+
49+## Continuous checks
50+
51+`.github/workflows/security.yml` runs on every push and pull request and weekly:
52+`go vet`, `go mod verify` + tidy check, `govulncheck` (stdlib and dependency
53+advisories reachable from this code), `gosec` (every accepted finding is
54+annotated in place with a reason), `staticcheck`, and a clean-tree release
55+build. `go.mod` pins the toolchain so builds do not silently use an older Go
56+with known CVEs.
57+
58+## Reporting
59+
60+If you find a vulnerability, please report it privately to the maintainer
61+(see the commit authors) rather than in a public issue; open an issue at
62+<https://rickub.com/rickub/cli/issues> for anything that is not sensitive.
new file mode 100644
@@ -0,0 +1,62 @@
1+# Security
2+
3+## What the CLI holds
4+
5+The only secret `rickub` handles is your personal access token (PAT).
6+
7+- It is stored in `~/.config/rickub/config.yaml` (honouring `XDG_CONFIG_HOME`),
8+ written atomically with mode `0600` in a `0700` directory. A save always
9+ re-applies `0600`, even over a file something else made world-readable.
10+- A stored token is bound to the host it was minted for and is only ever sent
11+ there. `--host` or `RICKUB_HOST` pointing elsewhere gets "no token stored
12+ for that host", never your production credential.
13+- A token given explicitly (`--token`, `RICKUB_TOKEN`) is sent to whatever host
14+ is in effect; the CLI warns before sending it over plain `http://` to a
15+ non-loopback address.
16+- `auth status` prints only the non-secret `rickub_pat_` prefix. `--token` is
17+ discouraged in the help text because arguments are visible to other
18+ processes and end up in shell history; prefer `auth login --with-token`
19+ (stdin) or the environment variable.
20+
21+## Network behaviour
22+
23+- Every request goes to `<host>/api/v1/…` with the token as a bearer header.
24+- Redirects are followed only on the same host and never from https to http;
25+ anything else is refused, so a redirect cannot carry the token away.
26+- Response bodies are capped (128 MiB) so a hostile host cannot exhaust memory.
27+- No TLS verification is ever disabled.
28+
29+## Shelling out
30+
31+- `repo clone` runs `git` with a URL assembled from a fixed scheme and host and
32+ the `owner/repo` you typed; anything after `--` is passed to git unchanged.
33+ With `--protocol https` the token reaches git through its environment as a
34+ header scoped to that one origin (`http.<origin>/.extraHeader`): never on
35+ the command line, never written to `.git/config`, never sent to a redirect
36+ target or another host's submodule.
37+- `browse` and the device login hand a URL to the platform opener only after
38+ checking it is `http`/`https` with a host, so no other scheme handler can be
39+ reached.
40+- Repo inference runs `git remote get-url origin` in the current directory.
41+
42+## Output
43+
44+Text that comes from the server (titles, bodies, names) is stripped of terminal
45+control sequences before it is printed for a human, so a crafted issue title
46+cannot repaint your terminal. `--json` output and `run logs` are passed through
47+untouched.
48+
49+## Continuous checks
50+
51+`.github/workflows/security.yml` runs on every push and pull request and weekly:
52+`go vet`, `go mod verify` + tidy check, `govulncheck` (stdlib and dependency
53+advisories reachable from this code), `gosec` (every accepted finding is
54+annotated in place with a reason), `staticcheck`, and a clean-tree release
55+build. `go.mod` pins the toolchain so builds do not silently use an older Go
56+with known CVEs.
57+
58+## Reporting
59+
60+If you find a vulnerability, please report it privately to the maintainer
61+(see the commit authors) rather than in a public issue; open an issue at
62+<https://rickub.com/rickub/cli/issues> for anything that is not sensitive.
modified cmd/browse.go +2 -0
@@ -87,5 +87,7 @@ func openBrowser(url string) error {
8787 name = "xdg-open"
8888 args = []string{url}
8989 }
90+ // #nosec G204 -- name is a fixed per-OS opener and url has just been checked
91+ // to be an http(s) URL with a host, so no other scheme handler can be reached.
9092 return exec.Command(name, args...).Start()
9193 }
@@ -87,5 +87,7 @@ func openBrowser(url string) error {
87 name = "xdg-open"87 name = "xdg-open"
88 args = []string{url}88 args = []string{url}
89 }89 }
90+ // #nosec G204 -- name is a fixed per-OS opener and url has just been checked
91+ // to be an http(s) URL with a host, so no other scheme handler can be reached.
90 return exec.Command(name, args...).Start()92 return exec.Command(name, args...).Start()
91 }93 }
modified cmd/collaborator.go +1 -2
@@ -68,8 +68,7 @@ func runCollabList(cmd *cobra.Command, args []string) error {
6868 for _, c := range cols {
6969 fmt.Fprintf(tw, "%s\t%s\t%s\n", c.Handle, c.Permission, dash(c.DisplayName))
7070 }
71- tw.Flush()
72- return nil
71+ return tw.Flush()
7372 }
7473
7574 func runCollabAdd(cmd *cobra.Command, args []string) error {
@@ -68,8 +68,7 @@ func runCollabList(cmd *cobra.Command, args []string) error {
68 for _, c := range cols {68 for _, c := range cols {
69 fmt.Fprintf(tw, "%s\t%s\t%s\n", c.Handle, c.Permission, dash(c.DisplayName))69 fmt.Fprintf(tw, "%s\t%s\t%s\n", c.Handle, c.Permission, dash(c.DisplayName))
70 }70 }
71- tw.Flush()71+ return tw.Flush()
72- return nil
73 }72 }
74 73
75 func runCollabAdd(cmd *cobra.Command, args []string) error {74 func runCollabAdd(cmd *cobra.Command, args []string) error {
modified cmd/issue.go +7 -6
@@ -175,7 +175,9 @@ func runIssueList(cmd *cobra.Command, _ []string) error {
175175 }
176176 fmt.Fprintf(tw, "%d\t%s\t%s\t%s\t%s\n", i.Number, i.State, dash(labels), dash(milestone), i.Title)
177177 }
178- tw.Flush()
178+ if err := tw.Flush(); err != nil {
179+ return err
180+ }
179181 printPageFooter(cmd, page.Page)
180182 return nil
181183 }
@@ -205,7 +207,7 @@ func runIssueView(cmd *cobra.Command, args []string) error {
205207 if i.Milestone != nil {
206208 state += " · " + i.Milestone.Title
207209 }
208- fmt.Fprintf(out, "Issue #%d %s [%s]\n", i.Number, i.Title, state)
210+ fmt.Fprintf(out, "Issue #%d %s [%s]\n", i.Number, clean(i.Title), state)
209211 fmt.Fprintf(out, "by %s · %s\n", dash(i.Author), humanTime(i.CreatedAt))
210212 if len(i.Labels) > 0 {
211213 names := make([]string, len(i.Labels))
@@ -223,10 +225,10 @@ func runIssueView(cmd *cobra.Command, args []string) error {
223225 }
224226 fmt.Fprintln(out)
225227 if i.Body != "" {
226- fmt.Fprintln(out, i.Body)
228+ fmt.Fprintln(out, clean(i.Body))
227229 }
228230 for _, c := range i.Comments {
229- fmt.Fprintf(out, "\n--- %s (%s) ---\n%s\n", c.Author, humanTime(c.CreatedAt), c.Body)
231+ fmt.Fprintf(out, "\n--- %s (%s) ---\n%s\n", clean(c.Author), humanTime(c.CreatedAt), clean(c.Body))
230232 }
231233 return nil
232234 }
@@ -449,6 +451,5 @@ func runIssueLabelsList(cmd *cobra.Command, _ []string) error {
449451 for _, l := range labels {
450452 fmt.Fprintf(tw, "%s\t#%s\n", l.Name, l.Color)
451453 }
452- tw.Flush()
453- return nil
454+ return tw.Flush()
454455 }
@@ -175,7 +175,9 @@ func runIssueList(cmd *cobra.Command, _ []string) error {
175 }175 }
176 fmt.Fprintf(tw, "%d\t%s\t%s\t%s\t%s\n", i.Number, i.State, dash(labels), dash(milestone), i.Title)176 fmt.Fprintf(tw, "%d\t%s\t%s\t%s\t%s\n", i.Number, i.State, dash(labels), dash(milestone), i.Title)
177 }177 }
178- tw.Flush()178+ if err := tw.Flush(); err != nil {
179+ return err
180+ }
179 printPageFooter(cmd, page.Page)181 printPageFooter(cmd, page.Page)
180 return nil182 return nil
181 }183 }
@@ -205,7 +207,7 @@ func runIssueView(cmd *cobra.Command, args []string) error {
205 if i.Milestone != nil {207 if i.Milestone != nil {
206 state += " · " + i.Milestone.Title208 state += " · " + i.Milestone.Title
207 }209 }
208- fmt.Fprintf(out, "Issue #%d %s [%s]\n", i.Number, i.Title, state)210+ fmt.Fprintf(out, "Issue #%d %s [%s]\n", i.Number, clean(i.Title), state)
209 fmt.Fprintf(out, "by %s · %s\n", dash(i.Author), humanTime(i.CreatedAt))211 fmt.Fprintf(out, "by %s · %s\n", dash(i.Author), humanTime(i.CreatedAt))
210 if len(i.Labels) > 0 {212 if len(i.Labels) > 0 {
211 names := make([]string, len(i.Labels))213 names := make([]string, len(i.Labels))
@@ -223,10 +225,10 @@ func runIssueView(cmd *cobra.Command, args []string) error {
223 }225 }
224 fmt.Fprintln(out)226 fmt.Fprintln(out)
225 if i.Body != "" {227 if i.Body != "" {
226- fmt.Fprintln(out, i.Body)228+ fmt.Fprintln(out, clean(i.Body))
227 }229 }
228 for _, c := range i.Comments {230 for _, c := range i.Comments {
229- fmt.Fprintf(out, "\n--- %s (%s) ---\n%s\n", c.Author, humanTime(c.CreatedAt), c.Body)231+ fmt.Fprintf(out, "\n--- %s (%s) ---\n%s\n", clean(c.Author), humanTime(c.CreatedAt), clean(c.Body))
230 }232 }
231 return nil233 return nil
232 }234 }
@@ -449,6 +451,5 @@ func runIssueLabelsList(cmd *cobra.Command, _ []string) error {
449 for _, l := range labels {451 for _, l := range labels {
450 fmt.Fprintf(tw, "%s\t#%s\n", l.Name, l.Color)452 fmt.Fprintf(tw, "%s\t#%s\n", l.Name, l.Color)
451 }453 }
452- tw.Flush()454+ return tw.Flush()
453- return nil
454 }455 }
modified cmd/milestone.go +1 -2
@@ -92,8 +92,7 @@ func runMilestoneList(cmd *cobra.Command, _ []string) error {
9292 progress := fmt.Sprintf("%d/%d", m.ClosedCount, m.OpenCount+m.ClosedCount)
9393 fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\n", m.ID, m.State, dash(deref(m.DueOn)), progress, m.Title)
9494 }
95- tw.Flush()
96- return nil
95+ return tw.Flush()
9796 }
9897
9998 func deref(s *string) string {
@@ -92,8 +92,7 @@ func runMilestoneList(cmd *cobra.Command, _ []string) error {
92 progress := fmt.Sprintf("%d/%d", m.ClosedCount, m.OpenCount+m.ClosedCount)92 progress := fmt.Sprintf("%d/%d", m.ClosedCount, m.OpenCount+m.ClosedCount)
93 fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\n", m.ID, m.State, dash(deref(m.DueOn)), progress, m.Title)93 fmt.Fprintf(tw, "%s\t%s\t%s\t%s\t%s\n", m.ID, m.State, dash(deref(m.DueOn)), progress, m.Title)
94 }94 }
95- tw.Flush()95+ return tw.Flush()
96- return nil
97 }96 }
98 97
99 func deref(s *string) string {98 func deref(s *string) string {
modified cmd/org.go +2 -4
@@ -77,8 +77,7 @@ func runOrgMembers(cmd *cobra.Command, args []string) error {
7777 for _, m := range members {
7878 fmt.Fprintf(tw, "%s\t%s\t%s\n", m.Handle, m.Role, dash(m.DisplayName))
7979 }
80- tw.Flush()
81- return nil
80+ return tw.Flush()
8281 }
8382
8483 func runOrgTeams(cmd *cobra.Command, args []string) error {
@@ -102,6 +101,5 @@ func runOrgTeams(cmd *cobra.Command, args []string) error {
102101 for _, t := range teams {
103102 fmt.Fprintf(tw, "%s\t%s\t%v\t%s\n", t.Slug, dash(t.Name), t.SubTeam, dash(t.Description))
104103 }
105- tw.Flush()
106- return nil
104+ return tw.Flush()
107105 }
@@ -77,8 +77,7 @@ func runOrgMembers(cmd *cobra.Command, args []string) error {
77 for _, m := range members {77 for _, m := range members {
78 fmt.Fprintf(tw, "%s\t%s\t%s\n", m.Handle, m.Role, dash(m.DisplayName))78 fmt.Fprintf(tw, "%s\t%s\t%s\n", m.Handle, m.Role, dash(m.DisplayName))
79 }79 }
80- tw.Flush()80+ return tw.Flush()
81- return nil
82 }81 }
83 82
84 func runOrgTeams(cmd *cobra.Command, args []string) error {83 func runOrgTeams(cmd *cobra.Command, args []string) error {
@@ -102,6 +101,5 @@ func runOrgTeams(cmd *cobra.Command, args []string) error {
102 for _, t := range teams {101 for _, t := range teams {
103 fmt.Fprintf(tw, "%s\t%s\t%v\t%s\n", t.Slug, dash(t.Name), t.SubTeam, dash(t.Description))102 fmt.Fprintf(tw, "%s\t%s\t%v\t%s\n", t.Slug, dash(t.Name), t.SubTeam, dash(t.Description))
104 }103 }
105- tw.Flush()104+ return tw.Flush()
106- return nil
107 }105 }
modified cmd/pr.go +6 -4
@@ -137,7 +137,9 @@ func runPRList(cmd *cobra.Command, _ []string) error {
137137 for _, mr := range page.Items {
138138 fmt.Fprintf(tw, "%d\t%s\t%s\t%s→%s\t%s\n", mr.Number, prStateLabel(mr), mr.Title, mr.HeadBranch, mr.BaseBranch, mr.Author)
139139 }
140- tw.Flush()
140+ if err := tw.Flush(); err != nil {
141+ return err
142+ }
141143 printPageFooter(cmd, page.Page)
142144 return nil
143145 }
@@ -163,13 +165,13 @@ func runPRView(cmd *cobra.Command, args []string) error {
163165 return printJSON(cmd.OutOrStdout(), mr)
164166 }
165167 out := cmd.OutOrStdout()
166- fmt.Fprintf(out, "#%d %s [%s]\n", mr.Number, mr.Title, prStateLabel(mr.MergeRequest))
168+ fmt.Fprintf(out, "#%d %s [%s]\n", mr.Number, clean(mr.Title), prStateLabel(mr.MergeRequest))
167169 fmt.Fprintf(out, "%s wants to merge %s → %s\n", mr.Author, mr.HeadBranch, mr.BaseBranch)
168170 if mr.CrossRepo {
169171 fmt.Fprintf(out, "cross-repo from %s/%s\n", mr.HeadOwner, mr.HeadRepo)
170172 }
171173 if mr.Body != "" {
172- fmt.Fprintf(out, "\n%s\n", mr.Body)
174+ fmt.Fprintf(out, "\n%s\n", clean(mr.Body))
173175 }
174176 if len(mr.Reviewers) > 0 {
175177 fmt.Fprintf(out, "\nReviewers: %s\n", joinSubjects(mr.Reviewers))
@@ -186,7 +188,7 @@ func runPRView(cmd *cobra.Command, args []string) error {
186188 if len(mr.Comments) > 0 {
187189 fmt.Fprintln(out, "\nComments:")
188190 for _, c := range mr.Comments {
189- fmt.Fprintf(out, " %s (%s):\n %s\n", c.Author, humanTime(c.CreatedAt), c.Body)
191+ fmt.Fprintf(out, " %s (%s):\n %s\n", clean(c.Author), humanTime(c.CreatedAt), clean(c.Body))
190192 }
191193 }
192194 return nil
@@ -137,7 +137,9 @@ func runPRList(cmd *cobra.Command, _ []string) error {
137 for _, mr := range page.Items {137 for _, mr := range page.Items {
138 fmt.Fprintf(tw, "%d\t%s\t%s\t%s→%s\t%s\n", mr.Number, prStateLabel(mr), mr.Title, mr.HeadBranch, mr.BaseBranch, mr.Author)138 fmt.Fprintf(tw, "%d\t%s\t%s\t%s→%s\t%s\n", mr.Number, prStateLabel(mr), mr.Title, mr.HeadBranch, mr.BaseBranch, mr.Author)
139 }139 }
140- tw.Flush()140+ if err := tw.Flush(); err != nil {
141+ return err
142+ }
141 printPageFooter(cmd, page.Page)143 printPageFooter(cmd, page.Page)
142 return nil144 return nil
143 }145 }
@@ -163,13 +165,13 @@ func runPRView(cmd *cobra.Command, args []string) error {
163 return printJSON(cmd.OutOrStdout(), mr)165 return printJSON(cmd.OutOrStdout(), mr)
164 }166 }
165 out := cmd.OutOrStdout()167 out := cmd.OutOrStdout()
166- fmt.Fprintf(out, "#%d %s [%s]\n", mr.Number, mr.Title, prStateLabel(mr.MergeRequest))168+ fmt.Fprintf(out, "#%d %s [%s]\n", mr.Number, clean(mr.Title), prStateLabel(mr.MergeRequest))
167 fmt.Fprintf(out, "%s wants to merge %s → %s\n", mr.Author, mr.HeadBranch, mr.BaseBranch)169 fmt.Fprintf(out, "%s wants to merge %s → %s\n", mr.Author, mr.HeadBranch, mr.BaseBranch)
168 if mr.CrossRepo {170 if mr.CrossRepo {
169 fmt.Fprintf(out, "cross-repo from %s/%s\n", mr.HeadOwner, mr.HeadRepo)171 fmt.Fprintf(out, "cross-repo from %s/%s\n", mr.HeadOwner, mr.HeadRepo)
170 }172 }
171 if mr.Body != "" {173 if mr.Body != "" {
172- fmt.Fprintf(out, "\n%s\n", mr.Body)174+ fmt.Fprintf(out, "\n%s\n", clean(mr.Body))
173 }175 }
174 if len(mr.Reviewers) > 0 {176 if len(mr.Reviewers) > 0 {
175 fmt.Fprintf(out, "\nReviewers: %s\n", joinSubjects(mr.Reviewers))177 fmt.Fprintf(out, "\nReviewers: %s\n", joinSubjects(mr.Reviewers))
@@ -186,7 +188,7 @@ func runPRView(cmd *cobra.Command, args []string) error {
186 if len(mr.Comments) > 0 {188 if len(mr.Comments) > 0 {
187 fmt.Fprintln(out, "\nComments:")189 fmt.Fprintln(out, "\nComments:")
188 for _, c := range mr.Comments {190 for _, c := range mr.Comments {
189- fmt.Fprintf(out, " %s (%s):\n %s\n", c.Author, humanTime(c.CreatedAt), c.Body)191+ fmt.Fprintf(out, " %s (%s):\n %s\n", clean(c.Author), humanTime(c.CreatedAt), clean(c.Body))
190 }192 }
191 }193 }
192 return nil194 return nil
modified cmd/release_smoke_test.go +35 -6
@@ -38,23 +38,52 @@ func TestCloneURLFor(t *testing.T) {
3838 }
3939 }
4040
41-// The token reaches git as an extraHeader through GIT_CONFIG_*, appended after
42-// any entries the caller's environment already carries, so nothing of the
43-// user's own git config is clobbered and the token never touches argv.
41+// The token reaches git as an extraHeader through GIT_CONFIG_*, scoped to the
42+// clone origin so it is never sent to another host, appended after any entries
43+// the caller's environment already carries, and never on argv.
4444 func TestGitAuthEnv(t *testing.T) {
4545 t.Setenv("GIT_CONFIG_COUNT", "")
46- got := gitAuthEnv("rickub_pat_x")
47- want := []string{"GIT_CONFIG_COUNT=1", "GIT_CONFIG_KEY_0=http.extraHeader", "GIT_CONFIG_VALUE_0=Authorization: Bearer rickub_pat_x"}
46+ got := gitAuthEnv("rickub_pat_x", "https://git.rickub.com/rickub/cli.git")
47+ want := []string{
48+ "GIT_CONFIG_COUNT=1",
49+ "GIT_CONFIG_KEY_0=http.https://git.rickub.com/.extraHeader",
50+ "GIT_CONFIG_VALUE_0=Authorization: Bearer rickub_pat_x",
51+ }
4852 if strings.Join(got, "\n") != strings.Join(want, "\n") {
4953 t.Errorf("gitAuthEnv = %q, want %q", got, want)
5054 }
5155 t.Setenv("GIT_CONFIG_COUNT", "2")
52- got = gitAuthEnv("rickub_pat_x")
56+ got = gitAuthEnv("rickub_pat_x", "https://git.rickub.com/rickub/cli.git")
5357 if got[0] != "GIT_CONFIG_COUNT=3" || !strings.HasPrefix(got[1], "GIT_CONFIG_KEY_2=") {
5458 t.Errorf("existing entries not preserved: %q", got)
5559 }
5660 }
5761
62+// Server-provided text is printed for humans without terminal control
63+// sequences: an issue title cannot clear the screen or retitle the window.
64+func TestCleanStripsTerminalControls(t *testing.T) {
65+ in := "ok\x1b[2J\x1b]0;pwned\x07 title\ttab\nline\x7f\u0085\u009b"
66+ got := clean(in)
67+ want := "ok[2J]0;pwned title\ttab\nline"
68+ if got != want {
69+ t.Errorf("clean(%q) = %q, want %q", in, got, want)
70+ }
71+ if clean("plain ünïcödé → text") != "plain ünïcödé → text" {
72+ t.Error("clean must leave ordinary text alone")
73+ }
74+ var b strings.Builder
75+ tw := newTabw(&b)
76+ if _, err := tw.Write([]byte("a\x1b[31mred\tb\n")); err != nil {
77+ t.Fatal(err)
78+ }
79+ if err := tw.Flush(); err != nil {
80+ t.Fatal(err)
81+ }
82+ if strings.Contains(b.String(), "\x1b") {
83+ t.Errorf("table output still carries ESC: %q", b.String())
84+ }
85+}
86+
5887 // pr create must say "draft": false out loud — the API's default is a draft,
5988 // which `pr merge` then refuses (issue #2) — and --draft flips it.
6089 func TestPRCreateSendsDraftExplicitly(t *testing.T) {
@@ -38,23 +38,52 @@ func TestCloneURLFor(t *testing.T) {
38 }38 }
39 }39 }
40 40
41-// The token reaches git as an extraHeader through GIT_CONFIG_*, appended after41+// The token reaches git as an extraHeader through GIT_CONFIG_*, scoped to the
42-// any entries the caller's environment already carries, so nothing of the42+// clone origin so it is never sent to another host, appended after any entries
43-// user's own git config is clobbered and the token never touches argv.43+// the caller's environment already carries, and never on argv.
44 func TestGitAuthEnv(t *testing.T) {44 func TestGitAuthEnv(t *testing.T) {
45 t.Setenv("GIT_CONFIG_COUNT", "")45 t.Setenv("GIT_CONFIG_COUNT", "")
46- got := gitAuthEnv("rickub_pat_x")46+ got := gitAuthEnv("rickub_pat_x", "https://git.rickub.com/rickub/cli.git")
47- want := []string{"GIT_CONFIG_COUNT=1", "GIT_CONFIG_KEY_0=http.extraHeader", "GIT_CONFIG_VALUE_0=Authorization: Bearer rickub_pat_x"}47+ want := []string{
48+ "GIT_CONFIG_COUNT=1",
49+ "GIT_CONFIG_KEY_0=http.https://git.rickub.com/.extraHeader",
50+ "GIT_CONFIG_VALUE_0=Authorization: Bearer rickub_pat_x",
51+ }
48 if strings.Join(got, "\n") != strings.Join(want, "\n") {52 if strings.Join(got, "\n") != strings.Join(want, "\n") {
49 t.Errorf("gitAuthEnv = %q, want %q", got, want)53 t.Errorf("gitAuthEnv = %q, want %q", got, want)
50 }54 }
51 t.Setenv("GIT_CONFIG_COUNT", "2")55 t.Setenv("GIT_CONFIG_COUNT", "2")
52- got = gitAuthEnv("rickub_pat_x")56+ got = gitAuthEnv("rickub_pat_x", "https://git.rickub.com/rickub/cli.git")
53 if got[0] != "GIT_CONFIG_COUNT=3" || !strings.HasPrefix(got[1], "GIT_CONFIG_KEY_2=") {57 if got[0] != "GIT_CONFIG_COUNT=3" || !strings.HasPrefix(got[1], "GIT_CONFIG_KEY_2=") {
54 t.Errorf("existing entries not preserved: %q", got)58 t.Errorf("existing entries not preserved: %q", got)
55 }59 }
56 }60 }
57 61
62+// Server-provided text is printed for humans without terminal control
63+// sequences: an issue title cannot clear the screen or retitle the window.
64+func TestCleanStripsTerminalControls(t *testing.T) {
65+ in := "ok\x1b[2J\x1b]0;pwned\x07 title\ttab\nline\x7f\u0085\u009b"
66+ got := clean(in)
67+ want := "ok[2J]0;pwned title\ttab\nline"
68+ if got != want {
69+ t.Errorf("clean(%q) = %q, want %q", in, got, want)
70+ }
71+ if clean("plain ünïcödé → text") != "plain ünïcödé → text" {
72+ t.Error("clean must leave ordinary text alone")
73+ }
74+ var b strings.Builder
75+ tw := newTabw(&b)
76+ if _, err := tw.Write([]byte("a\x1b[31mred\tb\n")); err != nil {
77+ t.Fatal(err)
78+ }
79+ if err := tw.Flush(); err != nil {
80+ t.Fatal(err)
81+ }
82+ if strings.Contains(b.String(), "\x1b") {
83+ t.Errorf("table output still carries ESC: %q", b.String())
84+ }
85+}
86+
58 // pr create must say "draft": false out loud — the API's default is a draft,87 // pr create must say "draft": false out loud — the API's default is a draft,
59 // which `pr merge` then refuses (issue #2) — and --draft flips it.88 // which `pr merge` then refuses (issue #2) — and --draft flips it.
60 func TestPRCreateSendsDraftExplicitly(t *testing.T) {89 func TestPRCreateSendsDraftExplicitly(t *testing.T) {
modified cmd/repo.go +23 -8
@@ -219,7 +219,9 @@ func runRepoList(cmd *cobra.Command, _ []string) error {
219219 for _, r := range page.Items {
220220 fmt.Fprintf(tw, "%s\t%s\t%s\n", r.FullName, r.Visibility, dash(r.Description))
221221 }
222- tw.Flush()
222+ if err := tw.Flush(); err != nil {
223+ return err
224+ }
223225 printPageFooter(cmd, page.Page)
224226 return nil
225227 }
@@ -324,6 +326,9 @@ func runRepoClone(cmd *cobra.Command, args []string) error {
324326 gitArgs := []string{"clone", cloneURL}
325327 gitArgs = append(gitArgs, args[1:]...)
326328 fmt.Fprintf(cmd.OutOrStdout(), "Cloning %s…\n", cloneURL)
329+ // #nosec G204 -- the program is a constant ("git"); the URL is assembled here
330+ // from a fixed scheme/host and the owner/repo the user named, and anything
331+ // after "--" is the user's own git options, passed through by design.
327332 g := exec.Command("git", gitArgs...)
328333 g.Stdout = cmd.OutOrStdout()
329334 g.Stderr = cmd.ErrOrStderr()
@@ -335,7 +340,7 @@ func runRepoClone(cmd *cobra.Command, args []string) error {
335340 // ps) and never written into the clone's .git/config.
336341 if token := config.ResolveToken(flagToken, cfg, host); token != "" {
337342 config.WarnIfInsecure(cmd.ErrOrStderr(), cloneURL)
338- g.Env = append(g.Env, gitAuthEnv(token)...)
343+ g.Env = append(g.Env, gitAuthEnv(token, cloneURL)...)
339344 }
340345 }
341346 return g.Run()
@@ -366,7 +371,14 @@ func cloneURLFor(host, owner, repo, protocol string) (string, error) {
366371
367372 // gitAuthEnv returns the GIT_CONFIG_* variables that make git send the token as
368373 // a bearer header, appended after any GIT_CONFIG_COUNT entries already present.
369-func gitAuthEnv(token string) []string {
374+// The header is scoped to the clone URL's origin with git's per-URL config
375+// (http.<origin>/.extraHeader), so it is never sent to another host: not to a
376+// redirect target, and not to a submodule hosted elsewhere.
377+func gitAuthEnv(token, cloneURL string) []string {
378+ origin := cloneURL
379+ if u, err := url.Parse(cloneURL); err == nil && u.Scheme != "" && u.Host != "" {
380+ origin = u.Scheme + "://" + u.Host + "/"
381+ }
370382 n := 0
371383 if v := os.Getenv("GIT_CONFIG_COUNT"); v != "" {
372384 if parsed, err := strconv.Atoi(v); err == nil && parsed > 0 {
@@ -375,7 +387,7 @@ func gitAuthEnv(token string) []string {
375387 }
376388 return []string{
377389 fmt.Sprintf("GIT_CONFIG_COUNT=%d", n+1),
378- fmt.Sprintf("GIT_CONFIG_KEY_%d=http.extraHeader", n),
390+ fmt.Sprintf("GIT_CONFIG_KEY_%d=http.%s.extraHeader", n, origin),
379391 fmt.Sprintf("GIT_CONFIG_VALUE_%d=Authorization: Bearer %s", n, token),
380392 }
381393 }
@@ -426,8 +438,7 @@ func runRepoFiles(cmd *cobra.Command, args []string) error {
426438 }
427439 fmt.Fprintf(tw, "%s\t%s\t%s\n", kind, e.Name, size)
428440 }
429- tw.Flush()
430- return nil
441+ return tw.Flush()
431442 }
432443
433444 func runRepoCat(cmd *cobra.Command, args []string) error {
@@ -510,7 +521,9 @@ func runRepoCommits(cmd *cobra.Command, args []string) error {
510521 for _, c := range page.Items {
511522 fmt.Fprintf(tw, "%s\t%s\t%s\t%s\n", c.Short, dash(c.Author), dash(c.Date), c.Subject)
512523 }
513- tw.Flush()
524+ if err := tw.Flush(); err != nil {
525+ return err
526+ }
514527 printPageFooter(cmd, page.Page)
515528 return nil
516529 }
@@ -540,7 +553,9 @@ func runRepoCompare(cmd *cobra.Command, args []string) error {
540553 for _, c := range cmp.Commits {
541554 fmt.Fprintf(tw, "%s\t%s\t%s\n", c.Short, dash(c.Author), c.Subject)
542555 }
543- tw.Flush()
556+ if err := tw.Flush(); err != nil {
557+ return err
558+ }
544559 }
545560 if len(cmp.Files) > 0 {
546561 fmt.Fprintf(out, "\n%d file(s) changed:\n", len(cmp.Files))
@@ -219,7 +219,9 @@ func runRepoList(cmd *cobra.Command, _ []string) error {
219 for _, r := range page.Items {219 for _, r := range page.Items {
220 fmt.Fprintf(tw, "%s\t%s\t%s\n", r.FullName, r.Visibility, dash(r.Description))220 fmt.Fprintf(tw, "%s\t%s\t%s\n", r.FullName, r.Visibility, dash(r.Description))
221 }221 }
222- tw.Flush()222+ if err := tw.Flush(); err != nil {
223+ return err
224+ }
223 printPageFooter(cmd, page.Page)225 printPageFooter(cmd, page.Page)
224 return nil226 return nil
225 }227 }
@@ -324,6 +326,9 @@ func runRepoClone(cmd *cobra.Command, args []string) error {
324 gitArgs := []string{"clone", cloneURL}326 gitArgs := []string{"clone", cloneURL}
325 gitArgs = append(gitArgs, args[1:]...)327 gitArgs = append(gitArgs, args[1:]...)
326 fmt.Fprintf(cmd.OutOrStdout(), "Cloning %s…\n", cloneURL)328 fmt.Fprintf(cmd.OutOrStdout(), "Cloning %s…\n", cloneURL)
329+ // #nosec G204 -- the program is a constant ("git"); the URL is assembled here
330+ // from a fixed scheme/host and the owner/repo the user named, and anything
331+ // after "--" is the user's own git options, passed through by design.
327 g := exec.Command("git", gitArgs...)332 g := exec.Command("git", gitArgs...)
328 g.Stdout = cmd.OutOrStdout()333 g.Stdout = cmd.OutOrStdout()
329 g.Stderr = cmd.ErrOrStderr()334 g.Stderr = cmd.ErrOrStderr()
@@ -335,7 +340,7 @@ func runRepoClone(cmd *cobra.Command, args []string) error {
335 // ps) and never written into the clone's .git/config.340 // ps) and never written into the clone's .git/config.
336 if token := config.ResolveToken(flagToken, cfg, host); token != "" {341 if token := config.ResolveToken(flagToken, cfg, host); token != "" {
337 config.WarnIfInsecure(cmd.ErrOrStderr(), cloneURL)342 config.WarnIfInsecure(cmd.ErrOrStderr(), cloneURL)
338- g.Env = append(g.Env, gitAuthEnv(token)...)343+ g.Env = append(g.Env, gitAuthEnv(token, cloneURL)...)
339 }344 }
340 }345 }
341 return g.Run()346 return g.Run()
@@ -366,7 +371,14 @@ func cloneURLFor(host, owner, repo, protocol string) (string, error) {
366 371
367 // gitAuthEnv returns the GIT_CONFIG_* variables that make git send the token as372 // gitAuthEnv returns the GIT_CONFIG_* variables that make git send the token as
368 // a bearer header, appended after any GIT_CONFIG_COUNT entries already present.373 // a bearer header, appended after any GIT_CONFIG_COUNT entries already present.
369-func gitAuthEnv(token string) []string {374+// The header is scoped to the clone URL's origin with git's per-URL config
375+// (http.<origin>/.extraHeader), so it is never sent to another host: not to a
376+// redirect target, and not to a submodule hosted elsewhere.
377+func gitAuthEnv(token, cloneURL string) []string {
378+ origin := cloneURL
379+ if u, err := url.Parse(cloneURL); err == nil && u.Scheme != "" && u.Host != "" {
380+ origin = u.Scheme + "://" + u.Host + "/"
381+ }
370 n := 0382 n := 0
371 if v := os.Getenv("GIT_CONFIG_COUNT"); v != "" {383 if v := os.Getenv("GIT_CONFIG_COUNT"); v != "" {
372 if parsed, err := strconv.Atoi(v); err == nil && parsed > 0 {384 if parsed, err := strconv.Atoi(v); err == nil && parsed > 0 {
@@ -375,7 +387,7 @@ func gitAuthEnv(token string) []string {
375 }387 }
376 return []string{388 return []string{
377 fmt.Sprintf("GIT_CONFIG_COUNT=%d", n+1),389 fmt.Sprintf("GIT_CONFIG_COUNT=%d", n+1),
378- fmt.Sprintf("GIT_CONFIG_KEY_%d=http.extraHeader", n),390+ fmt.Sprintf("GIT_CONFIG_KEY_%d=http.%s.extraHeader", n, origin),
379 fmt.Sprintf("GIT_CONFIG_VALUE_%d=Authorization: Bearer %s", n, token),391 fmt.Sprintf("GIT_CONFIG_VALUE_%d=Authorization: Bearer %s", n, token),
380 }392 }
381 }393 }
@@ -426,8 +438,7 @@ func runRepoFiles(cmd *cobra.Command, args []string) error {
426 }438 }
427 fmt.Fprintf(tw, "%s\t%s\t%s\n", kind, e.Name, size)439 fmt.Fprintf(tw, "%s\t%s\t%s\n", kind, e.Name, size)
428 }440 }
429- tw.Flush()441+ return tw.Flush()
430- return nil
431 }442 }
432 443
433 func runRepoCat(cmd *cobra.Command, args []string) error {444 func runRepoCat(cmd *cobra.Command, args []string) error {
@@ -510,7 +521,9 @@ func runRepoCommits(cmd *cobra.Command, args []string) error {
510 for _, c := range page.Items {521 for _, c := range page.Items {
511 fmt.Fprintf(tw, "%s\t%s\t%s\t%s\n", c.Short, dash(c.Author), dash(c.Date), c.Subject)522 fmt.Fprintf(tw, "%s\t%s\t%s\t%s\n", c.Short, dash(c.Author), dash(c.Date), c.Subject)
512 }523 }
513- tw.Flush()524+ if err := tw.Flush(); err != nil {
525+ return err
526+ }
514 printPageFooter(cmd, page.Page)527 printPageFooter(cmd, page.Page)
515 return nil528 return nil
516 }529 }
@@ -540,7 +553,9 @@ func runRepoCompare(cmd *cobra.Command, args []string) error {
540 for _, c := range cmp.Commits {553 for _, c := range cmp.Commits {
541 fmt.Fprintf(tw, "%s\t%s\t%s\n", c.Short, dash(c.Author), c.Subject)554 fmt.Fprintf(tw, "%s\t%s\t%s\n", c.Short, dash(c.Author), c.Subject)
542 }555 }
543- tw.Flush()556+ if err := tw.Flush(); err != nil {
557+ return err
558+ }
544 }559 }
545 if len(cmp.Files) > 0 {560 if len(cmp.Files) > 0 {
546 fmt.Fprintf(out, "\n%d file(s) changed:\n", len(cmp.Files))561 fmt.Fprintf(out, "\n%d file(s) changed:\n", len(cmp.Files))
modified cmd/root.go +49 -6
@@ -2,11 +2,11 @@
22 package cmd
33
44 import (
5- "context"
65 "encoding/json"
76 "fmt"
87 "io"
98 "os"
9+ "strings"
1010 "text/tabwriter"
1111 "time"
1212
@@ -79,9 +79,6 @@ func newClient() (*api.Client, error) {
7979 return api.New(host, token), nil
8080 }
8181
82-// ctx returns a background context (a place to add cancellation later).
83-func ctx() context.Context { return context.Background() }
84-
8582 // ---- output helpers ----
8683
8784 // printJSON marshals v as indented JSON to the given writer.
@@ -93,9 +90,55 @@ func printJSON(w io.Writer, v any) error {
9390 }
9491
9592 // newTabw returns a tabwriter over the given writer configured for the CLI's
96-// two-space-padded column style.
93+// two-space-padded column style. Table cells are server-provided text (titles,
94+// names, descriptions), so the output passes through cleanWriter: a terminal
95+// escape sequence smuggled into an issue title cannot repaint the screen,
96+// change the window title, or hide a row.
9797 func newTabw(w io.Writer) *tabwriter.Writer {
98- return tabwriter.NewWriter(w, 0, 4, 2, ' ', 0)
98+ return tabwriter.NewWriter(cleanWriter{w}, 0, 4, 2, ' ', 0)
99+}
100+
101+// clean strips terminal control characters from text that came from the
102+// server before it is printed for a human: every C0 control except tab,
103+// newline and carriage return, DEL, and the C1 range (U+0080–U+009F, which
104+// some terminals honour as 8-bit escape introducers). JSON output is never
105+// touched — it goes through printJSON untransformed.
106+func clean(s string) string {
107+ if !needsClean(s) {
108+ return s
109+ }
110+ var b strings.Builder
111+ b.Grow(len(s))
112+ for _, r := range s {
113+ switch {
114+ case r == '\t' || r == '\n' || r == '\r':
115+ b.WriteRune(r)
116+ case r < 0x20 || r == 0x7f || (r >= 0x80 && r <= 0x9f):
117+ // dropped
118+ default:
119+ b.WriteRune(r)
120+ }
121+ }
122+ return b.String()
123+}
124+
125+func needsClean(s string) bool {
126+ for _, r := range s {
127+ if (r < 0x20 && r != '\t' && r != '\n' && r != '\r') || r == 0x7f || (r >= 0x80 && r <= 0x9f) {
128+ return true
129+ }
130+ }
131+ return false
132+}
133+
134+// cleanWriter applies clean to everything written through it.
135+type cleanWriter struct{ w io.Writer }
136+
137+func (c cleanWriter) Write(p []byte) (int, error) {
138+ if _, err := c.w.Write([]byte(clean(string(p)))); err != nil {
139+ return 0, err
140+ }
141+ return len(p), nil
99142 }
100143
101144 // printPageFooter prints a pagination hint when more pages are available.
@@ -2,11 +2,11 @@
2 package cmd2 package cmd
3 3
4 import (4 import (
5- "context"
6 "encoding/json"5 "encoding/json"
7 "fmt"6 "fmt"
8 "io"7 "io"
9 "os"8 "os"
9+ "strings"
10 "text/tabwriter"10 "text/tabwriter"
11 "time"11 "time"
12 12
@@ -79,9 +79,6 @@ func newClient() (*api.Client, error) {
79 return api.New(host, token), nil79 return api.New(host, token), nil
80 }80 }
81 81
82-// ctx returns a background context (a place to add cancellation later).
83-func ctx() context.Context { return context.Background() }
84-
85 // ---- output helpers ----82 // ---- output helpers ----
86 83
87 // printJSON marshals v as indented JSON to the given writer.84 // printJSON marshals v as indented JSON to the given writer.
@@ -93,9 +90,55 @@ func printJSON(w io.Writer, v any) error {
93 }90 }
94 91
95 // newTabw returns a tabwriter over the given writer configured for the CLI's92 // newTabw returns a tabwriter over the given writer configured for the CLI's
96-// two-space-padded column style.93+// two-space-padded column style. Table cells are server-provided text (titles,
94+// names, descriptions), so the output passes through cleanWriter: a terminal
95+// escape sequence smuggled into an issue title cannot repaint the screen,
96+// change the window title, or hide a row.
97 func newTabw(w io.Writer) *tabwriter.Writer {97 func newTabw(w io.Writer) *tabwriter.Writer {
98- return tabwriter.NewWriter(w, 0, 4, 2, ' ', 0)98+ return tabwriter.NewWriter(cleanWriter{w}, 0, 4, 2, ' ', 0)
99+}
100+
101+// clean strips terminal control characters from text that came from the
102+// server before it is printed for a human: every C0 control except tab,
103+// newline and carriage return, DEL, and the C1 range (U+0080–U+009F, which
104+// some terminals honour as 8-bit escape introducers). JSON output is never
105+// touched — it goes through printJSON untransformed.
106+func clean(s string) string {
107+ if !needsClean(s) {
108+ return s
109+ }
110+ var b strings.Builder
111+ b.Grow(len(s))
112+ for _, r := range s {
113+ switch {
114+ case r == '\t' || r == '\n' || r == '\r':
115+ b.WriteRune(r)
116+ case r < 0x20 || r == 0x7f || (r >= 0x80 && r <= 0x9f):
117+ // dropped
118+ default:
119+ b.WriteRune(r)
120+ }
121+ }
122+ return b.String()
123+}
124+
125+func needsClean(s string) bool {
126+ for _, r := range s {
127+ if (r < 0x20 && r != '\t' && r != '\n' && r != '\r') || r == 0x7f || (r >= 0x80 && r <= 0x9f) {
128+ return true
129+ }
130+ }
131+ return false
132+}
133+
134+// cleanWriter applies clean to everything written through it.
135+type cleanWriter struct{ w io.Writer }
136+
137+func (c cleanWriter) Write(p []byte) (int, error) {
138+ if _, err := c.w.Write([]byte(clean(string(p)))); err != nil {
139+ return 0, err
140+ }
141+ return len(p), nil
99 }142 }
100 143
101 // printPageFooter prints a pagination hint when more pages are available.144 // printPageFooter prints a pagination hint when more pages are available.
modified cmd/run.go +4 -2
@@ -117,7 +117,9 @@ func runRunList(cmd *cobra.Command, _ []string) error {
117117 for _, r := range page.Items {
118118 fmt.Fprintf(tw, "%d\t%s\t%s\t%s\t%s\t%s\n", r.Number, dash(r.Workflow), dash(r.Event), dash(r.Branch), r.Status, short(r.HeadSHA))
119119 }
120- tw.Flush()
120+ if err := tw.Flush(); err != nil {
121+ return err
122+ }
121123 printPageFooter(cmd, page.Page)
122124 return nil
123125 }
@@ -150,7 +152,7 @@ func runRunView(cmd *cobra.Command, args []string) error {
150152 return printJSON(cmd.OutOrStdout(), r)
151153 }
152154 out := cmd.OutOrStdout()
153- fmt.Fprintf(out, "Run #%d %s [%s]\n", r.Number, dash(r.Workflow), r.Status)
155+ fmt.Fprintf(out, "Run #%d %s [%s]\n", r.Number, dash(clean(r.Workflow)), r.Status)
154156 fmt.Fprintf(out, "event: %s branch: %s commit: %s\n", dash(r.Event), dash(r.Branch), short(r.HeadSHA))
155157 if r.SecretsWithheld {
156158 fmt.Fprintln(out, "note: secrets were withheld from this run")
@@ -117,7 +117,9 @@ func runRunList(cmd *cobra.Command, _ []string) error {
117 for _, r := range page.Items {117 for _, r := range page.Items {
118 fmt.Fprintf(tw, "%d\t%s\t%s\t%s\t%s\t%s\n", r.Number, dash(r.Workflow), dash(r.Event), dash(r.Branch), r.Status, short(r.HeadSHA))118 fmt.Fprintf(tw, "%d\t%s\t%s\t%s\t%s\t%s\n", r.Number, dash(r.Workflow), dash(r.Event), dash(r.Branch), r.Status, short(r.HeadSHA))
119 }119 }
120- tw.Flush()120+ if err := tw.Flush(); err != nil {
121+ return err
122+ }
121 printPageFooter(cmd, page.Page)123 printPageFooter(cmd, page.Page)
122 return nil124 return nil
123 }125 }
@@ -150,7 +152,7 @@ func runRunView(cmd *cobra.Command, args []string) error {
150 return printJSON(cmd.OutOrStdout(), r)152 return printJSON(cmd.OutOrStdout(), r)
151 }153 }
152 out := cmd.OutOrStdout()154 out := cmd.OutOrStdout()
153- fmt.Fprintf(out, "Run #%d %s [%s]\n", r.Number, dash(r.Workflow), r.Status)155+ fmt.Fprintf(out, "Run #%d %s [%s]\n", r.Number, dash(clean(r.Workflow)), r.Status)
154 fmt.Fprintf(out, "event: %s branch: %s commit: %s\n", dash(r.Event), dash(r.Branch), short(r.HeadSHA))156 fmt.Fprintf(out, "event: %s branch: %s commit: %s\n", dash(r.Event), dash(r.Branch), short(r.HeadSHA))
155 if r.SecretsWithheld {157 if r.SecretsWithheld {
156 fmt.Fprintln(out, "note: secrets were withheld from this run")158 fmt.Fprintln(out, "note: secrets were withheld from this run")
modified cmd/search.go +3 -1
@@ -47,7 +47,9 @@ func runSearchRepos(cmd *cobra.Command, args []string) error {
4747 for _, r := range page.Items {
4848 fmt.Fprintf(tw, "%s\t%s\t%s\n", r.FullName, r.Visibility, dash(r.Description))
4949 }
50- tw.Flush()
50+ if err := tw.Flush(); err != nil {
51+ return err
52+ }
5153 printPageFooter(cmd, page.Page)
5254 return nil
5355 }
@@ -47,7 +47,9 @@ func runSearchRepos(cmd *cobra.Command, args []string) error {
47 for _, r := range page.Items {47 for _, r := range page.Items {
48 fmt.Fprintf(tw, "%s\t%s\t%s\n", r.FullName, r.Visibility, dash(r.Description))48 fmt.Fprintf(tw, "%s\t%s\t%s\n", r.FullName, r.Visibility, dash(r.Description))
49 }49 }
50- tw.Flush()50+ if err := tw.Flush(); err != nil {
51+ return err
52+ }
51 printPageFooter(cmd, page.Page)53 printPageFooter(cmd, page.Page)
52 return nil54 return nil
53 }55 }
modified go.mod +2 -0
@@ -2,6 +2,8 @@ module rickub.com/rickub/cli
22
33 go 1.26
44
5+toolchain go1.26.8
6+
57 require (
68 github.com/spf13/cobra v1.10.2
79 gopkg.in/yaml.v3 v3.0.1
@@ -2,6 +2,8 @@ module rickub.com/rickub/cli
2 2
3 go 1.263 go 1.26
4 4
5+toolchain go1.26.8
6+
5 require (7 require (
6 github.com/spf13/cobra v1.10.28 github.com/spf13/cobra v1.10.2
7 gopkg.in/yaml.v3 v3.0.19 gopkg.in/yaml.v3 v3.0.1
modified internal/api/client.go +37 -5
@@ -26,16 +26,44 @@ type Client struct {
2626 UserAgent string
2727 }
2828
29+// MaxResponseBytes caps how much of a response body the client will read. A
30+// misbehaving or hostile host must not be able to make the CLI buffer an
31+// unbounded body in memory; the largest legitimate responses are CI logs.
32+var MaxResponseBytes int64 = 128 << 20
33+
2934 // New builds a Client with a sane default HTTP client.
3035 func New(host, token string) *Client {
3136 return &Client{
32- Host: strings.TrimRight(host, "/"),
33- Token: token,
34- HTTPClient: &http.Client{Timeout: 30 * time.Second},
35- UserAgent: "rickub-cli",
37+ Host: strings.TrimRight(host, "/"),
38+ Token: token,
39+ HTTPClient: &http.Client{
40+ Timeout: 30 * time.Second,
41+ CheckRedirect: refuseUnsafeRedirect,
42+ },
43+ UserAgent: "rickub-cli",
3644 }
3745 }
3846
47+// refuseUnsafeRedirect is the redirect policy: a redirect may stay on the same
48+// host and may never downgrade https to http. The bearer token rides on every
49+// request, so following a redirect elsewhere would hand it to another party
50+// (Go already drops Authorization across domains, but a same-domain http
51+// downgrade or a subdomain hop would still carry it), and a CLI has no
52+// business being bounced to a different origin anyway.
53+func refuseUnsafeRedirect(req *http.Request, via []*http.Request) error {
54+ if len(via) >= 10 {
55+ return fmt.Errorf("stopped after 10 redirects")
56+ }
57+ first := via[0].URL
58+ if !strings.EqualFold(req.URL.Host, first.Host) {
59+ return fmt.Errorf("refusing redirect from %s to another host (%s)", first.Host, req.URL.Host)
60+ }
61+ if strings.EqualFold(first.Scheme, "https") && !strings.EqualFold(req.URL.Scheme, "https") {
62+ return fmt.Errorf("refusing redirect from https to %s", req.URL.Scheme)
63+ }
64+ return nil
65+}
66+
3967 // APIError is the parsed `{error:{code,message}}` envelope plus HTTP status.
4068 type APIError struct {
4169 Status int
@@ -113,10 +141,14 @@ func (c *Client) raw(ctx context.Context, r Request) (int, []byte, string, error
113141 }
114142 defer resp.Body.Close()
115143
116- data, err := io.ReadAll(resp.Body)
144+ data, err := io.ReadAll(io.LimitReader(resp.Body, MaxResponseBytes+1))
117145 if err != nil {
118146 return resp.StatusCode, nil, resp.Header.Get("Content-Type"), err
119147 }
148+ if int64(len(data)) > MaxResponseBytes {
149+ return resp.StatusCode, nil, resp.Header.Get("Content-Type"),
150+ fmt.Errorf("response from %s exceeds %d bytes; refusing to buffer it", c.Host, MaxResponseBytes)
151+ }
120152
121153 if resp.StatusCode >= 200 && resp.StatusCode < 300 {
122154 return resp.StatusCode, data, resp.Header.Get("Content-Type"), nil
@@ -26,16 +26,44 @@ type Client struct {
26 UserAgent string26 UserAgent string
27 }27 }
28 28
29+// MaxResponseBytes caps how much of a response body the client will read. A
30+// misbehaving or hostile host must not be able to make the CLI buffer an
31+// unbounded body in memory; the largest legitimate responses are CI logs.
32+var MaxResponseBytes int64 = 128 << 20
33+
29 // New builds a Client with a sane default HTTP client.34 // New builds a Client with a sane default HTTP client.
30 func New(host, token string) *Client {35 func New(host, token string) *Client {
31 return &Client{36 return &Client{
32- Host: strings.TrimRight(host, "/"),37+ Host: strings.TrimRight(host, "/"),
33- Token: token,38+ Token: token,
34- HTTPClient: &http.Client{Timeout: 30 * time.Second},39+ HTTPClient: &http.Client{
35- UserAgent: "rickub-cli",40+ Timeout: 30 * time.Second,
41+ CheckRedirect: refuseUnsafeRedirect,
42+ },
43+ UserAgent: "rickub-cli",
36 }44 }
37 }45 }
38 46
47+// refuseUnsafeRedirect is the redirect policy: a redirect may stay on the same
48+// host and may never downgrade https to http. The bearer token rides on every
49+// request, so following a redirect elsewhere would hand it to another party
50+// (Go already drops Authorization across domains, but a same-domain http
51+// downgrade or a subdomain hop would still carry it), and a CLI has no
52+// business being bounced to a different origin anyway.
53+func refuseUnsafeRedirect(req *http.Request, via []*http.Request) error {
54+ if len(via) >= 10 {
55+ return fmt.Errorf("stopped after 10 redirects")
56+ }
57+ first := via[0].URL
58+ if !strings.EqualFold(req.URL.Host, first.Host) {
59+ return fmt.Errorf("refusing redirect from %s to another host (%s)", first.Host, req.URL.Host)
60+ }
61+ if strings.EqualFold(first.Scheme, "https") && !strings.EqualFold(req.URL.Scheme, "https") {
62+ return fmt.Errorf("refusing redirect from https to %s", req.URL.Scheme)
63+ }
64+ return nil
65+}
66+
39 // APIError is the parsed `{error:{code,message}}` envelope plus HTTP status.67 // APIError is the parsed `{error:{code,message}}` envelope plus HTTP status.
40 type APIError struct {68 type APIError struct {
41 Status int69 Status int
@@ -113,10 +141,14 @@ func (c *Client) raw(ctx context.Context, r Request) (int, []byte, string, error
113 }141 }
114 defer resp.Body.Close()142 defer resp.Body.Close()
115 143
116- data, err := io.ReadAll(resp.Body)144+ data, err := io.ReadAll(io.LimitReader(resp.Body, MaxResponseBytes+1))
117 if err != nil {145 if err != nil {
118 return resp.StatusCode, nil, resp.Header.Get("Content-Type"), err146 return resp.StatusCode, nil, resp.Header.Get("Content-Type"), err
119 }147 }
148+ if int64(len(data)) > MaxResponseBytes {
149+ return resp.StatusCode, nil, resp.Header.Get("Content-Type"),
150+ fmt.Errorf("response from %s exceeds %d bytes; refusing to buffer it", c.Host, MaxResponseBytes)
151+ }
120 152
121 if resp.StatusCode >= 200 && resp.StatusCode < 300 {153 if resp.StatusCode >= 200 && resp.StatusCode < 300 {
122 return resp.StatusCode, data, resp.Header.Get("Content-Type"), nil154 return resp.StatusCode, data, resp.Header.Get("Content-Type"), nil
modified internal/api/client_test.go +67 -0
@@ -1,11 +1,13 @@
11 package api
22
33 import (
4+ "bytes"
45 "context"
56 "encoding/json"
67 "errors"
78 "net/http"
89 "net/http/httptest"
10+ "strings"
911 "testing"
1012 )
1113
@@ -219,3 +221,68 @@ func TestContentsPathEscaping(t *testing.T) {
219221 t.Errorf("path = %q, want %q", gotPath, want)
220222 }
221223 }
224+
225+// The bearer token rides on every request, so a redirect may not carry it to
226+// another host, and an https origin may not be downgraded to http.
227+func TestRedirectsStayOnHostAndScheme(t *testing.T) {
228+ other := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
229+ t.Errorf("the other host received %s %s with Authorization=%q", r.Method, r.URL.Path, r.Header.Get("Authorization"))
230+ }))
231+ defer other.Close()
232+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
233+ switch r.URL.Path {
234+ case "/api/v1/elsewhere":
235+ http.Redirect(w, r, other.URL+"/api/v1/user", http.StatusFound)
236+ case "/api/v1/downgrade":
237+ http.Redirect(w, r, "http://"+r.Host+"/api/v1/user", http.StatusFound)
238+ case "/api/v1/same":
239+ http.Redirect(w, r, "/api/v1/user", http.StatusFound)
240+ case "/api/v1/user":
241+ _, _ = w.Write([]byte(`{"handle":"rick"}`))
242+ default:
243+ w.WriteHeader(http.StatusNotFound)
244+ }
245+ }))
246+ defer srv.Close()
247+ c := New(srv.URL, "rickub_pat_x")
248+
249+ if _, _, err := c.RawJSON(context.Background(), "GET", "/elsewhere", nil, nil); err == nil || !strings.Contains(err.Error(), "another host") {
250+ t.Errorf("cross-host redirect must be refused, got %v", err)
251+ }
252+ if _, _, err := c.RawJSON(context.Background(), "GET", "/same", nil, nil); err != nil {
253+ t.Errorf("same-host redirect should be followed: %v", err)
254+ }
255+ // httptest is plain http, so exercise the downgrade rule directly.
256+ first, _ := http.NewRequest("GET", "https://rickub.com/api/v1/x", nil)
257+ next, _ := http.NewRequest("GET", "http://rickub.com/api/v1/x", nil)
258+ if err := refuseUnsafeRedirect(next, []*http.Request{first}); err == nil {
259+ t.Error("https→http redirect must be refused")
260+ }
261+ sub, _ := http.NewRequest("GET", "https://api.rickub.com/api/v1/x", nil)
262+ if err := refuseUnsafeRedirect(sub, []*http.Request{first}); err == nil {
263+ t.Error("redirect to a subdomain must be refused (Go would forward Authorization there)")
264+ }
265+ same, _ := http.NewRequest("GET", "https://rickub.com/api/v1/y", nil)
266+ if err := refuseUnsafeRedirect(same, []*http.Request{first}); err != nil {
267+ t.Errorf("same-origin redirect should pass: %v", err)
268+ }
269+}
270+
271+// A hostile or broken host cannot make the CLI buffer an unbounded body.
272+func TestResponseBodyIsCapped(t *testing.T) {
273+ old := MaxResponseBytes
274+ MaxResponseBytes = 1024
275+ t.Cleanup(func() { MaxResponseBytes = old })
276+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
277+ _, _ = w.Write(bytes.Repeat([]byte("x"), 4096))
278+ }))
279+ defer srv.Close()
280+ c := New(srv.URL, "rickub_pat_x")
281+ if _, _, err := c.RawJSON(context.Background(), "GET", "/big", nil, nil); err == nil || !strings.Contains(err.Error(), "exceeds") {
282+ t.Errorf("oversized body must be refused, got %v", err)
283+ }
284+ MaxResponseBytes = 8192
285+ if _, _, err := c.RawJSON(context.Background(), "GET", "/big", nil, nil); err != nil {
286+ t.Errorf("body under the cap should be read: %v", err)
287+ }
288+}
@@ -1,11 +1,13 @@
1 package api1 package api
2 2
3 import (3 import (
4+ "bytes"
4 "context"5 "context"
5 "encoding/json"6 "encoding/json"
6 "errors"7 "errors"
7 "net/http"8 "net/http"
8 "net/http/httptest"9 "net/http/httptest"
10+ "strings"
9 "testing"11 "testing"
10 )12 )
11 13
@@ -219,3 +221,68 @@ func TestContentsPathEscaping(t *testing.T) {
219 t.Errorf("path = %q, want %q", gotPath, want)221 t.Errorf("path = %q, want %q", gotPath, want)
220 }222 }
221 }223 }
224+
225+// The bearer token rides on every request, so a redirect may not carry it to
226+// another host, and an https origin may not be downgraded to http.
227+func TestRedirectsStayOnHostAndScheme(t *testing.T) {
228+ other := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
229+ t.Errorf("the other host received %s %s with Authorization=%q", r.Method, r.URL.Path, r.Header.Get("Authorization"))
230+ }))
231+ defer other.Close()
232+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
233+ switch r.URL.Path {
234+ case "/api/v1/elsewhere":
235+ http.Redirect(w, r, other.URL+"/api/v1/user", http.StatusFound)
236+ case "/api/v1/downgrade":
237+ http.Redirect(w, r, "http://"+r.Host+"/api/v1/user", http.StatusFound)
238+ case "/api/v1/same":
239+ http.Redirect(w, r, "/api/v1/user", http.StatusFound)
240+ case "/api/v1/user":
241+ _, _ = w.Write([]byte(`{"handle":"rick"}`))
242+ default:
243+ w.WriteHeader(http.StatusNotFound)
244+ }
245+ }))
246+ defer srv.Close()
247+ c := New(srv.URL, "rickub_pat_x")
248+
249+ if _, _, err := c.RawJSON(context.Background(), "GET", "/elsewhere", nil, nil); err == nil || !strings.Contains(err.Error(), "another host") {
250+ t.Errorf("cross-host redirect must be refused, got %v", err)
251+ }
252+ if _, _, err := c.RawJSON(context.Background(), "GET", "/same", nil, nil); err != nil {
253+ t.Errorf("same-host redirect should be followed: %v", err)
254+ }
255+ // httptest is plain http, so exercise the downgrade rule directly.
256+ first, _ := http.NewRequest("GET", "https://rickub.com/api/v1/x", nil)
257+ next, _ := http.NewRequest("GET", "http://rickub.com/api/v1/x", nil)
258+ if err := refuseUnsafeRedirect(next, []*http.Request{first}); err == nil {
259+ t.Error("https→http redirect must be refused")
260+ }
261+ sub, _ := http.NewRequest("GET", "https://api.rickub.com/api/v1/x", nil)
262+ if err := refuseUnsafeRedirect(sub, []*http.Request{first}); err == nil {
263+ t.Error("redirect to a subdomain must be refused (Go would forward Authorization there)")
264+ }
265+ same, _ := http.NewRequest("GET", "https://rickub.com/api/v1/y", nil)
266+ if err := refuseUnsafeRedirect(same, []*http.Request{first}); err != nil {
267+ t.Errorf("same-origin redirect should pass: %v", err)
268+ }
269+}
270+
271+// A hostile or broken host cannot make the CLI buffer an unbounded body.
272+func TestResponseBodyIsCapped(t *testing.T) {
273+ old := MaxResponseBytes
274+ MaxResponseBytes = 1024
275+ t.Cleanup(func() { MaxResponseBytes = old })
276+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
277+ _, _ = w.Write(bytes.Repeat([]byte("x"), 4096))
278+ }))
279+ defer srv.Close()
280+ c := New(srv.URL, "rickub_pat_x")
281+ if _, _, err := c.RawJSON(context.Background(), "GET", "/big", nil, nil); err == nil || !strings.Contains(err.Error(), "exceeds") {
282+ t.Errorf("oversized body must be refused, got %v", err)
283+ }
284+ MaxResponseBytes = 8192
285+ if _, _, err := c.RawJSON(context.Background(), "GET", "/big", nil, nil); err != nil {
286+ t.Errorf("body under the cap should be read: %v", err)
287+ }
288+}
modified internal/config/config.go +35 -4
@@ -98,7 +98,7 @@ func Load() (*Config, error) {
9898
9999 // LoadFrom reads config from an explicit path (used by tests).
100100 func LoadFrom(path string) (*Config, error) {
101- data, err := os.ReadFile(path)
101+ data, err := os.ReadFile(path) // #nosec G304 -- path is the CLI's own config file under $XDG_CONFIG_HOME/$HOME, not user-supplied input
102102 if err != nil {
103103 if errors.Is(err, os.ErrNotExist) {
104104 return &Config{}, nil
@@ -147,16 +147,47 @@ func (c *Config) Save() error {
147147 }
148148
149149 // SaveTo writes config to an explicit path (used by tests).
150+//
151+// The write is atomic and the result is always mode 0600: the bytes go to a
152+// fresh 0600 temp file in the same directory, which is then renamed over the
153+// config. A crash mid-write can therefore never leave a truncated file, and a
154+// config that an older tool or a careless chmod left world-readable is fixed
155+// on the next save rather than inherited (os.WriteFile keeps an existing
156+// file's mode).
150157 func (c *Config) SaveTo(path string) error {
151- if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
158+ dir := filepath.Dir(path)
159+ if err := os.MkdirAll(dir, 0o700); err != nil {
152160 return err
153161 }
154162 data, err := yaml.Marshal(c)
155163 if err != nil {
156164 return err
157165 }
158- // Write 0600 so the token is not world-readable.
159- return os.WriteFile(path, data, 0o600)
166+ tmp, err := os.CreateTemp(dir, ".config-*.yaml")
167+ if err != nil {
168+ return err
169+ }
170+ tmpPath := tmp.Name()
171+ cleanup := func() { _ = os.Remove(tmpPath) }
172+ if err := tmp.Chmod(0o600); err != nil {
173+ _ = tmp.Close()
174+ cleanup()
175+ return err
176+ }
177+ if _, err := tmp.Write(data); err != nil {
178+ _ = tmp.Close()
179+ cleanup()
180+ return err
181+ }
182+ if err := tmp.Close(); err != nil {
183+ cleanup()
184+ return err
185+ }
186+ if err := os.Rename(tmpPath, path); err != nil {
187+ cleanup()
188+ return err
189+ }
190+ return nil
160191 }
161192
162193 // TokenFor returns the stored token minted for host, or "" if none is stored
@@ -98,7 +98,7 @@ func Load() (*Config, error) {
98 98
99 // LoadFrom reads config from an explicit path (used by tests).99 // LoadFrom reads config from an explicit path (used by tests).
100 func LoadFrom(path string) (*Config, error) {100 func LoadFrom(path string) (*Config, error) {
101- data, err := os.ReadFile(path)101+ data, err := os.ReadFile(path) // #nosec G304 -- path is the CLI's own config file under $XDG_CONFIG_HOME/$HOME, not user-supplied input
102 if err != nil {102 if err != nil {
103 if errors.Is(err, os.ErrNotExist) {103 if errors.Is(err, os.ErrNotExist) {
104 return &Config{}, nil104 return &Config{}, nil
@@ -147,16 +147,47 @@ func (c *Config) Save() error {
147 }147 }
148 148
149 // SaveTo writes config to an explicit path (used by tests).149 // SaveTo writes config to an explicit path (used by tests).
150+//
151+// The write is atomic and the result is always mode 0600: the bytes go to a
152+// fresh 0600 temp file in the same directory, which is then renamed over the
153+// config. A crash mid-write can therefore never leave a truncated file, and a
154+// config that an older tool or a careless chmod left world-readable is fixed
155+// on the next save rather than inherited (os.WriteFile keeps an existing
156+// file's mode).
150 func (c *Config) SaveTo(path string) error {157 func (c *Config) SaveTo(path string) error {
151- if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {158+ dir := filepath.Dir(path)
159+ if err := os.MkdirAll(dir, 0o700); err != nil {
152 return err160 return err
153 }161 }
154 data, err := yaml.Marshal(c)162 data, err := yaml.Marshal(c)
155 if err != nil {163 if err != nil {
156 return err164 return err
157 }165 }
158- // Write 0600 so the token is not world-readable.166+ tmp, err := os.CreateTemp(dir, ".config-*.yaml")
159- return os.WriteFile(path, data, 0o600)167+ if err != nil {
168+ return err
169+ }
170+ tmpPath := tmp.Name()
171+ cleanup := func() { _ = os.Remove(tmpPath) }
172+ if err := tmp.Chmod(0o600); err != nil {
173+ _ = tmp.Close()
174+ cleanup()
175+ return err
176+ }
177+ if _, err := tmp.Write(data); err != nil {
178+ _ = tmp.Close()
179+ cleanup()
180+ return err
181+ }
182+ if err := tmp.Close(); err != nil {
183+ cleanup()
184+ return err
185+ }
186+ if err := os.Rename(tmpPath, path); err != nil {
187+ cleanup()
188+ return err
189+ }
190+ return nil
160 }191 }
161 192
162 // TokenFor returns the stored token minted for host, or "" if none is stored193 // TokenFor returns the stored token minted for host, or "" if none is stored
modified internal/config/config_test.go +34 -0
@@ -271,3 +271,37 @@ func TestLegacyTokenDefaultsToProductionHostAndNeverOverridesAnEntry(t *testing.
271271 t.Error("stale legacy token was kept")
272272 }
273273 }
274+
275+// Saving is atomic and always leaves the file at 0600, even when an older tool
276+// or a stray chmod left a world-readable config behind.
277+func TestSaveFixesModeAndLeavesNoTempFiles(t *testing.T) {
278+ dir := t.TempDir()
279+ path := filepath.Join(dir, "config.yaml")
280+ if err := os.WriteFile(path, []byte("host: https://rickub.com\n"), 0o644); err != nil {
281+ t.Fatal(err)
282+ }
283+ c := &Config{}
284+ c.SetToken("https://rickub.com", "rickub_pat_abc")
285+ if err := c.SaveTo(path); err != nil {
286+ t.Fatalf("SaveTo: %v", err)
287+ }
288+ info, err := os.Stat(path)
289+ if err != nil {
290+ t.Fatal(err)
291+ }
292+ if perm := info.Mode().Perm(); perm != 0o600 {
293+ t.Errorf("perm = %o after save over a 0644 file, want 600", perm)
294+ }
295+ entries, _ := os.ReadDir(dir)
296+ if len(entries) != 1 {
297+ names := []string{}
298+ for _, e := range entries {
299+ names = append(names, e.Name())
300+ }
301+ t.Errorf("temp file left behind: %v", names)
302+ }
303+ out, err := LoadFrom(path)
304+ if err != nil || out.TokenFor("https://rickub.com") != "rickub_pat_abc" {
305+ t.Errorf("round-trip after atomic save: %v %+v", err, out)
306+ }
307+}
@@ -271,3 +271,37 @@ func TestLegacyTokenDefaultsToProductionHostAndNeverOverridesAnEntry(t *testing.
271 t.Error("stale legacy token was kept")271 t.Error("stale legacy token was kept")
272 }272 }
273 }273 }
274+
275+// Saving is atomic and always leaves the file at 0600, even when an older tool
276+// or a stray chmod left a world-readable config behind.
277+func TestSaveFixesModeAndLeavesNoTempFiles(t *testing.T) {
278+ dir := t.TempDir()
279+ path := filepath.Join(dir, "config.yaml")
280+ if err := os.WriteFile(path, []byte("host: https://rickub.com\n"), 0o644); err != nil {
281+ t.Fatal(err)
282+ }
283+ c := &Config{}
284+ c.SetToken("https://rickub.com", "rickub_pat_abc")
285+ if err := c.SaveTo(path); err != nil {
286+ t.Fatalf("SaveTo: %v", err)
287+ }
288+ info, err := os.Stat(path)
289+ if err != nil {
290+ t.Fatal(err)
291+ }
292+ if perm := info.Mode().Perm(); perm != 0o600 {
293+ t.Errorf("perm = %o after save over a 0644 file, want 600", perm)
294+ }
295+ entries, _ := os.ReadDir(dir)
296+ if len(entries) != 1 {
297+ names := []string{}
298+ for _, e := range entries {
299+ names = append(names, e.Name())
300+ }
301+ t.Errorf("temp file left behind: %v", names)
302+ }
303+ out, err := LoadFrom(path)
304+ if err != nil || out.TokenFor("https://rickub.com") != "rickub_pat_abc" {
305+ t.Errorf("round-trip after atomic save: %v %+v", err, out)
306+ }
307+}