From 08cc25281d1cc24df40320eb5af486388e7fca30 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Wed, 11 Nov 2026 20:07:07 +0530 Subject: [PATCH] Add read-only mode for the library and CLI CI jobs, monitoring and audits often hold a token that can write but should never use it for writing. Add a way to make any backend refuse writes outright. forges.ReadOnlyHTTPClient(hc) returns a copy of hc whose transport only sends GET, HEAD and OPTIONS requests. Anything else fails with the new forges.ErrReadOnly before it is sent, and the request body is closed. Enforcing this at the transport covers every backend and every operation, including ones added later, without a wrapper per service. Each SDK keeps the error chain, so errors.Is(err, forges.ErrReadOnly) holds for GitHub, GitLab, Gitea/Forgejo, Bitbucket and Gerrit; tests go through each backend's exported constructor. The CLI gets a global --read-only flag and FORGE_READ_ONLY, parsed with strconv.ParseBool. The flag wins over the environment, so --read-only=false overrides FORGE_READ_ONLY=true, and an unparseable value is an error. resolve.HTTPClient applies the read-only transport, which also covers forge api. pr create --push is refused up front, since otherwise the branch would be pushed before the PR request is refused. newClientForBaseURL now passes the shared client to forges.Client too. Without that, forges found by probing were built with a nil client and fell back to http.DefaultClient, bypassing read-only mode. resetCmd in the CLI tests now clears slice flags with Replace(nil). Set("[]") had been adding "[]" as an element, which broke running forge api through rootCmd in a test. Closes #21 --- README.md | 25 +++++ bitbucket/readonly_test.go | 43 +++++++++ forge.go | 4 + gerrit/readonly_test.go | 42 ++++++++ gitea/readonly_test.go | 40 ++++++++ github/readonly_test.go | 39 ++++++++ gitlab/readonly_test.go | 42 ++++++++ internal/cli/auth_test.go | 5 + internal/cli/pr.go | 3 + internal/cli/readonly_test.go | 177 ++++++++++++++++++++++++++++++++++ internal/cli/root.go | 16 ++- internal/resolve/resolve.go | 3 +- internal/resolve/transport.go | 24 ++++- readonly.go | 37 +++++++ readonly_test.go | 106 ++++++++++++++++++++ 15 files changed, 602 insertions(+), 4 deletions(-) create mode 100644 bitbucket/readonly_test.go create mode 100644 gerrit/readonly_test.go create mode 100644 gitea/readonly_test.go create mode 100644 github/readonly_test.go create mode 100644 gitlab/readonly_test.go create mode 100644 internal/cli/readonly_test.go create mode 100644 readonly.go create mode 100644 readonly_test.go diff --git a/README.md b/README.md index b9d479d..a8e2051 100644 --- a/README.md +++ b/README.md @@ -148,6 +148,17 @@ Committed `.forge` files cannot set the API scheme. Precedence from highest to lowest: CLI flags, environment variables, `.forge`, `~/.config/forge/config`, built-in defaults. +### Read-only mode + +`--read-only`, or `FORGE_READ_ONLY=true` in the environment, stops forge from changing anything on the forge. Only GET, HEAD and OPTIONS requests are sent, so listing and viewing work as usual, and any command that would create, edit or delete something fails before its request leaves the machine. This covers `forge api` too, and `pr create --push` is refused before anything is pushed. It's meant for CI jobs, monitoring and audits that need a token but should never write with it. + +``` +FORGE_READ_ONLY=true forge issue list +forge --read-only label create bug # Error: create label: Post "...": write refused in read-only mode +``` + +`--read-only=false` overrides `FORGE_READ_ONLY` for a single command. + ## Library ```sh @@ -242,6 +253,20 @@ err := client.RegisterDomain(ctx, "git.example.com", token, forges.ForgeBuilders }) ``` +For read-only access, wrap the HTTP client with `forges.ReadOnlyHTTPClient` and give it to every backend you construct and to `WithHTTPClient`. Requests other than GET, HEAD and OPTIONS then fail with `forges.ErrReadOnly` without being sent, whichever backend or operation made them: + +```go +hc := forges.ReadOnlyHTTPClient(nil) +client := forges.NewClient( + forges.WithHTTPClient(hc), + forges.WithForge("github.com", github.New(token, hc)), +) + +f, _ := client.ForgeFor("github.com") +_, err := f.Issues().Create(ctx, "octocat", "hello-world", forges.CreateIssueOpts{Title: "x"}) +errors.Is(err, forges.ErrReadOnly) // true +``` + PURL support via `github.com/git-pkgs/purl`: ```go diff --git a/bitbucket/readonly_test.go b/bitbucket/readonly_test.go new file mode 100644 index 0000000..541ac5d --- /dev/null +++ b/bitbucket/readonly_test.go @@ -0,0 +1,43 @@ +package bitbucket + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + + forge "github.com/git-pkgs/forge" +) + +func TestBitbucketReadOnlyHTTPClient(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /2.0/user", func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{"username": "alice"}) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + origAPI := bitbucketAPI + setBitbucketAPI(srv.URL + "/2.0") + defer setBitbucketAPI(origAPI) + + f := New("test-token", forge.ReadOnlyHTTPClient(srv.Client())) + ctx := context.Background() + + u, err := f.GetAuthenticatedUser(ctx) + if err != nil { + t.Fatalf("GetAuthenticatedUser: %v", err) + } + assertEqual(t, "Login", "alice", u.Login) + + if _, err := f.Issues().Create(ctx, "alice", "hello", forge.CreateIssueOpts{Title: "x"}); !errors.Is(err, forge.ErrReadOnly) { + t.Errorf("Issues().Create: err = %v, want ErrReadOnly", err) + } + if err := f.Repos().Delete(ctx, "alice", "hello"); !errors.Is(err, forge.ErrReadOnly) { + t.Errorf("Repos().Delete: err = %v, want ErrReadOnly", err) + } +} diff --git a/forge.go b/forge.go index a600b28..efc5d02 100644 --- a/forge.go +++ b/forge.go @@ -76,6 +76,10 @@ func ClassifyStatus(status int, err error) error { return &classifiedError{sentinel: sentinel, err: err} } +// ErrReadOnly is returned when a client created by ReadOnlyHTTPClient refuses +// a request that could modify forge state. +var ErrReadOnly = errors.New("write refused in read-only mode") + // ResourceType identifies the kind of resource a URL points to. type ResourceType string diff --git a/gerrit/readonly_test.go b/gerrit/readonly_test.go new file mode 100644 index 0000000..634ed22 --- /dev/null +++ b/gerrit/readonly_test.go @@ -0,0 +1,42 @@ +package gerrit + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + forge "github.com/git-pkgs/forge" +) + +func TestReadOnlyHTTPClient(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /a/accounts/self", func(w http.ResponseWriter, r *http.Request) { + _, _ = fmt.Fprint(w, xssi+`{"username": "alice"}`) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + f := New(srv.URL, "alice:secret", forge.ReadOnlyHTTPClient(srv.Client())) + ctx := context.Background() + + u, err := f.GetAuthenticatedUser(ctx) + if err != nil { + t.Fatalf("GetAuthenticatedUser: %v", err) + } + if u.Login != "alice" { + t.Errorf("Login = %q, want alice", u.Login) + } + + if err := f.PullRequests().Close(ctx, "org", "repo", 42); !errors.Is(err, forge.ErrReadOnly) { + t.Errorf("PullRequests().Close: err = %v, want ErrReadOnly", err) + } + if err := f.Branches().Delete(ctx, "org", "repo", "topic"); !errors.Is(err, forge.ErrReadOnly) { + t.Errorf("Branches().Delete: err = %v, want ErrReadOnly", err) + } +} diff --git a/gitea/readonly_test.go b/gitea/readonly_test.go new file mode 100644 index 0000000..07dac59 --- /dev/null +++ b/gitea/readonly_test.go @@ -0,0 +1,40 @@ +package gitea + +import ( + "context" + "encoding/json" + "errors" + forge "github.com/git-pkgs/forge" + "net/http" + "net/http/httptest" + "testing" +) + +func TestGiteaReadOnlyHTTPClient(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /api/v1/version", giteaVersionHandler) + mux.HandleFunc("GET /api/v1/user", func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{"login": "alice"}) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + f := New(srv.URL, "test-token", forge.ReadOnlyHTTPClient(srv.Client())) + ctx := context.Background() + + u, err := f.GetAuthenticatedUser(ctx) + if err != nil { + t.Fatalf("GetAuthenticatedUser: %v", err) + } + assertEqual(t, "Login", "alice", u.Login) + + if _, err := f.Issues().Create(ctx, "alice", "hello", forge.CreateIssueOpts{Title: "x"}); !errors.Is(err, forge.ErrReadOnly) { + t.Errorf("Issues().Create: err = %v, want ErrReadOnly", err) + } + if err := f.Repos().Delete(ctx, "alice", "hello"); !errors.Is(err, forge.ErrReadOnly) { + t.Errorf("Repos().Delete: err = %v, want ErrReadOnly", err) + } +} diff --git a/github/readonly_test.go b/github/readonly_test.go new file mode 100644 index 0000000..f70cc64 --- /dev/null +++ b/github/readonly_test.go @@ -0,0 +1,39 @@ +package github + +import ( + "context" + "encoding/json" + "errors" + forge "github.com/git-pkgs/forge" + "net/http" + "net/http/httptest" + "testing" +) + +func TestGitHubReadOnlyHTTPClient(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /api/v3/user", func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{"login": "octocat"}) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + f := NewWithBase(srv.URL, "test-token", forge.ReadOnlyHTTPClient(srv.Client())) + ctx := context.Background() + + u, err := f.GetAuthenticatedUser(ctx) + if err != nil { + t.Fatalf("GetAuthenticatedUser: %v", err) + } + assertEqual(t, "Login", "octocat", u.Login) + + if _, err := f.Issues().Create(ctx, "octocat", "hello", forge.CreateIssueOpts{Title: "x"}); !errors.Is(err, forge.ErrReadOnly) { + t.Errorf("Issues().Create: err = %v, want ErrReadOnly", err) + } + if err := f.Repos().Delete(ctx, "octocat", "hello"); !errors.Is(err, forge.ErrReadOnly) { + t.Errorf("Repos().Delete: err = %v, want ErrReadOnly", err) + } +} diff --git a/gitlab/readonly_test.go b/gitlab/readonly_test.go new file mode 100644 index 0000000..adce174 --- /dev/null +++ b/gitlab/readonly_test.go @@ -0,0 +1,42 @@ +//go:build !tinygo + +package gitlab + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + + forge "github.com/git-pkgs/forge" +) + +func TestGitLabReadOnlyHTTPClient(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("GET /api/v4/user", func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(map[string]any{"username": "jdoe"}) + }) + mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + }) + srv := httptest.NewServer(mux) + defer srv.Close() + + f := New(srv.URL, "test-token", forge.ReadOnlyHTTPClient(srv.Client())) + ctx := context.Background() + + u, err := f.GetAuthenticatedUser(ctx) + if err != nil { + t.Fatalf("GetAuthenticatedUser: %v", err) + } + assertEqual(t, "Login", "jdoe", u.Login) + + if _, err := f.Issues().Create(ctx, "jdoe", "hello", forge.CreateIssueOpts{Title: "x"}); !errors.Is(err, forge.ErrReadOnly) { + t.Errorf("Issues().Create: err = %v, want ErrReadOnly", err) + } + if err := f.Repos().Delete(ctx, "jdoe", "hello"); !errors.Is(err, forge.ErrReadOnly) { + t.Errorf("Repos().Delete: err = %v, want ErrReadOnly", err) + } +} diff --git a/internal/cli/auth_test.go b/internal/cli/auth_test.go index c85b6ca..d564d21 100644 --- a/internal/cli/auth_test.go +++ b/internal/cli/auth_test.go @@ -17,6 +17,11 @@ import ( func resetCmd(cmd *cobra.Command) { reset := func(f *pflag.Flag) { f.Changed = false + // Set on a slice flag would add its "[]" default as an element. + if v, ok := f.Value.(pflag.SliceValue); ok { + _ = v.Replace(nil) + return + } _ = f.Value.Set(f.DefValue) } cmd.Flags().VisitAll(reset) diff --git a/internal/cli/pr.go b/internal/cli/pr.go index 53e47e4..3eeb005 100644 --- a/internal/cli/pr.go +++ b/internal/cli/pr.go @@ -291,6 +291,9 @@ func prCreateCmd() *cobra.Command { Aliases: []string{"new"}, Short: "Create a pull request", RunE: func(cmd *cobra.Command, args []string) error { + if flagPush && resolve.ReadOnly() { + return fmt.Errorf("--push cannot be used in read-only mode") + } if flagTitle == "" && !flagFill { return fmt.Errorf("--title is required") } diff --git a/internal/cli/readonly_test.go b/internal/cli/readonly_test.go new file mode 100644 index 0000000..7775c1f --- /dev/null +++ b/internal/cli/readonly_test.go @@ -0,0 +1,177 @@ +package cli + +import ( + "encoding/json" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" + + "github.com/git-pkgs/forge" + "github.com/git-pkgs/forge/internal/config" + "github.com/git-pkgs/forge/internal/resolve" +) + +// runReadOnlyCLI runs the root command against a fake Gitea server, which is +// probed when detect is set and configured as Gitea otherwise, and +// returns the writes the server received and the command's error. +func runReadOnlyCLI(t *testing.T, env string, detect bool, args ...string) ([]string, error) { + t.Helper() + resetCmd(rootCmd) + t.Cleanup(func() { + resetCmd(rootCmd) + rootCmd.SetArgs(nil) + resolve.SetReadOnly(false) + }) + resolve.ResetTestForge() + config.ResetCache() + t.Cleanup(config.ResetCache) + configHome := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", configHome) + t.Setenv("FORGE_HOST", "") + t.Setenv("FORGE_TOKEN", "") + t.Setenv("FORGE_READ_ONLY", env) + + var ( + mu sync.Mutex + writes []string + ) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/": + w.Header().Set("X-Gitea-Version", "1.26.0") + case r.URL.Path == "/api/v1/version": + _, _ = fmt.Fprint(w, `{"version":"1.26.0"}`) + case r.Method == http.MethodGet && r.URL.Path == "/api/v1/repos/owner/repo/labels": + _ = json.NewEncoder(w).Encode([]map[string]any{{"id": 1, "name": "bug", "color": "ff0000"}}) + case r.Method == http.MethodPost && r.URL.Path == "/api/v1/repos/owner/repo/labels": + mu.Lock() + writes = append(writes, r.Method+" "+r.URL.Path) + mu.Unlock() + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(map[string]any{"id": 2, "name": "docs", "color": "0000ff"}) + default: + t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) + http.NotFound(w, r) + } + })) + defer srv.Close() + + host := strings.TrimPrefix(srv.URL, "http://") + configDir := filepath.Join(configHome, "forge") + if err := os.MkdirAll(configDir, 0o700); err != nil { + t.Fatal(err) + } + section := "[" + host + "]\nscheme = http\n" + if !detect { + section += "type = gitea\n" + } + if err := os.WriteFile(filepath.Join(configDir, "config"), []byte(section), 0o600); err != nil { + t.Fatal(err) + } + + rootCmd.SetArgs(append([]string{"-R", host + "/owner/repo"}, args...)) + _, err := captureStdout(t, rootCmd.Execute) + + mu.Lock() + defer mu.Unlock() + return writes, err +} + +func TestReadOnlyRefusesWrites(t *testing.T) { + tests := []struct { + name string + env string + detect bool + args []string + }{ + {name: "flag", args: []string{"--read-only"}}, + {name: "env true", env: "true"}, + {name: "env 1", env: "1"}, + {name: "flag overrides env false", env: "false", args: []string{"--read-only"}}, + {name: "detected forge", detect: true, args: []string{"--read-only"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if _, err := runReadOnlyCLI(t, tt.env, tt.detect, append(tt.args, "label", "list")...); err != nil { + t.Fatalf("label list: %v", err) + } + + writes, err := runReadOnlyCLI(t, tt.env, tt.detect, append(tt.args, "label", "create", "docs", "--color", "0000ff")...) + if !errors.Is(err, forges.ErrReadOnly) { + t.Fatalf("label create: err = %v, want ErrReadOnly", err) + } + if len(writes) != 0 { + t.Errorf("server received writes: %v", writes) + } + }) + } +} + +func TestReadOnlyRefusesAPIWrites(t *testing.T) { + writes, err := runReadOnlyCLI(t, "", false, "--read-only", "api", "-X", "POST", "repos/{owner}/{repo}/labels") + if !errors.Is(err, forges.ErrReadOnly) { + t.Fatalf("api -X POST: err = %v, want ErrReadOnly", err) + } + if len(writes) != 0 { + t.Errorf("server received writes: %v", writes) + } +} + +func TestReadOnlyDisabled(t *testing.T) { + tests := []struct { + name string + env string + detect bool + args []string + }{ + {name: "default"}, + {name: "env false", env: "false"}, + {name: "flag false overrides env", env: "true", args: []string{"--read-only=false"}}, + {name: "detected forge", detect: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + writes, err := runReadOnlyCLI(t, tt.env, tt.detect, append(tt.args, "label", "create", "docs", "--color", "0000ff")...) + if err != nil { + t.Fatalf("label create: %v", err) + } + if len(writes) != 1 { + t.Errorf("server received writes %v, want one POST", writes) + } + }) + } +} + +func TestReadOnlyInvalidEnv(t *testing.T) { + _, err := runReadOnlyCLI(t, "maybe", false, "label", "list") + if err == nil || !strings.Contains(err.Error(), `invalid FORGE_READ_ONLY value "maybe"`) { + t.Fatalf("err = %v, want invalid FORGE_READ_ONLY error", err) + } +} + +func TestPRCreatePushRefusedInReadOnlyMode(t *testing.T) { + t.Chdir(t.TempDir()) + prs := &mockPRService{qualifiedHeads: true} + resolve.SetTestForge(&mockForge{prService: prs}, "owner", "repo", "example.com") + t.Cleanup(resolve.ResetTestForge) + resolve.SetReadOnly(true) + t.Cleanup(func() { resolve.SetReadOnly(false) }) + + cmd := prCreateCmd() + cmd.SetArgs([]string{"--title", "Test PR", "--head", "feature", "--push"}) + err := cmd.Execute() + if err == nil || !strings.Contains(err.Error(), "--push cannot be used in read-only mode") { + t.Fatalf("pr create --push error = %v, want read-only error", err) + } + if prs.createCalls != 0 { + t.Errorf("Create calls = %d, want 0", prs.createCalls) + } +} diff --git a/internal/cli/root.go b/internal/cli/root.go index 18caba1..4089b70 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "os" + "strconv" "github.com/git-pkgs/forge" "github.com/git-pkgs/forge/internal/config" @@ -18,6 +19,7 @@ var ( flagHost string flagOutput string flagRemote string + flagReadOnly bool ) var rootCmd = &cobra.Command{ @@ -25,7 +27,7 @@ var rootCmd = &cobra.Command{ Short: "Work with git forges from the command line", Long: "Supports GitHub, GitLab, Gitea, Forgejo, Bitbucket Cloud, Gerrit, and Tangled through a single interface.", SilenceUsage: true, - PersistentPreRun: func(cmd *cobra.Command, args []string) { + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { cfg, _ := config.Load() if cfg != nil { if !cmd.Flags().Changed("output") && cfg.Default.Output != "" { @@ -43,6 +45,17 @@ var rootCmd = &cobra.Command{ resolve.SetRemote(remote) resolve.SetHost(flagHost) resolve.SetForgeType(flagForgeType) + + readOnly := flagReadOnly + if v := os.Getenv("FORGE_READ_ONLY"); v != "" && !cmd.Flags().Changed("read-only") { + b, err := strconv.ParseBool(v) + if err != nil { + return fmt.Errorf("invalid FORGE_READ_ONLY value %q: use true or false", v) + } + readOnly = b + } + resolve.SetReadOnly(readOnly) + return nil }, } @@ -57,6 +70,7 @@ func init() { rootCmd.PersistentFlags().StringVar(&flagHost, "host", "", "Force forge host (e.g. gitea.com, http://forgejo.local:3000); overrides FORGE_HOST and remote detection") rootCmd.PersistentFlags().StringVarP(&flagOutput, "output", "o", "table", "Output format: table, json, plain") rootCmd.PersistentFlags().StringVar(&flagRemote, "remote", "", "Git remote to use when --repo (-R) is not specified (default from config, otherwise origin)") + rootCmd.PersistentFlags().BoolVar(&flagReadOnly, "read-only", false, "Refuse API requests that could modify the forge (also FORGE_READ_ONLY=true)") } // notSupported wraps ErrNotSupported with a user-friendly message diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index d0d25f6..d743504 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -410,7 +410,8 @@ func newClientForBaseURL(domain, baseURL string) *forges.Client { } // Share one client for detection and every backend, so forges found by - // probing get the same User-Agent and redirect policy as the defaults. + // probing get the same User-Agent, redirect policy and read-only mode as the + // defaults. hc := HTTPClient() opts = append(opts, forges.WithHTTPClient(hc)) diff --git a/internal/resolve/transport.go b/internal/resolve/transport.go index a1da077..8625ae4 100644 --- a/internal/resolve/transport.go +++ b/internal/resolve/transport.go @@ -4,6 +4,8 @@ import ( "errors" "fmt" "net/http" + + "github.com/git-pkgs/forge" ) // maxRedirects matches the limit net/http applies when CheckRedirect is nil. @@ -17,16 +19,34 @@ func SetUserAgent(ua string) { userAgent = ua } +var readOnly bool + +// SetReadOnly makes HTTPClient refuse requests that could modify forge +// state. The CLI calls this from the --read-only flag and FORGE_READ_ONLY. +func SetReadOnly(on bool) { + readOnly = on +} + +// ReadOnly reports whether read-only mode was enabled with SetReadOnly. +func ReadOnly() bool { + return readOnly +} + // HTTPClient returns an http.Client whose transport sets the User-Agent // header on outbound requests. Used for all forge API traffic so requests // are identifiable in server logs. It refuses redirects from HTTPS to HTTP, // since net/http would otherwise resend headers such as Authorization in -// cleartext when the redirect stays on the same host. +// cleartext when the redirect stays on the same host. In read-only mode the +// client only sends GET, HEAD and OPTIONS requests. func HTTPClient() *http.Client { - return &http.Client{ + hc := &http.Client{ Transport: &userAgentTransport{base: http.DefaultTransport}, CheckRedirect: checkRedirect, } + if readOnly { + return forges.ReadOnlyHTTPClient(hc) + } + return hc } // errInsecureRedirect is returned when a redirect would downgrade from diff --git a/readonly.go b/readonly.go new file mode 100644 index 0000000..a92be01 --- /dev/null +++ b/readonly.go @@ -0,0 +1,37 @@ +package forges + +import "net/http" + +// ReadOnlyHTTPClient returns a copy of hc that only sends GET, HEAD and +// OPTIONS requests. Any other request fails with ErrReadOnly before anything +// is sent. A nil hc copies http.DefaultClient. +// +// Pass the result to each backend constructor, and to WithHTTPClient so +// forges registered by RegisterDomain use it too. +func ReadOnlyHTTPClient(hc *http.Client) *http.Client { + if hc == nil { + hc = http.DefaultClient + } + c := *hc + base := c.Transport + if base == nil { + base = http.DefaultTransport + } + c.Transport = &readOnlyTransport{base: base} + return &c +} + +type readOnlyTransport struct { + base http.RoundTripper +} + +func (t *readOnlyTransport) RoundTrip(req *http.Request) (*http.Response, error) { + switch req.Method { + case "", http.MethodGet, http.MethodHead, http.MethodOptions: + return t.base.RoundTrip(req) + } + if req.Body != nil { + _ = req.Body.Close() + } + return nil, ErrReadOnly +} diff --git a/readonly_test.go b/readonly_test.go new file mode 100644 index 0000000..695d86d --- /dev/null +++ b/readonly_test.go @@ -0,0 +1,106 @@ +package forges + +import ( + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" +) + +func TestReadOnlyHTTPClientAllowsReads(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, r.Method) + })) + defer srv.Close() + + hc := ReadOnlyHTTPClient(srv.Client()) + for _, method := range []string{"", http.MethodGet, http.MethodHead, http.MethodOptions} { + req, err := http.NewRequest(method, srv.URL, nil) + if err != nil { + t.Fatal(err) + } + resp, err := hc.Do(req) + if err != nil { + t.Fatalf("%q: unexpected error: %v", method, err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Errorf("%q: status = %d, want 200", method, resp.StatusCode) + } + } +} + +type closeRecorder struct { + io.Reader + closed bool +} + +func (c *closeRecorder) Close() error { + c.closed = true + return nil +} + +func TestReadOnlyHTTPClientRefusesWrites(t *testing.T) { + var hits atomic.Int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + hits.Add(1) + })) + defer srv.Close() + + hc := ReadOnlyHTTPClient(srv.Client()) + methods := []string{http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete, http.MethodConnect, http.MethodTrace, "PROPFIND"} + for _, method := range methods { + body := &closeRecorder{Reader: strings.NewReader(`{"title":"x"}`)} + req, err := http.NewRequest(method, srv.URL+"/repos/o/r/issues", body) + if err != nil { + t.Fatal(err) + } + resp, err := hc.Do(req) + if resp != nil { + _ = resp.Body.Close() + } + if !errors.Is(err, ErrReadOnly) { + t.Errorf("%s: err = %v, want ErrReadOnly", method, err) + } + if !body.closed { + t.Errorf("%s: request body was not closed", method) + } + } + if n := hits.Load(); n != 0 { + t.Errorf("server received %d requests, want 0", n) + } +} + +func TestReadOnlyHTTPClientCopiesClient(t *testing.T) { + base := &http.Client{Timeout: time.Minute} + hc := ReadOnlyHTTPClient(base) + + if hc == base { + t.Fatal("ReadOnlyHTTPClient returned the client it was given") + } + if base.Transport != nil { + t.Errorf("original client's transport was changed to %T", base.Transport) + } + if hc.Timeout != time.Minute { + t.Errorf("Timeout = %v, want %v", hc.Timeout, time.Minute) + } +} + +func TestReadOnlyHTTPClientNil(t *testing.T) { + hc := ReadOnlyHTTPClient(nil) + if hc == http.DefaultClient { + t.Fatal("ReadOnlyHTTPClient(nil) returned http.DefaultClient") + } + if http.DefaultClient.Transport != nil { + t.Errorf("http.DefaultClient's transport was changed to %T", http.DefaultClient.Transport) + } + + _, err := hc.Post("http://127.0.0.1:0/", "application/json", strings.NewReader("{}")) + if !errors.Is(err, ErrReadOnly) { + t.Errorf("err = %v, want ErrReadOnly", err) + } +}